Skip to content

Commit a049852

Browse files
waleedlatif1claude
andcommitted
fix(realtime): mark a fold's output explicitly, and account in the tailer
A fold of an agent-only stream is stamped with the agent marker to preserve the no-persist guarantee, which made it indistinguishable from an ordinary agent preview frame. Excluding that marker from accounting therefore dropped preview deltas — the largest payloads there are, and the ones that caused the incident — while including it would let a snapshot arm the trigger against its own output. A dedicated field settles it without touching origin selection. With the ambiguity gone, accounting moves from the publish path to `applyEntry`, which observes every entry the room tails: this task's appends, a peer task's, and one published with no room attached anywhere. Publish-side accounting could only ever see local writes, and contributed nothing to the trigger before the tailer caught up regardless, since only entries at or before the fold boundary arm it. The ledger is now a Map keyed by entry id, so an entry observed twice is recorded once. Entries written before this field carry no marker and count as deltas, which over-arms by at most one fold that then trims them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 0529873 commit a049852

2 files changed

Lines changed: 119 additions & 48 deletions

File tree

apps/realtime/src/handlers/file-doc-store.test.ts

Lines changed: 77 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ const REDIS_URL = 'redis://fake'
131131

132132
interface StoreRoomInternals {
133133
lastId: string
134-
pendingDeltas: Array<{ id: string; bytes: number }>
134+
pendingDeltas: Map<string, number>
135135
realEdited: boolean
136136
publishes: number
137137
compactRetryAfter: number
@@ -141,6 +141,7 @@ interface StoreRoomInternals {
141141

142142
interface FileDocStoreInternals {
143143
rooms: Map<string, StoreRoomInternals>
144+
applyEntry(room: StoreRoomInternals, id: string, message: Record<string, string>): void
144145
appendUpdate(name: string, update: Uint8Array, agent?: boolean): Promise<void>
145146
write: { xTrim: (...args: unknown[]) => Promise<unknown> }
146147
maybeCompact(name: string, force?: boolean): Promise<void>
@@ -367,7 +368,7 @@ describe('FileDocStore', () => {
367368
lastId: '400-0',
368369
publishes: 0,
369370
compactRetryAfter: 0,
370-
pendingDeltas: [],
371+
pendingDeltas: new Map(),
371372
seededObserved: true,
372373
realEdited: true,
373374
})
@@ -500,7 +501,7 @@ describe('FileDocStore', () => {
500501
const doc = new Y.Doc()
501502
await a.attachRoom(NAME, doc)
502503
const room = internals(a).rooms.get(NAME)!
503-
room.pendingDeltas = [{ id: '1-0', bytes: 9 * 1024 * 1024 }]
504+
room.pendingDeltas = new Map([['1-0', 9 * 1024 * 1024]])
504505
room.realEdited = true
505506

506507
const write = internals(a).write
@@ -512,7 +513,7 @@ describe('FileDocStore', () => {
512513

513514
// A failed fold must not disarm the trigger — otherwise the stream stays oversized until
514515
// this task happens to append another full threshold's worth of deltas.
515-
expect(room.pendingDeltas).toEqual([{ id: '1-0', bytes: 9 * 1024 * 1024 }])
516+
expect([...room.pendingDeltas]).toEqual([['1-0', 9 * 1024 * 1024]])
516517

517518
// But it must not retry immediately either: the snapshot XADD lands before the XTRIM, so a
518519
// persistent trim failure would append a full-document snapshot on every attempt.
@@ -537,17 +538,17 @@ describe('FileDocStore', () => {
537538
// INCLUSIVE) and 9-0. Their bytes are still in Redis, and dropping them would disarm the
538539
// byte trigger while the stream kept growing.
539540
room.lastId = '5-0'
540-
room.pendingDeltas = [
541-
{ id: '3-0', bytes: 4 * 1024 * 1024 },
542-
{ id: '5-0', bytes: 6 * 1024 * 1024 },
543-
{ id: '9-0', bytes: 7 * 1024 * 1024 },
544-
]
541+
room.pendingDeltas = new Map([
542+
['3-0', 4 * 1024 * 1024],
543+
['5-0', 6 * 1024 * 1024],
544+
['9-0', 7 * 1024 * 1024],
545+
])
545546

546547
await internals(a).maybeCompact(NAME, true)
547548

548-
expect(room.pendingDeltas).toEqual([
549-
{ id: '5-0', bytes: 6 * 1024 * 1024 },
550-
{ id: '9-0', bytes: 7 * 1024 * 1024 },
549+
expect([...room.pendingDeltas]).toEqual([
550+
['5-0', 6 * 1024 * 1024],
551+
['9-0', 7 * 1024 * 1024],
551552
])
552553
doc.destroy()
553554
})
@@ -591,6 +592,67 @@ describe('FileDocStore', () => {
591592
seedDoc.destroy()
592593
})
593594

595+
it('counts agent preview deltas, which share a marker with an agent-only snapshot', async () => {
596+
const streamKey = `filedoc:stream:${NAME}`
597+
// Agent preview frames are the LARGE ones — a copilot file edit re-serialising a document is
598+
// what filled Redis. They carry the same marker as a fold of an agent-only stream, so keying
599+
// exclusion on that marker would drop exactly the payloads this bound exists for.
600+
const seedDoc = new Y.Doc()
601+
const updates: Uint8Array[] = []
602+
seedDoc.on('update', (u: Uint8Array) => updates.push(u))
603+
seedDoc.getText('body').insert(0, 'x'.repeat(9 * 1024 * 1024))
604+
seedDoc.getText('body').insert(0, 'tail')
605+
state.backing!.streams.set(
606+
streamKey,
607+
updates.map((update, index) => ({
608+
id: `${index + 1}-0`,
609+
message: { u: Buffer.from(update).toString('base64'), a: '1' },
610+
}))
611+
)
612+
state.backing!.seq = updates.length
613+
614+
const a = await newStore()
615+
const doc = new Y.Doc()
616+
await a.attachRoom(NAME, doc)
617+
618+
await vi.waitFor(() => {
619+
const stream = state.backing!.streams.get(streamKey)!
620+
expect(stream.some((entry) => entry.message.c === '1')).toBe(true)
621+
})
622+
623+
const rebuilt = new Y.Doc()
624+
Y.applyUpdate(rebuilt, (await a.getStreamState(NAME))!)
625+
expect(rebuilt.getText('body').length).toBe(9 * 1024 * 1024 + 4)
626+
rebuilt.destroy()
627+
doc.destroy()
628+
seedDoc.destroy()
629+
})
630+
631+
it("never counts a fold's own output, so a large document cannot arm the trigger against itself", async () => {
632+
const a = await newStore()
633+
const doc = new Y.Doc()
634+
await a.attachRoom(NAME, doc)
635+
const room = internals(a).rooms.get(NAME)!
636+
637+
internals(a).applyEntry(room, '7-0', { u: 'x'.repeat(9 * 1024 * 1024), a: '1', c: '1' })
638+
639+
expect(room.pendingDeltas.has('7-0')).toBe(false)
640+
doc.destroy()
641+
})
642+
643+
it('counts a delta published by a peer task, which this room only ever tails', async () => {
644+
const a = await newStore()
645+
const doc = new Y.Doc()
646+
await a.attachRoom(NAME, doc)
647+
const room = internals(a).rooms.get(NAME)!
648+
649+
// Never published locally, so publish-side accounting would miss it entirely.
650+
internals(a).applyEntry(room, '4-0', { u: 'x'.repeat(1024) })
651+
652+
expect(room.pendingDeltas.get('4-0')).toBe(1024)
653+
doc.destroy()
654+
})
655+
594656
it('stamps a compaction snapshot of an agent-ONLY stream as an agent frame (never persisted)', async () => {
595657
const streamKey = `filedoc:stream:${NAME}`
596658
const noop = Buffer.from(Y.encodeStateAsUpdate(new Y.Doc())).toString('base64')
@@ -609,7 +671,7 @@ describe('FileDocStore', () => {
609671
lastId: '400-0',
610672
publishes: 0,
611673
compactRetryAfter: 0,
612-
pendingDeltas: [],
674+
pendingDeltas: new Map(),
613675
seededObserved: true,
614676
realEdited: false,
615677
})
@@ -777,7 +839,7 @@ describe('FileDocStore', () => {
777839
lastId: '401-0',
778840
publishes: 0,
779841
compactRetryAfter: 0,
780-
pendingDeltas: [],
842+
pendingDeltas: new Map(),
781843
seededObserved: true,
782844
realEdited: true,
783845
})
@@ -786,7 +848,7 @@ describe('FileDocStore', () => {
786848
lastId: '400-0',
787849
publishes: 0,
788850
compactRetryAfter: 0,
789-
pendingDeltas: [],
851+
pendingDeltas: new Map(),
790852
seededObserved: true,
791853
realEdited: true,
792854
})

apps/realtime/src/handlers/file-doc-store.ts

Lines changed: 42 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,20 @@ const SNAPSHOT_FIELD = 's'
123123
/** Marks a stream entry as an AGENT-STREAMED preview frame, so the tailer applies it with
124124
* {@link REDIS_AGENT_ORIGIN} (never marks the doc edited). Present only on agent-frame entries. */
125125
const AGENT_FIELD = 'a'
126+
/**
127+
* Marks a stream entry as the OUTPUT of a compaction, for byte accounting only.
128+
*
129+
* {@link SNAPSHOT_FIELD} cannot serve this purpose: a fold of an agent-only stream is stamped
130+
* {@link AGENT_FIELD} instead, so it is indistinguishable from an ordinary agent preview frame —
131+
* and those are the large ones. Excluding both markers would drop preview deltas from accounting;
132+
* excluding neither would count a snapshot as something a fold can reclaim, arming the trigger
133+
* against its own output. A separate field settles it without touching origin selection, which
134+
* must keep treating an agent-only fold as an agent frame to preserve the no-persist guarantee.
135+
*
136+
* Entries written before this field existed carry no marker and are counted as deltas. That
137+
* over-arms by at most one fold, which then trims them.
138+
*/
139+
const COMPACTION_FIELD = 'c'
126140

127141
/** Sentinel token a DISABLED store returns from a lock acquire, so single-replica callers proceed
128142
* without special-casing; {@link FileDocStore.releaseLock} treats it as a no-op. Not a real UUID, so it
@@ -205,10 +219,10 @@ const streamKey = (name: string) => `${STREAM_PREFIX}${name}`
205219
*/
206220
function foldableDeltaBytes(room: StoreRoom): number {
207221
let bytes = 0
208-
for (const delta of room.pendingDeltas) {
222+
for (const [id, deltaBytes] of room.pendingDeltas) {
209223
// Strictly before the boundary: MINID is inclusive, so the entry AT `lastId` survives the
210224
// trim and folding cannot reclaim it.
211-
if (isAfterStreamId(room.lastId, delta.id)) bytes += delta.bytes
225+
if (isAfterStreamId(room.lastId, id)) bytes += deltaBytes
212226
}
213227
return bytes
214228
}
@@ -265,20 +279,22 @@ interface StoreRoom {
265279
/** Epoch ms before which no forced fold is attempted, after one failed. */
266280
compactRetryAfter: number
267281
/**
268-
* Deltas this task has appended and not yet folded, as `{id, bytes}` pairs in append order.
282+
* Unfolded delta bytes in the shared stream, by entry id.
283+
*
284+
* Recorded in {@link FileDocStore.applyEntry}, so it covers EVERY entry this room's tailer
285+
* observes — this task's own appends, a peer task's, and one published with no room attached
286+
* anywhere. Accounting on publish instead would see only this task's writes.
269287
*
270-
* Keyed by stream id rather than summed, because a fold trims to `room.lastId` and RETAINS
271-
* anything published past it — those bytes are still in Redis, so deducting them would
272-
* disarm the trigger while the stream keeps growing. Entries are dropped only once an
273-
* `XTRIM` has provably removed them.
288+
* Keyed by id rather than summed, because a fold trims to `room.lastId` and retains anything
289+
* from that boundary on. Those bytes are still in Redis, so dropping them would disarm the
290+
* trigger while the stream kept growing; entries are removed only once an `XTRIM` provably
291+
* removed them.
274292
*
275-
* Counts deltas only — never the snapshot a compaction writes, which is a function of
276-
* document size rather than of edit volume and would make a large document breach the
277-
* threshold permanently. Locally tracked, so it under-counts a peer task's appends: it is a
278-
* trigger, not an accounting, and {@link COMPACT_THRESHOLD} still covers many small edits
279-
* arriving from elsewhere.
293+
* Excludes what a fold produces (see {@link COMPACTION_FIELD}) — a snapshot is a function of
294+
* document size rather than edit volume, and counting one would make a large document breach
295+
* the threshold permanently.
280296
*/
281-
pendingDeltas: Array<{ id: string; bytes: number }>
297+
pendingDeltas: Map<string, number>
282298
/** Set once the doc has been observed seeded, so the seed transition itself is never mistaken for an
283299
* edit (mirrors the relay's `seededObserved`). */
284300
seededObserved: boolean
@@ -361,7 +377,7 @@ export class FileDocStore {
361377
lastId: '0',
362378
publishes: 0,
363379
compactRetryAfter: 0,
364-
pendingDeltas: [],
380+
pendingDeltas: new Map(),
365381
seededObserved: false,
366382
realEdited: false,
367383
}
@@ -391,24 +407,10 @@ export class FileDocStore {
391407
// the SEED after `seededObserved` latched would count it as a post-seed edit and let a
392408
// compaction snapshot claim content no user ever typed. Skip what this room already holds.
393409
if (!isAfterStreamId(entry.id, room.lastId)) continue
394-
// Adopt the accounting for what is already in the stream. A task taking one over would
395-
// otherwise start from an empty ledger, so a multi-megabyte stream under the entry
396-
// threshold would stay unfolded while this room's heartbeat keeps refreshing its TTL.
397-
// These entries are already being read to rebuild the doc, so this costs no extra work —
398-
// unlike seeding from a scan we would not otherwise do.
399-
//
400-
// Plain deltas only: a compaction snapshot is the RESULT of a fold, not something a fold
401-
// can reclaim, so counting one would arm the trigger against itself.
402-
if (!entry.message[SNAPSHOT_FIELD] && !entry.message[AGENT_FIELD]) {
403-
room.pendingDeltas.push({
404-
id: entry.id,
405-
bytes: entry.message[UPDATE_FIELD]?.length ?? 0,
406-
})
407-
}
408410
this.applyEntry(room, entry.id, entry.message)
409411
}
410412
await this.write.expire(streamKey(name), STREAM_TTL_SEC)
411-
// The adopted entries may already be past the ceiling, and nothing else re-checks until the
413+
// A stream taken over may already be past the ceiling, and nothing else re-checks until the
412414
// next local publish — which a read-only participant never makes.
413415
if (foldableDeltaBytes(room) >= COMPACT_BYTES_THRESHOLD) void this.maybeCompact(name, true)
414416
} catch (error) {
@@ -443,10 +445,9 @@ export class FileDocStore {
443445
const encoded = Buffer.from(update).toString('base64')
444446
const fields: Record<string, string> = { [UPDATE_FIELD]: encoded }
445447
if (agent) fields[AGENT_FIELD] = '1'
446-
let appendedId: string | null = null
447448
for (let attempt = 0; attempt <= PUBLISH_MAX_RETRIES; attempt++) {
448449
try {
449-
appendedId = await this.write.xAdd(streamKey(name), '*', fields)
450+
await this.write.xAdd(streamKey(name), '*', fields)
450451
break
451452
} catch (error) {
452453
if (attempt === PUBLISH_MAX_RETRIES) {
@@ -461,7 +462,6 @@ export class FileDocStore {
461462
await this.write.expire(streamKey(name), STREAM_TTL_SEC).catch(() => {})
462463
const room = this.rooms.get(name)
463464
if (!room) return
464-
if (appendedId) room.pendingDeltas.push({ id: appendedId, bytes: encoded.length })
465465
// Bytes are checked every publish: one entry can cross the ceiling on its own, so pacing this
466466
// check the way the entry count is paced would let a stream sit far over the ceiling for up to
467467
// COMPACT_CHECK_EVERY more appends. The check itself is a local sum over unfolded entries.
@@ -728,6 +728,12 @@ export class FileDocStore {
728728

729729
private applyEntry(room: StoreRoom, id: string, message: Record<string, string>): void {
730730
room.lastId = id
731+
// Account for every entry the tailer sees, whoever wrote it — this is the only point that
732+
// observes peer and roomless appends. A fold's own output is excluded so it cannot arm the
733+
// trigger against itself.
734+
if (!message[COMPACTION_FIELD]) {
735+
room.pendingDeltas.set(id, message[UPDATE_FIELD]?.length ?? 0)
736+
}
731737
// A compaction snapshot folds seed + edits into one frame; stamp it so the relay's edit-tracker
732738
// treats a fresh catch-up from it as edited (a snapshot only exists once real edits accumulated). An
733739
// agent-streamed preview frame is stamped separately so the tracker NEVER marks it edited.
@@ -856,6 +862,7 @@ export class FileDocStore {
856862
await this.write.xAdd(streamKey(name), '*', {
857863
[UPDATE_FIELD]: snapshot,
858864
[marker]: '1',
865+
[COMPACTION_FIELD]: '1',
859866
})
860867
// MINID keeps entries with id >= upTo: the snapshot, any un-integrated peer entries, and
861868
// `upTo` itself (redundant with the snapshot, harmless); it drops only the folded older deltas.
@@ -869,7 +876,9 @@ export class FileDocStore {
869876
// nothing and leaves the trigger armed.
870877
// `MINID upTo` is inclusive — it keeps the entry whose id EQUALS `upTo`, so that entry's
871878
// bytes are still in Redis and must stay counted. Keeps exactly what survived the trim.
872-
room.pendingDeltas = room.pendingDeltas.filter((delta) => !isAfterStreamId(upTo, delta.id))
879+
for (const id of room.pendingDeltas.keys()) {
880+
if (isAfterStreamId(upTo, id)) room.pendingDeltas.delete(id)
881+
}
873882
} finally {
874883
await this.releaseLock(key, token)
875884
}

0 commit comments

Comments
 (0)