From 5ea7df1a5b841fb1062df55eb9e43569262f2c99 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 11 Aug 2026 21:16:36 -0700 Subject: [PATCH] fix(terminal): make DECSET 2031 subscriptions silent (#13904) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fish arms `CSI ?2031h` before painting each prompt and withdraws it when it hands the tty to a child — a ~1ms window. Orca answered that subscribe with `CSI ?997;Nn` across a 1-3ms renderer hop, so the reply landed after the withdrawal and was read as stdin by the next child, corrupting `brew`/`npx` `[y/N]` prompts. The reply is not stale by Orca's own view when written (measured staleReplies: 0), so no suppress-the-stale-reply scheme can close this — the information needed to suppress does not exist yet. Nothing asked for the reply either. The Contour spec says a terminal "should only send out the DSR when the palette has been updated"; Ghostty (Termio.zig:729 — force=true reachable only from the ?996n DSR), iTerm2 (VT100Terminal.m:995 — flag only) and xterm.js (InputHandler.ts:2035 — flag only) all emit nothing on the DECSET. So stop entering the race: record the subscription, answer nothing. Of 17 real programs measured under a pty, only fish, tmux, claude and opencode subscribe; none block on a reply, and answering produces one redundant palette re-query and zero rendering difference. tmux is the only one that sends `?996n`, which Orca still answers. - Subscribes are record-only at all four emitters (live scan, hidden-gate fact, parked byte watcher, parked responder — the last is deleted, it only replied). - `?996n` answers, the subscription registry, and the theme-flip push are unchanged. `paneLastThemeMode` is still seeded at subscribe so the next appearance re-apply is not read as a flip. - Replay grammar carries `?2031l` alongside `?2031h`, so a late-attaching remote client no longer registers a subscription the TUI already retired. Also closes fish-integration gaps found alongside: `unset` (which fish lacks) becomes `set -e` on paths parsed by the client's login shell, `config.fish` is parsed for agent-home detection, and bracketed-paste startup delivery is made consistent across local/daemon/relay. Regression test drives real fish 4.7.1 under node-pty and asserts on what the child process reads; it fails against pre-fix code with the exact payload from the issue. CI installs fish 4 and fails loudly rather than skipping. Closes #9993 Co-authored-by: Orca --- .github/workflows/pr.yml | 48 +- src/main/codex/codex-pane-account-registry.ts | 2 + src/main/codex/codex-real-home-path.test.ts | 57 +- src/main/codex/codex-real-home-path.ts | 14 +- src/main/daemon/shell-ready.test.ts | 10 +- src/main/ipc/pty-hidden-delivery-gate.ts | 3 +- src/main/ipc/pty.ts | 23 +- src/main/providers/local-pty-provider.ts | 9 +- src/main/providers/local-pty-shell-ready.ts | 2 +- src/main/pty/shell-startup-env.test.ts | 238 ++++++++- src/main/pty/shell-startup-env.ts | 146 +++++- src/relay/plugin-overlay-env.test.ts | 27 + src/relay/plugin-overlay-env.ts | 6 +- ...-color-scheme-child-stdin.node-pty.test.ts | 487 ++++++++++++++++++ .../parked-terminal-byte-watcher.test.ts | 64 +-- .../parked-terminal-byte-watcher.ts | 27 +- ...parked-terminal-mode2031-responder.test.ts | 124 ----- .../parked-terminal-mode2031-responder.ts | 49 -- .../terminal-pane/pty-connection-types.ts | 10 +- .../terminal-pane/pty-connection.test.ts | 144 +++--- .../terminal-pane/pty-connection.ts | 75 +-- .../pty-input-write-queue.test.ts | 3 +- .../terminal-hidden-delivery-gate.ts | 2 +- .../terminal-parked-pty-watcher.ts | 7 +- .../terminal-parked-tab-watchers.test.ts | 8 - .../terminal-side-effect-facts-handler.ts | 4 +- .../use-terminal-pane-lifecycle.ts | 6 +- .../src/lib/ai-vault-resume-command.ts | 24 +- .../src/lib/ai-vault-resume-shell.test.ts | 150 ++++++ src/renderer/src/lib/ai-vault-resume-shell.ts | 39 +- src/renderer/src/lib/client-login-shell.ts | 15 + src/shared/ai-vault-resume-command.ts | 3 +- src/shared/fish-binary-requirement.test.ts | 54 ++ src/shared/fish-binary-requirement.ts | 65 +++ src/shared/hermes-startup-query.ts | 5 +- src/shared/startup-command-submission.test.ts | 24 +- src/shared/startup-command-submission.ts | 26 +- .../terminal-mode-2031-final-state.test.ts | 78 ++- src/shared/terminal-output-side-effects.ts | 2 +- src/shared/terminal-query-reply.ts | 5 +- src/shared/terminal-reply-query-scan.test.ts | 71 +++ src/shared/terminal-reply-query-scan.ts | 20 +- src/shared/terminal-side-effect-facts.ts | 5 +- src/shared/tui-agent-startup-shell.test.ts | 58 ++- src/shared/tui-agent-startup-shell.ts | 36 +- ...icial-opencode-hidden-pressure-scenario.ts | 4 +- ...cial-opencode-revisit-pressure-scenario.ts | 4 +- .../artificial-opencode-terminal-load.spec.ts | 9 +- ...terminal-hidden-tui-visual-restore.spec.ts | 2 +- ...terminal-tab-switch-visual-restore.spec.ts | 1 - .../capture-fish-mode2031-trace.mjs | 398 ++++++++++++++ 51 files changed, 2199 insertions(+), 494 deletions(-) create mode 100644 src/renderer/src/components/terminal-pane/fish-color-scheme-child-stdin.node-pty.test.ts delete mode 100644 src/renderer/src/components/terminal-pane/parked-terminal-mode2031-responder.test.ts delete mode 100644 src/renderer/src/components/terminal-pane/parked-terminal-mode2031-responder.ts create mode 100644 src/renderer/src/lib/ai-vault-resume-shell.test.ts create mode 100644 src/renderer/src/lib/client-login-shell.ts create mode 100644 src/shared/fish-binary-requirement.test.ts create mode 100644 src/shared/fish-binary-requirement.ts create mode 100644 tests/tools/fish-mode2031-repro/capture-fish-mode2031-trace.mjs diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index d10463173dd..50eea7fc6d2 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -176,6 +176,12 @@ jobs: shell_contracts: name: shell contracts runs-on: ubuntu-latest + env: + # Why: the suites below gate their live fish tests on the binary, which is + # right on a developer machine and wrong here — this job is a required check + # and its fish lane is the only end-to-end guard for #9993, so a skip would + # report green with nothing exercised. Turns those skips into failures. + ORCA_REQUIRE_FISH: '1' steps: - name: Checkout @@ -186,8 +192,46 @@ jobs: # Why fish: shell-ready.test.ts gates its live fish test on the binary being # present, so without this the fish barrier is only covered by config-shape # assertions and never actually exercised. + # Why release-4: DECSET 2031 arming lives in the fish 4.0 Rust tty_handoff, and + # fish-color-scheme-child-stdin.node-pty.test.ts (#9993) needs it. Noble ships + # 3.7, so the PPA is what makes that lane real. - name: Install zsh and fish - run: sudo apt-get update && sudo apt-get install -y zsh fish + run: | + # Why the update/PPA/fish steps are tolerant: a repo the runner image already + # ships can lack a Release file for this suite, and a failed add-apt-repository + # still leaves its list entry behind — either makes `apt-get update` exit + # non-zero and would red this required check over something unrelated to the + # PR. Every fish outcome is judged by the version gate below instead, so only + # the zsh install (which has no such gate) stays fatal here. + sudo apt-get update || true + # Why retry only here: adding the PPA is the network-flaky step, and the + # version gate below is fatal, so a transient Launchpad blip would + # otherwise red a required check on PRs unrelated to shells. + for attempt in 1 2 3; do + sudo add-apt-repository -y ppa:fish-shell/release-4 && break + echo "add-apt-repository attempt ${attempt} failed; retrying" >&2 + sudo add-apt-repository -y -r ppa:fish-shell/release-4 || true + sleep 5 + done + sudo apt-get update || true + # Why both shells on one line: pr-workflow-parallelism.test.mjs parses only the + # first install command in this step to prove the lane really installs them. + sudo apt-get install -y zsh fish + + # Separate from the install so the failure names the contract, not an apt error. + # ORCA_REQUIRE_FISH re-checks this at test time; this step just fails in seconds + # instead of after a full dependency install. + - name: Require fish 4+ + run: | + version="$(fish --version 2>/dev/null || true)" + major="${version##*version }" + major="${major%%.*}" + case "$major" in '' | *[!0-9]*) major=0 ;; esac + echo "${version:-}" + if [ "$major" -lt 4 ]; then + echo "::error::shell contracts needs fish 4+ (DECSET 2031 arming, #9993) but got '${version:-none}'. Fix the ppa:fish-shell/release-4 install rather than letting the fish lane skip." >&2 + exit 1 + fi - uses: ./.github/actions/install-node-dependencies with: @@ -201,6 +245,7 @@ jobs: src/main/providers/local-pty-shell-ready.test.ts \ src/main/providers/__tests__/shell-ready-framework-example.test.ts \ src/main/pty/omp-shell-wrapper.node-pty.test.ts \ + src/renderer/src/components/terminal-pane/fish-color-scheme-child-stdin.node-pty.test.ts \ src/shared/posix-command-path-lookup.test.ts test: @@ -235,6 +280,7 @@ jobs: --exclude=src/main/providers/local-pty-shell-ready.test.ts \ --exclude=src/main/providers/__tests__/shell-ready-framework-example.test.ts \ --exclude=src/main/pty/omp-shell-wrapper.node-pty.test.ts \ + --exclude=src/renderer/src/components/terminal-pane/fish-color-scheme-child-stdin.node-pty.test.ts \ --exclude=src/shared/posix-command-path-lookup.test.ts \ --exclude=tests/e2e/cross-version-wire/** \ --shard=${{ matrix.shard }}/${{ matrix.shard_total }} diff --git a/src/main/codex/codex-pane-account-registry.ts b/src/main/codex/codex-pane-account-registry.ts index 595f1d3e767..39853471683 100644 --- a/src/main/codex/codex-pane-account-registry.ts +++ b/src/main/codex/codex-pane-account-registry.ts @@ -111,6 +111,7 @@ function isShellStartupHomeOverride(value: unknown): value is CodexShellStartupH typeof context.home === 'string' && context.home.length > 0 && (context.shell === undefined || typeof context.shell === 'string') && + (context.configHome === undefined || typeof context.configHome === 'string') && typeof context.codexHome === 'string' && context.codexHome.length > 0 ) @@ -207,6 +208,7 @@ function shellStartupHomeOverridesEqual( return ( left?.home === right?.home && left?.shell === right?.shell && + left?.configHome === right?.configHome && left?.codexHome === right?.codexHome ) } diff --git a/src/main/codex/codex-real-home-path.test.ts b/src/main/codex/codex-real-home-path.test.ts index 7dc961c7a18..25d4f2b5bba 100644 --- a/src/main/codex/codex-real-home-path.test.ts +++ b/src/main/codex/codex-real-home-path.test.ts @@ -1,4 +1,4 @@ -import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join, sep } from 'node:path' import { afterEach, describe, expect, it } from 'vitest' @@ -11,9 +11,15 @@ import { import { __resetShellStartupEnvCache } from '../pty/shell-startup-env' const temporaryHomes: string[] = [] +const savedConfigHome = process.env.XDG_CONFIG_HOME afterEach(() => { __resetShellStartupEnvCache() + if (savedConfigHome === undefined) { + delete process.env.XDG_CONFIG_HOME + } else { + process.env.XDG_CONFIG_HOME = savedConfigHome + } for (const path of temporaryHomes.splice(0)) { rmSync(path, { recursive: true, force: true }) } @@ -56,6 +62,9 @@ describe('hasCustomCodexHomeOverride', () => { temporaryHomes.push(paneHome) writeFileSync(join(paneHome, '.zshrc'), 'export CODEX_HOME="$HOME/custom-codex-home"\n') + // Why cleared: the context records XDG_CONFIG_HOME, so a developer machine + // that sets one would otherwise change the recorded shape. + delete process.env.XDG_CONFIG_HOME expect(hasCustomCodexHomeOverrideForLaunch({ HOME: paneHome, SHELL: '/bin/zsh' })).toBe(true) const override = getCustomCodexHomeOverrideForLaunch({ HOME: paneHome, @@ -75,4 +84,50 @@ describe('hasCustomCodexHomeOverride', () => { ).toBe(true) } ) + + // Why: a fish user who exports XDG_CONFIG_HOME from config.fish never passes it to + // a Dock-launched Orca, so the launch env is the only place it appears. Reading the + // main process env instead scans ~/.config and misses the override entirely. + it.skipIf(process.platform === 'win32')( + 'resolves a fish override under the launch env XDG_CONFIG_HOME, not the process one', + () => { + const paneHome = mkdtempSync(join(tmpdir(), 'orca-codex-fish-home-')) + temporaryHomes.push(paneHome) + const configHome = join(paneHome, 'xdg') + mkdirSync(join(configHome, 'fish'), { recursive: true }) + writeFileSync( + join(configHome, 'fish', 'config.fish'), + 'set -gx CODEX_HOME "$HOME/custom-codex-home"\n' + ) + // The decoy fish reads only if XDG_CONFIG_HOME is ignored. + mkdirSync(join(paneHome, '.config', 'fish'), { recursive: true }) + writeFileSync( + join(paneHome, '.config', 'fish', 'config.fish'), + 'set -gx CODEX_HOME /wrong-default-config-home\n' + ) + delete process.env.XDG_CONFIG_HOME + + const launchEnv = { + HOME: paneHome, + SHELL: '/opt/homebrew/bin/fish', + XDG_CONFIG_HOME: configHome + } + const override = getCustomCodexHomeOverrideForLaunch(launchEnv) + + expect(override).toEqual({ + source: 'shell-startup', + context: { + home: paneHome, + shell: '/opt/homebrew/bin/fish', + configHome, + codexHome: join(paneHome, 'custom-codex-home') + } + }) + // And the recorded configHome is what a later re-check resolves against. + expect( + override?.source === 'shell-startup' && + shellStartupCodexHomeOverrideMatches(override.context) + ).toBe(true) + } + ) }) diff --git a/src/main/codex/codex-real-home-path.ts b/src/main/codex/codex-real-home-path.ts index b964f5ba518..60c644032f8 100644 --- a/src/main/codex/codex-real-home-path.ts +++ b/src/main/codex/codex-real-home-path.ts @@ -5,6 +5,8 @@ import { readShellStartupEnvVar } from '../pty/shell-startup-env' export type CodexShellStartupHomeOverride = { home: string shell?: string + /** Why recorded: fish reads config under it, so re-reads must use the same root. */ + configHome?: string codexHome: string } @@ -54,7 +56,10 @@ export function getCustomCodexHomeOverrideForLaunch( } const home = launchEnv ? getLaunchEnvValue(launchEnv, 'HOME') : process.env.HOME const shell = launchEnv ? getLaunchEnvValue(launchEnv, 'SHELL') : process.env.SHELL - const shellCodexHome = readShellStartupEnvVar('CODEX_HOME', home, shell) + const configHome = launchEnv + ? getLaunchEnvValue(launchEnv, 'XDG_CONFIG_HOME') + : process.env.XDG_CONFIG_HOME + const shellCodexHome = readShellStartupEnvVar('CODEX_HOME', home, shell, configHome) if (!home || !shellCodexHome || !hasCustomCodexHomeOverride({ CODEX_HOME: shellCodexHome })) { return null } @@ -63,6 +68,7 @@ export function getCustomCodexHomeOverrideForLaunch( context: { home, ...(shell ? { shell } : {}), + ...(configHome ? { configHome } : {}), codexHome: shellCodexHome } } @@ -85,7 +91,8 @@ export function shellStartupCodexHomeOverrideMatches( const currentCodexHome = readShellStartupEnvVar( 'CODEX_HOME', currentContext.home, - currentContext.shell + currentContext.shell, + currentContext.configHome ) return Boolean( currentCodexHome && @@ -101,13 +108,14 @@ export function shellStartupCodexHomeOverrideContextsEqual( return ( normalizePathForComparison(left.home) === normalizePathForComparison(right.home) && left.shell === right.shell && + left.configHome === right.configHome && normalizePathForComparison(left.codexHome) === normalizePathForComparison(right.codexHome) ) } function getLaunchEnvValue( launchEnv: NodeJS.ProcessEnv, - key: 'CODEX_HOME' | 'ORCA_CODEX_HOME' | 'HOME' | 'SHELL' + key: 'CODEX_HOME' | 'ORCA_CODEX_HOME' | 'HOME' | 'SHELL' | 'XDG_CONFIG_HOME' ): string | undefined { return Object.hasOwn(launchEnv, key) ? launchEnv[key] : process.env[key] } diff --git a/src/main/daemon/shell-ready.test.ts b/src/main/daemon/shell-ready.test.ts index 9610f2600d1..06261b56539 100644 --- a/src/main/daemon/shell-ready.test.ts +++ b/src/main/daemon/shell-ready.test.ts @@ -5,6 +5,7 @@ import { join } from 'node:path' import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' import type * as ShellReadyModule from './shell-ready' import { getZshShellReadyMarkerRegistrationBlock } from '../shell-templates' +import { fishRequirementViolation, resolveFishBinary } from '../../shared/fish-binary-requirement' async function importFreshShellReady(): Promise { vi.resetModules() @@ -16,8 +17,8 @@ const hasBash = process.platform !== 'win32' && spawnSync('bash', ['--version']) const itWithBash = hasBash ? it : it.skip const hasZsh = process.platform !== 'win32' && spawnSync('zsh', ['--version']).status === 0 const itWithZsh = hasZsh ? it : it.skip -const hasFish = process.platform !== 'win32' && spawnSync('fish', ['--version']).status === 0 -const itWithFish = hasFish ? it : it.skip +const FISH = resolveFishBinary() +const itWithFish = FISH.available ? it : it.skip const SHELL_READY_MARKER_OUTPUT = '\x1b]777;orca-shell-ready\x07' @@ -166,6 +167,11 @@ function expectFinalZdotdirRestoreContext(content: string) { } describePosix('daemon shell-ready launch config', () => { + // Always runs, so the CI lane cannot report green with every live fish test skipped. + it('has the fish the live tests need when CI requires one', () => { + expect(fishRequirementViolation(FISH)).toBeNull() + }) + let previousUserDataPath: string | undefined let previousOrcaOrigZdotdir: string | undefined let userDataPath: string diff --git a/src/main/ipc/pty-hidden-delivery-gate.ts b/src/main/ipc/pty-hidden-delivery-gate.ts index b1f089e385e..43fa8567b88 100644 --- a/src/main/ipc/pty-hidden-delivery-gate.ts +++ b/src/main/ipc/pty-hidden-delivery-gate.ts @@ -18,8 +18,7 @@ export type HiddenPtyDeliveryGateSettings = Pick< const hiddenRendererPtys = new Set() // Why: sidecar consumers (paste-draft pacing, background agent launches, -// automation observers, and the kill-switch-off parked 2031 responder) need -// live bytes even while no visible view exists. Any +// automation observers) need live bytes even while no visible view exists. Any // registered interest suppresses the gate for that PTY. const deliveryInterestRendererPtys = new Set() // Why: reveal must restore from the model only when bytes were actually diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index 7dc4c1cff49..42756d997b0 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -132,7 +132,7 @@ import { import { isRemoteAgentHooksEnabled } from '../../shared/agent-hook-relay' import { createTerminalSessionStateSaveFailureMessage } from '../../shared/terminal-session-state-save-failure' import { RendererTerminalSerializerReadiness } from './renderer-terminal-serializer-readiness' -import { readShellStartupEnvVar } from '../pty/shell-startup-env' +import { readSessionShellStartupEnvVar } from '../pty/shell-startup-env' import { isTerminalLeafId, makePaneKey, @@ -1507,11 +1507,7 @@ function resolvePiAgentSourceDir( if (kind === 'prime-agent') { return ( readEnvWithProcessFallback(baseEnv, primaryKey) ?? - readShellStartupEnvVar( - primaryKey, - baseEnv.HOME ?? process.env.HOME, - baseEnv.SHELL ?? process.env.SHELL - ) + readSessionShellStartupEnvVar(primaryKey, baseEnv) ) } @@ -1526,11 +1522,7 @@ function resolvePiAgentSourceDir( return publicDir } - return readShellStartupEnvVar( - primaryKey, - baseEnv.HOME ?? process.env.HOME, - baseEnv.SHELL ?? process.env.SHELL - ) + return readSessionShellStartupEnvVar(primaryKey, baseEnv) } function resolveScopedPiAgentSourceDir( @@ -1684,14 +1676,7 @@ function resolveOpenCodeSourceConfigDir(baseEnv: Record): string return undefined } - return ( - configDir ?? - readShellStartupEnvVar( - 'OPENCODE_CONFIG_DIR', - baseEnv.HOME ?? process.env.HOME, - baseEnv.SHELL ?? process.env.SHELL - ) - ) + return configDir ?? readSessionShellStartupEnvVar('OPENCODE_CONFIG_DIR', baseEnv) } /** diff --git a/src/main/providers/local-pty-provider.ts b/src/main/providers/local-pty-provider.ts index aafd04b9e1b..907695ea42f 100644 --- a/src/main/providers/local-pty-provider.ts +++ b/src/main/providers/local-pty-provider.ts @@ -13,6 +13,7 @@ import { existsSync } from 'node:fs' import * as pty from 'node-pty' import { getDefaultWslDistro, parseWslPath, isWslAvailableAsync } from '../wsl' import { splitWorktreeIdForFilesystem } from '../../shared/worktree-id' +import { isBracketedPasteSafeShell } from '../../shared/startup-command-submission' import { injectHistoryEnv, updateHistFileForFallback, @@ -1048,10 +1049,14 @@ export class LocalPtyProvider implements IPtyProvider { ptyDisposables.set(id, disposables) if (args.command && !startupCommandDeliveredInShellArgs) { - // Why: only POSIX bash/zsh have bracketed-paste armed so multiline startup prompts paste literally; others use raw submit. + // Why: shells with bracketed paste armed take a multiline startup prompt literally; others use raw submit. const spawnedShellName = getSpawnedShellName(shellPath).toLowerCase() const bracketedPasteSafe = - process.platform !== 'win32' && (spawnedShellName === 'bash' || spawnedShellName === 'zsh') + process.platform !== 'win32' && + isBracketedPasteSafeShell({ + shellName: spawnedShellName, + waitsForShellReady: shellReadyLaunch?.supportsReadyMarker === true + }) writeStartupCommandWhenShellReady( shellReadyPromise, proc, diff --git a/src/main/providers/local-pty-shell-ready.ts b/src/main/providers/local-pty-shell-ready.ts index 63885e23579..97973588a85 100644 --- a/src/main/providers/local-pty-shell-ready.ts +++ b/src/main/providers/local-pty-shell-ready.ts @@ -433,7 +433,7 @@ export function writeStartupCommandWhenShellReady( proc: pty.IPty, startupCommand: string, onExit: (cleanup: () => void) => void, - // Why: only Orca-wrapped bash/zsh have bracketed-paste active; other shells use the raw path to avoid echoing the ESC[200~ markers. + // Why: only shells with bracketed-paste active (see isBracketedPasteSafeShell) accept the wrapper; others use the raw path so ESC[200~ isn't echoed. options: { bracketedPasteSafe?: boolean } = {} ): void { let sent = false diff --git a/src/main/pty/shell-startup-env.test.ts b/src/main/pty/shell-startup-env.test.ts index 73aabc3b753..eaf8cffeba4 100644 --- a/src/main/pty/shell-startup-env.test.ts +++ b/src/main/pty/shell-startup-env.test.ts @@ -1,28 +1,41 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -const { existsSyncMock, readFileSyncMock } = vi.hoisted(() => ({ +const { existsSyncMock, readFileSyncMock, readdirSyncMock } = vi.hoisted(() => ({ existsSyncMock: vi.fn(), - readFileSyncMock: vi.fn() + readFileSyncMock: vi.fn(), + readdirSyncMock: vi.fn() })) vi.mock('fs', () => ({ existsSync: existsSyncMock, - readFileSync: readFileSyncMock + readFileSync: readFileSyncMock, + readdirSync: readdirSyncMock })) import { __resetShellStartupEnvCache, isShellStartupEnvProbeSupported, + readSessionShellStartupEnvVar, readShellStartupEnvVar } from './shell-startup-env' describe('readShellStartupEnvVar', () => { const originalPlatform = process.platform const originalShell = process.env.SHELL + // Why pinned: the fish branch defaults configHome to process.env.XDG_CONFIG_HOME, which CI + // runners set and dev machines usually do not — leaving it ambient makes these mocked paths + // resolve differently per host. + const originalXdgConfigHome = process.env.XDG_CONFIG_HOME beforeEach(() => { + delete process.env.XDG_CONFIG_HOME existsSyncMock.mockReset() readFileSyncMock.mockReset() + readdirSyncMock.mockReset() + // Why: only the fish branch lists a directory; every other case must see ENOENT. + readdirSyncMock.mockImplementation(() => { + throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }) + }) Object.defineProperty(process, 'platform', { configurable: true, value: 'darwin' }) process.env.SHELL = '/bin/zsh' __resetShellStartupEnvCache() @@ -35,6 +48,11 @@ describe('readShellStartupEnvVar', () => { } else { process.env.SHELL = originalShell } + if (originalXdgConfigHome === undefined) { + delete process.env.XDG_CONFIG_HOME + } else { + process.env.XDG_CONFIG_HOME = originalXdgConfigHome + } }) function mockStartupFiles(files: Record) { @@ -326,4 +344,218 @@ describe('readShellStartupEnvVar', () => { mockStartupFiles({ '.zshrc': '# export OPENCODE_CONFIG_DIR=/from-comment\n' }) expect(readShellStartupEnvVar('OPENCODE_CONFIG_DIR', '/home/alice')).toBeUndefined() }) + + describe('fish', () => { + const FISH = '/opt/homebrew/bin/fish' + + function mockFishFiles(files: Record, snippets: string[] = []) { + readdirSyncMock.mockImplementation((dir: string) => { + if (dir === '/home/alice/.config/fish/conf.d' || dir === '/cfg/fish/conf.d') { + return snippets + } + throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }) + }) + mockStartupFiles(files) + } + + it('reads an exported set from config.fish', () => { + mockFishFiles({ + '/home/alice/.config/fish/config.fish': 'set -gx CODEX_HOME /home/alice/.codex\n' + }) + expect(readShellStartupEnvVar('CODEX_HOME', '/home/alice', FISH)).toBe('/home/alice/.codex') + }) + + it('lets config.fish win over a conf.d snippet, matching fish source order', () => { + mockFishFiles( + { + '/home/alice/.config/fish/conf.d/10-agents.fish': 'set -gx CODEX_HOME /from/confd\n', + '/home/alice/.config/fish/config.fish': 'set -gx CODEX_HOME /from/config\n' + }, + ['10-agents.fish'] + ) + expect(readShellStartupEnvVar('CODEX_HOME', '/home/alice', FISH)).toBe('/from/config') + }) + + it('sources conf.d snippets in filename order', () => { + mockFishFiles( + { + '/home/alice/.config/fish/conf.d/aaa.fish': 'set -gx CODEX_HOME /from/aaa\n', + '/home/alice/.config/fish/conf.d/zzz.fish': 'set -gx CODEX_HOME /from/zzz\n' + }, + ['zzz.fish', 'aaa.fish', 'notes.txt'] + ) + expect(readShellStartupEnvVar('CODEX_HOME', '/home/alice', FISH)).toBe('/from/zzz') + }) + + it('honors XDG_CONFIG_HOME', () => { + mockFishFiles({ '/cfg/fish/config.fish': 'set -gx CODEX_HOME /from/xdg\n' }) + expect(readShellStartupEnvVar('CODEX_HOME', '/home/alice', FISH, '/cfg')).toBe('/from/xdg') + }) + + // Why also via the env: the argument defaults to process.env.XDG_CONFIG_HOME, so without + // this every other fish case here silently depends on the host not exporting it. + it('defaults configHome to XDG_CONFIG_HOME from the environment', () => { + process.env.XDG_CONFIG_HOME = '/cfg' + __resetShellStartupEnvCache() + mockFishFiles({ '/cfg/fish/config.fish': 'set -gx CODEX_HOME /from/env-xdg\n' }) + expect(readShellStartupEnvVar('CODEX_HOME', '/home/alice', FISH)).toBe('/from/env-xdg') + }) + + it('ignores sets that are not exported', () => { + mockFishFiles({ + '/home/alice/.config/fish/config.fish': + 'set -g CODEX_HOME /global\nset -l CODEX_HOME /local\nset CODEX_HOME /plain\n' + }) + expect(readShellStartupEnvVar('CODEX_HOME', '/home/alice', FISH)).toBeUndefined() + }) + + it('accepts every fish export spelling', () => { + for (const line of [ + 'set -x CODEX_HOME /a', + 'set -xg CODEX_HOME /a', + 'set -gx CODEX_HOME /a', + 'set -Ux CODEX_HOME /a', + 'set --export --global CODEX_HOME /a' + ]) { + __resetShellStartupEnvCache() + mockFishFiles({ '/home/alice/.config/fish/config.fish': `${line}\n` }) + expect(readShellStartupEnvVar('CODEX_HOME', '/home/alice', FISH)).toBe('/a') + } + }) + + it('expands $HOME in double quotes and keeps single quotes literal', () => { + mockFishFiles({ + '/home/alice/.config/fish/config.fish': 'set -gx CODEX_HOME "$HOME/.codex" # note\n' + }) + expect(readShellStartupEnvVar('CODEX_HOME', '/home/alice', FISH)).toBe('/home/alice/.codex') + + __resetShellStartupEnvCache() + mockFishFiles({ + '/home/alice/.config/fish/config.fish': "set -gx CODEX_HOME '$HOME/.codex'\n" + }) + expect(readShellStartupEnvVar('CODEX_HOME', '/home/alice', FISH)).toBe('$HOME/.codex') + }) + + it('does not read zsh or bash startup files for a fish user', () => { + mockFishFiles({ + '/home/alice/.zshrc': 'export CODEX_HOME=/from/zsh\n', + '/home/alice/.bash_profile': 'export CODEX_HOME=/from/bash\n' + }) + expect(readShellStartupEnvVar('CODEX_HOME', '/home/alice', FISH)).toBeUndefined() + }) + + it('ignores a commented assignment', () => { + mockFishFiles({ + '/home/alice/.config/fish/config.fish': '# set -gx CODEX_HOME /from-comment\n' + }) + expect(readShellStartupEnvVar('CODEX_HOME', '/home/alice', FISH)).toBeUndefined() + }) + + it('does not match a different variable with the same prefix', () => { + mockFishFiles({ + '/home/alice/.config/fish/config.fish': 'set -gx CODEX_HOME_BACKUP /backup\n' + }) + expect(readShellStartupEnvVar('CODEX_HOME', '/home/alice', FISH)).toBeUndefined() + }) + }) + + // Why these matter: a caller that plumbs HOME/SHELL but forgets XDG_CONFIG_HOME + // reads a different fish config than the shell will, so the same user gets one + // answer locally and another over relay. + describe('readSessionShellStartupEnvVar', () => { + const FISH = '/opt/homebrew/bin/fish' + const savedConfigHome = process.env.XDG_CONFIG_HOME + const savedHome = process.env.HOME + + afterEach(() => { + restoreEnv('XDG_CONFIG_HOME', savedConfigHome) + restoreEnv('HOME', savedHome) + }) + + function restoreEnv(key: string, value: string | undefined) { + if (value === undefined) { + delete process.env[key] + } else { + process.env[key] = value + } + } + + it("prefers the session env's XDG_CONFIG_HOME over the main process's", () => { + process.env.XDG_CONFIG_HOME = '/main-process-cfg' + mockStartupFiles({ + '/session-cfg/fish/config.fish': 'set -gx CODEX_HOME /from/session\n', + '/main-process-cfg/fish/config.fish': 'set -gx CODEX_HOME /from/main-process\n' + }) + + expect( + readSessionShellStartupEnvVar('CODEX_HOME', { + HOME: '/home/alice', + SHELL: FISH, + XDG_CONFIG_HOME: '/session-cfg' + }) + ).toBe('/from/session') + }) + + it('falls back to the main process XDG_CONFIG_HOME when the session env lacks one', () => { + process.env.XDG_CONFIG_HOME = '/main-process-cfg' + mockStartupFiles({ + '/main-process-cfg/fish/config.fish': 'set -gx CODEX_HOME /from/main-process\n' + }) + + expect( + readSessionShellStartupEnvVar('CODEX_HOME', { HOME: '/home/alice', SHELL: FISH }) + ).toBe('/from/main-process') + }) + + it("falls back to fish's own ~/.config default when neither env has one", () => { + delete process.env.XDG_CONFIG_HOME + mockStartupFiles({ + '/home/alice/.config/fish/config.fish': 'set -gx CODEX_HOME /from/default\n' + }) + + expect( + readSessionShellStartupEnvVar('CODEX_HOME', { HOME: '/home/alice', SHELL: FISH }) + ).toBe('/from/default') + }) + + it('resolves against the session HOME, not the main process HOME', () => { + delete process.env.XDG_CONFIG_HOME + process.env.HOME = '/home/root-user' + mockStartupFiles({ + '/home/alice/.config/fish/config.fish': 'set -gx CODEX_HOME "$HOME/.codex"\n', + '/home/root-user/.config/fish/config.fish': 'set -gx CODEX_HOME /from/wrong-home\n' + }) + + expect( + readSessionShellStartupEnvVar('CODEX_HOME', { HOME: '/home/alice', SHELL: FISH }) + ).toBe('/home/alice/.codex') + }) + + it('lets an explicit shell override beat the session SHELL', () => { + delete process.env.XDG_CONFIG_HOME + mockStartupFiles({ + '/home/alice/.config/fish/config.fish': 'set -gx CODEX_HOME /from/fish\n', + '/home/alice/.zshrc': 'export CODEX_HOME=/from/zsh\n' + }) + + expect( + readSessionShellStartupEnvVar( + 'CODEX_HOME', + { HOME: '/home/alice', SHELL: '/bin/zsh' }, + FISH + ) + ).toBe('/from/fish') + }) + + it('falls back to the main process HOME and SHELL with no session env at all', () => { + delete process.env.XDG_CONFIG_HOME + process.env.HOME = '/home/alice' + process.env.SHELL = FISH + mockStartupFiles({ + '/home/alice/.config/fish/config.fish': 'set -gx CODEX_HOME /from/process-env\n' + }) + + expect(readSessionShellStartupEnvVar('CODEX_HOME', undefined)).toBe('/from/process-env') + }) + }) }) diff --git a/src/main/pty/shell-startup-env.ts b/src/main/pty/shell-startup-env.ts index f423ef38f22..ad6392160a9 100644 --- a/src/main/pty/shell-startup-env.ts +++ b/src/main/pty/shell-startup-env.ts @@ -1,4 +1,4 @@ -import { existsSync, readFileSync } from 'node:fs' +import { existsSync, readdirSync, readFileSync } from 'node:fs' import { posix } from 'node:path' // Why: only files the user's actual shell would source. Mixing zsh and bash @@ -11,24 +11,45 @@ const ZSH_AFTER_ENV_FILES = ['.zprofile', '.zshrc', '.zlogin'] // .bash_profile / .bash_login / .profile but intentionally does NOT force // .bashrc. Scanning .bashrc would mirror values the live Orca bash never sees. const BASH_LOGIN_FILES = ['.bash_profile', '.bash_login', '.profile'] +// Why: fish sources conf.d/*.fish (sorted by name) before config.fish, for +// login and non-login shells alike — verified against fish 4.7. +const FISH_SNIPPET_DIR = 'conf.d' +const FISH_SNIPPET_SUFFIX = '.fish' +const FISH_CONFIG_FILE = 'config.fish' + +/** Assignment grammar of a startup file: `export NAME=value` vs fish `set -gx NAME value`. */ +type StartupFileSyntax = 'export' | 'fish-set' + +type ShellStartupFiles = { + paths: readonly string[] + syntax: StartupFileSyntax +} export function isShellStartupEnvProbeSupported(): boolean { return process.platform !== 'win32' } -function parseExportedValue(content: string, name: string, home: string): string | undefined { - const assignment = new RegExp(`^export\\s+${name}=(.+)$`) +function parseAssignedValue( + content: string, + name: string, + home: string, + syntax: StartupFileSyntax +): string | undefined { + const assignment = + syntax === 'fish-set' + ? new RegExp(`^set\\s+((?:-{1,2}[A-Za-z][\\w-]*\\s+)+)${name}\\s+(.+)$`) + : new RegExp(`^export\\s+${name}=()(.+)$`) let lastMatch: string | undefined for (const rawLine of content.split(/\r?\n/)) { const line = rawLine.trim() const match = assignment.exec(line) - if (!match?.[1]) { + if (!match?.[2] || (syntax === 'fish-set' && !fishFlagsExport(match[1] ?? ''))) { continue } // Why: strip trailing unquoted `# comment` first so quoted values like // `"$HOME/.opencode" # note` survive intact for unquoteShellValue. - const decommented = stripTrailingComment(match[1]) + const decommented = stripTrailingComment(match[2]) const { text, quoted } = unquoteShellValue(decommented) // Why: $HOME / ${HOME} / ~ expansion mimics what the live shell would // do for double-quoted and unquoted values; single-quoted is literal. @@ -41,6 +62,21 @@ function parseExportedValue(content: string, name: string, home: string): string return lastMatch } +function parseExportedValue(content: string, name: string, home: string): string | undefined { + return parseAssignedValue(content, name, home, 'export') +} + +// Why: only `set -x` / `--export` reaches child processes; `set -g`, `set -l` +// and function-local sets never appear in the PTY's environment. +function fishFlagsExport(flags: string): boolean { + return flags + .trim() + .split(/\s+/) + .some((flag) => + flag.startsWith('--') ? flag === '--export' : /^-[A-Za-z]*x[A-Za-z]*$/.test(flag) + ) +} + function readStartupFile(path: string): string | null { if (!existsSync(path)) { return null @@ -52,23 +88,55 @@ function readStartupFile(path: string): string | null { } } -function shellStartupFilePaths(home: string, shell: string | undefined): readonly string[] { +function shellStartupFiles( + home: string, + shell: string | undefined, + configHome: string | undefined +): ShellStartupFiles { if (!shell) { // Why: Orca's POSIX default shell is /bin/zsh when $SHELL is unset. - return zshStartupFilePaths(home) + return { paths: zshStartupFilePaths(home), syntax: 'export' } } const name = posix.basename(shell).toLowerCase() if (name === 'zsh') { - return zshStartupFilePaths(home) + return { paths: zshStartupFilePaths(home), syntax: 'export' } } if (name === 'bash') { - return BASH_LOGIN_FILES.map((file) => posix.join(home, file)) + return { + paths: BASH_LOGIN_FILES.map((file) => posix.join(home, file)), + syntax: 'export' + } } - // Why: unsupported explicit shells (fish, nushell, custom wrappers) do not - // use Orca's zsh/bash shell-ready startup files, so scanning those files + if (name === 'fish') { + return { + paths: fishStartupFilePaths(home, configHome), + syntax: 'fish-set' + } + } + // Why: unsupported explicit shells (nushell, custom wrappers) do not use + // Orca's zsh/bash/fish shell-ready startup files, so scanning those files // would mirror values the live PTY shell never sees. - return [] + return { paths: [], syntax: 'export' } +} + +function fishStartupFilePaths(home: string, configHome: string | undefined): readonly string[] { + const fishDir = posix.join(configHome?.trim() || posix.join(home, '.config'), 'fish') + return [ + ...fishSnippetPaths(posix.join(fishDir, FISH_SNIPPET_DIR)), + posix.join(fishDir, FISH_CONFIG_FILE) + ] +} + +function fishSnippetPaths(snippetDir: string): readonly string[] { + try { + return readdirSync(snippetDir) + .filter((entry) => entry.endsWith(FISH_SNIPPET_SUFFIX)) + .sort() + .map((entry) => posix.join(snippetDir, entry)) + } catch { + return [] + } } function zshStartupFilePaths(home: string): readonly string[] { @@ -139,21 +207,24 @@ const cache = new Map() * Limits (callers should treat the result as a hint, not authoritative): * - Conditionals (`[[ ... ]] && export FOO=...`), sourced files, and * `$VAR` substitution beyond `$HOME` / `${HOME}` / `~` are not evaluated. - * - Bare assignments (no `export` keyword) are ignored because POSIX shells - * do not export them to child processes. + * - Bare assignments (no `export` keyword, or fish `set` without `-x`) are + * ignored because they never reach child processes. * - Files are scanned in shell evaluation order for the user's $SHELL family - * only (zsh OR bash, not both); unsupported explicit shells scan nothing. - * LAST matching assignment wins. + * only (zsh OR bash OR fish, never mixed); unsupported explicit shells scan + * nothing. LAST matching assignment wins. + * - fish universal variables (`set -Ux` stored in fish_variables) are only + * seen when the assignment is also written in a config file. * - Windows is unsupported (PowerShell profile parsing is out of scope). * - * Results are memoized per (name, home, shell) for the process lifetime — - * shell startup files do not change mid-session in any practical scenario, - * and PTY spawn is on the hot path. + * Results are memoized per (name, home, shell, configHome) for the process + * lifetime — shell startup files do not change mid-session in any practical + * scenario, and PTY spawn is on the hot path. */ export function readShellStartupEnvVar( name: string, home = process.env.HOME, - shell = process.env.SHELL + shell = process.env.SHELL, + configHome = process.env.XDG_CONFIG_HOME ): string | undefined { if (!home || !isShellStartupEnvProbeSupported()) { return undefined @@ -164,20 +235,21 @@ export function readShellStartupEnvVar( return undefined } - const cacheKey = `${name}\0${home}\0${shell ?? ''}` + const cacheKey = `${name}\0${home}\0${shell ?? ''}\0${configHome ?? ''}` if (cache.has(cacheKey)) { return cache.get(cacheKey) } let lastMatch: string | undefined - for (const path of shellStartupFilePaths(home, shell)) { + const { paths, syntax } = shellStartupFiles(home, shell, configHome) + for (const path of paths) { const content = readStartupFile(path) if (content === null) { continue } - const match = parseExportedValue(content, name, home) + const match = parseAssignedValue(content, name, home, syntax) if (match !== undefined) { lastMatch = match } @@ -187,6 +259,34 @@ export function readShellStartupEnvVar( return lastMatch } +/** The env keys that decide which startup files a shell would actually source. */ +export type ShellStartupEnvSource = { + HOME?: string | undefined + SHELL?: string | undefined + XDG_CONFIG_HOME?: string | undefined +} + +/** + * Same probe, resolved against a session/PTY env with process.env as fallback. + * + * Why this exists rather than three inline `??` chains per call site: dropping + * XDG_CONFIG_HOME silently scans a *different* fish config than the shell will, + * so local launches and relay launches disagree about the same user's + * `set -gx` — a divergence with no visible symptom until it is wrong. + */ +export function readSessionShellStartupEnvVar( + name: string, + sessionEnv: ShellStartupEnvSource | undefined, + shellOverride?: string +): string | undefined { + return readShellStartupEnvVar( + name, + sessionEnv?.HOME ?? process.env.HOME, + shellOverride ?? sessionEnv?.SHELL ?? process.env.SHELL, + sessionEnv?.XDG_CONFIG_HOME ?? process.env.XDG_CONFIG_HOME + ) +} + /** * Test-only helper to reset the per-process cache between cases. * Why: production callers never invalidate (rc files don't change at diff --git a/src/relay/plugin-overlay-env.test.ts b/src/relay/plugin-overlay-env.test.ts index 78e08918297..7746307cf47 100644 --- a/src/relay/plugin-overlay-env.test.ts +++ b/src/relay/plugin-overlay-env.test.ts @@ -104,4 +104,31 @@ describe('plugin overlay env source resolution', () => { ) ).toBe('/remote/original-prime') }) + + // Why: the session env is the only place a fish user's XDG_CONFIG_HOME shows up + // (config.fish exports it, so no GUI-launched process inherits it). Dropping it + // here would scan ~/.config and disagree with the same lookup on the main side. + it.skipIf(process.platform === 'win32')( + 'reads fish config under the session XDG_CONFIG_HOME', + () => { + const configHome = join(homeDir, 'xdg') + mkdirSync(join(configHome, 'fish'), { recursive: true }) + writeFileSync( + join(configHome, 'fish', 'config.fish'), + 'set -gx OPENCODE_CONFIG_DIR "$HOME/company-opencode"\n' + ) + mkdirSync(join(homeDir, '.config', 'fish'), { recursive: true }) + writeFileSync( + join(homeDir, '.config', 'fish', 'config.fish'), + 'set -gx OPENCODE_CONFIG_DIR /wrong-default-config-home\n' + ) + + expect( + resolveOpenCodeSourceConfigDir( + { HOME: homeDir, XDG_CONFIG_HOME: configHome }, + '/opt/homebrew/bin/fish' + ) + ).toBe(join(homeDir, 'company-opencode')) + } + ) }) diff --git a/src/relay/plugin-overlay-env.ts b/src/relay/plugin-overlay-env.ts index 4b91da8ff62..1db43f4717e 100644 --- a/src/relay/plugin-overlay-env.ts +++ b/src/relay/plugin-overlay-env.ts @@ -1,4 +1,4 @@ -import { readShellStartupEnvVar } from '../main/pty/shell-startup-env' +import { readSessionShellStartupEnvVar } from '../main/pty/shell-startup-env' import { PRIMARY_AGENT_DIR_ENV_BY_KIND, SOURCE_AGENT_DIR_ENV_BY_KIND, @@ -14,7 +14,9 @@ function readStartupEnv( env: Record, shell: string | undefined ): string | undefined { - return readShellStartupEnvVar(name, env.HOME ?? process.env.HOME, shell ?? env.SHELL) + // Why the session env first: it is closer to the user's shell than the relay + // process env, and fish config lives under its XDG_CONFIG_HOME. + return readSessionShellStartupEnvVar(name, env, shell) } export function resolveOpenCodeSourceConfigDir( diff --git a/src/renderer/src/components/terminal-pane/fish-color-scheme-child-stdin.node-pty.test.ts b/src/renderer/src/components/terminal-pane/fish-color-scheme-child-stdin.node-pty.test.ts new file mode 100644 index 00000000000..a5e52d13e4b --- /dev/null +++ b/src/renderer/src/components/terminal-pane/fish-color-scheme-child-stdin.node-pty.test.ts @@ -0,0 +1,487 @@ +/** + * Real-fish regression for #9993: Orca must never write `CSI ?997;Nn` into a PTY + * because fish armed DECSET 2031 around a prompt. + * + * fish toggles `?2031h ... ?2031l` in tty_handoff.rs every time it paints a prompt and + * withdraws before handing the tty to a child, so the arm window is ~1ms — shorter than + * a renderer IPC hop. Any reply lands after the withdrawal and is read by whatever owns + * the tty next: it paints as literal `?997;1n` at the prompt, or is swallowed by a child + * that reads stdin (npx / brew confirm prompts). + * + * The assertion is therefore about what a CHILD PROCESS READS, not what renders: a + * screen-level check passes while the child's stdin is still corrupted. + * + * Real production code under test: connectPanePty's live mode-2031 chunk observer. The + * mock transport writes whatever the renderer sends straight into the real fish PTY, so a + * reply-on-subscribe reaches the child exactly as it does in the app. DA1/CPR/OSC-10/11 + * probes are answered by the harness because no real xterm is attached here — without + * them fish stalls ~10s on its DA1 wait and every timing claim becomes meaningless. + */ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import path from 'node:path' +import type * as React from 'react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + fishRequirementViolation, + resolveFishBinary +} from '../../../../shared/fish-binary-requirement' + +// Why a version floor and not just presence: DECSET 2031 arming lives in the Rust +// tty_handoff introduced by the fish 4.0 rewrite. An older fish cannot produce the +// subscribe at all, so the test would pass vacuously instead of guarding anything. +const FISH = resolveFishBinary(4) +const FISH_BIN = FISH.path +const itWithFish = FISH.available ? it : it.skip + +const PROMPT_MARK = 'ORCA997> ' +const COLOR_SCHEME_REPORT_PREFIX = '\x1b[?997' +const ARM_2031 = '\x1b[?2031h' +const WITHDRAW_2031 = '\x1b[?2031l' +const LEAF_1 = '11111111-1111-4111-8111-111111111111' + +type MutableState = Record +let mockStoreState: MutableState = {} +let storeSubscribers: ((state: MutableState) => void)[] = [] +let transportFactoryQueue: unknown[] = [] + +vi.mock('@/runtime/sync-runtime-graph', () => ({ scheduleRuntimeGraphSync: vi.fn() })) +vi.mock('./terminal-webgl-atlas-recovery', () => ({ + scheduleTerminalWebglAtlasRecovery: vi.fn() +})) +vi.mock('@/lib/codex-stale-pane-sweep', () => ({ notifyCodexPaneBoundForStaleSweep: vi.fn() })) +vi.mock('sonner', () => ({ toast: { info: vi.fn() } })) +vi.mock('./cache-timer-seeding', () => ({ shouldSeedCacheTimerOnInitialTitle: vi.fn(() => false) })) +vi.mock('@/store', () => ({ + useAppStore: { + getState: () => mockStoreState, + subscribe: (listener: (state: MutableState) => void) => { + storeSubscribers.push(listener) + return () => { + storeSubscribers = storeSubscribers.filter((candidate) => candidate !== listener) + } + } + } +})) +// Why: connectPanePty calls useNotificationDispatch's useCallback outside React. +vi.mock('react', async (importOriginal) => ({ + ...(await importOriginal()), + useCallback: (fn: T): T => fn +})) +vi.mock('./pty-transport', () => ({ + createIpcPtyTransport: vi.fn(() => { + const next = transportFactoryQueue.shift() + if (!next) { + throw new Error('No mock transport queued') + } + return next + }) +})) + +type ConnectCallbacks = { onData?: (data: string) => void } + +/** Mock transport whose renderer→PTY writes land in the real fish master fd. */ +function createPtyBackedTransport(write: (data: string) => void): { + transport: Record + sent: string[] + emit: (data: string) => void +} { + const sent: string[] = [] + const captured: { current: ((data: string) => void) | null } = { current: null } + const send = (data: string): boolean => { + sent.push(data) + write(data) + return true + } + const transport = { + attach: vi.fn(), + connect: vi.fn(async ({ callbacks }: { callbacks?: ConnectCallbacks }) => { + captured.current = callbacks?.onData ?? null + return 'fish-pty' + }), + disconnect: vi.fn(), + sendInput: vi.fn(send), + sendInputImmediate: vi.fn(send), + sendInputAccepted: vi.fn(async (data: string) => send(data)), + isConnected: vi.fn(() => true), + claimViewport: vi.fn(() => true), + resize: vi.fn(() => true), + getPtyId: vi.fn(() => 'fish-pty'), + getConnectionId: vi.fn(() => null) + } + return { transport, sent, emit: (data: string) => captured.current?.(data) } +} + +function createPane(paneId: number): Record { + const activeBuffer = { type: 'normal' as const, viewportY: 0, baseY: 0, cursorY: 0, cursorX: 0 } + const container = new EventTarget() + Object.defineProperty(container, 'dataset', { configurable: true, value: {} }) + return { + id: paneId, + leafId: LEAF_1, + stablePaneId: LEAF_1, + container, + fitAddon: { fit: vi.fn(), proposeDimensions: vi.fn(() => ({ cols: 120, rows: 30 })) }, + terminal: { + cols: 120, + rows: 30, + element: {}, + buffer: { active: activeBuffer }, + modes: { bracketedPasteMode: false, sendFocusMode: false }, + options: { scrollback: 5_000, theme: { foreground: '#eeeeee', background: '#111111' } }, + write: vi.fn(), + resize: vi.fn(), + clear: vi.fn(), + scrollToBottom: vi.fn(), + paste: vi.fn(), + onData: vi.fn(() => ({ dispose: vi.fn() })), + onResize: vi.fn(() => ({ dispose: vi.fn() })), + onRender: vi.fn(() => ({ dispose: vi.fn() })), + onTitleChange: vi.fn(() => ({ dispose: vi.fn() })), + hasSelection: vi.fn(() => false), + parser: { + registerCsiHandler: vi.fn(() => ({ dispose: vi.fn() })), + registerOscHandler: vi.fn(() => ({ dispose: vi.fn() })) + } + } + } +} + +function createDeps(): Record { + return { + tabId: 'tab-1', + worktreeId: 'wt-1', + cwd: '/tmp/wt-1', + startup: null, + restoredLeafId: null, + restoredPtyIdByLeafId: {}, + paneTransportsRef: { current: new Map() }, + paneMode2031Ref: { current: new Map() }, + paneKittyKeyboardModesRef: { current: new Map() }, + paneLastThemeModeRef: { current: new Map() }, + replayingPanesRef: { current: new Map() }, + isActiveRef: { current: true }, + isVisibleRef: { current: true }, + onPtyExitRef: { current: vi.fn() }, + onAgentExitedRef: { current: vi.fn() }, + onPtyErrorRef: { current: vi.fn() }, + clearTabPtyId: vi.fn(), + consumeSuppressedPtyExit: vi.fn(() => false), + isPtyShutdownPending: vi.fn(() => false), + updateTabTitle: vi.fn(), + setRuntimePaneTitle: vi.fn(), + clearRuntimePaneTitle: vi.fn(), + updateTabPtyId: vi.fn(), + markWorktreeUnread: vi.fn(), + markTerminalTabUnread: vi.fn(), + markTerminalPaneUnread: vi.fn(), + clearWorktreeUnread: vi.fn(), + clearTerminalTabUnread: vi.fn(), + clearTerminalPaneUnread: vi.fn(), + dispatchNotification: vi.fn(), + onShowSessionRestoredBanner: vi.fn(), + setCacheTimerStartedAt: vi.fn(), + syncPanePtyLayoutBinding: vi.fn(), + clearExitedPanePtyLayoutBinding: vi.fn() + } +} + +function createManager(pane: Record): Record { + return { + setPaneGpuRendering: vi.fn(), + markPaneHasComplexScriptOutput: vi.fn(), + rebuildPaneWebgl: vi.fn(), + hasWebglRenderer: vi.fn(() => false), + getPanes: vi.fn(() => [pane]), + closePane: vi.fn(), + getActivePane: vi.fn(() => pane), + getNumericIdForLeaf: vi.fn(() => 1), + setActivePane: vi.fn() + } +} + +const sleep = (msValue: number): Promise => + new Promise((resolve) => setTimeout(resolve, msValue)) + +const countOf = (haystack: string, needle: string): number => haystack.split(needle).length - 1 + +async function waitUntil(predicate: () => boolean, timeoutMs: number): Promise { + const deadline = Date.now() + timeoutMs + while (Date.now() < deadline) { + if (predicate()) { + return true + } + await sleep(10) + } + return false +} + +describe('fish never receives a color-scheme report it did not query (#9993)', () => { + let configHome: string | null = null + + // Always runs, so the CI lane cannot report green with the regression below skipped. + it('has the fish this suite needs when CI requires one', () => { + expect(fishRequirementViolation(FISH)).toBeNull() + }) + + beforeEach(() => { + vi.resetModules() + vi.clearAllMocks() + transportFactoryQueue = [] + storeSubscribers = [] + mockStoreState = { + activeWorktreeId: 'wt-1', + tabsByWorktree: { 'wt-1': [{ id: 'tab-1', ptyId: 'fish-pty' }] }, + ptyIdsByTabId: { 'tab-1': ['fish-pty'] }, + terminalLayoutsByTabId: { + 'tab-1': { + root: { type: 'leaf', leafId: LEAF_1 }, + activeLeafId: LEAF_1, + expandedLeafId: null, + ptyIdsByLeafId: { [LEAF_1]: 'fish-pty' } + } + }, + unreadTerminalTabs: {}, + deleteStateByWorktreeId: {}, + worktreesByRepo: { repo1: [{ id: 'wt-1', repoId: 'repo1', path: '/tmp/wt-1' }] }, + runtimeStatusByEnvironmentId: new Map(), + repos: [{ id: 'repo1', connectionId: null }], + projects: [], + sshConnectionStates: new Map(), + transientClearedAgentStatusConnectionIds: {}, + cacheTimerByKey: {}, + // Dark app mode: a pre-fix reply is `CSI ?997;1n`, the exact payload issue #9993 reports. + settings: { + theme: 'dark', + promptCacheTimerEnabled: true, + terminalMainSideEffectAuthority: false + }, + codexRestartNoticeByPtyId: {}, + deferredSshReconnectTargets: [], + deferredSshSessionIdsByTabId: {}, + removeDeferredSshReconnectTarget: vi.fn(), + removeDeferredSshSessionId: vi.fn(), + consumePendingColdRestore: vi.fn(() => null), + consumePendingSnapshot: vi.fn(() => null), + runtimePaneTitlesByTabId: {}, + agentStatusByPaneKey: {}, + retainedAgentsByPaneKey: {}, + paneForegroundAgentByPaneKey: {}, + sleepingAgentSessionsByPaneKey: {}, + suppressedPtyExitIds: {}, + agentLaunchConfigByPaneKey: {}, + getAgentLaunchConfigForStatusEntry: vi.fn(), + getAgentLaunchConfigForStatusMetadata: vi.fn(), + clearSleepingAgentSession: vi.fn(), + registerAgentLaunchConfig: vi.fn(), + clearAgentLaunchConfig: vi.fn(), + markWorktreeUnread: vi.fn(), + observeTerminalGitHubPullRequestLink: vi.fn(), + recordTerminalInput: vi.fn(), + setAgentStatus: vi.fn(), + removeAgentStatus: vi.fn(), + dropAgentStatus: vi.fn(), + retireAgentPaneAuthority: vi.fn(), + setPaneForegroundAgent: vi.fn(), + clearPaneForegroundAgent: vi.fn(), + markTerminalTabUnread: vi.fn(), + markTerminalPaneUnread: vi.fn(), + markAgentCompletionPaneUnread: vi.fn() + } + ;(globalThis as unknown as { window: unknown }).window = { + api: { + pty: { + kill: vi.fn(), + signal: vi.fn(), + listSessions: vi.fn().mockResolvedValue([]), + hasPty: vi.fn().mockResolvedValue(true), + getSize: vi.fn().mockResolvedValue(null), + reportGeometry: vi.fn(), + getMainBufferSnapshot: vi.fn().mockResolvedValue(null), + getForegroundProcess: vi.fn().mockResolvedValue(null), + inspectProcess: vi.fn().mockResolvedValue(null), + confirmForegroundProcess: vi.fn().mockResolvedValue(null), + hasChildProcesses: vi.fn().mockResolvedValue(false), + write: vi.fn(), + writeAccepted: vi.fn().mockResolvedValue(true), + setHiddenRendererPty: vi.fn(), + setPtyDeliveryInterest: vi.fn(), + ackColdRestore: vi.fn(), + onClearBufferRequest: vi.fn(() => vi.fn()), + onSerializeBufferRequest: vi.fn(() => vi.fn()), + sendSerializedBuffer: vi.fn(), + declarePendingPaneSerializer: vi.fn().mockResolvedValue(1), + settlePaneSerializer: vi.fn().mockResolvedValue(undefined), + clearPendingPaneSerializer: vi.fn().mockResolvedValue(undefined), + reportRendererSerializerReady: vi.fn().mockResolvedValue(undefined) + }, + platform: { get: vi.fn(() => ({ platform: process.platform, osRelease: '' })) }, + notifications: { + dispatch: vi.fn().mockResolvedValue({ delivered: true }), + playSound: vi.fn().mockResolvedValue({ played: true }) + }, + runtime: { restoreTerminalFit: vi.fn().mockResolvedValue({ restored: true }) }, + agentStatus: { inferInterrupt: vi.fn().mockResolvedValue(false) } + }, + dispatchEvent: vi.fn(), + addEventListener: vi.fn(), + removeEventListener: vi.fn() + } + globalThis.requestAnimationFrame = vi.fn((callback: FrameRequestCallback) => { + callback(0) + return 1 + }) + globalThis.cancelAnimationFrame = vi.fn() + }) + + afterEach(() => { + delete (globalThis as { requestAnimationFrame?: unknown }).requestAnimationFrame + delete (globalThis as { cancelAnimationFrame?: unknown }).cancelAnimationFrame + delete (globalThis as { window?: unknown }).window + if (configHome) { + rmSync(configHome, { recursive: true, force: true }) + configHome = null + } + }) + + itWithFish( + 'writes no CSI 997 while fish arms mode 2031 around its prompt, so a child reads clean stdin', + async () => { + const { connectPanePty } = await import('./pty-connection') + const nodePty = await import('node-pty') + + configHome = mkdtempSync(path.join(tmpdir(), 'orca-fish-2031-')) + mkdirSync(path.join(configHome, 'fish'), { recursive: true }) + // Plain prompt, no user config: fish core toggles DEC 2031 regardless of the prompt. + writeFileSync( + path.join(configHome, 'fish/config.fish'), + [ + 'set -g fish_greeting ""', + `function fish_prompt; printf '${PROMPT_MARK}'; end`, + 'function fish_right_prompt; end', + '' + ].join('\n') + ) + // Echoes the first stdin LINE back escaped, so leaked control bytes are visible on + // screen. Why a line and not the first chunk: leaked replies carry no newline, so a + // once('data') child reports them alone whenever they arrive in their own read — + // which is what the issue's own `sys.stdin.readline()` repro measures. + const childScript = path.join(configHome, 'read-stdin.mjs') + writeFileSync( + childScript, + "let buffered = ''\n" + + "process.stdin.on('data', (d) => {\n" + + " buffered += d.toString('utf8')\n" + + " if (!buffered.includes('\\n')) return\n" + + " process.stdout.write('CHILD-READ:' + JSON.stringify(buffered) + '\\n')\n" + + ' process.exit(0)\n' + + '})\n' + ) + + const term = nodePty.spawn(FISH_BIN as string, ['-l', '-i'], { + name: 'xterm-256color', + cols: 120, + rows: 30, + cwd: configHome, + env: { + PATH: process.env.PATH ?? '/usr/bin:/bin', + HOME: configHome, + TERM: 'xterm-256color', + COLORTERM: 'truecolor', + LANG: 'en_US.UTF-8', + XDG_CONFIG_HOME: configHome, + XDG_DATA_HOME: path.join(configHome, 'data'), + ORCA_NODE_BIN: process.execPath, + ORCA_CHILD_SCRIPT: childScript + } + }) + + let rendered = '' + const { transport, sent, emit } = createPtyBackedTransport((data) => term.write(data)) + transportFactoryQueue.push(transport) + + // Why answered here: no real xterm is attached, and fish blocks its first prompt ~10s + // on DA1 and re-probes every prompt. These are harness bytes, never renderer output. + term.onData((chunk) => { + rendered += chunk + if (chunk.includes('\x1b[0c') || chunk.includes('\x1b[c')) { + term.write('\x1b[?62;4;6;22c') + } + if (chunk.includes('\x1b[6n')) { + term.write('\x1b[1;1R') + } + if (chunk.includes('\x1b]10;?') || chunk.includes('\x1b]11;?')) { + term.write('\x1b]11;rgb:1e1e/1e1e/1e1e\x1b\\') + } + emit(chunk) + }) + + let exited = false + term.onExit(() => { + exited = true + }) + + try { + const pane = createPane(1) + const deps = createDeps() + const binding = connectPanePty(pane as never, createManager(pane) as never, deps as never) + + expect(await waitUntil(() => rendered.includes(PROMPT_MARK), 15_000)).toBe(true) + // The arm is the settle signal. + expect(await waitUntil(() => rendered.includes(ARM_2031), 5_000)).toBe(true) + // Vacuity guard: the renderer observer must actually have seen the subscribe, + // otherwise "sent nothing" is trivially true. Checked here and not at the end, + // because the last decision races between fish's subscribe and its withdrawal. + const paneMode2031 = (deps as { paneMode2031Ref: { current: Map } }) + .paneMode2031Ref.current + expect(await waitUntil(() => paneMode2031.get(1) === true, 5_000)).toBe(true) + + // Type-ahead is the deterministic leak shape: queue the child command while an + // external command still owns the tty, so fish repaints the prompt (`?2031h`) and + // consumes the buffered line in the same breath — handoff lands sub-millisecond + // after the subscribe, inside any reply's flight time. + term.write('sleep 0.4\r') + // Withdrawal #1: `sleep` owns the tty now, so the next line is typed ahead. + expect(await waitUntil(() => countOf(rendered, WITHDRAW_2031) >= 1, 5_000)).toBe(true) + term.write('"$ORCA_NODE_BIN" "$ORCA_CHILD_SCRIPT"\r') + // Withdrawal #2: fish re-armed for the prompt and handed the tty to the child. + expect(await waitUntil(() => countOf(rendered, WITHDRAW_2031) >= 2, 5_000)).toBe(true) + + const renderedBeforeChildInput = rendered.length + // Canonical mode buffers this in the tty, so it queues behind anything already + // written there — including a reply the renderer sent during the handoff. + term.write('hello\r') + expect( + await waitUntil( + () => rendered.slice(renderedBeforeChildInput).includes('CHILD-READ:'), + 10_000 + ) + ).toBe(true) + + const childRead = + rendered.slice(renderedBeforeChildInput).match(/CHILD-READ:[^\r\n]*/)?.[0] ?? '' + // The merge-blocking assertion: the child's STDIN, not the screen. A rendered-output + // check passes while npx/brew confirm prompts still eat the bytes. + // Scope note: the harness' own DA1/CPR/OSC-11 answers can still land here — that is + // the separate probe-reply family (fish asks for those; nobody asked for 997). + expect(childRead).not.toContain('997') + expect(childRead).toContain('hello') + // Nothing the renderer sent was a color-scheme report, on any path. + expect(sent.filter((data) => data.includes(COLOR_SCHEME_REPORT_PREFIX))).toEqual([]) + // And fish never echoed one back as literal prompt text. + expect(rendered).not.toContain('997;1n') + + binding.dispose() + } finally { + term.write('exit\r') + await waitUntil(() => exited, 3_000) + try { + term.kill() + } catch { + // already gone + } + } + }, + 30_000 + ) +}) diff --git a/src/renderer/src/components/terminal-pane/parked-terminal-byte-watcher.test.ts b/src/renderer/src/components/terminal-pane/parked-terminal-byte-watcher.test.ts index 93349254955..1a9653d15c8 100644 --- a/src/renderer/src/components/terminal-pane/parked-terminal-byte-watcher.test.ts +++ b/src/renderer/src/components/terminal-pane/parked-terminal-byte-watcher.test.ts @@ -108,19 +108,19 @@ describe('startParkedTerminalByteWatcher', () => { async function startWatcher( overrides: Partial = {} - ): Promise<{ dispose: () => void; sendInput: ReturnType }> { + ): Promise<{ dispose: () => void; ptyWrite: ReturnType }> { const { startParkedTerminalByteWatcher } = await import('./parked-terminal-byte-watcher') - const sendInput = vi.fn() + const ptyWrite = vi.fn() + ;(window as unknown as { api: { pty: Record } }).api.pty.write = ptyWrite const dispose = startParkedTerminalByteWatcher({ ptyId: PTY_ID, tabId: TAB_ID, worktreeId: WORKTREE_ID, leafId: LEAF_ID, paneId: PANE_ID, - sendInput, ...overrides }) - return { dispose, sendInput } + return { dispose, ptyWrite } } beforeEach(() => { @@ -360,31 +360,20 @@ describe('startParkedTerminalByteWatcher', () => { dispose() }) - it('answers a DECSET 2031 subscribe split across chunks via sendInput', async () => { - const { dispose, sendInput } = await startWatcher() + // Regression pin (#9993): DECSET 2031 subscribes to future color changes — it is not a + // query (that is CSI ?996n). A parked tab has no view to answer with and any reply lands + // after fish withdrew the mode, painting `?997;1n` at the prompt. + it('never writes a color-scheme reply for a DECSET 2031 subscribe', async () => { + const { dispose, ptyWrite } = await startWatcher() emit('\x1b[?20') - expect(sendInput).not.toHaveBeenCalled() - emit('31h') - expect(sendInput).toHaveBeenCalledTimes(1) - // theme=system + prefers-dark → dark reply per terminal-color-scheme-protocol. - expect(sendInput).toHaveBeenCalledWith('\x1b[?997;1n') - emit('\x1b[?2031l') - expect(sendInput).toHaveBeenCalledTimes(1) + + expect(ptyWrite).not.toHaveBeenCalled() dispose() }) - it('stops answering DECSET 2031 after dispose', async () => { - const { dispose, sendInput } = await startWatcher() - - dispose() - emit('\x1b[?2031h') - - expect(sendInput).not.toHaveBeenCalled() - }) - it('observes GitHub PR links across chunk boundaries', async () => { const { dispose } = await startWatcher() @@ -608,8 +597,7 @@ describe('startParkedTerminalByteWatcher', () => { // // With the kill switch on, the watcher must not register byte parsers — // main is the single byte parser and the watcher's policy block consumes - // pty:sideEffect facts instead. The byte sidecar stays ONLY for the 2031 - // reply (query authority never moves to main); PR links arrive as facts. + // pty:sideEffect facts instead; PR links arrive as facts. describe('with main side-effect authority on', () => { function enableMainAuthority(): void { mockStoreState.settings = { @@ -875,19 +863,15 @@ describe('startParkedTerminalByteWatcher', () => { dispose() }) - it('answers DECSET 2031 from the main 2031-subscribe fact, never the byte scan', async () => { - // Why: with the hidden-delivery gate on (default), parked PTY bytes are - // dropped in main — the fact is the only 2031 signal, and the byte - // sidecar must NOT exist (its registration would re-enable delivery). + it('stays silent on a main 2031-subscribe fact', async () => { + // Regression pin (#9993): the fact records a subscription for theme-flip + // pushes; a parked watcher must never answer it. enableMainAuthority() - const { dispose, sendInput } = await startWatcher() + const { dispose, ptyWrite } = await startWatcher() emit('\x1b[?2031h') - expect(sendInput).not.toHaveBeenCalled() - await dispatchFacts([{ kind: '2031-subscribe' }]) - expect(sendInput).toHaveBeenCalledTimes(1) - expect(sendInput).toHaveBeenCalledWith('\x1b[?997;1n') + expect(ptyWrite).not.toHaveBeenCalled() // Why: pr-link facts arrive on the channel; byte-scanning here too // would observe every link twice. @@ -912,7 +896,7 @@ describe('startParkedTerminalByteWatcher', () => { expect(setHiddenRendererPty).toHaveBeenLastCalledWith(PTY_ID, false) }) - it('keeps the byte 2031 responder and no hidden bit when the gate kill switch is off', async () => { + it('sets no hidden bit and stays silent on 2031 when the gate kill switch is off', async () => { enableMainAuthority() mockStoreState.settings = { ...mockStoreState.settings, @@ -922,20 +906,14 @@ describe('startParkedTerminalByteWatcher', () => { ;( window as unknown as { api: { pty: Record } } ).api.pty.setHiddenRendererPty = setHiddenRendererPty - const { dispose, sendInput } = await startWatcher() + const { dispose, ptyWrite } = await startWatcher() - // Gate off — bytes keep flowing, so the split-chunk byte scan answers. + // Gate off — bytes keep flowing, and they still elicit nothing (#9993). emit('\x1b[?20') - expect(sendInput).not.toHaveBeenCalled() emit('31h') - expect(sendInput).toHaveBeenCalledTimes(1) - expect(sendInput).toHaveBeenCalledWith('\x1b[?997;1n') - - // Why: a 2031-subscribe fact must not double-fire the reply in byte - // mode — exactly one responder owns the answer at any time. await dispatchFacts([{ kind: '2031-subscribe' }]) - expect(sendInput).toHaveBeenCalledTimes(1) + expect(ptyWrite).not.toHaveBeenCalled() expect(setHiddenRendererPty).not.toHaveBeenCalled() dispose() }) diff --git a/src/renderer/src/components/terminal-pane/parked-terminal-byte-watcher.ts b/src/renderer/src/components/terminal-pane/parked-terminal-byte-watcher.ts index 42dff141d34..8d24cfca7f7 100644 --- a/src/renderer/src/components/terminal-pane/parked-terminal-byte-watcher.ts +++ b/src/renderer/src/components/terminal-pane/parked-terminal-byte-watcher.ts @@ -5,12 +5,7 @@ import { isClaudeAgent } from '../../../../shared/agent-detection' import { makePaneKey } from '../../../../shared/stable-pane-id' import { useAppStore } from '@/store' -import { - mode2031SequenceFor, - resolveTerminalColorSchemeMode -} from '../../../../shared/terminal-color-scheme-protocol' import { createTerminalGitHubPRLinkDetector } from '../../../../shared/terminal-github-pr-link-detector' -import { getSystemPrefersDark } from '@/lib/terminal-theme' import { AGENT_TASK_COMPLETE_NOTIFICATION_GRACE_MS, isAgentTaskCompleteOsNotificationEnabledFromState, @@ -22,7 +17,6 @@ import { createParkedTerminalCommandStatusPolicy, readInFlightCommandCodeTurn } from './parked-terminal-command-status' -import { startParkedTerminalMode2031Responder } from './parked-terminal-mode2031-responder' import { subscribeToPtyData } from './pty-data-sidecar-subscriptions' import { createPtyOutputProcessor } from './pty-transport' import { isRendererHiddenPtyDeliveryGateEnabled } from './terminal-hidden-delivery-gate' @@ -61,8 +55,6 @@ export type ParkedTerminalByteWatcherOptions = { initialTitle?: string /** Pull main's title-only snapshot when a watcher starts before its pane ever mounted (ordinary park cycles already have a title). */ restoreTitleOnRegister?: boolean - /** Out-of-band reply channel to the PTY (mode-2031 color-scheme answers). */ - sendInput: (data: string) => void } const parkedWatcherDisposersByPtyId = new Map void>() @@ -70,7 +62,7 @@ const parkedWatcherDisposersByPtyId = new Map void>() export function startParkedTerminalByteWatcher( options: ParkedTerminalByteWatcherOptions ): () => void { - const { ptyId, tabId, worktreeId, paneId, sendInput } = options + const { ptyId, tabId, worktreeId, paneId } = options const remoteRuntimePty = isRemoteRuntimePtyId(ptyId) const drivesTabTitle = options.drivesTabTitle ?? true const paneKey = makePaneKey(tabId, options.leafId) @@ -213,16 +205,9 @@ export function startParkedTerminalByteWatcher( runtimeEnvironmentId: null }) const factSideEffectAuthority = mainSideEffectAuthority || remoteRuntimePty - // Why: decided once at watcher start — it picks which 2031 responder (byte sidecar vs fact reply) exists, so it must never flip per chunk. const hiddenDeliveryGateActive = mainSideEffectAuthority && isRendererHiddenPtyDeliveryGateEnabled(useAppStore.getState().settings) - const factOwnsMode2031 = hiddenDeliveryGateActive || remoteRuntimePty - - const sendMode2031Reply = (): void => { - const settings = useAppStore.getState().settings - sendInput(mode2031SequenceFor(resolveTerminalColorSchemeMode(settings, getSystemPrefersDark()))) - } // Why (byte-parser mode only): reuse the transport's output processor to keep exact live-path parsing semantics. // initialAgentTitle: an agent already working at park time still produces a working→idle transition. @@ -261,20 +246,13 @@ export function startParkedTerminalByteWatcher( onCommandCodeWorking: commandStatusPolicy.onCommandCodeWorking, onCommandCodeDone: commandStatusPolicy.onCommandCodeDone, onPrLink: (link) => - useAppStore.getState().observeTerminalGitHubPullRequestLink(worktreeId, link), - // Why (gate mode only): the 2031 subscribe arrives as a fact, but the reply stays here — query authority stays with the view/watcher (invariant 6). - ...(factOwnsMode2031 ? { onMode2031Subscribe: sendMode2031Reply } : {}) + useAppStore.getState().observeTerminalGitHubPullRequestLink(worktreeId, link) }, // Why: activation-deferred tabs can start a watcher before any pane restored the title; ordinary parked tabs avoid this IPC. restoreTitleOnRegister: options.restoreTitleOnRegister === true }) : null - // Why: no xterm answers DECSET 2031 while parked; with the gate ON, the responder's sidecar would force-feed bytes to the gated PTY, so skip it. - const stopMode2031Responder = factOwnsMode2031 - ? null - : startParkedTerminalMode2031Responder({ ptyId, sendInput }) - // Why: parked tabs are the canonical hidden view — mark the PTY gated so main stops renderer byte delivery. const releaseHiddenDeliveryClaim = hiddenDeliveryGateActive ? acquireHiddenRendererPtyDeliveryClaim(ptyId) @@ -308,7 +286,6 @@ export function startParkedTerminalByteWatcher( processor?.disposePendingSideEffectGauge() // Why: unhide BEFORE the reveal remount registers pane handlers, so main resumes delivery and emits the restore marker the pane consumes. releaseHiddenDeliveryClaim?.() - stopMode2031Responder?.() unsubscribeByteParsers?.() unregisterFactConsumer?.() // Why: clears tracker/timer/detector state so the watcher can't fire after the revealed pane's live parsers take over. diff --git a/src/renderer/src/components/terminal-pane/parked-terminal-mode2031-responder.test.ts b/src/renderer/src/components/terminal-pane/parked-terminal-mode2031-responder.test.ts deleted file mode 100644 index c619790fdb1..00000000000 --- a/src/renderer/src/components/terminal-pane/parked-terminal-mode2031-responder.test.ts +++ /dev/null @@ -1,124 +0,0 @@ -// Why: a parked tab has no xterm, so this byte sidecar is the only thing that -// answers DECSET 2031. fish toggles 2031 on and off around every prompt, so -// answering the sticky "an h appeared" flag writes `?997;1n` into a shell that -// already handed the tty to a child (#9993). -import { beforeEach, describe, expect, it, vi } from 'vitest' - -const ESC = '\x1b' -const PTY_ID = 'pty-parked-2031' - -// One fish prompt cycle: subscribe, paint, hand off the tty. -const FISH_PROMPT_HANDOFF = `${ESC}[?2031h${ESC}[0m~/orca ${ESC}[32m❯${ESC}[0m ${ESC}[?2031l` - -let sidecarWatcher: ((data: string) => void) | null = null -const unsubscribe = vi.fn() - -vi.mock('./pty-data-sidecar-subscriptions', () => ({ - subscribeToPtyData: (_ptyId: string, watcher: (data: string) => void) => { - sidecarWatcher = watcher - return unsubscribe - } -})) - -vi.mock('@/store', () => ({ - useAppStore: { - getState: () => ({ settings: { theme: 'dark' as const } }) - } -})) - -vi.mock('@/lib/terminal-theme', () => ({ - getSystemPrefersDark: () => true -})) - -const { startParkedTerminalMode2031Responder } = - await import('./parked-terminal-mode2031-responder') - -function startResponder(): { sendInput: ReturnType; feed: (data: string) => void } { - const sendInput = vi.fn() - startParkedTerminalMode2031Responder({ ptyId: PTY_ID, sendInput }) - return { - sendInput, - feed: (data: string) => sidecarWatcher?.(data) - } -} - -beforeEach(() => { - sidecarWatcher = null - vi.clearAllMocks() -}) - -describe('parked-tab DECSET 2031 responder honors the chunk-final state (#9993)', () => { - it('does not reply to a subscribe the same chunk withdrew', () => { - const { sendInput, feed } = startResponder() - - feed(FISH_PROMPT_HANDOFF) - - expect(sendInput).not.toHaveBeenCalled() - }) - - it('replies once to a TUI that subscribes and keeps listening', () => { - const { sendInput, feed } = startResponder() - - feed(`${ESC}[?2031h`) - - expect(sendInput).toHaveBeenCalledTimes(1) - expect(sendInput).toHaveBeenCalledWith(`${ESC}[?997;1n`) - }) - - it('stays silent across a run of fish prompts', () => { - const { sendInput, feed } = startResponder() - - feed(FISH_PROMPT_HANDOFF) - feed(FISH_PROMPT_HANDOFF) - feed(FISH_PROMPT_HANDOFF) - - expect(sendInput).not.toHaveBeenCalled() - }) - - it('replies when a TUI subscribes after a prompt cycle in the same chunk', () => { - const { sendInput, feed } = startResponder() - - feed(`${FISH_PROMPT_HANDOFF}${ESC}[?2031h`) - - expect(sendInput).toHaveBeenCalledTimes(1) - }) - - it('still answers a subscribe split across two chunks', () => { - const { sendInput, feed } = startResponder() - - feed(`${ESC}[?20`) - feed('31h') - - expect(sendInput).toHaveBeenCalledTimes(1) - }) - - it('does not reply when the withdrawal is split across two chunks', () => { - const { sendInput, feed } = startResponder() - - feed(`${ESC}[?2031h prompt ${ESC}[?20`) - expect(sendInput).not.toHaveBeenCalled() - feed('31l') - - expect(sendInput).not.toHaveBeenCalled() - }) - - it('does not reply when an unrelated private mode appends a split withdrawal', () => { - const { sendInput, feed } = startResponder() - - feed(`${ESC}[?2031h prompt ${ESC}[?25`) - expect(sendInput).not.toHaveBeenCalled() - feed(';2031l') - - expect(sendInput).not.toHaveBeenCalled() - }) - - it('replies after an ambiguous tail resolves to another mode', () => { - const { sendInput, feed } = startResponder() - - feed(`${ESC}[?2031h drawing ${ESC}[?20`) - expect(sendInput).not.toHaveBeenCalled() - feed('25h') - - expect(sendInput).toHaveBeenCalledTimes(1) - }) -}) diff --git a/src/renderer/src/components/terminal-pane/parked-terminal-mode2031-responder.ts b/src/renderer/src/components/terminal-pane/parked-terminal-mode2031-responder.ts deleted file mode 100644 index 4c2ca568de9..00000000000 --- a/src/renderer/src/components/terminal-pane/parked-terminal-mode2031-responder.ts +++ /dev/null @@ -1,49 +0,0 @@ -/** - * DECSET 2031 color-scheme responder for parked terminals (byte-scan mode). - * - * Why a dedicated byte sidecar: no xterm exists while a tab is parked, so - * nothing answers a TUI's mode-2031 theme subscription. Query authority stays - * with the view/watcher (model/view contract invariant 6), so this reply can - * never move to main. Phase 4: this subscribeToPtyData registration doubles - * as a delivery-interest signal, so it is only used while the hidden-delivery - * gate is OFF — gated parked PTYs answer from the main tracker's - * '2031-subscribe' fact instead (parked-terminal-byte-watcher.ts). - * - * Survives Phase 6 (skip-grammar deletion): mounted switch-off hidden panes - * answer 2031 from xterm once the background queue drains, but a PARKED tab - * has no xterm in any switch-off mode, and the '2031-subscribe' fact is only - * consumed while the gate is ON — this sidecar stays the only answerer here. - */ -import { - INITIAL_MODE_2031_REPLY_SCAN_STATE, - mode2031SequenceFor, - resolveTerminalColorSchemeMode, - scanMode2031ReplyDecision -} from '../../../../shared/terminal-color-scheme-protocol' -import { useAppStore } from '@/store' -import { getSystemPrefersDark } from '@/lib/terminal-theme' -import { subscribeToPtyData } from './pty-data-sidecar-subscriptions' - -export type ParkedTerminalMode2031ResponderOptions = { - ptyId: string - /** Out-of-band reply channel to the PTY (mode-2031 color-scheme answers). */ - sendInput: (data: string) => void -} - -export function startParkedTerminalMode2031Responder( - options: ParkedTerminalMode2031ResponderOptions -): () => void { - const { ptyId, sendInput } = options - let scanState = INITIAL_MODE_2031_REPLY_SCAN_STATE - return subscribeToPtyData(ptyId, (data) => { - const result = scanMode2031ReplyDecision(scanState, data) - scanState = result.state - if (result.decision !== 'subscribed') { - return - } - // Why: reply with the resolved theme so TUIs that subscribe while parked - // still learn it before the pane is ever revealed. - const settings = useAppStore.getState().settings - sendInput(mode2031SequenceFor(resolveTerminalColorSchemeMode(settings, getSystemPrefersDark()))) - }) -} diff --git a/src/renderer/src/components/terminal-pane/pty-connection-types.ts b/src/renderer/src/components/terminal-pane/pty-connection-types.ts index 9c2f717b8a6..18cae8d72c2 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-types.ts @@ -99,10 +99,8 @@ export type PtyConnectionDeps = { syncPanePtyLayoutBinding: (paneId: number, ptyId: string | null) => void clearExitedPanePtyLayoutBinding: (paneId: number, exitedPtyId: string) => void deferPtyInput?: (paneId: number, data: string, forward: (data: string) => void) => void - /** Records a DECSET 2031 subscription answered from main's - * '2031-subscribe' fact, mirroring the xterm CSI handler's registry write - * (paneMode2031 + last replied theme) so later theme flips push CSI 997. - * The reply itself is sent by the fact handler — query authority stays - * with the view (model/view contract invariant 6). */ - recordPaneMode2031Subscription?: (paneId: number, repliedMode: 'dark' | 'light') => void + /** Records a DECSET 2031 subscription seen through main's '2031-subscribe' + * fact (paneMode2031 + the mode at subscribe time) so later theme flips push + * CSI 997. Subscribing itself is silent — see #9993. */ + recordPaneMode2031Subscription?: (paneId: number, subscribedMode: 'dark' | 'light') => void } diff --git a/src/renderer/src/components/terminal-pane/pty-connection.test.ts b/src/renderer/src/components/terminal-pane/pty-connection.test.ts index c78ac18274b..fb6a97b8de6 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -12244,14 +12244,13 @@ describe('connectPanePty', () => { dataCallback('startup probe output\r\n') expect(setHiddenRendererPty).toHaveBeenCalledWith('pty-id', true) - // The fact stays the sole 2031 responder for gate-managed PTYs. + // The fact records the subscription for gate-managed PTYs; it never answers it (#9993). factsHandler._dispatchTerminalSideEffectBatchForTest({ ptyId: 'pty-id', seq: 8, facts: [{ kind: '2031-subscribe' }] }) - expect(transport.sendInput).toHaveBeenCalledTimes(1) - expect(transport.sendInput).toHaveBeenCalledWith('\x1b[?997;1n') + expect(transport.sendInput).not.toHaveBeenCalledWith(expect.stringMatching(/\?997/)) }) it('latches model restore from the out-of-band marker and restores on reveal', async () => { @@ -12287,7 +12286,8 @@ describe('connectPanePty', () => { ) }) - it('answers each 2031-subscribe fact exactly once, before any hidden mark exists', async () => { + it('never answers a 2031-subscribe fact, hidden or visible', async () => { + // Regression pin (#9993): a subscribe arms future notifications, it is not a query. enableMainAuthority() const deps = createDeps({ isVisibleRef: { current: false } }) const { transport } = await connectHiddenPane(deps) @@ -12298,27 +12298,23 @@ describe('connectPanePty', () => { transportOptions.onPtySpawn?.('pty-id') const factsHandler = await import('./terminal-side-effect-facts-handler') - // Why: the fact can outrun the hidden mark (codex startup race) and must still reply — ownership is structural, not mark-dependent. factsHandler._dispatchTerminalSideEffectBatchForTest({ ptyId: 'pty-id', seq: 12, facts: [{ kind: '2031-subscribe' }] }) - expect(transport.sendInput).toHaveBeenCalledTimes(1) - expect(transport.sendInput).toHaveBeenCalledWith('\x1b[?997;1n') - - // Why: a visible gated pane still answers via the fact — the lifecycle suppresses xterm's CSI reply for gate-managed panes. ;(deps.isVisibleRef as { current: boolean }).current = true factsHandler._dispatchTerminalSideEffectBatchForTest({ ptyId: 'pty-id', seq: 24, facts: [{ kind: '2031-subscribe' }] }) - expect(transport.sendInput).toHaveBeenCalledTimes(2) - expect(transport.sendInput).toHaveBeenLastCalledWith('\x1b[?997;1n') + + expect(transport.sendInput).not.toHaveBeenCalledWith(expect.stringMatching(/\?997/)) + expect(transport.sendInputImmediate).not.toHaveBeenCalledWith(expect.stringMatching(/\?997/)) }) - it('registers the fact-answered 2031 subscription for later theme flips', async () => { + it('registers the fact-observed 2031 subscription for later theme flips', async () => { enableMainAuthority() const recordPaneMode2031Subscription = vi.fn() const deps = createDeps({ @@ -12338,16 +12334,16 @@ describe('connectPanePty', () => { facts: [{ kind: '2031-subscribe' }] }) - expect(transport.sendInput).toHaveBeenCalledWith('\x1b[?997;1n') // Why: without the registry write, maybePushMode2031Flip won't push CSI 997 after a theme change, so the TUI keeps a stale theme. expect(recordPaneMode2031Subscription).toHaveBeenCalledWith(1, 'dark') + expect(transport.sendInput).not.toHaveBeenCalledWith(expect.stringMatching(/\?997/)) }) it('retires the fact-registered subscription when the TUI withdraws it', async () => { // The counterpart to the test above. A gated pane never sees the withdrawal bytes - // (main drops them) and both the chunk scanner and xterm's CSI handler are disabled - // for it, so this fact is the only observer that can retire the subscription. Left - // registered, the next theme flip pushes CSI 997 at the shell that replaced the TUI. + // (main drops them) and the chunk scanner is disabled for it, so this fact is the only + // observer that can retire the subscription. Left registered, the next theme flip + // pushes CSI 997 at the shell that replaced the TUI. enableMainAuthority() const paneMode2031Ref = { current: new Map() } const paneLastThemeModeRef = { current: new Map() } @@ -12356,9 +12352,9 @@ describe('connectPanePty', () => { paneMode2031Ref, paneLastThemeModeRef, // Exactly what use-terminal-pane-lifecycle wires up for this callback. - recordPaneMode2031Subscription: (paneId: number, repliedMode: 'dark' | 'light') => { + recordPaneMode2031Subscription: (paneId: number, subscribedMode: 'dark' | 'light') => { paneMode2031Ref.current.set(paneId, true) - paneLastThemeModeRef.current.set(paneId, repliedMode) + paneLastThemeModeRef.current.set(paneId, subscribedMode) } }) await connectHiddenPane(deps) @@ -12385,8 +12381,7 @@ describe('connectPanePty', () => { }) it('leaves the chunk scanner silent on a gate-managed PTY', async () => { - // Main's '2031-subscribe' fact already answers these; a chunk-boundary reply here - // would answer the same subscribe a second time. + // Main's '2031-subscribe' fact already records these; nothing on this path replies. enableMainAuthority() const deps = createDeps({ isVisibleRef: { current: false } }) const { transport, dataCallback } = await connectHiddenPane(deps) @@ -12746,6 +12741,7 @@ describe('connectPanePty', () => { dataCallback: (data: string, meta?: { seq?: number; rawLength?: number }) => void getMainBufferSnapshot: ReturnType transport: MockTransport + deps: ReturnType }> { enableMainAuthority() const deps = createDeps({ isVisibleRef: { current: true } }) @@ -12777,7 +12773,7 @@ describe('connectPanePty', () => { expect.any(Function) ) pane.terminal.write.mockClear() - return { pane, dataCallback, getMainBufferSnapshot, transport } + return { pane, dataCallback, getMainBufferSnapshot, transport, deps } } function writtenData(pane: ReturnType): string { @@ -12798,24 +12794,25 @@ describe('connectPanePty', () => { expect(writtenData(pane)).toContain('NEW') }) - it('answers a 2031 subscribe on a chunk the restored snapshot drops as duplicate', async () => { - // Why the scan runs before reconciliation: the snapshot restore replays bytes - // into xterm without answering queries, so this live delivery is the only - // chance to reply. Dropping the chunk as a duplicate must not drop the query. - // Gate off so the chunk scanner (not main's fact) owns this pane's reply. + it('records a 2031 subscribe on a chunk the restored snapshot drops as duplicate', async () => { + // Why the scan runs before reconciliation: the snapshot restore replays bytes into + // xterm without tracking modes, so this live delivery is the only chance to observe + // the subscription. Dropping the chunk as a duplicate must not drop that. + // Gate off so the chunk scanner (not main's fact) owns this pane's registry. enableMainAuthority() mockStoreState.settings = { ...mockStoreState.settings, terminalHiddenDeliveryGate: false } as StoreState['settings'] - const { pane, dataCallback, transport } = await restoreVisiblePaneToBaseline() + const { pane, dataCallback, transport, deps } = await restoreVisiblePaneToBaseline() transport.sendInput.mockClear() dataCallback(`SUB\x1b[?2031h`, { seq: 60, rawLength: 11 }) await flushAsyncTicks(8) expect(writtenData(pane)).not.toContain('SUB') - expect(transport.sendInput).toHaveBeenCalledWith('\x1b[?997;1n') + expect(deps.paneMode2031Ref.current.get(1)).toBe(true) + expect(transport.sendInput).not.toHaveBeenCalledWith(expect.stringMatching(/\?997/)) }) it('slices a partial overlap when raw and clean lengths match', async () => { @@ -13496,7 +13493,7 @@ describe('connectPanePty', () => { binding.dispose() }) - it('answers a mode 2031 subscribe once, from the raw chunk boundary', async () => { + it('records a mode 2031 subscribe from the raw chunk boundary without answering', async () => { const { connectPanePty } = await import('./pty-connection') const transport = createMockTransport('pty-id') const capturedDataCallback: { current: ((data: string) => void) | null } = { current: null } @@ -13526,8 +13523,8 @@ describe('connectPanePty', () => { // Why the scanner and not xterm's CSI handler: xterm batches PTY chunks into one // parse, so only this layer knows the chunk ended still subscribed (#9993). - expect(transport.sendInput).toHaveBeenCalledTimes(1) - expect(transport.sendInput).toHaveBeenCalledWith('\x1b[?997;2n') + expect(transport.sendInput).not.toHaveBeenCalledWith(expect.stringMatching(/\?997/)) + expect(transport.sendInputImmediate).not.toHaveBeenCalledWith(expect.stringMatching(/\?997/)) // The bytes still reach xterm so the emulator tracks the mode itself. expect(pane.terminal.write).toHaveBeenCalledWith('\x1b[?2031h') } finally { @@ -13537,12 +13534,12 @@ describe('connectPanePty', () => { binding.dispose() }) - // Why these three: fish 4.7.1 enables and disables 2031 around *every* prompt with no - // opt-out, so back-to-back chunks each carrying a toggle are the normal case, not an edge - // case. Each chunk owes exactly one decision, and xterm cannot make it — it parses several - // chunks in one synchronous batch, so a reply deferred to the parser is either dropped or - // answered against a subscription a later chunk already withdrew (#9993). - describe('mode 2031 replies are decided per raw PTY chunk', () => { + // Why: fish 4.7.1 enables and disables 2031 around *every* prompt with no opt-out, so + // back-to-back chunks each carrying a toggle are the normal case. Subscribing is never + // answered (#9993) — these pin that silence plus the per-chunk subscription bookkeeping + // that theme-flip pushes depend on. xterm cannot do that bookkeeping: it parses several + // chunks in one synchronous batch and only sees the net result. + describe('mode 2031 subscriptions are tracked per raw PTY chunk, never answered', () => { async function connectVisiblePane(): Promise<{ transport: ReturnType deps: ReturnType @@ -13581,56 +13578,60 @@ describe('connectPanePty', () => { const replies = (transport: { sendInput: { mock: { calls: unknown[][] } } }): unknown[] => transport.sendInput.mock.calls.flat().filter((arg) => String(arg).includes('997')) - it('answers a chunk that ends subscribed even when the next chunk withdraws', async () => { - const { transport, emit, dispose } = await connectVisiblePane() - // Two separate PTY chunks. xterm would parse both in one batch and see only the net - // result; the first chunk still owes a reply. + it('registers a chunk that ends subscribed, then retires it when the next withdraws', async () => { + const { transport, deps, emit, dispose } = await connectVisiblePane() + // Two separate PTY chunks. xterm would parse both in one batch and see only the net result. emit('\x1b[?2031h') + expect(deps.paneMode2031Ref.current.get(1)).toBe(true) emit('\x1b[?2031l') - expect(replies(transport)).toEqual(['\x1b[?997;2n']) + expect(deps.paneMode2031Ref.current.get(1)).toBeUndefined() + expect(replies(transport)).toEqual([]) dispose() }) - it('answers each chunk that ends subscribed', async () => { - const { transport, emit, dispose } = await connectVisiblePane() + it('re-registers a chunk that withdraws and subscribes again', async () => { + const { transport, deps, emit, dispose } = await connectVisiblePane() emit('\x1b[?2031h') emit('\x1b[?2031l\x1b[?2031h') - expect(replies(transport)).toHaveLength(2) + expect(deps.paneMode2031Ref.current.get(1)).toBe(true) + expect(replies(transport)).toEqual([]) dispose() }) - it('stays silent when one chunk both subscribes and withdraws', async () => { - const { transport, emit, dispose } = await connectVisiblePane() - // The fish prompt case: the subscription is gone before the program could read a reply. + it('registers nothing when one chunk both subscribes and withdraws', async () => { + const { transport, deps, emit, dispose } = await connectVisiblePane() + // The fish prompt case: the subscription is gone before the program could read anything. emit('\x1b[?2031h prompt \x1b[?2031l') + expect(deps.paneMode2031Ref.current.get(1)).toBeUndefined() expect(replies(transport)).toEqual([]) dispose() }) - it('stays silent when the withdrawal is split across two chunks', async () => { - const { transport, emit, dispose } = await connectVisiblePane() + it('registers nothing when the withdrawal is split across two chunks', async () => { + const { transport, deps, emit, dispose } = await connectVisiblePane() emit('\x1b[?2031h prompt \x1b[?20') - expect(replies(transport)).toEqual([]) emit('31l') + expect(deps.paneMode2031Ref.current.get(1)).toBeUndefined() expect(replies(transport)).toEqual([]) dispose() }) - it('stays silent when an unrelated private mode appends a split withdrawal', async () => { - const { transport, emit, dispose } = await connectVisiblePane() + it('registers nothing when an unrelated private mode appends a split withdrawal', async () => { + const { transport, deps, emit, dispose } = await connectVisiblePane() emit('\x1b[?2031h prompt \x1b[?25') - expect(replies(transport)).toEqual([]) emit(';2031l') + expect(deps.paneMode2031Ref.current.get(1)).toBeUndefined() expect(replies(transport)).toEqual([]) dispose() }) - it('answers after an ambiguous tail resolves to another mode', async () => { - const { transport, emit, dispose } = await connectVisiblePane() + it('registers the subscription once an ambiguous tail resolves to another mode', async () => { + const { transport, deps, emit, dispose } = await connectVisiblePane() emit('\x1b[?2031h drawing \x1b[?20') - expect(replies(transport)).toEqual([]) + expect(deps.paneMode2031Ref.current.get(1)).toBeUndefined() emit('25h') - expect(replies(transport)).toEqual(['\x1b[?997;2n']) + expect(deps.paneMode2031Ref.current.get(1)).toBe(true) + expect(replies(transport)).toEqual([]) dispose() }) @@ -13654,19 +13655,21 @@ describe('connectPanePty', () => { dispose() }) - it('answers once when a TUI subscribes at the end of a fish prompt chunk', async () => { - const { transport, emit, dispose } = await connectVisiblePane() + it('registers a TUI that subscribes at the end of a fish prompt chunk', async () => { + const { transport, deps, emit, dispose } = await connectVisiblePane() emit(`${FISH_PROMPT_HANDOFF}\x1b[?2031h`) - expect(replies(transport)).toHaveLength(1) + expect(deps.paneMode2031Ref.current.get(1)).toBe(true) + expect(replies(transport)).toEqual([]) dispose() }) - it('answers a subscribe whose withdrawal never arrives', async () => { - const { transport, emit, dispose } = await connectVisiblePane() + it('keeps a subscription whose withdrawal never arrives', async () => { + const { transport, deps, emit, dispose } = await connectVisiblePane() // A real TUI: subscribe now, unsubscribe minutes later on exit. emit('\x1b[?2031h') emit('painting the ui') - expect(replies(transport)).toHaveLength(1) + expect(deps.paneMode2031Ref.current.get(1)).toBe(true) + expect(replies(transport)).toEqual([]) dispose() }) @@ -13683,7 +13686,7 @@ describe('connectPanePty', () => { }) it('discards a half-read escape prefix when the pane swaps PTYs', async () => { - const { transport, emit, dispose } = await connectVisiblePane() + const { transport, deps, emit, dispose } = await connectVisiblePane() // Why: the tail is a byte range from the old stream. Splicing it onto the first // chunk of a replacement PTY fabricates a subscribe no program ever sent. transport.serializeBuffer = vi.fn().mockResolvedValue(null) @@ -13694,12 +13697,13 @@ describe('connectPanePty', () => { emit('31h') + expect(deps.paneMode2031Ref.current.get(1)).toBeUndefined() expect(replies(transport)).toEqual([]) dispose() }) }) - it('answers hidden Codex mode 2031 subscribes split across becoming visible', async () => { + it('records hidden Codex mode 2031 subscribes split across becoming visible', async () => { const { connectPanePty } = await import('./pty-connection') const transport = createMockTransport('pty-id') const capturedDataCallback: { current: ((data: string) => void) | null } = { current: null } @@ -13734,8 +13738,9 @@ describe('connectPanePty', () => { isVisibleRef.current = true capturedDataCallback.current?.('31h') - expect(transport.sendInputImmediate).toHaveBeenCalledWith('\x1b[?997;2n') + expect(transport.sendInputImmediate).not.toHaveBeenCalledWith(expect.stringMatching(/\?997/)) expect(paneMode2031Ref.current.get(1)).toBe(true) + // Seeded so the next appearance re-apply only pushes on a real color-mode flip. expect(paneLastThemeModeRef.current.get(1)).toBe('light') expect(pane.terminal.write).not.toHaveBeenCalledWith('31h', expect.any(Function)) @@ -14284,7 +14289,7 @@ describe('connectPanePty', () => { await flushAsyncTicks(20) expect(getMainBufferSnapshot).toHaveBeenCalled() - expect(transport.sendInput).toHaveBeenCalledWith('\x1b[?997;1n') + expect(transport.sendInput).not.toHaveBeenCalledWith(expect.stringMatching(/\?997/)) expect(pane.terminal.write).toHaveBeenCalledWith( expect.stringContaining('healed from snapshot'), expect.any(Function) @@ -14982,7 +14987,8 @@ describe('connectPanePty', () => { undefined, { connectionId: null } ) - expect(transport.sendInput).toHaveBeenCalledWith(expect.stringContaining('\x1b[')) + // Remote gated panes record the subscription from the fact and answer nothing (#9993). + expect(transport.sendInput).not.toHaveBeenCalledWith(expect.stringMatching(/\?997/)) expect(recordPaneMode2031Subscription).toHaveBeenCalledWith(1, expect.any(String)) deps.paneMode2031Ref.current.set(1, true) deps.paneLastThemeModeRef.current.set(1, 'dark') diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 8c32b1e1984..35d6f7abf6e 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -131,7 +131,6 @@ import { createShellReadyMarkerScanState, scanForShellReadyMarker } from './shel import { getSystemPrefersDark } from '@/lib/terminal-theme' import { INITIAL_MODE_2031_REPLY_SCAN_STATE, - mode2031SequenceFor, resolveTerminalColorSchemeMode, scanMode2031ReplyDecision } from '../../../../shared/terminal-color-scheme-protocol' @@ -529,11 +528,10 @@ const e2eTerminalHiddenSnapshotOverrides = new Map