Enforce repo checkpoint policy#1509
Draft
pfleidi wants to merge 1 commit into
Draft
Conversation
Read the repo checkpoint policy before committed checkpoint writes and skip unsupported hook writes without failing ordinary git operations. Refresh policy during pre-push, warn user-driven commands when the local policy requires a newer CLI, and document the offline/online behavior. Entire-Checkpoint: 8712032f8b90
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 83bbbfc. Configure here.
| condenseStart := time.Now() | ||
| if !committedCheckpointWriteAllowed(ctx, repo) { | ||
| return newSkippedResult(checkpointID, state.SessionID), nil | ||
| } |
There was a problem hiding this comment.
Doctor condense hides policy block
High Severity
When checkpoint policy blocks writes, CondenseSession returns the same skipped result as an empty session. CondenseSessionByID (used by entire doctor) treats that as success, sets FullyCondensed, and may clean up the shadow branch without persisting checkpoint data or telling the user to upgrade.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 83bbbfc. Configure here.
Contributor
There was a problem hiding this comment.
Pull request overview
Wires the repository’s stored checkpoint policy (refs/entire/policies/checkpoint) into runtime behavior, gating committed checkpoint writes and checkpoint pushes so older CLIs don’t emit incompatible metadata while keeping hook-triggered git workflows non-fatal.
Changes:
- Enforce local checkpoint policy on user-driven committed checkpoint writes (fail with an upgrade instruction when policy requires an unsupported writer).
- Make hook-triggered checkpoint writes and pre-push checkpoint pushes skip (and warn/log) when policy is unsupported; pre-push refreshes policy before deciding to push metadata.
- Add post-success warning printing when local policy indicates the CLI should be upgraded, plus supporting policy helper APIs and tests.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/architecture/sessions-and-checkpoints.md | Documents the checkpoint policy ref, fields, and enforcement behavior across hooks/user commands/pre-push. |
| cmd/entire/main.go | Prints checkpoint-policy upgrade warning after successful command execution (for non-excluded commands). |
| cmd/entire/cli/versioncheck/versioncheck.go | Adds UpdateCommandForCurrentBinary helper to pick either an installer command or releases URL. |
| cmd/entire/cli/versioncheck/versioncheck_test.go | Adds unit tests for UpdateCommandForCurrentBinary. |
| cmd/entire/cli/strategy/manual_commit_push.go | Gates checkpoint pushes on pre-push policy sync / divergence / unsupported write policy. |
| cmd/entire/cli/strategy/manual_commit_hooks.go | Skips turn-checkpoint finalization when committed writes are disallowed by policy. |
| cmd/entire/cli/strategy/manual_commit_condensation.go | Skips session condensation when committed writes are disallowed by policy. |
| cmd/entire/cli/strategy/checkpoint_policy.go | Introduces strategy-layer helpers for policy gating, pre-push sync, and warn/log behavior. |
| cmd/entire/cli/strategy/checkpoint_policy_test.go | Adds strategy tests asserting policy blocks checkpoint push on unsupported write and on divergence. |
| cmd/entire/cli/explain.go | Gates --generate summary writes on committed checkpoint write policy. |
| cmd/entire/cli/explain_test.go | Refactors summary test setup; adds test that summary generation is rejected under unsupported write policy. |
| cmd/entire/cli/checkpointpolicy/warning_test.go | Adds tests for RequiresUpgrade, UnsupportedWrite, and the upgrade warning message. |
| cmd/entire/cli/checkpointpolicy/policy.go | Adds RequiresUpgrade, UnsupportedWrite, and UpgradeWarning helpers. |
| cmd/entire/cli/checkpoint_policy_write.go | Adds ensureCommittedCheckpointWritePolicy for user-driven committed write gating. |
| cmd/entire/cli/checkpoint_policy_warning.go | Adds post-success warning emission and command-exclusion logic. |
| cmd/entire/cli/checkpoint_policy_warning_test.go | Adds tests for the warning emission and exclusion logic. |
| cmd/entire/cli/attach.go | Gates attach’s committed checkpoint write path on policy support. |
| cmd/entire/cli/attach_test.go | Adds test ensuring attach rejects unsupported committed checkpoint write policy. |
Comment on lines
46
to
+49
| refs := checkpoint.ResolveCommittedRefs(ctx) | ||
| if !syncCheckpointPolicyForPrePush(ctx) { | ||
| return nil | ||
| } |
Comment on lines
148
to
+152
| logCtx := logging.WithComponent(ctx, "checkpoint") | ||
| condenseStart := time.Now() | ||
| if !committedCheckpointWriteAllowed(ctx, repo) { | ||
| return newSkippedResult(checkpointID, state.SessionID), nil | ||
| } |
Comment on lines
+2788
to
+2791
| if !committedCheckpointWriteAllowed(ctx, repo) { | ||
| state.TurnCheckpointIDs = nil | ||
| return 0 | ||
| } |
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.
Why
This is the third PR in the checkpoint-policy stack. After #1507 adds per-checkpoint format compatibility and #1508 adds the hidden policy command/storage, this PR makes the recorded repo policy affect runtime behavior.
Stacked on #1508.
What changed
The CLI now reads the local checkpoint policy before committed checkpoint writes. User-driven write paths fail when the policy requires a checkpoint format this CLI cannot write, while hook-triggered writes skip checkpoint data without failing ordinary git operations.
Pre-push refreshes the policy from the checkpoint remote before deciding whether to push
entire/checkpoints/v1, and successful user-driven commands warn when the local policy indicates the CLI should be upgraded.Usage examples
A repository can record its policy with the hidden command from the previous PR:
If that policy later requires a newer checkpoint writer, commands that create committed checkpoint data fail with an upgrade instruction instead of writing data this CLI cannot encode:
Hook-triggered checkpoint writes stay non-fatal. When run from an interactive terminal they print the same upgrade warning; otherwise they log the skipped write and let the git operation continue.
Decisions made during development
Post-commit and agent lifecycle hooks only read the local policy ref so offline git workflows keep working.
Pre-push is the refresh point because it is already an online operation. It checks the remote policy ref, fetches only when needed, and evaluates the refreshed local policy before pushing checkpoint metadata.
Successful user-driven commands warn after command execution, excluding infrastructure commands such as hooks and analytics, so users see upgrade guidance without changing unrelated command behavior.
Policy commit signature verification is intentionally not part of this PR. Policy commits are written through the existing checkpoint commit signing path, but verifying signatures for trust is a separate design problem.
Technical tradeoffs
Hook paths prefer availability over strict enforcement: if policy reading or refresh fails, most hooks log or warn and continue rather than breaking commits made by editors, agents, or offline workflows.
Pre-push treats a diverged policy ref differently from a transient refresh failure. Divergence skips the checkpoint push because the local policy state cannot be reconciled safely, while refresh failures allow the user push to continue.
The warning is based on the local policy for most commands. That keeps command startup cheap and offline-friendly, but it means users may not see remote policy changes until the local ref is refreshed by
entire policy checkpointor pre-push.Reviewer notes
This PR assumes the policy command and storage from #1508. It does not reopen or update the closed predecessor PR #1496.
Note
Medium Risk
Changes checkpoint write/push gating across attach, explain summary generation, condensation hooks, and pre-push; hook paths deliberately fail open on policy read errors, which could allow writes when policy is unreadable.
Overview
This PR wires the stored checkpoint policy into runtime behavior so the CLI respects what formats a repo can read and write.
User-driven committed writes now call
ensureCommittedCheckpointWritePolicybefore persisting metadata (e.g.entire attach,entire explain --generate-summary). If local policy’scheckpoint_versionis not write-supported, the command fails with an upgrade hint fromversioncheck.UpdateCommandForCurrentBinary.Hook paths use
committedCheckpointWriteAllowed: unsupported write policy skips checkpoint condensation/finalization without breaking git. Interactive terminals get the sharedUpgradeWarning; non-interactive paths log and continue. Policy read failures on hooks still allow writes (availability-first).Pre-push runs
syncCheckpointPolicyForPrePushto refresh policy from the checkpoint remote before pushingentire/checkpoints/v1. Diverged local vs remote policy skips the checkpoint push; transient sync failures warn but do not block the user push. Unsupported write policy also skips checkpoint push.Post-success UX: after most commands (excluding
hooks, analytics, install helpers),WarnCheckpointPolicyIfNeededprints an upgrade warning whenRequiresUpgradeorUnsupportedWriteapplies.checkpointpolicygainsRequiresUpgrade,UnsupportedWrite, andUpgradeWarning; architecture docs describe hook vs pre-push vs user-command behavior.Reviewed by Cursor Bugbot for commit 83bbbfc. Configure here.