Skip to content

fix(control): control history prunes old unkeyed calls and their payloads (#1274) - #1330

Merged
Juliusolsson05 merged 11 commits into
mainfrom
fix/control-history-bound
Sep 27, 2026
Merged

Juliusolsson05 merged 11 commits into
mainfrom
fix/control-history-bound

Conversation

@Juliusolsson05

@Juliusolsson05 Juliusolsson05 commented Sep 27, 2026 •

Copy link
Copy Markdown
Owner

Refs #1274. This PR fixes the bulk of the growth; the residuals below keep the issue open.

Retention window: 90 days, the owner's decision (2026-09-27). "#1330, control history: 90 days", with the standing preference for infrequent deletion. This replaces the UNCONFIRMED 7-day default and is pinned in 70303a8e.

Problem

FileControlHistory appended every control call to events.jsonl, plus a payload file per prompt and result, and never removed anything. It also held the whole journal in memory. The owner's store is 129 MB after 22 days; about 110 MB of that is read-only transcripts.page results.

Change

  • Window: 90 days. On today's store (under 30 days old) it deletes nothing yet. At ~5 MB of payloads a day, steady state is up to ~450 MB, where it used to grow without bound in a journal held whole in memory.
    On a clean load (a launch that recovered the journal does not prune), open() prunes whole calls that are unkeyed, finished, older than the window, and not needed as evidence. A call is kept, whatever its age, when any of these holds:
  • a row carries a requestKey: the executor's dedupe ledger (steering q12);
  • it has no result row, or its result is not proven settled. Settled means a completed/ui_opened success, a not_started refusal, or an MCP transport echo. outcome_unknown, pending, and a missing or unreadable payload are kept;
  • it is a lifecycle task origin: a step payload, parsed like the task store parses it, names a task.* step, or cannot be read. operations.read/finish look tasks up by it;
  • a kept row names it as reusedCallId;
  • any quarantine file whose digest is not accepted names it, including a torn-tail one that blocks nothing. Its payload digests are kept too, even from a damaged line;
  • a row's time is unparseable (unknown age).

The kept rows are rewritten atomically, and only then are payload files no kept row or unaccepted quarantine names deleted; non-digest files such as .tmp are never touched.

  • A failed rewrite skips retention for that launch and serves the unpruned rows. It never fails the load or deletes a payload.
  • Once the rewrite's rename has landed, the rewritten rows are always served, even if the directory sync that follows fails (payload GC is then skipped for that launch, so a power loss that restores the old journal never finds its payloads deleted). A payload that cannot be deleted is warned about and retried next launch. So the next append can never number itself past the file. history.list/history.read describe exactly what is kept.

Plan: docs/plans/2026-09-26-control-history-retention.md, including the review round and the owner question.

Tests

FileControlHistory.test.ts, retention block, 13 cases. The fixture holds real journal rows from the owner (ids, times, callers, keys, digests only). Each recorded digest is remapped, in the test, to a real hashed body of the right shape; the other cases write the executor's and task writer's own row shapes.

  • Old unkeyed calls and their payloads are pruned. Keyed, reused and recent calls are kept, byte-identical apart from sequence. A shared digest survives, an orphan payload goes, and a .tmp stays. A second launch is clean.
  • Task origin: operations.read still returns the completed result after the window.
  • Unsettled results: outcome_unknown and pending are kept (the unknown payload stays readable); not_started, completed, ui_opened and a transport echo are pruned.
  • Quarantine: after a recovery, the next launch keeps the call and its payload; once its digest is accepted, it is pruned.
  • Rewrite failure: with the directory unwritable, the load serves every row and every payload file is untouched; the next writable launch prunes.
  • Reuse-only target and unknown-age row are kept. The window is pinned at 90 days ± 1 minute, at the exact edge (kept), and by value. The recorded-store cases move "now" 83 days later, the amount the window grew, so the same recorded rows are pruned and kept.
  • A launch that recovered does not prune; a real executor round trip still replays an old keyed call.
  • Round 2:
    • a payload deletion failing after the rewrite still serves the rewritten rows, the next append continues the file, and the next launch finds no damage;
    • a payload only an unaccepted torn-tail quarantine names survives until accepted;
    • a settled task origin whose step has another key order, or whose step file is corrupted, is kept.
  • Verification (b):
    • an EIO at the directory sync after the rename still serves the rewritten rows, keeps the expired payloads for that launch, and leaves the file consistent;
    • a quarantine-only payload survives while another call is pruned in the same launch.

Red on the round-1 head 436dc4ac: task origin, unsettled results, quarantine, and rewrite failure. Red on the round-2 head 65175615: the three round-2 cases. Red on 31d01b39: the directory-sync case. Mutations killed:

  • GC before rewrite;
  • treating every ok result as settled;
  • dropping the reuse clause;
  • NaN time counted as 0;
  • a 1-day window;
  • deleting the task guard;
  • deleting quarantine-only digests (in both the evidence scan and GC's referenced set).

src/main/control/: 87/87 pass. npx tsc -b is clean.

Residuals (why Refs, not Fixes)

  • The "90 days" in the history.read / history.list descriptions is not asserted by a test; both are correct at this head (verification a).
  • Keyed calls, task origins, and unknown or pending outcomes are never pruned. On the owner's rate that is about 45 rows a day, and their payloads are small. Pruning them needs a request-key and task lifetime in the control contract, which is an owner decision.
  • Pruning runs on load. A process that runs for weeks grows until its next launch.
  • The first control-history use after the upgrade reads every old expired candidate's payload once (about 100 MB on the owner's store) to classify its result. A reviewer measured 3.2–6.9 s on a clone of that store; later loads take about 35 ms.

🤖 Generated with Claude Code

Juliusolsson05 and others added 2 commits September 26, 2026 17:25
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…oads on load (#1274)

The journal and payloads grew forever (129 MB after 22 days) and lived
whole in memory. On load, finished calls with no request key, not reused,
and older than 7 days are dropped in one atomic rewrite, then unreferenced
payloads are collected. Keyed calls (the dedupe ledger) are kept forever.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Juliusolsson05 Juliusolsson05 added provider:claude Affects Claude Code integration provider:codex Affects Codex integration provider:opencode Affects OpenCode integration provider:pi Affects Pi integration type:bug Something works wrong class:C6-unbounded Unbounded growth sev:P2 Real bug with a workaround labels Sep 27, 2026
Juliusolsson05 and others added 2 commits September 26, 2026 18:08
…arantine evidence (#1330 review)

Retention deleted evidence that is still asked for: unkeyed task origins
(operations.read went not_found), outcome_unknown and pending results,
and payloads an unaccepted recovery quarantine names. It also failed the
whole load when the rewrite failed. These are now retained roots, a
failed rewrite skips retention and GC, and the survivors are pinned.

Refs #1274

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ines, parses task steps (#1330 round 2)

A payload deletion failing after the rewrite made open() serve the old
rows and the next append corrupt the journal; a torn-tail quarantine was
never scanned, so its only-named payload was deleted; task steps were
detected by byte prefix. Rewritten rows are always served, every
unaccepted quarantine is evidence, and steps are parsed.

Refs #1274

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Juliusolsson05

Copy link
Copy Markdown
Owner Author

Review disposition (#1330): 3 reviewers, 2 rounds; head b3444f04. Still on hold for the owner's retention window (7-day default, UNCONFIRMED).

Round 1

Finding Verdict Disposition
A1 / C1: unkeyed task origins pruned, so operations.read returns not_found valid, Major Fixed in 65175615: task origins are retained.
A2: payloads an unaccepted quarantine names were GC'd on the second launch valid, Major Fixed in 65175615. Widened in round 2 to every unaccepted quarantine.
A3: an unkeyed outcome_unknown result was pruned valid, Major Fixed: only proven-settled results are pruned; unknown, pending and unreadable are kept.
B1: payload GC before the rewrite was untested valid Fixed. A forced rewrite failure serves the unpruned rows and deletes nothing; the mutant is killed.
B2 / A: reuse-only target untested valid Pinned.
B3 / A: unknown-age (NaN) untested valid Pinned.
B4: non-digest GC boundary untested valid Pinned (.tmp kept).
B5: fixture shared-digest description wrong valid Corrected (dispatched rows).
C2: 7-day window is an unconfirmed destructive default valid Owner decision, open. The PR stays out of batches.
C3: plan keyed-receipt count (290) wrong valid Corrected to 220.
C survivor: 1-day window passed valid Pinned at 7 days ± 1 minute.

Round 2

Finding Verdict Disposition
a / b / c: a GC failure after the rewrite served the old rows; the next append corrupted the journal valid, Blocker Fixed in b3444f04: rewritten rows are always served, and deletions fail individually. Red on 65175615.
a / b / c: a torn-tail quarantine was never scanned, so its only-named payload was deleted valid, Major Fixed: every unaccepted quarantine file is evidence. Red on 65175615.
a / b / c: task detection by byte prefix (key order, corrupted step) valid, Major Fixed: the step is parsed through the integrity-checked read; unreadable is kept. Red on 65175615.
a / b survivors: task guard, quarantine-only digests valid test gaps Killed by the new cases.
b / c: first-load classification cost (3.2–6.9 s once) valid, Minor Accepted as a residual (in the body). It happens once, on the first control-history use after the upgrade, not at startup. A byte-sniffing shortcut was rejected as a guess.

@Juliusolsson05

Copy link
Copy Markdown
Owner Author

OWNER-APPROVED: control-history retention window = 90 days (not 7; the owner wants infrequent deletion). Tasks, quarantines and unknown outcomes are always kept (q49).
Owner, 2026-09-27T07:33:05Z: "just make these for me, just keep the fixtures in, they are innocent enough. do not delete stuff often" (manager applied it).

@Juliusolsson05

Copy link
Copy Markdown
Owner Author

Disposition, owner decision (2026-09-27): 90 days. Implemented in 70303a8e; main is merged in c289d676.

  • CONTROL_HISTORY_RETENTION_MS = 90 days, pinned by value in the boundary test (90 days ± 1 minute).
  • The history.read and history.list descriptions say 90 days.
  • The recorded-store retention cases move "now" 83 days later, the amount the window grew, so the same recorded rows are pruned and kept.
  • On today's store the window deletes nothing yet. What stays bounded is long-run growth.
  • tsc is clean; src/main/control passes 85/85.
  • Next: a quick verification by one reviewer.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Juliusolsson05

Copy link
Copy Markdown
Owner Author

Disposition, quick verification of the 90-day window (a: MERGE-READY).

  • a counted the recorded fixture: the new "now" minus 90 days and the old "now" minus 7 days are the same instant (2026-09-20T12:00Z), so the same 3 calls / 11 rows are pruned and 5 calls / 19 rows kept.
  • a's surviving >= → > mutation is now pinned (acc070d8): a call exactly at the edge is kept, per the owner's infrequent-deletion preference. The mutation fails the test.
  • Residual: the two tool descriptions' "90 days" is not asserted by a test. Both are correct at this head, and I am not adding a text-matching check.
  • tsc was clean at c289d676; src/main/control passes 85/85.

Juliusolsson05 and others added 2 commits September 27, 2026 01:41
…quarantine digests in GC (#1330 verification b)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Juliusolsson05

Copy link
Copy Markdown
Owner Author

Review disposition, verification pass (head f68c8e2c)

Verdicts: c MERGE-READY; b FIX-BEFORE-MERGE.

Finding Disposition
b1 (Blocker): a failed directory sync after the rewrite's rename still served the pre-rewrite rows, so the next append left a gap and the journal was quarantined Fixed. writeAtomic reports when the rename landed. After that point a failure serves the renumbered rows (with a warning) and skips payload GC for this launch, because a power loss could restore the old journal, which names those payloads. New real-file test with EIO injected at the post-rename sync: the served rows match the file, the payloads remain, the append continues the file, and the next launch has no recovery. It fails on 31d01b39.
b2 (survivor): the quarantine digests in GC's referenced set were unpinned Fixed. New test: a torn-tail quarantine, then another old settled call pruned in the same launch. The quarantine-only payload survives. The test fails with the digests removed.

Verification:

  • NODE_ENV=test npx vitest run src/main/control/history: 38/38.
  • rm -rf .tsc-out; npx tsc -b: clean.
  • b gets a focused recheck.

🤖 Generated with Claude Code

@Juliusolsson05

Copy link
Copy Markdown
Owner Author

Recheck (b) at f68c8e2c: MERGE-READY, with no findings. The post-rename EIO probe and the mixed quarantine-GC probe both pass, and their mutations are killed. All three reviewers are now MERGE-READY at this head; only CI remains.

🤖 Generated with Claude Code

@Juliusolsson05
Juliusolsson05 merged commit cdee833 into main Sep 27, 2026
2 checks passed
Juliusolsson05 added a commit that referenced this pull request Sep 27, 2026
@Juliusolsson05
Juliusolsson05 deleted the fix/control-history-bound branch September 27, 2026 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

class:C6-unbounded Unbounded growth provider:claude Affects Claude Code integration provider:codex Affects Codex integration provider:opencode Affects OpenCode integration provider:pi Affects Pi integration sev:P2 Real bug with a workaround type:bug Something works wrong

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant