Fix Claude Code StopFailure Handling

Review of trail #1472: Fill task records' tokens and Codex completion before commit

Branch: soph/task-record-fields (6 commits, 4 files). CI passes and the trail has no agent findings. The only failing gate is approvals.

Overall: The direction is right and both fixes are small. I found one real gap in which sessions the commit-time refresh covers, plus a misplaced doc comment. Neither blocks the Claude Code token fix.

Issues

1. The refresh can skip a session that the commit then condenses (lifecycle.go, refreshCodexInventoriesBeforeCommit)

• The refresh only looks at sessions where state.WorktreePath == worktreeRoot, or where the worktree is unknown.
• PostCommit picks a wider set through findSessionsForCommitLinking. That is the worktree match plus a "guest" session matched by process ancestry (findSessionByCommitAncestry, session_identity.go:39).
• So if a Codex session owned by worktree A commits in worktree B, PostCommit condenses it, but the refresh skips it. That brings back exactly the bug this PR fixes: the task record is stored as still in flight, with no files.
• Suggested fix: export a strategy helper that returns the commit-linking set, and refresh exactly those sessions. That way the two selections can't drift apart again. It would also cover the next point.

2. The refresh doesn't skip adopted-away sessions. A session moved to another worktree with entire session adopt leaves its old record behind as a tombstone (AdoptedIntoWorktreePath != ""). That tombstone keeps its old WorktreePath, so the refresh can rescan and modify it on every trailered commit. Other hook paths skip these explicitly (manual_commit_hooks.go:3005). This one is minor, and the shared helper from point 1 would handle it.

3. Misplaced doc comment (lifecycle.go ~1315)

• The new function and its doc comment were inserted between refreshCodexInventory's existing doc comment and its func line.
• As a result, refreshCodexInventoriesBeforeCommit now starts with the doc text that belongs to refreshCodexInventory ("snapshots the durable child ledger…"), and refreshCodexInventory has no doc at all.
• Fix: move the new function above that comment block.

Smaller points

• Trailer check differs from PostCommit. headHasCheckpointTrailer uses trailers.ParseCheckpoint(commit.Message). PostCommit uses stampedTrailersOf + condensationTarget, which ignores trailers inherited from an earlier commit. An amend that keeps an old trailer would run the refresh even though nothing condenses. That only costs time, but the helper from point 1 would remove the mismatch.
• Naming. refreshCodexInventoriesBeforeCommit runs inside the post-commit hook. Something like …BeforeCondensation would describe it more accurately.
• Empty transcripts. If a Claude subagent transcript exists but is empty, CalculateTotalTokenUsage returns a non-nil zero TokenUsage. That gets stored as an exact zero instead of "unknown". It's an edge case, but nil would be more honest there.
• Test coverage. There's one integration test for the happy path, but none for the new filters: no trailer, another worktree, ended session, unreadable parent. A table test would protect the cost limits that commits 9f7c167, c5dcb38 and 8dbaf39 added.

Things I checked that are fine

• The token fallback only reaches agents it should. Codex subagent stops are always provisional (codex/lifecycle.go:179), so they return early and never reach completeSubagentTaskRecord. That means the "fork counters aren't exact child usage" problem can't come back through this path. Cursor declares transcripts but has no token calculator, so it gets nil.
• No double counting. TaskRecord.TokenUsage is only written into task.json (manual_commit_condensation.go:424, persistent.go:517). It is never added to session totals.
• Ordering is safe. refreshCodexInventory writes through MutateSessionState, and PostCommit loads state fresh afterwards.
• The test refactor keeps the original assertions. Extracting newCodexSubagentScenario doesn't drop any, and adding ENTIRE_TEST_CODEX_SESSION_DIR to ExtraEnv is the right isolation fix for git-hook reads.

Verdict: Fix point 1 (it also takes care of point 2) and move the doc comment; after that it's good to approve. Want me to post these as comments on the trail, or request changes?

How a commit gets linked to sessions today

It happens across two git hooks. Both call the same selector, so the session the trailer is stamped for is the session that gets condensed.

