Skip to content

Commit 8aead66

Browse files
waleedlatif1claude
andcommitted
fix(redis): bound the three unbudgeted stream writers by bytes
The copilot stream buffer, the Tables event log and the realtime file-doc streams were each bounded by entry count and nothing else. An entry cap bounds how many entries a key holds and says nothing about how large each one is, so a single writer emitting large entries reaches gigabytes well inside its cap — which is how one file-edit stream filled a shared Redis under a 100,000-entry cap and evicted the whole keyspace. Each writer gets the bound its read semantics allow: - Copilot's replay buffer is read from a cursor and must stay contiguous, so it now reserves bytes against per-stream and per-user ceilings inside the same Lua that appends, and the writer soft-stops persistence on refusal rather than failing the live stream. - The Tables event log is a live feed whose readers already refetch on a prune, so it drops oldest-first once past a byte ceiling — the existing `pruned` path carries it, with the running total kept in meta under the same TTL as the bytes it counts. - The file-doc streams are Yjs deltas replayed in full by a task attaching later, so dropping the oldest would lose edits and a native MAXLEN bound is unsafe. Compaction, which folds deltas into a snapshot first, is lossless — it now triggers on appended bytes as well as entry count. Also folds `lib/execution/redis-budget.server.ts` into the shared module rather than leaving two definitions of the same prefix and ceilings writing the same Redis keys. Key layout and every execution limit are unchanged, and pinned by test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 599654b commit 8aead66

16 files changed

Lines changed: 949 additions & 199 deletions

File tree

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ vi.mock('redis', () => ({ createClient: () => makeClient() }))
128128
import { FileDocStore, REDIS_AGENT_ORIGIN, REDIS_ORIGIN } from '@/handlers/file-doc-store'
129129

130130
const REDIS_URL = 'redis://fake'
131+
const COMPACT_THRESHOLD_ENTRIES = 400
131132
const NAME = 'workspace-file-doc:file-1'
132133

