perf(server): prune events the projections already cover#29806

🟠 High orchestration-v2/ProjectionMaintenance.ts:102

Pruning deletes a prompt anchor still referenced by a surviving stream-head event, so replay restores that event with a null prompt instead of its original text. readPromptReferences(throughSequence, head, ...) excludes references from rows at or below the boundary, even though pruneEvents preserves each stream's newest row; include references from all rows that survive this pruning pass.

No longer relevant as of 0955847e6f138a012574634c0c9f818cc689f3ea

Fixed in 0955847: each pass now also collects promptSequence references from stream-head rows at or below the boundary (the only kept rows that can borrow a prompt) before deleting anything, so a retained head keeps its anchor. Covered by "keeps the anchor a retained stream head borrows from", which fails without the change.

🟠 High src/ws.ts:786

A resume can pass this pruning check and still receive an incomplete replay, leaving the client state stale instead of triggering a snapshot. Pruning can advance after this check but before or during the deferred, paginated readApplicationEvents/catchUpApplicationEvents read, which silently skips deleted events before the replay emits its synchronization marker and resumes live from highWater; protect the replay from concurrent pruning or recheck the boundary before accepting it.

No longer relevant as of e188f19585a2f3f9c34d2ad7cce5d256c6640ef8

Fixed in e188f19. Thread and shell resumes now read the replay first and then re-read prunedThroughSequence; if it moved past the cursor (the thread path also requires the cursor to be below the thread high-water), the resume falls back to a snapshot. Pruning publishes the boundary before deleting, so any pass that deleted rows in the replayed range during the read is caught. The shell replay is now collected before replying (it is already bounded by SHELL_RESUME_MAX_GAP and the payload budget). New test "%s resume loads a snapshot when pruning advances during the replay read" gates the replay query, prunes mid-read, and fails for both paths without the recheck.

External image Minor · Data Integrity & Integration · Quick win

Re-check the pruning boundary after materializing shell replay.

readApplicationEvents is lazy, so the initial prunedThroughSequence check does not protect the later paginated reads. If pruning advances the boundary during those reads, the shell can emit only the remaining rows and then resume live events from highWater. This omits the deleted events without selecting loadSnapshot().

Materialize the bounded replay before returning the stream, then read prunedThroughSequence again. If input.afterSequence is now below the boundary, discard the replay and return loadSnapshot() instead. This keeps the correction local and avoids relying on a later session to repair the shell.

✅ Confirmed as addressed by @juliusmarminge

Fixed in e188f19. Thread and shell resumes now read the replay first and then re-read prunedThroughSequence; if it moved past the cursor (the thread path also requires the cursor to be below the thread high-water), the resume falls back to a snapshot. Pruning publishes the boundary before deleting, so any pass that deleted rows in the replayed range during the read is caught. The shell replay is now collected before replying (it is already bounded by SHELL_RESUME_MAX_GAP and the payload budget). New test "%s resume loads a snapshot when pruning advances during the replay read" gates the replay query, prunes mid-read, and fails for both paths without the recheck.

@juliusmarminge The supplied ws.ts confirms the fix. Both resume paths collect replay before re-reading prunedThroughSequence and select a snapshot when needed. The thread path also preserves the idle-thread exception at highWater.

This addresses the pruning race in this finding. I did not run the regression test.

Review thread resolved.

_{You are interacting with an AI system.}

External image Major · Stability & Availability · Quick win

Retain replay events for accepted commands.

When a command with an accepted receipt is retried, EventSink reads the command's events to replay the result. The current predicate can delete the subagent.updated event required by delegated-task retries and the run.created event required by initial-message retries. Preserve only those event types for accepted commands. Do not exempt every accepted-receipt event, because that would also retain unrelated events such as checkpoint.captured.

Not adopting this one. The case is narrow: the first attempt was accepted but its response never reached the client, and the client retries with the same command id more than 24 hours later. The receipt still deduplicates it, so nothing runs twice; the replay just comes back without the pruned events. Closing it fully would also require keeping the prompt anchors that retained subagent.updated rows borrow, and there is no index to find those per pass (a receipt-driven scan took 162 s cold on a 1.45M-event database). The internal overview now states this behavior instead of claiming such retries answer from projections.