1. prepare-commit-msg decides whether to stamp a trailer

PrepareCommitMsg (manual_commit_hooks.go:340):

1. It skips rebase, cherry-pick and revert, and merge commits. Amend goes to prepareAmendCommitMsg.
2. It records any trailers inherited from squashed or redone commits in a marker file, entire-inherited-trailers.json in the per-worktree git dir. The point is that post-commit must never condense into one of those.
3. findSessionsForCommitLinking(ctx, worktreePath) picks the candidate sessions (see below).
4. If an agent commits mid-turn, tryAgentCommitFastPath stamps without checking content. Otherwise filterSessionsWithNewContent keeps only sessions that have something to condense, and stops if none do.
5. It stamps Entire-Checkpoint: <id>. The ID comes from checkpointIDForSessions.

2. post-commit condenses

PostCommit (manual_commit_hooks.go:1086):

1. It reads HEAD, then calls stampedTrailersOf. That parses all trailers and removes the inherited ones listed in the marker; reading the marker also deletes it. condensationTarget then picks the trailer to condense into and notes whether that checkpoint already existed (an amend).
2. With no stamped trailer, it calls postCommitUpdateBaseCommitOnly and returns.
3. With a stamped trailer, it calls findSessionsForCommitLinking again. The comment says it "must resolve the same way PrepareCommitMsg did, or the stamped trailer and the condensed session diverge".
4. For each session that isn't both FullyCondensed and ended, it calls postCommitProcessSessionLocked under the session lock. That function decides per session whether to condense, using file overlap with the commit, task content, and phase.
5. It cleans up shadow branches and logs if no session claimed the trailer.

The selector: findSessionsForCommitLinking (session_identity.go:31)

It returns the union of two sets:

A. Worktree matching (findSessionsForWorktreeFromStates, manual_commit_session.go:284):

• Exact match: sessions whose WorktreePath equals this worktree. If there are any, only these are used.
• Fallback, used only when nothing matches exactly: sessions from other worktrees of the same repo (same git common dir).
  • Sessions from a parent worktree (this worktree is nested inside it) are preferred.
  • If all candidates come from one worktree, they're linked.
  • If they span several worktrees, only sessions with activity in the last 15 minutes are kept, and they're linked only if a single worktree is left. Otherwise nothing is linked, and the user gets a hint to run entire session adopt if identity matching also finds nothing.

B. Identity matching (findSessionByCommitAncestry): this finds the session whose recorded owner process is an ancestor of the hook process. That's an agent committing from any worktree, called a "guest". It needs Owner and WorktreePath set, and skips imported and adopted-away sessions. If several match, the nearest ancestor wins.

The source list comes from listAllSessionStates, which also drops orphaned ended sessions that have no shadow branch.

Where the PR's filter falls short

The new refreshCodexInventoriesBeforeCommit only handles "exact match, or no worktree recorded". Compared with the selector above, it misses two kinds of session that PostCommit does condense:

In the two no rows, the child's task record is condensed in flight, with no completed_at and no files. That's exactly the bug this PR exists to fix.

There's a second, smaller gap. The PR's headHasCheckpointTrailer uses trailers.ParseCheckpoint on the raw message, so it treats inherited trailers as condensing when they don't. It doesn't consume the marker, so it doesn't break PostCommit; it just runs the refresh when it doesn't need to.

Suggested fix

Have the refresh use PostCommit's own selection instead of approximating it:

• Option 1 (smallest change): export something like (*ManualCommitStrategy).SessionsForCommitLinking(ctx, worktreePath). The refresh would loop over that set, applying the Codex, live-record and not-ended checks.
• Option 2 (cleaner): move the refresh into PostCommit itself as a hook or callback. It would run after condensationTarget reports found and after the sessions are resolved, before the process loop. That's one selection, one trailer check that already handles inherited trailers, and no second state listing. The catch is that strategy can't import cli, which is where refreshCodexInventory lives. So it would need an injected function, for example a field on the strategy, set by the post-commit command.

I'd go with option 2. The only way to keep the refresh set and the condensed set from drifting apart is to compute them once.

