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 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/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/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 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/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index 344d76b9a..192051eb2 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -2,6 +2,14 @@ ## [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 ### Breaking Changes 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..8b72ffcc4 --- /dev/null +++ b/packages/ai/src/providers/__tests__/google-auth.test.ts @@ -0,0 +1,144 @@ +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"; + +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(); + originalGac = Bun.env.GOOGLE_APPLICATION_CREDENTIALS; + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-vertex-adc-")); + }); + + 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 }); + }); + + 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"); + + // 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 a8004508f..0af7ddea8 100644 --- a/packages/ai/src/providers/google-auth.ts +++ b/packages/ai/src/providers/google-auth.ts @@ -42,7 +42,14 @@ interface AuthorizedUserCredentials { refresh_token: string; } -type AdcFileCredentials = ServiceAccountCredentials | AuthorizedUserCredentials; +interface ImpersonatedServiceAccountCredentials { + type: "impersonated_service_account"; + service_account_impersonation_url: string; + source_credentials: AuthorizedUserCredentials | ServiceAccountCredentials; + delegates?: string[]; +} + +type AdcFileCredentials = ServiceAccountCredentials | AuthorizedUserCredentials | ImpersonatedServiceAccountCredentials; interface TokenResponse { access_token: string; @@ -196,10 +203,52 @@ 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 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( + `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, + }, + ); + 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); 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/providers/transform-messages.ts b/packages/ai/src/providers/transform-messages.ts index 7959f7828..096b454e7 100644 --- a/packages/ai/src/providers/transform-messages.ts +++ b/packages/ai/src/providers/transform-messages.ts @@ -17,6 +17,96 @@ 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[] { + const seenToolCallIds = new Map(); + const pendingToolResultRewrites = new Map(); + + return messages.map(msg => { + if (msg.role === "toolResult") { + 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; + } + + 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]); + }; + + // 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); + enqueueToolResultRewrite(block.id, undefined); + return block; + } + + let duplicateIndex = previousCount; + let replacementId = appendDuplicateSuffix(block.id, `_dup${duplicateIndex}`); + while (seenToolCallIds.has(replacementId)) { + duplicateIndex += 1; + replacementId = appendDuplicateSuffix(block.id, `_dup${duplicateIndex}`); + } + seenToolCallIds.set(block.id, duplicateIndex + 1); + seenToolCallIds.set(replacementId, 1); + enqueueToolResultRewrite(block.id, { replacementId }); + contentChanged = true; + return { ...block, id: replacementId }; + }); + + if (!contentChanged) return msg; + 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 +142,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/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/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/duplicate-tool-results.test.ts b/packages/ai/test/duplicate-tool-results.test.ts index 1be33d66b..46f34ee2c 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,144 @@ 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" }, + ]); + }); + + 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" }, + ]); + }); + + 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" }, + ]); + }); + + 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" }, + ]); + }); }); /** 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"> = { 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)", () => { diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 7fd62d3d6..7494193fa 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -5,6 +5,22 @@ - Added isolated profile support via `--profile ` / `OMP_PROFILE` and shell alias bootstrap via `--alias `, including launch/ACP bootstrap handling, extension-flag-safe parsing, profile-scoped user config discovery, and symlinked extension-directory discovery. +### 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. +- 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 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 +- 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 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 ### Added @@ -9573,4 +9589,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 diff --git a/packages/coding-agent/src/cli/args.ts b/packages/coding-agent/src/cli/args.ts index 60c89f1ed..3432c8a45 100644 --- a/packages/coding-agent/src/cli/args.ts +++ b/packages/coding-agent/src/cli/args.ts @@ -225,7 +225,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 diff --git a/packages/coding-agent/src/cli/flag-tables.ts b/packages/coding-agent/src/cli/flag-tables.ts index 32d121a42..a286b8d56 100644 --- a/packages/coding-agent/src/cli/flag-tables.ts +++ b/packages/coding-agent/src/cli/flag-tables.ts @@ -94,6 +94,9 @@ const setResume: OptionalSetter = (result, value) => { * token starts with `-`. */ export const STRING_SETTERS: Record = { + "--cwd": (result, value) => { + result.cwd = value; + }, "--mode": (result, value) => { if (value === "text" || value === "json" || value === "rpc" || value === "acp" || value === "rpc-ui") { result.mode = value; 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..b3c2c890a --- /dev/null +++ b/packages/coding-agent/src/cli/startup-cwd.ts @@ -0,0 +1,68 @@ +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); + // 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/src/commands/launch.ts b/packages/coding-agent/src/commands/launch.ts index 39d594934..aee0f3559 100644 --- a/packages/coding-agent/src/commands/launch.ts +++ b/packages/coding-agent/src/commands/launch.ts @@ -55,6 +55,9 @@ export default class Index extends Command { alias: Flags.string({ description: "Create a shell shortcut for the selected profile and exit", }), + cwd: Flags.string({ + description: "Directory to start in (overrides the launch cwd)", + }), mode: Flags.string({ description: "Output mode: text (default), json, rpc, or rpc-ui", options: ["text", "json", "rpc", "acp", "rpc-ui"], diff --git a/packages/coding-agent/src/eval/js/context-manager.ts b/packages/coding-agent/src/eval/js/context-manager.ts index cea0c97e5..c1dcef642 100644 --- a/packages/coding-agent/src/eval/js/context-manager.ts +++ b/packages/coding-agent/src/eval/js/context-manager.ts @@ -53,7 +53,13 @@ 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 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: { sessionKey: string; @@ -191,9 +197,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/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 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 +361,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 +420,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 +454,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( @@ -480,56 +512,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. - } -} - /** 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 +727,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/src/mcp/tool-bridge.ts b/packages/coding-agent/src/mcp/tool-bridge.ts index 1b396ad4b..4063d8153 100644 --- a/packages/coding-agent/src/mcp/tool-bridge.ts +++ b/packages/coding-agent/src/mcp/tool-bridge.ts @@ -220,6 +220,7 @@ export class MCPTool 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; @@ -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/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 da83ea41b..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 @@ -499,17 +501,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 { @@ -589,7 +618,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 +632,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/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/src/sdk.ts b/packages/coding-agent/src/sdk.ts index a28d834e5..992396621 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: typeof tool.approval === "function" ? tool.approval.bind(tool) : tool.approval, mcpServerName: tool.mcpServerName, mcpToolName: tool.mcpToolName, execute: (toolCallId, params, signal, onUpdate, ctx) => diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index b53734c6f..cffaf46a0 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[]; @@ -6539,9 +6544,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", @@ -6554,12 +6563,26 @@ export class AgentSession { } #isEmptyAssistantStop(assistantMessage: AssistantMessage): boolean { - 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"; - }); + 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; + } } #emptyStopRetryReminder(): string { @@ -7874,11 +7897,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/src/session/session-manager.ts b/packages/coding-agent/src/session/session-manager.ts index 61f78cdcc..8ad4a4d95 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 = 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 && + 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/src/task/executor.ts b/packages/coding-agent/src/task/executor.ts index 6f47ab87a..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, 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 +260,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 +383,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 +416,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/src/utils/enhanced-paste.ts b/packages/coding-agent/src/utils/enhanced-paste.ts index 16fdf416e..83ec35bb6 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; } @@ -192,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/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"), 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" }); + }); }); diff --git a/packages/coding-agent/test/cli-cwd-flag.test.ts b/packages/coding-agent/test/cli-cwd-flag.test.ts new file mode 100644 index 000000000..0f131f153 --- /dev/null +++ b/packages/coding-agent/test/cli-cwd-flag.test.ts @@ -0,0 +1,62 @@ +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, normalizePathForComparison, setProjectDir } from "@oh-my-pi/pi-utils"; +import { parseArgs } from "../src/cli/args"; +import { applyStartupCwd } from "../src/cli/startup-cwd"; + +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"]); + + 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"]); + }); + + 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(normalizePathForComparison(process.cwd())).toBe(normalizePathForComparison(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); + }); +}); 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/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", 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/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, 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(); + }); +}); 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/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(); + }); +}); 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..751c16bd3 --- /dev/null +++ b/packages/coding-agent/test/session-manager/continue-relocation.test.ts @@ -0,0 +1,287 @@ +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; +} + +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; + 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(); + } + }); + + 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(); + } + }); +}); 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([ 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" }); + }); +}); diff --git a/packages/coding-agent/test/utils/enhanced-paste.test.ts b/packages/coding-agent/test/utils/enhanced-paste.test.ts index d319c1362..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( @@ -106,4 +104,73 @@ 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:pw=${password}:name=${pasteEventName};${textMime}${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)).toBe(`${OSC}type=read;${imageMime}${ST}`); + }); }); diff --git a/packages/hashline/CHANGELOG.md b/packages/hashline/CHANGELOG.md index 42e6dc391..5aee4827c 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. 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 ### 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..d4d67bfec 100644 --- a/packages/hashline/src/parser.ts +++ b/packages/hashline/src/parser.ts @@ -12,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"; @@ -81,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; @@ -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 }); + // 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; @@ -230,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", @@ -263,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 6e18950da..1199b02d0 100644 --- a/packages/hashline/src/prefixes.ts +++ b/packages/hashline/src/prefixes.ts @@ -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..3071e5a24 100644 --- a/packages/hashline/test/leniency.test.ts +++ b/packages/hashline/test/leniency.test.ts @@ -106,6 +106,34 @@ 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("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/); }); 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..193e50368 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -2,6 +2,17 @@ ## [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)). + +### 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)). +- 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)). +- 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 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/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/src/terminal.ts b/packages/tui/src/terminal.ts index 2069f6370..1d2a6b200 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 */ @@ -504,11 +556,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 @@ -978,7 +1039,23 @@ 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. 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); + } + } 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/src/tui.ts b/packages/tui/src/tui.ts index e71ee6818..17365e561 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: @@ -2049,7 +2055,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 +2445,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) { @@ -2478,7 +2497,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 +2507,85 @@ 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; + + 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 (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; + } + 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; + } + + #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/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/issue-2034-repro.test.ts b/packages/tui/test/issue-2034-repro.test.ts new file mode 100644 index 000000000..1391fe6ea --- /dev/null +++ b/packages/tui/test/issue-2034-repro.test.ts @@ -0,0 +1,203 @@ +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 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(() => { + 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"); + setEnv("WSL_DISTRO_NAME", originalWslDistro); + setEnv("WSL_INTEROP", originalWslInterop); + }); + + 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("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(); + 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]); + }); + }); +}); 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..1b839ae4a --- /dev/null +++ b/packages/tui/test/issue-2045-repro.test.ts @@ -0,0 +1,120 @@ +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("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`; + + tui.addChild(new RawLinesComponent([line])); + try { + tui.start(); + await settle(); + } finally { + tui.stop(); + } + + 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); + }); + + 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); + }); +}); 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); }); }); 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"); + }); +}); 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