From e7206f62a827f4fe0a2badf3eecd2167b8ac4285 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 15 Sep 2026 14:58:49 -0700 Subject: [PATCH] fix(mobile): retire a structured operation id the host has refused (#20868) `agentSession.cancel` kept its client operation id whenever the outcome came back unknown. One of those unknowns is not transport doubt: when the host answers `agent_session_operation_unknown` it has decided about that id and will not run it again, because cancel's mutation plan recovers no unknown ledger row. Every later Stop on that turn re-sent the same refused id, so Stop stayed unusable until the row expired. The RPC layer collapsed both cases into a bare `unknown`, discarding the difference between "the effect is in doubt" and "the host answered about this id". It now reports the second case, and cancel spends the id there while still replaying under genuine transport doubt. `agentSession.conversationCommand` deliberately keeps its id: its plan sets `recoverUnknownFromDurableState`, so a reused id can still replay or rerun. --- .../mobile-structured-agent-session-cancel.ts | 5 +- .../mobile-structured-agent-session-rpc.ts | 8 +- ...structured-operation-id-retirement.test.ts | 133 ++++++++++++++++++ 3 files changed, 143 insertions(+), 3 deletions(-) create mode 100644 mobile/src/session/mobile-structured-operation-id-retirement.test.ts diff --git a/mobile/src/session/mobile-structured-agent-session-cancel.ts b/mobile/src/session/mobile-structured-agent-session-cancel.ts index 9c67e7480d0..f144accc2e7 100644 --- a/mobile/src/session/mobile-structured-agent-session-cancel.ts +++ b/mobile/src/session/mobile-structured-agent-session-cancel.ts @@ -61,7 +61,10 @@ export async function requestMobileStructuredAgentSessionCancel(args: { fields, clientOperationId }) - if (result.status !== 'unknown') { + // Cancel's plan recovers no unknown ledger row, so an id the host answered that + // way earns the same refusal until it expires; keeping it leaves Stop unusable. + // Transport doubt proves nothing about delivery, so it stays a replay. + if (result.status !== 'unknown' || result.hostReportedOperationUnknown === true) { operationIds.delete(key) } if (result.status === 'accepted') { diff --git a/mobile/src/session/mobile-structured-agent-session-rpc.ts b/mobile/src/session/mobile-structured-agent-session-rpc.ts index 279893f6673..4e3861a058e 100644 --- a/mobile/src/session/mobile-structured-agent-session-rpc.ts +++ b/mobile/src/session/mobile-structured-agent-session-rpc.ts @@ -22,7 +22,11 @@ export type StructuredAgentSessionMutationCallResult = | { status: 'accepted'; value: TValue } | { status: 'refused'; code: AgentSessionWireRefusalCode; message: string } | { status: 'failed'; message: string } - | { status: 'unknown' } + /** `hostReportedOperationUnknown` separates a host answer about the id from doubt + * about the effect. Whether that id can still be retried is the method's own + * question: a plan that recovers an unknown ledger row replays or reruns it, one + * that does not refuses the same id until the row expires. */ + | { status: 'unknown'; hostReportedOperationUnknown?: true } export type StructuredAgentSessionMutationResult = | { status: 'accepted'; value: TValue; sameFence: boolean } @@ -169,7 +173,7 @@ export async function requestStructuredAgentSessionMutation(args: { (method === 'agentSession.cancel' || method === 'agentSession.conversationCommand') && result.refusal.code === 'agent_session_operation_unknown' ) { - return { status: 'unknown' } + return { status: 'unknown', hostReportedOperationUnknown: true } } return result.ok ? { status: 'accepted', value: result.value } diff --git a/mobile/src/session/mobile-structured-operation-id-retirement.test.ts b/mobile/src/session/mobile-structured-operation-id-retirement.test.ts new file mode 100644 index 00000000000..6c8b50e33b8 --- /dev/null +++ b/mobile/src/session/mobile-structured-operation-id-retirement.test.ts @@ -0,0 +1,133 @@ +import { describe, expect, it, vi } from 'vitest' +import type { StructuredAgentSessionState } from '../../../src/shared/structured-agent-session-reducer' +import type { RpcClient } from '../transport/rpc-client' +import { markRpcDeliveryUnknown } from '../transport/rpc-delivery-ambiguity' +import { requestMobileStructuredAgentSessionCancel } from './mobile-structured-agent-session-cancel' +import { requestStructuredAgentSessionMutation } from './mobile-structured-agent-session-rpc' + +type SentParams = { envelope: { clientOperationId: string } } + +function operationRefusedAsUnknown() { + return { + ok: true, + result: { + ok: false, + refusal: { + code: 'agent_session_operation_unknown', + message: 'The outcome of operation X is unknown; it was not run again.' + } + }, + _meta: { runtimeId: 'runtime-1' } + } +} + +function fakeClient( + sendRequest: (method: string, params: SentParams) => Promise +): RpcClient { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: both paths under test reach only `sendRequest`. + return { sendRequest } as unknown as RpcClient +} + +function runningState(): StructuredAgentSessionState { + const state = { + fence: 3, + items: [ + { + itemId: 'status-1', + revision: 1, + sequence: 1, + observedAt: 10, + body: { + kind: 'status', + text: 'Working', + turnLifecycle: { turnId: 'turn-1', state: 'running' } + } + } + ] + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: cancel reads only the fence and the running turn. + return state as unknown as StructuredAgentSessionState +} + +function cancelArgs(client: RpcClient, operationIds: Map) { + return { + client, + sessionId: 'session-1', + enabled: true, + stateRef: { current: runningState() }, + sessionKey: 'key-1', + operationIds, + promptCancelSupported: null, + onSendError: vi.fn() + } +} + +describe('structured mutation id retirement', () => { + it('marks a host answer about the id apart from doubt about the effect', async () => { + const result = await requestStructuredAgentSessionMutation({ + client: fakeClient(async () => operationRefusedAsUnknown()), + method: 'agentSession.cancel', + fingerprintMethod: 'agentSession.cancel', + sessionId: 'session-1', + expectedRuntimeFence: 3, + fields: { turnId: 'turn-1' }, + clientOperationId: `1900000000000-${'a'.repeat(32)}` + }) + + expect(result).toEqual({ status: 'unknown', hostReportedOperationUnknown: true }) + }) + + it('leaves the id replayable when only the transport was in doubt', async () => { + const result = await requestStructuredAgentSessionMutation({ + client: fakeClient(async () => { + throw markRpcDeliveryUnknown(new Error('Connection closed')) + }), + method: 'agentSession.cancel', + fingerprintMethod: 'agentSession.cancel', + sessionId: 'session-1', + expectedRuntimeFence: 3, + fields: { turnId: 'turn-1' }, + clientOperationId: `1900000000000-${'b'.repeat(32)}` + }) + + expect(result).toEqual({ status: 'unknown' }) + }) +}) + +describe('structured Stop after an unknown outcome', () => { + it('retries under a fresh id once the host has answered about the previous one', async () => { + const sent: string[] = [] + const client = fakeClient(async (_method, params) => { + sent.push(params.envelope.clientOperationId) + return operationRefusedAsUnknown() + }) + const operationIds = new Map() + const args = cancelArgs(client, operationIds) + + await requestMobileStructuredAgentSessionCancel(args) + await requestMobileStructuredAgentSessionCancel(args) + + expect(sent).toHaveLength(2) + // Reusing it earns the same refusal until the row expires, leaving Stop unusable. + expect(sent[1]).not.toBe(sent[0]) + expect(operationIds.size).toBe(0) + }) + + it('replays the same id when the host never answered', async () => { + const sent: string[] = [] + const client = fakeClient(async (_method, params) => { + sent.push(params.envelope.clientOperationId) + throw markRpcDeliveryUnknown(new Error('Connection closed')) + }) + const operationIds = new Map() + const args = cancelArgs(client, operationIds) + + await requestMobileStructuredAgentSessionCancel(args) + await requestMobileStructuredAgentSessionCancel(args) + + expect(sent).toHaveLength(2) + // Nothing proves the first Stop missed, so the retry must stay a replay. + expect(sent[1]).toBe(sent[0]) + expect(operationIds.size).toBe(1) + }) +})