Skip to content

Sign checkpoints at push time#1387

Open
pjbgf wants to merge 13 commits into
mainfrom
sign-at-push
Open

Sign checkpoints at push time#1387
pjbgf wants to merge 13 commits into
mainfrom
sign-at-push

Conversation

@pjbgf

@pjbgf pjbgf commented Jun 8, 2026

Copy link
Copy Markdown
Member

https://entire.io/gh/entireio/cli/trails/537

Summary

Defer GPG/SSH signing of checkpoint commits from creation time to push time. During an agent session, checkpoint commits accumulate
unsigned on the local entire/checkpoints/v1 ref — no GPG agent calls per step. At pre-push, every local-only commit is cherry-picked
onto the remote tip with strict signing, with a visible per-commit progress line and an interactive retry/skip/abort prompt on failure.

Why

  • Performance: active sessions create many checkpoint commits; the per-step SignCommitBestEffort round-trip to the GPG/SSH agent
    dominated commit latency.
  • Visibility: signing was silent — users couldn't tell when the agent was being invoked or which commit it was signing.
  • Correctness: local-only checkpoints don't need signatures; only what leaves the machine.

What changes

  • CreateCommit and the orphan metadata-branch init no longer sign.
  • createCherryPickCommit is split into buildCherryPickCommit + persistCherryPickCommit so a new push-time loop can sign each
    cherry-picked clone between build and persist.
  • New strict SignCommit(ctx, commit) error alongside the existing SignCommitBestEffort (now a logging wrapper). ErrSigningDisabled
    sentinel.
  • PrePush fetches the remote tip into the appropriate ref (temp ref for URL targets), runs the sign-loop over local-only commits,
    advances the local ref to the new signed tip, then pushes. On any abort the local ref stays untouched.
  • Cross-clone safety: empty-tree orphans are filtered out, and tree construction uses diff-based application (mirroring cherryPickOnto)
    so per-clone files accumulate onto the remote tip rather than overwriting it.

UX

[entire] Fetching latest from remote...
[entire] Signing commits:
10/10: Finalize transcript for Checkpoint: 17d941007e3b
[entire] Pushing entire/checkpoints/v1 to checkpoint remote... done

  • TTY: a static header and a single status line that rewrites in place — no scrollback noise.
  • Non-TTY (CI): one line per commit, preserved for the audit trail.
  • Signing failure: Retry, skip, or abort? [r/s/a]:. Non-TTY defaults to skip (with a logging.Warn).
  • Setting: sign_checkpoint_commits (default true) gates the whole loop.

Note

High Risk
Changes when and how metadata refs are rewritten before push (cherry-pick, ref advance, cross-clone filtering); signing failures can block push or leave unsigned commits on the chain.

Overview
Checkpoint commit signing moves from per-step creation to pre-push, so local entire/checkpoints/v1 history stays unsigned during sessions and only commits that will leave the machine are signed.

CreateCommit and orphan metadata ref init no longer call the signer. Signing is centralized in a new strict SignCommit (returns ErrSigningDisabled and propagates signer errors); SignCommitBestEffort is a thin logging wrapper. Cherry-pick helpers split into buildCherryPickCommit / persistCherryPickCommit so push can sign between build and store.

PrePush runs signLocalCommitsForPush before each checkpoint ref push: fetch remote tip, collect local-only commits (collectCommitsSince now supports exclude == ZeroHash), optionally drop empty-tree orphan dupes on cross-clone pushes, then signAndPersistCommits (diff-based cherry-pick + per-commit signing, TTY progress, retry/skip/abort on failure). Abort leaves the local ref unchanged; signing off via sign_checkpoint_commits skips the loop. Metadata reconcile still uses best-effort signing on its cherry-pick path.

Tests cover unsigned CreateCommit, strict SignCommit, and end-to-end pre-push signing behavior.

Reviewed by Cursor Bugbot for commit b2d3e84. Configure here.

Copilot AI review requested due to automatic review settings June 8, 2026 13:26

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ 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 b2d3e84. Configure here.

Comment thread cmd/entire/cli/strategy/push_signing.go

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR moves checkpoint commit signing from checkpoint creation time to pre-push time, so agent sessions can create many local commits without incurring per-step GPG/SSH signer latency. At push time, local-only commits are replayed onto the remote tip and (attempted to be) signed with user-visible progress and interactive failure handling.

