Skip to content

fix: cap the consolidation response, and stop reading a store too large to read (#346) - #347

Merged
fdaviddpt merged 1 commit into
mainfrom
fix/346-unbounded-growth
Aug 12, 2026
Merged

fix: cap the consolidation response, and stop reading a store too large to read (#346)#347
fdaviddpt merged 1 commit into
mainfrom
fix/346-unbounded-growth

Conversation

@fdaviddpt

Copy link
Copy Markdown
Contributor

Closes #346

The reporter's root cause is wrong, and the way it is wrong is the bug

The report says recent.md and archive.md "only ever get appended to". They have never been appended to: run-consolidation.sh:159-160 cps both wholesale from the model's response, and those two lines are unchanged since the initial commit. There is exactly one writer in the tree.

Consolidation caps its input and never caps its output.

  • pipeline/consolidate.py:315-322 refuses to send a prompt over consolidate_max_bytes (default 600000).
  • pipeline/haiku.py:686 runs the CLI with capture_output=True, timeout=timeout — bounded by a wall clock, not by bytes. The size of a response was never a quantity anything downstream measured.
  • pipeline/shell.py writes result.recent to a temp file verbatim and run-consolidation.sh:159 copies it over recent.md. No size check on that path.

Measured on a stub: one round wrote 52,428,825 bytes — 87× the cap the same function had refused to send seconds earlier.

One oversized write is permanent, and that is what produces the reported symptom. recent.md is part of the input the cap is measured on, so the round after an oversized write assembles an oversized prompt, raises ConsolidationTooLarge, and skips — and so does every round after it. _rotate_archive is no escape when the bulk is recent.md. The file then never grows again and never shrinks either, which from outside is indistinguishable from a file that is only ever appended to. The observation was right; the mechanism was inside out.

What this changes

  1. Refuse an oversized response (consolidate()). Deliberately a plain ConsolidationSkipped, not ConsolidationTooLarge: that subclass means "the input was too big, shrink it and retry", and the caller acts on it by rotating archive.md. Nothing about the input was wrong here, so a retry would spend another model call to be handed another oversized response and would rotate away a healthy archive for nothing.

  2. stat the store before reading it (cmd_consolidate()). The cap was enforced on the assembled prompt, so a 6.4 GB recent.md had to be read into memory and a prompt built around it before the pipeline was allowed to notice it was too large to send. That is the allocation that took the reporter's machine down, from a script that runs disowned beside a live session. It cannot produce a false skip: the assembled prompt is the template plus per-file labels plus these bytes, so it is strictly larger than their sum.

  3. Name, do not inject, an oversized memory file (session-start-hook.sh). Nothing above helps the 6.4 GB file someone already has on disk, and that file is what froze every claude launch in that project. Over thresholds.memory_inject_max_bytes (new, default 200000) the file is listed with its size instead of cat'd — the same "kept but not injected" trade Recall should read rotated archive-<date>.md siblings #124 makes for rotated archives, reached by size rather than by filename. The bytes stay on disk and stay greppable.

Not fixed here

  • A store that is already broken is not recovered. After this, such a store starts sessions and skips consolidation forever; the only cure is still deleting the file. Filed separately.
  • now.md / today-*.md are the genuinely append-only filesstaging_append is a literal >> with no cap, and five documented branches append a duplicate span without rolling it back. Different bug, different files, filed separately.
  • The 600-token compression instruction is advisory. Nothing enforces it, so a faithful re-emitter grows recent.md ~2 KB/round; measured hitting the cap at round 298. Bounded by the cap by construction, so it cannot reach a gigabyte on its own — it is the slow grower, not the multiplier. Noted in the CHANGELOG.

Verification

TDD: the test file was written first and failed 5/6 (the pass is the control — a normal response must still be written). Representative RED:

assert 'CONSOLIDATION_STATUS=skip' in 'STAGING_COUNT=1\nCONSOLIDATION_STATUS=ok\nRECENT_OUT=/var/...'
AssertionError: recent.md was read into memory to discover it is too large to read
AssertionError: an oversized recent.md was cat'd into the session (2400447 bytes of hook output)

GREEN, full suite:

1576 passed, 43 skipped, 1 warning in 1139.91s
Required test coverage of 80% reached. Total coverage: 92.95%

An existing test caught a real defect in the first attempt: the up-front rotation was not undone when the model call raised. Fixed with a trailing except Exception: _restore_rotation(); raise.

recent.md reached 6.4GB and archive.md 1.8GB in a reporter's store, freezing
every session start. Nothing appends to either file — the only writer is the
wholesale `cp` in run-consolidation.sh. The defect is that consolidate()
refuses to SEND a prompt over consolidate_max_bytes but nothing bounded what
it WRITES back, and recent.md is part of the input that cap is measured on,
so one oversized write froze the store permanently: it never grew again and
never shrank either, which reads from outside as append-only.

- consolidate(): refuse a response over the cap as non-conforming
  (ConsolidationSkipped, not ConsolidationTooLarge — retrying would not help)
- cmd_consolidate(): stat the store before reading it, so an oversized store
  is not loaded into RAM to discover it cannot be sent; archive rotation moves
  ahead of the read and is undone on every path that does not go through
- session-start: a memory file over thresholds.memory_inject_max_bytes (new,
  default 200000) is named with its size instead of cat'd, so an already
  broken store no longer hangs the launch

Co-Authored-By: Max <noreply>
@fdaviddpt
fdaviddpt merged commit 477a265 into main Aug 12, 2026
12 checks passed
@fdaviddpt
fdaviddpt deleted the fix/346-unbounded-growth branch August 12, 2026 22:05
fdaviddpt added a commit that referenced this pull request Aug 18, 2026
…orever (#348) (#356)

* fix: rotate recent.md out of the over-cap state instead of skipping forever (#348)

oversized file freezing the session. It did not get anybody out of it.
Once recent.md alone exceeded thresholds.consolidate_max_bytes, every
round sized the store, found it over, and skipped -- forever, because
_rotate_archive was the only escape hatch in the tree and it only ever
touched archive.md. Staging never retired either, since retirement
happens after a successful round. The only recovery was
`mv recent.md recent.md.bak && touch recent.md`, which discards history.

recent.md now rotates to a dated sibling the same way archive.md has
since #123, and the read path and history hint learned the second
family so the slice stays greppable (#124's finding, for the new name).

Which file moves is arithmetic, not a guess. Drop archive.md if that is
enough; otherwise, if the staging bytes alone would fit, drop recent.md
too -- and archive.md as well only when staging plus archive would still
not fit. If past-day staging is over the cap on its own, nothing is
rotated at all: no rotation available would change the next round, so
moving recent.md would split an unconsolidated span for nothing.

Two rotations can now happen in one round, so the undo discipline #347
established covers both. The narrow band where the pre-read guard passes
and the assembled prompt does not used to reach an exit that returned
without undoing anything.

doctor.sh gains the other half of the #347 session-start notice, which
told the user to run it against a report that said nothing about the
condition. The self-healing shape is a WARN whose remediation is do
nothing; staging-alone-over-cap is a FAIL with a verdict arm of its own,
because nothing in the pipeline clears it.

Co-Authored-By: Max <noreply>

* fix: give doctor's store measurement a third state, and a control for the configured cap (#348)

Both from the audit of cd0f71c.

An unreadable memory file contributed the same 0 as an absent one, so a
store whose recent.md could not be read summed to nothing and the report
said it fits the cap -- "I looked and found nothing" and "I could not
look" arriving as the same sentence, from the command whose whole job is
telling a human whether to worry. The unreadable files are now named,
the total says it is a floor, and a floor under the cap is reported as
"could not be determined" rather than OK.

The reader sets a variable instead of echoing one, because a caller
writing `x=$(_size_of f)` runs it in a subshell and the unreadable list
would die with it -- the same defect one level down.

Separately, the branch that reads thresholds.consolidate_max_bytes out
of the merged config had no test: every case left REMEMBER_CONFIG unset
and landed on the hardcoded 600000, which is the number the assertions
already used, so a wrong key or a JSON shape the pattern did not
anticipate would have kept the report silently answering against the
default. It now has a positive control and its paired no-config half.
That branch was already correct; what was missing was the evidence.

Co-Authored-By: Max <noreply>

* docs: move the 348 entry into changelog.d now that the fold exists (#348)

origin/main had no changelog.d/ when this branch was cut, so the entry
went into CHANGELOG.md under [Unreleased] -- the only mechanism that
existed on that base. #351 has since merged and brought the fragment
directory, the vendored assembler and a CI gate that fails any pull
request carrying neither a fragment nor the no-changelog label, so the
branch as it stood would have gone red on that job.

The rebase resolved the CHANGELOG.md conflict to origin/main byte for
byte, so this commit only adds the fragment: the fold now has exactly
one source. Shape checked against changelog.d/README.md and against
what .oss/assemble_changelog.py enforces rather than inferred from the
filename -- the two agree, and both legs of oss-changelog.yml pass here
(--check: 2 fragments, all names parse; --check-links: ok).

Co-Authored-By: Max <noreply>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

recent.md and archive.md grow unbounded — hit 6.4GB/1.8GB, froze sessions

1 participant