diff --git a/src/cli/runtime-client.test.ts b/src/cli/runtime-client.test.ts index ecb5030a73b..73aad83b6a7 100644 --- a/src/cli/runtime-client.test.ts +++ b/src/cli/runtime-client.test.ts @@ -332,7 +332,10 @@ describe.skipIf(process.platform === 'win32')('RuntimeClient', () => { }) }) - it('openOrca keeps the last unreachable reason when later polls have no metadata', async () => { + // STA-3969: the diagnosis is still worth surfacing once metadata disappears, but the newest + // poll no longer observes it -- so it is reported as the last failure seen, not as the live + // one. Presenting it under `unreachableReason` would be the same stale negative in machine form. + it('reports the last unreachable reason as history when later polls have no metadata', async () => { const userDataPath = mkdtempSync(join(tmpdir(), 'orca-runtime-client-')) writeMetadata(userDataPath, join(userDataPath, 'never-listened.sock'), 'token', process.pid) vi.mocked(launchOrcaApp).mockImplementationOnce(() => { @@ -341,11 +344,83 @@ describe.skipIf(process.platform === 'win32')('RuntimeClient', () => { const client = new RuntimeClient(userDataPath, 100) - await expect(client.openOrca(100)).rejects.toMatchObject({ - code: 'runtime_open_timeout', - message: expect.stringContaining('never-listened.sock'), - data: { unreachableReason: { code: 'endpoint_missing' } } + const failure = await client.openOrca(100).then( + () => null, + (error: unknown) => error as { code: string; message: string; data?: unknown } + ) + expect(failure?.code).toBe('runtime_open_timeout') + expect(failure?.message).toContain('never-listened.sock') + expect(failure?.message).toContain('The last failure it reported was') + expect(failure?.message).not.toContain('the Orca app process is running') + const data = failure?.data as { + unreachableReason?: unknown + lastObservedUnreachableReason?: { code?: string } + } + expect(data.unreachableReason).toBeUndefined() + expect(data.lastObservedUnreachableReason?.code).toBe('endpoint_missing') + }) + + // STA-3969: same stale negative one state further along -- the runtime was unreachable, then + // its PROCESS exited. The newest poll reports a dead process and no reason, so quoting the old + // endpoint failure claims a running process the latest status denies. + it('openOrca stops claiming the process is running once it exits mid-wait', async () => { + const userDataPath = mkdtempSync(join(tmpdir(), 'orca-runtime-client-')) + const endpoint = join(userDataPath, 'never-listened.sock') + writeMetadata(userDataPath, endpoint, 'token', process.pid) + vi.mocked(launchOrcaApp).mockImplementationOnce(() => { + writeMetadata(userDataPath, endpoint, 'token', findUnusedPid()) }) + + const client = new RuntimeClient(userDataPath, 100) + + const failure = await client.openOrca(100).then( + () => null, + (error: unknown) => error as { code: string; message: string; data?: unknown } + ) + expect(failure?.code).toBe('runtime_open_timeout') + expect(failure?.message).not.toContain('the Orca app process is running') + expect(failure?.message).toContain('no longer running') + expect( + (failure?.data as { unreachableReason?: unknown } | undefined)?.unreachableReason + ).toBeUndefined() + }) + + // STA-3969: `request_rejected` means the runtime ANSWERED and refused. Calling that + // "unreachable" contradicts the very reason being quoted alongside it. + it('openOrca says the runtime refused the request instead of calling it unreachable', async () => { + const userDataPath = mkdtempSync(join(tmpdir(), 'orca-runtime-client-')) + const endpoint = join(userDataPath, 'runtime.sock') + const server = createServer((socket) => { + sockets.add(socket) + socket.once('close', () => sockets.delete(socket)) + socket.on('data', (data) => { + const request = JSON.parse(String(data).trim()) as { id: string } + socket.write( + `${JSON.stringify({ + id: request.id, + ok: false, + error: { code: 'status_refused', message: 'status.get is disabled' } + })}\n` + ) + }) + }) + servers.add(server) + await new Promise((resolve) => server.listen(endpoint, resolve)) + writeMetadata(userDataPath, endpoint, 'token', process.pid) + + const client = new RuntimeClient(userDataPath, 100) + + const failure = await client.openOrca(100).then( + () => null, + (error: unknown) => error as { code: string; message: string; data?: unknown } + ) + expect(failure?.code).toBe('runtime_open_timeout') + expect( + (failure?.data as { unreachableReason?: { code?: string } } | undefined)?.unreachableReason + ?.code + ).toBe('request_rejected') + expect(failure?.message).toContain('refused the status request') + expect(failure?.message).not.toContain('its runtime is unreachable') }) // STA-3969: the poll carried the earlier reason forward with `?? lastReason`, so a runtime diff --git a/src/cli/runtime/client.ts b/src/cli/runtime/client.ts index a3043139bb3..28bd6e08d82 100644 --- a/src/cli/runtime/client.ts +++ b/src/cli/runtime/client.ts @@ -9,6 +9,7 @@ import { parsePairingCode, type PairingOffer } from '../../shared/pairing' import { launchOrcaApp } from './launch' import { getDefaultUserDataPath, readMetadata } from './metadata' import { getCliStatus, projectRemoteAppStatus } from './status' +import { describeOpenTimeout } from './runtime-open-timeout-reason' import { sendRequest } from './transport' import { RuntimeClientError, RuntimeRpcFailureError, type RuntimeRpcSuccess } from './types' import { attachMutationRecovery } from './client-error-recovery' @@ -263,8 +264,13 @@ export class RuntimeClient { } const startedAt = Date.now() - let lastReason = initial.result.runtime.unreachableReason - let runtimeAnswered = initial.result.runtime.reachable + // Why (STA-3969): the timeout must describe the NEWEST observation. A reason accumulated + // across polls outlives the poll that saw it and then gets reported as the current + // diagnosis -- first when the runtime recovered, and again when its process exited. So the + // loop keeps the latest status and the reason is read off that; the earlier one survives + // only as history, never as the live verdict. + let latest = initial.result + let lastObservedReason = initial.result.runtime.unreachableReason while (Date.now() - startedAt < timeoutMs) { const status = await this.getCliStatus() if (status.result.app.desktopWindowStatus === 'blocked') { @@ -273,31 +279,24 @@ export class RuntimeClient { if (status.result.app.desktopWindowStatus === 'available') { return status } - // Why (STA-3969): a runtime that answered is reachable NOW, so carrying the earlier - // reason forward would report a stale negative -- naming an endpoint it no longer - // uses -- as the current diagnosis. - runtimeAnswered = status.result.runtime.reachable - lastReason = runtimeAnswered + latest = status.result + // A poll that ANSWERED resolves the earlier failure outright, so it is not even history + // any more; only an unresolved failure we can no longer diagnose is worth carrying. + lastObservedReason = latest.runtime.reachable ? undefined - : (status.result.runtime.unreachableReason ?? lastReason) + : (latest.runtime.unreachableReason ?? lastObservedReason) await delay(250) } - // Why: STA-3969 — this loop polls getCliStatus, so when the runtime is - // unreachable it burns the whole timeout and then reported only that it timed - // out. Carry the reachability failure the poll already diagnosed. - // Why: the two timeouts are different failures. One never got an answer from the - // runtime; the other got answers the whole time and no window with them -- telling that - // user the runtime "may" be running headlessly understates what the poll already proved. - const timeoutDetail = lastReason - ? `: the Orca app process is running but its runtime is unreachable. ${lastReason.message}` - : runtimeAnswered - ? '. The Orca runtime is responding and still running headlessly; it did not open a window in time.' - : '. The runtime may still be running headlessly.' + const currentReason = latest.runtime.unreachableReason throw new RuntimeClientError( 'runtime_open_timeout', - `Timed out waiting for an Orca desktop window${timeoutDetail}`, - lastReason ? { unreachableReason: lastReason } : undefined + `Timed out waiting for an Orca desktop window${describeOpenTimeout(latest, lastObservedReason)}`, + currentReason + ? { unreachableReason: currentReason } + : lastObservedReason + ? { lastObservedUnreachableReason: lastObservedReason } + : undefined ) } } diff --git a/src/cli/runtime/runtime-open-timeout-reason.ts b/src/cli/runtime/runtime-open-timeout-reason.ts new file mode 100644 index 00000000000..29ceee4b8bc --- /dev/null +++ b/src/cli/runtime/runtime-open-timeout-reason.ts @@ -0,0 +1,32 @@ +import type { CliRuntimeUnreachableReason, CliStatusResult } from '../../shared/runtime-types' + +// Why (STA-3969): the rule this enforces is that every clause names the observation it came +// from. `unreachableReason` is produced in exactly one place -- the branch that checked the pid +// and found the process alive -- so its presence is what licenses saying the process is running, +// and its absence forbids it. A reason from an earlier poll is reported as history, tense marked, +// never folded into the live verdict. +export function describeOpenTimeout( + latest: CliStatusResult, + lastObservedReason: CliRuntimeUnreachableReason | undefined +): string { + const current = latest.runtime.unreachableReason + if (current) { + // A rejected request is an answer, so it cannot also be described as unreachable. + return current.code === 'request_rejected' + ? `: the Orca runtime answered but refused the status request. ${current.message}` + : `: the Orca app process is running but its runtime is unreachable. ${current.message}` + } + if (latest.runtime.reachable) { + return '. The Orca runtime is responding and still running headlessly; it did not open a window in time.' + } + if (latest.app.running) { + return '. The runtime may still be running headlessly.' + } + const state = + latest.runtime.state === 'stale_bootstrap' + ? '. The Orca app process is no longer running.' + : '. No Orca runtime is running.' + return lastObservedReason + ? `${state} The last failure it reported was: ${lastObservedReason.message}` + : state +}