fix(strategy): create checkpoints for commits during an active session - #1728
Closed
entire[bot] wants to merge 4 commits into
Closed
fix(strategy): create checkpoints for commits during an active session#1728entire[bot] wants to merge 4 commits into
entire[bot] wants to merge 4 commits into
Conversation
Commits made during an active agent session produced 0 checkpoints when the
commit reached post-commit without an Entire-Checkpoint trailer.
Root cause: PostCommit gated all session processing on the presence of the
Entire-Checkpoint trailer that PrepareCommitMsg adds. When PrepareCommitMsg
does not run -- a known failure mode when a global core.hooksPath is set or
`entire` is off the git hook's PATH, so only post-commit fires -- the commit
carries no trailer, PostCommit returned early (updating BaseCommit only), and
the ACTIVE session's work for that commit was lost. The state machine mandates
ACTIVE + GitCommit -> Condense, but that transition was never reached. The Stop
hook cannot recover it either: the files are already committed, so nothing is
"modified" ("no files modified during session, skipping checkpoint"). Result:
entire/checkpoints/v1 keeps only its metadata-init commit.
Fix: PostCommit now recovers a trailerless commit instead of dropping it. When
the commit has no trailer, was made non-interactively (agent, no TTY), and a
recent ACTIVE session's tracked work content-overlaps the committed files, it
generates a checkpoint ID and condenses via the normal EventGitCommit ->
Condense path; the recovered checkpoint is finalized with the full transcript
at Stop, like any mid-turn checkpoint. Guardrails preserve existing behavior:
- No-TTY only: a human at a terminal with no trailer may have declined linking;
that intent is honored, so interactive commits are never recovered.
- Content-overlap required: the ACTIVE-session overlap-skip in
shouldCondenseWithOverlapCheck is disabled on the recovery path, so an
unrelated agent/CI commit made while a session is active is not falsely linked.
Contract: docs Scenario 2 says a mid-session commit SHOULD checkpoint. The
trailer that links commit<->checkpoint is added by PrepareCommitMsg and cannot
be added to an already-made commit, so recovery preserves the work on
entire/checkpoints/v1 (no data loss) even though a commit-anchored
`entire explain <commit>` won't associate a trailerless commit. Documented as
caveat #10 in checkpoint-scenarios.md.
Tests: integration coverage for recovery, the unrelated-file and post-stop
(IDLE) non-recovery gates, and multiple trailerless commits each recovering.
Fixes #487
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a data-loss gap in the manual-commit strategy where commits made during an active agent session could produce 0 checkpoints if the commit reached post-commit without an Entire-Checkpoint trailer (e.g., prepare-commit-msg didn’t run due to core.hooksPath or PATH issues). The strategy now attempts a narrow recovery path for trailerless, non-interactive commits by condensing session work when it content-overlaps the committed files, and documents the resulting caveat around commit-anchored entire explain <commit>.
Changes:
- Add trailerless mid-session commit recovery in
PostCommit, including an overlap-check guardrail for ACTIVE sessions when recovering. - Add integration test helpers and new mid-session integration tests covering recovery, non-recovery cases, and multiple trailerless commits.
- Document the recovery behavior and limitations in checkpoint scenario docs.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| docs/architecture/checkpoint-scenarios.md | Documents trailerless mid-session commit recovery behavior and limitations. |
| cmd/entire/cli/strategy/manual_commit_hooks.go | Implements recovery for trailerless mid-session commits and tightens overlap gating during recovery. |
| cmd/entire/cli/integration_test/testenv.go | Adds helper to create trailerless agent commits and a utility to count checkpoint commits. |
| cmd/entire/cli/integration_test/mid_session_commit_test.go | Adds regression/behavior tests for trailerless commit recovery and guardrails. |
… overlap docs Audit hardening for the trailerless mid-session commit recovery (issue #487). No production-code change: the recovery in PostCommit was verified correct (reaches ACTIVE + GitCommit -> Condense, finalizes at Stop, is idempotent and amend-safe, and is mutually exclusive with the trailer path so it cannot double-checkpoint). These changes strengthen the test coverage and docs for the wrong-attribution risk area. - Add TestShadowStrategy_MidSessionCommit_ConcurrentDisjoint_OnlyOwningSessionRecovered: two concurrent ACTIVE sessions with disjoint tracked files; a trailerless commit of one session's file must recover ONLY that session and never cross-attribute the other. The recovery path's content-overlap gate makes this strictly more precise than the normal trailer path, which condenses every recent ACTIVE session regardless of overlap. Mutation-verified: dropping the noTrailerRecovery overlap gate makes the test fail with both sessions attributed to the checkpoint. - Clarify checkpoint-scenarios.md caveat #10: recovery overlap is the intersection of committed paths with the session's tracked files, verified content-exact for newly-added files (blob-hash match against the shadow branch) and by path for modified files -- the same heuristic PostCommit uses elsewhere. Documents that a commit touching only untracked files is left alone, a modified pre-existing file is linked even if bytes differ, and two concurrent sessions editing the same committed file are both linked (as in the trailer path).
…s mode (#487) The #487 recovery gated on no-TTY as a proxy for "agent commit", treating any commit made at a TTY as a possibly-declined link and never recovering it. But the #487 root cause -- a global core.hooksPath, or entire off the git hook PATH -- stops prepare-commit-msg from running for BOTH agent and human-typed commits, and no link prompt ever appears. A developer typing `git commit` at a terminal while an agent's ACTIVE session had overlapping uncommitted work therefore still lost that work: TTY presence was wrongly read as a decline. Fix: suppress recovery only for a genuine decline -- a TTY commit made while Entire's prepare-commit-msg hook was actually wired. PrepareCommitMsgHookWired resolves the hooks dir git uses (respecting core.hooksPath) and checks for Entire's prepare-commit-msg. When it is wired, an absent trailer means the human answered "no" to the prompt or deleted the offered trailer in their editor, which is honored. When it is NOT wired (the #487 broken-hooks mode), no linking was ever offered, so a missing trailer is not a decline and the ACTIVE session's overlapping work is recovered for human-typed and agent commits alike. Agent commits have no TTY and stay eligible regardless. Hook-wiring is a stronger signal than a "declined prompt" marker: it also honors the editor-flow trailer deletion (which never calls askConfirmTTY), and it is stateless -- no cross-hook marker to persist, go stale, or scope per worktree. Tests: a human-TTY commit in broken-hooks mode now recovers; a TTY commit with the prepare-commit-msg hook wired (a real decline) is still not recovered; unit test for PrepareCommitMsgHookWired. Both integration tests are mutation-verified. Contract documented in checkpoint-scenarios.md caveat #10.
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.
Trail: https://entire.io/gh/entireio/cli/trails/835
Summary
Commits made during an active agent session produced 0 checkpoints whenever the commit reached
post-commitwithout anEntire-Checkpointtrailer. This fixes that data loss.Root cause
PostCommitgated all session processing on theEntire-Checkpointtrailer thatPrepareCommitMsgadds. WhenPrepareCommitMsgdoes not run — a known failure mode when a globalcore.hooksPathis configured orentireis off the git hook's PATH, so onlypost-commitfires — the commit carries no trailer,PostCommitreturned early (updatingBaseCommitonly), and the ACTIVE session's work for that commit was lost.The documented state machine mandates
ACTIVE + GitCommit -> Condense, but that transition was never reached. The Stop hook cannot recover it either: the files are already committed, so nothing is "modified" (no files modified during session, skipping checkpoint). The result isentire/checkpoints/v1keeping only its metadata-init commit — exactly the reported symptom.Fix
PostCommitnow recovers a trailerless commit instead of dropping it. With no trailer, if the commit was made non-interactively (agent, no TTY) and a recent ACTIVE session's tracked work content-overlaps the committed files, it generates a checkpoint ID and condenses via the normalEventGitCommit -> Condensepath. The recovered checkpoint is finalized with the full transcript at Stop, like any mid-turn checkpoint.Guardrails preserve existing behavior:
shouldCondenseWithOverlapCheckis disabled on the recovery path, so an unrelated agent/CI commit made while a session is active is not falsely linked.The trailer that links commit↔checkpoint cannot be added to an already-made commit, so recovery preserves the work on
entire/checkpoints/v1(no data loss); a commit-anchoredentire explain <commit>still won't associate a trailerless commit. Documented as caveat #10 indocs/architecture/checkpoint-scenarios.md.Testing
Live reproduction with real git hooks (built binary on PATH) and real
git commits, neutralizingprepare-commit-msgso mid-session commits carry no trailer.Before (current behavior):
After (this change):
Live edge-case matrix (all pass):
prepare-commit-msg, trailered) — unchanged, no regressionGates:
mise run lint— 0 issuesvogonagentAudit — attribution hardening
An adversarial audit of the recovery path (focused on wrong-session attribution
and over-recovery) found the fix correct; no production-code change was needed. A
follow-up commit adds a mutation-verified regression test and tightens the overlap
docs.
Wrong-attribution verdict. Recovery never links more sessions than the normal
trailer path, and is strictly more precise for disjoint concurrent sessions:
Recovery disables the ACTIVE-session overlap-skip, so it requires the committed
paths to intersect the session's tracked files. Overlap is content-exact for
newly-added files (blob-hash match against the shadow branch) and path-based for
modified files — the same heuristic
PostCommituses elsewhere. A single-sessionfalse positive (an unrelated actor modifying a pre-existing file the session also
touched) preserves the session's work with no data loss and is baseline-equivalent
to the trailer path.
Also verified: trailer-present and recovery are mutually exclusive (
foundvs!found), so there is no double-checkpoint; repeatedpost-commiton the sametrailerless commit is idempotent (carry-forward clears the tracked files); amend of
a recovered commit does not runaway-duplicate; generating a recovery checkpoint ID
when no session ends up condensing writes no orphan (
updateCombinedAttributionno-ops on a missing summary); and the no-TTY gate is the correct proxy — the narrow
agent-env signal (
isAgentSubprocessEnv) misses Claude Code/Codex/Cursor, so no-TTYis what actually targets the reported scenario.
New test (mutation-verified):
TestShadowStrategy_MidSessionCommit_ConcurrentDisjoint_OnlyOwningSessionRecovered— asserts a trailerless commit of one session's file recovers only that session and
does not cross-attribute a concurrent disjoint session. Verified by mutation:
dropping the
noTrailerRecoveryoverlap gate makes it fail with both sessionsattributed to the checkpoint. The three existing #487 non-recovery/recovery tests
were also mutation-checked (neutralizing recovery, loosening the overlap gate, and
removing the ACTIVE-session requirement each fail exactly the intended test).
Gates: gofmt clean;
golangci-lintreports 0 issues on the strategy package andno new issues from the added test; strategy unit + mid-session integration tests
green. The full
mise run check/ vogon E2E canary depends on theroger-roger+tmuxexternal binaries (pre-existing, environment-gated); thevogon deterministic mid-turn-commit and trailer-removal scenarios run green.
Follow-up — human-typed commit in broken-hooks mode
Re-analysis surfaced a false-negative the original guardrail missed. The recovery
gated on no-TTY as a proxy for "agent commit", so any commit made at a TTY was
treated as a possibly-declined link and never recovered. But the #487 root cause
(a global
core.hooksPath, or Entire off the hook PATH) stopsprepare-commit-msgfrom running for both agent and human-typed commits, andno link prompt ever appears — so a developer typing
git commitat a terminalwhile an agent's ACTIVE session had overlapping uncommitted work still silently
lost it.
Contract decision. Recovery is suppressed only for a genuine decline: a TTY
commit made while Entire's
prepare-commit-msghook was actually wired(
PrepareCommitMsgHookWiredresolves the hooks dir git uses, respectingcore.hooksPath, and checks for Entire's hook). Wired + TTY + no trailer ⇒ thehuman answered "no" to the prompt or deleted the offered trailer in their editor
⇒ honored. Not wired (broken-hooks mode) ⇒ no linking was ever offered ⇒ a
missing trailer is not a decline ⇒ the overlapping active-session work is
recovered for human-typed and agent commits alike. Agent commits have no TTY and
stay eligible regardless.
Hook-wiring detection was chosen over an explicit "declined prompt" marker
because it also honors the editor-flow trailer deletion (which never calls
askConfirmTTY) and is stateless — no cross-hook marker to persist, go stale, orscope per worktree. Mutation-verified: reverting to the no-TTY-only gate breaks
the broken-hooks-human recovery test; making suppression never fire breaks the
wired-hook decline test. Documented in
checkpoint-scenarios.mdcaveat #10.Fixes #487