From b3a195d6625a29c6589e72f8d73ca32663041b59 Mon Sep 17 00:00:00 2001 From: ilseob lee Date: Thu, 21 May 2026 15:48:03 +0900 Subject: [PATCH 001/103] Avoid look_at status map wait hang --- src/tools/look-at/look-at-session-runner.ts | 11 ++++---- src/tools/look-at/session-poller.test.ts | 30 +++++++++++++++++++++ src/tools/look-at/session-poller.ts | 4 +-- 3 files changed, 37 insertions(+), 8 deletions(-) diff --git a/src/tools/look-at/look-at-session-runner.ts b/src/tools/look-at/look-at-session-runner.ts index db8850156..795ef8504 100644 --- a/src/tools/look-at/look-at-session-runner.ts +++ b/src/tools/look-at/look-at-session-runner.ts @@ -1,6 +1,6 @@ import type { PluginInput } from "@opencode-ai/plugin" import type { ToolContext } from "@opencode-ai/plugin/tool" -import { log, promptSyncWithModelSuggestionRetry } from "../../shared" +import { isAmbiguousPromptDispatchFailure, log, promptSyncWithModelSuggestionRetry } from "../../shared" import { extractLatestAssistantText } from "./assistant-message-extractor" import { MULTIMODAL_LOOKER_AGENT } from "./constants" import { READ_ENABLED, buildLookAtPrompt } from "./look-at-prompt" @@ -61,7 +61,7 @@ Original error: ${createResult.error}` log(`[look_at] Created session: ${sessionID}`) log(`[look_at] Sending prompt with ${isBase64Input ? "base64 image" : "file"} to session ${sessionID}`) - let promptFailed = false + let shouldWaitForStatus = true try { await promptSyncWithModelSuggestionRetry(ctx.client, { path: { id: sessionID }, @@ -84,16 +84,15 @@ Original error: ${createResult.error}` queueBehavior: "defer", }) } catch (promptError) { - promptFailed = true - log("[look_at] Prompt error (ignored, will still fetch messages):", promptError) + log("[look_at] Prompt dispatch failed; checking child session evidence:", promptError) + shouldWaitForStatus = isAmbiguousPromptDispatchFailure(promptError) } let observedMessages: unknown[] | undefined let observedText: string | undefined - if (typeof ctx.client.session.status === "function") { + if (shouldWaitForStatus && typeof ctx.client.session.status === "function") { const waitResult = await waitForLookAtSessionResult(ctx.client, sessionID, { allowStableIdleWithoutActivity: true, - allowEmptyStableIdleWithoutActivity: promptFailed, }) observedText = waitResult.outcome.text ?? undefined if (observedText) { diff --git a/src/tools/look-at/session-poller.test.ts b/src/tools/look-at/session-poller.test.ts index 69b4c8b9e..1996de762 100644 --- a/src/tools/look-at/session-poller.test.ts +++ b/src/tools/look-at/session-poller.test.ts @@ -90,6 +90,36 @@ describe("waitForLookAtSessionResult", () => { ).rejects.toThrow("timed out") }) + test("#given status omits session before it starts #when later idle has response #then waits instead of treating empty status as done", async () => { + const assistantMessages: RawMessage[] = [ + { info: { role: "user" }, parts: [{ type: "text", text: "analyze this" }] }, + { info: { role: "assistant" }, parts: [{ type: "text", text: "late result" }] }, + ] + let statusCalls = 0 + const client = { + session: { + status: mock(async () => { + statusCalls += 1 + if (statusCalls <= 3) return { data: {} } + if (statusCalls === 4) return { data: { ses_test: { type: "busy" } } } + return { data: { ses_test: { type: "idle" } } } + }), + messages: mock(async () => ({ + data: statusCalls >= 5 ? assistantMessages : [], + error: null, + })), + }, + } + + const result = await waitForLookAtSessionResult(unsafeTestValue(client), "ses_test", { + pollIntervalMs: 10, + timeoutMs: 5000, + }) + + expect(result.outcome.text).toBe("late result") + expect(statusCalls).toBe(5) + }) + test("#given session never becomes idle #when polling exceeds timeout #then rejects", async () => { const client = createMockClient( [{ data: { ses_test: { type: "busy" } } }], diff --git a/src/tools/look-at/session-poller.ts b/src/tools/look-at/session-poller.ts index e17e9cd2e..406d4f019 100644 --- a/src/tools/look-at/session-poller.ts +++ b/src/tools/look-at/session-poller.ts @@ -109,11 +109,11 @@ export async function waitForLookAtSessionResult( const { messages, error: messagesError } = await getSessionMessages(client, sessionID) const outcome = extractLatestAssistantOutcome(messages) - if (outcome.text && !isActive) { + if (outcome.text && (!isActive || supportedButNeverSeen)) { return { messages, outcome, statusType } } - if (outcome.errorName && !isActive) { + if (outcome.errorName && (!isActive || supportedButNeverSeen)) { return { messages, outcome, statusType } } From c4a51bee238ca18c7fc4e9daadab36c76f79d950 Mon Sep 17 00:00:00 2001 From: ilseob lee Date: Thu, 21 May 2026 16:03:35 +0900 Subject: [PATCH 002/103] Cover look_at permanently absent session output --- src/tools/look-at/session-poller.test.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/tools/look-at/session-poller.test.ts b/src/tools/look-at/session-poller.test.ts index 1996de762..6a104a899 100644 --- a/src/tools/look-at/session-poller.test.ts +++ b/src/tools/look-at/session-poller.test.ts @@ -90,6 +90,22 @@ describe("waitForLookAtSessionResult", () => { ).rejects.toThrow("timed out") }) + test("#given supported status never lists the session but assistant output exists #when polling #then resolves with observed output", async () => { + const assistantMessages: RawMessage[] = [ + { info: { role: "user" }, parts: [{ type: "text", text: "inspect this" }] }, + { info: { role: "assistant" }, parts: [{ type: "text", text: "observed result" }] }, + ] + const client = createMockClient([{ data: {} }], assistantMessages) + + const result = await waitForLookAtSessionResult(unsafeTestValue(client), "ses_test", { + pollIntervalMs: 10, + timeoutMs: 5000, + }) + + expect(result.outcome.text).toBe("observed result") + expect(client.session.status).toHaveBeenCalledTimes(1) + }) + test("#given status omits session before it starts #when later idle has response #then waits instead of treating empty status as done", async () => { const assistantMessages: RawMessage[] = [ { info: { role: "user" }, parts: [{ type: "text", text: "analyze this" }] }, From d788c3d1a976b78eceb7a88de73dac61e92253d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E5=86=A0=E8=BE=B0?= Date: Fri, 22 May 2026 11:36:22 +0800 Subject: [PATCH 003/103] fix(migration): stop rewriting explicit gpt-5.3-codex to gpt-5.4 (#3777) `openai/gpt-5.3-codex` is the codex-series powerhouse still recommended in docs/guide/agent-model-matching.md and listed in the default fallback chain in docs/reference/configuration.md, not a deprecated alias for `gpt-5.4`. The migration entry silently rewrote any user config that picked `gpt-5.3-codex` for its token efficiency, sending agents to a non-codex model on every startup. Drop the bogus mapping from MODEL_VERSION_MAP and add a regression test that explicit `gpt-5.3-codex` selections (including in nested fallback_models) survive `migrateModelVersions`. Users already auto-migrated previously can revert to `gpt-5.3-codex` by hand and it will now stick on subsequent loads regardless of the sidecar history. --- src/shared/migration.test.ts | 34 +++++++++++++++++++++++--- src/shared/migration/model-versions.ts | 7 +++++- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/src/shared/migration.test.ts b/src/shared/migration.test.ts index 290e43838..58d09da16 100644 --- a/src/shared/migration.test.ts +++ b/src/shared/migration.test.ts @@ -578,10 +578,18 @@ describe("MODEL_VERSION_MAP", () => { expect(MODEL_VERSION_MAP["anthropic/claude-opus-4-5"]).toBe("anthropic/claude-opus-4-7") }) - test("maps openai/gpt-5.3-codex to openai/gpt-5.4 for deep category migration", () => { + test("does not migrate openai/gpt-5.3-codex (still a supported codex variant, #3777)", () => { // given/when: Check MODEL_VERSION_MAP - // then: gpt-5.3-codex should migrate to gpt-5.4 - expect(MODEL_VERSION_MAP["openai/gpt-5.3-codex"]).toBe("openai/gpt-5.4") + // then: gpt-5.3-codex must remain user-selectable — it is the codex + // powerhouse documented in agent-model-matching.md, not a + // deprecated alias for gpt-5.4 + expect(MODEL_VERSION_MAP["openai/gpt-5.3-codex"]).toBeUndefined() + }) + + test("maps openai/gpt-5.4 to openai/gpt-5.5", () => { + // given/when: Check MODEL_VERSION_MAP + // then: gpt-5.4 should migrate to gpt-5.5 + expect(MODEL_VERSION_MAP["openai/gpt-5.4"]).toBe("openai/gpt-5.5") }) }) @@ -602,6 +610,26 @@ describe("migrateModelVersions", () => { expect(sisyphus.temperature).toBe(0.1) }) + test("#given a config with explicit gpt-5.3-codex (#3777) #when migrating #then preserves the codex variant", () => { + // given: User explicitly picked the codex powerhouse for token efficiency + const agents = { + sisyphus: { model: "openai/gpt-5.3-codex", variant: "medium" }, + hephaestus: { + model: "openai/gpt-5.3-codex", + fallback_models: [{ model: "openai/gpt-5.3-codex" }], + }, + } + + // when: Migrate model versions + const { migrated, changed, newMigrations } = migrateModelVersions(agents) + + // then: gpt-5.3-codex must remain — auto-rewriting silently broke configs + expect(changed).toBe(false) + expect(newMigrations).toEqual([]) + expect((migrated["sisyphus"] as Record).model).toBe("openai/gpt-5.3-codex") + expect((migrated["hephaestus"] as Record).model).toBe("openai/gpt-5.3-codex") + }) + test("replaces anthropic model version", () => { // given: Agent config with old anthropic model const agents = { diff --git a/src/shared/migration/model-versions.ts b/src/shared/migration/model-versions.ts index c529513c9..9c2632893 100644 --- a/src/shared/migration/model-versions.ts +++ b/src/shared/migration/model-versions.ts @@ -4,12 +4,17 @@ * bumps to newer model versions. * * Keys are full "provider/model" strings. Only openai and anthropic entries needed. + * + * Only include genuinely retired/superseded models here. Do NOT add mappings + * for current, user-selectable variants — `gpt-5.3-codex` is the canonical + * codex powerhouse referenced in docs/guide/agent-model-matching.md and is + * NOT a deprecated alias for `gpt-5.4`. Auto-rewriting an explicit user + * choice silently broke configurations (#3777). */ export const MODEL_VERSION_MAP: Record = { "anthropic/claude-opus-4-5": "anthropic/claude-opus-4-7", "anthropic/claude-opus-4-6": "anthropic/claude-opus-4-7", "anthropic/claude-sonnet-4-5": "anthropic/claude-sonnet-4-6", - "openai/gpt-5.3-codex": "openai/gpt-5.4", "openai/gpt-5.4": "openai/gpt-5.5", } From ccaf61e09b30282d2de6939a340d22d9ed518c88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E5=86=A0=E8=BE=B0?= Date: Fri, 22 May 2026 14:19:57 +0800 Subject: [PATCH 004/103] test(ast-grep): lock Windows backslash matching for ast_grep dist cli suffix (#4220) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dist-side bug reported in #4220 (`path.endsWith(\"dist/cli.js\")` failing on Windows backslash paths) has already been fixed in source by routing through `hasCliSuffix` in `src/mcp/ast-grep.ts` and `src/mcp/lsp.ts`. The `cli-suffix.test.ts` covered the lsp-tools-mcp shape but not the ast_grep shape. Adds a regression case that asserts a Windows-style absolute path containing `...\\packages\\ast-grep-mcp\\dist\\cli.js` matches both `\"dist/cli.js\"` and the fully-qualified `\"packages/ast-grep-mcp/dist/cli.js\"` suffix — closing the exact symptom from the bug report against future regressions. --- src/mcp/cli-suffix.test.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/mcp/cli-suffix.test.ts b/src/mcp/cli-suffix.test.ts index f0435d430..9e8d60ab6 100644 --- a/src/mcp/cli-suffix.test.ts +++ b/src/mcp/cli-suffix.test.ts @@ -29,4 +29,21 @@ describe("hasCliSuffix", () => { // then expect(result).toBe(false) }) + + // regression: issue #4220 — ast_grep MCP failed on Windows because the older + // dist used `path.endsWith("dist/cli.js")`. `hasCliSuffix` must match Windows + // backslash paths against the POSIX-shaped `dist/cli.js` suffix. + it("matches the ast_grep dist cli suffix on Windows path separators", () => { + // given + const windowsPath = "C:\\Users\\test\\AppData\\Local\\cache\\oh-my-opencode\\dist\\packages\\ast-grep-mcp\\dist\\cli.js" + + // when: matched against just the trailing `dist/cli.js` segment + const matchesShortSuffix = hasCliSuffix(windowsPath, "dist/cli.js") + // and the fully-qualified package suffix + const matchesPackageSuffix = hasCliSuffix(windowsPath, "packages/ast-grep-mcp/dist/cli.js") + + // then: both must succeed despite the backslashes + expect(matchesShortSuffix).toBe(true) + expect(matchesPackageSuffix).toBe(true) + }) }) From a7429cc22382100ad979564f8a0ed07a4fccec3b Mon Sep 17 00:00:00 2001 From: MoerAI Date: Fri, 22 May 2026 16:06:19 +0900 Subject: [PATCH 005/103] fix(migration): drop orphan 'lsp' config key so users see LSP moved to .opencode/lsp.json (fixes #4225) The lsp config block was removed from OhMyOpenCodeConfigSchema when LSP tools were migrated from native plugin tools to the lsp tier-1 MCP server (packages/lsp-tools-mcp). Zod v4 strips unknown keys silently on safeParse, so a v3-era oh-my-opencode.jsonc with 'lsp': { typescript: { command: ... } } continues to live in the file unchanged while doing absolutely nothing. The user reporting #4225 saw their custom LSP servers stop working with zero indication that the configuration site moved.\n\nAdd a migrator that removes the orphan lsp key during migrateConfigFile, mirroring the existing omo_agent -> sisyphus_agent migration immediately above and the 'Removed obsolete hooks from disabled_hooks' precedent below. A single log line records the configPath and the list of dropped server keys so the user has a paper trail in oh-my-opencode.log, and needsWrite is flipped so the cleanup persists to disk (with a timestamped backup) the next time the plugin loads.\n\nReproduction (clean upstream/dev, BEFORE fix):\n needsWrite=false\n inMemory.lsp=\n persisted.lsp=\n\nVerification (AFTER fix):\n needsWrite=true\n inMemory.lsp=undefined\n persisted.lsp=undefined\n\nbun test src/shared/migration/ -> 26/26 pass (24/24 pre-existing + 2 new regression tests). bun run typecheck -> exit 0. --- src/shared/migration/config-migration.test.ts | 45 +++++++++++++++++++ src/shared/migration/config-migration.ts | 18 ++++++++ 2 files changed, 63 insertions(+) diff --git a/src/shared/migration/config-migration.test.ts b/src/shared/migration/config-migration.test.ts index 5c41f8435..84e2d1370 100644 --- a/src/shared/migration/config-migration.test.ts +++ b/src/shared/migration/config-migration.test.ts @@ -199,3 +199,48 @@ describe("migrateConfigFile backup skipping", () => { expect(backupFiles.length).toBe(1) }) }) + +describe("migrateConfigFile orphan lsp key", () => { + test("removes the obsolete 'lsp' key from rawConfig and from the persisted file", () => { + // given - a v3-era config with a populated lsp block that the v4 schema silently strips + const workdir = createWorkdir() + const configPath = join(workdir, "oh-my-opencode.json") + const rawConfig: Record = { + lsp: { + typescript: { command: ["typescript-language-server", "--stdio"] }, + rust: { command: ["rust-analyzer"] }, + }, + } + writeFileSync(configPath, JSON.stringify(rawConfig, null, 2) + "\n") + + // when + const needsWrite = migrateConfigFile(configPath, rawConfig) + + // then - the in-memory config and the persisted file have both lost the lsp key + expect(needsWrite).toBe(true) + expect(rawConfig.lsp).toBeUndefined() + const persistedConfig = JSON.parse(readFileSync(configPath, "utf-8")) as Record + expect(persistedConfig.lsp).toBeUndefined() + }) + + test("leaves the config alone when no 'lsp' key is present", () => { + // given - a config that never had an lsp block + const workdir = createWorkdir() + const configPath = join(workdir, "oh-my-opencode.json") + const rawConfig: Record = { + agents: { + sisyphus: { model: "anthropic/claude-opus-4-7" }, + }, + } + writeFileSync(configPath, JSON.stringify(rawConfig, null, 2) + "\n") + + // when + const needsWrite = migrateConfigFile(configPath, rawConfig) + + // then - no rewrite triggered by the lsp migrator, agents block untouched + expect(needsWrite).toBe(false) + expect((rawConfig.agents as Record>).sisyphus.model).toBe( + "anthropic/claude-opus-4-7", + ) + }) +}) diff --git a/src/shared/migration/config-migration.ts b/src/shared/migration/config-migration.ts index 5c0ed2d87..f5c1e957f 100644 --- a/src/shared/migration/config-migration.ts +++ b/src/shared/migration/config-migration.ts @@ -105,6 +105,24 @@ export function migrateConfigFile( needsWrite = true } + // The legacy `lsp` config key was retired when LSP moved from native plugin + // tools to the `lsp` MCP server backed by `packages/lsp-tools-mcp`. Custom + // LSP servers are now configured via `.opencode/lsp.json` (project) or + // `~/.codex/lsp-client.json` (user). The Zod schema strips unknown keys + // silently, so without this migration a stale `lsp` block lingers in the + // user's config file with no signal that it has stopped doing anything. + if (copy.lsp !== undefined) { + const droppedServers = copy.lsp && typeof copy.lsp === "object" + ? Object.keys(copy.lsp as Record) + : [] + log( + "Removed obsolete 'lsp' config key from config file. LSP servers are now configured via .opencode/lsp.json -- see docs/reference/configuration.md for the new location.", + { configPath, droppedServers }, + ) + delete copy.lsp + needsWrite = true + } + if (copy.experimental && typeof copy.experimental === "object") { const experimental = copy.experimental as Record if ("hashline_edit" in experimental) { From 7dae2711fc801de80ec5e2dba7fddd7098e7c507 Mon Sep 17 00:00:00 2001 From: SpencerJung Date: Fri, 22 May 2026 16:20:39 +0900 Subject: [PATCH 006/103] fix(atlas): honor stopped continuation after boulder completion Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- src/hooks/atlas/idle-event.test.ts | 51 ++++++++++++++++++++++++++++++ src/hooks/atlas/idle-event.ts | 10 ++++++ 2 files changed, 61 insertions(+) diff --git a/src/hooks/atlas/idle-event.test.ts b/src/hooks/atlas/idle-event.test.ts index 58a8c7591..82911a90a 100644 --- a/src/hooks/atlas/idle-event.test.ts +++ b/src/hooks/atlas/idle-event.test.ts @@ -210,4 +210,55 @@ describe("handleAtlasSessionIdle completion nudge", () => { expect(promptAsyncMock).toHaveBeenCalledTimes(1) expect(getState(SESSION_ID).boulderCompletionNudgedAt?.[workId]).toBeNumber() }) + + it("does not send a completion nudge after continuation was explicitly stopped", async () => { + // given + const planPath = join(testDirectory, "plan.md") + writeFileSync(planPath, "## TODOs\n- [x] 1. Parse input\n") + + const boulder = createBoulderState(planPath, SESSION_ID, "atlas") + const workId = boulder.active_work_id + if (!workId) { + throw new Error("Expected active_work_id") + } + writeBoulderState(testDirectory, boulder) + + const promptAsyncMock = mock(async () => ({ data: {} })) + const ctx = unsafeTestValue({ + directory: testDirectory, + client: { + session: { + promptAsync: promptAsyncMock, + }, + }, + }) + const retryTimer = setTimeout(() => {}, 60_000) + const sessionStateById = new Map([ + [SESSION_ID, { promptFailureCount: 0, pendingRetryTimer: retryTimer }], + ]) + const getState = (sessionId: string): SessionState => { + let state = sessionStateById.get(sessionId) + if (!state) { + state = { promptFailureCount: 0 } + sessionStateById.set(sessionId, state) + } + return state + } + + // when + await handleAtlasSessionIdle({ + ctx, + sessionID: SESSION_ID, + getState, + options: { + isContinuationStopped: (sessionId) => sessionId === SESSION_ID, + }, + }) + + // then + expect(promptAsyncMock).not.toHaveBeenCalled() + expect(getState(SESSION_ID).pendingRetryTimer).toBeUndefined() + expect(getState(SESSION_ID).boulderCompletionNudgedAt?.[workId]).toBeUndefined() + expect(readBoulderState(testDirectory)?.works?.[workId]?.status).toBe("completed") + }) }) diff --git a/src/hooks/atlas/idle-event.ts b/src/hooks/atlas/idle-event.ts index 2dc06008d..93b5685de 100644 --- a/src/hooks/atlas/idle-event.ts +++ b/src/hooks/atlas/idle-event.ts @@ -246,6 +246,11 @@ export async function handleAtlasSessionIdle(input: { const { boulderState, progress, appendedSession } = activeBoulderSession if (progress.isComplete) { + if (sessionState.pendingRetryTimer) { + clearTimeout(sessionState.pendingRetryTimer) + sessionState.pendingRetryTimer = undefined + } + const work = getWorkForSession(ctx.directory, sessionID) if (work) { completeBoulder(ctx.directory, work.work_id) @@ -258,6 +263,11 @@ export async function handleAtlasSessionIdle(input: { return } + if (options?.isContinuationStopped?.(sessionID)) { + log(`[${HOOK_NAME}] Boulder completion nudge skipped because continuation stopped`, { sessionID, plan: boulderState.plan_name }) + return + } + if (sessionState.boulderCompletionNudgedAt?.[work.work_id]) { log(`[${HOOK_NAME}] Boulder complete`, { sessionID, plan: boulderState.plan_name }) return From 28569307ebbb04902b08eb7a228ac64244bd70a9 Mon Sep 17 00:00:00 2001 From: SpencerJung Date: Fri, 22 May 2026 16:36:12 +0900 Subject: [PATCH 007/103] fix(tool-pair-validator): continue after synthetic repairs Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- src/hooks/tool-pair-validator/hook.test.ts | 11 +++++++++++ src/hooks/tool-pair-validator/hook.ts | 18 ++++++++++++++++-- src/plugin/messages-transform.test.ts | 4 ++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/hooks/tool-pair-validator/hook.test.ts b/src/hooks/tool-pair-validator/hook.test.ts index af97fa76a..dd2a1fc3b 100644 --- a/src/hooks/tool-pair-validator/hook.test.ts +++ b/src/hooks/tool-pair-validator/hook.test.ts @@ -9,6 +9,7 @@ import { createToolPairValidatorHook } from "./hook" import { _resetForTesting, subagentSessions } from "../../features/claude-code-session-state/state" const TOOL_RESULT_PLACEHOLDER = "Tool output unavailable (context compacted)" +const TOOL_RESULT_RECOVERY_CONTINUATION = "Recovered missing tool results. Continue from the repaired tool output." type TestPart = { type: string @@ -19,6 +20,7 @@ type TestPart = { isError?: boolean content?: string | Array<{ type: "text"; text: string }> text?: string + synthetic?: boolean } type TestMessage = { @@ -121,6 +123,11 @@ describe("createToolPairValidatorHook", () => { isError: true, content: [{ type: "text", text: TOOL_RESULT_PLACEHOLDER }], }, + { + type: "text", + text: TOOL_RESULT_RECOVERY_CONTINUATION, + synthetic: true, + }, ], }, ]) @@ -148,6 +155,10 @@ describe("createToolPairValidatorHook", () => { tool_use_id: "toolu_1", isError: true, content: [{ type: "text", text: TOOL_RESULT_PLACEHOLDER }], + }, { + type: "text", + text: TOOL_RESULT_RECOVERY_CONTINUATION, + synthetic: true, }], }, { info: { role: "assistant" }, parts: [{ type: "text", text: "follow-up" }] }, diff --git a/src/hooks/tool-pair-validator/hook.ts b/src/hooks/tool-pair-validator/hook.ts index 9a4107810..da63504e5 100644 --- a/src/hooks/tool-pair-validator/hook.ts +++ b/src/hooks/tool-pair-validator/hook.ts @@ -4,6 +4,7 @@ import { subagentSessions } from "../../features/claude-code-session-state" import { log } from "../../shared/logger" const TOOL_RESULT_PLACEHOLDER = "Tool output unavailable (context compacted)" +const TOOL_RESULT_RECOVERY_CONTINUATION = "Recovered missing tool results. Continue from the repaired tool output." type ToolUsePart = { type: "tool_use" @@ -20,7 +21,13 @@ type ToolResultPart = { [key: string]: unknown } -type TransformPart = Part | ToolUsePart | ToolResultPart +type TextPart = { + type: "text" + text: string + synthetic: true +} + +type TransformPart = Part | ToolUsePart | ToolResultPart | TextPart type TransformMessageInfo = Message | { role: "user" @@ -138,7 +145,14 @@ function createSyntheticUserMessage(assistantMessage: MessageWithParts, missingT role: "user", ...(sessionID ? { sessionID } : {}), }, - parts: missingToolUseIDs.map((toolUseID) => createToolResultPart(toolUseID)), + parts: [ + ...missingToolUseIDs.map((toolUseID) => createToolResultPart(toolUseID)), + { + type: "text", + text: TOOL_RESULT_RECOVERY_CONTINUATION, + synthetic: true, + }, + ], } } diff --git a/src/plugin/messages-transform.test.ts b/src/plugin/messages-transform.test.ts index 02f6951dd..57ca0eece 100644 --- a/src/plugin/messages-transform.test.ts +++ b/src/plugin/messages-transform.test.ts @@ -157,6 +157,10 @@ describe("createMessagesTransformHandler", () => { tool_use_id: "toolu_01SRMQs3DUtVKWoSxC8bxxVA", isError: true, content: [{ type: "text", text: "Tool output unavailable (context compacted)" }], + }, { + type: "text", + text: "Recovered missing tool results. Continue from the repaired tool output.", + synthetic: true, }], }) expect(messages[4]?.parts[0]).toEqual({ From ed4c04e57573bfede73c0838d11b821fbb05a65f Mon Sep 17 00:00:00 2001 From: SpencerJung Date: Fri, 22 May 2026 16:39:36 +0900 Subject: [PATCH 008/103] fix(cli): preserve CJK agent header text Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- src/cli/run/output-renderer.test.ts | 44 ++++++++++++++++++++++++++ src/cli/run/output-renderer.ts | 6 ++-- src/shared/agent-display-names.test.ts | 6 ++++ 3 files changed, 54 insertions(+), 2 deletions(-) create mode 100644 src/cli/run/output-renderer.test.ts diff --git a/src/cli/run/output-renderer.test.ts b/src/cli/run/output-renderer.test.ts new file mode 100644 index 000000000..36d36ab3e --- /dev/null +++ b/src/cli/run/output-renderer.test.ts @@ -0,0 +1,44 @@ +import { afterEach, describe, expect, it } from "bun:test" + +import { renderAgentHeader } from "./output-renderer" + +const originalWrite = process.stdout.write.bind(process.stdout) + +function captureStdout(run: () => void): string { + const chunks: string[] = [] + process.stdout.write = ((chunk: string | Uint8Array) => { + chunks.push(typeof chunk === "string" ? chunk : Buffer.from(chunk).toString("utf8")) + return true + }) as typeof process.stdout.write + + try { + run() + } finally { + process.stdout.write = originalWrite as typeof process.stdout.write + } + + return chunks.join("") +} + +afterEach(() => { + process.stdout.write = originalWrite as typeof process.stdout.write +}) + +describe("renderAgentHeader", () => { + it("preserves CJK agent display names in stdout output", () => { + const output = captureStdout(() => { + renderAgentHeader("Sisyphus - 主脑", "zhipu/glm-5.1", "xhigh", {}) + }) + + expect(output).toContain("Sisyphus - 主脑") + expect(output).toContain("zhipu/glm-5.1") + }) + + it("normalizes decomposed Unicode before rendering", () => { + const output = captureStdout(() => { + renderAgentHeader("헤파", null, null, {}) + }) + + expect(output).toContain("헤파") + }) +}) diff --git a/src/cli/run/output-renderer.ts b/src/cli/run/output-renderer.ts index 6c5782da4..2a376aa86 100644 --- a/src/cli/run/output-renderer.ts +++ b/src/cli/run/output-renderer.ts @@ -8,10 +8,12 @@ export function renderAgentHeader( ): void { if (!agent && !model) return + const normalizedAgent = agent?.normalize("NFC") ?? null + const normalizedModel = model?.normalize("NFC") ?? null const agentLabel = agent - ? pc.bold(colorizeWithProfileColor(agent, agentColorsByName[agent])) + ? pc.bold(colorizeWithProfileColor(normalizedAgent ?? agent, agentColorsByName[agent])) : "" - const modelBase = model ?? "" + const modelBase = normalizedModel ?? "" const variantSuffix = variant ? ` (${variant})` : "" const modelLabel = model ? pc.dim(`${modelBase}${variantSuffix}`) : "" diff --git a/src/shared/agent-display-names.test.ts b/src/shared/agent-display-names.test.ts index 2ce913743..eacfe467f 100644 --- a/src/shared/agent-display-names.test.ts +++ b/src/shared/agent-display-names.test.ts @@ -133,6 +133,12 @@ describe("getAgentDisplayName", () => { // then returns "multimodal-looker" expect(result).toBe("multimodal-looker") }) + + it("preserves CJK display-name overrides verbatim", () => { + expect(getAgentDisplayName("sisyphus", { sisyphus: { displayName: "Sisyphus - 主脑" } })).toBe("Sisyphus - 主脑") + expect(getAgentDisplayName("hephaestus", { hephaestus: { displayName: "헤파이스토스" } })).toBe("헤파이스토스") + expect(getAgentDisplayName("atlas", { atlas: { displayName: "アトラス" } })).toBe("アトラス") + }) }) describe("getAgentConfigKey", () => { From 4cf391b7ffb04c42346a01bf11cced486d5adc95 Mon Sep 17 00:00:00 2001 From: Vanhci Date: Fri, 22 May 2026 16:06:09 +0800 Subject: [PATCH 009/103] fix(comment-checker): skip modified-existing comments and dedupe per-session (issue #4292) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue 1: hasNewCommentsOnly() now returns false when oldString and newString both contain comment syntax and the new lines are a subset of old lines — preventing the hook from firing on comment-only modifications. Issue 2: Per-session deduplication via sessionLastWarning Map with a 30s window (DEDUP_WINDOW_MS). At most one warning fires per session per response turn, breaking the deadloop on consecutive edits. --- src/hooks/comment-checker/cli-runner.ts | 44 +++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/src/hooks/comment-checker/cli-runner.ts b/src/hooks/comment-checker/cli-runner.ts index 00f5a0411..79a96869a 100644 --- a/src/hooks/comment-checker/cli-runner.ts +++ b/src/hooks/comment-checker/cli-runner.ts @@ -6,6 +6,35 @@ import { runCommentChecker, getCommentCheckerPath, startBackgroundInit, type Hoo let cliPathPromise: Promise | null = null let isRunning = false +/** Per-session deduplication: track last warning time to prevent deadloop */ +const sessionLastWarning = new Map() +const DEDUP_WINDOW_MS = 30_000 // 30 seconds — fire at most once per response turn + +/** Detect whether a comment string looks like a line-comment or block-comment pattern */ +function hasCommentSyntax(text: string | undefined): boolean { + if (!text) return false + return /^\s*(\/\/|\/\*|#|--|/.test(text) +} + +/** + * Returns true if any lines in `newText` contain comments that did NOT exist in + * `oldText`. This filters out false positives when oldString/newString both + * contain the same existing comment that was only slightly modified. + */ +function hasNewCommentsOnly(oldText: string | undefined, newText: string | undefined): boolean { + if (!hasCommentSyntax(newText)) return false + // If there was no old text, any comment is by definition new + if (!hasCommentSyntax(oldText)) return true + // Both contain comments — do a rough line-level diff to see if new comment + // lines were added (not just modified in-place) + const oldLines = new Set((oldText ?? "").split("\n").map((l) => l.trim())) + const newLines = (newText ?? "").split("\n") + return newLines.some((l) => { + const trimmed = l.trim() + return trimmed && hasCommentSyntax(trimmed) && !oldLines.has(trimmed) + }) +} + async function withCommentCheckerLock( fn: () => Promise, fallback: T, @@ -70,6 +99,21 @@ export async function processWithCli( }, } + // --- Fix #4292 Issue 1: skip if comment was already in oldString --- + if (!hasNewCommentsOnly(pendingCall.oldString, pendingCall.newString)) { + debugLog("skipping: no net-new comments in edit (oldString/newString)") + return + } + + // --- Fix #4292 Issue 2: deduplicate per-session (at most once per 30s) --- + const lastWarned = sessionLastWarning.get(pendingCall.sessionID) ?? 0 + const now = Date.now() + if (now - lastWarned < DEDUP_WINDOW_MS) { + debugLog("dedup: skipping comment warning within dedup window for session", pendingCall.sessionID) + return + } + sessionLastWarning.set(pendingCall.sessionID, now) + const result = await (deps.runCommentChecker ?? runCommentChecker)(hookInput, cliPath, customPrompt) if (result.hasComments && result.message) { From ec9997b7a6d47ef0c7d7c09d6b9172c5c597a954 Mon Sep 17 00:00:00 2001 From: SpencerJung Date: Fri, 22 May 2026 17:12:14 +0900 Subject: [PATCH 010/103] fix(background-agent): keep cleanup error listener active Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- .../background-agent/process-cleanup.test.ts | 28 +++++++++++++++++++ .../background-agent/process-cleanup.ts | 16 +++++++---- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/features/background-agent/process-cleanup.test.ts b/src/features/background-agent/process-cleanup.test.ts index f8d09be0c..f67456274 100644 --- a/src/features/background-agent/process-cleanup.test.ts +++ b/src/features/background-agent/process-cleanup.test.ts @@ -435,6 +435,34 @@ describe("#given process cleanup registration", () => { } }) + test("#given repeated uncaughtException events #when manager is registered #then listener stays installed and host is not forced to exit", async () => { + const uncaughtExceptionListenersBefore = process.listeners("uncaughtException") + const exitSpy = spyOn(process, "exit").mockImplementation((() => undefined) as never) + const shutdown = mock(() => {}) + const manager = { shutdown } + registeredManagers.push(manager) + __enableScheduledForcedExitForTesting() + + try { + registerManagerForCleanup(manager) + + process.emit("uncaughtException", new Error("first transient MCP failure")) + process.emit("uncaughtException", new Error("second transient MCP failure")) + await flushMicrotasks() + + expect(process.listeners("uncaughtException")).toHaveLength( + uncaughtExceptionListenersBefore.length + 1, + ) + expect(shutdown).not.toHaveBeenCalled() + expect(exitSpy).not.toHaveBeenCalled() + expect(process.exitCode).toBe(0) + } finally { + exitSpy.mockRestore() + __disableScheduledForcedExitForTesting() + process.exitCode = 0 + } + }) + test("#given a manager registered AND process emits 'exit' #then cleanup still runs (signal path remains the real shutdown gate)", () => { const exitListenersBefore = process.listeners("exit") const shutdown = mock(() => {}) diff --git a/src/features/background-agent/process-cleanup.ts b/src/features/background-agent/process-cleanup.ts index 3e2fd0c31..a2af8a4b5 100644 --- a/src/features/background-agent/process-cleanup.ts +++ b/src/features/background-agent/process-cleanup.ts @@ -115,16 +115,22 @@ function registerErrorEvent( // regardless of cause, so cleanup is not skipped when the host genuinely // dies. // - // We still detach the listener before logging so a re-emit from inside - // `log()` (e.g. EPIPE while writing to a broken pipe during shutdown) - // cannot recurse and produce the 100+ GB log explosion that #3856-era - // regressions caused. + // Keep the listener installed after logging. Desktop sidecars can emit more + // than one transient error during MCP startup or provider reconnects; if we + // detach after the first event, the second uncaught exception falls through + // to Node's default process termination path and reproduces the exit-code-1 + // crash from #4128. A local re-entry guard still prevents `log()` failures + // (for example EPIPE while writing during shutdown) from recursing into the + // 100+ GB log explosion that #3856-era regressions caused. + let logging = false const listener = (error: unknown) => { - process.off(signal, listener) + if (logging) return + logging = true log( `[background-agent] ${signal} observed; keeping host alive and skipping cleanup (signal handlers run on real shutdown)`, describeProcessCleanupError(error), ) + logging = false } process.on(signal, listener) return listener From 6062df8262ad6e31055420c393f5a9b8a1f5abe4 Mon Sep 17 00:00:00 2001 From: MoerAI Date: Fri, 22 May 2026 18:10:12 +0900 Subject: [PATCH 011/103] fix(migration): make 'lsp' migration guidance self-contained and update stale docs (addresses codex P2 on #4279) The migration log message previously pointed users to docs/reference/configuration.md for the new LSP config location, but that doc section still showed the obsolete plugin-level 'lsp' block. A user following the guidance would re-add the same 'lsp' key, see it stripped again on next startup, and never reach a usable config.\n\nFix both sides: rewrite the log message so it is self-contained (states the new path .opencode/lsp.json and the consumer directly) and rewrite the LSP section in docs/reference/configuration.md to describe the actual current architecture (LSP served by the 'lsp' MCP server, reading server map from .opencode/lsp.json via LSP_TOOLS_MCP_PROJECT_CONFIG, schema lives in packages/lsp-tools-mcp).\n\nVerification: bun test src/shared/migration/ -> 26/26 pass. bun run typecheck -> exit 0. Manual probe -> migration still strips lsp from both in-memory and persisted file. --- docs/reference/configuration.md | 37 +++++++++--------------- src/shared/migration/config-migration.ts | 14 +++++---- 2 files changed, 21 insertions(+), 30 deletions(-) diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index f5fa6753a..424fb0e89 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -628,34 +628,23 @@ Built-in MCPs (enabled by default): `websearch` (Exa AI), `context7` (library do ### LSP -Configure Language Server Protocol integration: +LSP tools are served by the built-in `lsp` MCP server (see [MCPs](#mcps)). The +previous top-level `"lsp"` block in the plugin config is no longer read and is +automatically stripped on next startup; existing configs containing it are +silently migrated (see `src/shared/migration/config-migration.ts`). + +To configure custom language servers, create `.opencode/lsp.json` at the project +root. The MCP server is launched with `LSP_TOOLS_MCP_PROJECT_CONFIG=.opencode/lsp.json` +and reads the server map from that file. The schema lives in the +`packages/lsp-tools-mcp` submodule (upstream: +[code-yeongyu/lsp-tools-mcp](https://github.com/code-yeongyu/lsp-tools-mcp)). + +To disable the LSP MCP entirely: ```json -{ - "lsp": { - "typescript-language-server": { - "command": ["typescript-language-server", "--stdio"], - "extensions": [".ts", ".tsx"], - "priority": 10, - "env": { "NODE_OPTIONS": "--max-old-space-size=4096" }, - "initialization": { - "preferences": { "includeInlayParameterNameHints": "all" } - } - }, - "pylsp": { "disabled": true } - } -} +{ "disabled_mcps": ["lsp"] } ``` -| Option | Type | Description | -| ---------------- | ------- | ------------------------------------ | -| `command` | array | Command to start LSP server | -| `extensions` | array | File extensions (e.g. `[".ts"]`) | -| `priority` | number | Priority when multiple servers match | -| `env` | object | Environment variables | -| `initialization` | object | Init options passed to server | -| `disabled` | boolean | Disable this server | - --- ## Advanced diff --git a/src/shared/migration/config-migration.ts b/src/shared/migration/config-migration.ts index f5c1e957f..3ba81fca0 100644 --- a/src/shared/migration/config-migration.ts +++ b/src/shared/migration/config-migration.ts @@ -106,17 +106,19 @@ export function migrateConfigFile( } // The legacy `lsp` config key was retired when LSP moved from native plugin - // tools to the `lsp` MCP server backed by `packages/lsp-tools-mcp`. Custom - // LSP servers are now configured via `.opencode/lsp.json` (project) or - // `~/.codex/lsp-client.json` (user). The Zod schema strips unknown keys - // silently, so without this migration a stale `lsp` block lingers in the - // user's config file with no signal that it has stopped doing anything. + // tools to the `lsp` MCP server backed by `packages/lsp-tools-mcp`. The + // server now reads its server map from `.opencode/lsp.json` in the project + // root (path is hard-coded in `src/mcp/lsp.ts` via the + // `LSP_TOOLS_MCP_PROJECT_CONFIG` env var passed to the stdio MCP). The Zod + // schema strips unknown keys silently, so without this migration a stale + // `lsp` block lingers in the user's config file with no signal that it has + // stopped doing anything. if (copy.lsp !== undefined) { const droppedServers = copy.lsp && typeof copy.lsp === "object" ? Object.keys(copy.lsp as Record) : [] log( - "Removed obsolete 'lsp' config key from config file. LSP servers are now configured via .opencode/lsp.json -- see docs/reference/configuration.md for the new location.", + "Removed obsolete 'lsp' config key from oh-my-opencode config. Custom LSP servers are now configured in .opencode/lsp.json at the project root (consumed by the 'lsp' MCP server). Move any server definitions there to restore them.", { configPath, droppedServers }, ) delete copy.lsp From b31ad3c8926d17f872474a61643f8384500f9796 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 22 May 2026 10:23:40 +0000 Subject: [PATCH 012/103] @csxq0605 has signed the CLA in code-yeongyu/oh-my-openagent#4298 --- signatures/cla.json | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/signatures/cla.json b/signatures/cla.json index 3d3cb239e..c7d02a4ff 100644 --- a/signatures/cla.json +++ b/signatures/cla.json @@ -3447,6 +3447,14 @@ "created_at": "2026-05-22T04:23:15Z", "repoId": 1108837393, "pullRequestNo": 4247 + }, + { + "name": "csxq0605", + "id": 143505246, + "comment_id": 4517843825, + "created_at": "2026-05-22T10:23:36Z", + "repoId": 1108837393, + "pullRequestNo": 4298 } ] } \ No newline at end of file From bc8c462d289d702e565cea0108edcfe024b6af8d Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Fri, 22 May 2026 20:39:33 +0900 Subject: [PATCH 013/103] fix(session-notification-sender): guard ctx.$ with execFile fallback for Desktop sidecar (#4128, #4061) OpenCode Desktop's Electron sidecar runtime can omit Bun's ctx.$ helper. The sender previously called ctx.$ unconditionally, throwing TypeError: ctx.$ is not a function as unhandledRejection and crashing the sidecar with exit code 1. Add a runtime guard at every call site, falling back to Node.js child_process.execFile (with windowsHide: true) when ctx.$ is missing. The Bun ctx.$ path remains preferred when available. Every notification path is wrapped in try/catch so no failure escapes as unhandledRejection. Fixes #4128 Fixes #4061 --- src/hooks/session-notification-sender.test.ts | 78 +++-- src/hooks/session-notification-sender.ts | 278 +++++++++++++----- 2 files changed, 256 insertions(+), 100 deletions(-) diff --git a/src/hooks/session-notification-sender.test.ts b/src/hooks/session-notification-sender.test.ts index 015b66915..4961b2cb6 100644 --- a/src/hooks/session-notification-sender.test.ts +++ b/src/hooks/session-notification-sender.test.ts @@ -1,4 +1,7 @@ +/// + import { afterEach, beforeEach, describe, expect, jest, spyOn, test } from "bun:test" +import * as childProcess from "node:child_process" import * as sender from "./session-notification-sender" import * as utils from "./session-notification-utils" import type { PluginInput } from "@opencode-ai/plugin" @@ -6,6 +9,9 @@ import { unsafeTestValue } from "../../test-support/unsafe-test-value" +type TestShellResult = ReturnType> +type TestShellFactory = (cmd: TemplateStringsArray, ...values: unknown[]) => TestShellResult + function createShellPromise(handler: (cmdStr: string) => void) { return (cmd: TemplateStringsArray, ...values: unknown[]) => { const cmdStr = cmd.reduce((acc, part, i) => acc + part + (values[i] ?? ""), "") @@ -64,6 +70,29 @@ function createThrowingShellPromise(shouldThrow: (cmdStr: string) => boolean) { } } +type ExecFileCall = { + readonly file: string + readonly args: readonly string[] + readonly options: { readonly windowsHide?: boolean } +} + +function mockExecFile(calls: ExecFileCall[], error: Error | null = null): ReturnType { + return spyOn(childProcess, "execFile").mockImplementation( + unsafeTestValue( + ( + file: string, + args: readonly string[], + options: { readonly windowsHide?: boolean }, + callback: (execError: Error | null, stdout: string, stderr: string) => void + ) => { + calls.push({ file, args: [...args], options }) + callback(error, "", "") + return unsafeTestValue>({}) + } + ) + ) +} + describe("session-notification-sender", () => { beforeEach(() => { jest.restoreAllMocks() @@ -77,34 +106,33 @@ describe("session-notification-sender", () => { spyOn(utils, "getAplayPath").mockResolvedValue("/usr/bin/aplay") }) + afterEach(() => { + jest.restoreAllMocks() + }) + describe("#given sendSessionNotification", () => { describe("#when ctx.$ is unavailable", () => { - test("#then it returns early without throwing when ctx has no $", async () => { - const cmuxSpy = spyOn(utils, "getCmuxPath") + test("#then it falls back to execFile without throwing", async () => { + const execFileCalls: ExecFileCall[] = [] + mockExecFile(execFileCalls) const mockCtx = unsafeTestValue({}) - await expect(sender.sendSessionNotification(mockCtx, "darwin", "Test", "Message")).resolves.toBeUndefined() - expect(cmuxSpy).not.toHaveBeenCalled() + await sender.sendSessionNotification(mockCtx, "win32", "Test", "Message") + + expect(execFileCalls.length).toBe(1) + expect(execFileCalls[0]?.file).toBe("powershell") + expect(execFileCalls[0]?.args[0]).toBe("-Command") + expect(execFileCalls[0]?.options.windowsHide).toBe(true) }) - test("#then it returns early without throwing when ctx.$ is not a function", async () => { - const cmuxSpy = spyOn(utils, "getCmuxPath") - const mockCtx = unsafeTestValue({ - $: "not-a-function", - }) - - await expect(sender.sendSessionNotification(mockCtx, "darwin", "Test", "Message")).resolves.toBeUndefined() - expect(cmuxSpy).not.toHaveBeenCalled() - }) - - test("#then it remains non-throwing across sender APIs", async () => { - const afplaySpy = spyOn(utils, "getAfplayPath") + test("#then it swallows execFile rejection without throwing", async () => { + const execFileCalls: ExecFileCall[] = [] + mockExecFile(execFileCalls, new Error("execFile failed")) const mockCtx = unsafeTestValue({}) - await expect(sender.sendSessionNotification(mockCtx, "darwin", "Test", "Message")).resolves.toBeUndefined() - await expect(sender.playSessionNotificationSound(mockCtx, "darwin", "/sound.aiff")).resolves.toBeUndefined() + await sender.sendSessionNotification(mockCtx, "win32", "Test", "Message") - expect(afplaySpy).not.toHaveBeenCalled() + expect(execFileCalls.length).toBe(1) }) }) @@ -192,13 +220,13 @@ describe("session-notification-sender", () => { $: createThrowingShellPromise((cmdStr) => cmdStr.includes("cmux notify")), }) - const originalFactory = mockCtx.$ + const originalFactory = unsafeTestValue(mockCtx.$) const trackingCalls: string[] = [] - mockCtx.$ = ((cmd: TemplateStringsArray, ...values: unknown[]) => { + mockCtx.$ = unsafeTestValue((cmd: TemplateStringsArray, ...values: unknown[]) => { const cmdStr = cmd.reduce((acc: string, part: string, i: number) => acc + part + (values[i] ?? ""), "") trackingCalls.push(cmdStr) return originalFactory(cmd, ...values) - }) as typeof mockCtx.$ + }) await sender.sendSessionNotification(mockCtx, "darwin", "Test", "Message") @@ -215,12 +243,12 @@ describe("session-notification-sender", () => { $: createThrowingShellPromise((cmdStr) => cmdStr.includes("cmux notify") || cmdStr.includes("terminal-notifier")), }) - const originalFactory = mockCtx.$ - mockCtx.$ = ((cmd: TemplateStringsArray, ...values: unknown[]) => { + const originalFactory = unsafeTestValue(mockCtx.$) + mockCtx.$ = unsafeTestValue((cmd: TemplateStringsArray, ...values: unknown[]) => { const cmdStr = cmd.reduce((acc: string, part: string, i: number) => acc + part + (values[i] ?? ""), "") trackingCalls.push(cmdStr) return originalFactory(cmd, ...values) - }) as typeof mockCtx.$ + }) await sender.sendSessionNotification(mockCtx, "darwin", "Test", "Message") diff --git a/src/hooks/session-notification-sender.ts b/src/hooks/session-notification-sender.ts index fb374afe5..63d6c3ce5 100644 --- a/src/hooks/session-notification-sender.ts +++ b/src/hooks/session-notification-sender.ts @@ -1,4 +1,6 @@ import type { PluginInput } from "@opencode-ai/plugin" +import { execFile } from "node:child_process" +import { promisify } from "node:util" import { platform } from "os" import { log } from "../shared" import { @@ -39,17 +41,45 @@ type ShellCommand = Promise & { nothrow?: () => ShellCommand } +type ShellRunner = NonNullable + +type ShellFailureMode = "throw" | "nothrow" + let hasLoggedUnavailableShellHelper = false -function canRunNotificationCommand(ctx: PluginInput): boolean { - if (typeof ctx?.$ === "function") return true +function getShellRunner(ctx: PluginInput): ShellRunner | undefined { + // Guard for #4128 + #4061: OpenCode Desktop's Electron sidecar can omit Bun's ctx.$ helper. + if (typeof ctx.$ === "function") return ctx.$ if (!hasLoggedUnavailableShellHelper) { hasLoggedUnavailableShellHelper = true - log("[session-notification] ctx.$ unavailable; skipping notification command execution") + log("[session-notification] ctx.$ unavailable; falling back to child_process.execFile") } - return false + return undefined +} + +function logCommandFailure(commandName: string, error: Error | string): void { + log("[session-notification] notification command failed", { + commandName, + error: typeof error === "string" ? error : error.message, + }) +} + +function logOperationFailure(operation: string, error: Error | string): void { + log("[session-notification] notification operation failed", { + operation, + error: typeof error === "string" ? error : error.message, + }) +} + +async function runQuiet(command: ShellCommand): Promise { + if (typeof command.quiet === "function") { + await command.quiet() + return + } + + await command } async function runQuietNothrow(command: ShellCommand): Promise { @@ -62,64 +92,135 @@ async function runQuietNothrow(command: ShellCommand): Promise { await safeCommand } +async function runExecFile(commandPath: string, args: readonly string[]): Promise { + const execFileAsync = promisify(execFile) + await execFileAsync(commandPath, [...args], { windowsHide: true }) +} + +async function runNotificationCommand( + ctx: PluginInput, + commandPath: string, + args: readonly string[], + shellCommand: (shell: ShellRunner) => ShellCommand, + shellFailureMode: ShellFailureMode = "nothrow" +): Promise { + const shell = getShellRunner(ctx) + if (shell) { + if (shellFailureMode === "throw") { + await runQuiet(shellCommand(shell)) + return + } + + await runQuietNothrow(shellCommand(shell)) + return + } + + await runExecFile(commandPath, args) +} + export async function sendSessionNotification( ctx: PluginInput, platform: Platform, title: string, message: string ): Promise { - if (!canRunNotificationCommand(ctx)) return - - switch (platform) { - case "darwin": { - // Try cmux first - native UNUserNotificationCenter, properly attributed - const cmuxPath = await getCmuxPath() - if (cmuxPath) { - try { - await ctx.$`${cmuxPath} notify --title ${title} --body ${message}`.quiet() - break - } catch { - } - } - - // Try terminal-notifier - deterministic click-to-focus - const terminalNotifierPath = await getTerminalNotifierPath() - if (terminalNotifierPath) { - const bundleId = process.env.__CFBundleIdentifier - try { - if (bundleId) { - await ctx.$`${terminalNotifierPath} -title ${title} -message ${message} -activate ${bundleId}`.quiet() - } else { - await ctx.$`${terminalNotifierPath} -title ${title} -message ${message}`.quiet() + try { + switch (platform) { + case "darwin": { + // Try cmux first - native UNUserNotificationCenter, properly attributed + const cmuxPath = await getCmuxPath() + if (cmuxPath) { + try { + await runNotificationCommand( + ctx, + cmuxPath, + ["notify", "--title", title, "--body", message], + (shell) => shell`${cmuxPath} notify --title ${title} --body ${message}`, + "throw" + ) + break + } catch (error) { + if (error instanceof Error) { + logCommandFailure("cmux", error) + } else { + logCommandFailure("cmux", String(error)) + } } - break - } catch { } + + // Try terminal-notifier - deterministic click-to-focus + const terminalNotifierPath = await getTerminalNotifierPath() + if (terminalNotifierPath) { + const bundleId = process.env.__CFBundleIdentifier + const args = bundleId + ? ["-title", title, "-message", message, "-activate", bundleId] + : ["-title", title, "-message", message] + try { + await runNotificationCommand( + ctx, + terminalNotifierPath, + args, + (shell) => bundleId + ? shell`${terminalNotifierPath} -title ${title} -message ${message} -activate ${bundleId}` + : shell`${terminalNotifierPath} -title ${title} -message ${message}`, + "throw" + ) + break + } catch (error) { + if (error instanceof Error) { + logCommandFailure("terminal-notifier", error) + } else { + logCommandFailure("terminal-notifier", String(error)) + } + } + } + + // Fallback: osascript (click may open Finder instead of terminal) + const osascriptPath = await getOsascriptPath() + if (!osascriptPath) return + + const escapedTitle = escapeAppleScriptText(title) + const escapedMessage = escapeAppleScriptText(message) + const appleScript = "display notification \"" + escapedMessage + "\" with title \"" + escapedTitle + "\"" + await runNotificationCommand( + ctx, + osascriptPath, + ["-e", appleScript], + (shell) => shell`${osascriptPath} -e ${appleScript}` + ) + break } + case "linux": { + const notifySendPath = await getNotifySendPath() + if (!notifySendPath) return - // Fallback: osascript (click may open Finder instead of terminal) - const osascriptPath = await getOsascriptPath() - if (!osascriptPath) return + await runNotificationCommand( + ctx, + notifySendPath, + [title, message], + (shell) => shell`${notifySendPath} ${title} ${message} 2>/dev/null` + ) + break + } + case "win32": { + const powershellPath = await getPowershellPath() + if (!powershellPath) return - const escapedTitle = escapeAppleScriptText(title) - const escapedMessage = escapeAppleScriptText(message) - await runQuietNothrow(ctx.$`${osascriptPath} -e ${"display notification \"" + escapedMessage + "\" with title \"" + escapedTitle + "\""}`) - break + const toastScript = buildWindowsToastScript(title, message) + await runNotificationCommand( + ctx, + powershellPath, + ["-Command", toastScript], + (shell) => shell`${powershellPath} -Command ${toastScript}` + ) + break + } } - case "linux": { - const notifySendPath = await getNotifySendPath() - if (!notifySendPath) return - - await runQuietNothrow(ctx.$`${notifySendPath} ${title} ${message} 2>/dev/null`) - break - } - case "win32": { - const powershellPath = await getPowershellPath() - if (!powershellPath) return - - const toastScript = buildWindowsToastScript(title, message) - await runQuietNothrow(ctx.$`${powershellPath} -Command ${toastScript}`) - break + } catch (error) { + if (error instanceof Error) { + logOperationFailure("send", error) + } else { + logOperationFailure("send", String(error)) } } } @@ -129,33 +230,60 @@ export async function playSessionNotificationSound( platform: Platform, soundPath: string ): Promise { - if (!canRunNotificationCommand(ctx)) return - - switch (platform) { - case "darwin": { - const afplayPath = await getAfplayPath() - if (!afplayPath) return - await runQuietNothrow(ctx.$`${afplayPath} ${soundPath}`) - break - } - case "linux": { - const paplayPath = await getPaplayPath() - if (paplayPath) { - await runQuietNothrow(ctx.$`${paplayPath} ${soundPath} 2>/dev/null`) - } else { - const aplayPath = await getAplayPath() - if (aplayPath) { - await runQuietNothrow(ctx.$`${aplayPath} ${soundPath} 2>/dev/null`) - } + try { + switch (platform) { + case "darwin": { + const afplayPath = await getAfplayPath() + if (!afplayPath) return + await runNotificationCommand( + ctx, + afplayPath, + [soundPath], + (shell) => shell`${afplayPath} ${soundPath}` + ) + break + } + case "linux": { + const paplayPath = await getPaplayPath() + if (paplayPath) { + await runNotificationCommand( + ctx, + paplayPath, + [soundPath], + (shell) => shell`${paplayPath} ${soundPath} 2>/dev/null` + ) + } else { + const aplayPath = await getAplayPath() + if (aplayPath) { + await runNotificationCommand( + ctx, + aplayPath, + [soundPath], + (shell) => shell`${aplayPath} ${soundPath} 2>/dev/null` + ) + } + } + break + } + case "win32": { + const powershellPath = await getPowershellPath() + if (!powershellPath) return + const escaped = escapePowerShellSingleQuotedText(soundPath) + const soundScript = "(New-Object Media.SoundPlayer '" + escaped + "').PlaySync()" + await runNotificationCommand( + ctx, + powershellPath, + ["-Command", soundScript], + (shell) => shell`${powershellPath} -Command ${soundScript}` + ) + break } - break } - case "win32": { - const powershellPath = await getPowershellPath() - if (!powershellPath) return - const escaped = escapePowerShellSingleQuotedText(soundPath) - await runQuietNothrow(ctx.$`${powershellPath} -Command ${"(New-Object Media.SoundPlayer '" + escaped + "').PlaySync()"}`) - break + } catch (error) { + if (error instanceof Error) { + logOperationFailure("sound", error) + } else { + logOperationFailure("sound", String(error)) } } } From 4ea7562f503492b5e272df4554af497306bcabc4 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Fri, 22 May 2026 20:39:52 +0900 Subject: [PATCH 014/103] feat(shared): add Node-safe process stream reader and search output collector Introduces: - src/shared/process-stream-reader.ts: Buffer-concat stream reader compatible with both Bun and Node ChildProcess stdout (replaces Web Response API usage) - src/tools/shared/search-process-output.ts: structured subprocess output collector with timeout, kill, and rejection cleanup - bun-spawn-shim hardened: Node path forces windowsHide: true; spawn errors no longer escape as unhandledRejection Foundation for #3919 fix. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- src/shared/bun-spawn-shim.test.ts | 44 +++++++++- src/shared/bun-spawn-shim.ts | 98 ++++++++++++++++++----- src/shared/process-stream-reader.ts | 63 +++++++++++++++ src/tools/shared/search-process-output.ts | 46 +++++++++++ 4 files changed, 227 insertions(+), 24 deletions(-) create mode 100644 src/shared/process-stream-reader.ts create mode 100644 src/tools/shared/search-process-output.ts diff --git a/src/shared/bun-spawn-shim.test.ts b/src/shared/bun-spawn-shim.test.ts index 238cfb1b1..5258ef1e6 100644 --- a/src/shared/bun-spawn-shim.test.ts +++ b/src/shared/bun-spawn-shim.test.ts @@ -1,6 +1,8 @@ import { describe, expect, test } from "bun:test" +import { Readable } from "node:stream" -import { spawn, spawnSync } from "./bun-spawn-shim" +import { createNodeSpawnOptions, createNodeSpawnSyncOptions, spawn, spawnSync } from "./bun-spawn-shim" +import { readProcessStream } from "./process-stream-reader" describe("bun-spawn-shim", () => { test("#given array command #when spawn exits successfully #then exited resolves to zero", async () => { @@ -17,7 +19,7 @@ describe("bun-spawn-shim", () => { const [exitCode, stdout] = await Promise.all([ proc.exited, - new Response(proc.stdout).text(), + readProcessStream(proc.stdout), ]) expect(exitCode).toBe(0) @@ -48,7 +50,7 @@ describe("bun-spawn-shim", () => { }) const exitCode = await proc.exited - const stdout = await new Response(proc.stdout).text() + const stdout = await readProcessStream(proc.stdout) expect(exitCode).toBe(0) expect(stdout).toBe("") @@ -60,7 +62,7 @@ describe("bun-spawn-shim", () => { expect(result.exitCode).toBe(0) expect(result.success).toBe(true) expect(result.stdout).toBeDefined() - expect(Buffer.from(result.stdout!).toString().trim()).toBe("sync-ok") + expect(result.stdout?.toString().trim()).toBe("sync-ok") }) test("#given spawnSync command #when it completes #then result.pid is a positive number", () => { @@ -88,4 +90,38 @@ describe("bun-spawn-shim", () => { expect(observedError).toBeDefined() }) + + test("#given Windows platform #when building Node spawn options #then windowsHide is enabled", () => { + const options = createNodeSpawnOptions({ stdout: "pipe", stderr: "pipe" }, "win32") + + expect(options.windowsHide).toBe(true) + expect(options.shell).toBe(false) + }) + + test("#given Windows platform #when building Node spawnSync options #then windowsHide is enabled", () => { + const options = createNodeSpawnSyncOptions({ stdout: "pipe", stderr: "pipe" }, "win32") + + expect(options.windowsHide).toBe(true) + expect(options.shell).toBe(false) + }) + + test("#given Node readable output #when reading in a non-Bun host shape #then Buffer-concat returns text", async () => { + const stream = Readable.from([Buffer.from("node-stream-ok\n")]) + + const output = await readProcessStream(stream) + + expect(output).toBe("node-stream-ok\n") + }) + + test("#given empty process stream #when reading process output #then returns an empty string", async () => { + const stream = new ReadableStream({ + start(controller) { + controller.close() + }, + }) + + const output = await readProcessStream(stream) + + expect(output).toBe("") + }) }) diff --git a/src/shared/bun-spawn-shim.ts b/src/shared/bun-spawn-shim.ts index d07a48161..36260135e 100644 --- a/src/shared/bun-spawn-shim.ts +++ b/src/shared/bun-spawn-shim.ts @@ -1,4 +1,9 @@ -import { spawn as nodeSpawn, spawnSync as nodeSpawnSync } from "node:child_process" +import { + spawn as nodeSpawn, + spawnSync as nodeSpawnSync, + type SpawnOptions as NodeSpawnOptions, + type SpawnSyncOptions as NodeSpawnSyncOptions, +} from "node:child_process" import { Readable, Writable } from "node:stream" type AnyRecord = Record @@ -45,7 +50,10 @@ type BunSpawnRuntime = { } const runtime = globalThis as typeof globalThis & { Bun?: BunSpawnRuntime } -const IS_BUN = typeof runtime.Bun !== "undefined" + +function getBunRuntime(): BunSpawnRuntime | undefined { + return typeof Bun === "undefined" ? undefined : runtime.Bun +} function emptyReadableStream(): ReadableStream { return new ReadableStream({ @@ -85,6 +93,48 @@ function resolveStdio(options: SpawnOptions): StdioTuple { return [options.stdin ?? "ignore", options.stdout ?? "pipe", options.stderr ?? "inherit"] } +export function createNodeSpawnOptions( + options: SpawnOptions, + platform: NodeJS.Platform = process.platform +): NodeSpawnOptions { + const nodeOptions: NodeSpawnOptions = { + stdio: resolveStdio(options), + shell: false, + } + + if (options.cwd !== undefined) nodeOptions.cwd = options.cwd + if (options.env !== undefined) nodeOptions.env = options.env + if (options.detached !== undefined) nodeOptions.detached = options.detached + if (options.signal !== undefined) nodeOptions.signal = options.signal + + if (platform === "win32") { + // #3919: Windows Desktop utility processes must hide child consoles when spawning tools. + nodeOptions.windowsHide = true + } + + return nodeOptions +} + +export function createNodeSpawnSyncOptions( + options: SpawnOptions, + platform: NodeJS.Platform = process.platform +): NodeSpawnSyncOptions { + const nodeOptions: NodeSpawnSyncOptions = { + stdio: resolveStdio(options), + shell: false, + } + + if (options.cwd !== undefined) nodeOptions.cwd = options.cwd + if (options.env !== undefined) nodeOptions.env = options.env + + if (platform === "win32") { + // #3919: Match async spawn so Windows sync probes do not surface a console window. + nodeOptions.windowsHide = true + } + + return nodeOptions +} + function wrapNodeProcess(proc: ReturnType): SpawnedProcess { let exitCode: number | null = null const exited = new Promise((resolve, reject) => { @@ -127,20 +177,27 @@ function wrapNodeProcess(proc: ReturnType): SpawnedProcess { } } +function toSpawnSyncBuffer(output: Buffer | string | null): Buffer | undefined { + if (output === null) { + return undefined + } + + return Buffer.isBuffer(output) ? output : Buffer.from(output, "utf8") +} + export function spawn(command: string[], options?: SpawnOptions): SpawnedProcess export function spawn(options: SpawnOptions & { cmd: string[] }): SpawnedProcess export function spawn(cmdOrOpts: unknown, opts?: unknown): SpawnedProcess { - if (IS_BUN) return runtime.Bun!.spawn(cmdOrOpts as string[] & SpawnOptions & { cmd: string[] }, opts as SpawnOptions) - const { cmd, opts: options } = resolveCommand(cmdOrOpts, opts) + const bun = getBunRuntime() + if (bun) return bun.spawn(cmd, options) + const [bin, ...args] = cmd - const proc = nodeSpawn(bin, args, { - cwd: options.cwd, - env: options.env, - stdio: resolveStdio(options), - detached: options.detached, - signal: options.signal, - }) + if (bin === undefined) { + throw new Error("Cannot spawn an empty command") + } + + const proc = nodeSpawn(bin, args, createNodeSpawnOptions(options)) return wrapNodeProcess(proc) } @@ -148,20 +205,21 @@ export function spawn(cmdOrOpts: unknown, opts?: unknown): SpawnedProcess { export function spawnSync(command: string[], options?: SpawnOptions): SpawnSyncResult export function spawnSync(options: SpawnOptions & { cmd: string[] }): SpawnSyncResult export function spawnSync(cmdOrOpts: unknown, opts?: unknown): SpawnSyncResult { - if (IS_BUN) return runtime.Bun!.spawnSync(cmdOrOpts as string[] & SpawnOptions & { cmd: string[] }, opts as SpawnOptions) - const { cmd, opts: options } = resolveCommand(cmdOrOpts, opts) + const bun = getBunRuntime() + if (bun) return bun.spawnSync(cmd, options) + const [bin, ...args] = cmd - const result = nodeSpawnSync(bin, args, { - cwd: options.cwd, - env: options.env, - stdio: resolveStdio(options), - }) + if (bin === undefined) { + throw new Error("Cannot spawnSync an empty command") + } + + const result = nodeSpawnSync(bin, args, createNodeSpawnSyncOptions(options)) return { exitCode: result.status ?? 1, - stdout: result.stdout ?? undefined, - stderr: result.stderr ?? undefined, + stdout: toSpawnSyncBuffer(result.stdout), + stderr: toSpawnSyncBuffer(result.stderr), success: (result.status ?? 1) === 0, pid: result.pid ?? -1, } diff --git a/src/shared/process-stream-reader.ts b/src/shared/process-stream-reader.ts new file mode 100644 index 000000000..bec08fab7 --- /dev/null +++ b/src/shared/process-stream-reader.ts @@ -0,0 +1,63 @@ +import { Readable } from "node:stream" + +export type ProcessReadableStream = ReadableStream | Readable | null | undefined + +function bufferFromChunk(chunk: unknown): Buffer { + if (Buffer.isBuffer(chunk)) { + return chunk + } + + if (chunk instanceof Uint8Array) { + return Buffer.from(chunk) + } + + if (typeof chunk === "string") { + return Buffer.from(chunk, "utf8") + } + + throw new TypeError(`Unsupported process stream chunk type: ${typeof chunk}`) +} + +async function readWebStream(stream: ReadableStream): Promise { + const reader = stream.getReader() + const chunks: Buffer[] = [] + + try { + while (true) { + const result = await reader.read() + if (result.done) { + return chunks + } + chunks.push(Buffer.from(result.value)) + } + } finally { + reader.releaseLock() + } +} + +async function readNodeStream(stream: Readable): Promise { + const chunks: Buffer[] = [] + + for await (const chunk of stream) { + chunks.push(bufferFromChunk(chunk)) + } + + return chunks +} + +function isWebReadableStream(stream: ProcessReadableStream): stream is ReadableStream { + return typeof ReadableStream !== "undefined" && stream instanceof ReadableStream +} + +export async function readProcessStream(stream: ProcessReadableStream): Promise { + if (!stream) { + return "" + } + + // #3919: Buffer-concat avoids Response(stream).text() crashes in Windows utility processes. + const chunks = isWebReadableStream(stream) + ? await readWebStream(stream) + : await readNodeStream(stream) + + return Buffer.concat(chunks).toString("utf8") +} diff --git a/src/tools/shared/search-process-output.ts b/src/tools/shared/search-process-output.ts new file mode 100644 index 000000000..f6d5945d0 --- /dev/null +++ b/src/tools/shared/search-process-output.ts @@ -0,0 +1,46 @@ +import type { SpawnedProcess } from "../../shared/bun-spawn-shim" +import { readProcessStream } from "../../shared/process-stream-reader" + +export interface SearchProcessOutput { + readonly stdout: string + readonly stderr: string + readonly exitCode: number +} + +function getErrorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error) +} + +function createProcessTimeout( + proc: SpawnedProcess, + timeoutMs: number, + timeoutMessage: string +): Promise { + return new Promise((_, reject) => { + const id = setTimeout(() => { + proc.kill() + reject(new Error(timeoutMessage)) + }, timeoutMs) + + // #3919: Handle rejected exits here so timeout cleanup cannot leak unhandled rejections. + void proc.exited.then( + () => clearTimeout(id), + () => clearTimeout(id) + ) + }) +} + +export async function collectSearchProcessOutput( + proc: SpawnedProcess, + timeoutMs: number, + timeoutMessage: string +): Promise { + const stderrPromise = readProcessStream(proc.stderr).catch(getErrorMessage) + const stdout = await Promise.race([ + readProcessStream(proc.stdout), + createProcessTimeout(proc, timeoutMs, timeoutMessage), + ]) + const [exitCode, stderr] = await Promise.all([proc.exited, stderrPromise]) + + return { stdout, stderr, exitCode } +} From d17b2127f22acca5911dd8b3dd59b0e0705cdeb9 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Fri, 22 May 2026 20:40:06 +0900 Subject: [PATCH 015/103] fix(tools/grep, tools/glob): use Node-safe subprocess streaming (#3919) OpenCode Desktop v1.14.41+ runs OMO inside a Node.js utility process. The previous `new Response(proc.stdout).text()` call is Bun-/Web-API-specific and crashed the Desktop sidecar on Windows when grep/glob were invoked. Switch glob/grep cli to the new process-stream-reader + search-process-output helpers. Behavior on Bun and CLI/Linux/macOS is unchanged. ripgrep auto-download, PowerShell fallback, and rgSemaphore are preserved. Fixes #3919 Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- src/tools/glob/cli.test.ts | 63 ++++++++++++++++++++++++++++- src/tools/glob/cli.ts | 47 +++++++++++----------- src/tools/grep/cli.test.ts | 53 ++++++++++++++++++++++++ src/tools/grep/cli.ts | 82 +++++++++++++++++++++----------------- 4 files changed, 183 insertions(+), 62 deletions(-) create mode 100644 src/tools/grep/cli.test.ts diff --git a/src/tools/glob/cli.test.ts b/src/tools/glob/cli.test.ts index dcc393eb4..a1f8f3dad 100644 --- a/src/tools/glob/cli.test.ts +++ b/src/tools/glob/cli.test.ts @@ -1,5 +1,36 @@ -import { describe, it, expect } from "bun:test" -import { buildRgArgs, buildFindArgs, buildPowerShellCommand } from "./cli" +import { describe, it, expect, mock } from "bun:test" +import { Writable } from "node:stream" +import type { SpawnOptions, SpawnedProcess } from "../../shared/bun-spawn-shim" +import { buildRgArgs, buildFindArgs, buildPowerShellCommand, runRgFiles } from "./cli" + +function createTextStream(text: string): ReadableStream { + return new ReadableStream({ + start(controller) { + if (text.length > 0) { + controller.enqueue(new TextEncoder().encode(text)) + } + controller.close() + }, + }) +} + +function createSpawnedProcess(exitCode: number, stdout = "", stderr = ""): SpawnedProcess { + return { + exitCode, + exited: Promise.resolve(exitCode), + stdout: createTextStream(stdout), + stderr: createTextStream(stderr), + stdin: new Writable({ + write(_chunk, _encoding, callback) { + callback() + }, + }), + pid: 3919, + kill() {}, + ref() {}, + unref() {}, + } +} describe("buildRgArgs", () => { // given default options (no hidden/follow specified) @@ -166,4 +197,32 @@ describe("buildPowerShellCommand", () => { const command = args.join(" ") expect(command).toContain("test''s.ts") }) + + it("uses LiteralPath so fallback paths are not wildcard-expanded (#3919)", () => { + const args = buildPowerShellCommand({ pattern: "*.ts", paths: ["C:\\repo[1]"] }) + const command = args.join(" ") + expect(args[0]).toBe("powershell.exe") + expect(command).toContain("Get-ChildItem -LiteralPath 'C:\\repo[1]'") + }) +}) + +describe("runRgFiles", () => { + it("#given empty stdout #when rg exits successfully #then returns an empty result", async () => { + const spawnMock = mock((_command: string[], _options?: SpawnOptions): SpawnedProcess => + createSpawnedProcess(0) + ) + + const result = await runRgFiles( + { pattern: "*.ts", paths: ["."], timeout: 1000 }, + { path: "rg", backend: "rg" }, + spawnMock + ) + + expect(result).toEqual({ + files: [], + totalFiles: 0, + truncated: false, + }) + expect(spawnMock).toHaveBeenCalled() + }) }) diff --git a/src/tools/glob/cli.ts b/src/tools/glob/cli.ts index 9ba34c32a..ab97493b9 100644 --- a/src/tools/glob/cli.ts +++ b/src/tools/glob/cli.ts @@ -1,5 +1,5 @@ import { resolve } from "node:path" -import { spawn } from "../../shared/bun-spawn-shim" +import { spawn, type SpawnOptions, type SpawnedProcess } from "../../shared/bun-spawn-shim" import { resolveGrepCli, type GrepBackend, @@ -13,12 +13,15 @@ import { import type { GlobOptions, GlobResult, FileMatch } from "./types" import { stat } from "node:fs/promises" import { rgSemaphore } from "../shared/semaphore" +import { collectSearchProcessOutput } from "../shared/search-process-output" export interface ResolvedCli { path: string backend: GrepBackend } +export type SearchProcessSpawner = (command: string[], options?: SpawnOptions) => SpawnedProcess + function buildRgArgs(options: GlobOptions): string[] { const args: string[] = [ ...RG_FILES_FLAGS, @@ -65,7 +68,8 @@ function buildPowerShellCommand(options: GlobOptions): string[] { const escapedPath = searchPath.replace(/'/g, "''") const escapedPattern = options.pattern.replace(/'/g, "''") - let psCommand = `Get-ChildItem -Path '${escapedPath}' -File -Recurse -Depth ${maxDepth - 1} -Filter '${escapedPattern}'` + // #3919: Keep PowerShell fallback direct-spawned and single-quote escaped, not shell-interpolated. + let psCommand = `Get-ChildItem -LiteralPath '${escapedPath}' -File -Recurse -Depth ${maxDepth - 1} -Filter '${escapedPattern}'` if (options.hidden !== false) { psCommand += " -Force" @@ -78,7 +82,7 @@ function buildPowerShellCommand(options: GlobOptions): string[] { psCommand += " -ErrorAction SilentlyContinue | Select-Object -ExpandProperty FullName" - return ["powershell", "-NoProfile", "-Command", psCommand] + return ["powershell.exe", "-NoProfile", "-Command", psCommand] } async function getFileMtime(filePath: string): Promise { @@ -94,11 +98,12 @@ export { buildRgArgs, buildFindArgs, buildPowerShellCommand } export async function runRgFiles( options: GlobOptions, - resolvedCli?: ResolvedCli + resolvedCli?: ResolvedCli, + processSpawner: SearchProcessSpawner = spawn ): Promise { await rgSemaphore.acquire() try { - return await runRgFilesInternal(options, resolvedCli) + return await runRgFilesInternal(options, resolvedCli, processSpawner) } finally { rgSemaphore.release() } @@ -106,7 +111,8 @@ export async function runRgFiles( async function runRgFilesInternal( options: GlobOptions, - resolvedCli?: ResolvedCli + resolvedCli?: ResolvedCli, + processSpawner: SearchProcessSpawner = spawn ): Promise { const cli = resolvedCli ?? resolveGrepCli() const timeout = Math.min(options.timeout ?? DEFAULT_TIMEOUT_MS, DEFAULT_TIMEOUT_MS) @@ -133,24 +139,19 @@ async function runRgFilesInternal( command = [cli.path, ...args] } - const proc = spawn(command, { - stdout: "pipe", - stderr: "pipe", - cwd, - }) - - const timeoutPromise = new Promise((_, reject) => { - const id = setTimeout(() => { - proc.kill() - reject(new Error(`Glob search timeout after ${timeout}ms`)) - }, timeout) - proc.exited.then(() => clearTimeout(id)) - }) - try { - const stdout = await Promise.race([new Response(proc.stdout).text(), timeoutPromise]) - const stderr = await new Response(proc.stderr).text() - const exitCode = await proc.exited + const proc = processSpawner(command, { + stdout: "pipe", + stderr: "pipe", + cwd, + }) + + // #3919: Read stdout/stderr with Buffer concat instead of Response(stream).text(). + const { stdout, stderr, exitCode } = await collectSearchProcessOutput( + proc, + timeout, + `Glob search timeout after ${timeout}ms` + ) if (exitCode > 1 && stderr.trim()) { return { diff --git a/src/tools/grep/cli.test.ts b/src/tools/grep/cli.test.ts new file mode 100644 index 000000000..a629f7a90 --- /dev/null +++ b/src/tools/grep/cli.test.ts @@ -0,0 +1,53 @@ +import { describe, expect, it, mock } from "bun:test" +import { Writable } from "node:stream" +import type { SpawnOptions, SpawnedProcess } from "../../shared/bun-spawn-shim" +import { runRg } from "./cli" + +function createTextStream(text: string): ReadableStream { + return new ReadableStream({ + start(controller) { + if (text.length > 0) { + controller.enqueue(new TextEncoder().encode(text)) + } + controller.close() + }, + }) +} + +function createSpawnedProcess(exited: Promise, stdout = "", stderr = ""): SpawnedProcess { + return { + exitCode: null, + exited, + stdout: createTextStream(stdout), + stderr: createTextStream(stderr), + stdin: new Writable({ + write(_chunk, _encoding, callback) { + callback() + }, + }), + pid: 3919, + kill() {}, + ref() {}, + unref() {}, + } +} + +describe("runRg", () => { + it("#given mocked spawn rejection #when grep runs #then returns a structured error result", async () => { + const spawnMock = mock((_command: string[], _options?: SpawnOptions): SpawnedProcess => + createSpawnedProcess(Promise.reject(new Error("spawn rejected"))) + ) + + const result = await runRg( + { pattern: "needle", paths: ["."], timeout: 1000 }, + { path: "rg", backend: "rg" }, + spawnMock + ) + + expect(result.matches).toEqual([]) + expect(result.totalMatches).toBe(0) + expect(result.filesSearched).toBe(0) + expect(result.truncated).toBe(false) + expect(result.error).toContain("spawn rejected") + }) +}) diff --git a/src/tools/grep/cli.ts b/src/tools/grep/cli.ts index 4b9684c66..59a12f61f 100644 --- a/src/tools/grep/cli.ts +++ b/src/tools/grep/cli.ts @@ -1,4 +1,4 @@ -import { spawn } from "../../shared/bun-spawn-shim" +import { spawn, type SpawnOptions, type SpawnedProcess } from "../../shared/bun-spawn-shim" import { resolveGrepCli, type ResolvedCli, @@ -17,6 +17,9 @@ import { } from "./constants" import type { GrepOptions, GrepMatch, GrepResult, CountResult } from "./types" import { rgSemaphore } from "../shared/semaphore" +import { collectSearchProcessOutput } from "../shared/search-process-output" + +export type SearchProcessSpawner = (command: string[], options?: SpawnOptions) => SpawnedProcess function buildRgArgs(options: GrepOptions): string[] { const args: string[] = [ @@ -154,16 +157,24 @@ function parseCountOutput(output: string): CountResult[] { return results } -export async function runRg(options: GrepOptions, resolvedCli?: ResolvedCli): Promise { +export async function runRg( + options: GrepOptions, + resolvedCli?: ResolvedCli, + processSpawner: SearchProcessSpawner = spawn +): Promise { await rgSemaphore.acquire() try { - return await runRgInternal(options, resolvedCli) + return await runRgInternal(options, resolvedCli, processSpawner) } finally { rgSemaphore.release() } } -async function runRgInternal(options: GrepOptions, resolvedCli?: ResolvedCli): Promise { +async function runRgInternal( + options: GrepOptions, + resolvedCli?: ResolvedCli, + processSpawner: SearchProcessSpawner = spawn +): Promise { const cli = resolvedCli ?? resolveGrepCli() const args = buildArgs(options, cli.backend) const timeout = Math.min(options.timeout ?? DEFAULT_TIMEOUT_MS, DEFAULT_TIMEOUT_MS) @@ -176,23 +187,18 @@ async function runRgInternal(options: GrepOptions, resolvedCli?: ResolvedCli): P const paths = options.paths?.length ? options.paths : ["."] args.push(...paths) - const proc = spawn([cli.path, ...args], { - stdout: "pipe", - stderr: "pipe", - }) - - const timeoutPromise = new Promise((_, reject) => { - const id = setTimeout(() => { - proc.kill() - reject(new Error(`Search timeout after ${timeout}ms`)) - }, timeout) - proc.exited.then(() => clearTimeout(id)) - }) - try { - const stdout = await Promise.race([new Response(proc.stdout).text(), timeoutPromise]) - const stderr = await new Response(proc.stderr).text() - const exitCode = await proc.exited + const proc = processSpawner([cli.path, ...args], { + stdout: "pipe", + stderr: "pipe", + }) + + // #3919: Read stdout/stderr with Buffer concat instead of Response(stream).text(). + const { stdout, stderr, exitCode } = await collectSearchProcessOutput( + proc, + timeout, + `Search timeout after ${timeout}ms` + ) const truncated = stdout.length >= DEFAULT_MAX_OUTPUT_BYTES const outputToProcess = truncated ? stdout.substring(0, DEFAULT_MAX_OUTPUT_BYTES) : stdout @@ -232,11 +238,12 @@ async function runRgInternal(options: GrepOptions, resolvedCli?: ResolvedCli): P export async function runRgCount( options: Omit, - resolvedCli?: ResolvedCli + resolvedCli?: ResolvedCli, + processSpawner: SearchProcessSpawner = spawn ): Promise { await rgSemaphore.acquire() try { - return await runRgCountInternal(options, resolvedCli) + return await runRgCountInternal(options, resolvedCli, processSpawner) } finally { rgSemaphore.release() } @@ -244,7 +251,8 @@ export async function runRgCount( async function runRgCountInternal( options: Omit, - resolvedCli?: ResolvedCli + resolvedCli?: ResolvedCli, + processSpawner: SearchProcessSpawner = spawn ): Promise { const cli = resolvedCli ?? resolveGrepCli() const args = buildArgs({ ...options, context: 0 }, cli.backend) @@ -259,21 +267,21 @@ async function runRgCountInternal( args.push(...paths) const timeout = Math.min(options.timeout ?? DEFAULT_TIMEOUT_MS, DEFAULT_TIMEOUT_MS) - const proc = spawn([cli.path, ...args], { - stdout: "pipe", - stderr: "pipe", - }) - - const timeoutPromise = new Promise((_, reject) => { - const id = setTimeout(() => { - proc.kill() - reject(new Error(`Search timeout after ${timeout}ms`)) - }, timeout) - proc.exited.then(() => clearTimeout(id)) - }) - try { - const stdout = await Promise.race([new Response(proc.stdout).text(), timeoutPromise]) + const proc = processSpawner([cli.path, ...args], { + stdout: "pipe", + stderr: "pipe", + }) + + // #3919: Count mode uses the same Node-safe stream reader as normal grep. + const { stdout, stderr, exitCode } = await collectSearchProcessOutput( + proc, + timeout, + `Search timeout after ${timeout}ms` + ) + if (exitCode > 1 && stderr.trim()) { + throw new Error(stderr.trim()) + } return parseCountOutput(stdout) } catch (e) { throw new Error(`Count search failed: ${e instanceof Error ? e.message : String(e)}`) From aded57ff1fc68f90d8c81e557b64bb0827bfa6a9 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Fri, 22 May 2026 20:40:24 +0900 Subject: [PATCH 016/103] fix(shared): harden ripgrep-cli, zip-extractor, binary-downloader subprocess paths Same Web-Response-on-Node hazard existed in ripgrep auto-download flow, zip extraction helpers, and binary downloader streams. Switch to the new Node-safe reader and ensure no spawn path escapes as unhandledRejection. Related to #3919. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- src/shared/binary-downloader.ts | 9 ++++++--- src/shared/ripgrep-cli.ts | 15 +++++++++++---- .../powershell-zip-entry-listing.ts | 6 ++++-- .../zip-entry-listing/python-zip-entry-listing.ts | 6 ++++-- .../zip-entry-listing/read-zip-symlink-target.ts | 6 ++++-- .../zip-entry-listing/tar-zip-entry-listing.ts | 6 ++++-- .../zipinfo-zip-entry-listing.ts | 6 ++++-- src/shared/zip-extractor.ts | 8 +++++--- 8 files changed, 42 insertions(+), 20 deletions(-) diff --git a/src/shared/binary-downloader.ts b/src/shared/binary-downloader.ts index a44206c2e..a36c4a89a 100644 --- a/src/shared/binary-downloader.ts +++ b/src/shared/binary-downloader.ts @@ -4,6 +4,7 @@ import { spawn } from "./bun-spawn-shim"; import { bunWrite } from "./bun-file-shim"; import { validateArchiveEntries, type ArchiveEntry } from "./archive-entry-validator"; import { extractZip } from "./zip-extractor"; +import { readProcessStream } from "./process-stream-reader"; function isTarTraversalErrorOutput(output: string): boolean { return /path contains '\.\.'|member name contains '\.\.'|removing leading [`'\"]?\.\.\//i.test(output) @@ -47,7 +48,8 @@ export async function extractTarGz( const exitCode = await proc.exited; if (exitCode !== 0) { - const stderr = await new Response(proc.stderr).text(); + // #3919: Avoid Response(stream).text() in Windows Desktop utility processes. + const stderr = await readProcessStream(proc.stderr); if (isTarTraversalErrorOutput(stderr)) { throw new Error(`Unsafe archive entry: path contains path traversal (${archivePath})`) @@ -107,8 +109,9 @@ async function listTarEntries(archivePath: string, cwd?: string): Promise