diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-restart-candidates.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-restart-candidates.ts new file mode 100644 index 00000000000..4a1b348b336 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-restart-candidates.ts @@ -0,0 +1,82 @@ +// Which of a set of markers the predicate would still act on, read off the host's own live journals. +// +// A different question from the claim's: the claim decides which markers are still OWED, this +// decides which of those describe work a resume may touch. It lives apart because both the offer, +// the click and the teardown write-back ask it, each with a different lease expectation. +// +// The per-session journal snapshot is cached for the length of one call: the predicate asks the +// same session for its items four times, and a snapshot that moved between those reads would let +// two clauses judge two different conversations. + +import { agentJournalSubmissionKey } from '../../../shared/agent-session-journal-item-key' +import type { AgentJournalRenderItem } from '../../../shared/agent-session-journal-types' +import type { AgentSessionRecord } from '../../../shared/agent-session-record' +import type { AgentSessionResumeMarker } from '../../../shared/agent-session-resume-marker' +import { + latestStructuredAgentSessionPrompt, + latestStructuredAgentSessionUserItem, + newestStructuredAgentSessionTurn, + projectStructuredAgentSessionStatus +} from '../../../shared/structured-agent-session-projection' +import type { AgentSessionJournal } from '../agent-session-journal/journal-store' +import type { StructuredAgentSessionAdapter } from './structured-agent-session-adapter' +import { adapterSupportsRecord } from './structured-agent-session-provider-support' +import { + structuredAgentSessionResumableSet, + type StructuredAgentSessionResumeCandidate +} from './structured-agent-session-restart-resume-set' + +/** The only part of a live session this reads. */ +export type StructuredAgentSessionRestartJournalSource = { journal: AgentSessionJournal } + +export type StructuredAgentSessionRestartCandidateReader = ( + markers: readonly AgentSessionResumeMarker[], + leaseState: 'must-be-released' | 'may-be-held', + /** Only teardown may judge before it rewrites the stopped child's running turn. */ + providerStopped?: boolean, + /** A continuation already in flight; its own submission is not newer user work. */ + pendingContinuationId?: string +) => StructuredAgentSessionResumeCandidate[] + +export function createStructuredAgentSessionRestartCandidateReader(deps: { + /** The host's LIVE session map — the only honest answer to "was this actually working". */ + sessions: ReadonlyMap + getRecord: (sessionId: string) => AgentSessionRecord | null + adapter: StructuredAgentSessionAdapter + now: () => number +}): StructuredAgentSessionRestartCandidateReader { + return (markers, leaseState, providerStopped = false, pendingContinuationId) => { + const items = new Map() + const itemsFor = (sessionId: string): AgentJournalRenderItem[] => { + let snapshot = items.get(sessionId) + if (!snapshot) { + snapshot = deps.sessions.get(sessionId)?.journal.snapshot().items ?? [] + if (pendingContinuationId) { + const ownItemId = agentJournalSubmissionKey(pendingContinuationId) + snapshot = snapshot.filter((item) => item.itemId !== ownItemId) + } + items.set(sessionId, snapshot) + } + return snapshot + } + return structuredAgentSessionResumableSet({ + markers, + getRecord: deps.getRecord, + supportsRecord: (record) => adapterSupportsRecord(deps.adapter, record), + waitingOnUser: (sessionId) => + projectStructuredAgentSessionStatus(itemsFor(sessionId)) === 'attention', + providerStopped, + journalTurn: (sessionId) => newestStructuredAgentSessionTurn(itemsFor(sessionId)), + journalSubmission: (sessionId, clientMessageId) => + deps.sessions + .get(sessionId) + ?.journal.submissions() + .find((submission) => submission.clientMessageId === clientMessageId) ?? null, + latestPrompt: (sessionId) => latestStructuredAgentSessionPrompt(itemsFor(sessionId)), + latestUserItemId: (sessionId) => + latestStructuredAgentSessionUserItem(itemsFor(sessionId))?.itemId ?? null, + now: deps.now(), + leaseState + }) + } +} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-restart-claim.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-restart-claim.test.ts index 11dfdebcce2..b1a3d8815fe 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-restart-claim.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-restart-claim.test.ts @@ -335,11 +335,11 @@ describe('the restart-resume surface', () => { }) // A launch that never read the offer must not answer for it — the flag was off, the first read - // failed, the window never mounted. Nothing revealed those sessions, so nothing here can judge - // them, and replacing the capsule with this launch's empty list deletes a recovery the user was - // never shown. + // failed, the window never mounted. NO SESSION IS INDEXED here, which is the whole point: that is + // what an unread offer looks like, and re-deriving against it refuses every marker, which is + // indistinguishable from deleting a recovery the user was never shown. it('leaves a durable offer this launch never claimed intact at teardown', async () => { - const { restartResume, live, recorded } = surface({}) + const { restartResume, live, recorded } = surface({ sessions: new Map() }) restartResume.captureMarkers('quit') await restartResume.recordMarkers() @@ -348,6 +348,62 @@ describe('the restart-resume surface', () => { expect([...live.keys()]).toEqual([SESSION]) }) + // Teardown is RETRIED when a phase fails, and its own write-back read must not make the second + // attempt look like a launch that had read the offer — that would re-derive against a session map + // eviction has already emptied, and answer "nothing is resumable" for every carried marker. + it('carries the same unread offer again when teardown is repeated', async () => { + const { restartResume, live, recorded } = surface({ sessions: new Map() }) + + await restartResume.recordMarkers() + await restartResume.recordMarkers() + + expect(recorded.map((entry) => entry.map((marker) => marker.sessionId))).toEqual([ + [SESSION], + [SESSION] + ]) + expect([...live.keys()]).toEqual([SESSION]) + }) + + // A take that failed is not an empty offer. The markers are still on disk, unread and unknowable, + // and an empty write over them deletes exactly the recovery nobody was shown. + it('writes nothing over a durable offer it could not read', async () => { + const warning = vi.spyOn(console, 'warn').mockImplementation(() => {}) + try { + const { restartResume, live, recorded } = surface({ clearFails: true, sessions: new Map() }) + + restartResume.captureMarkers('quit') + await restartResume.recordMarkers() + + expect(recorded).toEqual([]) + expect([...live.keys()]).toEqual([SESSION]) + } finally { + warning.mockRestore() + } + }) + + // ...but a witness from THIS teardown still has to reach disk, whatever the take did. + it('still records this teardown witness after a take it could not read', async () => { + const warning = vi.spyOn(console, 'warn').mockImplementation(() => {}) + try { + const { restartResume, recorded } = surface({ + clearFails: true, + sessions: new Map([ + [SESSION, { journal: journal([turnItem('turn-2', 'running')]), hasProviderChild: true }] + ]) + }) + + restartResume.captureMarkers('update') + restartResume.confirmStoppedMarker(SESSION) + await restartResume.recordMarkers() + + expect(recorded[0]).toMatchObject([ + { sessionId: SESSION, work: { kind: 'turn', id: 'turn-2' } } + ]) + } finally { + warning.mockRestore() + } + }) + it('persists a snoozed claimed offer across the next teardown', async () => { const { restartResume, recorded } = surface({}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-restart-claim.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-restart-claim.ts index 0faaba20434..1a102ae184e 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-restart-claim.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-restart-claim.ts @@ -23,14 +23,24 @@ export type StructuredAgentSessionRestartClaim = { /** What the host still advertises: evidence minus recovery. */ offered: () => AgentSessionResumeMarker[] /** - * What teardown must write back, and whether this launch ever claimed it. + * What teardown must write back, whether this launch ever READ the offer, and whether the durable + * copy could be read at all. * * `claimed: false` is not "nothing is owed" — the durable copy is still intact and nothing here * revealed it, so it is taken now and must be carried forward VERBATIM. Re-deriving it would * refuse every marker for want of a journal nobody opened, which is indistinguishable from - * deleting an offer the user was never shown. + * deleting an offer the user was never shown. Reading it HERE is not reading it: the flag stays + * false, so a teardown repeated after a failed one carries the same offer forward again instead + * of answering for a session map that has since been emptied. + * + * `unreadable: true` means the take itself failed. The durable copy is then intact AND unknowable, + * so nothing here may answer for it. */ - owed: () => Promise<{ markers: AgentSessionResumeMarker[]; claimed: boolean }> + owed: () => Promise<{ + markers: AgentSessionResumeMarker[] + claimed: boolean + unreadable: boolean + }> /** Acted on. In memory, because the durable copy is already gone. */ spend: (sessionId: string) => boolean /** This launch handed the provider back; see the header. */ @@ -47,6 +57,10 @@ export function createStructuredAgentSessionRestartClaim(deps: { }): StructuredAgentSessionRestartClaim { let claimed: AgentSessionResumeMarker[] | null = null let claiming: Promise | undefined + /** Anything in this launch LOOKED at the offer. Teardown's own write-back read is not a reader. */ + let read = false + /** The take failed, so an empty `claimed` says nothing about what is still on disk. */ + let unreadable = false const recovered = new Set() const claim = async (): Promise => { @@ -56,6 +70,7 @@ export function createStructuredAgentSessionRestartClaim(deps: { } catch { console.warn('[structured-agent-session] taking recovery capsule failed') claimed = [] + unreadable = true } })() await claiming @@ -67,6 +82,7 @@ export function createStructuredAgentSessionRestartClaim(deps: { return { evidence: async () => { + read = true for (const marker of await claim()) { if (!deps.isOpen(marker.sessionId)) { await deps.open(marker.sessionId).catch(() => null) @@ -76,9 +92,9 @@ export function createStructuredAgentSessionRestartClaim(deps: { }, offered, owed: async () => { - const claimed = claiming !== undefined + const readBeforeThisTake = read await claim() - return { markers: offered(), claimed } + return { markers: offered(), claimed: readBeforeThisTake, unreadable } }, spend: (sessionId) => { const before = claimed?.length ?? 0 @@ -93,6 +109,7 @@ export function createStructuredAgentSessionRestartClaim(deps: { } }, abandon: async () => { + read = true await claim() const spent = offered().length claimed = [] 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 b1b58643121..c8034b0af17 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 @@ -2,23 +2,13 @@ // still owes is written back during orderly teardown; a crash after take can still lose it. import { randomUUID } from 'node:crypto' -import { agentJournalSubmissionKey } from '../../../shared/agent-session-journal-item-key' import type { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' import type { AgentSessionRecoveryCapsule } from '../../runtime/agent-session-recovery-capsule' import type { AgentSessionResumeMarker, AgentSessionResumeTrigger } from '../../../shared/agent-session-resume-marker' -import { - latestStructuredAgentSessionPrompt, - latestStructuredAgentSessionUserItem, - newestStructuredAgentSessionTurn, - projectStructuredAgentSessionStatus -} from '../../../shared/structured-agent-session-projection' -import type { - AgentJournalMessageItem, - AgentJournalRenderItem -} from '../../../shared/agent-session-journal-types' +import type { AgentJournalMessageItem } from '../../../shared/agent-session-journal-types' import type { AgentSessionMutationEnvelope, AgentSessionMutationResult, @@ -26,12 +16,9 @@ import type { } from '../../../shared/agent-session-wire' import type { AgentSessionJournal } from '../agent-session-journal/journal-store' import type { StructuredAgentSessionAdapter } from './structured-agent-session-adapter' -import { adapterSupportsRecord } from './structured-agent-session-provider-support' +import { createStructuredAgentSessionRestartCandidateReader } from './structured-agent-session-restart-candidates' import { createStructuredAgentSessionRestartClaim } from './structured-agent-session-restart-claim' -import { - structuredAgentSessionResumableSet, - type StructuredAgentSessionResumeCandidate -} from './structured-agent-session-restart-resume-set' +import type { StructuredAgentSessionResumeCandidate } from './structured-agent-session-restart-resume-set' import { resumeStructuredAgentSessionsFromRestart, StructuredAgentSessionResumeAdmission, @@ -117,45 +104,14 @@ export function createStructuredAgentSessionRestartResume( open: (sessionId) => surfaces.revealSession(sessionId) }) - const derive = ( - markers: readonly AgentSessionResumeMarker[], - leaseState: 'must-be-released' | 'may-be-held', - providerStopped = false, - pendingContinuationId?: string - ): StructuredAgentSessionResumeCandidate[] => { - const items = new Map() - const itemsFor = (sessionId: string): AgentJournalRenderItem[] => { - let snapshot = items.get(sessionId) - if (!snapshot) { - snapshot = sessions.get(sessionId)?.journal.snapshot().items ?? [] - if (pendingContinuationId) { - const ownItemId = agentJournalSubmissionKey(pendingContinuationId) - snapshot = snapshot.filter((item) => item.itemId !== ownItemId) - } - items.set(sessionId, snapshot) - } - return snapshot - } - return structuredAgentSessionResumableSet({ - markers, - getRecord: deps.store.getRecord, - supportsRecord: (record) => adapterSupportsRecord(deps.adapter, record), - waitingOnUser: (sessionId) => - projectStructuredAgentSessionStatus(itemsFor(sessionId)) === 'attention', - providerStopped, - journalTurn: (sessionId) => newestStructuredAgentSessionTurn(itemsFor(sessionId)), - journalSubmission: (sessionId, clientMessageId) => - sessions - .get(sessionId) - ?.journal.submissions() - .find((submission) => submission.clientMessageId === clientMessageId) ?? null, - latestPrompt: (sessionId) => latestStructuredAgentSessionPrompt(itemsFor(sessionId)), - latestUserItemId: (sessionId) => - latestStructuredAgentSessionUserItem(itemsFor(sessionId))?.itemId ?? null, - now: surfaces.now(), - leaseState - }) - } + // The predicate's reader, built once: the offer, the click and the write-back all ask it, and a + // second copy is how two of them come to disagree about what is resumable. + const derive = createStructuredAgentSessionRestartCandidateReader({ + sessions, + getRecord: deps.store.getRecord, + adapter: deps.adapter, + now: surfaces.now + }) const list = async (): Promise => { await claim.evidence() @@ -307,10 +263,18 @@ export function createStructuredAgentSessionRestartResume( // capsule instead of just replacing it: those sessions were never revealed, so the predicate // has no journal to judge them by and would refuse every one. Answering for an offer nobody // read is how it gets deleted unseen, so it carries forward untouched. + // + // A take that FAILED is the third case. The durable copy is then intact and unknowable, so + // only this teardown's own witnesses may go out — and when it has none, writing at all would + // delete an offer no one here could even see. let carried: AgentSessionResumeMarker[] = [] try { const owed = await claim.owed() - if (!owed.claimed) { + if (owed.unreadable) { + if (confirmedMarkers.size === 0) { + return + } + } else if (!owed.claimed) { carried = owed.markers } else { const stillResumable = new Set( diff --git a/src/renderer/src/components/NativeChatResumeOnRestartModal.test.tsx b/src/renderer/src/components/NativeChatResumeOnRestartModal.test.tsx index aae036bc758..52ccc901e40 100644 --- a/src/renderer/src/components/NativeChatResumeOnRestartModal.test.tsx +++ b/src/renderer/src/components/NativeChatResumeOnRestartModal.test.tsx @@ -87,8 +87,7 @@ afterEach(() => { it.each([ ['Reconnect 1', 'agentSession.restartResume'], - ['Reconnect and continue', 'agentSession.restartContinue'], - ['Not now', 'agentSession.restartResumableDismiss'] + ['Reconnect and continue', 'agentSession.restartContinue'] ])('keeps next-launch preference out of the current %s action', async (label, method) => { const action = Promise.withResolvers() rpc.mockImplementation(async (_target, calledMethod) => { @@ -102,30 +101,30 @@ it.each([ await act(async () => checkbox(2).click()) await act(async () => button(label).click()) expect(useAppStore.getState().settings?.nativeChatResumeWorkOnRestart).toBe(true) - expect(rpc.mock.calls.map((call) => [call[1], call[2]])).toEqual( - method === 'agentSession.restartResumableDismiss' - ? [['agentSession.restartResumable', undefined]] - : [ - ['agentSession.restartResumable', undefined], - [method, { sessionIds: ['a'] }] - ] - ) + expect(rpc.mock.calls.map((call) => [call[1], call[2]])).toEqual([ + ['agentSession.restartResumable', undefined], + [method, { sessionIds: ['a'] }] + ]) await act(async () => action.resolve({ results: [{ sessionId: 'a', outcome: 'resumed' }], continued: [{ sessionId: 'a', outcome: 'continued' }] }) ) - expect(rpc).toHaveBeenCalledTimes(method === 'agentSession.restartResumableDismiss' ? 1 : 2) + expect(rpc).toHaveBeenCalledTimes(2) }) +// Snoozing saves the preference like every other way out of the dialog, and calls NOTHING: the +// offer is the host's and stays exactly where it was. it('keeps Not now available through the status-bar offer', async () => { rpc.mockImplementation(async (_target, method) => method === 'agentSession.restartResumable' ? { sessions: offered } : { results: [] } ) await act(async () => root.render()) + await act(async () => checkbox(2).click()) await act(async () => button('Not now').click()) - expect(rpc).toHaveBeenCalledTimes(1) + expect(useAppStore.getState().settings?.nativeChatResumeWorkOnRestart).toBe(true) + expect(rpc.mock.calls.map((call) => call[1])).toEqual(['agentSession.restartResumable']) expect(offerIds()).toEqual(['a', 'b']) expect(document.querySelector('[role="dialog"]')).toBeNull() })