Want me to draft that change, or post this on the trail as a request-changes comment?

What post-commit does with each linked session

After findSessionsForCommitLinking returns the set, PostCommit handles each session in turn (manual_commit_hooks.go:1199). It skips any session that is both FullyCondensed and ENDED. For every other one it calls postCommitProcessSessionLocked (:1585) while holding that session's lock. Each step below matters for this PR.

1. Guard against an interrupted condensation

If the session already reserved a different checkpoint ID (PendingCondensationID), the session is left alone and its shadow branch is kept.

2. Decide whether there is new content (hasNew)

• ACTIVE session: hasNew = true without checking. The comment's reasoning is that prepare-commit-msg already approved the trailer, and "subagent transcripts may not be available yet (subagent still running)".
• IDLE or ENDED session: it calls sessionHasNewContent against the shadow tree. If that check errors, it assumes there is new content.

It also saves filesTouchedBefore. For an ACTIVE session that comes from resolveFilesTouched, which reads the transcript; otherwise it's a copy of state.FilesTouched.

3. Run the GitCommit transition (session/phase.go)

4. The condense decision: shouldCondenseWithOverlapCheck (:935)

1. If !hasNew, it doesn't condense.
2. Trusted shapes: an ACTIVE session with a recent interaction (under 24h), or an IDLE session with a live task record started in the last 24h (idleWithLiveTaskRecord). These condense without needing file-overlap evidence. The exception is the read-only gate: if the session tracks no files and other sessions claim the committed files, it's skipped unless liveTaskCoauthored says one of its running subagents touched them.
3. Everything else has to prove overlap: files it touched that changed in this commit, with matching content (filesOverlapWithContent).

If it doesn't condense, it only calls updateBaseCommitIfChanged.

5. Condense: condenseAndUpdateState → CondenseSession

• It refuses to write into a checkpoint that already exists and that this session didn't stamp (stampedByAnotherCommit).
• It writes the transcript, metadata and attribution to the checkpoint.
• materializeTaskRecords (manual_commit_condensation.go:404) writes every task record into tasks/<id>/task.json (files, token_usage, started_at, completed_at) plus its transcript. Records that are still in flight are written exactly as they stand, with no completed_at, and they stay in state so the next condensation writes them again. Completed records are removed from state afterwards (resetCheckpointWindow).
• Guest-linked sessions (committed from another worktree): the transcript is condensed and the trailer is honoured, but all worktree-specific state changes are skipped (BaseCommit, shadow branch, carry-forward). That's because they belong to the session's home worktree.

6. Bookkeeping after the transition

• ACTIVE and condensed: the checkpoint ID is appended to TurnCheckpointIDs, so turn end can finalize it later with the full transcript.
• Carry-forward: only when the session condensed and this is its home worktree. Files the agent changed but weren't committed move to a new shadow branch, so the next commit gets its own checkpoint.
• ENDED with nothing left over: marked FullyCondensed, so later commits skip it.
• Review sessions: ended and marked condensed once they've condensed.
• ACTIVE, not condensed, with content on the shadow branch: the shadow branch is kept.

After the loop, PostCommit updates the combined attribution, deletes shadow branches nothing needs any more, and may warn about stale ended sessions.

What this means for PR #1472

• The scenario the PR fixes is a Codex parent that is still ACTIVE, or IDLE with a live record, so it takes a trusted shape and condenses no matter which worktree it's in. Step 5 then writes the child record exactly as it stands. That's why the refresh has to run for every session this loop will condense, including guest-linked and fallback-matched ones. The PR's exact-worktree filter covers only some of them.

• A session that won't condense doesn't need the refresh: an ENDED one with no files, or one failing the overlap check. Session end or turn end will reconcile its records later, so refreshing it only costs time. Ideally the refresh would sit inside the loop, just before TransitionAndLog, for sessions with live records.

