From d8f52aae7fdbf2a26d4824cdf21d7f455881f185 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Fri, 15 May 2026 16:26:57 +0900 Subject: [PATCH] test: run suite without split runner --- .github/workflows/ci.yml | 77 ++---- .github/workflows/publish.yml | 2 +- AGENTS.md | 8 +- bunfig.toml | 1 + package.json | 2 +- script/publish-workflow.test.ts | 9 +- script/run-ci-tests.test.ts | 45 ---- script/run-ci-tests.ts | 253 ------------------ script/tsconfig.json | 2 +- src/cli/run/integration.test.ts | 42 +-- src/cli/run/server-connection.test.ts | 49 ++-- src/cli/run/server-connection.ts | 90 +++++-- src/features/background-agent/manager.test.ts | 4 +- .../discovery.test.ts | 5 - .../resolve-caller-tmux-session.test.ts | 94 +++---- .../resolve-caller-tmux-session.ts | 14 +- src/features/tmux-subagent/manager.test.ts | 10 +- .../tmux-subagent/zombie-pane.test.ts | 10 +- src/hooks/AGENTS.md | 2 +- src/openclaw/AGENTS.md | 4 - src/plugin/event.test.ts | 4 + src/shared/model-resolution-pipeline.test.ts | 9 +- src/shared/model-resolution-pipeline.ts | 21 +- src/shared/model-resolver.test.ts | 26 +- src/shared/opencode-http-api.test.ts | 65 +++-- src/shared/opencode-http-api.ts | 63 +++-- src/shared/tmux/tmux-utils.test.ts | 96 +++---- src/shared/tmux/tmux-utils/server-health.ts | 41 ++- .../tmux/tmux-utils/session-spawn.test.ts | 146 +++++----- .../tmux/tmux-utils/window-spawn.test.ts | 135 ++++------ src/testing/module-mock-lifecycle.test.ts | 30 +++ src/testing/module-mock-lifecycle.ts | 3 +- src/tools/interactive-bash/tools.test.ts | 18 +- src/tools/interactive-bash/tools.ts | 136 +++++----- 34 files changed, 627 insertions(+), 889 deletions(-) delete mode 100644 script/run-ci-tests.test.ts delete mode 100644 script/run-ci-tests.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 42dcb7695..887a431c7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,70 +32,27 @@ jobs: echo "PR targets '${BASE_REF}' branch - OK" fi - test-isolated: - name: Isolated tests (${{ matrix.shard }}) - runs-on: ubuntu-latest - strategy: - fail-fast: false - matrix: - shard: [0, 1, 2, 3] - steps: - - uses: actions/checkout@v4 - - - uses: oven-sh/setup-bun@v2 - with: - bun-version: "1.3.12" - - - uses: actions/cache@v4 - with: - path: ~/.bun/install/cache - key: ${{ runner.os }}-bun-1.3.12-${{ hashFiles('bun.lock') }} - - - name: Install dependencies - run: bun install --frozen-lockfile - env: - BUN_INSTALL_ALLOW_SCRIPTS: "@ast-grep/napi" - - - name: Run isolated tests - run: bun run script/run-ci-tests.ts --phase=isolated --shard-count=4 --shard-index=${{ matrix.shard }} - - test-shared: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - - uses: oven-sh/setup-bun@v2 - with: - bun-version: "1.3.12" - - - uses: actions/cache@v4 - with: - path: ~/.bun/install/cache - key: ${{ runner.os }}-bun-1.3.12-${{ hashFiles('bun.lock') }} - - - name: Install dependencies - run: bun install --frozen-lockfile - env: - BUN_INSTALL_ALLOW_SCRIPTS: "@ast-grep/napi" - - - name: Run shared tests - run: bun run script/run-ci-tests.ts --phase=shared - test: runs-on: ubuntu-latest - needs: [test-isolated, test-shared] - if: ${{ always() }} steps: - - name: Verify test shards passed + - uses: actions/checkout@v4 + + - uses: oven-sh/setup-bun@v2 + with: + bun-version: "1.3.12" + + - uses: actions/cache@v4 + with: + path: ~/.bun/install/cache + key: ${{ runner.os }}-bun-1.3.12-${{ hashFiles('bun.lock') }} + + - name: Install dependencies + run: bun install --frozen-lockfile env: - ISOLATED_RESULT: ${{ needs.test-isolated.result }} - SHARED_RESULT: ${{ needs.test-shared.result }} - run: | - if [ "$ISOLATED_RESULT" != "success" ] || [ "$SHARED_RESULT" != "success" ]; then - echo "::error::test-isolated=${ISOLATED_RESULT}, test-shared=${SHARED_RESULT}" - exit 1 - fi - echo "All test shards passed" + BUN_INSTALL_ALLOW_SCRIPTS: "@ast-grep/napi" + + - name: Run tests + run: bun test typecheck: runs-on: ubuntu-latest diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 18f18f696..0e71653d2 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -46,7 +46,7 @@ jobs: BUN_INSTALL_ALLOW_SCRIPTS: "@ast-grep/napi" - name: Run tests - run: bun run script/run-ci-tests.ts + run: bun test typecheck: runs-on: ubuntu-latest diff --git a/AGENTS.md b/AGENTS.md index 7a82a99ba..4e268fbf0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -185,7 +185,7 @@ Schema autocomplete: `"$schema": "https://raw.githubusercontent.com/code-yeongyu - **Runtime:** Bun only (1.3.11 in CI). Never npm/yarn/pnpm. - **TypeScript:** strict mode, ESNext, bundler moduleResolution, `bun-types` (never `@types/node`). - **Tests:** Bun test (`bun:test`), co-located `*.test.ts`, given/when/then style — nested `describe` with `#given`/`#when`/`#then` prefixes, or inline `// given` / `// when` / `// then` comments. Never Arrange-Act-Assert comments. -- **CI test split:** `script/run-ci-tests.ts` auto-detects `mock.module()` and isolates those tests in separate processes. +- **CI tests:** plain `bun test` runs the root Bun suite in one process; no sharding or split isolation runner. - **Test setup:** `test-setup.ts` preloaded via `bunfig.toml` resets session/cache state between tests. - **Factory pattern:** `createXXX()` for all tools, hooks, agents. - **File naming:** kebab-case for files and directories. @@ -216,7 +216,7 @@ Schema autocomplete: `"$schema": "https://raw.githubusercontent.com/code-yeongyu ## COMMANDS ```bash -bun test # Bun test suite (auto-split mock-heavy tests via script/run-ci-tests.ts) +bun test # Root Bun test suite in one process bun run build # Build plugin (ESM bundle + .d.ts + cli bundle + schema generation) bun run build:all # Build + 11 platform binaries bun run build:schema # Regenerate assets/oh-my-opencode.schema.json @@ -233,7 +233,7 @@ bunx oh-my-opencode mcp-oauth login # Tier-3 MCP OAuth (PKCE + DCR | Workflow | Trigger | Purpose | |----------|---------|---------| -| `ci.yml` | push/PR to master/dev | Tests (split: mock-heavy isolated + batch), typecheck, build, schema auto-commit | +| `ci.yml` | push/PR to master/dev | Tests, typecheck, build, schema auto-commit | | `publish.yml` | manual dispatch | Version bump, dual npm publish (`oh-my-opencode` + `oh-my-openagent`), platform binaries, GitHub release | | `publish-platform.yml` | called by publish.yml | 11 platform binaries via `bun compile` (darwin/linux/windows) | | `sisyphus-agent.yml` | @mention or manual dispatch | AI agent handles issues/PRs | @@ -252,7 +252,7 @@ bunx oh-my-opencode mcp-oauth login # Tier-3 MCP OAuth (PKCE + DCR - **Two fallback systems:** `model-fallback` (proactive, chat.params, hardcoded chains) vs `runtime-fallback` (reactive, session.error, configurable per-category/agent). - **Config migration:** idempotent via `_migrations` tracking, atomic writes with timestamped backups. - **Build:** `bun build` (ESM) + `tsc --emitDeclarationOnly`, externals: `@ast-grep/napi`, `zod`. -- **CI test isolation:** `script/run-ci-tests.ts` auto-isolates files using `mock.module()` (plus `src/openclaw/__tests__/reply-listener-discord.test.ts`) — they run in separate processes. +- **CI tests:** root tests run through plain `bun test`; `web/**` has its own package-level CI workflow. - **122 barrel `index.ts` files** establish module boundaries. - **Architecture rules** enforced via `.sisyphus/rules/modular-code-enforcement.md` (when present in workspace). - **Windows builds:** run on `windows-latest` (not cross-compiled) to avoid Bun segfaults. diff --git a/bunfig.toml b/bunfig.toml index 9e75dd230..8cac6fdb2 100644 --- a/bunfig.toml +++ b/bunfig.toml @@ -1,2 +1,3 @@ [test] preload = ["./test-setup.ts"] +pathIgnorePatterns = ["web/**"] diff --git a/package.json b/package.json index 66c6c028f..404775223 100644 --- a/package.json +++ b/package.json @@ -35,7 +35,7 @@ "test:model-capabilities": "bun test src/shared/model-capability-aliases.test.ts src/shared/model-capability-guardrails.test.ts src/shared/model-capabilities.test.ts src/cli/doctor/checks/model-resolution.test.ts --bail", "typecheck": "tsgo --noEmit", "typecheck:script": "tsgo --noEmit -p script/tsconfig.json", - "test": "bun run script/run-ci-tests.ts" + "test": "bun test" }, "keywords": [ "opencode", diff --git a/script/publish-workflow.test.ts b/script/publish-workflow.test.ts index c22921d4b..ef2f3b070 100644 --- a/script/publish-workflow.test.ts +++ b/script/publish-workflow.test.ts @@ -7,18 +7,13 @@ const workflowChecks = [ { path: new URL("../.github/workflows/ci.yml", import.meta.url), testRuns: [ - "run: bun run script/run-ci-tests.ts --phase=isolated --shard-count=4 --shard-index=${{ matrix.shard }}", - "run: bun run script/run-ci-tests.ts --phase=shared", - "if: ${{ always() }}", - "ISOLATED_RESULT: ${{ needs.test-isolated.result }}", - "SHARED_RESULT: ${{ needs.test-shared.result }}", - "echo \"::error::test-isolated=${ISOLATED_RESULT}, test-shared=${SHARED_RESULT}\"", + "run: bun test", "run: bun test src/shared/dist-bundle-bun-globals.test.ts", ], }, { path: new URL("../.github/workflows/publish.yml", import.meta.url), - testRuns: ["run: bun run script/run-ci-tests.ts"], + testRuns: ["run: bun test"], }, ] diff --git a/script/run-ci-tests.test.ts b/script/run-ci-tests.test.ts deleted file mode 100644 index 0f54f4e81..000000000 --- a/script/run-ci-tests.test.ts +++ /dev/null @@ -1,45 +0,0 @@ -import { describe, expect, test } from "bun:test" -import { selectCiTestTargets } from "./run-ci-tests" - -describe("test script isolation", () => { - test("#given mock.module tests in the suite #then bun run test uses the isolated CI runner", async () => { - //#given - const packageJson = await Bun.file("package.json").json() - - //#then - expect(packageJson.scripts.test).toBe("bun run script/run-ci-tests.ts") - }) - - test("#given isolated test shards #when selecting targets #then shards are deterministic and complete", () => { - // given - const ciTestPlan = { - isolatedModuleMockFiles: [], - isolatedTestTargets: ["a.test.ts", "b.test.ts", "c.test.ts", "d.test.ts", "e.test.ts"], - sharedTestFiles: ["shared.test.ts"], - } - - // when - const shardOne = selectCiTestTargets(ciTestPlan, { phase: "isolated", shardCount: 2, shardIndex: 0 }) - const shardTwo = selectCiTestTargets(ciTestPlan, { phase: "isolated", shardCount: 2, shardIndex: 1 }) - - // then - expect(shardOne).toEqual({ isolatedTestTargets: ["a.test.ts", "c.test.ts", "e.test.ts"], sharedTestFiles: [] }) - expect(shardTwo).toEqual({ isolatedTestTargets: ["b.test.ts", "d.test.ts"], sharedTestFiles: [] }) - expect([...shardOne.isolatedTestTargets, ...shardTwo.isolatedTestTargets].sort()).toEqual(ciTestPlan.isolatedTestTargets) - }) - - test("#given shared phase #when selecting targets #then only shared tests run", () => { - // given - const ciTestPlan = { - isolatedModuleMockFiles: [], - isolatedTestTargets: ["isolated.test.ts"], - sharedTestFiles: ["shared.test.ts"], - } - - // when - const selectedTargets = selectCiTestTargets(ciTestPlan, { phase: "shared", shardCount: 1, shardIndex: 0 }) - - // then - expect(selectedTargets).toEqual({ isolatedTestTargets: [], sharedTestFiles: ["shared.test.ts"] }) - }) -}) diff --git a/script/run-ci-tests.ts b/script/run-ci-tests.ts deleted file mode 100644 index 1c77bc627..000000000 --- a/script/run-ci-tests.ts +++ /dev/null @@ -1,253 +0,0 @@ -/// - -type CiTestPlan = { - isolatedTestTargets: string[] - isolatedModuleMockFiles: string[] - sharedTestFiles: string[] -} - -type CiTestPhase = "all" | "isolated" | "shared" - -type CiTestRunOptions = { - phase: CiTestPhase - shardCount: number - shardIndex: number -} - -type CiTestTargetSelection = { - isolatedTestTargets: string[] - sharedTestFiles: string[] -} - -const TEST_ROOTS = ["bin", "script", "src"] as const -const MODULE_MOCK_PATTERN = "mock.module(" -const ALWAYS_ISOLATED_TEST_FILES = [ - "src/features/team-mode/team-mailbox/ack.test.ts", - "src/features/team-mode/team-mailbox/send.test.ts", - "src/features/team-mode/team-runtime/shutdown.test.ts", - "src/features/team-mode/team-runtime/status.test.ts", - "src/features/team-mode/team-state-store/resume.test.ts", - "src/features/team-mode/team-state-store/store.test.ts", - "src/features/boulder-state/storage.test.ts", - "src/hooks/anthropic-context-window-limit-recovery/aggressive-truncation-strategy.test.ts", - "src/hooks/session-notification-input-needed.test.ts", - "src/hooks/session-notification-sender.test.ts", - "src/hooks/session-notification.test.ts", - "src/openclaw/__tests__/reply-listener-discord.test.ts", - "src/tools/background-task/create-background-output.blocking.test.ts", - "src/tools/background-task/tools.test.ts", - "src/tools/interactive-bash/tmux-path-resolver.test.ts", - "src/tools/task/task-list.test.ts", -] as const - -async function collectTestFiles(rootDirectory: string): Promise { - const testFiles: string[] = [] - - for (const testRoot of TEST_ROOTS) { - const glob = new Bun.Glob("**/*.test.ts") - - for await (const testFile of glob.scan({ cwd: `${rootDirectory}/${testRoot}` })) { - testFiles.push(`${testRoot}/${testFile}`) - } - } - - return testFiles.sort((left, right) => left.localeCompare(right)) -} - -async function usesModuleMock(rootDirectory: string, testFile: string): Promise { - const testContents = await Bun.file(`${rootDirectory}/${testFile}`).text() - return testContents.includes(MODULE_MOCK_PATTERN) -} - -function toIsolatedTarget(testFile: string): string { - return testFile -} - -function isCoveredByTarget(testFile: string, isolatedTarget: string): boolean { - return testFile === isolatedTarget || testFile.startsWith(`${isolatedTarget}/`) -} - -function collapseNestedTargets(isolatedTargets: string[]): string[] { - return isolatedTargets.filter((isolatedTarget) => { - return !isolatedTargets.some((otherTarget) => { - return otherTarget !== isolatedTarget && isolatedTarget.startsWith(`${otherTarget}/`) - }) - }) -} - -function readFlagValue(args: string[], flagName: string): string | null { - const prefix = `${flagName}=` - const flag = args.find((arg) => arg.startsWith(prefix)) - - return flag?.slice(prefix.length) ?? null -} - -function parsePhase(rawPhase: string | null): CiTestPhase { - if (rawPhase === null) { - return "all" - } - - if (rawPhase === "all" || rawPhase === "isolated" || rawPhase === "shared") { - return rawPhase - } - - throw new Error(`Invalid --phase value: ${rawPhase}. Expected all, isolated, or shared.`) -} - -function parsePositiveIntegerFlag(args: string[], flagName: string, defaultValue: number): number { - const rawValue = readFlagValue(args, flagName) - if (rawValue === null) { - return defaultValue - } - - const parsedValue = Number(rawValue) - if (!Number.isInteger(parsedValue) || parsedValue < 1) { - throw new Error(`Invalid ${flagName} value: ${rawValue}. Expected a positive integer.`) - } - - return parsedValue -} - -function parseNonNegativeIntegerFlag(args: string[], flagName: string, defaultValue: number): number { - const rawValue = readFlagValue(args, flagName) - if (rawValue === null) { - return defaultValue - } - - const parsedValue = Number(rawValue) - if (!Number.isInteger(parsedValue) || parsedValue < 0) { - throw new Error(`Invalid ${flagName} value: ${rawValue}. Expected a non-negative integer.`) - } - - return parsedValue -} - -function parseCiTestRunOptions(args: string[]): CiTestRunOptions { - const phase = parsePhase(readFlagValue(args, "--phase")) - const shardCount = parsePositiveIntegerFlag(args, "--shard-count", 1) - const shardIndex = parseNonNegativeIntegerFlag(args, "--shard-index", 0) - - if (shardIndex >= shardCount) { - throw new Error(`Invalid --shard-index value: ${shardIndex}. Expected a value less than --shard-count ${shardCount}.`) - } - - if (shardCount > 1 && phase !== "isolated") { - throw new Error("Test sharding is only supported with --phase=isolated.") - } - - return { phase, shardCount, shardIndex } -} - -function selectShard(testTargets: string[], shardCount: number, shardIndex: number): string[] { - if (shardCount === 1) { - return testTargets - } - - return testTargets.filter((_, index) => index % shardCount === shardIndex) -} - -export function selectCiTestTargets(ciTestPlan: CiTestPlan, options: CiTestRunOptions): CiTestTargetSelection { - const isolatedTestTargets = options.phase === "shared" - ? [] - : selectShard(ciTestPlan.isolatedTestTargets, options.shardCount, options.shardIndex) - const sharedTestFiles = options.phase === "isolated" ? [] : ciTestPlan.sharedTestFiles - - return { isolatedTestTargets, sharedTestFiles } -} - -export async function createCiTestPlan(rootDirectory: string = process.cwd()): Promise { - const allTestFiles = await collectTestFiles(rootDirectory) - const isolatedModuleMockFiles: string[] = [] - - for (const testFile of allTestFiles) { - if (await usesModuleMock(rootDirectory, testFile)) { - isolatedModuleMockFiles.push(testFile) - } - } - - const isolatedTestFiles = Array.from( - new Set([...isolatedModuleMockFiles, ...ALWAYS_ISOLATED_TEST_FILES.filter((testFile) => allTestFiles.includes(testFile))]), - ) - const isolatedTestTargets = collapseNestedTargets( - isolatedTestFiles.map((testFile) => toIsolatedTarget(testFile)).sort((left, right) => - left.localeCompare(right), - ), - ) - const sharedTestFiles = allTestFiles.filter((testFile) => { - return !isolatedTestTargets.some((isolatedTarget) => isCoveredByTarget(testFile, isolatedTarget)) - }) - - return { - isolatedTestTargets, - isolatedModuleMockFiles, - sharedTestFiles, - } -} - -async function runBunTest(testFiles: string[], label: string): Promise { - if (testFiles.length === 0) { - return - } - - console.log(`::group::${label}`) - - const args = testFiles.map((testFile) => { - if (testFile.includes("/") && !testFile.endsWith(".test.ts")) { - return [testFile, "!_auc-*/**/*.test.ts"] - } - - return testFile - }).flat() - - const command = ["bun", "test", ...args] - const spawnedProcess = Bun.spawn(command, { - cwd: process.cwd(), - stdin: "inherit", - stdout: "inherit", - stderr: "inherit", - }) - const exitCode = await spawnedProcess.exited - console.log("::endgroup::") - - if (exitCode !== 0) { - throw new Error(`Command failed: ${command.join(" ")}`) - } -} - -async function main(): Promise { - const options = parseCiTestRunOptions(process.argv.slice(2)) - const ciTestPlan = await createCiTestPlan() - const selectedTargets = selectCiTestTargets(ciTestPlan, options) - - console.log( - `Detected ${ciTestPlan.isolatedModuleMockFiles.length} mock.module() test files, ${ciTestPlan.isolatedTestTargets.length} isolated targets, and ${ciTestPlan.sharedTestFiles.length} shared test files.`, - ) - - if (options.phase === "isolated" && options.shardCount > 1) { - console.log( - `Running isolated test shard ${options.shardIndex + 1}/${options.shardCount} with ${selectedTargets.isolatedTestTargets.length} targets.`, - ) - } - - for (const isolatedTestTarget of selectedTargets.isolatedTestTargets) { - await runBunTest([isolatedTestTarget], `Isolated ${isolatedTestTarget}`) - } - - await runBunTest(selectedTargets.sharedTestFiles, "Shared Bun test suite") -} - -export const moduleMockPattern = MODULE_MOCK_PATTERN -export const testRoots = TEST_ROOTS - -if (process.argv.includes("--print-plan")) { - const ciTestPlan = await createCiTestPlan() - console.log(JSON.stringify(ciTestPlan, null, 2)) -} else if (import.meta.main) { - try { - await main() - } catch (error) { - const message = error instanceof Error ? error.message : String(error) - console.error(message) - process.exit(1) - } -} diff --git a/script/tsconfig.json b/script/tsconfig.json index 44f60d25b..42970c20a 100644 --- a/script/tsconfig.json +++ b/script/tsconfig.json @@ -11,5 +11,5 @@ "allowImportingTsExtensions": true, "noEmit": true }, - "include": ["./publish-workflow.test.ts", "./run-ci-tests.ts"] + "include": ["./publish-workflow.test.ts"] } diff --git a/src/cli/run/integration.test.ts b/src/cli/run/integration.test.ts index 7f46b8dc3..1b9a6a431 100644 --- a/src/cli/run/integration.test.ts +++ b/src/cli/run/integration.test.ts @@ -1,14 +1,14 @@ -import { describe, it, expect, mock, spyOn, beforeEach, afterEach, afterAll } from "bun:test" +import { describe, it, expect, mock, spyOn, beforeEach, afterEach } from "bun:test" import type { RunResult } from "./types" import { createJsonOutputManager } from "./json-output" import { resolveSession } from "./session-resolver" import { executeOnCompleteHook } from "./on-complete-hook" import * as spawnWithWindowsHideModule from "../../shared/spawn-with-windows-hide" import type { OpencodeClient } from "./types" -import * as originalSdk from "@opencode-ai/sdk" -import * as originalPortUtils from "../../shared/port-utils" +import { createServerConnectionWithDeps, type ServerConnectionDeps, type ServerConnectionOptions } from "./server-connection" import { unsafeTestValue } from "../../../test-support/unsafe-test-value" +type TestClient = { session: Record } const mockServerClose = mock(() => {}) const mockCreateOpencode = mock(() => Promise.resolve({ @@ -19,25 +19,23 @@ const mockCreateOpencode = mock(() => const mockCreateOpencodeClient = mock(() => ({ session: {} })) const mockIsPortAvailable = mock(() => Promise.resolve(true)) const mockGetAvailableServerPort = mock(() => Promise.resolve({ port: 9999, wasAutoSelected: false })) +const mockWithWorkingOpencodePath = mock((startServer: () => Promise) => startServer()) +const mockInjectServerAuthIntoClient = mock(() => {}) -mock.module("@opencode-ai/sdk", () => ({ - createOpencode: mockCreateOpencode, - createOpencodeClient: mockCreateOpencodeClient, -})) +function createDeps(): ServerConnectionDeps { + return { + createOpencode: mockCreateOpencode, + createOpencodeClient: mockCreateOpencodeClient, + isPortAvailable: mockIsPortAvailable, + getAvailableServerPort: mockGetAvailableServerPort, + withWorkingOpencodePath: mockWithWorkingOpencodePath, + injectServerAuthIntoClient: mockInjectServerAuthIntoClient, + } +} -mock.module("../../shared/port-utils", () => ({ - isPortAvailable: mockIsPortAvailable, - getAvailableServerPort: mockGetAvailableServerPort, - DEFAULT_SERVER_PORT: 4096, -})) - -afterAll(() => { - mock.module("@opencode-ai/sdk", () => originalSdk) - mock.module("../../shared/port-utils", () => originalPortUtils) - mock.restore() -}) - -const { createServerConnection } = await import("./server-connection") +async function createServerConnection(options: ServerConnectionOptions) { + return await createServerConnectionWithDeps(options, createDeps()) +} interface MockWriteStream { write: (chunk: string) => boolean @@ -312,6 +310,10 @@ describe("integration: server connection", () => { mockCreateOpencode.mockClear() mockCreateOpencodeClient.mockClear() mockServerClose.mockClear() + mockIsPortAvailable.mockClear() + mockGetAvailableServerPort.mockClear() + mockWithWorkingOpencodePath.mockClear() + mockInjectServerAuthIntoClient.mockClear() }) afterEach(() => { diff --git a/src/cli/run/server-connection.test.ts b/src/cli/run/server-connection.test.ts index 2cd4ed660..ca39b6a1c 100644 --- a/src/cli/run/server-connection.test.ts +++ b/src/cli/run/server-connection.test.ts @@ -1,11 +1,8 @@ -import { describe, it, expect, mock, beforeEach, afterEach, afterAll } from "bun:test" - -import * as originalSdk from "@opencode-ai/sdk" -import * as originalPortUtils from "../../shared/port-utils" -import * as originalBinaryResolver from "./opencode-binary-resolver" -import * as originalServerAuth from "../../shared/opencode-server-auth" +import { describe, it, expect, mock, beforeEach, afterEach } from "bun:test" +import { createServerConnectionWithDeps, type ServerConnectionDeps, type ServerConnectionOptions } from "./server-connection" const originalConsole = globalThis.console +type TestClient = { session: Record, baseUrl?: string } const mockServerClose = mock(() => {}) const mockCreateOpencode = mock(() => @@ -24,34 +21,20 @@ const mockConsoleLog = mock(() => {}) const mockWithWorkingOpencodePath = mock((startServer: () => Promise) => startServer()) const mockInjectServerAuthIntoClient = mock(() => {}) -mock.module("@opencode-ai/sdk", () => ({ - createOpencode: mockCreateOpencode, - createOpencodeClient: mockCreateOpencodeClient, -})) +function createDeps(): ServerConnectionDeps { + return { + createOpencode: mockCreateOpencode, + createOpencodeClient: mockCreateOpencodeClient, + isPortAvailable: mockIsPortAvailable, + getAvailableServerPort: mockGetAvailableServerPort, + withWorkingOpencodePath: mockWithWorkingOpencodePath, + injectServerAuthIntoClient: mockInjectServerAuthIntoClient, + } +} -mock.module("../../shared/port-utils", () => ({ - isPortAvailable: mockIsPortAvailable, - getAvailableServerPort: mockGetAvailableServerPort, - DEFAULT_SERVER_PORT: 4096, -})) - -mock.module("./opencode-binary-resolver", () => ({ - withWorkingOpencodePath: mockWithWorkingOpencodePath, -})) - -mock.module("../../shared/opencode-server-auth", () => ({ - injectServerAuthIntoClient: mockInjectServerAuthIntoClient, -})) - -afterAll(() => { - mock.module("@opencode-ai/sdk", () => originalSdk) - mock.module("../../shared/port-utils", () => originalPortUtils) - mock.module("./opencode-binary-resolver", () => originalBinaryResolver) - mock.module("../../shared/opencode-server-auth", () => originalServerAuth) - mock.restore() -}) - -const { createServerConnection } = await import("./server-connection") +async function createServerConnection(options: ServerConnectionOptions) { + return await createServerConnectionWithDeps(options, createDeps()) +} describe("createServerConnection", () => { beforeEach(() => { diff --git a/src/cli/run/server-connection.ts b/src/cli/run/server-connection.ts index f92aa1a08..3f9ffe521 100644 --- a/src/cli/run/server-connection.ts +++ b/src/cli/run/server-connection.ts @@ -1,4 +1,4 @@ -import { createOpencode, createOpencodeClient } from "@opencode-ai/sdk" +import { createOpencode as createOpencodeSdk, createOpencodeClient as createOpencodeClientSdk } from "@opencode-ai/sdk" import pc from "picocolors" import type { ServerConnection } from "./types" import { injectServerAuthIntoClient } from "../../shared/opencode-server-auth" @@ -7,6 +7,40 @@ import { withWorkingOpencodePath } from "./opencode-binary-resolver" const LOOPBACK_HOSTS = new Set(["127.0.0.1", "localhost", "::1", "[::1]", "0.0.0.0"]) +export type ServerConnectionOptions = { + port?: number + attach?: string + signal: AbortSignal +} + +type OpencodeServer = { + client: TClient + server: { + url: string + close: () => void + } +} + +export type ServerConnectionDeps = { + createOpencode: (options: { signal: AbortSignal, port: number, hostname: string }) => Promise> + createOpencodeClient: (options: { baseUrl: string }) => TClient + injectServerAuthIntoClient: (client: TClient) => void + isPortAvailable: (port: number, hostname?: string) => Promise + getAvailableServerPort: (preferredPort?: number, hostname?: string) => Promise<{ port: number, wasAutoSelected: boolean }> + withWorkingOpencodePath: ( + startServer: () => Promise>, + ) => Promise> +} + +const defaultDeps: ServerConnectionDeps = { + createOpencode: createOpencodeSdk, + createOpencodeClient: createOpencodeClientSdk, + injectServerAuthIntoClient, + isPortAvailable, + getAvailableServerPort, + withWorkingOpencodePath, +} + function isLoopbackAttachUrl(url: string): boolean { try { const parsed = new URL(url) @@ -32,28 +66,30 @@ function isPortRangeExhausted(error: unknown): boolean { return error.message.includes("No available port found in range") } -async function startServer(options: { signal: AbortSignal, port: number }): Promise { +async function startServer( + options: { signal: AbortSignal, port: number }, + deps: ServerConnectionDeps, +): Promise<{ client: TClient, cleanup: () => void }> { const { signal, port } = options - const { client, server } = await withWorkingOpencodePath(() => - createOpencode({ signal, port, hostname: "127.0.0.1" }), + const { client, server } = await deps.withWorkingOpencodePath(() => + deps.createOpencode({ signal, port, hostname: "127.0.0.1" }), ) console.log(pc.dim("Server listening at"), pc.cyan(server.url)) return { client, cleanup: () => server.close() } } -export async function createServerConnection(options: { - port?: number - attach?: string - signal: AbortSignal -}): Promise { +export async function createServerConnectionWithDeps( + options: ServerConnectionOptions, + deps: ServerConnectionDeps, +): Promise<{ client: TClient, cleanup: () => void }> { const { port, attach, signal } = options if (attach !== undefined) { console.log(pc.dim("Attaching to existing server at"), pc.cyan(attach)) - const client = createOpencodeClient({ baseUrl: attach }) + const client = deps.createOpencodeClient({ baseUrl: attach }) if (isLoopbackAttachUrl(attach)) { - injectServerAuthIntoClient(client) + deps.injectServerAuthIntoClient(client) } return { client, cleanup: () => {} } } @@ -63,39 +99,39 @@ export async function createServerConnection(options: { throw new Error("Port must be between 1 and 65535") } - const available = await isPortAvailable(port, "127.0.0.1") + const available = await deps.isPortAvailable(port, "127.0.0.1") if (available) { console.log(pc.dim("Starting server on port"), pc.cyan(port.toString())) try { - return await startServer({ signal, port }) + return await startServer({ signal, port }, deps) } catch (error) { if (!isPortStartFailure(error, port)) { throw error } - const stillAvailable = await isPortAvailable(port, "127.0.0.1") + const stillAvailable = await deps.isPortAvailable(port, "127.0.0.1") if (stillAvailable) { throw error } console.log(pc.dim("Port"), pc.cyan(port.toString()), pc.dim("became occupied, attaching to existing server")) - const client = createOpencodeClient({ baseUrl: `http://127.0.0.1:${port}` }) - injectServerAuthIntoClient(client) + const client = deps.createOpencodeClient({ baseUrl: `http://127.0.0.1:${port}` }) + deps.injectServerAuthIntoClient(client) return { client, cleanup: () => {} } } } console.log(pc.dim("Port"), pc.cyan(port.toString()), pc.dim("is occupied, attaching to existing server")) - const client = createOpencodeClient({ baseUrl: `http://127.0.0.1:${port}` }) - injectServerAuthIntoClient(client) + const client = deps.createOpencodeClient({ baseUrl: `http://127.0.0.1:${port}` }) + deps.injectServerAuthIntoClient(client) return { client, cleanup: () => {} } } let selectedPort: number let wasAutoSelected: boolean try { - const selected = await getAvailableServerPort(DEFAULT_SERVER_PORT, "127.0.0.1") + const selected = await deps.getAvailableServerPort(DEFAULT_SERVER_PORT, "127.0.0.1") selectedPort = selected.port wasAutoSelected = selected.wasAutoSelected } catch (error) { @@ -103,14 +139,14 @@ export async function createServerConnection(options: { throw error } - const defaultPortIsAvailable = await isPortAvailable(DEFAULT_SERVER_PORT, "127.0.0.1") + const defaultPortIsAvailable = await deps.isPortAvailable(DEFAULT_SERVER_PORT, "127.0.0.1") if (defaultPortIsAvailable) { throw error } console.log(pc.dim("Port range exhausted, attaching to existing server on"), pc.cyan(DEFAULT_SERVER_PORT.toString())) - const client = createOpencodeClient({ baseUrl: `http://127.0.0.1:${DEFAULT_SERVER_PORT}` }) - injectServerAuthIntoClient(client) + const client = deps.createOpencodeClient({ baseUrl: `http://127.0.0.1:${DEFAULT_SERVER_PORT}` }) + deps.injectServerAuthIntoClient(client) return { client, cleanup: () => {} } } @@ -121,14 +157,18 @@ export async function createServerConnection(options: { } try { - return await startServer({ signal, port: selectedPort }) + return await startServer({ signal, port: selectedPort }, deps) } catch (error) { if (!isPortStartFailure(error, selectedPort)) { throw error } - const { port: retryPort } = await getAvailableServerPort(selectedPort + 1, "127.0.0.1") + const { port: retryPort } = await deps.getAvailableServerPort(selectedPort + 1, "127.0.0.1") console.log(pc.dim("Retrying server start on port"), pc.cyan(retryPort.toString())) - return await startServer({ signal, port: retryPort }) + return await startServer({ signal, port: retryPort }, deps) } } + +export async function createServerConnection(options: ServerConnectionOptions): Promise { + return await createServerConnectionWithDeps(options, defaultDeps) +} diff --git a/src/features/background-agent/manager.test.ts b/src/features/background-agent/manager.test.ts index bf722c5c5..0c198f1c7 100644 --- a/src/features/background-agent/manager.test.ts +++ b/src/features/background-agent/manager.test.ts @@ -5179,7 +5179,7 @@ describe("BackgroundManager.handleEvent - session.error", () => { { info: { role: "assistant", - time: { created: Date.now() }, + time: { created: 2_000 }, }, parts: [{ type: "text", text: "wake was already accepted" }], }, @@ -5214,7 +5214,7 @@ describe("BackgroundManager.handleEvent - session.error", () => { if (!wake) { throw new Error("Missing dispatched parent wake") } - wake.dispatchedAt = Date.now() - 1_000 + wake.dispatchedAt = 1_000 //#when manager.handleEvent({ diff --git a/src/features/claude-code-plugin-loader/discovery.test.ts b/src/features/claude-code-plugin-loader/discovery.test.ts index 7a5dd1b0d..984ce6486 100644 --- a/src/features/claude-code-plugin-loader/discovery.test.ts +++ b/src/features/claude-code-plugin-loader/discovery.test.ts @@ -3,11 +3,6 @@ import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs" import { tmpdir } from "node:os" import { join } from "node:path" -// NOTE: Do NOT import discoverInstalledPlugins at top level. -// loader.test.ts in the same directory mocks "./discovery" with name: "demo", -// and when run-ci-tests.ts groups this directory together, that mock leaks. -// Dynamic import inside each test avoids the contamination. - const originalClaudePluginsHome = process.env.CLAUDE_PLUGINS_HOME const temporaryDirectories: string[] = [] const originalCwd = process.cwd() diff --git a/src/features/team-mode/team-layout-tmux/resolve-caller-tmux-session.test.ts b/src/features/team-mode/team-layout-tmux/resolve-caller-tmux-session.test.ts index 9a3b6b9c3..487e83d46 100644 --- a/src/features/team-mode/team-layout-tmux/resolve-caller-tmux-session.test.ts +++ b/src/features/team-mode/team-layout-tmux/resolve-caller-tmux-session.test.ts @@ -1,95 +1,70 @@ -/// - -import { afterEach, beforeEach, describe, expect, test } from "bun:test" -import { chmod, mkdtemp, readFile, rm, writeFile } from "node:fs/promises" -import { tmpdir } from "node:os" -import path from "node:path" +import { describe, expect, mock, test } from "bun:test" +import type { TmuxCommandResult } from "../../../shared/tmux" import { resolveCallerTmuxSession } from "./resolve-caller-tmux-session" -type TmuxStub = { +type TmuxCall = { tmuxPath: string - logPath: string + args: string[] } -const temporaryDirectories: string[] = [] - -function shellSingleQuote(value: string): string { - return `'${value.split("'").join(`'"'"'`)}'` -} - -async function createTmuxStub(options: { stdout: string; windowStdout?: string; exitCode: number }): Promise { - const directory = await mkdtemp(path.join(tmpdir(), "resolve-caller-tmux-session-")) - temporaryDirectories.push(directory) - - const logPath = path.join(directory, "tmux.log") - const tmuxPath = path.join(directory, "tmux") - const script = [ - "#!/bin/sh", - `printf '%s\\n' \"$@\" >> ${shellSingleQuote(logPath)}`, - `case "$*" in *'#{session_name}:#{window_index}'*) printf '%s' ${shellSingleQuote(options.windowStdout ?? options.stdout)} ;; *) printf '%s' ${shellSingleQuote(options.stdout)} ;; esac`, - `exit ${options.exitCode}`, - ].join("\n") - - await writeFile(tmuxPath, script) - await chmod(tmuxPath, 0o755) - - return { tmuxPath, logPath } -} - -async function readLogLines(logPath: string): Promise { - try { - const content = await readFile(logPath, "utf8") - return content.split("\n").filter((line) => line.length > 0) - } catch { - return [] +function tmuxResult(output: string, exitCode: number = 0): TmuxCommandResult { + return { + success: exitCode === 0, + output, + stdout: output, + stderr: "", + exitCode, } } -beforeEach(() => { - delete process.env.TMUX_PANE -}) +function createRunCommandMock(results: TmuxCommandResult[]) { + const calls: TmuxCall[] = [] + const runCommand = mock(async (tmuxPath: string, args: string[]): Promise => { + calls.push({ tmuxPath, args }) + return results.shift() ?? tmuxResult("", 1) + }) -afterEach(async () => { - await Promise.all(temporaryDirectories.splice(0).map(async (directory) => rm(directory, { recursive: true, force: true }))) -}) + return { calls, runCommand } +} describe("resolveCallerTmuxSession", () => { test("#given TMUX_PANE unset #when resolve runs #then returns null and makes no tmux calls", async () => { // given - const stub = await createTmuxStub({ stdout: "$7", exitCode: 0 }) + const { calls, runCommand } = createRunCommandMock([tmuxResult("$7")]) // when - const result = await resolveCallerTmuxSession(stub.tmuxPath) + const result = await resolveCallerTmuxSession("tmux", "", runCommand) // then expect(result).toBeNull() - expect(await readLogLines(stub.logPath)).toHaveLength(0) + expect(calls).toHaveLength(0) }) test("#given TMUX_PANE=%42 and display returns session and window #when resolve runs #then returns caller tmux target", async () => { // given - process.env.TMUX_PANE = "%42" - const stub = await createTmuxStub({ stdout: "$7", windowStdout: "test-session:0", exitCode: 0 }) + const { calls, runCommand } = createRunCommandMock([ + tmuxResult("$7"), + tmuxResult("test-session:0"), + ]) // when - const result = await resolveCallerTmuxSession(stub.tmuxPath) + const result = await resolveCallerTmuxSession("tmux", "%42", runCommand) // then expect(result).toEqual({ sessionId: "$7", paneId: "%42", windowTarget: "test-session:0" }) - expect(await readLogLines(stub.logPath)).toEqual([ - "display", "-p", "-F", "#{session_id}", "-t", "%42", - "display", "-p", "-F", "#{session_name}:#{window_index}", "-t", "%42", + expect(calls).toEqual([ + { tmuxPath: "tmux", args: ["display", "-p", "-F", "#{session_id}", "-t", "%42"] }, + { tmuxPath: "tmux", args: ["display", "-p", "-F", "#{session_name}:#{window_index}", "-t", "%42"] }, ]) }) test("#given TMUX_PANE=%42 and display returns 'garbage' #when resolve runs #then returns null", async () => { // given - process.env.TMUX_PANE = "%42" - const stub = await createTmuxStub({ stdout: "garbage", exitCode: 0 }) + const { runCommand } = createRunCommandMock([tmuxResult("garbage")]) // when - const result = await resolveCallerTmuxSession(stub.tmuxPath) + const result = await resolveCallerTmuxSession("tmux", "%42", runCommand) // then expect(result).toBeNull() @@ -97,11 +72,10 @@ describe("resolveCallerTmuxSession", () => { test("#given TMUX_PANE=%42 and display exits non-success #when resolve runs #then returns null", async () => { // given - process.env.TMUX_PANE = "%42" - const stub = await createTmuxStub({ stdout: "$7", exitCode: 1 }) + const { runCommand } = createRunCommandMock([tmuxResult("$7", 1)]) // when - const result = await resolveCallerTmuxSession(stub.tmuxPath) + const result = await resolveCallerTmuxSession("tmux", "%42", runCommand) // then expect(result).toBeNull() diff --git a/src/features/team-mode/team-layout-tmux/resolve-caller-tmux-session.ts b/src/features/team-mode/team-layout-tmux/resolve-caller-tmux-session.ts index 3d7cb7e6f..fcb6d786a 100644 --- a/src/features/team-mode/team-layout-tmux/resolve-caller-tmux-session.ts +++ b/src/features/team-mode/team-layout-tmux/resolve-caller-tmux-session.ts @@ -1,4 +1,5 @@ import { runTmuxCommand } from "../../../shared/tmux" +import type { TmuxCommandResult } from "../../../shared/tmux" type ResolvedCallerTmuxSession = { sessionId: string @@ -6,16 +7,21 @@ type ResolvedCallerTmuxSession = { windowTarget: string } +type RunTmuxCommand = (tmuxPath: string, args: string[]) => Promise + const TMUX_SESSION_ID_PATTERN = /^\$[0-9]+$/ const TMUX_WINDOW_TARGET_PATTERN = /^[^:]+:[0-9]+$/ -export async function resolveCallerTmuxSession(tmuxPath: string): Promise { - const callerPaneId = process.env.TMUX_PANE +export async function resolveCallerTmuxSession( + tmuxPath: string, + callerPaneId: string | undefined = process.env.TMUX_PANE, + runCommand: RunTmuxCommand = runTmuxCommand, +): Promise { if (!callerPaneId) { return null } - const sessionResult = await runTmuxCommand(tmuxPath, ["display", "-p", "-F", "#{session_id}", "-t", callerPaneId]) + const sessionResult = await runCommand(tmuxPath, ["display", "-p", "-F", "#{session_id}", "-t", callerPaneId]) if (!sessionResult.success) { return null } @@ -25,7 +31,7 @@ export async function resolveCallerTmuxSession(tmuxPath: string): Promise -import { describe, test, expect, mock, beforeEach, spyOn, afterAll } from 'bun:test' +import { describe, test, expect, mock, beforeEach, spyOn, afterAll, afterEach } from 'bun:test' import type { TmuxConfig } from '../../config/schema' import type { WindowState, PaneAction } from './types' import type { ActionResult, ExecuteContext } from './action-executor' import type { TmuxSessionManager as TmuxSessionManagerType, TmuxUtilDeps } from './manager' import * as sharedModule from '../../shared' +import * as sharedTmuxOriginal from '../../shared/tmux' + +const sharedTmuxSnapshot = { ...sharedTmuxOriginal } type ExecuteActionsResult = { success: boolean @@ -131,6 +134,11 @@ function registerModuleMocks(): void { afterAll(() => { mock.restore() }) +afterEach(() => { + mock.restore() + mock.module('../../shared/tmux', () => sharedTmuxSnapshot) +}) + const trackedSessions = new Set() const readySessions = new Set() diff --git a/src/features/tmux-subagent/zombie-pane.test.ts b/src/features/tmux-subagent/zombie-pane.test.ts index e5b5c7ac2..326b48c7f 100644 --- a/src/features/tmux-subagent/zombie-pane.test.ts +++ b/src/features/tmux-subagent/zombie-pane.test.ts @@ -1,9 +1,12 @@ /// -import { beforeEach, describe, expect, mock, test, afterAll } from "bun:test" +import { afterEach, beforeEach, describe, expect, mock, test, afterAll } from "bun:test" import type { TmuxConfig } from "../../config/schema" import type { ActionResult, ExecuteContext, ExecuteActionsResult } from "./action-executor" import type { TmuxUtilDeps } from "./manager" import type { TrackedSession, WindowState } from "./types" +import * as sharedTmuxOriginal from "../../shared/tmux" + +const sharedTmuxSnapshot = { ...sharedTmuxOriginal } const mockQueryWindowState = mock<(paneId: string) => Promise>(async () => ({ windowWidth: 220, @@ -53,6 +56,11 @@ function registerModuleMocks(): void { afterAll(() => { mock.restore() }) +afterEach(() => { + mock.restore() + mock.module("../../shared/tmux", () => sharedTmuxSnapshot) +}) + const mockTmuxDeps: TmuxUtilDeps = { isInsideTmux: mockIsInsideTmux, getCurrentPaneId: mockGetCurrentPaneId, diff --git a/src/hooks/AGENTS.md b/src/hooks/AGENTS.md index da7255125..c7abd54da 100644 --- a/src/hooks/AGENTS.md +++ b/src/hooks/AGENTS.md @@ -142,6 +142,6 @@ hooks/ ## NOTES - **Tier order matters within a phase:** within Session tier the registration order in `create-session-hooks.ts` determines invocation order — earlier hooks see un-mutated input, later hooks see accumulated output. -- **Mock files** (`zauc-mocks-*`, `zauc-sync-mocks`) are NOT hooks. They are placed inside `src/hooks/` purely so `bun:test` discovers them in the right order — auto-isolated by `script/run-ci-tests.ts` because they use `mock.module()`. +- **Mock files** (`zauc-mocks-*`, `zauc-sync-mocks`) are NOT hooks. They are placed inside `src/hooks/` purely so `bun:test` discovers them with the hook test fixtures. - **`atlasHook` vs `todoContinuationEnforcer`:** atlas handles boulder/ralph/subagent sessions, todoContinuationEnforcer handles the main Sisyphus session. Both fire on `session.idle` but check session type first. - **`runtime-fallback` vs `model-fallback`:** runtime-fallback is reactive (after error); model-fallback is proactive (chat.params). They operate independently. diff --git a/src/openclaw/AGENTS.md b/src/openclaw/AGENTS.md index 44fab8b73..cc11b0041 100644 --- a/src/openclaw/AGENTS.md +++ b/src/openclaw/AGENTS.md @@ -76,7 +76,3 @@ initializeOpenClaw(config) - **Authorized users**: Inbound replies filtered by allowed user ID list - **Token redaction**: Secrets masked in logs and error messages - **Rate limiting**: Reply injection throttled per pane - -## TESTING NOTE - -`reply-listener-discord.test.ts` is **always isolated** in CI (listed in `ALWAYS_ISOLATED_TEST_FILES` of `script/run-ci-tests.ts`). Reason: mocks `globalThis.fetch` for Discord API simulation — needs process isolation to avoid interference with shared test batch. diff --git a/src/plugin/event.test.ts b/src/plugin/event.test.ts index e089d9ddd..6d02eb27b 100644 --- a/src/plugin/event.test.ts +++ b/src/plugin/event.test.ts @@ -8,6 +8,9 @@ import * as openclawRuntimeDispatch from "../openclaw/runtime-dispatch" import { _resetForTesting, setMainSession, subagentSessions } from "../features/claude-code-session-state" import { clearPendingModelFallback, createModelFallbackHook } from "../hooks/model-fallback/hook" import { getSessionPromptParams, setSessionPromptParams } from "../shared/session-prompt-params-state" +import * as sharedTmuxOriginal from "../shared/tmux" + +const sharedTmuxSnapshot = { ...sharedTmuxOriginal } type EventInput = { event: { type: string; properties?: unknown } } type EventHandlerArgs = Parameters[0] @@ -135,6 +138,7 @@ async function flushMicrotasks(turns: number = 5): Promise { afterEach(() => { mock.restore() + mock.module("../shared/tmux", () => sharedTmuxSnapshot) _resetForTesting() }) diff --git a/src/shared/model-resolution-pipeline.test.ts b/src/shared/model-resolution-pipeline.test.ts index 26992da09..a08ecc85c 100644 --- a/src/shared/model-resolution-pipeline.test.ts +++ b/src/shared/model-resolution-pipeline.test.ts @@ -1,13 +1,6 @@ -import { describe, expect, mock, test } from "bun:test" +import { describe, expect, test } from "bun:test" import { resolveModelPipeline } from "./model-resolution-pipeline" -// Force test-runner isolation: files that import mock.module are auto-detected -// by run-ci-tests.ts and executed in their own bun process so they cannot be -// contaminated by (or contaminate) mock.module calls in other test files. -mock.module("./logger", () => ({ - log: () => {}, -})) - describe("resolveModelPipeline", () => { test("does not return unused explicit user config metadata in override result", () => { // given diff --git a/src/shared/model-resolution-pipeline.ts b/src/shared/model-resolution-pipeline.ts index c51cad371..96636a5f9 100644 --- a/src/shared/model-resolution-pipeline.ts +++ b/src/shared/model-resolution-pipeline.ts @@ -1,10 +1,29 @@ -import { log } from "./logger" +import { log as writeLog } from "./logger" import * as connectedProvidersCache from "./connected-providers-cache" import { fuzzyMatchModel } from "./model-availability" import type { FallbackEntry } from "./model-requirements" import { transformModelForProvider } from "./provider-model-id-transform" import { normalizeModel } from "./model-normalization" +type LogImplementation = typeof writeLog + +let logImplementationForTesting: LogImplementation | undefined + +function log(message: string, data?: unknown): void { + const logImplementation = logImplementationForTesting ?? writeLog + if (arguments.length === 1) { + logImplementation(message) + return + } + logImplementation(message, data) +} + +export function _setModelResolutionLogImplementationForTesting( + logImplementation: LogImplementation | undefined, +): void { + logImplementationForTesting = logImplementation +} + export type ModelResolutionRequest = { intent?: { uiSelectedModel?: string diff --git a/src/shared/model-resolver.test.ts b/src/shared/model-resolver.test.ts index 0e546c312..9644f28de 100644 --- a/src/shared/model-resolver.test.ts +++ b/src/shared/model-resolver.test.ts @@ -1,12 +1,11 @@ import { describe, expect, test, spyOn, beforeEach, afterEach, mock } from "bun:test" -// Isolate from other tests that mock.module the logger (CI cross-contamination fix) -mock.module("./logger", () => ({ log: (..._args: unknown[]) => {} })) - import { resolveModel, resolveModelWithFallback, type ModelResolutionInput, type ExtendedModelResolutionInput, type ModelResolutionResult, type ModelSource } from "./model-resolver" -import * as logger from "./logger" +import { _setModelResolutionLogImplementationForTesting } from "./model-resolution-pipeline" import * as connectedProvidersCache from "./connected-providers-cache" +const logMock = mock(() => {}) + describe("resolveModel", () => { describe("priority chain", () => { test("returns userModel when all three are set", () => { @@ -107,14 +106,13 @@ describe("resolveModel", () => { }) describe("resolveModelWithFallback", () => { - let logSpy: ReturnType - beforeEach(() => { - logSpy = spyOn(logger, "log") + logMock.mockClear() + _setModelResolutionLogImplementationForTesting(logMock) }) afterEach(() => { - logSpy.mockRestore() + _setModelResolutionLogImplementationForTesting(undefined) }) describe("Step 1: UI Selection (highest priority)", () => { @@ -136,7 +134,7 @@ describe("resolveModelWithFallback", () => { // then expect(result!.model).toBe("opencode/big-pickle") expect(result!.source).toBe("override") - expect(logSpy).toHaveBeenCalledWith("Model resolved via UI selection", { model: "opencode/big-pickle" }) + expect(logMock).toHaveBeenCalledWith("Model resolved via UI selection", { model: "opencode/big-pickle" }) }) test("UI selection takes priority over config override", () => { @@ -170,7 +168,7 @@ describe("resolveModelWithFallback", () => { // then expect(result!.model).toBe("anthropic/claude-opus-4-7") - expect(logSpy).toHaveBeenCalledWith("Model resolved via config override", { model: "anthropic/claude-opus-4-7" }) + expect(logMock).toHaveBeenCalledWith("Model resolved via config override", { model: "anthropic/claude-opus-4-7" }) }) test("empty string uiSelectedModel falls through to config override", () => { @@ -208,7 +206,7 @@ describe("resolveModelWithFallback", () => { // then expect(result!.model).toBe("anthropic/claude-opus-4-7") expect(result!.source).toBe("override") - expect(logSpy).toHaveBeenCalledWith("Model resolved via config override", { model: "anthropic/claude-opus-4-7" }) + expect(logMock).toHaveBeenCalledWith("Model resolved via config override", { model: "anthropic/claude-opus-4-7" }) }) test("override takes priority even if model not in availableModels", () => { @@ -284,7 +282,7 @@ describe("resolveModelWithFallback", () => { // then expect(result!.model).toBe("github-copilot/claude-opus-4-7-preview") expect(result!.source).toBe("provider-fallback") - expect(logSpy).toHaveBeenCalledWith("Model resolved via fallback chain (availability confirmed)", { + expect(logMock).toHaveBeenCalledWith("Model resolved via fallback chain (availability confirmed)", { provider: "github-copilot", model: "claude-opus-4-7", match: "github-copilot/claude-opus-4-7-preview", @@ -410,7 +408,7 @@ describe("resolveModelWithFallback", () => { // then - should find glm-5 from opencode via cross-provider fuzzy match expect(result!.model).toBe("opencode/glm-5") expect(result!.source).toBe("provider-fallback") - expect(logSpy).toHaveBeenCalledWith("Model resolved via fallback chain (cross-provider fuzzy match)", { + expect(logMock).toHaveBeenCalledWith("Model resolved via fallback chain (cross-provider fuzzy match)", { model: "glm-5", match: "opencode/glm-5", variant: undefined, @@ -490,7 +488,7 @@ describe("resolveModelWithFallback", () => { // then expect(result!.model).toBe("google/gemini-3.1-pro") expect(result!.source).toBe("system-default") - expect(logSpy).toHaveBeenCalledWith("No available model found in fallback chain, falling through to system default") + expect(logMock).toHaveBeenCalledWith("No available model found in fallback chain, falling through to system default") }) test("returns undefined when availableModels empty and no connected providers cache exists", () => { diff --git a/src/shared/opencode-http-api.test.ts b/src/shared/opencode-http-api.test.ts index 80b86bae6..723e0cf1c 100644 --- a/src/shared/opencode-http-api.test.ts +++ b/src/shared/opencode-http-api.test.ts @@ -1,20 +1,25 @@ -import { describe, it, expect, vi, beforeEach } from "bun:test" -import { getServerBaseUrl, patchPart, deletePart } from "./opencode-http-api" +import { describe, it, expect, mock, beforeEach } from "bun:test" -// Mock fetch globally -const mockFetch = vi.fn() -global.fetch = mockFetch +type OpencodeHttpApi = typeof import("./opencode-http-api") -// Mock log -vi.mock("./logger", () => ({ - log: vi.fn(), -})) +const opencodeHttpApiSpecifier = import.meta.resolve("./opencode-http-api") -import { log } from "./logger" +const log = mock(() => {}) +const getServerBasicAuthHeader = mock(() => "Basic b3BlbmNvZGU6dGVzdHBhc3N3b3Jk") +const fetchImplementation = mock(async (): Promise => new Response(null, { status: 200 })) + +async function loadOpencodeHttpApi(): Promise { + const opencodeHttpApi = await import(`${opencodeHttpApiSpecifier}?test=${crypto.randomUUID()}`) + opencodeHttpApi._setFetchImplementationForTesting(fetchImplementation) + opencodeHttpApi._setLogImplementationForTesting(log) + opencodeHttpApi._setServerBasicAuthHeaderResolverForTesting(getServerBasicAuthHeader) + return opencodeHttpApi +} describe("getServerBaseUrl", () => { - it("returns baseUrl from client._client.getConfig().baseUrl", () => { + it("returns baseUrl from client._client.getConfig().baseUrl", async () => { // given + const { getServerBaseUrl } = await loadOpencodeHttpApi() const mockClient = { _client: { getConfig: () => ({ baseUrl: "https://api.example.com" }), @@ -28,8 +33,9 @@ describe("getServerBaseUrl", () => { expect(result).toBe("https://api.example.com") }) - it("returns baseUrl from client.session._client.getConfig().baseUrl when first attempt fails", () => { + it("returns baseUrl from client.session._client.getConfig().baseUrl when first attempt fails", async () => { // given + const { getServerBaseUrl } = await loadOpencodeHttpApi() const mockClient = { _client: { getConfig: () => ({}), @@ -48,8 +54,9 @@ describe("getServerBaseUrl", () => { expect(result).toBe("https://session.example.com") }) - it("returns null for incompatible client", () => { + it("returns null for incompatible client", async () => { // given + const { getServerBaseUrl } = await loadOpencodeHttpApi() const mockClient = {} // when @@ -62,14 +69,16 @@ describe("getServerBaseUrl", () => { describe("patchPart", () => { beforeEach(() => { - vi.clearAllMocks() - mockFetch.mockResolvedValue({ ok: true }) - process.env.OPENCODE_SERVER_PASSWORD = "testpassword" - process.env.OPENCODE_SERVER_USERNAME = "opencode" + log.mockClear() + getServerBasicAuthHeader.mockClear() + getServerBasicAuthHeader.mockReturnValue("Basic b3BlbmNvZGU6dGVzdHBhc3N3b3Jk") + fetchImplementation.mockClear() + fetchImplementation.mockResolvedValue(new Response(null, { status: 200 })) }) it("constructs correct URL and sends PATCH with auth", async () => { // given + const { patchPart } = await loadOpencodeHttpApi() const mockClient = { _client: { getConfig: () => ({ baseUrl: "https://api.example.com" }), @@ -85,7 +94,7 @@ describe("patchPart", () => { // then expect(result).toBe(true) - expect(mockFetch).toHaveBeenCalledWith( + expect(fetchImplementation).toHaveBeenCalledWith( "https://api.example.com/session/ses123/message/msg456/part/part789", expect.objectContaining({ method: "PATCH", @@ -101,12 +110,13 @@ describe("patchPart", () => { it("returns false on network error", async () => { // given + const { patchPart } = await loadOpencodeHttpApi() const mockClient = { _client: { getConfig: () => ({ baseUrl: "https://api.example.com" }), }, } - mockFetch.mockRejectedValue(new Error("Network error")) + fetchImplementation.mockRejectedValue(new Error("Network error")) // when const result = await patchPart(mockClient, "ses123", "msg456", "part789", {}) @@ -122,14 +132,16 @@ describe("patchPart", () => { describe("deletePart", () => { beforeEach(() => { - vi.clearAllMocks() - mockFetch.mockResolvedValue({ ok: true }) - process.env.OPENCODE_SERVER_PASSWORD = "testpassword" - process.env.OPENCODE_SERVER_USERNAME = "opencode" + log.mockClear() + getServerBasicAuthHeader.mockClear() + getServerBasicAuthHeader.mockReturnValue("Basic b3BlbmNvZGU6dGVzdHBhc3N3b3Jk") + fetchImplementation.mockClear() + fetchImplementation.mockResolvedValue(new Response(null, { status: 200 })) }) it("constructs correct URL and sends DELETE", async () => { // given + const { deletePart } = await loadOpencodeHttpApi() const mockClient = { _client: { getConfig: () => ({ baseUrl: "https://api.example.com" }), @@ -144,7 +156,7 @@ describe("deletePart", () => { // then expect(result).toBe(true) - expect(mockFetch).toHaveBeenCalledWith( + expect(fetchImplementation).toHaveBeenCalledWith( "https://api.example.com/session/ses123/message/msg456/part/part789", expect.objectContaining({ method: "DELETE", @@ -158,12 +170,13 @@ describe("deletePart", () => { it("returns false on non-ok response", async () => { // given + const { deletePart } = await loadOpencodeHttpApi() const mockClient = { _client: { getConfig: () => ({ baseUrl: "https://api.example.com" }), }, } - mockFetch.mockResolvedValue({ ok: false, status: 404 }) + fetchImplementation.mockResolvedValue(new Response(null, { status: 404 })) // when const result = await deletePart(mockClient, "ses123", "msg456", "part789") @@ -175,4 +188,4 @@ describe("deletePart", () => { url: "https://api.example.com/session/ses123/message/msg456/part/part789", }) }) -}) \ No newline at end of file +}) diff --git a/src/shared/opencode-http-api.ts b/src/shared/opencode-http-api.ts index 451d98e6a..c471e2e99 100644 --- a/src/shared/opencode-http-api.ts +++ b/src/shared/opencode-http-api.ts @@ -1,8 +1,41 @@ -import { getServerBasicAuthHeader } from "./opencode-server-auth" -import { log } from "./logger" +import { getServerBasicAuthHeader as resolveServerBasicAuthHeader } from "./opencode-server-auth" +import { log as writeLog } from "./logger" import { isRecord } from "./record-type-guard" type UnknownRecord = Record +type FetchImplementation = typeof fetch +type LogImplementation = typeof writeLog +type ServerBasicAuthHeaderResolver = typeof resolveServerBasicAuthHeader + +let fetchImplementationForTesting: FetchImplementation | undefined +let logImplementationForTesting: LogImplementation | undefined +let serverBasicAuthHeaderResolverForTesting: ServerBasicAuthHeaderResolver | undefined + +function getFetchImplementation(): FetchImplementation { + return fetchImplementationForTesting ?? fetch +} + +function getLogImplementation(): LogImplementation { + return logImplementationForTesting ?? writeLog +} + +function getServerBasicAuthHeaderImplementation(): ServerBasicAuthHeaderResolver { + return serverBasicAuthHeaderResolverForTesting ?? resolveServerBasicAuthHeader +} + +export function _setFetchImplementationForTesting(fetchImplementation: FetchImplementation | undefined): void { + fetchImplementationForTesting = fetchImplementation +} + +export function _setLogImplementationForTesting(logImplementation: LogImplementation | undefined): void { + logImplementationForTesting = logImplementation +} + +export function _setServerBasicAuthHeaderResolverForTesting( + resolver: ServerBasicAuthHeaderResolver | undefined, +): void { + serverBasicAuthHeaderResolverForTesting = resolver +} function getInternalClient(client: unknown): UnknownRecord | null { if (!isRecord(client)) { @@ -61,20 +94,20 @@ export async function patchPart( ): Promise { const baseUrl = getServerBaseUrl(client) if (!baseUrl) { - log("[opencode-http-api] Could not extract baseUrl from client") + getLogImplementation()("[opencode-http-api] Could not extract baseUrl from client") return false } - const auth = getServerBasicAuthHeader() + const auth = getServerBasicAuthHeaderImplementation()() if (!auth) { - log("[opencode-http-api] No auth header available") + getLogImplementation()("[opencode-http-api] No auth header available") return false } const url = `${baseUrl}/session/${encodeURIComponent(sessionID)}/message/${encodeURIComponent(messageID)}/part/${encodeURIComponent(partID)}` try { - const response = await fetch(url, { + const response = await getFetchImplementation()(url, { method: "PATCH", headers: { "Content-Type": "application/json", @@ -85,14 +118,14 @@ export async function patchPart( }) if (!response.ok) { - log("[opencode-http-api] PATCH failed", { status: response.status, url }) + getLogImplementation()("[opencode-http-api] PATCH failed", { status: response.status, url }) return false } return true } catch (error) { const message = error instanceof Error ? error.message : String(error) - log("[opencode-http-api] PATCH error", { message, url }) + getLogImplementation()("[opencode-http-api] PATCH error", { message, url }) return false } } @@ -105,20 +138,20 @@ export async function deletePart( ): Promise { const baseUrl = getServerBaseUrl(client) if (!baseUrl) { - log("[opencode-http-api] Could not extract baseUrl from client") + getLogImplementation()("[opencode-http-api] Could not extract baseUrl from client") return false } - const auth = getServerBasicAuthHeader() + const auth = getServerBasicAuthHeaderImplementation()() if (!auth) { - log("[opencode-http-api] No auth header available") + getLogImplementation()("[opencode-http-api] No auth header available") return false } const url = `${baseUrl}/session/${encodeURIComponent(sessionID)}/message/${encodeURIComponent(messageID)}/part/${encodeURIComponent(partID)}` try { - const response = await fetch(url, { + const response = await getFetchImplementation()(url, { method: "DELETE", headers: { "Authorization": auth, @@ -127,14 +160,14 @@ export async function deletePart( }) if (!response.ok) { - log("[opencode-http-api] DELETE failed", { status: response.status, url }) + getLogImplementation()("[opencode-http-api] DELETE failed", { status: response.status, url }) return false } return true } catch (error) { const message = error instanceof Error ? error.message : String(error) - log("[opencode-http-api] DELETE error", { message, url }) + getLogImplementation()("[opencode-http-api] DELETE error", { message, url }) return false } -} \ No newline at end of file +} diff --git a/src/shared/tmux/tmux-utils.test.ts b/src/shared/tmux/tmux-utils.test.ts index 421cc070b..53746c89e 100644 --- a/src/shared/tmux/tmux-utils.test.ts +++ b/src/shared/tmux/tmux-utils.test.ts @@ -1,4 +1,4 @@ -import { describe, test, expect, beforeEach, afterEach, mock } from "bun:test" +import { describe, test, expect, beforeEach, afterEach } from "bun:test" import { isInsideTmux, isServerRunning, @@ -9,15 +9,22 @@ import { applyLayout, } from "./tmux-utils" import { isInsideTmuxEnvironment } from "./tmux-utils/environment" +import { createServerHealthStateForTesting } from "./tmux-utils/server-health" -function createFetchMock(responseFactory: () => Promise): typeof fetch & ReturnType { - const fetchMock = mock(async (_input: RequestInfo | URL, _init?: RequestInit) => responseFactory()) +function createFetchRecorder(responseFactory: () => Promise): typeof fetch & { calls: Array<[RequestInfo | URL, RequestInit | undefined]> } { + const calls: Array<[RequestInfo | URL, RequestInit | undefined]> = [] + const fetchRecorder = async (input: RequestInfo | URL, init?: RequestInit): Promise => { + calls.push([input, init]) + return await responseFactory() + } const preconnect = globalThis.fetch.preconnect?.bind(globalThis.fetch) - return Object.assign(fetchMock, { + return Object.assign(fetchRecorder, { + calls, preconnect, - }) as typeof fetch & ReturnType + }) as typeof fetch & { calls: Array<[RequestInfo | URL, RequestInit | undefined]> } } + describe("isInsideTmux", () => { test("returns true when TMUX env is set", () => { // given @@ -62,22 +69,17 @@ describe("isInsideTmux", () => { }) describe("isServerRunning", () => { - const originalFetch = globalThis.fetch - beforeEach(() => { resetServerCheck() }) - afterEach(() => { - globalThis.fetch = originalFetch - }) - test("returns true when server responds OK", async () => { // given - globalThis.fetch = createFetchMock(async () => new Response(null, { status: 200 })) + const state = createServerHealthStateForTesting() + const fetchMock = createFetchRecorder(async () => new Response(null, { status: 200 })) // when - const result = await isServerRunning("http://localhost:4096") + const result = await isServerRunning("http://localhost:4096", { fetchImplementation: fetchMock, state }) // then expect(result).toBe(true) @@ -85,12 +87,13 @@ describe("isServerRunning", () => { test("returns false when server not reachable", async () => { // given - globalThis.fetch = createFetchMock(async () => { + const state = createServerHealthStateForTesting() + const fetchMock = createFetchRecorder(async () => { throw new Error("ECONNREFUSED") }) // when - const result = await isServerRunning("http://localhost:4096") + const result = await isServerRunning("http://localhost:4096", { fetchImplementation: fetchMock, state }) // then expect(result).toBe(false) @@ -98,10 +101,11 @@ describe("isServerRunning", () => { test("returns false when fetch returns not ok", async () => { // given - globalThis.fetch = createFetchMock(async () => new Response(null, { status: 500 })) + const state = createServerHealthStateForTesting() + const fetchMock = createFetchRecorder(async () => new Response(null, { status: 500 })) // when - const result = await isServerRunning("http://localhost:4096") + const result = await isServerRunning("http://localhost:4096", { fetchImplementation: fetchMock, state }) // then expect(result).toBe(false) @@ -109,43 +113,43 @@ describe("isServerRunning", () => { test("caches successful result", async () => { // given - const fetchMock = createFetchMock(async () => new Response(null, { status: 200 })) - globalThis.fetch = fetchMock + const state = createServerHealthStateForTesting() + const fetchMock = createFetchRecorder(async () => new Response(null, { status: 200 })) // when - await isServerRunning("http://localhost:4096") - await isServerRunning("http://localhost:4096") + await isServerRunning("http://localhost:4096", { fetchImplementation: fetchMock, state }) + await isServerRunning("http://localhost:4096", { fetchImplementation: fetchMock, state }) // then - should only call fetch once due to caching - expect(fetchMock.mock.calls.length).toBe(1) + expect(fetchMock.calls.length).toBe(1) }) test("does not cache failed result", async () => { // given - const fetchMock = createFetchMock(async () => { + const state = createServerHealthStateForTesting() + const fetchMock = createFetchRecorder(async () => { throw new Error("ECONNREFUSED") }) - globalThis.fetch = fetchMock // when - await isServerRunning("http://localhost:4096") - await isServerRunning("http://localhost:4096") + await isServerRunning("http://localhost:4096", { fetchImplementation: fetchMock, state }) + await isServerRunning("http://localhost:4096", { fetchImplementation: fetchMock, state }) // then - should call fetch 4 times (2 attempts per call, 2 calls) - expect(fetchMock.mock.calls.length).toBe(4) + expect(fetchMock.calls.length).toBe(4) }) test("uses different cache for different URLs", async () => { // given - const fetchMock = createFetchMock(async () => new Response(null, { status: 200 })) - globalThis.fetch = fetchMock + const state = createServerHealthStateForTesting() + const fetchMock = createFetchRecorder(async () => new Response(null, { status: 200 })) // when - await isServerRunning("http://localhost:4096") - await isServerRunning("http://localhost:5000") + await isServerRunning("http://localhost:4096", { fetchImplementation: fetchMock, state }) + await isServerRunning("http://localhost:5000", { fetchImplementation: fetchMock, state }) // then - should call fetch twice for different URLs - expect(fetchMock.mock.calls.length).toBe(2) + expect(fetchMock.calls.length).toBe(2) }) }) @@ -157,25 +161,22 @@ describe("resetServerCheck", () => { test("allows re-checking after reset", async () => { // given - const originalFetch = globalThis.fetch - const fetchMock = createFetchMock(async () => new Response(null, { status: 200 })) - globalThis.fetch = fetchMock + const state = createServerHealthStateForTesting() + const fetchMock = createFetchRecorder(async () => new Response(null, { status: 200 })) // when - await isServerRunning("http://localhost:4096") - resetServerCheck() - await isServerRunning("http://localhost:4096") + await isServerRunning("http://localhost:4096", { fetchImplementation: fetchMock, state }) + state.serverAvailable = null + state.serverCheckUrl = null + await isServerRunning("http://localhost:4096", { fetchImplementation: fetchMock, state }) // then - should call fetch twice after reset - expect(fetchMock.mock.calls.length).toBe(2) + expect(fetchMock.calls.length).toBe(2) - // cleanup - globalThis.fetch = originalFetch }) }) describe("markServerRunningInProcess", () => { - const originalFetch = globalThis.fetch const SERVER_RUNNING_KEY = Symbol.for("oh-my-opencode:server-running-in-process") beforeEach(() => { @@ -184,22 +185,21 @@ describe("markServerRunningInProcess", () => { }) afterEach(() => { - globalThis.fetch = originalFetch delete (globalThis as Record)[SERVER_RUNNING_KEY] }) test("skips HTTP fetch when marked as running in-process", async () => { // given - const fetchMock = createFetchMock(async () => new Response(null, { status: 200 })) - globalThis.fetch = fetchMock - markServerRunningInProcess() + const state = createServerHealthStateForTesting() + state.serverRunningInProcess = true + const fetchMock = createFetchRecorder(async () => new Response(null, { status: 200 })) // when - const result = await isServerRunning("http://localhost:4096") + const result = await isServerRunning("http://localhost:4096", { fetchImplementation: fetchMock, state }) // then expect(result).toBe(true) - expect(fetchMock.mock.calls.length).toBe(0) + expect(fetchMock.calls.length).toBe(0) }) test("uses globalThis so flag survives across module instances", () => { diff --git a/src/shared/tmux/tmux-utils/server-health.ts b/src/shared/tmux/tmux-utils/server-health.ts index a4c5c6806..59c758568 100644 --- a/src/shared/tmux/tmux-utils/server-health.ts +++ b/src/shared/tmux/tmux-utils/server-health.ts @@ -3,6 +3,17 @@ let serverCheckUrl: string | null = null const SERVER_RUNNING_KEY = Symbol.for("oh-my-opencode:server-running-in-process") +export type ServerHealthState = { + serverAvailable: boolean | null + serverCheckUrl: string | null + serverRunningInProcess: boolean +} + +type IsServerRunningOptions = { + fetchImplementation?: typeof fetch + state?: ServerHealthState +} + function delay(milliseconds: number): Promise { return new Promise((resolve) => setTimeout(resolve, milliseconds)) } @@ -15,12 +26,25 @@ function isMarkedRunningInProcess(): boolean { return (globalThis as Record)[SERVER_RUNNING_KEY] === true } -export async function isServerRunning(serverUrl: string): Promise { - if (isMarkedRunningInProcess()) { +export function createServerHealthStateForTesting(): ServerHealthState { + return { + serverAvailable: null, + serverCheckUrl: null, + serverRunningInProcess: false, + } +} + +export async function isServerRunning(serverUrl: string, options: IsServerRunningOptions = {}): Promise { + const fetchImplementation = options.fetchImplementation ?? fetch + const state = options.state + const markedRunning = state?.serverRunningInProcess ?? isMarkedRunningInProcess() + if (markedRunning) { return true } - if (serverCheckUrl === serverUrl && serverAvailable === true) { + const cachedUrl = state?.serverCheckUrl ?? serverCheckUrl + const cachedAvailable = state?.serverAvailable ?? serverAvailable + if (cachedUrl === serverUrl && cachedAvailable === true) { return true } @@ -33,14 +57,19 @@ export async function isServerRunning(serverUrl: string): Promise { const timeout = setTimeout(() => controller.abort(), timeoutMs) try { - const response = await fetch(healthUrl, { + const response = await fetchImplementation(healthUrl, { signal: controller.signal, }).catch(() => null) clearTimeout(timeout) if (response?.ok) { - serverCheckUrl = serverUrl - serverAvailable = true + if (state) { + state.serverCheckUrl = serverUrl + state.serverAvailable = true + } else { + serverCheckUrl = serverUrl + serverAvailable = true + } return true } } finally { diff --git a/src/shared/tmux/tmux-utils/session-spawn.test.ts b/src/shared/tmux/tmux-utils/session-spawn.test.ts index 315c6f150..689ad7c5d 100644 --- a/src/shared/tmux/tmux-utils/session-spawn.test.ts +++ b/src/shared/tmux/tmux-utils/session-spawn.test.ts @@ -1,9 +1,8 @@ -import { beforeEach, describe, expect, it, mock } from "bun:test" +import { describe, expect, it } from "bun:test" import type { TmuxConfig } from "../../../config/schema" import type { TmuxCommandResult } from "../runner" - -const sessionSpawnSpecifier = import.meta.resolve("./session-spawn") +import { spawnTmuxSession } from "./session-spawn" const enabledTmuxConfig = { enabled: true, @@ -14,17 +13,7 @@ const enabledTmuxConfig = { isolation: "inline", } satisfies TmuxConfig -const runTmuxCommandMock = mock(async (): Promise => ({ - success: true, - output: "", - stdout: "", - stderr: "", - exitCode: 0, -})) -const isInsideTmuxMock = mock((): boolean => true) -const isServerRunningMock = mock(async (): Promise => true) -const getTmuxPathMock = mock(async (): Promise => "sh") -const logMock = mock(() => undefined) +type SpawnTmuxSessionDeps = NonNullable[6]> function toStringArray(value: unknown): string[] { if (!Array.isArray(value)) { @@ -38,83 +27,74 @@ function toStringArray(value: unknown): string[] { return items } -function getRunTmuxCommandCall(index: number): [string, string[]] { - const call = Reflect.get(runTmuxCommandMock.mock.calls, index) - const command = Reflect.get(call, 0) - const args = Reflect.get(call, 1) - if (!Array.isArray(call) || typeof command !== "string" || !Array.isArray(args)) { - throw new Error(`Expected tmux runner call at index ${index}`) - } - - return [command, toStringArray(args)] +function defaultTmuxCommandResults(): TmuxCommandResult[] { + return [ + { success: true, output: "120,40", stdout: "120,40", stderr: "", exitCode: 0 }, + { success: false, output: "", stdout: "", stderr: "", exitCode: 1 }, + { success: true, output: "%42", stdout: "%42", stderr: "", exitCode: 0 }, + { success: true, output: "", stdout: "", stderr: "", exitCode: 0 }, + ] } -function getSpawnCommand(): string { - const newSessionCall = getRunTmuxCommandCall(2) - const newSessionCommand = newSessionCall[1][newSessionCall[1].length - 1] - if (newSessionCommand === undefined) { - throw new Error("Expected new-session command") +function createHarness() { + const calls: Array<[string, string[]]> = [] + const logs: string[] = [] + const tmuxCommandResults = defaultTmuxCommandResults() + const runTmuxCommand = async (command: string, args: string[]): Promise => { + calls.push([command, [...args]]) + const nextResult = tmuxCommandResults.shift() + if (!nextResult) { + throw new Error("No more tmux command results configured") + } + return nextResult + } + const deps: SpawnTmuxSessionDeps = { + log: (message) => { + logs.push(message) + }, + runTmuxCommand, + isInsideTmux: (): boolean => true, + isServerRunning: async (): Promise => true, + getTmuxPath: async (): Promise => "sh", } - return newSessionCommand -} + function getRunTmuxCommandCall(index: number): [string, string[]] { + const call = calls[index] + if (!call) { + throw new Error(`Expected tmux runner call at index ${index}; logs: ${logs.join(", ")}`) + } -function createDeps(): NonNullable[6]> { - return { - log: logMock, - runTmuxCommand: runTmuxCommandMock, - isInsideTmux: isInsideTmuxMock, - isServerRunning: isServerRunningMock, - getTmuxPath: getTmuxPathMock, + return [call[0], toStringArray(call[1])] } -} -async function loadSpawnTmuxSession(): Promise { - const module = await import(`${sessionSpawnSpecifier}?test=${crypto.randomUUID()}`) - return module.spawnTmuxSession + function getSpawnCommand(): string { + const newSessionCall = getRunTmuxCommandCall(2) + const newSessionCommand = newSessionCall[1][newSessionCall[1].length - 1] + if (newSessionCommand === undefined) { + throw new Error("Expected new-session command") + } + + return newSessionCommand + } + + return { deps, getRunTmuxCommandCall, getSpawnCommand } } describe("spawnTmuxSession runner integration", () => { - beforeEach(() => { - mock.restore() - runTmuxCommandMock.mockClear() - isInsideTmuxMock.mockClear() - isServerRunningMock.mockClear() - getTmuxPathMock.mockClear() - logMock.mockClear() - - const tmuxCommandResults: TmuxCommandResult[] = [ - { success: true, output: "120,40", stdout: "120,40", stderr: "", exitCode: 0 }, - { success: false, output: "", stdout: "", stderr: "", exitCode: 1 }, - { success: true, output: "%42", stdout: "%42", stderr: "", exitCode: 0 }, - { success: true, output: "", stdout: "", stderr: "", exitCode: 0 }, - ] - runTmuxCommandMock.mockImplementation(async (): Promise => { - const nextResult = tmuxCommandResults.shift() - if (!nextResult) { - throw new Error("No more tmux command results configured") - } - return nextResult - }) - isInsideTmuxMock.mockReturnValue(true) - isServerRunningMock.mockResolvedValue(true) - getTmuxPathMock.mockResolvedValue("sh") - }) - it("#given source pane available #when spawnTmuxSession called #then delegates display, has-session, new-session, and select-pane to shared runner", async () => { // given - const spawnTmuxSession = await loadSpawnTmuxSession() + const harness = createHarness() const directory = "/tmp/omo-project/(session)" // when - const result = await spawnTmuxSession("session-1", "worker", enabledTmuxConfig, "http://127.0.0.1:1234", directory, "%0", createDeps()) + const result = await spawnTmuxSession("session-1", "worker", enabledTmuxConfig, "http://127.0.0.1:1234", directory, "%0", harness.deps) // then - const displayCall = getRunTmuxCommandCall(0) - const hasSessionCall = getRunTmuxCommandCall(1) - const newSessionCall = getRunTmuxCommandCall(2) - const selectPaneCall = getRunTmuxCommandCall(3) expect(result).toEqual({ success: true, paneId: "%42" }) + const displayCall = harness.getRunTmuxCommandCall(0) + const hasSessionCall = harness.getRunTmuxCommandCall(1) + const newSessionCall = harness.getRunTmuxCommandCall(2) + const selectPaneCall = harness.getRunTmuxCommandCall(3) expect(displayCall[1]).toEqual(["display", "-p", "-t", "%0", "#{window_width},#{window_height}"]) expect(hasSessionCall[1][0]).toBe("has-session") expect(hasSessionCall[1][1]).toBe("-t") @@ -122,39 +102,39 @@ describe("spawnTmuxSession runner integration", () => { expect(newSessionCall[1].slice(0, 4)).toEqual(["new-session", "-d", "-s", newSessionCall[1][3]]) expect(String(newSessionCall[1][3]).startsWith("omo-agents-")).toBe(true) expect(selectPaneCall[1]).toEqual(["select-pane", "-t", "%42", "-T", "omo-subagent-worker"]) - expect(getSpawnCommand()).toContain(` --dir '${directory}'`) + expect(harness.getSpawnCommand()).toContain(` --dir '${directory}'`) }) it("#given directory with spaces #when spawnTmuxSession called #then wraps --dir value in single quotes", async () => { // given - const spawnTmuxSession = await loadSpawnTmuxSession() + const harness = createHarness() // when - await spawnTmuxSession("session-1", "worker", enabledTmuxConfig, "http://127.0.0.1:1234", "/path with spaces/here", "%0", createDeps()) + await spawnTmuxSession("session-1", "worker", enabledTmuxConfig, "http://127.0.0.1:1234", "/path with spaces/here", "%0", harness.deps) // then - expect(getSpawnCommand()).toContain("--dir '/path with spaces/here'") + expect(harness.getSpawnCommand()).toContain("--dir '/path with spaces/here'") }) it("#given empty directory #when spawnTmuxSession called #then falls back to process cwd", async () => { // given - const spawnTmuxSession = await loadSpawnTmuxSession() + const harness = createHarness() // when - await spawnTmuxSession("session-1", "worker", enabledTmuxConfig, "http://127.0.0.1:1234", "", "%0", createDeps()) + await spawnTmuxSession("session-1", "worker", enabledTmuxConfig, "http://127.0.0.1:1234", "", "%0", harness.deps) // then - expect(getSpawnCommand()).toContain(`--dir '${process.cwd()}'`) + expect(harness.getSpawnCommand()).toContain(`--dir '${process.cwd()}'`) }) it("#given directory with single quotes #when spawnTmuxSession called #then escapes the value with POSIX-safe single quoting", async () => { // given - const spawnTmuxSession = await loadSpawnTmuxSession() + const harness = createHarness() // when - await spawnTmuxSession("session-1", "worker", enabledTmuxConfig, "http://127.0.0.1:1234", "/path/with'quote", "%0", createDeps()) + await spawnTmuxSession("session-1", "worker", enabledTmuxConfig, "http://127.0.0.1:1234", "/path/with'quote", "%0", harness.deps) // then - expect(getSpawnCommand()).toContain("--dir '/path/with'\\''quote'") + expect(harness.getSpawnCommand()).toContain("--dir '/path/with'\\''quote'") }) }) diff --git a/src/shared/tmux/tmux-utils/window-spawn.test.ts b/src/shared/tmux/tmux-utils/window-spawn.test.ts index a8abdcf92..03d7e46ef 100644 --- a/src/shared/tmux/tmux-utils/window-spawn.test.ts +++ b/src/shared/tmux/tmux-utils/window-spawn.test.ts @@ -1,9 +1,8 @@ -import { beforeEach, describe, expect, it, mock } from "bun:test" +import { describe, expect, it } from "bun:test" import type { TmuxConfig } from "../../../config/schema" import type { TmuxCommandResult } from "../runner" - -const windowSpawnSpecifier = import.meta.resolve("./window-spawn") +import { spawnTmuxWindow } from "./window-spawn" const enabledTmuxConfig = { enabled: true, @@ -14,17 +13,7 @@ const enabledTmuxConfig = { isolation: "inline", } satisfies TmuxConfig -const runTmuxCommandMock = mock(async (): Promise => ({ - success: true, - output: "%42", - stdout: "%42", - stderr: "", - exitCode: 0, -})) -const isInsideTmuxMock = mock((): boolean => true) -const isServerRunningMock = mock(async (): Promise => true) -const getTmuxPathMock = mock(async (): Promise => "sh") -const logMock = mock(() => undefined) +type SpawnTmuxWindowDeps = NonNullable[5]> function toStringArray(value: unknown): string[] { if (!Array.isArray(value)) { @@ -38,114 +27,102 @@ function toStringArray(value: unknown): string[] { return items } -function getRunTmuxCommandCall(index: number): [string, string[]] { - const call = Reflect.get(runTmuxCommandMock.mock.calls, index) - const command = Reflect.get(call, 0) - const args = Reflect.get(call, 1) - if (!Array.isArray(call) || typeof command !== "string" || !Array.isArray(args)) { - throw new Error(`Expected tmux runner call at index ${index}`) - } - - return [command, toStringArray(args)] +function defaultTmuxCommandResults(): TmuxCommandResult[] { + return [ + { success: true, output: "%42", stdout: "%42", stderr: "", exitCode: 0 }, + { success: true, output: "", stdout: "", stderr: "", exitCode: 0 }, + ] } -function getNewWindowCommand(): string { - const firstCall = getRunTmuxCommandCall(0) - const newWindowCommand = firstCall[1][7] - if (newWindowCommand === undefined) { - throw new Error("Expected new-window command") +function createHarness() { + const calls: Array<[string, string[]]> = [] + const tmuxCommandResults = defaultTmuxCommandResults() + const runTmuxCommand = async (command: string, args: string[]): Promise => { + calls.push([command, [...args]]) + const nextResult = tmuxCommandResults.shift() + if (!nextResult) { + throw new Error("No more tmux command results configured") + } + return nextResult + } + const deps: SpawnTmuxWindowDeps = { + log: () => undefined, + runTmuxCommand, + isInsideTmux: (): boolean => true, + isServerRunning: async (): Promise => true, + getTmuxPath: async (): Promise => "sh", } - return newWindowCommand -} + function getRunTmuxCommandCall(index: number): [string, string[]] { + const call = calls[index] + if (!call) { + throw new Error(`Expected tmux runner call at index ${index}`) + } -function createDeps(): NonNullable[5]> { - return { - log: logMock, - runTmuxCommand: runTmuxCommandMock, - isInsideTmux: isInsideTmuxMock, - isServerRunning: isServerRunningMock, - getTmuxPath: getTmuxPathMock, + return [call[0], toStringArray(call[1])] } -} -async function loadSpawnTmuxWindow(): Promise { - const module = await import(`${windowSpawnSpecifier}?test=${crypto.randomUUID()}`) - return module.spawnTmuxWindow + function getNewWindowCommand(): string { + const firstCall = getRunTmuxCommandCall(0) + const newWindowCommand = firstCall[1][7] + if (newWindowCommand === undefined) { + throw new Error("Expected new-window command") + } + + return newWindowCommand + } + + return { deps, getRunTmuxCommandCall, getNewWindowCommand } } describe("spawnTmuxWindow runner integration", () => { - beforeEach(() => { - mock.restore() - runTmuxCommandMock.mockClear() - isInsideTmuxMock.mockClear() - isServerRunningMock.mockClear() - getTmuxPathMock.mockClear() - logMock.mockClear() - - const tmuxCommandResults: TmuxCommandResult[] = [ - { success: true, output: "%42", stdout: "%42", stderr: "", exitCode: 0 }, - { success: true, output: "", stdout: "", stderr: "", exitCode: 0 }, - ] - runTmuxCommandMock.mockImplementation(async (): Promise => { - const nextResult = tmuxCommandResults.shift() - if (!nextResult) { - throw new Error("No more tmux command results configured") - } - return nextResult - }) - isInsideTmuxMock.mockReturnValue(true) - isServerRunningMock.mockResolvedValue(true) - getTmuxPathMock.mockResolvedValue("sh") - }) - it("#given healthy tmux environment #when spawnTmuxWindow called #then delegates new-window and select-pane to shared runner", async () => { // given - const spawnTmuxWindow = await loadSpawnTmuxWindow() + const harness = createHarness() const directory = "/tmp/omo-project/(window)" // when - const result = await spawnTmuxWindow("session-1", "worker", enabledTmuxConfig, "http://127.0.0.1:1234", directory, createDeps()) + const result = await spawnTmuxWindow("session-1", "worker", enabledTmuxConfig, "http://127.0.0.1:1234", directory, harness.deps) // then - const firstCall = getRunTmuxCommandCall(0) - const secondCall = getRunTmuxCommandCall(1) + const firstCall = harness.getRunTmuxCommandCall(0) + const secondCall = harness.getRunTmuxCommandCall(1) expect(result).toEqual({ success: true, paneId: "%42" }) expect(firstCall[1].slice(0, 7)).toEqual(["new-window", "-d", "-n", "omo-agents", "-P", "-F", "#{pane_id}"]) expect(secondCall[1]).toEqual(["select-pane", "-t", "%42", "-T", "omo-subagent-worker"]) - expect(getNewWindowCommand()).toContain(` --dir '${directory}'`) + expect(harness.getNewWindowCommand()).toContain(` --dir '${directory}'`) }) it("#given directory with spaces #when spawnTmuxWindow called #then wraps --dir value in single quotes", async () => { // given - const spawnTmuxWindow = await loadSpawnTmuxWindow() + const harness = createHarness() // when - await spawnTmuxWindow("session-1", "worker", enabledTmuxConfig, "http://127.0.0.1:1234", "/path with spaces/here", createDeps()) + await spawnTmuxWindow("session-1", "worker", enabledTmuxConfig, "http://127.0.0.1:1234", "/path with spaces/here", harness.deps) // then - expect(getNewWindowCommand()).toContain("--dir '/path with spaces/here'") + expect(harness.getNewWindowCommand()).toContain("--dir '/path with spaces/here'") }) it("#given empty directory #when spawnTmuxWindow called #then falls back to process cwd", async () => { // given - const spawnTmuxWindow = await loadSpawnTmuxWindow() + const harness = createHarness() // when - await spawnTmuxWindow("session-1", "worker", enabledTmuxConfig, "http://127.0.0.1:1234", "", createDeps()) + await spawnTmuxWindow("session-1", "worker", enabledTmuxConfig, "http://127.0.0.1:1234", "", harness.deps) // then - expect(getNewWindowCommand()).toContain(`--dir '${process.cwd()}'`) + expect(harness.getNewWindowCommand()).toContain(`--dir '${process.cwd()}'`) }) it("#given directory with single quotes #when spawnTmuxWindow called #then escapes the value with POSIX-safe single quoting", async () => { // given - const spawnTmuxWindow = await loadSpawnTmuxWindow() + const harness = createHarness() // when - await spawnTmuxWindow("session-1", "worker", enabledTmuxConfig, "http://127.0.0.1:1234", "/path/with'quote", createDeps()) + await spawnTmuxWindow("session-1", "worker", enabledTmuxConfig, "http://127.0.0.1:1234", "/path/with'quote", harness.deps) // then - expect(getNewWindowCommand()).toContain("--dir '/path/with'\\''quote'") + expect(harness.getNewWindowCommand()).toContain("--dir '/path/with'\\''quote'") }) }) diff --git a/src/testing/module-mock-lifecycle.test.ts b/src/testing/module-mock-lifecycle.test.ts index 7a7c210f3..3cf209c88 100644 --- a/src/testing/module-mock-lifecycle.test.ts +++ b/src/testing/module-mock-lifecycle.test.ts @@ -31,6 +31,36 @@ describe("installModuleMockLifecycle", () => { ]) }) + test("restores original exports after the delegate restore runs", () => { + // given + const events: string[] = [] + const mockApi = { + module: (specifier: string, factory: () => Record) => { + events.push(`module:${specifier}:${String(factory().named)}`) + }, + restore: mock(() => { + events.push("delegate:restore") + }), + } + + installModuleMockLifecycle(mockApi, { + getCallerUrl: () => "file:///repo/tests/example.test.ts", + resolveSpecifier: (specifier) => `resolved:${specifier}`, + loadOriginalModule: () => ({ ok: true, value: { named: "original" } }), + }) + + // when + mockApi.module("./dependency", () => ({ named: "mocked" })) + mockApi.restore() + + // then + expect(events).toEqual([ + "module:./dependency:mocked", + "delegate:restore", + "module:resolved:./dependency:original", + ]) + }) + test("captures the original module only once per resolved specifier", () => { // given let loadCount = 0 diff --git a/src/testing/module-mock-lifecycle.ts b/src/testing/module-mock-lifecycle.ts index d9b549eb1..0e702550b 100644 --- a/src/testing/module-mock-lifecycle.ts +++ b/src/testing/module-mock-lifecycle.ts @@ -135,8 +135,9 @@ export function installModuleMockLifecycle( } mockApi.restore = (): unknown => { + const result = delegateRestore() restoreModuleMocks() - return delegateRestore() + return result } return { restoreModuleMocks } diff --git a/src/tools/interactive-bash/tools.test.ts b/src/tools/interactive-bash/tools.test.ts index 9c0d20249..83a6a119b 100644 --- a/src/tools/interactive-bash/tools.test.ts +++ b/src/tools/interactive-bash/tools.test.ts @@ -1,19 +1,7 @@ /// import { describe, expect, test } from "bun:test" -import type { ToolContext } from "@opencode-ai/plugin/tool" -import { interactive_bash } from "./tools" - -const mockContext = { - sessionID: "test-session", - messageID: "test-message", - agent: "test-agent", - directory: "/project", - worktree: "/project", - abort: new AbortController().signal, - metadata: () => {}, - ask: async () => {}, -} satisfies ToolContext +import { executeInteractiveBash } from "./tools" describe("interactive_bash", () => { test("#given kill-server command #when executed #then returns a strong prohibition without running tmux", async () => { @@ -21,7 +9,7 @@ describe("interactive_bash", () => { const args = { tmux_command: "kill-server" } // when - const output = await interactive_bash.execute(args, mockContext) + const output = await executeInteractiveBash(args) // then expect(output).toContain("Error: 'kill-server' is prohibited in interactive_bash.") @@ -34,7 +22,7 @@ describe("interactive_bash", () => { const args = { tmux_command: "-L omo-socket kill-server" } // when - const output = await interactive_bash.execute(args, mockContext) + const output = await executeInteractiveBash(args) // then expect(output).toContain("Error: 'kill-server' is prohibited in interactive_bash.") diff --git a/src/tools/interactive-bash/tools.ts b/src/tools/interactive-bash/tools.ts index ba6eee2be..d41fb9b6e 100644 --- a/src/tools/interactive-bash/tools.ts +++ b/src/tools/interactive-bash/tools.ts @@ -144,74 +144,80 @@ tmux kill-session -t If you created an omo-* session, kill only that exact session. Do not retry kill-server with Bash or any other tool.` } +type InteractiveBashArgs = { + tmux_command: string +} + +export async function executeInteractiveBash(args: InteractiveBashArgs): Promise { + try { + const tmuxPath = getCachedTmuxPath() ?? "tmux" + + const parts = tokenizeCommand(args.tmux_command) + + if (parts.length === 0) { + return "Error: Empty tmux command" + } + + const subcommandIndex = findSubcommandIndex(parts) + const rawSubcommand = subcommandIndex === -1 ? "" : parts[subcommandIndex] + const subcommand = rawSubcommand.toLowerCase() + + if (PROHIBITED_TMUX_SUBCOMMANDS.includes(subcommand)) { + return buildProhibitedTmuxCommandMessage(rawSubcommand) + } + + if (BLOCKED_TMUX_SUBCOMMANDS.includes(subcommand)) { + return buildBlockedTmuxCommandMessage(rawSubcommand, parts) + } + + const proc = spawnWithWindowsHide([...resolveTmuxExecutable(tmuxPath), ...parts], { + stdout: "pipe", + stderr: "pipe", + }) + + const timeoutPromise = new Promise((_, reject) => { + const id = setTimeout(() => { + const timeoutError = new Error(`Timeout after ${DEFAULT_TIMEOUT_MS}ms`) + try { + proc.kill() + // Fire-and-forget: wait for process exit in background to avoid zombies + void proc.exited.catch(() => {}) + } catch { + // Ignore kill errors; we'll still reject with timeoutError below + } + reject(timeoutError) + }, DEFAULT_TIMEOUT_MS) + proc.exited + .then(() => clearTimeout(id)) + .catch(() => clearTimeout(id)) + }) + + // Read stdout and stderr in parallel to avoid race conditions + const [stdout, stderr, exitCode] = await Promise.race([ + Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]), + timeoutPromise, + ]) + + // Check exitCode properly - return error even if stderr is empty + if (exitCode !== 0) { + const errorMsg = stderr.trim() || `Command failed with exit code ${exitCode}` + return `Error: ${errorMsg}` + } + + return stdout || "(no output)" + } catch (e) { + return `Error: ${e instanceof Error ? e.message : String(e)}` + } +} + export const interactive_bash: ToolDefinition = tool({ description: INTERACTIVE_BASH_DESCRIPTION, args: { tmux_command: tool.schema.string().describe("The tmux command to execute (without 'tmux' prefix)"), }, - execute: async (args) => { - try { - const tmuxPath = getCachedTmuxPath() ?? "tmux" - - const parts = tokenizeCommand(args.tmux_command) - - if (parts.length === 0) { - return "Error: Empty tmux command" - } - - const subcommandIndex = findSubcommandIndex(parts) - const rawSubcommand = subcommandIndex === -1 ? "" : parts[subcommandIndex] - const subcommand = rawSubcommand.toLowerCase() - - if (PROHIBITED_TMUX_SUBCOMMANDS.includes(subcommand)) { - return buildProhibitedTmuxCommandMessage(rawSubcommand) - } - - if (BLOCKED_TMUX_SUBCOMMANDS.includes(subcommand)) { - return buildBlockedTmuxCommandMessage(rawSubcommand, parts) - } - - const proc = spawnWithWindowsHide([...resolveTmuxExecutable(tmuxPath), ...parts], { - stdout: "pipe", - stderr: "pipe", - }) - - const timeoutPromise = new Promise((_, reject) => { - const id = setTimeout(() => { - const timeoutError = new Error(`Timeout after ${DEFAULT_TIMEOUT_MS}ms`) - try { - proc.kill() - // Fire-and-forget: wait for process exit in background to avoid zombies - void proc.exited.catch(() => {}) - } catch { - // Ignore kill errors; we'll still reject with timeoutError below - } - reject(timeoutError) - }, DEFAULT_TIMEOUT_MS) - proc.exited - .then(() => clearTimeout(id)) - .catch(() => clearTimeout(id)) - }) - - // Read stdout and stderr in parallel to avoid race conditions - const [stdout, stderr, exitCode] = await Promise.race([ - Promise.all([ - new Response(proc.stdout).text(), - new Response(proc.stderr).text(), - proc.exited, - ]), - timeoutPromise, - ]) - - // Check exitCode properly - return error even if stderr is empty - if (exitCode !== 0) { - const errorMsg = stderr.trim() || `Command failed with exit code ${exitCode}` - return `Error: ${errorMsg}` - } - - return stdout || "(no output)" - } catch (e) { - return `Error: ${e instanceof Error ? e.message : String(e)}` - } - }, + execute: executeInteractiveBash, })