Collect the capture that can never be staged, and let a user remove one now - #373
Merged
Conversation
CommitLore — record lintTrailers: clean — 1 commit in Active constraints for the paths this PR touchesLimits (88)
Ruled out (195)
Warnings (46)
Truncated: 10 lines omitted — the comment hit GitHub's 65000 character limit. Trailer violations fail this check. Active constraints are informational — they are what the repository already decided, not a verdict on this PR. |
…ne now `expires_at` is stamped at stage time, so a transaction that stops at `prepared` or `verified` carries `expires_at: null` for the rest of its life. `pending-gc.ts` requires a parseable expiry before it will consider a file and keeps it otherwise, which reads as fail-closed and is not. Fail-closed protects a file that might still be finalised; on these two phases that branch could never fire at all, so the file was kept for ever. `commitlore pending` had `ls` and `show` and no `rm`, so nothing could remove it either -- `pending ls` could only name the condition, which is what its `never-collected` flag was saying. because it is -- most commits carry nothing worth recording -- and every skip is a `verified` transaction deliberately never staged. The intended common outcome leaked a file each time, and `pending ls`, which exists to show what is genuinely in flight, filled with what is not. `prepared` and `verified` now age out on `created_at`, the only timestamp they have. Age alone is not permission: the file is collected only once HEAD has moved past `base_head`, which is exactly what `stageCaptureRecord` refuses on, so a transaction that could still be staged is left alone however old it is. `pending ls` already derived that as its `stale` flag; both callers now ask one function, because two copies of "can this still be finalised" is how a listing and a collector come to disagree about the same file. The window is 24 hours, chosen rather than inherited. Staleness is what makes collection safe; the window is margin for the one way that proof goes temporarily wrong, since an amend, a reset or a branch round-trip can put HEAD back on `base_head`, and those cycles run in seconds rather than days. It is the same 24 hours as the consumed window because nothing here argues for two retention stories in one directory. Every other branch is untouched: `staged` and `applied` are still protected outright, `consumed` still ages on its own clock, and a file whose phase or age cannot be read is still skipped. `pending rm <nonce>` covers the user who wants the file gone before then. It refuses `staged` and `applied` and names the phase in the refusal, because the post-commit hook may still finalise either into a record -- the same reason gc protects them -- and it refuses a file it cannot read for that reason inverted: an unknown phase might be one of those two. Record-Id: r-gcunstageable Limit: ADR-0021 fixes the pending format and stamps `expires_at` at stage only, so giving these phases an expiry earlier is a format change rather than a fix Limit: gc runs only when `capture gc` is invoked -- nothing schedules it, so a leaked file goes at the next run rather than at the 24-hour mark Limit: staleness is derived from `base_head` against HEAD; a transaction whose staged diff moved while HEAD did not is equally unstageable and is still kept, which is the conservative half of the same test Ruled-out: Collecting on age alone, without the staleness test | a transaction still sitting on the HEAD it was prepared for can be staged and committed at any time, so age alone would delete work the user is still holding Ruled-out: Stamping expires_at at prepare or verify | it moves a field ADR-0021 declares normative and whose null-before-stage shape an mcp-capture oracle pins, and it would expire a capture while the user is still being asked about it Ruled-out: A second staleness rule inside pending-gc.ts | `pending ls` already computes the same question, and a listing that calls a file collectable while the collector disagrees is worse than either answer alone Ruled-out: Deleting the transaction where the skill skips it | the skip lives in a skill a host is free not to run, and a leak avoided only on the well-behaved path is not fixed Ruled-out: A --force for `pending rm` on a staged or unreadable file | an unreadable file may be a newer format this binary cannot parse and another still can, and the two phases it protects are the ones a hook may be seconds from finalising Limit: a `staged` transaction that is never applied is still kept for ever -- the hook skips it once `expires_at` passes and gc protects the phase -- which is a separate leak this change deliberately does not touch Certainty: firm Blast: module Undo: easy Verified: RED first -- the two collection assertions in test/pending-gc.test.ts failed while the four protective oracles beside them passed; after the change pending-gc at 23, pending-command at 14, and the capture, cli, mcp-capture, hook-capture, help-text-honesty, source-guards and manifest suites with them at 246 over 18 files; the whole suite at 78 files, 1962 passed and 1 skipped Verified: the issue's reproduction re-run against the rebuilt bundle -- `capture gc` keeps the prepared file while HEAD is unchanged, keeps it once stale but inside the window, and removes it with `created_at` back-dated 25 hours; `pending rm` deletes a prepared one and refuses a staged one by naming the phase Verified: npm run typecheck clean and dist/ rebuilt by npm run build Unverified: whether 24 hours matches how long a prepared transaction actually sits in practice -- the window is argued from amend-cycle duration and the existing consumed window, not measured against real usage Unverified: the HEAD round-trip the window exists for is reasoned about rather than exercised -- no test moves HEAD off `base_head` and back before gc runs
MongLong0214
force-pushed
the
fix-issue-367
branch
from
August 1, 2026 05:42
c085eaa to
5cd6b8f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #367.
The defect
expires_atis stamped at stage time (capture-stage.ts), so a transaction thatstops at
preparedorverifiedcarriesexpires_at: nullfor the rest of itslife.
pending-gc.tsrequires a parseable expiry before it will consider a fileand keeps it otherwise. On those two phases that branch could never fire at all,
so the file was kept for ever — and
commitlore pendinghadlsandshowandno
rm, so nothing could remove it.pending lscould only name the condition,which is what its
never-collectedflag was saying.#341 made that the ordinary path: skipping is stated there as ordinary because it
is, and every skip is a
verifiedtransaction deliberately never staged.Before
The change
preparedandverifiedage out oncreated_at, the only timestamp they have.Age alone is not permission: the file is collected only once HEAD has moved past
base_head— the conditionstageCaptureRecorditself refuses on — so atransaction that could still be staged is left alone however old it is.
pending lsalready derived that as itsstaleflag, and both callers now ask onefunction rather than keeping two copies of "can this still be finalised".
The retention window is 24 hours, chosen rather than inherited. Staleness is
what makes collection safe; the window is margin for the one way that proof goes
temporarily wrong — an amend, a reset or a branch round-trip can put HEAD back on
base_head, and those cycles run in seconds rather than days. It is the same 24hours as the consumed window because nothing here argues for two retention
stories in one directory.
Nothing else moves.
stagedandappliedare still protected outright,consumedstill ages on its own clock, and a file whose phase, age or basecannot be read is still skipped.
commitlore pending rm <nonce>is added for the user who wants the file gonebefore then. It takes a nonce prefix like
show, and refuses:stagedorappliedtransaction — the post-commit hook may still finaliseeither into a record, which is the same reason gc protects them;
After
Verification
RED first: the two collection assertions in
test/pending-gc.test.tsfailedbefore the change while the four protective oracles alongside them passed, so
they are not vacuous.
test/pending-gc.test.ts— 23, including oracles for an aged transaction whosebase is still HEAD, an aged
stagedone, and one with no recorded base.test/pending-command.test.ts— 14, coveringrmand its four refusals.pending,cli, the capture suites,mcp-capture, bothcapture hooks,
help-text-honesty,source-guardsandmanifest— 246 over18 files.
npx vitest run— 78 files, 1962 passed, 1 skipped.npm run typecheckclean,npm run buildrerun,dist/committed.The reproduction above was re-run against the rebuilt bundle.
ADR-0028 recorded this leak as an accepted consequence; it carries an amendment
saying it is closed and how, rather than being rewritten.