Changes:

  • Introduces a new push-time signing/replay flow (signLocalCommitsForPush + signAndPersistCommits) with progress output and retry/skip/abort handling.
  • Splits cherry-pick commit creation into “build” vs “persist” steps to allow signing between them.
  • Adds strict checkpoint.SignCommit (with ErrSigningDisabled) and updates SignCommitBestEffort to wrap it; removes eager signing from commit creation/orphan init, and adds targeted tests.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
cmd/entire/cli/testutil/testutil.go Adds bare-repo + remote setup helpers for push-related tests.
cmd/entire/cli/strategy/push_signing.go New push-time signing/replay implementation (fetch remote tip, cherry-pick via diffs, per-commit signing, progress + prompts).
cmd/entire/cli/strategy/push_signing_test.go New unit/E2E-style tests covering push-time signing behavior and UI formatting.
cmd/entire/cli/strategy/push_common.go Extends commit collection logic to support “exclude == ZeroHash” ranges.
cmd/entire/cli/strategy/metadata_reconcile.go Splits cherry-pick commit construction vs persistence; keeps best-effort signing for reconcile path.
cmd/entire/cli/strategy/manual_commit_push.go Hooks push-time signing into PrePush before pushing each checkpoint/metadata ref.
cmd/entire/cli/strategy/common.go Removes eager signing from orphan metadata ref initialization.
cmd/entire/cli/checkpoint/objectsigner_strict_test.go Adds tests for strict SignCommit semantics (disabled, propagate errors, success).
cmd/entire/cli/checkpoint/objectsigner_errors.go Adds ErrSigningDisabled + ShouldSkipPushSigning helper.
cmd/entire/cli/checkpoint/committed.go Removes eager signing from CreateCommit; adds strict SignCommit and reimplements SignCommitBestEffort as a wrapper.
cmd/entire/cli/checkpoint/committed_no_eager_sign_test.go Ensures CreateCommit never calls the signer even when signing is enabled/configured.

Comment thread cmd/entire/cli/strategy/push_signing.go
Comment thread cmd/entire/cli/strategy/push_signing.go
Comment thread cmd/entire/cli/strategy/push_signing.go
pjbgf added 12 commits June 9, 2026 21:20
Introduces SignCommit (returns error) and ErrSigningDisabled sentinel.
The existing SignCommitBestEffort becomes a logging wrapper around
SignCommit, preserving its semantics for the metadata-reconcile
divergence path.

Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Paulo Gomes <paulo@entire.io>
Entire-Checkpoint: 3ea196124c6a
CreateCommit and the orphan metadata-branch initializer no longer invoke
the signer. Signing is deferred to pre-push (added in a follow-up commit).

Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Paulo Gomes <paulo@entire.io>
Entire-Checkpoint: 17bb9c4d640c
Lets a future push-time loop sign the new commit between build and
persist. The existing divergence path keeps best-effort signing via the
unchanged createCherryPickCommit wrapper.

Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Paulo Gomes <paulo@entire.io>
Entire-Checkpoint: 2961d515a4c0
signAndPersistCommits cherry-picks each commit onto a moving tip, signing
each new commit before persisting it. On signing failure it consults the
user (r/s/a). Non-TTY paths default to skip. Stderr gets one
'Signing commit i/N: <subject>' line per commit.

Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Paulo Gomes <paulo@entire.io>
Entire-Checkpoint: c9b4f22b2f9d
PrePush now fetches the remote tracking ref, finds the local-only
commits above it, signs and cherry-picks them onto the remote tip, and
advances the local ref to the signed tip before pushing. Local ref
stays untouched on abort.

Includes correctness fixes in collectCommitsSince (ZeroHash exclude
guard) and buildCherryPickCommit (root-commit handling) that the new
first-push flow exposed.

Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Paulo Gomes <paulo@entire.io>
Entire-Checkpoint: a97bfecae458
signLocalCommitsForPush fell back to repo.Head() when no remote-tracking
ref existed, which on first push cherry-picked the orphan-rooted v1
chain onto HEAD instead of preserving its orphan root. The existing
test masked the bug by setting up the chain rooted at HEAD; both the
test setup and the fallback are now corrected to match production
(orphan-rooted chain) with an explicit orphan-root assertion.

Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Paulo Gomes <paulo@entire.io>
Entire-Checkpoint: 80f1dc39b648
…sh signing