• The refresh can't be done inside postCommitProcessSessionLocked as written. refreshCodexInventory takes the session lock itself and reads rollouts outside the lock, and the loop already holds that lock. So the practical placement is in PostCommit itself:

  1. After findSessionsForCommitLinking, and before the process loop, call an injected preCondense(ctx, sessions) callback.
  2. The cli package registers that callback; it filters to Codex sessions with live records that aren't ended and runs refreshCodexInventory on each.
  3. The loop's MutateSessionStateOnSaved then reloads state, so it sees the refreshed records.

  This uses the same set, the same trailer decision (inherited trailers already excluded), and no second state listing.

Should I prototype that injection and a test for a guest-linked commit?

Partly, but shadow branches are only one of the two things post-commit keeps track of. The other is the one this PR is about.

What post-commit keeps in sync

1. Shadow branches (worktree content)

• Each session has a shadow branch, keyed by BaseCommit and WorktreeID. It holds snapshots of the agent's file changes that haven't been committed yet.
• When the session condenses, post-commit moves what belongs to this commit into the permanent checkpoint on the metadata branch.
• It then does one of three things with the shadow branch:
  • Delete it, if no session on that branch still needs it.
  • Carry forward files that weren't committed to a new shadow branch, so the next commit gets its own checkpoint.
  • Keep it, if an ACTIVE session that didn't condense still has content there.
• It also moves BaseCommit forward so the session follows HEAD.
• Guest-linked sessions don't touch any of this, because the shadow branch belongs to their home worktree.

2. Session state (in .git/entire-sessions/, not on any branch)

• This is where the transcript offsets, FilesTouched, token counts, TurnCheckpointIDs, FullyCondensed, and task records live.
• Task records never live on the shadow branch. That's why isOrphanedSessionState keeps a session that has records even when its shadow branch is gone.
• Condensation reads them straight from session state (materializeTaskRecords) and writes them to task.json exactly as they stand at that moment.

Why that matters for #1472

The shadow-branch side is fine whichever worktree commits. The bug is entirely on the session-state side: a Codex child record is still marked in flight when condensation reads it. The child's rollout already shows its turn complete, but nothing has reconciled the record yet, so it's stored without completed_at, files or tokens.

So the refresh doesn't need to care about shadow branches at all. It needs to make sure that every session the post-commit loop is about to condense has up-to-date task records. That's the full set findSessionsForCommitLinking returns, not only the sessions whose WorktreePath matches.

If you meant something else, like moving task records onto the shadow branch, tell me and I'll look at that instead.

Not quite. A missed refresh doesn't damage the session state; it damages the checkpoint that gets written from it.

What happens when the refresh misses a session

Take a guest-linked Codex parent that commits after its child has finished but before its own turn ends:

1. Post-commit condenses it anyway, because an ACTIVE session or an IDLE one with a live record is trusted. materializeTaskRecords writes the child's task.json as it stands then: no completed_at, no files, no token_usage.
2. Session state itself is fine. An in-flight record stays in state on purpose (manual_commit_condensation.go:382). Later, the parent's turn end or session end reads the child rollouts and completes the record properly.
3. The checkpoint already written is never fixed. At turn end, finalizeAllTurnCheckpoints (manual_commit_hooks.go:3551) goes back over TurnCheckpointIDs, but it only replaces the session transcript. It doesn't rewrite tasks/<id>/task.json. So that commit's checkpoint permanently shows the child as unfinished, with none of its files.
4. The completed record ends up somewhere else. It's written into whichever checkpoint condenses next, which is a different commit than the one that contains the child's work. If no commit follows, it waits as an ENDED session with task content, and only gets condensed by entire doctor or the background zombie sweep.

Impact

• State: recovers by itself.
• The commit's checkpoint: stays wrong. entire explain on that commit can't attribute the child's edits to the subagent, and its tokens are missing.
• Attribution: the completed record is linked to a later, unrelated commit.

That's the same failure the PR describes, so it isn't new. The PR's filter just leaves it in place for guest-linked and fallback-matched sessions. Fixing point 1 by running the refresh over the exact set the post-commit loop condenses closes that gap. A second, separate improvement would be letting turn-end finalization also rewrite task records in TurnCheckpointIDs. That would fix any remaining race after the fact, but it's a bigger change than this PR needs.

