From cabdd75064faf481159c9a5791f99e8261a4c934 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 29 Sep 2026 15:52:37 -0700 Subject: [PATCH] fix(native-chat): the draft queue follows #23524's /clear: an unfinished clear holds nothing #23726 and #23524 merged minutes apart. #23726's queue gate takes its "blocked" answer from the shared send check, which #23524 changed: a /clear that never committed changed nothing the chat reads, so it refuses no send. Three #23726 tests still seeded a "prepared" /clear record and expected it to hold the queue, and the /clear carry passed a possibly-undefined replacement id where #23726's helper takes a string, so main failed its unit tests and tc:node. The gate code already follows the new rule; the tests now say it: a /clear an older build left prepared holds no draft and Send-now sends it, and a send made while a /clear is starting its replacement is still told to wait. The carry reads the replacement id from the ids it already checked. --- ...tured-agent-session-queued-command.test.ts | 10 ++------- ...ructured-agent-session-queued-gate.test.ts | 11 +++++----- ...ured-agent-session-queued-messages.test.ts | 21 ++----------------- ...tructured-agent-session-queued-messages.ts | 8 +++---- ...ructured-agent-session-queued-mutations.ts | 3 +-- .../structured-conversation-command.ts | 4 ++-- 6 files changed, 16 insertions(+), 41 deletions(-) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-command.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-command.test.ts index 080cd7acc4dd..9cc22f9547a0 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-command.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-command.test.ts @@ -14,7 +14,6 @@ import { type QueuedMessageTestRig } from './structured-agent-session-queued-message-rig.test-fixture' import { - HOST_TEST_SESSION as SESSION, HOST_TEST_THREAD as THREAD, hostTestMessage, hostTestOperationId @@ -110,13 +109,8 @@ describe('/clear', () => { }) try { const cleared = command('clear') - await eventually(() => - expect(rig.store.getRecord(SESSION)?.conversationCommand).toMatchObject({ - command: 'clear', - phase: 'prepared', - replacementSessionId: expect.any(String) - }) - ) + // Nothing is recorded before the clear commits; wait until it is starting the replacement. + await eventually(() => expect(spy).toHaveBeenCalled()) expect(await rig.send('sent while clearing', 'queue-if-active').result).toEqual(WAIT_REFUSAL) release?.() const done = await cleared diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-gate.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-gate.test.ts index 710d4c73f6d8..bc4697be6df8 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-gate.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-gate.test.ts @@ -41,13 +41,14 @@ const settleRejected: QueuedMessageTestRig['settleRejected'] = (...args) => rig.settleRejected(...args) describe('the one queue gate', () => { - it('Send-now refuses while a clear is in doubt, with the refusal any send gets', async () => { - const working = await workingSend() + it('a clear an older build left prepared holds nothing: Send-now sends the draft', async () => { + await workingSend() const queued = await send('queued behind the clear', 'queue-if-active').result if (!queued.ok || !('queued' in queued.value)) { throw new Error('expected a queued receipt') } const draftId = queued.value.queued.messageId + // An unfinished /clear changed nothing the chat reads, so it refuses no send. await store.setConversationCommand(SESSION, 1, { command: 'clear', runtimeFence: 1, @@ -56,10 +57,8 @@ describe('the one queue gate', () => { phase: 'prepared', state: 'unknown' }) - expect(await sendNow(draftId)).toMatchObject({ ok: false }) - await settleAccepted(working, 'a') - await new Promise((resolve) => setTimeout(resolve, 150)) - expect(await rig.handoff(draftId)).toBeUndefined() + expect(await sendNow(draftId)).toMatchObject({ ok: true }) + await eventually(async () => expect(await rig.handoff(draftId)).toBeDefined()) }) it('Send-now refuses on a pending prompt and overrides a running turn', async () => { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-messages.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-messages.test.ts index c10e9760ffa8..aa9d31cbf105 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-messages.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-messages.test.ts @@ -702,39 +702,22 @@ describe('/clear', () => { expect(await drafts()).toHaveLength(0) }) - it('a draft held by a clear left prepared drains when the retried clear fails with no journal commit', async () => { + it('a clear an older build left prepared holds no draft: it drains when the turn settles', async () => { const working = await workingSend() const queued = await send('behind the clear', 'queue-if-active').result if (!queued.ok || !('queued' in queued.value)) { throw new Error('expected a queued receipt') } const draftId = queued.value.queued.messageId - // A clear that threw left its prepared phase behind; the settling turn's drain step meets it. - const operationId = hostTestOperationId() await store.setConversationCommand(SESSION, 1, { command: 'clear', runtimeFence: 1, - operationId, + operationId: hostTestOperationId(), callerKey: CALLER.callerKey, phase: 'prepared', state: 'unknown' }) await settleAccepted(working, 'a') - await new Promise((resolve) => setTimeout(resolve, 250)) - expect(await rig.handoff(draftId)).toBeUndefined() - // The retried clear fails definitively: it settles on the record alone. - const attach = vi.spyOn(host, 'attach').mockResolvedValueOnce({ - ok: false, - refusal: { code: 'structured_agent_session_unsupported', message: 'unsupported' } - }) - try { - expect(await clear(operationId)).toMatchObject({ - ok: true, - value: { command: 'clear', state: 'completed', error: expect.any(String) } - }) - } finally { - attach.mockRestore() - } await eventually(async () => expect(await rig.handoff(draftId)).toBeDefined()) }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-messages.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-messages.ts index 81676bfa5e0c..273af6dfee65 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-messages.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-messages.ts @@ -92,8 +92,8 @@ function oldestActionableQueuedMessage( * Send-now: overrides only `working` (plus FIFO order and the stored hold); * `blocked` and `prompt` refuse readably. * - * `blocked` is whatever refuses any send (an uncertain rewind, a clear in doubt, - * a cleared source); the rest are waits. A /compact is a queued message and then a turn, + * `blocked` is whatever refuses any send (an uncertain rewind, a cleared source); + * the rest are waits. A /compact is a queued message and then a turn, * so it holds the queue as `working`; an older build's compaction record belongs * to a child this host no longer runs and holds nothing. Host-local vocabulary — * never on the wire. @@ -105,8 +105,8 @@ export function structuredQueueHold(input: { record: AgentSessionRecord | null fence: number }): StructuredQueueHold | null { - // Whatever refuses any send refuses the queue too: an uncertain rewind, a clear in - // doubt, or a source a clear superseded. One rule, the immediate path's own. + // Whatever refuses any send refuses the queue too: an uncertain rewind or a source a + // clear superseded. One rule, the immediate path's own. if (structuredAgentSessionSendBlock(input.record)) { return 'blocked' } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-mutations.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-mutations.ts index aa2c39d59300..89fc2b0c432e 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-mutations.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-mutations.ts @@ -80,8 +80,7 @@ export async function withdrawQueuedMessagesForOperation( * transaction, so the drain never sees a carried card unpaused and no pause is * left over an empty queue if an insert fails. Runs after the * replacement's attach succeeded and before the clear commits. Each insert is - * idempotent on (session, message), so the clear's rerun-while-prepared replays - * it safely; the source rows are then tombstoned. Bookkeeping around the clear: + * idempotent on (session, message), so a retried clear replays it safely; the source rows are then tombstoned. Bookkeeping around the clear: * a failure leaves the cards on the superseded source — whose supersession * fence already blocks the drain — reported, never gating the clear. A crash * between the copy and the tombstone leaves both, which the fence also makes 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 d58f1929f2d5..065ead6e3433 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 @@ -270,8 +270,8 @@ export function runStructuredConversationCommand( // the cards stay visible where the user now is, and no text rides the wire. // Bookkeeping — a failure is reported and never fails the clear. await carryQueuedMessagesToClearReplacement(ctx, { - replacementSessionId, - replacementJournal: context.sessions.get(replacementSessionId)?.journal, + replacementSessionId: ids.sessionId, + replacementJournal: context.sessions.get(ids.sessionId)?.journal, callerKey: caller.callerKey, operationId: clientOperationId })