Skip to content

Commit 6de1280

Browse files
committed
fix(chat): close the re-audit's delivery gaps
- a message not delivered whose id became a turn in the chat is dropped, not shown twice, with or without a streamId, and a history read wakes the settle for it - a fork neither cuts at nor copies a steering message no turn applied, so the worker's cut cannot 404 and a copy cannot hold a dead bubble - a journal entry stamps writtenAt; one still on its way past the age limit restores as not delivered, with no network call - a dismiss the worker answers with a hash conflict removes the record and the message - the storage adapter names sessionStorage inside its try, including at module load - disposition reads get their own counter again: a message read as not applied goes out without inheriting read backoff, and reads leave the send budget alone
1 parent af97413 commit 6de1280

12 files changed

Lines changed: 429 additions & 40 deletions

File tree

‎apps/sim/app/workspace/[workspaceId]/home/hooks/outgoing-state.test.ts‎

Lines changed: 75 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {
2121
fromJournaled,
2222
hasOutgoingFollowUp,
2323
holdAfterErroredTurn,
24+
isDeliveredAsTurn,
2425
MAX_OUTGOING_DELIVERY_ATTEMPTS,
2526
nextOutgoingStep,
2627
ORPHAN_AUTO_SEND_MAX_AGE_MS,
@@ -40,6 +41,8 @@ function entry(id: string, patch: Partial<OutgoingMessage> = {}): OutgoingMessag
4041
display: { id, role: 'user', content: id },
4142
status: 'pending',
4243
attempts: 0,
44+
reads: 0,
45+
writtenAt: NOW,
4346
...patch,
4447
}
4548
}
@@ -222,22 +225,67 @@ describe('transitions', () => {
222225
).toBe('failed')
223226
})
224227

