From 0c7cdef30504ebe5066040dc41e1e628c9b85089 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 29 Sep 2026 12:09:13 -0700 Subject: [PATCH 01/17] fix(native-chat): give every chat write press its own operation id A client kept an operation id per method and payload across presses, so a later identical press replayed an earlier action instead of running: an option picked again stayed on the other one, a goal set again stayed cleared, and a second Stop or phone Stop did nothing. Every press now mints its own id, on desktop and phone, and no client keeps a retry id. The host answers a harmless repeat from what the chat records: the same prompt answer again returns the resolution it holds, and a Cancel of a prompt already cancelled answers ok. A second Stop of the same turn while the first is on its way joins it. --- .../mobile-structured-agent-session-cancel.ts | 60 ++-- .../mobile-structured-agent-session-rpc.ts | 49 +--- ...mobile-structured-composer-command.test.ts | 63 ++-- .../mobile-structured-composer-command.ts | 17 +- ...structured-operation-id-retirement.test.ts | 94 ++++-- ...ctured-session-operation-retention.test.ts | 79 ----- .../use-mobile-structured-agent-session.ts | 28 +- ...tured-agent-session-grouped-prompt.test.ts | 53 ++++ .../structured-agent-session-host.test.ts | 62 +++- ...ctured-agent-session-prompt-cancel.test.ts | 71 +++++ .../structured-agent-session-prompt-state.ts | 10 + ...uctured-agent-session-repeat-press.test.ts | 209 +++++++++++++ .../structured-agent-session-turns-cancel.ts | 13 +- .../structured-agent-session-turns-prompt.ts | 22 +- .../structured-conversation-command-send.ts | 7 - ...e-structured-agent-session-mutate.test.tsx | 277 ++++++++++++++++++ .../use-structured-agent-session-mutate.ts | 99 ++++--- ...use-structured-agent-session-stop.test.tsx | 37 ++- .../use-structured-agent-session.test.tsx | 5 +- .../use-structured-agent-session.ts | 4 +- ...d-agent-session-operation-identity.test.ts | 39 --- ...ctured-agent-session-operation-identity.ts | 17 -- 22 files changed, 934 insertions(+), 381 deletions(-) delete mode 100644 mobile/src/session/mobile-structured-session-operation-retention.test.ts create mode 100644 src/main/native-chat/agent-session-wire/structured-agent-session-repeat-press.test.ts create mode 100644 src/renderer/src/components/native-chat/use-structured-agent-session-mutate.test.tsx delete mode 100644 src/shared/structured-agent-session-operation-identity.test.ts delete mode 100644 src/shared/structured-agent-session-operation-identity.ts diff --git a/mobile/src/session/mobile-structured-agent-session-cancel.ts b/mobile/src/session/mobile-structured-agent-session-cancel.ts index 4666cad48175..e00dc69f5ecf 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,66 @@ export async function requestMobileStructuredAgentSessionCancel(args: { sessionId: string | null enabled: boolean stateRef: { readonly current: StructuredAgentSessionState } - sessionKey: string - operationIds: Map promptCancelSupported: boolean | null prompt?: PromptIdentity + /** Stops still on their way, by what they stop; a press for the same one joins it. */ + 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) - ) + const key = `${sessionId}:agentSession.cancel:${JSON.stringify(fields)}` + const joined = inFlight.get(key) + if (joined) { + return joined + } + // Every press is its own Stop: a kept id would be answered from the last one and stop nothing. + const stopping = sendStop({ client, sessionId, fence: current.fence, fields, onSendError }) + 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-rpc.ts b/mobile/src/session/mobile-structured-agent-session-rpc.ts index bca14e293f75..e6e362f3b37a 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 @@ -31,11 +26,7 @@ export type StructuredAgentSessionMutationCallResult = | { status: 'accepted'; value: TValue } | { status: 'refused'; code: AgentSessionWireRefusalCode; message: string } | { status: 'failed'; message: string } - /** `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 } @@ -75,42 +66,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 @@ -172,7 +127,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 d37e337f0913..776f599e229a 100644 --- a/mobile/src/session/mobile-structured-composer-command.test.ts +++ b/mobile/src/session/mobile-structured-composer-command.test.ts @@ -3,19 +3,19 @@ import type { RpcClient } from '../transport/rpc-client' import { dispatchMobileStructuredCommand } from './mobile-structured-composer-command' function setup() { - const sendRequest = vi.fn(async (_method: string, _params: unknown, _options: unknown) => ({ - ok: true, - result: { ok: true, value: { command: 'compact', state: 'completed' } } - })) + const sendRequest = vi.fn( + async (_method: string, _params: unknown, _options: unknown): Promise => ({ + ok: true, + result: { ok: true, value: { command: 'compact', state: 'completed' } } + }) + ) const input: Parameters[0] = { text: '/compact', hasAttachments: false, client: { sendRequest } as unknown as RpcClient, sessionId: 'session', fence: 1, - sessionKey: 'session:1', pending: { current: false }, - operationIds: new Map(), controller: { agent: 'codex', snapshot: [], @@ -40,41 +40,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..9afb3e431eec 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,20 @@ function runningState(): StructuredAgentSessionState { return state as unknown as StructuredAgentSessionState } -function cancelArgs(client: RpcClient, operationIds: Map) { +function cancelArgs(client: RpcClient) { return { client, sessionId: 'session-1', enabled: true, stateRef: { current: runningState() }, - sessionKey: 'key-1', - operationIds, promptCancelSupported: null, + 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 +73,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 +100,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 +108,81 @@ 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('joins the Stop of that turn still on its way instead of sending a second', 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) }) }) 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-session.ts b/mobile/src/session/use-mobile-structured-agent-session.ts index 2fa5c2b2a96b..37d79e40b6b8 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 { dispatchMobileStructuredCommand } from './mobile-structured-composer-command' import { structuredAgentSessionSendBody, @@ -21,7 +21,6 @@ import { } from './mobile-structured-agent-prompts' import { requestStructuredAgentSessionMutation, - retainStructuredSessionOperationId as retainStructuredOpId, timeoutForDeadline, type StructuredAgentSessionMutationResult } from './mobile-structured-agent-session-rpc' @@ -96,9 +95,9 @@ export function useMobileStructuredAgentSession(args: { } = args const promptCancelSupported = hostSupport?.promptCancel ?? null const sessionKey = encodeNativeChatTranscriptIdentity([sourceIdentity, agent, sessionId]) - const operationIdsRef = useRef(new Map()) const commandPendingRef = useRef(false) - useEffect(() => () => operationIdsRef.current.clear(), []) + // A Stop of the turn one is already stopping joins it. + const inFlightStopsRef = useRef(new Map>()) const stateArgs = { client, sessionId, sessionKey, enabled, connected } const { state, stateRef, loadingOlder, loadEarlier } = useMobileStructuredAgentState(stateArgs) useMobileStructuredSendOperationReconciliation(state.submissions) @@ -114,23 +113,15 @@ export function useMobileStructuredAgentSession(args: { return { status: 'rejected' } } const targetFence = current.fence - const key = `${sessionKey}:${fingerprintMethod}:${JSON.stringify(fields)}` - const clientOperationId = retainStructuredOpId( - operationIdsRef.current, - key, - operationIdsRef.current.get(key) - ) const result = await requestStructuredAgentSessionMutation({ client, method, fingerprintMethod, sessionId, expectedRuntimeFence: targetFence, - fields, - clientOperationId + fields }) if (result.status === 'accepted') { - operationIdsRef.current.delete(key) return { status: 'accepted', value: result.value, @@ -138,14 +129,12 @@ export function useMobileStructuredAgentSession(args: { } } if (result.status === 'unknown') { - operationIdsRef.current.delete(key) return result } - operationIdsRef.current.delete(key) onSendError(result.message) return { status: 'rejected' } }, - [client, enabled, onSendError, sessionId, sessionKey] + [client, enabled, onSendError, sessionId] ) const options = useMobileStructuredAgentOptions({ @@ -187,9 +176,7 @@ export function useMobileStructuredAgentSession(args: { client, sessionId, fence: currentFence, - sessionKey, pending: commandPendingRef, - operationIds: operationIdsRef.current, controller: { agent: agent === 'claude' ? 'claude' : 'codex', snapshot: optionSnapshot, @@ -251,15 +238,14 @@ export function useMobileStructuredAgentSession(args: { requestMobileStructuredAgentSessionCancel({ client, enabled, + inFlight: inFlightStopsRef.current, onSendError, - operationIds: operationIdsRef.current, prompt, promptCancelSupported, sessionId, - sessionKey, stateRef }), - [client, enabled, onSendError, promptCancelSupported, sessionId, sessionKey, stateRef] + [client, enabled, onSendError, promptCancelSupported, sessionId, stateRef] ) const messages = useMemo( 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 72317e8ed814..3d33d461627c 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 @@ -221,3 +221,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 5db9e5d0d742..11e064371117 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 @@ -270,6 +270,50 @@ describe('cancel', () => { expect(cancelTurn).not.toHaveBeenCalled() }) + 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() + }) + it('refuses cancellation after an answer has already resolved the prompt', async () => { await attach() const prompt = await seedApproval() @@ -412,16 +456,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), @@ -429,12 +474,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.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-prompt-cancel.test.ts index e0ae5da9d812..c62a3c651537 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 @@ -144,6 +144,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 } + ) + }) + 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 } + ) + 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) 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..2b41233ddcb8 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 @@ -19,6 +19,16 @@ 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 +} + export function validatePendingPrompt( ctx: Pick, input: { 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..fdae6dda068c --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-repeat-press.test.ts @@ -0,0 +1,209 @@ +// 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 { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' +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 AgentSessionRecordStore.open({ directory: join(root, 'store'), hostId: 'local' }) + 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 + }, + journalRoot: 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) + + // 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-turns-cancel.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-turns-cancel.ts index 4bd5b412300f..d2ff41ee9d9a 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 @@ -9,7 +9,7 @@ import { structuredAgentSessionCommandWasStopped, structuredAgentSessionStopNoteIdentity } from './structured-agent-session-command-turn' -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 async function performCancel( @@ -28,7 +28,16 @@ export async function performCancel( if (input.prompt) { const validated = validatePendingPrompt(ctx, input.prompt) if (!validated.ok) { - return validated + // Another Cancel of a prompt already cancelled: nothing is left to do. + return settledPrompt(ctx, input.prompt.itemId)?.prompt.resolution.state === 'cancelled' + ? { + ok: true, + value: { + ...(input.turnId !== undefined ? { turnId: input.turnId } : {}), + cancelled: false + } + } + : validated } } let cancelled = false 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/renderer/src/components/native-chat/structured-conversation-command-send.ts b/src/renderer/src/components/native-chat/structured-conversation-command-send.ts index 51575e4ae03a..44e1fbbb3b21 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 @@ -44,10 +44,3 @@ export async function sendStructuredConversationCommand(input: { input.pending.current = false } } - -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-mutate.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session-mutate.test.tsx new file mode 100644 index 000000000000..ff7c438c6f9f --- /dev/null +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-mutate.test.tsx @@ -0,0 +1,277 @@ +// @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(), + 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 +})) + +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.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]) + }) +}) + +describe('a Stop pressed again', () => { + 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, answerAll: () => answers.splice(0).forEach((answer) => answer()) } + } + const stop = (current: ReturnType['current'], turnId = 'turn-1') => + current.mutate('agentSession.cancel', 'agentSession.cancel', { turnId }) + + 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 () => { + host.answerAll() + 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)] + 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) + host.answerAll() + 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', {}) + ] + host.answerAll() + 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')] + host.answerAll() + await Promise.all(pressed) + }) + + expect(mocks.call).toHaveBeenCalledTimes(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 57b7acdaf093..062a5f1a7631 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,24 +1,20 @@ // 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. 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. A Stop of a named turn or task pressed while an identical one is still in +// flight joins it instead of stopping twice. Every result is discarded unless the runtime fence it +// was issued against is still the current one. 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' @@ -32,12 +28,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 @@ -46,6 +37,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 @@ -58,41 +60,31 @@ 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) useEffect(() => { // Why: update the gate after commit so render stays free of ref mutations. enabledRef.current = enabled }, [enabled]) - const write = useCallback( + const send = useCallback( async ( method: string, fingerprintMethod: string, - fields: Record, - operationIdOverride?: string | null + fields: Record ): Promise> => { if (!enabled || !enabledRef.current || stateRef.current.fence === null) { return { kind: 'dropped' } } const targetFence = stateRef.current.fence - 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, @@ -117,15 +109,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 enabledRef.current && stateRef.current.fence === targetFence ? { kind: 'not-done', @@ -139,14 +122,44 @@ export function useStructuredAgentSessionMutate(args: { if (!enabledRef.current || stateRef.current.fence !== targetFence) { 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( + ( + method: string, + fingerprintMethod: string, + fields: Record + ): Promise> => { + if (!namesWhatItStops(fingerprintMethod, fields)) { + return send(method, fingerprintMethod, fields) + } + 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) + 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] + ) + 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-stop.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session-stop.test.tsx index 5d517bf57bb9..e2050b7077fc 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 @@ -330,28 +330,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.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session.test.tsx index 719fa29be616..797282ba9c1f 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 @@ -322,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') { @@ -379,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 ce66d47cbce9..d79e015aeded 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 @@ -193,8 +193,8 @@ export function useStructuredAgentSession(args: { : Promise.resolve(null) }, 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/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 bae28095695f..000000000000 --- a/src/shared/structured-agent-session-operation-identity.ts +++ /dev/null @@ -1,17 +0,0 @@ -/** - * Whether a structured-session cancel names what it stops (a turn, a prompt, a task), so another - * cancel with the same payload is the same intent and may reuse its operation id to replay. A Stop - * naming no turn, or one stopping every background task, acts on whatever is in flight 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.cancel') { - return true - } - return fields.scope === 'background-tasks' - ? fields.taskId !== undefined - : fields.turnId !== undefined -} From 869b03f2b2e7b5df836cda302908501142b13aea Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 29 Sep 2026 12:24:14 -0700 Subject: [PATCH 02/17] fix(native-chat): answer a /clear pressed again after it committed with that clear Every press now carries its own operation id, so a /clear that reaches the host after this caller's /clear already committed (a double press, or a retype after a lost answer) no longer replays the first id. It started the cleared conversation's agent and then refused it with "This conversation has been cleared." The host now answers such a /clear from the committed record: the same caller, on a conversation whose tab moved to its replacement, gets that clear's result, replacement included, before admission starts anything. No second clear runs. Another window, or a cleared conversation reopened from history, still reads "cleared". --- ...ructured-conversation-command-admission.ts | 21 ++++++ .../structured-conversation-command.test.ts | 75 +++++++++++++++++++ .../structured-conversation-command.ts | 49 +++++++++++- .../methods/structured-chat-tab-table.test.ts | 39 ++++++++++ 4 files changed, 180 insertions(+), 4 deletions(-) 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 937f63733c5e..4a34531a8446 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 @@ -1,3 +1,4 @@ +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 { activeStructuredAgentSessionTurnId } from '../../../shared/structured-agent-session-projection' @@ -15,6 +16,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 c1c81dda62a2..80b8fc330342 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 @@ -670,3 +670,78 @@ 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) + 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('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 b2a9700a39f6..7951faed4ecc 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, @@ -137,5 +178,5 @@ export function runStructuredConversationCommand( } } }) - ) + }) } 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 7e7872d78c47..616327de93eb 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 @@ -220,6 +220,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) From 85fa9bb2de2ad0712fd24c7c34e5f28200750cdb Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 29 Sep 2026 12:27:13 -0700 Subject: [PATCH 03/17] test(native-chat): scope the prompt-cancel tests' journal rows to the thread --- .../structured-agent-session-prompt-cancel.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 c62a3c651537..fb8d9ee862cc 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,7 +163,7 @@ describe('performCancel for a pending prompt', () => { resolvedAt: null } }, - { fence: 1 } + { fence: 1, turnScope: AGENT_JOURNAL_THREAD_SCOPE } ) }) const ctx = context(journal, cancelTurn, flush) @@ -198,7 +198,7 @@ describe('performCancel for a pending prompt', () => { ...current.body, resolution: { state: 'resolved', selectedOptionId: 'allow', resolvedBy: 'c', resolvedAt: 1 } }, - { fence: 1 } + { fence: 1, turnScope: AGENT_JOURNAL_THREAD_SCOPE } ) const cancelTurn = vi.fn(async () => ({ cancelled: true })) From 19ae46369e7b0a923102e58772e2f450182667eb Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 29 Sep 2026 15:22:03 -0700 Subject: [PATCH 04/17] test(native-chat): give the repeated prompt Cancel its own host test file --- .../structured-agent-session-host.test.ts | 44 ------------- ...agent-session-prompt-cancel-repeat.test.ts | 66 +++++++++++++++++++ 2 files changed, 66 insertions(+), 44 deletions(-) create mode 100644 src/main/native-chat/agent-session-wire/structured-agent-session-prompt-cancel-repeat.test.ts 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 11e064371117..3dca4a38b72f 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 @@ -270,50 +270,6 @@ describe('cancel', () => { expect(cancelTurn).not.toHaveBeenCalled() }) - 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() - }) - it('refuses cancellation after an answer has already resolved the prompt', async () => { await attach() const prompt = await seedApproval() 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() + }) +}) From b044e60b309f92a0e39426c119a50ae67550b8a4 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 29 Sep 2026 15:32:18 -0700 Subject: [PATCH 05/17] chore(reliability): drop the deleted mobile id-retention test from the gates that ran it --- config/reliability-gates.jsonc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/config/reliability-gates.jsonc b/config/reliability-gates.jsonc index 3f210c238238..0b062d542da1 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." From f98f6aa45b95479d91c41abcb026b2fe85f68e3e Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 30 Sep 2026 00:33:19 -0700 Subject: [PATCH 06/17] test(native-chat): a repeated draft press under its own id is answered by the draft's state --- ...e-structured-agent-session-queued.test.tsx | 26 +++++++ ...-agent-session-queued-repeat-press.test.ts | 72 +++++++++++++++++++ 2 files changed, 98 insertions(+) create mode 100644 src/main/native-chat/agent-session-wire/structured-agent-session-queued-repeat-press.test.ts 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..03ac65acdcca 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 @@ -605,6 +605,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/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() + }) +}) From 5f67247dfcf74b38e8d98f788fbc0971078cf3ad Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 30 Sep 2026 01:02:16 -0700 Subject: [PATCH 07/17] fix(native-chat): the host joins a second Stop of a turn it is still stopping A client joins it itself only against a host older than this, which runs both and writes a false 'already finished' row for the second. --- .../mobile-structured-agent-session-cancel.ts | 14 ++- ...uctured-agent-session-host-support.test.ts | 35 +++++- ...e-structured-agent-session-host-support.ts | 8 +- ...structured-operation-id-retirement.test.ts | 21 +++- ...tured-agent-session-prompt-cancel.test.tsx | 3 +- ...tured-agent-session-queued.test-fixture.ts | 3 +- .../use-mobile-structured-agent-session.ts | 6 +- ...structured-agent-session-host-mutations.ts | 118 +++++++----------- ...ructured-agent-session-mutation-context.ts | 63 ++++++++++ ...structured-agent-session-stop-join.test.ts | 113 +++++++++++++++++ .../structured-agent-session-stop-join.ts | 61 +++++++++ ...e-structured-agent-session-mutate.test.tsx | 64 +++++++--- .../use-structured-agent-session-mutate.ts | 24 ++-- .../structured-agent-session-client.ts | 8 ++ src/shared/protocol-version.ts | 5 + 15 files changed, 434 insertions(+), 112 deletions(-) create mode 100644 src/main/native-chat/agent-session-wire/structured-agent-session-mutation-context.ts create mode 100644 src/main/native-chat/agent-session-wire/structured-agent-session-stop-join.test.ts create mode 100644 src/main/native-chat/agent-session-wire/structured-agent-session-stop-join.ts diff --git a/mobile/src/session/mobile-structured-agent-session-cancel.ts b/mobile/src/session/mobile-structured-agent-session-cancel.ts index e00dc69f5ecf..aa77d2e58f47 100644 --- a/mobile/src/session/mobile-structured-agent-session-cancel.ts +++ b/mobile/src/session/mobile-structured-agent-session-cancel.ts @@ -28,7 +28,10 @@ export async function requestMobileStructuredAgentSessionCancel(args: { stateRef: { readonly current: StructuredAgentSessionState } promptCancelSupported: boolean | null prompt?: PromptIdentity - /** Stops still on their way, by what they stop; a press for the same one joins it. */ + /** Whether the host joins a second Stop itself; null until the status probe answers. */ + hostJoinsStops: boolean | null + /** Stops still on their way, by what they stop; against a host that does not join them, a + * press for the same one joins it here. */ inFlight: Map> onSendError: (message: string) => void }): Promise { @@ -44,13 +47,18 @@ export async function requestMobileStructuredAgentSessionCancel(args: { turnId, ...(args.prompt && args.promptCancelSupported === true ? { prompt: args.prompt } : {}) } + // Every press is its own Stop: a kept id would be answered from the last one and stop nothing. + const stop = () => sendStop({ client, sessionId, fence: current.fence, fields, onSendError }) + if (args.hostJoinsStops === true) { + return stop() + } + // Temporary, for a host older than its Stop join: remove once every supported host has it. const key = `${sessionId}:agentSession.cancel:${JSON.stringify(fields)}` const joined = inFlight.get(key) if (joined) { return joined } - // Every press is its own Stop: a kept id would be answered from the last one and stop nothing. - const stopping = sendStop({ client, sessionId, fence: current.fence, fields, onSendError }) + const stopping = stop() inFlight.set(key, stopping) // Gone once it settles, so the next press is a new Stop. void stopping.finally(() => { 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..0d679a876af1 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_STOP_JOIN_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, + stopJoin: false }) expect( structuredAgentSessionHostSupport([AGENT_SESSION_QUESTION_ANSWERS_RUNTIME_CAPABILITY]) - ).toEqual({ promptCancel: false, questionAnswers: true, queuedMessages: false }) + ).toEqual({ + promptCancel: false, + questionAnswers: true, + queuedMessages: false, + stopJoin: false + }) expect( structuredAgentSessionHostSupport([AGENT_SESSION_PROMPT_CANCEL_RUNTIME_CAPABILITY]) - ).toEqual({ promptCancel: true, questionAnswers: false, queuedMessages: false }) + ).toEqual({ + promptCancel: true, + questionAnswers: false, + queuedMessages: false, + stopJoin: false + }) expect( structuredAgentSessionHostSupport([AGENT_SESSION_QUEUED_MESSAGES_RUNTIME_CAPABILITY]) - ).toEqual({ promptCancel: false, questionAnswers: false, queuedMessages: true }) + ).toEqual({ + promptCancel: false, + questionAnswers: false, + queuedMessages: true, + stopJoin: false + }) + expect(structuredAgentSessionHostSupport([AGENT_SESSION_STOP_JOIN_RUNTIME_CAPABILITY])).toEqual( + { + promptCancel: false, + questionAnswers: false, + queuedMessages: false, + stopJoin: 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..ebdb02a98036 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_STOP_JOIN_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 + /** The host joins a second Stop of a turn it is still stopping. */ + stopJoin: 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), + stopJoin: capabilities.includes(AGENT_SESSION_STOP_JOIN_RUNTIME_CAPABILITY) } } 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 9afb3e431eec..aaf6be77116b 100644 --- a/mobile/src/session/mobile-structured-operation-id-retirement.test.ts +++ b/mobile/src/session/mobile-structured-operation-id-retirement.test.ts @@ -49,13 +49,14 @@ function runningState(): StructuredAgentSessionState { return state as unknown as StructuredAgentSessionState } -function cancelArgs(client: RpcClient) { +function cancelArgs(client: RpcClient, hostJoinsStops: boolean | null = false) { return { client, sessionId: 'session-1', enabled: true, stateRef: { current: runningState() }, promptCancelSupported: null, + hostJoinsStops, inFlight: new Map>(), onSendError: vi.fn() } @@ -156,7 +157,7 @@ describe('a phone Stop pressed again', () => { return { client, sent, answerAll: () => answers.splice(0).forEach((answer) => answer()) } } - it('joins the Stop of that turn still on its way instead of sending a second', async () => { + it('against a host older than its Stop join, joins the Stop of that turn still on its way', async () => { const host = heldStops() const args = cancelArgs(host.client) @@ -184,6 +185,22 @@ describe('a phone Stop pressed again', () => { expect(host.sent[1]).not.toBe(host.sent[0]) expect(args.inFlight.size).toBe(0) }) + + it('against a host that joins Stops, 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) + }) }) describe('what a structured refusal says on the phone', () => { 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..27659811f856 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, + stopJoin: 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..64cc7bd03af3 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, + stopJoin: false } export const LEGACY: StructuredAgentSessionHostSupport = { ...CAPABLE, queuedMessages: false } diff --git a/mobile/src/session/use-mobile-structured-agent-session.ts b/mobile/src/session/use-mobile-structured-agent-session.ts index bdbe45ebd2b7..4cfd6dad8dd7 100644 --- a/mobile/src/session/use-mobile-structured-agent-session.ts +++ b/mobile/src/session/use-mobile-structured-agent-session.ts @@ -97,9 +97,10 @@ 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 hostJoinsStops = hostSupport?.stopJoin ?? null const sessionKey = encodeNativeChatTranscriptIdentity([sourceIdentity, agent, sessionId]) const commandPendingRef = useRef(false) - // A Stop of the turn one is already stopping joins it. + // Against a host older than its Stop join, a Stop of the turn one is already stopping joins it. const inFlightStopsRef = useRef(new Map>()) const stateArgs = { client, sessionId, sessionKey, enabled, connected } const { state, stateRef, queuedMessages, queuePause, loadingOlder, loadEarlier } = @@ -194,6 +195,7 @@ export function useMobileStructuredAgentSession(args: { requestMobileStructuredAgentSessionCancel({ client, enabled, + hostJoinsStops, inFlight: inFlightStopsRef.current, onSendError, prompt, @@ -201,7 +203,7 @@ export function useMobileStructuredAgentSession(args: { sessionId, stateRef }), - [client, enabled, onSendError, promptCancelSupported, sessionId, stateRef] + [client, enabled, hostJoinsStops, onSendError, promptCancelSupported, sessionId, stateRef] ) return { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-mutations.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-mutations.ts index 01143eb4bb47..95a6add7d742 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-mutations.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-mutations.ts @@ -33,11 +33,6 @@ import { performCancel } from './structured-agent-session-turns-cancel' import { structuredAgentSessionConversationFence } from './structured-agent-session-provider-child' -import { - admitAndRunAgentSessionMutation, - type AgentSessionMutationRequest, - type AgentSessionMutationSessionPreparation -} from './structured-agent-session-mutation-admission' import { openForWrite, openWithAgent, @@ -49,68 +44,28 @@ import { cancelPlan, promptPlan, sendPlan, - setOptionPlan, - type MutationPlan + setOptionPlan } from './structured-agent-session-mutation-plans' import { runQueueableStructuredAgentSessionSend } from './structured-agent-session-queued-send' import { runStopWithQueuePause } from './structured-agent-session-queued-stop' -import type { - StructuredAgentSessionCaller, - StructuredAgentSessionHostDeps, - StructuredAgentSessionHostSession -} from './structured-agent-session-host-types' +import { + joinStructuredAgentSessionStop, + type StructuredAgentSessionStopsInFlight +} from './structured-agent-session-stop-join' +import type { StructuredAgentSessionCaller } from './structured-agent-session-host-types' +import { + mutateStructuredAgentSession, + type StructuredAgentSessionMutationContext +} from './structured-agent-session-mutation-context' import { readStructuredAgentSessionOptions, recordStructuredAgentSessionOptionIntent } from './structured-agent-session-options-read' -export type StructuredAgentSessionMutationContext = { - deps: StructuredAgentSessionHostDeps - sessions: Map - publish: (sessionId: string, journal: StructuredAgentSessionHostSession['journal']) => void - flushStreamedEvents: (sessionId: string) => Promise - /** The host's accessor, for a caller outside the session's serialize. */ - conversation: (sessionId: string) => Promise - serialize: (sessionId: string, task: () => Promise) => Promise - /** The session's conversation, opened when closed; inside the caller's serialize. */ - openConversation: (sessionId: string) => Promise - /** Gives the session a provider child; inside the caller's serialize. */ - ensureAgent: (sessionId: string) => Promise - /** A message was accepted: the session's delivery loop hands it over. */ - wakeDelivery: (sessionId: string) => void - /** Stops the session's provider child, keeping its conversation; inside the caller's serialize. */ - stopAgent: (sessionId: string) => Promise - /** Only for gate inputs living in the RECORD store, which can settle with no - * journal commit (a conversation command). Draft-table changes need no call: - * the draft store notifies through the journal's own commit listener. */ - wakeQueuedDrain?: (sessionId: string) => void - now: () => number -} - -/** Admits the envelope and runs the plan inside the session's serialize. */ -export function mutateStructuredAgentSession( - context: StructuredAgentSessionMutationContext, - caller: StructuredAgentSessionCaller, - envelope: AgentSessionMutationEnvelope, - plan: MutationPlan, - prepareSession?: AgentSessionMutationRequest['prepareSession'] -): Promise> { - return context.serialize(envelope.sessionId, () => - admitAndRunAgentSessionMutation({ - store: context.deps.store, - adapter: context.deps.adapter, - callerKey: caller.callerKey, - envelope, - plan, - journal: () => context.sessions.get(envelope.sessionId)?.journal, - prepareSession, - publish: (journal) => context.publish(envelope.sessionId, journal), - flushStreamedEvents: context.flushStreamedEvents, - providerChildPhase: () => context.sessions.get(envelope.sessionId)?.child?.phase, - now: () => context.now() - }) - ) -} +export { + mutateStructuredAgentSession, + type StructuredAgentSessionMutationContext +} from './structured-agent-session-mutation-context' export function sendStructuredAgentSessionTurn( context: StructuredAgentSessionMutationContext, @@ -150,16 +105,39 @@ export function sendStructuredAgentSessionTurn( ) } +type CancelParams = { + envelope: AgentSessionMutationEnvelope + turnId?: string + scope?: 'background-tasks' + taskId?: string + prompt?: { itemId: string; expectedRevision: number } +} + export function cancelStructuredAgentSessionTurn( context: StructuredAgentSessionMutationContext, caller: StructuredAgentSessionCaller, - params: { - envelope: AgentSessionMutationEnvelope - turnId?: string - scope?: 'background-tasks' - taskId?: string - prompt?: { itemId: string; expectedRevision: number } - } + params: CancelParams, + stopsInFlight: StructuredAgentSessionStopsInFlight +): Promise> { + return joinStructuredAgentSessionStop( + stopsInFlight, + params, + () => runStructuredAgentSessionStop(context, caller, params), + (value) => + mutateStructuredAgentSession( + context, + caller, + params.envelope, + { ...cancelPlan(params), run: async () => ({ ok: true, value }) }, + openForWrite(context, params.envelope) + ) + ) +} + +function runStructuredAgentSessionStop( + context: StructuredAgentSessionMutationContext, + caller: StructuredAgentSessionCaller, + params: CancelParams ): Promise> { if (params.scope || params.prompt) { return mutateStructuredAgentSession( @@ -321,11 +299,11 @@ export async function settleStructuredAgentSessionLateDispatch( export function structuredAgentSessionMutationDelegates( context: () => StructuredAgentSessionMutationContext ) { + // One host's Stops still on their way. + const stopsInFlight: StructuredAgentSessionStopsInFlight = new Map() return { - cancel: ( - caller: StructuredAgentSessionCaller, - params: Parameters[2] - ) => cancelStructuredAgentSessionTurn(context(), caller, params), + cancel: (caller: StructuredAgentSessionCaller, params: CancelParams) => + cancelStructuredAgentSessionTurn(context(), caller, params, stopsInFlight), respondToPrompt: ( caller: StructuredAgentSessionCaller, params: Parameters[2] diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-context.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-context.ts new file mode 100644 index 000000000000..319a7fb74083 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-context.ts @@ -0,0 +1,63 @@ +import type { + AgentSessionMutationEnvelope, + AgentSessionMutationResult +} from '../../../shared/agent-session-wire' +import { + admitAndRunAgentSessionMutation, + type AgentSessionMutationRequest, + type AgentSessionMutationSessionPreparation +} from './structured-agent-session-mutation-admission' +import type { MutationPlan } from './structured-agent-session-mutation-plans' +import type { + StructuredAgentSessionCaller, + StructuredAgentSessionHostDeps, + StructuredAgentSessionHostSession +} from './structured-agent-session-host-types' + +export type StructuredAgentSessionMutationContext = { + deps: StructuredAgentSessionHostDeps + sessions: Map + publish: (sessionId: string, journal: StructuredAgentSessionHostSession['journal']) => void + flushStreamedEvents: (sessionId: string) => Promise + /** The host's accessor, for a caller outside the session's serialize. */ + conversation: (sessionId: string) => Promise + serialize: (sessionId: string, task: () => Promise) => Promise + /** The session's conversation, opened when closed; inside the caller's serialize. */ + openConversation: (sessionId: string) => Promise + /** Gives the session a provider child; inside the caller's serialize. */ + ensureAgent: (sessionId: string) => Promise + /** A message was accepted: the session's delivery loop hands it over. */ + wakeDelivery: (sessionId: string) => void + /** Stops the session's provider child, keeping its conversation; inside the caller's serialize. */ + stopAgent: (sessionId: string) => Promise + /** Only for gate inputs living in the RECORD store, which can settle with no + * journal commit (a conversation command). Draft-table changes need no call: + * the draft store notifies through the journal's own commit listener. */ + wakeQueuedDrain?: (sessionId: string) => void + now: () => number +} + +/** Admits the envelope and runs the plan inside the session's serialize. */ +export function mutateStructuredAgentSession( + context: StructuredAgentSessionMutationContext, + caller: StructuredAgentSessionCaller, + envelope: AgentSessionMutationEnvelope, + plan: MutationPlan, + prepareSession?: AgentSessionMutationRequest['prepareSession'] +): Promise> { + return context.serialize(envelope.sessionId, () => + admitAndRunAgentSessionMutation({ + store: context.deps.store, + adapter: context.deps.adapter, + callerKey: caller.callerKey, + envelope, + plan, + journal: () => context.sessions.get(envelope.sessionId)?.journal, + prepareSession, + publish: (journal) => context.publish(envelope.sessionId, journal), + flushStreamedEvents: context.flushStreamedEvents, + providerChildPhase: () => context.sessions.get(envelope.sessionId)?.child?.phase, + now: () => context.now() + }) + ) +} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-stop-join.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-stop-join.test.ts new file mode 100644 index 000000000000..c40e8610ada2 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-stop-join.test.ts @@ -0,0 +1,113 @@ +// A Stop naming its turn or task while a Stop of that same target is still on its way joins +// it, from any client: one interrupt, and every press answered with that Stop's result. +// Each press still passes its own admission and replays from its own receipt. + +import { beforeEach, describe, expect, it, type Mock } from 'vitest' +import type { AgentSessionMutationEnvelope } from '../../../shared/agent-session-wire' +import type { StructuredAgentSessionAdapter } from './structured-agent-session-adapter' +import type { StructuredAgentSessionHost } from './structured-agent-session-host' +import { + attach, + CALLER, + envelope, + hostTestState +} from './structured-agent-session-host-test-harness' + +let host: StructuredAgentSessionHost +let cancelTurn: Mock + +beforeEach(() => { + ;({ host, cancelTurn } = hostTestState()) +}) + +function stopTurn( + turnId: string, + caller = CALLER, + overrides: Partial = {} +) { + return host.cancel(caller, { + envelope: envelope('agentSession.cancel', { turnId }, overrides), + turnId + }) +} + +/** The provider holds the first interrupt until the test lets it answer. */ +function holdFirstInterrupt(): () => void { + const answer = Promise.withResolvers() + cancelTurn.mockImplementationOnce(async () => { + await answer.promise + return { cancelled: true } + }) + return () => answer.resolve(undefined) +} + +describe('a second Stop of the same turn while the first is on its way', () => { + it('joins it: one interrupt, and both presses answered with its result', async () => { + await attach() + const release = holdFirstInterrupt() + const first = stopTurn('turn-1') + const second = stopTurn('turn-1', { callerKey: 'client-2' }) + release() + + expect(await first).toMatchObject({ ok: true, value: { turnId: 'turn-1', cancelled: true } }) + expect(await second).toMatchObject({ + ok: true, + replayed: false, + value: { turnId: 'turn-1', cancelled: true } + }) + expect(cancelTurn).toHaveBeenCalledOnce() + }) + + it("replays a joined press's own id from its receipt, without interrupting again", async () => { + await attach() + const release = holdFirstInterrupt() + const joinedEnvelope = envelope('agentSession.cancel', { turnId: 'turn-1' }) + const first = stopTurn('turn-1') + const joined = host.cancel(CALLER, { envelope: joinedEnvelope, turnId: 'turn-1' }) + release() + await first + expect(await joined).toMatchObject({ ok: true, value: { cancelled: true } }) + + expect(await host.cancel(CALLER, { envelope: joinedEnvelope, turnId: 'turn-1' })).toMatchObject( + { ok: true, replayed: true } + ) + expect(cancelTurn).toHaveBeenCalledOnce() + }) + + it('still refuses a joined press its own admission turns away', async () => { + await attach() + const release = holdFirstInterrupt() + const first = stopTurn('turn-1') + const mismatched = stopTurn('turn-1', CALLER, { payloadFingerprint: 'not-this-stop' }) + release() + + expect(await first).toMatchObject({ ok: true }) + expect(await mismatched).toMatchObject({ ok: false }) + expect(cancelTurn).toHaveBeenCalledOnce() + }) + + it('answers an older client that sends both presses under one kept id with one interrupt', async () => { + await attach() + const release = holdFirstInterrupt() + const kept = envelope('agentSession.cancel', { turnId: 'turn-1' }) + const presses = [1, 2].map(() => host.cancel(CALLER, { envelope: kept, turnId: 'turn-1' })) + release() + + const [first, second] = await Promise.all(presses) + expect(first).toMatchObject({ ok: true, replayed: false, value: { cancelled: true } }) + // The kept id replays from the first press's receipt, as it always has. + expect(second).toMatchObject({ ok: true, replayed: true }) + expect(cancelTurn).toHaveBeenCalledOnce() + }) + + it('does not join a Stop of a different turn', async () => { + await attach() + const release = holdFirstInterrupt() + const first = stopTurn('turn-1') + const other = stopTurn('turn-2') + release() + await Promise.all([first, other]) + + expect(cancelTurn.mock.calls.map(([input]) => input.turnId)).toEqual(['turn-1', 'turn-2']) + }) +}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-stop-join.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-stop-join.ts new file mode 100644 index 000000000000..2eb960fe9823 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-stop-join.ts @@ -0,0 +1,61 @@ +import type { + AgentSessionCancelResult, + AgentSessionMutationResult +} from '../../../shared/agent-session-wire' + +type StopResult = AgentSessionMutationResult + +/** This host's Stops still on their way, by the session and the target they name. */ +export type StructuredAgentSessionStopsInFlight = Map> + +/** A Stop naming its turn or task. One naming neither acts on whatever runs when the host + * reaches it, so another press is a Stop of its own; a prompt Cancel's repeat is answered + * by the prompt's state. */ +function namedStopKey( + sessionId: string, + params: { turnId?: string; scope?: 'background-tasks'; taskId?: string; prompt?: unknown } +): string | null { + if (params.prompt !== undefined) { + return null + } + const target = params.scope === 'background-tasks' ? params.taskId : params.turnId + return target === undefined ? null : JSON.stringify([sessionId, params.scope ?? 'turn', target]) +} + +/** + * A Stop of a target another Stop is still stopping, from any client, waits for that Stop and + * answers with its result instead of interrupting again. It still passes its own admission + * (`answerWith`), so its id replays from its own receipt. A first Stop that answered nothing + * leaves the press to run as a Stop of its own. + */ +export async function joinStructuredAgentSessionStop( + inFlight: StructuredAgentSessionStopsInFlight, + params: { + envelope: { sessionId: string } + turnId?: string + scope?: 'background-tasks' + taskId?: string + prompt?: unknown + }, + run: () => Promise, + answerWith: (value: AgentSessionCancelResult) => Promise +): Promise { + const key = namedStopKey(params.envelope.sessionId, params) + if (key === null) { + return run() + } + const running = inFlight.get(key) + if (running) { + const first = await running.catch(() => null) + return first?.ok ? answerWith(first.value) : run() + } + const stopping = run() + inFlight.set(key, stopping) + const settle = (): void => { + if (inFlight.get(key) === stopping) { + inFlight.delete(key) + } + } + stopping.then(settle, settle) + return stopping +} 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 index ff7c438c6f9f..a0f7014d478f 100644 --- 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 @@ -7,6 +7,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' const mocks = vi.hoisted(() => ({ call: vi.fn(), + hostJoinsStops: vi.fn(async () => false), toastError: vi.fn(), operations: 0 })) @@ -14,7 +15,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, + supportsStructuredAgentSessionStopJoin: mocks.hostJoinsStops })) vi.mock('./use-structured-agent-session-outbox', () => ({ @@ -74,6 +76,7 @@ function render() { beforeEach(() => { vi.clearAllMocks() + mocks.hostJoinsStops.mockResolvedValue(false) mocks.operations = 0 }) @@ -178,21 +181,29 @@ describe('a conversation command typed again', () => { }) }) -describe('a Stop pressed again', () => { - 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, answerAll: () => answers.splice(0).forEach((answer) => answer()) } +/** 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 }) +} +const stop = (current: ReturnType['current'], turnId = 'turn-1') => + current.mutate('agentSession.cancel', 'agentSession.cancel', { turnId }) + +describe('a Stop pressed again, against a host older than its Stop join', () => { it('joins the one still in flight instead of sending a second', async () => { const host = heldStops() const { current } = render() @@ -204,7 +215,7 @@ describe('a Stop pressed again', () => { second = stop(current) }) await act(async () => { - host.answerAll() + await host.answerWhenSent(1) await Promise.all([first, second]) }) @@ -221,6 +232,7 @@ describe('a Stop pressed again', () => { 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) }) @@ -236,7 +248,7 @@ describe('a Stop pressed again', () => { for (let press = 0; press < 2; press += 1) { await act(async () => { const pressed = stop(current) - host.answerAll() + await host.answerWhenSent(1) await pressed }) } @@ -255,7 +267,7 @@ describe('a Stop pressed again', () => { current.mutate('agentSession.cancel', 'agentSession.cancel', {}), current.mutate('agentSession.cancel', 'agentSession.cancel', {}) ] - host.answerAll() + await host.answerWhenSent(2) await Promise.all(pressed) }) @@ -268,10 +280,26 @@ describe('a Stop pressed again', () => { await act(async () => { const pressed = [stop(current, 'turn-1'), stop(current, 'turn-2')] - host.answerAll() + await host.answerWhenSent(2) await Promise.all(pressed) }) expect(mocks.call).toHaveBeenCalledTimes(2) }) }) + +describe('a Stop pressed again, against a host that joins it', () => { + it('sends every press under its own id, even while an earlier one is on its way', async () => { + mocks.hostJoinsStops.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 062a5f1a7631..70cb5ad23062 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,10 +1,11 @@ // One structured-session mutation, fenced and idempotent. // // Every call is its own action with its own client operation id, so a press is never answered -// from an earlier one. A Stop of a named turn or task pressed while an identical one is still in -// flight joins it instead of stopping twice. Every result is discarded unless the runtime fence it -// was issued against is still the current one. 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. +// from an earlier one. The host joins a Stop of a named turn or task pressed while one is still on +// its way; against an older host that does not, this client joins it instead of stopping twice. +// Every result is discarded unless the runtime fence it was issued against is still the current +// one. 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' @@ -17,7 +18,10 @@ import { import { structuredAgentSessionPayloadFingerprint } from '../../../../shared/structured-agent-session-mutation' 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, + supportsStructuredAgentSessionStopJoin +} from '@/runtime/structured-agent-session-client' import { structuredSessionOperationId } from './use-structured-agent-session-outbox' import { agentSessionWriteFailureText } from './agent-session-write-notice-text' @@ -128,14 +132,18 @@ export function useStructuredAgentSessionMutate(args: { ) const write = useCallback( - ( + async ( method: string, fingerprintMethod: string, fields: Record ): Promise> => { - if (!namesWhatItStops(fingerprintMethod, fields)) { + if ( + !namesWhatItStops(fingerprintMethod, fields) || + (await supportsStructuredAgentSessionStopJoin(target)) + ) { return send(method, fingerprintMethod, fields) } + // Temporary, for a host older than its Stop join: remove once every supported host has it. const key = `${sessionId}:${method}:${JSON.stringify(fields)}` const joined = inFlightStops.current.get(key) if (joined) { @@ -157,7 +165,7 @@ export function useStructuredAgentSessionMutate(args: { }) return stopping }, - [send, sessionId] + [send, sessionId, target] ) const mutate = useCallback( diff --git a/src/renderer/src/runtime/structured-agent-session-client.ts b/src/renderer/src/runtime/structured-agent-session-client.ts index d50fed2b4c33..3257b7b2ec83 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_STOP_JOIN_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 joins a second Stop of a target it is still stopping. */ +export function supportsStructuredAgentSessionStopJoin( + target: RuntimeClientTarget +): Promise { + return structuredAgentSessionHostSupports(target, AGENT_SESSION_STOP_JOIN_RUNTIME_CAPABILITY) +} + export function supportsStructuredAgentSessionQuestionAnswers( target: RuntimeClientTarget ): Promise { diff --git a/src/shared/protocol-version.ts b/src/shared/protocol-version.ts index d3880313897a..d0cb2c66d6d5 100644 --- a/src/shared/protocol-version.ts +++ b/src/shared/protocol-version.ts @@ -237,6 +237,10 @@ export const AGENT_SESSION_PROMPT_CANCEL_RUNTIME_CAPABILITY = // `answers` to an older host; they fall back to the answer packed into `optionId`. export const AGENT_SESSION_QUESTION_ANSWERS_RUNTIME_CAPABILITY = 'agent-session.question-answers.v1' as const +// Why: the host joins a second Stop of a turn or task it is still stopping. An older host runs +// both, and the second writes a false "already finished" row, so a client joins them itself there. +// Transitional: drop the client join once no supported host lacks this. +export const AGENT_SESSION_STOP_JOIN_RUNTIME_CAPABILITY = 'agent-session.stop-join.v1' as const // Why: the host now publishes rows for work that is live inside a turn, and such // a row carries `stoppable: false` because no targeted stop can reach it. A // reader that predates the field draws a per-row Stop on every row it is given, @@ -414,6 +418,7 @@ export const RUNTIME_CAPABILITIES = [ AGENT_SESSION_BACKGROUND_TASK_STOP_CAPABILITY, AGENT_SESSION_PROMPT_CANCEL_RUNTIME_CAPABILITY, AGENT_SESSION_QUESTION_ANSWERS_RUNTIME_CAPABILITY, + AGENT_SESSION_STOP_JOIN_RUNTIME_CAPABILITY, AGENT_SESSION_TURN_ITEM_CAPABILITY, AGENT_SESSION_BACKGROUND_TASK_ROW_STOP_CAPABILITY, AGENT_SESSION_KIMI_RESUME_RUNTIME_CAPABILITY, From 1b0e7264a854702833a9efec724b7ea6af854fdf Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 30 Sep 2026 01:09:30 -0700 Subject: [PATCH 08/17] fix(mobile): keep the Stop's fence narrowed, and type the held card answer --- mobile/src/session/mobile-structured-agent-session-cancel.ts | 3 ++- .../use-mobile-structured-agent-session-queued.test.tsx | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/mobile/src/session/mobile-structured-agent-session-cancel.ts b/mobile/src/session/mobile-structured-agent-session-cancel.ts index aa77d2e58f47..75a4d138da22 100644 --- a/mobile/src/session/mobile-structured-agent-session-cancel.ts +++ b/mobile/src/session/mobile-structured-agent-session-cancel.ts @@ -48,7 +48,8 @@ export async function requestMobileStructuredAgentSessionCancel(args: { ...(args.prompt && args.promptCancelSupported === true ? { prompt: args.prompt } : {}) } // Every press is its own Stop: a kept id would be answered from the last one and stop nothing. - const stop = () => sendStop({ client, sessionId, fence: current.fence, fields, onSendError }) + const fence = current.fence + const stop = () => sendStop({ client, sessionId, fence, fields, onSendError }) if (args.hostJoinsStops === true) { return stop() } 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 03ac65acdcca..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' @@ -606,7 +607,7 @@ describe('mobile structured queued messages', () => { }) it('each press of a card action carries its own id, even while an earlier press is unanswered', async () => { - const held = Promise.withResolvers() + const held = Promise.withResolvers() sendRequest.mockImplementation(async (method) => method === 'agentSession.queuedMessageSend' || method === 'agentSession.queuedMessageDelete' ? held.promise From 7801b8fcde921f08a1db95c85ec8a758e9ae0ed3 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 30 Sep 2026 01:17:09 -0700 Subject: [PATCH 09/17] test(native-chat): open the repeat-press host on the shared journal database --- .../structured-agent-session-repeat-press.test.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) 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 index fdae6dda068c..48675c292bce 100644 --- 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 @@ -9,7 +9,9 @@ import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vite 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 { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' +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 { @@ -38,7 +40,7 @@ beforeEach(async () => { changeThreadGoal = vi.fn(async () => ({ ok: true as const })) stopBackgroundTasks = vi.fn(async () => ({ cancelled: true })) cancelTurn = vi.fn(async () => ({ cancelled: true })) - store = await AgentSessionRecordStore.open({ directory: join(root, 'store'), hostId: 'local' }) + store = await openTestAgentSessionRecordStore(root) host = new StructuredAgentSessionHost({ store, adapter: { @@ -63,7 +65,7 @@ beforeEach(async () => { supportsThreadGoal: () => true, stopBackgroundTasks }, - journalRoot: root, + journalDatabase: openTestJournalHostDatabase(root), claimKeyId: 'key-1', mintSpawnToken: () => 'spawn-1', now: () => NOW From b0a654a5bfb4ad2308029798a4d6b7e3677a476d Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 30 Sep 2026 01:50:34 -0700 Subject: [PATCH 10/17] fix(native-chat): a second Stop of a turn an earlier Stop answered for adds no row Read from the earlier Stop's note on that turn in the journal, so it holds however late the second Stop lands and from whichever client, and across a host restart. Replaces the in-memory join. The capability now says the host answers a repeated Stop quietly; clients still join a Stop on its way only for a host without it. --- .../mobile-structured-agent-session-cancel.ts | 12 +- ...uctured-agent-session-host-support.test.ts | 26 ++-- ...e-structured-agent-session-host-support.ts | 8 +- ...structured-operation-id-retirement.test.ts | 8 +- ...tured-agent-session-prompt-cancel.test.tsx | 2 +- ...tured-agent-session-queued.test-fixture.ts | 2 +- .../use-mobile-structured-agent-session.ts | 16 ++- .../structured-agent-session-command-turn.ts | 21 +-- ...structured-agent-session-host-mutations.ts | 118 +++++++++------- ...ructured-agent-session-mutation-context.ts | 63 --------- ...ctured-agent-session-repeated-stop.test.ts | 133 ++++++++++++++++++ ...structured-agent-session-stop-join.test.ts | 113 --------------- .../structured-agent-session-stop-join.ts | 61 -------- ...tructured-agent-session-turn-stop-notes.ts | 35 +++++ .../structured-agent-session-turns-cancel.ts | 21 ++- ...e-structured-agent-session-mutate.test.tsx | 12 +- .../use-structured-agent-session-mutate.ts | 10 +- ...tured-agent-session-prompt-cancel.test.tsx | 1 + ...uctured-agent-session-provisional.test.tsx | 3 +- ...use-structured-agent-session-stop.test.tsx | 1 + ...tured-agent-session-write-refusal.test.tsx | 3 +- ...tured-agent-session.queued-gating.test.tsx | 5 +- .../use-structured-agent-session.test.tsx | 3 +- .../structured-agent-session-client.ts | 8 +- src/shared/protocol-version.ts | 11 +- 25 files changed, 341 insertions(+), 355 deletions(-) delete mode 100644 src/main/native-chat/agent-session-wire/structured-agent-session-mutation-context.ts create mode 100644 src/main/native-chat/agent-session-wire/structured-agent-session-repeated-stop.test.ts delete mode 100644 src/main/native-chat/agent-session-wire/structured-agent-session-stop-join.test.ts delete mode 100644 src/main/native-chat/agent-session-wire/structured-agent-session-stop-join.ts create mode 100644 src/main/native-chat/agent-session-wire/structured-agent-session-turn-stop-notes.ts diff --git a/mobile/src/session/mobile-structured-agent-session-cancel.ts b/mobile/src/session/mobile-structured-agent-session-cancel.ts index 75a4d138da22..3a3286510529 100644 --- a/mobile/src/session/mobile-structured-agent-session-cancel.ts +++ b/mobile/src/session/mobile-structured-agent-session-cancel.ts @@ -28,10 +28,10 @@ export async function requestMobileStructuredAgentSessionCancel(args: { stateRef: { readonly current: StructuredAgentSessionState } promptCancelSupported: boolean | null prompt?: PromptIdentity - /** Whether the host joins a second Stop itself; null until the status probe answers. */ - hostJoinsStops: boolean | null - /** Stops still on their way, by what they stop; against a host that does not join them, a - * press for the same one joins it here. */ + /** 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 { @@ -50,10 +50,10 @@ export async function requestMobileStructuredAgentSessionCancel(args: { // 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.hostJoinsStops === true) { + if (args.hostAnswersRepeatedStops === true) { return stop() } - // Temporary, for a host older than its Stop join: remove once every supported host has it. + // 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) { 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 0d679a876af1..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 @@ -3,7 +3,7 @@ import { AGENT_SESSION_PROMPT_CANCEL_RUNTIME_CAPABILITY, AGENT_SESSION_QUESTION_ANSWERS_RUNTIME_CAPABILITY, AGENT_SESSION_QUEUED_MESSAGES_RUNTIME_CAPABILITY, - AGENT_SESSION_STOP_JOIN_RUNTIME_CAPABILITY + AGENT_SESSION_REPEATED_STOP_RUNTIME_CAPABILITY } from '../../../src/shared/protocol-version' import { structuredAgentSessionHostSupport } from './mobile-structured-agent-session-host-support' @@ -13,7 +13,7 @@ describe('structuredAgentSessionHostSupport', () => { promptCancel: false, questionAnswers: false, queuedMessages: false, - stopJoin: false + quietRepeatedStop: false }) expect( structuredAgentSessionHostSupport([AGENT_SESSION_QUESTION_ANSWERS_RUNTIME_CAPABILITY]) @@ -21,7 +21,7 @@ describe('structuredAgentSessionHostSupport', () => { promptCancel: false, questionAnswers: true, queuedMessages: false, - stopJoin: false + quietRepeatedStop: false }) expect( structuredAgentSessionHostSupport([AGENT_SESSION_PROMPT_CANCEL_RUNTIME_CAPABILITY]) @@ -29,7 +29,7 @@ describe('structuredAgentSessionHostSupport', () => { promptCancel: true, questionAnswers: false, queuedMessages: false, - stopJoin: false + quietRepeatedStop: false }) expect( structuredAgentSessionHostSupport([AGENT_SESSION_QUEUED_MESSAGES_RUNTIME_CAPABILITY]) @@ -37,15 +37,15 @@ describe('structuredAgentSessionHostSupport', () => { promptCancel: false, questionAnswers: false, queuedMessages: true, - stopJoin: false + quietRepeatedStop: false + }) + expect( + structuredAgentSessionHostSupport([AGENT_SESSION_REPEATED_STOP_RUNTIME_CAPABILITY]) + ).toEqual({ + promptCancel: false, + questionAnswers: false, + queuedMessages: false, + quietRepeatedStop: true }) - expect(structuredAgentSessionHostSupport([AGENT_SESSION_STOP_JOIN_RUNTIME_CAPABILITY])).toEqual( - { - promptCancel: false, - questionAnswers: false, - queuedMessages: false, - stopJoin: 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 ebdb02a98036..e08081ca8da4 100644 --- a/mobile/src/session/mobile-structured-agent-session-host-support.ts +++ b/mobile/src/session/mobile-structured-agent-session-host-support.ts @@ -2,7 +2,7 @@ import { AGENT_SESSION_PROMPT_CANCEL_RUNTIME_CAPABILITY, AGENT_SESSION_QUESTION_ANSWERS_RUNTIME_CAPABILITY, AGENT_SESSION_QUEUED_MESSAGES_RUNTIME_CAPABILITY, - AGENT_SESSION_STOP_JOIN_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. */ @@ -11,8 +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 - /** The host joins a second Stop of a turn it is still stopping. */ - stopJoin: boolean + /** A Stop of a turn an earlier Stop already answered for adds no row. */ + quietRepeatedStop: boolean } export function structuredAgentSessionHostSupport( @@ -22,6 +22,6 @@ export function structuredAgentSessionHostSupport( 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), - stopJoin: capabilities.includes(AGENT_SESSION_STOP_JOIN_RUNTIME_CAPABILITY) + quietRepeatedStop: capabilities.includes(AGENT_SESSION_REPEATED_STOP_RUNTIME_CAPABILITY) } } 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 aaf6be77116b..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,14 +49,14 @@ function runningState(): StructuredAgentSessionState { return state as unknown as StructuredAgentSessionState } -function cancelArgs(client: RpcClient, hostJoinsStops: boolean | null = false) { +function cancelArgs(client: RpcClient, hostAnswersRepeatedStops: boolean | null = false) { return { client, sessionId: 'session-1', enabled: true, stateRef: { current: runningState() }, promptCancelSupported: null, - hostJoinsStops, + hostAnswersRepeatedStops, inFlight: new Map>(), onSendError: vi.fn() } @@ -157,7 +157,7 @@ describe('a phone Stop pressed again', () => { return { client, sent, answerAll: () => answers.splice(0).forEach((answer) => answer()) } } - it('against a host older than its Stop join, joins the Stop of that turn still on its way', async () => { + 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) @@ -186,7 +186,7 @@ describe('a phone Stop pressed again', () => { expect(args.inFlight.size).toBe(0) }) - it('against a host that joins Stops, sends every press under its own id', async () => { + 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) 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 27659811f856..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 @@ -125,7 +125,7 @@ function Harness({ promptCancel: promptCancelSupported, questionAnswers: questionAnswersSupported, queuedMessages: false, - stopJoin: 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 64cc7bd03af3..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 @@ -16,7 +16,7 @@ export const CAPABLE: StructuredAgentSessionHostSupport = { promptCancel: false, questionAnswers: false, queuedMessages: true, - stopJoin: false + quietRepeatedStop: false } export const LEGACY: StructuredAgentSessionHostSupport = { ...CAPABLE, queuedMessages: false } diff --git a/mobile/src/session/use-mobile-structured-agent-session.ts b/mobile/src/session/use-mobile-structured-agent-session.ts index 4cfd6dad8dd7..653d7fbaaa1a 100644 --- a/mobile/src/session/use-mobile-structured-agent-session.ts +++ b/mobile/src/session/use-mobile-structured-agent-session.ts @@ -97,10 +97,10 @@ 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 hostJoinsStops = hostSupport?.stopJoin ?? null + const hostAnswersRepeatedStops = hostSupport?.quietRepeatedStop ?? null const sessionKey = encodeNativeChatTranscriptIdentity([sourceIdentity, agent, sessionId]) const commandPendingRef = useRef(false) - // Against a host older than its Stop join, a Stop of the turn one is already stopping joins it. + // 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 } = @@ -195,7 +195,7 @@ export function useMobileStructuredAgentSession(args: { requestMobileStructuredAgentSessionCancel({ client, enabled, - hostJoinsStops, + hostAnswersRepeatedStops, inFlight: inFlightStopsRef.current, onSendError, prompt, @@ -203,7 +203,15 @@ export function useMobileStructuredAgentSession(args: { sessionId, stateRef }), - [client, enabled, hostJoinsStops, onSendError, promptCancelSupported, sessionId, stateRef] + [ + client, + enabled, + hostAnswersRepeatedStops, + onSendError, + promptCancelSupported, + sessionId, + stateRef + ] ) return { 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 cb83dba870a1..d26e8f58f13c 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 @@ -126,6 +126,12 @@ export function structuredAgentSessionStopNoteIdentity( return { provider: 'orca', clientMessageId: `${STOP_NOTE_PREFIX}${clientOperationId}` } } +/** 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 * journal, so nothing is held that could outlive the command. */ export function structuredAgentSessionCommandWasStopped( @@ -133,15 +139,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-host-mutations.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-mutations.ts index 95a6add7d742..01143eb4bb47 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-mutations.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-mutations.ts @@ -33,6 +33,11 @@ import { performCancel } from './structured-agent-session-turns-cancel' import { structuredAgentSessionConversationFence } from './structured-agent-session-provider-child' +import { + admitAndRunAgentSessionMutation, + type AgentSessionMutationRequest, + type AgentSessionMutationSessionPreparation +} from './structured-agent-session-mutation-admission' import { openForWrite, openWithAgent, @@ -44,28 +49,68 @@ import { cancelPlan, promptPlan, sendPlan, - setOptionPlan + setOptionPlan, + type MutationPlan } from './structured-agent-session-mutation-plans' import { runQueueableStructuredAgentSessionSend } from './structured-agent-session-queued-send' import { runStopWithQueuePause } from './structured-agent-session-queued-stop' -import { - joinStructuredAgentSessionStop, - type StructuredAgentSessionStopsInFlight -} from './structured-agent-session-stop-join' -import type { StructuredAgentSessionCaller } from './structured-agent-session-host-types' -import { - mutateStructuredAgentSession, - type StructuredAgentSessionMutationContext -} from './structured-agent-session-mutation-context' +import type { + StructuredAgentSessionCaller, + StructuredAgentSessionHostDeps, + StructuredAgentSessionHostSession +} from './structured-agent-session-host-types' import { readStructuredAgentSessionOptions, recordStructuredAgentSessionOptionIntent } from './structured-agent-session-options-read' -export { - mutateStructuredAgentSession, - type StructuredAgentSessionMutationContext -} from './structured-agent-session-mutation-context' +export type StructuredAgentSessionMutationContext = { + deps: StructuredAgentSessionHostDeps + sessions: Map + publish: (sessionId: string, journal: StructuredAgentSessionHostSession['journal']) => void + flushStreamedEvents: (sessionId: string) => Promise + /** The host's accessor, for a caller outside the session's serialize. */ + conversation: (sessionId: string) => Promise + serialize: (sessionId: string, task: () => Promise) => Promise + /** The session's conversation, opened when closed; inside the caller's serialize. */ + openConversation: (sessionId: string) => Promise + /** Gives the session a provider child; inside the caller's serialize. */ + ensureAgent: (sessionId: string) => Promise + /** A message was accepted: the session's delivery loop hands it over. */ + wakeDelivery: (sessionId: string) => void + /** Stops the session's provider child, keeping its conversation; inside the caller's serialize. */ + stopAgent: (sessionId: string) => Promise + /** Only for gate inputs living in the RECORD store, which can settle with no + * journal commit (a conversation command). Draft-table changes need no call: + * the draft store notifies through the journal's own commit listener. */ + wakeQueuedDrain?: (sessionId: string) => void + now: () => number +} + +/** Admits the envelope and runs the plan inside the session's serialize. */ +export function mutateStructuredAgentSession( + context: StructuredAgentSessionMutationContext, + caller: StructuredAgentSessionCaller, + envelope: AgentSessionMutationEnvelope, + plan: MutationPlan, + prepareSession?: AgentSessionMutationRequest['prepareSession'] +): Promise> { + return context.serialize(envelope.sessionId, () => + admitAndRunAgentSessionMutation({ + store: context.deps.store, + adapter: context.deps.adapter, + callerKey: caller.callerKey, + envelope, + plan, + journal: () => context.sessions.get(envelope.sessionId)?.journal, + prepareSession, + publish: (journal) => context.publish(envelope.sessionId, journal), + flushStreamedEvents: context.flushStreamedEvents, + providerChildPhase: () => context.sessions.get(envelope.sessionId)?.child?.phase, + now: () => context.now() + }) + ) +} export function sendStructuredAgentSessionTurn( context: StructuredAgentSessionMutationContext, @@ -105,39 +150,16 @@ export function sendStructuredAgentSessionTurn( ) } -type CancelParams = { - envelope: AgentSessionMutationEnvelope - turnId?: string - scope?: 'background-tasks' - taskId?: string - prompt?: { itemId: string; expectedRevision: number } -} - export function cancelStructuredAgentSessionTurn( context: StructuredAgentSessionMutationContext, caller: StructuredAgentSessionCaller, - params: CancelParams, - stopsInFlight: StructuredAgentSessionStopsInFlight -): Promise> { - return joinStructuredAgentSessionStop( - stopsInFlight, - params, - () => runStructuredAgentSessionStop(context, caller, params), - (value) => - mutateStructuredAgentSession( - context, - caller, - params.envelope, - { ...cancelPlan(params), run: async () => ({ ok: true, value }) }, - openForWrite(context, params.envelope) - ) - ) -} - -function runStructuredAgentSessionStop( - context: StructuredAgentSessionMutationContext, - caller: StructuredAgentSessionCaller, - params: CancelParams + params: { + envelope: AgentSessionMutationEnvelope + turnId?: string + scope?: 'background-tasks' + taskId?: string + prompt?: { itemId: string; expectedRevision: number } + } ): Promise> { if (params.scope || params.prompt) { return mutateStructuredAgentSession( @@ -299,11 +321,11 @@ export async function settleStructuredAgentSessionLateDispatch( export function structuredAgentSessionMutationDelegates( context: () => StructuredAgentSessionMutationContext ) { - // One host's Stops still on their way. - const stopsInFlight: StructuredAgentSessionStopsInFlight = new Map() return { - cancel: (caller: StructuredAgentSessionCaller, params: CancelParams) => - cancelStructuredAgentSessionTurn(context(), caller, params, stopsInFlight), + cancel: ( + caller: StructuredAgentSessionCaller, + params: Parameters[2] + ) => cancelStructuredAgentSessionTurn(context(), caller, params), respondToPrompt: ( caller: StructuredAgentSessionCaller, params: Parameters[2] diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-context.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-context.ts deleted file mode 100644 index 319a7fb74083..000000000000 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-context.ts +++ /dev/null @@ -1,63 +0,0 @@ -import type { - AgentSessionMutationEnvelope, - AgentSessionMutationResult -} from '../../../shared/agent-session-wire' -import { - admitAndRunAgentSessionMutation, - type AgentSessionMutationRequest, - type AgentSessionMutationSessionPreparation -} from './structured-agent-session-mutation-admission' -import type { MutationPlan } from './structured-agent-session-mutation-plans' -import type { - StructuredAgentSessionCaller, - StructuredAgentSessionHostDeps, - StructuredAgentSessionHostSession -} from './structured-agent-session-host-types' - -export type StructuredAgentSessionMutationContext = { - deps: StructuredAgentSessionHostDeps - sessions: Map - publish: (sessionId: string, journal: StructuredAgentSessionHostSession['journal']) => void - flushStreamedEvents: (sessionId: string) => Promise - /** The host's accessor, for a caller outside the session's serialize. */ - conversation: (sessionId: string) => Promise - serialize: (sessionId: string, task: () => Promise) => Promise - /** The session's conversation, opened when closed; inside the caller's serialize. */ - openConversation: (sessionId: string) => Promise - /** Gives the session a provider child; inside the caller's serialize. */ - ensureAgent: (sessionId: string) => Promise - /** A message was accepted: the session's delivery loop hands it over. */ - wakeDelivery: (sessionId: string) => void - /** Stops the session's provider child, keeping its conversation; inside the caller's serialize. */ - stopAgent: (sessionId: string) => Promise - /** Only for gate inputs living in the RECORD store, which can settle with no - * journal commit (a conversation command). Draft-table changes need no call: - * the draft store notifies through the journal's own commit listener. */ - wakeQueuedDrain?: (sessionId: string) => void - now: () => number -} - -/** Admits the envelope and runs the plan inside the session's serialize. */ -export function mutateStructuredAgentSession( - context: StructuredAgentSessionMutationContext, - caller: StructuredAgentSessionCaller, - envelope: AgentSessionMutationEnvelope, - plan: MutationPlan, - prepareSession?: AgentSessionMutationRequest['prepareSession'] -): Promise> { - return context.serialize(envelope.sessionId, () => - admitAndRunAgentSessionMutation({ - store: context.deps.store, - adapter: context.deps.adapter, - callerKey: caller.callerKey, - envelope, - plan, - journal: () => context.sessions.get(envelope.sessionId)?.journal, - prepareSession, - publish: (journal) => context.publish(envelope.sessionId, journal), - flushStreamedEvents: context.flushStreamedEvents, - providerChildPhase: () => context.sessions.get(envelope.sessionId)?.child?.phase, - now: () => context.now() - }) - ) -} 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..cbdf66d9f16a --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-repeated-stop.test.ts @@ -0,0 +1,133 @@ +// A Stop naming a turn that an earlier Stop already answered for adds no row, however late it +// lands and from whichever client: the earlier Stop's note, on the turn in the journal, says so. + +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.' +const ALREADY_FINISHED = 'The provider had already finished this turn.' + +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 of a turn an earlier Stop already answered for', () => { + it('answers quietly 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('stays quiet after a first Stop that found the turn already over', 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([ALREADY_FINISHED]) + }) + + it('reads the earlier Stop from the journal, so a restarted host answers the same', 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 no earlier Stop answered for', () => { + it('still says the turn had already finished when it ended by itself', 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([ALREADY_FINISHED]) + }) +}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-stop-join.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-stop-join.test.ts deleted file mode 100644 index c40e8610ada2..000000000000 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-stop-join.test.ts +++ /dev/null @@ -1,113 +0,0 @@ -// A Stop naming its turn or task while a Stop of that same target is still on its way joins -// it, from any client: one interrupt, and every press answered with that Stop's result. -// Each press still passes its own admission and replays from its own receipt. - -import { beforeEach, describe, expect, it, type Mock } from 'vitest' -import type { AgentSessionMutationEnvelope } from '../../../shared/agent-session-wire' -import type { StructuredAgentSessionAdapter } from './structured-agent-session-adapter' -import type { StructuredAgentSessionHost } from './structured-agent-session-host' -import { - attach, - CALLER, - envelope, - hostTestState -} from './structured-agent-session-host-test-harness' - -let host: StructuredAgentSessionHost -let cancelTurn: Mock - -beforeEach(() => { - ;({ host, cancelTurn } = hostTestState()) -}) - -function stopTurn( - turnId: string, - caller = CALLER, - overrides: Partial = {} -) { - return host.cancel(caller, { - envelope: envelope('agentSession.cancel', { turnId }, overrides), - turnId - }) -} - -/** The provider holds the first interrupt until the test lets it answer. */ -function holdFirstInterrupt(): () => void { - const answer = Promise.withResolvers() - cancelTurn.mockImplementationOnce(async () => { - await answer.promise - return { cancelled: true } - }) - return () => answer.resolve(undefined) -} - -describe('a second Stop of the same turn while the first is on its way', () => { - it('joins it: one interrupt, and both presses answered with its result', async () => { - await attach() - const release = holdFirstInterrupt() - const first = stopTurn('turn-1') - const second = stopTurn('turn-1', { callerKey: 'client-2' }) - release() - - expect(await first).toMatchObject({ ok: true, value: { turnId: 'turn-1', cancelled: true } }) - expect(await second).toMatchObject({ - ok: true, - replayed: false, - value: { turnId: 'turn-1', cancelled: true } - }) - expect(cancelTurn).toHaveBeenCalledOnce() - }) - - it("replays a joined press's own id from its receipt, without interrupting again", async () => { - await attach() - const release = holdFirstInterrupt() - const joinedEnvelope = envelope('agentSession.cancel', { turnId: 'turn-1' }) - const first = stopTurn('turn-1') - const joined = host.cancel(CALLER, { envelope: joinedEnvelope, turnId: 'turn-1' }) - release() - await first - expect(await joined).toMatchObject({ ok: true, value: { cancelled: true } }) - - expect(await host.cancel(CALLER, { envelope: joinedEnvelope, turnId: 'turn-1' })).toMatchObject( - { ok: true, replayed: true } - ) - expect(cancelTurn).toHaveBeenCalledOnce() - }) - - it('still refuses a joined press its own admission turns away', async () => { - await attach() - const release = holdFirstInterrupt() - const first = stopTurn('turn-1') - const mismatched = stopTurn('turn-1', CALLER, { payloadFingerprint: 'not-this-stop' }) - release() - - expect(await first).toMatchObject({ ok: true }) - expect(await mismatched).toMatchObject({ ok: false }) - expect(cancelTurn).toHaveBeenCalledOnce() - }) - - it('answers an older client that sends both presses under one kept id with one interrupt', async () => { - await attach() - const release = holdFirstInterrupt() - const kept = envelope('agentSession.cancel', { turnId: 'turn-1' }) - const presses = [1, 2].map(() => host.cancel(CALLER, { envelope: kept, turnId: 'turn-1' })) - release() - - const [first, second] = await Promise.all(presses) - expect(first).toMatchObject({ ok: true, replayed: false, value: { cancelled: true } }) - // The kept id replays from the first press's receipt, as it always has. - expect(second).toMatchObject({ ok: true, replayed: true }) - expect(cancelTurn).toHaveBeenCalledOnce() - }) - - it('does not join a Stop of a different turn', async () => { - await attach() - const release = holdFirstInterrupt() - const first = stopTurn('turn-1') - const other = stopTurn('turn-2') - release() - await Promise.all([first, other]) - - expect(cancelTurn.mock.calls.map(([input]) => input.turnId)).toEqual(['turn-1', 'turn-2']) - }) -}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-stop-join.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-stop-join.ts deleted file mode 100644 index 2eb960fe9823..000000000000 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-stop-join.ts +++ /dev/null @@ -1,61 +0,0 @@ -import type { - AgentSessionCancelResult, - AgentSessionMutationResult -} from '../../../shared/agent-session-wire' - -type StopResult = AgentSessionMutationResult - -/** This host's Stops still on their way, by the session and the target they name. */ -export type StructuredAgentSessionStopsInFlight = Map> - -/** A Stop naming its turn or task. One naming neither acts on whatever runs when the host - * reaches it, so another press is a Stop of its own; a prompt Cancel's repeat is answered - * by the prompt's state. */ -function namedStopKey( - sessionId: string, - params: { turnId?: string; scope?: 'background-tasks'; taskId?: string; prompt?: unknown } -): string | null { - if (params.prompt !== undefined) { - return null - } - const target = params.scope === 'background-tasks' ? params.taskId : params.turnId - return target === undefined ? null : JSON.stringify([sessionId, params.scope ?? 'turn', target]) -} - -/** - * A Stop of a target another Stop is still stopping, from any client, waits for that Stop and - * answers with its result instead of interrupting again. It still passes its own admission - * (`answerWith`), so its id replays from its own receipt. A first Stop that answered nothing - * leaves the press to run as a Stop of its own. - */ -export async function joinStructuredAgentSessionStop( - inFlight: StructuredAgentSessionStopsInFlight, - params: { - envelope: { sessionId: string } - turnId?: string - scope?: 'background-tasks' - taskId?: string - prompt?: unknown - }, - run: () => Promise, - answerWith: (value: AgentSessionCancelResult) => Promise -): Promise { - const key = namedStopKey(params.envelope.sessionId, params) - if (key === null) { - return run() - } - const running = inFlight.get(key) - if (running) { - const first = await running.catch(() => null) - return first?.ok ? answerWith(first.value) : run() - } - const stopping = run() - inFlight.set(key, stopping) - const settle = (): void => { - if (inFlight.get(key) === stopping) { - inFlight.delete(key) - } - } - stopping.then(settle, settle) - return stopping -} 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..c00a1f1719e5 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-turn-stop-notes.ts @@ -0,0 +1,35 @@ +// A Stop's note sits on the turn it named. A later Stop of that turn reads the earlier note from the +// journal, so a repeated press adds no row, however late and from whichever client it arrives. + +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' +import { isStructuredAgentSessionStopNote } from './structured-agent-session-command-turn' + +export const STOP_NOTE_CANCELLATION_REQUESTED = 'Cancellation requested.' +export const STOP_NOTE_ALREADY_FINISHED = 'The provider had already finished this turn.' + +type TurnScope = Extract + +/** The turn `turnId` names, running or ended, and whether an earlier Stop's note on it already + * said the Stop took effect or found the turn over: all a later Stop of it could say. */ +export function structuredAgentSessionNamedTurnStop( + journal: Pick, + turnId: string +): { turnScope: TurnScope; answered: boolean } | null { + const items = journal.snapshot().items + const turn = items.findLast((item) => readAgentJournalTurn(item.body)?.turnId === turnId) + if (!turn) { + return null + } + const answered = items.some( + (item) => + item.turnScope?.kind === 'turn' && + item.turnScope.turnItemId === turn.itemId && + isStructuredAgentSessionStopNote(item.itemId) && + item.body.kind === 'status' && + (item.body.text === STOP_NOTE_CANCELLATION_REQUESTED || + item.body.text === STOP_NOTE_ALREADY_FINISHED) + ) + return { turnScope: { kind: 'turn', turnItemId: turn.itemId }, answered } +} 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 d6a0c0961dea..eb8f373d7bf8 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 @@ -10,6 +10,11 @@ import { structuredAgentSessionStopNoteIdentity } from './structured-agent-session-command-turn' import { settledPrompt, validatePendingPrompt } from './structured-agent-session-prompt-state' +import { + STOP_NOTE_ALREADY_FINISHED, + STOP_NOTE_CANCELLATION_REQUESTED, + structuredAgentSessionNamedTurnStop +} 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' @@ -61,9 +66,17 @@ export async function performCancel( } } let cancelled = false - let note: AgentJournalStatusItem | null = { kind: 'status', text: 'Cancellation requested.' } + // A turn an earlier Stop already answered for gets no second row, however late this one lands. + const named = + input.turnId !== undefined && !input.scope + ? structuredAgentSessionNamedTurnStop(ctx.journal, input.turnId) + : null + const answeredBefore = named?.answered === true + let note: AgentJournalStatusItem | null = answeredBefore + ? 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 = named?.turnScope ?? 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. @@ -106,7 +119,7 @@ export async function performCancel( cancelled = true note = null } else if (!cancelled && input.turnId !== undefined) { - note = { kind: 'status', text: 'The provider had already finished this turn.' } + note = answeredBefore ? null : { kind: 'status', text: STOP_NOTE_ALREADY_FINISHED } } else if (!cancelled && input.prompt) { note = null } else if (!cancelled) { @@ -133,7 +146,7 @@ export async function performCancel( if (runningCommand && !cancelled) { await input.stopChild?.() cancelled = true - note = { kind: 'status', text: 'Cancellation requested.' } + note = { kind: 'status', text: STOP_NOTE_CANCELLATION_REQUESTED } } if (cancelled && input.prompt) { await ctx.flushStreamedEvents() 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 index a0f7014d478f..81f61815dd9a 100644 --- 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 @@ -7,7 +7,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' const mocks = vi.hoisted(() => ({ call: vi.fn(), - hostJoinsStops: vi.fn(async () => false), + hostAnswersRepeatedStops: vi.fn(async () => false), toastError: vi.fn(), operations: 0 })) @@ -16,7 +16,7 @@ vi.mock('sonner', () => ({ toast: { error: mocks.toastError, message: vi.fn() } vi.mock('@/runtime/structured-agent-session-client', () => ({ callStructuredAgentSession: mocks.call, - supportsStructuredAgentSessionStopJoin: mocks.hostJoinsStops + supportsStructuredAgentSessionQuietRepeatedStop: mocks.hostAnswersRepeatedStops })) vi.mock('./use-structured-agent-session-outbox', () => ({ @@ -76,7 +76,7 @@ function render() { beforeEach(() => { vi.clearAllMocks() - mocks.hostJoinsStops.mockResolvedValue(false) + mocks.hostAnswersRepeatedStops.mockResolvedValue(false) mocks.operations = 0 }) @@ -203,7 +203,7 @@ function heldStops() { const stop = (current: ReturnType['current'], turnId = 'turn-1') => current.mutate('agentSession.cancel', 'agentSession.cancel', { turnId }) -describe('a Stop pressed again, against a host older than its Stop join', () => { +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() @@ -288,9 +288,9 @@ describe('a Stop pressed again, against a host older than its Stop join', () => }) }) -describe('a Stop pressed again, against a host that joins it', () => { +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.hostJoinsStops.mockResolvedValue(true) + mocks.hostAnswersRepeatedStops.mockResolvedValue(true) const host = heldStops() const { current } = render() 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 70cb5ad23062..7f3b6683a431 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,8 +1,8 @@ // One structured-session mutation, fenced and idempotent. // // Every call is its own action with its own client operation id, so a press is never answered -// from an earlier one. The host joins a Stop of a named turn or task pressed while one is still on -// its way; against an older host that does not, this client joins it instead of stopping twice. +// 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. 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. @@ -20,7 +20,7 @@ import type { RuntimeClientTarget } from '@/runtime/runtime-rpc-client' import { RuntimeRpcCallError } from '@/runtime/runtime-rpc-result' import { callStructuredAgentSession, - supportsStructuredAgentSessionStopJoin + supportsStructuredAgentSessionQuietRepeatedStop } from '@/runtime/structured-agent-session-client' import { structuredSessionOperationId } from './use-structured-agent-session-outbox' import { agentSessionWriteFailureText } from './agent-session-write-notice-text' @@ -139,11 +139,11 @@ export function useStructuredAgentSessionMutate(args: { ): Promise> => { if ( !namesWhatItStops(fingerprintMethod, fields) || - (await supportsStructuredAgentSessionStopJoin(target)) + (await supportsStructuredAgentSessionQuietRepeatedStop(target)) ) { return send(method, fingerprintMethod, fields) } - // Temporary, for a host older than its Stop join: remove once every supported host has it. + // 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) { 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 149fe7b22f03..33591ba702c7 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 633faf0f4226..fb09b9d772de 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 e2050b7077fc..42f40160b7b2 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 @@ -25,6 +25,7 @@ let fence = 3 vi.mock('@/runtime/structured-agent-session-client', () => ({ callStructuredAgentSession: mocks.call, + supportsStructuredAgentSessionQuietRepeatedStop: vi.fn(async () => false), supportsStructuredAgentSessionPromptCancel: vi.fn(async () => false) })) 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 db9f70e7ba94..d272b1332612 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 cd4de967f5d8..d23f5277d28b 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 @@ -211,7 +211,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' @@ -235,7 +236,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 797282ba9c1f..d401ee063640 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', () => ({ diff --git a/src/renderer/src/runtime/structured-agent-session-client.ts b/src/renderer/src/runtime/structured-agent-session-client.ts index 3257b7b2ec83..6cab0f1d7252 100644 --- a/src/renderer/src/runtime/structured-agent-session-client.ts +++ b/src/renderer/src/runtime/structured-agent-session-client.ts @@ -11,7 +11,7 @@ import { AGENT_SESSION_PROMPT_CANCEL_RUNTIME_CAPABILITY, AGENT_SESSION_QUESTION_ANSWERS_RUNTIME_CAPABILITY, AGENT_SESSION_REWIND_RUNTIME_CAPABILITY, - AGENT_SESSION_STOP_JOIN_RUNTIME_CAPABILITY, + AGENT_SESSION_REPEATED_STOP_RUNTIME_CAPABILITY, type RuntimeCapability } from '../../../shared/protocol-version' import { @@ -48,11 +48,11 @@ export function supportsStructuredAgentSessionPromptCancel( return structuredAgentSessionHostSupports(target, AGENT_SESSION_PROMPT_CANCEL_RUNTIME_CAPABILITY) } -/** Whether the host joins a second Stop of a target it is still stopping. */ -export function supportsStructuredAgentSessionStopJoin( +/** Whether the host answers a Stop of a turn an earlier Stop already answered for quietly. */ +export function supportsStructuredAgentSessionQuietRepeatedStop( target: RuntimeClientTarget ): Promise { - return structuredAgentSessionHostSupports(target, AGENT_SESSION_STOP_JOIN_RUNTIME_CAPABILITY) + return structuredAgentSessionHostSupports(target, AGENT_SESSION_REPEATED_STOP_RUNTIME_CAPABILITY) } export function supportsStructuredAgentSessionQuestionAnswers( diff --git a/src/shared/protocol-version.ts b/src/shared/protocol-version.ts index d0cb2c66d6d5..7c40da2a1c32 100644 --- a/src/shared/protocol-version.ts +++ b/src/shared/protocol-version.ts @@ -237,10 +237,11 @@ export const AGENT_SESSION_PROMPT_CANCEL_RUNTIME_CAPABILITY = // `answers` to an older host; they fall back to the answer packed into `optionId`. export const AGENT_SESSION_QUESTION_ANSWERS_RUNTIME_CAPABILITY = 'agent-session.question-answers.v1' as const -// Why: the host joins a second Stop of a turn or task it is still stopping. An older host runs -// both, and the second writes a false "already finished" row, so a client joins them itself there. -// Transitional: drop the client join once no supported host lacks this. -export const AGENT_SESSION_STOP_JOIN_RUNTIME_CAPABILITY = 'agent-session.stop-join.v1' as const +// Why: a Stop of a turn an earlier Stop already answered for adds no row on this host. 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 // Why: the host now publishes rows for work that is live inside a turn, and such // a row carries `stoppable: false` because no targeted stop can reach it. A // reader that predates the field draws a per-row Stop on every row it is given, @@ -418,7 +419,7 @@ export const RUNTIME_CAPABILITIES = [ AGENT_SESSION_BACKGROUND_TASK_STOP_CAPABILITY, AGENT_SESSION_PROMPT_CANCEL_RUNTIME_CAPABILITY, AGENT_SESSION_QUESTION_ANSWERS_RUNTIME_CAPABILITY, - AGENT_SESSION_STOP_JOIN_RUNTIME_CAPABILITY, + AGENT_SESSION_REPEATED_STOP_RUNTIME_CAPABILITY, AGENT_SESSION_TURN_ITEM_CAPABILITY, AGENT_SESSION_BACKGROUND_TASK_ROW_STOP_CAPABILITY, AGENT_SESSION_KIMI_RESUME_RUNTIME_CAPABILITY, From 25541b7fc07f0a396f41325e3a007b1c64905395 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 30 Sep 2026 02:53:47 -0700 Subject: [PATCH 11/17] fix(native-chat): a /compact or /clear pressed again is answered from the one it repeats A /compact from the same caller joins its last /compact while that one waits or runs, and is answered by it once it compacted with nothing sent since, read from the journal, so a retry after a lost reply never compacts twice. The same caller's second /clear waits for the first and is answered by the committed clear. Another caller or another command is still refused. --- ...uctured-conversation-command-controller.ts | 3 +- .../structured-conversation-command.test.ts | 27 +++ ...red-conversation-compaction-repeat.test.ts | 195 ++++++++++++++++++ ...ructured-conversation-compaction-repeat.ts | 41 ++++ .../structured-conversation-compaction.ts | 9 + 5 files changed, 274 insertions(+), 1 deletion(-) create mode 100644 src/main/native-chat/agent-session-wire/structured-conversation-compaction-repeat.test.ts create mode 100644 src/main/native-chat/agent-session-wire/structured-conversation-compaction-repeat.ts diff --git a/src/main/native-chat/agent-session-wire/structured-conversation-command-controller.ts b/src/main/native-chat/agent-session-wire/structured-conversation-command-controller.ts index fae3c944ee26..fc42113851ad 100644 --- a/src/main/native-chat/agent-session-wire/structured-conversation-command-controller.ts +++ b/src/main/native-chat/agent-session-wire/structured-conversation-command-controller.ts @@ -32,7 +32,8 @@ export class StructuredConversationCommandController { if (params.command === 'compact') { return runStructuredCompaction(this.context(), this.host, caller, params) } - const key = JSON.stringify([caller.callerKey, params.envelope.clientOperationId]) + // The same caller's same command waits its turn and is answered from what the first did. + const key = JSON.stringify([caller.callerKey, params.command]) const pending = this.pending.get(params.envelope.sessionId) if (pending && pending.key !== key) { return Promise.resolve({ ok: false as const, refusal: conversationCommandInFlight() }) 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 80ea1a111b99..e545d31ed368 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 @@ -725,6 +725,33 @@ describe('a /clear pressed again after it committed', () => { await expectAnsweredWithTheCommittedClear(replacement!) }) + it('answers a second /clear pressed while the first is still running with that clear', async () => { + const [first, second] = await Promise.all([ + host.conversationCommand(caller, commandParams('clear')), + host.conversationCommand(caller, commandParams('clear')) + ]) + const replacement = first.ok ? first.value.replacementSessionId : undefined + expect(replacement).toBeTruthy() + expect(second).toMatchObject({ + ok: true, + value: { phase: 'committed', state: 'completed', replacementSessionId: replacement } + }) + expect(replacementsOtherThanTheSource()).toEqual([replacement]) + }) + + it("still refuses another window's /clear while this one's is running", async () => { + const [first, other] = await Promise.all([ + host.conversationCommand(caller, commandParams('clear')), + host.conversationCommand({ callerKey: 'mobile' }, commandParams('clear')) + ]) + expect(first).toMatchObject({ ok: true }) + expect(other).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( diff --git a/src/main/native-chat/agent-session-wire/structured-conversation-compaction-repeat.test.ts b/src/main/native-chat/agent-session-wire/structured-conversation-compaction-repeat.test.ts new file mode 100644 index 000000000000..762fa3bb6437 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-conversation-compaction-repeat.test.ts @@ -0,0 +1,195 @@ +// A /compact pressed again under its own id, as every press now is, is answered from the /compact +// it repeats: joined while that one waits or runs, answered once it compacted, and run again only +// once anything newer happened or that one did not compact. + +import { beforeEach, describe, expect, it, vi, type Mock } from 'vitest' +import { AGENT_JOURNAL_THREAD_SCOPE } from '../../../shared/agent-session-journal-types' +import { agentJournalItemKey } from '../../../shared/agent-session-journal-item-key' +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 { readAgentJournalTurn } from '../../../shared/agent-session-turn-record' +import { + HOST_TEST_NOW, + HOST_TEST_SESSION as SESSION, + hostTestMessage +} from './structured-agent-session-host-test-data' +import { startAgent } from './structured-agent-session-restart-interruption-test-harness' + +let state: ReturnType +let compact: Mock> + +beforeEach(() => { + state = hostTestState() + compact = vi.fn(async () => ({ state: 'accepted' as const, providerIdentity: null })) + Object.assign(state.host.deps.adapter, { compact, closeSession: vi.fn(async () => true) }) +}) + +const compactPress = () => ({ + command: 'compact' as const, + envelope: envelope('agentSession.conversationCommand', { command: 'compact' }) +}) + +/** The provider ends the command, as its translator writes it. */ +function finish(outcome: 'success' | 'failure'): void { + const { command } = compact.mock.calls.at(-1)![0] + const events = state.acquire.mock.calls.at(-1)![0].events! + const turnScope = { kind: 'turn' as const, turnItemId: agentJournalItemKey(command.identity) } + events.appendLifecycleBatch!( + `turn-completed:${command.clientMessageId}`, + [ + { + kind: 'item', + identity: command.resultIdentity, + body: + outcome === 'success' + ? { kind: 'status', text: 'Context compacted', presentation: 'compaction' } + : { kind: 'status', text: 'Compaction failed.', tone: 'error' }, + turnScope + }, + { + kind: 'item', + identity: command.identity, + body: { ...command.running, state: 'completed', outcome, completedAt: HOST_TEST_NOW }, + turnScope: AGENT_JOURNAL_THREAD_SCOPE + } + ], + { lifecycle: true } + ) +} + +/** The command's turn as the journal reads it. */ +async function commandTurnState(): Promise { + const { command } = compact.mock.calls.at(-1)![0] + const items = (await state.host.journalSnapshot(SESSION)).items + return readAgentJournalTurn( + items.find((item) => item.itemId === agentJournalItemKey(command.identity))?.body + )?.state +} + +async function ended(outcome: 'success' | 'failure'): Promise { + finish(outcome) + await vi.waitFor(async () => expect(await commandTurnState()).toBe('completed')) +} + +async function compacted(): Promise { + expect(await state.host.conversationCommand(CALLER, compactPress())).toMatchObject({ ok: true }) + await vi.waitFor(() => expect(compact).toHaveBeenCalledOnce()) + await ended('success') +} + +/** Long enough for the delivery loop to have handed a /compact over, had it accepted one. */ +const settle = () => new Promise((resolve) => setTimeout(resolve, 300)) + +describe('a /compact pressed again', () => { + it('while the first is still on its way, joins it: one run and no refusal', async () => { + await attach() + const [first, second] = await Promise.all([ + state.host.conversationCommand(CALLER, compactPress()), + state.host.conversationCommand(CALLER, compactPress()) + ]) + expect(first).toMatchObject({ ok: true, value: { command: 'compact', state: 'completed' } }) + expect(second).toMatchObject({ ok: true, value: { command: 'compact', state: 'completed' } }) + await settle() + expect(compact).toHaveBeenCalledOnce() + }) + + it('while the first runs, answers with it instead of refusing', async () => { + await attach() + await state.host.conversationCommand(CALLER, compactPress()) + await vi.waitFor(() => expect(compact).toHaveBeenCalledOnce()) + + expect(await state.host.conversationCommand(CALLER, compactPress())).toMatchObject({ + ok: true, + value: { command: 'compact', state: 'completed' } + }) + await settle() + expect(compact).toHaveBeenCalledOnce() + }) + + it('after the first compacted with nothing newer, answers with it and runs nothing', async () => { + await attach() + await compacted() + + expect(await state.host.conversationCommand(CALLER, compactPress())).toMatchObject({ + ok: true, + value: { command: 'compact', state: 'completed' } + }) + await settle() + expect(compact).toHaveBeenCalledOnce() + }) + + it('reads the earlier /compact from the journal, so a restarted host answers the same', async () => { + const { root, store } = state + await attach() + await compacted() + await store.renewLeases([]) + const relaunchedStore = await openTestAgentSessionRecordStore(root) + const relaunched = new StructuredAgentSessionHost({ + store: relaunchedStore, + adapter: { ...adapter(), compact }, + journalDatabase: openTestJournalHostDatabase(root), + claimKeyId: 'key-1', + mintSpawnToken: () => 'spawn-next', + probeOwner: async () => ({ outcome: 'pid-absent' }), + now: () => HOST_TEST_NOW + 1 + }) + await relaunched.reconcileRestartLeases() + replaceHostTestState({ store: relaunchedStore, host: relaunched }) + await startAgent({ host: relaunched, store: relaunchedStore }) + + expect(await relaunched.conversationCommand(CALLER, compactPress())).toMatchObject({ + ok: true, + value: { command: 'compact', state: 'completed' } + }) + await settle() + expect(compact).toHaveBeenCalledOnce() + }) + + it('runs again once a newer message was sent', async () => { + await attach() + await compacted() + const body = hostTestMessage('after the compaction') + await state.host.send(CALLER, { envelope: envelope('agentSession.send', { body }), body }) + await vi.waitFor(() => expect(state.dispatch).toHaveBeenCalledOnce()) + + expect(await state.host.conversationCommand(CALLER, compactPress())).toMatchObject({ ok: true }) + await vi.waitFor(() => expect(compact).toHaveBeenCalledTimes(2)) + }) + + it('runs again when the first did not compact', async () => { + await attach() + expect(await state.host.conversationCommand(CALLER, compactPress())).toMatchObject({ ok: true }) + await vi.waitFor(() => expect(compact).toHaveBeenCalledOnce()) + await ended('failure') + + expect(await state.host.conversationCommand(CALLER, compactPress())).toMatchObject({ ok: true }) + await vi.waitFor(() => expect(compact).toHaveBeenCalledTimes(2)) + }) + + it("does not join another window's /compact, or a /clear pressed while one runs", async () => { + await attach() + await state.host.conversationCommand(CALLER, compactPress()) + await vi.waitFor(() => expect(compact).toHaveBeenCalledOnce()) + + expect( + await state.host.conversationCommand({ callerKey: 'mobile' }, compactPress()) + ).toMatchObject({ ok: false, refusal: { details: { reason: 'turnActive' } } }) + expect( + await state.host.conversationCommand(CALLER, { + command: 'clear', + envelope: envelope('agentSession.conversationCommand', { command: 'clear' }) + }) + ).toMatchObject({ ok: false, refusal: { details: { reason: 'turnActive' } } }) + expect(compact).toHaveBeenCalledOnce() + }) +}) diff --git a/src/main/native-chat/agent-session-wire/structured-conversation-compaction-repeat.ts b/src/main/native-chat/agent-session-wire/structured-conversation-compaction-repeat.ts new file mode 100644 index 000000000000..fc6eb803c43d --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-conversation-compaction-repeat.ts @@ -0,0 +1,41 @@ +// Every /compact press carries its own id, so a retry after a lost reply is a new operation. Read +// from the journal, it repeats the caller's last /compact while that one waits, runs, or compacted +// with nothing sent since; a /compact that did not compact, or anything newer, lets it run again. + +import { agentJournalSubmissionKey } from '../../../shared/agent-session-journal-item-key' +import type { AgentJournalSubmission } from '../../../shared/agent-session-journal-types' +import { readAgentJournalTurn } from '../../../shared/agent-session-turn-record' +import type { AgentSessionJournal } from '../agent-session-journal/journal-store' +import { + STRUCTURED_AGENT_SESSION_COMPACT_COMMAND, + structuredAgentSessionCommandTurn +} from './structured-agent-session-command-turn' + +/** The /compact a press repeats, or null when it should run. */ +export function repeatedStructuredCompaction( + journal: Pick, + sentByCaller: (clientMessageId: string) => boolean +): AgentJournalSubmission | null { + const last = journal.submissions().at(-1) + const body = last && journal.itemBody(agentJournalSubmissionKey(last.clientMessageId)) + if ( + !last || + body?.kind !== 'message' || + body.command?.name !== STRUCTURED_AGENT_SESSION_COMPACT_COMMAND || + !sentByCaller(last.clientMessageId) + ) { + return null + } + const commandTurn = structuredAgentSessionCommandTurn(last.clientMessageId) + const turn = readAgentJournalTurn(journal.itemBody(commandTurn.itemId) ?? undefined) + if (!turn) { + // Not handed over yet: it still waits, so the press joins it. + return last.dispatchState === 'pending' ? last : null + } + if (journal.newestTurn()?.turnId !== commandTurn.turnId) { + return null + } + return turn.state === 'running' || (turn.state === 'completed' && turn.outcome === 'success') + ? last + : null +} diff --git a/src/main/native-chat/agent-session-wire/structured-conversation-compaction.ts b/src/main/native-chat/agent-session-wire/structured-conversation-compaction.ts index 7ad7f9bb1bc2..6458e56dd850 100644 --- a/src/main/native-chat/agent-session-wire/structured-conversation-compaction.ts +++ b/src/main/native-chat/agent-session-wire/structured-conversation-compaction.ts @@ -34,6 +34,7 @@ import { type ConversationCommandParams } from './structured-conversation-command' import { conversationCommandBlocked } from './structured-conversation-command-admission' +import { repeatedStructuredCompaction } from './structured-conversation-compaction-repeat' /** * `/compact` from a client that asks through the command RPC: accepted into the conversation like @@ -134,6 +135,14 @@ function acceptStructuredConversationCommand( { ...plan, run: async (ctx) => { + // A press repeating this caller's last /compact is answered as that one is: at its handover. + const earlier = repeatedStructuredCompaction( + ctx.journal, + (id) => context.deps.store.getOperationRow(caller.callerKey, id) !== null + ) + if (earlier) { + return { ok: true, value: { clientMessageId: earlier.clientMessageId } } + } const record = context.deps.store.getRecord(ctx.sessionId) const refusal = record && From f5e0acee4ca946752368249151525a1bdc4becd5 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 30 Sep 2026 11:49:54 -0700 Subject: [PATCH 12/17] Revert "fix(native-chat): a /compact or /clear pressed again is answered from the one it repeats" This reverts commit 25541b7fc07f0a396f41325e3a007b1c64905395. --- ...uctured-conversation-command-controller.ts | 3 +- .../structured-conversation-command.test.ts | 27 --- ...red-conversation-compaction-repeat.test.ts | 195 ------------------ ...ructured-conversation-compaction-repeat.ts | 41 ---- .../structured-conversation-compaction.ts | 9 - 5 files changed, 1 insertion(+), 274 deletions(-) delete mode 100644 src/main/native-chat/agent-session-wire/structured-conversation-compaction-repeat.test.ts delete mode 100644 src/main/native-chat/agent-session-wire/structured-conversation-compaction-repeat.ts diff --git a/src/main/native-chat/agent-session-wire/structured-conversation-command-controller.ts b/src/main/native-chat/agent-session-wire/structured-conversation-command-controller.ts index fc42113851ad..fae3c944ee26 100644 --- a/src/main/native-chat/agent-session-wire/structured-conversation-command-controller.ts +++ b/src/main/native-chat/agent-session-wire/structured-conversation-command-controller.ts @@ -32,8 +32,7 @@ export class StructuredConversationCommandController { if (params.command === 'compact') { return runStructuredCompaction(this.context(), this.host, caller, params) } - // The same caller's same command waits its turn and is answered from what the first did. - const key = JSON.stringify([caller.callerKey, params.command]) + const key = JSON.stringify([caller.callerKey, params.envelope.clientOperationId]) const pending = this.pending.get(params.envelope.sessionId) if (pending && pending.key !== key) { return Promise.resolve({ ok: false as const, refusal: conversationCommandInFlight() }) 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 e545d31ed368..80ea1a111b99 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 @@ -725,33 +725,6 @@ describe('a /clear pressed again after it committed', () => { await expectAnsweredWithTheCommittedClear(replacement!) }) - it('answers a second /clear pressed while the first is still running with that clear', async () => { - const [first, second] = await Promise.all([ - host.conversationCommand(caller, commandParams('clear')), - host.conversationCommand(caller, commandParams('clear')) - ]) - const replacement = first.ok ? first.value.replacementSessionId : undefined - expect(replacement).toBeTruthy() - expect(second).toMatchObject({ - ok: true, - value: { phase: 'committed', state: 'completed', replacementSessionId: replacement } - }) - expect(replacementsOtherThanTheSource()).toEqual([replacement]) - }) - - it("still refuses another window's /clear while this one's is running", async () => { - const [first, other] = await Promise.all([ - host.conversationCommand(caller, commandParams('clear')), - host.conversationCommand({ callerKey: 'mobile' }, commandParams('clear')) - ]) - expect(first).toMatchObject({ ok: true }) - expect(other).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( diff --git a/src/main/native-chat/agent-session-wire/structured-conversation-compaction-repeat.test.ts b/src/main/native-chat/agent-session-wire/structured-conversation-compaction-repeat.test.ts deleted file mode 100644 index 762fa3bb6437..000000000000 --- a/src/main/native-chat/agent-session-wire/structured-conversation-compaction-repeat.test.ts +++ /dev/null @@ -1,195 +0,0 @@ -// A /compact pressed again under its own id, as every press now is, is answered from the /compact -// it repeats: joined while that one waits or runs, answered once it compacted, and run again only -// once anything newer happened or that one did not compact. - -import { beforeEach, describe, expect, it, vi, type Mock } from 'vitest' -import { AGENT_JOURNAL_THREAD_SCOPE } from '../../../shared/agent-session-journal-types' -import { agentJournalItemKey } from '../../../shared/agent-session-journal-item-key' -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 { readAgentJournalTurn } from '../../../shared/agent-session-turn-record' -import { - HOST_TEST_NOW, - HOST_TEST_SESSION as SESSION, - hostTestMessage -} from './structured-agent-session-host-test-data' -import { startAgent } from './structured-agent-session-restart-interruption-test-harness' - -let state: ReturnType -let compact: Mock> - -beforeEach(() => { - state = hostTestState() - compact = vi.fn(async () => ({ state: 'accepted' as const, providerIdentity: null })) - Object.assign(state.host.deps.adapter, { compact, closeSession: vi.fn(async () => true) }) -}) - -const compactPress = () => ({ - command: 'compact' as const, - envelope: envelope('agentSession.conversationCommand', { command: 'compact' }) -}) - -/** The provider ends the command, as its translator writes it. */ -function finish(outcome: 'success' | 'failure'): void { - const { command } = compact.mock.calls.at(-1)![0] - const events = state.acquire.mock.calls.at(-1)![0].events! - const turnScope = { kind: 'turn' as const, turnItemId: agentJournalItemKey(command.identity) } - events.appendLifecycleBatch!( - `turn-completed:${command.clientMessageId}`, - [ - { - kind: 'item', - identity: command.resultIdentity, - body: - outcome === 'success' - ? { kind: 'status', text: 'Context compacted', presentation: 'compaction' } - : { kind: 'status', text: 'Compaction failed.', tone: 'error' }, - turnScope - }, - { - kind: 'item', - identity: command.identity, - body: { ...command.running, state: 'completed', outcome, completedAt: HOST_TEST_NOW }, - turnScope: AGENT_JOURNAL_THREAD_SCOPE - } - ], - { lifecycle: true } - ) -} - -/** The command's turn as the journal reads it. */ -async function commandTurnState(): Promise { - const { command } = compact.mock.calls.at(-1)![0] - const items = (await state.host.journalSnapshot(SESSION)).items - return readAgentJournalTurn( - items.find((item) => item.itemId === agentJournalItemKey(command.identity))?.body - )?.state -} - -async function ended(outcome: 'success' | 'failure'): Promise { - finish(outcome) - await vi.waitFor(async () => expect(await commandTurnState()).toBe('completed')) -} - -async function compacted(): Promise { - expect(await state.host.conversationCommand(CALLER, compactPress())).toMatchObject({ ok: true }) - await vi.waitFor(() => expect(compact).toHaveBeenCalledOnce()) - await ended('success') -} - -/** Long enough for the delivery loop to have handed a /compact over, had it accepted one. */ -const settle = () => new Promise((resolve) => setTimeout(resolve, 300)) - -describe('a /compact pressed again', () => { - it('while the first is still on its way, joins it: one run and no refusal', async () => { - await attach() - const [first, second] = await Promise.all([ - state.host.conversationCommand(CALLER, compactPress()), - state.host.conversationCommand(CALLER, compactPress()) - ]) - expect(first).toMatchObject({ ok: true, value: { command: 'compact', state: 'completed' } }) - expect(second).toMatchObject({ ok: true, value: { command: 'compact', state: 'completed' } }) - await settle() - expect(compact).toHaveBeenCalledOnce() - }) - - it('while the first runs, answers with it instead of refusing', async () => { - await attach() - await state.host.conversationCommand(CALLER, compactPress()) - await vi.waitFor(() => expect(compact).toHaveBeenCalledOnce()) - - expect(await state.host.conversationCommand(CALLER, compactPress())).toMatchObject({ - ok: true, - value: { command: 'compact', state: 'completed' } - }) - await settle() - expect(compact).toHaveBeenCalledOnce() - }) - - it('after the first compacted with nothing newer, answers with it and runs nothing', async () => { - await attach() - await compacted() - - expect(await state.host.conversationCommand(CALLER, compactPress())).toMatchObject({ - ok: true, - value: { command: 'compact', state: 'completed' } - }) - await settle() - expect(compact).toHaveBeenCalledOnce() - }) - - it('reads the earlier /compact from the journal, so a restarted host answers the same', async () => { - const { root, store } = state - await attach() - await compacted() - await store.renewLeases([]) - const relaunchedStore = await openTestAgentSessionRecordStore(root) - const relaunched = new StructuredAgentSessionHost({ - store: relaunchedStore, - adapter: { ...adapter(), compact }, - journalDatabase: openTestJournalHostDatabase(root), - claimKeyId: 'key-1', - mintSpawnToken: () => 'spawn-next', - probeOwner: async () => ({ outcome: 'pid-absent' }), - now: () => HOST_TEST_NOW + 1 - }) - await relaunched.reconcileRestartLeases() - replaceHostTestState({ store: relaunchedStore, host: relaunched }) - await startAgent({ host: relaunched, store: relaunchedStore }) - - expect(await relaunched.conversationCommand(CALLER, compactPress())).toMatchObject({ - ok: true, - value: { command: 'compact', state: 'completed' } - }) - await settle() - expect(compact).toHaveBeenCalledOnce() - }) - - it('runs again once a newer message was sent', async () => { - await attach() - await compacted() - const body = hostTestMessage('after the compaction') - await state.host.send(CALLER, { envelope: envelope('agentSession.send', { body }), body }) - await vi.waitFor(() => expect(state.dispatch).toHaveBeenCalledOnce()) - - expect(await state.host.conversationCommand(CALLER, compactPress())).toMatchObject({ ok: true }) - await vi.waitFor(() => expect(compact).toHaveBeenCalledTimes(2)) - }) - - it('runs again when the first did not compact', async () => { - await attach() - expect(await state.host.conversationCommand(CALLER, compactPress())).toMatchObject({ ok: true }) - await vi.waitFor(() => expect(compact).toHaveBeenCalledOnce()) - await ended('failure') - - expect(await state.host.conversationCommand(CALLER, compactPress())).toMatchObject({ ok: true }) - await vi.waitFor(() => expect(compact).toHaveBeenCalledTimes(2)) - }) - - it("does not join another window's /compact, or a /clear pressed while one runs", async () => { - await attach() - await state.host.conversationCommand(CALLER, compactPress()) - await vi.waitFor(() => expect(compact).toHaveBeenCalledOnce()) - - expect( - await state.host.conversationCommand({ callerKey: 'mobile' }, compactPress()) - ).toMatchObject({ ok: false, refusal: { details: { reason: 'turnActive' } } }) - expect( - await state.host.conversationCommand(CALLER, { - command: 'clear', - envelope: envelope('agentSession.conversationCommand', { command: 'clear' }) - }) - ).toMatchObject({ ok: false, refusal: { details: { reason: 'turnActive' } } }) - expect(compact).toHaveBeenCalledOnce() - }) -}) diff --git a/src/main/native-chat/agent-session-wire/structured-conversation-compaction-repeat.ts b/src/main/native-chat/agent-session-wire/structured-conversation-compaction-repeat.ts deleted file mode 100644 index fc6eb803c43d..000000000000 --- a/src/main/native-chat/agent-session-wire/structured-conversation-compaction-repeat.ts +++ /dev/null @@ -1,41 +0,0 @@ -// Every /compact press carries its own id, so a retry after a lost reply is a new operation. Read -// from the journal, it repeats the caller's last /compact while that one waits, runs, or compacted -// with nothing sent since; a /compact that did not compact, or anything newer, lets it run again. - -import { agentJournalSubmissionKey } from '../../../shared/agent-session-journal-item-key' -import type { AgentJournalSubmission } from '../../../shared/agent-session-journal-types' -import { readAgentJournalTurn } from '../../../shared/agent-session-turn-record' -import type { AgentSessionJournal } from '../agent-session-journal/journal-store' -import { - STRUCTURED_AGENT_SESSION_COMPACT_COMMAND, - structuredAgentSessionCommandTurn -} from './structured-agent-session-command-turn' - -/** The /compact a press repeats, or null when it should run. */ -export function repeatedStructuredCompaction( - journal: Pick, - sentByCaller: (clientMessageId: string) => boolean -): AgentJournalSubmission | null { - const last = journal.submissions().at(-1) - const body = last && journal.itemBody(agentJournalSubmissionKey(last.clientMessageId)) - if ( - !last || - body?.kind !== 'message' || - body.command?.name !== STRUCTURED_AGENT_SESSION_COMPACT_COMMAND || - !sentByCaller(last.clientMessageId) - ) { - return null - } - const commandTurn = structuredAgentSessionCommandTurn(last.clientMessageId) - const turn = readAgentJournalTurn(journal.itemBody(commandTurn.itemId) ?? undefined) - if (!turn) { - // Not handed over yet: it still waits, so the press joins it. - return last.dispatchState === 'pending' ? last : null - } - if (journal.newestTurn()?.turnId !== commandTurn.turnId) { - return null - } - return turn.state === 'running' || (turn.state === 'completed' && turn.outcome === 'success') - ? last - : null -} diff --git a/src/main/native-chat/agent-session-wire/structured-conversation-compaction.ts b/src/main/native-chat/agent-session-wire/structured-conversation-compaction.ts index 6458e56dd850..7ad7f9bb1bc2 100644 --- a/src/main/native-chat/agent-session-wire/structured-conversation-compaction.ts +++ b/src/main/native-chat/agent-session-wire/structured-conversation-compaction.ts @@ -34,7 +34,6 @@ import { type ConversationCommandParams } from './structured-conversation-command' import { conversationCommandBlocked } from './structured-conversation-command-admission' -import { repeatedStructuredCompaction } from './structured-conversation-compaction-repeat' /** * `/compact` from a client that asks through the command RPC: accepted into the conversation like @@ -135,14 +134,6 @@ function acceptStructuredConversationCommand( { ...plan, run: async (ctx) => { - // A press repeating this caller's last /compact is answered as that one is: at its handover. - const earlier = repeatedStructuredCompaction( - ctx.journal, - (id) => context.deps.store.getOperationRow(caller.callerKey, id) !== null - ) - if (earlier) { - return { ok: true, value: { clientMessageId: earlier.clientMessageId } } - } const record = context.deps.store.getRecord(ctx.sessionId) const refusal = record && From 7e950635fda8f12acc6fdae13c46ae1fee79ea1c Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 30 Sep 2026 12:00:22 -0700 Subject: [PATCH 13/17] fix(native-chat): a Stop writes its one note only when it stopped something, keyed by the turn A Stop that cancelled nothing writes no row, and the note is keyed by the turn it stopped, so another Stop of that turn rewrites it. The earlier-note lookup goes. A /compact or /clear pressed again while one runs is refused as before, and one pressed after it ended runs. --- ...d-agent-session-claude-queued-stop.test.ts | 2 +- .../structured-agent-session-command-turn.ts | 8 ++--- ...ed-agent-session-conversation-stop.test.ts | 5 ++- ...ctured-agent-session-prompt-cancel.test.ts | 3 +- ...ctured-agent-session-repeated-stop.test.ts | 21 ++++++------ ...tructured-agent-session-turn-stop-notes.ts | 34 +++++-------------- .../structured-agent-session-turns-cancel.ts | 28 +++++++-------- .../structured-agent-session-turns.test.ts | 8 ++--- .../structured-conversation-command.test.ts | 13 +++++++ ...structured-conversation-compaction.test.ts | 20 +++++++++++ 10 files changed, 74 insertions(+), 68 deletions(-) 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 d26e8f58f13c..706019e288b2 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 @@ -119,11 +119,9 @@ 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. */ 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 2eeae4787ce5..bd91efd01599 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 @@ -319,7 +318,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({ @@ -327,7 +326,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 }> { 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 1c7f280f1757..f2060a1aab09 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 @@ -232,8 +232,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-repeated-stop.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-repeated-stop.test.ts index cbdf66d9f16a..b11b9ad19b1b 100644 --- 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 @@ -1,5 +1,5 @@ -// A Stop naming a turn that an earlier Stop already answered for adds no row, however late it -// lands and from whichever client: the earlier Stop's note, on the turn in the journal, says so. +// 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' @@ -24,7 +24,6 @@ import { import { startAgent } from './structured-agent-session-restart-interruption-test-harness' const REQUESTED = 'Cancellation requested.' -const ALREADY_FINISHED = 'The provider had already finished this turn.' let host: StructuredAgentSessionHost let acquire: Mock @@ -58,8 +57,8 @@ async function statusRows(): Promise { ) } -describe('a Stop of a turn an earlier Stop already answered for', () => { - it('answers quietly once the first Stop has settled, however late it arrives', async () => { +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 } }) @@ -81,7 +80,7 @@ describe('a Stop of a turn an earlier Stop already answered for', () => { expect(await statusRows()).toEqual([REQUESTED]) }) - it('stays quiet after a first Stop that found the turn already over', async () => { + it('writes nothing when neither Stop found anything to stop', async () => { await attach() await turn('running') await turn('completed') @@ -89,10 +88,10 @@ describe('a Stop of a turn an earlier Stop already answered for', () => { await stopTurn() expect(await stopTurn()).toMatchObject({ ok: true, value: { cancelled: false } }) - expect(await statusRows()).toEqual([ALREADY_FINISHED]) + expect(await statusRows()).toEqual([]) }) - it('reads the earlier Stop from the journal, so a restarted host answers the same', async () => { + it('writes nothing more on a restarted host', async () => { const { root, store } = hostTestState() await attach() await turn('running') @@ -120,14 +119,14 @@ describe('a Stop of a turn an earlier Stop already answered for', () => { }) }) -describe('a Stop of a turn no earlier Stop answered for', () => { - it('still says the turn had already finished when it ended by itself', async () => { +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([ALREADY_FINISHED]) + 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 index c00a1f1719e5..676b31797658 100644 --- 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 @@ -1,35 +1,19 @@ -// A Stop's note sits on the turn it named. A later Stop of that turn reads the earlier note from the -// journal, so a repeated press adds no row, however late and from whichever client it arrives. +// 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' -import { isStructuredAgentSessionStopNote } from './structured-agent-session-command-turn' export const STOP_NOTE_CANCELLATION_REQUESTED = 'Cancellation requested.' -export const STOP_NOTE_ALREADY_FINISHED = 'The provider had already finished this turn.' -type TurnScope = Extract - -/** The turn `turnId` names, running or ended, and whether an earlier Stop's note on it already - * said the Stop took effect or found the turn over: all a later Stop of it could say. */ -export function structuredAgentSessionNamedTurnStop( +/** 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 -): { turnScope: TurnScope; answered: boolean } | null { - const items = journal.snapshot().items - const turn = items.findLast((item) => readAgentJournalTurn(item.body)?.turnId === turnId) - if (!turn) { - return null - } - const answered = items.some( - (item) => - item.turnScope?.kind === 'turn' && - item.turnScope.turnItemId === turn.itemId && - isStructuredAgentSessionStopNote(item.itemId) && - item.body.kind === 'status' && - (item.body.text === STOP_NOTE_CANCELLATION_REQUESTED || - item.body.text === STOP_NOTE_ALREADY_FINISHED) - ) - return { turnScope: { kind: 'turn', turnItemId: turn.itemId }, answered } +): 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 eb8f373d7bf8..e36c40021829 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,9 +11,8 @@ import { } from './structured-agent-session-command-turn' import { settledPrompt, validatePendingPrompt } from './structured-agent-session-prompt-state' import { - STOP_NOTE_ALREADY_FINISHED, STOP_NOTE_CANCELLATION_REQUESTED, - structuredAgentSessionNamedTurnStop + 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' @@ -66,17 +65,15 @@ export async function performCancel( } } let cancelled = false - // A turn an earlier Stop already answered for gets no second row, however late this one lands. - const named = - input.turnId !== undefined && !input.scope - ? structuredAgentSessionNamedTurnStop(ctx.journal, input.turnId) - : null - const answeredBefore = named?.answered === true - let note: AgentJournalStatusItem | null = answeredBefore - ? null - : { kind: 'status', text: STOP_NOTE_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 = named?.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. @@ -119,7 +116,8 @@ export async function performCancel( cancelled = true note = null } else if (!cancelled && input.turnId !== undefined) { - note = answeredBefore ? null : { kind: 'status', text: STOP_NOTE_ALREADY_FINISHED } + // Nothing was left to stop: a Stop that ends nothing writes no row. + note = null } else if (!cancelled && input.prompt) { note = null } else if (!cancelled) { @@ -155,9 +153,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.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-turns.test.ts index 2a9ed84012b7..179826e01682 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 @@ -171,8 +171,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.' } + } ]) }) @@ -323,9 +322,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.test.ts b/src/main/native-chat/agent-session-wire/structured-conversation-command.test.ts index 80ea1a111b99..216cf0130dc7 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 @@ -725,6 +725,19 @@ describe('a /clear pressed again after it committed', () => { 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( 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 28eae2dff687..edd1d13d542d 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)) +}) From 8867990f21d7589a6b5df9a1b0dbf66f524810cd Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 30 Sep 2026 12:25:28 -0700 Subject: [PATCH 14/17] chore: the repeated-Stop capability's comments say what it now means --- .../src/session/mobile-structured-agent-session-host-support.ts | 2 +- src/renderer/src/runtime/structured-agent-session-client.ts | 2 +- src/shared/protocol-version.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) 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 e08081ca8da4..9ae1d1e90b1a 100644 --- a/mobile/src/session/mobile-structured-agent-session-host-support.ts +++ b/mobile/src/session/mobile-structured-agent-session-host-support.ts @@ -11,7 +11,7 @@ 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 of a turn an earlier Stop already answered for adds no row. */ + /** A Stop that stopped nothing adds no row, so a repeated Stop is quiet. */ quietRepeatedStop: boolean } diff --git a/src/renderer/src/runtime/structured-agent-session-client.ts b/src/renderer/src/runtime/structured-agent-session-client.ts index 6cab0f1d7252..634781b9388f 100644 --- a/src/renderer/src/runtime/structured-agent-session-client.ts +++ b/src/renderer/src/runtime/structured-agent-session-client.ts @@ -48,7 +48,7 @@ export function supportsStructuredAgentSessionPromptCancel( return structuredAgentSessionHostSupports(target, AGENT_SESSION_PROMPT_CANCEL_RUNTIME_CAPABILITY) } -/** Whether the host answers a Stop of a turn an earlier Stop already answered for quietly. */ +/** Whether the host writes no row for a Stop that stopped nothing, so a repeated Stop is quiet. */ export function supportsStructuredAgentSessionQuietRepeatedStop( target: RuntimeClientTarget ): Promise { diff --git a/src/shared/protocol-version.ts b/src/shared/protocol-version.ts index 7c40da2a1c32..90251f9d8012 100644 --- a/src/shared/protocol-version.ts +++ b/src/shared/protocol-version.ts @@ -237,7 +237,7 @@ export const AGENT_SESSION_PROMPT_CANCEL_RUNTIME_CAPABILITY = // `answers` to an older host; they fall back to the answer packed into `optionId`. export const AGENT_SESSION_QUESTION_ANSWERS_RUNTIME_CAPABILITY = 'agent-session.question-answers.v1' as const -// Why: a Stop of a turn an earlier Stop already answered for adds no row on this host. An older +// 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 = From 4380ad85a9a0510e3ffde53abec4e5b4f3cbf01e Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 30 Sep 2026 15:34:17 -0700 Subject: [PATCH 15/17] test(native-chat): the command-start test's client mock knows the repeated-Stop capability --- ...use-structured-agent-session-command-start-failure.test.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 8700678c030e..bfdc686fc08b 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 @@ -15,7 +15,8 @@ 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: vi.fn(async () => false) })) vi.mock('./use-structured-agent-session-read', () => ({ From 65bde90440e1d14a2021f1acd0594742bdd38089 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 30 Sep 2026 15:36:12 -0700 Subject: [PATCH 16/17] fix(native-chat): a Stop keeps the fence it was pressed at while the client checks the host The desktop's capability check before a named Stop could let the runtime move underneath, so the Stop went out against the new one; it now carries the fence read at the press. --- ...d-agent-session-command-start-failure.test.tsx | 4 ++++ .../use-structured-agent-session-mutate.ts | 15 +++++++++------ 2 files changed, 13 insertions(+), 6 deletions(-) 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 bfdc686fc08b..88905d12e639 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 @@ -308,6 +308,8 @@ describe('any other write across a fence move', () => { 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 }) @@ -323,6 +325,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.ts b/src/renderer/src/components/native-chat/use-structured-agent-session-mutate.ts index a9c60fda1359..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 @@ -82,12 +82,14 @@ export function useStructuredAgentSessionMutate(args: { async ( method: string, fingerprintMethod: string, - fields: Record + fields: Record, + /** 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 @@ -157,11 +159,12 @@ export function useStructuredAgentSessionMutate(args: { fingerprintMethod: string, fields: Record ): Promise> => { + const pressedFence = stateRef.current.fence if ( !namesWhatItStops(fingerprintMethod, fields) || (await supportsStructuredAgentSessionQuietRepeatedStop(target)) ) { - return send(method, fingerprintMethod, fields) + 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)}` @@ -175,7 +178,7 @@ export function useStructuredAgentSessionMutate(args: { (outcome as StructuredAgentSessionWriteOutcome) ) } - const stopping = send(method, fingerprintMethod, fields) + 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(() => { @@ -185,7 +188,7 @@ export function useStructuredAgentSessionMutate(args: { }) return stopping }, - [send, sessionId, target] + [send, sessionId, stateRef, target] ) const mutate = useCallback( From 3e11a8f5e3d6dc01bd571e0f208137a95e590f87 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 30 Sep 2026 15:37:15 -0700 Subject: [PATCH 17/17] test(native-chat): a Stop keeps its pressed fence against either kind of host --- ...ent-session-command-start-failure.test.tsx | 51 +++++++++++-------- 1 file changed, 31 insertions(+), 20 deletions(-) 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 88905d12e639..5651de3cc83e 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,7 +8,11 @@ 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[] = [] @@ -16,7 +20,7 @@ vi.mock('sonner', () => ({ toast: { error: mocks.toastError, message: vi.fn() } vi.mock('@/runtime/structured-agent-session-client', () => ({ callStructuredAgentSession: mocks.call, - supportsStructuredAgentSessionQuietRepeatedStop: vi.fn(async () => false) + supportsStructuredAgentSessionQuietRepeatedStop: mocks.quietRepeatedStop })) vi.mock('./use-structured-agent-session-read', () => ({ @@ -298,24 +302,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 () => { - // 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.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()