From a7fda48fe3faa55b6248cd570165095be042e769 Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Tue, 1 Sep 2026 22:33:39 -0400 Subject: [PATCH 1/8] feat(telemetry): measure macOS stale-daemon adoption and cwd denials (#18043) * feat(telemetry): measure macOS stale-daemon adoption and cwd denials Adds two enum-only PostHog events so #17696 can be sized instead of guessed at: - daemon_adopted: once per macOS launch that keeps a daemon an earlier app launch forked (invisible to daemon_lifecycle, which only sees replacements). Carries app-version match, spawner-path class (installed app / Squirrel ShipIt cache / other / missing), the existing TCC attribution verdict, and the bucketed live-session count. - daemon_pty_cwd_denied: the symptom itself. The daemon probes the requested cwd in its own process (only its TCC context counts) and returns an additive cwdReadableByDaemon field; the app emits only when the daemon was denied AND the app can read the same path, so a missing or genuinely unreadable cwd never counts. Non-permission errors read as readable on purpose. Both emitters swallow every failure; nothing here can delay or fail daemon startup or a PTY spawn. Off macOS neither event fires. The new wire field is optional, so older daemons and clients are unaffected. * fix(telemetry): keep cwd-denial classification inside the swallow guard Read the pid record at emit time (inside the try) rather than passing the adapter's startup snapshot: a throwing app-environment read can no longer escape spawn(), and a denial after a respawn is billed to the daemon that actually spawned the PTY. --- .../daemon-adoption-telemetry-event.test.ts | 168 ++++++++++++++++++ .../daemon/daemon-adoption-telemetry-event.ts | 81 +++++++++ .../daemon/daemon-create-or-attach-result.ts | 6 + .../daemon/daemon-init-dependency-mocks.ts | 9 +- src/main/daemon/daemon-init-fresh-import.ts | 4 +- src/main/daemon/daemon-init-mock-types.ts | 3 + .../daemon-init-provider-installation.test.ts | 45 +++++ src/main/daemon/daemon-init-test-harness.ts | 4 +- .../daemon/daemon-out-of-process-launcher.ts | 1 + src/main/daemon/daemon-provider-init.ts | 31 ++++ src/main/daemon/daemon-pty-session-spawn.ts | 4 + src/main/daemon/daemon-spawner.ts | 2 + src/main/daemon/daemon-terminal-admission.ts | 5 +- .../daemon/terminal-host-create-contract.ts | 2 + .../terminal-host-cwd-readability.test.ts | 75 ++++++++ .../daemon/terminal-host-session-create.ts | 17 ++ src/main/ipc/telemetry.ts | 2 + src/shared/daemon-adoption-telemetry.test.ts | 89 ++++++++++ src/shared/daemon-adoption-telemetry.ts | 67 +++++++ src/shared/telemetry-daemon-event-schemas.ts | 27 +++ src/shared/telemetry-event-registry.ts | 4 + 21 files changed, 642 insertions(+), 4 deletions(-) create mode 100644 src/main/daemon/daemon-adoption-telemetry-event.test.ts create mode 100644 src/main/daemon/daemon-adoption-telemetry-event.ts create mode 100644 src/main/daemon/terminal-host-cwd-readability.test.ts create mode 100644 src/shared/daemon-adoption-telemetry.test.ts create mode 100644 src/shared/daemon-adoption-telemetry.ts diff --git a/src/main/daemon/daemon-adoption-telemetry-event.test.ts b/src/main/daemon/daemon-adoption-telemetry-event.test.ts new file mode 100644 index 00000000000..5a5cd7406c4 --- /dev/null +++ b/src/main/daemon/daemon-adoption-telemetry-event.test.ts @@ -0,0 +1,168 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { ParsedDaemonPid } from './daemon-pid-file-parse' +import { validate } from '../telemetry/validator' + +const { trackMock, accessSyncMock, existsSyncMock, readFileSyncMock, getVersionMock } = vi.hoisted( + () => ({ + trackMock: vi.fn(), + accessSyncMock: vi.fn(), + existsSyncMock: vi.fn(() => true), + readFileSyncMock: vi.fn(), + getVersionMock: vi.fn(() => '1.4.191') + }) +) +vi.mock('../telemetry/client', () => ({ track: trackMock })) +vi.mock('node:fs', async (importOriginal) => ({ + ...(await importOriginal>()), + accessSync: accessSyncMock, + existsSync: existsSyncMock, + readFileSync: readFileSyncMock +})) +vi.mock('node:os', async (importOriginal) => ({ + ...(await importOriginal>()), + homedir: () => '/Users/alice' +})) +vi.mock('../../shared/app-environment', () => ({ + getAppEnvironment: () => ({ getVersion: getVersionMock }) +})) + +import { + classifyDaemonAdoptionOrigin, + trackDaemonAdopted, + trackDaemonPtyCwdDeniedIfDiverged +} from './daemon-adoption-telemetry-event' + +const stalePidRecord: ParsedDaemonPid = { + pid: 1530, + startedAtMs: 1, + entryPath: '/x/daemon-entry.js', + appVersion: '1.4.187', + launchNonce: 'n', + linuxStartTicks: null, + bootId: null, + spawnerExecPath: + '/Users/alice/Library/Caches/com.stablyai.orca.ShipIt/u/Orca.app/Contents/MacOS/Orca' +} +const origin = { app_version_match: 'different', spawner_path_class: 'updater-cache' } as const +const PID_PATH = '/fake/daemon.pid' + +beforeEach(() => { + trackMock.mockReset() + accessSyncMock.mockReset() + existsSyncMock.mockReset().mockReturnValue(true) + readFileSyncMock.mockReset().mockReturnValue(JSON.stringify(stalePidRecord)) + vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin') +}) + +afterEach(() => { + vi.restoreAllMocks() +}) + +describe('classifyDaemonAdoptionOrigin', () => { + it('compares the recorded app version and classifies the spawner path', () => { + expect(classifyDaemonAdoptionOrigin(stalePidRecord)).toEqual(origin) + expect(classifyDaemonAdoptionOrigin({ ...stalePidRecord, appVersion: '1.4.191' })).toEqual({ + app_version_match: 'same', + spawner_path_class: 'updater-cache' + }) + expect(classifyDaemonAdoptionOrigin(null)).toEqual({ + app_version_match: 'unknown', + spawner_path_class: 'unknown' + }) + }) +}) + +describe('trackDaemonAdopted', () => { + it('emits a validator-accepted payload', () => { + trackDaemonAdopted(stalePidRecord, 'intact', 7) + expect(trackMock).toHaveBeenCalledTimes(1) + const [name, props] = trackMock.mock.calls[0] + expect(name).toBe('daemon_adopted') + expect(props).toEqual({ + ...origin, + tcc_attribution: 'intact', + live_session_count_bucket: '6+' + }) + expect(validate('daemon_adopted', props).ok).toBe(true) + }) + + it('swallows a throwing telemetry client', () => { + trackMock.mockImplementationOnce(() => { + throw new Error('posthog exploded') + }) + expect(() => trackDaemonAdopted(null, 'unknown', null)).not.toThrow() + }) +}) + +describe('trackDaemonPtyCwdDeniedIfDiverged', () => { + it('emits only when the daemon was denied and the app can read the same cwd', () => { + trackDaemonPtyCwdDeniedIfDiverged('/Users/alice/Documents/repo', false, PID_PATH) + expect(accessSyncMock).toHaveBeenCalledWith('/Users/alice/Documents/repo', expect.any(Number)) + expect(trackMock).toHaveBeenCalledTimes(1) + const [name, props] = trackMock.mock.calls[0] + expect(name).toBe('daemon_pty_cwd_denied') + expect(props).toEqual({ cwd_class: 'documents', ...origin }) + expect(validate('daemon_pty_cwd_denied', props).ok).toBe(true) + }) + + // False positives would drown the signal this event exists to measure, so every + // non-divergent shape must stay silent. + it('stays silent when the daemon could read the cwd or did not report', () => { + trackDaemonPtyCwdDeniedIfDiverged('/Users/alice/Documents/repo', true, PID_PATH) + trackDaemonPtyCwdDeniedIfDiverged('/Users/alice/Documents/repo', undefined, PID_PATH) + trackDaemonPtyCwdDeniedIfDiverged(undefined, false, PID_PATH) + expect(accessSyncMock).not.toHaveBeenCalled() + expect(trackMock).not.toHaveBeenCalled() + }) + + it('stays silent when the app cannot read the cwd either (no divergence)', () => { + accessSyncMock.mockImplementation(() => { + throw Object.assign(new Error('EACCES'), { code: 'EACCES' }) + }) + trackDaemonPtyCwdDeniedIfDiverged('/Users/alice/Documents/repo', false, PID_PATH) + expect(trackMock).not.toHaveBeenCalled() + }) + + it('attributes the denial to the daemon recorded right now, not a startup snapshot', () => { + readFileSyncMock.mockReturnValue( + JSON.stringify({ + ...stalePidRecord, + appVersion: '1.4.191', + spawnerExecPath: '/Applications/Orca.app/Contents/MacOS/Orca' + }) + ) + trackDaemonPtyCwdDeniedIfDiverged('/Users/alice/Documents/repo', false, PID_PATH) + expect(readFileSyncMock).toHaveBeenCalledWith(PID_PATH, 'utf8') + expect(trackMock.mock.calls[0][1]).toEqual({ + cwd_class: 'documents', + app_version_match: 'same', + spawner_path_class: 'applications' + }) + }) + + it('swallows a throwing app environment or pid-record read instead of failing the spawn', () => { + getVersionMock.mockImplementationOnce(() => { + throw new Error('AppEnvironment not initialized') + }) + expect(() => + trackDaemonPtyCwdDeniedIfDiverged('/Users/alice/Documents/repo', false, PID_PATH) + ).not.toThrow() + expect(trackMock).not.toHaveBeenCalled() + }) + + it('stays silent off macOS', () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('linux') + trackDaemonPtyCwdDeniedIfDiverged('/home/alice/Documents/repo', false, PID_PATH) + expect(accessSyncMock).not.toHaveBeenCalled() + expect(trackMock).not.toHaveBeenCalled() + }) + + it('swallows a throwing telemetry client', () => { + trackMock.mockImplementationOnce(() => { + throw new Error('posthog exploded') + }) + expect(() => + trackDaemonPtyCwdDeniedIfDiverged('/Users/alice/Documents/repo', false, PID_PATH) + ).not.toThrow() + }) +}) diff --git a/src/main/daemon/daemon-adoption-telemetry-event.ts b/src/main/daemon/daemon-adoption-telemetry-event.ts new file mode 100644 index 00000000000..47f554bf9bb --- /dev/null +++ b/src/main/daemon/daemon-adoption-telemetry-event.ts @@ -0,0 +1,81 @@ +// App-side emitters for `daemon_adopted` and `daemon_pty_cwd_denied` (#17696). Both sit on the +// daemon launch / PTY spawn path, so every failure dies here — telemetry can never cost a terminal. + +import { accessSync, constants as fsConstants, existsSync } from 'node:fs' +import { homedir } from 'node:os' +import { getAppEnvironment } from '../../shared/app-environment' +import { + classifyDaemonPtyCwd, + classifyDaemonSpawnerPath, + type DaemonAdoptedAppVersionMatch, + type DaemonSpawnerPathClass +} from '../../shared/daemon-adoption-telemetry' +import { bucketDaemonLiveSessionCount } from '../../shared/daemon-lifecycle-telemetry' +import type { EventProps } from '../../shared/telemetry-events' +import { track } from '../telemetry/client' +import { readDaemonPidRecord } from './daemon-endpoint-incarnation' +import type { ParsedDaemonPid } from './daemon-pid-file-parse' +import type { MacDaemonTccAttributionHealth } from './daemon-tcc-attribution' + +export type DaemonAdoptionOrigin = Pick< + EventProps<'daemon_pty_cwd_denied'>, + 'app_version_match' | 'spawner_path_class' +> + +/** Classifies the adopted daemon's pid record against the running app; enum-only by construction. */ +export function classifyDaemonAdoptionOrigin( + pidRecord: ParsedDaemonPid | null +): DaemonAdoptionOrigin { + const appVersionMatch: DaemonAdoptedAppVersionMatch = !pidRecord?.appVersion + ? 'unknown' + : pidRecord.appVersion === getAppEnvironment().getVersion() + ? 'same' + : 'different' + const spawnerPathClass: DaemonSpawnerPathClass = classifyDaemonSpawnerPath( + pidRecord?.spawnerExecPath ?? null, + existsSync + ) + return { app_version_match: appVersionMatch, spawner_path_class: spawnerPathClass } +} + +// Adopted a daemon that a previous app launch forked (macOS only; that is where attribution matters). +export function trackDaemonAdopted( + pidRecord: ParsedDaemonPid | null, + tccAttribution: MacDaemonTccAttributionHealth, + liveSessionCount: number | null +): void { + try { + track('daemon_adopted', { + ...classifyDaemonAdoptionOrigin(pidRecord), + tcc_attribution: tccAttribution, + live_session_count_bucket: bucketDaemonLiveSessionCount(liveSessionCount) + }) + } catch { + // Telemetry is best-effort; a dropped event must not fail daemon adoption. + } +} + +/** + * Emits only on proven divergence: the daemon reported the cwd unreadable AND this process can + * read it. A cwd neither can read (chmod, ENOENT, unmounted volume) is not the #17696 shape. + */ +export function trackDaemonPtyCwdDeniedIfDiverged( + cwd: string | undefined, + cwdReadableByDaemon: boolean | undefined, + pidPath: string | null +): void { + try { + if (process.platform !== 'darwin' || !cwd || cwdReadableByDaemon !== false) { + return + } + accessSync(cwd, fsConstants.R_OK | fsConstants.X_OK) + // Why read now, not the adapter's startup snapshot: a respawn swaps the daemon under a + // long-lived adapter, and the denial must be attributed to the daemon that just spawned. + track('daemon_pty_cwd_denied', { + cwd_class: classifyDaemonPtyCwd(cwd, homedir()), + ...classifyDaemonAdoptionOrigin(readDaemonPidRecord(pidPath)) + }) + } catch { + // Either the app cannot read it (no divergence) or telemetry failed; neither may reach the caller. + } +} diff --git a/src/main/daemon/daemon-create-or-attach-result.ts b/src/main/daemon/daemon-create-or-attach-result.ts index d6668342487..92a7e451a10 100644 --- a/src/main/daemon/daemon-create-or-attach-result.ts +++ b/src/main/daemon/daemon-create-or-attach-result.ts @@ -14,6 +14,12 @@ export type DaemonCreateOrAttachResult = { wslDistro?: string | null agentSessionEnsure?: AgentSessionClaimedSpawnResult incarnationId?: PtyIncarnationId + /** + * Whether the daemon process itself could read the requested cwd at spawn. Only the daemon's own + * verdict counts: macOS TCC scopes folder access per process tree, so the app's view of the same + * path proves nothing about the daemon's (#17696). Omitted by daemons predating this field. + */ + cwdReadableByDaemon?: boolean } export function getDaemonSessionResultMetadata(session: { diff --git a/src/main/daemon/daemon-init-dependency-mocks.ts b/src/main/daemon/daemon-init-dependency-mocks.ts index d920a13866e..d7217d4df63 100644 --- a/src/main/daemon/daemon-init-dependency-mocks.ts +++ b/src/main/daemon/daemon-init-dependency-mocks.ts @@ -50,7 +50,8 @@ export function createDaemonInitModuleFactories(state: DaemonInitMockState) { unbindLocalProviderListenersMock, rebindLocalProviderListenersMock, trackDaemonReplacedMock, - trackDaemonRetiredMock + trackDaemonRetiredMock, + trackDaemonAdoptedMock } = state // Why: both fakes are annotated with constructor types so the exported factories widen to @@ -82,6 +83,9 @@ export function createDaemonInitModuleFactories(state: DaemonInitMockState) { if (result.mode) { this.handle.mode = result.mode } + if (result.adopted) { + this.handle.adopted = true + } return { socketPath: result.socketPath, tokenPath: result.tokenPath @@ -199,6 +203,9 @@ export function createDaemonInitModuleFactories(state: DaemonInitMockState) { trackDaemonReplaced: trackDaemonReplacedMock, trackDaemonRetired: trackDaemonRetiredMock }), + daemonAdoptionTelemetryEvent: () => ({ + trackDaemonAdopted: trackDaemonAdoptedMock + }), daemonSpawner: () => ({ DaemonSpawner: MockDaemonSpawner, getDaemonSocketPath: (_dir: string, version?: number) => diff --git a/src/main/daemon/daemon-init-fresh-import.ts b/src/main/daemon/daemon-init-fresh-import.ts index e1cc0416337..39f3625c063 100644 --- a/src/main/daemon/daemon-init-fresh-import.ts +++ b/src/main/daemon/daemon-init-fresh-import.ts @@ -41,7 +41,8 @@ export async function importFreshDaemonInit(state: DaemonInitMockState) { unbindLocalProviderListenersMock, rebindLocalProviderListenersMock, trackDaemonReplacedMock, - trackDaemonRetiredMock + trackDaemonRetiredMock, + trackDaemonAdoptedMock } = state vi.resetModules() @@ -64,6 +65,7 @@ export async function importFreshDaemonInit(state: DaemonInitMockState) { rebindLocalProviderListenersMock.mockClear() trackDaemonReplacedMock.mockClear() trackDaemonRetiredMock.mockClear() + trackDaemonAdoptedMock.mockClear() checkDaemonHealthMock.mockClear() checkDaemonHealthMock.mockResolvedValue('healthy') healthCheckDaemonMock.mockClear() diff --git a/src/main/daemon/daemon-init-mock-types.ts b/src/main/daemon/daemon-init-mock-types.ts index 8c8b805740f..341fcd26df7 100644 --- a/src/main/daemon/daemon-init-mock-types.ts +++ b/src/main/daemon/daemon-init-mock-types.ts @@ -47,6 +47,7 @@ export type MockAdapterConstructor = new (opts: MockAdapter['options']) => MockA /** Handle the fake spawner hands back from ensureRunning/getHandle. */ export type MockSpawnerHandle = { mode?: 'degraded-new-pty-fallback' + adopted?: true releaseAdoptionLease?: () => void shutdown: () => Promise } @@ -95,6 +96,7 @@ export type EnsureRunningOverride = () => Promise<{ socketPath: string tokenPath: string mode?: 'degraded-new-pty-fallback' + adopted?: true }> /** Every stub daemon-init's suites share, plus the control knobs they mutate per test. */ @@ -143,6 +145,7 @@ export type DaemonInitMockState = { rebindLocalProviderListenersMock: Mock<(...args: unknown[]) => void> trackDaemonReplacedMock: Mock<(...args: unknown[]) => void> trackDaemonRetiredMock: Mock<(...args: unknown[]) => void> + trackDaemonAdoptedMock: Mock<(...args: unknown[]) => void> } /** net.connect stubs the suites install in beforeEach. */ diff --git a/src/main/daemon/daemon-init-provider-installation.test.ts b/src/main/daemon/daemon-init-provider-installation.test.ts index ceb7e9dfa69..423ee9ee34f 100644 --- a/src/main/daemon/daemon-init-provider-installation.test.ts +++ b/src/main/daemon/daemon-init-provider-installation.test.ts @@ -2,6 +2,8 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' const { isPackagedMock, + getMacDaemonTccAttributionHealthMock, + trackDaemonAdoptedMock, probeSocketExistsMock, readFileSyncMock, unlinkSyncMock, @@ -42,6 +44,7 @@ vi.mock('./daemon-process-start-time', () => moduleFactories.daemonProcessStartT vi.mock('./daemon-pid-file-parse', () => moduleFactories.daemonPidFileParse()) vi.mock('./client', () => moduleFactories.client()) vi.mock('./daemon-lifecycle-event', () => moduleFactories.daemonLifecycleEvent()) +vi.mock('./daemon-adoption-telemetry-event', () => moduleFactories.daemonAdoptionTelemetryEvent()) vi.mock('./daemon-spawner', () => moduleFactories.daemonSpawner()) vi.mock('./daemon-pty-adapter', () => moduleFactories.daemonPtyAdapter()) vi.mock('../ipc/pty', () => moduleFactories.ipcPty()) @@ -228,6 +231,48 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { expect(adapterInstances[1].disconnectOnly).toHaveBeenCalledOnce() }) + // #17696: adopting a daemon from an earlier app launch is invisible to daemon_lifecycle, so + // it gets its own event — macOS only, and only for adopted (not freshly forked) daemons. + it('reports a macOS daemon adoption with its TCC attribution and live session bucket', async () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin') + const mod = await importFresh() + ensureRunningOverrides.push(async () => ({ + socketPath: '/fake/adopted-socket', + tokenPath: '/fake/adopted-token', + adopted: true + })) + getMacDaemonTccAttributionHealthMock.mockResolvedValueOnce('severed') + defaultListSessionsSessions.push({ sessionId: 'wt-1@@a' }, { sessionId: 'wt-1@@b' }) + + await mod.initDaemonPtyProvider() + await vi.waitFor(() => expect(trackDaemonAdoptedMock).toHaveBeenCalledOnce()) + + // null pid record: the harness has no pid file, which the emitter classifies as 'unknown'. + expect(trackDaemonAdoptedMock).toHaveBeenCalledWith(null, 'severed', 2) + vi.restoreAllMocks() + }) + + it('does not report adoption for a freshly forked daemon or off macOS', async () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin') + const mod = await importFresh() + await mod.initDaemonPtyProvider() + await new Promise((resolve) => setImmediate(resolve)) + expect(trackDaemonAdoptedMock).not.toHaveBeenCalled() + vi.restoreAllMocks() + + vi.spyOn(process, 'platform', 'get').mockReturnValue('linux') + const linuxMod = await importFresh() + ensureRunningOverrides.push(async () => ({ + socketPath: '/fake/adopted-socket', + tokenPath: '/fake/adopted-token', + adopted: true + })) + await linuxMod.initDaemonPtyProvider() + await new Promise((resolve) => setImmediate(resolve)) + expect(trackDaemonAdoptedMock).not.toHaveBeenCalled() + vi.restoreAllMocks() + }) + it('routes fresh PTYs to the local fallback when a preserved daemon cannot spawn new PTYs', async () => { const mod = await importFresh() ensureRunningOverrides.push(async () => ({ diff --git a/src/main/daemon/daemon-init-test-harness.ts b/src/main/daemon/daemon-init-test-harness.ts index 6c38720a40b..6060ed9b759 100644 --- a/src/main/daemon/daemon-init-test-harness.ts +++ b/src/main/daemon/daemon-init-test-harness.ts @@ -156,6 +156,7 @@ function createDaemonInitMockState(): DaemonInitMockState { const rebindLocalProviderListenersMock = vi.fn() const trackDaemonReplacedMock = vi.fn() const trackDaemonRetiredMock = vi.fn() + const trackDaemonAdoptedMock = vi.fn() return { getPathMock, @@ -197,7 +198,8 @@ function createDaemonInitMockState(): DaemonInitMockState { unbindLocalProviderListenersMock, rebindLocalProviderListenersMock, trackDaemonReplacedMock, - trackDaemonRetiredMock + trackDaemonRetiredMock, + trackDaemonAdoptedMock } } diff --git a/src/main/daemon/daemon-out-of-process-launcher.ts b/src/main/daemon/daemon-out-of-process-launcher.ts index b59535a249a..21ff31918ac 100644 --- a/src/main/daemon/daemon-out-of-process-launcher.ts +++ b/src/main/daemon/daemon-out-of-process-launcher.ts @@ -41,6 +41,7 @@ function createPreservedDaemonHandle( mode?: 'degraded-new-pty-fallback' ): DaemonProcessHandle { const handle: DaemonProcessHandle = { + adopted: true, shutdown: async () => { await cleanupDaemonForProtocol(runtimeDir, protocolVersion) } diff --git a/src/main/daemon/daemon-provider-init.ts b/src/main/daemon/daemon-provider-init.ts index 1881e276e97..fa794257bda 100644 --- a/src/main/daemon/daemon-provider-init.ts +++ b/src/main/daemon/daemon-provider-init.ts @@ -24,7 +24,10 @@ import { import type { DaemonProvider } from './daemon-provider-routing' import { installDaemonProvider } from './daemon-provider-state' import { DegradedDaemonPtyProvider } from './degraded-daemon-pty-provider' +import { trackDaemonAdopted } from './daemon-adoption-telemetry-event' +import { readDaemonPidRecord } from './daemon-endpoint-incarnation' import { trackDaemonRetired } from './daemon-lifecycle-event' +import { getMacDaemonTccAttributionHealth } from './daemon-tcc-attribution' import { DaemonPtyAdapter } from './daemon-pty-adapter' import type { DaemonRespawnReason } from './daemon-pty-runtime-state' import { DaemonPtyRouter } from './daemon-pty-router' @@ -156,9 +159,37 @@ export async function initDaemonPtyProvider( logDaemonMilestone('daemon-init-done', { legacyAdapters: legacyAdapters.length }) + if (process.platform === 'darwin' && newSpawner.getHandle()?.adopted) { + void reportDaemonAdoption(runtimeDir, info.socketPath, info.tokenPath, newAdapter) + } await reconcileSeededClaudeLivePtys(routedAdapter) } +// Why off the init path: this is measurement of an adopted daemon (#17696), and neither its probes nor their failure may delay or fail startup. +async function reportDaemonAdoption( + runtimeDir: string, + socketPath: string, + tokenPath: string, + adapter: DaemonPtyAdapter +): Promise { + try { + const [tccAttribution, liveSessionCount] = await Promise.all([ + getMacDaemonTccAttributionHealth(runtimeDir, socketPath, tokenPath), + adapter.listSessions().then( + (sessions) => sessions.length, + () => null + ) + ]) + trackDaemonAdopted( + readDaemonPidRecord(getDaemonPidPath(runtimeDir)), + tccAttribution, + liveSessionCount + ) + } catch { + // Best-effort measurement only. + } +} + // Why: release gate ids only for daemon-confirmed-dead sessions; keep seeds on listing failure since releasing early can rotate a live CLI's refresh token. async function reconcileSeededClaudeLivePtys(provider: DaemonProvider): Promise { if (!hasSeededUnconfirmedClaudePtys()) { diff --git a/src/main/daemon/daemon-pty-session-spawn.ts b/src/main/daemon/daemon-pty-session-spawn.ts index 62c073f403e..235b286b0bc 100644 --- a/src/main/daemon/daemon-pty-session-spawn.ts +++ b/src/main/daemon/daemon-pty-session-spawn.ts @@ -4,6 +4,7 @@ import type { HistoryRecoveryContext, PendingDaemonSpawnOperation } from './daemon-pty-runtime-state' +import { trackDaemonPtyCwdDeniedIfDiverged } from './daemon-adoption-telemetry-event' import { STABLE_PANE_ATTACH_ONLY_DAEMON_PROTOCOL_VERSION } from './daemon-protocol-version' import { TerminalKilledError } from './daemon-pty-lifecycle-errors' import { DaemonPtySpawnResult } from './daemon-pty-spawn-result' @@ -246,6 +247,9 @@ export abstract class DaemonPtySessionSpawn extends DaemonPtySpawnResult { } activeSpawnContext = context const result = await this.createOrAttachSpawn(context, context.historySeedSegments) + if (result.isNew && !attachOnly) { + trackDaemonPtyCwdDeniedIfDiverged(effectiveCwd, result.cwdReadableByDaemon, this.pidPath) + } return this.finishSpawn(context, result) } diff --git a/src/main/daemon/daemon-spawner.ts b/src/main/daemon/daemon-spawner.ts index a0376ef0fc0..8c50b764b05 100644 --- a/src/main/daemon/daemon-spawner.ts +++ b/src/main/daemon/daemon-spawner.ts @@ -31,6 +31,8 @@ export type DaemonPidFile = { export type DaemonProcessHandle = { mode?: 'degraded-new-pty-fallback' + /** Set when the launcher kept a daemon some earlier app launch forked, rather than forking one. */ + adopted?: true releaseAdoptionLease?(): void shutdown(): Promise } diff --git a/src/main/daemon/daemon-terminal-admission.ts b/src/main/daemon/daemon-terminal-admission.ts index b47b497fe43..83dadf5f5d2 100644 --- a/src/main/daemon/daemon-terminal-admission.ts +++ b/src/main/daemon/daemon-terminal-admission.ts @@ -161,7 +161,10 @@ export class DaemonTerminalAdmission { ...(result.launchAgent ? { launchAgent: result.launchAgent } : {}), wslDistro: result.wslDistro, ...(result.historySeeded !== undefined ? { historySeeded: result.historySeeded } : {}), - ...(result.agentSessionEnsure ? { agentSessionEnsure: result.agentSessionEnsure } : {}) + ...(result.agentSessionEnsure ? { agentSessionEnsure: result.agentSessionEnsure } : {}), + ...(result.cwdReadableByDaemon !== undefined + ? { cwdReadableByDaemon: result.cwdReadableByDaemon } + : {}) } } diff --git a/src/main/daemon/terminal-host-create-contract.ts b/src/main/daemon/terminal-host-create-contract.ts index aaaffaeb8e8..42f5bf457f4 100644 --- a/src/main/daemon/terminal-host-create-contract.ts +++ b/src/main/daemon/terminal-host-create-contract.ts @@ -54,4 +54,6 @@ export type CreateOrAttachResult = { attachToken: symbol incarnationId: PtyIncarnationId agentSessionEnsure?: AgentSessionClaimedSpawnResult + /** Daemon-process verdict on the spawn cwd; only set on a fresh spawn that was given a cwd. */ + cwdReadableByDaemon?: boolean } diff --git a/src/main/daemon/terminal-host-cwd-readability.test.ts b/src/main/daemon/terminal-host-cwd-readability.test.ts new file mode 100644 index 00000000000..aa9e08379d7 --- /dev/null +++ b/src/main/daemon/terminal-host-cwd-readability.test.ts @@ -0,0 +1,75 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { SubprocessHandle } from './session-subprocess-handle' +import { TerminalHost, type TerminalHostOptions } from './terminal-host' + +vi.mock('../pty-descendant-termination', () => ({ killWithDescendantSweep: vi.fn() })) + +function createMockSubprocess(): SubprocessHandle { + let onExitCb: ((code: number) => void) | null = null + return { + pid: 99999, + getForegroundProcess: vi.fn(() => null), + write: vi.fn(), + resize: vi.fn(), + kill: vi.fn(() => { + setTimeout(() => onExitCb?.(0), 5) + }), + terminateOwnedTree: () => 'unavailable' as const, + forceKill: vi.fn(() => onExitCb?.(137)), + signal: vi.fn(), + onData() {}, + onExit(cb) { + onExitCb = cb + }, + dispose: vi.fn() + } +} + +// #17696: only the daemon process can say whether TCC lets it read the cwd, so its verdict +// rides on the create result. A non-permission failure must never read as denial. +describe('TerminalHost cwd readability verdict', () => { + let host: TerminalHost + let platformDescriptor: PropertyDescriptor | undefined + + beforeEach(() => { + platformDescriptor = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { configurable: true, value: 'linux' }) + const spawnSubprocess: TerminalHostOptions['spawnSubprocess'] = () => createMockSubprocess() + host = new TerminalHost({ spawnSubprocess }) + }) + + afterEach(async () => { + await host.dispose() + if (platformDescriptor) { + Object.defineProperty(process, 'platform', platformDescriptor) + } + }) + + const create = (sessionId: string, cwd?: string) => + host.createOrAttach({ + sessionId, + cols: 80, + rows: 24, + ...(cwd ? { cwd } : {}), + streamClient: { onData: vi.fn(), onExit: vi.fn() } + }) + + it('reports a readable cwd as readable', async () => { + expect((await create('readable', process.cwd())).cwdReadableByDaemon).toBe(true) + }) + + it('reports a missing cwd as readable — absence is not a permission denial', async () => { + expect((await create('missing', '/definitely/not/a/real/dir')).cwdReadableByDaemon).toBe(true) + }) + + it('omits the verdict when no cwd was requested', async () => { + expect((await create('no-cwd')).cwdReadableByDaemon).toBeUndefined() + }) + + it('omits the verdict on attach to an existing session', async () => { + await create('attach', process.cwd()) + const attached = await create('attach', process.cwd()) + expect(attached.isNew).toBe(false) + expect(attached.cwdReadableByDaemon).toBeUndefined() + }) +}) diff --git a/src/main/daemon/terminal-host-session-create.ts b/src/main/daemon/terminal-host-session-create.ts index e1c8a22e750..9ee51c9968d 100644 --- a/src/main/daemon/terminal-host-session-create.ts +++ b/src/main/daemon/terminal-host-session-create.ts @@ -1,3 +1,4 @@ +import { accessSync, constants as fsConstants } from 'node:fs' import { buildStartupCommandSubmission } from '../../shared/startup-command-submission' import { resolvePtyOwnerBackend } from '../../shared/pty-owner-backend' import { getDaemonSessionResultMetadata } from './daemon-create-or-attach-result' @@ -88,6 +89,8 @@ async function spawnAndPublishSession( ctx: { size: { cols: number; rows: number }; wslDistro: string | undefined } ): Promise { const { size, wslDistro } = ctx + // Why before the fork: the shell's own cwd may already have fallen back, so probe the requested path. + const cwdReadableByDaemon = opts.cwd && !wslDistro ? isCwdReadableByThisProcess(opts.cwd) : null const subprocess = await deps.spawnSubprocess({ sessionId: opts.sessionId, cols: size.cols, @@ -184,6 +187,20 @@ async function spawnAndPublishSession( shellState: session.shellState, incarnationId: session.incarnationId, ...getDaemonSessionResultMetadata(session), + ...(cwdReadableByDaemon !== null ? { cwdReadableByDaemon } : {}), attachToken: token } } + +// Why R_OK|X_OK: listing a directory needs read, and entering it needs search — both are what +// TCC withholds. A non-permission failure (ENOENT, ENOTDIR) reads as readable so it can never +// masquerade as a permission denial. +function isCwdReadableByThisProcess(cwd: string): boolean { + try { + accessSync(cwd, fsConstants.R_OK | fsConstants.X_OK) + return true + } catch (error) { + const code = (error as NodeJS.ErrnoException).code + return code !== 'EACCES' && code !== 'EPERM' + } +} diff --git a/src/main/ipc/telemetry.ts b/src/main/ipc/telemetry.ts index c1fe7d0f68d..5e649f51b84 100644 --- a/src/main/ipc/telemetry.ts +++ b/src/main/ipc/telemetry.ts @@ -24,7 +24,9 @@ let storeRef: Store | null = null const MAIN_OWNED_TELEMETRY_EVENTS = new Set([ 'app_starred_orca', + 'daemon_adopted', 'daemon_audit_eligibility', + 'daemon_pty_cwd_denied', 'star_nag_outcome', 'feature_interaction_usage_bucket_reached' ]) diff --git a/src/shared/daemon-adoption-telemetry.test.ts b/src/shared/daemon-adoption-telemetry.test.ts new file mode 100644 index 00000000000..f02aa431506 --- /dev/null +++ b/src/shared/daemon-adoption-telemetry.test.ts @@ -0,0 +1,89 @@ +import { describe, expect, it } from 'vitest' +import { classifyDaemonPtyCwd, classifyDaemonSpawnerPath } from './daemon-adoption-telemetry' +import { eventSchemas } from './telemetry-event-registry' + +describe('classifyDaemonSpawnerPath', () => { + const alwaysExists = () => true + + it('classifies the installed app, the ShipIt staging area, and everything else', () => { + expect( + classifyDaemonSpawnerPath('/Applications/Orca.app/Contents/MacOS/Orca', alwaysExists) + ).toBe('applications') + expect( + classifyDaemonSpawnerPath('/private/Applications/Orca.app/Contents/MacOS/Orca', alwaysExists) + ).toBe('applications') + expect( + classifyDaemonSpawnerPath( + '/Users/a/Library/Caches/com.stablyai.orca.ShipIt/update.abc/Orca.app/Contents/MacOS/Orca', + alwaysExists + ) + ).toBe('updater-cache') + expect( + classifyDaemonSpawnerPath('/Users/a/Applications/Orca.app/Contents/MacOS/Orca', alwaysExists) + ).toBe('other') + expect(classifyDaemonSpawnerPath('/tmp/OrcaA.app/Contents/MacOS/Orca', alwaysExists)).toBe( + 'other' + ) + }) + + it('reports a deleted spawner as missing and an unrecorded one as unknown', () => { + expect( + classifyDaemonSpawnerPath('/Applications/Orca.app/Contents/MacOS/Orca', () => false) + ).toBe('missing') + expect(classifyDaemonSpawnerPath(null, alwaysExists)).toBe('unknown') + }) +}) + +describe('classifyDaemonPtyCwd', () => { + it('maps the TCC-protected home folders and separates the rest of home from outside it', () => { + expect(classifyDaemonPtyCwd('/Users/a/Documents/repo', '/Users/a')).toBe('documents') + expect(classifyDaemonPtyCwd('/Users/a/Desktop', '/Users/a/')).toBe('desktop') + expect(classifyDaemonPtyCwd('/Users/a/Downloads/x/y', '/Users/a')).toBe('downloads') + expect(classifyDaemonPtyCwd('/Users/a/projects/repo', '/Users/a')).toBe('other-home') + expect(classifyDaemonPtyCwd('/Users/a', '/Users/a')).toBe('other-home') + expect(classifyDaemonPtyCwd('/Volumes/ext/repo', '/Users/a')).toBe('outside-home') + // A sibling home that merely shares the prefix is not inside this home. + expect(classifyDaemonPtyCwd('/Users/ab/Documents', '/Users/a')).toBe('outside-home') + }) +}) + +// Privacy invariant: enum-only. A raw path, version, or exact count must be rejected by .strict(). +describe('daemon_adopted / daemon_pty_cwd_denied schemas', () => { + const adopted = { + app_version_match: 'different', + spawner_path_class: 'updater-cache', + tcc_attribution: 'intact', + live_session_count_bucket: '2-5' + } + const denied = { + cwd_class: 'documents', + app_version_match: 'different', + spawner_path_class: 'updater-cache' + } + + it('accepts the enum payloads', () => { + expect(eventSchemas.daemon_adopted.safeParse(adopted).success).toBe(true) + expect(eventSchemas.daemon_pty_cwd_denied.safeParse(denied).success).toBe(true) + }) + + it('rejects leaked paths, versions, counts, and unknown enum values', () => { + for (const leak of [ + { spawner_exec_path: '/Users/alice/Library/Caches/ShipIt/Orca.app' }, + { app_version: '1.4.187' }, + { live_session_count: 3 }, + { cwd: '/Users/alice/Documents' } + ]) { + expect(eventSchemas.daemon_adopted.safeParse({ ...adopted, ...leak }).success).toBe(false) + expect(eventSchemas.daemon_pty_cwd_denied.safeParse({ ...denied, ...leak }).success).toBe( + false + ) + } + expect( + eventSchemas.daemon_adopted.safeParse({ ...adopted, spawner_path_class: '/Applications' }) + .success + ).toBe(false) + expect( + eventSchemas.daemon_pty_cwd_denied.safeParse({ ...denied, cwd_class: 'Documents' }).success + ).toBe(false) + }) +}) diff --git a/src/shared/daemon-adoption-telemetry.ts b/src/shared/daemon-adoption-telemetry.ts new file mode 100644 index 00000000000..72c21647cd3 --- /dev/null +++ b/src/shared/daemon-adoption-telemetry.ts @@ -0,0 +1,67 @@ +// Enums for the `daemon_adopted` and `daemon_pty_cwd_denied` telemetry events (#17696). +// Both exist to measure how often a macOS app runs on a daemon left behind by an earlier app +// bundle, and how often such a daemon actually spawns a terminal whose cwd it cannot read. +// Enum-only: no paths, versions, or exact counts ever reach the wire. + +/** How the adopted daemon's recorded app version compares to the running app. */ +export const DAEMON_ADOPTED_APP_VERSION_MATCH = ['same', 'different', 'unknown'] as const +export type DaemonAdoptedAppVersionMatch = (typeof DAEMON_ADOPTED_APP_VERSION_MATCH)[number] + +/** + * Where the binary that forked the adopted daemon lives now. `updater-cache` is the Squirrel + * ShipIt staging area — a daemon attributed there is the reported #17696 shape. + */ +export const DAEMON_SPAWNER_PATH_CLASSES = [ + 'applications', + 'updater-cache', + 'other', + 'missing', + 'unknown' +] as const +export type DaemonSpawnerPathClass = (typeof DAEMON_SPAWNER_PATH_CLASSES)[number] + +export const DAEMON_TCC_ATTRIBUTION_VALUES = ['intact', 'severed', 'unknown'] as const + +/** Which macOS-protected folder class the denied cwd falls under. */ +export const DAEMON_PTY_CWD_CLASSES = [ + 'documents', + 'desktop', + 'downloads', + 'other-home', + 'outside-home' +] as const +export type DaemonPtyCwdClass = (typeof DAEMON_PTY_CWD_CLASSES)[number] + +export function classifyDaemonSpawnerPath( + spawnerExecPath: string | null, + exists: (path: string) => boolean +): DaemonSpawnerPathClass { + if (!spawnerExecPath) { + return 'unknown' + } + if (!exists(spawnerExecPath)) { + return 'missing' + } + if (/\/Library\/Caches\/[^/]*ShipIt\//.test(spawnerExecPath)) { + return 'updater-cache' + } + return /^(?:\/private)?\/Applications\//.test(spawnerExecPath) ? 'applications' : 'other' +} + +export function classifyDaemonPtyCwd(cwd: string, homeDir: string): DaemonPtyCwdClass { + const home = homeDir.replace(/\/+$/, '') + if (!home || !(cwd === home || cwd.startsWith(`${home}/`))) { + return 'outside-home' + } + const topLevel = cwd.slice(home.length + 1).split('/')[0] + switch (topLevel) { + case 'Documents': + return 'documents' + case 'Desktop': + return 'desktop' + case 'Downloads': + return 'downloads' + default: + return 'other-home' + } +} diff --git a/src/shared/telemetry-daemon-event-schemas.ts b/src/shared/telemetry-daemon-event-schemas.ts index a0543d4d49b..c6b2795a333 100644 --- a/src/shared/telemetry-daemon-event-schemas.ts +++ b/src/shared/telemetry-daemon-event-schemas.ts @@ -14,6 +14,12 @@ import { DAEMON_AUDIT_TRIGGER_VALUES, DAEMON_EVIDENCE_SOURCE_VALUES } from './daemon-audit-eligibility' +import { + DAEMON_ADOPTED_APP_VERSION_MATCH, + DAEMON_PTY_CWD_CLASSES, + DAEMON_SPAWNER_PATH_CLASSES, + DAEMON_TCC_ATTRIBUTION_VALUES +} from './daemon-adoption-telemetry' import { errorClassSchema, settingsChangedKeySchema } from './telemetry-property-schemas' // Why: daemon start-failure signal (fleet-wide outage like v1.4.129-rc.1); enum-only so raw stderr never reaches the wire. @@ -50,6 +56,27 @@ export const mainThreadHangDetectedSchema = z }) .strict() +// Why: #17696 — a macOS app adopting a daemon from an earlier bundle is invisible to +// `daemon_lifecycle` (nothing is replaced). Once per macOS launch that adopts; enum-only. +export const daemonAdoptedSchema = z + .object({ + app_version_match: z.enum(DAEMON_ADOPTED_APP_VERSION_MATCH), + spawner_path_class: z.enum(DAEMON_SPAWNER_PATH_CLASSES), + tcc_attribution: z.enum(DAEMON_TCC_ATTRIBUTION_VALUES), + live_session_count_bucket: z.enum(DAEMON_LIFECYCLE_SESSION_BUCKETS) + }) + .strict() + +// Why: the #17696 symptom itself — the daemon spawned a terminal into a cwd it cannot read while +// the app can. Emitted only on that proven divergence, so a missing or app-unreadable cwd never counts. +export const daemonPtyCwdDeniedSchema = z + .object({ + cwd_class: z.enum(DAEMON_PTY_CWD_CLASSES), + app_version_match: z.enum(DAEMON_ADOPTED_APP_VERSION_MATCH), + spawner_path_class: z.enum(DAEMON_SPAWNER_PATH_CLASSES) + }) + .strict() + // Why: daemon replace/retire lifecycle signal — issue #7936 was undiagnosable without asking a user for daemon.log. // Enum-only + bucketed session count so no paths, raw versions, or exact counts reach the wire. // The union keeps each reason pinned to its transition, so a death can't be reported as a replace. diff --git a/src/shared/telemetry-event-registry.ts b/src/shared/telemetry-event-registry.ts index 29479f363cb..5a91640359a 100644 --- a/src/shared/telemetry-event-registry.ts +++ b/src/shared/telemetry-event-registry.ts @@ -14,8 +14,10 @@ import { agentHookTransportBlockedSchema, agentHookUnattributedSchema, codexTrustGrantSchema, + daemonAdoptedSchema, daemonAuditEligibilitySchema, daemonLifecycleSchema, + daemonPtyCwdDeniedSchema, daemonStartFailedSchema, mainThreadHangDetectedSchema, remoteOutboundBudgetCloseSchema, @@ -122,6 +124,8 @@ export const eventSchemas = { daemon_start_failed: daemonStartFailedSchema, main_thread_hang_detected: mainThreadHangDetectedSchema, daemon_lifecycle: daemonLifecycleSchema, + daemon_adopted: daemonAdoptedSchema, + daemon_pty_cwd_denied: daemonPtyCwdDeniedSchema, daemon_audit_eligibility: daemonAuditEligibilitySchema, runtime_rpc_start_failed: runtimeRpcStartFailedSchema, remote_outbound_budget_close: remoteOutboundBudgetCloseSchema, From 7f6cf271ceedb0030c280eae4db4458bdd892c28 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 1 Sep 2026 19:53:11 -0700 Subject: [PATCH 2/8] fix(terminal): preserve panes when restored PTY owner is unverifiable (#17860) * fix(terminal): preserve unverifiable restored pane bindings * test(terminal): cover unverifiable restored pane identity * fix(terminal): settle direct SSH retry on unverifiable owner * fix(terminal): make owner warning actionable * fix(terminal): harden owner warning recovery feedback * test(terminal): consolidate fixture imports --------- Co-authored-by: Merge Sim --- .../terminal-pane/TerminalErrorToast.test.ts | 84 ++++++++++++++++- .../terminal-pane/TerminalErrorToast.tsx | 84 ++++++++++++++--- .../terminal-pane/TerminalPaneSurface.tsx | 22 ++++- ...-connection-direct-ssh-spawn-retry.test.ts | 92 ++++++++++++++++++- .../pty-connection-session-liveness.test.ts | 65 +++++++++++++ .../deferred-session-reattach-connect.ts | 20 ++++ src/renderer/src/i18n/locales/en.json | 6 +- 7 files changed, 354 insertions(+), 19 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/TerminalErrorToast.test.ts b/src/renderer/src/components/terminal-pane/TerminalErrorToast.test.ts index 069ed371ec0..220f968cd98 100644 --- a/src/renderer/src/components/terminal-pane/TerminalErrorToast.test.ts +++ b/src/renderer/src/components/terminal-pane/TerminalErrorToast.test.ts @@ -1,7 +1,7 @@ // @vitest-environment happy-dom import React from 'react' -import { cleanup, render, waitFor } from '@testing-library/react' +import { cleanup, fireEvent, render, waitFor } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' const environmentMocks = vi.hoisted(() => ({ @@ -15,6 +15,7 @@ vi.mock('@/lib/client-environment-info', () => ({ import { TerminalErrorToast, humanizeTerminalError, + isPaneOwnerUnverifiedError, isExplainedTerminalError, isSshReconnectOwnedTerminalError, shouldOfferDaemonRestart, @@ -69,7 +70,15 @@ describe('humanizeTerminalError', () => { it('replaces the pane-owner-unverified code with actionable copy', () => { const humanized = humanizeTerminalError('terminal_pane_owner_unverified') expect(humanized).not.toContain('terminal_pane_owner_unverified') - expect(humanized).toContain('Reopen this pane to retry') + expect(humanized).toContain('Click Retry to try reconnecting now') + expect(humanized).toContain('Orca left the saved session unchanged') + expect(humanized).not.toContain('was not closed or deleted') + }) + + it('identifies the owner-unverified safety state', () => { + expect(isPaneOwnerUnverifiedError('terminal_pane_owner_unverified')).toBe(true) + expect(isPaneOwnerUnverifiedError('Paste failed.')).toBe(false) + expect(isPaneOwnerUnverifiedError('Paste failed.\nterminal_pane_owner_unverified')).toBe(false) }) it('humanizes an IPC-wrapped pane-owner-unverified error', () => { @@ -78,6 +87,22 @@ describe('humanizeTerminalError', () => { expect(humanizeTerminalError(wrapped)).not.toContain('terminal_pane_owner_unverified') }) + it('humanizes an owner marker without classifying mixed errors as safe warnings', () => { + const mixed = humanizeTerminalError('Paste failed.\nterminal_pane_owner_unverified') + expect(mixed).toContain('Paste failed.') + expect(mixed).toContain("Orca couldn't verify this terminal's owner.") + expect(mixed).not.toContain('terminal_pane_owner_unverified') + expect(isPaneOwnerUnverifiedError('Paste failed.\nterminal_pane_owner_unverified')).toBe(false) + }) + + it('humanizes every owner marker in an aggregated warning', () => { + const repeated = humanizeTerminalError( + "terminal_pane_owner_unverified\nError invoking remote method 'pty:spawn': Error: terminal_pane_owner_unverified" + ) + + expect(repeated).not.toContain('terminal_pane_owner_unverified') + }) + it('leaves other errors untouched', () => { expect(humanizeTerminalError('Paste failed.')).toBe('Paste failed.') }) @@ -302,4 +327,59 @@ describe('TerminalErrorToast environment footer', () => { await waitFor(() => expect(environmentMocks.resolveFooter).not.toHaveBeenCalled()) }) + + it('renders owner-unverified as a warning without an issue link', () => { + const onRetry = vi.fn().mockResolvedValue(true) + const view = render( + React.createElement(TerminalErrorToast, { + error: 'terminal_pane_owner_unverified', + onDismiss: vi.fn(), + onRetry + }) + ) + + const toast = view.container.querySelector('[data-terminal-error-toast]') + expect(toast?.getAttribute('data-terminal-error-kind')).toBe('owner-unverified') + expect(toast?.querySelector('a')).toBeNull() + expect(toast?.textContent).toContain('Orca left the saved session unchanged') + expect(view.getByRole('button', { name: 'Retry' }).getAttribute('data-slot')).toBe('button') + fireEvent.click(view.getByRole('button', { name: 'Retry' })) + expect(onRetry).toHaveBeenCalledTimes(1) + }) + + it('keeps Retry available when the recovery attempt rejects', async () => { + const onRetry = vi.fn().mockRejectedValue(new Error('recovery unavailable')) + const view = render( + React.createElement(TerminalErrorToast, { + error: 'terminal_pane_owner_unverified', + onDismiss: vi.fn(), + onRetry + }) + ) + + fireEvent.click(view.getByRole('button', { name: 'Retry' })) + + await waitFor(() => + expect((view.getByRole('button', { name: 'Retry' }) as HTMLButtonElement).disabled).toBe( + false + ) + ) + expect(onRetry).toHaveBeenCalledTimes(1) + }) + + it('explains when Retry is temporarily unavailable', async () => { + const onRetry = vi.fn().mockResolvedValue(false) + const view = render( + React.createElement(TerminalErrorToast, { + error: 'terminal_pane_owner_unverified', + onDismiss: vi.fn(), + onRetry + }) + ) + + fireEvent.click(view.getByRole('button', { name: 'Retry' })) + await waitFor(() => + expect(view.container.textContent).toContain('Retry could not reconnect yet') + ) + }) }) diff --git a/src/renderer/src/components/terminal-pane/TerminalErrorToast.tsx b/src/renderer/src/components/terminal-pane/TerminalErrorToast.tsx index ee876ae719e..2ba98a180d6 100644 --- a/src/renderer/src/components/terminal-pane/TerminalErrorToast.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalErrorToast.tsx @@ -1,6 +1,7 @@ import { useEffect, useState } from 'react' import { translate } from '@/i18n/i18n' import { resolveClientEnvironmentFooter } from '@/lib/client-environment-info' +import { Button } from '@/components/ui/button' import { hasClientEnvironmentFooter } from '../../../../shared/client-environment-info' const SSH_PREFIX = 'SSH connection is not active' @@ -78,6 +79,11 @@ export function isExplainedTerminalError(error: string): boolean { ) } +export function isPaneOwnerUnverifiedError(error: string): boolean { + const lines = error.split('\n').filter((line) => line.length > 0) + return lines.length > 0 && lines.every((line) => line.includes(PANE_OWNER_UNVERIFIED_MARKER)) +} + function humanizeUnreattachableSession(error: string): string { const explanation = translate( 'auto.components.terminal.pane.TerminalErrorToast.sessionUnavailable', @@ -94,13 +100,16 @@ function humanizeUnreattachableSession(error: string): string { export function humanizeTerminalError(error: string): string { let humanized = error if (humanized.includes(PANE_OWNER_UNVERIFIED_MARKER)) { - humanized = humanized.replace( - PANE_OWNER_UNVERIFIED_MARKER, - translate( - 'auto.components.terminal.pane.TerminalErrorToast.7ee11bc0db', - "Orca couldn't confirm whether this terminal's previous session is still running, so it left the session untouched. Reopen this pane to retry." - ) - ) + const explanation = isPaneOwnerUnverifiedError(humanized) + ? translate( + 'auto.components.terminal.pane.TerminalErrorToast.42b283ecfc', + "Orca couldn't safely reconnect this terminal because the host couldn't verify its saved session. Orca left the saved session unchanged. Click Retry to try reconnecting now. If it still cannot reconnect, open a new terminal." + ) + : translate( + 'auto.components.terminal.pane.TerminalErrorToast.ownerUnknown', + "Orca couldn't verify this terminal's owner." + ) + humanized = humanized.replaceAll(PANE_OWNER_UNVERIFIED_MARKER, () => explanation) } humanized = humanizeUnreattachableSession(humanized) if (!isExplainedTerminalError(humanized)) { @@ -127,17 +136,23 @@ export function humanizeTerminalError(error: string): string { export function TerminalErrorToast({ error, onDismiss, - onRestartDaemon + onRestartDaemon, + onRetry }: { error: string onDismiss: () => void onRestartDaemon?: () => void + onRetry?: () => Promise }): React.JSX.Element { const ssh = isSshError(error) + const paneOwnerUnverified = isPaneOwnerUnverifiedError(error) const showDaemonRestart = !ssh && onRestartDaemon && shouldOfferDaemonRestart(error) // Restart cannot recover a session after its owning daemon exits. - const showIssueLink = !ssh && !showDaemonRestart && !isExplainedTerminalError(error) + const showIssueLink = + !ssh && !paneOwnerUnverified && !showDaemonRestart && !isExplainedTerminalError(error) const displayError = humanizeTerminalError(error) + const [retrying, setRetrying] = useState(false) + const [retryFailed, setRetryFailed] = useState(false) const [environmentFooter, setEnvironmentFooter] = useState<{ error: string footer: string @@ -160,10 +175,26 @@ export function TerminalErrorToast({ }, [displayError, ssh]) const footer = environmentFooter?.error === displayError ? environmentFooter.footer : '' + const handleRetry = async (): Promise => { + if (!onRetry || retrying) { + return + } + setRetrying(true) + setRetryFailed(false) + try { + setRetryFailed(!(await onRetry())) + } catch { + // Keep the safety warning available when a best-effort remount cannot start. + setRetryFailed(true) + } finally { + setRetrying(false) + } + } return (
) : null} {!ssh && footer ? `\n\n${footer}` : null} + {paneOwnerUnverified && retryFailed + ? `\n${translate( + 'auto.components.terminal.pane.TerminalErrorToast.retryUnavailable', + 'Retry could not reconnect yet. Try again shortly.' + )}` + : null} {showDaemonRestart ? ( + ) : null}