From ad444f652cbf42038eddc1f6fc72eeab77e2aaf9 Mon Sep 17 00:00:00 2001 From: DarkPhilosophy <19309990+DarkPhilosophy@users.noreply.github.com> Date: Fri, 5 Jun 2026 20:30:16 +0300 Subject: [PATCH 01/46] fix(coding-agent): retry orphaned toolUse stops to prevent history corruption MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When Anthropic Claude returns stopReason: "toolUse" but the assistant message contains no actual toolCall, the session would append a spurious toolResult entry. This creates tool_result blocks without matching tool_use blocks — structurally invalid for Anthropic's API validator. The symptom is overloaded_error on every subsequent request (in: 0 out: 0), even though servers aren't overloaded. Other providers handle the same corrupted history more leniently, making the problem appear Anthropic-specific. The #isEmptyAssistantStop guard now checks for stopReason === "toolUse" with no text and no toolCall. When the retry cap is hit, tool-use orphans are still removed from active context (unlike regular empty stops) because they corrupt message history. A regression test covers both the retry and cap scenarios. For affected sessions: run /compact to drop the corrupted history tail. Fixes review feedback from chatgpt-codex-connector. --- .../coding-agent/src/session/agent-session.ts | 13 ++++- .../agent-session-empty-stop-guard.test.ts | 47 +++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 84f78cdce..b29f817c3 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -6443,9 +6443,13 @@ export class AgentSession { this.#retryAttempt = 0; } this.#resolveRetry(); + // Tool-use orphans corrupt Anthropic message history (tool_result without + // matching tool_use). Always remove them even when the retry cap is hit. + if (assistantMessage.stopReason === "toolUse") { + this.#removeEmptyStopFromActiveContext(assistantMessage); + } return true; } - this.#removeEmptyStopFromActiveContext(assistantMessage); this.agent.appendMessage({ role: "developer", @@ -6458,6 +6462,13 @@ export class AgentSession { } #isEmptyAssistantStop(assistantMessage: AssistantMessage): boolean { + const hasText = assistantMessage.content.some( + content => content.type === "text" && content.text.trim().length > 0, + ); + const hasToolCall = assistantMessage.content.some(content => content.type === "toolCall"); + if (assistantMessage.stopReason === "toolUse") { + return !hasText && !hasToolCall; + } if (assistantMessage.stopReason !== "stop") return false; return !assistantMessage.content.some(content => { if (content.type === "text") return content.text.trim().length > 0; diff --git a/packages/coding-agent/test/agent-session-empty-stop-guard.test.ts b/packages/coding-agent/test/agent-session-empty-stop-guard.test.ts index 16cd9ecb1..eb9495ef0 100644 --- a/packages/coding-agent/test/agent-session-empty-stop-guard.test.ts +++ b/packages/coding-agent/test/agent-session-empty-stop-guard.test.ts @@ -51,6 +51,14 @@ function emptyStop(): MockResponse { }; } +function orphanedToolUseStop(): MockResponse { + return { + content: [{ type: "thinking", thinking: "I should call a tool next." }], + stopReason: "toolUse", + usage: { output: 1, cacheRead: 100 }, + }; +} + async function createHarness( responses: MockResponse[], settingsOverrides: SettingsOverrides = {}, @@ -177,6 +185,45 @@ describe("AgentSession empty stop guard", () => { ).toHaveLength(1); }); + it("retries a tool-use stop that has no tool call or text", async () => { + const { session, mock } = await createHarness([ + recordCall("orphan", "call-record-orphan"), + orphanedToolUseStop(), + { content: ["finished after orphaned tool-use retry"], stopReason: "stop" }, + ]); + + await session.prompt("record orphan"); + await session.waitForIdle(); + + expect(mock.calls).toHaveLength(3); + expect(assistantText(session.agent.state.messages)).toContain("finished after orphaned tool-use retry"); + expect(reminderMessages(session.agent.state.messages)).toHaveLength(1); + }); + + it("removes orphaned tool-use stops even when retry cap is hit", async () => { + const { session, mock } = await createHarness([ + recordCall("gamma", "call-record-gamma"), + orphanedToolUseStop(), + orphanedToolUseStop(), + orphanedToolUseStop(), + orphanedToolUseStop(), + ]); + await session.prompt("record gamma"); + await session.waitForIdle(); + expect(mock.calls).toHaveLength(5); + expect(reminderMessages(session.agent.state.messages)).toHaveLength(3); + const activeBranchMessages = session.sessionManager + .getBranch() + .filter(entry => entry.type === "message") + .map(entry => entry.message as AgentMessage); + const orphanedToolUseStops = activeBranchMessages.filter( + message => + message.role === "assistant" && + message.stopReason === "toolUse" && + !message.content.some(content => content.type === "toolCall"), + ); + expect(orphanedToolUseStops).toHaveLength(0); + }); it("caps empty stop retries at three attempts", async () => { const { session, mock } = await createHarness([ recordCall("beta", "call-record-beta"), From e744ce28944cdd2a5da010c0792fe5fbc6137506 Mon Sep 17 00:00:00 2001 From: DarkPhilosophy <19309990+DarkPhilosophy@users.noreply.github.com> Date: Sat, 6 Jun 2026 02:22:36 +0300 Subject: [PATCH 02/46] perf(coding-agent): single-pass empty-stop content scan --- .../coding-agent/src/session/agent-session.ts | 30 +++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index c52a65311..31775300d 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -6484,19 +6484,25 @@ export class AgentSession { } #isEmptyAssistantStop(assistantMessage: AssistantMessage): boolean { - const hasText = assistantMessage.content.some( - content => content.type === "text" && content.text.trim().length > 0, - ); - const hasToolCall = assistantMessage.content.some(content => content.type === "toolCall"); - if (assistantMessage.stopReason === "toolUse") { - return !hasText && !hasToolCall; + const { stopReason } = assistantMessage; + if (stopReason !== "stop" && stopReason !== "toolUse") return false; + + // Single pass over content; the three flags cover every emptiness rule below. + let hasText = false; + let hasThinking = false; + let hasToolCall = false; + for (const content of assistantMessage.content) { + if (content.type === "text") hasText ||= content.text.trim().length > 0; + else if (content.type === "thinking") hasThinking ||= content.thinking.trim().length > 0; + else if (content.type === "toolCall") hasToolCall = true; } - if (assistantMessage.stopReason !== "stop") return false; - return !assistantMessage.content.some(content => { - if (content.type === "text") return content.text.trim().length > 0; - if (content.type === "thinking") return content.thinking.trim().length > 0; - return content.type === "toolCall"; - }); + + // An orphaned toolUse stop (no tool_use block) corrupts Anthropic history: + // a later tool_result has nothing to anchor to. Thinking alone cannot anchor + // a tool_result, so it does not rescue a toolUse stop here. + if (stopReason === "toolUse") return !hasText && !hasToolCall; + // A plain stop is empty only when it carries no usable content at all. + return !hasText && !hasThinking && !hasToolCall; } #emptyStopRetryReminder(): string { From 489b83d1ae7e78ca6f0d8156302ae741c04ee058 Mon Sep 17 00:00:00 2001 From: tycronk Date: Sat, 6 Jun 2026 03:15:27 -0400 Subject: [PATCH 03/46] docs: document Pi extension export drift in porting guide MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Section 3: add the @earendil-works/* scope alias and @mariozechner/pi-utils to the import-scope replacements, and note that bare typebox is not an @oh-my-pi/* scope. Section 15 (Extensions divergence): record the dropped/renamed exports that break a straight scope-rename port — StringEnum (pi-ai), formatSize (pi-coding-agent), and the removed DefaultResourceLoader/DefaultPackageManager/ SettingsManager/createEventBus discovery classes — with their OMP replacements. --- docs/porting-from-pi-mono.md | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/docs/porting-from-pi-mono.md b/docs/porting-from-pi-mono.md index 3bfb97bc1..23e4dfcd1 100644 --- a/docs/porting-from-pi-mono.md +++ b/docs/porting-from-pi-mono.md @@ -47,6 +47,9 @@ Upstream uses different package scopes. Replace them consistently. - `@mariozechner/pi-agent-core` → `@oh-my-pi/pi-agent-core` - `@mariozechner/pi-tui` → `@oh-my-pi/pi-tui` - `@mariozechner/pi-ai` → `@oh-my-pi/pi-ai` + - `@mariozechner/pi-utils` → `@oh-my-pi/pi-utils` +- Some upstream packages publish under the `@earendil-works/*` scope instead of `@mariozechner/*`. Map it the same way (`@earendil-works/pi-coding-agent` → `@oh-my-pi/pi-coding-agent`, and so on). +- The bare `typebox` package is not an `@oh-my-pi/*` scope; do not rewrite it as one. See the Extensions divergence in section 15 for how tool-parameter schemas map. ## 4) Use Bun APIs where they improve on Node @@ -353,10 +356,13 @@ Our fork has architectural decisions that differ from upstream. **Do not port th ### Extensions -| Upstream | Our Fork | -| ----------------------------- | ------------------------------------------------- | -| `jiti` for TypeScript loading | Native Bun `import()` | -| `pkg.pi` manifest field | `pkg.omp` preferred; fallback to `pkg.pi` remains | +| Upstream | Our Fork | +| ---------------------------------------------------------------- | ------------------------------------------------------------------------------------------------- | +| `jiti` for TypeScript loading | Native Bun `import()` | +| `pkg.pi` manifest field | `pkg.omp` preferred; fallback to `pkg.pi` remains | +| `StringEnum` from `pi-ai` | `Type.Enum` from the `pi.typebox` shim (or author the schema with `pi.zod`); `pi-ai` no longer exports `StringEnum` | +| `formatSize` from `pi-coding-agent` | `formatBytes` from `@oh-my-pi/pi-utils` | +| `DefaultResourceLoader` / `DefaultPackageManager` / `SettingsManager` / `createEventBus` | Capability-based discovery (`loadCapability(...)`) plus the `Settings` singleton and `EventBus` | ### Skip These Upstream Features From 236bba96b996b6d11575fbe121e3e78279c4ec15 Mon Sep 17 00:00:00 2001 From: Asaf Mahlev Date: Sat, 6 Jun 2026 11:56:23 +0300 Subject: [PATCH 04/46] fix(eval): floor JS worker init timeout to stop terminate-mid-init flake Worker-ready wait reused Bun's 5s default per-test timeout as its floor, so a slow cold-start under --isolate + high CI concurrency was aborted at 5s. The catch then terminate()s a still-initializing Bun worker -- the documented SIGILL/SIGTRAP crash trigger -- crashing the whole test file and intermittently failing unrelated PRs. Introduce WORKER_INIT_TIMEOUT_MS=15s as a fixed infrastructure floor (independent of, still dominated by, a larger per-cell timeout) and set a 20s file-local setDefaultTimeout in js-executor/js-workflow-helpers tests so cold starts complete instead of being torn down. --- packages/coding-agent/CHANGELOG.md | 3 +++ .../coding-agent/src/eval/js/context-manager.ts | 13 +++++++++---- packages/coding-agent/test/core/js-executor.test.ts | 7 ++++++- .../test/core/js-workflow-helpers.test.ts | 7 ++++++- 4 files changed, 24 insertions(+), 6 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index e14f2353f..da2371269 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,6 +1,9 @@ # Changelog ## [Unreleased] +### Fixed + +- Fixed a flaky JS eval worker startup that intermittently failed unrelated CI runs. The worker-ready wait reused Bun's 5s default per-test timeout as its floor, so a slow cold-start under `--isolate` + high concurrency was aborted mid-init; terminating a still-initializing Bun worker is the documented SIGILL/SIGTRAP crash trigger, which took down the whole test file. Worker init now floors at a fixed 15s infrastructure budget (independent of, and still dominated by, a larger per-cell `timeout`), and the JS eval test suites set a 20s file-local timeout so cold starts complete instead of being torn down. ## [15.9.5] - 2026-06-05 ### Added diff --git a/packages/coding-agent/src/eval/js/context-manager.ts b/packages/coding-agent/src/eval/js/context-manager.ts index cea0c97e5..8e7da951a 100644 --- a/packages/coding-agent/src/eval/js/context-manager.ts +++ b/packages/coding-agent/src/eval/js/context-manager.ts @@ -53,7 +53,12 @@ interface JsSession { const sessions = new Map(); const startingSessions = new Map>(); const resettingSessions = new Set(); -const READY_TIMEOUT_MS_DEFAULT = 5_000; +// Worker startup (module-graph import + WorkerCore construction) is infrastructure +// cost, not user compute. Floor it independently of Bun's 5s default per-test timeout +// so a slow cold-start under load isn't aborted mid-init — terminating a still- +// initializing Bun worker is the documented SIGILL/SIGTRAP crash trigger (see +// shared/indirect-eval.ts). Callers that pass a larger per-cell budget still dominate. +const WORKER_INIT_TIMEOUT_MS = 15_000; export async function executeInVmContext(options: { sessionKey: string; @@ -191,9 +196,9 @@ async function acquireSession(sessionKey: string, snapshot: SessionSnapshot, tim handleSessionMessage(session, msg); }); try { - // Cold-start can exceed 5s on slow hosts. Let the caller's per-cell timeout dominate so - // users can grant more headroom when they raise `timeout` on a cell. - const readyTimeoutMs = Math.max(READY_TIMEOUT_MS_DEFAULT, timeoutMs ?? 0); + // Init headroom is the fixed infrastructure floor; the caller's per-cell timeout + // dominates when larger so users can grant more by raising `timeout` on a cell. + const readyTimeoutMs = Math.max(WORKER_INIT_TIMEOUT_MS, timeoutMs ?? 0); await raceWithTimeout(readyPromise, readyTimeoutMs, "Timed out initializing JS eval worker"); worker.send({ type: "init", snapshot }); sessions.set(sessionKey, session); diff --git a/packages/coding-agent/test/core/js-executor.test.ts b/packages/coding-agent/test/core/js-executor.test.ts index 3d33d369d..7776bfbb2 100644 --- a/packages/coding-agent/test/core/js-executor.test.ts +++ b/packages/coding-agent/test/core/js-executor.test.ts @@ -1,4 +1,4 @@ -import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "bun:test"; +import { afterAll, afterEach, beforeAll, describe, expect, it, setDefaultTimeout, vi } from "bun:test"; import * as path from "node:path"; import type { AgentTool, AgentToolResult } from "@oh-my-pi/pi-agent-core"; import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; @@ -8,6 +8,11 @@ import * as z from "zod/v4"; import { disposeAllVmContexts } from "../../src/eval/js/context-manager"; import { executeJs, type JsResult } from "../../src/eval/js/executor"; +// JS eval cold-starts a Bun worker; under --isolate + high CI concurrency that startup +// can exceed Bun's 5s default per-test timeout, flaking the suite. Give the worker-backed +// tests headroom above the worker-init floor (context-manager WORKER_INIT_TIMEOUT_MS). +setDefaultTimeout(20_000); + function createTool( name: string, execute: (toolCallId: string, args: unknown, signal?: AbortSignal) => Promise, diff --git a/packages/coding-agent/test/core/js-workflow-helpers.test.ts b/packages/coding-agent/test/core/js-workflow-helpers.test.ts index 8e005bab8..d8592165a 100644 --- a/packages/coding-agent/test/core/js-workflow-helpers.test.ts +++ b/packages/coding-agent/test/core/js-workflow-helpers.test.ts @@ -1,4 +1,4 @@ -import { afterAll, beforeAll, describe, expect, it } from "bun:test"; +import { afterAll, beforeAll, describe, expect, it, setDefaultTimeout } from "bun:test"; import * as path from "node:path"; import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools"; @@ -6,6 +6,11 @@ import { TempDir } from "@oh-my-pi/pi-utils"; import { disposeAllVmContexts } from "../../src/eval/js/context-manager"; import { executeJs, type JsResult } from "../../src/eval/js/executor"; +// JS eval cold-starts a Bun worker; under --isolate + high CI concurrency that startup +// can exceed Bun's 5s default per-test timeout, flaking the suite. Give the worker-backed +// tests headroom above the worker-init floor (context-manager WORKER_INIT_TIMEOUT_MS). +setDefaultTimeout(20_000); + function statusEvents(result: JsResult) { return result.displayOutputs.filter( (output): output is Extract => output.type === "status", From ea8d58d3c696cf48afca46719e886ff8abfda7a2 Mon Sep 17 00:00:00 2001 From: Asaf Mahlev Date: Sat, 6 Jun 2026 12:43:53 +0300 Subject: [PATCH 05/46] docs(eval): clarify indirect-eval cross-reference in worker-init comment Address review: shared/indirect-eval.ts documents the vm.runInContext mid-execution terminate-race, not a mid-init one. Reword so a reader doesn't grep that file for an init-specific note. --- packages/coding-agent/src/eval/js/context-manager.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/coding-agent/src/eval/js/context-manager.ts b/packages/coding-agent/src/eval/js/context-manager.ts index 8e7da951a..c1dcef642 100644 --- a/packages/coding-agent/src/eval/js/context-manager.ts +++ b/packages/coding-agent/src/eval/js/context-manager.ts @@ -56,8 +56,9 @@ const resettingSessions = new Set(); // Worker startup (module-graph import + WorkerCore construction) is infrastructure // cost, not user compute. Floor it independently of Bun's 5s default per-test timeout // so a slow cold-start under load isn't aborted mid-init — terminating a still- -// initializing Bun worker is the documented SIGILL/SIGTRAP crash trigger (see -// shared/indirect-eval.ts). Callers that pass a larger per-cell budget still dominate. +// initializing Bun worker triggers the same kind of terminate-race that motivates +// avoiding `vm.runInContext` (see shared/indirect-eval.ts), here surfacing as a +// SIGILL/SIGSEGV. Callers that pass a larger per-cell budget still dominate. const WORKER_INIT_TIMEOUT_MS = 15_000; export async function executeInVmContext(options: { From 102bd398549962cb3b1869e9a2ab53a8fa5f959b Mon Sep 17 00:00:00 2001 From: WodenJay Date: Sun, 7 Jun 2026 00:07:05 +0800 Subject: [PATCH 06/46] fix(hashline): strip N: line-number prefix from auto-piped bare body rows (#1492) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace recursive stripLeadingHashlinePrefixes with single-pass stripOneLeadingHashlinePrefix in #handleRaw to prevent over-stripping content whose own text starts with digits:colon (e.g. YAML ports, timestamps: 2:42:hello → 42:hello, not hello) - Export stripOneLeadingHashlinePrefix from prefixes.ts - Move hashline CHANGELOG entry to [Unreleased] section - Add regression tests for nested N: prefix and +N: asymmetry --- packages/coding-agent/CHANGELOG.md | 1 + packages/coding-agent/test/core/hashline.test.ts | 4 ++-- packages/hashline/CHANGELOG.md | 6 +++++- packages/hashline/src/parser.ts | 10 +++++++++- packages/hashline/src/prefixes.ts | 12 +++++++++++- packages/hashline/test/format-v2.test.ts | 9 +++++++++ packages/hashline/test/leniency.test.ts | 16 ++++++++++++++++ 7 files changed, 53 insertions(+), 5 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 6515a5a39..5d24d55f3 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -15,6 +15,7 @@ - Fixed framed read results rendering with an extra blank row above and below the output block. - Fixed collapsed search result previews that could show only "… N more matches" when the first grouped section exceeded the preview budget. Collapsed search output now compacts to match rows, fills the budget with visible hits before the summary, and keeps truncation details out of the bottom user-visible notice. - Fixed boolean environment flag overrides that were ORed with settings, so `PI_INTENT_TRACING=0`, `PI_AUTO_QA=0`, and per-backend eval flags now take precedence when present while falling back to config when unset. +- Stripped read-output line-number prefixes (`N:`) from auto-piped bare body rows in the hashline edit parser, so pasting `3:text` without a `+` prefix no longer injects `3:` as literal content. Uses single-pass stripping to avoid corrupting content whose own text starts with `digits:` ([#1492](https://github.com/can1357/oh-my-pi/issues/1492)). ## [15.9.67] - 2026-06-06 ### Added diff --git a/packages/coding-agent/test/core/hashline.test.ts b/packages/coding-agent/test/core/hashline.test.ts index 4f281a67e..9f0a29d7d 100644 --- a/packages/coding-agent/test/core/hashline.test.ts +++ b/packages/coding-agent/test/core/hashline.test.ts @@ -508,10 +508,10 @@ describe("hashline parser — range-anchor syntax", () => { expect(warnings).toEqual([]); }); - it("auto-pipes read-output `N:TEXT` lines inside a pending hunk as literal text", () => { + it("strips read-output `N:` line-number prefix from auto-piped bare body rows", () => { const diff = `replace ${tag(2, "bbb")}..${tag(4, "ddd")}:\n${repl("line one")}\n${tag(3, "ccc")}:line two`; const { edits, warnings } = parseHashline(diff); - expect(applyHashlineEdits("aaa\nbbb\nccc\nddd\neee", edits).lines).toBe("aaa\nline one\n3:line two\neee"); + expect(applyHashlineEdits("aaa\nbbb\nccc\nddd\neee", edits).lines).toBe("aaa\nline one\nline two\neee"); expect(warnings.some(w => /Auto-prefixed bare body row/.test(w))).toBe(true); }); diff --git a/packages/hashline/CHANGELOG.md b/packages/hashline/CHANGELOG.md index 42e6dc391..889e72c6e 100644 --- a/packages/hashline/CHANGELOG.md +++ b/packages/hashline/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Stripped read-output line-number prefixes (`N:`) from auto-piped bare body rows so that pasting `3:text` without a `+` prefix no longer injects `3:` as literal content. Uses single-pass stripping to avoid corrupting content whose own text starts with `digits:` (e.g. YAML port maps, timestamps) ([#1492](https://github.com/can1357/oh-my-pi/issues/1492)). + ## [15.9.67] - 2026-06-06 ### Breaking Changes @@ -40,7 +44,7 @@ ### Added - Added `maxPaths` and `maxVersionsPerPath` options to `InMemorySnapshotStore` to bound tracked paths and per-path snapshot history -- Re-introduced balance-validated boundary repair in `applyEdits`. A replacement hunk (`replace N..M:` + body) is normalized so its payload preserves the deleted region's delimiter balance: when the body restates a closing delimiter that survives just outside the range (duplicate `}` / `);` / `]`) the echo is dropped, and when the range deletes a structural closer the body never restates (missing closer) the closer is spared instead of deleted. A repair fires only when one boundary operation drives the per-channel `()` / `[]` / `{}` imbalance to exactly zero while leaving surrounding text byte-identical (single-line ops are limited to pure structural-closer lines), so balance-preserving edits and intentional balanced duplicates are never touched. Bracket counting skips strings, template literals, and comments. Each repair surfaces a `delimiter-balance` warning through `ApplyResult.warnings`. +- Re-introduced balance-validated boundary repair in `applyEdits`. A replacement hunk (`replace N..M:` + body) is normalized so its payload preserves the deleted region's delimiter balance: when the body restates a closing delimiter that survives just outside the range (duplicate `}` / `);` / `]`) the echo is dropped, and when the range deletes a structural closer the body never restates (missing closer) the closer is spared instead of deleted. A repair fires only when one boundary operation drives the per-channel `()` / `[]` / `{}` imbalance to exactly zero while leaving surrounding text byte-identical (single-line ops are limited to pure structural-closer lines), so balance-preserving edits and intentional balanced duplicates are never touched. Bracket couples are also bounded by line count: structural balance delta repair is capped to 10 duplicate lines across all channels combined, massive balanced blocks are skipped. ### Changed diff --git a/packages/hashline/src/parser.ts b/packages/hashline/src/parser.ts index 47e56b699..83b7fdb97 100644 --- a/packages/hashline/src/parser.ts +++ b/packages/hashline/src/parser.ts @@ -4,6 +4,7 @@ * applier. */ import { HL_PAYLOAD_REPLACE } from "./format"; +import { stripOneLeadingHashlinePrefix } from "./prefixes"; import { BARE_BODY_AUTO_PIPED_WARNING, DELETE_BLOCK_TAKES_NO_BODY, @@ -220,7 +221,14 @@ export class Executor { throw new Error(`line ${lineNum}: ${DELETE_BLOCK_TAKES_NO_BODY}`); if (text.trimStart().charCodeAt(0) === 45 /* - */) throw new Error(`line ${lineNum}: ${MINUS_ROW_REJECTED}`); if (!this.#warnings.includes(BARE_BODY_AUTO_PIPED_WARNING)) this.#warnings.push(BARE_BODY_AUTO_PIPED_WARNING); - this.#pending.payloads.push({ kind: "literal", text, lineNum }); + // Strip at most one read-output line-number prefix (e.g. "3:text") + // from bare body rows. Lines with an explicit "+" prefix go through + // #handleLiteralPayload which skips this stripping — "+3:text" is + // intentional literal content, while bare "3:text" is almost always + // a copy-paste artifact from snapshot output. Single-pass only: + // recursive stripping would corrupt content whose own text starts + // with `digits:` (e.g. YAML ports "42:hello", timestamps "12:30"). + this.#pending.payloads.push({ kind: "literal", text: stripOneLeadingHashlinePrefix(text), lineNum }); return; } if (text.trim().length === 0) return; diff --git a/packages/hashline/src/prefixes.ts b/packages/hashline/src/prefixes.ts index 6e18950da..9af5c1b3a 100644 --- a/packages/hashline/src/prefixes.ts +++ b/packages/hashline/src/prefixes.ts @@ -22,7 +22,7 @@ const HL_HEADER_RE = new RegExp(`^\\s*\\[[^#\\r\\n]+#[0-9a-fA-F]{${HL_FILE_HASH_ const DIFF_PLUS_RE = /^[+](?![+])/; const READ_TRUNCATION_NOTICE_RE = /^\[(?:Showing lines \d+-\d+ of \d+|\d+ more lines? in (?:file|\S+))\b.*\bUse :L?\d+/; -function stripLeadingHashlinePrefixes(line: string): string { +export function stripLeadingHashlinePrefixes(line: string): string { let result = line; let previous: string; do { @@ -31,6 +31,16 @@ function stripLeadingHashlinePrefixes(line: string): string { } while (result !== previous); return result; } +/** + * Single-pass variant of {@link stripLeadingHashlinePrefixes} that strips at + * most one leading hashline prefix (`N:`, `>>>N:`, `+N:` etc.) and does NOT + * loop. Use this when the input carries at most one snapshot prefix (e.g. a + * bare body row paste from `read` output) — recursive stripping would corrupt + * content whose own text starts with `digits:`. + */ +export function stripOneLeadingHashlinePrefix(line: string): string { + return line.replace(HL_PREFIX_RE, ""); +} interface LinePrefixStats { nonEmpty: number; diff --git a/packages/hashline/test/format-v2.test.ts b/packages/hashline/test/format-v2.test.ts index c18c1419f..5a47e7c06 100644 --- a/packages/hashline/test/format-v2.test.ts +++ b/packages/hashline/test/format-v2.test.ts @@ -52,6 +52,15 @@ describe("hashline format v4", () => { expect(warnings.some(w => /Auto-prefixed bare body row/.test(w))).toBe(true); }); + it("strips read-output line number prefix from auto-piped bare body rows", () => { + const text = "a\nb\nc"; + // Without this fix, "3:text" becomes literal "3:text" in the file. + // With the fix, the "3:" prefix is stripped, yielding just "text". + const { edits, warnings } = parsePatch("replace 2..2:\n3:replaced"); + expect(applyEdits(text, edits).text).toBe("a\nreplaced\nc"); + expect(warnings.some(w => /Auto-prefixed bare body row/.test(w))).toBe(true); + }); + it("validates insert anchors against file bounds", () => { const edits = parsePatch("insert before 4:\n+x").edits; expect(() => applyEdits("a\nb", edits)).toThrow(/Line 4 does not exist/); diff --git a/packages/hashline/test/leniency.test.ts b/packages/hashline/test/leniency.test.ts index b28f57c27..bf9250205 100644 --- a/packages/hashline/test/leniency.test.ts +++ b/packages/hashline/test/leniency.test.ts @@ -106,6 +106,22 @@ describe("hashline body contracts", () => { expect(result.warnings.some(w => /Auto-prefixed bare body row/.test(w))).toBe(true); }); + it("strips read-output line number prefix from auto-piped bare body rows", () => { + const result = parsePatch("replace 2..2:\n2:hello"); + expect(applyEdits(FILE, result.edits).text).toBe("a\nhello\nc\nd\ne"); + expect(result.warnings.some(w => /Auto-prefixed bare body row/.test(w))).toBe(true); + }); + it("preserves `+N:` literal payloads without stripping", () => { + const result = parsePatch("replace 2..2:\n+3:keep"); + expect(applyEdits(FILE, result.edits).text).toBe("a\n3:keep\nc\nd\ne"); + expect(result.warnings.some(w => /Auto-prefixed/.test(w))).toBe(false); + }); + it("strips only one N: prefix from bare body rows (preserves nested digits:colon)", () => { + // "2:42:hello" → should yield "42:hello", NOT "hello" (recursive would over-strip) + const result = parsePatch("replace 2..2:\n2:42:hello"); + expect(applyEdits(FILE, result.edits).text).toBe("a\n42:hello\nc\nd\ne"); + }); + it("rejects `-` body rows with a teaching error", () => { expect(() => parsePatch("replace 2..2:\n-old\n+new")).toThrow(/`-` rows are not valid/); }); From 1c2e22068af4ebbb82eadca79ba20a5db0f43f3c Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 7 Jun 2026 08:49:15 +0000 Subject: [PATCH 07/46] fix(tui): chunked oversized writes on Windows ConPTY so viewport tracks the cursor Windows ConPTY ties viewport tracking to per-WriteFile boundaries: when a single process.stdout.write exceeds ~32-64 KB, the pseudo-console stops following the cursor and the host UI's scroll position stays parked at wherever the write began. Full paints (resume, history rebuild, large permission dialogs) showed only the first screenful until Alt+Tab forced the host to re-query the cursor; the bytes always landed in scrollback. ProcessTerminal#safeWrite now routes win32 writes through a new chunkForConPTY helper that splits oversized buffers into <=8 KiB pieces at \n boundaries, keeping every WriteFile well below the threshold and escape sequences intact (TUI paints separate rows with \r\n, so a line break almost always exists in range). Non-win32 platforms keep the single-write fast path. Fixes #2034 --- packages/tui/CHANGELOG.md | 4 + packages/tui/src/terminal.ts | 66 +++++++- packages/tui/test/issue-2034-repro.test.ts | 173 +++++++++++++++++++++ 3 files changed, 242 insertions(+), 1 deletion(-) create mode 100644 packages/tui/test/issue-2034-repro.test.ts diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index 56903eddc..ed1709c60 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed Windows ConPTY hosts (Windows Terminal, Tabby, Hyper, VS Code) parking the viewport at the top of a full paint after a `/resume` or any long-session repaint. `ProcessTerminal#safeWrite` now splits oversized writes into ≤ 8 KiB pieces at line boundaries on `win32` so each underlying `WriteFile` stays below the ~32 KiB threshold where ConPTY stops tracking the cursor; the data was always delivered, but the host UI's scroll position would not follow until any focus event forced a re-query. ([#2034](https://github.com/can1357/oh-my-pi/issues/2034)) + ## [15.10.1] - 2026-06-07 ### Breaking Changes diff --git a/packages/tui/src/terminal.ts b/packages/tui/src/terminal.ts index 2069f6370..6c04f0805 100644 --- a/packages/tui/src/terminal.ts +++ b/packages/tui/src/terminal.ts @@ -9,6 +9,58 @@ const TERMINAL_PROGRESS_KEEPALIVE_MS = 1000; const TERMINAL_PROGRESS_ACTIVE_SEQUENCE = "\x1b]9;4;3\x07"; const TERMINAL_PROGRESS_CLEAR_SEQUENCE = "\x1b]9;4;0;\x07"; +/** + * Maximum bytes per `process.stdout.write` call on Windows. + * + * Windows ConPTY ties viewport tracking to per-`WriteFile` boundaries: when a + * single write exceeds ~32-64 KB, the pseudo-console stops following the + * cursor and the host UI's viewport stays parked at whatever scroll position + * the write started from. The visible symptom is that a full-paint of a long + * session (resume, history rebuild, large permission dialog) shows only the + * first ~30 lines until any focus event forces the host to re-query the + * cursor. The data is delivered correctly — it's purely a viewport-sync bug. + * + * 8 KiB is well below the 32 KiB threshold reported on Windows Terminal and + * leaves headroom for the other ConPTY hosts (Tabby, Hyper, VS Code) where + * the exact limit is undocumented. The cost is a handful of extra syscalls + * per full paint — invisible compared to the cost of the paint itself. + */ +const MAX_CONPTY_WRITE_CHUNK = 8 * 1024; + +/** + * Split `data` into chunks no larger than `maxChunkSize`, preferring a line + * boundary (`\n`) as the cut point so escape sequences (which never contain + * `\n`) stay intact. The TUI's full-paint buffers are line-structured + * (`buffer += "\r\n"` between rows), so a newline almost always exists within + * the window. The fallback for a buffer with no newline in range is a hard + * cut at `maxChunkSize`: the ConPTY viewport bug from a single oversized + * write is strictly worse than a one-frame escape-sequence glitch on a buffer + * the renderer effectively never produces. + * + * Exported for unit testing of the chunking contract; `#safeWrite` is the + * sole production caller. + */ +export function chunkForConPTY(data: string, maxChunkSize: number = MAX_CONPTY_WRITE_CHUNK): string[] { + if (data.length <= maxChunkSize) return [data]; + const chunks: string[] = []; + let pos = 0; + while (pos < data.length) { + const remaining = data.length - pos; + if (remaining <= maxChunkSize) { + chunks.push(data.slice(pos)); + break; + } + const windowEnd = pos + maxChunkSize; + // Prefer the last newline inside the window so escape sequences stay + // intact within their chunk; hard-cut at `windowEnd` otherwise. + const nl = data.lastIndexOf("\n", windowEnd - 1); + const cut = nl >= pos ? nl + 1 : windowEnd; + chunks.push(data.slice(pos, cut)); + pos = cut; + } + return chunks; +} + /** * Minimal terminal interface for TUI */ @@ -978,7 +1030,19 @@ export class ProcessTerminal implements Terminal { // files). They serve no purpose there and would surface as visible noise. if (!process.stdout.isTTY) return; try { - process.stdout.write(data); + // Windows ConPTY drops viewport tracking when a single write exceeds + // ~32-64 KB: the host UI's scroll position stays parked at wherever + // the write began, even though every byte landed in scrollback. Split + // large paints into newline-aligned chunks so each underlying + // `WriteFile` stays well below the threshold. Non-win32 PTYs do not + // share the bug, so they keep the single-write fast path. See #2034. + if (process.platform === "win32" && data.length > MAX_CONPTY_WRITE_CHUNK) { + for (const chunk of chunkForConPTY(data, MAX_CONPTY_WRITE_CHUNK)) { + process.stdout.write(chunk); + } + } else { + process.stdout.write(data); + } } catch (err) { // Any write failure means terminal is dead - no recovery possible this.#dead = true; diff --git a/packages/tui/test/issue-2034-repro.test.ts b/packages/tui/test/issue-2034-repro.test.ts new file mode 100644 index 000000000..e928d73a4 --- /dev/null +++ b/packages/tui/test/issue-2034-repro.test.ts @@ -0,0 +1,173 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; +import { chunkForConPTY, ProcessTerminal } from "@oh-my-pi/pi-tui/terminal"; + +// Regression test for https://github.com/can1357/oh-my-pi/issues/2034 +// +// Windows ConPTY ties viewport tracking to per-`WriteFile` boundaries: when +// a single `process.stdout.write` exceeds ~32-64 KB, the pseudo-console +// stops following the cursor and the host UI's scroll position stays parked +// at wherever the write began. The data lands in scrollback — Alt+Tab forces +// the host to re-query the cursor and the viewport jumps to the bottom — +// but until then the user sees only the first screenful of a long session +// or resume payload. +// +// Fix: `ProcessTerminal#safeWrite` chunks oversized writes into ≤ 8 KiB +// pieces on `process.platform === "win32"`. Non-win32 PTYs do not share the +// bug and keep the single-write fast path. + +const ESC = "\x1b"; + +function buildFullPaint(lines: number, lineLength: number): string { + // Mirrors the shape of `TUI#emitFullPaint`'s buffer: a clear-screen prefix, + // rows terminated with `\r\n` and a per-line SGR reset, and a cursor/end + // sequence trailer. The exact bytes do not matter for the chunker — only + // that escapes are present and the buffer crosses the ConPTY threshold. + let buf = `${ESC}[2J${ESC}[H${ESC}[3J`; + for (let i = 0; i < lines; i++) { + if (i > 0) buf += "\r\n"; + const content = `${ESC}[38;5;${i % 256}mrow-${i.toString().padStart(4, "0")}: ${"x".repeat(lineLength)}${ESC}[0m`; + buf += content; + } + buf += `${ESC}[H${ESC}[?25h`; + return buf; +} + +describe("issue #2034: chunk large terminal writes on Windows ConPTY", () => { + describe("chunkForConPTY()", () => { + it("returns the original buffer untouched when under the chunk size", () => { + const data = "small payload"; + expect(chunkForConPTY(data, 1024)).toEqual([data]); + }); + + it("splits a large multi-line buffer into pieces no larger than the chunk size", () => { + const data = buildFullPaint(2000, 60); + const max = 8 * 1024; + expect(data.length).toBeGreaterThan(max); + + const chunks = chunkForConPTY(data, max); + + expect(chunks.length).toBeGreaterThan(1); + for (const chunk of chunks) { + expect(chunk.length).toBeLessThanOrEqual(max); + } + }); + + it("preserves the full payload across chunks (no data loss or reordering)", () => { + const data = buildFullPaint(500, 120); + const chunks = chunkForConPTY(data, 4 * 1024); + expect(chunks.join("")).toBe(data); + }); + + it("splits at newline boundaries so escape sequences are never sliced apart", () => { + // Every row is bracketed by SGR escapes. If the chunker cut inside a + // chunk's escape sequence, the trailing chunk would not start with + // either an escape or the post-newline state — instead it would + // start with a stray CSI byte (`[`, digits, `m`). + const data = buildFullPaint(400, 80); + const chunks = chunkForConPTY(data, 4 * 1024); + // Exclude the head chunk (starts with the clear-screen prefix). + for (const chunk of chunks.slice(1)) { + // Every subsequent chunk begins on a fresh line: either the new + // line's first byte is the SGR escape, the row's plaintext + // prefix, or — for the trailing tail — the cursor sequence. + const firstByte = chunk.charCodeAt(0); + const startsWithEsc = chunk.startsWith(ESC); + const startsWithRowText = chunk.startsWith("row-"); + expect(startsWithEsc || startsWithRowText).toBe(true); + if (!startsWithEsc) { + // Plain-text starts cannot be control characters that would + // indicate a sliced escape (CSI `[`, digits, or `m`). + expect(firstByte).toBeGreaterThanOrEqual(0x20); + } + } + }); + + it("makes forward progress on a single line longer than the chunk size", () => { + // Pathological case: one very long line with no embedded `\n`. The + // chunker must not loop, and the joined chunks must equal the input. + const giantLine = "a".repeat(20_000); + const data = `${giantLine}\nshort\n`; + const chunks = chunkForConPTY(data, 4 * 1024); + expect(chunks.length).toBeGreaterThanOrEqual(2); + expect(chunks.join("")).toBe(data); + }); + + it("falls back to a raw split when the buffer contains no newlines", () => { + const data = "x".repeat(20_000); + const chunks = chunkForConPTY(data, 4 * 1024); + expect(chunks.join("")).toBe(data); + expect(chunks.length).toBeGreaterThan(1); + // Every chunk except possibly the tail is exactly the chunk size. + for (const chunk of chunks.slice(0, -1)) { + expect(chunk.length).toBe(4 * 1024); + } + }); + }); + + describe("ProcessTerminal#write on win32", () => { + const stdinIsTtyDescriptor = Object.getOwnPropertyDescriptor(process.stdin, "isTTY"); + const stdoutIsTtyDescriptor = Object.getOwnPropertyDescriptor(process.stdout, "isTTY"); + const platformDescriptor = Object.getOwnPropertyDescriptor(process, "platform"); + + beforeEach(() => { + Object.defineProperty(process.stdin, "isTTY", { value: true, configurable: true }); + Object.defineProperty(process.stdout, "isTTY", { value: true, configurable: true }); + }); + + afterEach(() => { + vi.restoreAllMocks(); + if (platformDescriptor) Object.defineProperty(process, "platform", platformDescriptor); + if (stdinIsTtyDescriptor) Object.defineProperty(process.stdin, "isTTY", stdinIsTtyDescriptor); + else Reflect.deleteProperty(process.stdin, "isTTY"); + if (stdoutIsTtyDescriptor) Object.defineProperty(process.stdout, "isTTY", stdoutIsTtyDescriptor); + else Reflect.deleteProperty(process.stdout, "isTTY"); + }); + + function captureStdoutWrites(): string[] { + const writes: string[] = []; + vi.spyOn(process.stdout, "write").mockImplementation(chunk => { + writes.push(typeof chunk === "string" ? chunk : chunk.toString()); + return true; + }); + return writes; + } + + it("splits >8 KiB writes into chunks on win32 so ConPTY can track the viewport", () => { + Object.defineProperty(process, "platform", { value: "win32", configurable: true }); + const writes = captureStdoutWrites(); + const terminal = new ProcessTerminal(); + const payload = buildFullPaint(2000, 60); + + terminal.write(payload); + + const conptyChunks = writes.filter(w => w.length > 0); + expect(conptyChunks.length).toBeGreaterThan(1); + for (const chunk of conptyChunks) { + expect(chunk.length).toBeLessThanOrEqual(8 * 1024); + } + expect(conptyChunks.join("")).toBe(payload); + }); + + it("keeps the single-write fast path on non-win32 platforms", () => { + Object.defineProperty(process, "platform", { value: "linux", configurable: true }); + const writes = captureStdoutWrites(); + const terminal = new ProcessTerminal(); + const payload = buildFullPaint(2000, 60); + + terminal.write(payload); + + expect(writes).toEqual([payload]); + }); + + it("does not chunk small writes on win32", () => { + Object.defineProperty(process, "platform", { value: "win32", configurable: true }); + const writes = captureStdoutWrites(); + const terminal = new ProcessTerminal(); + const payload = `${ESC}[H${ESC}[K`; + + terminal.write(payload); + + expect(writes).toEqual([payload]); + }); + }); +}); From 271b9ed62059dcd2bcbfe601e41c29460c4cf095 Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 7 Jun 2026 08:54:07 +0000 Subject: [PATCH 08/46] fix(tui): extended ConPTY chunking gate to WSL WSL processes report process.platform === 'linux', but their stdout still funnels through wslhost into the same Windows ConPTY pipe, so the >32 KB viewport-tracking regression reproduces there as well. The gate now fires whenever process.platform === 'win32' OR isWindowsSubsystemForLinux() detects WSL_DISTRO_NAME/WSL_INTEROP. Genuine POSIX terminals keep the single-write fast path. Added a WSL-specific test alongside the existing win32 / clean-linux cases to lock the broader gate. Fixes #2034 --- packages/tui/CHANGELOG.md | 2 +- packages/tui/src/terminal.ts | 10 +++++-- packages/tui/test/issue-2034-repro.test.ts | 34 ++++++++++++++++++++-- 3 files changed, 40 insertions(+), 6 deletions(-) diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index ed1709c60..43d1599f9 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -4,7 +4,7 @@ ### Fixed -- Fixed Windows ConPTY hosts (Windows Terminal, Tabby, Hyper, VS Code) parking the viewport at the top of a full paint after a `/resume` or any long-session repaint. `ProcessTerminal#safeWrite` now splits oversized writes into ≤ 8 KiB pieces at line boundaries on `win32` so each underlying `WriteFile` stays below the ~32 KiB threshold where ConPTY stops tracking the cursor; the data was always delivered, but the host UI's scroll position would not follow until any focus event forced a re-query. ([#2034](https://github.com/can1357/oh-my-pi/issues/2034)) +- Fixed Windows ConPTY hosts (Windows Terminal, Tabby, Hyper, VS Code) parking the viewport at the top of a full paint after a `/resume` or any long-session repaint. `ProcessTerminal#safeWrite` now splits oversized writes into ≤ 8 KiB pieces at line boundaries on `win32` and inside WSL (where stdout still crosses ConPTY at the `wslhost` boundary) so each underlying `WriteFile` stays below the ~32 KiB threshold where ConPTY stops tracking the cursor; the data was always delivered, but the host UI's scroll position would not follow until any focus event forced a re-query. ([#2034](https://github.com/can1357/oh-my-pi/issues/2034)) ## [15.10.1] - 2026-06-07 ### Breaking Changes diff --git a/packages/tui/src/terminal.ts b/packages/tui/src/terminal.ts index 6c04f0805..90a74803b 100644 --- a/packages/tui/src/terminal.ts +++ b/packages/tui/src/terminal.ts @@ -1034,9 +1034,13 @@ export class ProcessTerminal implements Terminal { // ~32-64 KB: the host UI's scroll position stays parked at wherever // the write began, even though every byte landed in scrollback. Split // large paints into newline-aligned chunks so each underlying - // `WriteFile` stays well below the threshold. Non-win32 PTYs do not - // share the bug, so they keep the single-write fast path. See #2034. - if (process.platform === "win32" && data.length > MAX_CONPTY_WRITE_CHUNK) { + // `WriteFile` stays well below the threshold. The gate also covers + // WSL — `process.platform === "linux"` there, but stdout still + // crosses into ConPTY at the `wslhost` boundary, so the same per- + // WriteFile cap applies. Non-ConPTY PTYs keep the single-write fast + // path. See #2034. + const conptyHosted = process.platform === "win32" || isWindowsSubsystemForLinux(); + if (conptyHosted && data.length > MAX_CONPTY_WRITE_CHUNK) { for (const chunk of chunkForConPTY(data, MAX_CONPTY_WRITE_CHUNK)) { process.stdout.write(chunk); } diff --git a/packages/tui/test/issue-2034-repro.test.ts b/packages/tui/test/issue-2034-repro.test.ts index e928d73a4..1391fe6ea 100644 --- a/packages/tui/test/issue-2034-repro.test.ts +++ b/packages/tui/test/issue-2034-repro.test.ts @@ -104,14 +104,24 @@ describe("issue #2034: chunk large terminal writes on Windows ConPTY", () => { }); }); - describe("ProcessTerminal#write on win32", () => { + describe("ProcessTerminal#write platform gate", () => { const stdinIsTtyDescriptor = Object.getOwnPropertyDescriptor(process.stdin, "isTTY"); const stdoutIsTtyDescriptor = Object.getOwnPropertyDescriptor(process.stdout, "isTTY"); const platformDescriptor = Object.getOwnPropertyDescriptor(process, "platform"); + const originalWslDistro = Bun.env.WSL_DISTRO_NAME; + const originalWslInterop = Bun.env.WSL_INTEROP; + + function setEnv(key: string, value: string | undefined): void { + if (value === undefined) delete Bun.env[key]; + else Bun.env[key] = value; + } beforeEach(() => { Object.defineProperty(process.stdin, "isTTY", { value: true, configurable: true }); Object.defineProperty(process.stdout, "isTTY", { value: true, configurable: true }); + // Clear WSL markers by default; tests opt in. + setEnv("WSL_DISTRO_NAME", undefined); + setEnv("WSL_INTEROP", undefined); }); afterEach(() => { @@ -121,6 +131,8 @@ describe("issue #2034: chunk large terminal writes on Windows ConPTY", () => { else Reflect.deleteProperty(process.stdin, "isTTY"); if (stdoutIsTtyDescriptor) Object.defineProperty(process.stdout, "isTTY", stdoutIsTtyDescriptor); else Reflect.deleteProperty(process.stdout, "isTTY"); + setEnv("WSL_DISTRO_NAME", originalWslDistro); + setEnv("WSL_INTEROP", originalWslInterop); }); function captureStdoutWrites(): string[] { @@ -148,7 +160,25 @@ describe("issue #2034: chunk large terminal writes on Windows ConPTY", () => { expect(conptyChunks.join("")).toBe(payload); }); - it("keeps the single-write fast path on non-win32 platforms", () => { + it("splits >8 KiB writes inside WSL because stdout still crosses ConPTY at wslhost", () => { + Object.defineProperty(process, "platform", { value: "linux", configurable: true }); + setEnv("WSL_DISTRO_NAME", "Ubuntu"); + setEnv("WSL_INTEROP", "/run/WSL/123_interop"); + const writes = captureStdoutWrites(); + const terminal = new ProcessTerminal(); + const payload = buildFullPaint(2000, 60); + + terminal.write(payload); + + const conptyChunks = writes.filter(w => w.length > 0); + expect(conptyChunks.length).toBeGreaterThan(1); + for (const chunk of conptyChunks) { + expect(chunk.length).toBeLessThanOrEqual(8 * 1024); + } + expect(conptyChunks.join("")).toBe(payload); + }); + + it("keeps the single-write fast path on non-ConPTY platforms (clean linux, darwin)", () => { Object.defineProperty(process, "platform", { value: "linux", configurable: true }); const writes = captureStdoutWrites(); const terminal = new ProcessTerminal(); From df5abffe24d8c184c50f0cad2286e092f88116c6 Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 7 Jun 2026 09:13:37 +0000 Subject: [PATCH 09/46] fix(coding-agent): guard ctrl+z handler against missing SIGTSTP handleCtrlZ called process.kill(0, "SIGTSTP") unconditionally; on Windows the runtime rejects the signal name with TypeError, which propagated out of the TUI input dispatcher and crashed the agent with an [Uncaught Exception]. Even on POSIX the call could fail (sandboxes that block sending to pid=0, runtime-reserved signals), and a thrown error after we already stopped the TUI and registered a one-shot SIGCONT listener left the UI stranded with a leaked handler that would fire on the next unrelated continue and try to re-start() an already-running TUI. Branch on process.platform first so Windows shows a status notice and never calls process.kill, and wrap the POSIX kill in try/catch that removes the SIGCONT listener and re-starts the TUI on failure. Fixes #2036 --- packages/coding-agent/CHANGELOG.md | 4 + .../src/modes/controllers/input-controller.ts | 39 +++++- .../test/input-controller-suspend.test.ts | 128 ++++++++++++++++++ 3 files changed, 165 insertions(+), 6 deletions(-) create mode 100644 packages/coding-agent/test/input-controller-suspend.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 0243ed841..abe213a68 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed Ctrl+Z crashing the agent on Windows with `TypeError: Unknown signal: SIGTSTP`. `InputController.handleCtrlZ` called `process.kill(0, "SIGTSTP")` unconditionally, but `SIGTSTP` is POSIX job-control and Bun/Node on Windows rejects the signal name from the JS side; the throw propagated out of the TUI input dispatcher as an uncaught exception. The handler now no-ops with a "Suspend (Ctrl+Z) is not supported on this platform" status on Windows, and on POSIX wraps `process.kill` in a try/catch that detaches the registered SIGCONT resume hook and re-`start()`s the TUI on failure so a rejected signal can never leave the UI stranded with a leaked listener ([#2036](https://github.com/can1357/oh-my-pi/issues/2036)). + ## [15.10.1] - 2026-06-07 ### Added diff --git a/packages/coding-agent/src/modes/controllers/input-controller.ts b/packages/coding-agent/src/modes/controllers/input-controller.ts index da83ea41b..d00c66d86 100644 --- a/packages/coding-agent/src/modes/controllers/input-controller.ts +++ b/packages/coding-agent/src/modes/controllers/input-controller.ts @@ -499,17 +499,44 @@ export class InputController { } handleCtrlZ(): void { - // Set up handler to restore TUI when resumed - process.once("SIGCONT", () => { + // SIGTSTP is POSIX job-control: Windows has no equivalent and + // `process.kill(_, "SIGTSTP")` throws `TypeError: Unknown signal: + // SIGTSTP` there, taking the whole agent down via an uncaught + // exception (issue #2036). No-op on platforms that cannot suspend. + if (process.platform === "win32") { + this.ctx.showStatus("Suspend (Ctrl+Z) is not supported on this platform"); + return; + } + + // Capture the listener so we can detach it if the signal never + // fires; otherwise a failed suspend would leave a stale SIGCONT + // handler that fires on the next unrelated continue and tries to + // re-`start()` an already-running TUI. + const onResume = (): void => { this.ctx.ui.start(); this.ctx.ui.requestRender(true); - }); + }; + process.once("SIGCONT", onResume); - // Stop the TUI (restore terminal to normal mode) + // Stop the TUI (restore terminal to normal mode) before sending the + // signal so the parent shell sees a sane terminal state. this.ctx.ui.stop(); - // Send SIGTSTP to process group (pid=0 means all processes in group) - process.kill(0, "SIGTSTP"); + try { + // pid=0 → entire foreground process group; the shell receives + // SIGTSTP and parks the job. + process.kill(0, "SIGTSTP"); + } catch (err) { + // Either the runtime refused the signal or the kernel rejected + // it (some sandboxes block sending to pid=0). Tear the resume + // hook down and bring the TUI back so the user is not stranded + // on a frozen prompt. + process.removeListener("SIGCONT", onResume); + this.ctx.ui.start(); + this.ctx.ui.requestRender(true); + const reason = err instanceof Error ? err.message : String(err); + this.ctx.showError(`Failed to suspend: ${reason}`); + } } handleDequeue(): void { diff --git a/packages/coding-agent/test/input-controller-suspend.test.ts b/packages/coding-agent/test/input-controller-suspend.test.ts new file mode 100644 index 000000000..73945e9c9 --- /dev/null +++ b/packages/coding-agent/test/input-controller-suspend.test.ts @@ -0,0 +1,128 @@ +import { afterEach, describe, expect, it, type Mock, vi } from "bun:test"; +import { InputController } from "../src/modes/controllers/input-controller"; +import type { InteractiveModeContext } from "../src/modes/types"; + +interface SuspendCtx { + ctx: InteractiveModeContext; + ui: { + start: Mock<() => void>; + stop: Mock<() => void>; + requestRender: Mock<(force?: boolean) => void>; + }; + showStatus: Mock<(message: string) => void>; + showError: Mock<(message: string) => void>; +} + +function createCtx(): SuspendCtx { + const ui = { + start: vi.fn(), + stop: vi.fn(), + requestRender: vi.fn(), + }; + const showStatus = vi.fn(); + const showError = vi.fn(); + const ctx = { + ui: ui as unknown as InteractiveModeContext["ui"], + showStatus, + showError, + } as unknown as InteractiveModeContext; + return { ctx, ui, showStatus, showError }; +} + +const originalPlatform = process.platform; + +function setPlatform(value: NodeJS.Platform): void { + Object.defineProperty(process, "platform", { value, configurable: true, writable: true }); +} + +afterEach(() => { + Object.defineProperty(process, "platform", { value: originalPlatform, configurable: true, writable: true }); + vi.restoreAllMocks(); + // Drop any SIGCONT listener a passing test left behind so a later test + // (or the next file) doesn't get spurious callbacks. + process.removeAllListeners("SIGCONT"); +}); + +describe("InputController.handleCtrlZ", () => { + it("no-ops on Windows so the unsupported SIGTSTP signal can't crash the process (#2036)", () => { + setPlatform("win32"); + const killSpy = vi.spyOn(process, "kill").mockImplementation(() => { + throw new Error("process.kill must not be called on win32"); + }); + const onceSpy = vi.spyOn(process, "once"); + const { ctx, ui, showStatus, showError } = createCtx(); + + const controller = new InputController(ctx); + expect(() => controller.handleCtrlZ()).not.toThrow(); + + expect(killSpy).not.toHaveBeenCalled(); + expect(onceSpy).not.toHaveBeenCalledWith("SIGCONT", expect.anything()); + expect(ui.stop).not.toHaveBeenCalled(); + expect(ui.start).not.toHaveBeenCalled(); + expect(showStatus).toHaveBeenCalledTimes(1); + expect(showStatus.mock.calls[0]?.[0]).toMatch(/not supported/i); + expect(showError).not.toHaveBeenCalled(); + }); + + it("sends SIGTSTP to the process group and registers a SIGCONT resume hook on POSIX", () => { + setPlatform("linux"); + const killSpy = vi.spyOn(process, "kill").mockImplementation(() => true); + const onceSpy = vi.spyOn(process, "once"); + const { ctx, ui, showError } = createCtx(); + + const controller = new InputController(ctx); + controller.handleCtrlZ(); + + // Resume hook registered BEFORE the signal is sent so a same-tick + // SIGCONT delivery can't race past us. + expect(onceSpy).toHaveBeenCalledWith("SIGCONT", expect.any(Function)); + const sigcontOrder = onceSpy.mock.invocationCallOrder[0] ?? Infinity; + const stopOrder = ui.stop.mock.invocationCallOrder[0] ?? Infinity; + const killOrder = killSpy.mock.invocationCallOrder[0] ?? Infinity; + expect(sigcontOrder).toBeLessThan(stopOrder); + expect(stopOrder).toBeLessThan(killOrder); + + expect(killSpy).toHaveBeenCalledTimes(1); + expect(killSpy).toHaveBeenCalledWith(0, "SIGTSTP"); + expect(ui.start).not.toHaveBeenCalled(); + expect(showError).not.toHaveBeenCalled(); + + // Simulating the kernel-delivered SIGCONT drives the TUI back up. + const resume = onceSpy.mock.calls.find(([sig]) => sig === "SIGCONT")?.[1] as (() => void) | undefined; + expect(resume).toBeDefined(); + resume?.(); + expect(ui.start).toHaveBeenCalledTimes(1); + expect(ui.requestRender).toHaveBeenCalledWith(true); + }); + + it("restores the TUI and drops the SIGCONT listener when process.kill rejects the signal", () => { + setPlatform("linux"); + const killSpy = vi.spyOn(process, "kill").mockImplementation(() => { + throw new Error("Unknown signal: SIGTSTP"); + }); + const onceSpy = vi.spyOn(process, "once"); + const removeSpy = vi.spyOn(process, "removeListener"); + const { ctx, ui, showError, showStatus } = createCtx(); + + const controller = new InputController(ctx); + // Critical contract: the failure must not bubble up to the caller — + // otherwise the TUI's stdin reader (which invoked us) crashes the + // whole process via `[Uncaught Exception]`. + expect(() => controller.handleCtrlZ()).not.toThrow(); + + // The exact listener we registered for SIGCONT is the one we + // remove; otherwise a leaked handler would fire on the next + // unrelated continue and re-`start()` an already-running TUI. + const registered = onceSpy.mock.calls.find(([sig]) => sig === "SIGCONT")?.[1]; + expect(registered).toBeDefined(); + expect(removeSpy).toHaveBeenCalledWith("SIGCONT", registered); + + expect(killSpy).toHaveBeenCalledTimes(1); + expect(ui.stop).toHaveBeenCalledTimes(1); + expect(ui.start).toHaveBeenCalledTimes(1); + expect(ui.requestRender).toHaveBeenCalledWith(true); + expect(showError).toHaveBeenCalledTimes(1); + expect(showError.mock.calls[0]?.[0]).toMatch(/Failed to suspend/); + expect(showStatus).not.toHaveBeenCalled(); + }); +}); From f31b060e150ee7d4cbfcd13e55e4e48c8b4a349e Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 7 Jun 2026 09:24:20 +0000 Subject: [PATCH 10/46] fix(tui): applied follow-up slash commands Route follow-up shortcut submissions through the builtin slash-command dispatcher before queueing them as deferred prompts. Added regression coverage for /goal set submitted through InputController.handleFollowUp while a stream is active. Fixes #2038 --- packages/coding-agent/CHANGELOG.md | 4 ++++ .../src/modes/controllers/input-controller.ts | 12 +++++++++- .../test/input-controller-skill-queue.test.ts | 22 ++++++++++++++++++- 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 0243ed841..7a3932802 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed follow-up shortcut submission of builtin slash commands so `/goal set ...` applies goal mode instead of queueing as plain text. + ## [15.10.1] - 2026-06-07 ### Added diff --git a/packages/coding-agent/src/modes/controllers/input-controller.ts b/packages/coding-agent/src/modes/controllers/input-controller.ts index da83ea41b..d1e0ef6ad 100644 --- a/packages/coding-agent/src/modes/controllers/input-controller.ts +++ b/packages/coding-agent/src/modes/controllers/input-controller.ts @@ -589,7 +589,7 @@ export class InputController { /** Send editor text as a follow-up message (queued behind current stream). */ async handleFollowUp(): Promise { - const text = this.ctx.editor.getText().trim(); + let text = this.ctx.editor.getText().trim(); if (!text) return; // Compaction first: while compacting, free text gets queued via @@ -603,6 +603,16 @@ export class InputController { return; } + const slashResult = await executeBuiltinSlashCommand(text, { + ctx: this.ctx, + }); + if (slashResult === true) { + return; + } + if (typeof slashResult === "string") { + text = slashResult; + } + // Skill commands invoke through the custom-message path regardless of // which keybinding submitted them. Enter routes them as `steer`; // Ctrl+Enter (this handler) routes them as `followUp`. diff --git a/packages/coding-agent/test/input-controller-skill-queue.test.ts b/packages/coding-agent/test/input-controller-skill-queue.test.ts index 93dbba9ef..36154cc31 100644 --- a/packages/coding-agent/test/input-controller-skill-queue.test.ts +++ b/packages/coding-agent/test/input-controller-skill-queue.test.ts @@ -71,6 +71,8 @@ function createStubInputControllerContext(opts: { skillCommands: Map {}); + const prompt = vi.fn(async (_text: string, _options?: unknown) => {}); + const handleGoalModeCommand = vi.fn(async (_rest?: string) => {}); const updatePendingMessagesDisplay = vi.fn(); const requestRender = vi.fn(); const showError = vi.fn(); @@ -86,9 +88,12 @@ function createStubInputControllerContext(opts: { skillCommands: Map unknown) => fn(), } as unknown as InteractiveModeContext; - return { ctx, editor, enqueueCustomMessageDisplay, promptCustomMessage }; + return { ctx, editor, enqueueCustomMessageDisplay, prompt, promptCustomMessage, handleGoalModeCommand }; } describe("InputController #invokeSkillCommand (E1-E3)", () => { @@ -166,6 +171,21 @@ describe("InputController #invokeSkillCommand (E1-E3)", () => { expect(messageArg.details.__pendingDisplayTag).toBe("sk-test-0"); }); + it("E2b: streaming follow-up applies builtin slash commands instead of queueing them", async () => { + const { ctx, editor, prompt, handleGoalModeCommand } = createStubInputControllerContext({ + skillCommands, + isStreaming: true, + }); + + const controller = new InputController(ctx); + editor.setText("/goal set Ship the release"); + await controller.handleFollowUp(); + + expect(handleGoalModeCommand).toHaveBeenCalledWith("set Ship the release"); + expect(prompt).not.toHaveBeenCalled(); + expect(editor.getText()).toBe(""); + }); + it("E3: not streaming -> enqueueCustomMessageDisplay NOT called and tag absent", async () => { const { ctx, editor, enqueueCustomMessageDisplay, promptCustomMessage } = createStubInputControllerContext({ skillCommands, From d5183a14f9b4a5655a94169a41c162027041d43b Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 7 Jun 2026 10:41:31 +0000 Subject: [PATCH 11/46] fix(tui): honored kitty reply when DA1 sentinel arrives first The kitty keyboard progressive-enhancement probe sends `CSI ? u CSI c` and used the DA1 reply as a fallback sentinel: if no kitty reply arrived the modifyOtherKeys fallback was engaged. The kitty handler then refused to enable kitty whenever modifyOtherKeysActive was already true, which silently dropped the reply on terminals that answer DA1 before CSI ? u (Superset / xterm-on-Electron). The protocol never enabled, so Shift+Enter was delivered as a bare \r and the input box submitted. Make the kitty reply authoritative regardless of ordering: drop the `!modifyOtherKeysActive` guard, and if the premature DA1 already engaged the modifyOtherKeys fallback, send `CSI > 4 ; 0 m` to disable it before pushing the kitty stack frame. Kitty is strictly preferred (per the existing #3259-fallback comment), so this is a clean cutover. Add a regression test that drives a real ProcessTerminal through the existing render harness across three orderings: kitty-then-DA1 (control), DA1-then-kitty (#2042 regression), and DA1-only (modifyOtherKeys fallback stays). Fixes #2042 --- packages/tui/CHANGELOG.md | 4 ++ packages/tui/src/terminal.ts | 11 ++- .../test/kitty-keyboard-da1-ordering.test.ts | 69 +++++++++++++++++++ 3 files changed, 83 insertions(+), 1 deletion(-) create mode 100644 packages/tui/test/kitty-keyboard-da1-ordering.test.ts diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index 56903eddc..b871a7aa9 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed the kitty keyboard progressive-enhancement probe to honor the `CSI ? u` reply even when the terminal answers the DA1 sentinel first. Previously the kitty reply was discarded once the DA1-driven `modifyOtherKeys` fallback engaged, so terminals like Superset/xterm-on-Electron stayed on the fallback and delivered Shift+Enter as a bare `\r` ([#2042](https://github.com/can1357/oh-my-pi/issues/2042)). + ## [15.10.1] - 2026-06-07 ### Breaking Changes diff --git a/packages/tui/src/terminal.ts b/packages/tui/src/terminal.ts index 2069f6370..5dd0314f0 100644 --- a/packages/tui/src/terminal.ts +++ b/packages/tui/src/terminal.ts @@ -504,11 +504,20 @@ export class ProcessTerminal implements Terminal { } const match = sequence.match(kittyResponsePattern); - if (match && !this.#modifyOtherKeysActive) { + if (match) { if (this.#modifyOtherKeysTimeout) { clearTimeout(this.#modifyOtherKeysTimeout); this.#modifyOtherKeysTimeout = undefined; } + // A DA1 sentinel that beat the kitty reply may have already + // engaged the modifyOtherKeys fallback (terminals such as + // Superset/xterm-on-Electron answer DA1 before `\x1b[?u`). + // Kitty is strictly preferred — undo the fallback so the two + // modes do not stack. See #2042. + if (this.#modifyOtherKeysActive) { + this.#safeWrite("\x1b[>4;0m"); + this.#modifyOtherKeysActive = false; + } // Any reply to `\x1b[?u` means the terminal speaks the kitty keyboard // protocol. The reported flag value is the *current* stack-top — fresh // terminals report 0 — so support is implied by the reply itself, not by diff --git a/packages/tui/test/kitty-keyboard-da1-ordering.test.ts b/packages/tui/test/kitty-keyboard-da1-ordering.test.ts new file mode 100644 index 000000000..0baf01f3c --- /dev/null +++ b/packages/tui/test/kitty-keyboard-da1-ordering.test.ts @@ -0,0 +1,69 @@ +import { afterEach, describe, expect, it } from "bun:test"; +import { + createProcessTerminalRenderHarness, + type ProcessTerminalRenderHarness, +} from "./process-terminal-render-harness"; + +// Progressive-enhancement probe ordering contract. omp sends `CSI ? u \\ CSI c` +// at startup: the kitty reply (`CSI ? u`) authoritatively says the +// terminal speaks the kitty keyboard protocol; the DA1 reply (`CSI ? ... c`) +// is only a sentinel that guarantees a reply even from terminals that ignore +// `CSI ? u`. Some terminals (Superset / xterm-on-Electron) answer DA1 first; +// the kitty reply must still be honored regardless of ordering. +describe("ProcessTerminal kitty keyboard progressive-enhancement ordering", () => { + let harness: ProcessTerminalRenderHarness | undefined; + + afterEach(() => { + harness?.dispose(); + harness = undefined; + }); + + it("enables kitty when the kitty reply arrives before the DA1 sentinel", async () => { + harness = createProcessTerminalRenderHarness(100, 30); + await harness.settle(); + expect(harness.writes.join("")).toContain("\x1b[?u\x1b[c"); + harness.writes.length = 0; + + await harness.feed("\x1b[?0u", "\x1b[?1;2c"); + + const out = harness.writes.join(""); + expect(harness.terminal.kittyProtocolActive).toBe(true); + expect(out).toContain("\x1b[>1u"); + expect(out).not.toContain("\x1b[>4;2m"); + }); + + it("enables kitty when the DA1 sentinel arrives before the kitty reply (#2042)", async () => { + harness = createProcessTerminalRenderHarness(100, 30); + await harness.settle(); + harness.writes.length = 0; + + // Superset/Electron-xterm answers DA1 before `CSI ? u`. The kitty reply + // must override the premature modifyOtherKeys fallback. + await harness.feed("\x1b[?1;2c", "\x1b[?0u"); + + const out = harness.writes.join(""); + expect(harness.terminal.kittyProtocolActive).toBe(true); + expect(out).toContain("\x1b[>1u"); + const enableIdx = out.indexOf("\x1b[>4;2m"); + const disableIdx = out.indexOf("\x1b[>4;0m"); + const kittyIdx = out.indexOf("\x1b[>1u"); + expect(enableIdx).toBeGreaterThanOrEqual(0); + expect(disableIdx).toBeGreaterThan(enableIdx); + expect(kittyIdx).toBeGreaterThan(enableIdx); + }); + + it("keeps the modifyOtherKeys fallback when only DA1 ever replies", async () => { + harness = createProcessTerminalRenderHarness(100, 30); + await harness.settle(); + harness.writes.length = 0; + + // Terminals that ignore `CSI ? u` answer DA1 only — modifyOtherKeys is + // the right answer there. + await harness.feed("\x1b[?1;2c"); + + const out = harness.writes.join(""); + expect(harness.terminal.kittyProtocolActive).toBe(false); + expect(out).toContain("\x1b[>4;2m"); + expect(out).not.toContain("\x1b[>1u"); + }); +}); From c3c4c2749dd24a10c403f5921ec0e3c8c6038a9d Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 7 Jun 2026 11:14:02 +0000 Subject: [PATCH 12/46] fix(tui): bounded oversized render rows Bounded the raw row span passed into TUI line fitting so ANSI-heavy subagent output cannot grow terminal write buffers independently of viewport width. Added issue #2045 regression coverage for pathological zero-width ANSI rows. Fixes #2045 --- packages/tui/CHANGELOG.md | 4 ++ packages/tui/src/tui.ts | 19 ++++- packages/tui/test/issue-2045-repro.test.ts | 83 ++++++++++++++++++++++ 3 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 packages/tui/test/issue-2045-repro.test.ts diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index 56903eddc..d0f709ad0 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Bounded TUI line fitting for oversized raw rows so ANSI-heavy subagent output cannot grow render buffers independently of the viewport ([#2045](https://github.com/can1357/oh-my-pi/issues/2045)). + ## [15.10.1] - 2026-06-07 ### Breaking Changes diff --git a/packages/tui/src/tui.ts b/packages/tui/src/tui.ts index e71ee6818..0d6fe6aee 100644 --- a/packages/tui/src/tui.ts +++ b/packages/tui/src/tui.ts @@ -47,6 +47,12 @@ const SEGMENT_RESET = "\x1b[0m"; const LINE_TERMINATOR = "\x1b[0m\x1b]8;;\x07"; const ERASE_LINE = "\x1b[2K"; const ERASE_TO_END_OF_LINE = "\x1b[K"; +// Bound the raw code-unit span handed to native width/truncation. A terminal +// row can only display `width` cells, so oversized component rows should not +// force proportional JS/native copies while deciding what the viewport shows. +const LINE_FIT_MIN_SOURCE_CODE_UNITS = 4096; +const LINE_FIT_MAX_SOURCE_CODE_UNITS = 65536; +const LINE_FIT_SOURCE_WIDTH_MULTIPLIER = 64; // Hide the hardware cursor before each paint/move write. Ghostty-style bar // cursors can otherwise leave visual afterimages while the TUI repaints the // row under a visible cursor. Paint writes also disable terminal autowrap: @@ -2478,7 +2484,8 @@ export class TUI extends Container { if (TERMINAL.isImageLine(raw)) { return { raw, width, line: raw }; } - const normalized = normalizeTerminalOutput(raw); + const source = this.#lineFitSource(raw, width); + const normalized = normalizeTerminalOutput(source); const asciiWidth = this.#ansiAsciiLineWidth(normalized, width); if ((asciiWidth ?? visibleWidth(normalized)) <= width) { return { raw, width, line: normalized }; @@ -2487,6 +2494,16 @@ export class TUI extends Container { return { raw, width, line }; } + #lineFitSource(raw: string, width: number): string { + const safeWidth = Number.isFinite(width) ? Math.max(1, Math.trunc(width)) : 1; + const maxSourceLength = Math.min( + LINE_FIT_MAX_SOURCE_CODE_UNITS, + Math.max(LINE_FIT_MIN_SOURCE_CODE_UNITS, safeWidth * LINE_FIT_SOURCE_WIDTH_MULTIPLIER), + ); + if (raw.length <= maxSourceLength) return raw; + return raw.slice(0, maxSourceLength) + SEGMENT_RESET; + } + #ansiAsciiLineWidth(line: string, maxWidth: number): number | undefined { let col = 0; for (let i = 0; i < line.length; ) { diff --git a/packages/tui/test/issue-2045-repro.test.ts b/packages/tui/test/issue-2045-repro.test.ts new file mode 100644 index 000000000..bf44eefcf --- /dev/null +++ b/packages/tui/test/issue-2045-repro.test.ts @@ -0,0 +1,83 @@ +import { describe, expect, it } from "bun:test"; +import { type Component, TUI } from "@oh-my-pi/pi-tui"; +import type { Terminal, TerminalAppearance } from "@oh-my-pi/pi-tui/terminal"; + +class CaptureTerminal implements Terminal { + writes: string[] = []; + #columns: number; + #rows: number; + + constructor(columns = 80, rows = 4) { + this.#columns = columns; + this.#rows = rows; + } + + get columns(): number { + return this.#columns; + } + + get rows(): number { + return this.#rows; + } + + get kittyProtocolActive(): boolean { + return false; + } + + get appearance(): TerminalAppearance | undefined { + return undefined; + } + + start(): void {} + stop(): void {} + async drainInput(): Promise {} + write(data: string): void { + this.writes.push(data); + } + moveBy(): void {} + hideCursor(): void {} + showCursor(): void {} + clearLine(): void {} + clearFromCursor(): void {} + clearScreen(): void {} + setTitle(): void {} + setProgress(): void {} + onAppearanceChange(): void {} +} + +class RawLinesComponent implements Component { + #lines: string[]; + + constructor(lines: string[]) { + this.#lines = lines; + } + + invalidate(): void {} + + render(): string[] { + return this.#lines; + } +} + +async function settle(): Promise { + await Bun.sleep(0); +} + +describe("issue #2045: renderer bounds oversized rows", () => { + it("clips pathological zero-width ANSI rows before building terminal writes", async () => { + const term = new CaptureTerminal(80, 4); + const tui = new TUI(term); + const line = `${"\x1b[31m".repeat(20_000)}payload`; + + tui.addChild(new RawLinesComponent([line])); + try { + tui.start(); + await settle(); + } finally { + tui.stop(); + } + + const renderedBytes = term.writes.join("").length; + expect(renderedBytes).toBeLessThan(12_000); + }); +}); From ee7e1e3ae33cb3cd4e733ff75d25544047f0ebb2 Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 7 Jun 2026 11:14:29 +0000 Subject: [PATCH 13/46] fix(tui): preserved tmux viewport on offscreen shrink Skipped repainting tmux panes when an offscreen shrink leaves the visible tail unchanged, and added regression coverage for the no-content-bytes contract. Fixes #2046 --- packages/tui/CHANGELOG.md | 2 ++ packages/tui/src/tui.ts | 15 ++++++++- packages/tui/test/render-regressions.test.ts | 34 ++++++++++++++++++++ 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index 56903eddc..3274aa894 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -29,6 +29,8 @@ ### Fixed +- Fixed tmux offscreen-shrink frames to skip repainting when the visible tail is unchanged, avoiding intermittent blank/refresh flashes in pane terminals ([#2046](https://github.com/can1357/oh-my-pi/issues/2046)). + - Fixed `Loader` text updates to skip identical messages and preserve the rendered `Text` cache instead of invalidating it every timer tick. - Fixed fullscreen overlay alt-frame rendering to reuse the current line-preparation path instead of calling removed fitting helpers. diff --git a/packages/tui/src/tui.ts b/packages/tui/src/tui.ts index e71ee6818..68dfc9d8e 100644 --- a/packages/tui/src/tui.ts +++ b/packages/tui/src/tui.ts @@ -2049,7 +2049,9 @@ export class TUI extends Container { newLines.length < this.#previousLines.length && naturalViewportTop !== prevViewportTop ) { - return { kind: "viewportRepaint" }; + return this.#bottomAnchoredViewportUnchanged(newLines, height) + ? { kind: "deferredMutation" } + : { kind: "viewportRepaint" }; } // Direct-input shrink can also move the natural viewport upward even when @@ -2437,6 +2439,17 @@ export class TUI extends Container { return { kind: "liveRegionPinned", appendFrom, appendTo, renderViewportTop }; } + #bottomAnchoredViewportUnchanged(newLines: string[], height: number): boolean { + const previousViewportTop = Math.max(0, this.#previousLines.length - height); + const newViewportTop = Math.max(0, newLines.length - height); + for (let row = 0; row < height; row++) { + if ((newLines[newViewportTop + row] ?? "") !== (this.#previousLines[previousViewportTop + row] ?? "")) { + return false; + } + } + return true; + } + #planDeferredTailRepaint(newLines: string[], prevViewportTop: number, height: number): RenderIntent { const row = prevViewportTop + height - 1; if (row < 0 || row >= this.#previousLines.length || newLines.length !== this.#previousLines.length) { diff --git a/packages/tui/test/render-regressions.test.ts b/packages/tui/test/render-regressions.test.ts index 79e6385f7..29ff72ace 100644 --- a/packages/tui/test/render-regressions.test.ts +++ b/packages/tui/test/render-regressions.test.ts @@ -1469,6 +1469,40 @@ describe("TUI terminal-state regressions", () => { }); }); + it("tmux: offscreen shrink preserving the visible tail emits no repaint bytes", async () => { + await withEnvPatch({ TMUX: "1", STY: undefined, ZELLIJ: undefined }, async () => { + const term = new UnknownViewportTerminal(40, 4, 10_000); + const tui = new TUI(term); + const component = new MutableLinesComponent([ + "old-0", + "remove-me", + "old-2", + "old-3", + "tail-0", + "tail-1", + "tail-2", + "tail-3", + ]); + tui.addChild(component); + + try { + tui.start(); + await settle(term); + expect(visible(term)).toEqual(["tail-0", "tail-1", "tail-2", "tail-3"]); + + const writes = captureWrites(term); + component.setLines(["old-0", "old-2", "old-3", "tail-0", "tail-1", "tail-2", "tail-3"]); + tui.requestRender(); + await settle(term); + + expect(visible(term)).toEqual(["tail-0", "tail-1", "tail-2", "tail-3"]); + expect(writes).toEqual([]); + } finally { + tui.stop(); + } + }); + }); + // Root cause family: the dirty/replay machinery assumes native scrollback // can be cleared and rebuilt, which is never true inside a multiplexer — // tmux owns pane history, reflows it on resize itself, and a "replay" can From 2b29fef9efe1a63dbee29f5a4e9cadcdcb706bc0 Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 7 Jun 2026 11:18:56 +0000 Subject: [PATCH 14/46] docs(tui): moved #2046 changelog entry under Unreleased --- packages/tui/CHANGELOG.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index 3274aa894..b53b77b51 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed tmux offscreen-shrink frames to skip repainting when the visible tail is unchanged, avoiding intermittent blank/refresh flashes in pane terminals ([#2046](https://github.com/can1357/oh-my-pi/issues/2046)). + ## [15.10.1] - 2026-06-07 ### Breaking Changes @@ -29,8 +33,6 @@ ### Fixed -- Fixed tmux offscreen-shrink frames to skip repainting when the visible tail is unchanged, avoiding intermittent blank/refresh flashes in pane terminals ([#2046](https://github.com/can1357/oh-my-pi/issues/2046)). - - Fixed `Loader` text updates to skip identical messages and preserve the rendered `Text` cache instead of invalidating it every timer tick. - Fixed fullscreen overlay alt-frame rendering to reuse the current line-preparation path instead of calling removed fitting helpers. From d78e29e17ada7ce6f04c69f5097389cc2512a30b Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 7 Jun 2026 11:20:41 +0000 Subject: [PATCH 15/46] fix(tui): preserved text after long ansi prefixes Scanned oversized render rows by ANSI sequence so bounded line fitting skips leading zero-width control spans without dropping subsequent visible cells. Extended issue #2045 regression coverage to assert visible text survives long SGR and OSC hyperlink prefixes. --- packages/tui/src/tui.ts | 50 +++++++++++++++++++++- packages/tui/test/issue-2045-repro.test.ts | 25 +++++++++-- 2 files changed, 71 insertions(+), 4 deletions(-) diff --git a/packages/tui/src/tui.ts b/packages/tui/src/tui.ts index 0d6fe6aee..d6104e2e5 100644 --- a/packages/tui/src/tui.ts +++ b/packages/tui/src/tui.ts @@ -2501,7 +2501,55 @@ export class TUI extends Container { Math.max(LINE_FIT_MIN_SOURCE_CODE_UNITS, safeWidth * LINE_FIT_SOURCE_WIDTH_MULTIPLIER), ); if (raw.length <= maxSourceLength) return raw; - return raw.slice(0, maxSourceLength) + SEGMENT_RESET; + + const chunks: string[] = []; + let emitted = 0; + for (let i = 0; i < raw.length && emitted < maxSourceLength; ) { + if (raw.charCodeAt(i) === 0x1b) { + const end = this.#ansiSequenceEnd(raw, i); + if (end === -1) break; + const sequenceLength = end - i; + if (emitted > 0 && sequenceLength <= maxSourceLength - emitted) { + chunks.push(raw.slice(i, end)); + emitted += sequenceLength; + } + i = end; + continue; + } + + const start = i; + const end = Math.min(raw.length, start + maxSourceLength - emitted); + while (i < end && raw.charCodeAt(i) !== 0x1b) i++; + if (i === start) break; + chunks.push(raw.slice(start, i)); + emitted += i - start; + } + + return chunks.join("") + SEGMENT_RESET; + } + + #ansiSequenceEnd(line: string, start: number): number { + const next = line.charCodeAt(start + 1); + if (next === 0x5b) { + let i = start + 2; + while (i < line.length) { + const final = line.charCodeAt(i); + if (final >= 0x40 && final <= 0x7e) return i + 1; + i++; + } + return -1; + } + if (next === 0x5d) { + let i = start + 2; + while (i < line.length) { + const osc = line.charCodeAt(i); + if (osc === 0x07) return i + 1; + if (osc === 0x1b && line.charCodeAt(i + 1) === 0x5c) return i + 2; + i++; + } + return -1; + } + return start + 2 <= line.length ? start + 2 : -1; } #ansiAsciiLineWidth(line: string, maxWidth: number): number | undefined { diff --git a/packages/tui/test/issue-2045-repro.test.ts b/packages/tui/test/issue-2045-repro.test.ts index bf44eefcf..04cf54912 100644 --- a/packages/tui/test/issue-2045-repro.test.ts +++ b/packages/tui/test/issue-2045-repro.test.ts @@ -64,7 +64,7 @@ async function settle(): Promise { } describe("issue #2045: renderer bounds oversized rows", () => { - it("clips pathological zero-width ANSI rows before building terminal writes", async () => { + it("preserves visible text after pathological zero-width ANSI prefixes", async () => { const term = new CaptureTerminal(80, 4); const tui = new TUI(term); const line = `${"\x1b[31m".repeat(20_000)}payload`; @@ -77,7 +77,26 @@ describe("issue #2045: renderer bounds oversized rows", () => { tui.stop(); } - const renderedBytes = term.writes.join("").length; - expect(renderedBytes).toBeLessThan(12_000); + const rendered = term.writes.join(""); + expect(rendered).toContain("payload"); + expect(rendered.length).toBeLessThan(12_000); + }); + + it("preserves visible text after oversized OSC hyperlink prefixes", async () => { + const term = new CaptureTerminal(80, 4); + const tui = new TUI(term); + const line = `\x1b]8;;https://example.com/${"a".repeat(70_000)}\x07link-label\x1b]8;;\x07`; + + tui.addChild(new RawLinesComponent([line])); + try { + tui.start(); + await settle(); + } finally { + tui.stop(); + } + + const rendered = term.writes.join(""); + expect(rendered).toContain("link-label"); + expect(rendered.length).toBeLessThan(12_000); }); }); From 4053fadf71949e2a65825b66f39cd49839f742c2 Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 7 Jun 2026 11:26:09 +0000 Subject: [PATCH 16/46] fix(tui): preserved osc66 spans in bounded fitting Detected OSC 66 text-sizing sequences in the bounded line fitter and included them whole so payloads (e.g. Markdown headings) are not skipped as zero-width. Added a regression assertion that OSC 66 visible cells survive the bound when the row begins with the sized span. --- packages/tui/src/tui.ts | 21 +++++++++++++++++++++ packages/tui/test/issue-2045-repro.test.ts | 18 ++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/packages/tui/src/tui.ts b/packages/tui/src/tui.ts index d6104e2e5..71055e653 100644 --- a/packages/tui/src/tui.ts +++ b/packages/tui/src/tui.ts @@ -2509,6 +2509,16 @@ export class TUI extends Container { const end = this.#ansiSequenceEnd(raw, i); if (end === -1) break; const sequenceLength = end - i; + if (this.#ansiSequenceHasVisiblePayload(raw, i)) { + // OSC 66 text-sizing spans carry their visible cells inside the + // OSC payload. Always include the whole sequence — splitting it + // would corrupt the terminator — and let the next loop iteration + // terminate on the budget overflow. + chunks.push(raw.slice(i, end)); + emitted += sequenceLength; + i = end; + continue; + } if (emitted > 0 && sequenceLength <= maxSourceLength - emitted) { chunks.push(raw.slice(i, end)); emitted += sequenceLength; @@ -2552,6 +2562,17 @@ export class TUI extends Container { return start + 2 <= line.length ? start + 2 : -1; } + #ansiSequenceHasVisiblePayload(line: string, start: number): boolean { + // OSC 66 (`\x1b]66;META;TEXT\x1b\\`) carries its visible cells inside the + // payload, mirroring the special case in {@link #ansiAsciiLineWidth}. + return ( + line.charCodeAt(start + 1) === 0x5d && + line.charCodeAt(start + 2) === 0x36 && + line.charCodeAt(start + 3) === 0x36 && + line.charCodeAt(start + 4) === 0x3b + ); + } + #ansiAsciiLineWidth(line: string, maxWidth: number): number | undefined { let col = 0; for (let i = 0; i < line.length; ) { diff --git a/packages/tui/test/issue-2045-repro.test.ts b/packages/tui/test/issue-2045-repro.test.ts index 04cf54912..1b839ae4a 100644 --- a/packages/tui/test/issue-2045-repro.test.ts +++ b/packages/tui/test/issue-2045-repro.test.ts @@ -99,4 +99,22 @@ describe("issue #2045: renderer bounds oversized rows", () => { expect(rendered).toContain("link-label"); expect(rendered.length).toBeLessThan(12_000); }); + + it("preserves OSC 66 text-sizing payloads at the start of long rows", async () => { + const term = new CaptureTerminal(80, 4); + const tui = new TUI(term); + const visibleText = "H".repeat(70); + const line = `\x1b]66;s=1;${visibleText}\x1b\\${"\x1b[31m".repeat(20_000)}`; + + tui.addChild(new RawLinesComponent([line])); + try { + tui.start(); + await settle(); + } finally { + tui.stop(); + } + + const rendered = term.writes.join(""); + expect(rendered).toContain(visibleText); + }); }); From 10fd51425bc679b2bd2390dd67fa731ab4875275 Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 7 Jun 2026 11:37:13 +0000 Subject: [PATCH 17/46] fix(ai): parsed minimax think tags across providers Promoted MiniMax model/provider detection into stream markup healing so OpenCode Zen MiniMax streams use the existing thinking-tag parser instead of exposing raw tags. Added a regression test for OpenCode Zen minimax-m3 chunks split across boundaries. Fixes #2049 --- .../ai/src/providers/openai-completions.ts | 5 +-- .../ai/src/utils/stream-markup-healing.ts | 7 +++- .../ai/test/stream-markup-healing.test.ts | 34 +++++++++++++++++++ 3 files changed, 41 insertions(+), 5 deletions(-) diff --git a/packages/ai/src/providers/openai-completions.ts b/packages/ai/src/providers/openai-completions.ts index e58e4b8fd..4816e492b 100644 --- a/packages/ai/src/providers/openai-completions.ts +++ b/packages/ai/src/providers/openai-completions.ts @@ -537,7 +537,6 @@ export const streamOpenAICompletions: StreamFunction<"openai-completions"> = ( } stream.push({ type: "start", partial: output }); - const parseMiniMaxThinkTags = model.provider === "minimax-code" || model.provider === "minimax-code-cn"; // Some OpenAI-compatible DeepSeek hosts (including NVIDIA NIM and DeepSeek's // native API) leak chat-template tool-call markers in `delta.content` even // though tool calls are also surfaced structurally. Strip the leaked markers @@ -678,9 +677,7 @@ export const streamOpenAICompletions: StreamFunction<"openai-completions"> = ( } }; - const streamMarkupHealingPattern = getStreamMarkupHealingPattern(model.provider, model.id, { - parseThinkingTags: parseMiniMaxThinkTags, - }); + const streamMarkupHealingPattern = getStreamMarkupHealingPattern(model.provider, model.id); const streamMarkupHealing = streamMarkupHealingPattern ? new StreamMarkupHealing({ pattern: streamMarkupHealingPattern }) : undefined; diff --git a/packages/ai/src/utils/stream-markup-healing.ts b/packages/ai/src/utils/stream-markup-healing.ts index 6cb1c6531..7114b9019 100644 --- a/packages/ai/src/utils/stream-markup-healing.ts +++ b/packages/ai/src/utils/stream-markup-healing.ts @@ -600,12 +600,17 @@ export function modelMayLeakDsmlToolCalls(provider: string, modelId: string): bo ); } +/** Cheap model/provider gate for MiniMax plain thinking tag leaks. */ +export function modelMayLeakThinkingTags(provider: string, modelId: string): boolean { + return /minimax/i.test(provider) || /minimax/i.test(modelId); +} + export function getStreamMarkupHealingPattern( provider: string, modelId: string, options?: { readonly parseThinkingTags?: boolean }, ): StreamMarkupHealingPattern | undefined { - if (options?.parseThinkingTags) return "thinking"; + if (options?.parseThinkingTags || modelMayLeakThinkingTags(provider, modelId)) return "thinking"; if (modelMayLeakKimiToolCalls(provider, modelId)) return "kimi"; if (modelMayLeakDsmlToolCalls(provider, modelId)) return "dsml"; return undefined; diff --git a/packages/ai/test/stream-markup-healing.test.ts b/packages/ai/test/stream-markup-healing.test.ts index 935eb9481..79b6b74b5 100644 --- a/packages/ai/test/stream-markup-healing.test.ts +++ b/packages/ai/test/stream-markup-healing.test.ts @@ -148,6 +148,7 @@ describe("StreamMarkupHealing pattern selection", () => { expect(getStreamMarkupHealingPattern("minimax-code", "MiniMax-M2.5", { parseThinkingTags: true })).toBe( "thinking", ); + expect(getStreamMarkupHealingPattern("opencode-zen", "minimax-m3")).toBe("thinking"); expect(getStreamMarkupHealingPattern("nanogpt", "deepseek/deepseek-v4-pro")).toBe("dsml"); expect(getStreamMarkupHealingPattern("ollama-cloud", "gpt-oss:120b")).toBeUndefined(); expect(getStreamMarkupHealingPattern("openai", "deepseek-v4-pro")).toBeUndefined(); @@ -582,6 +583,39 @@ describe("Ollama provider DSML envelope healing", () => { }); }); +describe("OpenAI completions MiniMax thinking healing", () => { + it("parses OpenCode Zen MiniMax think tags into a thinking block", async () => { + const model: Model<"openai-completions"> = { + id: "minimax-m3", + name: "MiniMax M3", + api: "openai-completions", + provider: "opencode-zen", + baseUrl: "https://opencode.ai/zen/v1", + reasoning: true, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 200_000, + maxTokens: 8_192, + }; + global.fetch = mockFetch([ + chunk(model.id, { content: "visible hidden reasoning" }), + chunk(model.id, { content: " answer" }), + chunk(model.id, {}, "stop"), + "[DONE]", + ]); + + const result = await streamOpenAICompletions(model, baseContext(), { apiKey: "test-key" }).result(); + + expect(result.content).toEqual([ + { type: "text", text: "visible " }, + { type: "thinking", thinking: "hidden reasoning", thinkingSignature: undefined }, + { type: "text", text: " answer" }, + ]); + }); +}); + describe("OpenAI completions provider DSML envelope healing", () => { it("heals the envelope into a structured tool call and suppresses leaked text", async () => { const model: Model<"openai-completions"> = { From be71d53842e7c7273288d4308eaa6ffdcc5ffe0f Mon Sep 17 00:00:00 2001 From: DarkPhilosophy <19309990+DarkPhilosophy@users.noreply.github.com> Date: Sun, 7 Jun 2026 15:08:47 +0300 Subject: [PATCH 18/46] fix(coding-agent): optimize orphaned tool-use stop guard --- packages/coding-agent/CHANGELOG.md | 4 ++ .../coding-agent/src/session/agent-session.ts | 42 +++++++++++-------- 2 files changed, 28 insertions(+), 18 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 0243ed841..321d5264a 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed Anthropic empty `toolUse` stops without tool calls corrupting session history by retrying them and removing orphaned turns even at the retry cap. + ## [15.10.1] - 2026-06-07 ### Added diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index f4558c2fd..deb63f71a 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -283,6 +283,11 @@ export type AgentSessionEventListener = (event: AgentSessionEvent) => void; export type AsyncJobSnapshotItem = Pick; const EMPTY_STOP_MAX_RETRIES = 3; +const NON_WHITESPACE_RE = /\S/; + +function hasNonWhitespace(value: string): boolean { + return NON_WHITESPACE_RE.test(value); +} export interface AsyncJobSnapshot { running: AsyncJobSnapshotItem[]; @@ -6558,25 +6563,26 @@ export class AgentSession { } #isEmptyAssistantStop(assistantMessage: AssistantMessage): boolean { - const { stopReason } = assistantMessage; - if (stopReason !== "stop" && stopReason !== "toolUse") return false; - - // Single pass over content; the three flags cover every emptiness rule below. - let hasText = false; - let hasThinking = false; - let hasToolCall = false; - for (const content of assistantMessage.content) { - if (content.type === "text") hasText ||= content.text.trim().length > 0; - else if (content.type === "thinking") hasThinking ||= content.thinking.trim().length > 0; - else if (content.type === "toolCall") hasToolCall = true; + switch (assistantMessage.stopReason) { + case "stop": + for (const content of assistantMessage.content) { + if (content.type === "toolCall") return false; + if (content.type === "text" && hasNonWhitespace(content.text)) return false; + if (content.type === "thinking" && hasNonWhitespace(content.thinking)) return false; + } + return true; + case "toolUse": + // An orphaned toolUse stop (no tool_use block) corrupts Anthropic history: + // a later tool_result has nothing to anchor to. Thinking alone cannot anchor + // a tool_result, so it does not rescue a toolUse stop here. + for (const content of assistantMessage.content) { + if (content.type === "toolCall") return false; + if (content.type === "text" && hasNonWhitespace(content.text)) return false; + } + return true; + default: + return false; } - - // An orphaned toolUse stop (no tool_use block) corrupts Anthropic history: - // a later tool_result has nothing to anchor to. Thinking alone cannot anchor - // a tool_result, so it does not rescue a toolUse stop here. - if (stopReason === "toolUse") return !hasText && !hasToolCall; - // A plain stop is empty only when it carries no usable content at all. - return !hasText && !hasThinking && !hasToolCall; } #emptyStopRetryReminder(): string { From 43477234e2eb93f80ea6f0f27e8a200552c92ef8 Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 7 Jun 2026 12:10:52 +0000 Subject: [PATCH 19/46] fix(tui): handle kitty osc 5522 dot-listing paste responses Kitty implements the OSC 5522 "list available MIME types" reply by sending one DATA packet whose `mime` field decodes to the literal `.` and whose payload carries the available types as a whitespace-separated list (see `fulfill_read_request` in kovidgoyal/kitty:kitty/clipboard.py when the requested MIME is `TARGETS_MIME = '.'`). The 5522-mode ancillary spec (rockorager.dev/misc/bracketed-paste-mime) instead encodes each available type as its own DATA packet with an empty payload. The TUI's EnhancedPasteController only honored the ancillary form. When Kitty delivered the dot-listing form for a plain-text paste, the parser pushed `.` into the candidate list, `choosePasteMime(["."])` matched nothing, and the editor surfaced "Clipboard paste has no supported text or image data" instead of inserting the text. Decode the payload as a UTF-8 whitespace-separated MIME list when the DATA packet's mime is the dot sentinel, then fall through to the existing per-type behavior. Add two regression tests covering the single-type and multi-type Kitty payload shapes. Fixes #2051 --- packages/coding-agent/CHANGELOG.md | 4 ++ .../coding-agent/src/utils/enhanced-paste.ts | 20 ++++++ .../test/utils/enhanced-paste.test.ts | 71 +++++++++++++++++++ 3 files changed, 95 insertions(+) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 0243ed841..018c3d42f 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed Kitty OSC 5522 paste rejecting plain text as "no supported text or image data": the listing parser now decodes the `mime="."` DATA payload (whitespace-separated MIME list) Kitty actually sends, in addition to the per-type DATA packets described by the ancillary 5522-mode spec ([#2051](https://github.com/can1357/oh-my-pi/issues/2051)) + ## [15.10.1] - 2026-06-07 ### Added diff --git a/packages/coding-agent/src/utils/enhanced-paste.ts b/packages/coding-agent/src/utils/enhanced-paste.ts index 16fdf416e..3a3e2a564 100644 --- a/packages/coding-agent/src/utils/enhanced-paste.ts +++ b/packages/coding-agent/src/utils/enhanced-paste.ts @@ -7,6 +7,8 @@ const PASTE_EVENT_NAME_BASE64 = Buffer.from("Paste event", "utf8").toString("bas const IMAGE_MIME_PRIORITY = ["image/png", "image/jpeg", "image/webp", "image/gif"] as const; const TEXT_MIME_TYPE = "text/plain"; +/** Kitty's "give me the list of available MIME types" sentinel — see `TARGETS_MIME` in `kitty/clipboard.py`. */ +const MIME_LISTING_TARGET = "."; type PasteReadKind = "image" | "text"; @@ -144,6 +146,24 @@ export class EnhancedPasteController { if (!mimeType) return; if (state.phase === "listing") { + // Kitty (as of writing) implements the "list available MIME types" + // response shape by sending a single DATA packet with `mime="."` and + // the available types packed into the payload as a whitespace- + // separated list (see `fulfill_read_request` in + // kovidgoyal/kitty:kitty/clipboard.py). The 5522-mode ancillary + // spec instead encodes each type as its own DATA packet with an + // empty payload. Support both — fall through to the per-packet + // form when the dot sentinel has no payload, or when the packet + // already names a concrete MIME type. + if (mimeType === MIME_LISTING_TARGET) { + if (!packet.payload) return; + const listing = decodeBase64Utf8(packet.payload); + if (!listing) return; + for (const candidate of listing.split(/\s+/)) { + if (candidate && candidate !== MIME_LISTING_TARGET) state.mimes.push(candidate); + } + return; + } state.mimes.push(mimeType); return; } diff --git a/packages/coding-agent/test/utils/enhanced-paste.test.ts b/packages/coding-agent/test/utils/enhanced-paste.test.ts index d319c1362..f1d71d76d 100644 --- a/packages/coding-agent/test/utils/enhanced-paste.test.ts +++ b/packages/coding-agent/test/utils/enhanced-paste.test.ts @@ -106,4 +106,75 @@ describe("EnhancedPasteController", () => { expect(statuses).toEqual(["Clipboard paste has no supported text or image data"]); }); + + it("decodes Kitty's dot-listing DATA payload to discover plain-text and request it", () => { + const writes: string[] = []; + const pastedText: string[] = []; + const controller = new EnhancedPasteController({ + write: data => writes.push(data), + pasteText: text => pastedText.push(text), + pasteImage: () => { + throw new Error("unexpected image paste"); + }, + showStatus: message => pastedText.push(`status:${message}`), + }); + + const dot = Buffer.from(".", "utf8").toString("base64"); + const textMime = Buffer.from("text/plain", "utf8").toString("base64"); + const password = Buffer.from("secret-token-123", "utf8").toString("base64"); + const pasteEventName = Buffer.from("Paste event", "utf8").toString("base64"); + + // Kitty bundles the available MIME types into a single DATA packet + // whose `mime` field is the literal `.` and whose payload carries a + // whitespace-separated, base64-encoded list (e.g. "text/plain\n"). + controller.handleInput(packet(`type=read:status=OK:pw=${password}`)); + controller.handleInput( + packet( + `type=read:status=DATA:mime=${dot}:pw=${password}`, + Buffer.from("text/plain\n", "utf8").toString("base64"), + ), + ); + controller.handleInput(packet(`type=read:status=DONE:pw=${password}`)); + + expect(writes.at(-1)).toBe( + `${OSC}type=read:mime=${textMime}:pw=${password}:name=${pasteEventName}${ST}`, + ); + + controller.handleInput(packet("type=read:status=OK")); + controller.handleInput( + packet(`type=read:status=DATA:mime=${textMime}`, Buffer.from("hello", "utf8").toString("base64")), + ); + controller.handleInput( + packet(`type=read:status=DATA:mime=${textMime}`, Buffer.from(" world", "utf8").toString("base64")), + ); + controller.handleInput(packet("type=read:status=DONE")); + + expect(pastedText).toEqual(["hello world"]); + }); + + it("prefers images when Kitty's dot-listing payload advertises multiple MIME types", () => { + const writes: string[] = []; + const controller = new EnhancedPasteController({ + write: data => writes.push(data), + pasteText: () => { + throw new Error("unexpected text paste"); + }, + pasteImage: () => {}, + showStatus: () => {}, + }); + + const dot = Buffer.from(".", "utf8").toString("base64"); + const imageMime = Buffer.from("image/png", "utf8").toString("base64"); + + controller.handleInput(packet("type=read:status=OK")); + controller.handleInput( + packet( + `type=read:status=DATA:mime=${dot}`, + Buffer.from("text/plain image/png text/html\n", "utf8").toString("base64"), + ), + ); + controller.handleInput(packet("type=read:status=DONE")); + + expect(writes.at(-1)).toContain(`mime=${imageMime}`); + }); }); From 29ca22e9c7602a9e16003dcc45ab1e0a555e163b Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 7 Jun 2026 12:34:33 +0000 Subject: [PATCH 20/46] fix(ai): deduplicated replayed tool call ids Added a transform pass that rewrites repeated tool call ids and the immediately paired tool result so provider replay keeps a one-result-per-call contract. Covered missing-result synthesis and generated-id collision regressions.\n\nFixes #2055 --- packages/ai/CHANGELOG.md | 1 + .../ai/src/providers/transform-messages.ts | 273 +++++++++++------- .../ai/test/duplicate-tool-results.test.ts | 97 +++++++ 3 files changed, 264 insertions(+), 107 deletions(-) diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index 344d76b9a..cdff682db 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -21,6 +21,7 @@ ### Fixed +- Fixed duplicate upstream `tool_call_id` values collapsing distinct tool calls during message transformation, preserving one call/result pairing per emitted tool call before provider replay. ([#2055](https://github.com/can1357/oh-my-pi/issues/2055)) - Fixed streaming auth retries to handle `401` and usage-limit errors before replay-unsafe content is emitted, including failures surfaced only via `errorStatus` - Fixed tool argument validation to coerce singleton non-string values into arrays when the schema expects an array, preventing Anthropic-compatible models that emit `todo.ops` as an object from getting stuck in repeated validation-error loops. ([#2026](https://github.com/can1357/oh-my-pi/issues/2026)) - Fixed streaming retries to buffer and suppress partial `start` events from failed auth attempts so only clean retried events are delivered diff --git a/packages/ai/src/providers/transform-messages.ts b/packages/ai/src/providers/transform-messages.ts index 7959f7828..dc2691467 100644 --- a/packages/ai/src/providers/transform-messages.ts +++ b/packages/ai/src/providers/transform-messages.ts @@ -17,6 +17,61 @@ const enum ToolCallStatus { Aborted = 2, } +type PendingToolResultRewrite = { originalId: string; replacementId: string } | undefined; + +function deduplicateToolCallIds(messages: Message[]): Message[] { + const seenToolCallIds = new Map(); + let pendingToolResultRewrites: PendingToolResultRewrite[] = []; + let pendingToolResultRewriteIndex = 0; + + return messages.map(msg => { + if (msg.role === "toolResult") { + const rewrite = pendingToolResultRewrites[pendingToolResultRewriteIndex]; + pendingToolResultRewriteIndex += 1; + if (pendingToolResultRewriteIndex >= pendingToolResultRewrites.length) { + pendingToolResultRewrites = []; + pendingToolResultRewriteIndex = 0; + } + if (rewrite && msg.toolCallId === rewrite.originalId) return { ...msg, toolCallId: rewrite.replacementId }; + return msg; + } + + pendingToolResultRewrites = []; + pendingToolResultRewriteIndex = 0; + if (msg.role !== "assistant") return msg; + + let contentChanged = false; + const nextToolResultRewrites: PendingToolResultRewrite[] = []; + const content = msg.content.map(block => { + if (block.type !== "toolCall") return block; + + const previousCount = seenToolCallIds.get(block.id) ?? 0; + if (previousCount === 0) { + seenToolCallIds.set(block.id, 1); + nextToolResultRewrites.push(undefined); + return block; + } + + let duplicateIndex = previousCount; + let replacementId = `${block.id}_dup${duplicateIndex}`; + while (seenToolCallIds.has(replacementId)) { + duplicateIndex += 1; + replacementId = `${block.id}_dup${duplicateIndex}`; + } + seenToolCallIds.set(block.id, duplicateIndex + 1); + seenToolCallIds.set(replacementId, 1); + nextToolResultRewrites.push({ originalId: block.id, replacementId }); + contentChanged = true; + return { ...block, id: replacementId }; + }); + + if (!contentChanged) return msg; + pendingToolResultRewrites = nextToolResultRewrites; + pendingToolResultRewriteIndex = 0; + return { ...msg, content }; + }); +} + function shouldDropTruncatedThinkingOnlyAssistant(msg: AssistantMessage): boolean { const isTruncatedStop = msg.stopReason === "length" || msg.stopReason === "error" || msg.stopReason === "aborted"; return isTruncatedStop && !msg.content.some(block => block.type === "toolCall" || block.type === "text"); @@ -52,116 +107,120 @@ export function transformMessages( const latestSurvivingAssistantIndex = getLatestSurvivingAssistantIndex(messages); // First pass: transform messages (thinking blocks, tool call ID normalization) - const transformed = messages.map((msg, index) => { - // User and developer messages pass through unchanged - if (msg.role === "user" || msg.role === "developer") { - return msg; - } + const transformed = deduplicateToolCallIds( + messages.map((msg, index) => { + // User and developer messages pass through unchanged + if (msg.role === "user" || msg.role === "developer") { + return msg; + } - // Handle toolResult messages - normalize toolCallId if we have a mapping - if (msg.role === "toolResult") { - const normalizedId = toolCallIdMap.get(msg.toolCallId); - if (normalizedId && normalizedId !== msg.toolCallId) { - return { ...msg, toolCallId: normalizedId }; + // Handle toolResult messages - normalize toolCallId if we have a mapping + if (msg.role === "toolResult") { + const normalizedId = toolCallIdMap.get(msg.toolCallId); + if (normalizedId && normalizedId !== msg.toolCallId) { + return { ...msg, toolCallId: normalizedId }; + } + return msg; + } + + // Assistant messages need transformation check + if (msg.role === "assistant") { + const assistantMsg = msg as AssistantMessage; + const isSameModel = + assistantMsg.provider === model.provider && + assistantMsg.api === model.api && + assistantMsg.model === model.id; + + const mustPreserveLatestAnthropicThinking = + index === latestSurvivingAssistantIndex && + model.api === "anthropic-messages" && + assistantMsg.api === "anthropic-messages"; + // Aborted/errored messages may have partially-streamed thinking signatures. + // A partial signature is invalid and will be rejected by the API, so we must + // strip signatures from thinking blocks in these messages. + // + // Abandoned tool-use turns get the same treatment once they are no longer + // the latest assistant message. When a turn carries toolCall blocks but did + // NOT request tool execution (stopReason !== "toolUse" — e.g. + // adaptive-thinking Opus emitting tool calls and then ending the turn on + // `end_turn`/`stop`), the agent loop pairs those calls with placeholder + // tool_results to keep the tool_use/tool_result contract valid. Historical + // abandoned turns cannot safely replay their end_turn-bound signatures in + // that continuation, so stripping downgrades them to plain text downstream. + // Latest abandoned turns are exempt because Anthropic requires thinking + // blocks from its most recent response to remain byte-for-byte unmodified. + const invalidStopReason = assistantMsg.stopReason === "aborted" || assistantMsg.stopReason === "error"; + const abandonedToolUse = + !invalidStopReason && + assistantMsg.stopReason !== "toolUse" && + assistantMsg.content.some(b => b.type === "toolCall"); + const hasInvalidSignatures = invalidStopReason || abandonedToolUse; + + const transformedContent = assistantMsg.content.flatMap(block => { + if (block.type === "thinking") { + // Strip untrustworthy signatures so the encoder can downgrade to text. + const sanitized = + hasInvalidSignatures && block.thinkingSignature + ? { ...block, thinkingSignature: undefined } + : block; + if (mustPreserveLatestAnthropicThinking) return abandonedToolUse ? block : sanitized; + // For same model: keep thinking blocks with signatures (needed for replay) + // even if the thinking text is empty (OpenAI encrypted reasoning) + if (isSameModel && sanitized.thinkingSignature) return sanitized; + // Skip empty thinking blocks, convert others to plain text + if (!sanitized.thinking || sanitized.thinking.trim() === "") return []; + if (isSameModel) return sanitized; + return { + type: "text" as const, + text: sanitized.thinking, + }; + } + + if (block.type === "redactedThinking") { + if (mustPreserveLatestAnthropicThinking) return block; + if (isSameModel) return block; + return []; + } + + if (block.type === "text") { + if (isSameModel) return block; + return { + type: "text" as const, + text: block.text, + }; + } + + if (block.type === "toolCall") { + const toolCall = block as ToolCall; + let normalizedToolCall: ToolCall = toolCall; + + if (!isSameModel && toolCall.thoughtSignature) { + normalizedToolCall = { ...toolCall }; + delete (normalizedToolCall as { thoughtSignature?: string }).thoughtSignature; + } + + if (!isSameModel && normalizeToolCallId) { + const normalizedId = normalizeToolCallId(toolCall.id, model, assistantMsg); + if (normalizedId !== toolCall.id) { + toolCallIdMap.set(toolCall.id, normalizedId); + normalizedToolCall = { ...normalizedToolCall, id: normalizedId }; + } + } + + return normalizedToolCall; + } + + return block; + }); + + return { + ...assistantMsg, + content: transformedContent, + }; } return msg; - } - - // Assistant messages need transformation check - if (msg.role === "assistant") { - const assistantMsg = msg as AssistantMessage; - const isSameModel = - assistantMsg.provider === model.provider && - assistantMsg.api === model.api && - assistantMsg.model === model.id; - - const mustPreserveLatestAnthropicThinking = - index === latestSurvivingAssistantIndex && - model.api === "anthropic-messages" && - assistantMsg.api === "anthropic-messages"; - // Aborted/errored messages may have partially-streamed thinking signatures. - // A partial signature is invalid and will be rejected by the API, so we must - // strip signatures from thinking blocks in these messages. - // - // Abandoned tool-use turns get the same treatment once they are no longer - // the latest assistant message. When a turn carries toolCall blocks but did - // NOT request tool execution (stopReason !== "toolUse" — e.g. - // adaptive-thinking Opus emitting tool calls and then ending the turn on - // `end_turn`/`stop`), the agent loop pairs those calls with placeholder - // tool_results to keep the tool_use/tool_result contract valid. Historical - // abandoned turns cannot safely replay their end_turn-bound signatures in - // that continuation, so stripping downgrades them to plain text downstream. - // Latest abandoned turns are exempt because Anthropic requires thinking - // blocks from its most recent response to remain byte-for-byte unmodified. - const invalidStopReason = assistantMsg.stopReason === "aborted" || assistantMsg.stopReason === "error"; - const abandonedToolUse = - !invalidStopReason && - assistantMsg.stopReason !== "toolUse" && - assistantMsg.content.some(b => b.type === "toolCall"); - const hasInvalidSignatures = invalidStopReason || abandonedToolUse; - - const transformedContent = assistantMsg.content.flatMap(block => { - if (block.type === "thinking") { - // Strip untrustworthy signatures so the encoder can downgrade to text. - const sanitized = - hasInvalidSignatures && block.thinkingSignature ? { ...block, thinkingSignature: undefined } : block; - if (mustPreserveLatestAnthropicThinking) return abandonedToolUse ? block : sanitized; - // For same model: keep thinking blocks with signatures (needed for replay) - // even if the thinking text is empty (OpenAI encrypted reasoning) - if (isSameModel && sanitized.thinkingSignature) return sanitized; - // Skip empty thinking blocks, convert others to plain text - if (!sanitized.thinking || sanitized.thinking.trim() === "") return []; - if (isSameModel) return sanitized; - return { - type: "text" as const, - text: sanitized.thinking, - }; - } - - if (block.type === "redactedThinking") { - if (mustPreserveLatestAnthropicThinking) return block; - if (isSameModel) return block; - return []; - } - - if (block.type === "text") { - if (isSameModel) return block; - return { - type: "text" as const, - text: block.text, - }; - } - - if (block.type === "toolCall") { - const toolCall = block as ToolCall; - let normalizedToolCall: ToolCall = toolCall; - - if (!isSameModel && toolCall.thoughtSignature) { - normalizedToolCall = { ...toolCall }; - delete (normalizedToolCall as { thoughtSignature?: string }).thoughtSignature; - } - - if (!isSameModel && normalizeToolCallId) { - const normalizedId = normalizeToolCallId(toolCall.id, model, assistantMsg); - if (normalizedId !== toolCall.id) { - toolCallIdMap.set(toolCall.id, normalizedId); - normalizedToolCall = { ...normalizedToolCall, id: normalizedId }; - } - } - - return normalizedToolCall; - } - - return block; - }); - - return { - ...assistantMsg, - content: transformedContent, - }; - } - return msg; - }); + }), + ); const realToolResultsById = new Map(); for (const msg of transformed) { if (msg.role === "toolResult" && !realToolResultsById.has(msg.toolCallId)) { diff --git a/packages/ai/test/duplicate-tool-results.test.ts b/packages/ai/test/duplicate-tool-results.test.ts index 1be33d66b..2c196d9c7 100644 --- a/packages/ai/test/duplicate-tool-results.test.ts +++ b/packages/ai/test/duplicate-tool-results.test.ts @@ -32,6 +32,43 @@ describe("Duplicate Tool Results Regression", () => { reasoning: true, }; + const makeEvalAssistantMessage = (id: string, timestamp: number): AssistantMessage => ({ + role: "assistant", + content: [{ type: "toolCall", id, name: "eval", arguments: {} }], + api: "anthropic-messages", + provider: "anthropic", + model: "claude-3-5-sonnet-20241022", + usage: { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 0, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + stopReason: "toolUse", + timestamp, + }); + + const makeEvalToolResult = (id: string, text: string, timestamp: number): ToolResultMessage => ({ + role: "toolResult", + toolCallId: id, + toolName: "eval", + content: [{ type: "text", text }], + isError: false, + timestamp, + }); + + const getAssistantToolIds = (messages: Message[]): string[] => + messages.flatMap(message => + message.role === "assistant" + ? message.content.filter((block): block is ToolCall => block.type === "toolCall").map(block => block.id) + : [], + ); + + const getToolResults = (messages: Message[]): ToolResultMessage[] => + messages.filter((message): message is ToolResultMessage => message.role === "toolResult"); + it("should not duplicate tool results for errored messages when results already exist", () => { const toolCallId = "toolu_019xqMTvqWZiTDy8XxmjxrTo"; @@ -316,6 +353,66 @@ describe("Duplicate Tool Results Regression", () => { expect(result2.length).toBe(1); expect(result3.length).toBe(1); }); + + it("deduplicates repeated tool call ids and preserves call/result pairing", () => { + const duplicateId = "functions.eval:301"; + const distinctId = "functions.eval:302"; + + const messages: Message[] = [ + makeEvalAssistantMessage(duplicateId, 1), + makeEvalToolResult(duplicateId, "first", 2), + makeEvalAssistantMessage(duplicateId, 3), + makeEvalToolResult(duplicateId, "second", 4), + makeEvalAssistantMessage(duplicateId, 5), + makeEvalAssistantMessage(distinctId, 6), + makeEvalToolResult(distinctId, "third", 7), + ]; + + const transformed = transformMessages(messages, model); + const assistantToolIds = getAssistantToolIds(transformed); + const toolResults = getToolResults(transformed); + + expect(assistantToolIds).toEqual([duplicateId, `${duplicateId}_dup1`, `${duplicateId}_dup2`, distinctId]); + expect(toolResults.map(result => result.toolCallId)).toEqual([ + duplicateId, + `${duplicateId}_dup1`, + `${duplicateId}_dup2`, + distinctId, + ]); + expect(toolResults.find(result => result.toolCallId === `${duplicateId}_dup1`)?.content).toEqual([ + { type: "text", text: "second" }, + ]); + expect(toolResults.find(result => result.toolCallId === `${duplicateId}_dup2`)?.content).toEqual([ + { type: "text", text: "No result provided" }, + ]); + }); + + it("deduplicates repeated ids without colliding with existing generated-looking ids", () => { + const duplicateId = "functions.eval:301"; + const generatedLookingId = `${duplicateId}_dup1`; + const messages: Message[] = [ + makeEvalAssistantMessage(duplicateId, 1), + makeEvalToolResult(duplicateId, "first", 2), + makeEvalAssistantMessage(generatedLookingId, 3), + makeEvalToolResult(generatedLookingId, "already-used", 4), + makeEvalAssistantMessage(duplicateId, 5), + makeEvalToolResult(duplicateId, "second", 6), + ]; + + const transformed = transformMessages(messages, model); + const assistantToolIds = getAssistantToolIds(transformed); + const toolResults = getToolResults(transformed); + + expect(assistantToolIds).toEqual([duplicateId, generatedLookingId, `${duplicateId}_dup2`]); + expect(toolResults.map(result => result.toolCallId)).toEqual([ + duplicateId, + generatedLookingId, + `${duplicateId}_dup2`, + ]); + expect(toolResults.find(result => result.toolCallId === `${duplicateId}_dup2`)?.content).toEqual([ + { type: "text", text: "second" }, + ]); + }); }); /** From 56240e527d7aec96a8de53b44ddcd205d7bd2171 Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 7 Jun 2026 12:36:58 +0000 Subject: [PATCH 21/46] fix(agent): retried generic upstream gateway failures Classified generic upstream gateway failures as transient so configured session auto-retry starts when auth-gateway surfaces upstream_error: Upstream request failed. Added a focused AgentSession regression covering the retry event and recovery path.\n\nFixes #2056 --- packages/coding-agent/CHANGELOG.md | 2 + .../coding-agent/src/session/agent-session.ts | 7 ++- .../test/agent-session-retry-cap.test.ts | 56 +++++++++++++++++++ 3 files changed, 62 insertions(+), 3 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 0243ed841..cf6b9ecde 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -50,6 +50,8 @@ ### Fixed +- Fixed session auto-retry for generic `upstream_error: Upstream request failed` gateway failures. + - Fixed inline `find` and `search` result blocks to align with grouped `read` output and render their success headers with the normal tool-title color instead of accent blue. - Fixed the working-status shimmer to opt into the loader's 30fps animated-message repaint path while keeping both the status spinner and pending bash/eval tool spinners on their normal 80 ms glyph cadence. diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index b53734c6f..62c443541 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -7874,11 +7874,12 @@ export class AgentSession { #isTransientTransportErrorMessage(errorMessage: string): boolean { // Match: overloaded_error, provider returned error, rate limit, 429, 500, 502, 503, 504, // service unavailable, provider-suggested retry, network/connection/socket errors, fetch failed, - // terminated, retry delay exceeded, Bun HTTP/2 stream resets (RST_STREAM / REFUSED_STREAM / - // ENHANCE_YOUR_CALM, surfaced verbatim from src/http/h2_client/dispatch.zig) + // gateway upstream failures, terminated, retry delay exceeded, Bun HTTP/2 stream resets + // (RST_STREAM / REFUSED_STREAM / ENHANCE_YOUR_CALM, surfaced verbatim from + // src/http/h2_client/dispatch.zig) return ( isUnexpectedSocketCloseMessage(errorMessage) || - /overloaded|provider.?returned.?error|rate.?limit|too many requests|429|500|502|503|504|service.?unavailable|server.?error|internal.?error|retry your request|network.?error|connection.?error|connection.?refused|other side closed|fetch failed|upstream.?connect|reset before headers|socket hang up|timed? out|timeout|terminated|retry delay|stream stall|no error details in response|HTTP2(?:StreamReset|RefusedStream|EnhanceYourCalm)/i.test( + /overloaded|provider.?returned.?error|rate.?limit|too many requests|429|500|502|503|504|service.?unavailable|server.?error|internal.?error|retry your request|network.?error|connection.?error|connection.?refused|other side closed|fetch failed|upstream.?connect|upstream.?request.?failed|reset before headers|socket hang up|timed? out|timeout|terminated|retry delay|stream stall|no error details in response|HTTP2(?:StreamReset|RefusedStream|EnhanceYourCalm)/i.test( errorMessage, ) ); diff --git a/packages/coding-agent/test/agent-session-retry-cap.test.ts b/packages/coding-agent/test/agent-session-retry-cap.test.ts index f7b04d26c..465b80e13 100644 --- a/packages/coding-agent/test/agent-session-retry-cap.test.ts +++ b/packages/coding-agent/test/agent-session-retry-cap.test.ts @@ -336,4 +336,60 @@ describe("AgentSession retry delay cap", () => { const last = lastAssistant(session); expect(last.stopReason).toBe("stop"); }); + it("retries generic upstream_error gateway failures", async () => { + const model = getBundledModel("anthropic", "claude-sonnet-4-5"); + if (!model) { + throw new Error("Expected bundled Anthropic test model to exist"); + } + + const mock = createMockModel({ + responses: [ + { throw: "upstream_error: Upstream request failed" }, + { content: ["recovered after generic gateway upstream error"] }, + ], + }); + const agent = new Agent({ + getApiKey: provider => `${provider}-test-key`, + initialState: { + model, + systemPrompt: ["Test"], + tools: [], + messages: [], + }, + streamFn: mock.stream, + }); + + const settings = Settings.isolated({ + "compaction.enabled": false, + "retry.baseDelayMs": 5, + "retry.maxDelayMs": 5_000, + "retry.maxRetries": 1, + }); + settings.setModelRole("default", `${model.provider}/${model.id}`); + + session = new AgentSession({ + agent, + sessionManager: SessionManager.inMemory(), + settings, + modelRegistry, + }); + + vi.spyOn(scheduler, "wait").mockResolvedValue(undefined); + const retryStartEvents: AutoRetryStartEvent[] = []; + const retryEndEvents: AutoRetryEndEvent[] = []; + session.subscribe(event => { + if (event.type === "auto_retry_start") retryStartEvents.push(event); + if (event.type === "auto_retry_end") retryEndEvents.push(event); + }); + + await session.prompt("Trigger generic upstream_error"); + await session.waitForIdle(); + + expect(retryStartEvents).toHaveLength(1); + expect(retryEndEvents).toHaveLength(1); + expect(retryEndEvents[0]).toMatchObject({ success: true, attempt: 1 }); + const last = lastAssistant(session); + expect(last.stopReason).toBe("stop"); + expect(last.content).toContainEqual({ type: "text", text: "recovered after generic gateway upstream error" }); + }); }); From f04cca885eeb62904470ac6fbe0ae56f1166e7dc Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 7 Jun 2026 12:38:35 +0000 Subject: [PATCH 22/46] fix(ai): preserved delayed duplicate tool outputs Kept duplicate tool-call result rewrites pending by original id across user/developer gaps so delayed real outputs are paired with the rewritten call instead of being dropped.\n\nFixes #2055 --- .../ai/src/providers/transform-messages.ts | 36 ++++++++++--------- .../ai/test/duplicate-tool-results.test.ts | 21 +++++++++++ 2 files changed, 40 insertions(+), 17 deletions(-) diff --git a/packages/ai/src/providers/transform-messages.ts b/packages/ai/src/providers/transform-messages.ts index dc2691467..5c9b56f07 100644 --- a/packages/ai/src/providers/transform-messages.ts +++ b/packages/ai/src/providers/transform-messages.ts @@ -17,38 +17,42 @@ const enum ToolCallStatus { Aborted = 2, } -type PendingToolResultRewrite = { originalId: string; replacementId: string } | undefined; +type PendingToolResultRewrite = { replacementId: string } | undefined; function deduplicateToolCallIds(messages: Message[]): Message[] { const seenToolCallIds = new Map(); - let pendingToolResultRewrites: PendingToolResultRewrite[] = []; - let pendingToolResultRewriteIndex = 0; + const pendingToolResultRewrites = new Map(); return messages.map(msg => { if (msg.role === "toolResult") { - const rewrite = pendingToolResultRewrites[pendingToolResultRewriteIndex]; - pendingToolResultRewriteIndex += 1; - if (pendingToolResultRewriteIndex >= pendingToolResultRewrites.length) { - pendingToolResultRewrites = []; - pendingToolResultRewriteIndex = 0; - } - if (rewrite && msg.toolCallId === rewrite.originalId) return { ...msg, toolCallId: rewrite.replacementId }; + const rewrites = pendingToolResultRewrites.get(msg.toolCallId); + if (!rewrites || rewrites.length === 0) return msg; + + const rewrite = rewrites.shift(); + if (rewrites.length === 0) pendingToolResultRewrites.delete(msg.toolCallId); + if (rewrite) return { ...msg, toolCallId: rewrite.replacementId }; return msg; } - pendingToolResultRewrites = []; - pendingToolResultRewriteIndex = 0; if (msg.role !== "assistant") return msg; + const enqueueToolResultRewrite = (id: string, rewrite: PendingToolResultRewrite): void => { + const rewrites = pendingToolResultRewrites.get(id); + if (rewrites) { + rewrites.push(rewrite); + return; + } + pendingToolResultRewrites.set(id, [rewrite]); + }; + let contentChanged = false; - const nextToolResultRewrites: PendingToolResultRewrite[] = []; const content = msg.content.map(block => { if (block.type !== "toolCall") return block; const previousCount = seenToolCallIds.get(block.id) ?? 0; if (previousCount === 0) { seenToolCallIds.set(block.id, 1); - nextToolResultRewrites.push(undefined); + enqueueToolResultRewrite(block.id, undefined); return block; } @@ -60,14 +64,12 @@ function deduplicateToolCallIds(messages: Message[]): Message[] { } seenToolCallIds.set(block.id, duplicateIndex + 1); seenToolCallIds.set(replacementId, 1); - nextToolResultRewrites.push({ originalId: block.id, replacementId }); + enqueueToolResultRewrite(block.id, { replacementId }); contentChanged = true; return { ...block, id: replacementId }; }); if (!contentChanged) return msg; - pendingToolResultRewrites = nextToolResultRewrites; - pendingToolResultRewriteIndex = 0; return { ...msg, content }; }); } diff --git a/packages/ai/test/duplicate-tool-results.test.ts b/packages/ai/test/duplicate-tool-results.test.ts index 2c196d9c7..9507c624e 100644 --- a/packages/ai/test/duplicate-tool-results.test.ts +++ b/packages/ai/test/duplicate-tool-results.test.ts @@ -413,6 +413,27 @@ describe("Duplicate Tool Results Regression", () => { { type: "text", text: "second" }, ]); }); + + it("preserves delayed duplicate tool results across message gaps", () => { + const duplicateId = "functions.eval:301"; + const developerMessage: DeveloperMessage = { role: "developer", content: "handoff summary", timestamp: 4 }; + const messages: Message[] = [ + makeEvalAssistantMessage(duplicateId, 1), + makeEvalToolResult(duplicateId, "first", 2), + makeEvalAssistantMessage(duplicateId, 3), + developerMessage, + makeEvalToolResult(duplicateId, "second", 5), + ]; + + const transformed = transformMessages(messages, model); + const toolResults = getToolResults(transformed); + + expect(getAssistantToolIds(transformed)).toEqual([duplicateId, `${duplicateId}_dup1`]); + expect(toolResults.map(result => result.toolCallId)).toEqual([duplicateId, `${duplicateId}_dup1`]); + expect(toolResults.find(result => result.toolCallId === `${duplicateId}_dup1`)?.content).toEqual([ + { type: "text", text: "second" }, + ]); + }); }); /** From ce69ec0e92a57b70cc71ed7f17846d07d351b9b0 Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 7 Jun 2026 12:45:31 +0000 Subject: [PATCH 23/46] fix(ai): scoped duplicate tool rewrites to live call window Dropped carried-over pending tool-result rewrites the first time a new assistant turn re-emits the same upstream id, so the later real result pairs with the latest call instead of being stolen by an orphan _dup id.\n\nFixes #2055 --- .../ai/src/providers/transform-messages.ts | 16 ++++++++++ .../ai/test/duplicate-tool-results.test.ts | 29 +++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/packages/ai/src/providers/transform-messages.ts b/packages/ai/src/providers/transform-messages.ts index 5c9b56f07..6ae6a2225 100644 --- a/packages/ai/src/providers/transform-messages.ts +++ b/packages/ai/src/providers/transform-messages.ts @@ -45,10 +45,26 @@ function deduplicateToolCallIds(messages: Message[]): Message[] { pendingToolResultRewrites.set(id, [rewrite]); }; + // Ids this turn has already touched; used to scope the "drop carried-over + // pending rewrites" semantics to the FIRST occurrence per turn so multiple + // blocks of the same id within one turn still accumulate as duplicates. + const idsTouchedInTurn = new Set(); let contentChanged = false; const content = msg.content.map(block => { if (block.type !== "toolCall") return block; + // Drop any pending rewrites carried over from a prior assistant turn + // for this id on its first appearance this turn. When a later turn + // re-emits the same id, the older duplicate call's expected result + // never landed in time — the second pass synthesizes + // "No result provided" for it, and the upcoming real result(id) must + // route to one of THIS turn's calls. Without this guard the older + // `_dup` id would steal the next result. + if (!idsTouchedInTurn.has(block.id)) { + pendingToolResultRewrites.delete(block.id); + idsTouchedInTurn.add(block.id); + } + const previousCount = seenToolCallIds.get(block.id) ?? 0; if (previousCount === 0) { seenToolCallIds.set(block.id, 1); diff --git a/packages/ai/test/duplicate-tool-results.test.ts b/packages/ai/test/duplicate-tool-results.test.ts index 9507c624e..bac919989 100644 --- a/packages/ai/test/duplicate-tool-results.test.ts +++ b/packages/ai/test/duplicate-tool-results.test.ts @@ -434,6 +434,35 @@ describe("Duplicate Tool Results Regression", () => { { type: "text", text: "second" }, ]); }); + + it("routes the late result to the most recent duplicate call when a new turn re-emits the id across a gap", () => { + const duplicateId = "functions.eval:301"; + const developerMessage: DeveloperMessage = { role: "developer", content: "handoff summary", timestamp: 4 }; + const messages: Message[] = [ + makeEvalAssistantMessage(duplicateId, 1), + makeEvalToolResult(duplicateId, "first", 2), + makeEvalAssistantMessage(duplicateId, 3), + developerMessage, + makeEvalAssistantMessage(duplicateId, 5), + makeEvalToolResult(duplicateId, "second", 6), + ]; + + const transformed = transformMessages(messages, model); + const toolResults = getToolResults(transformed); + + expect(getAssistantToolIds(transformed)).toEqual([duplicateId, `${duplicateId}_dup1`, `${duplicateId}_dup2`]); + expect(toolResults.map(result => result.toolCallId)).toEqual([ + duplicateId, + `${duplicateId}_dup1`, + `${duplicateId}_dup2`, + ]); + expect(toolResults.find(result => result.toolCallId === `${duplicateId}_dup1`)?.content).toEqual([ + { type: "text", text: "No result provided" }, + ]); + expect(toolResults.find(result => result.toolCallId === `${duplicateId}_dup2`)?.content).toEqual([ + { type: "text", text: "second" }, + ]); + }); }); /** From 1823597c5b56de480b518677736405ae450fb6a8 Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 7 Jun 2026 12:51:16 +0000 Subject: [PATCH 24/46] fix(ai): capped duplicate tool id rewrites at 64 chars Truncated the original-id prefix when minting _dupN replacements so post-normalization ids never grow past the 64-char Anthropic/Google/Codex ceiling, preserving the suffix that keeps the rewrite unique.\n\nFixes #2055 --- .../ai/src/providers/transform-messages.ts | 21 ++++++++++++-- .../ai/test/duplicate-tool-results.test.ts | 28 +++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/packages/ai/src/providers/transform-messages.ts b/packages/ai/src/providers/transform-messages.ts index 6ae6a2225..096b454e7 100644 --- a/packages/ai/src/providers/transform-messages.ts +++ b/packages/ai/src/providers/transform-messages.ts @@ -17,6 +17,23 @@ const enum ToolCallStatus { Aborted = 2, } +/** + * Maximum tool-call id length the strictest replay provider accepts. + * + * Anthropic requires `^[a-zA-Z0-9_-]+$` with a 64-char cap; Google and Codex + * `normalizeToolCallId` implementations cap individual id segments to the same + * 64-char ceiling. Replacement ids minted here flow back through + * `convertAnthropicMessages` (and friends) unchanged, so the `_dupN` suffix + * MUST not push a normalized id past this bound. + */ +const MAX_TOOL_CALL_ID_LENGTH = 64; + +function appendDuplicateSuffix(originalId: string, suffix: string): string { + if (originalId.length + suffix.length <= MAX_TOOL_CALL_ID_LENGTH) return `${originalId}${suffix}`; + const prefixBudget = Math.max(0, MAX_TOOL_CALL_ID_LENGTH - suffix.length); + return `${originalId.slice(0, prefixBudget)}${suffix}`; +} + type PendingToolResultRewrite = { replacementId: string } | undefined; function deduplicateToolCallIds(messages: Message[]): Message[] { @@ -73,10 +90,10 @@ function deduplicateToolCallIds(messages: Message[]): Message[] { } let duplicateIndex = previousCount; - let replacementId = `${block.id}_dup${duplicateIndex}`; + let replacementId = appendDuplicateSuffix(block.id, `_dup${duplicateIndex}`); while (seenToolCallIds.has(replacementId)) { duplicateIndex += 1; - replacementId = `${block.id}_dup${duplicateIndex}`; + replacementId = appendDuplicateSuffix(block.id, `_dup${duplicateIndex}`); } seenToolCallIds.set(block.id, duplicateIndex + 1); seenToolCallIds.set(replacementId, 1); diff --git a/packages/ai/test/duplicate-tool-results.test.ts b/packages/ai/test/duplicate-tool-results.test.ts index bac919989..46f34ee2c 100644 --- a/packages/ai/test/duplicate-tool-results.test.ts +++ b/packages/ai/test/duplicate-tool-results.test.ts @@ -463,6 +463,34 @@ describe("Duplicate Tool Results Regression", () => { { type: "text", text: "second" }, ]); }); + + it("keeps duplicate-id rewrites within the 64-char tool-call id limit", () => { + const baseId = `toolu_${"a".repeat(58)}`; + expect(baseId.length).toBe(64); + const messages: Message[] = [ + makeEvalAssistantMessage(baseId, 1), + makeEvalToolResult(baseId, "first", 2), + makeEvalAssistantMessage(baseId, 3), + makeEvalToolResult(baseId, "second", 4), + ]; + + const transformed = transformMessages(messages, model); + const assistantToolIds = getAssistantToolIds(transformed); + const toolResults = getToolResults(transformed); + + expect(assistantToolIds).toHaveLength(2); + for (const id of assistantToolIds) { + expect(id.length).toBeLessThanOrEqual(64); + expect(id).toMatch(/^[A-Za-z0-9_-]+$/); + } + const rewrittenId = assistantToolIds[1]; + expect(rewrittenId).not.toBe(baseId); + expect(rewrittenId.endsWith("_dup1")).toBe(true); + expect(toolResults.map(result => result.toolCallId)).toEqual([baseId, rewrittenId]); + expect(toolResults.find(result => result.toolCallId === rewrittenId)?.content).toEqual([ + { type: "text", text: "second" }, + ]); + }); }); /** From 66ae45f6f564dbef6930df7ade9f287dc0ab0026 Mon Sep 17 00:00:00 2001 From: Marek Schmidt Date: Sun, 7 Jun 2026 13:07:12 +0000 Subject: [PATCH 25/46] fix(ai): support impersonated service account ADC for vertex --- packages/ai/src/providers/google-auth.ts | 43 +++++++++++++++++++++--- 1 file changed, 38 insertions(+), 5 deletions(-) diff --git a/packages/ai/src/providers/google-auth.ts b/packages/ai/src/providers/google-auth.ts index a8004508f..499ab5957 100644 --- a/packages/ai/src/providers/google-auth.ts +++ b/packages/ai/src/providers/google-auth.ts @@ -42,7 +42,13 @@ interface AuthorizedUserCredentials { refresh_token: string; } -type AdcFileCredentials = ServiceAccountCredentials | AuthorizedUserCredentials; +interface ImpersonatedServiceAccountCredentials { + type: "impersonated_service_account"; + service_account_impersonation_url: string; + source_credentials: AuthorizedUserCredentials | ServiceAccountCredentials; +} + +type AdcFileCredentials = ServiceAccountCredentials | AuthorizedUserCredentials | ImpersonatedServiceAccountCredentials; interface TokenResponse { access_token: string; @@ -196,10 +202,37 @@ async function resolveAccessTokenUncached( ): Promise<{ source: string; token: TokenResponse }> { const adc = await loadAdcCredentials(); if (adc) { - const token = - adc.creds.type === "service_account" - ? await exchangeJwtForToken(adc.creds, signal, fetchImpl) - : await exchangeRefreshToken(adc.creds, signal, fetchImpl); + const creds = adc.creds; + let token: TokenResponse; + + if (creds.type === "impersonated_service_account") { + const sourceToken = + creds.source_credentials.type === "service_account" + ? await exchangeJwtForToken(creds.source_credentials, signal, fetchImpl) + : await exchangeRefreshToken(creds.source_credentials, signal, fetchImpl); + + const response = await fetchImpl(creds.service_account_impersonation_url, { + method: "POST", + headers: { + "Content-Type": "application/json", + Authorization: `Bearer ${sourceToken.access_token}`, + }, + body: JSON.stringify({ delegates: [], scope: [CLOUD_PLATFORM_SCOPE], lifetime: "3600s" }), + signal, + }); + if (!response.ok) { + const detail = await response.text().catch(() => ""); + throw new Error(`Google Impersonation token exchange failed (${response.status}): ${detail}`); + } + const data = (await response.json()) as { accessToken: string; expireTime: string }; + const expiresIn = Math.max(0, Math.floor((new Date(data.expireTime).getTime() - Date.now()) / 1000)); + token = { access_token: data.accessToken, expires_in: expiresIn, token_type: "Bearer" }; + } else { + token = + creds.type === "service_account" + ? await exchangeJwtForToken(creds, signal, fetchImpl) + : await exchangeRefreshToken(creds, signal, fetchImpl); + } return { source: adc.source, token }; } const metadata = await fetchMetadataToken(signal, fetchImpl); From 29730eb643da418833c103b392cb86c2498d8bf0 Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 7 Jun 2026 12:11:58 +0000 Subject: [PATCH 26/46] fix(tui): handle kitty osc 5522 dot-listing paste responses Kitty implements OSC 5522 paste-event listings by sending one DATA packet whose mime field decodes to the literal dot sentinel and whose payload carries the available types as a whitespace-separated list. The TUI previously only honored the ancillary per-type DATA-packet form, so plain-text Kitty pastes were rejected as unsupported. Decode the dot-listing payload during the listing phase and keep the per-type packet path as a fallback. Also emit the follow-up OSC 5522 read request using the Kitty protocol shape: the selected MIME list belongs in the request payload, not in mime metadata. Sending it as metadata makes Kitty parse an empty requested MIME list and returns no clipboard bytes, which surfaces as an empty paste. Add regression coverage for Kitty's text-only and multi-type listings, and assert the payload-form read requests for default and primary selection paste events. Fixes #2051 --- packages/coding-agent/src/utils/enhanced-paste.ts | 5 +++-- .../coding-agent/test/utils/enhanced-paste.test.ts | 14 +++++--------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/packages/coding-agent/src/utils/enhanced-paste.ts b/packages/coding-agent/src/utils/enhanced-paste.ts index 3a3e2a564..83ec35bb6 100644 --- a/packages/coding-agent/src/utils/enhanced-paste.ts +++ b/packages/coding-agent/src/utils/enhanced-paste.ts @@ -212,11 +212,12 @@ export class EnhancedPasteController { chunks: [], }; - const metadata = [`type=read`, `mime=${Buffer.from(selected.mimeType, "utf8").toString("base64")}`]; + const encodedMime = Buffer.from(selected.mimeType, "utf8").toString("base64"); + const metadata = ["type=read"]; if (state.loc) metadata.push(`loc=${state.loc}`); if (state.pw) { metadata.push(`pw=${state.pw}`, `name=${PASTE_EVENT_NAME_BASE64}`); } - this.#handlers.write(`${OSC5522_PREFIX}${metadata.join(":")}${OSC_TERMINATOR_ST}`); + this.#handlers.write(`${OSC5522_PREFIX}${metadata.join(":")};${encodedMime}${OSC_TERMINATOR_ST}`); } } diff --git a/packages/coding-agent/test/utils/enhanced-paste.test.ts b/packages/coding-agent/test/utils/enhanced-paste.test.ts index f1d71d76d..5f026c92e 100644 --- a/packages/coding-agent/test/utils/enhanced-paste.test.ts +++ b/packages/coding-agent/test/utils/enhanced-paste.test.ts @@ -34,7 +34,7 @@ describe("EnhancedPasteController", () => { controller.handleInput(packet("type=read:status=DONE")); const pasteEventName = Buffer.from("Paste event", "utf8").toString("base64"); - expect(writes.at(-1)).toBe(`${OSC}type=read:mime=${imageMime}:pw=${password}:name=${pasteEventName}${ST}`); + expect(writes.at(-1)).toBe(`${OSC}type=read:pw=${password}:name=${pasteEventName};${imageMime}${ST}`); controller.handleInput(packet("type=read:status=OK")); controller.handleInput( @@ -68,15 +68,13 @@ describe("EnhancedPasteController", () => { const textMime = Buffer.from("text/plain", "utf8").toString("base64"); const password = Buffer.from("secret456", "utf8").toString("base64"); + const pasteEventName = Buffer.from("Paste event", "utf8").toString("base64"); expect(controller.handleInput("plain text")).toBe(false); controller.handleInput(packet(`type=read:status=OK:loc=primary:pw=${password}`)); controller.handleInput(packet(`type=read:status=DATA:mime=${textMime}`)); controller.handleInput(packet("type=read:status=DONE")); - expect(writes).toHaveLength(1); - expect(writes[0]).toContain(`mime=${textMime}`); - expect(writes[0]).toContain("loc=primary"); - expect(writes[0]).toContain(`pw=${password}`); + expect(writes).toEqual([`${OSC}type=read:loc=primary:pw=${password}:name=${pasteEventName};${textMime}${ST}`]); controller.handleInput(packet("type=read:status=OK")); controller.handleInput( @@ -136,9 +134,7 @@ describe("EnhancedPasteController", () => { ); controller.handleInput(packet(`type=read:status=DONE:pw=${password}`)); - expect(writes.at(-1)).toBe( - `${OSC}type=read:mime=${textMime}:pw=${password}:name=${pasteEventName}${ST}`, - ); + expect(writes.at(-1)).toBe(`${OSC}type=read:pw=${password}:name=${pasteEventName};${textMime}${ST}`); controller.handleInput(packet("type=read:status=OK")); controller.handleInput( @@ -175,6 +171,6 @@ describe("EnhancedPasteController", () => { ); controller.handleInput(packet("type=read:status=DONE")); - expect(writes.at(-1)).toContain(`mime=${imageMime}`); + expect(writes.at(-1)).toBe(`${OSC}type=read;${imageMime}${ST}`); }); }); From be0bd63023331a1f4ceec2932a520ad1d9968abf Mon Sep 17 00:00:00 2001 From: Marek Schmidt Date: Sun, 7 Jun 2026 13:44:50 +0000 Subject: [PATCH 27/46] fix(ai): reconstruct IAM URL, add test coverage, update changelog --- packages/ai/CHANGELOG.md | 5 +++ .../providers/__tests__/google-auth.test.ts | 41 +++++++++++++++++++ packages/ai/src/providers/google-auth.ts | 15 ++++++- 3 files changed, 59 insertions(+), 2 deletions(-) create mode 100644 packages/ai/src/providers/__tests__/google-auth.test.ts diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index 344d76b9a..a8963617e 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -1,3 +1,8 @@ +## [Unreleased] + +### Added + +- Added support for `impersonated_service_account` Application Default Credentials (ADC) in Vertex AI to enable chained impersonation without failing via 401 `invalid_client`. # Changelog ## [Unreleased] diff --git a/packages/ai/src/providers/__tests__/google-auth.test.ts b/packages/ai/src/providers/__tests__/google-auth.test.ts new file mode 100644 index 000000000..e6f1aea53 --- /dev/null +++ b/packages/ai/src/providers/__tests__/google-auth.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, it, mock } from "bun:test"; +import { getVertexAccessToken, __resetVertexTokenCache } from "../google-auth"; + +describe("getVertexAccessToken", () => { + it("should exchange impersonated ADC correctly", async () => { + __resetVertexTokenCache(); + Bun.env.GOOGLE_APPLICATION_CREDENTIALS = "/tmp/mock-impersonated-adc.json"; + + await Bun.write("/tmp/mock-impersonated-adc.json", JSON.stringify({ + type: "impersonated_service_account", + service_account_impersonation_url: "https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/target@project.iam.gserviceaccount.com:generateAccessToken", + source_credentials: { + type: "authorized_user", + client_id: "client-id", + client_secret: "client-secret", + refresh_token: "refresh-token" + }, + delegates: ["delegate1"] + })); + + let fetchCalls: any[] = []; + const mockFetch = mock(async (url: string, opts: any) => { + fetchCalls.push({ url, opts }); + if (url.includes("oauth2.googleapis.com/token")) { + return { ok: true, json: async () => ({ access_token: "source-token", expires_in: 3600 }) }; + } + if (url.includes("iamcredentials.googleapis.com")) { + return { ok: true, json: async () => ({ accessToken: "impersonated-token", expireTime: new Date(Date.now() + 3600000).toISOString() }) }; + } + return { ok: false }; + }) as any; + + const token = await getVertexAccessToken({ fetch: mockFetch }); + expect(token).toBe("impersonated-token"); + expect(fetchCalls.length).toBe(2); + expect(fetchCalls[0].url).toContain("oauth2.googleapis.com"); + expect(fetchCalls[1].url).toBe("https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/target@project.iam.gserviceaccount.com:generateAccessToken"); + expect(fetchCalls[1].opts.headers.Authorization).toBe("Bearer source-token"); + expect(JSON.parse(fetchCalls[1].opts.body).delegates).toEqual(["delegate1"]); + }); +}); diff --git a/packages/ai/src/providers/google-auth.ts b/packages/ai/src/providers/google-auth.ts index 499ab5957..39a81922c 100644 --- a/packages/ai/src/providers/google-auth.ts +++ b/packages/ai/src/providers/google-auth.ts @@ -46,6 +46,7 @@ interface ImpersonatedServiceAccountCredentials { type: "impersonated_service_account"; service_account_impersonation_url: string; source_credentials: AuthorizedUserCredentials | ServiceAccountCredentials; + delegates?: string[]; } type AdcFileCredentials = ServiceAccountCredentials | AuthorizedUserCredentials | ImpersonatedServiceAccountCredentials; @@ -206,18 +207,28 @@ async function resolveAccessTokenUncached( let token: TokenResponse; if (creds.type === "impersonated_service_account") { + const targetPrincipalMatch = /(?[^/]+):(generateAccessToken|generateIdToken)$/.exec( + creds.service_account_impersonation_url, + ); + const targetPrincipal = targetPrincipalMatch?.groups?.target; + if (!targetPrincipal) { + throw new RangeError( + `Cannot extract target principal from ${creds.service_account_impersonation_url}`, + ); + } + const sourceToken = creds.source_credentials.type === "service_account" ? await exchangeJwtForToken(creds.source_credentials, signal, fetchImpl) : await exchangeRefreshToken(creds.source_credentials, signal, fetchImpl); - const response = await fetchImpl(creds.service_account_impersonation_url, { + const response = await fetchImpl(`https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/${targetPrincipal}:generateAccessToken`, { method: "POST", headers: { "Content-Type": "application/json", Authorization: `Bearer ${sourceToken.access_token}`, }, - body: JSON.stringify({ delegates: [], scope: [CLOUD_PLATFORM_SCOPE], lifetime: "3600s" }), + body: JSON.stringify({ delegates: creds.delegates ?? [], scope: [CLOUD_PLATFORM_SCOPE], lifetime: "3600s" }), signal, }); if (!response.ok) { From 9c870ec106ec3992c61d97f66e1e766406a8e4dc Mon Sep 17 00:00:00 2001 From: Marek Schmidt Date: Sun, 7 Jun 2026 13:50:25 +0000 Subject: [PATCH 28/46] fix(ai): updated changelog --- packages/ai/CHANGELOG.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index a8963617e..04d37ccac 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -1,11 +1,10 @@ +# Changelog ## [Unreleased] ### Added - Added support for `impersonated_service_account` Application Default Credentials (ADC) in Vertex AI to enable chained impersonation without failing via 401 `invalid_client`. -# Changelog -## [Unreleased] ## [15.10.1] - 2026-06-07 From 83c8105be60bf49daf11b5bfcd4de97f5f6e2ecd Mon Sep 17 00:00:00 2001 From: Guts Date: Sun, 7 Jun 2026 15:42:35 +0200 Subject: [PATCH 29/46] fix(mcp): declare approval tier for MCP tools to prevent hangs in non-yolo mode MCPTool and DeferredMCPTool now declare approval = 'write' instead of implicitly defaulting to 'exec'. Without this, the approval system requires user confirmation for every MCP tool call in non-yolo modes, but the confirmation prompt never renders in the TUI while streaming, causing the agent to hang indefinitely. Also propagate the approval property through customToolToDefinition() in sdk.ts, which was silently dropping it during CustomTool -> ToolDefinition conversion. --- packages/coding-agent/CHANGELOG.md | 4 ++++ .../src/extensibility/extensions/types.ts | 11 ++++++++++- packages/coding-agent/src/mcp/tool-bridge.ts | 2 ++ packages/coding-agent/src/sdk.ts | 1 + 4 files changed, 17 insertions(+), 1 deletion(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 0243ed841..ff32bd8db 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed MCP tools hanging in non-yolo modes by declaring `approval = "write"` on `MCPTool` and `DeferredMCPTool`, and propagating the `approval` property through `customToolToDefinition()` in `sdk.ts` + ## [15.10.1] - 2026-06-07 ### Added diff --git a/packages/coding-agent/src/extensibility/extensions/types.ts b/packages/coding-agent/src/extensibility/extensions/types.ts index beeae8e39..17b78e334 100644 --- a/packages/coding-agent/src/extensibility/extensions/types.ts +++ b/packages/coding-agent/src/extensibility/extensions/types.ts @@ -7,7 +7,13 @@ * - Register commands, keyboard shortcuts, and CLI flags * - Interact with the user via UI primitives */ -import type { AgentMessage, AgentToolResult, AgentToolUpdateCallback, ThinkingLevel } from "@oh-my-pi/pi-agent-core"; +import type { + AgentMessage, + AgentToolResult, + AgentToolUpdateCallback, + ThinkingLevel, + ToolApproval, +} from "@oh-my-pi/pi-agent-core"; import type { CompactionResult } from "@oh-my-pi/pi-agent-core/compaction"; import type { Api, @@ -392,6 +398,9 @@ export interface ToolDefinition { readonly mcpToolName: string; /** Server name */ readonly mcpServerName: string; + readonly approval = "write" as const; /** Render completed MCP calls with the result header replacing the pending call header. */ readonly mergeCallAndResult = true; @@ -305,6 +306,7 @@ export class DeferredMCPTool implements CustomTool { readonly mcpToolName: string; /** Server name */ readonly mcpServerName: string; + readonly approval = "write" as const; /** Render completed MCP calls with the result header replacing the pending call header. */ readonly mergeCallAndResult = true; diff --git a/packages/coding-agent/src/sdk.ts b/packages/coding-agent/src/sdk.ts index a28d834e5..748a8e651 100644 --- a/packages/coding-agent/src/sdk.ts +++ b/packages/coding-agent/src/sdk.ts @@ -709,6 +709,7 @@ function customToolToDefinition(tool: CustomTool): ToolDefinition { parameters: tool.parameters, hidden: tool.hidden, deferrable: tool.deferrable, + approval: tool.approval, mcpServerName: tool.mcpServerName, mcpToolName: tool.mcpToolName, execute: (toolCallId, params, signal, onUpdate, ctx) => From e796a7db4d07887ad34a20a748819c263af6ab28 Mon Sep 17 00:00:00 2001 From: Guts Date: Sun, 7 Jun 2026 16:42:19 +0200 Subject: [PATCH 30/46] test(approval): add regression tests for MCP tool approval tier --- packages/coding-agent/test/tools/approval.test.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/coding-agent/test/tools/approval.test.ts b/packages/coding-agent/test/tools/approval.test.ts index 04761a543..11f64bc01 100644 --- a/packages/coding-agent/test/tools/approval.test.ts +++ b/packages/coding-agent/test/tools/approval.test.ts @@ -115,6 +115,17 @@ describe("MCP fallback and prompt formatting", () => { expect(resolveApproval(subject, {}, "yolo")).toMatchObject({ policy: "allow", tier: "exec" }); }); + it("allows MCP tools with write approval in write mode", () => { + const subject = tool("mcp__server__safe", "write"); + expect(resolveApproval(subject, {}, "write")).toMatchObject({ policy: "allow", tier: "write" }); + expect(resolveApproval(subject, {}, "yolo")).toMatchObject({ policy: "allow", tier: "write" }); + }); + + it("prompts for MCP tools with write approval in always-ask mode", () => { + const subject = tool("mcp__server__safe", "write"); + expect(resolveApproval(subject, {}, "always-ask")).toMatchObject({ policy: "prompt", tier: "write" }); + }); + it("formats MCP origin, reason, and per-tool details", () => { const subject = tool("mcp__server__dangerous", undefined, () => ["Path: /tmp/out", "Content:\nhello"]); expect(formatApprovalPrompt(subject, {}, "Needs confirmation").split("\n")).toEqual([ From 72e11ca5e905984dc74b97fb981fccd82fdcbd1d Mon Sep 17 00:00:00 2001 From: Guts Date: Sun, 7 Jun 2026 16:58:25 +0200 Subject: [PATCH 31/46] fix(sdk): bind dynamic approval callbacks to original tool instance --- packages/coding-agent/src/sdk.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/coding-agent/src/sdk.ts b/packages/coding-agent/src/sdk.ts index 748a8e651..992396621 100644 --- a/packages/coding-agent/src/sdk.ts +++ b/packages/coding-agent/src/sdk.ts @@ -709,7 +709,7 @@ function customToolToDefinition(tool: CustomTool): ToolDefinition { parameters: tool.parameters, hidden: tool.hidden, deferrable: tool.deferrable, - approval: tool.approval, + approval: typeof tool.approval === "function" ? tool.approval.bind(tool) : tool.approval, mcpServerName: tool.mcpServerName, mcpToolName: tool.mcpToolName, execute: (toolCallId, params, signal, onUpdate, ctx) => From 5bfad210a66569b261c5c0dc298fdd653e188d8f Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 7 Jun 2026 16:17:29 +0000 Subject: [PATCH 32/46] fix(tui): recognized Ghostty's super+alt+backspace as word delete Ghostty on macOS reports Option+Backspace as kitty modifier 11 (wire) = 10 (mask) = super(8) | alt(2). The native key matcher only recognised shift/ctrl/alt, so: - format_kitty_key in crates/pi-natives/src/keys.rs rejected any sequence whose effective modifier had bits beyond shift/ctrl/alt set; parseKey returned undefined for \x1b[127;11u. - parse_key_id recognised only those three modifiers, so even an explicit 'super+alt+backspace' degraded to 'alt+backspace' and could not match an actual super+alt encoded press. - The editor's matchesKey(data, 'alt+backspace') branch was never reached and Option+Backspace either no-op'd or fell through to single-character delete. Added a MOD_SUPER constant, taught parse_key_id, format_with_mods, and format_kitty_key about the super bit, added super+alt+backspace (and super+alt+delete / super+alt+d) to tui.editor.deleteWord* defaults, and broadened the editor's direct matchesKey calls to accept the super+alt variants. Existing alt+backspace / alt+d / alt+delete paths are untouched, so the legacy ESC+DEL workaround keeps working too. Added Rust unit tests for the super modifier (positive and negative for hyper/meta sequences we still drop) and TS regressions in keys.test.ts and editor.test.ts that exercise \x1b[127;11u end-to-end. Fixes #2064 --- crates/pi-natives/src/keys.rs | 44 +++++++++++++++++++++++++-- packages/natives/CHANGELOG.md | 4 +++ packages/tui/CHANGELOG.md | 4 +++ packages/tui/src/components/editor.ts | 14 ++++++--- packages/tui/src/keybindings.ts | 4 +-- packages/tui/test/editor.test.ts | 11 +++++++ packages/tui/test/keys.test.ts | 17 ++++++++++- 7 files changed, 89 insertions(+), 9 deletions(-) diff --git a/crates/pi-natives/src/keys.rs b/crates/pi-natives/src/keys.rs index ccdb9a935..d132d70e8 100644 --- a/crates/pi-natives/src/keys.rs +++ b/crates/pi-natives/src/keys.rs @@ -70,6 +70,7 @@ const CP_KP_EQUALS: i32 = 57415; const MOD_SHIFT: u32 = 1; const MOD_ALT: u32 = 2; const MOD_CTRL: u32 = 4; +const MOD_SUPER: u32 = 8; const MOD_NUM_LOCK: u32 = 128; /// Event types from Kitty keyboard protocol (flag 2). @@ -457,6 +458,10 @@ fn parse_key_id(key_id: &str) -> Option> { modifier |= MOD_SHIFT; continue; }, + b's' | b'S' if p.eq_ignore_ascii_case("super") => { + modifier |= MOD_SUPER; + continue; + }, b'a' | b'A' if p.eq_ignore_ascii_case("alt") => { modifier |= MOD_ALT; continue; @@ -1378,7 +1383,7 @@ fn parse_functional(bytes: &[u8]) -> Option { fn format_kitty_key(parsed: &ParsedKittySequence) -> Option> { let effective_mod = parsed.modifier & !LOCK_MASK; - if effective_mod & !(MOD_SHIFT | MOD_CTRL | MOD_ALT) != 0 { + if effective_mod & !(MOD_SHIFT | MOD_CTRL | MOD_ALT | MOD_SUPER) != 0 { return None; } let effective_codepoint = @@ -1480,6 +1485,9 @@ fn format_with_mods(mods: u32, key_name: &str) -> String { if mods & MOD_ALT != 0 { result.push_str("alt+"); } + if mods & MOD_SUPER != 0 { + result.push_str("super+"); + } result.push_str(key_name); result } @@ -1585,7 +1593,11 @@ mod tests { #[test] fn parse_key_ignores_kitty_sequences_with_unsupported_modifiers() { - assert_eq!(parse_key_inner(b"\x1b[99;9u", true).as_deref(), None); + // Hyper (16) and meta (32) are kitty modifier bits we do not surface + // because nothing in the editor binds them. Wire mod 17 = mask 16 = hyper. + assert_eq!(parse_key_inner(b"\x1b[99;17u", true).as_deref(), None); + // Wire mod 33 = mask 32 = meta. + assert_eq!(parse_key_inner(b"\x1b[99;33u", true).as_deref(), None); } #[test] @@ -1675,4 +1687,32 @@ mod tests { assert!(matches_key_inner(b"\x1b[109;7u", "ctrl+alt+m", true)); assert!(matches_key_inner(b"\x1b[27;7;109~", "ctrl+alt+m", false)); } + + #[test] + fn super_alt_backspace_matches_ghostty_default() { + // Issue #2064: Ghostty on macOS reports Option+Backspace as kitty + // modifier 11 (wire) = 10 (mask) = super(8)|alt(2). Before super + // support landed, the matcher rejected this entirely. + assert!(matches_key_inner(b"\x1b[127;11u", "super+alt+backspace", true)); + assert!(matches_key_inner(b"\x1b[127;11u", "alt+super+backspace", true)); + assert_eq!(parse_key_inner(b"\x1b[127;11u", true).as_deref(), Some("alt+super+backspace")); + // Plain alt+backspace must still NOT match — the modifier really is super|alt. + assert!(!matches_key_inner(b"\x1b[127;11u", "alt+backspace", true)); + // And plain backspace (mod 0) must still not match a super+alt-modified press. + assert!(!matches_key_inner(b"\x1b[127;11u", "backspace", true)); + // Release events stay ignored: super+alt+backspace release must not match a press. + assert!(!matches_key_inner(b"\x1b[127;11:3u", "super+alt+backspace", true)); + assert_eq!(parse_key_inner(b"\x1b[127;11:3u", true).as_deref(), None); + } + + #[test] + fn super_modifier_parses_for_arbitrary_keys() { + // Cmd+letter on macOS under kitty flag=1+: super(8)+'a'(97) → wire mod 9. + assert!(matches_key_inner(b"\x1b[97;9u", "super+a", true)); + assert_eq!(parse_key_inner(b"\x1b[97;9u", true).as_deref(), Some("super+a")); + // Cmd+Shift+letter: super(8)|shift(1) = 9 mask, wire 10. + assert!(matches_key_inner(b"\x1b[97;10u", "super+shift+a", true)); + assert!(matches_key_inner(b"\x1b[97;10u", "shift+super+a", true)); + assert_eq!(parse_key_inner(b"\x1b[97;10u", true).as_deref(), Some("shift+super+a")); + } } diff --git a/packages/natives/CHANGELOG.md b/packages/natives/CHANGELOG.md index c89da28eb..c2335e74e 100644 --- a/packages/natives/CHANGELOG.md +++ b/packages/natives/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Added + +- Added the `super` modifier to `matchesKey` / `parseKey` / `parseKittySequence`. Key identifiers may now include `super+` (anywhere in the modifier prefix), and Kitty CSI-u sequences whose modifier mask contains the super bit (8) — e.g. Ghostty's macOS Option+Backspace `ESC [127;11u` — are now recognised instead of dropped ([#2064](https://github.com/can1357/oh-my-pi/issues/2064)). + ## [15.10.1] - 2026-06-07 ### Fixed diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index 56903eddc..f5be769ec 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Added + +- Added `super` modifier support to native key parsing/matching and bound `super+alt+backspace` / `super+alt+delete` (and `super+alt+d`) into the word-delete defaults so Ghostty's default macOS Option+Backspace wire (`ESC [127;11u` — kitty modifier 11 = super|alt) deletes a word instead of falling through to single-char delete ([#2064](https://github.com/can1357/oh-my-pi/issues/2064)). + ## [15.10.1] - 2026-06-07 ### Breaking Changes diff --git a/packages/tui/src/components/editor.ts b/packages/tui/src/components/editor.ts index 2ea05e43e..e252a1ea0 100644 --- a/packages/tui/src/components/editor.ts +++ b/packages/tui/src/components/editor.ts @@ -1109,12 +1109,18 @@ export class Editor implements Component, Focusable { else if (matchesKey(data, "ctrl+w")) { this.#deleteWordBackwards(); } - // Option/Alt+Backspace - Delete word backwards - else if (matchesKey(data, "alt+backspace")) { + // Option/Alt+Backspace - Delete word backwards. + // Ghostty on macOS reports Option+Backspace as super+alt (kitty mod 11) — see #2064. + else if (matchesKey(data, "alt+backspace") || matchesKey(data, "super+alt+backspace")) { this.#deleteWordBackwards(); } - // Option/Alt+D - Delete word forwards - else if (matchesKey(data, "alt+d") || matchesKey(data, "alt+delete")) { + // Option/Alt+D and Option+Delete - Delete word forwards. Same Ghostty quirk applies. + else if ( + matchesKey(data, "alt+d") || + matchesKey(data, "alt+delete") || + matchesKey(data, "super+alt+d") || + matchesKey(data, "super+alt+delete") + ) { this.#deleteWordForwards(); } // Ctrl+Y - Yank from kill ring diff --git a/packages/tui/src/keybindings.ts b/packages/tui/src/keybindings.ts index e03954f09..876b77c5a 100644 --- a/packages/tui/src/keybindings.ts +++ b/packages/tui/src/keybindings.ts @@ -100,11 +100,11 @@ export const TUI_KEYBINDINGS = { description: "Delete character forward", }, "tui.editor.deleteWordBackward": { - defaultKeys: ["ctrl+w", "alt+backspace", "ctrl+backspace"], + defaultKeys: ["ctrl+w", "alt+backspace", "ctrl+backspace", "super+alt+backspace"], description: "Delete word backward", }, "tui.editor.deleteWordForward": { - defaultKeys: ["alt+delete", "alt+d"], + defaultKeys: ["alt+delete", "alt+d", "super+alt+delete", "super+alt+d"], description: "Delete word forward", }, "tui.editor.deleteToLineStart": { diff --git a/packages/tui/test/editor.test.ts b/packages/tui/test/editor.test.ts index 640eb9601..7a1506974 100644 --- a/packages/tui/test/editor.test.ts +++ b/packages/tui/test/editor.test.ts @@ -3,6 +3,7 @@ import { stripVTControlCharacters } from "node:util"; import { CURSOR_MARKER } from "@oh-my-pi/pi-tui"; import { CombinedAutocompleteProvider } from "@oh-my-pi/pi-tui/autocomplete"; import { Editor } from "@oh-my-pi/pi-tui/components/editor"; +import { setKittyProtocolActive } from "@oh-my-pi/pi-tui/keys"; import { visibleWidth } from "@oh-my-pi/pi-tui/utils"; import { setDefaultTabWidth } from "@oh-my-pi/pi-utils"; import { KeybindingsManager, setKeybindings, TUI_KEYBINDINGS } from "../src/keybindings"; @@ -616,6 +617,16 @@ describe("Editor component", () => { editor.setText("foo bar"); editor.handleInput("\x1b\x7f"); // Alt+Backspace (legacy) expect(editor.getText()).toBe("foo "); + + // Issue #2064: Ghostty on macOS reports Option+Backspace as `ESC [127;11u` + // (kitty modifier 11 wire = super(8)|alt(2)). Without super support the + // editor used to ignore this entirely and the previous word survived. + setKittyProtocolActive(true); + editor.setText("foo bar"); + editor.handleInput("\x1b[F"); // End — park cursor at EOL + editor.handleInput("\x1b[127;11u"); // Ghostty Option+Backspace + expect(editor.getText()).toBe("foo "); + setKittyProtocolActive(false); }); it("navigates words correctly with Ctrl+Left/Right", () => { diff --git a/packages/tui/test/keys.test.ts b/packages/tui/test/keys.test.ts index 48943e754..3645ee31e 100644 --- a/packages/tui/test/keys.test.ts +++ b/packages/tui/test/keys.test.ts @@ -77,6 +77,19 @@ describe("matchesKey", () => { expect(matchesKey("\x1b[57400;133u", "1")).toBe(false); setKittyProtocolActive(false); }); + + it("recognises super+alt+backspace from Ghostty's default Option+Backspace wire (#2064)", () => { + setKittyProtocolActive(true); + // Modifier 11 (wire) = 10 (mask) = super(8)|alt(2). Ghostty's keyboard + // inspector on macOS reports `ESC [127;11u` for Option+Backspace without + // any user-defined keybind. + expect(matchesKey("\x1b[127;11u", "super+alt+backspace")).toBe(true); + expect(matchesKey("\x1b[127;11u", "alt+super+backspace")).toBe(true); + // Plain alt+backspace must NOT consume this — the modifier truly is super|alt. + expect(matchesKey("\x1b[127;11u", "alt+backspace")).toBe(false); + expect(matchesKey("\x1b[127;11u", "backspace")).toBe(false); + setKittyProtocolActive(false); + }); }); describe("parseKey", () => { @@ -132,7 +145,9 @@ describe("parseKey", () => { it("ignores Kitty sequences with unsupported modifiers", () => { setKittyProtocolActive(true); - expect(parseKey("\x1b[99;9u")).toBeUndefined(); + // Hyper (16) and meta (32) bits aren't surfaced because nothing binds them. + expect(parseKey("\x1b[99;17u")).toBeUndefined(); // hyper-only + expect(parseKey("\x1b[99;33u")).toBeUndefined(); // meta-only setKittyProtocolActive(false); }); }); From a58f0bcbebfb9a46480951cd45eecf5e0acab85f Mon Sep 17 00:00:00 2001 From: DarkPhilosophy <19309990+DarkPhilosophy@users.noreply.github.com> Date: Sun, 7 Jun 2026 20:46:44 +0300 Subject: [PATCH 33/46] fix(coding-agent): parse launch cwd flag --- packages/coding-agent/CHANGELOG.md | 4 ++++ packages/coding-agent/src/cli/args.ts | 2 ++ packages/coding-agent/src/commands/launch.ts | 3 +++ .../coding-agent/test/cli-cwd-flag.test.ts | 18 ++++++++++++++++++ 4 files changed, 27 insertions(+) create mode 100644 packages/coding-agent/test/cli-cwd-flag.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 0243ed841..c374cc647 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed the `--cwd` launch flag so it is parsed and can override the startup directory instead of always falling back to the current process directory or home auto-switch target. + ## [15.10.1] - 2026-06-07 ### Added diff --git a/packages/coding-agent/src/cli/args.ts b/packages/coding-agent/src/cli/args.ts index 707d87c54..feec309ba 100644 --- a/packages/coding-agent/src/cli/args.ts +++ b/packages/coding-agent/src/cli/args.ts @@ -109,6 +109,8 @@ export function parseArgs(inputArgs: string[], extensionFlags?: Map { + it("parses --cwd with a space-separated directory", () => { + const result = parseArgs(["--cwd", "/work/project", "hello"]); + + expect(result.cwd).toBe("/work/project"); + expect(result.messages).toEqual(["hello"]); + }); + + it("parses --cwd=value without leaking the value into messages", () => { + const result = parseArgs(["--cwd=/work/project", "hello"]); + + expect(result.cwd).toBe("/work/project"); + expect(result.messages).toEqual(["hello"]); + }); +}); From 5f7db975609c99e8841a9aa85b2f277a2f63ebee Mon Sep 17 00:00:00 2001 From: DarkPhilosophy <19309990+DarkPhilosophy@users.noreply.github.com> Date: Sun, 7 Jun 2026 21:06:02 +0300 Subject: [PATCH 34/46] fix(coding-agent): apply cwd before startup discovery --- packages/coding-agent/src/main.ts | 10 +++++++- .../coding-agent/test/cli-cwd-flag.test.ts | 25 ++++++++++++++++++- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/packages/coding-agent/src/main.ts b/packages/coding-agent/src/main.ts index 8d4d30b46..491ed2c37 100644 --- a/packages/coding-agent/src/main.ts +++ b/packages/coding-agent/src/main.ts @@ -530,6 +530,14 @@ async function maybeAutoChdir(parsed: Args): Promise { } } +export async function applyStartupCwd(parsed: Args): Promise { + if (parsed.cwd) { + setProjectDir(parsed.cwd); + return; + } + await maybeAutoChdir(parsed); +} + /** Discover SYSTEM.md file if no CLI system prompt was provided */ function discoverSystemPromptFile(): string | undefined { // Check project-local first (.omp/SYSTEM.md, .pi/SYSTEM.md legacy) @@ -745,7 +753,7 @@ export async function runRootCommand( await logger.time("initTheme:initial", initTheme); const parsedArgs = parsed; - await logger.time("maybeAutoChdir", maybeAutoChdir, parsedArgs); + await logger.time("applyStartupCwd", applyStartupCwd, parsedArgs); const notifs: (InteractiveModeNotify | null)[] = []; diff --git a/packages/coding-agent/test/cli-cwd-flag.test.ts b/packages/coding-agent/test/cli-cwd-flag.test.ts index b2c9c2147..7949ec5dc 100644 --- a/packages/coding-agent/test/cli-cwd-flag.test.ts +++ b/packages/coding-agent/test/cli-cwd-flag.test.ts @@ -1,6 +1,16 @@ -import { describe, expect, it } from "bun:test"; +import { afterEach, describe, expect, it } from "bun:test"; +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; +import { getProjectDir, setProjectDir } from "@oh-my-pi/pi-utils"; import { parseArgs } from "../src/cli/args"; +import { applyStartupCwd } from "../src/main"; +const originalProjectDir = getProjectDir(); + +afterEach(() => { + setProjectDir(originalProjectDir); +}); describe("parseArgs — --cwd flag", () => { it("parses --cwd with a space-separated directory", () => { const result = parseArgs(["--cwd", "/work/project", "hello"]); @@ -15,4 +25,17 @@ describe("parseArgs — --cwd flag", () => { expect(result.cwd).toBe("/work/project"); expect(result.messages).toEqual(["hello"]); }); + + it("applies --cwd before session lookup callers read the project directory", async () => { + const launchDir = fs.mkdtempSync(path.join(os.tmpdir(), "omp-cwd-launch-")); + const targetDir = fs.mkdtempSync(path.join(os.tmpdir(), "omp-cwd-target-")); + setProjectDir(launchDir); + + const parsed = parseArgs(["--cwd", targetDir, "--continue"]); + await applyStartupCwd(parsed); + + expect(parsed.continue).toBe(true); + expect(getProjectDir()).toBe(targetDir); + expect(process.cwd()).toBe(targetDir); + }); }); From 60dde9d33f26b7b1d9a07f0163409f6ef6475850 Mon Sep 17 00:00:00 2001 From: DarkPhilosophy <19309990+DarkPhilosophy@users.noreply.github.com> Date: Sun, 7 Jun 2026 21:12:17 +0300 Subject: [PATCH 35/46] fix(coding-agent): isolate startup cwd helper --- packages/coding-agent/src/cli/startup-cwd.ts | 63 +++++++++++++++++++ packages/coding-agent/src/main.ts | 61 +----------------- .../coding-agent/test/cli-cwd-flag.test.ts | 2 +- 3 files changed, 65 insertions(+), 61 deletions(-) create mode 100644 packages/coding-agent/src/cli/startup-cwd.ts diff --git a/packages/coding-agent/src/cli/startup-cwd.ts b/packages/coding-agent/src/cli/startup-cwd.ts new file mode 100644 index 000000000..afabff095 --- /dev/null +++ b/packages/coding-agent/src/cli/startup-cwd.ts @@ -0,0 +1,63 @@ +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import { getProjectDir, normalizePathForComparison, setProjectDir } from "@oh-my-pi/pi-utils"; +import type { Args } from "./args"; + +async function maybeAutoChdir(parsed: Args): Promise { + if (parsed.allowHome || parsed.cwd) { + return; + } + + const home = os.homedir(); + if (!home) { + return; + } + + const normalizePath = normalizePathForComparison; + + const cwd = normalizePath(getProjectDir()); + const normalizedHome = normalizePath(home); + if (cwd !== normalizedHome) { + return; + } + + const isDirectory = async (p: string) => { + try { + const s = await fs.stat(p); + return s.isDirectory(); + } catch { + return false; + } + }; + + const candidates = [path.join(home, "tmp"), "/tmp", "/var/tmp"]; + for (const candidate of candidates) { + try { + if (!(await isDirectory(candidate))) { + continue; + } + setProjectDir(candidate); + return; + } catch { + // Try next candidate. + } + } + + try { + const fallback = os.tmpdir(); + if (fallback && normalizePath(fallback) !== cwd && (await isDirectory(fallback))) { + setProjectDir(fallback); + } + } catch { + // Ignore fallback errors. + } +} + +export async function applyStartupCwd(parsed: Args): Promise { + if (parsed.cwd) { + setProjectDir(parsed.cwd); + return; + } + await maybeAutoChdir(parsed); +} diff --git a/packages/coding-agent/src/main.ts b/packages/coding-agent/src/main.ts index 491ed2c37..64d033fb4 100644 --- a/packages/coding-agent/src/main.ts +++ b/packages/coding-agent/src/main.ts @@ -5,9 +5,7 @@ * createAgentSession() options. The SDK does the heavy lifting. */ -import * as fs from "node:fs/promises"; import * as os from "node:os"; -import * as path from "node:path"; import { createInterface } from "node:readline/promises"; import { EventLoopKeepalive } from "@oh-my-pi/pi-agent-core"; import type { ImageContent } from "@oh-my-pi/pi-ai"; @@ -28,6 +26,7 @@ import { processFileArguments } from "./cli/file-processor"; import { buildInitialMessage } from "./cli/initial-message"; import { runListModelsCommand } from "./cli/list-models"; import { selectSession } from "./cli/session-picker"; +import { applyStartupCwd } from "./cli/startup-cwd"; import { findConfigFile } from "./config"; import { ModelRegistry, ModelsConfigFile } from "./config/model-registry"; import { resolveCliModel, resolveModelRoleValue, resolveModelScope, type ScopedModel } from "./config/model-resolver"; @@ -480,64 +479,6 @@ export async function createSessionManager( return undefined; } -async function maybeAutoChdir(parsed: Args): Promise { - if (parsed.allowHome || parsed.cwd) { - return; - } - - const home = os.homedir(); - if (!home) { - return; - } - - const normalizePath = normalizePathForComparison; - - const cwd = normalizePath(getProjectDir()); - const normalizedHome = normalizePath(home); - if (cwd !== normalizedHome) { - return; - } - - const isDirectory = async (p: string) => { - try { - const s = await fs.stat(p); - return s.isDirectory(); - } catch { - return false; - } - }; - - const candidates = [path.join(home, "tmp"), "/tmp", "/var/tmp"]; - for (const candidate of candidates) { - try { - if (!(await isDirectory(candidate))) { - continue; - } - setProjectDir(candidate); - return; - } catch { - // Try next candidate. - } - } - - try { - const fallback = os.tmpdir(); - if (fallback && normalizePath(fallback) !== cwd && (await isDirectory(fallback))) { - setProjectDir(fallback); - } - } catch { - // Ignore fallback errors. - } -} - -export async function applyStartupCwd(parsed: Args): Promise { - if (parsed.cwd) { - setProjectDir(parsed.cwd); - return; - } - await maybeAutoChdir(parsed); -} - /** Discover SYSTEM.md file if no CLI system prompt was provided */ function discoverSystemPromptFile(): string | undefined { // Check project-local first (.omp/SYSTEM.md, .pi/SYSTEM.md legacy) diff --git a/packages/coding-agent/test/cli-cwd-flag.test.ts b/packages/coding-agent/test/cli-cwd-flag.test.ts index 7949ec5dc..973d61a4c 100644 --- a/packages/coding-agent/test/cli-cwd-flag.test.ts +++ b/packages/coding-agent/test/cli-cwd-flag.test.ts @@ -4,7 +4,7 @@ import * as os from "node:os"; import * as path from "node:path"; import { getProjectDir, setProjectDir } from "@oh-my-pi/pi-utils"; import { parseArgs } from "../src/cli/args"; -import { applyStartupCwd } from "../src/main"; +import { applyStartupCwd } from "../src/cli/startup-cwd"; const originalProjectDir = getProjectDir(); From dd8b50649a5c3db197514ba1c8b4a7658bcbbbc9 Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 7 Jun 2026 21:07:43 +0000 Subject: [PATCH 36/46] fix(coding-agent): suppressed reviewer findings injection when schema rejects it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `finalizeSubprocessOutput` always spliced collected `report_finding` entries onto a top-level `findings` array regardless of the active output schema. A caller-supplied schema with `additionalProperties: false` and no `findings` property would accept the raw payload in-tool (via the `yield` validator, which only sees the pre-injection data) but then fail post-mortem validation — emitting `schema_violation: findings: must not be present` and propagating as a fatal `RuntimeError` through `agent-bridge.ts` and the eval Python/JS preludes, collapsing the entire workflow cell along with any prior successful subagent work. `normalizeCompleteData` now takes the resolved validator and only performs the injection when the augmented candidate validates. When the schema rejects it, the raw payload is returned instead — which the in- tool yield validator already accepted, so the lockstep guarantee documented at the top of `output-schema-validator.ts` is honored. Findings remain visible via the agent progress stream and JSONL artifact, so no information is dropped when injection is suppressed. Both finalize call paths (yield-success and no-yield fallback) now share the single validator build instead of constructing it twice, and the yield-path schema_violation branch is now reached only via the explicit malformed-schema check, never via spurious findings rejection. Fixes #2070 --- packages/coding-agent/CHANGELOG.md | 4 + packages/coding-agent/src/task/executor.ts | 49 +++-- .../coding-agent/test/tools/review.test.ts | 167 ++++++++++++++++++ 3 files changed, 205 insertions(+), 15 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 0243ed841..b66fce1cb 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed reviewer-style subagent yields crashing the calling eval cell when a caller-supplied output schema declares `additionalProperties: false` without a `findings` property. `normalizeCompleteData` now consults the active validator before splicing collected `report_finding` entries onto the yielded payload, so injection is suppressed when the schema would reject it — keeping the executor's post-mortem validation in lockstep with the in-tool `yield` validation that already accepted the same raw payload ([#2070](https://github.com/can1357/oh-my-pi/issues/2070)) + ## [15.10.1] - 2026-06-07 ### Added diff --git a/packages/coding-agent/src/task/executor.ts b/packages/coding-agent/src/task/executor.ts index 6f47ab87a..f197f534e 100644 --- a/packages/coding-agent/src/task/executor.ts +++ b/packages/coding-agent/src/task/executor.ts @@ -34,7 +34,7 @@ import { SessionManager } from "../session/session-manager"; import { truncateTail } from "../session/streaming-output"; import type { ContextFileEntry } from "../tools"; import { normalizeSchema } from "../tools/jtd-to-json-schema"; -import { buildOutputValidator, summarizeValidationFailure } from "../tools/output-schema-validator"; +import { buildOutputValidator, type OutputValidator, summarizeValidationFailure } from "../tools/output-schema-validator"; import { type ReportFindingDetails, toReviewFinding } from "../tools/review"; import { ToolAbortError } from "../tools/tool-errors"; @@ -256,21 +256,40 @@ function extractCompletionData(parsed: unknown): unknown { return parsed; } -function normalizeCompleteData(data: unknown, reportFindings?: ReviewFinding[]): unknown { - let normalized = parseStringifiedJson(data ?? null); +/** + * Resolve the final yielded payload, optionally splicing collected + * `report_finding` entries into a top-level `findings` array. + * + * Injection is suppressed when an active validator would reject the augmented + * payload (e.g. a caller-supplied schema with `additionalProperties: false` + * that does not declare `findings`). That keeps the in-tool yield validator + * (which only sees the raw, pre-injection data) in lockstep with this + * post-mortem validator — honoring the "accepted in-tool ⇒ accepted + * post-mortem" guarantee documented in `output-schema-validator.ts`. The + * dropped findings are still preserved verbatim in the agent's progress + * stream and JSONL artifact, so no information is lost when injection is + * suppressed. + */ +function normalizeCompleteData( + data: unknown, + reportFindings: ReviewFinding[] | undefined, + validator: OutputValidator | undefined, +): unknown { + const normalized = parseStringifiedJson(data ?? null); if ( - Array.isArray(reportFindings) && - reportFindings.length > 0 && - normalized && - typeof normalized === "object" && - !Array.isArray(normalized) + !Array.isArray(reportFindings) || + reportFindings.length === 0 || + !normalized || + typeof normalized !== "object" || + Array.isArray(normalized) ) { - const record = normalized as Record; - if (!("findings" in record)) { - normalized = { ...record, findings: reportFindings }; - } + return normalized; } - return normalized; + const record = normalized as Record; + if ("findings" in record) return normalized; + const injected = { ...record, findings: reportFindings }; + if (validator && !validator.validate(injected).success) return normalized; + return injected; } function resolveFallbackCompletion(rawOutput: string, outputSchema: unknown): { data: unknown } | null { @@ -360,13 +379,13 @@ export function finalizeSubprocessOutput(args: FinalizeSubprocessOutputArgs): Fi if (submitData === null || submitData === undefined) { rawOutput = rawOutput ? `${SUBAGENT_WARNING_NULL_YIELD}\n\n${rawOutput}` : SUBAGENT_WARNING_NULL_YIELD; } else { - const completeData = normalizeCompleteData(submitData, reportFindings); const { validator, error: schemaError } = buildOutputValidator(outputSchema); if (schemaError) { rawOutput = `{"error":"schema_violation","message":"invalid output schema: ${schemaError.replace(/"/g, '\\"')}"}`; stderr = `schema_violation: invalid output schema: ${schemaError}`; exitCode = 1; } else { + const completeData = normalizeCompleteData(submitData, reportFindings, validator); const result = validator?.validate(completeData) ?? { success: true as const }; if (!result.success) { const summary = summarizeValidationFailure(result, completeData, validator?.requiredFields ?? []); @@ -393,8 +412,8 @@ export function finalizeSubprocessOutput(args: FinalizeSubprocessOutputArgs): Fi const hasOutputSchema = normalizedSchema !== undefined && !schemaError; const fallback = allowFallback ? resolveFallbackCompletion(rawOutput, outputSchema) : null; if (fallback) { - const completeData = normalizeCompleteData(fallback.data, reportFindings); const { validator } = buildOutputValidator(outputSchema); + const completeData = normalizeCompleteData(fallback.data, reportFindings, validator); const result = validator?.validate(completeData) ?? { success: true as const }; if (!result.success) { const summary = summarizeValidationFailure(result, completeData, validator?.requiredFields ?? []); diff --git a/packages/coding-agent/test/tools/review.test.ts b/packages/coding-agent/test/tools/review.test.ts index 989eec31c..c25ac9754 100644 --- a/packages/coding-agent/test/tools/review.test.ts +++ b/packages/coding-agent/test/tools/review.test.ts @@ -132,3 +132,170 @@ describe("toReviewFinding", () => { expect(parsed.findings[0].priority).toBe(2); }); }); + +describe("findings injection respects active output schema", () => { + const finding = toReviewFinding({ + title: "[P0] Example finding", + body: "Details", + priority: "P0", + confidence: 0.95, + file_path: "/tmp/example.ts", + line_start: 10, + line_end: 12, + }); + + // Reproduces #2070: a caller-supplied JSON Schema with + // `additionalProperties: false` and no `findings` property is silently + // rejected post-mortem after the in-tool yield accepted it, because the + // executor auto-injects `findings` from `report_finding`. The injection + // must respect the active schema so that "accepted in-tool ⇒ accepted + // post-mortem" is honored. + it("suppresses findings injection when the schema forbids additional properties", () => { + const callerSchema = { + type: "object", + additionalProperties: false, + required: ["verdict", "acceptance_summary"], + properties: { + verdict: { enum: ["accept", "needs-work", "honest-stop"] }, + acceptance_summary: { type: "string" }, + }, + }; + const data = { verdict: "accept", acceptance_summary: "All good." }; + + const result = finalizeSubprocessOutput({ + rawOutput: "", + exitCode: 0, + stderr: "", + doneAborted: false, + signalAborted: false, + yieldItems: [{ status: "success", data }], + reportFindings: [finding], + outputSchema: callerSchema, + }); + + expect(result.exitCode).toBe(0); + expect(result.stderr).toBe(""); + const parsed = JSON.parse(result.rawOutput) as Record; + expect(parsed).toEqual(data); + expect("findings" in parsed).toBe(false); + }); + + it("still injects findings when the schema declares them (bundled reviewer JTD)", () => { + // Mirrors the bundled reviewer agent's output: findings live under + // `optionalProperties`, so injection must continue to flow through. + const reviewerSchema = { + properties: { + overall_correctness: { enum: ["correct", "incorrect"] }, + explanation: { type: "string" }, + confidence: { type: "number" }, + }, + optionalProperties: { + findings: { + elements: { + properties: { + title: { type: "string" }, + body: { type: "string" }, + priority: { type: "number" }, + confidence: { type: "number" }, + file_path: { type: "string" }, + line_start: { type: "number" }, + line_end: { type: "number" }, + }, + }, + }, + }, + }; + const data = { + overall_correctness: "incorrect", + explanation: "Found one bug", + confidence: 0.9, + }; + + const result = finalizeSubprocessOutput({ + rawOutput: "", + exitCode: 0, + stderr: "", + doneAborted: false, + signalAborted: false, + yieldItems: [{ status: "success", data }], + reportFindings: [finding], + outputSchema: reviewerSchema, + }); + + expect(result.exitCode).toBe(0); + expect(result.stderr).toBe(""); + const parsed = JSON.parse(result.rawOutput) as { findings: Array<{ priority: number }> }; + expect(parsed.findings).toHaveLength(1); + expect(parsed.findings[0].priority).toBe(0); + }); + + it("still injects findings when no schema is declared (legacy free-form)", () => { + const data = { note: "freeform" }; + + const result = finalizeSubprocessOutput({ + rawOutput: "", + exitCode: 0, + stderr: "", + doneAborted: false, + signalAborted: false, + yieldItems: [{ status: "success", data }], + reportFindings: [finding], + outputSchema: undefined, + }); + + expect(result.exitCode).toBe(0); + const parsed = JSON.parse(result.rawOutput) as { findings?: unknown[] }; + expect(parsed.findings).toHaveLength(1); + }); + + it("still injects findings when additionalProperties is open", () => { + const openSchema = { + type: "object", + required: ["verdict"], + properties: { verdict: { type: "string" } }, + }; + const data = { verdict: "accept" }; + + const result = finalizeSubprocessOutput({ + rawOutput: "", + exitCode: 0, + stderr: "", + doneAborted: false, + signalAborted: false, + yieldItems: [{ status: "success", data }], + reportFindings: [finding], + outputSchema: openSchema, + }); + + expect(result.exitCode).toBe(0); + const parsed = JSON.parse(result.rawOutput) as { findings?: unknown[]; verdict: string }; + expect(parsed.verdict).toBe("accept"); + expect(parsed.findings).toHaveLength(1); + }); + + it("suppresses injection on the fallback (no-yield) path when the schema forbids it", () => { + const callerSchema = { + type: "object", + additionalProperties: false, + required: ["verdict"], + properties: { verdict: { type: "string" } }, + }; + const rawJson = JSON.stringify({ data: { verdict: "accept" } }); + + const result = finalizeSubprocessOutput({ + rawOutput: rawJson, + exitCode: 0, + stderr: "", + doneAborted: false, + signalAborted: false, + yieldItems: undefined, + reportFindings: [finding], + outputSchema: callerSchema, + }); + + expect(result.exitCode).toBe(0); + expect(result.stderr).toBe(""); + const parsed = JSON.parse(result.rawOutput) as Record; + expect(parsed).toEqual({ verdict: "accept" }); + }); +}); From 0f243f4505fef0e74fb2100932f0968d1bd6fdcc Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 7 Jun 2026 21:07:48 +0000 Subject: [PATCH 37/46] style: bun run fix --- packages/coding-agent/src/task/executor.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/coding-agent/src/task/executor.ts b/packages/coding-agent/src/task/executor.ts index f197f534e..f6450f2f6 100644 --- a/packages/coding-agent/src/task/executor.ts +++ b/packages/coding-agent/src/task/executor.ts @@ -34,7 +34,11 @@ import { SessionManager } from "../session/session-manager"; import { truncateTail } from "../session/streaming-output"; import type { ContextFileEntry } from "../tools"; import { normalizeSchema } from "../tools/jtd-to-json-schema"; -import { buildOutputValidator, type OutputValidator, summarizeValidationFailure } from "../tools/output-schema-validator"; +import { + buildOutputValidator, + type OutputValidator, + summarizeValidationFailure, +} from "../tools/output-schema-validator"; import { type ReportFindingDetails, toReviewFinding } from "../tools/review"; import { ToolAbortError } from "../tools/tool-errors"; From 7ffec148638aad2ea5bddb295e369c9dbc8af7d9 Mon Sep 17 00:00:00 2001 From: can1357 Date: Mon, 8 Jun 2026 00:32:47 +0200 Subject: [PATCH 38/46] ci: force JavaScript actions to run on Node 24 --- .github/workflows/ci.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 09245ef0a..fad426fb6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,6 +16,9 @@ concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true +env: + FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true + jobs: # scripts/release.ts pushes the version-bump commit and its `v*` tag # atomically (`git push --atomic origin main refs/tags/v*`), so a release From 5ddae194b4f303000102f38038b6c7a7880a8e7c Mon Sep 17 00:00:00 2001 From: can1357 Date: Mon, 8 Jun 2026 00:33:42 +0200 Subject: [PATCH 39/46] test(ai): cover impersonated_service_account ADC Vertex auth flow Add unit and E2E coverage for the impersonated_service_account ADC path: RS256 JWT-signed service_account sources, IAM URL reconstruction, delegate forwarding, and end-to-end routing so the impersonated token (not the source token) authorizes the Vertex request. Reflow google-auth.ts to match. --- .../providers/__tests__/google-auth.test.ts | 171 ++++++++++++++---- packages/ai/src/providers/google-auth.ts | 27 +-- packages/ai/test/stream.test.ts | 132 ++++++++++++++ 3 files changed, 285 insertions(+), 45 deletions(-) diff --git a/packages/ai/src/providers/__tests__/google-auth.test.ts b/packages/ai/src/providers/__tests__/google-auth.test.ts index e6f1aea53..8b72ffcc4 100644 --- a/packages/ai/src/providers/__tests__/google-auth.test.ts +++ b/packages/ai/src/providers/__tests__/google-auth.test.ts @@ -1,41 +1,144 @@ -import { describe, expect, it, mock } from "bun:test"; -import { getVertexAccessToken, __resetVertexTokenCache } from "../google-auth"; +import { afterEach, beforeEach, describe, expect, it } from "bun:test"; +import { Buffer } from "node:buffer"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import type { FetchImpl } from "../../types"; +import { __resetVertexTokenCache, getVertexAccessToken } from "../google-auth"; -describe("getVertexAccessToken", () => { - it("should exchange impersonated ADC correctly", async () => { +const CLOUD_PLATFORM_SCOPE = "https://www.googleapis.com/auth/cloud-platform"; +const JWT_BEARER_GRANT = "urn:ietf:params:oauth:grant-type:jwt-bearer"; + +/** Generate a real RS256 private key so signJwtRs256 / pemToPkcs8 run for real. */ +async function generateServiceAccountPem(): Promise { + const keyPair = (await globalThis.crypto.subtle.generateKey( + { name: "RSASSA-PKCS1-v1_5", modulusLength: 2048, publicExponent: new Uint8Array([1, 0, 1]), hash: "SHA-256" }, + true, + ["sign", "verify"], + )) as CryptoKeyPair; + const pkcs8 = new Uint8Array(await globalThis.crypto.subtle.exportKey("pkcs8", keyPair.privateKey)); + const body = ( + Buffer.from(pkcs8) + .toString("base64") + .match(/.{1,64}/g) ?? [] + ).join("\n"); + return `-----BEGIN PRIVATE KEY-----\n${body}\n-----END PRIVATE KEY-----\n`; +} + +function urlOf(input: string | URL | Request): string { + if (typeof input === "string") return input; + if (input instanceof URL) return input.toString(); + return input.url; +} + +describe("getVertexAccessToken impersonated_service_account ADC", () => { + let tmpDir: string; + let originalGac: string | undefined; + + beforeEach(async () => { __resetVertexTokenCache(); - Bun.env.GOOGLE_APPLICATION_CREDENTIALS = "/tmp/mock-impersonated-adc.json"; - - await Bun.write("/tmp/mock-impersonated-adc.json", JSON.stringify({ - type: "impersonated_service_account", - service_account_impersonation_url: "https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/target@project.iam.gserviceaccount.com:generateAccessToken", - source_credentials: { - type: "authorized_user", - client_id: "client-id", - client_secret: "client-secret", - refresh_token: "refresh-token" - }, - delegates: ["delegate1"] - })); + originalGac = Bun.env.GOOGLE_APPLICATION_CREDENTIALS; + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-vertex-adc-")); + }); - let fetchCalls: any[] = []; - const mockFetch = mock(async (url: string, opts: any) => { - fetchCalls.push({ url, opts }); - if (url.includes("oauth2.googleapis.com/token")) { - return { ok: true, json: async () => ({ access_token: "source-token", expires_in: 3600 }) }; - } - if (url.includes("iamcredentials.googleapis.com")) { - return { ok: true, json: async () => ({ accessToken: "impersonated-token", expireTime: new Date(Date.now() + 3600000).toISOString() }) }; - } - return { ok: false }; - }) as any; + afterEach(async () => { + __resetVertexTokenCache(); + if (originalGac === undefined) delete Bun.env.GOOGLE_APPLICATION_CREDENTIALS; + else Bun.env.GOOGLE_APPLICATION_CREDENTIALS = originalGac; + await fs.rm(tmpDir, { recursive: true, force: true }); + }); - const token = await getVertexAccessToken({ fetch: mockFetch }); + it("rejects a malformed service_account_impersonation_url before any network call", async () => { + const adcPath = path.join(tmpDir, "impersonated-bad-url.json"); + await Bun.write( + adcPath, + JSON.stringify({ + type: "impersonated_service_account", + // Missing the trailing ":generateAccessToken" the principal parser requires. + service_account_impersonation_url: + "https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/target@project.iam.gserviceaccount.com", + source_credentials: { + type: "authorized_user", + client_id: "client-id", + client_secret: "client-secret", + refresh_token: "refresh-token", + }, + }), + ); + Bun.env.GOOGLE_APPLICATION_CREDENTIALS = adcPath; + + const calls: string[] = []; + const fetchImpl: FetchImpl = async input => { + calls.push(urlOf(input)); + return new Response("{}"); + }; + + // The principal is parsed before the source exchange, so a bad URL must fail + // up front rather than after burning a source-token round trip. + await expect(getVertexAccessToken({ fetch: fetchImpl })).rejects.toBeInstanceOf(RangeError); + expect(calls).toEqual([]); + }); + + it("signs an RS256 JWT for a service_account source and reconstructs the IAM URL", async () => { + const pem = await generateServiceAccountPem(); + const adcPath = path.join(tmpDir, "impersonated-sa.json"); + await Bun.write( + adcPath, + JSON.stringify({ + type: "impersonated_service_account", + // Non-canonical project segment proves the request URL is rebuilt, not echoed. + service_account_impersonation_url: + "https://iamcredentials.googleapis.com/v1/projects/explicit-proj/serviceAccounts/target@project.iam.gserviceaccount.com:generateAccessToken", + source_credentials: { + type: "service_account", + client_email: "source@project.iam.gserviceaccount.com", + private_key: pem, + private_key_id: "key-1", + }, + // delegates intentionally omitted — the IAM body must default to []. + }), + ); + Bun.env.GOOGLE_APPLICATION_CREDENTIALS = adcPath; + + const calls: { url: string; init?: RequestInit }[] = []; + const fetchImpl: FetchImpl = async (input, init) => { + const url = urlOf(input); + calls.push({ url, init }); + if (url === "https://oauth2.googleapis.com/token") { + return new Response(JSON.stringify({ access_token: "sa-source-token", expires_in: 3600 })); + } + if (url.startsWith("https://iamcredentials.googleapis.com/")) { + return new Response( + JSON.stringify({ + accessToken: "impersonated-token", + expireTime: new Date(Date.now() + 3_600_000).toISOString(), + }), + ); + } + return new Response("unexpected", { status: 404 }); + }; + + const token = await getVertexAccessToken({ fetch: fetchImpl }); expect(token).toBe("impersonated-token"); - expect(fetchCalls.length).toBe(2); - expect(fetchCalls[0].url).toContain("oauth2.googleapis.com"); - expect(fetchCalls[1].url).toBe("https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/target@project.iam.gserviceaccount.com:generateAccessToken"); - expect(fetchCalls[1].opts.headers.Authorization).toBe("Bearer source-token"); - expect(JSON.parse(fetchCalls[1].opts.body).delegates).toEqual(["delegate1"]); + + // Source JWT exchange happens first, then the impersonation exchange. + expect(calls.map(c => c.url)).toEqual([ + "https://oauth2.googleapis.com/token", + "https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/target@project.iam.gserviceaccount.com:generateAccessToken", + ]); + + // Source credential is exchanged via a signed JWT bearer assertion, not a refresh grant. + const sourceBody = new URLSearchParams(String(calls[0].init?.body)); + expect(sourceBody.get("grant_type")).toBe(JWT_BEARER_GRANT); + expect((sourceBody.get("assertion") ?? "").split(".")).toHaveLength(3); + + // The IAM call carries the source-derived bearer token and defaults delegates to []. + const iamHeaders = calls[1].init?.headers as Record; + expect(iamHeaders.Authorization).toBe("Bearer sa-source-token"); + expect(JSON.parse(String(calls[1].init?.body))).toEqual({ + delegates: [], + scope: [CLOUD_PLATFORM_SCOPE], + lifetime: "3600s", + }); }); }); diff --git a/packages/ai/src/providers/google-auth.ts b/packages/ai/src/providers/google-auth.ts index 39a81922c..0af7ddea8 100644 --- a/packages/ai/src/providers/google-auth.ts +++ b/packages/ai/src/providers/google-auth.ts @@ -212,9 +212,7 @@ async function resolveAccessTokenUncached( ); const targetPrincipal = targetPrincipalMatch?.groups?.target; if (!targetPrincipal) { - throw new RangeError( - `Cannot extract target principal from ${creds.service_account_impersonation_url}`, - ); + throw new RangeError(`Cannot extract target principal from ${creds.service_account_impersonation_url}`); } const sourceToken = @@ -222,15 +220,22 @@ async function resolveAccessTokenUncached( ? await exchangeJwtForToken(creds.source_credentials, signal, fetchImpl) : await exchangeRefreshToken(creds.source_credentials, signal, fetchImpl); - const response = await fetchImpl(`https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/${targetPrincipal}:generateAccessToken`, { - method: "POST", - headers: { - "Content-Type": "application/json", - Authorization: `Bearer ${sourceToken.access_token}`, + const response = await fetchImpl( + `https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/${targetPrincipal}:generateAccessToken`, + { + method: "POST", + headers: { + "Content-Type": "application/json", + Authorization: `Bearer ${sourceToken.access_token}`, + }, + body: JSON.stringify({ + delegates: creds.delegates ?? [], + scope: [CLOUD_PLATFORM_SCOPE], + lifetime: "3600s", + }), + signal, }, - body: JSON.stringify({ delegates: creds.delegates ?? [], scope: [CLOUD_PLATFORM_SCOPE], lifetime: "3600s" }), - signal, - }); + ); if (!response.ok) { const detail = await response.text().catch(() => ""); throw new Error(`Google Impersonation token exchange failed (${response.status}): ${detail}`); diff --git a/packages/ai/test/stream.test.ts b/packages/ai/test/stream.test.ts index aaf0c6070..3d5325d42 100644 --- a/packages/ai/test/stream.test.ts +++ b/packages/ai/test/stream.test.ts @@ -652,6 +652,138 @@ describe("Generate E2E Tests", () => { else Bun.env.GOOGLE_APPLICATION_CREDENTIALS = originalGac; } }); + + it("routes impersonated_service_account ADC through IAM to the Vertex request", async () => { + const originalProject = Bun.env.GOOGLE_CLOUD_PROJECT; + const originalGcpProject = Bun.env.GCP_PROJECT; + const originalGcloudProject = Bun.env.GCLOUD_PROJECT; + const originalVertexLocation = Bun.env.GOOGLE_VERTEX_LOCATION; + const originalCloudLocation = Bun.env.GOOGLE_CLOUD_LOCATION; + const originalLocation = Bun.env.VERTEX_LOCATION; + const originalApiKey = Bun.env.GOOGLE_CLOUD_API_KEY; + const originalGac = Bun.env.GOOGLE_APPLICATION_CREDENTIALS; + const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-vertex-impersonation-")); + const adcPath = path.join(tmpDir, "impersonated-adc.json"); + await Bun.write( + adcPath, + JSON.stringify({ + type: "impersonated_service_account", + service_account_impersonation_url: + "https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/target@project.iam.gserviceaccount.com:generateAccessToken", + source_credentials: { + type: "authorized_user", + client_id: "client-id", + client_secret: "client-secret", + refresh_token: "refresh-token", + }, + delegates: ["projects/-/serviceAccounts/delegate@project.iam.gserviceaccount.com"], + }), + ); + const model: Model<"anthropic-messages"> = { + id: "claude-sonnet-4@20250514", + name: "Claude Sonnet 4", + api: "anthropic-messages", + provider: "google-vertex", + baseUrl: + "https://{location}-aiplatform.googleapis.com/v1/projects/{project}/locations/{location}/publishers/anthropic/models/claude-sonnet-4@20250514:streamRawPredict", + reasoning: true, + input: ["text", "image"], + cost: { input: 3, output: 15, cacheRead: 0.3, cacheWrite: 3.75 }, + contextWindow: 200_000, + maxTokens: 64_000, + }; + const callOrder: string[] = []; + let iamRequest: { url: string; authorization: string | null; body: unknown } | undefined; + const captured = Promise.withResolvers<{ url: string; authorization: string | null }>(); + + try { + __resetVertexTokenCache(); + Bun.env.GOOGLE_CLOUD_PROJECT = "vertex-project"; + Bun.env.GOOGLE_VERTEX_LOCATION = "global"; + delete Bun.env.GCP_PROJECT; + delete Bun.env.GCLOUD_PROJECT; + delete Bun.env.GOOGLE_CLOUD_LOCATION; + delete Bun.env.VERTEX_LOCATION; + delete Bun.env.GOOGLE_CLOUD_API_KEY; + Bun.env.GOOGLE_APPLICATION_CREDENTIALS = adcPath; + + const events = stream( + model, + { messages: [{ role: "user", content: "Hello", timestamp: Date.now() }] }, + { + apiKey: "", + fetch: async (input, init) => { + const url = input instanceof Request ? input.url : input.toString(); + const headers = input instanceof Request ? input.headers : new Headers(init?.headers); + if (url === "https://oauth2.googleapis.com/token") { + callOrder.push("source"); + return new Response(JSON.stringify({ access_token: "source-token", expires_in: 3600 })); + } + if (url.startsWith("https://iamcredentials.googleapis.com/")) { + callOrder.push("iam"); + const bodyText = + input instanceof Request ? await input.clone().text() : String(init?.body ?? ""); + iamRequest = { url, authorization: headers.get("authorization"), body: JSON.parse(bodyText) }; + return new Response( + JSON.stringify({ + accessToken: "impersonated-token", + expireTime: new Date(Date.now() + 3_600_000).toISOString(), + }), + ); + } + callOrder.push("vertex"); + captured.resolve({ url, authorization: headers.get("authorization") }); + return new Response(JSON.stringify({ error: { message: "stop after capture" } }), { status: 400 }); + }, + }, + ); + + for await (const _event of events) { + } + + const request = await captured.promise; + + // Source refresh, then IAM generateAccessToken, then the actual Vertex call. + expect(callOrder).toEqual(["source", "iam", "vertex"]); + + // IAM exchange is authorized by the freshly minted source token, posts the + // reconstructed canonical URL, and forwards the configured delegates verbatim. + expect(iamRequest?.url).toBe( + "https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/target@project.iam.gserviceaccount.com:generateAccessToken", + ); + expect(iamRequest?.authorization).toBe("Bearer source-token"); + expect(iamRequest?.body).toEqual({ + delegates: ["projects/-/serviceAccounts/delegate@project.iam.gserviceaccount.com"], + scope: ["https://www.googleapis.com/auth/cloud-platform"], + lifetime: "3600s", + }); + + // The impersonated token (not the source token) authorizes the Vertex request. + expect(request.url).toBe( + "https://aiplatform.googleapis.com/v1/projects/vertex-project/locations/global/publishers/anthropic/models/claude-sonnet-4@20250514:streamRawPredict", + ); + expect(request.authorization).toBe("Bearer impersonated-token"); + } finally { + __resetVertexTokenCache(); + await fs.rm(tmpDir, { recursive: true, force: true }); + if (originalProject === undefined) delete Bun.env.GOOGLE_CLOUD_PROJECT; + else Bun.env.GOOGLE_CLOUD_PROJECT = originalProject; + if (originalGcpProject === undefined) delete Bun.env.GCP_PROJECT; + else Bun.env.GCP_PROJECT = originalGcpProject; + if (originalGcloudProject === undefined) delete Bun.env.GCLOUD_PROJECT; + else Bun.env.GCLOUD_PROJECT = originalGcloudProject; + if (originalVertexLocation === undefined) delete Bun.env.GOOGLE_VERTEX_LOCATION; + else Bun.env.GOOGLE_VERTEX_LOCATION = originalVertexLocation; + if (originalCloudLocation === undefined) delete Bun.env.GOOGLE_CLOUD_LOCATION; + else Bun.env.GOOGLE_CLOUD_LOCATION = originalCloudLocation; + if (originalLocation === undefined) delete Bun.env.VERTEX_LOCATION; + else Bun.env.VERTEX_LOCATION = originalLocation; + if (originalApiKey === undefined) delete Bun.env.GOOGLE_CLOUD_API_KEY; + else Bun.env.GOOGLE_CLOUD_API_KEY = originalApiKey; + if (originalGac === undefined) delete Bun.env.GOOGLE_APPLICATION_CREDENTIALS; + else Bun.env.GOOGLE_APPLICATION_CREDENTIALS = originalGac; + } + }); }); describe("Google Vertex Provider (gemini-3-flash-preview)", () => { From cfa9a2d53e6bfa9ba05ac95cceaef3b0c4ebd1b5 Mon Sep 17 00:00:00 2001 From: bling <291592093+blingdivinity@users.noreply.github.com> Date: Sun, 7 Jun 2026 11:34:46 -0400 Subject: [PATCH 40/46] fix(coding-agent): resume sessions after a worktree move/rename When a session's working directory is moved or renamed (e.g. `git worktree move`), the session file stays under the old cwd-encoded bucket while the new directory is empty. Resuming was lossy: - `--continue` rejected the terminal breadcrumb purely on cwd mismatch and then found nothing in the new bucket, silently starting a fresh empty session. - cross-project `--resume ` only offered to *fork* (duplicate) the session into the new directory, forcing manual id selection and leaving a stale copy. Detect relocation via the strong, low-false-positive signal "recorded cwd no longer exists on disk" and re-root in place with the existing `moveTo()`: - `continueRecent` re-roots the terminal's last session into the current directory when its recorded cwd is gone and the new location has no sessions of its own (otherwise behavior is unchanged). `readTerminalBreadcrumb` is refactored into `readTerminalBreadcrumbEntry` returning the raw cwd + session file so callers can interpret a cwd mismatch. - cross-project `--resume ` offers "Move (re-root)" instead of fork when the source directory is gone; a still-existing different project still forks. Tests: continue-relocation (re-root on move, no-hijack on plain cd, prefer local recent) and cross-project move-vs-fork routing. --- ...ion-operations-export-share-fork-resume.md | 22 +- packages/coding-agent/CHANGELOG.md | 4 + packages/coding-agent/src/main.ts | 42 +++- .../src/session/session-manager.ts | 100 ++++++-- .../test/main-cross-project-resume.test.ts | 84 +++++-- .../continue-relocation.test.ts | 233 ++++++++++++++++++ 6 files changed, 433 insertions(+), 52 deletions(-) create mode 100644 packages/coding-agent/test/session-manager/continue-relocation.test.ts diff --git a/docs/session-operations-export-share-fork-resume.md b/docs/session-operations-export-share-fork-resume.md index 0d6de4f70..a82ef075d 100644 --- a/docs/session-operations-export-share-fork-resume.md +++ b/docs/session-operations-export-share-fork-resume.md @@ -24,8 +24,8 @@ This document describes operator-visible behavior for session export/share/fork/ | `--fork ` | CLI startup | Yes after session creation | Creates a new session fork from the selected source into current cwd/session dir | None | | `/resume` | Interactive slash command | Yes (active in-memory state replaced) | Switches to selected existing session file | None | | `--resume` | CLI startup picker | Yes after session creation | Opens selected existing session file | None | -| `--resume ` | CLI startup | Yes after session creation | Opens existing session; global cross-project match can fork into current project | None | -| `--continue` | CLI startup | Yes after session creation | Opens terminal breadcrumb or most-recent session; creates new one if none exists | None | +| `--resume ` | CLI startup | Yes after session creation | Opens existing session; global cross-project match re-roots (moved dir) or forks into current project | None | +| `--continue` | CLI startup | Yes after session creation | Opens terminal breadcrumb (re-roots it if its dir was moved) or most-recent session; creates new one if none exists | None | ## Export and dump @@ -215,19 +215,23 @@ Notes: Cross-project id match behavior: -- If matched session cwd differs from current cwd, CLI asks: - - `Session found in different project ... Fork into current directory? [y/N]` -- On yes: `SessionManager.forkFrom(match.path, cwd, sessionDir)` creates a new local forked file. -- On no/non-TTY default: command errors. +- If matched session cwd differs from current cwd, behavior depends on whether the matched session's recorded directory still exists: + - **Directory gone (moved/renamed, e.g. `git worktree move`)**: CLI asks `Session's directory no longer exists (...). Move (re-root) it into the current directory? [Y/n]`. + - On yes (default): `SessionManager.open(match.path)` then `manager.moveTo(cwd)` re-roots the existing session into the current directory (no duplicate file). + - On no: command cancels (returns no session). On non-TTY: command errors. + - **Directory still exists (genuinely different project)**: CLI asks `Session found in different project ... Fork into current directory? [y/N]`. + - On yes: `SessionManager.forkFrom(match.path, cwd, sessionDir)` creates a new local forked file. + - On no: command cancels. On non-TTY: command errors. ## CLI `--continue` `SessionManager.continueRecent(cwd, sessionDir)`: 1. Resolves session dir for current cwd. -2. Reads terminal-scoped breadcrumb first. -3. Falls back to most recently modified session file. -4. Opens found session; if none exists, creates new session. +2. Reads the terminal-scoped breadcrumb. +3. If the breadcrumb points at a session recorded under a different cwd whose directory no longer exists (moved/renamed) **and** the current directory has no sessions of its own, re-roots that session into the current directory via `moveTo` instead of starting fresh. +4. Otherwise, if the breadcrumb's cwd matches the current cwd, uses the breadcrumb session; else falls back to the most recently modified session file. +5. Opens the found session; if none exists, creates a new session. This is startup-only behavior; there is no interactive `/continue` slash command. diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 8b29b5f31..a56a615d5 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -17,6 +17,10 @@ - Fixed MCP tools hanging in non-yolo modes by declaring `approval = "write"` on `MCPTool` and `DeferredMCPTool`, and propagating the `approval` property through `customToolToDefinition()` in `sdk.ts` +### Fixed + +- Fixed session resumption after a working directory is moved/renamed (e.g. `git worktree move`): `--continue` now re-roots the terminal's last session into the new directory when its original directory no longer exists, instead of silently starting a fresh empty session; cross-project `--resume ` offers to move (re-root) the session rather than only forking a duplicate copy when the source directory is gone + ## [15.10.1] - 2026-06-07 ### Added diff --git a/packages/coding-agent/src/main.ts b/packages/coding-agent/src/main.ts index 8d4d30b46..507c59e70 100644 --- a/packages/coding-agent/src/main.ts +++ b/packages/coding-agent/src/main.ts @@ -5,6 +5,7 @@ * createAgentSession() options. The SDK does the heavy lifting. */ +import * as fsSync from "node:fs"; import * as fs from "node:fs/promises"; import * as os from "node:os"; import * as path from "node:path"; @@ -344,11 +345,11 @@ async function runInteractiveMode( } } -type ForkSessionPromptResult = "accepted" | "declined" | "unavailable"; +type SessionPromptResult = "accepted" | "declined" | "unavailable"; -type ForkSessionPrompt = (session: SessionInfo) => Promise; +type SessionPrompt = (session: SessionInfo) => Promise; -async function promptForkSession(session: SessionInfo): Promise { +async function promptForkSession(session: SessionInfo): Promise { if (!process.stdin.isTTY) { return "unavailable"; } @@ -362,6 +363,20 @@ async function promptForkSession(session: SessionInfo): Promise { + if (!process.stdin.isTTY) { + return "unavailable"; + } + const message = `Session's directory no longer exists (${session.cwd}). Move (re-root) it into the current directory? [Y/n] `; + const rl = createInterface({ input: process.stdin, output: process.stdout }); + try { + const answer = (await rl.question(message)).trim().toLowerCase(); + return answer === "" || answer === "y" || answer === "yes" ? "accepted" : "declined"; + } finally { + rl.close(); + } +} + async function getChangelogForDisplay(parsed: Args): Promise { if (parsed.continue || parsed.resume) { return undefined; @@ -407,7 +422,8 @@ export async function createSessionManager( parsed: Args, cwd: string, activeSettings: Settings = settings, - askToForkSession: ForkSessionPrompt = promptForkSession, + askToForkSession: SessionPrompt = promptForkSession, + askToMoveSession: SessionPrompt = promptMoveSession, ): Promise { if (parsed.fork) { if (parsed.noSession) { @@ -440,6 +456,24 @@ export async function createSessionManager( const normalizedCwd = normalizePathForComparison(cwd); const normalizedMatchCwd = normalizePathForComparison(match.session.cwd || cwd); if (normalizedCwd !== normalizedMatchCwd) { + // If the session's recorded directory no longer exists, it was almost + // certainly moved/renamed (e.g. `git worktree move`). Re-root the existing + // session here instead of forking a duplicate copy. + const sourceCwd = match.session.cwd; + if (sourceCwd && !fsSync.existsSync(sourceCwd)) { + const movePromptResult = await askToMoveSession(match.session); + if (movePromptResult === "unavailable") { + throw new Error( + `Session "${sessionArg}" belongs to a directory that no longer exists (${sourceCwd}); run interactively to move it into the current project.`, + ); + } + if (movePromptResult === "declined") { + return undefined; + } + const manager = await SessionManager.open(match.session.path, parsed.sessionDir); + await manager.moveTo(cwd, parsed.sessionDir); + return manager; + } const forkPromptResult = await askToForkSession(match.session); if (forkPromptResult === "unavailable") { throw new Error( diff --git a/packages/coding-agent/src/session/session-manager.ts b/packages/coding-agent/src/session/session-manager.ts index 61f78cdcc..58f5a71bb 100644 --- a/packages/coding-agent/src/session/session-manager.ts +++ b/packages/coding-agent/src/session/session-manager.ts @@ -845,11 +845,18 @@ function writeTerminalBreadcrumb(cwd: string, sessionFile: string): void { Bun.write(breadcrumbFile, content).catch(() => {}); } +interface TerminalBreadcrumb { + cwd: string; + sessionFile: string; +} + /** - * Read the terminal breadcrumb for the current terminal, scoped to a cwd. - * Returns the session file path if it exists and matches the cwd, null otherwise. + * Read the raw terminal breadcrumb for the current terminal. + * Returns the recorded cwd + session file (verified to exist) regardless of + * whether the recorded cwd still matches the current one. Callers decide how + * to interpret a cwd mismatch (e.g. a moved/renamed worktree). */ -async function readTerminalBreadcrumb(cwd: string): Promise { +async function readTerminalBreadcrumbEntry(): Promise { const terminalId = getTerminalId(); if (!terminalId) return null; @@ -862,12 +869,9 @@ async function readTerminalBreadcrumb(cwd: string): Promise { const breadcrumbCwd = lines[0]; const sessionFile = lines[1]; - // Only return if cwd matches (user might have cd'd) - if (path.resolve(breadcrumbCwd) !== path.resolve(cwd)) return null; - // Verify the session file still exists const stat = fs.statSync(sessionFile, { throwIfNoEntry: false }); - if (stat?.isFile()) return sessionFile; + if (stat?.isFile()) return { cwd: breadcrumbCwd, sessionFile }; } catch (err) { if (!isEnoent(err)) logger.debug("Terminal breadcrumb read failed", { err }); // Breadcrumb doesn't exist or is corrupt — fall through @@ -2163,19 +2167,24 @@ export class SessionManager { /** * Move the session to a new working directory. * Moves session files and artifacts on disk, updates all internal references, - * and rewrites the session header with the new cwd. + * and rewrites the session header with the new cwd. When provided, + * `targetSessionDir` is used instead of deriving the default directory for + * the new cwd (for `--continue --session-dir` / `--resume --session-dir`). */ - async moveTo(newCwd: string): Promise { + async moveTo(newCwd: string, targetSessionDir?: string): Promise { const resolvedCwd = path.resolve(newCwd); - if (resolvedCwd === this.cwd) return; + if (resolvedCwd === this.cwd && (!targetSessionDir || path.resolve(targetSessionDir) === this.sessionDir)) return; const managedSessionsRoot = resolveManagedSessionRoot(this.sessionDir, this.cwd); - const newSessionDir = managedSessionsRoot - ? computeDefaultSessionDir(resolvedCwd, this.storage, managedSessionsRoot) - : computeDefaultSessionDir(resolvedCwd, this.storage); + const newSessionDir = targetSessionDir + ? path.resolve(targetSessionDir) + : managedSessionsRoot + ? computeDefaultSessionDir(resolvedCwd, this.storage, managedSessionsRoot) + : computeDefaultSessionDir(resolvedCwd, this.storage); let hadSessionFile = false; if (this.persist && this.#sessionFile) { + this.storage.ensureDirSync(newSessionDir); // Close the persist writer before moving files await this.#closePersistWriter(); this.#persistChain = Promise.resolve(); @@ -2186,25 +2195,29 @@ export class SessionManager { const newSessionFile = path.join(newSessionDir, path.basename(oldSessionFile)); const oldArtifactDir = oldSessionFile.slice(0, -6); // strip .jsonl const newArtifactDir = newSessionFile.slice(0, -6); + const sameSessionFile = path.resolve(oldSessionFile) === path.resolve(newSessionFile); + const sameArtifactDir = path.resolve(oldArtifactDir) === path.resolve(newArtifactDir); hadSessionFile = this.storage.existsSync(oldSessionFile); let movedSessionFile = false; let movedArtifactDir = false; try { // Guard: session file may not exist yet (no assistant messages persisted) - if (hadSessionFile) { + if (hadSessionFile && !sameSessionFile) { await fs.promises.rename(oldSessionFile, newSessionFile); movedSessionFile = true; } - try { - const stat = await fs.promises.stat(oldArtifactDir); - if (stat.isDirectory()) { - await fs.promises.rename(oldArtifactDir, newArtifactDir); - movedArtifactDir = true; + if (!sameArtifactDir) { + try { + const stat = await fs.promises.stat(oldArtifactDir); + if (stat.isDirectory()) { + await fs.promises.rename(oldArtifactDir, newArtifactDir); + movedArtifactDir = true; + } + } catch (err) { + if (!isEnoent(err)) throw err; } - } catch (err) { - if (!isEnoent(err)) throw err; } } catch (err) { if (movedArtifactDir) { @@ -3491,8 +3504,49 @@ export class SessionManager { ): Promise { const dir = sessionDir ?? SessionManager.getDefaultSessionDir(cwd, undefined, storage); // Prefer terminal-scoped breadcrumb (handles concurrent sessions correctly) - const terminalSession = await readTerminalBreadcrumb(cwd); - const mostRecent = terminalSession ?? (await findMostRecentSession(dir, storage)); + const breadcrumb = await readTerminalBreadcrumbEntry(); + const breadcrumbCwd = breadcrumb ? path.resolve(breadcrumb.cwd) : undefined; + const resolvedCwd = path.resolve(cwd); + let mostRecent: string | null | undefined; + if (breadcrumb && breadcrumbCwd !== resolvedCwd) { + // The terminal's last session was started in a different cwd. If that cwd no + // longer exists (e.g. `git worktree move`/dir rename) and the new location has + // no sessions of its own, re-root the session here instead of silently starting + // fresh — otherwise the relocated session would be unreachable via --continue. + // When an explicit sessionDir is reused across the move, the stale breadcrumb + // file itself may be the most recent entry there; don't count it as a + // current-directory session. If that shared dir also contains an older session + // that already belongs to the current cwd, prefer that local session instead + // of re-rooting the stale breadcrumb over it. + const resolvedBreadcrumbCwd = breadcrumbCwd ?? path.resolve(breadcrumb.cwd); + mostRecent = await findMostRecentSession(dir, storage); + const sourceCwdGone = !fs.existsSync(resolvedBreadcrumbCwd); + const breadcrumbSessionFile = path.resolve(breadcrumb.sessionFile); + const mostRecentIsBreadcrumb = + mostRecent !== null && mostRecent !== undefined && path.resolve(mostRecent) === breadcrumbSessionFile; + let hasCurrentCwdSession = false; + if (sourceCwdGone && mostRecentIsBreadcrumb) { + const currentCwdSession = (await SessionManager.list(cwd, dir, storage)).find( + session => + path.resolve(session.path) !== breadcrumbSessionFile && + session.cwd !== undefined && + path.resolve(session.cwd) === resolvedCwd, + ); + if (currentCwdSession) { + mostRecent = currentCwdSession.path; + hasCurrentCwdSession = true; + } + } + const relocated = sourceCwdGone && (mostRecent === null || (mostRecentIsBreadcrumb && !hasCurrentCwdSession)); + if (relocated) { + process.stderr.write(`Re-rooting moved session from ${resolvedBreadcrumbCwd} to ${resolvedCwd}.\n`); + const manager = await SessionManager.open(breadcrumb.sessionFile, undefined, storage); + await manager.moveTo(cwd, sessionDir); + return manager; + } + } + const terminalSession = breadcrumb && breadcrumbCwd === resolvedCwd ? breadcrumb.sessionFile : null; + if (mostRecent === undefined) mostRecent = terminalSession ?? (await findMostRecentSession(dir, storage)); const manager = new SessionManager(cwd, dir, true, storage); if (mostRecent) { await manager.#initSessionFile(mostRecent); diff --git a/packages/coding-agent/test/main-cross-project-resume.test.ts b/packages/coding-agent/test/main-cross-project-resume.test.ts index 02348fc4d..6688ca44c 100644 --- a/packages/coding-agent/test/main-cross-project-resume.test.ts +++ b/packages/coding-agent/test/main-cross-project-resume.test.ts @@ -2,8 +2,16 @@ * Regression: declining the cross-project fork prompt during `--resume ` * must exit cleanly, while non-interactive resume still fails instead of * silently succeeding. See #1668. + * + * Also covers the moved/renamed-worktree path: when the matched session's + * recorded directory no longer exists, `--resume ` offers to *move* + * (re-root) the session rather than fork a duplicate. */ -import { afterEach, describe, expect, it, vi } from "bun:test"; +import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; +import * as fs from "node:fs"; +import * as fsp from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; import type { Args } from "@oh-my-pi/pi-coding-agent/cli/args"; import type { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import { createSessionManager } from "@oh-my-pi/pi-coding-agent/main"; @@ -37,20 +45,27 @@ function buildGlobalMatch(cwd: string): { session: SessionInfo; scope: "global" }; } +const stubSettings = { get: () => undefined } as unknown as Settings; + describe("createSessionManager — cross-project --resume cancellation (#1668)", () => { - afterEach(() => { + // An existing directory so the match is treated as a genuinely different + // project (fork path), not a moved/renamed worktree (move path). + let existingProject: string; + + beforeEach(async () => { + existingProject = await fsp.mkdtemp(path.join(os.tmpdir(), "omp-xproj-")); + }); + + afterEach(async () => { vi.restoreAllMocks(); + await fsp.rm(existingProject, { recursive: true, force: true }); }); it("returns undefined when an interactive user declines the fork prompt instead of throwing", async () => { - const sessionCwd = "/some/other/project"; - vi.spyOn(sessionManagerModule, "resolveResumableSession").mockResolvedValue(buildGlobalMatch(sessionCwd)); - - const args = buildArgs("019e84ed"); - const stubSettings = { get: () => undefined } as unknown as Settings; + vi.spyOn(sessionManagerModule, "resolveResumableSession").mockResolvedValue(buildGlobalMatch(existingProject)); const result = await createSessionManager( - args, + buildArgs("019e84ed"), "/current/project", stubSettings, async () => "declined" as const, @@ -61,15 +76,52 @@ describe("createSessionManager — cross-project --resume cancellation (#1668)", it("throws when the cross-project fork prompt is unavailable in non-interactive mode", async () => { expect(process.stdin.isTTY).toBeFalsy(); + vi.spyOn(sessionManagerModule, "resolveResumableSession").mockResolvedValue(buildGlobalMatch(existingProject)); - const sessionCwd = "/some/other/project"; - vi.spyOn(sessionManagerModule, "resolveResumableSession").mockResolvedValue(buildGlobalMatch(sessionCwd)); - - const args = buildArgs("019e84ed"); - const stubSettings = { get: () => undefined } as unknown as Settings; - - await expect(createSessionManager(args, "/current/project", stubSettings)).rejects.toThrow( - 'Session "019e84ed" is in another project (/some/other/project); run interactively to fork it into the current project.', + await expect(createSessionManager(buildArgs("019e84ed"), "/current/project", stubSettings)).rejects.toThrow( + `Session "019e84ed" is in another project (${existingProject}); run interactively to fork it into the current project.`, + ); + }); +}); + +describe("createSessionManager — cross-project --resume relocation (moved worktree)", () => { + let missingRoot: string; + let missingProject: string; + + beforeEach(async () => { + missingRoot = await fsp.mkdtemp(path.join(os.tmpdir(), "omp-moved-xproj-")); + missingProject = path.join(missingRoot, "worktree-gone"); + }); + + afterEach(async () => { + vi.restoreAllMocks(); + await fsp.rm(missingRoot, { recursive: true, force: true }); + }); + + it("offers move (not fork) and returns undefined when the user declines", async () => { + vi.spyOn(sessionManagerModule, "resolveResumableSession").mockResolvedValue(buildGlobalMatch(missingProject)); + expect(fs.existsSync(missingProject)).toBe(false); + + const forkPrompt = vi.fn(async () => "accepted" as const); + const result = await createSessionManager( + buildArgs("019e84ed"), + "/current/project", + stubSettings, + forkPrompt, + async () => "declined" as const, + ); + + expect(result).toBeUndefined(); + // The fork prompt must NOT be used for a relocated (gone-dir) session. + expect(forkPrompt).not.toHaveBeenCalled(); + }); + + it("throws the move-specific error when unavailable in non-interactive mode", async () => { + expect(process.stdin.isTTY).toBeFalsy(); + vi.spyOn(sessionManagerModule, "resolveResumableSession").mockResolvedValue(buildGlobalMatch(missingProject)); + + await expect(createSessionManager(buildArgs("019e84ed"), "/current/project", stubSettings)).rejects.toThrow( + `Session "019e84ed" belongs to a directory that no longer exists (${missingProject}); run interactively to move it into the current project.`, ); }); }); diff --git a/packages/coding-agent/test/session-manager/continue-relocation.test.ts b/packages/coding-agent/test/session-manager/continue-relocation.test.ts new file mode 100644 index 000000000..681802f78 --- /dev/null +++ b/packages/coding-agent/test/session-manager/continue-relocation.test.ts @@ -0,0 +1,233 @@ +import { afterEach, beforeEach, describe, expect, it } from "bun:test"; +import * as fs from "node:fs"; +import * as fsp from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import { + loadEntriesFromFile, + type SessionHeader, + SessionManager, +} from "@oh-my-pi/pi-coding-agent/session/session-manager"; +import { getTerminalId } from "@oh-my-pi/pi-tui"; +import { getConfigRootDir, getTerminalSessionsDir, setAgentDir } from "@oh-my-pi/pi-utils"; + +import { makeAssistantMessage } from "./helpers"; + +function getHeader(entries: unknown[]): SessionHeader | undefined { + return entries.find( + (e): e is SessionHeader => + typeof e === "object" && e !== null && "type" in e && (e as { type: unknown }).type === "session", + ); +} + +function writeBreadcrumb(cwd: string, sessionFile: string): string { + const terminalId = getTerminalId(); + if (!terminalId) throw new Error("Expected a terminal id for breadcrumb test"); + const dir = getTerminalSessionsDir(); + fs.mkdirSync(dir, { recursive: true }); + const file = path.join(dir, terminalId); + fs.writeFileSync(file, `${cwd}\n${sessionFile}\n`); + return file; +} + +describe("SessionManager.continueRecent relocation", () => { + let testAgentDir: string; + let cwdA: string; + let cwdB: string; + const originalAgentDir = process.env.PI_CODING_AGENT_DIR; + const originalTmuxPane = process.env.TMUX_PANE; + const fallbackAgentDir = path.join(getConfigRootDir(), "agent"); + + beforeEach(async () => { + // Force a deterministic, non-TTY terminal id so breadcrumb read/write is stable. + process.env.TMUX_PANE = "%relocation-test"; + testAgentDir = await fsp.mkdtemp(path.join(os.tmpdir(), "omp-reloc-test-")); + setAgentDir(testAgentDir); + cwdA = path.join(testAgentDir, "worktree-old"); + cwdB = path.join(testAgentDir, "worktree-new"); + fs.mkdirSync(cwdA, { recursive: true }); + fs.mkdirSync(cwdB, { recursive: true }); + }); + + afterEach(async () => { + if (originalTmuxPane === undefined) delete process.env.TMUX_PANE; + else process.env.TMUX_PANE = originalTmuxPane; + if (originalAgentDir) { + setAgentDir(originalAgentDir); + } else { + setAgentDir(fallbackAgentDir); + delete process.env.PI_CODING_AGENT_DIR; + } + await fsp.rm(testAgentDir, { recursive: true, force: true }); + }); + + it("re-roots the terminal's session when its directory was moved/renamed", async () => { + const session = SessionManager.create(cwdA); + session.appendMessage({ role: "user", content: "before move", timestamp: 1 }); + session.appendMessage(makeAssistantMessage()); + await session.flush(); + const oldFile = session.getSessionFile(); + if (!oldFile) throw new Error("Expected persisted session file"); + await session.close(); + + // Breadcrumb points at the old session, recorded under the old cwd. + writeBreadcrumb(cwdA, oldFile); + // Simulate `git worktree move`: the old directory no longer exists. + await fsp.rm(cwdA, { recursive: true, force: true }); + + const resumed = await SessionManager.continueRecent(cwdB); + try { + // The relocated session is adopted, not discarded for a fresh one. + expect(resumed.getCwd()).toBe(path.resolve(cwdB)); + const newFile = resumed.getSessionFile(); + if (!newFile) throw new Error("Expected re-rooted session file"); + expect(newFile).not.toBe(oldFile); + expect(fs.existsSync(oldFile)).toBe(false); + + const entries = await loadEntriesFromFile(newFile); + expect(getHeader(entries)?.cwd).toBe(path.resolve(cwdB)); + const userMessages = entries.filter(e => e.type === "message" && e.message.role === "user"); + expect(userMessages).toHaveLength(1); + } finally { + await resumed.close(); + } + }); + + it("does not hijack the session when the recorded directory still exists (plain cd)", async () => { + const session = SessionManager.create(cwdA); + session.appendMessage({ role: "user", content: "other project", timestamp: 1 }); + session.appendMessage(makeAssistantMessage()); + await session.flush(); + const oldFile = session.getSessionFile(); + if (!oldFile) throw new Error("Expected persisted session file"); + await session.close(); + + // Breadcrumb from a still-existing different project; user just cd'd elsewhere. + writeBreadcrumb(cwdA, oldFile); + + const resumed = await SessionManager.continueRecent(cwdB); + try { + // Old project's session is left untouched; a fresh session starts in cwdB. + expect(fs.existsSync(oldFile)).toBe(true); + expect(resumed.getSessionFile()).not.toBe(oldFile); + expect(resumed.getEntries()).toHaveLength(0); + } finally { + await resumed.close(); + } + }); + + it("does not re-root when the new directory already has its own sessions", async () => { + const moved = SessionManager.create(cwdA); + moved.appendMessage({ role: "user", content: "moved", timestamp: 1 }); + moved.appendMessage(makeAssistantMessage()); + await moved.flush(); + const movedFile = moved.getSessionFile(); + if (!movedFile) throw new Error("Expected persisted session file"); + await moved.close(); + + // cwdB already owns a local session. + const local = SessionManager.create(cwdB); + local.appendMessage({ role: "user", content: "local", timestamp: 2 }); + local.appendMessage(makeAssistantMessage()); + await local.flush(); + const localFile = local.getSessionFile(); + if (!localFile) throw new Error("Expected persisted local session file"); + await local.close(); + + writeBreadcrumb(cwdA, movedFile); + await fsp.rm(cwdA, { recursive: true, force: true }); + + const resumed = await SessionManager.continueRecent(cwdB); + try { + // Prefer cwdB's own recent session over re-rooting the moved one. + expect(resumed.getSessionFile()).toBe(localFile); + expect(fs.existsSync(movedFile)).toBe(true); + } finally { + await resumed.close(); + } + }); + + it("moves a relocated breadcrumb session into an explicit sessionDir", async () => { + const session = SessionManager.create(cwdA); + session.appendMessage({ role: "user", content: "explicit dir", timestamp: 1 }); + session.appendMessage(makeAssistantMessage()); + await session.flush(); + const oldFile = session.getSessionFile(); + if (!oldFile) throw new Error("Expected persisted session file"); + await session.close(); + + const explicitSessionDir = path.join(testAgentDir, "custom-sessions"); + writeBreadcrumb(cwdA, oldFile); + await fsp.rm(cwdA, { recursive: true, force: true }); + + const resumed = await SessionManager.continueRecent(cwdB, explicitSessionDir); + try { + const newFile = resumed.getSessionFile(); + if (!newFile) throw new Error("Expected re-rooted session file"); + expect(path.dirname(newFile)).toBe(path.resolve(explicitSessionDir)); + expect(fs.existsSync(oldFile)).toBe(false); + expect(getHeader(await loadEntriesFromFile(newFile))?.cwd).toBe(path.resolve(cwdB)); + } finally { + await resumed.close(); + } + }); + + it("re-roots when the stale breadcrumb file is already in the explicit sessionDir", async () => { + const explicitSessionDir = path.join(testAgentDir, "shared-custom-sessions"); + const session = SessionManager.create(cwdA, explicitSessionDir); + session.appendMessage({ role: "user", content: "same explicit dir", timestamp: 1 }); + session.appendMessage(makeAssistantMessage()); + await session.flush(); + const oldFile = session.getSessionFile(); + if (!oldFile) throw new Error("Expected persisted session file"); + expect(path.dirname(oldFile)).toBe(path.resolve(explicitSessionDir)); + await session.close(); + + writeBreadcrumb(cwdA, oldFile); + await fsp.rm(cwdA, { recursive: true, force: true }); + + const resumed = await SessionManager.continueRecent(cwdB, explicitSessionDir); + try { + const newFile = resumed.getSessionFile(); + if (!newFile) throw new Error("Expected re-rooted session file"); + expect(newFile).toBe(oldFile); + expect(resumed.getCwd()).toBe(path.resolve(cwdB)); + expect(getHeader(await loadEntriesFromFile(newFile))?.cwd).toBe(path.resolve(cwdB)); + } finally { + await resumed.close(); + } + }); + + it("prefers an existing current-cwd session in a shared explicit sessionDir", async () => { + const explicitSessionDir = path.join(testAgentDir, "shared-current-sessions"); + const local = SessionManager.create(cwdB, explicitSessionDir); + local.appendMessage({ role: "user", content: "local current cwd", timestamp: 1 }); + local.appendMessage(makeAssistantMessage()); + await local.flush(); + const localFile = local.getSessionFile(); + if (!localFile) throw new Error("Expected persisted local session file"); + await local.close(); + + // Ensure the stale moved session is newer than the local current-cwd session. + await new Promise(resolve => setTimeout(resolve, 20)); + const moved = SessionManager.create(cwdA, explicitSessionDir); + moved.appendMessage({ role: "user", content: "newer stale moved cwd", timestamp: 2 }); + moved.appendMessage(makeAssistantMessage()); + await moved.flush(); + const movedFile = moved.getSessionFile(); + if (!movedFile) throw new Error("Expected persisted moved session file"); + await moved.close(); + + writeBreadcrumb(cwdA, movedFile); + await fsp.rm(cwdA, { recursive: true, force: true }); + + const resumed = await SessionManager.continueRecent(cwdB, explicitSessionDir); + try { + expect(resumed.getSessionFile()).toBe(localFile); + expect(resumed.getCwd()).toBe(path.resolve(cwdB)); + expect(fs.existsSync(movedFile)).toBe(true); + } finally { + await resumed.close(); + } + }); +}); From bad1e2acd6d419204b433a132e734c89e151470a Mon Sep 17 00:00:00 2001 From: can1357 Date: Mon, 8 Jun 2026 00:42:23 +0200 Subject: [PATCH 41/46] fix: restricted GitHub Copilot auth to COPILOT_GITHUB_TOKEN - Changed the `github-copilot` service provider to resolve credentials only from `COPILOT_GITHUB_TOKEN`. - Updated CLI extra help text to document `COPILOT_GITHUB_TOKEN` as the GitHub Copilot environment variable. - Reworded environment variable docs to reflect the revised Copilot/GitHub token usage and order. --- docs/environment-variables.md | 12 ++++++------ packages/ai/src/stream.ts | 3 +-- packages/coding-agent/src/cli/args.ts | 2 +- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/docs/environment-variables.md b/docs/environment-variables.md index 60b1da7aa..7d2bc9b75 100644 --- a/docs/environment-variables.md +++ b/docs/environment-variables.md @@ -81,13 +81,13 @@ These are consumed via `getEnvApiKey()` (`packages/ai/src/stream.ts`) unless not | `WAFER_SERVERLESS_API_KEY` | Wafer Serverless auth | Using `wafer-serverless` provider | Pay-as-you-go Wafer SKU; validated against `https://pass.wafer.ai/v1/models` | | `GITLAB_TOKEN` | GitLab Duo auth | Using `gitlab-duo` provider | | -### GitHub/Copilot token chains +### GitHub/Copilot tokens -| Variable | Used for | Chain | -| ---------------------- | ------------------------------------------------ | ---------------------------------------------------- | -| `COPILOT_GITHUB_TOKEN` | GitHub Copilot provider auth | `COPILOT_GITHUB_TOKEN` → `GH_TOKEN` → `GITHUB_TOKEN` | -| `GH_TOKEN` | Copilot fallback; GitHub API auth in web scraper | In web scraper: `GITHUB_TOKEN` → `GH_TOKEN` | -| `GITHUB_TOKEN` | Copilot fallback; GitHub API auth in web scraper | In web scraper: checked before `GH_TOKEN` | +| Variable | Used for | Notes | +| ---------------------- | ------------------------------------------------ | ------------------------------------------ | +| `COPILOT_GITHUB_TOKEN` | GitHub Copilot provider auth | Generic GitHub tokens are not used here | +| `GH_TOKEN` | GitHub API auth in web scraper | Web scraper fallback after `GITHUB_TOKEN` | +| `GITHUB_TOKEN` | GitHub API auth in web scraper | Web scraper checks this before `GH_TOKEN` | ### Auth broker / auth gateway (remote credential vault) diff --git a/packages/ai/src/stream.ts b/packages/ai/src/stream.ts index f592dcd89..f44c0a5b4 100644 --- a/packages/ai/src/stream.ts +++ b/packages/ai/src/stream.ts @@ -209,8 +209,7 @@ const serviceProviderMap: Record = { tavily: "TAVILY_API_KEY", parallel: "PARALLEL_API_KEY", kagi: "KAGI_API_KEY", - // GitHub Copilot uses GitHub personal access token - "github-copilot": () => $pickenv("COPILOT_GITHUB_TOKEN", "GH_TOKEN", "GITHUB_TOKEN"), + "github-copilot": "COPILOT_GITHUB_TOKEN", // Foundry mode optionally switches Anthropic auth to enterprise gateway credentials. anthropic: () => isFoundryEnabled() diff --git a/packages/coding-agent/src/cli/args.ts b/packages/coding-agent/src/cli/args.ts index 707d87c54..f81c24563 100644 --- a/packages/coding-agent/src/cli/args.ts +++ b/packages/coding-agent/src/cli/args.ts @@ -258,7 +258,7 @@ export function getExtraHelpText(): string { NODE_EXTRA_CA_CERTS - CA bundle path (or inline PEM) for server certificate validation OPENAI_API_KEY - OpenAI GPT models GEMINI_API_KEY - Google Gemini models - GITHUB_TOKEN - GitHub Copilot (or GH_TOKEN, COPILOT_GITHUB_TOKEN) + COPILOT_GITHUB_TOKEN - GitHub Copilot ${chalk.dim("# Additional LLM Providers")} AZURE_OPENAI_API_KEY - Azure OpenAI models From ba4abb7e2f8863e2aef1a8ab25d9d032c1fe2f1c Mon Sep 17 00:00:00 2001 From: can1357 Date: Mon, 8 Jun 2026 00:47:39 +0200 Subject: [PATCH 42/46] fix(coding-agent): excluded cwd-less legacy sessions from relocation guard - Guard now skips sessions with empty cwd, not just undefined. - Prevents path.resolve("") collision from hijacking a moved session. --- .../src/session/session-manager.ts | 4 +- .../continue-relocation.test.ts | 54 +++++++++++++++++++ 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/packages/coding-agent/src/session/session-manager.ts b/packages/coding-agent/src/session/session-manager.ts index 58f5a71bb..8ad4a4d95 100644 --- a/packages/coding-agent/src/session/session-manager.ts +++ b/packages/coding-agent/src/session/session-manager.ts @@ -3518,7 +3518,7 @@ export class SessionManager { // current-directory session. If that shared dir also contains an older session // that already belongs to the current cwd, prefer that local session instead // of re-rooting the stale breadcrumb over it. - const resolvedBreadcrumbCwd = breadcrumbCwd ?? path.resolve(breadcrumb.cwd); + const resolvedBreadcrumbCwd = path.resolve(breadcrumb.cwd); mostRecent = await findMostRecentSession(dir, storage); const sourceCwdGone = !fs.existsSync(resolvedBreadcrumbCwd); const breadcrumbSessionFile = path.resolve(breadcrumb.sessionFile); @@ -3529,7 +3529,7 @@ export class SessionManager { const currentCwdSession = (await SessionManager.list(cwd, dir, storage)).find( session => path.resolve(session.path) !== breadcrumbSessionFile && - session.cwd !== undefined && + session.cwd && path.resolve(session.cwd) === resolvedCwd, ); if (currentCwdSession) { diff --git a/packages/coding-agent/test/session-manager/continue-relocation.test.ts b/packages/coding-agent/test/session-manager/continue-relocation.test.ts index 681802f78..751c16bd3 100644 --- a/packages/coding-agent/test/session-manager/continue-relocation.test.ts +++ b/packages/coding-agent/test/session-manager/continue-relocation.test.ts @@ -30,6 +30,17 @@ function writeBreadcrumb(cwd: string, sessionFile: string): string { return file; } +function stripHeaderCwd(file: string): void { + const lines = fs.readFileSync(file, "utf8").split("\n"); + const rewritten = lines.map(line => { + if (!line.trim()) return line; + const obj = JSON.parse(line) as { type?: string; cwd?: unknown }; + if (obj.type === "session") delete obj.cwd; + return JSON.stringify(obj); + }); + fs.writeFileSync(file, rewritten.join("\n")); +} + describe("SessionManager.continueRecent relocation", () => { let testAgentDir: string; let cwdA: string; @@ -230,4 +241,47 @@ describe("SessionManager.continueRecent relocation", () => { await resumed.close(); } }); + + it("re-roots past a cwd-less legacy session in a shared explicit sessionDir", async () => { + // Regression: SessionInfo.cwd is "" for sessions whose header has no cwd, and + // path.resolve("") === process.cwd(). A guard that only excluded `undefined` + // treated such a legacy session as "belongs to the current cwd" whenever + // --continue ran from process.cwd(), hijacking the moved session. Resume must + // be invoked with process.cwd() to reproduce the path.resolve("") collision. + const explicitSessionDir = path.join(testAgentDir, "shared-legacy-sessions"); + const currentCwd = process.cwd(); + + // Older session with no recorded cwd (header cwd stripped → "" on load). + const legacy = SessionManager.create(cwdB, explicitSessionDir); + legacy.appendMessage({ role: "user", content: "legacy cwd-less", timestamp: 1 }); + legacy.appendMessage(makeAssistantMessage()); + await legacy.flush(); + const legacyFile = legacy.getSessionFile(); + if (!legacyFile) throw new Error("Expected persisted legacy session file"); + await legacy.close(); + stripHeaderCwd(legacyFile); + + // Newer moved session, recorded under the now-missing worktree cwd. + await new Promise(resolve => setTimeout(resolve, 20)); + const moved = SessionManager.create(cwdA, explicitSessionDir); + moved.appendMessage({ role: "user", content: "newer moved cwd", timestamp: 2 }); + moved.appendMessage(makeAssistantMessage()); + await moved.flush(); + const movedFile = moved.getSessionFile(); + if (!movedFile) throw new Error("Expected persisted moved session file"); + await moved.close(); + + writeBreadcrumb(cwdA, movedFile); + await fsp.rm(cwdA, { recursive: true, force: true }); + + const resumed = await SessionManager.continueRecent(currentCwd, explicitSessionDir); + try { + // The moved session is re-rooted; the cwd-less legacy session is not hijacked. + expect(resumed.getSessionFile()).toBe(movedFile); + expect(resumed.getCwd()).toBe(path.resolve(currentCwd)); + expect(fs.existsSync(legacyFile)).toBe(true); + } finally { + await resumed.close(); + } + }); }); From 181419b65baca245b3b0b2a7a20914a8ea65f04b Mon Sep 17 00:00:00 2001 From: can1357 Date: Mon, 8 Jun 2026 00:53:43 +0200 Subject: [PATCH 43/46] fix(coding-agent/modes): fixed Esc interrupt acknowledgement to update loader immediately - Added an interrupt state in EventController to switch the working label to `Interrupting...` and suspend intent-driven updates until the turn resets. - Called `notifyInterrupting()` from streaming-interrupt paths in InputController and exposed it on InteractiveModeContext so Esc acknowledgement shows immediately while tools tear down. - Added tests to verify loader acknowledgement, freeze of late intent updates during interruption, and label updates resuming on the next agent start. --- packages/coding-agent/CHANGELOG.md | 1 + .../src/modes/controllers/event-controller.ts | 26 ++++ .../src/modes/controllers/input-controller.ts | 2 + .../src/modes/interactive-mode.ts | 4 + packages/coding-agent/src/modes/types.ts | 3 + .../test/input-controller-escape.test.ts | 1 + .../event-controller-interrupt.test.ts | 124 ++++++++++++++++++ 7 files changed, 161 insertions(+) create mode 100644 packages/coding-agent/test/modes/controllers/event-controller-interrupt.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index d3f1b5f18..8a62c2743 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -3,6 +3,7 @@ ## [Unreleased] ### Fixed +- Fixed the working spinner appearing to ignore Esc for 2-3 seconds when an interrupt lands mid-tool. Esc fires the abort synchronously, but the agent loop only stops the loader at `agent_end`, which it cannot reach until every in-flight tool settles in `executeToolCalls`' `await Promise.allSettled(...)` — and process/subagent/kernel-owning tools tear down gracefully (SIGTERM, 2-3s grace, SIGKILL), so the loader kept showing the unchanged "Working…/" line and read as a dropped keypress. The loader now switches to "Interrupting…" the instant Esc requests the abort and freezes intent-driven label updates until the turn unwinds (`EventController.notifyInterrupting`), so the interrupt is acknowledged immediately even while teardown completes. - Fixed a flaky JS eval worker startup that intermittently failed unrelated CI runs. The worker-ready wait reused Bun's 5s default per-test timeout as its floor, so a slow cold-start under `--isolate` + high concurrency was aborted mid-init; terminating a still-initializing Bun worker is the documented SIGILL/SIGTRAP crash trigger, which took down the whole test file. Worker init now floors at a fixed 15s infrastructure budget (independent of, and still dominated by, a larger per-cell `timeout`), and the JS eval test suites set a 20s file-local timeout so cold starts complete instead of being torn down. - Fixed reviewer-style subagent yields crashing the calling eval cell when a caller-supplied output schema declares `additionalProperties: false` without a `findings` property. `normalizeCompleteData` now consults the active validator before splicing collected `report_finding` entries onto the yielded payload, so injection is suppressed when the schema would reject it — keeping the executor's post-mortem validation in lockstep with the in-tool `yield` validation that already accepted the same raw payload ([#2070](https://github.com/can1357/oh-my-pi/issues/2070)) - Fixed Anthropic empty `toolUse` stops without tool calls corrupting session history by retrying them and removing orphaned turns even at the retry cap. diff --git a/packages/coding-agent/src/modes/controllers/event-controller.ts b/packages/coding-agent/src/modes/controllers/event-controller.ts index d1f1e8e28..97e571d0e 100644 --- a/packages/coding-agent/src/modes/controllers/event-controller.ts +++ b/packages/coding-agent/src/modes/controllers/event-controller.ts @@ -26,6 +26,16 @@ type AgentSessionEventKind = AgentSessionEvent["type"]; const IRC_MESSAGE_VISIBLE_TTL_MS = 10_000; +/** + * Loader label shown the instant a user interrupt (Esc) is requested, kept until + * the agent turn fully unwinds. Esc fires the abort synchronously, but the loop + * only stops the spinner at `agent_end`, which it cannot reach until every + * in-flight tool settles its abort in `executeToolCalls` (`Promise.allSettled`). + * Swapping the steady "Working…" for this acknowledges the keypress instead of + * reading as an ignored Esc for the seconds a slow tool takes to tear down. + */ +export const INTERRUPTING_WORKING_MESSAGE = "Interrupting…"; + // Events that change foreground streaming state, or that reset a turn. The TUI // eager native-scrollback rebuild mode is recomputed only on these so unrelated // IRC/notices/status refreshes do not toggle scrollback replay policy. @@ -57,6 +67,7 @@ export class EventController { #backgroundToolCallIds = new Set(); #assistantMessageStreaming = false; #agentTurnActive = false; + #interrupting = false; #readToolCallArgs = new Map>(); #readToolCallAssistantComponents = new Map(); #lastAssistantComponent: AssistantMessageComponent | undefined = undefined; @@ -167,6 +178,7 @@ export class EventController { return true; } #updateWorkingMessageFromIntent(intent: unknown): void { + if (this.#interrupting) return; // Streamed JSON can deliver non-string `_i` (object, number, boolean) before // schema validation; `?.` only guards null/undefined, so guard the type too. if (typeof intent !== "string") return; @@ -176,6 +188,19 @@ export class EventController { this.ctx.setWorkingMessage(`${trimmed}${interruptHint()}`); } + /** + * Acknowledge a user interrupt (Esc) immediately: switch the loader to + * `INTERRUPTING_WORKING_MESSAGE` and freeze intent-driven working-message + * updates for the rest of the turn so a late `tool_execution_start` intent + * cannot repaint a "Working…/" line over the acknowledgment. Reset at + * the next `agent_start`. No-op outside an active turn or if already set. + */ + notifyInterrupting(): void { + if (!this.#agentTurnActive || this.#interrupting) return; + this.#interrupting = true; + this.ctx.setWorkingMessage(INTERRUPTING_WORKING_MESSAGE); + } + subscribeToAgent(): void { this.ctx.unsubscribe = this.ctx.session.subscribe(async (event: AgentSessionEvent) => { await this.handleEvent(event); @@ -220,6 +245,7 @@ export class EventController { async #handleAgentStart(_event: Extract): Promise { this.#agentTurnActive = true; + this.#interrupting = false; this.#lastIntent = undefined; this.#readToolCallArgs.clear(); this.#readToolCallAssistantComponents.clear(); diff --git a/packages/coding-agent/src/modes/controllers/input-controller.ts b/packages/coding-agent/src/modes/controllers/input-controller.ts index c2df60aa4..6ae1c42b6 100644 --- a/packages/coding-agent/src/modes/controllers/input-controller.ts +++ b/packages/coding-agent/src/modes/controllers/input-controller.ts @@ -94,6 +94,7 @@ export class InputController { if (this.ctx.loopModeEnabled) { this.ctx.pauseLoop(); if (this.ctx.session.isStreaming) { + this.ctx.notifyInterrupting(); void this.ctx.session.abort({ reason: USER_INTERRUPT_LABEL }); } else { this.ctx.cancelPendingSubmission(); @@ -124,6 +125,7 @@ export class InputController { this.ctx.isPythonMode = false; this.ctx.updateEditorBorderColor(); } else if (this.ctx.session.isStreaming) { + this.ctx.notifyInterrupting(); void this.ctx.session.abort({ reason: USER_INTERRUPT_LABEL }); } else if (!this.ctx.editor.getText().trim()) { // Double-interrupt with empty editor triggers /tree, /branch, or nothing based on setting diff --git a/packages/coding-agent/src/modes/interactive-mode.ts b/packages/coding-agent/src/modes/interactive-mode.ts index 0b0e42641..2c7edd6f5 100644 --- a/packages/coding-agent/src/modes/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive-mode.ts @@ -2707,6 +2707,10 @@ export class InteractiveMode implements InteractiveModeContext { this.setWorkingMessage(message); } + notifyInterrupting(): void { + this.#eventController.notifyInterrupting(); + } + showNewVersionNotification(newVersion: string): void { this.#uiHelpers.showNewVersionNotification(newVersion); } diff --git a/packages/coding-agent/src/modes/types.ts b/packages/coding-agent/src/modes/types.ts index 59fdce436..7848bd95b 100644 --- a/packages/coding-agent/src/modes/types.ts +++ b/packages/coding-agent/src/modes/types.ts @@ -182,6 +182,9 @@ export interface InteractiveModeContext { flushPendingModelSwitch(): Promise; setWorkingMessage(message?: string): void; applyPendingWorkingMessage(): void; + /** Acknowledge a user interrupt (Esc) by switching the loader to an + * "Interrupting…" label until the agent turn unwinds. */ + notifyInterrupting(): void; ensureLoadingAnimation(): void; startPendingSubmission(input: { text: string; diff --git a/packages/coding-agent/test/input-controller-escape.test.ts b/packages/coding-agent/test/input-controller-escape.test.ts index f458d9568..eebe70619 100644 --- a/packages/coding-agent/test/input-controller-escape.test.ts +++ b/packages/coding-agent/test/input-controller-escape.test.ts @@ -154,6 +154,7 @@ function createContext(): { addMessageToChat, cancelPendingSubmission, ensureLoadingAnimation, + notifyInterrupting: vi.fn(), finishPendingSubmission: vi.fn(), flushPendingBashComponents: vi.fn(), markPendingSubmissionStarted: vi.fn(() => true), diff --git a/packages/coding-agent/test/modes/controllers/event-controller-interrupt.test.ts b/packages/coding-agent/test/modes/controllers/event-controller-interrupt.test.ts new file mode 100644 index 000000000..88adf8539 --- /dev/null +++ b/packages/coding-agent/test/modes/controllers/event-controller-interrupt.test.ts @@ -0,0 +1,124 @@ +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "bun:test"; +import { resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import { + EventController, + INTERRUPTING_WORKING_MESSAGE, +} from "@oh-my-pi/pi-coding-agent/modes/controllers/event-controller"; +import { initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; +import type { InteractiveModeContext } from "@oh-my-pi/pi-coding-agent/modes/types"; +import type { AgentSessionEvent } from "@oh-my-pi/pi-coding-agent/session/agent-session"; + +function createContext() { + const setWorkingMessage = vi.fn(); + const pendingTools = new Map(); + const ctx = { + isInitialized: true, + settings: { get: () => false }, + statusLine: { invalidate: vi.fn() }, + updateEditorTopBorder: vi.fn(), + pendingTools, + hideThinkingBlock: false, + setWorkingMessage, + clearPinnedError: vi.fn(), + ensureLoadingAnimation: vi.fn(), + ui: { setEagerNativeScrollbackRebuild: vi.fn(), requestRender: vi.fn() }, + session: { getToolByName: () => undefined }, + } as unknown as InteractiveModeContext; + return { ctx, pendingTools, setWorkingMessage }; +} + +const AGENT_START = { type: "agent_start" } as unknown as AgentSessionEvent; + +/** A `tool_execution_start` whose toolCallId is pre-seeded into `pendingTools`, + * so the handler only runs the intent->working-message path and skips component + * construction (which needs far heavier mocks). */ +function toolStartWithIntent(toolCallId: string, intent: string): AgentSessionEvent { + return { + type: "tool_execution_start", + toolCallId, + toolName: "search", + args: {}, + intent, + } as unknown as AgentSessionEvent; +} + +describe("EventController user interrupt acknowledgement", () => { + beforeAll(async () => { + await initTheme(false); + }); + + beforeEach(async () => { + resetSettingsForTest(); + await Settings.init({ inMemory: true }); + }); + + afterEach(() => { + vi.restoreAllMocks(); + resetSettingsForTest(); + }); + + it("swaps the loader to the interrupting label once a turn is active", async () => { + const { ctx, setWorkingMessage } = createContext(); + const controller = new EventController(ctx); + await controller.handleEvent(AGENT_START); + + controller.notifyInterrupting(); + + expect(setWorkingMessage).toHaveBeenCalledWith(INTERRUPTING_WORKING_MESSAGE); + }); + + it("freezes intent-driven working-message updates while interrupting", async () => { + const { ctx, pendingTools, setWorkingMessage } = createContext(); + const controller = new EventController(ctx); + await controller.handleEvent(AGENT_START); + controller.notifyInterrupting(); + setWorkingMessage.mockClear(); + + // A tool whose args already started streaming before the abort still emits a + // late tool_execution_start; without the freeze its intent would repaint the + // loader over the "Interrupting…" acknowledgement. + pendingTools.set("late-call", {}); + await controller.handleEvent(toolStartWithIntent("late-call", "Reticulating splines")); + + expect(setWorkingMessage).not.toHaveBeenCalled(); + }); + + it("lets intent updates drive the loader when not interrupting", async () => { + const { ctx, pendingTools, setWorkingMessage } = createContext(); + const controller = new EventController(ctx); + await controller.handleEvent(AGENT_START); + setWorkingMessage.mockClear(); + + pendingTools.set("call-1", {}); + await controller.handleEvent(toolStartWithIntent("call-1", "Searching files")); + + expect(setWorkingMessage).toHaveBeenCalledTimes(1); + expect(setWorkingMessage.mock.calls[0]?.[0]).toContain("Searching files"); + }); + + it("clears the interrupt freeze at the next agent_start", async () => { + const { ctx, pendingTools, setWorkingMessage } = createContext(); + const controller = new EventController(ctx); + await controller.handleEvent(AGENT_START); + controller.notifyInterrupting(); + + // New turn: the freeze must lift so the next turn's intents render again. + await controller.handleEvent(AGENT_START); + setWorkingMessage.mockClear(); + + pendingTools.set("call-2", {}); + await controller.handleEvent(toolStartWithIntent("call-2", "Editing module")); + + expect(setWorkingMessage).toHaveBeenCalledTimes(1); + expect(setWorkingMessage.mock.calls[0]?.[0]).toContain("Editing module"); + }); + + it("is a no-op before any turn starts", () => { + const { ctx, setWorkingMessage } = createContext(); + const controller = new EventController(ctx); + + controller.notifyInterrupting(); + + expect(setWorkingMessage).not.toHaveBeenCalled(); + }); +}); From 1e163e4c80135cd17db19406e01d2d849e10d946 Mon Sep 17 00:00:00 2001 From: can1357 Date: Mon, 8 Jun 2026 00:56:05 +0200 Subject: [PATCH 44/46] fix(hashline): strip bare-row N: prefixes only when every bare row carries one Defer read-output line-number stripping from #handleRaw to #flushPending and gate it on uniformity: a bare "N:text" row is only a pasted-snapshot artifact when *every* bare body row in the hunk carries the prefix. A mixed set (e.g. "3:keep" next to "plain") is treated as genuine content and left intact, avoiding silent corruption of bodies that legitimately start with "digits:". Removes at most one prefix per row; "+"-prefixed literal rows stay untouched. Reverts the now-orphaned stripLeadingHashlinePrefixes export back to private. --- packages/hashline/CHANGELOG.md | 2 +- packages/hashline/src/parser.ts | 41 +++++++++++++++++++------ packages/hashline/src/prefixes.ts | 2 +- packages/hashline/test/leniency.test.ts | 12 ++++++++ 4 files changed, 45 insertions(+), 12 deletions(-) diff --git a/packages/hashline/CHANGELOG.md b/packages/hashline/CHANGELOG.md index 889e72c6e..5aee4827c 100644 --- a/packages/hashline/CHANGELOG.md +++ b/packages/hashline/CHANGELOG.md @@ -4,7 +4,7 @@ ### Fixed -- Stripped read-output line-number prefixes (`N:`) from auto-piped bare body rows so that pasting `3:text` without a `+` prefix no longer injects `3:` as literal content. Uses single-pass stripping to avoid corrupting content whose own text starts with `digits:` (e.g. YAML port maps, timestamps) ([#1492](https://github.com/can1357/oh-my-pi/issues/1492)). +- Stripped read-output line-number prefixes (`N:`) from auto-piped bare body rows so that pasting `3:text` without a `+` prefix no longer injects `3:` as literal content. Stripping is applied only when *every* bare row in the hunk carries the prefix (the signature of a pasted snapshot) and removes at most one prefix per row, so a genuine body that merely starts with `digits:` (YAML port maps, timestamps) is left intact ([#1492](https://github.com/can1357/oh-my-pi/issues/1492)). ## [15.9.67] - 2026-06-06 diff --git a/packages/hashline/src/parser.ts b/packages/hashline/src/parser.ts index 83b7fdb97..d4d67bfec 100644 --- a/packages/hashline/src/parser.ts +++ b/packages/hashline/src/parser.ts @@ -4,7 +4,6 @@ * applier. */ import { HL_PAYLOAD_REPLACE } from "./format"; -import { stripOneLeadingHashlinePrefix } from "./prefixes"; import { BARE_BODY_AUTO_PIPED_WARNING, DELETE_BLOCK_TAKES_NO_BODY, @@ -13,6 +12,7 @@ import { EMPTY_INSERT, MINUS_ROW_REJECTED, } from "./messages"; +import { stripOneLeadingHashlinePrefix } from "./prefixes"; import { type BlockTarget, cloneCursor, type ParsedRange, type Token, Tokenizer } from "./tokenizer"; import type { Anchor, Cursor, Edit } from "./types"; @@ -82,7 +82,7 @@ interface PendingComment { text: string; } -type PayloadRow = { kind: "literal"; text: string; lineNum: number }; +type PayloadRow = { kind: "literal"; text: string; lineNum: number; bare?: boolean }; interface Pending { target: BlockTarget; @@ -221,14 +221,14 @@ export class Executor { throw new Error(`line ${lineNum}: ${DELETE_BLOCK_TAKES_NO_BODY}`); if (text.trimStart().charCodeAt(0) === 45 /* - */) throw new Error(`line ${lineNum}: ${MINUS_ROW_REJECTED}`); if (!this.#warnings.includes(BARE_BODY_AUTO_PIPED_WARNING)) this.#warnings.push(BARE_BODY_AUTO_PIPED_WARNING); - // Strip at most one read-output line-number prefix (e.g. "3:text") - // from bare body rows. Lines with an explicit "+" prefix go through - // #handleLiteralPayload which skips this stripping — "+3:text" is - // intentional literal content, while bare "3:text" is almost always - // a copy-paste artifact from snapshot output. Single-pass only: - // recursive stripping would corrupt content whose own text starts - // with `digits:` (e.g. YAML ports "42:hello", timestamps "12:30"). - this.#pending.payloads.push({ kind: "literal", text: stripOneLeadingHashlinePrefix(text), lineNum }); + // Defer read-output line-number stripping to #flushPending: a bare + // "N:text" row is only a copy-paste artifact from snapshot output + // when *every* bare row in the hunk carries that prefix. Stripping a + // row in isolation would corrupt a genuine body that merely starts + // with "digits:" (YAML ports "42:hello", timestamps "12:30") when it + // sits next to an unprefixed sibling. Rows with an explicit "+" go + // through #handleLiteralPayload and are never bare, never stripped. + this.#pending.payloads.push({ kind: "literal", text, lineNum, bare: true }); return; } if (text.trim().length === 0) return; @@ -238,6 +238,26 @@ export class Executor { ); } + /** + * Strip a single read-output line-number prefix (`N:`) from every bare body + * row, but only when *all* bare rows carry one. A uniform set of prefixes is + * the signature of content pasted straight from `read`/`search` output; a + * mixed set means the `N:` is genuine payload content and must stay. Rows + * authored with an explicit `+` are not bare and are never touched. + */ + #stripBarePrefixesIfUniform(payloads: PayloadRow[]): void { + let sawBare = false; + for (const row of payloads) { + if (!row.bare) continue; + sawBare = true; + if (stripOneLeadingHashlinePrefix(row.text) === row.text) return; + } + if (!sawBare) return; + for (const row of payloads) { + if (row.bare) row.text = stripOneLeadingHashlinePrefix(row.text); + } + } + #pushInsert(cursor: Cursor, text: string, lineNum: number, mode?: "replacement"): void { this.#edits.push({ kind: "insert", @@ -271,6 +291,7 @@ export class Executor { const pending = this.#pending; if (!pending) return; const { target, lineNum, payloads } = pending; + this.#stripBarePrefixesIfUniform(payloads); this.#pending = undefined; if (target.kind === "delete") { for (const anchor of expandRange(target.range)) this.#pushDelete(anchor, lineNum); diff --git a/packages/hashline/src/prefixes.ts b/packages/hashline/src/prefixes.ts index 9af5c1b3a..1199b02d0 100644 --- a/packages/hashline/src/prefixes.ts +++ b/packages/hashline/src/prefixes.ts @@ -22,7 +22,7 @@ const HL_HEADER_RE = new RegExp(`^\\s*\\[[^#\\r\\n]+#[0-9a-fA-F]{${HL_FILE_HASH_ const DIFF_PLUS_RE = /^[+](?![+])/; const READ_TRUNCATION_NOTICE_RE = /^\[(?:Showing lines \d+-\d+ of \d+|\d+ more lines? in (?:file|\S+))\b.*\bUse :L?\d+/; -export function stripLeadingHashlinePrefixes(line: string): string { +function stripLeadingHashlinePrefixes(line: string): string { let result = line; let previous: string; do { diff --git a/packages/hashline/test/leniency.test.ts b/packages/hashline/test/leniency.test.ts index bf9250205..3071e5a24 100644 --- a/packages/hashline/test/leniency.test.ts +++ b/packages/hashline/test/leniency.test.ts @@ -122,6 +122,18 @@ describe("hashline body contracts", () => { expect(applyEdits(FILE, result.edits).text).toBe("a\n42:hello\nc\nd\ne"); }); + it("strips N: prefixes only when every bare body row carries one", () => { + const result = parsePatch("replace 2..3:\n2:foo\n3:bar"); + expect(applyEdits(FILE, result.edits).text).toBe("a\nfoo\nbar\nd\ne"); + }); + + it("leaves bare body rows untouched when only some carry an N: prefix", () => { + // "3:keep" looks like a snapshot prefix but "plain" does not, so the body + // is genuine content (not a pasted snapshot) — strip nothing. + const result = parsePatch("replace 2..3:\n3:keep\nplain"); + expect(applyEdits(FILE, result.edits).text).toBe("a\n3:keep\nplain\nd\ne"); + }); + it("rejects `-` body rows with a teaching error", () => { expect(() => parsePatch("replace 2..2:\n-old\n+new")).toThrow(/`-` rows are not valid/); }); From 5be2600b92badc287002af39173077f6e557b13e Mon Sep 17 00:00:00 2001 From: can1357 Date: Mon, 8 Jun 2026 00:58:02 +0200 Subject: [PATCH 45/46] fix(coding-agent): normalized relative --cwd flag before downstream use - Updated startup handling so `applyStartupCwd` now re-syncs `parsed.cwd` to the resolved absolute project directory after `setProjectDir` runs. - Adjusted the CLI cwd tests to verify a relative `--cwd` argument is normalized to an absolute path and does not double-resolve against the new process cwd. --- packages/coding-agent/CHANGELOG.md | 1 + packages/coding-agent/src/cli/startup-cwd.ts | 5 +++++ .../coding-agent/test/cli-cwd-flag.test.ts | 21 +++++++++++++++++++ 3 files changed, 27 insertions(+) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index f35498894..38dd899ac 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -12,6 +12,7 @@ - Fixed Kitty OSC 5522 paste rejecting plain text as "no supported text or image data": the listing parser now decodes the `mime="."` DATA payload (whitespace-separated MIME list) Kitty actually sends, in addition to the per-type DATA packets described by the ancillary 5522-mode spec ([#2051](https://github.com/can1357/oh-my-pi/issues/2051)) - Fixed follow-up shortcut submission of builtin slash commands so `/goal set ...` applies goal mode instead of queueing as plain text. - Fixed Ctrl+Z crashing the agent on Windows with `TypeError: Unknown signal: SIGTSTP`. `InputController.handleCtrlZ` called `process.kill(0, "SIGTSTP")` unconditionally, but `SIGTSTP` is POSIX job-control and Bun/Node on Windows rejects the signal name from the JS side; the throw propagated out of the TUI input dispatcher as an uncaught exception. The handler now no-ops with a "Suspend (Ctrl+Z) is not supported on this platform" status on Windows, and on POSIX wraps `process.kill` in a try/catch that detaches the registered SIGCONT resume hook and re-`start()`s the TUI on failure so a rejected signal can never leave the UI stranded with a leaked listener ([#2036](https://github.com/can1357/oh-my-pi/issues/2036)). +- Fixed a relative `--cwd` target (e.g. `omp --cwd repo` launched from `/tmp`) leaking the raw relative string into the session config. `applyStartupCwd` chdired into the resolved directory via `setProjectDir` but left `parsed.cwd` as `"repo"`, so `buildSessionOptions` (which prefers `parsed.cwd` over `getProjectDir()`) handed downstream settings/discovery/session creation a value that re-resolved against the new process cwd (`/tmp/repo/repo`) or persisted a relative session cwd. `parsed.cwd` is now re-synced to the resolved absolute project dir after the chdir. ### Fixed diff --git a/packages/coding-agent/src/cli/startup-cwd.ts b/packages/coding-agent/src/cli/startup-cwd.ts index afabff095..b3c2c890a 100644 --- a/packages/coding-agent/src/cli/startup-cwd.ts +++ b/packages/coding-agent/src/cli/startup-cwd.ts @@ -57,6 +57,11 @@ async function maybeAutoChdir(parsed: Args): Promise { export async function applyStartupCwd(parsed: Args): Promise { if (parsed.cwd) { setProjectDir(parsed.cwd); + // setProjectDir resolves the (possibly relative) target against the launch + // cwd and chdirs into it. Re-sync parsed.cwd to the resolved absolute path + // so downstream consumers (buildSessionOptions, settings/discovery, session + // persistence) don't re-resolve a relative string against the new cwd. + parsed.cwd = getProjectDir(); return; } await maybeAutoChdir(parsed); diff --git a/packages/coding-agent/test/cli-cwd-flag.test.ts b/packages/coding-agent/test/cli-cwd-flag.test.ts index 973d61a4c..5fb734d7c 100644 --- a/packages/coding-agent/test/cli-cwd-flag.test.ts +++ b/packages/coding-agent/test/cli-cwd-flag.test.ts @@ -38,4 +38,25 @@ describe("parseArgs — --cwd flag", () => { expect(getProjectDir()).toBe(targetDir); expect(process.cwd()).toBe(targetDir); }); + + it("normalizes a relative --cwd target to the resolved absolute path", async () => { + const launchDir = fs.mkdtempSync(path.join(os.tmpdir(), "omp-cwd-rel-")); + const childName = "repo"; + const childDir = path.join(launchDir, childName); + fs.mkdirSync(childDir); + setProjectDir(launchDir); + + const parsed = parseArgs(["--cwd", childName]); + await applyStartupCwd(parsed); + + // parsed.cwd must be the resolved absolute target, not the raw relative + // string that would re-resolve against the new cwd (e.g. repo/repo). + expect(path.isAbsolute(parsed.cwd ?? "")).toBe(true); + expect(parsed.cwd).toBe(getProjectDir()); + expect(getProjectDir()).toBe(childDir); + // Re-resolving the normalized value against the (now changed) process cwd + // is idempotent — no doubled "repo/repo" segment. + expect(path.resolve(parsed.cwd ?? "")).toBe(getProjectDir()); + expect(parsed.cwd?.endsWith(`${childName}${path.sep}${childName}`)).toBe(false); + }); }); From d3476c6664c50035ba9bd2c2a0058a148adc330c Mon Sep 17 00:00:00 2001 From: can1357 Date: Mon, 8 Jun 2026 00:58:40 +0200 Subject: [PATCH 46/46] chore: fix changelogs --- packages/ai/CHANGELOG.md | 5 ++++- packages/coding-agent/CHANGELOG.md | 11 ++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index 321ae1a0b..192051eb2 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -1,10 +1,14 @@ # Changelog + ## [Unreleased] ### Added - Added support for `impersonated_service_account` Application Default Credentials (ADC) in Vertex AI to enable chained impersonation without failing via 401 `invalid_client`. +### Fixed + +- Fixed duplicate upstream `tool_call_id` values collapsing distinct tool calls during message transformation, preserving one call/result pairing per emitted tool call before provider replay. ([#2055](https://github.com/can1357/oh-my-pi/issues/2055)) ## [15.10.1] - 2026-06-07 @@ -25,7 +29,6 @@ ### Fixed -- Fixed duplicate upstream `tool_call_id` values collapsing distinct tool calls during message transformation, preserving one call/result pairing per emitted tool call before provider replay. ([#2055](https://github.com/can1357/oh-my-pi/issues/2055)) - Fixed streaming auth retries to handle `401` and usage-limit errors before replay-unsafe content is emitted, including failures surfaced only via `errorStatus` - Fixed tool argument validation to coerce singleton non-string values into arrays when the schema expects an array, preventing Anthropic-compatible models that emit `todo.ops` as an object from getting stuck in repeated validation-error loops. ([#2026](https://github.com/can1357/oh-my-pi/issues/2026)) - Fixed streaming retries to buffer and suppress partial `start` events from failed auth attempts so only clean retried events are delivered diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 38dd899ac..1ee9d374f 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ## [Unreleased] + ### Fixed - Fixed the working spinner appearing to ignore Esc for 2-3 seconds when an interrupt lands mid-tool. Esc fires the abort synchronously, but the agent loop only stops the loader at `agent_end`, which it cannot reach until every in-flight tool settles in `executeToolCalls`' `await Promise.allSettled(...)` — and process/subagent/kernel-owning tools tear down gracefully (SIGTERM, 2-3s grace, SIGKILL), so the loader kept showing the unchanged "Working…/" line and read as a dropped keypress. The loader now switches to "Interrupting…" the instant Esc requests the abort and freezes intent-driven label updates until the turn unwinds (`EventController.notifyInterrupting`), so the interrupt is acknowledged immediately even while teardown completes. @@ -13,10 +14,9 @@ - Fixed follow-up shortcut submission of builtin slash commands so `/goal set ...` applies goal mode instead of queueing as plain text. - Fixed Ctrl+Z crashing the agent on Windows with `TypeError: Unknown signal: SIGTSTP`. `InputController.handleCtrlZ` called `process.kill(0, "SIGTSTP")` unconditionally, but `SIGTSTP` is POSIX job-control and Bun/Node on Windows rejects the signal name from the JS side; the throw propagated out of the TUI input dispatcher as an uncaught exception. The handler now no-ops with a "Suspend (Ctrl+Z) is not supported on this platform" status on Windows, and on POSIX wraps `process.kill` in a try/catch that detaches the registered SIGCONT resume hook and re-`start()`s the TUI on failure so a rejected signal can never leave the UI stranded with a leaked listener ([#2036](https://github.com/can1357/oh-my-pi/issues/2036)). - Fixed a relative `--cwd` target (e.g. `omp --cwd repo` launched from `/tmp`) leaking the raw relative string into the session config. `applyStartupCwd` chdired into the resolved directory via `setProjectDir` but left `parsed.cwd` as `"repo"`, so `buildSessionOptions` (which prefers `parsed.cwd` over `getProjectDir()`) handed downstream settings/discovery/session creation a value that re-resolved against the new process cwd (`/tmp/repo/repo`) or persisted a relative session cwd. `parsed.cwd` is now re-synced to the resolved absolute project dir after the chdir. - -### Fixed - - Fixed the `--cwd` launch flag so it is parsed and can override the startup directory instead of always falling back to the current process directory or home auto-switch target. +- Fixed session auto-retry for generic `upstream_error: Upstream request failed` gateway failures. +- Stripped read-output line-number prefixes (`N:`) from auto-piped bare body rows in the hashline edit parser, so pasting `3:text` without a `+` prefix no longer injects `3:` as literal content. Uses single-pass stripping to avoid corrupting content whose own text starts with `digits:` ([#1492](https://github.com/can1357/oh-my-pi/issues/1492)). ## [15.10.1] - 2026-06-07 @@ -66,8 +66,6 @@ ### Fixed -- Fixed session auto-retry for generic `upstream_error: Upstream request failed` gateway failures. - - Fixed inline `find` and `search` result blocks to align with grouped `read` output and render their success headers with the normal tool-title color instead of accent blue. - Fixed the working-status shimmer to opt into the loader's 30fps animated-message repaint path while keeping both the status spinner and pending bash/eval tool spinners on their normal 80 ms glyph cadence. @@ -150,7 +148,6 @@ - Fixed collapsed search result previews that could show only "… N more matches" when the first grouped section exceeded the preview budget. Collapsed search output now compacts to match rows, fills the budget with visible hits before the summary, and keeps truncation details out of the bottom user-visible notice. - Fixed the expanded search result view dumping every match when all hits live in one file. A single file's matches collapse into one blank-line group, and the expanded tree list ignored the line budget, so a hot file whose matches span its whole length rendered every row (e.g. lines 374–2858). Expanded search output is now bounded by a larger-than-collapsed budget (`EXPANDED_LINES × 2`), keeps surrounding context rows, and appends a `… N more matches` summary when truncated. - Fixed boolean environment flag overrides that were ORed with settings, so `PI_INTENT_TRACING=0`, `PI_AUTO_QA=0`, and per-backend eval flags now take precedence when present while falling back to config when unset. -- Stripped read-output line-number prefixes (`N:`) from auto-piped bare body rows in the hashline edit parser, so pasting `3:text` without a `+` prefix no longer injects `3:` as literal content. Uses single-pass stripping to avoid corrupting content whose own text starts with `digits:` ([#1492](https://github.com/can1357/oh-my-pi/issues/1492)). - Fixed custom-rendered tools that set `mergeCallAndResult` (e.g. `lsp`) rendering a redundant tool-name line above the framed result once a result arrived. `ToolExecutionComponent`'s custom-tool branch now emits the fallback label only when the tool has no `renderCall` and the call is not suppressed by an existing result, matching the built-in renderer branch. - Fixed the `write` tool result rendering with a green success checkmark even when the write failed. `writeToolRenderer.renderResult` now branches on `result.isError`, rendering the error status icon plus the failure message instead of the success header and content preview. - Fixed Perplexity OAuth/cookie web search returning a refusal answer ("I don't currently have access to the web-search tools in this turn") despite returning real sources. `callPerplexityOAuth` was prepending the API-style `web-search` system prompt to the query (`query_str = systemPrompt + "\n\n" + query`), but the consumer `www.perplexity.ai/rest/sse/perplexity_ask` endpoint has no system-message slot and reads the prepended instruction as a meta-prompt, making the model decline. The OAuth/cookie path now sends the bare query; the API-key path still passes the system prompt as a proper `system` message. @@ -9589,4 +9586,4 @@ Initial public release. - Git branch display in footer - Message queueing during streaming responses - OAuth integration for Gmail and Google Calendar access -- HTML export with syntax highlighting and collapsible sections \ No newline at end of file +- HTML export with syntax highlighting and collapsible sections