Resolve Trail Findings and Commit Fix

There are two new findings on Trail #2635, one on cli/1706 and one on cli/1717. cli/1714, cli/1727 and cli/1729 have none.

1. cli/1706: Medium, open (cmd/entire/cli/attach.go:689). It's a real problem and nothing later in the stack fixes it.

• Problem: commit 7d57d1c4fc made post-commit create the checkpoint when the stamped Entire-Checkpoint trailer ends up outside the final block. That happens when you add something like Refs: JIRA-123 as its own paragraph after the trailer. But planAttachLink and checkpointForCommit (used by explain --generate) still look only at the final block, through strategy.LinkedCheckpoints, so they treat the commit as unlinked.
• Effect: the reviewer reproduced it on cli/1729, the top of the stack.
  • explain --generate fails with "names checkpoint … only outside its final trailer block".
  • session attach rewrites the commit and creates a second checkpoint. The message ends up with two Entire-Checkpoint trailers: the stamped ID first, then the new one after Refs: JIRA-123.
  • On main, both commands work.
• Proposed fix (the reviewer's suggestion): when post-commit uses a stamped ID found outside the final block, also record the commit in that checkpoint. Attach already does this when it links a pushed commit without a trailer. Then attach and --generate would check that record before treating the commit as unlinked or rewriting it.
• Scope: the change goes in cli/1706, then restack 1714, 1717, 1727 and 1729 on top. I haven't designed the exact change yet.

2. cli/1717: Low, open (cmd/entire/cli/strategy/manual_commit_hooks.go:607). It's already fixed higher up the stack.

• Problem: on 1717's branch, the editor-flow default: case writes the "Remove the Entire-Checkpoint trailer above…" comment lines even for commits with no editor, such as commit -C, merge --no-edit and --no-edit amends. With default cleanup those lines stay in the commit message.
• Already fixed: 8b9c05a845 on cli/1727 adds the rules.edited && rules.stripped check, so the stack head (line 607) no longer has this bug. Only 1717's branch still does.
• Two options:
  • Resolve it with a note pointing to cli/1727 (-m "Fixed in cli/1727 (8b9c05a845): the comment is written only when rules.edited && rules.stripped.").
  • Move the check down into 1717 so that branch is correct on its own. That's worth it if 1717 might merge before 1727.

Do you want me to plan the code change for #1? And for #2, should I resolve it with that note (a Trail write), or move the check into 1717?

I've worked out the concrete change for #1. Here's the brief before I edit anything.

Problem. A stamped trailer can get pushed out of the final block, for example when you add Refs: JIRA-123 as its own paragraph after it. Post-commit still creates that checkpoint (7d57d1c4fc). But two commands only look at the final block through LinkedCheckpoints, so they treat the commit as unlinked:

• entire session attach: in planAttachLink (attach.go:689), it rewrites the commit and creates a second checkpoint.
• explain --commit … --generate: in checkpointForCommit (explain.go), it fails.

Root cause. The stamp record that proves the ID belongs to this commit is used up in post-commit, so nothing records the link afterwards. Notably, explain <sha> without --commit already falls back to recorded links (explain.go:642). Only the --commit path and attach skip that fallback.

Proposed change (all on cli/1706):

1. Post-commit records the link. When the condensation target is a stamped ID from outside the final block, add the commit to that checkpoint's LinkedCommits. This goes through a new condenseOpts field and buildCondensationWriteOptions. It's the same record attach writes for a pushed commit, and unionLinkedCommits keeps existing links.
2. Attach joins the recorded checkpoint. In planAttachLink, if there's no final-block link, look for a recorded link (checkpointLinkedTo) before the pushed/rewrite decision. If one exists, join it with attachJoinExisting instead of rewriting.
3. explain --commit uses the same fallback as the auto path, through one shared helper. That way --generate and the read-only modes resolve the same way.
4. One extra check: in steps 2 and 3, only trust a recorded link when the commit message itself names that checkpoint ID somewhere. This stops a checkpoint pushed by someone else from claiming an unpushed local commit. Attach's existing pushed-commit path doesn't change.

Tests. First I'd add an integration test of the reviewer's repro: stamp, add Refs: JIRA-123, commit, then attach and run --generate. I'd confirm it fails on 1706 before writing the code.

Tradeoff. Commits whose stamped trailer was moved out of the final block now carry a LinkedCommits entry, so explain <checkpoint> lists them as recorded links. The explain note currently says "recorded by entire session attach", which would be wrong for these, so I'd reword it.

Afterwards. Restacking 1714, 1717, 1727 and 1729, plus any push or force-push, needs your separate approval.

OK to go ahead with steps 1–4 on cli/1706?

Understood. Here's a tighter scope, and a proposal for keeping that note.

#1 scope, trimmed to what the finding names:

1. Post-commit records the commit in LinkedCommits when the stamped ID came from outside the final block.
2. planAttachLink joins a recorded link instead of rewriting the commit.
3. explain --commit --generate uses the same recorded-link fallback that explain <sha> already has.

I've dropped the extra "message must name the ID" check, since the finding doesn't ask for it. The explain note rewording is dropped unless the test shows it's actually wrong. The plan is still an integration test of the reviewer's repro first, then the code, all on cli/1706. Nothing goes out without your approval.

Keeping the note. The resolve note on 01M63DST6BBP already records it on the finding. If you also want it visible on the Trail itself, I'd add this section to the Trail description with /update-trail-context:

And to make it a habit for future reviews, this line for Agent Memory.md:

Should I start on #1, and do you want the Trail section, the memory line, or both?

I added the line to Agent Memory.md, and yes, that's the right place for it. It's about how you like to work across all repos, not a fact about this one, so it doesn't belong in the repo's Entire Brain. I left the Trail description alone.

Entire Brain: it's available here, but I hadn't used it until just now. I went straight to entire trail and the source, which skipped the "Brain before guessing" step in your instructions. I've now run entire brain brief for #1:

• It reports freshness: unsafe, so the index may be behind the current source.
• It points at the right area (manual_commit_provenance.go, stampedTrailersOf, finalTrailerBlock).
• It surfaces two older findings from this branch's history about stamped vs. inherited trailer order in pickCondensationTarget.

Nothing in it changes the trimmed plan for #1. I'll check the Brain first on the next steps.

Should I start #1 on cli/1706 now, with the integration test first and then steps 1–3?

Here's where things stand.

There's no new medium finding. The only medium on the Trail is still 01M63J4V2RH0 on cli/1706, which is the one I'm fixing. It will stay open until the fix is pushed and resolved. The other four Changes have no open findings, and the cli/1717 one is resolved.

I can't see the risk score from here. Neither entire trail show nor finding list returns a risk or score field; it only appears on the entire.com page. I don't want to guess what's driving it. Can you paste the risk breakdown or reasons from the Change page? Then I can tell you which parts we can actually lower. What I can say now:

• The open medium is the one thing we know counts against it, and the fix below addresses it.
• The 1706 fix makes that Change slightly bigger, but every addition has a test.

#1 on cli/1706: done locally. Not committed or pushed.

• What changed:
  • Post-commit now records the commit in its checkpoint's LinkedCommits when the stamped trailer sits outside the final block (manual_commit_provenance.go, manual_commit_hooks.go, manual_commit_condensation.go).
  • Attach joins that checkpoint instead of rewriting an unpushed commit (attach.go).
  • explain --commit now uses the same recorded-link fallback as explain <sha>, through one shared helper (explain.go).
• Behaviour change: read-only explain --commit on a commit that only has a recorded link now shows its checkpoint, where before it said there was none. explain <sha> already did this.
• Test: I added TestTrailerAuthority_StampedTrailerBelowAParagraphStaysLinked. Before the fix it failed exactly as the reviewer described: attach rewrote the commit and created a second checkpoint. With the fix:
  • attach joins the stamped checkpoint and leaves HEAD unchanged;
  • explain --commit --generate gets past checkpoint resolution and stops at a fake summary provider, so no real provider is called.
• Checks: lint is clean. The full unit and integration suites had one failure each, in TestResolveClaudeConfigDir_PrefersClaudesAnswer and TestExternalCommand_SigintReachesPlugin. Neither touches this code, and both pass when run alone, so they look load-related.
• Not fixed: the explain note still says the link was "recorded by entire session attach", which isn't accurate for a link post-commit records. I left it out to keep scope tight; it's a one-line follow-up if you want it.

Next steps, each needing your go-ahead:

1. Commit on cli/1706 with this message:
2. Restack 1714, 1717, 1727 and 1729 onto it. That means force-pushing all four.
3. Push, then resolve 01M63J4V2RH0 with a short note naming the commit.