133134
function docWithText(text: string): Y.Doc {
@@ -401,6 +402,40 @@ describe('FileDocStore', () => {
401402
doc.destroy()
402403
})
403404

405+
it('compacts on appended bytes, before the entry threshold is anywhere near reached', async () => {
406+
const streamKey = `filedoc:stream:${NAME}`
407+
const a = await newStore()
408+
const doc = new Y.Doc()
409+
await a.attachRoom(NAME, doc)
410+
411+
// A handful of large pastes: far below COMPACT_THRESHOLD entries, far above the byte ceiling.
412+
// Before bytes were counted this stream held tens of megabytes and never compacted.
413+
const updates: Uint8Array[] = []
414+
doc.on('update', (u: Uint8Array) => updates.push(u))
415+
for (let i = 0; i < 4; i++) {
416+
doc.getText('body').insert(0, 'x'.repeat(3 * 1024 * 1024))
417+
}
418+
for (const update of updates) {
419+
await a.publishAndWait(NAME, update)
420+
}
421+
422+
await vi.waitFor(
423+
() => {
424+
const stream = state.backing!.streams.get(streamKey)!
425+
expect(stream.length).toBeLessThan(COMPACT_THRESHOLD_ENTRIES)
426+
expect(stream.some((entry) => entry.message.s === '1')).toBe(true)
427+
},
428+
{ timeout: 5000 }
429+
)
430+
431+
// Compaction must be lossless: the whole document is still reconstructable from what remains.
432+
const rebuilt = new Y.Doc()
433+
Y.applyUpdate(rebuilt, (await a.getStreamState(NAME))!)
434+
expect(rebuilt.getText('body').length).toBe(4 * 3 * 1024 * 1024)
435+
rebuilt.destroy()
436+
doc.destroy()
437+
})
438+
404439
it('stamps a compaction snapshot of an agent-ONLY stream as an agent frame (never persisted)', async () => {
405440
const streamKey = `filedoc:stream:${NAME}`
406441
const noop = Buffer.from(Y.encodeStateAsUpdate(new Y.Doc())).toString('base64')

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

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,20 @@ const IDLE_POLL_MS = 250
140140
const READ_COUNT = 200
141141
/** Compact a stream once it exceeds this many entries (snapshot + trim). */
142142
const COMPACT_THRESHOLD = 400
143+
/**
144+
* Compact a stream once its appended deltas exceed this many bytes, whichever comes first.
145+
*
146+
* The entry threshold alone bounds how many entries a stream holds and says nothing about
147+
* how large each one is: one pasted block is a single entry carrying megabytes, so a stream
148+
* can sit at a few dozen entries and hundreds of megabytes and never reach
149+
* {@link COMPACT_THRESHOLD} before its TTL. Folding by bytes as well keeps a stream's cost
150+
* proportional to its document rather than to the size of the edits that produced it.
151+
*
152+
* Compaction is the only safe way to shrink one of these streams: a task attaching later
153+
* replays every entry to rebuild the doc, so dropping the oldest entries — what a native
154+
* `MAXLEN` retention bound would do — loses edits outright. A snapshot folds them first.
155+
*/
156+
const COMPACT_BYTES_THRESHOLD = 8 * 1024 * 1024
143157
/** Check whether compaction is due only every Nth local publish, to avoid an XLEN per keystroke. */
144158
const COMPACT_CHECK_EVERY = 64
145159
/** Compaction critical section (snapshot + xAdd + xTrim) is fast; a generous TTL covers a slow Redis
@@ -218,6 +232,13 @@ interface StoreRoom {
218232
lastId: string
219233
/** Local publish count, to pace compaction checks. */
220234
publishes: number
235+
/**
236+
* Bytes this task has appended since the last compaction it observed, so the byte threshold
237+
* costs no extra round-trip. Locally tracked, so it under-counts a peer task's appends — it
238+
* is a trigger, not an accounting, and {@link COMPACT_THRESHOLD} still covers the case where
239+
* many small edits arrive from elsewhere.
240+
*/
241+
appendedBytes: number
221242
/** Set once the doc has been observed seeded, so the seed transition itself is never mistaken for an
222243
* edit (mirrors the relay's `seededObserved`). */
223244
seededObserved: boolean
@@ -299,6 +320,7 @@ export class FileDocStore {
299320
doc,
300321
lastId: '0',
301322
publishes: 0,
323+
appendedBytes: 0,
302324
seededObserved: false,
303325
realEdited: false,
304326
}
@@ -379,7 +401,15 @@ export class FileDocStore {
379401
}
380402
await this.write.expire(streamKey(name), STREAM_TTL_SEC).catch(() => {})
381403
const room = this.rooms.get(name)
382-
if (room && ++room.publishes % COMPACT_CHECK_EVERY === 0) void this.maybeCompact(name)
404+
if (!room) return
405+
room.appendedBytes += encoded.length
406+
// Bytes are checked every publish: one entry can cross the ceiling on its own, so pacing this
407+
// check the way the entry count is paced would let a stream sit far over the ceiling for up to
408+
// COMPACT_CHECK_EVERY more appends. The check itself is a local comparison.
409+
const overBytes = room.appendedBytes >= COMPACT_BYTES_THRESHOLD
410+
if (overBytes || ++room.publishes % COMPACT_CHECK_EVERY === 0) {
411+
void this.maybeCompact(name, overBytes)
412+
}
383413
}
384414

385415
/**
@@ -734,12 +764,12 @@ export class FileDocStore {
734764
* only one task compacts a given stream at a time (concurrent snapshot+trim would race). Trims only up
735765
* to what the snapshot provably contains — never un-integrated peer entries (see below).
736766
*/
737-
private async maybeCompact(name: string): Promise<void> {
767+
private async maybeCompact(name: string, force = false): Promise<void> {
738768
if (!this.write) return
739769
const room = this.rooms.get(name)
740770
if (!room) return
741771
try {
742-
if ((await this.write.xLen(streamKey(name))) < COMPACT_THRESHOLD) return
772+
if (!force && (await this.write.xLen(streamKey(name))) < COMPACT_THRESHOLD) return
743773
const key = `${COMPACT_LOCK_PREFIX}${name}`
744774
const token = await this.acquireLock(key, COMPACT_LOCK_TTL_MS)
745775
if (!token) return
@@ -752,6 +782,10 @@ export class FileDocStore {
752782
// appended snapshot id instead would silently drop those un-integrated peer entries.
753783
const upTo = room.lastId
754784
const snapshot = Buffer.from(Y.encodeStateAsUpdate(room.doc)).toString('base64')
785+
// The folded deltas are about to be trimmed; what remains of this task's contribution is the
786+
// snapshot. Reset before the appends so a concurrent publish's bytes are counted against the
787+
// new baseline rather than the one being retired.
788+
room.appendedBytes = snapshot.length
755789
// Stamp the snapshot by what it folds: a real edit → SNAPSHOT_FIELD (a fresh catch-up treats it
756790
// as edited content, not a bare seed). An agent-ONLY stream (no real edit yet) → AGENT_FIELD, so a
757791
// peer catching up applies it as REDIS_AGENT_ORIGIN and never marks the doc edited — preserving

apps/sim/lib/copilot/request/lifecycle/start.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ export function createSSEStream(params: StreamingOrchestrationParams): ReadableS
116116
const abortController = new AbortController()
117117
registerActiveStream(streamId, abortController)
118118

119-
const publisher = new StreamWriter({ streamId, chatId, requestId })
119+
const publisher = new StreamWriter({ streamId, chatId, requestId, userId })
120120

121121
// Declared at function scope (same rationale as `cancelReason` below) so the
122122
// leak backstop in the orchestration's outer finally can always reach them:

apps/sim/lib/copilot/request/session/buffer.test.ts

Lines changed: 106 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,33 @@ const createRedisStub = () => {
6464
return Promise.resolve('OK')
6565
}),
6666
get: vi.fn().mockImplementation((key: string) => Promise.resolve(values.get(key) ?? null)),
67+
/**
68+
* Stands in for `APPEND_EVENTS_SCRIPT`. It reproduces the script's observable
69+
* effects — dedupe, zadd, rank-trim, seq — so the read-path tests still exercise
70+
* real data, and exposes `budgetRefusal` so the refusal branch can be driven
71+
* without reimplementing the budget arithmetic here.
72+
*/
73+
budgetRefusal: null as null | [number, string, number],
74+
eval: vi.fn().mockImplementation((...args: unknown[]) => {
75+
const numKeys = Number(args[1])
76+
const keys = args.slice(2, 2 + numKeys) as string[]
77+
const argv = args.slice(2 + numKeys) as Array<string | number>
78+
if (api.budgetRefusal) return Promise.resolve(api.budgetRefusal)
79+
80+
const [eventsKey, seqKey] = keys
81+
const eventLimit = Number(argv[1])
82+
const lastSeq = String(argv[5])
83+
const entries = sortedSets.get(eventsKey) ?? []
84+
for (let i = 6; i < argv.length; i += 2) {
85+
const score = Number(argv[i])
86+
const value = String(argv[i + 1])
87+
if (!entries.some((entry) => entry.value === value)) entries.push({ score, value })
88+
}
89+
entries.sort((a, b) => a.score - b.score)
90+
sortedSets.set(eventsKey, entries.slice(Math.max(0, entries.length - eventLimit)))
91+
values.set(seqKey, lastSeq)
92+
return Promise.resolve([1])
93+
}),
6794
pipeline: vi.fn().mockImplementation(() => {
6895
const operations: Array<() => Promise<unknown>> = []
6996
const pipeline = {
@@ -103,6 +130,7 @@ let mockRedis: ReturnType<typeof createRedisStub>
103130
import {
104131
allocateCursor,
105132
appendEvent,
133+
appendEvents,
106134
clearBuffer,
107135
readEvents,
108136
scheduleBufferCleanup,
@@ -161,11 +189,84 @@ describe('mothership-stream-outbox', () => {
161189
})
162190
)
163191

164-
expect(mockRedis.zremrangebyrank).toHaveBeenCalledWith(
165-
'mothership_stream:stream-1:events',
166-
0,
167-
-100_001
168-
)
192+
// KEYS: [events, seq, budgetOwner]; ARGV follows.
193+
const [, numKeys, eventsKey, seqKey, ownerKey, ...argv] = mockRedis.eval.mock.calls[0]
194+
expect(numKeys).toBe(3)
195+
expect(eventsKey).toBe('mothership_stream:stream-1:events')
196+
expect(seqKey).toBe('mothership_stream:stream-1:seq')
197+
expect(ownerKey).toBe('execution:redis-budget:copilot_stream:stream-1')
198+
// ARGV: [ttl, eventLimit, ownerLimit, userLimit, budgetTtl, lastSeq, ...zaddArgs]
199+
expect(argv[1]).toBe(100_000)
200+
})
201+
202+
/**
203+
* The stream's replay copy is charged to a budget, and a refusal is reported rather
204+
* than thrown: `flush()` rethrows what it is handed, and that throw reaches the
205+
* error-path finalize, which would reject a response stream whose bytes the user
206+
* already received.
207+
*/
208+
it('reports a budget refusal instead of throwing', async () => {
209+
const cursor = await allocateCursor('stream-1')
210+
mockRedis.budgetRefusal = [0, 'owner_redis_bytes', 40_000_000]
211+
212+
const result = await appendEvents([
213+
createEvent({
214+
streamId: 'stream-1',
215+
cursor: cursor.cursor,
216+
seq: cursor.seq,
217+
requestId: 'req-1',
218+
type: MothershipStreamV1EventType.text,
219+
payload: { channel: MothershipStreamV1TextChannel.assistant, text: 'hello' },
220+
}),
221+
])
222+
223+
expect(result.persisted).toBe(false)
224+
if (!result.persisted) {
225+
expect(result.refusal.resource).toBe('owner_redis_bytes')
226+
expect(result.refusal.currentBytes).toBe(40_000_000)
227+
}
228+
})
229+
230+
it('refuses a batch past the single-write ceiling without reaching Redis', async () => {
231+
const cursor = await allocateCursor('stream-1')
232+
233+
const result = await appendEvents([
234+
createEvent({
235+
streamId: 'stream-1',
236+
cursor: cursor.cursor,
237+
seq: cursor.seq,
238+
requestId: 'req-1',
239+
type: MothershipStreamV1EventType.text,
240+
payload: {
241+
channel: MothershipStreamV1TextChannel.assistant,
242+
text: 'x'.repeat(2 * 1024 * 1024),
243+
},
244+
}),
245+
])
246+
247+
expect(result.persisted).toBe(false)
248+
expect(mockRedis.eval).not.toHaveBeenCalled()
249+
})
250+
251+
it('charges the user ceiling only when a user is in scope', async () => {
252+
const cursor = await allocateCursor('stream-1')
253+
const envelope = createEvent({
254+
streamId: 'stream-1',
255+
cursor: cursor.cursor,
256+
seq: cursor.seq,
257+
requestId: 'req-1',
258+
type: MothershipStreamV1EventType.text,
259+
payload: { channel: MothershipStreamV1TextChannel.assistant, text: 'hello' },
260+
})
261+
262+
await appendEvents([envelope], { streamId: 'stream-1' })
263+
expect(mockRedis.eval.mock.calls[0][1]).toBe(3)
264+
expect(mockRedis.eval.mock.calls[0][4]).toBe('execution:redis-budget:copilot_stream:stream-1')
265+
266+
mockRedis.eval.mockClear()
267+
await appendEvents([envelope], { streamId: 'stream-1', userId: 'user-1' })
268+
expect(mockRedis.eval.mock.calls[0][1]).toBe(4)
269+
expect(mockRedis.eval.mock.calls[0][5]).toBe('execution:redis-budget:user:user-1')
169270
})
170271

171272
it('clears persisted stream state during teardown cleanup', async () => {

0 commit comments

Comments
 (0)