Repository navigation
HistoryService: direct mutations bypass the mutation FIFO and can drop a concurrent add #3443
Description
Found during an independent review of the 0.12.0 merge branch. Pre-existing, not a regression — verified present at premerge/0.12.0-start.
What the FIFO does cover
HistoryService serializes the paths that matter most for compression. replaceAll goes through enqueueAsynchronousHistoryMutation (HistoryServiceCore.ts:816-839), an add() during compression enters the phase queue (HistoryServiceCore.ts:506-537), and endCompression routes release events and flushed closures through runSynchronousHistoryMutation (HistoryService.ts:526-546). If an async replacement is in flight, those closures queue behind it and completeAsynchronousHistoryMutation drains them in order.
That path is sound. This issue is about the methods that sit outside it.
The gap
Four methods mutate this.history directly, bypassing both queues:
| method | location | note |
|---|---|---|
summarizeOldHistory |
HistoryService.ts:477-494 |
awaits, then assigns |
validateAndFix |
HistoryService.ts:407-429 |
direct splice() |
removeLastIfMatches / pop |
HistoryService.ts:302-324 |
direct mutation |
dispose |
HistoryService.ts:217-239 |
direct reset |
summarizeOldHistory has the clearest data-loss shape:
const result = await summarizeOldHistoryHelper(this.history, keepRecentCount, summarizeFn);
if (result) {
for (const item of result) this.chronology.stamp(item);
this.history = result; // any add() during the await is discarded
await this.recalculateTotalTokens();
}result is computed from a snapshot of this.history taken before the await. Any add() landing while summarizeFn is in flight — a network call — is silently overwritten.
Exposure
summarizeOldHistoryhas no production caller in the current tree. Only its own definition and thehistoryContextWindow.ts:52helper it imports. Test-only surface today, but it is public API on a service other packages hold.validateAndFixis called in production, frompackages/agents/src/core/client.ts:725. It splices synchronously, so it cannot interleave with itself, but it can land in the middle of a queued async replacement and operate on history that is about to be replaced wholesale.
Suggested fix
Route these through enqueueAsynchronousHistoryMutation / runSynchronousHistoryMutation like their neighbours. For summarizeOldHistory specifically, either re-read this.history after the await and merge, or take the mutation lock for the whole operation so no add() can interleave.
Worth confirming whether the direct-mutation methods are intended to be lock-free, in which case the fix is documenting that rather than changing them.
Provenance
Reported by an independent review agent auditing the 65-PR 0.12.0 merge for integration defects. Both findings were checked against premerge/0.12.0-start and are present there, so neither is caused by that merge. Raising separately so the merge does not carry unrelated changes.
Activity
github-actions commentedon Aug 31, 2026
Summary
HistoryServicewhere direct mutations can bypass the existing mutation FIFO and drop concurrentadd()work.summarizeOldHistory,validateAndFix,removeLastIfMatches/pop, anddispose.packages/coreand preserve existing public behavior/API shape.HistoryService.*.test.tsfiles; no new test file unless justified.Sizing audit
packages/core)Test plan
packages/core/src/services/history/HistoryService.management.test.tssummarizeOldHistory+add()test that asserts no added content is lost when summarization awaits.validateAndFixconcurrency test showing queued ordering with an in-flight async mutation.removeLastIfMatches/poptests that verify mutation ordering and token recalculation behavior.disposetest verifying cleanup without leaking mutation-queue state and that subsequent operations are safely no-op/error-resilient.packages/core/src/services/history/HistoryService.basic.test.tssummarizeOldHistorypreserving interleavedadd()items.Implementation steps
summarizeOldHistorythrough the async mutation queue, or hold the mutation lock for the whole operation and re-merge any items added during the await.validateAndFixthrough the synchronous mutation path so it cannot interleave with an in-flight async replacement.removeLastIfMatchesandpopthrough the synchronous mutation path; preserve token recalculation semantics.disposedrain/reset mutation state safely and stop queued work from resurrecting history after reset.npm run lint:cinpm run lint:eslint-guardnpm run typechecknpm run testPrompt for AI agents
packages/core/src/services/history/HistoryService.tsand the two existing test files above.runSynchronousHistoryMutation/enqueueAsynchronousHistoryMutation.acoliver commentedon Oct 13, 2026
#3852 (journal-backed history, #854) rewrites HistoryService as a facade over an on-disk journal, so the direct-mutation methods named here no longer mutate an in-memory array. Re-verify against the #3852 code after it merges; close with evidence or adopt under #3868.