Skip to content

CheckpointPerISC: don't stage a concurrent session's work - #1826

Closed
justinkatz94-glitch wants to merge 1 commit into
danielmiessler:mainfrom
justinkatz94-glitch:checkpoint-session-attribution
Closed

CheckpointPerISC: don't stage a concurrent session's work#1826
justinkatz94-glitch wants to merge 1 commit into
danielmiessler:mainfrom
justinkatz94-glitch:checkpoint-session-attribution

Conversation

@justinkatz94-glitch

Copy link
Copy Markdown
Contributor

The problem

CheckpointPerISC already refuses git add -A and scopes staging by mtime. Its own comment names what is left:

Residual: a file another session dirties mid-run still has a fresh mtime and can be swept; this closes the common case, not every case.

I hit that residual. A checkpoint commit for one run captured four files written by a different session running concurrently in the same tree. The content was intact but the subject line described someone else's work, which corrupts the record the Algorithm treats as authoritative ("the changelog is git", and git log -- <path> is the provenance).

Root cause: a timestamp carries no attribution. Two sessions writing one tree in the same window are indistinguishable by mtime alone.

The fix

The attribution is already on disk and was simply never read. Every hook receives transcript_path, and a transcript records the file_path of every write tool call.

foreignClaimedPaths() reads sibling transcripts modified inside the run window and excludes paths another session provably wrote. No new hook, no new state file, no new ledger.

Why exclusion rather than inclusion

I built the obvious version first: stage only what THIS session's transcript claims. Tested against a real session it recovered 14 of 33 paths, because edits made by scripts rather than by Write/Edit tool calls are never named in a transcript. That silently drops real work, which is worse than a wrong subject line.

Exclusion leaves anything unattributed staging on mtime exactly as before. Only another session's provable work is removed, which is precisely the reported defect and nothing more.

Verification

Symmetry across two real concurrent sessions:

Simulated hook file written by session A file written by session B
session B's hook excluded, correct staged, correct
session A's hook staged, correct excluded, correct

Worth flagging: my first test run reported both files excluded, which is impossible. The transcript path was passed relative while the scan built absolute paths, so self-exclusion silently failed and the session's own transcript counted as foreign. Testing one direction could not have caught it, since a design that excludes everything looks identical to one that works if you only check what you expected to be excluded.

Compatibility

No behaviour change when transcripts are absent, unreadable, or yield nothing: the set is empty and staging falls through to the existing mtime filter. The set is computed once per invocation and bounded by filtering to transcripts modified within the run window.

Smoke tested on non-ISA paths, missing ISA paths, and with a real transcript supplied; all exit 0 with {"continue":true}.

The hook already refuses `git add -A` and scopes staging by mtime, and its own
comment names the remaining gap:

  "Residual: a file another session dirties mid-run still has a fresh mtime and
   can be swept; this closes the common case, not every case."

I hit that residual. A checkpoint commit for one run captured four files written
by a different session running concurrently in the same tree. Content intact,
subject line describing someone else's work, which corrupts the record the
Algorithm treats as authoritative ("the changelog is git").

Root cause is that a timestamp carries no attribution. Two sessions writing one
tree in the same window are indistinguishable by mtime alone.

The attribution is already on disk and was simply never read: every hook receives
`transcript_path`, and a transcript records the file_path of every write tool
call. This adds `foreignClaimedPaths()`, which reads sibling transcripts modified
inside the run window and excludes paths another session provably wrote.

Deliberately exclusion-shaped rather than inclusion-shaped. I tried the obvious
version first (stage only what THIS session's transcript claims) and it was worse:
tested against a real session it recovered 14 of 33 paths, because edits made by
scripts rather than by Write/Edit tool calls are never named in a transcript. That
silently drops work, which beats a wrong subject line in the wrong direction.
Exclusion leaves unattributed files staging on mtime exactly as before.

Verified by symmetry across two real concurrent sessions: each session's hook
stages what it wrote and excludes what the other wrote, in both directions. A
first test run reported both files excluded, which is impossible; the transcript
path was relative while the scan built absolute paths, so self-exclusion silently
failed. Testing one direction could not have caught that.

No behaviour change when transcripts are absent or unreadable: the set is empty
and staging falls through to the existing mtime filter.
@justinkatz94-glitch

Copy link
Copy Markdown
Contributor Author

Withdrawing for now to verify CI and re-check against upstream properly before asking for review time. The fix and its test evidence are sound; I would rather resubmit clean.

@justinkatz94-glitch
justinkatz94-glitch deleted the checkpoint-session-attribution branch August 12, 2026 00:22
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.

1 participant