225-
it('reads a held message again later, counting toward the one delivery cap', () => {
228+
it('reads a held message again later, and gives up at its own cap', () => {
226229
const context = { streamId: 'turn', turnErrored: false, now: NOW }
227230
const steered = entry('a', { status: 'steered', streamId: 'turn' })
228231
const later = afterDispositionRead(steered, { status: 'queued' }, context)
229-
expect(later).toMatchObject({ attempts: 1, unconfirmed: false })
232+
expect(later).toMatchObject({ reads: 1, unconfirmed: false })
230233
expect(later.notBefore).toBeGreaterThan(NOW)
231234
expect(afterDispositionRead(steered, { status: 'unknown' }, context).unconfirmed).toBe(true)
232235
expect(
233236
afterDispositionRead(
234-
{ ...steered, attempts: MAX_OUTGOING_DELIVERY_ATTEMPTS - 1 },
237+
{ ...steered, reads: MAX_OUTGOING_DELIVERY_ATTEMPTS - 1 },
235238
{ status: 'queued' },
236239
context
237240
).status
238241
).toBe('failed')
239242
})
240243

244+
it('sends a message read as not applied at once, whatever the reads before it took', () => {
245+
const steered = entry('a', { status: 'steered', streamId: 'turn', reads: 6 })
246+
const patch = afterDispositionRead(
247+
steered,
248+
{ status: 'not_applied' },
249+
{ streamId: 'turn', turnErrored: false, now: NOW }
250+
)
251+
expect(patch.status).toBe('pending')
252+
expect(patch.notBefore).toBeUndefined()
253+
})
254+
255+
it('still backs off a message the chat kept refusing as busy', () => {
256+
const steered = entry('a', { status: 'steered', streamId: 'turn', attempts: 2 })
257+
const { notBefore } = afterDispositionRead(
258+
steered,
259+
{ status: 'not_applied' },
260+
{ streamId: 'turn', turnErrored: false, now: NOW }
261+
)
262+
expect(notBefore).toBeGreaterThan(NOW)
263+
})
264+
265+
it('leaves reads out of the budget for sends', () => {
266+
const read = afterDispositionRead(
267+
entry('a', { status: 'steered', streamId: 'turn' }),
268+
{ status: 'queued' },
269+
{ streamId: 'turn', turnErrored: false, now: NOW }
270+
)
271+
expect(read.attempts).toBeUndefined()
272+
const sent = afterUnsentAttempt(entry('a', { reads: MAX_OUTGOING_DELIVERY_ATTEMPTS }), {
273+
online: true,
274+
now: NOW,
275+
})
276+
expect(sent).toMatchObject({ status: 'pending', attempts: 1 })
277+
})
278+
279+
it('counts a message not delivered or steered as delivered once its id is a turn', () => {
280+
const admitted = new Set(['a'])
281+
expect(isDeliveredAsTurn(entry('a', { status: 'failed' }), admitted)).toBe(true)
282+
expect(isDeliveredAsTurn(entry('a', { status: 'failed', streamId: 'turn' }), admitted)).toBe(
283+
true
284+
)
285+
expect(isDeliveredAsTurn(entry('a', { status: 'steered' }), admitted)).toBe(true)
286+
expect(isDeliveredAsTurn(entry('b', { status: 'failed' }), admitted)).toBe(false)
287+
})
288+
241289
it('reads a steered message again on Retry, and sends one that never reached a turn', () => {
242290
expect(afterRetry(entry('a', { status: 'failed', streamId: 'turn' })).status).toBe('steered')
243291
expect(afterRetry(entry('a', { status: 'failed', refusedBy: 'turn' })).status).toBe('pending')
@@ -268,7 +316,7 @@ describe('transitions', () => {
268316
describe('journal', () => {
269317
it('restores a send or steer that was out as possibly held, so it cannot steer again', () => {
270318
for (const status of ['sending', 'steering'] as const) {
271-
const restored = fromJournaled('chat', toJournaled(entry('a', { status })))
319+
const restored = fromJournaled('chat', toJournaled(entry('a', { status })), NOW)
272320
expect(restored.unconfirmed).toBe(true)
273321
}
274322
})
@@ -278,7 +326,8 @@ describe('journal', () => {
278326
'chat',
279327
toJournaled(
280328
entry('a', { status: 'applied', streamId: 'turn', textOffset: 4, refusedBy: 'earlier' })
281-
)
329+
),
330+
NOW
282331
)
283332
expect(restored).toMatchObject({
284333
status: 'applied',
@@ -289,6 +338,27 @@ describe('journal', () => {
289338
})
290339
})
291340

341+
describe('a journal restored past the age limit', () => {
342+
const restore = (status: OutgoingMessage['status'], age: number) =>
343+
fromJournaled(
344+
'chat',
345+
toJournaled(entry('a', { status, streamId: 'turn', writtenAt: NOW - age })),
346+
NOW
347+
).status
348+
349+
it('shows a message on its way as not delivered', () => {
350+
expect(restore('pending', ORPHAN_AUTO_SEND_MAX_AGE_MS)).toBe('failed')
351+
expect(restore('steered', ORPHAN_AUTO_SEND_MAX_AGE_MS)).toBe('failed')
352+
expect(restore('sending', ORPHAN_AUTO_SEND_MAX_AGE_MS)).toBe('failed')
353+
})
354+
355+
it('resumes a recent one where it stood, and leaves one already applied alone', () => {
356+
expect(restore('pending', ORPHAN_AUTO_SEND_MAX_AGE_MS - 1)).toBe('pending')
357+
expect(restore('steered', ORPHAN_AUTO_SEND_MAX_AGE_MS - 1)).toBe('steered')
358+
expect(restore('applied', ORPHAN_AUTO_SEND_MAX_AGE_MS)).toBe('applied')
359+
})
360+
})
361+
292362
describe('deliveryOf', () => {
293363
it('says a message waits for the turn only when it cannot steer it', () => {
294364
expect(deliveryOf(entry('a', { unconfirmed: true }), true)).toBe('waiting')

‎apps/sim/app/workspace/[workspaceId]/home/hooks/outgoing-state.ts‎

Lines changed: 47 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,10 @@ import type { ChatContext } from '@/stores/panel'
1313

1414
/** The worker's cap on steering text; a longer message goes out as a turn. */
1515
const MAX_STEERING_CONTENT_LENGTH = 32_768
16-
/** Rounds that came back without delivering a message before it shows as not delivered. */
16+
/** Sends that came back without delivering a message before it shows as not delivered. */
1717
export const MAX_OUTGOING_DELIVERY_ATTEMPTS = 8
18+
/** Reads of a turn's end that still found a message held before it shows as not delivered. */
19+
const MAX_DISPOSITION_READS = 8
1820
const OUTGOING_RETRY_BASE_MS = 1000
1921
const OUTGOING_RETRY_MAX_MS = 30_000
2022
/**
@@ -68,8 +70,12 @@ export interface OutgoingMessage {
6870
textOffset?: number
6971
/** A turn that refused it: it waits for that turn to end rather than steering it again. */
7072
refusedBy?: string
71-
/** Delivery rounds that came back without delivering it, whichever way they did. */
73+
/** Sends that came back without delivering it (no answer, or the chat was busy). */
7274
attempts: number
75+
/** Reads after its turn ended that found it still held; they say nothing about a send. */
76+
reads: number
77+
/** Epoch ms it was written, which decides whether it may still go out on its own. */
78+
writtenAt: number
7379
/** Epoch ms before which it is not tried again. */
7480
notBefore?: number
7581
}
@@ -141,15 +147,30 @@ export function toJournaled(message: OutgoingMessage): JournaledOutgoing {
141147
? { unconfirmed: true }
142148
: {}),
143149
...(message.textOffset !== undefined ? { textOffset: message.textOffset } : {}),
150+
writtenAt: message.writtenAt,
144151
}
145152
}
146153

147-
export function fromJournaled(chatKey: string, entry: JournaledOutgoing): OutgoingMessage {
154+
/**
155+
* The entry a journal restores. One on its way that has sat past the age limit (a
156+
* browser session restored days later) shows as not delivered, with no network call
157+
* and nothing going out until the user asks.
158+
*/
159+
export function fromJournaled(
160+
chatKey: string,
161+
entry: JournaledOutgoing,
162+
now: number
163+
): OutgoingMessage {
164+
const stillOnItsWay = entry.status === 'pending' || entry.status === 'steered'
148165
return {
149166
...entry,
167+
...(stillOnItsWay && now - entry.writtenAt >= ORPHAN_AUTO_SEND_MAX_AGE_MS
168+
? { status: 'failed' as const }
169+
: {}),
150170
chatKey,
151171
display: optimisticUserMessageFor(entry.id, entry.payload),
152172
attempts: 0,
173+
reads: 0,
153174
}
154175
}
155176

@@ -176,6 +197,19 @@ export function admittedMessageIds(history: MothershipChatHistory | undefined):
176197
return admitted
177198
}
178199

200+
/**
201+
* Whether the chat's history shows this message is a turn of its own, whatever this
202+
* tab last knew: another tab sent it, or a send that got no answer was admitted.
203+
*/
204+
export function isDeliveredAsTurn(
205+
message: OutgoingMessage,
206+
admittedIds: ReadonlySet<string>
207+
): boolean {
208+
return (
209+
(message.status === 'failed' || message.status === 'steered') && admittedIds.has(message.id)
210+
)
211+
}
212+
179213
export type OutgoingStep =
180214
/** Nothing can go now: nothing is pending, or what is first must wait. */
181215
| { kind: 'idle' }
@@ -281,13 +315,13 @@ export function afterDispositionRead(
281315
}
282316
/* Still held (or no answer): read again later. A run that never settles it, such
283317
as one whose owner died, does not keep it waiting forever. */
284-
const attempts = message.attempts + 1
285-
return attempts >= MAX_OUTGOING_DELIVERY_ATTEMPTS
286-
? { status: 'failed', attempts }
318+
const reads = message.reads + 1
319+
return reads >= MAX_DISPOSITION_READS
320+
? { status: 'failed', reads }
287321
: {
288-
attempts,
322+
reads,
289323
unconfirmed: outcome.status === 'unknown',
290-
notBefore: context.now + outgoingRetryDelay(attempts),
324+
notBefore: context.now + outgoingRetryDelay(reads),
291325
}
292326
}
293327

@@ -300,6 +334,7 @@ export function afterRetry(message: OutgoingMessage): Partial<OutgoingMessage> {
300334
return {
301335
status: message.streamId ? 'steered' : 'pending',
302336
attempts: 0,
337+
reads: 0,
303338
notBefore: undefined,
304339
}
305340
}
@@ -363,16 +398,18 @@ export function adoptOrphanRow(row: PersistedRow, chatKey: string, now: number):
363398
...(contexts?.length ? { contexts } : {}),
364399
...(row.requestMode ? { requestMode: row.requestMode } : {}),
365400
})
366-
const age = now - Date.parse(row.timestamp)
401+
const writtenAt = Date.parse(row.timestamp) || 0
367402
return {
368403
id: row.id,
369404
chatKey,
370405
payload,
371406
display: optimisticUserMessageFor(row.id, payload),
372-
status: age < ORPHAN_AUTO_SEND_MAX_AGE_MS ? 'steered' : 'failed',
407+
status: now - writtenAt < ORPHAN_AUTO_SEND_MAX_AGE_MS ? 'steered' : 'failed',
373408
streamId: row.steering?.streamId,
374409
unconfirmed: true,
375410
attempts: 0,
411+
reads: 0,
412+
writtenAt,
376413
}
377414
}
378415

0 commit comments

Comments
 (0)