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 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/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/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-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..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 @@ -33,7 +34,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 +266,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' } } } @@ -975,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') @@ -1081,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/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 +} 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. 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 a646a9625b5..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' @@ -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,135 @@ 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 }) + } + }) +}) + +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/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/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() + }) }) 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/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 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. 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 { 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 `
@@ -836,13 +855,22 @@ export function AgentsPane({ {isBlankDefault && } - {enabledDetectedAgents.map((agent) => { + {defaultAgentPills.map((agent) => { const isActive = defaultAgent === agent.id + const isUndetected = detectedIds !== null && !detectedIds.has(agent.id) return ( setDefault(agent.id)} + title={ + isUndetected + ? translate( + 'auto.components.settings.AgentsPane.storedDefaultUndetected', + 'Saved as your default, but not detected right now' + ) + : undefined + } > {agent.label} @@ -877,6 +905,33 @@ export function AgentsPane({ + {detectedAgents.length === 0 && detectedIds !== null && !detectionFailed && ( +
+ {/* Why here and not only inside Installed: that section renders under + `detectedAgents.length > 0`, so the only Refresh control vanished + in precisely the state that needs a retry (#15256). */} + + {translate( + 'auto.components.settings.AgentsPane.noAgentsDetected', + 'No agents detected. If one is installed, the probe may have timed out.' + )} + + +
+ )} + {detectedAgents.length > 0 && (
{ + 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 { + +
+ + + ) +} 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/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/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 596ef2f4e3d..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}}", @@ -6240,7 +6248,9 @@ "3f1bdf3cb4": "Environment text is too large to parse safely.", "codexSessionSource": "Codex home to import from", "codexSessionSourceInfo": "About importing Codex history", - "codexSessionSourceTooltip": "Orca runs Codex in an isolated home. Point this at your existing Codex home to import that session history. Empty uses ~/.codex." + "codexSessionSourceTooltip": "Orca runs Codex in an isolated home. Point this at your existing Codex home to import that session history. Empty uses ~/.codex.", + "storedDefaultUndetected": "Saved as your default, but not detected right now", + "noAgentsDetected": "No agents detected. If one is installed, the probe may have timed out." }, "AppIconSelector": { "d5a112dc9b": "Next icon", 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}}", 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/posix-command-path-lookup.ts b/src/shared/posix-command-path-lookup.ts index fb29be82294..ea09ca83e2f 100644 --- a/src/shared/posix-command-path-lookup.ts +++ b/src/shared/posix-command-path-lookup.ts @@ -4,12 +4,59 @@ 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) + // `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+set}" = set ] || _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 +81,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 ;;', 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/windows-long-path-git-args.test.ts b/src/shared/windows-long-path-git-args.test.ts new file mode 100644 index 00000000000..5ecf0cbde0f --- /dev/null +++ b/src/shared/windows-long-path-git-args.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, it } from 'vitest' + +import { windowsLongPathGitArgs } from './windows-long-path-git-args' + +describe('windowsLongPathGitArgs', () => { + 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] +} 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, 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) diff --git a/tests/e2e/pet-status-segment-layout.spec.ts b/tests/e2e/pet-status-segment-layout.spec.ts new file mode 100644 index 00000000000..b7d575117e5 --- /dev/null +++ b/tests/e2e/pet-status-segment-layout.spec.ts @@ -0,0 +1,83 @@ +import type { Page, TestInfo } from '@stablyai/playwright-test' +import { expect, test } from './helpers/orca-app' +import { waitForSessionReady } from './helpers/store' + +// Why: post-fix trailing overhang is ~0px; 16px is subpixel headroom that still +// fails the old pr-[6.5rem] (~104px) reservation. +const MAX_TRAILING_CHROME_PX = 16 +const VIEWPORTS = [ + { name: 'desktop', size: { width: 1440, height: 900 } }, + { name: 'mobile', size: { width: 390, height: 844 } } +] as const + +async function enableExperimentalPet(page: Page): Promise { + await page.evaluate(async () => { + const store = window.__store + if (!store) { + throw new Error('window.__store is not available') + } + // Why: updateSettings swallows errors; throw so a failed persist fails loudly. + await store.getState().updateSettingsOrThrow({ experimentalPet: true }) + }) + await expect + .poll( + async () => + page.evaluate(() => window.__store?.getState().settings?.experimentalPet === true), + { message: 'experimentalPet did not become true after updateSettingsOrThrow' } + ) + .toBe(true) +} + +async function assertPetTriggerFitsLabel(page: Page): Promise { + const trigger = page.getByRole('button', { name: 'Pet menu' }) + // Why: only the label span is a direct child; avoid matching nested menu spans + // if the open state ever leaks into the measurement. + const label = trigger.locator(':scope > span') + + await expect(trigger).toBeVisible() + await expect(label).toBeVisible() + // Why: measure trailing overhang (trigger right − label right), not total width + // delta — the regression is empty space *after* the label (the old pr-[6.5rem]). + await expect + .poll( + async () => { + const [triggerBox, labelBox] = await Promise.all([ + trigger.boundingBox(), + label.boundingBox() + ]) + if (!triggerBox || !labelBox) { + return Number.POSITIVE_INFINITY + } + return triggerBox.x + triggerBox.width - (labelBox.x + labelBox.width) + }, + { message: 'pet menu trigger still reserves trailing space after its label' } + ) + .toBeLessThanOrEqual(MAX_TRAILING_CHROME_PX) +} + +async function attachTriggerScreenshot( + page: Page, + testInfo: TestInfo, + name: string +): Promise { + const trigger = page.getByRole('button', { name: 'Pet menu' }) + const screenshotPath = testInfo.outputPath(`pet-status-trigger-${name}.png`) + await trigger.screenshot({ path: screenshotPath }) + await testInfo.attach(`pet-status-trigger-${name}`, { + path: screenshotPath, + contentType: 'image/png' + }) +} + +test.describe('Pet status segment layout', () => { + test('does not reserve empty space after its label', async ({ orcaPage }, testInfo) => { + await waitForSessionReady(orcaPage) + await enableExperimentalPet(orcaPage) + + for (const { name, size } of VIEWPORTS) { + await orcaPage.setViewportSize(size) + await assertPetTriggerFitsLabel(orcaPage) + await attachTriggerScreenshot(orcaPage, testInfo, name) + } + }) +})