diff --git a/config/reliability-gates.jsonc b/config/reliability-gates.jsonc index 2c974d9bf59a..5b4c135c1da4 100644 --- a/config/reliability-gates.jsonc +++ b/config/reliability-gates.jsonc @@ -949,7 +949,7 @@ "oracle": "Inject adapter acknowledgement loss, RPC response loss, caller replacement, logical-client close after response, auth recovery with a written request, missing journal submissions, legacy pending rows, stale fences, operation expiry, mobile remount, and durable-journal capacity. Assert one provider dispatch or one operation id for every ambiguous retry, fresh identity only after rejection, and no eviction of ambiguous mobile ids.", "commands": [ "ORCA_BACKGROUND_LAUNCH=1 pnpm test src/shared/agent-session-operation-ledger.test.ts src/shared/structured-agent-session-send-disposition.test.ts src/main/runtime/agent-session-operation-admission.test.ts src/main/native-chat/agent-session-wire/structured-agent-session-refusal-retry.test.ts src/main/native-chat/agent-session-wire/structured-agent-session-send.test.ts src/main/runtime/orchestration/structured-mailbox-pointer-delivery.test.ts src/main/runtime/orchestration/structured-mailbox-pointer-host.test.ts src/main/runtime/orchestration/structured-pointer-operation-id.test.ts src/renderer/src/components/native-chat/use-structured-agent-session-outbox.test.tsx src/renderer/src/components/native-chat/use-structured-agent-session.test.tsx src/renderer/src/components/native-chat/NativeChatStructuredSessionDelivery.test.tsx src/renderer/src/lib/launch-structured-agent-session.test.ts", - "ORCA_BACKGROUND_LAUNCH=1 pnpm --dir mobile test ../mobile/src/session/mobile-native-chat-image-attachment.test.ts ../mobile/src/session/use-mobile-native-chat-image-attachments.test.ts ../mobile/src/session/mobile-structured-send-operation-journal.test.ts ../mobile/src/session/mobile-structured-session-operation-retention.test.ts ../mobile/src/session/mobile-structured-send-delivery.test.ts ../mobile/src/session/use-mobile-structured-agent-session-send.test.tsx ../mobile/src/session/use-mobile-structured-agent-session.test.tsx ../mobile/src/transport/mobile-relay-rpc-session.test.ts ../mobile/src/transport/rpc-client-delivery-ambiguity.test.ts ../mobile/src/transport/stable-logical-rpc-client.test.ts" + "ORCA_BACKGROUND_LAUNCH=1 pnpm --dir mobile test ../mobile/src/session/mobile-native-chat-image-attachment.test.ts ../mobile/src/session/use-mobile-native-chat-image-attachments.test.ts ../mobile/src/session/mobile-structured-send-operation-journal.test.ts ../mobile/src/session/mobile-structured-send-delivery.test.ts ../mobile/src/session/use-mobile-structured-agent-session-send.test.tsx ../mobile/src/session/use-mobile-structured-agent-session.test.tsx ../mobile/src/transport/mobile-relay-rpc-session.test.ts ../mobile/src/transport/rpc-client-delivery-ambiguity.test.ts ../mobile/src/transport/stable-logical-rpc-client.test.ts" ], "testFiles": [ "src/shared/agent-session-operation-ledger.test.ts", @@ -967,7 +967,6 @@ "mobile/src/session/mobile-native-chat-image-attachment.test.ts", "mobile/src/session/use-mobile-native-chat-image-attachments.test.ts", "mobile/src/session/mobile-structured-send-operation-journal.test.ts", - "mobile/src/session/mobile-structured-session-operation-retention.test.ts", "mobile/src/session/mobile-structured-send-delivery.test.ts", "mobile/src/session/use-mobile-structured-agent-session-send.test.tsx", "mobile/src/session/use-mobile-structured-agent-session.test.tsx", @@ -1046,7 +1045,7 @@ "date": "2026-09-12", "runner": "local", "platform": "macos", - "command": "ORCA_BACKGROUND_LAUNCH=1 pnpm --dir mobile test ../mobile/src/session/mobile-native-chat-image-attachment.test.ts ../mobile/src/session/use-mobile-native-chat-image-attachments.test.ts ../mobile/src/session/mobile-structured-send-operation-journal.test.ts ../mobile/src/session/mobile-structured-session-operation-retention.test.ts ../mobile/src/session/mobile-structured-send-delivery.test.ts ../mobile/src/session/use-mobile-structured-agent-session-send.test.tsx ../mobile/src/session/use-mobile-structured-agent-session.test.tsx ../mobile/src/transport/mobile-relay-rpc-session.test.ts ../mobile/src/transport/rpc-client-delivery-ambiguity.test.ts ../mobile/src/transport/stable-logical-rpc-client.test.ts", + "command": "ORCA_BACKGROUND_LAUNCH=1 pnpm --dir mobile test ../mobile/src/session/mobile-native-chat-image-attachment.test.ts ../mobile/src/session/use-mobile-native-chat-image-attachments.test.ts ../mobile/src/session/mobile-structured-send-operation-journal.test.ts ../mobile/src/session/mobile-structured-send-delivery.test.ts ../mobile/src/session/use-mobile-structured-agent-session-send.test.tsx ../mobile/src/session/use-mobile-structured-agent-session.test.tsx ../mobile/src/transport/mobile-relay-rpc-session.test.ts ../mobile/src/transport/rpc-client-delivery-ambiguity.test.ts ../mobile/src/transport/stable-logical-rpc-client.test.ts", "result": "passed", "durationSeconds": 2.37, "summary": "Ten focused mobile session, attachment, and transport files passed 113 tests." diff --git a/mobile/src/session/mobile-structured-agent-session-cancel.ts b/mobile/src/session/mobile-structured-agent-session-cancel.ts index 4666cad48175..3a3286510529 100644 --- a/mobile/src/session/mobile-structured-agent-session-cancel.ts +++ b/mobile/src/session/mobile-structured-agent-session-cancel.ts @@ -5,7 +5,6 @@ import { activeStructuredAgentSessionTurnId } from '../../../src/shared/structur import type { RpcClient } from '../transport/rpc-client' import { requestStructuredAgentSessionMutation, - retainStructuredSessionOperationId, type StructuredAgentSessionMutationCallResult } from './mobile-structured-agent-session-rpc' @@ -27,55 +26,75 @@ export async function requestMobileStructuredAgentSessionCancel(args: { sessionId: string | null enabled: boolean stateRef: { readonly current: StructuredAgentSessionState } - sessionKey: string - operationIds: Map promptCancelSupported: boolean | null prompt?: PromptIdentity + /** Whether the host answers a repeated Stop of a turn quietly; null until the status probe answers. */ + hostAnswersRepeatedStops: boolean | null + /** Stops still on their way, by what they stop; against a host that does not answer a repeat + * quietly, a press for the same one joins it here. */ + inFlight: Map> onSendError: (message: string) => void }): Promise { - const { client, enabled, onSendError, operationIds, sessionId, sessionKey, stateRef } = args + const { client, enabled, inFlight, onSendError, sessionId, stateRef } = args const current = stateRef.current const turnId = activeStructuredAgentSessionTurnId(current.items) if (!client || !sessionId || !enabled || current.fence === null || !turnId) { onSendError('Stop not sent') return false } - // Check the capability before fields enter either the fingerprint or operation key. + // Check the capability before fields enter the fingerprint. const fields = { turnId, ...(args.prompt && args.promptCancelSupported === true ? { prompt: args.prompt } : {}) } - const key = `${sessionKey}:agentSession.cancel:${JSON.stringify(fields)}` - const clientOperationId = retainStructuredSessionOperationId( - operationIds, - key, - operationIds.get(key) - ) + // Every press is its own Stop: a kept id would be answered from the last one and stop nothing. + const fence = current.fence + const stop = () => sendStop({ client, sessionId, fence, fields, onSendError }) + if (args.hostAnswersRepeatedStops === true) { + return stop() + } + // Temporary, for a host that predates the quiet repeated Stop: remove once none is supported. + const key = `${sessionId}:agentSession.cancel:${JSON.stringify(fields)}` + const joined = inFlight.get(key) + if (joined) { + return joined + } + const stopping = stop() + inFlight.set(key, stopping) + // Gone once it settles, so the next press is a new Stop. + void stopping.finally(() => { + if (inFlight.get(key) === stopping) { + inFlight.delete(key) + } + }) + return stopping +} + +async function sendStop(input: { + client: RpcClient + sessionId: string + fence: number + fields: Record + onSendError: (message: string) => void +}): Promise { const result: StructuredAgentSessionMutationCallResult = await requestStructuredAgentSessionMutation({ - client, + client: input.client, method: 'agentSession.cancel', fingerprintMethod: 'agentSession.cancel', - sessionId, - expectedRuntimeFence: current.fence, - fields, - clientOperationId + sessionId: input.sessionId, + expectedRuntimeFence: input.fence, + fields: input.fields }) - // 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') { return true } if (result.status === 'unknown') { - onSendError('Stop unconfirmed — check chat before retrying') + input.onSendError('Stop unconfirmed — check chat before retrying') } else if (result.status === 'refused') { - onSendError(result.message) + input.onSendError(result.message) } else if (result.status === 'failed') { - onSendError(result.message) + input.onSendError(result.message) } return false } diff --git a/mobile/src/session/mobile-structured-agent-session-host-support.test.ts b/mobile/src/session/mobile-structured-agent-session-host-support.test.ts index 1cb211affb6e..0902f318ae3b 100644 --- a/mobile/src/session/mobile-structured-agent-session-host-support.test.ts +++ b/mobile/src/session/mobile-structured-agent-session-host-support.test.ts @@ -2,7 +2,8 @@ import { describe, expect, it } from 'vitest' import { AGENT_SESSION_PROMPT_CANCEL_RUNTIME_CAPABILITY, AGENT_SESSION_QUESTION_ANSWERS_RUNTIME_CAPABILITY, - AGENT_SESSION_QUEUED_MESSAGES_RUNTIME_CAPABILITY + AGENT_SESSION_QUEUED_MESSAGES_RUNTIME_CAPABILITY, + AGENT_SESSION_REPEATED_STOP_RUNTIME_CAPABILITY } from '../../../src/shared/protocol-version' import { structuredAgentSessionHostSupport } from './mobile-structured-agent-session-host-support' @@ -11,16 +12,40 @@ describe('structuredAgentSessionHostSupport', () => { expect(structuredAgentSessionHostSupport([])).toEqual({ promptCancel: false, questionAnswers: false, - queuedMessages: false + queuedMessages: false, + quietRepeatedStop: false }) expect( structuredAgentSessionHostSupport([AGENT_SESSION_QUESTION_ANSWERS_RUNTIME_CAPABILITY]) - ).toEqual({ promptCancel: false, questionAnswers: true, queuedMessages: false }) + ).toEqual({ + promptCancel: false, + questionAnswers: true, + queuedMessages: false, + quietRepeatedStop: false + }) expect( structuredAgentSessionHostSupport([AGENT_SESSION_PROMPT_CANCEL_RUNTIME_CAPABILITY]) - ).toEqual({ promptCancel: true, questionAnswers: false, queuedMessages: false }) + ).toEqual({ + promptCancel: true, + questionAnswers: false, + queuedMessages: false, + quietRepeatedStop: false + }) expect( structuredAgentSessionHostSupport([AGENT_SESSION_QUEUED_MESSAGES_RUNTIME_CAPABILITY]) - ).toEqual({ promptCancel: false, questionAnswers: false, queuedMessages: true }) + ).toEqual({ + promptCancel: false, + questionAnswers: false, + queuedMessages: true, + quietRepeatedStop: false + }) + expect( + structuredAgentSessionHostSupport([AGENT_SESSION_REPEATED_STOP_RUNTIME_CAPABILITY]) + ).toEqual({ + promptCancel: false, + questionAnswers: false, + queuedMessages: false, + quietRepeatedStop: true + }) }) }) diff --git a/mobile/src/session/mobile-structured-agent-session-host-support.ts b/mobile/src/session/mobile-structured-agent-session-host-support.ts index 8994bb73cacf..9ae1d1e90b1a 100644 --- a/mobile/src/session/mobile-structured-agent-session-host-support.ts +++ b/mobile/src/session/mobile-structured-agent-session-host-support.ts @@ -1,7 +1,8 @@ import { AGENT_SESSION_PROMPT_CANCEL_RUNTIME_CAPABILITY, AGENT_SESSION_QUESTION_ANSWERS_RUNTIME_CAPABILITY, - AGENT_SESSION_QUEUED_MESSAGES_RUNTIME_CAPABILITY + AGENT_SESSION_QUEUED_MESSAGES_RUNTIME_CAPABILITY, + AGENT_SESSION_REPEATED_STOP_RUNTIME_CAPABILITY } from '../../../src/shared/protocol-version' /** Structured-session features the connected host advertised; null until the status probe answers. */ @@ -10,6 +11,8 @@ export type StructuredAgentSessionHostSupport = { questionAnswers: boolean /** Mid-turn sends queue as host-held drafts; an older host keeps today's immediate path. */ queuedMessages: boolean + /** A Stop that stopped nothing adds no row, so a repeated Stop is quiet. */ + quietRepeatedStop: boolean } export function structuredAgentSessionHostSupport( @@ -18,6 +21,7 @@ export function structuredAgentSessionHostSupport( return { promptCancel: capabilities.includes(AGENT_SESSION_PROMPT_CANCEL_RUNTIME_CAPABILITY), questionAnswers: capabilities.includes(AGENT_SESSION_QUESTION_ANSWERS_RUNTIME_CAPABILITY), - queuedMessages: capabilities.includes(AGENT_SESSION_QUEUED_MESSAGES_RUNTIME_CAPABILITY) + queuedMessages: capabilities.includes(AGENT_SESSION_QUEUED_MESSAGES_RUNTIME_CAPABILITY), + quietRepeatedStop: capabilities.includes(AGENT_SESSION_REPEATED_STOP_RUNTIME_CAPABILITY) } } diff --git a/mobile/src/session/mobile-structured-agent-session-rpc.ts b/mobile/src/session/mobile-structured-agent-session-rpc.ts index 4476b798230e..7ccbc10e2198 100644 --- a/mobile/src/session/mobile-structured-agent-session-rpc.ts +++ b/mobile/src/session/mobile-structured-agent-session-rpc.ts @@ -1,8 +1,3 @@ -import { - AGENT_SESSION_MAX_OPERATION_REPLAY_AGE_MS, - AGENT_SESSION_OPERATION_FUTURE_SKEW_MS, - parseAgentSessionOperationTimestamp -} from '../../../src/shared/agent-session-host-authority' import type { AgentSessionMutationResult, AgentSessionWireRefusalCode @@ -36,11 +31,7 @@ export type StructuredAgentSessionMutationCallResult = * it, so the same request can never be accepted there. An auth refusal does not set it: * it says nothing about an earlier delivery of the same id. */ | { status: 'failed'; message: string; hostRejectedByRequestSchema?: true } - /** `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 } + | { status: 'unknown' } export type StructuredAgentSessionMutationResult = | { status: 'accepted'; value: TValue; sameFence: boolean } @@ -108,42 +99,6 @@ export async function callAgentSession( return response.result as TResult } -function isReplayableStructuredSessionOperationId(operationId: string, now: number): boolean { - const timestamp = parseAgentSessionOperationTimestamp(operationId) - return ( - timestamp !== null && - timestamp <= now + AGENT_SESSION_OPERATION_FUTURE_SKEW_MS && - now - timestamp <= AGENT_SESSION_MAX_OPERATION_REPLAY_AGE_MS - ) -} - -/** - * Retains transient non-send mutation ids while the host can still replay them. Structured sends - * use the durable journal because delivery ambiguity itself does not expire. - */ -export function retainStructuredSessionOperationId( - operationIds: Map, - key: string, - operationId?: string, - now: number = Date.now() -): string { - const retainedOperationId = - operationId && isReplayableStructuredSessionOperationId(operationId, now) - ? operationId - : structuredSessionOperationId(now) - operationIds.delete(key) - operationIds.set(key, retainedOperationId) - for (const [retainedKey, retainedId] of operationIds) { - if (retainedKey === key) { - continue - } - if (!isReplayableStructuredSessionOperationId(retainedId, now)) { - operationIds.delete(retainedKey) - } - } - return retainedOperationId -} - export function timeoutForDeadline(deadline: number | undefined): number | null { if (deadline === undefined) { return STRUCTURED_SEND_TIMEOUT_MS @@ -205,7 +160,7 @@ export async function requestStructuredAgentSessionMutation(args: { (method === 'agentSession.cancel' || method === 'agentSession.conversationCommand') && result.refusal.code === 'agent_session_operation_unknown' ) { - return { status: 'unknown', hostReportedOperationUnknown: true } + return { status: 'unknown' } } return result.ok ? { status: 'accepted', value: result.value } diff --git a/mobile/src/session/mobile-structured-composer-command.test.ts b/mobile/src/session/mobile-structured-composer-command.test.ts index 41a8a986b1ca..8a54140a7dc3 100644 --- a/mobile/src/session/mobile-structured-composer-command.test.ts +++ b/mobile/src/session/mobile-structured-composer-command.test.ts @@ -24,9 +24,7 @@ function setup() { client: { sendRequest } as unknown as RpcClient, sessionId: 'session', fence: 1, - sessionKey: 'session:1', pending: { current: false }, - operationIds: new Map(), controller: { agent: 'codex', snapshot: [], @@ -61,41 +59,34 @@ describe('mobile structured conversation commands', () => { expect.objectContaining({ command: text.slice(1) }), expect.anything() ) - expect(input.operationIds.size).toBe(0) } ) - it('retains the exact operation ID after an unknown response', async () => { - const { input, sendRequest } = setup() - sendRequest.mockResolvedValueOnce({ - ok: true, - result: { ok: true, value: { command: 'compact', state: 'unknown' } } - }) - expect(await dispatchMobileStructuredCommand(input)).toBe('unknown') - expect(await dispatchMobileStructuredCommand(input)).toBe('accepted') - expect(sendRequest.mock.calls[0]?.[1]).toEqual(sendRequest.mock.calls[1]?.[1]) - }) - it('retains operation identity when the host explicitly reports an unknown ledger outcome', async () => { - const { input, sendRequest } = setup() - sendRequest.mockResolvedValueOnce({ - ok: true, - result: { - ok: false, - refusal: { code: 'agent_session_operation_unknown', message: 'unconfirmed' } + it.each([ + [ + 'the host reports the command unconfirmed', + { ok: true, result: { ok: true, value: { command: 'compact', state: 'unknown' } } } + ], + [ + 'the host cannot say what became of it', + { + ok: true, + result: { + ok: false, + refusal: { code: 'agent_session_operation_unknown', message: 'unconfirmed' } + } } - } as never) - expect(await dispatchMobileStructuredCommand(input)).toBe('unknown') - expect(await dispatchMobileStructuredCommand(input)).toBe('accepted') - expect(sendRequest.mock.calls[0]?.[1]).toEqual(sendRequest.mock.calls[1]?.[1]) - }) - it('retains operation identity when the host fails after starting the command', async () => { + ], + [ + 'the host fails after starting it', + { ok: false, error: { code: 'runtime_error', message: 'settlement failed' } } + ] + ])('sends the next press as a new command after %s', async (_case, answer) => { const { input, sendRequest } = setup() - sendRequest.mockResolvedValueOnce({ - ok: false, - error: { code: 'runtime_error', message: 'settlement failed' } - } as never) + sendRequest.mockResolvedValueOnce(answer) expect(await dispatchMobileStructuredCommand(input)).toBe('unknown') expect(await dispatchMobileStructuredCommand(input)).toBe('accepted') - expect(sendRequest.mock.calls[0]?.[1]).toEqual(sendRequest.mock.calls[1]?.[1]) + // Only the operation id differs between the two requests. + expect(sendRequest.mock.calls[1]?.[1]).not.toEqual(sendRequest.mock.calls[0]?.[1]) }) it.each(['attachments', 'old host', 'arguments', 'pending work'])( 'guards %s without provider dispatch', diff --git a/mobile/src/session/mobile-structured-composer-command.ts b/mobile/src/session/mobile-structured-composer-command.ts index 28a3b3ad28e1..ebe30fe88031 100644 --- a/mobile/src/session/mobile-structured-composer-command.ts +++ b/mobile/src/session/mobile-structured-composer-command.ts @@ -6,10 +6,7 @@ import { } from '../../../src/shared/structured-agent-session-composer' import type { RpcClient } from '../transport/rpc-client' import type { MobileNativeChatSendOutcome } from './mobile-native-chat-send' -import { - requestStructuredAgentSessionMutation, - retainStructuredSessionOperationId -} from './mobile-structured-agent-session-rpc' +import { requestStructuredAgentSessionMutation } from './mobile-structured-agent-session-rpc' export async function dispatchMobileStructuredCommand(input: { text: string @@ -17,9 +14,7 @@ export async function dispatchMobileStructuredCommand(input: { client: RpcClient sessionId: string fence: number - sessionKey: string pending: { current: boolean } - operationIds: Map controller: StructuredAgentSessionComposerOptions canRun: () => boolean onError: (message: string) => void @@ -46,12 +41,6 @@ export async function dispatchMobileStructuredCommand(input: { } } input.pending.current = true - const key = `${input.sessionKey}:agentSession.conversationCommand:${command}` - const clientOperationId = retainStructuredSessionOperationId( - input.operationIds, - key, - input.operationIds.get(key) - ) try { const result = await requestStructuredAgentSessionMutation({ @@ -61,7 +50,6 @@ export async function dispatchMobileStructuredCommand(input: { method: 'agentSession.conversationCommand', fingerprintMethod: 'agentSession.conversationCommand', fields: { command }, - clientOperationId, timeoutMs: Math.max(input.timeoutMs, 195_000) }) if ( @@ -71,10 +59,9 @@ export async function dispatchMobileStructuredCommand(input: { unknown = true return { accepted: false, - error: 'Conversation operation is unconfirmed; retry checks the same operation.' + error: 'Conversation operation was not confirmed.' } } - input.operationIds.delete(key) return result.status === 'accepted' ? { accepted: !result.value.error, error: result.value.error ?? null } : { accepted: false, error: result.message } diff --git a/mobile/src/session/mobile-structured-operation-id-retirement.test.ts b/mobile/src/session/mobile-structured-operation-id-retirement.test.ts index 82ccd1e19dc6..6ca270745684 100644 --- a/mobile/src/session/mobile-structured-operation-id-retirement.test.ts +++ b/mobile/src/session/mobile-structured-operation-id-retirement.test.ts @@ -49,21 +49,21 @@ function runningState(): StructuredAgentSessionState { return state as unknown as StructuredAgentSessionState } -function cancelArgs(client: RpcClient, operationIds: Map) { +function cancelArgs(client: RpcClient, hostAnswersRepeatedStops: boolean | null = false) { return { client, sessionId: 'session-1', enabled: true, stateRef: { current: runningState() }, - sessionKey: 'key-1', - operationIds, promptCancelSupported: null, + hostAnswersRepeatedStops, + inFlight: new Map>(), onSendError: vi.fn() } } describe('structured mutation id retirement', () => { - it('marks a host answer about the id apart from doubt about the effect', async () => { + it('reports a host that cannot say what became of the id as unknown', async () => { const result = await requestStructuredAgentSessionMutation({ client: fakeClient(async () => operationRefusedAsUnknown()), method: 'agentSession.cancel', @@ -74,10 +74,10 @@ describe('structured mutation id retirement', () => { clientOperationId: `1900000000000-${'a'.repeat(32)}` }) - expect(result).toEqual({ status: 'unknown', hostReportedOperationUnknown: true }) + expect(result).toEqual({ status: 'unknown' }) }) - it('leaves the id replayable when only the transport was in doubt', async () => { + it('reports doubt about the transport as unknown', async () => { const result = await requestStructuredAgentSessionMutation({ client: fakeClient(async () => { throw markRpcDeliveryUnknown(new Error('Connection closed')) @@ -101,8 +101,7 @@ describe('structured Stop after an unknown outcome', () => { sent.push(params.envelope.clientOperationId) return operationRefusedAsUnknown() }) - const operationIds = new Map() - const args = cancelArgs(client, operationIds) + const args = cancelArgs(client) await requestMobileStructuredAgentSessionCancel(args) await requestMobileStructuredAgentSessionCancel(args) @@ -110,25 +109,97 @@ describe('structured Stop after an unknown outcome', () => { 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[] = [] + it('stops on a second press after the first lost its answer', async () => { + const ran = new Set() + let lostAnswers = 1 const client = fakeClient(async (_method, params) => { + // As the host's ledger does: an id it already ran replays as handled and stops nothing. + const operationId = params.envelope.clientOperationId + const cancelled = !ran.has(operationId) + ran.add(operationId) + if (lostAnswers > 0) { + lostAnswers -= 1 + throw markRpcDeliveryUnknown(new Error('Connection closed')) + } + return { + ok: true, + result: { ok: true, value: { turnId: 'turn-1', cancelled } }, + _meta: { runtimeId: 'runtime-1' } + } + }) + const args = cancelArgs(client) + + expect(await requestMobileStructuredAgentSessionCancel(args)).toBe(false) + expect(await requestMobileStructuredAgentSessionCancel(args)).toBe(true) + + expect(ran.size).toBe(2) + }) +}) + +describe('a phone Stop pressed again', () => { + function heldStops() { + const answers: (() => void)[] = [] + const sent: string[] = [] + const client = fakeClient((_method, params) => { sent.push(params.envelope.clientOperationId) - throw markRpcDeliveryUnknown(new Error('Connection closed')) + return new Promise((resolve) => + answers.push(() => + resolve({ + ok: true, + result: { ok: true, value: { turnId: 'turn-1', cancelled: true } }, + _meta: { runtimeId: 'runtime-1' } + }) + ) + ) }) - const operationIds = new Map() - const args = cancelArgs(client, operationIds) + return { client, sent, answerAll: () => answers.splice(0).forEach((answer) => answer()) } + } - await requestMobileStructuredAgentSessionCancel(args) - await requestMobileStructuredAgentSessionCancel(args) + it('against a host without the quiet repeated Stop, joins the Stop of that turn still on its way', async () => { + const host = heldStops() + const args = cancelArgs(host.client) - 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) + const presses = [ + requestMobileStructuredAgentSessionCancel(args), + requestMobileStructuredAgentSessionCancel(args) + ] + host.answerAll() + + expect(await Promise.all(presses)).toEqual([true, true]) + expect(host.sent).toHaveLength(1) + }) + + it('sends a new Stop once the first has settled', async () => { + const host = heldStops() + const args = cancelArgs(host.client) + + for (let press = 0; press < 2; press += 1) { + const pressed = requestMobileStructuredAgentSessionCancel(args) + host.answerAll() + await pressed + } + + expect(host.sent).toHaveLength(2) + expect(host.sent[1]).not.toBe(host.sent[0]) + expect(args.inFlight.size).toBe(0) + }) + + it('against a host that answers a repeated Stop quietly, sends every press under its own id', async () => { + const host = heldStops() + const args = cancelArgs(host.client, true) + + const presses = [ + requestMobileStructuredAgentSessionCancel(args), + requestMobileStructuredAgentSessionCancel(args) + ] + await vi.waitFor(() => expect(host.sent).toHaveLength(2)) + host.answerAll() + + expect(await Promise.all(presses)).toEqual([true, true]) + expect(host.sent[1]).not.toBe(host.sent[0]) + expect(args.inFlight.size).toBe(0) }) }) diff --git a/mobile/src/session/mobile-structured-session-operation-retention.test.ts b/mobile/src/session/mobile-structured-session-operation-retention.test.ts deleted file mode 100644 index a9a6a7a35d33..000000000000 --- a/mobile/src/session/mobile-structured-session-operation-retention.test.ts +++ /dev/null @@ -1,79 +0,0 @@ -import { describe, expect, it } from 'vitest' -import { AGENT_SESSION_MAX_OPERATION_REPLAY_AGE_MS } from '../../../src/shared/agent-session-host-authority' -import { retainStructuredSessionOperationId } from './mobile-structured-agent-session-rpc' - -const NOW = 1_900_000_000_000 - -function operationIdAt(timestamp: number, entropy: string): string { - return `${timestamp}-${entropy.repeat(32).slice(0, 32)}` -} - -describe('structured session operation retention', () => { - it('keeps every unconfirmed operation id past the old 128-entry cap', () => { - const operationIds = new Map() - for (let index = 0; index < 400; index += 1) { - retainStructuredSessionOperationId( - operationIds, - `request-${index}`, - operationIdAt(NOW, 'a'), - NOW - ) - } - - expect(operationIds.size).toBe(400) - // Why: the first mutation still needs its id if the user returns to it. - expect(operationIds.get('request-0')).toBe(operationIdAt(NOW, 'a')) - }) - - it('releases only ids the host would already refuse as expired', () => { - const expired = operationIdAt(NOW - AGENT_SESSION_MAX_OPERATION_REPLAY_AGE_MS - 1, 'b') - const admissible = operationIdAt(NOW - AGENT_SESSION_MAX_OPERATION_REPLAY_AGE_MS, 'c') - const operationIds = new Map([ - ['stale', expired], - ['live', admissible] - ]) - - retainStructuredSessionOperationId(operationIds, 'fresh', operationIdAt(NOW, 'd'), NOW) - - expect(operationIds.has('stale')).toBe(false) - expect(operationIds.get('live')).toBe(admissible) - expect(operationIds.get('fresh')).toBe(operationIdAt(NOW, 'd')) - }) - - it('drops ids the host could never admit and re-keys a repeated mutation', () => { - const operationIds = new Map([['unparseable', 'not-an-operation-id']]) - const reused = retainStructuredSessionOperationId( - operationIds, - 'mutation', - operationIdAt(NOW, 'e'), - NOW - ) - - // A retry of the same mutation reuses the retained id rather than minting a duplicate. - expect( - retainStructuredSessionOperationId( - operationIds, - 'mutation', - operationIds.get('mutation'), - NOW - ) - ).toBe(reused) - expect(operationIds.has('unparseable')).toBe(false) - }) - - it('rotates a transient mutation id once the host would refuse it', () => { - const expired = operationIdAt(NOW - AGENT_SESSION_MAX_OPERATION_REPLAY_AGE_MS - 1, 'f') - const operationIds = new Map([['mutation', expired]]) - - const rotated = retainStructuredSessionOperationId( - operationIds, - 'mutation', - operationIds.get('mutation'), - NOW - ) - - expect(rotated).not.toBe(expired) - expect(rotated.startsWith(`${NOW}-`)).toBe(true) - expect(operationIds.get('mutation')).toBe(rotated) - }) -}) diff --git a/mobile/src/session/use-mobile-structured-agent-mutation.ts b/mobile/src/session/use-mobile-structured-agent-mutation.ts index 2d3c95f613e5..4eb8a203121b 100644 --- a/mobile/src/session/use-mobile-structured-agent-mutation.ts +++ b/mobile/src/session/use-mobile-structured-agent-mutation.ts @@ -1,13 +1,12 @@ // The one mutation seam the structured session hook hands its features: builds -// the envelope, retains replayable operation ids per intent, and folds refusals -// into the session's error channel. +// the envelope, with a fresh operation id per call, and folds refusals into the +// session's error channel. import { useCallback } from 'react' import type { StructuredAgentSessionState } from '../../../src/shared/structured-agent-session-reducer' import type { RpcClient } from '../transport/rpc-client' import { requestStructuredAgentSessionMutation, - retainStructuredSessionOperationId, type StructuredAgentSessionMutationResult } from './mobile-structured-agent-session-rpc' @@ -20,13 +19,11 @@ export type MobileStructuredAgentMutate = ( export function useMobileStructuredAgentMutate(args: { client: RpcClient | null sessionId: string | null - sessionKey: string enabled: boolean stateRef: { readonly current: StructuredAgentSessionState } - operationIds: Map onSendError: (message: string) => void }): MobileStructuredAgentMutate { - const { client, enabled, onSendError, operationIds, sessionId, sessionKey, stateRef } = args + const { client, enabled, onSendError, sessionId, stateRef } = args return useCallback( async ( method: string, @@ -38,23 +35,15 @@ export function useMobileStructuredAgentMutate(args: { return { status: 'rejected' } } const targetFence = current.fence - const key = `${sessionKey}:${fingerprintMethod}:${JSON.stringify(fields)}` - const clientOperationId = retainStructuredSessionOperationId( - operationIds, - key, - operationIds.get(key) - ) const result = await requestStructuredAgentSessionMutation({ client, method, fingerprintMethod, sessionId, expectedRuntimeFence: targetFence, - fields, - clientOperationId + fields }) if (result.status === 'accepted') { - operationIds.delete(key) return { status: 'accepted', value: result.value, @@ -62,13 +51,11 @@ export function useMobileStructuredAgentMutate(args: { } } if (result.status === 'unknown') { - operationIds.delete(key) return result } - operationIds.delete(key) onSendError(result.message) return { status: 'rejected' } }, - [client, enabled, onSendError, operationIds, sessionId, sessionKey, stateRef] + [client, enabled, onSendError, sessionId, stateRef] ) } diff --git a/mobile/src/session/use-mobile-structured-agent-session-prompt-cancel.test.tsx b/mobile/src/session/use-mobile-structured-agent-session-prompt-cancel.test.tsx index 97591878d7e2..0bb7a8f74285 100644 --- a/mobile/src/session/use-mobile-structured-agent-session-prompt-cancel.test.tsx +++ b/mobile/src/session/use-mobile-structured-agent-session-prompt-cancel.test.tsx @@ -124,7 +124,8 @@ function Harness({ hostSupport: { promptCancel: promptCancelSupported, questionAnswers: questionAnswersSupported, - queuedMessages: false + queuedMessages: false, + quietRepeatedStop: false }, onSendError: vi.fn() }) diff --git a/mobile/src/session/use-mobile-structured-agent-session-queued.test-fixture.ts b/mobile/src/session/use-mobile-structured-agent-session-queued.test-fixture.ts index e55a26e40c06..bbfe297ef7f3 100644 --- a/mobile/src/session/use-mobile-structured-agent-session-queued.test-fixture.ts +++ b/mobile/src/session/use-mobile-structured-agent-session-queued.test-fixture.ts @@ -15,7 +15,8 @@ export const SESSION_ID = 'session-1' export const CAPABLE: StructuredAgentSessionHostSupport = { promptCancel: false, questionAnswers: false, - queuedMessages: true + queuedMessages: true, + quietRepeatedStop: false } export const LEGACY: StructuredAgentSessionHostSupport = { ...CAPABLE, queuedMessages: false } diff --git a/mobile/src/session/use-mobile-structured-agent-session-queued.test.tsx b/mobile/src/session/use-mobile-structured-agent-session-queued.test.tsx index 0b27efac61d3..277beb17d75a 100644 --- a/mobile/src/session/use-mobile-structured-agent-session-queued.test.tsx +++ b/mobile/src/session/use-mobile-structured-agent-session-queued.test.tsx @@ -13,6 +13,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { structuredAgentSessionPayloadFingerprint } from '../../../src/shared/structured-agent-session-mutation' import type { AgentSessionSubscribeEvent } from '../../../src/shared/agent-session-wire' import type { RpcClient } from '../transport/rpc-client' +import type { RpcResponse } from '../transport/types' import { markRpcDeliveryUnknown } from '../transport/rpc-delivery-ambiguity' import { resetMobileStructuredSendOperationJournalForTests } from './mobile-structured-send-operation-journal' import type { StructuredAgentSessionHostSupport } from './mobile-structured-agent-session-host-support' @@ -605,6 +606,32 @@ describe('mobile structured queued messages', () => { expect(requestOf('agentSession.queuedMessageSend').params.messageId).toBe('draft-1') }) + it('each press of a card action carries its own id, even while an earlier press is unanswered', async () => { + const held = Promise.withResolvers() + sendRequest.mockImplementation(async (method) => + method === 'agentSession.queuedMessageSend' || method === 'agentSession.queuedMessageDelete' + ? held.promise + : method === 'agentSession.options' + ? ok({ models: [], current: {} }) + : ok({}) + ) + await mountSession(CAPABLE) + const presses: Promise[] = [] + act(() => { + presses.push(hook!.queued.send('draft-1'), hook!.queued.send('draft-1')) + presses.push(hook!.queued.delete('draft-1'), hook!.queued.delete('draft-1')) + }) + await act(async () => { + held.resolve(ok({})) + await Promise.all(presses) + }) + for (const method of ['agentSession.queuedMessageSend', 'agentSession.queuedMessageDelete']) { + expect(requestOf(method, 1).envelope.clientOperationId).not.toBe( + requestOf(method, 0).envelope.clientOperationId + ) + } + }) + it('Delete reads the union result: a dispatched draft was already sent', async () => { sendRequest.mockImplementation(async (method) => { if (method === 'agentSession.queuedMessageDelete') { diff --git a/mobile/src/session/use-mobile-structured-agent-session.ts b/mobile/src/session/use-mobile-structured-agent-session.ts index 8b7e15514019..653d7fbaaa1a 100644 --- a/mobile/src/session/use-mobile-structured-agent-session.ts +++ b/mobile/src/session/use-mobile-structured-agent-session.ts @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useMemo, useRef } from 'react' +import { useCallback, useMemo, useRef } from 'react' import { encodeNativeChatTranscriptIdentity } from '../../../src/shared/native-chat-transcript-retention' import type { MobileNativeChatSendOutcome } from './mobile-native-chat-send' import { projectStructuredAgentSessionMessages } from '../../../src/shared/structured-agent-session-message-projection' @@ -97,10 +97,11 @@ export function useMobileStructuredAgentSession(args: { // Old host ⇒ exactly today's behavior: no delivery field, no cards, plain Stop. const queueCapable = hostSupport?.queuedMessages === true const promptCancelSupported = hostSupport?.promptCancel ?? null + const hostAnswersRepeatedStops = hostSupport?.quietRepeatedStop ?? null const sessionKey = encodeNativeChatTranscriptIdentity([sourceIdentity, agent, sessionId]) - const operationIdsRef = useRef(new Map()) const commandPendingRef = useRef(false) - useEffect(() => () => operationIdsRef.current.clear(), []) + // Against a host that predates the quiet repeated Stop, a Stop of a turn still being stopped joins it. + const inFlightStopsRef = useRef(new Map>()) const stateArgs = { client, sessionId, sessionKey, enabled, connected } const { state, stateRef, queuedMessages, queuePause, loadingOlder, loadEarlier } = useMobileStructuredAgentState(stateArgs) @@ -109,10 +110,8 @@ export function useMobileStructuredAgentSession(args: { const mutate = useMobileStructuredAgentMutate({ client, sessionId, - sessionKey, enabled, stateRef, - operationIds: operationIdsRef.current, onSendError }) @@ -147,7 +146,6 @@ export function useMobileStructuredAgentSession(args: { queueCapable, stateRef, commandPending: commandPendingRef, - operationIds: operationIdsRef.current, controller: sendController, onSendError }) @@ -197,15 +195,23 @@ export function useMobileStructuredAgentSession(args: { requestMobileStructuredAgentSessionCancel({ client, enabled, + hostAnswersRepeatedStops, + inFlight: inFlightStopsRef.current, onSendError, - operationIds: operationIdsRef.current, prompt, promptCancelSupported, sessionId, - sessionKey, stateRef }), - [client, enabled, onSendError, promptCancelSupported, sessionId, sessionKey, stateRef] + [ + client, + enabled, + hostAnswersRepeatedStops, + onSendError, + promptCancelSupported, + sessionId, + stateRef + ] ) return { diff --git a/mobile/src/session/use-mobile-structured-send-with-outcome.ts b/mobile/src/session/use-mobile-structured-send-with-outcome.ts index 937cc58b0eba..44c94f777a21 100644 --- a/mobile/src/session/use-mobile-structured-send-with-outcome.ts +++ b/mobile/src/session/use-mobile-structured-send-with-outcome.ts @@ -36,7 +36,6 @@ export function useMobileStructuredSendWithOutcome(args: { queueCapable: boolean stateRef: { readonly current: StructuredAgentSessionState } commandPending: { current: boolean } - operationIds: Map controller: Pick< StructuredAgentSessionComposerOptions, 'snapshot' | 'setOption' | 'invokeAction' | 'conversationCommands' @@ -56,7 +55,6 @@ export function useMobileStructuredSendWithOutcome(args: { controller, enabled, onSendError, - operationIds, queueCapable, sessionId, sessionKey, @@ -90,9 +88,7 @@ export function useMobileStructuredSendWithOutcome(args: { client, sessionId, fence: currentFence, - sessionKey, pending: commandPending, - operationIds, controller: { agent: agent === 'claude' ? 'claude' : 'codex', ...controller @@ -133,7 +129,6 @@ export function useMobileStructuredSendWithOutcome(args: { controller, enabled, onSendError, - operationIds, queueCapable, sessionId, sessionKey, diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-claude-queued-stop.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-queued-stop.test.ts index f78be712f7cf..879f83b8a6ca 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-claude-queued-stop.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-queued-stop.test.ts @@ -314,5 +314,5 @@ it('withdraws a host-queued follow-up but leaves a newer turn running when the S const rows = (await host.journalSnapshot(SESSION)).items.flatMap((item) => item.body.kind === 'status' ? [item.body.text] : [] ) - expect(rows).toContain('The provider had already finished this turn.') + expect(rows).not.toContain('The provider had already finished this turn.') }, 15_000) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-command-turn.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-command-turn.ts index 25b6addb3c58..9eb5cb02ea47 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-command-turn.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-command-turn.ts @@ -120,11 +120,15 @@ export function isStructuredAgentSessionCommandTurnId(turnId: string): boolean { const STOP_NOTE_PREFIX = 'stop:' -/** A Stop's note, on the turn it named. The key says what it is, so a later Stop can read it. */ -export function structuredAgentSessionStopNoteIdentity( - clientOperationId: string -): AgentJournalItemIdentity { - return { provider: 'orca', clientMessageId: `${STOP_NOTE_PREFIX}${clientOperationId}` } +/** A Stop's note, keyed by the turn it stopped (or, with no turn, by its operation). */ +export function structuredAgentSessionStopNoteIdentity(stopKey: string): AgentJournalItemIdentity { + return { provider: 'orca', clientMessageId: `${STOP_NOTE_PREFIX}${stopKey}` } +} + +/** Whether a journal row is a Stop's note. */ +export function isStructuredAgentSessionStopNote(itemId: string): boolean { + const identity = parseAgentJournalItemKey(itemId) + return identity?.provider === 'orca' && identity.clientMessageId.startsWith(STOP_NOTE_PREFIX) } /** Whether an earlier Stop already asked the running command `turnId` names to end. Read from the @@ -134,15 +138,14 @@ export function structuredAgentSessionCommandWasStopped( turnId: string ): boolean { const { itemId } = structuredAgentSessionCommandTurn(turnId.slice('compact:'.length)) - return journal.snapshot().items.some((item) => { - const identity = parseAgentJournalItemKey(item.itemId) - return ( - item.turnScope?.kind === 'turn' && - item.turnScope.turnItemId === itemId && - identity?.provider === 'orca' && - identity.clientMessageId.startsWith(STOP_NOTE_PREFIX) + return journal + .snapshot() + .items.some( + (item) => + item.turnScope?.kind === 'turn' && + item.turnScope.turnItemId === itemId && + isStructuredAgentSessionStopNote(item.itemId) ) - }) } export type StructuredAgentSessionCommandHandoverContext = { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-stop.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-stop.test.ts index 488dcdfd2d01..55f58d06a591 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-stop.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-stop.test.ts @@ -28,7 +28,6 @@ import { import { openTestJournalHostDatabase } from '../agent-session-journal/journal-host-database-test-support' const CALLER = { callerKey: 'client-1' } -const ALREADY_FINISHED = 'The provider had already finished this turn.' let root: string let store: AgentSessionRecordStore @@ -325,7 +324,7 @@ describe('a Stop that names no turn', () => { }) describe('a Stop that names its turn, as an older client sends it', () => { - it('reaches the provider with that turn and keeps its not-cancelled note', async () => { + it('reaches the provider with that turn, and writes no row when it stopped nothing', async () => { cancelTurn.mockResolvedValueOnce({ cancelled: false }) expect(await stop('turn-1')).toMatchObject({ @@ -333,7 +332,7 @@ describe('a Stop that names its turn, as an older client sends it', () => { value: { turnId: 'turn-1', cancelled: false } }) expect(cancelTurn).toHaveBeenCalledWith(expect.objectContaining({ turnId: 'turn-1' })) - expect(await statusRows()).toEqual([ALREADY_FINISHED]) + expect(await statusRows()).toEqual([]) }) async function queueOnHost(): Promise<{ id: string; release: () => void }> { @@ -399,6 +398,25 @@ describe('a Stop on a provider whose Stop ends its session', () => { expect(await statusRows()).toEqual(['Cancellation requested.']) }) + it('ends the child and says it was asked when the provider declined a Stop naming the live turn', async () => { + stopEndsSession = true + await handedOver() + events!.appendItem( + { provider: 'legacy', agent: 'codex', sessionId: SESSION, recordId: 'turn:turn-1' }, + { kind: 'turn', turnId: 'turn-1', state: 'running' }, + { turnScope: AGENT_JOURNAL_THREAD_SCOPE } + ) + await host.flushStreamedEvents(SESSION) + cancelTurn.mockResolvedValueOnce({ cancelled: false }) + + // Ending the session stops the turn, so this Stop stopped something and writes its one note. + expect(await stop('turn-1')).toMatchObject({ ok: true, value: { cancelled: true } }) + await laneDrained() + + expect(closeSession).toHaveBeenCalledWith(SESSION, 'user-stop') + expect(await statusRows()).toEqual(['Cancellation requested.']) + }) + it('keeps the child of a provider whose Stop is not a session boundary', async () => { await handedOver() cancelTurn.mockResolvedValueOnce({ cancelled: true }) @@ -417,7 +435,8 @@ describe('a Stop on a provider whose Stop ends its session', () => { await laneDrained() expect(closeSession).not.toHaveBeenCalled() - expect(await statusRows()).toEqual([ALREADY_FINISHED]) + // It stopped nothing, so it writes no row. + expect(await statusRows()).toEqual([]) }) it("ends the child when the provider's cancel of a Stop naming a turn no longer live fails", async () => { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-grouped-prompt.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-grouped-prompt.test.ts index 7c37b9542536..b18279392b92 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-grouped-prompt.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-grouped-prompt.test.ts @@ -223,3 +223,56 @@ describe('grouped question admission', () => { expect(answerPrompt).not.toHaveBeenCalled() }) }) + +describe('a grouped question answered again', () => { + const answers = [ + { questionId: 'q1', optionIds: ['target-web'] }, + { questionId: 'q2', optionIds: [], other: 'SSH host' } + ] + + async function answer( + prompt: { itemId: string; revision: number }, + given: typeof answers + ): ReturnType { + const fields = { itemId: prompt.itemId, expectedRevision: prompt.revision, answers: given } + return host.respondToPrompt(CALLER, { + envelope: envelope('agentSession.respondTo:question', fields), + kind: 'question', + ...fields + }) + } + + it('answers a re-click with the same answers from the resolution it holds', async () => { + expect((await host.attach(CALLER, attachParams())).ok).toBe(true) + const prompt = await seedGroupedQuestion() + const first = await answer(prompt, answers) + + // The reply to the first was lost, so the re-click still names the revision it saw. + const again = await answer(prompt, answers) + + expect(first.ok).toBe(true) + expect(again).toEqual({ ...first, replayed: false }) + expect(answerPrompt).toHaveBeenCalledTimes(1) + }) + + it('refuses different answers to a resolved question as moved on', async () => { + expect((await host.attach(CALLER, attachParams())).ok).toBe(true) + const prompt = await seedGroupedQuestion() + await answer(prompt, answers) + + const different = await answer(prompt, [ + { questionId: 'q1', optionIds: ['target-mobile'] }, + { questionId: 'q2', optionIds: [], other: 'SSH host' } + ]) + + expect(different).toMatchObject({ + ok: false, + refusal: { + code: 'agent_session_item_revision_stale', + details: { reason: 'promptMoved' }, + resolution: { answers } + } + }) + expect(answerPrompt).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host.test.ts index 46bd84905921..4b1993ea3026 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host.test.ts @@ -413,16 +413,17 @@ describe('respondToPrompt', () => { }) }) - it('refuses a second answer to one prompt and says which answer won', async () => { + it('tells a second answer with the same choice that it holds, and asks the provider once', async () => { await attach() const prompt = await seedApproval() const fields = { itemId: prompt.itemId, expectedRevision: prompt.revision, optionId: 'allow' } - await host.respondToPrompt(CALLER, { + const first = await host.respondToPrompt(CALLER, { envelope: envelope('agentSession.respondTo:approval', fields), kind: 'approval', ...fields }) - const loser = await host.respondToPrompt( + // Another device, or a re-click after a lost reply: a new operation making the same choice. + const second = await host.respondToPrompt( { callerKey: 'client-2' }, { envelope: envelope('agentSession.respondTo:approval', fields), @@ -430,12 +431,11 @@ describe('respondToPrompt', () => { ...fields } ) - expect(loser).toMatchObject({ - ok: false, - refusal: { - code: 'agent_session_item_revision_stale', - resolution: { selectedOptionId: 'allow' } - } + expect(first.ok).toBe(true) + expect(second).toEqual({ ...first, replayed: false }) + expect(second).toMatchObject({ + ok: true, + value: { resolution: { selectedOptionId: 'allow', resolvedBy: 'client-1' } } }) expect(answerPrompt).toHaveBeenCalledTimes(1) }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-prompt-cancel-repeat.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-prompt-cancel-repeat.test.ts new file mode 100644 index 000000000000..6d73178c952c --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-prompt-cancel-repeat.test.ts @@ -0,0 +1,66 @@ +import { AGENT_JOURNAL_THREAD_SCOPE } from '../../../shared/agent-session-journal-types' +import { beforeEach, describe, expect, it, type Mock } from 'vitest' +import type { StructuredAgentSessionAdapter } from './structured-agent-session-adapter' +import type { StructuredAgentSessionHost } from './structured-agent-session-host' +import { + attach, + CALLER, + envelope, + hostTestState, + seedApproval +} from './structured-agent-session-host-test-harness' +import { HOST_TEST_THREAD as THREAD } from './structured-agent-session-host-test-data' + +let host: StructuredAgentSessionHost +let acquire: Mock +let cancelTurn: Mock + +beforeEach(() => { + ;({ host, acquire, cancelTurn } = hostTestState()) +}) + +describe('a prompt Cancel pressed again', () => { + it('answers a second Cancel of a prompt quietly, sent before or after the first settles', async () => { + await attach() + const prompt = await seedApproval() + const identity = { provider: 'codex' as const, threadId: THREAD, turnId: 'turn-1', ordinal: 99 } + const first = Promise.withResolvers() + cancelTurn.mockImplementationOnce(async () => { + await first.promise + // As a provider does: interrupting the turn cancels the prompt it was waiting on. + acquire.mock.calls.at(-1)?.[0].events?.appendItem( + identity, + { + kind: 'approval', + title: 'Run the command?', + detail: null, + options: [{ id: 'allow', label: 'Allow' }], + resolution: { + state: 'cancelled', + selectedOptionId: null, + resolvedBy: null, + resolvedAt: null + } + }, + { turnScope: AGENT_JOURNAL_THREAD_SCOPE } + ) + return { cancelled: true } + }) + const fields = { + turnId: 'turn-1', + prompt: { itemId: prompt.itemId, expectedRevision: prompt.revision } + } + const cancel = () => + host.cancel(CALLER, { envelope: envelope('agentSession.cancel', fields), ...fields }) + + const pressed = cancel() + const whileInFlight = cancel() + first.resolve(undefined) + const after = cancel() + + expect(await pressed).toMatchObject({ ok: true, value: { cancelled: true } }) + expect(await whileInFlight).toMatchObject({ ok: true, value: { cancelled: false } }) + expect(await after).toMatchObject({ ok: true, value: { cancelled: false } }) + expect(cancelTurn).toHaveBeenCalledOnce() + }) +}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-prompt-cancel.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-prompt-cancel.test.ts index 6e0a44e60cac..1dd8fb734164 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-prompt-cancel.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-prompt-cancel.test.ts @@ -163,6 +163,77 @@ describe('performCancel for a pending prompt', () => { ]) }) + it('answers another Cancel of the prompt it cancelled quietly, without reaching the provider', async () => { + const { journal, itemId } = await pendingPrompt() + const cancelTurn = vi.fn(async () => ({ cancelled: true })) + const flush = vi.fn(async () => { + const current = journal.snapshot().items.find((item) => item.itemId === itemId)! + if (current.body.kind !== 'approval') { + throw new Error('expected approval prompt') + } + await journal.appendItem( + PROMPT_IDENTITY, + { + ...current.body, + resolution: { + state: 'cancelled', + selectedOptionId: null, + resolvedBy: null, + resolvedAt: null + } + }, + { fence: 1, turnScope: AGENT_JOURNAL_THREAD_SCOPE } + ) + }) + const ctx = context(journal, cancelTurn, flush) + const cancel = (clientOperationId: string) => + performCancel(ctx, { + clientOperationId, + turnId: 'turn-1', + prompt: { itemId, expectedRevision: 1 } + }) + + await cancel('cancel-1') + const rows = journal.snapshot().items.length + + // The second press names the revision it saw, which the first moved on. + await expect(cancel('cancel-2')).resolves.toEqual({ + ok: true, + value: { turnId: 'turn-1', cancelled: false } + }) + expect(cancelTurn).toHaveBeenCalledOnce() + expect(journal.snapshot().items).toHaveLength(rows) + }) + + it('still refuses a Cancel of a prompt that was answered', async () => { + const { journal, itemId } = await pendingPrompt() + const current = journal.snapshot().items.find((item) => item.itemId === itemId)! + if (current.body.kind !== 'approval') { + throw new Error('expected approval prompt') + } + await journal.appendItem( + PROMPT_IDENTITY, + { + ...current.body, + resolution: { state: 'resolved', selectedOptionId: 'allow', resolvedBy: 'c', resolvedAt: 1 } + }, + { fence: 1, turnScope: AGENT_JOURNAL_THREAD_SCOPE } + ) + const cancelTurn = vi.fn(async () => ({ cancelled: true })) + + const result = await performCancel(context(journal, cancelTurn, vi.fn()), { + clientOperationId: 'cancel-1', + turnId: 'turn-1', + prompt: { itemId, expectedRevision: 1 } + }) + + expect(result).toMatchObject({ + ok: false, + refusal: { code: 'agent_session_item_revision_stale', details: { reason: 'promptMoved' } } + }) + expect(cancelTurn).not.toHaveBeenCalled() + }) + it('keeps the callback answerable when interruption is declined', async () => { const { journal, itemId } = await pendingPrompt() const flush = vi.fn(async () => undefined) @@ -180,8 +251,7 @@ describe('performCancel for a pending prompt', () => { expect(flush).not.toHaveBeenCalled() expect(journal.snapshot().items.map((item) => item.body)).toEqual([ - expect.objectContaining({ resolution: expect.objectContaining({ state: 'pending' }) }), - { kind: 'status', text: 'The provider had already finished this turn.' } + expect.objectContaining({ resolution: expect.objectContaining({ state: 'pending' }) }) ]) }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-prompt-cancel.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-prompt-cancel.ts index 5e3272b4252c..3719d5a9364f 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-prompt-cancel.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-prompt-cancel.ts @@ -9,6 +9,7 @@ import { refuse, type AgentSessionCancelResult } from '../../../shared/agent-ses import { AgentSessionPromptUnavailableError } from './structured-agent-session-adapter' import type { StructuredAgentSessionChatStopRun } from './structured-agent-session-chat-stop' import { + answerCancelOfSettledPrompt, validatePendingPrompt, type PendingPromptValidation } from './structured-agent-session-prompt-state' @@ -27,7 +28,7 @@ export async function cancelStructuredAgentSessionPrompt( ): Promise { const validated = validatePendingPrompt(ctx, input.prompt) if (!validated.ok) { - return validated + return answerCancelOfSettledPrompt(ctx, input, validated) } const route = ctx.adapter.routePromptCancel?.({ sessionId: ctx.sessionId, @@ -49,7 +50,7 @@ export async function cancelStructuredAgentSessionPrompt( await ctx.flushStreamedEvents().catch(() => undefined) const current = validatePendingPrompt(ctx, input.prompt) if (!current.ok) { - return current + return answerCancelOfSettledPrompt(ctx, input, current) } if (!raisedByLiveTurn(ctx, current)) { // A request that outlived its turn, such as a background agent's: the turn running now is not diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-prompt-state.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-prompt-state.ts index 7324e3be9ec5..8cc9b7e4bc92 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-prompt-state.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-prompt-state.ts @@ -2,8 +2,12 @@ import type { AgentJournalItemBody, AgentJournalRenderItem } from '../../../shared/agent-session-journal-types' -import { refuse, type AgentSessionWireRefusal } from '../../../shared/agent-session-wire' -import type { AgentSessionTurnContext } from './structured-agent-session-turns' +import { + refuse, + type AgentSessionCancelResult, + type AgentSessionWireRefusal +} from '../../../shared/agent-session-wire' +import type { AgentSessionTurnContext, TurnOutcome } from './structured-agent-session-turns' type PendingPromptBody = Extract @@ -19,6 +23,30 @@ function promptGone(message: string): PendingPromptValidation { } } +/** The prompt item the client named, once it is no longer waiting on anyone. */ +export function settledPrompt( + ctx: Pick, + itemId: string +): { item: AgentJournalRenderItem; prompt: PendingPromptBody } | null { + const item = ctx.journal.snapshot().items.find((entry) => entry.itemId === itemId) + const prompt = item?.body.kind === 'approval' || item?.body.kind === 'question' ? item.body : null + return item && prompt && prompt.resolution.state !== 'pending' ? { item, prompt } : null +} + +/** A Cancel of a prompt already cancelled has nothing left to do: it is answered, not refused. */ +export function answerCancelOfSettledPrompt( + ctx: Pick, + input: { turnId?: string; prompt: { itemId: string } }, + refused: Extract +): TurnOutcome { + return settledPrompt(ctx, input.prompt.itemId)?.prompt.resolution.state === 'cancelled' + ? { + ok: true, + value: { ...(input.turnId !== undefined ? { turnId: input.turnId } : {}), cancelled: false } + } + : refused +} + export function validatePendingPrompt( ctx: Pick, input: { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-repeat-press.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-repeat-press.test.ts new file mode 100644 index 000000000000..cada58ceecbb --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-repeat-press.test.ts @@ -0,0 +1,72 @@ +// Every draft-button press carries its own operation id, so a second press of +// Send-now, Delete or Resume reaches the host as a new operation. The host +// answers it from the draft's state: nothing sends twice and nothing refuses. + +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { + createQueuedMessageTestRig, + eventually, + type QueuedMessageTestRig +} from './structured-agent-session-queued-message-rig.test-fixture' +import { HOST_TEST_SESSION as SESSION } from './structured-agent-session-host-test-data' + +let rig: QueuedMessageTestRig + +beforeEach(async () => { + rig = await createQueuedMessageTestRig() +}) + +afterEach(() => rig.dispose()) + +async function queuedDraft(text: string): Promise { + const queued = await rig.send(text, 'queue-if-active').result + if (!queued.ok || !('queued' in queued.value)) { + throw new Error('expected a queued receipt') + } + return queued.value.queued.messageId +} + +describe('a repeated draft press under a fresh operation id', () => { + it('Send-now pressed twice sends the draft once and answers both presses with its submission', async () => { + await rig.workingSend() + const draftId = await queuedDraft('send me now') + const [first, second] = await Promise.all([rig.sendNow(draftId), rig.sendNow(draftId)]) + expect(first).toMatchObject({ ok: true, replayed: false }) + expect(second).toMatchObject({ ok: true, replayed: false }) + if (!first.ok || !second.ok) { + throw new Error('expected both presses answered') + } + expect(second.value.clientMessageId).toBe(first.value.clientMessageId) + const handoffs = (await rig.host.journalSnapshot(SESSION)).submissions.filter( + (entry) => entry.queuedMessageId === draftId + ) + expect(handoffs).toHaveLength(1) + await eventually(() => expect(rig.dispatch).toHaveBeenCalledTimes(2)) + }) + + it('Delete pressed twice withdraws the draft once; the second press says it is already gone', async () => { + await rig.workingSend() + const draftId = await queuedDraft('delete me') + const [first, second] = await Promise.all([ + rig.deleteQueued(draftId), + rig.deleteQueued(draftId) + ]) + expect(first).toMatchObject({ ok: true, value: { deleted: true, messageId: draftId } }) + expect(second).toMatchObject({ + ok: true, + value: { deleted: false, messageId: draftId, disposition: 'withdrawn' } + }) + expect(await rig.drafts()).toHaveLength(0) + }) + + it('Resume pressed twice lifts the pause once; the second press lifts nothing and is not refused', async () => { + await rig.workingSend() + await queuedDraft('held by stop') + await rig.stop() + expect(await rig.queuePause()).toEqual({ reason: 'stopped' }) + const [first, second] = await Promise.all([rig.resume(), rig.resume()]) + expect(first).toMatchObject({ ok: true, value: { resumed: true } }) + expect(second).toMatchObject({ ok: true, value: { resumed: false } }) + expect(await rig.queuePause()).toBeNull() + }) +}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-repeat-press.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-repeat-press.test.ts new file mode 100644 index 000000000000..0eaaf48448cc --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-repeat-press.test.ts @@ -0,0 +1,211 @@ +// What the host does with a second press of the same control, against the real host, store and +// journal: under the first press's id it answers from that press and does nothing more, which is +// why a client sends every press under a new id; under a new id it acts. + +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest' +import { computeAgentSessionPayloadFingerprint } from '../../../shared/agent-session-mutation-envelope' +import { agentSessionRefusalOperationState } from '../../../shared/agent-session-refusal-retry' +import type { AgentSessionThreadGoalChange } from '../../../shared/agent-session-wire' +import type { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' +import { openTestAgentSessionRecordStore } from '../../runtime/agent-session-record-store-test-harness' +import { openTestJournalHostDatabase } from '../agent-session-journal/journal-host-database-test-support' +import type { StructuredAgentSessionAdapter } from './structured-agent-session-adapter' +import { StructuredAgentSessionHost } from './structured-agent-session-host' +import { + HOST_TEST_NOW as NOW, + HOST_TEST_SESSION as SESSION, + HOST_TEST_THREAD as THREAD, + hostTestAttachParams, + hostTestOperationId, + resetHostTestOperationIds +} from './structured-agent-session-host-test-data' + +const CALLER = { callerKey: 'client-1' } + +let root: string +let store: AgentSessionRecordStore +let host: StructuredAgentSessionHost +let setOption: Mock +let changeThreadGoal: Mock> +let stopBackgroundTasks: Mock> +let cancelTurn: Mock + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), 'orca-repeat-press-')) + resetHostTestOperationIds() + setOption = vi.fn(async () => undefined) + changeThreadGoal = vi.fn(async () => ({ ok: true as const })) + stopBackgroundTasks = vi.fn(async () => ({ cancelled: true })) + cancelTurn = vi.fn(async () => ({ cancelled: true })) + store = await openTestAgentSessionRecordStore(root) + host = new StructuredAgentSessionHost({ + store, + adapter: { + acquire: async ({ fence, spawnToken }) => ({ + process: { hostId: 'local', pid: 4242, processStartTimeMs: 1_700_000_000_000, spawnToken }, + acquisitionGeneration: 'generation-1', + link: { + linkId: `link-${fence}`, + handle: { provider: 'codex' as const, threadId: THREAD }, + origin: fence > 1 ? ('resumed' as const) : ('created' as const), + mintedAtFence: fence, + observedAt: NOW + } + }), + dispatch: vi.fn(async () => ({ state: 'admitted' as const })), + closeSession: vi.fn(async () => true), + releaseAcquisition: vi.fn(async () => true), + cancelTurn, + answerPrompt: vi.fn(async () => undefined), + setOption, + changeThreadGoal, + supportsThreadGoal: () => true, + stopBackgroundTasks + }, + journalDatabase: openTestJournalHostDatabase(root), + claimKeyId: 'key-1', + mintSpawnToken: () => 'spawn-1', + now: () => NOW + }) + expect(await host.attach(CALLER, hostTestAttachParams(null))).toMatchObject({ ok: true }) +}) + +afterEach(async () => { + await host.flushAllStreamedEvents() + await rm(root, { recursive: true, force: true }) +}) + +function envelope( + method: string, + fields: Record, + clientOperationId: string, + expectedRuntimeFence: number | null = 1 +) { + return { + sessionId: SESSION, + clientOperationId, + expectedRuntimeFence, + payloadFingerprint: computeAgentSessionPayloadFingerprint({ + method, + sessionId: SESSION, + fields + }) + } +} + +function pick(value: string, id = hostTestOperationId(), fence: number | null = 1) { + const fields = { key: 'model', value } + return host.setOption(CALLER, { + envelope: envelope('agentSession.setOption', fields, id, fence), + ...fields + }) +} + +describe('a press sent again', () => { + it('leaves an option on the other pick under the first id, and applies it under a new one', async () => { + const first = hostTestOperationId() + await pick('A', first) + await pick('B') + + expect(await pick('A', first)).toMatchObject({ ok: true, replayed: true }) + expect(setOption.mock.calls.map(([input]) => input.value)).toEqual(['A', 'B']) + + expect(await pick('A')).toMatchObject({ ok: true, replayed: false }) + expect(setOption.mock.calls.map(([input]) => input.value)).toEqual(['A', 'B', 'A']) + }) + + it('leaves a cleared goal cleared under the first id, and sets it under a new one', async () => { + const change = (next: AgentSessionThreadGoalChange, id = hostTestOperationId()) => + host.changeThreadGoal(CALLER, { + envelope: envelope('agentSession.threadGoal', { change: next }, id), + change: next + }) + const set: AgentSessionThreadGoalChange = { kind: 'set', objective: 'Ship the parser' } + const first = hostTestOperationId() + await change(set, first) + await change({ kind: 'clear' }) + + expect(await change(set, first)).toMatchObject({ ok: true, replayed: true }) + expect(changeThreadGoal).toHaveBeenCalledTimes(2) + + expect(await change(set)).toMatchObject({ ok: true, replayed: false }) + expect(changeThreadGoal).toHaveBeenCalledTimes(3) + }) + + it('stops a background task only under a new id once the first Stop ran', async () => { + const fields = { turnId: 'background-tasks', scope: 'background-tasks', taskId: 'task-1' } + const stop = (id = hostTestOperationId()) => + host.cancel(CALLER, { + envelope: envelope('agentSession.cancel', fields, id), + turnId: 'background-tasks', + scope: 'background-tasks' as const, + taskId: 'task-1' + }) + // The first Stop reached the task before it could be stopped; its answer never arrived. + stopBackgroundTasks.mockResolvedValueOnce({ cancelled: false }) + const first = hostTestOperationId() + await stop(first) + + expect(await stop(first)).toMatchObject({ + ok: true, + replayed: true, + value: { cancelled: false } + }) + expect(stopBackgroundTasks).toHaveBeenCalledOnce() + + expect(await stop()).toMatchObject({ ok: true, value: { cancelled: true } }) + expect(stopBackgroundTasks).toHaveBeenCalledTimes(2) + }) + + it('interrupts a named turn only under a new id once the first Stop ran', async () => { + const stop = (id = hostTestOperationId()) => + host.cancel(CALLER, { + envelope: envelope('agentSession.cancel', { turnId: 'turn-1' }, id, null), + turnId: 'turn-1' + }) + cancelTurn.mockResolvedValueOnce({ cancelled: false }) + const first = hostTestOperationId() + await stop(first) + + expect(await stop(first)).toMatchObject({ + ok: true, + replayed: true, + value: { cancelled: false } + }) + expect(cancelTurn).toHaveBeenCalledOnce() + + expect(await stop()).toMatchObject({ ok: true, value: { cancelled: true } }) + expect(cancelTurn).toHaveBeenCalledTimes(2) + }) + + it('keeps nothing of a press it turned away before running, so its own id runs next time', async () => { + const fields = { turnId: 'background-tasks', scope: 'background-tasks' } + const id = hostTestOperationId() + const stop = (fence: number) => + host.cancel(CALLER, { + envelope: envelope('agentSession.cancel', fields, id, fence), + turnId: 'background-tasks', + scope: 'background-tasks' as const + }) + await host.close(SESSION, 'evict') + + // No agent owns the chat, so the host turns the Stop away as not ready. + expect(await stop(1)).toMatchObject({ ok: false }) + const refused = await stop(1) + expect(refused.ok ? null : agentSessionRefusalOperationState(refused.refusal.code)).toBe( + 'pending-admission' + ) + const fence = store.getRecord(SESSION)!.lease.runtimeFence + expect(await host.attach(CALLER, hostTestAttachParams(fence))).toMatchObject({ ok: true }) + + expect(await stop(store.getRecord(SESSION)!.lease.runtimeFence)).toMatchObject({ + ok: true, + replayed: false, + value: { cancelled: true } + }) + expect(stopBackgroundTasks).toHaveBeenCalledOnce() + }) +}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-repeated-stop.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-repeated-stop.test.ts new file mode 100644 index 000000000000..b11b9ad19b1b --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-repeated-stop.test.ts @@ -0,0 +1,132 @@ +// A Stop writes its one note only when it stopped something, keyed by the turn it stopped: a repeated +// Stop, however late and from whichever client, rewrites that row or writes nothing. + +import { beforeEach, describe, expect, it, type Mock } from 'vitest' +import { AGENT_JOURNAL_THREAD_SCOPE } from '../../../shared/agent-session-journal-types' +import type { AgentJournalTurnLifecycleState } from '../../../shared/agent-session-journal-types' +import { openTestAgentSessionRecordStore } from '../../runtime/agent-session-record-store-test-harness' +import { openTestJournalHostDatabase } from '../agent-session-journal/journal-host-database-test-support' +import type { StructuredAgentSessionAdapter } from './structured-agent-session-adapter' +import { StructuredAgentSessionHost } from './structured-agent-session-host' +import { + adapter, + attach, + CALLER, + envelope, + hostTestState, + replaceHostTestState +} from './structured-agent-session-host-test-harness' +import { + HOST_TEST_NOW as NOW, + HOST_TEST_SESSION as SESSION, + HOST_TEST_THREAD as THREAD +} from './structured-agent-session-host-test-data' +import { startAgent } from './structured-agent-session-restart-interruption-test-harness' + +const REQUESTED = 'Cancellation requested.' + +let host: StructuredAgentSessionHost +let acquire: Mock +let cancelTurn: Mock + +beforeEach(() => { + ;({ host, acquire, cancelTurn } = hostTestState()) +}) + +function stopTurn(turnId = 'turn-1') { + return host.cancel(CALLER, { envelope: envelope('agentSession.cancel', { turnId }), turnId }) +} + +/** The provider's record of turn-1, as its translator writes it. */ +async function turn(state: AgentJournalTurnLifecycleState): Promise { + const events = acquire.mock.calls.at(-1)?.[0].events + if (!events) { + throw new Error('turn requires an acquired session') + } + events.appendItem( + { provider: 'codex', threadId: THREAD, turnId: 'turn-1', ordinal: 900 }, + { kind: 'turn', turnId: 'turn-1', state }, + { turnScope: AGENT_JOURNAL_THREAD_SCOPE } + ) + await host.flushStreamedEvents(SESSION) +} + +async function statusRows(): Promise { + return (await host.journalSnapshot(SESSION)).items.flatMap((item) => + item.body.kind === 'status' ? [item.body.text] : [] + ) +} + +describe('a Stop pressed again', () => { + it('writes nothing once the first Stop has settled, however late it arrives', async () => { + await attach() + await turn('running') + expect(await stopTurn()).toMatchObject({ ok: true, value: { cancelled: true } }) + await turn('interrupted') + // The provider has no turn left to stop. + cancelTurn.mockResolvedValueOnce({ cancelled: false }) + + expect(await stopTurn()).toMatchObject({ ok: true, value: { cancelled: false } }) + expect(await statusRows()).toEqual([REQUESTED]) + }) + + it('adds one row for two Stops at once, whatever the provider says to the second', async () => { + await attach() + await turn('running') + const [first, second] = await Promise.all([stopTurn(), stopTurn()]) + + expect(first).toMatchObject({ ok: true, value: { cancelled: true } }) + expect(second).toMatchObject({ ok: true }) + expect(await statusRows()).toEqual([REQUESTED]) + }) + + it('writes nothing when neither Stop found anything to stop', async () => { + await attach() + await turn('running') + await turn('completed') + cancelTurn.mockResolvedValue({ cancelled: false }) + + await stopTurn() + expect(await stopTurn()).toMatchObject({ ok: true, value: { cancelled: false } }) + expect(await statusRows()).toEqual([]) + }) + + it('writes nothing more on a restarted host', async () => { + const { root, store } = hostTestState() + await attach() + await turn('running') + await stopTurn() + await turn('interrupted') + await host.flushAllStreamedEvents() + await store.renewLeases([]) + const relaunchedStore = await openTestAgentSessionRecordStore(root) + const relaunched = new StructuredAgentSessionHost({ + store: relaunchedStore, + adapter: adapter(), + journalDatabase: openTestJournalHostDatabase(root), + claimKeyId: 'key-1', + mintSpawnToken: () => 'spawn-next', + probeOwner: async () => ({ outcome: 'pid-absent' }), + now: () => NOW + 1 + }) + replaceHostTestState({ store: relaunchedStore, host: relaunched }) + host = relaunched + await startAgent({ host, store: relaunchedStore }) + cancelTurn.mockResolvedValueOnce({ cancelled: false }) + + expect(await stopTurn()).toMatchObject({ ok: true, value: { cancelled: false } }) + expect(await statusRows()).toEqual([REQUESTED]) + }) +}) + +describe('a Stop of a turn that ended by itself', () => { + it('writes no row', async () => { + await attach() + await turn('running') + await turn('completed') + cancelTurn.mockResolvedValueOnce({ cancelled: false }) + + expect(await stopTurn()).toMatchObject({ ok: true, value: { cancelled: false } }) + expect(await statusRows()).toEqual([]) + }) +}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-turn-stop-notes.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-turn-stop-notes.ts new file mode 100644 index 000000000000..676b31797658 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-turn-stop-notes.ts @@ -0,0 +1,19 @@ +// A Stop's note sits on the turn it stopped, found by the turn's id whether it still runs or has +// ended, and keyed by that turn, so a repeated Stop rewrites the one row instead of adding one. + +import type { AgentJournalTurnScope } from '../../../shared/agent-session-journal-types' +import { readAgentJournalTurn } from '../../../shared/agent-session-turn-record' +import type { AgentSessionJournal } from '../agent-session-journal/journal-store' + +export const STOP_NOTE_CANCELLATION_REQUESTED = 'Cancellation requested.' + +/** The scope of the turn `turnId` names, running or ended; null when the journal has no such turn. */ +export function structuredAgentSessionNamedTurnScope( + journal: Pick, + turnId: string +): Extract | null { + const turn = journal + .snapshot() + .items.findLast((item) => readAgentJournalTurn(item.body)?.turnId === turnId) + return turn ? { kind: 'turn', turnItemId: turn.itemId } : null +} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-turns-cancel.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-turns-cancel.ts index cdb70ff41d45..d4adf43853b8 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-turns-cancel.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-turns-cancel.ts @@ -11,7 +11,14 @@ import { structuredAgentSessionCommandWasStopped, structuredAgentSessionStopNoteIdentity } from './structured-agent-session-command-turn' -import { validatePendingPrompt } from './structured-agent-session-prompt-state' +import { + answerCancelOfSettledPrompt, + validatePendingPrompt +} from './structured-agent-session-prompt-state' +import { + STOP_NOTE_CANCELLATION_REQUESTED, + structuredAgentSessionNamedTurnScope +} from './structured-agent-session-turn-stop-notes' import { isStructuredAgentSessionMainAgentWorking } from '../../../shared/structured-agent-session-main-agent-working' import type { AgentSessionTurnContext, TurnOutcome } from './structured-agent-session-turns' @@ -82,13 +89,19 @@ export async function performCancel( if (input.prompt) { const validated = validatePendingPrompt(ctx, input.prompt) if (!validated.ok) { - return validated + return answerCancelOfSettledPrompt(ctx, { ...input, prompt: input.prompt }, validated) } } let cancelled = false - let note: AgentJournalStatusItem | null = { kind: 'status', text: 'Cancellation requested.' } + let note: AgentJournalStatusItem | null = { + kind: 'status', + text: STOP_NOTE_CANCELLATION_REQUESTED + } // The turn the Stop names, read before the cancel settles it: the note reports on that turn. - const turnScope = ctx.journal.liveTurnScope() + const turnScope = + (input.turnId !== undefined && !input.scope + ? structuredAgentSessionNamedTurnScope(ctx.journal, input.turnId) + : null) ?? ctx.journal.liveTurnScope() // Only the provider's end or the child's ends a command. A command the provider has not opened a // turn for, would not interrupt, or was already asked to stop, ends with its child; that child's // dead-generation settlement writes the command's verdict. @@ -137,11 +150,9 @@ export async function performCancel( // named or not. The journal judges it: providers differ on refusing a turn that has ended. cancelled = true note = null - } else if (!cancelled && input.turnId !== undefined) { - note = { kind: 'status', text: 'The provider had already finished this turn.' } } else if (!cancelled && input.prompt) { note = null - } else if (!cancelled) { + } else if (!cancelled && input.turnId === undefined) { // Sent only while the chat reads working, so a Stop that ended nothing must say why. const detail = outcome.refusal?.detail note = { @@ -179,7 +190,11 @@ export async function performCancel( } else if (runningCommand && !cancelled) { await input.stopChild?.() cancelled = true - note = { kind: 'status', text: 'Cancellation requested.' } + note = { kind: 'status', text: STOP_NOTE_CANCELLATION_REQUESTED } + } + if (!cancelled && taken === false && input.turnId !== undefined) { + // Nothing was left of the turn it named and nothing else ended: a Stop that ends nothing writes no row. + note = null } if (cancelled && input.prompt) { await ctx.flushStreamedEvents() @@ -188,9 +203,9 @@ export async function performCancel( if (input.scope || note === null) { return { ok: true, value } } - // Keyed by the operation id so a replayed cancel upserts one item, not two. + // Keyed by the turn it stopped, so another Stop of that turn rewrites this row, never adds one. await ctx.journal.appendItem( - structuredAgentSessionStopNoteIdentity(input.clientOperationId), + structuredAgentSessionStopNoteIdentity(input.turnId ?? liveTurnId ?? input.clientOperationId), note, { fence: ctx.fence, turnScope } ) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-turns-prompt.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-turns-prompt.ts index 370dd12500d5..3a2b9a205f28 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-turns-prompt.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-turns-prompt.ts @@ -23,7 +23,7 @@ import { AgentSessionPromptAnswerRejectedError, AgentSessionPromptUnavailableError } from './structured-agent-session-adapter' -import { validatePendingPrompt } from './structured-agent-session-prompt-state' +import { settledPrompt, validatePendingPrompt } from './structured-agent-session-prompt-state' import type { AgentSessionTurnContext, TurnOutcome } from './structured-agent-session-turns' export type AgentSessionPromptRequest = { @@ -70,13 +70,31 @@ function readPromptChoice( : { response: { kind: 'answers', answers }, selectedOptionId } } +/** The item is already resolved with the choice this request makes: the answer holds, so say so. */ +function resolvedWithSameChoice( + ctx: AgentSessionTurnContext, + input: AgentSessionPromptRequest +): AgentSessionPromptResult | null { + const settled = settledPrompt(ctx, input.itemId) + if (settled?.prompt.kind !== input.kind || settled.prompt.resolution.state !== 'resolved') { + return null + } + const { item, prompt } = settled + const choice = readPromptChoice(prompt, input) + return choice?.selectedOptionId === prompt.resolution.selectedOptionId + ? { itemId: item.itemId, revision: item.revision, resolution: prompt.resolution } + : null +} + export async function performPrompt( ctx: AgentSessionTurnContext, input: AgentSessionPromptRequest ): Promise> { const validated = validatePendingPrompt(ctx, input) if (!validated.ok) { - return validated + // A re-click after a lost reply is a new operation; the prompt's state answers it. + const held = resolvedWithSameChoice(ctx, input) + return held ? { ok: true, value: held } : validated } const { prompt } = validated const choice = readPromptChoice(prompt, input) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-turns.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-turns.test.ts index 7c5ecf1ee15d..3e497c311ba5 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-turns.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-turns.test.ts @@ -172,8 +172,7 @@ describe('performCancel', () => { kind: 'status', text: 'Agent is working…', turnLifecycle: { turnId: 'turn-1', state: 'running' } - }, - { kind: 'status', text: 'The provider had already finished this turn.' } + } ]) }) @@ -350,9 +349,6 @@ describe('what a conversation Stop reports when the provider stopped nothing', ( { turnId: 'turn-0', withdrewQueued: true }, 'lands-on-flush' ) - expect(reported).toEqual({ - cancelled: false, - rows: ['The provider had already finished this turn.'] - }) + expect(reported).toEqual({ cancelled: false, rows: [] }) }) }) diff --git a/src/main/native-chat/agent-session-wire/structured-conversation-command-admission.ts b/src/main/native-chat/agent-session-wire/structured-conversation-command-admission.ts index 86bf49946922..dfb4c48f1a9c 100644 --- a/src/main/native-chat/agent-session-wire/structured-conversation-command-admission.ts +++ b/src/main/native-chat/agent-session-wire/structured-conversation-command-admission.ts @@ -2,6 +2,7 @@ import { agentChildWorkViewOffersStop, type AgentSessionBackgroundTaskStops } from '../../../shared/agent-child-work-stop-targets' +import type { AgentSessionConversationCommandRecord } from '../../../shared/agent-session-conversation-command' import type { AgentSessionRecord } from '../../../shared/agent-session-record' import { isQueuedAgentJournalSubmission } from '../../../shared/agent-session-queued-submission' import { agentChildWorkLiveness } from '../../../shared/agent-status-child-work-liveness' @@ -33,6 +34,26 @@ function blocked( return refuse('agent_session_operation_invalid', { reason }, message) } +/** + * The /clear this caller committed on a conversation whose tab has since moved to its replacement. + * A /clear it presses there again asks for what that one already did; any other caller, or a + * cleared conversation opened again from history, reads `conversationCleared` instead. + */ +export function committedClearOfCaller( + record: AgentSessionRecord | null, + callerKey: string, + tabId: string | null +): AgentSessionConversationCommandRecord | null { + const command = record?.conversationCommand + return command?.command === 'clear' && + command.phase === 'committed' && + command.replacementSessionId && + command.callerKey === callerKey && + tabId === null + ? command + : null +} + export function conversationCommandInFlight(): AgentSessionWireRefusal { return blocked('conversationCommandInFlight', 'Wait for the conversation operation to finish.') } diff --git a/src/main/native-chat/agent-session-wire/structured-conversation-command.test.ts b/src/main/native-chat/agent-session-wire/structured-conversation-command.test.ts index 336e120d8baf..380869ed21d6 100644 --- a/src/main/native-chat/agent-session-wire/structured-conversation-command.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-conversation-command.test.ts @@ -669,3 +669,91 @@ describe('what an older build left', () => { expect(startsFor(replacement)).toBe(0) }) }) + +// Every press carries its own operation id, so a /clear pressed after one committed is a new call. +describe('a /clear pressed again after it committed', () => { + function replacementsOtherThanTheSource(): string[] { + return store + .listRecords() + .flatMap((record) => (record.sessionId === HOST_TEST_SESSION ? [] : [record.sessionId])) + } + + function startsForSource(): number { + return vi + .mocked(adapter.acquire) + .mock.calls.filter(([input]) => input.identity.sessionId === HOST_TEST_SESSION).length + } + + async function expectAnsweredWithTheCommittedClear(replacement: string): Promise { + const starts = startsForSource() + const result = await host.conversationCommand(caller, commandParams('clear')) + expect(result).toMatchObject({ + ok: true, + value: { phase: 'committed', state: 'completed', replacementSessionId: replacement } + }) + expect(result.ok && result.value.error).toBeFalsy() + expect(replacementsOtherThanTheSource()).toEqual([replacement]) + // The cleared conversation's agent is not started to answer it. + expect(startsForSource()).toBe(starts) + expect(store.listVisibleSessionIds()).toEqual([replacement]) + expect(host.conversationReplacements().map((entry) => entry.sessionId)).toEqual([replacement]) + } + + it('answers a double press with the clear the first press committed', async () => { + const first = await host.conversationCommand(caller, commandParams('clear')) + const replacement = first.ok ? first.value.replacementSessionId! : '' + expect(replacement).toBeTruthy() + // What the RPC handler does once the first answer is out. + await host.close(HOST_TEST_SESSION, 'user-close') + await expectAnsweredWithTheCommittedClear(replacement) + }) + + it('answers a retyped /clear whose committed answer was lost with that clear', async () => { + const persist = store.recordOperationOutcome.bind(store) + let lost = false + vi.spyOn(store, 'recordOperationOutcome').mockImplementation(async (input) => { + if (!lost && input.outcome.status === 'succeeded' && input.outcome.conversationCommand) { + lost = true + throw new Error('connection lost') + } + return persist(input) + }) + await expect(host.conversationCommand(caller, commandParams('clear'))).rejects.toThrow( + 'connection lost' + ) + const [replacement] = replacementsOtherThanTheSource() + await expectAnsweredWithTheCommittedClear(replacement!) + }) + + it('refuses a second /clear pressed under a new id while the first is still running', async () => { + const [first, second] = await Promise.all([ + host.conversationCommand(caller, commandParams('clear')), + host.conversationCommand(caller, commandParams('clear')) + ]) + expect(first).toMatchObject({ ok: true }) + expect(second).toMatchObject({ + ok: false, + refusal: { details: { reason: 'conversationCommandInFlight' } } + }) + expect(replacementsOtherThanTheSource()).toHaveLength(1) + }) + + it('still tells another window the conversation was cleared', async () => { + expect((await host.conversationCommand(caller, commandParams('clear'))).ok).toBe(true) + expect( + await host.conversationCommand({ callerKey: 'mobile' }, commandParams('clear')) + ).toMatchObject({ ok: false, refusal: { details: { reason: 'conversationCleared' } } }) + expect(replacementsOtherThanTheSource()).toHaveLength(1) + }) + + // Opened again from history on purpose, so that tab stays on the cleared conversation. + it('still tells a cleared conversation opened from history that it was cleared', async () => { + expect((await host.conversationCommand(caller, commandParams('clear'))).ok).toBe(true) + await host.setSessionTabVisibility(HOST_TEST_SESSION, true) + expect(await host.conversationCommand(caller, commandParams('clear'))).toMatchObject({ + ok: false, + refusal: { details: { reason: 'conversationCleared' } } + }) + expect(replacementsOtherThanTheSource()).toHaveLength(1) + }) +}) diff --git a/src/main/native-chat/agent-session-wire/structured-conversation-command.ts b/src/main/native-chat/agent-session-wire/structured-conversation-command.ts index 4a825d6081d5..30a4bfecf512 100644 --- a/src/main/native-chat/agent-session-wire/structured-conversation-command.ts +++ b/src/main/native-chat/agent-session-wire/structured-conversation-command.ts @@ -12,7 +12,11 @@ import type { StructuredAgentSessionMutationContext } from './structured-agent-s import { sendPreparation } from './structured-agent-session-send-preparation' import type { StructuredAgentSessionCaller } from './structured-agent-session-host-types' import type { StructuredAgentSessionHost } from './structured-agent-session-host' -import { conversationCommandBlocked } from './structured-conversation-command-admission' +import { + committedClearOfCaller, + conversationCommandBlocked +} from './structured-conversation-command-admission' +import { computeAgentSessionPayloadFingerprint } from '../../../shared/agent-session-mutation-envelope' import type { AgentSessionFailureFact } from '../../../shared/agent-session-failure' import { agentSessionFailureWords, @@ -43,6 +47,39 @@ export type ConversationReplacement = { agent: 'claude' | 'codex' } +const clearFingerprintOf = (sessionId: string) => + computeAgentSessionPayloadFingerprint({ + method: 'agentSession.conversationCommand', + sessionId, + fields: { command: 'clear' } + }) + +/** This caller's committed /clear, for a /clear it presses again on the conversation that one + * cleared. Answered before admission, which would refuse it as cleared. */ +async function answerFromCommittedClear( + context: StructuredAgentSessionMutationContext, + caller: StructuredAgentSessionCaller, + { envelope, command }: ConversationCommandParams +): Promise | null> { + const { store } = context.deps + const record = store.getRecord(envelope.sessionId) + const committed = + command === 'clear' && envelope.payloadFingerprint === clearFingerprintOf(envelope.sessionId) + ? committedClearOfCaller(record, caller.callerKey, store.getSessionTabId(envelope.sessionId)) + : null + const session = + committed && (await context.openConversation(envelope.sessionId).catch(() => null)) + return record && committed && session + ? { + ok: true, + replayed: true, + fence: record.lease.runtimeFence, + cursor: session.journal.cursor(), + value: committed + } + : null +} + /** * `/clear`: one write that points this conversation at a new, at-rest one and moves its tab there. * The new conversation's first send starts its agent. @@ -62,8 +99,12 @@ export function runStructuredConversationCommand( ? record : null } - return context.serialize(sessionId, () => - admitAndRunAgentSessionMutation({ + return context.serialize(sessionId, async () => { + const committed = await answerFromCommittedClear(context, caller, params) + if (committed) { + return committed + } + return admitAndRunAgentSessionMutation({ store, adapter: context.deps.adapter, callerKey: caller.callerKey, @@ -138,5 +179,5 @@ export function runStructuredConversationCommand( } } }) - ) + }) } diff --git a/src/main/native-chat/agent-session-wire/structured-conversation-compaction.test.ts b/src/main/native-chat/agent-session-wire/structured-conversation-compaction.test.ts index 283f7c4f934c..22766f28a416 100644 --- a/src/main/native-chat/agent-session-wire/structured-conversation-compaction.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-conversation-compaction.test.ts @@ -749,3 +749,23 @@ it('never lets a provider echo alias the command entry', async () => { true ) }) + +it('refuses a /compact pressed again under a new id while one runs, and runs one pressed after it ended', async () => { + await attach() + await state.host.conversationCommand(CALLER, compactParams()) + await vi.waitFor(() => expect(compact).toHaveBeenCalledOnce()) + + expect(await state.host.conversationCommand(CALLER, compactParams())).toMatchObject({ + ok: false, + refusal: { details: { reason: 'turnActive' } } + }) + expect(compact).toHaveBeenCalledOnce() + + finish({ outcome: 'success' }) + await vi.waitFor(async () => + expect(await state.host.conversationCommand(CALLER, compactParams())).toMatchObject({ + ok: true + }) + ) + await vi.waitFor(() => expect(compact).toHaveBeenCalledTimes(2)) +}) diff --git a/src/main/runtime/rpc/methods/structured-chat-tab-table.test.ts b/src/main/runtime/rpc/methods/structured-chat-tab-table.test.ts index e8f7108f9f99..f437e8068990 100644 --- a/src/main/runtime/rpc/methods/structured-chat-tab-table.test.ts +++ b/src/main/runtime/rpc/methods/structured-chat-tab-table.test.ts @@ -222,6 +222,45 @@ describe('a chat tab across /clear', () => { expect(store.listVisibleSessionIds()).toEqual([current]) }) + // Each press carries its own operation id, so a later /clear is a new call the host answers. + it('answers a /clear pressed again after it committed with that clear, starting nothing', async () => { + await createChat(HOST_TEST_SESSION) + const replacement = await clear(HOST_TEST_SESSION) + const started = acquisitions + expect(await clear(HOST_TEST_SESSION)).toBe(replacement) + expect(acquisitions).toBe(started) + expect((await snapshot()).tabs.map((tab) => tab.id)).toEqual([`agent-session:${replacement}`]) + }) + + it('answers a retyped /clear whose answer was lost with that clear, the tab on its replacement', async () => { + await createChat(HOST_TEST_SESSION) + const persist = store.recordOperationOutcome.bind(store) + let lost = false + vi.spyOn(store, 'recordOperationOutcome').mockImplementation(async (input) => { + if (!lost && input.outcome.status === 'succeeded' && input.outcome.conversationCommand) { + lost = true + throw new Error('connection lost') + } + return persist(input) + }) + const lostAnswer = await call('agentSession.conversationCommand', { + command: 'clear', + envelope: envelopeFor('agentSession.conversationCommand', HOST_TEST_SESSION, { + command: 'clear' + }) + }) + expect(lostAnswer.result?.ok).not.toBe(true) + const started = acquisitions + const replacement = await clear(HOST_TEST_SESSION) + expect(acquisitions).toBe(started) + expect(store.getSessionTabId(replacement)).toBe(SOURCE_TAB) + expect((await snapshot()).tabs.map((tab) => tab.id)).toEqual([`agent-session:${replacement}`]) + expect(await send(replacement, 'after the retry')).toMatchObject({ + ok: true, + result: { ok: true } + }) + }) + it('reveals a cleared conversation in its own tab without activating the current chat', async () => { await createChat(HOST_TEST_SESSION) const replacement = await clear(HOST_TEST_SESSION) diff --git a/src/renderer/src/components/native-chat/structured-conversation-command-send.ts b/src/renderer/src/components/native-chat/structured-conversation-command-send.ts index 23c1fa3b93c7..0a351db917f1 100644 --- a/src/renderer/src/components/native-chat/structured-conversation-command-send.ts +++ b/src/renderer/src/components/native-chat/structured-conversation-command-send.ts @@ -82,10 +82,3 @@ function conversationCommandFailureText( sayAgentSessionFailureTranslated ) } - -export function isUnconfirmedConversationCommand(method: string, value: unknown): boolean { - return ( - method === 'agentSession.conversationCommand' && - (value as AgentSessionConversationCommandResult).state === 'unknown' - ) -} diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-command-start-failure.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session-command-start-failure.test.tsx index 1a76607a4848..698d83e61065 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session-command-start-failure.test.tsx +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-command-start-failure.test.tsx @@ -8,14 +8,19 @@ import { agentJournalItemKey } from '../../../../shared/agent-session-journal-it import type { AgentJournalRenderItem } from '../../../../shared/agent-session-journal-types' import { structuredAgentSessionStartFailureRowIdentity } from '../../../../shared/structured-agent-session-start-failure-row-key' -const mocks = vi.hoisted(() => ({ call: vi.fn(), toastError: vi.fn() })) +const mocks = vi.hoisted(() => ({ + call: vi.fn(), + toastError: vi.fn(), + quietRepeatedStop: vi.fn(async () => false) +})) let fence = 3 let items: AgentJournalRenderItem[] = [] vi.mock('sonner', () => ({ toast: { error: mocks.toastError, message: vi.fn() } })) vi.mock('@/runtime/structured-agent-session-client', () => ({ - callStructuredAgentSession: mocks.call + callStructuredAgentSession: mocks.call, + supportsStructuredAgentSessionQuietRepeatedStop: mocks.quietRepeatedStop })) vi.mock('./use-structured-agent-session-read', () => ({ @@ -296,22 +301,31 @@ describe('a conversation command whose reply lands after the fence moved', () => }) describe('any other write across a fence move', () => { - it('drops the result: it answered for the runtime this pane replaced', async () => { - const answer = heldWrites() - const { result, rerender } = renderPane() - let stopped: Promise = Promise.resolve('unset') - act(() => { - stopped = result.current.cancel('turn-1') - }) - fence = 5 - rerender({ sessionId: 'session-1', transportEnabled: true }) - await act(async () => { - answer({ ok: true, replayed: false, fence: 3, value: { cancelled: true } }) - await stopped - }) - expect(await stopped).toBeNull() - expect(mocks.toastError).not.toHaveBeenCalled() - }) + it.each([ + ['a host without the quiet repeated Stop', false], + ['a host with it', true] + ])( + 'drops the result: it answered for the runtime this pane replaced (%s)', + async (_host, quiet) => { + mocks.quietRepeatedStop.mockResolvedValueOnce(quiet) + const answer = heldWrites() + const { result, rerender } = renderPane() + let stopped: Promise = Promise.resolve('unset') + act(() => { + stopped = result.current.cancel('turn-1') + }) + fence = 5 + rerender({ sessionId: 'session-1', transportEnabled: true }) + await act(async () => { + // The Stop reaches the host once the client knows the host's capabilities. + await vi.waitFor(() => expect(mocks.call).toHaveBeenCalled()) + answer({ ok: true, replayed: false, fence: 3, value: { cancelled: true } }) + await stopped + }) + expect(await stopped).toBeNull() + expect(mocks.toastError).not.toHaveBeenCalled() + } + ) it('keeps a result the same fence answers', async () => { const answer = heldWrites() @@ -321,6 +335,8 @@ describe('any other write across a fence move', () => { stopped = result.current.cancel('turn-1') }) await act(async () => { + // The Stop reaches the host once the client knows the host's capabilities. + await vi.waitFor(() => expect(mocks.call).toHaveBeenCalled()) answer({ ok: true, replayed: false, fence: 3, value: { cancelled: true } }) await stopped }) diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-mutate.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session-mutate.test.tsx new file mode 100644 index 000000000000..81f61815dd9a --- /dev/null +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-mutate.test.tsx @@ -0,0 +1,305 @@ +// @vitest-environment happy-dom + +// One press is one action: a later press of the same control is never answered from an earlier one. + +import { act, renderHook } from '@testing-library/react' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ + call: vi.fn(), + hostAnswersRepeatedStops: vi.fn(async () => false), + toastError: vi.fn(), + operations: 0 +})) + +vi.mock('sonner', () => ({ toast: { error: mocks.toastError, message: vi.fn() } })) + +vi.mock('@/runtime/structured-agent-session-client', () => ({ + callStructuredAgentSession: mocks.call, + supportsStructuredAgentSessionQuietRepeatedStop: mocks.hostAnswersRepeatedStops +})) + +vi.mock('./use-structured-agent-session-outbox', () => ({ + structuredSessionOperationId: () => `operation-${++mocks.operations}` +})) + +import { useStructuredAgentSessionMutate } from './use-structured-agent-session-mutate' + +type Params = { + envelope: { clientOperationId: string } + key?: string + value?: string + change?: { kind: 'set'; objective: string } | { kind: 'clear' } +} +type Answer = + | { ok: true; value: unknown } + | { ok: false; refusal: { code: string; message: string } } + +const LOST = new Error('the connection dropped before the host answered') + +/** The host's ledger: an id it already ran gets that answer again, and nothing runs. */ +function ledgerHost(run: (method: string, params: Params) => Answer) { + const ledger = new Map() + const ids: string[] = [] + let lostAnswers = 0 + mocks.call.mockImplementation(async (_target, method: string, params: Params) => { + const id = params.envelope.clientOperationId + ids.push(id) + const answer = ledger.get(id) ?? run(method, params) + if (answer.ok) { + ledger.set(id, answer) + } + if (lostAnswers > 0) { + lostAnswers -= 1 + throw LOST + } + return answer + }) + return { + ids, + /** The host runs the next write, but its answer never reaches the client. */ + loseNextAnswer: () => { + lostAnswers += 1 + } + } +} + +function render() { + return renderHook(() => + useStructuredAgentSessionMutate({ + sessionId: 'session-1', + target: { kind: 'local' }, + stateRef: { current: { fence: 3 } } + }) + ).result +} + +beforeEach(() => { + vi.clearAllMocks() + mocks.hostAnswersRepeatedStops.mockResolvedValue(false) + mocks.operations = 0 +}) + +describe('a write pressed again', () => { + it('applies an option picked again after a pick whose answer was lost', async () => { + let option: unknown = null + const host = ledgerHost((_method, params) => { + option = params.value + return { ok: true, value: { key: params.key, value: params.value } } + }) + const { current } = render() + + host.loseNextAnswer() + for (const value of ['A', 'B', 'A']) { + await act(async () => { + await current.mutate('agentSession.setOption', 'agentSession.setOption', { + key: 'model', + value + }) + }) + } + + expect(option).toBe('A') + expect(new Set(host.ids).size).toBe(3) + }) + + it('sets a goal again after it was cleared, when the first set lost its answer', async () => { + let goal: string | null = null + const host = ledgerHost((_method, params) => { + goal = params.change?.kind === 'set' ? params.change.objective : null + return { ok: true, value: { change: params.change?.kind } } + }) + const { current } = render() + const set = { change: { kind: 'set', objective: 'Ship the parser' } } + + host.loseNextAnswer() + for (const fields of [set, { change: { kind: 'clear' } }, set]) { + await act(async () => { + await current.mutate('agentSession.threadGoal', 'agentSession.threadGoal', fields) + }) + } + + expect(goal).toBe('Ship the parser') + }) + + it('says nothing when the same answer is clicked again after its reply was lost', async () => { + const resolution = { state: 'resolved', selectedOptionId: 'allow' } + // A host that already resolved the prompt that way answers with the resolution it holds. + const host = ledgerHost(() => ({ + ok: true, + value: { itemId: 'item-1', revision: 2, resolution } + })) + const { current } = render() + const answer = { itemId: 'item-1', expectedRevision: 1, optionId: 'allow' } + + host.loseNextAnswer() + let second: unknown + await act(async () => { + await current.mutate( + 'agentSession.respondToApproval', + 'agentSession.respondTo:approval', + answer + ) + mocks.toastError.mockClear() + second = await current.mutate( + 'agentSession.respondToApproval', + 'agentSession.respondTo:approval', + answer + ) + }) + + expect(second).toMatchObject({ resolution }) + expect(mocks.toastError).not.toHaveBeenCalled() + expect(host.ids[1]).not.toBe(host.ids[0]) + }) +}) + +describe('a conversation command typed again', () => { + it('goes out as a new command, which the host resumes or runs', async () => { + const ids: string[] = [] + mocks.call.mockImplementation(async (_target, _method, params: Params) => { + ids.push(params.envelope.clientOperationId) + if (ids.length === 1) { + throw LOST + } + return { ok: true, value: { command: 'clear', state: 'completed' } } + }) + const { current } = render() + + for (let press = 0; press < 2; press += 1) { + await act(async () => { + await current.write( + 'agentSession.conversationCommand', + 'agentSession.conversationCommand', + { command: 'clear' } + ) + }) + } + + expect(ids).toHaveLength(2) + expect(ids[1]).not.toBe(ids[0]) + }) +}) + +/** A host that holds each Stop until the test answers the ones it was sent. */ +function heldStops() { + const answers: (() => void)[] = [] + const ids: string[] = [] + mocks.call.mockImplementation((_target, _method, params: Params) => { + ids.push(params.envelope.clientOperationId) + return new Promise((resolve) => + answers.push(() => resolve({ ok: true, value: { turnId: 'turn-1', cancelled: true } })) + ) + }) + return { + ids, + answerWhenSent: async (count: number) => { + await vi.waitFor(() => expect(answers).toHaveLength(count)) + answers.splice(0).forEach((answer) => answer()) + } + } +} + +const stop = (current: ReturnType['current'], turnId = 'turn-1') => + current.mutate('agentSession.cancel', 'agentSession.cancel', { turnId }) + +describe('a Stop pressed again, against a host without the quiet repeated Stop', () => { + it('joins the one still in flight instead of sending a second', async () => { + const host = heldStops() + const { current } = render() + + let first: Promise = Promise.resolve() + let second: Promise = Promise.resolve() + act(() => { + first = stop(current) + second = stop(current) + }) + await act(async () => { + await host.answerWhenSent(1) + await Promise.all([first, second]) + }) + + expect(mocks.call).toHaveBeenCalledOnce() + expect(await second).toEqual(await first) + }) + + it('reports a failed Stop once, not once per press that joined it', async () => { + const answers: (() => void)[] = [] + mocks.call.mockImplementation( + () => new Promise((_resolve, reject) => answers.push(() => reject(LOST))) + ) + const { current } = render() + + await act(async () => { + const pressed = [stop(current), stop(current)] + await vi.waitFor(() => expect(answers).toHaveLength(1)) + answers.splice(0).forEach((answer) => answer()) + await Promise.all(pressed) + }) + + expect(mocks.call).toHaveBeenCalledOnce() + expect(mocks.toastError).toHaveBeenCalledOnce() + }) + + it('sends a new Stop once the first has settled', async () => { + const host = heldStops() + const { current } = render() + + for (let press = 0; press < 2; press += 1) { + await act(async () => { + const pressed = stop(current) + await host.answerWhenSent(1) + await pressed + }) + } + + const { ids } = host + expect(ids).toHaveLength(2) + expect(ids[1]).not.toBe(ids[0]) + }) + + it('sends a Stop naming no turn of its own, since it stops whatever then runs', async () => { + const host = heldStops() + const { current } = render() + + await act(async () => { + const pressed = [ + current.mutate('agentSession.cancel', 'agentSession.cancel', {}), + current.mutate('agentSession.cancel', 'agentSession.cancel', {}) + ] + await host.answerWhenSent(2) + await Promise.all(pressed) + }) + + expect(host.ids).toHaveLength(2) + }) + + it('sends a Stop of another turn even while one is in flight', async () => { + const host = heldStops() + const { current } = render() + + await act(async () => { + const pressed = [stop(current, 'turn-1'), stop(current, 'turn-2')] + await host.answerWhenSent(2) + await Promise.all(pressed) + }) + + expect(mocks.call).toHaveBeenCalledTimes(2) + }) +}) + +describe('a Stop pressed again, against a host that answers a repeat quietly', () => { + it('sends every press under its own id, even while an earlier one is on its way', async () => { + mocks.hostAnswersRepeatedStops.mockResolvedValue(true) + const host = heldStops() + const { current } = render() + + await act(async () => { + const pressed = [stop(current), stop(current)] + await host.answerWhenSent(2) + await Promise.all(pressed) + }) + + expect(new Set(host.ids).size).toBe(2) + }) +}) diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-mutate.ts b/src/renderer/src/components/native-chat/use-structured-agent-session-mutate.ts index a9ea417cda69..1f5116718a50 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session-mutate.ts +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-mutate.ts @@ -1,29 +1,28 @@ // One structured-session mutation, fenced and idempotent. // -// The client operation id is keyed on (session, method, payload) so a retry of -// the same request reuses it and the host upserts one row instead of two (a -// payload naming no target gets a new one on every call), and -// every result is discarded unless the runtime fence it was issued against is -// still the current one; the reply to the conversation command a pane still -// waits on is kept, since that command may move the fence itself. A write that -// did not happen is reported once, in the person's words, by the caller that -// knows where to say it; nothing latches. +// Every call is its own action with its own client operation id, so a press is never answered +// from an earlier one. The host answers a second Stop of a named turn or task quietly; against an +// older host that does not, this client joins one still on its way instead of stopping twice. +// Every result is discarded unless the runtime fence it was issued against is still the current +// one, except the reply to the conversation command a pane still waits on, since that command may +// move the fence itself. A write that did not happen is reported once, in the person's words, by +// the caller that knows where to say it; nothing latches. import { useCallback, useEffect, useRef } from 'react' import { toast } from 'sonner' -import * as conversationCommands from './structured-conversation-command-send' import type { AgentSessionMutationResult } from '../../../../shared/agent-session-wire' import { agentSessionRefusalFailure, agentSessionThrownFailure, agentSessionWriteKindForMethod as writeKind } from '../../../../shared/agent-session-write-failure' -import { agentSessionRefusalOperationState } from '../../../../shared/agent-session-refusal-retry' import { structuredAgentSessionPayloadFingerprint } from '../../../../shared/structured-agent-session-mutation' -import { structuredAgentSessionWriteNamesItsTarget } from '../../../../shared/structured-agent-session-operation-identity' import type { RuntimeClientTarget } from '@/runtime/runtime-rpc-client' import { RuntimeRpcCallError } from '@/runtime/runtime-rpc-result' -import { callStructuredAgentSession } from '@/runtime/structured-agent-session-client' +import { + callStructuredAgentSession, + supportsStructuredAgentSessionQuietRepeatedStop +} from '@/runtime/structured-agent-session-client' import { structuredSessionOperationId } from './use-structured-agent-session-outbox' import { agentSessionWriteFailureText } from './agent-session-write-notice-text' @@ -34,12 +33,7 @@ export type StructuredAgentSessionWriteOutcome = /** Settled for an owner or session this pane no longer shows; there is nothing to say. */ | { kind: 'dropped' } -type WriteArgs = [ - method: string, - fingerprintMethod: string, - fields: Record, - operationIdOverride?: string | null -] +type WriteArgs = [method: string, fingerprintMethod: string, fields: Record] export type StructuredAgentSessionWrite = ( ...args: WriteArgs @@ -48,6 +42,17 @@ export type StructuredAgentSessionWrite = ( /** A write whose failure is shown as a toast; the control that sent it is the way to try again. */ export type StructuredAgentSessionMutate = (...args: WriteArgs) => Promise +/** A Stop naming its turn or task; one naming neither acts on whatever runs when the host reaches + * it, so a second press is a Stop of its own. */ +function namesWhatItStops(fingerprintMethod: string, fields: Record): boolean { + if (fingerprintMethod !== 'agentSession.cancel') { + return false + } + return fields.scope === 'background-tasks' + ? fields.taskId !== undefined + : fields.turnId !== undefined +} + export function useStructuredAgentSessionMutate(args: { sessionId: string target: RuntimeClientTarget @@ -60,7 +65,9 @@ export function useStructuredAgentSessionMutate(args: { mutate: StructuredAgentSessionMutate } { const { enabled = true, sessionId, stateRef, target } = args - const operationIds = useRef(new Map()) + const inFlightStops = useRef( + new Map>>() + ) const enabledRef = useRef(enabled) // The conversation command this pane still waits on; only its reply may outlive a fence move. A // newer command replaces it, and disabling the pane forgets it. @@ -71,17 +78,18 @@ export function useStructuredAgentSessionMutate(args: { commandInFlight.current = null }, [enabled, sessionId]) - const write = useCallback( + const send = useCallback( async ( method: string, fingerprintMethod: string, fields: Record, - operationIdOverride?: string | null + /** The fence when the press happened, read before any await, so a write never retargets. */ + pressedFence: number | null = stateRef.current.fence ): Promise> => { - if (!enabled || !enabledRef.current || stateRef.current.fence === null) { + if (!enabled || !enabledRef.current || pressedFence === null) { return { kind: 'dropped' } } - const targetFence = stateRef.current.fence + const targetFence = pressedFence const request = {} if (fingerprintMethod === 'agentSession.conversationCommand') { commandInFlight.current = request @@ -97,23 +105,12 @@ export function useStructuredAgentSessionMutate(args: { } return enabledRef.current && (stateRef.current.fence === targetFence || waitedOn) } - const key = `${sessionId}:${fingerprintMethod}:${JSON.stringify(fields)}` - // A write naming no target acts on whatever is in flight when the host reaches it, so every - // press is its own write; one naming its target keeps its id for a retry to replay. - const namesTarget = structuredAgentSessionWriteNamesItsTarget(fingerprintMethod, fields) - const clientOperationId = - operationIdOverride ?? - (namesTarget ? operationIds.current.get(key) : undefined) ?? - structuredSessionOperationId() - if (namesTarget) { - operationIds.current.set(key, clientOperationId) - } let result: AgentSessionMutationResult try { result = await callStructuredAgentSession>(target, method, { envelope: { sessionId, - clientOperationId, + clientOperationId: structuredSessionOperationId(), expectedRuntimeFence: targetFence, payloadFingerprint: structuredAgentSessionPayloadFingerprint({ method: fingerprintMethod, @@ -138,15 +135,6 @@ export function useStructuredAgentSessionMutate(args: { : { kind: 'dropped' } } if (!result.ok) { - const operationState = agentSessionRefusalOperationState(result.refusal.code) - // Cancel's plan recovers no unknown ledger row, so a kept id would earn the same refusal - // until it expires, and Stop for that turn would do nothing. - if ( - operationState === 'settled-rejected' || - (operationState === 'unknown' && fingerprintMethod === 'agentSession.cancel') - ) { - operationIds.current.delete(key) - } return settlesHere() ? { kind: 'not-done', @@ -160,14 +148,49 @@ export function useStructuredAgentSessionMutate(args: { if (!settlesHere()) { return { kind: 'dropped' } } - if (!conversationCommands.isUnconfirmedConversationCommand(fingerprintMethod, result.value)) { - operationIds.current.delete(key) - } return { kind: 'done', value: result.value } }, [enabled, sessionId, stateRef, target] ) + const write = useCallback( + async ( + method: string, + fingerprintMethod: string, + fields: Record + ): Promise> => { + const pressedFence = stateRef.current.fence + if ( + !namesWhatItStops(fingerprintMethod, fields) || + (await supportsStructuredAgentSessionQuietRepeatedStop(target)) + ) { + return send(method, fingerprintMethod, fields, pressedFence) + } + // Temporary, for a host that predates the quiet repeated Stop: remove once none is supported. + const key = `${sessionId}:${method}:${JSON.stringify(fields)}` + const joined = inFlightStops.current.get(key) + if (joined) { + // The first press reports a failure; the one that joined it stays quiet. + return joined.then((outcome) => + outcome.kind === 'not-done' + ? { kind: 'dropped' } + : // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the key names the method and fields, so the joined promise is this very call and settles to its T. + (outcome as StructuredAgentSessionWriteOutcome) + ) + } + const stopping = send(method, fingerprintMethod, fields, pressedFence) + inFlightStops.current.set(key, stopping) + // Gone once it settles, so the next press is a new Stop. + void stopping.finally(() => { + if (inFlightStops.current.get(key) === stopping) { + inFlightStops.current.delete(key) + } + }) + return stopping + }, + [send, sessionId, stateRef, target] + ) + const mutate = useCallback( async (...writeArgs: WriteArgs): Promise => { const outcome = await write(...writeArgs) diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-prompt-cancel.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session-prompt-cancel.test.tsx index 3b782209a5e4..8e1259ad0d94 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session-prompt-cancel.test.tsx +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-prompt-cancel.test.tsx @@ -11,6 +11,7 @@ const mocks = vi.hoisted(() => ({ vi.mock('@/runtime/structured-agent-session-client', () => ({ callStructuredAgentSession: mocks.call, + supportsStructuredAgentSessionQuietRepeatedStop: vi.fn(async () => false), supportsStructuredAgentSessionPromptCancel: mocks.promptCancelSupported })) vi.mock('./use-structured-agent-session-read', () => ({ diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-provisional.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session-provisional.test.tsx index 2d6800695ce8..fd4445b68b60 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session-provisional.test.tsx +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-provisional.test.tsx @@ -16,7 +16,8 @@ const mocks = vi.hoisted(() => ({ let readState: StructuredAgentSessionState vi.mock('@/runtime/structured-agent-session-client', () => ({ - callStructuredAgentSession: mocks.call + callStructuredAgentSession: mocks.call, + supportsStructuredAgentSessionQuietRepeatedStop: vi.fn(async () => false) })) vi.mock('./use-structured-agent-session-hold', () => ({ diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-stop.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session-stop.test.tsx index c54f0f44d5f2..7dbd9ca3610e 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session-stop.test.tsx +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-stop.test.tsx @@ -24,6 +24,7 @@ let fence = 3 vi.mock('@/runtime/structured-agent-session-client', () => ({ callStructuredAgentSession: mocks.call, + supportsStructuredAgentSessionQuietRepeatedStop: vi.fn(async () => false), supportsStructuredAgentSessionPromptCancel: vi.fn(async () => false) })) @@ -327,28 +328,35 @@ describe('Stop against a host that stops the conversation', () => { expect(ids[1]).not.toBe(ids[0]) }) - it('replays a lost stop of one background task, which names what it stops', async () => { - const answers: (() => unknown)[] = [ - () => { + it('stops one background task on a second press after the first lost its answer', async () => { + const ran = new Set() + let lostAnswers = 1 + mocks.call.mockImplementation(async (_target, method, params) => { + if (method !== 'agentSession.cancel') { + return null + } + // As the host's ledger does: an id it already ran replays as handled and stops nothing. + const operationId: string = params.envelope.clientOperationId + const value = { cancelled: !ran.has(operationId) } + ran.add(operationId) + if (lostAnswers > 0) { + lostAnswers -= 1 throw new Error('the connection dropped before the host answered') } - ] - mocks.call.mockImplementation(async (_target, method) => - method === 'agentSession.cancel' - ? (answers.shift()?.() ?? { ok: true, value: { cancelled: true } }) - : null - ) + return { ok: true, value } + }) const { result } = render() - for (let press = 0; press < 2; press += 1) { - await act(async () => { - await result.current.stopBackgroundTask('task-1') - }) - } + let second: unknown + await act(async () => { + await result.current.stopBackgroundTask('task-1') + second = await result.current.stopBackgroundTask('task-1') + }) const ids = cancelOperationIds() expect(ids).toHaveLength(2) - expect(ids[1]).toBe(ids[0]) + expect(ids[1]).not.toBe(ids[0]) + expect(second).toEqual({ cancelled: true }) }) it('sends a new cancel of a named turn after the host could not settle the last one', async () => { diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-write-refusal.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session-write-refusal.test.tsx index 441b279868a5..034c2a361b3b 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session-write-refusal.test.tsx +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-write-refusal.test.tsx @@ -11,7 +11,8 @@ const mocks = vi.hoisted(() => ({ vi.mock('sonner', () => ({ toast: { error: mocks.toastError, message: vi.fn() } })) vi.mock('@/runtime/structured-agent-session-client', () => ({ - callStructuredAgentSession: mocks.call + callStructuredAgentSession: mocks.call, + supportsStructuredAgentSessionQuietRepeatedStop: vi.fn(async () => false) })) vi.mock('./use-structured-agent-session-read', () => ({ diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session.queued-gating.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session.queued-gating.test.tsx index 6d5a4f675b4e..ebfd23060bbf 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session.queued-gating.test.tsx +++ b/src/renderer/src/components/native-chat/use-structured-agent-session.queued-gating.test.tsx @@ -210,7 +210,8 @@ describe('against a capable host', () => { ) }) - it("an unconfirmed /clear's next press replays its operation id (the host refuses any other)", async () => { + // The host finds an earlier /clear from its own journal, so the client keeps no id for it. + it("an unconfirmed /clear's next press goes out under its own id", async () => { items = [] mocks.call.mockImplementation(async (_target, method) => method === 'agentSession.conversationCommand' @@ -234,7 +235,7 @@ describe('against a capable host', () => { .filter(([, method]) => method === 'agentSession.conversationCommand') .map(([, , params]) => ConversationCommandParams.parse(params).envelope.clientOperationId) expect(ids).toHaveLength(2) - expect(ids[1]).toBe(ids[0]) + expect(ids[1]).not.toBe(ids[0]) }) it('a mid-turn queue send is never a transcript bubble, before or after the host holds it', () => { diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session.test.tsx index 850407511d2d..d3f8fd9dc0fb 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session.test.tsx +++ b/src/renderer/src/components/native-chat/use-structured-agent-session.test.tsx @@ -17,7 +17,8 @@ let items: AgentJournalRenderItem[] = [] let submissions: AgentJournalSubmission[] = [] vi.mock('@/runtime/structured-agent-session-client', () => ({ - callStructuredAgentSession: mocks.call + callStructuredAgentSession: mocks.call, + supportsStructuredAgentSessionQuietRepeatedStop: vi.fn(async () => false) })) vi.mock('./native-chat-session-option-settings-write', () => ({ @@ -321,7 +322,7 @@ describe('useStructuredAgentSession options', () => { ).toEqual(['operation-1', 'operation-2']) }) - it('reuses an option operation after a pending admission refusal', async () => { + it('sends the pick again as a new operation after a pending admission refusal', async () => { let attempts = 0 mocks.call.mockImplementation((_target, method) => { if (method !== 'agentSession.setOption') { @@ -378,14 +379,13 @@ describe('useStructuredAgentSession options', () => { ([, , params]) => (params as { envelope: { clientOperationId: string } }).envelope.clientOperationId ) - ).toEqual(['operation-1', 'operation-1']) + ).toEqual(['operation-1', 'operation-2']) expect( mutations.map( ([, , params]) => (params as { envelope: { expectedRuntimeFence: number } }).envelope.expectedRuntimeFence ) ).toEqual([3, 4]) - expect(mocks.operationId).toHaveBeenCalledTimes(1) }) it('ignores an option failure from a superseded fence', async () => { diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session.ts b/src/renderer/src/components/native-chat/use-structured-agent-session.ts index d63a5263baa3..96ad805560d9 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session.ts +++ b/src/renderer/src/components/native-chat/use-structured-agent-session.ts @@ -243,8 +243,8 @@ export function useStructuredAgentSession(args: { }, queuedMessages: queuedController, cancel: async (turnId: string, prompt?: StructuredPromptCancelTarget) => { - // Capability negotiation must complete before mutate constructs the payload - // fingerprint and operation id: older hosts reject the strict prompt field. + // Capability negotiation must complete before mutate fingerprints the payload: + // older hosts reject the strict prompt field. const promptSupported = prompt !== undefined && (await supportsStructuredAgentSessionPromptCancel(target)) return mutate('agentSession.cancel', 'agentSession.cancel', { diff --git a/src/renderer/src/runtime/structured-agent-session-client.ts b/src/renderer/src/runtime/structured-agent-session-client.ts index d50fed2b4c33..634781b9388f 100644 --- a/src/renderer/src/runtime/structured-agent-session-client.ts +++ b/src/renderer/src/runtime/structured-agent-session-client.ts @@ -11,6 +11,7 @@ import { AGENT_SESSION_PROMPT_CANCEL_RUNTIME_CAPABILITY, AGENT_SESSION_QUESTION_ANSWERS_RUNTIME_CAPABILITY, AGENT_SESSION_REWIND_RUNTIME_CAPABILITY, + AGENT_SESSION_REPEATED_STOP_RUNTIME_CAPABILITY, type RuntimeCapability } from '../../../shared/protocol-version' import { @@ -47,6 +48,13 @@ export function supportsStructuredAgentSessionPromptCancel( return structuredAgentSessionHostSupports(target, AGENT_SESSION_PROMPT_CANCEL_RUNTIME_CAPABILITY) } +/** Whether the host writes no row for a Stop that stopped nothing, so a repeated Stop is quiet. */ +export function supportsStructuredAgentSessionQuietRepeatedStop( + target: RuntimeClientTarget +): Promise { + return structuredAgentSessionHostSupports(target, AGENT_SESSION_REPEATED_STOP_RUNTIME_CAPABILITY) +} + export function supportsStructuredAgentSessionQuestionAnswers( target: RuntimeClientTarget ): Promise { diff --git a/src/shared/agent-session-stop-capabilities.ts b/src/shared/agent-session-stop-capabilities.ts new file mode 100644 index 000000000000..c113161f96d8 --- /dev/null +++ b/src/shared/agent-session-stop-capabilities.ts @@ -0,0 +1,22 @@ +// The capabilities that gate what an `agentSession.cancel` may carry and what a Stop writes. + +// Why: `agentSession.cancel` params are strict and older hosts require `turnId`. A host advertising +// this takes a cancel naming no turn as "stop what the conversation has in flight", which is the +// only Stop a client can send before the provider has opened a turn. +export const AGENT_SESSION_CONVERSATION_STOP_RUNTIME_CAPABILITY = + 'agent-session.conversation-stop.v1' as const +// Why: agentSession.cancel has a strict schema, so clients must not send prompt identity to an +// older host that would reject the whole cancellation instead of falling back to turn stop. +export const AGENT_SESSION_PROMPT_CANCEL_RUNTIME_CAPABILITY = + 'agent-session.prompt-cancel.v1' as const +// Why: a Stop that stopped nothing adds no row on this host, and its note is keyed by turn. An older +// host has the second write a false "already finished" row, so a client joins a Stop still on its +// way itself there. Transitional: drop the client join once no supported host lacks this. +export const AGENT_SESSION_REPEATED_STOP_RUNTIME_CAPABILITY = + 'agent-session.repeated-stop.v1' as const + +export const AGENT_SESSION_STOP_RUNTIME_CAPABILITIES = [ + AGENT_SESSION_CONVERSATION_STOP_RUNTIME_CAPABILITY, + AGENT_SESSION_PROMPT_CANCEL_RUNTIME_CAPABILITY, + AGENT_SESSION_REPEATED_STOP_RUNTIME_CAPABILITY +] as const diff --git a/src/shared/protocol-version.ts b/src/shared/protocol-version.ts index 55c99feed2fb..4d0c0ca6976b 100644 --- a/src/shared/protocol-version.ts +++ b/src/shared/protocol-version.ts @@ -1,4 +1,5 @@ import { AGENT_SESSION_BACKGROUND_TASK_CHILD_VIEWS_CAPABILITY } from './agent-session-background-task-child-views-capability' +import { AGENT_SESSION_STOP_RUNTIME_CAPABILITIES } from './agent-session-stop-capabilities' import { REMOTE_SERVER_UPDATE_CAPABILITY } from './remote-server-update' import { SKILL_BUNDLE_INSTALL_CAPABILITY, @@ -152,6 +153,7 @@ export const SESSION_TABS_RETIREMENT_PROOF_DELTA_RUNTIME_CAPABILITY = export const AGENT_SESSION_BOUNDARY_RUNTIME_CAPABILITY = 'agent-session.session-boundary.v1' as const export { REMOTE_SERVER_UPDATE_CAPABILITY } from './remote-server-update' +export * from './agent-session-stop-capabilities' export const AGENT_SESSION_HOST_AUTHORITY_RUNTIME_CAPABILITY = 'agent-session.host-authority.v1' as const // Older launch schemas reject unknown fields; advertise before clients send keyboard support. @@ -172,11 +174,6 @@ export const AGENT_SESSION_PENDING_SEND_RESULT_RUNTIME_CAPABILITY = // mobile client lacks the capability; mobile must first show a rejected message in place. export const AGENT_SESSION_ACCEPTED_SEND_RUNTIME_CAPABILITY = 'agent-session.accepted-send.v1' as const -// Why: `agentSession.cancel` params are strict and older hosts require `turnId`. A host advertising -// this takes a cancel naming no turn as "stop what the conversation has in flight", which is the -// only Stop a client can send before the provider has opened a turn. -export const AGENT_SESSION_CONVERSATION_STOP_RUNTIME_CAPABILITY = - 'agent-session.conversation-stop.v1' as const // Why: `agentSession.send`'s params are strict, so an older host rejects `delivery`; and only a // capable client can render the `queued` result arm, the draft list, and returned cards. DARK ON // PURPOSE — not in RUNTIME_CAPABILITIES: advertising still requires the integrated Codex steer @@ -230,10 +227,6 @@ export const AGENT_SESSION_REWIND_RUNTIME_CAPABILITY = 'agent-session.rewind.v1' export const AGENT_SESSION_TURN_ITEM_CAPABILITY = 'agent-session.turn-item.v1' as const export const AGENT_SESSION_BACKGROUND_TASK_STOP_CAPABILITY = 'agent-session.background-task-stop.v1' as const -// Why: agentSession.cancel has a strict schema, so clients must not send prompt identity to an -// older host that would reject the whole cancellation instead of falling back to turn stop. -export const AGENT_SESSION_PROMPT_CANCEL_RUNTIME_CAPABILITY = - 'agent-session.prompt-cancel.v1' as const // Why: agentSession.respondToQuestion has a strict schema, so clients must not send structured // `answers` to an older host; they fall back to the answer packed into `optionId`. export const AGENT_SESSION_QUESTION_ANSWERS_RUNTIME_CAPABILITY = @@ -353,6 +346,7 @@ export const AGENT_SESSION_CREATE_TAB_ID_RUNTIME_CAPABILITY = 'agentSession.create.tab-id.v1' as const export const RUNTIME_CAPABILITIES = [ + ...AGENT_SESSION_STOP_RUNTIME_CAPABILITIES, AGENT_SESSION_CREATE_TAB_ID_RUNTIME_CAPABILITY, ANTIGRAVITY_CONFIGURED_MODEL_RUNTIME_CAPABILITY, 'files.pathsExist', @@ -411,7 +405,6 @@ export const RUNTIME_CAPABILITIES = [ // The host side: it accepts a send before any agent has it, and a Stop with no writer before a // turn starts, so a client may gate on either. AGENT_SESSION_ACCEPTED_SEND_RUNTIME_CAPABILITY, - AGENT_SESSION_CONVERSATION_STOP_RUNTIME_CAPABILITY, STRUCTURED_AGENT_SESSION_HOLD_RUNTIME_CAPABILITY, STRUCTURED_AGENT_SESSION_REVEAL_RUNTIME_CAPABILITY, STRUCTURED_AGENT_SESSION_RESUME_HISTORY_RUNTIME_CAPABILITY, @@ -420,7 +413,6 @@ export const RUNTIME_CAPABILITIES = [ AGENT_SESSION_REWIND_RUNTIME_CAPABILITY, AGENT_SESSION_CONVERSATION_OUTLINE_RUNTIME_CAPABILITY, AGENT_SESSION_BACKGROUND_TASK_STOP_CAPABILITY, - AGENT_SESSION_PROMPT_CANCEL_RUNTIME_CAPABILITY, AGENT_SESSION_QUESTION_ANSWERS_RUNTIME_CAPABILITY, AGENT_SESSION_TURN_ITEM_CAPABILITY, AGENT_SESSION_BACKGROUND_TASK_ROW_STOP_CAPABILITY, diff --git a/src/shared/structured-agent-session-operation-identity.test.ts b/src/shared/structured-agent-session-operation-identity.test.ts deleted file mode 100644 index f0b05ed86d3a..000000000000 --- a/src/shared/structured-agent-session-operation-identity.test.ts +++ /dev/null @@ -1,39 +0,0 @@ -import { describe, expect, it } from 'vitest' -import { structuredAgentSessionWriteNamesItsTarget } from './structured-agent-session-operation-identity' - -describe('structuredAgentSessionWriteNamesItsTarget', () => { - it('holds for a cancel naming its turn, prompt or task, as every mobile cancel does', () => { - expect(structuredAgentSessionWriteNamesItsTarget('agentSession.cancel', { turnId: 't' })).toBe( - true - ) - expect( - structuredAgentSessionWriteNamesItsTarget('agentSession.cancel', { - turnId: 't', - prompt: { itemId: 'i', expectedRevision: 1 } - }) - ).toBe(true) - expect( - structuredAgentSessionWriteNamesItsTarget('agentSession.cancel', { - turnId: 'background-tasks', - scope: 'background-tasks', - taskId: 'task-1' - }) - ).toBe(true) - }) - - it('fails for a Stop naming no turn, and for a stop of every background task', () => { - expect(structuredAgentSessionWriteNamesItsTarget('agentSession.cancel', {})).toBe(false) - expect( - structuredAgentSessionWriteNamesItsTarget('agentSession.cancel', { - turnId: 'background-tasks', - scope: 'background-tasks' - }) - ).toBe(false) - }) - - it('leaves every other write as it was', () => { - expect( - structuredAgentSessionWriteNamesItsTarget('agentSession.setOption', { key: 'k', value: 'v' }) - ).toBe(true) - }) -}) diff --git a/src/shared/structured-agent-session-operation-identity.ts b/src/shared/structured-agent-session-operation-identity.ts deleted file mode 100644 index 519c917bcafd..000000000000 --- a/src/shared/structured-agent-session-operation-identity.ts +++ /dev/null @@ -1,20 +0,0 @@ -/** - * Whether a structured-session write names what it acts on (a turn, a prompt, a task, a draft), so - * another with the same payload is the same intent and may reuse its operation id to replay. A Stop - * naming no turn, one stopping every background task, or a queue Resume acts on whatever holds when - * the host reaches it: its payload is shared with every later one, so each press gets its own id. - */ -export function structuredAgentSessionWriteNamesItsTarget( - fingerprintMethod: string, - fields: Readonly> -): boolean { - if (fingerprintMethod === 'agentSession.queuedMessagesResume') { - return false - } - if (fingerprintMethod !== 'agentSession.cancel') { - return true - } - return fields.scope === 'background-tasks' - ? fields.taskId !== undefined - : fields.turnId !== undefined -}