Want me to post this as a request-changes comment on #1472?

Here's the review again, updated with what we found in the post-commit flow. Point 1 is now precise, point 2 is folded into it, and its impact is confirmed.

Findings for trail #1472

1. The refresh misses sessions that post-commit condenses (blocking)

Where: lifecycle.go, the state.WorktreePath != worktreeRoot filter in refreshCodexInventoriesBeforeCommit.

Problem: PostCommit condenses whatever findSessionsForCommitLinking returns. The refresh only checks "exact worktree, or worktree unknown", so it skips two kinds of session:

• Fallback-matched: nothing matches this worktree exactly, so sessions from a sibling or parent worktree of the same repo get linked. For example, Codex started in the main checkout and the commit is made from a linked worktree.
• Guest-linked: a Codex agent commits from another worktree and is matched by process ancestry (findSessionByCommitAncestry).

Both shapes are trusted to condense (ACTIVE recent, or IDLE with a live record), so materializeTaskRecords writes the child's task.json with no completed_at, files or token_usage. Session state recovers at turn end or session end. But finalizeAllTurnCheckpoints only rewrites the session transcript, so that commit's checkpoint stays wrong for good, and the completed record lands in a later, unrelated checkpoint.

The opposite mistake also happens: adopted-away tombstones (AdoptedIntoWorktreePath != "") that still carry this worktree's WorktreePath get rescanned on every trailered commit, even though post-commit would never link them.

Fix: refresh exactly the set post-commit condenses. The cleanest way is an injected hook in PostCommit:

1. After condensationTarget reports found and findSessionsForCommitLinking returns, and before the process loop, call preCondense(ctx, sessions).
2. The cli package registers that hook; it filters to Codex sessions with live records that aren't ended and calls refreshCodexInventory on each.
3. Each pass of the loop reloads state through MutateSessionStateOnSaved, so it sees the refreshed records.

The hook has to run before the loop, not inside postCommitProcessSessionLocked, because refreshCodexInventory takes the session lock itself.

Test: an integration test where the commit happens in a second worktree, guest-linked by ancestry or fallback-matched, and the stored task.json is still completed with its files.

2. Trailer check differs from post-commit's (minor; goes away with fix 1)

headHasCheckpointTrailer checks the raw message with trailers.ParseCheckpoint. PostCommit uses stampedTrailersOf + condensationTarget, which leave out inherited trailers. So squash merges, commit -C and similar run the refresh even though nothing condenses. It doesn't consume the inherited-trailers marker, so PostCommit still behaves correctly; it's only wasted work. Fix 1 removes this helper entirely.

3. Misplaced doc comment (nit)

Where: lifecycle.go ~1315. The new function was inserted between refreshCodexInventory's doc comment and its func line. The new function's doc now starts with text that belongs to the other one, and refreshCodexInventory has no doc. Move the new function above that comment block.

4. Naming (nit)

refreshCodexInventoriesBeforeCommit runs in post-commit; …BeforeCondensation would describe it accurately. If it becomes the hook from fix 1, name it after the hook.

5. Empty transcript stored as exact zero (nit)

subagentTokenUsage: if a Claude subagent transcript exists but is empty, CalculateTotalTokenUsage returns &TokenUsage{}, not nil. That stores an exact zero instead of "unknown". Returning nil when the data is empty would fix it.

6. Missing filter tests (minor)

There's no unit coverage for the skip rules the follow-up commits added: no trailer, ended session, unreadable parent, other worktree. With fix 1, the important tests become "the refresh set equals the linking set" and "non-Codex sessions or sessions with no live records are skipped".

Checked and fine

• The token fallback can't reach Codex, since its subagent stops are always provisional.
• Cursor declares transcripts but has no token calculator, so it gets nil.
• TaskRecord.TokenUsage only goes into task.json, so nothing is double-counted.
• The refresh's state writes happen before the loop loads state.
• The integration test refactor keeps every original assertion.

Verdict: request changes on point 1. Points 3–5 can go in the same pass.