Two related fixes for cross-clone push scenarios:

1. Skip empty-tree (orphan-init) commits when remote already has its own
   anchor. Each clone has its own orphan-init; including it in the push
   would produce duplicate empty-tree commits on the remote chain.
2. Use diff-based tree construction (treeChangesForCherryPick +
   ApplyTreeChanges) instead of original.TreeHash, mirroring
   cherryPickOnto. This ensures per-clone checkpoint files accumulate
   onto the remote tip rather than overwriting it.

Together these make TestHTTPS_OutOfSyncCheckpointBranchRebases pass.

Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Paulo Gomes <paulo@entire.io>
Entire-Checkpoint: 33401e3c6510
fetchRefBestEffort previously skipped the pre-sign fetch when the push
target was a URL (e.g. checkpoint_remote), which left
lookupRemoteTipForSigning returning ZeroHash. The signing loop would
then sign every local commit and the push would fail as
non-fast-forward; the reconcile path would re-sign each cherry-picked
commit via createCherryPickCommit. Mirror the temp-ref approach
fetchAndRebaseRefCommon already uses for URL targets so the pre-sign
path can locate the remote tip and sign each commit exactly once.

Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Paulo Gomes <paulo@entire.io>
Entire-Checkpoint: 93cafd27b3e9
For a 10-commit push, accumulating one line per signed commit dominated
the terminal output and made the actual push progress easy to miss. On
a TTY the progress now renders in place: the three most-recent
"Signing commit i/N: ..." lines stay visible, the oldest scrolls off as
new ones arrive. Non-TTY output is unchanged so CI logs still record
every line.

Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Paulo Gomes <paulo@entire.io>
Entire-Checkpoint: 739560adbc6b
…e line

Replace the 3-line rolling buffer with a fixed "[entire] Signing commits:"
header printed once, plus a single indented "        i/N: <subject>"
status line that rewrites in place per commit on a TTY. Also announce
the pre-sign fetch with "[entire] Fetching latest from remote..." so
the user sees the network step before signing begins.

Non-TTY callers still get one line per commit so CI logs preserve the
full record.

Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Paulo Gomes <paulo@entire.io>
Entire-Checkpoint: 7a2b5fc2ddd8
CanPromptInteractively only guarantees a controlling /dev/tty exists,
not that the caller's stderr writer is a terminal. When pre-push runs
as a git hook with stderr captured or piped, the existing gate
returned true, the prompt question went to a writer no human reads,
and the hook then blocked on os.Stdin for an answer that never came.
Add a second gate on IsTerminalWriter(stderr) so the prompt only fires
when both the question is visible AND the answer can come in.

Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Paulo Gomes <paulo@entire.io>
Entire-Checkpoint: e405b889c45e
The len(changes) == 0 branch previously fell through to
treeHash = original.TreeHash for every no-op commit, including non-root
ones whose tree just happens to equal the local parent. In cross-clone
pushes, reusing original.TreeHash overwrites the accumulated tree on
the remote tip — files contributed by the other clone get silently
dropped. Only true root commits (no parents or at the shallow
boundary) keep that special-case path; non-root no-op commits are
skipped, matching the same skip in cherryPickOnto.

Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Paulo Gomes <paulo@entire.io>
Entire-Checkpoint: bb8892d783b1
@pjbgf pjbgf marked this pull request as ready for review June 9, 2026 12:40
@pjbgf pjbgf requested a review from a team as a code owner June 9, 2026 12:40
When the empty-tree filter dropped every local-only commit (clone has
only its own orphan-init against a populated remote), the pre-sign
path returned without touching the local ref. The subsequent push
then sent the orphan chain to a populated remote, which rejected it
as non-fast-forward; the disconnected-reconcile path then had to
recover after a failed push.

Mirror what the reconcile path already does in that case: advance the
local ref to the remote tip so the push that follows is a clean
fast-forward no-op.

Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Paulo Gomes <paulo@entire.io>
Entire-Checkpoint: 42188258f035
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants