fix(notes): returned text reaches the draft before its notes are cleared

When a message carrying notes came back to the composer, the notes were cleared first and the
text written to the draft after. A crash between the two could lose both. The text is now saved
to the draft first, on every path that hands it back (the host's answer, the journal, a Stop).
This commit is contained in:
Brennan Benson
2026-10-04 05:07:41 -07:00
parent dac2cac7c8
commit 862a444064
4 changed files with 38 additions and 14 deletions
@@ -93,11 +93,13 @@ export function requeueInterruptedStructuredAgentSessionDispatches(
/**
* Commits a settlement: a message coming back goes to its conversation's draft before its entry
* leaves the outbox, so a failure between the two repeats the text and never loses it.
* ends (which clears the notes it carried) and before it leaves the outbox, so a failure between
* them repeats the text and never loses it.
*/
export function commitStructuredAgentSessionSettledOutbox(
sessionId: string,
settled: StructuredAgentSessionSettledOutbox
settled: StructuredAgentSessionSettledOutbox,
endEntry: () => void = () => {}
): void {
if (settled.returned) {
returnStructuredAgentSessionMessage(settled.returned.entry)
@@ -105,6 +107,7 @@ export function commitStructuredAgentSessionSettledOutbox(
setStructuredAgentSessionChatLine(sessionId, settled.returned.words)
}
}
endEntry()
commitStructuredAgentSessionOutbox(sessionId, settled.entries)
}
@@ -154,9 +157,6 @@ export function settleStructuredAgentSessionOutboxEntry(
return
}
const ending = structuredAgentSessionSettlementEnding(settlement)
if (ending) {
endStructuredAgentSessionEntry(entry, ending)
}
// A send a Stop outran never goes again: no answer leaves it waiting for the Stop's.
const kept =
entry.stoppedBy !== undefined && settlement.kind === 'unanswered'
@@ -164,7 +164,12 @@ export function settleStructuredAgentSessionOutboxEntry(
: settlement
commitStructuredAgentSessionSettledOutbox(
sessionId,
applyStructuredAgentSessionSendSettlement(current, clientMessageId, kept)
applyStructuredAgentSessionSendSettlement(current, clientMessageId, kept),
() => {
if (ending) {
endStructuredAgentSessionEntry(entry, ending)
}
}
)
sayStructuredAgentSessionSettlement(sessionId, clientMessageId, kept)
}
@@ -55,19 +55,20 @@ export function settleStructuredAgentSessionOutboxFromJournal(
if (entries === current) {
return entries
}
for (const { entry, settlement } of settled) {
const ending = structuredAgentSessionSettlementEnding(settlement)
if (ending) {
endStructuredAgentSessionEntry(entry, ending)
}
}
// Each returned message goes to the draft before the outbox that drops it is saved.
// Each returned message goes to the draft before its entry ends (clearing the notes it carried)
// and before the outbox that drops it is saved.
for (const back of returned) {
returnStructuredAgentSessionMessage(back.entry)
if (back.words) {
setStructuredAgentSessionChatLine(sessionId, back.words)
}
}
for (const { entry, settlement } of settled) {
const ending = structuredAgentSessionSettlementEnding(settlement)
if (ending) {
endStructuredAgentSessionEntry(entry, ending)
}
}
commitStructuredAgentSessionOutbox(sessionId, entries)
for (const { entry, settlement } of settled) {
sayStructuredAgentSessionSettlement(sessionId, entry.clientMessageId, settlement)
@@ -295,8 +295,9 @@ export function useStructuredAgentSessionOutbox(args: {
return
}
for (const entry of next.withdrawn) {
endStructuredAgentSessionEntry(entry, 'returned')
// The draft holds the text before the notes it carried are cleared.
returnStructuredAgentSessionMessage(entry)
endStructuredAgentSessionEntry(entry, 'returned')
// Back in the composer, it is no longer being sent, so what the line said about it goes.
clearStructuredAgentSessionChatLineHeldBy(sessionId, entry.clientMessageId)
}
@@ -224,6 +224,17 @@ function openChat(sessionId: string, fence: number | null = 1) {
)
}
/** What the session's draft held each time notes were cleared: the text must already be there,
* so a crash between the two never loses both. */
function draftWhenNotesClear(sessionId: string): string[] {
const seen: string[] = []
mocks.clearDeliveredDiffComments.mockImplementation(async () => {
seen.push(readNativeChatDraftCache(structuredAgentSessionDraftScopeKey(sessionId)))
return true
})
return seen
}
/** A store change, as a workspace's notes loading makes. */
function notifyStore(): void {
for (const listener of mocks.storeListeners) {
@@ -301,6 +312,7 @@ describe('notes sent to a new agent', () => {
reload()
expect(isNoteInFlight(KEY_A)).toBe(true)
const cleared = draftWhenNotesClear(chat.sessionId)
openChat(chat.sessionId)
await waitFor(() => readOutbox(chat.sessionId).length === 0)
expect(isNoteInFlight(KEY_A)).toBe(false)
@@ -309,6 +321,7 @@ describe('notes sent to a new agent', () => {
)
// One owner: the draft holds the text, so the notes leave the shelf.
expect(mocks.clearDeliveredDiffComments).toHaveBeenCalledExactlyOnceWith(WORKTREE_ID, [NOTE_A])
expect(cleared).toEqual([NOTES])
})
it('stay held while a failed chat keeps them to start again, and leave once it delivers', async () => {
@@ -373,6 +386,7 @@ describe('notes sent to a chat already open', () => {
})
expect(isNoteInFlight(KEY_A)).toBe(true)
const cleared = draftWhenNotesClear(target.sessionId)
mocks.callStructuredAgentSession.mockResolvedValue(REFUSED)
openChat(target.sessionId)
await waitFor(() => readOutbox(target.sessionId).length === 0)
@@ -381,6 +395,7 @@ describe('notes sent to a chat already open', () => {
NOTES
)
expect(mocks.clearDeliveredDiffComments).toHaveBeenCalledExactlyOnceWith(WORKTREE_ID, [NOTE_A])
expect(cleared).toEqual([NOTES])
})
it('are used when a Stop takes back the message before it went out', async () => {
@@ -390,6 +405,7 @@ describe('notes sent to a chat already open', () => {
target,
carriedNoteKeys: [KEY_A]
})
const cleared = draftWhenNotesClear(target.sessionId)
const view = openChat(target.sessionId, null)
view.result.current.stop('stop-1')
@@ -399,6 +415,7 @@ describe('notes sent to a chat already open', () => {
)
expect(isNoteInFlight(KEY_A)).toBe(false)
expect(mocks.clearDeliveredDiffComments).toHaveBeenCalledExactlyOnceWith(WORKTREE_ID, [NOTE_A])
expect(cleared).toEqual([NOTES])
})
it('are cleared once their workspace loads, when the host took the message before that', async () => {