Should I post this on the trail with entire trail request-changes 1472? I can also put point 1 as a separate trail comment and the nits inline.

I posted all six findings to trail #1472. Each is anchored to a line in cmd/entire/cli/lifecycle.go at the PR's current head (52c9b699). I checked the full text of the high-severity finding after posting and it's intact. The script's first attempt broke after one post, so I posted the other five separately.

The trail's findings gate showed "passed" before this, and an open high finding will probably flip it to blocking. I haven't run entire trail request-changes, so the trail has no formal review verdict yet. Should I do that with a short summary pointing to the high finding?

The fixes close all six of my findings, and the PR's new tests pass locally. But the new hook causes a regression I reproduced: in one case the commit gets an Entire-Checkpoint trailer and no checkpoint is written.

Re-review of trail #1472 (head 98129f89f)

Two new commits:

• 140442f67: PostCommit now runs a before-condense hook (strategy.SetBeforeCondense) on the exact set of sessions it is about to condense.
• 98129f89f: an integration test for a guest-linked commit.

My earlier findings: all resolved

Locally, the new unit tests, TestPostCommit_BeforeCondenseSeesTheCondensedSessions, and all TestCodexSubagent* integration tests pass.

New issue: an IDLE parent can stop condensing (medium, reproduced)

For an IDLE parent, the live child record is what makes the session trusted to condense:

1. prepare-commit-msg stamps the trailer because of idleWithLiveTaskRecord.
2. In post-commit, the hook completes that record before the loop runs.
3. The loop reloads the session under its lock. idleWithLiveTaskRecord is now false, so shouldCondenseWithOverlapCheck falls back to the content-overlap check.
4. For a new file the child created, overlap needs the file on the parent's shadow branch. A shadow branch made by an earlier parent turn doesn't have it, so the check fails and the session isn't condensed.

Result: the commit has a trailer but no checkpoint. The completed record is then written into the next commit's checkpoint, or waits for session end.

I probed this with the PR's own Codex scenario: parent set to IDLE, plus a shadow branch at BaseCommit that doesn't contain docs/red.md.

So in this case the hook makes things worse than before: before, the commit got a checkpoint with an incomplete record; now it gets none. ACTIVE parents aren't affected, because they're trusted on recent interaction. The probe test is saved at scratchpad/zz_idle_probe_test.go.

Possible fixes:

• Take the idleWithLiveTaskRecord snapshot before running the hook, and let the handler keep trusting those sessions.
• Or have the hook return the IDs of records it completed, and treat those like live records in shouldCondenseWithOverlapCheck.

Either way, the probe should become a regression test.

The bot's two open findings

• Medium (manual_commit_hooks.go:1167, failed reload keeps the old snapshot): this is real, but less serious than it sounds. Each loop pass reloads its session under the lock, so condensation always sees fresh state. The old snapshot only feeds collectCommittedFileClaims and the fully-condensed skip.
  • The better fix is to reload the same session IDs (LoadSessionState). Calling findSessionsForCommitLinking again repeats the git-common-dir and ancestry lookups and could, in principle, return a different set than the one the hook just refreshed.
• Low (hooks_git_cmd.go:199, hook never cleared): effectively not a problem. The CLI is one process per hook, and the only test that sets the hook isn't parallel and cleans up after itself. A field on ManualCommitStrategy would still be cleaner than a mutable package-level variable, so it's optional.

Smaller point

The hook runs before isRebase is computed, so during a rebase it refreshes Codex ledgers for every replayed commit. During a rebase, post-commit skips transitions and condenses nothing, so that scan is wasted. Skip the hook when isGitSequenceOperation(ctx) is true.

Cleanup

01M41TJSRZ81 (silent return when GetByAgentType fails) is still marked open but stale. That code is gone and the replacement logs, so it can be resolved.

Verdict: nearly there. The IDLE regression is the only thing I'd block on. Should I post it as a finding with the probe attached, along with the rebase note and the reload-by-ID suggestion?

Reproduced with the PR binary. Now running the same script with a main binary for comparison.