From 730e4c9ab0e72183ef6d3f3d2eaf9fbca8aa98b3 Mon Sep 17 00:00:00 2001 From: Merge Sim Date: Fri, 4 Sep 2026 13:24:40 -0700 Subject: [PATCH] fix(agent-session): refuse a pre-commit structured create with an envelope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The create route refused by throwing, which reaches a client as a generic transport error indistinguishable from a lost answer — so desktop parked the launch as visibility-unknown with no chat and no terminal. Convert the whole pre-commit span, everything before `attach`, into a refusal envelope carrying a code, and name the definitive-refusal allowlist the fallback decision needs. --- ...ed-agent-session-precommit-refusal.test.ts | 235 ++++++++++++++++++ ...uctured-agent-session-precommit-refusal.ts | 71 ++++++ .../rpc/methods/structured-agent-session.ts | 103 ++++---- .../agent-session-definitive-refusal.test.ts | 48 ++++ .../agent-session-definitive-refusal.ts | 35 +++ 5 files changed, 447 insertions(+), 45 deletions(-) create mode 100644 src/main/runtime/rpc/methods/structured-agent-session-precommit-refusal.test.ts create mode 100644 src/main/runtime/rpc/methods/structured-agent-session-precommit-refusal.ts create mode 100644 src/shared/agent-session-definitive-refusal.test.ts create mode 100644 src/shared/agent-session-definitive-refusal.ts diff --git a/src/main/runtime/rpc/methods/structured-agent-session-precommit-refusal.test.ts b/src/main/runtime/rpc/methods/structured-agent-session-precommit-refusal.test.ts new file mode 100644 index 00000000000..33c90250218 --- /dev/null +++ b/src/main/runtime/rpc/methods/structured-agent-session-precommit-refusal.test.ts @@ -0,0 +1,235 @@ +// The create route's pre-commit boundary: a failure before `attach` must reach the client as a +// refusal it can classify, and a failure at or after `attach` must not. + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { StructuredAgentSessionHost } from '../../../native-chat/agent-session-wire/structured-agent-session-host' +import { setStructuredAgentSessionHost } from '../../../native-chat/agent-session-wire/structured-agent-session-registry' +import { computeAgentSessionPayloadFingerprint } from '../../../../shared/agent-session-mutation-envelope' +import { isDefinitiveAgentSessionCreateRefusal } from '../../../../shared/agent-session-definitive-refusal' +import { STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY } from '../../../../shared/protocol-version' +import type { OrcaRuntimeService } from '../../orca-runtime' +import type { RpcResponse } from '../core' +import { RpcDispatcher } from '../dispatcher' +import { STRUCTURED_AGENT_SESSION_METHODS } from './structured-agent-session' + +const SESSION = 'session-alpha' +const OPERATION = '1800000000000-00000000000000000000000000000001' +const WORKTREE = 'id:workspace-1' + +const STRUCTURED_CLIENT = { + clientKind: 'runtime' as const, + clientCapabilities: [STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY] +} + +function createParams(overrides: Record = {}) { + return { + envelope: { + sessionId: SESSION, + clientOperationId: OPERATION, + expectedRuntimeFence: null, + payloadFingerprint: computeAgentSessionPayloadFingerprint({ + method: 'agentSession.create', + sessionId: SESSION, + fields: { worktree: WORKTREE, agent: 'codex' } + }), + ...(overrides.envelope as Record | undefined) + }, + worktree: WORKTREE, + agent: 'codex' + } +} + +let attach: ReturnType + +function hostStub(): StructuredAgentSessionHost { + attach = vi.fn(async () => ({ + ok: true, + replayed: false, + fence: 1, + cursor: { epoch: 'epoch-a', sequence: 0 }, + value: { sessionId: SESSION, fence: 1, page: {}, unconfirmedClientMessageIds: [] } + })) + return { attach } as unknown as StructuredAgentSessionHost +} + +const resolvedIntent = { + location: { + executionHostId: 'local', + wslDistro: null, + workspaceId: 'workspace-1', + workspaceKind: 'git-worktree' + }, + provider: 'codex', + agent: 'codex', + accountHome: { variable: 'CODEX_HOME', path: '/host/.codex' }, + runtimeKind: 'native' +} + +async function create( + runtimeOverrides: Record = {}, + params: unknown = createParams() +): Promise { + const runtime = { + getRuntimeId: () => 'runtime-1', + registerSubscriptionCleanup: vi.fn(), + cleanupSubscription: vi.fn(), + cleanupSubscriptionsByPrefix: vi.fn(), + ensureStructuredAgentSessionHost: vi.fn(async () => undefined), + resolveStructuredAgentSessionCreateIntent: vi.fn(async (input: { envelope: unknown }) => ({ + envelope: input.envelope, + ...resolvedIntent + })), + publishStructuredAgentSessionTab: vi.fn(async () => undefined), + ...runtimeOverrides + } + const replies: RpcResponse[] = [] + await new RpcDispatcher({ + runtime: runtime as unknown as OrcaRuntimeService, + methods: STRUCTURED_AGENT_SESSION_METHODS + }).dispatchStreaming( + { id: 'request-1', authToken: 'token', method: 'agentSession.create', params }, + (raw) => replies.push(JSON.parse(raw) as RpcResponse), + STRUCTURED_CLIENT + ) + const first = replies[0] + if (!first) { + throw new Error('no reply for agentSession.create') + } + return first +} + +/** The refusal a client can act on, or null when the reply was not one. */ +function refusalOf(response: RpcResponse): { code: string; message: string } | null { + if (!response.ok) { + return null + } + const result = response.result as { ok: boolean; refusal?: { code: string; message: string } } + return result.ok ? null : (result.refusal ?? null) +} + +beforeEach(() => { + setStructuredAgentSessionHost(hostStub()) + vi.spyOn(console, 'warn').mockImplementation(() => undefined) +}) + +afterEach(() => { + setStructuredAgentSessionHost(null) + vi.restoreAllMocks() +}) + +describe('a create refused before it commits', () => { + it('answers a code-carrying refusal as a definitive envelope', async () => { + const response = await create({ + resolveStructuredAgentSessionCreateIntent: vi.fn(async () => { + throw new Error('structured_agent_session_unsupported') + }) + }) + + const refusal = refusalOf(response) + expect(refusal?.code).toBe('structured_agent_session_unsupported') + expect(isDefinitiveAgentSessionCreateRefusal(refusal?.code)).toBe(true) + expect(attach).not.toHaveBeenCalled() + }) + + it('answers a code-less failure as a definitive envelope too, keeping the cause in the message', async () => { + // The class no per-site conversion catches: an unresolvable worktree throws prose, not a code. + const response = await create({ + resolveStructuredAgentSessionCreateIntent: vi.fn(async () => { + throw new Error('No worktree matches id:workspace-1') + }) + }) + + const refusal = refusalOf(response) + expect(isDefinitiveAgentSessionCreateRefusal(refusal?.code)).toBe(true) + expect(refusal?.message).toContain('No worktree matches id:workspace-1') + expect(attach).not.toHaveBeenCalled() + }) + + it('answers a host that will not install as a definitive envelope', async () => { + setStructuredAgentSessionHost(null) + + const response = await create({ + ensureStructuredAgentSessionHost: vi.fn(async () => { + throw new Error('EACCES: could not open the session store') + }) + }) + + const refusal = refusalOf(response) + expect(isDefinitiveAgentSessionCreateRefusal(refusal?.code)).toBe(true) + expect(refusal?.message).toContain('could not open the session store') + }) + + it('answers a missing host as a definitive envelope rather than a thrown code', async () => { + setStructuredAgentSessionHost(null) + + const response = await create() + + const refusal = refusalOf(response) + expect(refusal?.code).toBe('structured_agent_session_unsupported') + expect(isDefinitiveAgentSessionCreateRefusal(refusal?.code)).toBe(true) + }) + + it('still refuses a fingerprint conflict with its own code, not a pre-commit one', async () => { + const response = await create( + {}, + createParams({ envelope: { payloadFingerprint: 'a'.repeat(64) } }) + ) + + expect(refusalOf(response)?.code).toBe('agent_session_operation_conflict') + expect(attach).not.toHaveBeenCalled() + }) +}) + +describe('the boundary the envelope stops at', () => { + it('leaves a failure at attach unknown, because it may have committed', async () => { + attach.mockRejectedValueOnce(new Error('attach exploded')) + + const response = await create() + + expect(response).toMatchObject({ ok: false, error: { code: 'runtime_error' } }) + expect(refusalOf(response)).toBeNull() + }) + + it('leaves a committed create whose tab could not be published unknown', async () => { + const response = await create({ + publishStructuredAgentSessionTab: vi.fn(async () => { + throw new Error('publish failed') + }) + }) + + const refusal = refusalOf(response) + expect(refusal?.code).toBe('agent_session_operation_unknown') + expect(isDefinitiveAgentSessionCreateRefusal(refusal?.code)).toBe(false) + }) + + it('keeps hiding the surface from a client that never advertised it', async () => { + const replies: RpcResponse[] = [] + await new RpcDispatcher({ + runtime: { getRuntimeId: () => 'runtime-1' } as unknown as OrcaRuntimeService, + methods: STRUCTURED_AGENT_SESSION_METHODS + }).dispatchStreaming( + { + id: 'request-1', + authToken: 'token', + method: 'agentSession.create', + params: createParams() + }, + (raw) => replies.push(JSON.parse(raw) as RpcResponse), + { clientKind: 'runtime', clientCapabilities: [] } + ) + + expect(replies[0]).toMatchObject({ + ok: false, + error: { message: expect.stringContaining('structured_agent_session_unsupported') } + }) + }) + + it('keeps a client-declared fence a programming error, not a refusal', async () => { + const response = await create({}, createParams({ envelope: { expectedRuntimeFence: 1 } })) + + expect(response).toMatchObject({ + ok: false, + error: { code: 'agent_session_operation_invalid' } + }) + }) +}) diff --git a/src/main/runtime/rpc/methods/structured-agent-session-precommit-refusal.ts b/src/main/runtime/rpc/methods/structured-agent-session-precommit-refusal.ts new file mode 100644 index 00000000000..d55376ffc74 --- /dev/null +++ b/src/main/runtime/rpc/methods/structured-agent-session-precommit-refusal.ts @@ -0,0 +1,71 @@ +// Nothing before `attach` commits a session, so every failure in that span definitively created +// nothing. Thrown, it reaches a remote client as a generic transport error, indistinguishable from +// an answer that was lost on the way back — and a client that cannot tell those apart either +// strands the user with no chat and no terminal, or spawns a sibling beside a session that may +// already exist. So the whole span answers with a refusal envelope carrying a code, whatever it +// failed on. +// +// Converting the span rather than each throw site is deliberate: alongside the throws that carry a +// code there is a code-less class — an unresolvable worktree, a store that will not open, a host +// that will not install — that no per-site list catches, and it is exactly the class that reaches +// the user as nothing at all. + +import { + AGENT_SESSION_WIRE_REFUSAL_CODES, + type AgentSessionWireRefusal, + type AgentSessionWireRefusalCode +} from '../../../../shared/agent-session-wire' + +export type StructuredCreateRefused = { refusal: AgentSessionWireRefusal } + +/** A pre-commit failure with no code of its own still proves the host could not serve a structured + * session for this request and did not create one, which is what `unsupported` says on the wire. + * A new code would say it more precisely, but only to clients new enough to know it. */ +const UNCODED_PRECOMMIT_REFUSAL_CODE: AgentSessionWireRefusalCode = + 'structured_agent_session_unsupported' + +function wireRefusalCode(error: unknown): AgentSessionWireRefusalCode | null { + const candidates = [ + error instanceof Error && 'code' in error ? (error as { code: unknown }).code : undefined, + error instanceof Error ? error.message : String(error) + ] + for (const candidate of candidates) { + if ( + typeof candidate === 'string' && + (AGENT_SESSION_WIRE_REFUSAL_CODES as readonly string[]).includes(candidate) + ) { + return candidate as AgentSessionWireRefusalCode + } + } + return null +} + +function precommitRefusal(error: unknown): AgentSessionWireRefusal { + const code = wireRefusalCode(error) + if (code) { + return { code, message: 'Orca cannot open a structured Codex chat for this workspace.' } + } + const message = error instanceof Error ? error.message : String(error) + // A code-less failure here is often a defect, not a policy answer; the refusal keeps the user + // moving, the log keeps the cause findable. + console.warn('[agent-session] create refused before it committed anything', error) + return { + code: UNCODED_PRECOMMIT_REFUSAL_CODE, + message: `Orca could not prepare a Codex chat for this workspace: ${message}` + } +} + +/** + * Runs the pre-commit half of a create. Anything it throws becomes a refusal; a refusal it returns + * itself passes through. Must not wrap `attach` or anything after it — past that point a failure no + * longer proves the session does not exist. + */ +export async function resolveUncommittedStructuredCreate( + prepare: () => Promise +): Promise { + try { + return await prepare() + } catch (error) { + return { refusal: precommitRefusal(error) } + } +} diff --git a/src/main/runtime/rpc/methods/structured-agent-session.ts b/src/main/runtime/rpc/methods/structured-agent-session.ts index ffd23499a3e..772d0ec7871 100644 --- a/src/main/runtime/rpc/methods/structured-agent-session.ts +++ b/src/main/runtime/rpc/methods/structured-agent-session.ts @@ -19,6 +19,7 @@ import { supportsStructuredSessions } from './structured-agent-session-gate' import { STRUCTURED_AGENT_SESSION_HOLD_METHODS } from './structured-agent-session-hold' +import { resolveUncommittedStructuredCreate } from './structured-agent-session-precommit-refusal' import { AttachParams, CancelParams, @@ -62,57 +63,69 @@ export const STRUCTURED_AGENT_SESSION_METHODS: RpcAnyMethod[] = [ if (params.envelope.expectedRuntimeFence !== null) { throw new Error('agent_session_operation_invalid') } - if ('worktree' in params) { - const intentFingerprint = computeAgentSessionPayloadFingerprint({ - method: 'agentSession.create', - sessionId: params.envelope.sessionId, - fields: { worktree: params.worktree, agent: params.agent } - }) - const conflict = agentSessionFingerprintConflict(params.envelope, intentFingerprint) - if (conflict) { - return { ok: false, refusal: conflict } - } - const resolved = await ctx.runtime.resolveStructuredAgentSessionCreateIntent(params) - const hostFingerprint = computeAgentSessionPayloadFingerprint({ - method: 'agentSession.attach', - sessionId: params.envelope.sessionId, - fields: { - location: resolved.location, - provider: resolved.provider, - agent: resolved.agent, - accountHome: resolved.accountHome, - runtimeKind: resolved.runtimeKind, - expectedRuntimeFence: null + // Everything up to `attach` is pre-commit, and answers with a refusal rather than a throw so + // a client can tell "nothing was created" from "the outcome is unknown". + const prepared = await resolveUncommittedStructuredCreate(async () => { + if ('worktree' in params) { + const intentFingerprint = computeAgentSessionPayloadFingerprint({ + method: 'agentSession.create', + sessionId: params.envelope.sessionId, + fields: { worktree: params.worktree, agent: params.agent } + }) + const conflict = agentSessionFingerprintConflict(params.envelope, intentFingerprint) + if (conflict) { + return { refusal: conflict } } - }) + const resolved = await ctx.runtime.resolveStructuredAgentSessionCreateIntent(params) + const hostFingerprint = computeAgentSessionPayloadFingerprint({ + method: 'agentSession.attach', + sessionId: params.envelope.sessionId, + fields: { + location: resolved.location, + provider: resolved.provider, + agent: resolved.agent, + accountHome: resolved.accountHome, + runtimeKind: resolved.runtimeKind, + expectedRuntimeFence: null + } + }) + await ensureHostInstalled(ctx) + return { + host: requireHost(ctx), + attachParams: { + ...resolved, + envelope: { ...params.envelope, payloadFingerprint: hostFingerprint } + }, + tab: resolved.agent === 'codex' ? { workspaceId: resolved.location.workspaceId } : null + } + } await ensureHostInstalled(ctx) - const result = await requireHost(ctx).attach(callerFor(ctx), { - ...resolved, - envelope: { ...params.envelope, payloadFingerprint: hostFingerprint } - }) - if (result.ok && resolved.agent === 'codex') { - try { - await ctx.runtime.publishStructuredAgentSessionTab({ - workspaceId: resolved.location.workspaceId, - sessionId: result.value.sessionId, - agent: 'codex', - activate: true - }) - } catch (error) { - console.warn('[agent-session] create committed before tab publication failed', error) - return { - ok: false, - refusal: { - code: 'agent_session_operation_unknown', - message: 'The Codex chat may have been created, but its tab could not be confirmed.' - } + return { host: requireHost(ctx), attachParams: params, tab: null } + }) + if ('refusal' in prepared) { + return { ok: false, refusal: prepared.refusal } + } + const result = await prepared.host.attach(callerFor(ctx), prepared.attachParams) + if (result.ok && prepared.tab) { + try { + await ctx.runtime.publishStructuredAgentSessionTab({ + workspaceId: prepared.tab.workspaceId, + sessionId: result.value.sessionId, + agent: 'codex', + activate: true + }) + } catch (error) { + console.warn('[agent-session] create committed before tab publication failed', error) + return { + ok: false, + refusal: { + code: 'agent_session_operation_unknown', + message: 'The Codex chat may have been created, but its tab could not be confirmed.' } } } - return result } - await ensureHostInstalled(ctx) - return requireHost(ctx).attach(callerFor(ctx), params) + return result } }), defineMethod({ diff --git a/src/shared/agent-session-definitive-refusal.test.ts b/src/shared/agent-session-definitive-refusal.test.ts new file mode 100644 index 00000000000..74dd3529cf6 --- /dev/null +++ b/src/shared/agent-session-definitive-refusal.test.ts @@ -0,0 +1,48 @@ +import { describe, expect, it } from 'vitest' +import { AGENT_SESSION_WIRE_REFUSAL_CODES } from './agent-session-wire' +import { agentSessionRefusalOperationState } from './agent-session-refusal-retry' +import { isDefinitiveAgentSessionCreateRefusal } from './agent-session-definitive-refusal' + +describe('definitive agent-session create refusals', () => { + it('treats an unsupported structured session as definitive', () => { + expect(isDefinitiveAgentSessionCreateRefusal('structured_agent_session_unsupported')).toBe(true) + }) + + it('never treats an unproven outcome as definitive', () => { + expect(isDefinitiveAgentSessionCreateRefusal('agent_session_operation_unknown')).toBe(false) + expect(isDefinitiveAgentSessionCreateRefusal('agent_session_ownership_unknown')).toBe(false) + }) + + it('leaves transport failures, timeouts and a missing code unknown', () => { + expect(isDefinitiveAgentSessionCreateRefusal('runtime_error')).toBe(false) + expect(isDefinitiveAgentSessionCreateRefusal('remote_runtime_unavailable')).toBe(false) + expect(isDefinitiveAgentSessionCreateRefusal('timeout')).toBe(false) + expect(isDefinitiveAgentSessionCreateRefusal('runtime_timeout')).toBe(false) + expect(isDefinitiveAgentSessionCreateRefusal(undefined)).toBe(false) + expect(isDefinitiveAgentSessionCreateRefusal(null)).toBe(false) + expect(isDefinitiveAgentSessionCreateRefusal('')).toBe(false) + }) + + it('counts a method an old host never registered as definitive', () => { + expect(isDefinitiveAgentSessionCreateRefusal('method_not_found')).toBe(true) + }) + + it('is an allowlist: every other wire refusal code is unknown', () => { + const definitive = AGENT_SESSION_WIRE_REFUSAL_CODES.filter((code) => + isDefinitiveAgentSessionCreateRefusal(code) + ) + expect(definitive).toEqual(['structured_agent_session_unsupported']) + }) + + it('does not answer the durable-settlement question, which disagrees on the one code that matters', () => { + // Guards the reuse this allowlist exists to avoid: settlement state calls the definitive + // refusal pending-admission, which would rule out the fallback it is meant to allow. + expect( + agentSessionRefusalOperationState( + 'agentSession.create', + 'structured_agent_session_unsupported' + ) + ).toBe('pending-admission') + expect(isDefinitiveAgentSessionCreateRefusal('structured_agent_session_unsupported')).toBe(true) + }) +}) diff --git a/src/shared/agent-session-definitive-refusal.ts b/src/shared/agent-session-definitive-refusal.ts new file mode 100644 index 00000000000..80721592ed9 --- /dev/null +++ b/src/shared/agent-session-definitive-refusal.ts @@ -0,0 +1,35 @@ +/** + * "May a caller create something else instead?" — the fallback question. + * + * Deliberately NOT `agentSessionRefusalOperationState`: that answers "did this operation durably + * settle?", and for that question `structured_agent_session_unsupported` is correctly + * pending-admission. Reused here it would rule out a fallback on the one refusal that most needs + * one. The two questions only look alike. + * + * An allowlist, never a negation: falling back on an outcome the host could not describe is how a + * user ends up with two sessions for one intent. Everything absent — transport failures, timeouts, + * `agent_session_operation_unknown`, `agent_session_ownership_unknown` — is unknown, and unknown + * never falls back. + */ + +import type { AgentSessionWireRefusalCode } from './agent-session-wire' + +/** Proves the host neither created a session nor will on a retry. */ +const DEFINITIVE_REFUSAL_CODES: ReadonlySet = new Set([ + 'structured_agent_session_unsupported' +]) + +/** A dispatcher that never registered the method ran no handler at all, which is as definitive as + * a refusal — and the only transport-level answer that is. */ +const DEFINITIVE_RPC_ERROR_CODES: ReadonlySet = new Set(['method_not_found']) + +/** + * True only when the code proves nothing was created. Accepts a wire refusal code or an RPC error + * code; the two namespaces are disjoint. + */ +export function isDefinitiveAgentSessionCreateRefusal(code: string | null | undefined): boolean { + if (typeof code !== 'string') { + return false + } + return DEFINITIVE_REFUSAL_CODES.has(code) || DEFINITIVE_RPC_ERROR_CODES.has(code) +}