From 1c9331e7eace444ee1cf91850d7c3c660cd4bc25 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 15 Sep 2026 14:09:16 -0700 Subject: [PATCH] fix(native-chat): capture awaits-user on the marker instead of re-deriving it The awaits-user clause could never fire. It asked the live projection for `attention`, which needs a prompt whose resolution is still `pending` -- but teardown CANCELS that prompt a few phases after it writes the marker. By the next launch the evidence is gone, for precisely the sessions the clause was written for. QA measured the injection still being offered and then resumed. This is the same shape as the leaf-drift bug: state read after teardown is not the state that justified the marker. The discriminator, now applied across the whole predicate: - a fact teardown itself destroys or mutates must be CAPTURED on the marker while it is still true; - a fact that evolves on its own must be RE-DERIVED at read time, never snapshotted. So `awaitsUser` is now recorded at teardown and the predicate reads the recorded value. Teardown still declines to mint a marker for such a session, so the recorded flag is the second line rather than the only one. Audit of every other clause against the same test: - turn id (captured) -- teardown rewrites turn STATE but never the id. Correct. - provider handle root (captured) -- the close path appends a resumed link, and appendAgentSessionProviderHandleLink refuses one that changes the root, so the root is invariant under exactly the mutation that broke the key. Correct. - turn state (re-derived) -- DELIBERATE exception, stated here rather than left implicit: we are not reading the state that justified the marker, we are reading teardown's receipt that it settled the turn. A turn still `running` means eviction never finished, and we refuse. Correct, and intentionally so. - lease reconciled / released / handoff stage (re-derived) -- these answer a different, launch-time question: may this host take the lease NOW. The teardown-time value would be meaningless, and `unreconciled` is cleared by this launch's own reconciliation. Correct. - adapter support, marker TTL, marker consumption (re-derived) -- all evolve independently of teardown. Correct. Only awaitsUser was on the wrong side. --- ...tured-agent-session-restart-resume-host.ts | 8 +------ ...ctured-agent-session-restart-resume-set.ts | 10 ++++---- ...tured-agent-session-restart-resume.test.ts | 24 +++++++------------ ...tured-agent-session-working-at-teardown.ts | 7 +++++- .../agent-session-resume-marker-store.test.ts | 1 + src/shared/agent-session-resume-marker.ts | 9 +++++++ 6 files changed, 29 insertions(+), 30 deletions(-) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-restart-resume-host.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-restart-resume-host.ts index c9370d76b7f..034af0a13a7 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-restart-resume-host.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-restart-resume-host.ts @@ -9,8 +9,7 @@ import type { AgentSessionRecordStore } from '../../runtime/agent-session-record import type { AgentSessionResumeTrigger } from '../../../shared/agent-session-resume-marker' import { latestStructuredAgentSessionPrompt, - newestStructuredAgentSessionTurn, - projectStructuredAgentSessionStatus + newestStructuredAgentSessionTurn } from '../../../shared/structured-agent-session-projection' import type { AgentSessionResumeMarker } from '../../../shared/agent-session-resume-marker' import type { AgentSessionJournal } from '../agent-session-journal/journal-store' @@ -77,11 +76,6 @@ export function createStructuredAgentSessionRestartResume( getRecord: deps.store.getRecord, supportsRecord: (record) => adapterSupportsRecord(deps.adapter, record), journalTurn: (sessionId) => newestStructuredAgentSessionTurn(itemsFor(sessionId)), - // The projection tests for a pending approval or question BEFORE it looks at turn state, so - // it still reports `attention` after eviction has rewritten the turn to `interrupted`. That - // makes it the durable signal, and the same one teardown gates on. - awaitsUser: (sessionId) => - projectStructuredAgentSessionStatus(itemsFor(sessionId)) === 'attention', latestPrompt: (sessionId) => latestStructuredAgentSessionPrompt(itemsFor(sessionId)), now: surfaces.now(), leaseState diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-restart-resume-set.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-restart-resume-set.ts index e131e91f60c..2c429c75239 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-restart-resume-set.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-restart-resume-set.ts @@ -39,8 +39,6 @@ export type StructuredAgentSessionResumeSetInput = { * not be read. Deliberately not the live-turn reader: eviction has already rewritten that turn * to `interrupted` by the time this runs. */ journalTurn: (sessionId: string) => AgentJournalTurnLifecycle | null - /** Whether the chat is blocked on the USER — a pending approval or question. */ - awaitsUser: (sessionId: string) => boolean latestPrompt: (sessionId: string) => string now: number /** @@ -86,10 +84,10 @@ export function structuredAgentSessionResumableSet( if (turn.state !== 'interrupted' && turn.state !== 'unverifiable') { continue } - // Symmetric with teardown, which already refuses to MINT a marker for a chat awaiting the user. - // Without the same clause here an injected or pre-existing marker would still be offered — the - // exact asymmetry the completed-turn case had. - if (input.awaitsUser(marker.sessionId)) { + // Read off the MARKER, never re-derived. Teardown cancels the pending prompt a few phases after + // it writes the marker, so by now the live journal no longer reports `attention` for exactly the + // sessions this refuses — which is what made the re-derived version inert. + if (marker.awaitsUser) { continue } candidates.push({ diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-restart-resume.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-restart-resume.test.ts index 5888ad4a719..c46aae64019 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-restart-resume.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-restart-resume.test.ts @@ -13,7 +13,6 @@ import { type AgentSessionResumeMarker } from '../../../shared/agent-session-resume-marker' import { newestStructuredAgentSessionTurn } from '../../../shared/structured-agent-session-live-turn' -import { projectStructuredAgentSessionStatus } from '../../../shared/structured-agent-session-projection' import { structuredAgentSessionResumableSet } from './structured-agent-session-restart-resume-set' import { resumeStructuredAgentSessionsFromRestart, @@ -137,6 +136,7 @@ function marker(overrides: Partial = {}): AgentSession recordedAt: NOW, trigger: 'quit', providerHandleRoot: HANDLE_ROOT, + awaitsUser: false, ...overrides } } @@ -153,9 +153,6 @@ function resumableSet(input: { getRecord: () => record(input.chain === undefined ? {} : { chain: input.chain }), supportsRecord: () => true, journalTurn: () => newestStructuredAgentSessionTurn(items), - // Derived from the same items the real host reads, so adding a pending prompt to a fixture - // exercises this exactly as production would. - awaitsUser: () => projectStructuredAgentSessionStatus(items) === 'attention', latestPrompt: () => 'fix the auth bug', now: input.now ?? NOW }) @@ -178,6 +175,7 @@ describe('deriving what was working at teardown', () => { turnId: 'turn-1', recordedAt: NOW, trigger: 'quit', + awaitsUser: false, providerHandleRoot: HANDLE_ROOT } ]) @@ -335,7 +333,6 @@ describe('the resumable set', () => { getRecord: () => claudeRecord('5aed93d6-advanced-leaf'), supportsRecord: () => true, journalTurn: () => ({ turnId: 'turn-1', state: 'interrupted' }), - awaitsUser: () => false, latestPrompt: () => '', now: NOW }) @@ -350,7 +347,6 @@ describe('the resumable set', () => { getRecord: () => claudeRecord(null, 'prov-session-2'), supportsRecord: () => true, journalTurn: () => ({ turnId: 'turn-1', state: 'interrupted' }), - awaitsUser: () => false, latestPrompt: () => '', now: NOW }) @@ -371,16 +367,11 @@ describe('the resumable set', () => { ) }) - // Teardown already refuses to MINT a marker for this, but the set predicate needs the same clause - // or a marker that exists by any other route is offered. The projection still says `attention` - // here because it tests for a pending prompt before it looks at turn state. - it('refuses a marker for a chat that is blocked on the user', () => { - expect( - resumableSet({ - markers: [marker()], - items: [turnItem('turn-1', 'interrupted'), pendingApproval()] - }) - ).toEqual([]) + // The flag is CAPTURED at teardown because teardown then cancels the prompt: by the time this + // predicate runs, the live journal no longer reports `attention`, so only the recorded value can + // still refuse. Re-deriving it here was inert for exactly the sessions it was written for. + it('refuses a marker recorded while the chat was blocked on the user', () => { + expect(resumableSet({ markers: [marker({ awaitsUser: true })] })).toEqual([]) }) it('offers a turn whose end the host could not verify', () => { @@ -548,6 +539,7 @@ describe('the restart-resume surface', () => { turnId: 'turn-2', recordedAt: NOW, trigger: 'update', + awaitsUser: false, providerHandleRoot: HANDLE_ROOT } ]) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-working-at-teardown.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-working-at-teardown.ts index d1f31d48e91..4996ec90143 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-working-at-teardown.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-working-at-teardown.ts @@ -43,10 +43,14 @@ export function structuredAgentSessionsWorkingAtTeardown(input: { continue } const snapshot = session.journal.snapshot() + const status = projectStructuredAgentSessionStatus(snapshot.items, snapshot.submissions) + // Captured HERE, while it is still true. A later teardown phase cancels the pending prompt, so + // nothing downstream can re-derive this fact — the marker has to carry it. + const awaitsUser = status === 'attention' // The product's own classification, so the marker rule cannot disagree with what the UI calls // working. A turn blocked on an approval or a question projects as `attention`: the agent is // waiting on the USER, and that is not interrupted work to hand back. - if (projectStructuredAgentSessionStatus(snapshot.items, snapshot.submissions) !== 'working') { + if (status !== 'working') { continue } const turnId = activeStructuredAgentSessionTurnId(snapshot.items) @@ -64,6 +68,7 @@ export function structuredAgentSessionsWorkingAtTeardown(input: { turnId, recordedAt: input.now, trigger: input.trigger, + awaitsUser, // Root, not key: the close path advances Claude's leaf moments after this runs, and a key // comparison would then refuse the session forever. providerHandleRoot: agentSessionProviderHandleRoot(head.handle) diff --git a/src/main/runtime/agent-session-resume-marker-store.test.ts b/src/main/runtime/agent-session-resume-marker-store.test.ts index 3142fdd09e3..5d77292745e 100644 --- a/src/main/runtime/agent-session-resume-marker-store.test.ts +++ b/src/main/runtime/agent-session-resume-marker-store.test.ts @@ -22,6 +22,7 @@ function marker(overrides: Partial = {}): AgentSession recordedAt: NOW, trigger: 'quit', providerHandleRoot: 'codex:"thread-1"', + awaitsUser: false, ...overrides } } diff --git a/src/shared/agent-session-resume-marker.ts b/src/shared/agent-session-resume-marker.ts index 66639b44d7d..79172fe0b26 100644 --- a/src/shared/agent-session-resume-marker.ts +++ b/src/shared/agent-session-resume-marker.ts @@ -34,6 +34,14 @@ export type AgentSessionResumeMarker = { * preserve — a resume that changes it forked — which is exactly what this guard is for. */ providerHandleRoot: string + /** + * Whether the chat was blocked on the USER — a pending approval or question — at teardown. + * + * CAPTURED, never re-derived, because teardown itself destroys the evidence: a later phase + * cancels the pending prompt, so by the next launch the projection no longer reports `attention` + * for precisely the sessions this exists to refuse. Re-reading it was inert. + */ + awaitsUser: boolean } const MAX_FIELD_LENGTH = 512 @@ -51,6 +59,7 @@ export function isAgentSessionResumeMarker(value: unknown): value is AgentSessio isMarkerField(marker.sessionId) && isMarkerField(marker.turnId) && isMarkerField(marker.providerHandleRoot) && + typeof marker.awaitsUser === 'boolean' && Number.isSafeInteger(marker.recordedAt) && (marker.recordedAt as number) >= 0 && (marker.trigger === 'quit' || marker.trigger === 'update')