From 8d1557c225f5b6e02cc45c43b0a80b3dd0051121 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 22 Aug 2026 23:00:23 -0700 Subject: [PATCH 01/14] fix(skills): discover Hermes home skills (#15862) Co-authored-by: kriptoburak --- src/main/skills/discovery.test.ts | 75 +++++++++++++++++++ src/main/skills/discovery.ts | 4 +- .../skill-discovery-concurrency.test.ts | 7 +- src/main/skills/skill-discovery-sources.ts | 14 +++- .../skill-discovery-wsl-plugins.test.ts | 9 ++- .../skill-provider-runtime-roots.test.ts | 67 +++++++++++++++++ .../skills/skill-provider-runtime-roots.ts | 43 +++++++++++ 7 files changed, 214 insertions(+), 5 deletions(-) diff --git a/src/main/skills/discovery.test.ts b/src/main/skills/discovery.test.ts index 3d905a01d84..740c4af7cd0 100644 --- a/src/main/skills/discovery.test.ts +++ b/src/main/skills/discovery.test.ts @@ -48,11 +48,16 @@ function makeRepo(path: string, connectionId: string | null = null): Repo { beforeEach(() => { // Roots are shared between scans for a few seconds; each case owns its own tree. clearSkillRootScanCache() + // Why: the Hermes root is the only home root that env vars can move outside the + // per-case tree, so a developer's real install must not leak into these scans. + vi.stubEnv('HERMES_HOME', '') + vi.stubEnv('LOCALAPPDATA', '') vi.spyOn(console, 'info').mockImplementation(() => undefined) }) afterEach(() => { unavailableRootPath = null + vi.unstubAllEnvs() vi.restoreAllMocks() }) @@ -367,6 +372,7 @@ describe('skill discovery', () => { '/home/test/.config/opencode/skills', '/home/test/.pi/agent/skills', '/home/test/.omp/agent/skills', + '/home/test/.hermes/skills', '/home/test/.gemini/skills', '/home/test/.gemini/antigravity/skills', '/home/test/.cursor/skills', @@ -393,6 +399,9 @@ describe('skill discovery', () => { expect( roots.find((root) => root.path.replace(/\\/g, '/') === '/home/test/.omp/agent/skills')?.owner ).toBe('omp') + expect( + roots.find((root) => root.path.replace(/\\/g, '/') === '/home/test/.hermes/skills')?.owner + ).toBe('hermes') }) it('does not add runtime-owned repository paths to local scan roots', () => { @@ -471,6 +480,72 @@ describe('skill discovery', () => { expect(skill?.providers).toEqual(['agent-skills']) }) + it('discovers Skills installed in the Hermes home', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-skills-')) + const home = join(root, 'home') + const skillDir = join(home, '.hermes', 'skills', 'social-media', 'research') + await mkdir(skillDir, { recursive: true }) + await writeFile( + join(skillDir, 'SKILL.md'), + ['---', 'name: social-research', 'description: Research social sources.', '---', ''].join( + '\n' + ) + ) + + const result = await discoverSkills({ + homeDir: home, + cwd: join(root, 'missing-cwd') + }) + + expect(result.skills).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + name: 'social-research', + sourceKind: 'home', + sourceLabel: 'Hermes home', + directoryPath: skillDir, + providers: ['agent-skills'] + }) + ]) + ) + expect(result.sources.find((source) => source.id === 'home-hermes')).toMatchObject({ + owner: 'hermes', + exists: true + }) + }) + + it('discovers Hermes Skills under a relocated HERMES_HOME profile', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-skills-')) + const home = join(root, 'home') + const hermesHome = join(root, 'profiles', 'coder') + const skillDir = join(hermesHome, 'skills', 'research') + await mkdir(skillDir, { recursive: true }) + await writeFile( + join(skillDir, 'SKILL.md'), + ['---', 'name: profile-research', 'description: Research sources.', '---', ''].join('\n') + ) + vi.stubEnv('HERMES_HOME', hermesHome) + + const result = await discoverSkills({ + homeDir: home, + cwd: join(root, 'missing-cwd') + }) + + expect(result.skills).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + name: 'profile-research', + sourceLabel: 'Hermes home', + directoryPath: skillDir + }) + ]) + ) + expect(result.sources.find((source) => source.id === 'home-hermes')).toMatchObject({ + path: join(hermesHome, 'skills'), + exists: true + }) + }) + it('discovers worktree .agents skill symlinks from the requested cwd', async () => { const root = await mkdtemp(join(tmpdir(), 'orca-skills-')) const home = join(root, 'home') diff --git a/src/main/skills/discovery.ts b/src/main/skills/discovery.ts index c5b8515c319..77e1e477ffd 100644 --- a/src/main/skills/discovery.ts +++ b/src/main/skills/discovery.ts @@ -34,11 +34,11 @@ const MAX_MARKDOWN_BYTES = 256 * 1024 // them for a few seconds is what bounds that fan-out. export const SKILL_ROOT_SCAN_TTL_MS = 10_000 // Why: sized off the root formula, not a round number. One scan builds -// `12 fixed home roots + 2 per local repo (+ cwd) + plugin roots`, so a bound +// `17 fixed home roots + 7 per local repo (+ cwd) + plugin roots`, so a bound // smaller than a single scan's root count makes that scan evict its own earlier // entries and the cache degrades to a ~0% hit rate — exactly the walk this // exists to prevent. The live key space is the union across targets — the fixed -// home roots plus two per repo plus two per distinct workspace cwd — so this holds +// home roots plus seven per repo plus seven per distinct workspace cwd — so this holds // a few hundred repos with panes open, not an unbounded install. Past that the LRU // keeps the hot home roots and the repo roots thrash, which degrades rather than // breaks. Most repo roots do not exist, and a missing root caches as diff --git a/src/main/skills/skill-discovery-concurrency.test.ts b/src/main/skills/skill-discovery-concurrency.test.ts index da7079331bd..a51d6dc9d52 100644 --- a/src/main/skills/skill-discovery-concurrency.test.ts +++ b/src/main/skills/skill-discovery-concurrency.test.ts @@ -60,11 +60,16 @@ function readdirCountUnder(path: string): number { beforeEach(() => { clearSkillRootScanCache() readdirPaths.length = 0 + // Why: the Hermes root is the one home root an env var can move outside the + // fixture, so a developer with a real Hermes install would score it `present`. + vi.stubEnv('HERMES_HOME', '') + vi.stubEnv('LOCALAPPDATA', '') vi.spyOn(console, 'info').mockImplementation(() => undefined) }) afterEach(() => { clearSkillRootScanCache() + vi.unstubAllEnvs() vi.restoreAllMocks() }) @@ -180,7 +185,7 @@ describe('bounded concurrent skill discovery', () => { const line = String(info.mock.calls.at(0)?.at(0)) // `present` is the signal that separates "big tree" from "big root set", and // is not derivable from the other counts. - expect(line).toContain('[skills] scan roots=23 present=3 walked=23 skills=3') + expect(line).toContain('[skills] scan roots=24 present=3 walked=24 skills=3') expect(line).toContain('home-claude') expect(line).not.toContain(home) expect(line).not.toContain(tmpdir()) diff --git a/src/main/skills/skill-discovery-sources.ts b/src/main/skills/skill-discovery-sources.ts index 91221a55413..9d9781c3687 100644 --- a/src/main/skills/skill-discovery-sources.ts +++ b/src/main/skills/skill-discovery-sources.ts @@ -11,7 +11,11 @@ import type { AgentType } from '../../shared/agent-status-types' import type { Repo } from '../../shared/repo-types' import { getRepoExecutionHostId, LOCAL_EXECUTION_HOST_ID } from '../../shared/execution-host' import type { SkillProviderRootOverrides } from './skill-provider-destinations' -import { resolveEnvironmentSkillProviderRoots } from './skill-provider-runtime-roots' +import { + resolveDefaultHermesSkillsRoot, + resolveEnvironmentHermesSkillsRoot, + resolveEnvironmentSkillProviderRoots +} from './skill-provider-runtime-roots' export type SkillScanRoot = Omit type SkillDiscoveryPathApi = Pick @@ -77,6 +81,13 @@ export function buildSkillDiscoverySources( const cwd = args.cwd ?? process.cwd() const providerRootOverrides = args.providerRootOverrides ?? (args.pathApi ? {} : resolveEnvironmentSkillProviderRoots()) + // Why: HERMES_HOME moves the whole profile tree, so the default home path + // finds nothing for `hermes -p `. Only this process's own host can + // read it — a custom pathApi means the home belongs to another host, whose + // Hermes install is POSIX-shaped even when this process runs on Windows. + const hermesSkillsRoot = args.pathApi + ? pathApi.join(home, '.hermes', 'skills') + : (resolveEnvironmentHermesSkillsRoot() ?? resolveDefaultHermesSkillsRoot({ homeDir: home })) const roots: SkillScanRoot[] = [ source( 'home-codex', @@ -144,6 +155,7 @@ export function buildSkillDiscoverySources( ['agent-skills'], 'omp' ), + source('home-hermes', 'Hermes home', hermesSkillsRoot, 'home', ['agent-skills'], 'hermes'), source( 'home-prime-agent', 'Prime Agent home', diff --git a/src/main/skills/skill-discovery-wsl-plugins.test.ts b/src/main/skills/skill-discovery-wsl-plugins.test.ts index d3b5913fc57..f3ee9024ff2 100644 --- a/src/main/skills/skill-discovery-wsl-plugins.test.ts +++ b/src/main/skills/skill-discovery-wsl-plugins.test.ts @@ -1,6 +1,6 @@ import type { WslResult } from '../wsl/wsl-runner' import { posix as pathPosix } from 'node:path' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' const runWslProcessMock = vi.hoisted(() => vi.fn()) @@ -19,10 +19,15 @@ function wslResult(stdout: string): WslResult { describe('WSL Claude plugin skill discovery', () => { beforeEach(() => runWslProcessMock.mockReset()) + afterEach(() => vi.unstubAllEnvs()) it('reads enabled plugin metadata and scans the selected install inside the distro', async () => { const homeDir = '/home/alice' const cwd = '/work/orca' + // Why: a Windows host's own Hermes location says nothing about the distro's, + // so neither variable may reach the posix scan script. + vi.stubEnv('HERMES_HOME', 'C:\\Users\\alice\\hermes') + vi.stubEnv('LOCALAPPDATA', 'C:\\Users\\alice\\AppData\\Local') const pluginId = 'compound-engineering@compound-engineering-plugin' const installPath = '/home/alice/.claude/plugins/cache/compound/3.14.3' const installed = JSON.stringify({ @@ -58,6 +63,8 @@ describe('WSL Claude plugin skill discovery', () => { expect(runWslProcessMock).toHaveBeenCalledTimes(2) const scanScript = runWslProcessMock.mock.calls[1]?.[0].script as string + expect(scanScript).toContain('/home/alice/.hermes/skills') + expect(scanScript).not.toContain('AppData') expect(scanScript).toContain(`${installPath}/skills`) expect(result.skills).toEqual([ expect.objectContaining({ diff --git a/src/main/skills/skill-provider-runtime-roots.test.ts b/src/main/skills/skill-provider-runtime-roots.test.ts index 07d9d422dac..f933d4b2ff0 100644 --- a/src/main/skills/skill-provider-runtime-roots.test.ts +++ b/src/main/skills/skill-provider-runtime-roots.test.ts @@ -1,6 +1,8 @@ import { join } from 'node:path' import { describe, expect, it } from 'vitest' import { + resolveDefaultHermesSkillsRoot, + resolveEnvironmentHermesSkillsRoot, resolveEnvironmentSkillProviderRoots, resolveWslGrokSkillProviderRoot, withClaudeSkillProviderRoot @@ -30,6 +32,71 @@ describe('skill provider runtime roots', () => { }) }) + it('maps a relocated HERMES_HOME to its skill root and ignores relative ones', () => { + expect(resolveEnvironmentHermesSkillsRoot({ HERMES_HOME: join('/srv', 'hermes') })).toBe( + join('/srv', 'hermes', 'skills') + ) + expect(resolveEnvironmentHermesSkillsRoot({ HERMES_HOME: '../hermes' })).toBeNull() + expect(resolveEnvironmentHermesSkillsRoot({})).toBeNull() + }) + + it('defaults the Hermes skills root to the home dotfolder off Windows', () => { + expect( + resolveDefaultHermesSkillsRoot({ + homeDir: join('/users', 'alice'), + platform: 'linux', + env: { LOCALAPPDATA: join('/local') }, + directoryExists: () => true + }) + ).toBe(join('/users', 'alice', '.hermes', 'skills')) + }) + + it('defaults the Hermes skills root under LOCALAPPDATA on Windows', () => { + const resolved = resolveDefaultHermesSkillsRoot({ + homeDir: join('/users', 'alice'), + platform: 'win32', + env: { LOCALAPPDATA: join('/local') }, + directoryExists: (candidate) => candidate === join('/local', 'hermes') + }) + expect(resolved).toBe(join('/local', 'hermes', 'skills')) + }) + + it('keeps a pre-LOCALAPPDATA Windows dotfolder install discoverable', () => { + const resolved = resolveDefaultHermesSkillsRoot({ + homeDir: join('/users', 'alice'), + platform: 'win32', + env: { LOCALAPPDATA: join('/local') }, + directoryExists: (candidate) => candidate === join('/users', 'alice', '.hermes') + }) + expect(resolved).toBe(join('/users', 'alice', '.hermes', 'skills')) + }) + + it('prefers the LOCALAPPDATA tree on Windows when both layouts exist', () => { + const resolved = resolveDefaultHermesSkillsRoot({ + homeDir: join('/users', 'alice'), + platform: 'win32', + env: { LOCALAPPDATA: join('/local') }, + directoryExists: () => true + }) + expect(resolved).toBe(join('/local', 'hermes', 'skills')) + }) + + it('falls back to the dotfolder when Windows exposes no usable LOCALAPPDATA', () => { + const seen: string[] = [] + const resolved = resolveDefaultHermesSkillsRoot({ + homeDir: join('/users', 'alice'), + platform: 'win32', + env: { LOCALAPPDATA: 'relative\\local' }, + directoryExists: (candidate) => { + seen.push(candidate) + return true + } + }) + expect(resolved).toBe(join('/users', 'alice', '.hermes', 'skills')) + // A rejected LOCALAPPDATA must not cost a stat: there is nothing to compare. + expect(seen).toEqual([]) + }) + it('maps the WSL login shell GROK_HOME to a host-readable skill root', async () => { await expect( resolveWslGrokSkillProviderRoot('Ubuntu-24.04', async () => '/srv/grok\n') diff --git a/src/main/skills/skill-provider-runtime-roots.ts b/src/main/skills/skill-provider-runtime-roots.ts index dd58e84d211..d4dec7bfba7 100644 --- a/src/main/skills/skill-provider-runtime-roots.ts +++ b/src/main/skills/skill-provider-runtime-roots.ts @@ -1,3 +1,4 @@ +import { statSync } from 'node:fs' import { isAbsolute, join, resolve } from 'node:path' import { toWindowsWslPath } from '../../shared/wsl-paths' import { runWslProcess } from '../wsl/wsl-runner' @@ -37,6 +38,48 @@ export function resolveEnvironmentSkillProviderRoots( } } +// Why: `HERMES_HOME` relocates a whole Hermes profile tree (`hermes -p coder`), +// and the rest of Orca already treats it as authoritative. Hermes is not an +// install provider, so it stays out of `SkillProviderRootOverrides`. +export function resolveEnvironmentHermesSkillsRoot( + env: NodeJS.ProcessEnv = process.env +): string | null { + const hermesHome = normalizedRoot(env.HERMES_HOME) + return hermesHome ? join(hermesHome, 'skills') : null +} + +function isExistingDirectory(candidate: string): boolean { + return statSync(candidate, { throwIfNoEntry: false })?.isDirectory() === true +} + +/** + * The Hermes home when `HERMES_HOME` is unset. Windows installs land in + * `%LOCALAPPDATA%\hermes`, not a dotfolder in the user profile, so the POSIX + * `~/.hermes` default discovers nothing there. The dotfolder still wins on + * Windows when it exists and the LOCALAPPDATA tree does not, which is the + * pre-LOCALAPPDATA install Hermes itself keeps honouring rather than orphaning. + */ +export function resolveDefaultHermesSkillsRoot(input: { + homeDir: string + env?: NodeJS.ProcessEnv + platform?: NodeJS.Platform + directoryExists?: (candidate: string) => boolean +}): string { + const dotfolderHome = join(input.homeDir, '.hermes') + if ((input.platform ?? process.platform) !== 'win32') { + return join(dotfolderHome, 'skills') + } + const localAppData = normalizedRoot((input.env ?? process.env).LOCALAPPDATA) + if (!localAppData) { + return join(dotfolderHome, 'skills') + } + const localAppDataHome = join(localAppData, 'hermes') + const directoryExists = input.directoryExists ?? isExistingDirectory + return !directoryExists(localAppDataHome) && directoryExists(dotfolderHome) + ? join(dotfolderHome, 'skills') + : join(localAppDataHome, 'skills') +} + export function withClaudeSkillProviderRoot( roots: SkillProviderRootOverrides, claudeConfigDirectory: string | null | undefined From af2e825626f620278deb1bfd42259cdb1bd93fb7 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 22 Aug 2026 23:00:27 -0700 Subject: [PATCH 02/14] fix(worktree): stop warning about a stale local base branch that does not exist yet (#15331) (#15871) Co-authored-by: vam --- .../worktree-add-local-base-refresh.test.ts | 116 +++++++++++++++++- src/main/git/worktree-base-ref-probe.ts | 29 +++++ src/main/git/worktree.ts | 12 +- src/main/ipc/worktree-remote.ts | 31 +++-- .../worktrees-ssh-local-base-refresh.test.ts | 114 +++++++++++++++++ 5 files changed, 290 insertions(+), 12 deletions(-) diff --git a/src/main/git/worktree-add-local-base-refresh.test.ts b/src/main/git/worktree-add-local-base-refresh.test.ts index bfe76b397a7..cec1151e0eb 100644 --- a/src/main/git/worktree-add-local-base-refresh.test.ts +++ b/src/main/git/worktree-add-local-base-refresh.test.ts @@ -347,13 +347,19 @@ describe('addWorktree', () => { it('skips updating the local branch when it has diverged', async () => { gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: 'abc123\n' }) // rev-parse refs/remotes/origin/main^{commit} gitExecFileAsyncMock.mockRejectedValueOnce(new Error('not a fast-forward')) + gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: 'refs/heads/main\n' }) // for-each-ref refs/heads/main (exists) gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '' }) // worktree add resolveCreationBaseConfigWrite() gitExecFileAsyncMock.mockRejectedValueOnce(Object.assign(new Error('key unset'), { code: 1 })) // config --get push.autoSetupRemote (unset) gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '' }) // config --local set push.autoSetupRemote - await addWorktree('/repo', '/repo-feature', 'feature/test', 'origin/main', true) + const result = await addWorktree('/repo', '/repo-feature', 'feature/test', 'origin/main', true) + expect(result.localBaseRefRefresh).toEqual({ + status: 'skipped_not_fast_forward', + baseRef: 'origin/main', + localBranch: 'main' + }) expect(gitExecFileAsyncMock.mock.calls).toEqual([ [ ['rev-parse', '--verify', '--quiet', 'refs/remotes/origin/main^{commit}'], @@ -363,6 +369,10 @@ describe('addWorktree', () => { ['rev-list', '--left-right', '--count', 'refs/heads/main...refs/remotes/origin/main'], expect.objectContaining({ cwd: '/repo' }) ], + [ + ['for-each-ref', '--count=1', '--format=%(refname)', 'refs/heads/main'], + expect.objectContaining({ cwd: '/repo' }) + ], [ [ 'worktree', @@ -396,12 +406,112 @@ describe('addWorktree', () => { ]) }) + // #15331: evaluation runs before `-b ` exists, so rev-list fails on the missing local ref. + it('does not warn when worktree add itself creates the local base branch', async () => { + gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'abc123\n' }) // rev-parse --verify --quiet refs/remotes/origin/feature-x^{commit} + .mockRejectedValueOnce( + new Error( + "fatal: ambiguous argument 'refs/heads/feature-x...refs/remotes/origin/feature-x': unknown revision or path not in the working tree." + ) + ) // rev-list: refs/heads/feature-x does not exist yet + .mockResolvedValueOnce({ stdout: '' }) // for-each-ref refs/heads/feature-x (missing) + .mockResolvedValueOnce({ stdout: '' }) // worktree add + .mockResolvedValueOnce({ stdout: '' }) // config --local --replace-all branch..base + .mockRejectedValueOnce(Object.assign(new Error('key unset'), { code: 1 })) // config --get push.autoSetupRemote (unset) + .mockResolvedValueOnce({ stdout: '' }) // config --local set push.autoSetupRemote + + const result = await addWorktree( + '/repo', + '/repo-feature-x', + 'feature-x', + 'origin/feature-x', + true + ) + + expect(result.localBaseRefRefresh).toBeUndefined() + expect(gitExecFileAsyncMock.mock.calls.map((call) => call[0])).toContainEqual([ + 'worktree', + 'add', + '--no-track', + '-b', + 'feature-x', + '/repo-feature-x', + 'refs/remotes/origin/feature-x' + ]) + // Nothing was refreshed, so no ref mutation. + expect(gitExecFileAsyncMock.mock.calls.map((call) => call[0][0])).not.toContain('update-ref') + expect(gitExecFileAsyncMock.mock.calls.map((call) => call[0][0])).not.toContain('reset') + }) + + // #15331: same missing-local-branch class, but the new branch name differs from the base's. + it('does not warn when the local base branch does not exist in a fetch-only clone', async () => { + gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'abc123\n' }) // rev-parse --verify --quiet refs/remotes/origin/main^{commit} + .mockRejectedValueOnce(new Error('unknown revision refs/heads/main')) // rev-list: no local main + .mockResolvedValueOnce({ stdout: '' }) // for-each-ref refs/heads/main (missing) + .mockResolvedValueOnce({ stdout: '' }) // worktree add + .mockResolvedValueOnce({ stdout: '' }) // config --local --replace-all branch..base + .mockRejectedValueOnce(Object.assign(new Error('key unset'), { code: 1 })) // config --get push.autoSetupRemote (unset) + .mockResolvedValueOnce({ stdout: '' }) // config --local set push.autoSetupRemote + + const result = await addWorktree('/repo', '/repo-feature', 'my-feature', 'origin/main', true) + + expect(result.localBaseRefRefresh).toBeUndefined() + expect(gitExecFileAsyncMock.mock.calls.map((call) => call[0])).toContainEqual([ + 'for-each-ref', + '--count=1', + '--format=%(refname)', + 'refs/heads/main' + ]) + }) + + // A failed probe is not proof of absence, so the warning must survive it. + it('keeps the warning when the local base ref probe itself fails', async () => { + gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'abc123\n' }) // rev-parse --verify --quiet refs/remotes/origin/main^{commit} + .mockRejectedValueOnce(new Error('rev-list failed')) // drift probe + .mockRejectedValueOnce(new Error('fatal: not a git repository')) // for-each-ref probe could not run + .mockResolvedValueOnce({ stdout: '' }) // worktree add + .mockResolvedValueOnce({ stdout: '' }) // config --local --replace-all branch..base + .mockRejectedValueOnce(Object.assign(new Error('key unset'), { code: 1 })) // config --get push.autoSetupRemote (unset) + .mockResolvedValueOnce({ stdout: '' }) // config --local set push.autoSetupRemote + + const result = await addWorktree('/repo', '/repo-feature', 'my-feature', 'origin/main', true) + + expect(result.localBaseRefRefresh).toEqual({ + status: 'skipped_not_fast_forward', + baseRef: 'origin/main', + localBranch: 'main' + }) + }) + + it('still suggests nothing but keeps the warning when the local base ref exists and diverged', async () => { + gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'abc123\n' }) // rev-parse --verify --quiet refs/remotes/origin/main^{commit} + .mockResolvedValueOnce({ stdout: '2\t3\n' }) // rev-list: 2 local-only commits + .mockResolvedValueOnce({ stdout: '' }) // worktree add + .mockResolvedValueOnce({ stdout: '' }) // config --local --replace-all branch..base + .mockRejectedValueOnce(Object.assign(new Error('key unset'), { code: 1 })) // config --get push.autoSetupRemote (unset) + .mockResolvedValueOnce({ stdout: '' }) // config --local set push.autoSetupRemote + + const result = await addWorktree('/repo', '/repo-feature', 'main', 'origin/main', true) + + // Same branch name as the base, but rev-list succeeded: real divergence must still warn. + expect(result.localBaseRefRefresh).toEqual({ + status: 'skipped_not_fast_forward', + baseRef: 'origin/main', + localBranch: 'main' + }) + }) + it('skips local base refresh when captured OIDs are no longer ancestor-safe', async () => { gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: 'abc123\n' }) // rev-parse refs/remotes/origin/main^{commit} gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '0\t2\n' }) // stale rev-list result gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: 'new-local\n' }) // rev-parse refs/heads/main^{commit} gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: 'remote-main\n' }) // rev-parse refs/remotes/origin/main^{commit} gitExecFileAsyncMock.mockRejectedValueOnce(new Error('not an ancestor')) // merge-base captured OIDs + gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: 'refs/heads/main\n' }) // for-each-ref refs/heads/main (exists) gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '' }) // worktree add resolveCreationBaseConfigWrite() gitExecFileAsyncMock.mockRejectedValueOnce(Object.assign(new Error('key unset'), { code: 1 })) // config --get push.autoSetupRemote (unset) @@ -435,6 +545,10 @@ describe('addWorktree', () => { ['merge-base', '--is-ancestor', 'new-local', 'remote-main'], expect.objectContaining({ cwd: '/repo' }) ], + [ + ['for-each-ref', '--count=1', '--format=%(refname)', 'refs/heads/main'], + expect.objectContaining({ cwd: '/repo' }) + ], [ [ 'worktree', diff --git a/src/main/git/worktree-base-ref-probe.ts b/src/main/git/worktree-base-ref-probe.ts index 44b8d4d85e9..85c8c8baa8f 100644 --- a/src/main/git/worktree-base-ref-probe.ts +++ b/src/main/git/worktree-base-ref-probe.ts @@ -37,3 +37,32 @@ export async function hasWorktreeBaseCommitRef( ): Promise { return (await resolveWorktreeBaseCommitOid(repoPath, qualifiedRef, options)) !== null } + +export type WorktreeBaseRefPresence = 'present' | 'absent' | 'unknown' + +/** + * Distinguish "the ref does not exist" from "the probe itself failed". + * + * Why for-each-ref: it exits 0 whether or not the pattern matches, so an empty result + * proves absence while a rejection still means the probe never ran (broken repo, dead + * SSH transport). `rev-parse --verify --quiet` exits 1 for both, and reading that as + * "absent" would silently drop warnings the caller must still surface. + * + * Executor-injected so the SSH path can route the same argv through the relay. + */ +export async function probeWorktreeBaseRefPresence( + runGit: (args: string[]) => Promise<{ stdout: string }>, + qualifiedRef: string +): Promise { + try { + const { stdout } = await runGit([ + 'for-each-ref', + '--count=1', + '--format=%(refname)', + qualifiedRef + ]) + return stdout.trim() === qualifiedRef ? 'present' : 'absent' + } catch { + return 'unknown' + } +} diff --git a/src/main/git/worktree.ts b/src/main/git/worktree.ts index 1f96523899e..fcd2a0067fa 100644 --- a/src/main/git/worktree.ts +++ b/src/main/git/worktree.ts @@ -33,7 +33,7 @@ import { import { withLocalGitCapabilityCacheForExecution } from './git-capability-state' import { gitExecFileAsync, translateWslOutputPaths } from './runner' import { resolveGitDir, runWithGitReadCacheInvalidation } from './status' -import { hasWorktreeBaseCommitRef } from './worktree-base-ref-probe' +import { hasWorktreeBaseCommitRef, probeWorktreeBaseRefPresence } from './worktree-base-ref-probe' export type AddWorktreeResult = { localBaseRefRefresh?: LocalBaseRefRefreshResult @@ -265,6 +265,16 @@ async function evaluateLocalBaseRefRefreshability( ) drift = parsedDrift } catch { + // Why (#15331): the probes above also fail when refs/heads/ is simply absent; a branch that + // does not exist yet cannot be stale, so report nothing instead of a bogus divergence warning. + // Only a proven absence suppresses: an unusable repo leaves the warning alone. + const presence = await probeWorktreeBaseRefPresence( + (args) => gitExecFileAsync(args, gitExecOptions(repoPath, options)), + parsed.fullRef + ) + if (presence === 'absent') { + return undefined + } return { refreshable: false, result: { ...resultBase, status: 'skipped_not_fast_forward' } } } diff --git a/src/main/ipc/worktree-remote.ts b/src/main/ipc/worktree-remote.ts index 72ef373758e..1d63cf97deb 100644 --- a/src/main/ipc/worktree-remote.ts +++ b/src/main/ipc/worktree-remote.ts @@ -35,6 +35,7 @@ import { } from '../git/repo' import { resolveLocalGitUsername, getSshGitUsername } from '../git/git-username' import { hasCommitObjectViaGitExec } from '../git/commit-object-ref' +import { probeWorktreeBaseRefPresence } from '../git/worktree-base-ref-probe' import { resolveWorktreeCreateBase } from '../worktree-create-base' import { resolveWorktreeAddBaseRef } from '../../shared/worktree/base-ref' import { getHostedReviewForBranch } from '../source-control/hosted-review' @@ -181,7 +182,8 @@ type RemoteLocalBaseRefRefreshability = } | { refreshable: false - result: LocalBaseRefRefreshResult + // undefined = nothing to refresh (no local branch yet), so the caller reports no status at all. + result: LocalBaseRefRefreshResult | undefined } function appendWorktreeCreateWarning(current: string | undefined, next: string): string { @@ -698,8 +700,7 @@ async function hasRemoteWorktreeBaseRef( repoPath: string, baseRef: string ): Promise { - const refExists = (qualifiedRef: string) => - hasRemoteTrackingRefSsh(provider, repoPath, qualifiedRef) + const refExists = (qualifiedRef: string) => hasCommitRefSsh(provider, repoPath, qualifiedRef) const resolvedBaseRef = await resolveWorktreeAddBaseRef(baseRef, refExists) if (resolvedBaseRef !== baseRef) { return true @@ -710,8 +711,8 @@ async function hasRemoteWorktreeBaseRef( return hasRemoteCommitObject(provider, repoPath, baseRef) } -// Why: hasRemoteCommitObject resolves only SHAs, not symbolic remote-tracking refs; detect those directly for the fetch-failed local fallback. -async function hasRemoteTrackingRefSsh( +// Why: hasRemoteCommitObject resolves only SHAs, not symbolic refs; resolve any qualified ref (remote-tracking or local head) directly. +async function hasCommitRefSsh( provider: SshGitProvider, repoPath: string, ref: string @@ -1262,7 +1263,7 @@ async function resolveRemoteWorktreeCreateBasePlan( baseBranchCandidate ) if (remoteTrackingBase) { - if (await hasRemoteTrackingRefSsh(provider, repo.path, remoteTrackingBase.ref)) { + if (await hasCommitRefSsh(provider, repo.path, remoteTrackingBase.ref)) { return true } return hasRemoteWorktreeBaseRef(provider, repo.path, baseBranchCandidate) @@ -1313,7 +1314,7 @@ export async function prefetchRemoteWorktreeCreateBase( } if (basePlan.remoteTrackingBase) { if ( - (await hasRemoteTrackingRefSsh(provider, repo.path, basePlan.remoteTrackingBase.ref)) || + (await hasCommitRefSsh(provider, repo.path, basePlan.remoteTrackingBase.ref)) || !(await hasRemoteWorktreeBaseRef(provider, repo.path, basePlan.baseBranch)) ) { await refreshRemoteTrackingBaseForWorktreeCreate(provider, repo, basePlan.remoteTrackingBase) @@ -1333,7 +1334,7 @@ async function refreshLocalBaseRefForRemoteWorktreeCreate( provider: SshGitProvider, repoPath: string, remoteTrackingBase: RemoteTrackingBase -): Promise { +): Promise { const evaluation = await evaluateRemoteLocalBaseRefRefreshability( provider, repoPath, @@ -1393,6 +1394,16 @@ async function evaluateRemoteLocalBaseRefRefreshability( } } } catch { + // Why (#15331): the probes above also fail when refs/heads/ is simply absent; the relay's + // `worktree add -b` is about to create it, so there is nothing stale to warn about. Only a proven + // absence suppresses: a dropped relay connection is not evidence the branch is missing. + const presence = await probeWorktreeBaseRefPresence( + (args) => provider.exec(args, repoPath), + fullRef + ) + if (presence === 'absent') { + return { refreshable: false, result: undefined } + } return { refreshable: false, result: { ...resultBase, status: 'skipped_not_fast_forward' } } } @@ -1550,7 +1561,7 @@ export async function createRemoteWorktree( let baseFallback: WorktreeCreateBaseFallback | undefined if (remoteTrackingBase) { - const hasRemoteTrackingBaseRef = await hasRemoteTrackingRefSsh( + const hasRemoteTrackingBaseRef = await hasCommitRefSsh( provider, repo.path, remoteTrackingBase.ref @@ -1700,7 +1711,7 @@ export async function createRemoteWorktree( await refreshRemoteTrackingBaseForWorktreeCreate(provider, repo, remoteTrackingBase) } catch { // Why: a refresh failure shouldn't block create if a usable (stale) local base ref exists; probe after registerRoot and hard-fail only when none does. - if (!(await hasRemoteTrackingRefSsh(provider, repo.path, remoteTrackingBase.ref))) { + if (!(await hasCommitRefSsh(provider, repo.path, remoteTrackingBase.ref))) { throw new Error( `Could not refresh base ref "${baseBranch}" from "${remoteTrackingBase.remote}". Check your network and try again.` ) diff --git a/src/main/ipc/worktrees-ssh-local-base-refresh.test.ts b/src/main/ipc/worktrees-ssh-local-base-refresh.test.ts index b739b482884..0b76025a979 100644 --- a/src/main/ipc/worktrees-ssh-local-base-refresh.test.ts +++ b/src/main/ipc/worktrees-ssh-local-base-refresh.test.ts @@ -477,4 +477,118 @@ describe('registerWorktreeHandlers', () => { }) expect(result.localBaseRefUpdateSuggestion).toBeUndefined() }) + // #15331: the pre-create merge-base probe fails when refs/heads/ does not exist yet. + const buildMissingLocalBaseSshCase = (presence: 'absent' | 'present' | 'probe-failed') => { + const repo = { + id: 'repo-ssh', + path: '/remote/repo', + displayName: 'ssh', + badgeColor: '#000', + addedAt: 0, + connectionId: 'conn-1', + worktreeBaseRef: 'origin/main' + } + const provider = { + exec: vi.fn().mockImplementation(async (args: string[]) => { + if (args[0] === 'remote') { + return { stdout: 'origin\n', stderr: '' } + } + if (args[0] === 'for-each-ref' && args.at(-1) === 'refs/heads/main') { + if (presence === 'probe-failed') { + throw new Error('ssh: connection closed by remote host') + } + return { stdout: presence === 'present' ? 'refs/heads/main\n' : '', stderr: '' } + } + if (args[0] === 'rev-parse') { + const ref = args.at(-1) ?? '' + if (ref.startsWith('refs/heads/main')) { + return { stdout: presence === 'present' ? 'local-main\n' : '', stderr: '' } + } + // Other refs/heads probes are the new-branch conflict check; it must stay unresolvable. + return { stdout: ref.startsWith('refs/heads/') ? '' : 'remote-main\n', stderr: '' } + } + if (args[0] === 'merge-base') { + throw new Error('fatal: Not a valid object name refs/heads/main') + } + return { stdout: '', stderr: '' } + }), + fetchRemoteTrackingRef: vi.fn().mockResolvedValue(undefined), + addWorktree: vi.fn().mockResolvedValue(undefined), + listWorktrees: vi.fn().mockResolvedValue([ + { + path: '/remote/repo-improve-dashboard', + head: 'abc123', + branch: 'refs/heads/improve-dashboard', + isBare: false, + isMainWorktree: false + } + ]), + worktreeIsClean: vi.fn().mockResolvedValue({ clean: true }), + refreshLocalBaseRefForWorktreeCreate: vi.fn().mockResolvedValue(undefined) + } + store.getSettings.mockReturnValue({ + branchPrefix: 'none', + nestWorkspaces: false, + refreshLocalBaseRefOnWorktreeCreate: true, + workspaceDir: '/workspace' + }) + store.getRepos.mockReturnValue([repo]) + store.getRepo.mockReturnValue(repo) + getSshGitProviderMock.mockReturnValue(provider) + getActiveMultiplexerMock.mockReturnValue({ + request: vi.fn().mockResolvedValue(undefined), + notify: vi.fn() + }) + store.setWorktreeMeta.mockImplementation((_worktreeId, meta) => meta) + return provider + } + + it('does not report an SSH local base refresh when the local base branch does not exist', async () => { + const provider = buildMissingLocalBaseSshCase('absent') + + const result = (await handlers['worktrees:create'](null, { + repoId: 'repo-ssh', + name: 'improve-dashboard' + })) as CreateWorktreeResult + + expect(provider.exec).toHaveBeenCalledWith( + ['for-each-ref', '--count=1', '--format=%(refname)', 'refs/heads/main'], + '/remote/repo' + ) + expect(result.localBaseRefRefresh).toBeUndefined() + expect(provider.refreshLocalBaseRefForWorktreeCreate).not.toHaveBeenCalled() + }) + + it('keeps the SSH not-fast-forward status when the local base branch exists and diverged', async () => { + const provider = buildMissingLocalBaseSshCase('present') + + const result = (await handlers['worktrees:create'](null, { + repoId: 'repo-ssh', + name: 'improve-dashboard' + })) as CreateWorktreeResult + + expect(result.localBaseRefRefresh).toEqual({ + status: 'skipped_not_fast_forward', + baseRef: 'origin/main', + localBranch: 'main' + }) + expect(provider.refreshLocalBaseRefForWorktreeCreate).not.toHaveBeenCalled() + }) + + // Losing the relay mid-probe is not evidence the branch is missing. + it('keeps the SSH not-fast-forward status when the local base ref probe fails', async () => { + const provider = buildMissingLocalBaseSshCase('probe-failed') + + const result = (await handlers['worktrees:create'](null, { + repoId: 'repo-ssh', + name: 'improve-dashboard' + })) as CreateWorktreeResult + + expect(result.localBaseRefRefresh).toEqual({ + status: 'skipped_not_fast_forward', + baseRef: 'origin/main', + localBranch: 'main' + }) + expect(provider.refreshLocalBaseRefForWorktreeCreate).not.toHaveBeenCalled() + }) }) From 202d74a8a4be3fe6537e1b4eac0e9e4901cf7d96 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 22 Aug 2026 23:00:31 -0700 Subject: [PATCH 03/14] fix(git): enable Windows long paths for worktree creation (local, sparse, and SSH hosts) (#15866) Co-authored-by: hwantage --- src/main/git/add-sparse-worktree.test.ts | 43 +++++- .../git/worktree-add-creation-config.test.ts | 91 +++++++++++- src/main/git/worktree.ts | 14 +- src/relay/git-handler-worktree-ops.test.ts | 134 ++++++++++++++---- src/relay/git-handler-worktree-ops.ts | 17 ++- src/shared/windows-long-path-git-args.test.ts | 29 ++++ src/shared/windows-long-path-git-args.ts | 25 ++++ 7 files changed, 320 insertions(+), 33 deletions(-) create mode 100644 src/shared/windows-long-path-git-args.test.ts create mode 100644 src/shared/windows-long-path-git-args.ts diff --git a/src/main/git/add-sparse-worktree.test.ts b/src/main/git/add-sparse-worktree.test.ts index 84aef686719..2541b822438 100644 --- a/src/main/git/add-sparse-worktree.test.ts +++ b/src/main/git/add-sparse-worktree.test.ts @@ -1,5 +1,5 @@ import type * as FsPromises from 'node:fs/promises' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi, type MockInstance } from 'vitest' const { gitExecFileAsyncMock, @@ -67,6 +67,10 @@ beforeEach(() => { }) describe('addSparseWorktree', () => { + // Why: argv now depends on the host OS, so pin a non-Windows default or the exact-argv + // assertions below would fail for a maintainer running vitest on Windows. + let platformSpy: MockInstance<() => NodeJS.Platform> + beforeEach(() => { resetWorktreeGitMocks({ gitExecFileAsyncMock, @@ -75,6 +79,43 @@ describe('addSparseWorktree', () => { statMock, resolveGitDirMock }) + platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin') + }) + + afterEach(() => { + platformSpy.mockRestore() + }) + + it('enables long paths on the Windows commands that actually write the deep checkout', async () => { + // Why: `worktree add --no-checkout` writes nothing, so the long-path flag on it alone + // left sparse creation failing with "Filename too long" (issue #15785). + platformSpy.mockReturnValue('win32') + gitExecFileAsyncMock.mockResolvedValue({ stdout: '', stderr: '' }) + + await addSparseWorktree('C:\\repo', 'C:\\repo-feature', 'feature/test', ['packages/web']) + + const calls = getGitCalls() + expect(calls).toEqual( + expect.arrayContaining([ + 'git -c core.longpaths=true sparse-checkout set -- packages/web', + 'git -c core.longpaths=true checkout feature/test' + ]) + ) + }) + + it('omits the long-path option on non-Windows hosts', async () => { + gitExecFileAsyncMock.mockResolvedValue({ stdout: '', stderr: '' }) + + await addSparseWorktree('/repo', '/repo-feature', 'feature/test', ['packages/web']) + + const calls = getGitCalls() + expect(calls).toEqual( + expect.arrayContaining([ + 'git sparse-checkout set -- packages/web', + 'git checkout feature/test' + ]) + ) + expect(calls.some((call) => call.includes('core.longpaths'))).toBe(false) }) it('separates sparse checkout directory operands from options', async () => { diff --git a/src/main/git/worktree-add-creation-config.test.ts b/src/main/git/worktree-add-creation-config.test.ts index f6b3d0d51d6..44394b7728d 100644 --- a/src/main/git/worktree-add-creation-config.test.ts +++ b/src/main/git/worktree-add-creation-config.test.ts @@ -1,5 +1,5 @@ // addWorktree: checkout creation, branch-base/push.autoSetupRemote config writes, ref qualification. -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi, type MockInstance } from 'vitest' const { gitExecFileAsyncMock, @@ -40,10 +40,19 @@ describe('addWorktree', () => { gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '' }) // config --local --replace-all branch..base } + // Why: argv now depends on the host OS, so pin a non-Windows default or every + // exact-argv assertion below would fail for a maintainer running vitest on Windows. + let platformSpy: MockInstance<() => NodeJS.Platform> + beforeEach(() => { gitExecFileAsyncMock.mockReset() gitExecFileSyncMock.mockReset() translateWslOutputPathsMock.mockClear() + platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin') + }) + + afterEach(() => { + platformSpy.mockRestore() }) it('creates the worktree without touching the local base ref by default', async () => { @@ -99,6 +108,86 @@ describe('addWorktree', () => { ]) }) + it('enables long paths for native Windows worktree creation', async () => { + platformSpy.mockReturnValue('win32') + gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '' }) // worktree add + + await addWorktree( + 'C:\\repo', + 'C:\\repo-feature', + 'feature/test', + 'feature/test', + false, + false, + { checkoutExistingBranch: true } + ) + + expect(gitExecFileAsyncMock).toHaveBeenCalledWith( + ['-c', 'core.longpaths=true', 'worktree', 'add', 'C:\\repo-feature', 'feature/test'], + { cwd: 'C:\\repo', timeout: WORKTREE_ADD_TIMEOUT_MS } + ) + }) + + it('still enables long paths for a Windows-path repo that has a WSL distro configured', async () => { + // Why: a C:\ cwd can be served by host git.exe even with wslDistro set, and that + // is exactly the MAX_PATH-prone case; Linux git parses and ignores the key. + platformSpy.mockReturnValue('win32') + gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '' }) // worktree add + + await addWorktree( + 'C:\\repo', + 'C:\\repo-feature', + 'feature/test', + 'feature/test', + false, + false, + { checkoutExistingBranch: true, wslDistro: 'Ubuntu' } + ) + + expect(gitExecFileAsyncMock).toHaveBeenCalledWith( + ['-c', 'core.longpaths=true', 'worktree', 'add', 'C:\\repo-feature', 'feature/test'], + { cwd: 'C:\\repo', wslDistro: 'Ubuntu', timeout: WORKTREE_ADD_TIMEOUT_MS } + ) + }) + + it('does not pass the Windows-only long-path option for a WSL UNC repo path', async () => { + platformSpy.mockReturnValue('win32') + gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '' }) // worktree add + + const repoPath = '\\\\wsl.localhost\\Ubuntu\\home\\dev\\repo' + await addWorktree( + repoPath, + '\\\\wsl.localhost\\Ubuntu\\home\\dev\\repo-feature', + 'feature/test', + 'feature/test', + false, + false, + { checkoutExistingBranch: true, wslDistro: 'Ubuntu' } + ) + + expect(gitExecFileAsyncMock).toHaveBeenCalledWith( + ['worktree', 'add', '\\\\wsl.localhost\\Ubuntu\\home\\dev\\repo-feature', 'feature/test'], + { cwd: repoPath, wslDistro: 'Ubuntu', timeout: WORKTREE_ADD_TIMEOUT_MS } + ) + }) + + it.each(['darwin', 'linux'] as const)( + 'does not pass the long-path option on %s', + async (platform) => { + platformSpy.mockReturnValue(platform) + gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '' }) // worktree add + + await addWorktree('/repo', '/repo-feature', 'feature/test', 'feature/test', false, false, { + checkoutExistingBranch: true + }) + + expect(gitExecFileAsyncMock).toHaveBeenCalledWith( + ['worktree', 'add', '/repo-feature', 'feature/test'], + { cwd: '/repo', timeout: WORKTREE_ADD_TIMEOUT_MS } + ) + } + ) + it('bounds the worktree add call with a positive timeout (STA-1292 OneDrive stall guard)', async () => { // Why: without a timeout, a OneDrive cloud-placeholder checkout can stall // `git worktree add` for minutes. Assert the runner receives a non-zero diff --git a/src/main/git/worktree.ts b/src/main/git/worktree.ts index fcd2a0067fa..55c5d33cf79 100644 --- a/src/main/git/worktree.ts +++ b/src/main/git/worktree.ts @@ -14,6 +14,7 @@ import { scheduleWorktreeTrashDeletion } from '../worktree-trash' import { parseWslPath } from '../wsl' +import { windowsLongPathGitArgs } from '../../shared/windows-long-path-git-args' import type { LocalBaseRefRefreshResult, LocalBaseRefUpdateSuggestion @@ -985,7 +986,8 @@ async function performAddWorktree( ): Promise { let localBaseRefRefresh: LocalBaseRefRefreshResult | undefined let localBaseRefUpdateSuggestion: LocalBaseRefUpdateSuggestion | undefined - const args = ['worktree', 'add'] + // Why: enable long paths for this Windows checkout without changing user Git config. + const args = [...windowsLongPathGitArgs(repoPath), 'worktree', 'add'] let effectiveBase: string | undefined if (noCheckout) { args.push('--no-checkout') @@ -1091,15 +1093,21 @@ export async function addSparseWorktree( options ) created = true + // Why: `worktree add --no-checkout` writes no files, so these are the calls that + // actually materialize the deep path and need the long-path escape hatch. + const longPathArgs = windowsLongPathGitArgs(worktreePath) await gitExecFileAsync( ['sparse-checkout', 'init', '--cone'], gitExecOptions(worktreePath, options) ) await gitExecFileAsync( - ['sparse-checkout', 'set', '--', ...directories], + [...longPathArgs, 'sparse-checkout', 'set', '--', ...directories], + gitExecOptions(worktreePath, options) + ) + await gitExecFileAsync( + [...longPathArgs, 'checkout', branch], gitExecOptions(worktreePath, options) ) - await gitExecFileAsync(['checkout', branch], gitExecOptions(worktreePath, options)) return addResult } catch (error) { const wrapped: SparseWorktreeCreateError = diff --git a/src/relay/git-handler-worktree-ops.test.ts b/src/relay/git-handler-worktree-ops.test.ts index cedcf8b797c..0e7dfd282fc 100644 --- a/src/relay/git-handler-worktree-ops.test.ts +++ b/src/relay/git-handler-worktree-ops.test.ts @@ -31,12 +31,16 @@ describe('addWorktreeOp', () => { it('writes durable branch base config after creating an SSH new-branch worktree', async () => { const git = vi.fn(async () => ({ stdout: '', stderr: '' })) - await addWorktreeOp(git, { - repoPath: '/repo', - branchName: 'feature/test', - targetDir: '/repo-feature', - base: 'origin/main' - }) + await addWorktreeOp( + git, + { + repoPath: '/repo', + branchName: 'feature/test', + targetDir: '/repo-feature', + base: 'origin/main' + }, + 'linux' + ) expect(git.mock.calls.map((call) => call[0])).toEqual([ ['rev-parse', '--verify', '--quiet', 'refs/remotes/origin/main^{commit}'], @@ -75,13 +79,17 @@ describe('addWorktreeOp', () => { it('does not write branch base config when checking out an existing SSH branch', async () => { const git = vi.fn(async () => ({ stdout: '', stderr: '' })) - await addWorktreeOp(git, { - repoPath: '/repo', - branchName: 'feature/test', - targetDir: '/repo-feature', - base: 'origin/main', - checkoutExistingBranch: true - }) + await addWorktreeOp( + git, + { + repoPath: '/repo', + branchName: 'feature/test', + targetDir: '/repo-feature', + base: 'origin/main', + checkoutExistingBranch: true + }, + 'linux' + ) expect(git.mock.calls.map((call) => call[0])).toEqual([ ['worktree', 'add', '/repo-feature', 'feature/test'] @@ -91,11 +99,15 @@ describe('addWorktreeOp', () => { it('does not write branch base config when SSH creation has no base', async () => { const git = vi.fn(async () => ({ stdout: '', stderr: '' })) - await addWorktreeOp(git, { - repoPath: '/repo', - branchName: 'feature/no-base', - targetDir: '/repo-feature' - }) + await addWorktreeOp( + git, + { + repoPath: '/repo', + branchName: 'feature/no-base', + targetDir: '/repo-feature' + }, + 'linux' + ) expect(git.mock.calls.map((call) => call[0])).toEqual([ ['worktree', 'add', '--no-track', '-b', 'feature/no-base', '/repo-feature'], @@ -103,6 +115,76 @@ describe('addWorktreeOp', () => { ]) }) + it('enables long paths when the SSH execution host is Windows', async () => { + // Why: only the host's OS matters — a macOS client can drive a Windows SSH host, + // which hits the same MAX_PATH ceiling (issue #15785). + const git = vi.fn(async () => ({ stdout: '', stderr: '' })) + + await addWorktreeOp( + git, + { + repoPath: 'C:\\repo', + branchName: 'feature/test', + targetDir: 'C:\\repo-feature', + checkoutExistingBranch: true + }, + 'win32' + ) + + expect(git.mock.calls.map((call) => call[0])).toEqual([ + ['-c', 'core.longpaths=true', 'worktree', 'add', 'C:\\repo-feature', 'feature/test'] + ]) + }) + + it('keeps --no-checkout ahead of -b once the long-path prefix is present', async () => { + const git = vi.fn(async () => ({ stdout: '', stderr: '' })) + + await addWorktreeOp( + git, + { + repoPath: 'C:\\repo', + branchName: 'feature/test', + targetDir: 'C:\\repo-feature', + noCheckout: true + }, + 'win32' + ) + + expect(git.mock.calls[0][0]).toEqual([ + '-c', + 'core.longpaths=true', + 'worktree', + 'add', + '--no-track', + '--no-checkout', + '-b', + 'feature/test', + 'C:\\repo-feature' + ]) + }) + + it('omits the long-path option on a WSL UNC target on a Windows SSH host', async () => { + const git = vi.fn(async () => ({ stdout: '', stderr: '' })) + + await addWorktreeOp( + git, + { + repoPath: '\\\\wsl.localhost\\Ubuntu\\home\\dev\\repo', + branchName: 'feature/test', + targetDir: '\\\\wsl.localhost\\Ubuntu\\home\\dev\\repo-feature', + checkoutExistingBranch: true + }, + 'win32' + ) + + expect(git.mock.calls[0][0]).toEqual([ + 'worktree', + 'add', + '\\\\wsl.localhost\\Ubuntu\\home\\dev\\repo-feature', + 'feature/test' + ]) + }) + it('warns and unsets stale branch base config when SSH base persistence fails', async () => { const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) const git = vi.fn(async (args) => { @@ -113,12 +195,16 @@ describe('addWorktreeOp', () => { }) await expect( - addWorktreeOp(git, { - repoPath: '/repo', - branchName: 'feature/test', - targetDir: '/repo-feature', - base: 'origin/main' - }) + addWorktreeOp( + git, + { + repoPath: '/repo', + branchName: 'feature/test', + targetDir: '/repo-feature', + base: 'origin/main' + }, + 'linux' + ) ).resolves.toBeUndefined() expect(warnSpy).toHaveBeenCalledWith( diff --git a/src/relay/git-handler-worktree-ops.ts b/src/relay/git-handler-worktree-ops.ts index 0cdb0761b47..4095851a82f 100644 --- a/src/relay/git-handler-worktree-ops.ts +++ b/src/relay/git-handler-worktree-ops.ts @@ -1,5 +1,6 @@ import * as path from 'node:path' import { resolveWorktreeAddBaseRef } from '../shared/worktree/base-ref' +import { windowsLongPathGitArgs } from '../shared/windows-long-path-git-args' import type { GitExec } from './git-handler-ops' export { removeWorktreeOp } from './git-handler-worktree-remove' export { readRelayWorktreeList } from './git-handler-worktree-list' @@ -28,7 +29,12 @@ async function persistRelayWorktreeCreationBase( } } -export async function addWorktreeOp(git: GitExec, params: Record): Promise { +export async function addWorktreeOp( + git: GitExec, + params: Record, + // Why: only the execution host's OS matters here — the client may be macOS while the SSH host is Windows. + platform: NodeJS.Platform = process.platform +): Promise { const repoPath = params.repoPath as string const branchName = params.branchName as string const targetDir = params.targetDir as string @@ -62,11 +68,14 @@ export async function addWorktreeOp(git: GitExec, params: Record { + it('enables long paths for a Windows drive path', () => { + expect(windowsLongPathGitArgs('C:\\Users\\dev\\repo', 'win32')).toEqual([ + '-c', + 'core.longpaths=true' + ]) + }) + + it.each(['darwin', 'linux'] as const)('returns nothing on %s', (platform) => { + expect(windowsLongPathGitArgs('/home/dev/repo', platform)).toEqual([]) + }) + + it.each(['\\\\wsl.localhost\\Ubuntu\\home\\dev\\repo', '\\\\wsl$\\Ubuntu\\home\\dev\\repo'])( + 'returns nothing for the WSL UNC path %s', + (cwd) => { + expect(windowsLongPathGitArgs(cwd, 'win32')).toEqual([]) + } + ) + + it('never mutates the shared constant', () => { + const first = windowsLongPathGitArgs('C:\\repo', 'win32') + first.push('--bogus') + expect(windowsLongPathGitArgs('C:\\repo', 'win32')).toEqual(['-c', 'core.longpaths=true']) + }) +}) diff --git a/src/shared/windows-long-path-git-args.ts b/src/shared/windows-long-path-git-args.ts new file mode 100644 index 00000000000..c22393fcaa0 --- /dev/null +++ b/src/shared/windows-long-path-git-args.ts @@ -0,0 +1,25 @@ +import { parseWslUncPath } from './wsl-paths' + +const WINDOWS_LONG_PATH_GIT_ARGS = ['-c', 'core.longpaths=true'] as const + +/** + * Global `git -c` options that let a Windows checkout exceed MAX_PATH. + * + * Why command scope: Git for Windows aborts deep checkouts with "Filename too + * long" unless core.longpaths is on, and `-c` applies it to this invocation + * only — never `--global`, `--system`, or `--local`, so no user config is + * written. Available since Git 1.9, well under the 2.25 baseline. + * + * Why keyed off cwd rather than a wslDistro option: a `C:\...` cwd can still be + * served by host git.exe even when a distro is configured, and Linux git parses + * and ignores the key, so only a true `\\wsl.localhost\...` path opts out. + */ +export function windowsLongPathGitArgs( + cwd: string, + platform: NodeJS.Platform = process.platform +): string[] { + if (platform !== 'win32' || parseWslUncPath(cwd)) { + return [] + } + return [...WINDOWS_LONG_PATH_GIT_ARGS] +} From 1375c57b16758916c103a4c931872f583f788858 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 22 Aug 2026 23:56:01 -0700 Subject: [PATCH 04/14] fix(secrets): report the protection gap on change, not on every launch (#16044) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gap warning fired every startup with no way to stop it. It usually needs a keyring installed and unlocked to fix, so repeating it every launch is nagging the user cannot act on and will learn to ignore. It now reports when the answer changes: once when the gap starts being true, again if it becomes true for a different reason, and once when it is fixed — because silence after a "your secrets are not protected" warning would leave the user assuming that is still the case. State lives beside the profile data file, which is why the call moved out of the port bootstrap: that state has nowhere to live until the profile exists. A corrupt state file re-reports rather than trusting it, and a failed write logs instead of failing startup, since re-reporting next launch is the safe direction. ORCA_ALWAYS_REPORT_SECRET_PROTECTION=1 forces a re-report for support without disturbing the stored state. --- .../host/secret-protection-report.test.ts | 105 ++++++++++++++---- src/main/host/secret-protection-report.ts | 70 ++++++++++-- src/main/index.ts | 9 +- 3 files changed, 150 insertions(+), 34 deletions(-) diff --git a/src/main/host/secret-protection-report.test.ts b/src/main/host/secret-protection-report.test.ts index 2288bf11806..d5aefe664c4 100644 --- a/src/main/host/secret-protection-report.test.ts +++ b/src/main/host/secret-protection-report.test.ts @@ -1,38 +1,97 @@ -import { describe, expect, it } from 'vitest' -import { setSecretStore, _resetSecretStoreForTests } from '../../shared/secret-store' +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { _resetSecretStoreForTests, setSecretStore } from '../../shared/secret-store' import { reportSecretProtectionGap } from './secret-protection-report' -function installStore(gap: string | null): void { - setSecretStore({ - isEncryptionAvailable: () => true, - encryptString: (plainText) => Buffer.from(plainText), - decryptString: (cipher) => cipher.toString(), - describeProtectionGap: () => gap - }) -} +const WEAK = 'Secrets are obfuscated with a built-in key.' describe('reportSecretProtectionGap', () => { - it('logs and returns the gap when protection is weaker than a user would assume', () => { - installStore('Secrets are obfuscated with a built-in key.') - const logged: string[] = [] - expect(reportSecretProtectionGap((m) => logged.push(m))).toMatch(/built-in key/) - expect(logged).toHaveLength(1) - expect(logged[0]).toContain('[secrets]') + let dir: string + let dataFile: string + let logged: string[] + + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'orca-secret-report-')) + dataFile = join(dir, 'orca-data.json') + logged = [] }) - it('stays silent when secrets are properly sealed', () => { + afterEach(() => { + rmSync(dir, { recursive: true, force: true }) + }) + + function installStore(gap: string | null): void { + setSecretStore({ + isEncryptionAvailable: () => true, + encryptString: (plainText) => Buffer.from(plainText), + decryptString: (cipher) => cipher.toString(), + describeProtectionGap: () => gap + }) + } + + const report = (force = false): string | null => + reportSecretProtectionGap({ dataFile, log: (m) => void logged.push(m), force }) + + it('reports a new gap once, then stays quiet on every later launch', () => { + // Why this is the point: the gap usually needs a keyring install to fix, so + // repeating it every start is nagging the user cannot act on. + installStore(WEAK) + expect(report()).toBe(WEAK) + expect(logged).toHaveLength(1) + + report() + report() + expect(logged).toHaveLength(1) + }) + + it('reports again when the gap changes to a different reason', () => { + installStore(WEAK) + report() + installStore('The OS keyring is unavailable.') + report() + expect(logged).toHaveLength(2) + expect(logged[1]).toContain('keyring is unavailable') + }) + + it('announces recovery, so an earlier warning is not left standing', () => { + installStore(WEAK) + report() installStore(null) - const logged: string[] = [] - expect(reportSecretProtectionGap((m) => logged.push(m))).toBeNull() + report() + expect(logged).toHaveLength(2) + expect(logged[1]).toMatch(/now provided by the OS keyring/) + + report() + expect(logged).toHaveLength(2) + }) + + it('stays silent from a clean start when secrets are properly sealed', () => { + installStore(null) + expect(report()).toBeNull() expect(logged).toEqual([]) }) + it('re-reports on demand for support, without disturbing stored state', () => { + installStore(WEAK) + report() + expect(report(true)).toBe(WEAK) + expect(logged).toHaveLength(2) + report() + expect(logged).toHaveLength(2) + }) + + it('re-reports when the stored state is corrupt rather than trusting it', () => { + installStore(WEAK) + writeFileSync(join(dir, 'orca-secret-protection.json'), '{ not json', 'utf-8') + expect(report()).toBe(WEAK) + expect(logged).toHaveLength(1) + }) + it('does not throw startup when no store is installed', () => { - // Why: this is diagnostics running early in bootstrap. The useful error is the one - // raised by the first real read, not a crash from the thing reporting on it. _resetSecretStoreForTests() - const logged: string[] = [] - expect(() => reportSecretProtectionGap((m) => logged.push(m))).not.toThrow() + expect(() => report()).not.toThrow() expect(logged[0]).toContain('could not determine at-rest protection') }) }) diff --git a/src/main/host/secret-protection-report.ts b/src/main/host/secret-protection-report.ts index 511b2a4425c..8bdc59b57b3 100644 --- a/src/main/host/secret-protection-report.ts +++ b/src/main/host/secret-protection-report.ts @@ -1,19 +1,53 @@ +import { dirname, join } from 'node:path' +import { readFileSync, writeFileSync } from 'node:fs' import { getSecretStore } from '../../shared/secret-store' /** - * Report at-rest secret protection once at startup. + * Report at-rest secret protection, but only when the answer changes. * * Why this exists: the port has always been able to describe a protection gap, and * nothing read it — so a Linux user whose secrets are obfuscated with a built-in key - * was told nothing at all. Reading it here is the difference between the port - * documenting a promise and keeping one. + * was told nothing at all. + * + * Why not every launch: the gap is usually not something the user can fix right now + * (it needs a keyring installed and unlocked), so repeating it every start is nagging + * they cannot act on and will learn to ignore. Reporting on change means they hear it + * when it starts being true, hear it again if it becomes true for a different reason, + * and are told once when it is fixed. * * Deliberately not fatal and not a dialog: sealing still works, so blocking startup * would be worse than the gap it reports. */ -export function reportSecretProtectionGap( - log: (message: string) => void = (message) => console.warn(message) -): string | null { + +type ReportOptions = { + /** Profile data file; the report state lives beside it. */ + dataFile: string + log?: (message: string) => void + /** Escape hatch for support: re-report even if unchanged. */ + force?: boolean +} + +type ReportState = { lastReportedGap: string | null } + +function stateFile(dataFile: string): string { + return join(dirname(dataFile), 'orca-secret-protection.json') +} + +function readState(path: string): ReportState | null { + try { + const parsed = JSON.parse(readFileSync(path, 'utf-8')) as unknown + if (parsed && typeof parsed === 'object' && 'lastReportedGap' in parsed) { + const value = (parsed as ReportState).lastReportedGap + return { lastReportedGap: typeof value === 'string' ? value : null } + } + } catch { + // Missing or corrupt: treat as never reported and report again. + } + return null +} + +export function reportSecretProtectionGap(options: ReportOptions): string | null { + const log = options.log ?? ((message: string) => console.warn(message)) let gap: string | null try { gap = getSecretStore().describeProtectionGap() @@ -23,8 +57,28 @@ export function reportSecretProtectionGap( log(`[secrets] could not determine at-rest protection: ${String(error)}`) return null } - if (gap) { - log(`[secrets] ${gap}`) + + const path = stateFile(options.dataFile) + const previous = readState(path) + const changed = previous === null || previous.lastReportedGap !== gap + + if (changed || options.force) { + if (gap) { + log(`[secrets] ${gap}`) + } else if (previous?.lastReportedGap) { + // Why announce the recovery: the previous warning said secrets were not protected, + // so silence would leave the user assuming that is still true. + log('[secrets] At-rest protection is now provided by the OS keyring.') + } + } + + if (changed) { + try { + writeFileSync(path, JSON.stringify({ lastReportedGap: gap }), 'utf-8') + } catch (error) { + // Best effort: re-reporting next launch is strictly better than failing startup. + log(`[secrets] could not persist the protection report state: ${String(error)}`) + } } return gap } diff --git a/src/main/index.ts b/src/main/index.ts index 5686dd41e33..ae8fed9981d 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -884,9 +884,6 @@ if (hasSingleInstanceLock) { // the app.setName ordering the userData captures below depend on. setAppEnvironment(new ElectronAppEnvironment()) setSecretStore(new ElectronSecretStore()) - // Why right after install: this is the first moment the answer is knowable, and a - // silent weak backend is the gap users most need told about. - reportSecretProtectionGap() // Why at process level, not per-window: pty.ts registers against injected surfaces so // it can load without electron, and an Electron main process always has ipcMain — // whether a window exists is irrelevant. Installing this in attachMainWindowServices @@ -2310,6 +2307,12 @@ void app.whenReady().then(async () => { const activeOrcaProfile = ensureActiveOrcaProfile() store = new Store({ dataFile: activeOrcaProfile.dataFile }) + // Why here and not at install time: the report remembers what it last said, and that + // state lives beside the profile data file, which does not exist until now. + reportSecretProtectionGap({ + dataFile: activeOrcaProfile.dataFile, + force: process.env.ORCA_ALWAYS_REPORT_SECRET_PROTECTION === '1' + }) // Why here: the host key store is a sidecar of the same profile, and every SSH connect consults // it. Left unbound it reports nothing trusted, which is safe but silently discards our own // accept records on every launch. From 7a739c6bf57d83f7880e6cb66bd332e9a69dbdbb Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 23 Aug 2026 00:38:28 -0700 Subject: [PATCH 05/14] fix(secrets): probe the Linux-only storage backend defensively (#16046) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `safeStorage.getSelectedStorageBackend` is `@platform linux`, so it is genuinely undefined on macOS and Windows — confirmed against the installed Electron 43, where it reads `undefined` on darwin and `function` on Linux. The shipped code called it behind a `process.platform === 'linux'` check, so it never threw, but the guard was the only thing standing between that call and a startup TypeError. The platform check now lives with the probe, alongside a typeof check and a try/catch, and an unreadable or unknown backend reports no gap — claiming one we cannot prove would be its own kind of lie. The gap this closes is in the tests, not just the code: every suite here mocks safeStorage with the method present, so the suite could stay green while the shipped app threw. The new case deletes the member from the live mock rather than re-mocking, because the module already holds that object and a later vi.doMock is inert — the first version of this test passed against the unguarded code, which is the failure mode it exists to catch. Verified against the expression currently on main: three cases fail. --- src/main/host/electron-secret-store.test.ts | 37 +++++++++++++++++++++ src/main/host/electron-secret-store.ts | 37 ++++++++++++++++++--- 2 files changed, 70 insertions(+), 4 deletions(-) diff --git a/src/main/host/electron-secret-store.test.ts b/src/main/host/electron-secret-store.test.ts index 0a1f3127cb6..42cbb043578 100644 --- a/src/main/host/electron-secret-store.test.ts +++ b/src/main/host/electron-secret-store.test.ts @@ -63,6 +63,43 @@ describe('ElectronSecretStore', () => { expect(safeStorageMock.getSelectedStorageBackend).not.toHaveBeenCalled() }) + it('does not throw when the Linux-only backend probe is absent', () => { + // Why this test exists: getSelectedStorageBackend is @platform linux, so it is + // genuinely undefined on macOS and Windows — verified against Electron 43. Every + // other test here mocks safeStorage with the method present, so without this the + // suite could stay green while the shipped app threw at startup. + // + // Why delete the key rather than re-mock: the module already holds this object, so + // a later vi.doMock is inert and the test would pass against unguarded code. + const probe = safeStorageMock.getSelectedStorageBackend + // @ts-expect-error deleting a required member is the condition under test + delete safeStorageMock.getSelectedStorageBackend + try { + withPlatform('linux', () => { + expect(() => new ElectronSecretStore().describeProtectionGap()).not.toThrow() + expect(new ElectronSecretStore().describeProtectionGap()).toBeNull() + }) + } finally { + safeStorageMock.getSelectedStorageBackend = probe + } + }) + + it('reports no gap when the backend probe throws', () => { + safeStorageMock.getSelectedStorageBackend.mockImplementation(() => { + throw new Error('backend unavailable') + }) + withPlatform('linux', () => { + expect(new ElectronSecretStore().describeProtectionGap()).toBeNull() + }) + }) + + it('reports no gap for an unknown backend rather than guessing', () => { + safeStorageMock.getSelectedStorageBackend.mockReturnValue('unknown') + withPlatform('linux', () => { + expect(new ElectronSecretStore().describeProtectionGap()).toBeNull() + }) + }) + it('reports the missing-keyring gap before considering the backend', () => { safeStorageMock.isEncryptionAvailable.mockReturnValue(false) withPlatform('linux', () => { diff --git a/src/main/host/electron-secret-store.ts b/src/main/host/electron-secret-store.ts index ae9f8643a74..690fb07ff79 100644 --- a/src/main/host/electron-secret-store.ts +++ b/src/main/host/electron-secret-store.ts @@ -31,9 +31,38 @@ export class ElectronSecretStore implements SecretStore { // It round-trips, so sealing and unsealing genuinely work and must keep working — // reporting it unavailable would strand every credential already stored this way. // But it protects nothing, and reporting it as sealed is the actual lie. - if (process.platform === 'linux' && safeStorage.getSelectedStorageBackend() === 'basic_text') { - return 'Secrets are obfuscated with a built-in key, not protected by the OS keyring. Install and unlock gnome-keyring or kwallet, then restart Orca, to seal them properly.' - } - return null + return describeLinuxBackendGap() } } + +/** + * Why probed rather than called directly: `getSelectedStorageBackend` is `@platform + * linux`, so it is genuinely absent on macOS and Windows — reading it unguarded is a + * TypeError, and the type declaration does not say so. Tests that mock `safeStorage` + * supply the method, so a mocked suite cannot catch that; this keeps the runtime honest + * regardless of platform or Electron version. + * + * Returns null when the backend is unknown or unreadable: claiming a gap we cannot + * prove would be its own kind of lie, and the caller has already established that + * sealing works. + */ +function describeLinuxBackendGap(): string | null { + if (process.platform !== 'linux') { + return null + } + const probe = (safeStorage as Partial).getSelectedStorageBackend + if (typeof probe !== 'function') { + return null + } + let backend: string + try { + backend = probe.call(safeStorage) + } catch { + return null + } + // Why only basic_text: it "encrypts" with a hardcoded password, so it round-trips and + // must keep working — but it protects nothing, and reporting it as sealed is the lie. + return backend === 'basic_text' + ? 'Secrets are obfuscated with a built-in key, not protected by the OS keyring. Install and unlock gnome-keyring or kwallet, then restart Orca, to seal them properly.' + : null +} From 0e9f02baf3789c8fe14e4745820652ae4b7a6d31 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 23 Aug 2026 00:56:36 -0700 Subject: [PATCH 06/14] fix(settings): an empty agent detection must not erase the saved default (#16043) Three bugs in one screen, reported in #15256 with a diff of the user's orca-data.json showing defaultTuiAgent going from "claude" to null. 1. The Auto pill's handler writes null, and it was rendered as the ACTIVE choice whenever the stored agent was merely not detected right now. So the pill that already looked selected was destructive: one click erased the setting, and a later successful detection did not bring it back. Auto is now active only when null is actually stored. Detection is a transient fact; the stored value is not, and this control reports the stored value. 2. With zero agents detected there were no agent pills at all, so the stored choice was both invisible and unrecoverable -- nothing to click to put it back. The stored agent is now always offered, labelled as saved but not currently detected. 3. Refresh lived inside the Installed section, which only renders when at least one agent was found, so the only retry control vanished in exactly the state that needs it. An empty result now renders its own Refresh. This matters more now than when it was filed: #16028 makes WSL detection legitimately return an empty set on machines where the only agent was a Windows binary reached through interop, so the empty path is about to get more traffic. Each of the three tests was verified to fail with its own fix reverted. --- .../components/settings/AgentsPane.test.tsx | 47 ++++++++++++ .../src/components/settings/AgentsPane.tsx | 73 ++++++++++++++++--- src/renderer/src/i18n/locales/en.json | 4 +- 3 files changed, 114 insertions(+), 10 deletions(-) diff --git a/src/renderer/src/components/settings/AgentsPane.test.tsx b/src/renderer/src/components/settings/AgentsPane.test.tsx index 1081f5d4553..ada6d8f1938 100644 --- a/src/renderer/src/components/settings/AgentsPane.test.tsx +++ b/src/renderer/src/components/settings/AgentsPane.test.tsx @@ -630,3 +630,50 @@ describe('AgentsPane', () => { await secondWrite }) }) + +describe('empty agent detection must not cost the saved default (#15256)', () => { + const withEmptyDetection = (run: () => T): T => { + const previous = detectedAgentsMock.detectedIds + detectedAgentsMock.detectedIds = [] + try { + return run() + } finally { + detectedAgentsMock.detectedIds = previous + } + } + + /** The rendered ` + + )} + {detectedAgents.length > 0 && (
Date: Sun, 23 Aug 2026 00:57:05 -0700 Subject: [PATCH 07/14] fix(preflight): do not count a Windows binary reached through interop as a WSL install (#16028) * fix(preflight): do not count a Windows binary reached through interop as a WSL install WSL appends the Windows PATH to the guest PATH by default, so on a distro with no guest `claude`, `command -v claude` resolves to `/mnt/c/Users/me/.../claude.exe`. That path is POSIX-absolute, so the existing absolute-path check accepted it and preflight reported the agent as installed in the distro. That is worse than reporting it absent. Absent tells the user to install it; a false positive launches a Windows executable inside a Linux session, where it sees Windows paths, no guest $HOME and none of the distro's config -- and the failure surfaces later, somewhere less obvious. Rejects `/mnt//` and any `.exe`, case-insensitively. A genuine guest install is unaffected. * fix(preflight): skip Windows mounts during the PATH walk, not after it The review caught this and it is the more important half of the fix. Rejecting the interop path in TypeScript happens after the guest walk has already stopped on it: the lookup breaks at the first executable, and the version-manager fallback dirs are APPENDED, so they sit behind the Windows entries WSL appends. A user with claude in nvm AND on the Windows PATH therefore went from a false positive to "not installed" -- the exact #9725 population the fallback dirs exist to serve. Worse than the bug being fixed. The lookup now takes `skipWindowsMountDirs` and skips those PATH components mid-walk, so the guest binary behind the shadow is still found. Matched by mount metadata from /proc/mounts (drvfs/9p/virtiofs), not by a `/mnt` name: the automount root is configurable, and `/mnt` is an ordinary directory on a Linux box. That also closes the custom-root hole the reviewers found in the name-based predicate. The TypeScript check stays as a secondary net for a mount the guest does not report, with a comment saying why it must never be the thing that decides. Proven with a real /bin/sh: a Windows `claude` ahead of an nvm `claude` on PATH now resolves to the nvm one. Credit: review counsel, and community PR #12794 (spfcraze), which proposed this shape first. * fix(preflight): let the mount table be the only word on what is a Windows path The name-based check could veto a path the walk had deliberately kept. /mnt/d is a perfectly ordinary Linux mount, so a guest binary there was resolved correctly by the walk and then discarded by its name -- the #9725 false negative, reintroduced by the belt-and-braces net I added "just in case". And if awk were missing, the name rule became the only rule, which is precisely the failure it was supposed to backstop. The walk skips components the guest itself reports as drvfs/9p/virtiofs. That is authoritative. Without a mount table we now degrade to main's behaviour (the old false positive) rather than inventing a new false negative. Net: one predicate, three fixtures and an import deleted. --- .../ipc/preflight-wsl-agent-detection.test.ts | 79 ++++++++++++++++++- src/main/ipc/preflight-wsl-agent-detection.ts | 12 ++- src/shared/posix-command-path-lookup.ts | 43 +++++++++- 3 files changed, 125 insertions(+), 9 deletions(-) diff --git a/src/main/ipc/preflight-wsl-agent-detection.test.ts b/src/main/ipc/preflight-wsl-agent-detection.test.ts index a646a9625b5..648c7f4f543 100644 --- a/src/main/ipc/preflight-wsl-agent-detection.test.ts +++ b/src/main/ipc/preflight-wsl-agent-detection.test.ts @@ -56,10 +56,11 @@ describe('detectWslCommandsOnPath', () => { await detectWslCommandsOnPath({ distro: 'Ubuntu' }, ['claude', 'codex']) const { script } = lastSpec() - const lookupScript = buildPosixCommandPathLookupScript({ - kind: 'shell-variable', - name: 'cmd' - }) + const lookupScript = buildPosixCommandPathLookupScript( + { kind: 'shell-variable', name: 'cmd' }, + // The WSL probe opts into skipping Windows mounts mid-walk. + { skipWindowsMountDirs: true } + ) expect(script).toContain(lookupScript) expect(script).not.toContain('type -P') }) @@ -132,6 +133,36 @@ describe('detectWslCommandsOnPath', () => { }) }) +it('does not veto a guest binary on an ordinary Linux mount under /mnt', async () => { + // The walk skips PATH components the guest reports as Windows drives. A name + // rule on top of that could only do harm: /mnt/d can be an ext4 volume, and + // vetoing a path the walk deliberately kept turns the false positive back + // into the #9725 false negative. + runWslProcessMock.mockResolvedValue({ + environmentResolved: true, + code: 0, + stdout: '__ORCA_AGENT_PATH__claude\t/mnt/d/tools/claude\n', + stderr: '', + timedOut: false + }) + await expect(detectWslCommandsOnPath({ distro: 'Ubuntu' }, ['claude'])).resolves.toEqual( + new Set(['claude']) + ) +}) + +it('still counts a genuine guest install', async () => { + runWslProcessMock.mockResolvedValue({ + environmentResolved: true, + code: 0, + stdout: '__ORCA_AGENT_PATH__claude\t/home/alice/.nvm/versions/node/v20.1.0/bin/claude\n', + stderr: '', + timedOut: false + }) + expect(await detectWslCommandsOnPath({ distro: 'Ubuntu' }, ['claude'])).toEqual( + new Set(['claude']) + ) +}) + describe('the detection script itself, run by a real POSIX shell', () => { // Not a mock: the fallback is shell globbing and `[ -x ]`, which only the // shell can be trusted about. Skipped on Windows, where /bin/sh is absent. @@ -220,3 +251,43 @@ describe('the detection script itself, run by a real POSIX shell', () => { expect(await runScript()).toContain('__ORCA_AGENT_PATH__orca-fake-cli') }) }) + +describe('the PATH walk, run by a real POSIX shell', () => { + const itPosix = process.platform === 'win32' ? it.skip : it + + itPosix('finds the guest install behind a Windows binary that shadows it', async () => { + // WSL appends the Windows PATH, so a Windows `claude` sits ahead of an + // nvm one. Rejecting the resolved path afterwards cannot resume the walk, + // so it reports "not installed" for a user who has both -- turning the + // false positive into the #9725 false negative. Skipping the component + // mid-walk is what actually finds the guest binary. + const root = mkdtempSync(join(tmpdir(), 'orca-walk-')) + try { + const win = join(root, 'winmnt/c/npm') + const nvm = join(root, 'home/.nvm/bin') + for (const [dir, body] of [ + [win, 'win'], + [nvm, 'guest'] + ] as const) { + mkdirSync(dir, { recursive: true }) + writeFileSync(join(dir, 'claude'), `#!/bin/sh\necho ${body}\n`) + chmodSync(join(dir, 'claude'), 0o755) + } + const script = buildPosixCommandPathLookupScript( + { kind: 'literal', value: 'claude' }, + { skipWindowsMountDirs: true } + // Stand in for /proc/mounts, which a test host does not have. + ).replace(/_orca_win_mounts=\$\([^)]*\)/, `_orca_win_mounts=${join(root, 'winmnt')}`) + const options: ExecFileSyncOptions = { + encoding: 'utf8', + env: { PATH: `${win}:${nvm}:/usr/bin:/bin` } + } + const out = String( + execFileSync('/bin/sh', ['-c', `${script}\nprintf %s "$resolved"`], options) + ) + expect(out).toBe(join(nvm, 'claude')) + } finally { + rmSync(root, { recursive: true, force: true }) + } + }) +}) diff --git a/src/main/ipc/preflight-wsl-agent-detection.ts b/src/main/ipc/preflight-wsl-agent-detection.ts index 701da37f8c4..a0e843b43b1 100644 --- a/src/main/ipc/preflight-wsl-agent-detection.ts +++ b/src/main/ipc/preflight-wsl-agent-detection.ts @@ -20,10 +20,14 @@ export async function detectWslCommandsOnPath( } const commandList = uniqueCommands.map(shellQuote).join(' ') - const lookupScript = buildPosixCommandPathLookupScript({ - kind: 'shell-variable', - name: 'cmd' - }) + const lookupScript = buildPosixCommandPathLookupScript( + { kind: 'shell-variable', name: 'cmd' }, + // Skip Windows mounts DURING the walk, not after it: WSL appends the + // Windows PATH, so a Windows `claude` can shadow a real guest install, and + // discarding the result afterwards reports "not installed" for a user who + // has both -- the #9725 population the fallback dirs exist to serve. + { skipWindowsMountDirs: true } + ) // Newlines keep the loop valid in every POSIX shell used here. const script = [ // The same fallback the preflight command runner uses: append the diff --git a/src/shared/posix-command-path-lookup.ts b/src/shared/posix-command-path-lookup.ts index fb29be82294..ac179f941fb 100644 --- a/src/shared/posix-command-path-lookup.ts +++ b/src/shared/posix-command-path-lookup.ts @@ -4,12 +4,52 @@ export type PosixCommandPathLookupTarget = const SHELL_VARIABLE_NAME_PATTERN = /^[A-Za-z_][A-Za-z0-9_]*$/ -export function buildPosixCommandPathLookupScript(target: PosixCommandPathLookupTarget): string { +export type PosixCommandPathLookupOptions = { + /** + * Skip PATH components that are Windows drives mounted into the guest. + * + * WSL appends the Windows PATH to the guest PATH, so a Windows `claude` can + * sit ahead of a real guest install. Rejecting the result AFTER the walk is + * not equivalent: the walk stops at the first hit, so discarding it reports + * "not installed" for a user who has both. Skipping the component keeps the + * walk going and finds the guest binary behind it. + * + * Matched by mount metadata, not by a `/mnt` name: the automount root is + * configurable, and `/mnt` is an ordinary directory name on a Linux box. + */ + skipWindowsMountDirs?: boolean +} + +export function buildPosixCommandPathLookupScript( + target: PosixCommandPathLookupTarget, + options: PosixCommandPathLookupOptions = {} +): string { const commandAssignment = buildCommandAssignment(target) + // `-t drvfs` is what WSL mounts a Windows drive as, wherever the automount + // root is; 9p/virtiofs cover the WSL2 shapes. Read once, outside the loop. + const mountPrelude = options.skipWindowsMountDirs + ? [ + '_orca_win_mounts=$(awk \'$3 == "drvfs" || $3 == "9p" || $3 == "virtiofs" { print $2 }\' /proc/mounts 2>/dev/null)' + ] + : [] + const skipMountComponent = options.skipWindowsMountDirs + ? [ + ' for _orca_win_mount in $_orca_win_mounts; do', + ' case "$_orca_lookup_component/" in', + ' "$_orca_win_mount"/*) _orca_lookup_component= ;;', + ' esac', + ' done', + ' if [ -z "$_orca_lookup_component" ]; then', + ' [ -n "$_orca_lookup_has_more" ] || break', + ' continue', + ' fi' + ] + : [] // Shell command resolution can be masked by aliases, functions, and builtins, so inspect PATH. return [ `_orca_lookup_command=${commandAssignment}`, 'resolved=', + ...mountPrelude, 'case "$_orca_lookup_command" in', ' */*)', ' case "$_orca_lookup_command" in', @@ -34,6 +74,7 @@ export function buildPosixCommandPathLookupScript(target: PosixCommandPathLookup ' _orca_lookup_has_more=', ' ;;', ' esac', + ...skipMountComponent, ' [ -n "$_orca_lookup_component" ] || _orca_lookup_component=.', ' case "$_orca_lookup_component" in', ' /*) _orca_lookup_candidate=$_orca_lookup_component/$_orca_lookup_command ;;', From e26f8496827a822da8d1c571d06d6e7e9e3578d3 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 23 Aug 2026 00:57:34 -0700 Subject: [PATCH 08/14] fix(wsl): budget the whole command line, not just the script (#16032) * fix(wsl): budget the whole command line, not just the script The argv/stdin threshold measured `script.length`, but the cap applies to the finished command line -- which also carries `PATH=` and `HOME=`. A login PATH is itself a few KB. That produced a perverse band: with a long enough PATH, a 7,999-char hook was placed on argv and CreateProcess refused it, while the SAME hook at 8,001 chars flipped to stdin and ran. Size decided how a hook behaved, in the wrong direction, and the failure looked like "your setup hook failed" with nothing pointing at length. Now the argv form is built, measured, and only used if the whole line fits; otherwise the script goes to stdin as before. The count over-estimates slightly (it charges quoting for every argument) because over-counting is the safe direction for a cap. The regression test uses a 7,000-char script -- deliberately under any script-only threshold -- with a 27KB PATH, and asserts it lands on stdin. My first attempt used 7,999 + `echo `, which is 8,004 and flipped under the old rule too, so it passed either way and proved nothing. Credit: Grok. * fix(wsl): charge quoting and measure the line that is actually spawned Two under-counts the review found in the estimator I added. The doc comment claimed it over-counts. It did not: libuv escapes every `"` and doubles a backslash run before a quote, so a quote-dense script costs more than its length. And `wsl.exe` plus `-d --exec` are prepended AFTER the measurement, so ~45 characters of the budget were never counted. Together those put a quote-heavy ~26KB script on argv and over the real 32767 ceiling -- where the old script-only rule would have sent it to stdin and it would have run. A narrower band than the one this PR removes, but the same shape of bug, so worth closing before merge rather than after. Now charges one character per `"` or backslash and measures the full spawn line. New test: 26,000 quote characters must land on stdin; verified to fail with the quoting charge removed. --- src/main/wsl/wsl-runner.test.ts | 31 +++++++++++++++++++++++++++- src/main/wsl/wsl-runner.ts | 36 ++++++++++++++++++++++++++++----- 2 files changed, 61 insertions(+), 6 deletions(-) diff --git a/src/main/wsl/wsl-runner.test.ts b/src/main/wsl/wsl-runner.test.ts index f92da4d18e3..4e6c36d45da 100644 --- a/src/main/wsl/wsl-runner.test.ts +++ b/src/main/wsl/wsl-runner.test.ts @@ -159,13 +159,42 @@ describe('scripts', () => { // command line at 32767, so argv would fail to spawn at all; stdin still // runs it, which is the lesser loss. seedWslGuestEnvironmentForTests(undefined, ENVIRONMENT) - const script = `echo ${'x'.repeat(9_000)}` + const script = `echo ${'x'.repeat(31_000)}` await runWslProcess({ loginPath: 'preferred', shell: 'bash', script }) const spec = runProcessMock.mock.calls.at(-1)?.[0] expect(spec.input).toBe(script) expect(lastArgv().slice(-3)).toEqual(['bash', '-s', '--']) }) + it('flips to stdin when the login PATH, not the script, is what overflows', async () => { + // The perverse band a script-only threshold created: with a long enough + // PATH spliced in as `PATH=...`, a 7,999-char hook went to argv and failed + // to spawn, while the SAME hook at 8,001 chars flipped to stdin and ran. + // Size decided how a hook behaved, in the wrong direction. + seedWslGuestEnvironmentForTests(undefined, { + ...ENVIRONMENT, + path: '/opt/x/bin:'.repeat(2_500) + }) + // Deliberately UNDER any script-only threshold: 7k of script with 27k of + // PATH overflows the line, and the old rule sent exactly this to argv. + const script = `echo ${'x'.repeat(7_000)}` + expect(script.length).toBeLessThan(8_000) + await runWslProcess({ loginPath: 'preferred', shell: 'bash', script }) + const spec = runProcessMock.mock.calls.at(-1)?.[0] + expect(spec.input).toBe(script) + expect(spec.args.join(' ')).not.toContain(script) + }) + + it('charges quoting, so a quote-dense script cannot slip over the real cap', async () => { + // libuv escapes every `"` and doubles backslash runs, so a script's cost is + // more than its length. Counting length alone put a quote-heavy ~26KB + // script on argv and over the 32767 ceiling. + seedWslGuestEnvironmentForTests(undefined, ENVIRONMENT) + const script = '"'.repeat(26_000) + await runWslProcess({ loginPath: 'preferred', shell: 'bash', script }) + expect(runProcessMock.mock.calls.at(-1)?.[0].input).toBe(script) + }) + it('keeps an ordinary-sized script in argv', async () => { seedWslGuestEnvironmentForTests(undefined, ENVIRONMENT) const script = `echo ${'x'.repeat(100)}` diff --git a/src/main/wsl/wsl-runner.ts b/src/main/wsl/wsl-runner.ts index f1a51d4b923..6a0fbc051ac 100644 --- a/src/main/wsl/wsl-runner.ts +++ b/src/main/wsl/wsl-runner.ts @@ -169,8 +169,14 @@ function withGuestCwd(cwd: string | undefined, argv: readonly string[]): string[ * failing to spawn at all and accepting the stdin caveat. Degrading beats * failing: a large script that also reads stdin was already broken, while a * large script that does not now works where it would have died. + * + * Measured on the WHOLE command line, not on the script alone. A login PATH is + * itself a few KB and is spliced in as `PATH=...`, so a script-only threshold + * produced a perverse band: with a long enough PATH, a 7,999-char hook went to + * argv and failed to spawn while the same hook at 8,001 chars flipped to stdin + * and ran. Size decided how a hook behaved, in the wrong direction. */ -const MAX_ARGV_SCRIPT_CHARS = 8_000 +const MAX_COMMAND_LINE_CHARS = 30_000 /** ` -c`/`-s` for a script, otherwise the program itself. */ function guestCommandArgv(spec: WslSpec, delivery: 'argv' | 'stdin'): string[] { @@ -184,6 +190,19 @@ function guestCommandArgv(spec: WslSpec, delivery: 'argv' | 'stdin'): string[] { : [shell, '-c', spec.script, '--', ...(spec.args ?? [])] } +/** + * What `CreateProcess` will count. + * + * libuv escapes every `"` and doubles a backslash run before a quote, so a + * quote-dense script costs more than its length. Charging one extra character + * per `"` or `\\` keeps the estimate on the safe side of the cap; an earlier + * version claimed to over-count and in fact under-counted, which put a + * quote-heavy ~26KB script on argv and over the real limit. + */ +function commandLineLength(args: readonly string[]): number { + return args.reduce((total, arg) => total + arg.length + 3 + (arg.match(/["\\]/g)?.length ?? 0), 0) +} + /** Shell-free argv, with the cached environment applied when one is available. */ function buildGuestArgv( environment: WslGuestEnvironment | null, @@ -229,11 +248,18 @@ export async function runWslProcess(spec: WslSpec): Promise { // the probe most often fails *because* the distro is slow, so the fallback // would hit the hazard exactly when it is worst. Run shell-free with the // distro's default PATH instead: degraded, never blocking. - // Resolved once: argv shape and stdin payload must agree, and computing it - // in both places invites them to drift. + // Build the argv form first and measure it: the env prefix is part of the + // budget, so only the finished line can say whether argv fits. Resolved once + // here because the argv shape and the stdin payload must agree. + const argvForm = buildGuestArgv(environment, spec, 'argv') + // Measure what is actually spawned: `wsl.exe` and `-d --exec` are + // prepended after this point and are part of the same budget. + const fullLine = [resolveWslExecutablePath(), ...buildWslExecArgs(spec.distro, argvForm)] const delivery: 'argv' | 'stdin' = - spec.script !== undefined && spec.script.length > MAX_ARGV_SCRIPT_CHARS ? 'stdin' : 'argv' - const argv = buildGuestArgv(environment, spec, delivery) + spec.script !== undefined && commandLineLength(fullLine) > MAX_COMMAND_LINE_CHARS + ? 'stdin' + : 'argv' + const argv = delivery === 'argv' ? argvForm : buildGuestArgv(environment, spec, 'stdin') // One budget for the whole call: the probe used to run on its own 10s timer // ahead of the timed leg, so a 5s caller could wait 15s. From 4c984d4c1b3b55651cec217bc78e94e4b99852f1 Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Sun, 23 Aug 2026 01:24:37 -0700 Subject: [PATCH 09/14] Fix Windows Git Bash console-capacity failures (#16045) * fix(terminal): retain failed local console panes * fix(terminal): preserve failed pane restart context * fix(terminal): scope capacity recovery to PTY binding --------- Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com> --- .../components/terminal-pane/TerminalPane.tsx | 68 +++++- .../TerminalProcessExitOverlay.test.tsx | 45 ++++ .../TerminalProcessExitOverlay.tsx | 61 +++++ .../git-bash-console-capacity.test.ts | 20 ++ .../git-bash-console-capacity.ts | 23 ++ .../pty-connection-pty-exit-teardown.test.ts | 213 ++++++++++++++++++ .../terminal-pane/pty-connection-types.ts | 62 ++--- .../terminal-pane/pty-connection.ts | 129 ++++++++--- ...ty-transport-detach-attach-handoff.test.ts | 2 +- .../pty-transport-handler-suspension.test.ts | 4 +- .../pty-transport-reattach-admission.test.ts | 2 +- .../terminal-pane/pty-transport-types.ts | 2 +- .../components/terminal-pane/pty-transport.ts | 2 +- .../terminal-process-exit-restart.test.ts | 39 ++++ .../terminal-process-exit-restart.ts | 7 + .../use-terminal-pane-lifecycle.ts | 4 + src/renderer/src/i18n/locales/en.json | 8 + src/renderer/src/i18n/locales/es.json | 8 + src/renderer/src/i18n/locales/ja.json | 8 + src/renderer/src/i18n/locales/ko.json | 8 + src/renderer/src/i18n/locales/zh.json | 8 + 21 files changed, 661 insertions(+), 62 deletions(-) create mode 100644 src/renderer/src/components/terminal-pane/TerminalProcessExitOverlay.test.tsx create mode 100644 src/renderer/src/components/terminal-pane/TerminalProcessExitOverlay.tsx create mode 100644 src/renderer/src/components/terminal-pane/git-bash-console-capacity.test.ts create mode 100644 src/renderer/src/components/terminal-pane/git-bash-console-capacity.ts create mode 100644 src/renderer/src/components/terminal-pane/terminal-process-exit-restart.test.ts create mode 100644 src/renderer/src/components/terminal-pane/terminal-process-exit-restart.ts diff --git a/src/renderer/src/components/terminal-pane/TerminalPane.tsx b/src/renderer/src/components/terminal-pane/TerminalPane.tsx index 3d66ead0734..2b58732f0ea 100644 --- a/src/renderer/src/components/terminal-pane/TerminalPane.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalPane.tsx @@ -60,6 +60,7 @@ import { RUNNING_CLOSE_PROBE_TIMEOUT_MS } from '../terminal/running-terminal-clo import CodexRestartChip from '../CodexRestartChip' import { MobileDriverOverlay } from './MobileDriverOverlay' import { stripSshReconnectOwnedErrorLines, TerminalErrorToast } from './TerminalErrorToast' +import { TerminalProcessExitOverlay } from './TerminalProcessExitOverlay' import { TerminalSessionStateSaveFailureDialog } from './TerminalSessionStateSaveFailureDialog' import TerminalContextMenu from './TerminalContextMenu' import TerminalPaneHeaderOverlay, { type PaneTitleOverlayRect } from './TerminalPaneHeaderOverlay' @@ -100,6 +101,8 @@ import type { PreparedAgentSessionFork } from './terminal-agent-session-fork' import type { AgentSessionContinuationRequest } from '@/lib/agent-session-continuation' import { useNotificationDispatch } from './use-notification-dispatch' import { connectPanePty } from './pty-connection' +import type { PaneProcessExit, PtyConnectionDeps } from './pty-connection-types' +import { resolveTerminalProcessExitRestartStartup } from './terminal-process-exit-restart' import { resolveTerminalLayoutActiveLeafId } from './terminal-layout-leaf-ids' import { shouldPreserveTerminalScrollbackBuffers } from '../../../../shared/workspace-session-terminal-buffers' import { @@ -399,6 +402,15 @@ function TerminalPane( const [agentSessionContinuation, setAgentSessionContinuation] = useState(null) const [terminalError, setTerminalError] = useState(null) + const [paneProcessExitsByPaneId, setPaneProcessExitsByPaneId] = useState< + Record + >({}) + const handlePaneProcessDied = useCallback((processExit: PaneProcessExit) => { + setPaneProcessExitsByPaneId((current) => ({ + ...current, + [processExit.paneId]: processExit + })) + }, []) const [ptyRecoveryStatesByPaneId, setPtyRecoveryStatesByPaneId] = useState< Record >({}) @@ -1368,6 +1380,7 @@ function TerminalPane( onPtyExitRef, onAgentExitedRef, onPtyErrorRef, + onPaneProcessDied: handlePaneProcessDied, onPtyRecoveryStateRef, clearTabPtyId, consumeSuppressedPtyExit: useAppStore((store) => store.consumeSuppressedPtyExit), @@ -1535,7 +1548,10 @@ function TerminalPane( }, []) const handleRestartCodexPane = useCallback( - (paneId: number) => { + ( + paneId: number, + restartStartup: PtyConnectionDeps['startup'] = CODEX_ACCOUNT_RESTART_STARTUP + ) => { const manager = managerRef.current const pane = manager?.getPanes().find((candidate) => candidate.id === paneId) if (!manager || !pane) { @@ -1565,7 +1581,7 @@ function TerminalPane( tabId, worktreeId, cwd, - startup: CODEX_ACCOUNT_RESTART_STARTUP, + startup: restartStartup, mountFollowsTerminalPark: false, paneTransportsRef, paneMode2031Ref, @@ -1577,6 +1593,7 @@ function TerminalPane( onPtyExitRef, onAgentExitedRef, onPtyErrorRef, + onPaneProcessDied: handlePaneProcessDied, onPtyRecoveryStateRef, clearTabPtyId, consumeSuppressedPtyExit: useAppStore.getState().consumeSuppressedPtyExit, @@ -1607,6 +1624,7 @@ function TerminalPane( clearTabPtyId, cwd, dispatchNotification, + handlePaneProcessDied, markWorktreeUnread, markTerminalTabUnread, markTerminalPaneUnread, @@ -1627,6 +1645,36 @@ function TerminalPane( ] ) + const clearPaneProcessExit = useCallback((paneId: number) => { + setPaneProcessExitsByPaneId((current) => { + if (current[paneId] === undefined) { + return current + } + const next = { ...current } + delete next[paneId] + return next + }) + }, []) + + const handleRestartExitedPane = useCallback( + (processExit: PaneProcessExit) => { + clearPaneProcessExit(processExit.paneId) + handleRestartCodexPane( + processExit.paneId, + resolveTerminalProcessExitRestartStartup(processExit) + ) + }, + [clearPaneProcessExit, handleRestartCodexPane] + ) + + const handleCloseExitedPane = useCallback( + (paneId: number) => { + clearPaneProcessExit(paneId) + executeClosePane(paneId) + }, + [clearPaneProcessExit, executeClosePane] + ) + // Why leaf bindings are a dep: a parked or deferred tab mounts with no // transport, so a queued restart has no ptyId to match on the mount pass. The // reconnected PTY rewrites this map when it binds — `ptyIdsByTabId` does not, @@ -3031,6 +3079,22 @@ function TerminalPane( onRestartDaemon={() => daemonActions.setPending('restart')} /> ) : null} + {isActive + ? managedPanes.map((pane) => { + const processExit = paneProcessExitsByPaneId[pane.id] + return processExit + ? createPortal( + handleRestartExitedPane(processExit)} + onClose={() => handleCloseExitedPane(pane.id)} + />, + pane.container, + `process-exit-${pane.id}` + ) + : null + }) + : null} {/* Why: portal into the pane so the banner stacks above the xterm canvas (sibling mount painted under WebGL). */} {showSshReconnectOverlay && sshReconnectTargetId && sshReconnectStatus ? managedPanes.map((pane) => diff --git a/src/renderer/src/components/terminal-pane/TerminalProcessExitOverlay.test.tsx b/src/renderer/src/components/terminal-pane/TerminalProcessExitOverlay.test.tsx new file mode 100644 index 00000000000..6270a5de91e --- /dev/null +++ b/src/renderer/src/components/terminal-pane/TerminalProcessExitOverlay.test.tsx @@ -0,0 +1,45 @@ +// @vitest-environment happy-dom + +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { TerminalProcessExitOverlay } from './TerminalProcessExitOverlay' + +describe('TerminalProcessExitOverlay', () => { + afterEach(cleanup) + + it('explains the Git Bash limit and exposes recovery actions', () => { + const onRestart = vi.fn() + const onClose = vi.fn() + render( + + ) + + expect(screen.getByRole('alert').textContent).toContain('128-console limit') + fireEvent.click(screen.getByRole('button', { name: 'Restart' })) + fireEvent.click(screen.getByRole('button', { name: 'Close' })) + + expect(onRestart).toHaveBeenCalledOnce() + expect(onClose).toHaveBeenCalledOnce() + }) + + it('preserves the exit code for other shell failures', () => { + render( + + ) + + expect(screen.getByRole('alert').textContent).toContain('exit code 7') + }) +}) diff --git a/src/renderer/src/components/terminal-pane/TerminalProcessExitOverlay.tsx b/src/renderer/src/components/terminal-pane/TerminalProcessExitOverlay.tsx new file mode 100644 index 00000000000..5e450c24b05 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/TerminalProcessExitOverlay.tsx @@ -0,0 +1,61 @@ +import { RotateCw, X } from 'lucide-react' +import { Button } from '@/components/ui/button' +import { translate } from '@/i18n/i18n' +import type { PaneProcessExit } from './pty-connection-types' + +export function TerminalProcessExitOverlay({ + processExit, + onRestart, + onClose +}: { + processExit: PaneProcessExit + onRestart: () => void + onClose: () => void +}): React.JSX.Element { + const capacityError = processExit.reason === 'git-bash-console-capacity' + const title = capacityError + ? translate( + 'auto.components.terminal.pane.TerminalProcessExitOverlay.capacityTitle', + 'Git Bash console limit reached' + ) + : translate( + 'auto.components.terminal.pane.TerminalProcessExitOverlay.failedTitle', + 'Terminal exited' + ) + const detail = capacityError + ? translate( + 'auto.components.terminal.pane.TerminalProcessExitOverlay.capacityDetail', + 'Git Bash reached its 128-console limit. Close unused Git Bash terminals, then restart this terminal.' + ) + : translate( + 'auto.components.terminal.pane.TerminalProcessExitOverlay.failedDetail', + 'The shell process ended with exit code {{code}}. Its output is preserved.' + ).replace('{{code}}', String(processExit.exitCode)) + + return ( +
+
+
+
{title}
+
{detail}
+
+
+ + +
+
+
+ ) +} diff --git a/src/renderer/src/components/terminal-pane/git-bash-console-capacity.test.ts b/src/renderer/src/components/terminal-pane/git-bash-console-capacity.test.ts new file mode 100644 index 00000000000..27067b16828 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/git-bash-console-capacity.test.ts @@ -0,0 +1,20 @@ +import { describe, expect, it } from 'vitest' +import { createGitBashConsoleCapacityDetector } from './git-bash-console-capacity' + +describe('Git Bash console capacity detection', () => { + it('recognizes the MSYS fatal message across PTY chunks', () => { + const detector = createGitBashConsoleCapacityDetector() + + detector.observe('console device allocation failure - too many consoles ') + detector.observe('in use, max consoles is 128') + + expect(detector.detected()).toBe(true) + }) + + it('ignores unrelated shell failures', () => { + const detector = createGitBashConsoleCapacityDetector() + detector.observe('bash: command not found') + + expect(detector.detected()).toBe(false) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/git-bash-console-capacity.ts b/src/renderer/src/components/terminal-pane/git-bash-console-capacity.ts new file mode 100644 index 00000000000..6f70a8b2a2e --- /dev/null +++ b/src/renderer/src/components/terminal-pane/git-bash-console-capacity.ts @@ -0,0 +1,23 @@ +const GIT_BASH_CONSOLE_CAPACITY_MARKER = 'too many consoles in use, max consoles is 128' + +export type GitBashConsoleCapacityDetector = { + observe: (data: string) => void + detected: () => boolean +} + +export function createGitBashConsoleCapacityDetector(): GitBashConsoleCapacityDetector { + let tail = '' + let matched = false + + return { + observe(data) { + if (matched || data.length === 0) { + return + } + const candidate = (tail + data).toLowerCase() + matched = candidate.includes(GIT_BASH_CONSOLE_CAPACITY_MARKER) + tail = candidate.slice(-(GIT_BASH_CONSOLE_CAPACITY_MARKER.length - 1)) + }, + detected: () => matched + } +} diff --git a/src/renderer/src/components/terminal-pane/pty-connection-pty-exit-teardown.test.ts b/src/renderer/src/components/terminal-pane/pty-connection-pty-exit-teardown.test.ts index ae55be07860..a577488b3e6 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-pty-exit-teardown.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-pty-exit-teardown.test.ts @@ -1,6 +1,8 @@ import type * as React from 'react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { makePaneKey } from '../../../../shared/stable-pane-id' import { sendTerminalInputThroughPane } from './pty-connection-test-dom' +import { flushAsyncTicks } from './pty-connection-test-async' import { LEAF_1, LEAF_2, @@ -129,6 +131,38 @@ function createDeps(overrides: Record = {}) { return buildPaneConnectionDeps(() => mockStoreState, overrides) } +function installSleepingCodexResumeState(restoredPtyId?: string) { + const paneKey = makePaneKey('tab-1', LEAF_1) + const launchConfig = { + agentCommand: "codex '--model' 'gpt-5'", + agentArgs: '--model gpt-5', + agentEnv: { CODEX_PROFILE: 'captured' } + } + mockStoreState = { + ...mockStoreState, + tabsByWorktree: { + 'wt-1': [{ id: 'tab-1', ...(restoredPtyId ? { ptyId: restoredPtyId } : {}) }] + }, + settings: { ...mockStoreState.settings, agentCmdOverrides: {} }, + agentStatusByPaneKey: {}, + sleepingAgentSessionsByPaneKey: { + [paneKey]: { + paneKey, + tabId: 'tab-1', + worktreeId: 'wt-1', + agent: 'codex', + providerSession: { key: 'session_id', id: 'codex-session-1' }, + prompt: 'finish the task', + state: 'working', + capturedAt: 1, + updatedAt: 1, + launchConfig + } + } + } as StoreState + return launchConfig +} + describe('connectPanePty', () => { beforeEach(() => { vi.resetModules() @@ -388,6 +422,185 @@ describe('connectPanePty', () => { expect(manager.closePane).not.toHaveBeenCalled() }) + it('keeps a failed local terminal visible after user input', async () => { + const { connectPanePty } = await import('./pty-connection') + const pane = createPane(1) + const transport = createMockTransport('tab-pty') + transportFactoryQueue.push(transport) + const manager = createManager(1) + const deps = createDeps({ onPaneProcessDied: vi.fn() }) + + connectPanePty(pane as never, manager as never, deps as never) + const onPtyExit = createdTransportOptions[0]?.onPtyExit as + | ((ptyId: string, exitCode?: number) => void) + | undefined + + sendTerminalInputThroughPane(pane, 'agent startup\r') + onPtyExit?.('tab-pty', 1) + + expect(deps.onPaneProcessDied).toHaveBeenCalledWith({ + paneId: 1, + exitCode: 1, + startup: null, + reason: 'process-failed' + }) + expect(deps.onPtyExitRef.current).not.toHaveBeenCalled() + expect(manager.closePane).not.toHaveBeenCalled() + }) + + it('keeps a failed local split pane visible', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('tab-pty') + transportFactoryQueue.push(transport) + const manager = createManager(2) + const deps = createDeps({ onPaneProcessDied: vi.fn() }) + + connectPanePty(createPane(1) as never, manager as never, deps as never) + const onPtyExit = createdTransportOptions[0]?.onPtyExit as + | ((ptyId: string, exitCode?: number) => void) + | undefined + onPtyExit?.('tab-pty', 1) + + expect(deps.onPaneProcessDied).toHaveBeenCalledWith({ + paneId: 1, + exitCode: 1, + startup: null, + reason: 'process-failed' + }) + expect(manager.closePane).not.toHaveBeenCalled() + }) + + it('classifies the Git Bash capacity failure before retaining its pane', async () => { + const { connectPanePty } = await import('./pty-connection') + const capturedDataCallback: { current: ((data: string) => void) | null } = { current: null } + const transport = createMockTransport('tab-pty') + transport.connect.mockImplementation(async ({ callbacks }: { callbacks: ConnectCallbacks }) => { + capturedDataCallback.current = callbacks.onData ?? null + return 'tab-pty' + }) + transportFactoryQueue.push(transport) + const manager = createManager(1) + const startup = { command: 'codex --resume session-1' } + const deps = createDeps({ onPaneProcessDied: vi.fn(), startup }) + + connectPanePty(createPane(1) as never, manager as never, deps as never) + const onPtyExit = createdTransportOptions[0]?.onPtyExit as + | ((ptyId: string, exitCode?: number) => void) + | undefined + + capturedDataCallback.current?.( + 'console device allocation failure - too many consoles in use, max consoles is 128' + ) + onPtyExit?.('tab-pty', 1) + + expect(deps.onPaneProcessDied).toHaveBeenCalledWith({ + paneId: 1, + exitCode: 1, + startup, + reason: 'git-bash-console-capacity' + }) + expect(deps.onPtyExitRef.current).not.toHaveBeenCalled() + }) + + it('retains the cold-restore resume startup when its replacement hits capacity', async () => { + const { connectPanePty } = await import('./pty-connection') + const callbacks: ConnectCallbacks[] = [] + const transport = createMockTransport('resume-pty') + transport.connect.mockImplementation(async (options: { callbacks: ConnectCallbacks }) => { + callbacks.push(options.callbacks) + return 'resume-pty' + }) + transportFactoryQueue.push(transport) + const launchConfig = installSleepingCodexResumeState() + const deps = createDeps({ + startup: { command: 'codex stale-startup' }, + onPaneProcessDied: vi.fn() + }) + + connectPanePty(createPane(1) as never, createManager(1) as never, deps as never) + await flushAsyncTicks(20) + callbacks[0]?.onData?.('too many consoles in use, max consoles is 128') + const onPtyExit = createdTransportOptions[0]?.onPtyExit as + | ((ptyId: string, exitCode?: number) => void) + | undefined + onPtyExit?.('resume-pty', 1) + + expect(deps.onPaneProcessDied).toHaveBeenCalledWith({ + paneId: 1, + exitCode: 1, + startup: expect.objectContaining({ + command: expect.stringContaining("'resume' 'codex-session-1'"), + launchConfig, + resumeProviderSession: { key: 'session_id', id: 'codex-session-1' }, + launchAgent: 'codex', + showSessionRestoredBanner: true + }), + reason: 'git-bash-console-capacity' + }) + }) + + it('does not carry capacity detection into a cold-restore replacement', async () => { + const { connectPanePty } = await import('./pty-connection') + const callbacks: ConnectCallbacks[] = [] + let currentPtyId = 'lost-pty' + const transport = createMockTransport(currentPtyId) + transport.getPtyId.mockImplementation(() => currentPtyId) + transport.connect.mockImplementation( + async (options: { sessionId?: string; callbacks: ConnectCallbacks }) => { + callbacks.push(options.callbacks) + if (options.sessionId) { + options.callbacks.onData?.('too many consoles in use, max consoles is 128') + return { id: currentPtyId, sessionExpired: true } + } + currentPtyId = 'resume-pty' + return currentPtyId + } + ) + transportFactoryQueue.push(transport) + installSleepingCodexResumeState('lost-pty') + const deps = createDeps({ + onPaneProcessDied: vi.fn(), + restoredLeafId: LEAF_1, + restoredPtyIdByLeafId: { [LEAF_1]: 'lost-pty' } + }) + + connectPanePty(createPane(1) as never, createManager(1) as never, deps as never) + await flushAsyncTicks(30) + expect(callbacks).toHaveLength(2) + const onPtyExit = createdTransportOptions[0]?.onPtyExit as + | ((ptyId: string, exitCode?: number) => void) + | undefined + onPtyExit?.('resume-pty', 1) + + expect(deps.onPaneProcessDied).toHaveBeenCalledWith({ + paneId: 1, + exitCode: 1, + startup: null, + reason: 'process-failed' + }) + }) + + it('does not retain a failed direct-SSH terminal locally', async () => { + const { connectPanePty } = await import('./pty-connection') + mockStoreState = { + ...mockStoreState, + repos: [{ ...mockStoreState.repos[0], connectionId: 'ssh-1' }] + } as StoreState + const transport = createMockTransport('tab-pty') + transportFactoryQueue.push(transport) + const manager = createManager(1) + const deps = createDeps({ onPaneProcessDied: vi.fn() }) + + connectPanePty(createPane(1) as never, manager as never, deps as never) + const onPtyExit = createdTransportOptions[0]?.onPtyExit as + | ((ptyId: string, exitCode?: number) => void) + | undefined + onPtyExit?.('tab-pty', 1) + + expect(deps.onPaneProcessDied).not.toHaveBeenCalled() + expect(deps.onPtyExitRef.current).toHaveBeenCalledWith('tab-pty') + }) + it('tears down the sole terminal when a reattached (not freshly spawned) PTY exits', async () => { // Why: reattach/coldRestore skip onPtySpawn, so a now-dead previously-live session must still route through onPtyExitRef; the keep-mounted guard is only for brand-new shells. const { connectPanePty } = await import('./pty-connection') 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 f502360ebea..1fc9411cdfe 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-types.ts @@ -17,36 +17,45 @@ import type { PtyTransportRecoveryState } from './pty-transport-types' import type { SessionOptionValue } from '../../../../shared/native-chat-session-options' import type { DirectSshPaneRetryAttemptId } from '@/store/slices/direct-ssh-terminal-recovery' +export type PtyPaneStartup = { + command: string + /** Renderer-delivered startup input for callers that need xterm paste + * semantics before the submit Enter. */ + delivery?: 'terminal-paste' + startupCommandDelivery?: StartupCommandDelivery + env?: Record + envToDelete?: string[] + launchConfig?: SleepingAgentLaunchConfig + resumeProviderSession?: AgentProviderSessionMetadata + launchToken?: string + launchAgent?: TuiAgent + /** Explicit CLI override for host-owned agent launches; omission uses host settings. */ + agentArgsOverride?: string | null + draftPrompt?: string + sessionOptions?: Record + /** Telemetry payload for `agent_started`. Forwarded to `pty:spawn` + * so main fires the event only after the spawn succeeds. */ + telemetry?: EventProps<'agent_started'> + /** Initial prompt-start status for agents that lack native prompt hooks. */ + initialAgentStatus?: { agent: TuiAgent; prompt: string } + /** Show the restored-session banner when this startup command mounts. */ + showSessionRestoredBanner?: boolean + /** Initial startup may be paired with a setup split that changes its grid. */ + waitForSetupSplitDirection?: SetupSplitDirection +} | null + +export type PaneProcessExit = { + paneId: number + exitCode: number + reason: 'git-bash-console-capacity' | 'process-failed' + startup: PtyPaneStartup +} + export type PtyConnectionDeps = { tabId: string worktreeId: string cwd?: string - startup?: { - command: string - /** Renderer-delivered startup input for callers that need xterm paste - * semantics before the submit Enter. */ - delivery?: 'terminal-paste' - startupCommandDelivery?: StartupCommandDelivery - env?: Record - envToDelete?: string[] - launchConfig?: SleepingAgentLaunchConfig - resumeProviderSession?: AgentProviderSessionMetadata - launchToken?: string - launchAgent?: TuiAgent - /** Explicit CLI override for host-owned agent launches; omission uses host settings. */ - agentArgsOverride?: string | null - draftPrompt?: string - sessionOptions?: Record - /** Telemetry payload for `agent_started`. Forwarded to `pty:spawn` - * so main fires the event only after the spawn succeeds. */ - telemetry?: EventProps<'agent_started'> - /** Initial prompt-start status for agents that lack native prompt hooks. */ - initialAgentStatus?: { agent: TuiAgent; prompt: string } - /** Show the restored-session banner when this startup command mounts. */ - showSessionRestoredBanner?: boolean - /** Initial startup may be paired with a setup split that changes its grid. */ - waitForSetupSplitDirection?: SetupSplitDirection - } | null + startup?: PtyPaneStartup restoredLeafId?: string | null restoredPtyIdByLeafId?: Record /** Park intent sampled at render time, before the host disposes the tab's @@ -66,6 +75,7 @@ export type PtyConnectionDeps = { onPtyExitRef: React.RefObject<(ptyId: string) => void> onAgentExitedRef: React.RefObject<(leafId: string) => void> onPtyErrorRef?: React.RefObject<(paneId: number, message: string) => void> + onPaneProcessDied?: (processExit: PaneProcessExit) => void onPtyRecoveryStateRef?: React.RefObject< (paneId: number, state: PtyTransportRecoveryState | null) => void > diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 3050f81708e..e1927dd6ece 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -59,7 +59,11 @@ import { shouldReconcileMissingSession, type HasPty } from './terminal-dead-session-reconcile' -import type { PtyConnectionDeps } from './pty-connection-types' +import type { PtyConnectionDeps, PtyPaneStartup } from './pty-connection-types' +import { + createGitBashConsoleCapacityDetector, + type GitBashConsoleCapacityDetector +} from './git-bash-console-capacity' import type { SessionRestoredBannerReason } from './session-restored-banner-pane-state' import { consumeCommittedPtyShutdownExit, @@ -2659,7 +2663,11 @@ export function connectPanePty( }) return claimKey } - const onExit = (ptyId: string, opts: { preserveRendererBinding?: boolean } = {}): void => { + const onExit = ( + ptyId: string, + exitCode = 0, + opts: { preserveRendererBinding?: boolean } = {} + ): void => { if (handledExitPtyId === ptyId) { return } @@ -2667,7 +2675,7 @@ export function connectPanePty( // Why: the transport emits exit once; replay it only after a verified commit so rollback keeps renderer state retryable. deferPtyShutdownExit(ptyId, (settlement) => { if (settlement === 'committed') { - onExit(ptyId, { preserveRendererBinding: true }) + onExit(ptyId, exitCode, { preserveRendererBinding: true }) } }) return @@ -2681,6 +2689,7 @@ export function connectPanePty( // Why: an old transport can deliver a late exit after this pane has // rebound to a replacement PTY; only clear ownership for the exited id. handledExitPtyId = ptyId + processExitStateByPtyId.delete(ptyId) if (!preserveRendererBinding) { deps.clearTabPtyId(deps.tabId, ptyId) } @@ -2689,6 +2698,8 @@ export function connectPanePty( return } handledExitPtyId = ptyId + const processExitState = processExitStateByPtyId.get(ptyId) ?? currentProcessExitState + processExitStateByPtyId.delete(ptyId) agentCompletionCoordinator.dispose() dropSideEffectFactConsumer() // Why: main clears gate state on PTY exit too; this only resets the @@ -2765,6 +2776,17 @@ export function connectPanePty( return } manager.setPaneGpuRendering(pane.id, true) + const failedLocalProcess = !connectionId && runtimeEnvironmentId === null && exitCode !== 0 + if (failedLocalProcess && deps.onPaneProcessDied) { + const gitBashConsoleCapacityFailure = processExitState.detector.detected() + deps.onPaneProcessDied({ + paneId: pane.id, + exitCode, + startup: gitBashConsoleCapacityFailure ? processExitState.startup : null, + reason: gitBashConsoleCapacityFailure ? 'git-bash-console-capacity' : 'process-failed' + }) + return + } const panes = manager.getPanes() if (panes.length <= 1) { // Why: a worktree's sole newborn terminal can die on shell startup — e.g. @@ -3030,6 +3052,25 @@ export function connectPanePty( } const observeTerminalGitHubPRLink = createTerminalGitHubPRLinkDetector() + type ProcessExitState = { + startup: PtyPaneStartup + detector: GitBashConsoleCapacityDetector + } + const createProcessExitState = (startup: PtyPaneStartup): ProcessExitState => ({ + startup, + detector: createGitBashConsoleCapacityDetector() + }) + let currentProcessExitState = createProcessExitState(paneStartup) + const processExitStateByPtyId = new Map() + const bindProcessExitState = (ptyId: string, replacedPtyId?: string): void => { + const state = + (replacedPtyId ? processExitStateByPtyId.get(replacedPtyId) : undefined) ?? + currentProcessExitState + processExitStateByPtyId.set(ptyId, state) + if (replacedPtyId && replacedPtyId !== ptyId) { + processExitStateByPtyId.delete(replacedPtyId) + } + } const reportPanePtyVisibility = (ptyId: string | null | undefined, visible: boolean): void => { if (!ptyId || isRemoteRuntimePtyId(ptyId)) { // Why: remote-runtime PTYs use a relay path outside main's local @@ -3047,6 +3088,7 @@ export function connectPanePty( sampleVisibleForegroundAgent?: boolean } = {} ): void => { + bindProcessExitState(ptyId, options.replacePtyId) if (activePanePtyBinding && activePanePtyBinding !== ptyId) { reportPanePtyVisibility(activePanePtyBinding, false) } @@ -5205,6 +5247,20 @@ export function connectPanePty( : {}) } : undefined + const toProcessExitStartup = ( + startup: PendingStartupCommand | ColdRestoreAgentResumeStartup | null + ): PtyPaneStartup => + startup && 'launchConfig' in startup && 'agent' in startup + ? { + command: startup.command, + env: startup.env, + launchConfig: startup.launchConfig, + resumeProviderSession: startup.resumeProviderSession, + launchToken: startup.launchToken, + launchAgent: startup.agent, + showSessionRestoredBanner: true + } + : startup const startFreshColdRestoreAgentResume = ( startup: ColdRestoreAgentResumeStartup | null = buildColdRestoreAgentResumeStartup(), options: FreshSpawnOptions = {} @@ -5368,7 +5424,11 @@ export function connectPanePty( : window.api.pty.declarePendingPaneSerializer(cacheKey).catch(() => null) transportConnectInFlightSince = Date.now() - const outputCallbacks = captureTransportOutputCallbacks(reportError) + const effectiveStartup = startupOverride === undefined ? paneStartup : startupOverride + const outputCallbacks = captureTransportOutputCallbacks( + reportError, + toProcessExitStartup(coldRestoreOverride ?? effectiveStartup) + ) const spawnedRaw = transport.connect({ url: '', cols, @@ -5873,7 +5933,10 @@ export function connectPanePty( scheduleReplayDataDrain() } - const captureTransportOutputCallbacks = (onError: (message: string) => void) => { + const captureTransportOutputCallbacks = ( + onError: (message: string) => void, + startup: PtyPaneStartup + ) => { // Why: a new stream generation cannot inherit an old replay's pending // destination-grid fit or keep its live-data waiter open. pendingHiddenSnapshotFit?.cancel() @@ -5881,6 +5944,8 @@ export function connectPanePty( pendingReattachFit?.cancel() pendingReattachFit = null const generation = (transportStreamGeneration += 1) + const processExitState = createProcessExitState(startup) + currentProcessExitState = processExitState const isCurrent = (): boolean => !disposed && generation === transportStreamGeneration return { generation, @@ -5907,6 +5972,7 @@ export function connectPanePty( }, onData: (data: string, meta?: PtyDataMeta): void => { if (isCurrent()) { + processExitState.detector.observe(data) dataCallback(data, meta, generation) } }, @@ -8312,6 +8378,7 @@ export function connectPanePty( }) return false } + bindProcessExitState(ptyId) setPanePtyFitBinding(ptyId) reportPanePtyVisibility(ptyId, deps.isVisibleRef.current) registerSideEffectFactConsumerForPty(ptyId) @@ -8722,7 +8789,7 @@ export function connectPanePty( authoritativeReattachGeneration += 1 clearPaneMode2031State() clearHiddenOutputRestoreState() - const outputCallbacks = captureTransportOutputCallbacks(reportError) + const outputCallbacks = captureTransportOutputCallbacks(reportError, null) transport.attach({ existingPtyId: ptyId, callbacks: outputCallbacks.callbacks @@ -8905,16 +8972,19 @@ export function connectPanePty( const coldRestoreStartup = buildColdRestoreAgentResumeStartup() clearPaneMode2031State() clearHiddenOutputRestoreState() - const outputCallbacks = captureTransportOutputCallbacks((message) => { - if (isSshSessionExpiredError(message)) { - expiredReattachError = true - return - } - if (!isCapturedDirectSshReattachCurrent(pendingSessionId)) { - return - } - reportError(message) - }) + const outputCallbacks = captureTransportOutputCallbacks( + (message) => { + if (isSshSessionExpiredError(message)) { + expiredReattachError = true + return + } + if (!isCapturedDirectSshReattachCurrent(pendingSessionId)) { + return + } + reportError(message) + }, + toProcessExitStartup(coldRestoreStartup ?? paneStartup) + ) beginReattachLiveDataDeferral(outputCallbacks.generation) transportConnectInFlightSince = Date.now() const reattachPromise = transport.connect({ @@ -9152,16 +9222,19 @@ export function connectPanePty( let expiredReattachError = false const coldRestoreStartup = buildColdRestoreAgentResumeStartup() - const outputCallbacks = captureTransportOutputCallbacks((message) => { - if (isSshSessionExpiredError(message)) { - expiredReattachError = true - return - } - if (!isCapturedDirectSshReattachCurrent(deferredReattachSessionId)) { - return - } - reportError(message) - }) + const outputCallbacks = captureTransportOutputCallbacks( + (message) => { + if (isSshSessionExpiredError(message)) { + expiredReattachError = true + return + } + if (!isCapturedDirectSshReattachCurrent(deferredReattachSessionId)) { + return + } + reportError(message) + }, + toProcessExitStartup(coldRestoreStartup ?? paneStartup) + ) beginReattachLiveDataDeferral(outputCallbacks.generation) transportConnectInFlightSince = Date.now() const reattachPromise = transport.connect({ @@ -9303,7 +9376,7 @@ export function connectPanePty( try { clearPaneMode2031State() clearHiddenOutputRestoreState() - const outputCallbacks = captureTransportOutputCallbacks(reportError) + const outputCallbacks = captureTransportOutputCallbacks(reportError, null) transport.attach({ existingPtyId: attachPtyId, cols, @@ -9358,7 +9431,7 @@ export function connectPanePty( } clearPaneMode2031State() clearHiddenOutputRestoreState() - const outputCallbacks = captureTransportOutputCallbacks(reportError) + const outputCallbacks = captureTransportOutputCallbacks(reportError, null) transport.attach({ existingPtyId: spawnedPtyId, cols, diff --git a/src/renderer/src/components/terminal-pane/pty-transport-detach-attach-handoff.test.ts b/src/renderer/src/components/terminal-pane/pty-transport-detach-attach-handoff.test.ts index ee9b867c538..7cac9384052 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-detach-attach-handoff.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-detach-attach-handoff.test.ts @@ -165,7 +165,7 @@ describe('createIpcPtyTransport', () => { onExit?.({ id: 'pty-detached', code: 0 }) - expect(onPtyExit).toHaveBeenCalledWith('pty-detached') + expect(onPtyExit).toHaveBeenCalledWith('pty-detached', 0) expect(transport.getPtyId()).toBeNull() }) diff --git a/src/renderer/src/components/terminal-pane/pty-transport-handler-suspension.test.ts b/src/renderer/src/components/terminal-pane/pty-transport-handler-suspension.test.ts index 5341033c8b2..9823d5b5282 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-handler-suspension.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-handler-suspension.test.ts @@ -54,7 +54,7 @@ describe('createIpcPtyTransport', () => { onExit?.({ id: 'pty-new', code: 0 }) - expect(onPtyExit).toHaveBeenCalledWith('pty-new') + expect(onPtyExit).toHaveBeenCalledWith('pty-new', 0) expect(transport.getPtyId()).toBeNull() expect(transport.isConnected()).toBe(false) }) @@ -127,7 +127,7 @@ describe('createIpcPtyTransport', () => { // Exit handler should still work (exit handlers are kept alive) onExit?.({ id: 'pty-1', code: -1 }) - expect(onPtyExit).toHaveBeenCalledWith('pty-1') + expect(onPtyExit).toHaveBeenCalledWith('pty-1', -1) }) it('marks a host reversible-stop exit before delivering it to the pane', async () => { diff --git a/src/renderer/src/components/terminal-pane/pty-transport-reattach-admission.test.ts b/src/renderer/src/components/terminal-pane/pty-transport-reattach-admission.test.ts index bc36037c6cb..41014dae389 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-reattach-admission.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-reattach-admission.test.ts @@ -254,7 +254,7 @@ describe('createIpcPtyTransport', () => { expect(onDataCallback).toHaveBeenCalledWith('final output') expect(onExitCallback).toHaveBeenCalledWith(17) expect(onDisconnect).toHaveBeenCalledTimes(1) - expect(onPtyExit).toHaveBeenCalledWith(sessionId) + expect(onPtyExit).toHaveBeenCalledWith(sessionId, 17) expect(transport.isConnected()).toBe(false) }) diff --git a/src/renderer/src/components/terminal-pane/pty-transport-types.ts b/src/renderer/src/components/terminal-pane/pty-transport-types.ts index 57e0c9e2e4e..d8bdc63f62a 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-types.ts @@ -245,7 +245,7 @@ export type IpcPtyTransportOptions = { projectRuntime?: ProjectExecutionRuntimeResolution terminalColorQueryReplies?: TerminalOscColorQueryReplyColors telemetry?: EventProps<'agent_started'> - onPtyExit?: (ptyId: string) => void + onPtyExit?: (ptyId: string, exitCode?: number) => void onTitleChange?: (title: string, rawTitle: string) => void onPtySpawn?: (ptyId: string) => void /** Rebind an existing pane after its provider replaces the PTY identity. */ diff --git a/src/renderer/src/components/terminal-pane/pty-transport.ts b/src/renderer/src/components/terminal-pane/pty-transport.ts index 9eddfae0155..f713ee6c2f9 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport.ts @@ -756,7 +756,7 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra unregisterPtyHandlers(id) storedCallbacks.onExit?.(code) storedCallbacks.onDisconnect?.() - onPtyExit?.(id) + onPtyExit?.(id, code) } ptyExitHandlers.set(id, exitHandler) ownedExitHandlers.set(id, exitHandler) diff --git a/src/renderer/src/components/terminal-pane/terminal-process-exit-restart.test.ts b/src/renderer/src/components/terminal-pane/terminal-process-exit-restart.test.ts new file mode 100644 index 00000000000..78d4f469361 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-process-exit-restart.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, it } from 'vitest' +import { resolveTerminalProcessExitRestartStartup } from './terminal-process-exit-restart' + +describe('terminal process exit restart', () => { + it('retries the original startup after a Git Bash capacity failure', () => { + const startup = { command: 'codex --resume session-1' } + + expect( + resolveTerminalProcessExitRestartStartup({ + paneId: 1, + exitCode: 1, + reason: 'git-bash-console-capacity', + startup + }) + ).toBe(startup) + }) + + it('restarts a shell when a capacity failure had no startup request', () => { + expect( + resolveTerminalProcessExitRestartStartup({ + paneId: 1, + exitCode: 1, + reason: 'git-bash-console-capacity', + startup: null + }) + ).toBeNull() + }) + + it('restarts other process failures as a shell', () => { + expect( + resolveTerminalProcessExitRestartStartup({ + paneId: 1, + exitCode: 7, + reason: 'process-failed', + startup: { command: 'codex' } + }) + ).toBeNull() + }) +}) diff --git a/src/renderer/src/components/terminal-pane/terminal-process-exit-restart.ts b/src/renderer/src/components/terminal-pane/terminal-process-exit-restart.ts new file mode 100644 index 00000000000..d5e0ea5e4a2 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-process-exit-restart.ts @@ -0,0 +1,7 @@ +import type { PaneProcessExit, PtyPaneStartup } from './pty-connection-types' + +export function resolveTerminalProcessExitRestartStartup( + processExit: PaneProcessExit +): PtyPaneStartup { + return processExit.reason === 'git-bash-console-capacity' ? processExit.startup : null +} diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts index 91c98a4285f..16c00e6c64e 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts @@ -26,6 +26,7 @@ import { buildTerminalKeyboardProtocolOptions } from '@/lib/pane-manager/termina import { resolvePaneKeyboardProtocolAgent } from './terminal-keyboard-protocol-pane-agent' import { useAppStore } from '@/store' import type { DirectSshPaneRetryAttemptId } from '@/store/slices/direct-ssh-terminal-recovery' +import type { PaneProcessExit } from './pty-connection-types' import { createFilePathLinkProvider, getTerminalFileOpenHint, @@ -292,6 +293,7 @@ type UseTerminalPaneLifecycleDeps = { onPtyExitRef: React.RefObject<(ptyId: string) => void> onAgentExitedRef: React.RefObject<(leafId: string) => void> onPtyErrorRef?: React.RefObject<(paneId: number, message: string) => void> + onPaneProcessDied?: (processExit: PaneProcessExit) => void onPtyRecoveryStateRef?: React.RefObject< (paneId: number, state: PtyTransportRecoveryState | null) => void > @@ -698,6 +700,7 @@ export function useTerminalPaneLifecycle({ onPtyExitRef, onAgentExitedRef, onPtyErrorRef, + onPaneProcessDied, onPtyRecoveryStateRef, clearTabPtyId, consumeSuppressedPtyExit, @@ -940,6 +943,7 @@ export function useTerminalPaneLifecycle({ onPtyExitRef, onAgentExitedRef, onPtyErrorRef, + onPaneProcessDied, onPtyRecoveryStateRef, clearTabPtyId, consumeSuppressedPtyExit, diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 77aa006aff9..7f559834060 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -2934,6 +2934,14 @@ "e16012e31e": "The terminal daemon that owned this session exited, so the session and its scrollback could not be recovered. Open a new terminal to continue.", "sessionUnavailable": "Orca couldn't reattach to this pane's terminal session on the host. Open a new terminal to continue." }, + "TerminalProcessExitOverlay": { + "capacityTitle": "Git Bash console limit reached", + "capacityDetail": "Git Bash reached its 128-console limit. Close unused Git Bash terminals, then restart this terminal.", + "failedTitle": "Terminal exited", + "failedDetail": "The shell process ended with exit code {{code}}. Its output is preserved.", + "close": "Close", + "restart": "Restart" + }, "TerminalPane": { "ac112e9036": "Remove title", "f984ab2a30": "Remove pane title: {{value0}}", diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index 3689e21bd12..27136599f39 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -2740,6 +2740,14 @@ "5c8ce20be6": "Si esto persiste, por favor", "cc6d997c65": "Reinicia el servicio del terminal desde aquí para borrar el estado obsoleto." }, + "TerminalProcessExitOverlay": { + "capacityTitle": "Se alcanzó el límite de consolas de Git Bash", + "capacityDetail": "Git Bash alcanzó su límite de 128 consolas. Cierra las terminales de Git Bash que no uses y reinicia esta terminal.", + "failedTitle": "La terminal se cerró", + "failedDetail": "El proceso del shell terminó con el código de salida {{code}}. Se conservó su salida.", + "close": "Cerrar", + "restart": "Reiniciar" + }, "TerminalPane": { "ac112e9036": "Quitar título", "f984ab2a30": "Eliminar título del panel: {{value0}}", diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index 304d5f1d5eb..1a2f3e9f95a 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -2740,6 +2740,14 @@ "5c8ce20be6": "この状態が続く場合は、", "cc6d997c65": "ここからターミナルデーモンを再起動して、古いデーモン状態をクリアします。" }, + "TerminalProcessExitOverlay": { + "capacityTitle": "Git Bash のコンソール上限に達しました", + "capacityDetail": "Git Bash が 128 個のコンソール上限に達しました。不要な Git Bash ターミナルを閉じてから、このターミナルを再起動してください。", + "failedTitle": "ターミナルが終了しました", + "failedDetail": "シェルプロセスが終了コード {{code}} で終了しました。出力は保持されています。", + "close": "閉じる", + "restart": "再起動" + }, "TerminalPane": { "ac112e9036": "タイトルを削除", "f984ab2a30": "ペインのタイトルを削除: {{value0}}", diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index 1108cdd735e..a360d1331ec 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -2742,6 +2742,14 @@ "5c8ce20be6": "문제가 계속되면", "cc6d997c65": "오래된 데몬 상태를 지우려면 여기에서 terminal 데몬을 다시 시작하세요." }, + "TerminalProcessExitOverlay": { + "capacityTitle": "Git Bash 콘솔 한도에 도달했습니다", + "capacityDetail": "Git Bash가 128개 콘솔 한도에 도달했습니다. 사용하지 않는 Git Bash 터미널을 닫은 후 이 터미널을 다시 시작하세요.", + "failedTitle": "터미널이 종료되었습니다", + "failedDetail": "셸 프로세스가 종료 코드 {{code}}로 끝났습니다. 출력은 보존됩니다.", + "close": "닫기", + "restart": "다시 시작" + }, "TerminalPane": { "ac112e9036": "제목 삭제", "f984ab2a30": "창 제목 제거: {{value0}}", diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index cbb41b0fa66..1774a4afa4b 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -2752,6 +2752,14 @@ "5c8ce20be6": "如果这种情况持续存在，请", "cc6d997c65": "从此处重新启动终端守护进程以清除失效的守护进程状态。" }, + "TerminalProcessExitOverlay": { + "capacityTitle": "已达到 Git Bash 控制台上限", + "capacityDetail": "Git Bash 已达到 128 个控制台的上限。关闭未使用的 Git Bash 终端，然后重新启动此终端。", + "failedTitle": "终端已退出", + "failedDetail": "Shell 进程以退出代码 {{code}} 结束。输出已保留。", + "close": "关闭", + "restart": "重新启动" + }, "TerminalPane": { "ac112e9036": "删除标题", "f984ab2a30": "删除窗格标题：{{value0}}", From b2902cb61e1534098e3d6a1fe5a32f11d3cd76fb Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 23 Aug 2026 02:26:52 -0700 Subject: [PATCH 10/14] fix(agent-resume): restore Kimi Code sessions after restart (#15883) Co-authored-by: Melih --- .../session/ai-vault-resume-launch.test.ts | 19 +++++ ...-pty-transport-host-session-launch.test.ts | 83 +++++++++++++++++++ .../remote-runtime-pty-transport.ts | 13 +-- ...ult-resume-command.resumable-agent.test.ts | 83 +++++++++++++++++++ ...t-resume-host-authority-capability.test.ts | 57 +++++++++++++ .../agent-resume-host-authority-capability.ts | 45 ++++++++++ .../remote-agent-session-launch.test.ts | 20 +++++ ...me-session-terminal-host-authority.test.ts | 70 ++++++++++++++++ .../src/runtime/web-runtime-session.ts | 12 +-- ...atus-quit-capture-resumable-agents.test.ts | 42 ++++++++++ src/shared/agent-session-resume.test.ts | 18 +++- src/shared/agent-session-resume.ts | 6 +- src/shared/protocol-version.ts | 5 ++ ...pace-session-schema.sleeping-agent.test.ts | 36 ++++++++ 14 files changed, 494 insertions(+), 15 deletions(-) create mode 100644 src/renderer/src/lib/ai-vault-resume-command.resumable-agent.test.ts create mode 100644 src/renderer/src/runtime/agent-resume-host-authority-capability.test.ts create mode 100644 src/renderer/src/runtime/agent-resume-host-authority-capability.ts create mode 100644 src/renderer/src/store/slices/agent-status-quit-capture-resumable-agents.test.ts diff --git a/mobile/src/session/ai-vault-resume-launch.test.ts b/mobile/src/session/ai-vault-resume-launch.test.ts index d021a48bc6f..7db13fc7e78 100644 --- a/mobile/src/session/ai-vault-resume-launch.test.ts +++ b/mobile/src/session/ai-vault-resume-launch.test.ts @@ -121,6 +121,25 @@ describe('buildMobileAiVaultResumeCommand', () => { }) describe('buildMobileAiVaultResumeLaunch', () => { + it('routes Kimi through the resumable-agent startup plan', () => { + // Why: kimi joining RESUMABLE_TUI_AGENTS moves it off the plain command fallback, so the + // cd prefix Kimi needs (sessions are work-dir-scoped) must survive the new branch. + const launch = buildMobileAiVaultResumeLaunch({ + session: session({ + agent: 'kimi', + sessionId: 'session_431324d7-2165-42f0-9ecd-9f93437b3201' + }), + hostPlatform: 'darwin' + }) + + expect(launch).toMatchObject({ + command: + "cd '/Users/ada/repo' && kimi '--yolo' '--session' 'session_431324d7-2165-42f0-9ecd-9f93437b3201'", + launchConfig: { agentCommand: "kimi '--yolo'" }, + launchAgent: 'kimi' + }) + }) + it('preserves an arbitrary OMP transcript locator for later cold resume', () => { const launch = buildMobileAiVaultResumeLaunch({ session: session({ diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-host-session-launch.test.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-host-session-launch.test.ts index 12eb9b672aa..6185198d59e 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-host-session-launch.test.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-host-session-launch.test.ts @@ -348,6 +348,89 @@ describe('createRemoteRuntimePtyTransport', () => { ) }) + it('degrades a Kimi resume to a legacy launch when the host predates the resume capability', async () => { + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const transport = createRemoteRuntimePtyTransport('env-1', { + worktreeId: 'repo1::/remote/wt', + command: "kimi '--session' 'session_431324d7'", + launchAgent: 'kimi', + resumeProviderSession: { key: 'session_id', id: 'session_431324d7' }, + tabId: 'tab-1', + leafId: '11111111-1111-4111-8111-111111111111' + }) + + await transport.connect({ url: '', callbacks: {} }) + + // Why: the beforeEach host advertises host-authority.v1 but not the Kimi gate, and it answers + // the widened ensureAgentSession enum with invalid_argument — not a fallback code. Only the + // per-agent probe keeps the pane alive. + expect(runtimeCall).not.toHaveBeenCalledWith( + expect.objectContaining({ method: 'terminal.ensureAgentSession' }) + ) + expect(runtimeCall).toHaveBeenCalledWith( + expect.objectContaining({ + method: 'terminal.create', + params: expect.objectContaining({ command: "kimi '--session' 'session_431324d7'" }) + }) + ) + }) + + it('claims the Kimi provider session on a host that advertises the resume capability', async () => { + runtimeCall.mockImplementation(async (args: { method?: string }) => + args.method === 'status.get' + ? { + id: 'rpc-status', + ok: true, + result: { + runtimeProtocolVersion: 3, + minCompatibleRuntimeClientVersion: 2, + capabilities: ['agent-session.host-authority.v1', 'agent-session.kimi-resume.v1'] + }, + _meta: { runtimeId: 'runtime-remote' } + } + : { + id: 'rpc-create', + ok: true, + result: { + terminal: { + handle: 'term-remote', + worktreeId: 'repo1::/remote/wt', + title: null, + surface: 'background' + } + }, + _meta: { runtimeId: 'runtime-remote' } + } + ) + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const transport = createRemoteRuntimePtyTransport('env-1', { + worktreeId: 'repo1::/remote/wt', + command: "kimi '--session' 'session_431324d7'", + launchAgent: 'kimi', + resumeProviderSession: { key: 'session_id', id: 'session_431324d7' }, + tabId: 'tab-1', + leafId: '11111111-1111-4111-8111-111111111111' + }) + + await transport.connect({ url: '', callbacks: {} }) + + expect(runtimeCall).toHaveBeenCalledWith( + expect.objectContaining({ + method: 'terminal.ensureAgentSession', + params: expect.objectContaining({ + agent: 'kimi', + providerSession: { key: 'session_id', id: 'session_431324d7' } + }) + }) + ) + expect(runtimeCall).not.toHaveBeenCalledWith( + expect.objectContaining({ + method: 'terminal.create', + params: expect.objectContaining({ command: expect.any(String) }) + }) + ) + }) + it('treats an explicitly killed remote session as normal retirement', async () => { runtimeCall.mockImplementation(async (args: { method?: string }) => args.method === 'terminal.create' diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts index 1ea15c9c0dd..7d50b9f48de 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts @@ -19,10 +19,8 @@ import type { RuntimeTerminalResolvePane, RuntimeTerminalSend } from '../../../../shared/runtime-types' -import { - AGENT_SESSION_OMP_RESUME_PATH_RUNTIME_CAPABILITY, - TERMINAL_CREATE_IDEMPOTENCY_RUNTIME_CAPABILITY -} from '../../../../shared/protocol-version' +import { TERMINAL_CREATE_IDEMPOTENCY_RUNTIME_CAPABILITY } from '../../../../shared/protocol-version' +import { agentResumeHostAuthorityCapability } from '../../runtime/agent-resume-host-authority-capability' import { isTerminalInputTooLargeWithDeferredMeasurement, iterateTerminalInputChunks @@ -2104,14 +2102,17 @@ export function createRemoteRuntimePtyTransport( createEnvironmentId, connectLifecycleEpoch ) + const resumeHostAuthorityCapability = resumeProviderSessionToSend + ? agentResumeHostAuthorityCapability(launchAgentToSend) + : undefined const created = launchAgentToSend ? agentSessionRequiresHostAuthorityReplay ? await hostAuthorityCreate() : await runRemoteAgentSessionLaunch({ environmentId: createEnvironmentId, hostAuthority: hostAuthorityCreate, - ...(resumeProviderSessionToSend && launchAgentToSend === 'omp' - ? { hostAuthorityCapability: AGENT_SESSION_OMP_RESUME_PATH_RUNTIME_CAPABILITY } + ...(resumeHostAuthorityCapability + ? { hostAuthorityCapability: resumeHostAuthorityCapability } : {}), legacy: legacyCreate }) diff --git a/src/renderer/src/lib/ai-vault-resume-command.resumable-agent.test.ts b/src/renderer/src/lib/ai-vault-resume-command.resumable-agent.test.ts new file mode 100644 index 00000000000..b0cdda90fd3 --- /dev/null +++ b/src/renderer/src/lib/ai-vault-resume-command.resumable-agent.test.ts @@ -0,0 +1,83 @@ +import { describe, expect, it, vi } from 'vitest' +import type { AppState } from '@/store/types' +import { + buildAiVaultResumeCopyCommandForWorktree, + buildAiVaultResumeStartupForWorktree +} from './ai-vault-resume-command' + +vi.mock('@/lib/new-workspace', () => ({ + CLIENT_PLATFORM: 'darwin' +})) + +type ResumableAgentState = Pick< + AppState, + | 'activeRepoId' + | 'activeWorktreeId' + | 'folderWorkspaces' + | 'projectGroups' + | 'projects' + | 'repos' + | 'settings' + | 'worktreesByRepo' +> + +function makeState(): ResumableAgentState { + return { + activeRepoId: 'repo-1', + activeWorktreeId: 'repo-1::worktree-1', + folderWorkspaces: [], + projectGroups: [], + repos: [{ id: 'repo-1', path: '/Users/ada/repo' }], + projects: [{ id: 'repo-1', sourceRepoIds: ['repo-1'] }], + settings: { + agentDefaultArgs: { claude: '', codex: '' }, + agentDefaultEnv: { claude: {}, codex: {} } + }, + worktreesByRepo: { + 'repo-1': [{ id: 'repo-1::worktree-1', repoId: 'repo-1', path: '/Users/ada/repo' }] + } + } as unknown as ResumableAgentState +} + +const KIMI_SESSION = { + agent: 'kimi' as const, + sessionId: 'session_431324d7-2165-42f0-9ecd-9f93437b3201', + cwd: '/Users/ada/repo/packages/api', + codexHome: null +} + +describe('AI Vault resume for Kimi', () => { + it('keeps the cd prefix on the copied resume line', () => { + // Why: Kimi sessions are work-dir-scoped — resuming from the worktree root instead of the + // session's own cwd fails with "created under a different directory". + expect( + buildAiVaultResumeCopyCommandForWorktree({ + state: makeState(), + worktreeId: 'repo-1::worktree-1', + session: KIMI_SESSION + }) + ).toBe( + "cd '/Users/ada/repo/packages/api' && kimi '--yolo' '--session' 'session_431324d7-2165-42f0-9ecd-9f93437b3201'" + ) + }) + + it('takes the resumable-agent startup plan so the pane claims the provider session', () => { + // Why: the plan branch applies Kimi's default launch args, so a resumed pane starts with the + // same permission flag Orca gives a fresh one. + expect( + buildAiVaultResumeStartupForWorktree({ + state: makeState(), + worktreeId: 'repo-1::worktree-1', + session: KIMI_SESSION + }) + ).toMatchObject({ + command: "kimi '--yolo' '--session' 'session_431324d7-2165-42f0-9ecd-9f93437b3201'", + cwd: '/Users/ada/repo/packages/api', + launchConfig: { agentCommand: "kimi '--yolo'" }, + providerSession: { + key: 'session_id', + id: 'session_431324d7-2165-42f0-9ecd-9f93437b3201' + } + }) + }) +}) diff --git a/src/renderer/src/runtime/agent-resume-host-authority-capability.test.ts b/src/renderer/src/runtime/agent-resume-host-authority-capability.test.ts new file mode 100644 index 00000000000..83b6a9493d9 --- /dev/null +++ b/src/renderer/src/runtime/agent-resume-host-authority-capability.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, it } from 'vitest' +import { RESUMABLE_TUI_AGENTS } from '../../../shared/agent-session-resume' +import { + AGENT_SESSION_KIMI_RESUME_RUNTIME_CAPABILITY, + AGENT_SESSION_OMP_RESUME_PATH_RUNTIME_CAPABILITY, + RUNTIME_CAPABILITIES +} from '../../../shared/protocol-version' +import { agentResumeHostAuthorityCapability } from './agent-resume-host-authority-capability' + +describe('agentResumeHostAuthorityCapability', () => { + it('gates Kimi resume behind its own capability', () => { + expect(agentResumeHostAuthorityCapability('kimi')).toBe( + AGENT_SESSION_KIMI_RESUME_RUNTIME_CAPABILITY + ) + }) + + it('keeps the OMP resume-path gate', () => { + expect(agentResumeHostAuthorityCapability('omp')).toBe( + AGENT_SESSION_OMP_RESUME_PATH_RUNTIME_CAPABILITY + ) + }) + + it('leaves agents shipped with host authority on the generic probe', () => { + expect(agentResumeHostAuthorityCapability('codex')).toBeUndefined() + expect(agentResumeHostAuthorityCapability(null)).toBeUndefined() + expect(agentResumeHostAuthorityCapability(undefined)).toBeUndefined() + }) + + it('advertises the Kimi resume capability from the host', () => { + expect(RUNTIME_CAPABILITIES).toContain(AGENT_SESSION_KIMI_RESUME_RUNTIME_CAPABILITY) + }) + + it('pins the gate for every resumable agent so a new member is a deliberate decision', () => { + // Why: silently defaulting a newly resumable agent to the generic probe is the exact skew + // failure this module exists to prevent — the mapping must be reviewed, not inherited. + expect( + Object.fromEntries( + RESUMABLE_TUI_AGENTS.map((agent) => [agent, agentResumeHostAuthorityCapability(agent)]) + ) + ).toEqual({ + claude: undefined, + codex: undefined, + gemini: undefined, + antigravity: undefined, + opencode: undefined, + pi: undefined, + 'mimo-code': undefined, + droid: undefined, + grok: undefined, + devin: undefined, + 'prime-agent': undefined, + copilot: undefined, + omp: AGENT_SESSION_OMP_RESUME_PATH_RUNTIME_CAPABILITY, + kimi: AGENT_SESSION_KIMI_RESUME_RUNTIME_CAPABILITY + }) + }) +}) diff --git a/src/renderer/src/runtime/agent-resume-host-authority-capability.ts b/src/renderer/src/runtime/agent-resume-host-authority-capability.ts new file mode 100644 index 00000000000..b1f83f1d5dc --- /dev/null +++ b/src/renderer/src/runtime/agent-resume-host-authority-capability.ts @@ -0,0 +1,45 @@ +import type { ResumableTuiAgent } from '../../../shared/agent-session-resume' +import type { TuiAgent } from '../../../shared/tui-agent' +import { + AGENT_SESSION_KIMI_RESUME_RUNTIME_CAPABILITY, + AGENT_SESSION_OMP_RESUME_PATH_RUNTIME_CAPABILITY, + type RuntimeCapability +} from '../../../shared/protocol-version' + +// Why: every agent added to RESUMABLE_TUI_AGENTS after agent-session.host-authority.v1 widens the +// host's ensureAgentSession enum. An older host answers the unknown member with invalid_argument, +// which runRemoteAgentSessionLaunch does not treat as a fallback signal, so the pane dies instead +// of degrading to a legacy launch. Probing the agent's own capability keeps version skew safe. +// The exhaustive Record makes the next RESUMABLE_TUI_AGENTS entry a compile error until its own +// gate — or a deliberate `undefined` — is declared here. +const RESUME_HOST_AUTHORITY_CAPABILITY_BY_AGENT = { + // These shipped inside agent-session.host-authority.v1's enum, so the generic probe covers them. + claude: undefined, + codex: undefined, + gemini: undefined, + antigravity: undefined, + opencode: undefined, + pi: undefined, + 'mimo-code': undefined, + droid: undefined, + grok: undefined, + devin: undefined, + // Ungated on purpose: prime-agent's host-side status hooks and its enum entry both shipped in + // a1f61ef, so no host emits a prime-agent provider session without also accepting the member. + 'prime-agent': undefined, + // Ungated to match how main shipped copilot resume; gating it is its own change. + copilot: undefined, + omp: AGENT_SESSION_OMP_RESUME_PATH_RUNTIME_CAPABILITY, + kimi: AGENT_SESSION_KIMI_RESUME_RUNTIME_CAPABILITY +} satisfies Record + +export function agentResumeHostAuthorityCapability( + agent: TuiAgent | null | undefined +): RuntimeCapability | undefined { + if (!agent) { + return undefined + } + return ( + RESUME_HOST_AUTHORITY_CAPABILITY_BY_AGENT as Partial> + )[agent] +} diff --git a/src/renderer/src/runtime/remote-agent-session-launch.test.ts b/src/renderer/src/runtime/remote-agent-session-launch.test.ts index 0fb6399d688..a282b3b5878 100644 --- a/src/renderer/src/runtime/remote-agent-session-launch.test.ts +++ b/src/renderer/src/runtime/remote-agent-session-launch.test.ts @@ -17,6 +17,7 @@ vi.mock('./runtime-rpc-client', () => ({ import { RuntimeRpcCallError } from './runtime-rpc-client' import { runRemoteAgentSessionLaunch } from './remote-agent-session-launch' +import { agentResumeHostAuthorityCapability } from './agent-resume-host-authority-capability' describe('remote agent-session launch routing', () => { beforeEach(() => { @@ -44,6 +45,25 @@ describe('remote agent-session launch routing', () => { expect(legacy).not.toHaveBeenCalled() }) + it('falls back to legacy when an older host lacks the Kimi resume capability', async () => { + const hostAuthority = vi.fn().mockResolvedValue('structured') + const legacy = vi.fn().mockResolvedValue('legacy') + mocks.supportsCapability.mockResolvedValue(false) + + // Why: an old host rejects the widened agent enum with invalid_argument, which is not a + // fallback code — so the probe, not the error handler, has to keep the pane alive. + await expect( + runRemoteAgentSessionLaunch({ + environmentId: 'env-1', + hostAuthority, + hostAuthorityCapability: agentResumeHostAuthorityCapability('kimi'), + legacy + }) + ).resolves.toBe('legacy') + expect(mocks.supportsCapability).toHaveBeenCalledWith('env-1', 'agent-session.kimi-resume.v1') + expect(hostAuthority).not.toHaveBeenCalled() + }) + it('preserves the exact legacy path when the capability is absent', async () => { const hostAuthority = vi.fn().mockResolvedValue('structured') const legacy = vi.fn().mockResolvedValue('legacy') diff --git a/src/renderer/src/runtime/web-runtime-session-terminal-host-authority.test.ts b/src/renderer/src/runtime/web-runtime-session-terminal-host-authority.test.ts index 13ec579a2f7..209cd182da9 100644 --- a/src/renderer/src/runtime/web-runtime-session-terminal-host-authority.test.ts +++ b/src/renderer/src/runtime/web-runtime-session-terminal-host-authority.test.ts @@ -191,6 +191,76 @@ describe('createWebRuntimeSessionTerminal', () => { } ) + it.each([ + { gated: false, authority: false }, + { gated: true, authority: true } + ])( + 'routes a Kimi resume by capability (advertised=$gated) instead of the generic host-authority probe', + async ({ gated, authority }) => { + // Why: an old host answers the widened ensureAgentSession enum with invalid_argument, which + // runRemoteAgentSessionLaunch does not retry on — the per-agent probe is the only thing + // keeping a remote Kimi resume from dying instead of degrading to a legacy launch. + const runtimeCall = vi.fn(async (request: { method: string }) => { + if (request.method === 'status.get') { + return { + id: 'status', + ok: true, + result: { + runtimeId: 'runtime-1', + graphStatus: 'ready', + runtimeProtocolVersion: 3, + minCompatibleRuntimeClientVersion: 2, + capabilities: [ + 'agent-session.host-authority.v1', + ...(gated ? ['agent-session.kimi-resume.v1'] : []) + ] + } + } + } + if (request.method === 'terminal.ensureAgentSession') { + return { + id: 'ensure', + ok: true, + result: { + terminal: { + handle: 'term-kimi', + worktreeId: WORKTREE_ID, + tabId: 'host-tab-kimi', + paneKey: `host-tab-kimi:${FOCUS_LEAF_ID}` + }, + disposition: 'created' + } + } + } + if (request.method === 'session.tabs.createTerminal') { + return { + id: 'legacy-create', + ok: true, + result: { tab: { id: 'host-tab-kimi', leafId: FOCUS_LEAF_ID } } + } + } + return { id: 'list', ok: true, result: makeSnapshot() } + }) + vi.stubGlobal('window', { + api: { runtimeEnvironments: { call: runtimeCall } } + }) + + await expect( + createWebRuntimeSessionTerminal({ + worktreeId: WORKTREE_ID, + agentSessionKind: 'resume', + launchAgent: 'kimi', + command: "kimi '--session' 'session_431324d7'", + providerSession: { key: 'session_id', id: 'session_431324d7' } + }) + ).resolves.toEqual({ status: 'created' }) + + const methods = runtimeCall.mock.calls.map(([request]) => request.method) + expect(methods.includes('terminal.ensureAgentSession')).toBe(authority) + expect(methods.includes('session.tabs.createTerminal')).toBe(!authority) + } + ) + it('creates paired web agents through host authority so activation is mirrored', async () => { const snapshot = { ...makeSnapshot(), diff --git a/src/renderer/src/runtime/web-runtime-session.ts b/src/renderer/src/runtime/web-runtime-session.ts index 3bba0d64107..f8ceac02da2 100644 --- a/src/renderer/src/runtime/web-runtime-session.ts +++ b/src/renderer/src/runtime/web-runtime-session.ts @@ -19,10 +19,8 @@ import type { SleepingAgentLaunchConfig, AgentProviderSessionMetadata } from '../../../shared/agent-session-resume' -import { - AGENT_SESSION_OMP_RESUME_PATH_RUNTIME_CAPABILITY, - BROWSER_TAB_CREATE_KNOWN_ID_RUNTIME_CAPABILITY -} from '../../../shared/protocol-version' +import { BROWSER_TAB_CREATE_KNOWN_ID_RUNTIME_CAPABILITY } from '../../../shared/protocol-version' +import { agentResumeHostAuthorityCapability } from './agent-resume-host-authority-capability' import type { AgentLaunchPreferences, AgentPromptDelivery, @@ -337,13 +335,15 @@ async function createWebRuntimeSessionTerminalResult( })) as RuntimeRpcResponse ) ) + const resumeHostAuthorityCapability = + args.agentSessionKind === 'resume' ? agentResumeHostAuthorityCapability(agent) : undefined const created = await runRemoteAgentSessionLaunch<{ terminal: CreatedAgentTerminalIdentity }>({ environmentId, ...(hostAuthority ? { hostAuthority } : {}), - ...(args.agentSessionKind === 'resume' && agent === 'omp' - ? { hostAuthorityCapability: AGENT_SESSION_OMP_RESUME_PATH_RUNTIME_CAPABILITY } + ...(resumeHostAuthorityCapability + ? { hostAuthorityCapability: resumeHostAuthorityCapability } : {}), legacy: async () => { const response = await callEnvironment({ diff --git a/src/renderer/src/store/slices/agent-status-quit-capture-resumable-agents.test.ts b/src/renderer/src/store/slices/agent-status-quit-capture-resumable-agents.test.ts new file mode 100644 index 00000000000..7915f8a9b5d --- /dev/null +++ b/src/renderer/src/store/slices/agent-status-quit-capture-resumable-agents.test.ts @@ -0,0 +1,42 @@ +import { describe, expect, it } from 'vitest' +import type { AppState } from '../types' +import { createTestStore, makeTab } from './store-test-helpers' + +describe('quit-time capture for newly resumable agents', () => { + it('checkpoints a live Kimi provider session before quit-time capture', () => { + const store = createTestStore() + store.setState({ + tabsByWorktree: { + 'wt-1': [makeTab({ id: 'tab-1', worktreeId: 'wt-1' })] + } + } as Partial) + + store.getState().setAgentStatus( + 'tab-1:leaf-1', + { + state: 'working', + prompt: 'finish the task', + agentType: 'kimi' + }, + 'Kimi', + { updatedAt: 10, stateStartedAt: 10 }, + { tabId: 'tab-1', worktreeId: 'wt-1' }, + { + providerSession: { + key: 'session_id', + id: 'session_431324d7-2165-42f0-9ecd-9f93437b3201' + } + } + ) + + // Why: this is the #15155 regression — without kimi in RESUMABLE_TUI_AGENTS no sleeping + // record is ever captured, so restart drops the session instead of resuming it. + expect(store.getState().sleepingAgentSessionsByPaneKey['tab-1:leaf-1']).toMatchObject({ + agent: 'kimi', + worktreeId: 'wt-1', + tabId: 'tab-1', + providerSession: { key: 'session_id', id: 'session_431324d7-2165-42f0-9ecd-9f93437b3201' }, + origin: 'live' + }) + }) +}) diff --git a/src/shared/agent-session-resume.test.ts b/src/shared/agent-session-resume.test.ts index 0fffb9728a9..c6025936431 100644 --- a/src/shared/agent-session-resume.test.ts +++ b/src/shared/agent-session-resume.test.ts @@ -24,6 +24,10 @@ describe('agent session resume metadata', () => { expect(isResumableTuiAgent('copilot')).toBe(true) }) + it('treats Kimi Code as a resumable TUI agent', () => { + expect(isResumableTuiAgent('kimi')).toBe(true) + }) + it.each([ ['claude', { session_id: 'claude-session' }, { key: 'session_id', id: 'claude-session' }], ['codex', { session_id: 'codex-session' }, { key: 'session_id', id: 'codex-session' }], @@ -54,7 +58,12 @@ describe('agent session resume metadata', () => { { session_id: '940237d9-c712-48e8-bca1-fd75fc4a8d4b' }, { key: 'session_id', id: '940237d9-c712-48e8-bca1-fd75fc4a8d4b' } ], - ['copilot', { sessionId: 'copilot-camel' }, { key: 'session_id', id: 'copilot-camel' }] + ['copilot', { sessionId: 'copilot-camel' }, { key: 'session_id', id: 'copilot-camel' }], + [ + 'kimi', + { session_id: 'session_431324d7-2165-42f0-9ecd-9f93437b3201' }, + { key: 'session_id', id: 'session_431324d7-2165-42f0-9ecd-9f93437b3201' } + ] ] as const)('extracts %s provider session ids', (source, payload, expected) => { expect(extractAgentProviderSession(source, payload)).toEqual(expected) }) @@ -80,7 +89,12 @@ describe('agent session resume metadata', () => { { key: 'session_id', id: 's1', transcriptPath: '/tmp/prime-session.jsonl' }, ['prime-agent', '--resume', '/tmp/prime-session.jsonl'] ], - ['copilot', { key: 'session_id', id: 's1' }, ['copilot', '--resume=s1']] + ['copilot', { key: 'session_id', id: 's1' }, ['copilot', '--resume=s1']], + [ + 'kimi', + { key: 'session_id', id: 'session_431324d7' }, + ['kimi', '--session', 'session_431324d7'] + ] ] as const)('builds %s resume argv', (agent, providerSession, expected) => { expect(getAgentResumeArgv(agent, providerSession)).toEqual(expected) }) diff --git a/src/shared/agent-session-resume.ts b/src/shared/agent-session-resume.ts index 5cf14c8dd57..3e763fc20a6 100644 --- a/src/shared/agent-session-resume.ts +++ b/src/shared/agent-session-resume.ts @@ -15,7 +15,8 @@ export const RESUMABLE_TUI_AGENTS = [ 'devin', 'omp', 'prime-agent', - 'copilot' + 'copilot', + 'kimi' ] as const satisfies readonly TuiAgent[] export type ResumableTuiAgent = (typeof RESUMABLE_TUI_AGENTS)[number] @@ -287,5 +288,8 @@ export function getAgentResumeArgv( // resume commands, so local and remote resumes agree on one spelling. case 'copilot': return providerSession.key === 'session_id' ? ['copilot', `--resume=${id}`] : null + // Why: Kimi resumes by id with --session; sessions are work-dir-scoped (enforced by callers). + case 'kimi': + return providerSession.key === 'session_id' ? ['kimi', '--session', id] : null } } diff --git a/src/shared/protocol-version.ts b/src/shared/protocol-version.ts index 15d376192e1..d9d356c676a 100644 --- a/src/shared/protocol-version.ts +++ b/src/shared/protocol-version.ts @@ -101,6 +101,10 @@ export const AGENT_SESSION_HOST_AUTHORITY_RUNTIME_CAPABILITY = 'agent-session.host-authority.v1' as const export const AGENT_SESSION_OMP_RESUME_PATH_RUNTIME_CAPABILITY = 'agent-session.omp-resume-path.v1' as const +// Why: adding kimi to RESUMABLE_TUI_AGENTS grows terminal.ensureAgentSession's enum, and an +// older host answers the unknown member with invalid_argument — a code the launch fallback does +// not retry on — so clients must probe before taking the host-authority path. +export const AGENT_SESSION_KIMI_RESUME_RUNTIME_CAPABILITY = 'agent-session.kimi-resume.v1' as const // Why: older runtimes strip mutation owner fields, so clients must fence writes before RPC. export const FILE_MUTATION_OWNERSHIP_RUNTIME_CAPABILITY = 'files.mutation-ownership.v1' as const export const FILE_MUTATION_OWNERSHIP_UPDATE_REQUIRED_MESSAGE = @@ -144,6 +148,7 @@ export const RUNTIME_CAPABILITIES = [ REMOTE_SERVER_UPDATE_CAPABILITY, AGENT_SESSION_HOST_AUTHORITY_RUNTIME_CAPABILITY, AGENT_SESSION_OMP_RESUME_PATH_RUNTIME_CAPABILITY, + AGENT_SESSION_KIMI_RESUME_RUNTIME_CAPABILITY, FILE_MUTATION_OWNERSHIP_RUNTIME_CAPABILITY, WORKTREE_VISIBILITY_DEFAULTS_RUNTIME_CAPABILITY, WORKTREE_VISIBILITY_SOURCE_DEFAULTS_RUNTIME_CAPABILITY, diff --git a/src/shared/workspace-session-schema.sleeping-agent.test.ts b/src/shared/workspace-session-schema.sleeping-agent.test.ts index 16f499ac7d5..0f34528bcb5 100644 --- a/src/shared/workspace-session-schema.sleeping-agent.test.ts +++ b/src/shared/workspace-session-schema.sleeping-agent.test.ts @@ -41,6 +41,42 @@ describe('parseWorkspaceSession sleeping agents', () => { } }) + it('hydrates a persisted Kimi sleeping agent record', () => { + const result = parseWorkspaceSession({ + activeRepoId: null, + activeWorktreeId: null, + activeTabId: null, + tabsByWorktree: {}, + terminalLayoutsByTabId: {}, + sleepingAgentSessionsByPaneKey: { + 'tab1:pane-1': { + paneKey: 'tab1:pane-1', + tabId: 'tab1', + worktreeId: 'wt', + agent: 'kimi', + providerSession: { + key: 'session_id', + id: 'session_431324d7-2165-42f0-9ecd-9f93437b3201' + }, + prompt: 'continue', + state: 'working', + capturedAt: 10, + updatedAt: 9, + terminalTitle: 'Kimi', + origin: 'live' + } + } + }) + expect(result.ok).toBe(true) + if (result.ok) { + // Why: the record must survive the resumable-agent refine, or restore silently drops it. + expect(result.value.sleepingAgentSessionsByPaneKey?.['tab1:pane-1']).toMatchObject({ + agent: 'kimi', + providerSession: { key: 'session_id', id: 'session_431324d7-2165-42f0-9ecd-9f93437b3201' } + }) + } + }) + it('preserves the authoritative Pi session file through hydration', () => { const result = parseWorkspaceSession({ activeRepoId: null, From c3a1694b1d3023d7359f7c5811a5d9ea8c63c294 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 23 Aug 2026 02:58:40 -0700 Subject: [PATCH 11/14] perf(preflight): read the WSL mount table once, and make launch agree with detection (#16053) * perf(preflight): read the WSL mount table once per shell, not once per CLI The prelude is embedded in the lookup script, and the caller wraps that in `for cmd in `, so the unconditional assignment forked awk once per probed CLI -- 36 of them inside the distro against a 10s detection budget. The comment claimed it was read once outside the loop; it was not. `${x+set}` rather than `[ -n ... ]`: a host with no Windows mounts yields the empty string, which must still count as read. Pinned by counting real awk forks through /bin/sh with a stub on PATH, because nothing covered this expression at all -- a wrong-field mutation shipped green. Verified to bind: the unconditional form counts 4 for 4 commands. * fix(wsl): make launch resolve the same binary detection reported Agent detection skips Windows mounts during the PATH walk; the Codex WSL command builder and the WSL branch of isCommandOnPath did not. So Orca could report the guest codex as installed and then launch the Windows one sitting ahead of it on PATH, or disagree with itself between preflight and detection about the same distro. Both now pass the same option. Verified on a real Windows host against a real WSL2 distro, with a Windows binary planted ahead of a guest one on PATH: plain `command -v orcaprobe` -> /mnt/c/Users/neil/orca-agree/orcaprobe this lookup -> /home/neil/.orca-agree/bin/orcaprobe That host reports /mnt/c as 9p, which the mount expression matches, so the fstype list is confirmed against hardware rather than fixtures. * test(preflight): prove the memoised mount list applies past the first command Counting awk forks with a stub that reports no mounts cannot see what the hoist trades correctness for. A mutant that empties `_orca_win_mounts` inside the walk keeps the fork count at 1 and keeps every existing test green, while every agent after the first stops skipping /mnt. This runs two commands behind a stubbed Windows mount and asserts both resolve to the guest binary. Verified against that exact mutant. Credit: review counsel. --- src/main/codex-accounts/wsl-codex-command.ts | 8 +- src/main/ipc/preflight-command-exec.test.ts | 7 +- src/main/ipc/preflight-command-exec.ts | 8 +- .../ipc/preflight-wsl-agent-detection.test.ts | 94 ++++++++++++++++++- src/shared/posix-command-path-lookup.ts | 13 ++- 5 files changed, 123 insertions(+), 7 deletions(-) diff --git a/src/main/codex-accounts/wsl-codex-command.ts b/src/main/codex-accounts/wsl-codex-command.ts index af8ef64011a..1831a2f215c 100644 --- a/src/main/codex-accounts/wsl-codex-command.ts +++ b/src/main/codex-accounts/wsl-codex-command.ts @@ -77,7 +77,13 @@ export function buildWslCodexLoginArgs(distro: string, linuxHomePath: string): s } function buildCodexPathLookup(): string { - return buildPosixCommandPathLookupScript({ kind: 'literal', value: 'codex' }) + // Same skip as agent detection: otherwise detection can report the guest + // codex while this resolves the Windows one ahead of it on PATH, and Orca + // launches a different binary than the one it said was installed. + return buildPosixCommandPathLookupScript( + { kind: 'literal', value: 'codex' }, + { skipWindowsMountDirs: true } + ) } function buildWslCodexShellArgs(distro: string, command: string): string[] { diff --git a/src/main/ipc/preflight-command-exec.test.ts b/src/main/ipc/preflight-command-exec.test.ts index 204b8a5f6e1..63c2b35aceb 100644 --- a/src/main/ipc/preflight-command-exec.test.ts +++ b/src/main/ipc/preflight-command-exec.test.ts @@ -35,7 +35,12 @@ describe('isCommandOnPath', () => { expect(runPreflightCommandInWslMock).toHaveBeenCalledOnce() const [, command] = runPreflightCommandInWslMock.mock.calls[0] as [{ distro: string }, string] expect(command).toContain( - buildPosixCommandPathLookupScript({ kind: 'literal', value: 'codex' }) + buildPosixCommandPathLookupScript( + { kind: 'literal', value: 'codex' }, + // The WSL branch skips Windows mounts, so detection and this check + // cannot disagree about the same distro. + { skipWindowsMountDirs: true } + ) ) expect(command).toContain( ['if [ -n "$resolved" ]; then', `printf '${sentinel}%s\\n' "$resolved"`, 'fi'].join('\n') diff --git a/src/main/ipc/preflight-command-exec.ts b/src/main/ipc/preflight-command-exec.ts index 9dbdbdd343d..0d602b9827f 100644 --- a/src/main/ipc/preflight-command-exec.ts +++ b/src/main/ipc/preflight-command-exec.ts @@ -100,7 +100,13 @@ export async function isCommandOnPath( const { stdout } = await execCommandInWsl( wslTarget, [ - buildPosixCommandPathLookupScript({ kind: 'literal', value: command }), + // Same skip as agent detection: without it this branch answers "yes" + // for a Windows binary reached through interop, so preflight and the + // detector disagree about the same distro. + buildPosixCommandPathLookupScript( + { kind: 'literal', value: command }, + { skipWindowsMountDirs: true } + ), 'if [ -n "$resolved" ]; then', `printf '${WSL_COMMAND_PATH_SENTINEL}%s\\n' "$resolved"`, 'fi' diff --git a/src/main/ipc/preflight-wsl-agent-detection.test.ts b/src/main/ipc/preflight-wsl-agent-detection.test.ts index 648c7f4f543..0f9cdb0d350 100644 --- a/src/main/ipc/preflight-wsl-agent-detection.test.ts +++ b/src/main/ipc/preflight-wsl-agent-detection.test.ts @@ -1,5 +1,5 @@ import { execFileSync, type ExecFileSyncOptions } from 'node:child_process' -import { chmodSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { chmodSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' @@ -291,3 +291,95 @@ describe('the PATH walk, run by a real POSIX shell', () => { } }) }) + +describe('the mount table read, counted against a real shell', () => { + const itPosix = process.platform === 'win32' ? it.skip : it + + const runWithCountingAwk = async (commands: string[]): Promise => { + const root = mkdtempSync(join(tmpdir(), 'orca-awk-')) + try { + const counter = join(root, 'count') + const bin = join(root, 'bin') + mkdirSync(bin, { recursive: true }) + // A stub that records each fork, then answers as awk would on a host + // with no Windows mounts. + writeFileSync(join(bin, 'awk'), `#!/bin/sh\necho x >> ${counter}\nexit 0\n`) + chmodSync(join(bin, 'awk'), 0o755) + writeFileSync(counter, '') + + runWslProcessMock.mockResolvedValue({ + environmentResolved: true, + code: 0, + stdout: '', + stderr: '', + timedOut: false + }) + await detectWslCommandsOnPath({ distro: 'Ubuntu' }, commands) + const script = String(runWslProcessMock.mock.calls.at(-1)?.[0].script) + const options: ExecFileSyncOptions = { + encoding: 'utf8', + env: { PATH: `${bin}:/usr/bin:/bin`, HOME: root } + } + execFileSync('/bin/sh', ['-c', script], options) + return readFileSync(counter, 'utf8').trim().split('\n').filter(Boolean).length + } finally { + rmSync(root, { recursive: true, force: true }) + } + } + + itPosix('reads /proc/mounts once, not once per probed command', async () => { + // The prelude is embedded in the lookup script, which the caller wraps in + // `for cmd in `. An unconditional assignment forked awk once + // per CLI -- 36 of them inside the distro against a 10s budget. + expect(await runWithCountingAwk(['claude', 'codex', 'gemini', 'opencode'])).toBe(1) + }) + + itPosix('still reads it once when only one command is probed', async () => { + expect(await runWithCountingAwk(['claude'])).toBe(1) + }) + + itPosix('applies the memoised mount list to every command, not just the first', async () => { + // Counting forks with a stub that reports NO mounts cannot see the thing + // the hoist trades correctness for: a refactor that empties or clobbers + // the variable inside the walk keeps the fork count at 1 and keeps the + // single-command test green, while every agent after the first stops + // skipping /mnt. A surviving mutant proved that gap. + const root = mkdtempSync(join(tmpdir(), 'orca-memo-')) + try { + const win = join(root, 'winmnt/c/npm') + const guest = join(root, 'home/bin') + for (const [dir, body] of [ + [win, 'win'], + [guest, 'guest'] + ] as const) { + mkdirSync(dir, { recursive: true }) + for (const name of ['claude', 'codex']) { + writeFileSync(join(dir, name), `#!/bin/sh\necho ${body}\n`) + chmodSync(join(dir, name), 0o755) + } + } + runWslProcessMock.mockResolvedValue({ + environmentResolved: true, + code: 0, + stdout: '', + stderr: '', + timedOut: false + }) + await detectWslCommandsOnPath({ distro: 'Ubuntu' }, ['claude', 'codex']) + const script = String(runWslProcessMock.mock.calls.at(-1)?.[0].script).replace( + /_orca_win_mounts=\$\([^)]*\)/, + `_orca_win_mounts=${join(root, 'winmnt')}` + ) + const options: ExecFileSyncOptions = { + encoding: 'utf8', + env: { PATH: `${win}:${guest}:/usr/bin:/bin`, HOME: root } + } + const out = String(execFileSync('/bin/sh', ['-c', script], options)) + // BOTH must resolve behind the mount, not just the first. + expect(out).toContain(`__ORCA_AGENT_PATH__claude\t${join(guest, 'claude')}`) + expect(out).toContain(`__ORCA_AGENT_PATH__codex\t${join(guest, 'codex')}`) + } finally { + rmSync(root, { recursive: true, force: true }) + } + }) +}) diff --git a/src/shared/posix-command-path-lookup.ts b/src/shared/posix-command-path-lookup.ts index ac179f941fb..ea09ca83e2f 100644 --- a/src/shared/posix-command-path-lookup.ts +++ b/src/shared/posix-command-path-lookup.ts @@ -25,11 +25,18 @@ export function buildPosixCommandPathLookupScript( options: PosixCommandPathLookupOptions = {} ): string { const commandAssignment = buildCommandAssignment(target) - // `-t drvfs` is what WSL mounts a Windows drive as, wherever the automount - // root is; 9p/virtiofs cover the WSL2 shapes. Read once, outside the loop. + // `drvfs` is what WSL mounts a Windows drive as, wherever the automount root + // is; 9p/virtiofs cover the WSL2 shapes. + // + // `${x+set}` so the table is read once per SHELL, not once per lookup: the + // caller embeds this script inside `for cmd in `, so an + // unconditional assignment forked awk once per probed CLI -- 36 of them + // against a 10s budget. The variable outlives the iteration, so the second + // pass finds it set. Deliberately not `[ -n ... ]`: a host with no Windows + // mounts yields the empty string, which must still count as read. const mountPrelude = options.skipWindowsMountDirs ? [ - '_orca_win_mounts=$(awk \'$3 == "drvfs" || $3 == "9p" || $3 == "virtiofs" { print $2 }\' /proc/mounts 2>/dev/null)' + '[ "${_orca_win_mounts+set}" = set ] || _orca_win_mounts=$(awk \'$3 == "drvfs" || $3 == "9p" || $3 == "virtiofs" { print $2 }\' /proc/mounts 2>/dev/null)' ] : [] const skipMountComponent = options.skipWindowsMountDirs From e50cc309c3466e02b55e794c3ee910f1e2dd6c2b Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sun, 23 Aug 2026 09:45:52 -0700 Subject: [PATCH 12/14] fix(runtime): prevent restored workers from appearing idle while busy (#15998) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(runtime): classify tui-idle from the visible screen only The adopted-PTY tui-idle probe added in #15569 read the provider snapshot as `scrollbackAnsi + data`, and the Codex readiness classifier matches the startup banner. For a daemon-hosted adopted worker — where the retained tail stays empty forever — every wait re-probed and could resolve `satisfied: true` off banner history while Codex was actively working, turning a loud timeout into a silent false ready. - probe now requests and parses the visible grid, never scrollback - retirement of a timed-out provider acquisition is checked before the re-acquire branch, so a wider row request can no longer resurrect a hung provider - probe builds its result before clearing the poll interval, so a stale handle cannot leave the waiter with neither poll nor probe Fixture follow-ups from the same review: - resume legs pin the captured `launchConfig.agentCommand` to the fake instead of bare `codex`, which resolved the machine's real Codex off PATH - the command override is quoted for the Windows shell the runtime will actually use, and specs pin that shell alongside the override - fake agents acknowledge a bare submit after a short grace, so an unbracketed delivery path fails with a diagnosable ACK instead of a suite timeout Refs STA-4907, STA-4885 * test: assert tui-idle probes serialize visible grid only - Verify idle timeout probes exclude scrollback from serialization - Add test case for Git Bash shell path quoting with apostrophes - Simplify verbose test helper comments * test: improve fake agent paste protocol validation Refactor paste end detection to properly track both begin and end markers, validate bracketed paste protocol (RFC 2544) through chronological event sequencing, and emit correct error messages for protocol violations. This ensures reliable detection of when pastes complete even when delivered across multiple chunks, and correctly distinguishes between bracketed and unbracketed paste modes. * fix(runtime): reject provider snapshots when live output advances Provider snapshots become stale when live output is received after the snapshot is requested. Reject snapshots where the current output sequence exceeds the snapshot sequence, preventing callers from consuming outdated terminal state. Add tests verifying stale frame rejection. --- src/main/runtime/orca-runtime.test.ts | 191 ++++++++++++++++-- src/main/runtime/orca-runtime.ts | 114 ++++++++--- ...completed-worker-retirement-resume.spec.ts | 45 ++++- .../completed-worker-retirement-fixture.ts | 15 +- .../helpers/fake-agent-command-override.ts | 19 +- .../fake-agent-command-override.unit.test.ts | 16 ++ .../helpers/fake-agent-paste-end-scanner.ts | 69 ++++++- .../fake-agent-paste-end-scanner.unit.test.ts | 88 +++++++- ...y-worker-missing-terminal-recovery.spec.ts | 42 ++-- ...ion-legacy-worker-restart-recovery.spec.ts | 76 +++++-- ...tion-worker-settlement-release-cli.spec.ts | 34 ++-- 11 files changed, 584 insertions(+), 125 deletions(-) diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 3c7e27b062c..3688fec9c39 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -18308,6 +18308,111 @@ describe('OrcaRuntimeService', () => { expect(read.tail.join('\n')).not.toContain('Stale TUI') }) + it('rejects a provider visible frame after live non-mode-switch output advances', async () => { + type Snapshot = { + data: string + cols: number + rows: number + seq: number + source: 'headless' + alternateScreen: boolean + } + let resolveSnapshot!: (snapshot: Snapshot) => void + const serializeProviderBuffer = vi.fn( + () => + new Promise((resolve) => { + resolveSnapshot = resolve + }) + ) + const runtime = new OrcaRuntimeService(store) + runtime.setPtyController({ + write: () => true, + kill: () => true, + getForegroundProcess: async () => null, + serializeProviderBuffer, + hasRendererSerializer: () => false + }) + syncSinglePty(runtime) + runtime.onPtyData('pty-1', 'shell history\r\n', 100) + runtime.synchronizePtyOutputSequenceFromProvider( + 'pty-1', + { value: 900, generation: 'continued' }, + 0 + ) + const [terminal] = (await runtime.listTerminals()).terminals + + const readPromise = runtime.readTerminal(terminal.handle) + await vi.waitFor(() => expect(serializeProviderBuffer).toHaveBeenCalledOnce()) + runtime.onPtyData('pty-1', 'live progress\r\n', 101) + resolveSnapshot({ + data: '\x1b[?1049h\x1b[2J\x1b[HStale Ready screen\r\n', + cols: 80, + rows: 24, + seq: 900, + source: 'headless', + alternateScreen: true + }) + + const read = await readPromise + expect(read.tail).toEqual(['shell history']) + expect(read.tail.join('\n')).not.toContain('Stale Ready screen') + }) + + it('rejects a full provider screen frame after live output advances', async () => { + type Snapshot = { + data: string + scrollbackAnsi: string + cols: number + rows: number + seq: number + source: 'headless' + alternateScreen: boolean + } + let resolveSnapshot!: (snapshot: Snapshot) => void + const serializeProviderBuffer = vi + .fn() + .mockResolvedValueOnce(null) + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveSnapshot = resolve + }) + ) + const runtime = new OrcaRuntimeService(store) + runtime.setPtyController({ + write: () => true, + kill: () => true, + getForegroundProcess: async () => null, + serializeProviderBuffer, + hasRendererSerializer: () => false + }) + syncSinglePty(runtime) + runtime.onPtyData('pty-1', 'shell history\r\n', 100) + runtime.synchronizePtyOutputSequenceFromProvider( + 'pty-1', + { value: 900, generation: 'continued' }, + 0 + ) + const [terminal] = (await runtime.listTerminals()).terminals + + const readPromise = runtime.readTerminal(terminal.handle, { screen: true }) + await vi.waitFor(() => expect(serializeProviderBuffer).toHaveBeenCalledTimes(2)) + runtime.onPtyData('pty-1', 'live progress\r\n', 101) + resolveSnapshot({ + data: '\x1b[?1049h\x1b[2J\x1b[HStale full screen\r\n', + scrollbackAnsi: '', + cols: 80, + rows: 24, + seq: 900, + source: 'headless', + alternateScreen: true + }) + + const read = await readPromise + expect(read.source).toBe('screen-unavailable') + expect(read.tail.join('\n')).not.toContain('Stale full screen') + }) + it('shares one provider snapshot between concurrent read and show calls', async () => { type Snapshot = { data: string @@ -20557,16 +20662,22 @@ describe('OrcaRuntimeService', () => { } as unknown as OrchestrationDb) const write = vi.fn(() => true) const kill = vi.fn(() => true) - const serializeProviderBuffer = vi.fn().mockResolvedValue({ - data: '', - scrollbackAnsi: - ' >_ OpenAI Codex (v0.131.0)\r\n model: gpt-5.5 high\r\n directory: /repo\r\n', - cols: 80, - rows: 24, - seq: 100, - source: 'headless' as const, - alternateScreen: false - }) + const READY_SCREEN = + ' >_ OpenAI Codex (v0.131.0)\r\n model: gpt-5.5 high\r\n directory: /repo\r\n' + // Why scrollbackRows-aware: a visible-only request gets the grid in `data`; + // a scrollback request gets history. Collapsing the two would let a test + // pass on evidence the caller never asked for. + const serializeProviderBuffer = vi + .fn() + .mockImplementation(async (_ptyId: string, opts?: { scrollbackRows?: number }) => ({ + data: opts?.scrollbackRows === 0 ? READY_SCREEN : '', + scrollbackAnsi: opts?.scrollbackRows === 0 ? '' : READY_SCREEN, + cols: 80, + rows: 24, + seq: 100, + source: 'headless' as const, + alternateScreen: false + })) runtime.setPtyController({ write, kill, @@ -20650,9 +20761,50 @@ describe('OrcaRuntimeService', () => { await expect( runtime.waitForTerminal(terminal.handle, { condition: 'tui-idle', timeoutMs: 100 }) ).resolves.toMatchObject({ satisfied: true }) + // Why: the ready banner stays in scrollback for the whole session, so a + // working grid must not inherit idleness from its own history (#15569 review). serializeProviderBuffer.mockResolvedValueOnce({ - data: '', - scrollbackAnsi: 'Do you trust this workspace directory?\r\n1. Yes\r\n2. No\r\n', + data: ' working on it (12s)\r\n Esc to interrupt\r\n', + scrollbackAnsi: READY_SCREEN, + cols: 80, + rows: 24, + seq: 101, + source: 'headless' as const, + alternateScreen: false + }) + await expect( + runtime.waitForTerminal(terminal.handle, { condition: 'tui-idle', timeoutMs: 50 }) + ).rejects.toThrow('timeout') + const lateReadySnapshot = deferred<{ + data: string + scrollbackAnsi: string + cols: number + rows: number + seq: number + source: 'headless' + alternateScreen: boolean + }>() + const snapshotSequence = runtime.getPtyOutputSequence('pty-legacy') + serializeProviderBuffer.mockImplementationOnce(() => lateReadySnapshot.promise) + const staleReadyWait = runtime.waitForTerminal(terminal.handle, { + condition: 'tui-idle', + timeoutMs: 50 + }) + await vi.waitFor(() => expect(serializeProviderBuffer).toHaveBeenCalledTimes(4)) + runtime.onPtyData('pty-legacy', '\x1b[H', Date.now()) + lateReadySnapshot.resolve({ + data: READY_SCREEN, + scrollbackAnsi: '', + cols: 80, + rows: 24, + seq: snapshotSequence, + source: 'headless', + alternateScreen: false + }) + await expect(staleReadyWait).rejects.toThrow('timeout') + serializeProviderBuffer.mockResolvedValueOnce({ + data: 'Do you trust this workspace directory?\r\n1. Yes\r\n2. No\r\n', + scrollbackAnsi: '', cols: 80, rows: 24, seq: 101, @@ -20669,9 +20821,18 @@ describe('OrcaRuntimeService', () => { await expect( runtime.waitForTerminal(terminal.handle, { condition: 'tui-idle', timeoutMs: 50 }) ).rejects.toThrow('timeout') - expect(serializeProviderBuffer).toHaveBeenCalledTimes(4) - await expect(runtime.readTerminal(terminal.handle)).resolves.toMatchObject({ tail: [] }) - expect(serializeProviderBuffer).toHaveBeenCalledTimes(4) + expect(serializeProviderBuffer).toHaveBeenCalledTimes(6) + // Why args, not counts: the one-shot responses above ignore their options, + // so only this asserts every idle probe asked for the visible grid alone. + expect(serializeProviderBuffer.mock.calls.slice(1)).toEqual([ + ['pty-legacy', { scrollbackRows: 0 }], + ['pty-legacy', { scrollbackRows: 0 }], + ['pty-legacy', { scrollbackRows: 0 }], + ['pty-legacy', { scrollbackRows: 0 }], + ['pty-legacy', { scrollbackRows: 0 }] + ]) + await expect(runtime.readTerminal(terminal.handle)).resolves.toBeDefined() + expect(serializeProviderBuffer).toHaveBeenCalledTimes(6) expect( runtime.verifyOrchestrationCompatibilityCaller({ terminalHandle: 'term_legacy', diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 8ddbfa4fdf8..aac73f5e7f4 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -2987,6 +2987,16 @@ export type RuntimeRendererReloadFence = Readonly<{ recovery: 'renderer' | 'headless' | 'reloading' }> +/** How a caller wants the provider-held screen fetched when the runtime has no + * bytes of its own. `visibleScreenOnly` narrows the result to the current grid: + * scrollback can still hold a ready banner a working agent printed minutes ago, + * which is history, not evidence of what the agent is doing now. */ +type ProviderSnapshotReadOptions = { + timeoutMs?: number + retireOnTimeout?: boolean + visibleScreenOnly?: boolean +} + export class OrcaRuntimeService { private readonly runtimeId = randomUUID() private readonly startedAt = Date.now() @@ -13238,6 +13248,13 @@ export class OrcaRuntimeService { const generation = this.getPtyLifecycleGeneration(ptyId) const scrollbackRows = Math.max(0, Math.floor(opts.scrollbackRows ?? 0)) let acquisition = this.providerBufferAcquisitionsByPtyId.get(ptyId) + // Why before the re-acquire branch: an unresponsive provider is a property of + // the process, not of the row count one caller asked for. Checking retirement + // only after re-acquiring let a wider request replace the retired entry and + // hang again; the hung call's own settle still clears it and allows recovery. + if (acquisition?.generation === generation && acquisition.timedOut) { + return null + } if ( !acquisition || acquisition.generation !== generation || @@ -13342,7 +13359,7 @@ export class OrcaRuntimeService { ptyId: string, read: RuntimeTerminalRead, opts: { cursor?: number; limit?: number } = {}, - providerWait: { timeoutMs?: number; retireOnTimeout?: boolean } = {} + providerSnapshot: ProviderSnapshotReadOptions = {} ): Promise { if (typeof opts.cursor === 'number') { return read @@ -13360,7 +13377,7 @@ export class OrcaRuntimeService { const providerLines = await this.readProviderTerminalTailLines( ptyId, opts.limit, - providerWait + providerSnapshot ) if (providerLines.length > 0) { return buildVisibleSnapshotReadFallback(read, providerLines, opts.limit) @@ -13400,16 +13417,30 @@ export class OrcaRuntimeService { private async readProviderTerminalTailLines( ptyId: string, limit: number | undefined, - wait: { timeoutMs?: number; retireOnTimeout?: boolean } = {} + snapshotOptions: ProviderSnapshotReadOptions = {} ): Promise { + const generation = this.getPtyLifecycleGeneration(ptyId) const lineLimit = terminalReadLimit(limit, DEFAULT_TERMINAL_READ_LIMIT) const snapshot = await this.serializeProviderTerminalBuffer( ptyId, - { scrollbackRows: lineLimit }, - wait + { scrollbackRows: snapshotOptions.visibleScreenOnly ? 0 : lineLimit }, + snapshotOptions ) - const data = snapshot ? `${snapshot.scrollbackAnsi ?? ''}${snapshot.data}` : '' - if (!snapshot || data.length === 0) { + if (!snapshot) { + return [] + } + // Why: a cached acquisition can carry scrollback this caller did not ask for, + // so visible-only reads parse the grid itself rather than trusting the request. + if (snapshotOptions.visibleScreenOnly) { + const lines = await this.parseVisibleSnapshotLines(snapshot) + // Live bytes ordered after the provider frame make that frame stale. + return this.getPtyLifecycleGeneration(ptyId) === generation && + this.getPtyOutputSequence(ptyId) <= snapshot.seq + ? lines + : [] + } + const data = `${snapshot.scrollbackAnsi ?? ''}${snapshot.data}` + if (data.length === 0) { return [] } const emulator = new HeadlessEmulator({ @@ -13419,7 +13450,11 @@ export class OrcaRuntimeService { }) try { await emulator.write(data) - return visibleNonBlankTerminalLines(emulator.getBufferTailLines(lineLimit)) + const lines = visibleNonBlankTerminalLines(emulator.getBufferTailLines(lineLimit)) + return this.getPtyLifecycleGeneration(ptyId) === generation && + this.getPtyOutputSequence(ptyId) <= snapshot.seq + ? lines + : [] } finally { emulator.dispose() } @@ -13495,7 +13530,10 @@ export class OrcaRuntimeService { } } const lines = await this.parseVisibleSnapshotLines(snapshot) - if (this.getPtyLifecycleGeneration(ptyId) !== generation) { + if ( + this.getPtyLifecycleGeneration(ptyId) !== generation || + this.getPtyOutputSequence(ptyId) > snapshot.seq + ) { return null } const visibleState: RuntimeVisibleTerminalState = { @@ -13504,9 +13542,7 @@ export class OrcaRuntimeService { sequence: snapshot.seq, generation } - if (this.getPtyOutputSequence(ptyId) <= snapshot.seq) { - this.providerVisibleStateByPtyId.set(ptyId, visibleState) - } + this.providerVisibleStateByPtyId.set(ptyId, visibleState) return visibleState } @@ -18463,14 +18499,14 @@ export class OrcaRuntimeService { async readTerminal( handle: string, opts: { cursor?: number; limit?: number; screen?: boolean } = {}, - providerWait: { timeoutMs?: number; retireOnTimeout?: boolean } = {} + providerSnapshot: ProviderSnapshotReadOptions = {} ): Promise { const pty = this.getLivePtyForHandle(handle) if (pty) { const read = this.readPtyTerminal(handle, pty.pty, opts) const visibleRead = opts.screen ? await this.readRenderedScreen(pty.pty.ptyId, read, opts) - : await this.withVisibleSnapshotFallback(pty.pty.ptyId, read, opts, providerWait) + : await this.withVisibleSnapshotFallback(pty.pty.ptyId, read, opts, providerSnapshot) this.assertLiveTerminalHandleTargetsPty(handle, pty.pty.ptyId) return labelTerminalReadSource(visibleRead) } @@ -18492,7 +18528,7 @@ export class OrcaRuntimeService { } const visibleRead = opts.screen ? await this.readRenderedScreen(leaf.ptyId, read, opts) - : await this.withVisibleSnapshotFallback(leaf.ptyId, read, opts, providerWait) + : await this.withVisibleSnapshotFallback(leaf.ptyId, read, opts, providerSnapshot) this.assertLiveTerminalHandleTargetsPty(handle, leaf.ptyId) return labelTerminalReadSource(visibleRead) } @@ -35558,6 +35594,11 @@ export class OrcaRuntimeService { } } + /** One bounded look at the provider's screen for an adopted PTY whose retained + * readiness metadata was lost. Deliberately single-shot: it answers "is the + * screen already showing a settled prompt", and the poll above owns every + * later transition. A provider screen that is still working when this fires + * resolves through the poll, not here. */ private startTuiIdleVisibleReadProbe(waiter: TerminalWaiter, waiterTimeoutMs: number): void { const snapshotTimeoutMs = Math.min( VISIBLE_TERMINAL_SNAPSHOT_TIMEOUT_MS, @@ -35569,7 +35610,10 @@ export class OrcaRuntimeService { {}, { timeoutMs: snapshotTimeoutMs, - retireOnTimeout: true + retireOnTimeout: true, + // Why: the ready banner stays in scrollback for the whole session, so + // classifying history would call a working agent idle (#15569 review). + visibleScreenOnly: true } ), snapshotTimeoutMs, @@ -35588,31 +35632,35 @@ export class OrcaRuntimeService { if (!blockedReason && !isKnownReadyPromptPreview(snapshotText)) { return } + // Why resolve before clearing: a stale handle throws while locating the + // record, and a cleared interval would leave the waiter with no poll and + // no probe — able to end only in timeout. + const result = this.buildTuiIdleProbeResult(waiter.handle, blockedReason) if (waiter.pollInterval) { clearInterval(waiter.pollInterval) waiter.pollInterval = null } - const pty = this.getLivePtyForHandle(waiter.handle) - if (pty) { - this.resolveWaiter( - waiter, - blockedReason - ? buildPtyTerminalWaitBlockedResult(waiter.handle, 'tui-idle', pty.pty, blockedReason) - : buildPtyTerminalWaitResult(waiter.handle, 'tui-idle', pty.pty) - ) - return - } - const { leaf } = this.getLiveLeafForHandle(waiter.handle) - this.resolveWaiter( - waiter, - blockedReason - ? buildTerminalWaitBlockedResult(waiter.handle, 'tui-idle', leaf, blockedReason) - : buildTerminalWaitResult(waiter.handle, 'tui-idle', leaf) - ) + this.resolveWaiter(waiter, result) }) .catch(() => {}) } + private buildTuiIdleProbeResult( + handle: string, + blockedReason: RuntimeTerminalWaitBlockedReason | null + ): RuntimeTerminalWait { + const pty = this.getLivePtyForHandle(handle) + if (pty) { + return blockedReason + ? buildPtyTerminalWaitBlockedResult(handle, 'tui-idle', pty.pty, blockedReason) + : buildPtyTerminalWaitResult(handle, 'tui-idle', pty.pty) + } + const { leaf } = this.getLiveLeafForHandle(handle) + return blockedReason + ? buildTerminalWaitBlockedResult(handle, 'tui-idle', leaf, blockedReason) + : buildTerminalWaitResult(handle, 'tui-idle', leaf) + } + private getAdoptedPtyExplicitIdleStatus(pty: RuntimePtyWorktreeRecord): AgentStatus | null { for (const leaf of this.leaves.values()) { if (leaf.ptyId !== pty.ptyId) { diff --git a/tests/e2e/completed-worker-retirement-resume.spec.ts b/tests/e2e/completed-worker-retirement-resume.spec.ts index 27c009b4677..69f6e0af375 100644 --- a/tests/e2e/completed-worker-retirement-resume.spec.ts +++ b/tests/e2e/completed-worker-retirement-resume.spec.ts @@ -5,6 +5,7 @@ import { waitForActivePanePtyId, waitForActiveTerminalManager } from './helpers/terminal' +import { FAKE_AGENT_WINDOWS_SHELL } from './helpers/fake-agent-command-override' import { cleanupCompletedWorkerFixture, clearCompletedWorkerLedger, @@ -46,13 +47,20 @@ for (const closeMode of ['terminal-close-cli', 'worker-release'] as const) { await ensureTerminalVisible(orcaPage) await waitForActiveTerminalManager(orcaPage) await waitForActivePanePtyId(orcaPage) - await orcaPage.evaluate(async (agentCommand) => { - await window.__store?.getState().updateSettings({ - agentCmdOverrides: { codex: agentCommand }, - disabledTuiAgents: [], - terminalHiddenViewParking: false - }) - }, completedWorkerFakeCodexCommand) + await orcaPage.evaluate( + async ({ agentCommand, terminalWindowsShell }) => { + await window.__store?.getState().updateSettings({ + agentCmdOverrides: { codex: agentCommand }, + terminalWindowsShell, + disabledTuiAgents: [], + terminalHiddenViewParking: false + }) + }, + { + agentCommand: completedWorkerFakeCodexCommand, + terminalWindowsShell: FAKE_AGENT_WINDOWS_SHELL + } + ) const userDataDir = await electronApp.evaluate(({ app }) => app.getPath('userData')) const isolatedHome = await electronApp.evaluate(({ app }) => app.getPath('home')) @@ -183,6 +191,13 @@ for (const closeMode of ['terminal-close-cli', 'worker-release'] as const) { return dispatchCapability }) .not.toBeNull() + await expect + .poll(() => + readCompletedWorkerLedger() + .filter((event) => event.event === 'ack') + .map((event) => event.mode) + ) + .toEqual(['bracketed']) if (!dispatchCapability) { throw new Error('Background worker did not receive its dispatch capability') } @@ -194,7 +209,15 @@ for (const closeMode of ['terminal-close-cli', 'worker-release'] as const) { ) await orcaPage.evaluate( - ({ paneKey, providerSessionId, tabId, terminalHandle, transcriptPath, worktreeId }) => { + ({ + agentCommand, + paneKey, + providerSessionId, + tabId, + terminalHandle, + transcriptPath, + worktreeId + }) => { const state = window.__store?.getState() if (!state) { throw new Error('Renderer store unavailable') @@ -208,7 +231,10 @@ for (const closeMode of ['terminal-close-cli', 'worker-release'] as const) { const recovery = { providerSession, launchConfig: { - agentCommand: 'codex', + // Why not bare 'codex': resume prefers the captured command over + // agentCmdOverrides, so a bare name would resolve the machine's real + // Codex off PATH and unpin the adoption leg this spec exercises. + agentCommand, agentArgs: '--dangerously-bypass-approvals-and-sandbox', agentEnv: {} } @@ -231,6 +257,7 @@ for (const closeMode of ['terminal-close-cli', 'worker-release'] as const) { ) }, { + agentCommand: completedWorkerFakeCodexCommand, paneKey: workerPaneKey, providerSessionId: PROVIDER_SESSION_ID, tabId: worker.tabId, diff --git a/tests/e2e/helpers/completed-worker-retirement-fixture.ts b/tests/e2e/helpers/completed-worker-retirement-fixture.ts index cbf895c21c8..43de5782648 100644 --- a/tests/e2e/helpers/completed-worker-retirement-fixture.ts +++ b/tests/e2e/helpers/completed-worker-retirement-fixture.ts @@ -36,13 +36,11 @@ if (args.includes('app-server')) { append({ event: 'spawn', args }) process.stdout.write('\\u001b]0;Codex Ready\\u0007OpenAI Codex\\nmodel: e2e\\ndirectory: e2e\\n') ${FAKE_AGENT_PASTE_END_SCANNER_SOURCE} -let pasteEnded = false process.stdin.on('data', (chunk) => { const input = chunk.toString() const pasteEndScan = scanFakeAgentPasteEnd(fakeAgentPasteEndTail, input) fakeAgentPasteEndTail = pasteEndScan.tail - if (pasteEndScan.ended) { - pasteEnded = true + if (pasteEndScan.pasteEndOffset !== null) { process.stdout.write('\\x1b[?25h') } append({ event: 'input', input }) @@ -50,10 +48,12 @@ process.stdin.on('data', (chunk) => { append({ event: 'normal-exit' }) process.exit(0) } - if (pasteEnded && input.includes('\\r')) { - process.stdout.write('\\u001b]0;Codex Working\\u0007ACK\\n') + fakeAgentMaybeAck(pasteEndScan, input, (mode) => { + append({ event: 'ack', mode }) + const message = mode === 'bracketed' ? 'ACK' : 'PASTE_PROTOCOL_ERROR' + process.stdout.write('\\u001b]0;Codex Working\\u0007' + message + '\\n') setTimeout(() => process.stdout.write('\\u001b]0;Codex Ready\\u0007'), 10) - } + }) }) process.stdin.setRawMode?.(true) process.stdin.resume() @@ -79,9 +79,10 @@ export const completedWorkerLaunchEnv = { export type LifecycleEvent = { pid: number - event: 'spawn' | 'input' | 'normal-exit' + event: 'spawn' | 'input' | 'ack' | 'normal-exit' args?: string[] input?: string + mode?: 'bracketed' | 'unbracketed' } export type TerminalIdentity = Pick< diff --git a/tests/e2e/helpers/fake-agent-command-override.ts b/tests/e2e/helpers/fake-agent-command-override.ts index ac5e9958e3e..3e358f80043 100644 --- a/tests/e2e/helpers/fake-agent-command-override.ts +++ b/tests/e2e/helpers/fake-agent-command-override.ts @@ -1,10 +1,21 @@ -import { quoteStartupArg } from '../../../src/shared/tui-agent-startup-shell' +import { quoteStartupArg, resolveStartupShell } from '../../../src/shared/tui-agent-startup-shell' +import { resolveLocalWindowsAgentStartupShell } from '../../../src/shared/windows-terminal-shell' + +/** Specs must apply this through `updateSettings`; the override below is quoted for it. */ +export const FAKE_AGENT_WINDOWS_SHELL = 'powershell.exe' export function buildFakeAgentCommandOverride( executablePath: string, - platform: NodeJS.Platform = process.platform + platform: NodeJS.Platform = process.platform, + terminalWindowsShell: string = FAKE_AGENT_WINDOWS_SHELL ): string { - const shell = platform === 'win32' ? 'powershell' : 'posix' + // Why resolve rather than assume PowerShell on win32: the startup shell comes + // from the user's terminalWindowsShell setting, and a cmd/Git Bash/WSL host + // would silently fail to launch a PowerShell-quoted path. + const shell = resolveStartupShell( + platform, + resolveLocalWindowsAgentStartupShell({ platform, isRemote: false, terminalWindowsShell }) + ) const quotedPath = quoteStartupArg(executablePath, shell) - return platform === 'win32' ? `& ${quotedPath}` : quotedPath + return shell === 'powershell' ? `& ${quotedPath}` : quotedPath } diff --git a/tests/e2e/helpers/fake-agent-command-override.unit.test.ts b/tests/e2e/helpers/fake-agent-command-override.unit.test.ts index 15d947a8943..f0eb7a61b86 100644 --- a/tests/e2e/helpers/fake-agent-command-override.unit.test.ts +++ b/tests/e2e/helpers/fake-agent-command-override.unit.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from 'vitest' +import { WINDOWS_GIT_BASH_SHELL } from '../../../src/shared/windows-terminal-shell' import { buildFakeAgentCommandOverride } from './fake-agent-command-override' describe('buildFakeAgentCommandOverride', () => { @@ -13,4 +14,19 @@ describe('buildFakeAgentCommandOverride', () => { "'/tmp/fake agent'\"'\"'s/codex'" ) }) + + it('drops the PowerShell call operator when the Windows host runs cmd', () => { + expect( + buildFakeAgentCommandOverride('C:\\Temp\\fake agent\\codex.cmd', 'win32', 'cmd.exe') + ).toBe('"C:\\Temp\\fake agent\\codex.cmd"') + }) + + it('uses POSIX quoting when the Windows host runs Git Bash or WSL', () => { + expect(buildFakeAgentCommandOverride('/tmp/fake agent/codex', 'win32', 'wsl.exe')).toBe( + "'/tmp/fake agent/codex'" + ) + expect( + buildFakeAgentCommandOverride("/tmp/fake agent's/codex", 'win32', WINDOWS_GIT_BASH_SHELL) + ).toBe("'/tmp/fake agent'\"'\"'s/codex'") + }) }) diff --git a/tests/e2e/helpers/fake-agent-paste-end-scanner.ts b/tests/e2e/helpers/fake-agent-paste-end-scanner.ts index 3fded291cea..a81e449b926 100644 --- a/tests/e2e/helpers/fake-agent-paste-end-scanner.ts +++ b/tests/e2e/helpers/fake-agent-paste-end-scanner.ts @@ -1,15 +1,76 @@ -export type FakeAgentPasteEndScan = { tail: string; ended: boolean } +export type FakeAgentPasteEndScan = { + tail: string + pasteBeginOffset: number | null + pasteEndOffset: number | null +} export function scanFakeAgentPasteEnd(tail: string, input: string): FakeAgentPasteEndScan { - const marker = '\x1b[201~' + const beginMarker = '\x1b[200~' + const endMarker = '\x1b[201~' const candidate = tail + input + const beginIndex = candidate.indexOf(beginMarker) + const endIndex = candidate.indexOf(endMarker) return { - tail: candidate.slice(1 - marker.length), - ended: candidate.includes(marker) + tail: candidate.slice(1 - endMarker.length), + pasteBeginOffset: beginIndex === -1 ? null : beginIndex + beginMarker.length - tail.length, + pasteEndOffset: endIndex === -1 ? null : endIndex + endMarker.length - tail.length } } +/** Fallback ACK delay so an unbracketed delivery path fails an assertion instead of timing out the suite. */ +export const FAKE_AGENT_UNBRACKETED_ACK_GRACE_MS = 2_000 + export const FAKE_AGENT_PASTE_END_SCANNER_SOURCE = ` const scanFakeAgentPasteEnd = ${scanFakeAgentPasteEnd.toString()} let fakeAgentPasteEndTail = '' +let fakeAgentInsidePaste = false +let fakeAgentSawPasteEnd = false +let fakeAgentSawSubmit = false +let fakeAgentUnbracketedAckTimer = null +function fakeAgentMaybeAck(scan, input, ack) { + const events = [] + if (scan.pasteBeginOffset !== null) { + events.push({ offset: scan.pasteBeginOffset, type: 'begin' }) + } + if (scan.pasteEndOffset !== null) { + events.push({ offset: scan.pasteEndOffset, type: 'end' }) + } + for (let offset = input.indexOf('\\r'); offset >= 0; offset = input.indexOf('\\r', offset + 1)) { + events.push({ offset, type: 'submit' }) + } + events.sort((left, right) => left.offset - right.offset || (left.type === 'submit' ? 1 : -1)) + for (const event of events) { + if (fakeAgentSawSubmit) break + if (event.type === 'begin') { + fakeAgentInsidePaste = true + } else if (event.type === 'end') { + if (fakeAgentInsidePaste) { + fakeAgentInsidePaste = false + fakeAgentSawPasteEnd = true + } + } else if (!fakeAgentInsidePaste) { + fakeAgentSawSubmit = true + } + } + if (!fakeAgentSawSubmit) return + if (fakeAgentSawPasteEnd) { + if (fakeAgentUnbracketedAckTimer) { + clearTimeout(fakeAgentUnbracketedAckTimer) + fakeAgentUnbracketedAckTimer = null + } + fakeAgentInsidePaste = false + fakeAgentSawPasteEnd = false + fakeAgentSawSubmit = false + ack('bracketed') + return + } + if (fakeAgentUnbracketedAckTimer) return + fakeAgentUnbracketedAckTimer = setTimeout(() => { + fakeAgentUnbracketedAckTimer = null + fakeAgentInsidePaste = false + fakeAgentSawPasteEnd = false + fakeAgentSawSubmit = false + ack('unbracketed') + }, ${FAKE_AGENT_UNBRACKETED_ACK_GRACE_MS}) +} ` diff --git a/tests/e2e/helpers/fake-agent-paste-end-scanner.unit.test.ts b/tests/e2e/helpers/fake-agent-paste-end-scanner.unit.test.ts index bcf6d93e337..fe68c2063e0 100644 --- a/tests/e2e/helpers/fake-agent-paste-end-scanner.unit.test.ts +++ b/tests/e2e/helpers/fake-agent-paste-end-scanner.unit.test.ts @@ -14,26 +14,104 @@ describe('scanFakeAgentPasteEnd', () => { const first = scanFakeAgentPasteEnd('', PASTE_END.slice(0, splitAt)) const second = scanFakeAgentPasteEnd(first.tail, PASTE_END.slice(splitAt)) - expect(first.ended).toBe(false) - expect(second.ended).toBe(true) + expect(first.pasteBeginOffset).toBeNull() + expect(first.pasteEndOffset).toBeNull() + expect(second.pasteBeginOffset).toBeNull() + expect(second.pasteEndOffset).toBe(PASTE_END.length - splitAt) } ) it('keeps only the possible marker prefix between chunks', () => { const scan = scanFakeAgentPasteEnd('', `worker prompt${PASTE_END.slice(0, -1)}`) - expect(scan).toEqual({ tail: PASTE_END.slice(0, -1), ended: false }) + expect(scan).toEqual({ + tail: PASTE_END.slice(0, -1), + pasteBeginOffset: null, + pasteEndOffset: null + }) }) it('emits standalone JavaScript for the fake agent process', () => { const context = vm.createContext({ result: null }) vm.runInContext( `${FAKE_AGENT_PASTE_END_SCANNER_SOURCE} - const first = scanFakeAgentPasteEnd('', '\\x1b[20') + const first = scanFakeAgentPasteEnd('', '\x1b[20') result = scanFakeAgentPasteEnd(first.tail, '1~')`, context ) - expect(context.result).toEqual({ tail: PASTE_END.slice(1), ended: true }) + expect(context.result).toEqual({ + tail: PASTE_END.slice(1), + pasteBeginOffset: null, + pasteEndOffset: 2 + }) + }) + + it('recognizes a paste frame and submit delivered in separate chunks', () => { + const context = vm.createContext({ result: null }) + vm.runInContext( + `${FAKE_AGENT_PASTE_END_SCANNER_SOURCE} + const modes = [] + fakeAgentMaybeAck({ pasteBeginOffset: 1, pasteEndOffset: null }, '', (mode) => modes.push(mode)) + fakeAgentMaybeAck({ pasteBeginOffset: null, pasteEndOffset: 1 }, '', (mode) => modes.push(mode)) + fakeAgentMaybeAck({ pasteBeginOffset: null, pasteEndOffset: null }, '\\r', (mode) => modes.push(mode)) + result = modes`, + context + ) + + expect(context.result).toEqual(['bracketed']) + }) + + it('does not upgrade a paste frame received after submit', () => { + const timers: (() => void)[] = [] + const context = vm.createContext({ + result: null, + setTimeout: (fn: () => void) => timers.push(fn) + }) + vm.runInContext( + `${FAKE_AGENT_PASTE_END_SCANNER_SOURCE} + const modes = [] + fakeAgentMaybeAck({ pasteBeginOffset: null, pasteEndOffset: null }, '\\r', (mode) => modes.push(mode)) + fakeAgentMaybeAck({ pasteBeginOffset: 1, pasteEndOffset: null }, '', (mode) => modes.push(mode)) + fakeAgentMaybeAck({ pasteBeginOffset: null, pasteEndOffset: 1 }, '', (mode) => modes.push(mode)) + result = modes`, + context + ) + expect(context.result).toEqual([]) + timers.forEach((timer) => timer()) + + expect(vm.runInContext('modes', context)).toEqual(['unbracketed']) + }) + + it('ignores multiline carriage returns inside one bracketed paste chunk', () => { + const context = vm.createContext({ result: null }) + vm.runInContext( + `${FAKE_AGENT_PASTE_END_SCANNER_SOURCE} + const modes = [] + const input = '\\x1b[200~line one\\rline two\\x1b[201~\\r' + const scan = scanFakeAgentPasteEnd('', input) + fakeAgentMaybeAck(scan, input, (mode) => modes.push(mode)) + result = modes`, + context + ) + + expect(context.result).toEqual(['bracketed']) + }) + + it('ignores multiline carriage returns across bracketed paste chunks', () => { + const context = vm.createContext({ result: null }) + vm.runInContext( + `${FAKE_AGENT_PASTE_END_SCANNER_SOURCE} + const modes = [] + for (const input of ['\\x1b[200~line one', '\\rline two', '\\x1b[201~', '\\r']) { + const scan = scanFakeAgentPasteEnd(fakeAgentPasteEndTail, input) + fakeAgentPasteEndTail = scan.tail + fakeAgentMaybeAck(scan, input, (mode) => modes.push(mode)) + } + result = modes`, + context + ) + + expect(context.result).toEqual(['bracketed']) }) }) diff --git a/tests/e2e/orchestration-legacy-worker-missing-terminal-recovery.spec.ts b/tests/e2e/orchestration-legacy-worker-missing-terminal-recovery.spec.ts index a2ff0dd2eb0..73527afa50d 100644 --- a/tests/e2e/orchestration-legacy-worker-missing-terminal-recovery.spec.ts +++ b/tests/e2e/orchestration-legacy-worker-missing-terminal-recovery.spec.ts @@ -19,7 +19,10 @@ import Database from '../../src/main/sqlite/sync-database' import { LEGACY_CONTRACT_VERSION } from '../../src/main/runtime/orchestration/db' import { DEFAULT_LOCAL_ORCA_PROFILE_ID } from '../../src/shared/orca-profiles' import type { RuntimeTerminalListResult, RuntimeTerminalRead } from '../../src/shared/runtime-types' -import { buildFakeAgentCommandOverride } from './helpers/fake-agent-command-override' +import { + buildFakeAgentCommandOverride, + FAKE_AGENT_WINDOWS_SHELL +} from './helpers/fake-agent-command-override' import { FAKE_AGENT_PASTE_END_SCANNER_SOURCE } from './helpers/fake-agent-paste-end-scanner' const PROVIDER_SESSION_ID = 'e2e-missing-legacy-worker' @@ -46,22 +49,23 @@ appendLedger('ORCA_E2E_SPAWN_LEDGER', { event: 'spawn' }) process.stdout.write('\\u001b]0;Codex Ready\\u0007OpenAI Codex\\nmodel: e2e\\ndirectory: e2e\\n') let acknowledged = false ${FAKE_AGENT_PASTE_END_SCANNER_SOURCE} -let pasteEnded = false process.stdin.on('data', (chunk) => { const input = chunk.toString() const pasteEndScan = scanFakeAgentPasteEnd(fakeAgentPasteEndTail, input) fakeAgentPasteEndTail = pasteEndScan.tail - if (pasteEndScan.ended) { - pasteEnded = true + if (pasteEndScan.pasteEndOffset !== null) { process.stdout.write('\\x1b[?25h') } if (input.includes('\\x03')) { appendLedger('ORCA_E2E_INTERRUPTION_LEDGER', { event: 'stdin-ctrl-c' }) } - if (!acknowledged && pasteEnded && input.includes('\\r')) { - acknowledged = true - process.stdout.write('\\u001b]0;Codex Working\\u0007ACK\\n') - setTimeout(() => process.stdout.write('\\u001b]0;Codex Ready\\u0007'), 10) + if (!acknowledged) { + fakeAgentMaybeAck(pasteEndScan, input, (mode) => { + acknowledged = true + const message = mode === 'bracketed' ? 'ACK' : 'PASTE_PROTOCOL_ERROR' + process.stdout.write('\\u001b]0;Codex Working\\u0007' + message + '\\n') + setTimeout(() => process.stdout.write('\\u001b]0;Codex Ready\\u0007'), 10) + }) } }) process.stdin.setRawMode?.(true) @@ -215,11 +219,15 @@ test('a missing legacy worker cannot spawn a replacement during restart recovery firstApp = first.app const worktreeId = await attachRepoAndOpenTerminal(first.page, repoPath) await waitForSessionReady(first.page) - await first.page.evaluate(async (agentCommand) => { - await window.__store?.getState().updateSettings({ - agentCmdOverrides: { codex: agentCommand } - }) - }, fakeCodexCommand) + await first.page.evaluate( + async ({ agentCommand, terminalWindowsShell }) => { + await window.__store?.getState().updateSettings({ + agentCmdOverrides: { codex: agentCommand }, + terminalWindowsShell + }) + }, + { agentCommand: fakeCodexCommand, terminalWindowsShell: FAKE_AGENT_WINDOWS_SHELL } + ) await ensureTerminalVisible(first.page) await getActiveTabId(first.page) await waitForActivePanePtyId(first.page) @@ -285,7 +293,7 @@ test('a missing legacy worker cannot spawn a replacement during restart recovery const transcriptPath = session.seedCodexResumeRollout(PROVIDER_SESSION_ID, repoPath) await first.page.evaluate( - ({ paneKey, tabId, workerWorktreeId, terminalHandle, transcript }) => { + ({ agentCommand, paneKey, tabId, workerWorktreeId, terminalHandle, transcript }) => { window.__store?.getState().setAgentStatus( paneKey, { state: 'working', prompt: 'Respond ACK and remain idle', agentType: 'codex' }, @@ -299,7 +307,10 @@ test('a missing legacy worker cannot spawn a replacement during restart recovery transcriptPath: transcript }, launchConfig: { - agentCommand: 'codex', + // Why not bare 'codex': resume prefers the captured command over + // agentCmdOverrides, so a bare name would resolve the machine's real + // Codex off PATH and unpin the adoption leg this spec exercises. + agentCommand, agentArgs: '--dangerously-bypass-approvals-and-sandbox', agentEnv: {} } @@ -308,6 +319,7 @@ test('a missing legacy worker cannot spawn a replacement during restart recovery window.__store?.getState().captureAllSleepingAgentSessions('quit') }, { + agentCommand: fakeCodexCommand, paneKey: workerPaneKey, tabId: worker!.tabId, workerWorktreeId: worker!.worktreeId, diff --git a/tests/e2e/orchestration-legacy-worker-restart-recovery.spec.ts b/tests/e2e/orchestration-legacy-worker-restart-recovery.spec.ts index 3d7af2d7b6f..68c508eca62 100644 --- a/tests/e2e/orchestration-legacy-worker-restart-recovery.spec.ts +++ b/tests/e2e/orchestration-legacy-worker-restart-recovery.spec.ts @@ -24,7 +24,10 @@ import { import { DEFAULT_LOCAL_ORCA_PROFILE_ID } from '../../src/shared/orca-profiles' import type { RuntimeTerminalListResult, RuntimeTerminalRead } from '../../src/shared/runtime-types' import { listAllOrchestrationRuns } from './orchestration-run-pages' -import { buildFakeAgentCommandOverride } from './helpers/fake-agent-command-override' +import { + buildFakeAgentCommandOverride, + FAKE_AGENT_WINDOWS_SHELL +} from './helpers/fake-agent-command-override' import { FAKE_AGENT_PASTE_END_SCANNER_SOURCE } from './helpers/fake-agent-paste-end-scanner' const PROVIDER_SESSION_ID = 'e2e-legacy-orchestration-worker' @@ -46,7 +49,7 @@ function appendLedger(envName, event) { appendFileSync(ledgerPath, JSON.stringify({ pid: process.pid, at: Date.now(), ...event }) + '\\n') } catch {} } -async function emitAuthorityHook() { +async function emitAuthorityHook(hookEventName) { const port = process.env.ORCA_AGENT_HOOK_PORT const token = process.env.ORCA_AGENT_HOOK_TOKEN const launchToken = process.env.ORCA_AGENT_LAUNCH_TOKEN @@ -66,12 +69,16 @@ async function emitAuthorityHook() { version: process.env.ORCA_AGENT_HOOK_VERSION, launchToken, payload: { - hook_event_name: 'UserPromptSubmit', + hook_event_name: hookEventName, prompt: 'Respond ACK and remain idle' } }) }) - appendLedger('ORCA_E2E_AUTHORITY_LEDGER', { event: 'authority-hook', status: response.status }) + appendLedger('ORCA_E2E_AUTHORITY_LEDGER', { + event: 'authority-hook', + hookEventName, + status: response.status + }) } catch (error) { appendLedger('ORCA_E2E_AUTHORITY_LEDGER', { event: 'authority-hook-error', @@ -85,26 +92,28 @@ if (process.argv.slice(2).includes('app-server')) { } appendLedger('ORCA_E2E_SPAWN_LEDGER', { event: 'spawn', argv: process.argv.slice(2) }) process.stdout.write('\\u001b]0;Codex Ready\\u0007OpenAI Codex\\nmodel: e2e\\ndirectory: e2e\\n') -void emitAuthorityHook() +const sessionStartHook = emitAuthorityHook('SessionStart') let acknowledged = false let lifecycleSent = false ${FAKE_AGENT_PASTE_END_SCANNER_SOURCE} -let pasteEnded = false process.stdin.on('data', (chunk) => { const input = chunk.toString() const pasteEndScan = scanFakeAgentPasteEnd(fakeAgentPasteEndTail, input) fakeAgentPasteEndTail = pasteEndScan.tail - if (pasteEndScan.ended) { - pasteEnded = true + if (pasteEndScan.pasteEndOffset !== null) { process.stdout.write('\\x1b[?25h') } if (input.includes('\\x03')) { appendLedger('ORCA_E2E_INTERRUPTION_LEDGER', { event: 'stdin-ctrl-c' }) } - if (!acknowledged && pasteEnded && input.includes('\\r')) { - acknowledged = true - process.stdout.write('\\u001b]0;Codex Working\\u0007ACK\\n') - setTimeout(() => process.stdout.write('\\u001b]0;Codex Ready\\u0007'), 10) + if (!acknowledged) { + fakeAgentMaybeAck(pasteEndScan, input, (mode) => { + acknowledged = true + void sessionStartHook.then(() => emitAuthorityHook('UserPromptSubmit')) + const message = mode === 'bracketed' ? 'ACK' : 'PASTE_PROTOCOL_ERROR' + process.stdout.write('\\u001b]0;Codex Working\\u0007' + message + '\\n') + setTimeout(() => process.stdout.write('\\u001b]0;Codex Ready\\u0007'), 10) + }) } const legacyCompletion = input.match(/ORCA_E2E_RUN_LEGACY_DONE:([A-Za-z0-9+/=]+)/) if (!lifecycleSent && legacyCompletion) { @@ -178,6 +187,7 @@ type LedgerEvent = { stdout?: string stderr?: string error?: string + hookEventName?: string } type PersistedWorkspaceSession = { @@ -429,11 +439,15 @@ for (const contractVersion of [LEGACY_CONTRACT_VERSION, CURRENT_CONTRACT_VERSION firstApp = first.app const worktreeId = await attachRepoAndOpenTerminal(first.page, repoPath) await waitForSessionReady(first.page) - await first.page.evaluate(async (agentCommand) => { - await window.__store?.getState().updateSettings({ - agentCmdOverrides: { codex: agentCommand } - }) - }, fakeCodexCommand) + await first.page.evaluate( + async ({ agentCommand, terminalWindowsShell }) => { + await window.__store?.getState().updateSettings({ + agentCmdOverrides: { codex: agentCommand }, + terminalWindowsShell + }) + }, + { agentCommand: fakeCodexCommand, terminalWindowsShell: FAKE_AGENT_WINDOWS_SHELL } + ) await ensureTerminalVisible(first.page) const coordinatorTabId = await getActiveTabId(first.page) expect(coordinatorTabId).toBeTruthy() @@ -531,11 +545,29 @@ for (const contractVersion of [LEGACY_CONTRACT_VERSION, CURRENT_CONTRACT_VERSION expect(readLedger(interruptionLedgerPath)).toEqual([]) await expect .poll(() => readLedger(authorityLedgerPath)) - .toEqual([expect.objectContaining({ event: 'authority-hook', status: 204 })]) + .toEqual([ + expect.objectContaining({ + event: 'authority-hook', + hookEventName: 'SessionStart', + status: 204 + }), + expect.objectContaining({ + event: 'authority-hook', + hookEventName: 'UserPromptSubmit', + status: 204 + }) + ]) const transcriptPath = session.seedCodexResumeRollout(PROVIDER_SESSION_ID, repoPath) await first.page.evaluate( - ({ paneKey, tabId, worktreeId: workerWorktreeId, terminalHandle, transcript }) => { + ({ + agentCommand, + paneKey, + tabId, + worktreeId: workerWorktreeId, + terminalHandle, + transcript + }) => { window.__store?.getState().setAgentStatus( paneKey, { state: 'working', prompt: 'Respond ACK and remain idle', agentType: 'codex' }, @@ -549,7 +581,10 @@ for (const contractVersion of [LEGACY_CONTRACT_VERSION, CURRENT_CONTRACT_VERSION transcriptPath: transcript }, launchConfig: { - agentCommand: 'codex', + // Why not bare 'codex': resume prefers the captured command over + // agentCmdOverrides, so a bare name would resolve the machine's real + // Codex off PATH and unpin the adoption leg this spec exercises. + agentCommand, agentArgs: '--dangerously-bypass-approvals-and-sandbox', agentEnv: {} } @@ -558,6 +593,7 @@ for (const contractVersion of [LEGACY_CONTRACT_VERSION, CURRENT_CONTRACT_VERSION window.__store?.getState().captureAllSleepingAgentSessions('quit') }, { + agentCommand: fakeCodexCommand, paneKey: workerPaneKey, tabId: worker!.tabId, worktreeId: worker!.worktreeId, diff --git a/tests/e2e/orchestration-worker-settlement-release-cli.spec.ts b/tests/e2e/orchestration-worker-settlement-release-cli.spec.ts index b8a092e4ea3..f71668306b2 100644 --- a/tests/e2e/orchestration-worker-settlement-release-cli.spec.ts +++ b/tests/e2e/orchestration-worker-settlement-release-cli.spec.ts @@ -8,7 +8,10 @@ import { ensureTerminalVisible, waitForActiveWorktree, waitForSessionReady } fro import { RuntimeClient } from '../../src/cli/runtime-client' import Database from '../../src/main/sqlite/sync-database' import type { RuntimeTerminalListResult, RuntimeTerminalRead } from '../../src/shared/runtime-types' -import { buildFakeAgentCommandOverride } from './helpers/fake-agent-command-override' +import { + buildFakeAgentCommandOverride, + FAKE_AGENT_WINDOWS_SHELL +} from './helpers/fake-agent-command-override' import { FAKE_AGENT_PASTE_END_SCANNER_SOURCE } from './helpers/fake-agent-paste-end-scanner' const fakeCliDir = mkdtempSync(path.join(os.tmpdir(), 'orca-e2e-settlement-release-')) @@ -27,21 +30,22 @@ if (process.argv.slice(2).includes('app-server')) { let capability = null let acknowledged = false ${FAKE_AGENT_PASTE_END_SCANNER_SOURCE} -let pasteEnded = false process.stdout.write('\\u001b]0;Codex Ready\\u0007OpenAI Codex\\nmodel: e2e\\ndirectory: e2e\\n') process.stdin.on('data', (chunk) => { const input = chunk.toString() const pasteEndScan = scanFakeAgentPasteEnd(fakeAgentPasteEndTail, input) fakeAgentPasteEndTail = pasteEndScan.tail - if (pasteEndScan.ended) { - pasteEnded = true + if (pasteEndScan.pasteEndOffset !== null) { process.stdout.write('\\x1b[?25h') } capability ||= input.match(/--dispatch-capability (dcap_[A-Za-z0-9_-]+)/)?.[1] || null - if (!acknowledged && pasteEnded && input.includes('\\r')) { - acknowledged = true - process.stdout.write('\\u001b]0;Codex Working\\u0007ACK\\n') - setTimeout(() => process.stdout.write('\\u001b]0;Codex Ready\\u0007'), 10) + if (!acknowledged) { + fakeAgentMaybeAck(pasteEndScan, input, (mode) => { + acknowledged = true + const message = mode === 'bracketed' ? 'ACK' : 'PASTE_PROTOCOL_ERROR' + process.stdout.write('\\u001b]0;Codex Working\\u0007' + message + '\\n') + setTimeout(() => process.stdout.write('\\u001b]0;Codex Ready\\u0007'), 10) + }) } const encoded = input.match(/ORCA_E2E_WORKER_DONE:([A-Za-z0-9+/=]+)/)?.[1] if (!encoded || !capability) return @@ -140,11 +144,15 @@ test('compiled CLI rejects false completion then reconciles the dead retained wo test.setTimeout(180_000) rmSync(cliLedgerPath, { force: true }) await waitForSessionReady(orcaPage) - await orcaPage.evaluate(async (agentCommand) => { - await window.__store?.getState().updateSettings({ - agentCmdOverrides: { codex: agentCommand } - }) - }, fakeCodexCommand) + await orcaPage.evaluate( + async ({ agentCommand, terminalWindowsShell }) => { + await window.__store?.getState().updateSettings({ + agentCmdOverrides: { codex: agentCommand }, + terminalWindowsShell + }) + }, + { agentCommand: fakeCodexCommand, terminalWindowsShell: FAKE_AGENT_WINDOWS_SHELL } + ) const worktreeId = await waitForActiveWorktree(orcaPage) await ensureTerminalVisible(orcaPage) await waitForActivePanePtyId(orcaPage) From e3327c2f31691ee3ce16795b6ffd45b9c16fc371 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun, 23 Aug 2026 18:27:41 +0000 Subject: [PATCH 13/14] Update README downloads badge --- docs/assets/readme-downloads.svg | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/assets/readme-downloads.svg b/docs/assets/readme-downloads.svg index 0d9bf545b35..a9566939925 100644 --- a/docs/assets/readme-downloads.svg +++ b/docs/assets/readme-downloads.svg @@ -1,5 +1,5 @@ - - downloads: 26m + + downloads: 27m @@ -15,7 +15,7 @@ downloads downloads - 26m - 26m + 27m + 27m From 853afdf80eb044df2db43e5db8e37ea2d5226963 Mon Sep 17 00:00:00 2001 From: Isaque Coelho Date: Sun, 23 Aug 2026 16:21:15 -0300 Subject: [PATCH 14/14] fix(status-bar): remove pet menu reserved space (#13067) * fix(status-bar): remove pet menu reserved space * test(status-bar): add pet segment layout validation tests - Unit test guards against pr-[6.5rem] padding reintroduction - E2E test measures trailing overhang instead of total width delta for more accurate layout validation - Extract enableExperimentalPet helper for test clarity --------- Co-authored-by: m4air --- .../PetStatusSegment.layout.test.ts | 15 ++++ .../status-bar/PetStatusSegment.tsx | 2 +- tests/e2e/pet-status-segment-layout.spec.ts | 83 +++++++++++++++++++ 3 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 src/renderer/src/components/status-bar/PetStatusSegment.layout.test.ts create mode 100644 tests/e2e/pet-status-segment-layout.spec.ts diff --git a/src/renderer/src/components/status-bar/PetStatusSegment.layout.test.ts b/src/renderer/src/components/status-bar/PetStatusSegment.layout.test.ts new file mode 100644 index 00000000000..e213100816e --- /dev/null +++ b/src/renderer/src/components/status-bar/PetStatusSegment.layout.test.ts @@ -0,0 +1,15 @@ +import { readFileSync } from 'node:fs' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' + +describe('PetStatusSegment layout', () => { + it('does not reserve fixed right padding on the pet menu trigger', () => { + const source = readFileSync(join(__dirname, 'PetStatusSegment.tsx'), 'utf8') + + // Why: the old pr-[6.5rem] reserved ~104px of empty space after the label and + // shoved neighboring status-bar segments left; unit-guard so CI catches a + // reintroduction without needing Electron Playwright. + expect(source).not.toMatch(/pr-\[\d+(?:\.\d+)?rem\]/) + expect(source).toMatch(/className="group inline-flex items-center cursor-pointer pl-1 py-0\.5"/) + }) +}) diff --git a/src/renderer/src/components/status-bar/PetStatusSegment.tsx b/src/renderer/src/components/status-bar/PetStatusSegment.tsx index 48cd2f0292b..8cf2b1b5dcf 100644 --- a/src/renderer/src/components/status-bar/PetStatusSegment.tsx +++ b/src/renderer/src/components/status-bar/PetStatusSegment.tsx @@ -116,7 +116,7 @@ function PetStatusSegmentInner(): React.JSX.Element {