@juliusmarminge Understood. I withdraw this finding. Preserving duplicate-execution protection does not require preserving replay results beyond the retained history.

My suggestion treated full replay after 24 hours as a requirement. It also missed the prompt anchors needed by retained subagent.updated rows. The documented limitation and the scan cost you measured justify leaving the pruning predicate unchanged.

Review thread resolved.

_{You are interacting with an AI system.}

External image Minor · Stability & Availability · Quick win

Re-check the pruning boundary after the replay read.

The code reads prunedThrough once and then reads replay stats and events in separate queries. A pruning pass can run in between. That pass publishes a higher EVENT_LOG_PRUNED_THROUGH value and deletes events in (afterSequence, newBoundary]. The replay then returns a partial event list with no error. The client resumes from a state with gaps instead of taking a snapshot.

The trigger is a client whose cursor is older than the retention window. This is the same client class the boundary check is meant to protect. The window lasts as long as the paged replay read. It also overlaps the throttled pruning pass.

Read prunedThroughSequence again after loadReplayThrough. If input.afterSequence < prunedThrough, call snapshotThenLive(). canReplayPersistedRange has the same check-then-read order, but it is outside the changed lines.

✅ Confirmed as addressed by @juliusmarminge

Fixed in e188f19. Thread and shell resumes now read the replay first and then re-read prunedThroughSequence; if it moved past the cursor (the thread path also requires the cursor to be below the thread high-water), the resume falls back to a snapshot. Pruning publishes the boundary before deleting, so any pass that deleted rows in the replayed range during the read is caught. The shell replay is now collected before replying (it is already bounded by SHELL_RESUME_MAX_GAP and the payload budget). New test "%s resume loads a snapshot when pruning advances during the replay read" gates the replay query, prunes mid-read, and fails for both paths without the recheck.

@juliusmarminge The supplied ws.ts confirms the fix. Both resume paths collect replay events before rechecking prunedThroughSequence and fall back to a snapshot when required. The thread path also preserves the idle-thread high-water exception and error mapping.

This addresses the pruning race I flagged. I did not run the regression test.

Review thread resolved.

_{You are interacting with an AI system.}

External image Minor · Functional Correctness · Quick win

Assert that the idle resume subscription succeeds.

The mock omits streamStoredEventsFrom. The replay branch calls that method while it builds the live stream, so the subscription fails with a TypeError defect. The assertion accepts any failure except "snapshot", so this defect passes the test. A broken idle resume that fails with another error passes the same way.

Add the missing method to the mock. Then assert that the subscription succeeds. A snapshot load still fails this check, because getThreadSnapshot dies with "snapshot".

Do not assert a synchronization marker here. The marker is emitted only when requestCompletionMarker is true, and this test does not set it.

✅ Confirmed as addressed by @juliusmarminge

Fixed: the mock now provides streamStoredEventsFrom, and the test asserts the subscription succeeds (a snapshot load still dies with "snapshot"). It fails with "expected false to be true" on the previous resume condition.

@juliusmarminge The supplied code includes both fixes. The success assertion now rejects snapshot loads and unrelated subscription failures. Your reported failure with the previous resume condition confirms the regression check.

Review thread resolved.

_{You are interacting with an AI system.}

External image Minor · Data Integrity & Integration · Quick win

Qualify the command-receipt retention rule.

ProjectionMaintenance deletes receipts for imported legacy thread commands. Therefore, “Command receipts stay for idempotency” is not universal. State that retained V2 receipts provide this guarantee and document the imported-legacy exception. Otherwise, maintainers may rely on deduplication after the receipt was deleted. (raw.githubusercontent.com)

Source: Path instructions

✅ Confirmed as addressed by @juliusmarminge

Fixed in e2ecefd: the overview now says V2 command receipts stay for idempotency and that receipts of imported legacy commands are removed once their threads are imported.

@juliusmarminge Thanks. Your confirmed change in e2ecefd addresses the receipt-retention finding.

Review thread resolved.

_{You are interacting with an AI system.}