fix(attach): harden explicit-target mode for general CLI use

Review findings on the explicit-target attach, fixed for the CLI as a
public command rather than platform plumbing:

- Validate --commit-sha names a real commit in the repository (it was only
  shape-checked); silently recording an unknown SHA would produce
  provenance pointing at nothing.
- Refresh remote state for the explicit checkpoint ID before treating it
  as new (ensureExplicitCheckpointFreshness): a retried attach from a
  fresh clone would otherwise rebuild the ID as an orphan that clobbers
  the original on push. Same-session hits stay idempotent no-ops; an ID
  that exists with other content is refused, never modified. Fetch
  failures degrade to 'not present' with a warning, preserving offline
  operation.
- Keep explicit-target attaches out of trailer-linked session state: the
  SHA-bound review checkpoint is not the session's code checkpoint, so
  BaseCommit/LastCheckpointID are no longer overwritten (amend hooks and
  resume must never treat the review checkpoint as trailer-linked).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Entire-Checkpoint: 01KY4W9TBQX9Y7JSSSHMW9J5ZC

Author: dipree

cmd/entire/cli/attach.go

@@ -32,6 +32,7 @@ import (
 
 	"charm.land/huh/v2"
 	"github.com/go-git/go-git/v6"
+	"github.com/go-git/go-git/v6/plumbing"
 	"github.com/go-git/go-git/v6/plumbing/object"
 	"github.com/spf13/cobra"
 )
@@ -281,6 +282,19 @@ func runAttach(ctx context.Context, w, errW io.Writer, sessionID string, agentNa
 		return err
 	}
 
+	// The recorded commit binding must name a real commit: --commit-sha is
+	// only shape-validated at the flag layer, and silently persisting a SHA
+	// this repository has never seen would produce provenance pointing at
+	// nothing.
+	if opts.explicitTarget() {
+		if _, chkErr := repo.CommitObject(plumbing.NewHash(opts.CommitSHA)); chkErr != nil {
+			return fmt.Errorf(
+				"--commit-sha %s does not name a commit in this repository: %w",
+				opts.CommitSHA, chkErr,
+			)
+		}
+	}
+
 	// If session already has a checkpoint, just offer to link it.
 	// Explicit-target mode is exempt: it authors a separate new checkpoint
 	// under the caller-supplied ID, so the session's own checkpoint (e.g.
@@ -391,17 +405,44 @@ func runAttach(ctx context.Context, w, errW io.Writer, sessionID string, agentNa
 		}
 	}
 
-	// Explicit-target idempotency: a retried attach with the same server-minted
-	// checkpoint ID and session is a no-op success, not an error — the caller
-	// (platform provenance publication) retries the whole script.
+	// Explicit-target IDs are usually new — but "usually" is not "always": a
+	// retried attach from a fresh clone would not have the earlier attempt's
+	// data locally, and treating the ID as brand-new would rebuild it as an
+	// orphan that clobbers the original on push. Refresh remote state for the
+	// ID first, then either no-op (same session already recorded — the caller
+	// retries whole scripts), refuse (ID exists with other content — never
+	// modify an existing checkpoint), or proceed (genuinely new).
 	if opts.explicitTarget() {
-		exists, readErr := checkpointHasSessionMetadata(ctx, repo, refs, checkpointID, sessionID)
-		if readErr != nil {
-			return fmt.Errorf("failed to check checkpoint %s for session %s: %w", checkpointID.String(), sessionID, readErr)
+		freshRepo, present, freshErr := ensureExplicitCheckpointFreshness(ctx, logCtx, repo, refs, checkpointID)
+		if freshRepo != nil && freshRepo != repo {
+			oldRepo := repo
+			repo = freshRepo
+			if closeErr := oldRepo.Close(); closeErr != nil {
+				logging.Warn(logCtx, "failed to close stale repository handle after explicit checkpoint refresh",
+					slog.String("error", closeErr.Error()))
+			}
+			// The store handle wraps the stale repo; reopen against the fresh one.
+			store, err = openAttachStore(ctx, repo, refs)
+			if err != nil {
+				return err
+			}
 		}
-		if exists {
-			fmt.Fprintf(w, "Session %s already attached to checkpoint %s\n", sessionID, checkpointID.String())
-			return nil
+		if freshErr != nil {
+			return freshErr
+		}
+		if present {
+			exists, readErr := checkpointHasSessionMetadata(ctx, repo, refs, checkpointID, sessionID)
+			if readErr != nil {
+				return fmt.Errorf("failed to check checkpoint %s for session %s: %w", checkpointID.String(), sessionID, readErr)
+			}
+			if exists {
+				fmt.Fprintf(w, "Session %s already attached to checkpoint %s\n", sessionID, checkpointID.String())
+				return nil
+			}
+			return fmt.Errorf(
+				"checkpoint %s already exists without session %s; refusing to modify an existing checkpoint",
+				checkpointID.String(), sessionID,
+			)
 		}
 	}
 
@@ -626,6 +667,43 @@ func ensureCheckpointAvailable(ctx, logCtx context.Context, repo *git.Repository
 	return repo, missingCheckpointError(logCtx, checkpointID, primaryIsRefs)
 }
 
+// ensureExplicitCheckpointFreshness surfaces any remote copy of an
+// explicit-target checkpoint ID locally and reports whether the checkpoint
+// exists. Unlike ensureCheckpointAvailable, a checkpoint that is still missing
+// after the refresh is NOT an error — an explicit-target ID is normally brand
+// new; the refresh exists so a retry from a fresh clone sees the earlier
+// attempt instead of rebuilding the ID as an orphan that would clobber it on
+// push. Fetch failures degrade to "not present" with a warning, matching
+// ensureCheckpointAvailable's tolerance for offline operation.
+func ensureExplicitCheckpointFreshness(ctx, logCtx context.Context, repo *git.Repository, refs cpkg.PersistentRefs, checkpointID id.CheckpointID) (*git.Repository, bool, error) {
+	cfg, err := settings.LoadCheckpointsConfig(ctx)
+	if err != nil {
+		return repo, false, fmt.Errorf("resolve checkpoints config: %w", err)
+	}
+	primaryIsRefs := cpkg.PrimaryIsRefs(cfg)
+
+	present, readErr := checkpointPresentLocally(ctx, repo, refs, checkpointID, primaryIsRefs)
+	if readErr != nil {
+		return repo, false, fmt.Errorf("failed to read checkpoint %s: %w", checkpointID, readErr)
+	}
+	if present {
+		return repo, true, nil
+	}
+
+	freshRepo, fetchErr := refreshCheckpoint(ctx, checkpointID, primaryIsRefs)
+	if fetchErr != nil {
+		logging.Warn(logCtx, "failed to refresh explicit checkpoint before attach; treating as new",
+			slog.String("checkpoint_id", checkpointID.String()),
+			slog.String("error", fetchErr.Error()))
+		return repo, false, nil
+	}
+	present, readErr = checkpointPresentLocally(ctx, freshRepo, refs, checkpointID, primaryIsRefs)
+	if readErr != nil {
+		return freshRepo, false, fmt.Errorf("failed to read checkpoint %s after refresh: %w", checkpointID, readErr)
+	}
+	return freshRepo, present, nil
+}
+
 // refreshCheckpoint fetches the checkpoint referenced by HEAD from the remote and
 // returns a freshly-opened repo so go-git sees the newly-fetched refs/packfiles.
 // The fetch is backend-aware: git-refs fetches just this checkpoint's ref, while
@@ -753,21 +831,26 @@ func saveAttachSessionState(ctx context.Context, repo *git.Repository, existingS
 		}
 	}
 
-	// Populate BaseCommit from HEAD if not already set, so the session becomes
-	// active and future commits in the same session receive Entire-Checkpoint trailers.
-	if state.BaseCommit == "" {
-		if head, headErr := repo.Head(); headErr == nil {
-			headHash := head.Hash().String()
-			state.BaseCommit = headHash
-			state.AttributionBaseCommit = headHash
+	// Trailer-linked attaches establish the session's code-checkpoint linkage:
+	// BaseCommit lets later commits/amends recognize the session, and
+	// LastCheckpointID is the trailer the amend hook restores. An explicit-target
+	// review checkpoint is instead bound through commit_sha metadata and must not
+	// replace the session's own checkpoint/base or appear in resume/session output.
+	if !opts.explicitTarget() {
+		if state.BaseCommit == "" {
+			if head, headErr := repo.Head(); headErr == nil {
+				headHash := head.Hash().String()
+				state.BaseCommit = headHash
+				state.AttributionBaseCommit = headHash
+			}
 		}
+		state.LastCheckpointID = checkpointID
 	}
 
 	state.CLIVersion = versioninfo.Version
 	state.AttachedManually = true
 	state.AgentType = agentType
 	state.TranscriptPath = transcriptPath
-	state.LastCheckpointID = checkpointID
 	// Only transition to Ended if the session is not already active — avoid
 	// breaking an ongoing session whose BaseCommit has just been restored above.
 	if !state.Phase.IsActive() {

cmd/entire/cli/attach_test.go

@@ -1259,6 +1259,27 @@ func TestAttach_ReviewExplicitTargetCreatesShaBoundCheckpoint(t *testing.T) {
 		t.Errorf("session metadata Kind = %q, want agent_review", metadata.Kind)
 	}
 
+	// The commit-SHA-bound review checkpoint is not the session's code
+	// checkpoint. Preserve the session-stop checkpoint and leave its empty base
+	// untouched so amend hooks and resume/session output never treat targetID as
+	// trailer-linked session state.
+	attachedState, err := stateStore.Load(context.Background(), sessionID)
+	if err != nil {
+		t.Fatalf("load attached session state: %v", err)
+	}
+	if attachedState == nil {
+		t.Fatal("attached session state is missing")
+	}
+	if attachedState.LastCheckpointID != ownCheckpointID {
+		t.Errorf("LastCheckpointID = %s, want original session checkpoint %s", attachedState.LastCheckpointID, ownCheckpointID)
+	}
+	if attachedState.BaseCommit != "" {
+		t.Errorf("BaseCommit = %q, want empty; explicit-target attach must not bind session state to HEAD", attachedState.BaseCommit)
+	}
+	if attachedState.AttributionBaseCommit != "" {
+		t.Errorf("AttributionBaseCommit = %q, want empty; explicit-target attach must not bind attribution state to HEAD", attachedState.AttributionBaseCommit)
+	}
+
 	// HEAD must be untouched: same commit, no trailer appended.
 	newHeadRef, err := repo.Head()
 	if err != nil {
@@ -1292,6 +1313,72 @@ func TestAttach_ReviewExplicitTargetCreatesShaBoundCheckpoint(t *testing.T) {
 	}
 }
 
+// A well-formed --commit-sha that names no commit in the repository must be
+// rejected: silently recording it would produce provenance pointing at nothing.
+func TestAttach_ExplicitTargetRejectsUnknownCommit(t *testing.T) {
+	setupAttachTestRepo(t)
+
+	sessionID := "review-session-unknown-commit"
+	setupClaudeTranscript(t, sessionID, `{"type":"user","message":{"role":"user","content":"please review"},"uuid":"u1"}
+`)
+
+	var out bytes.Buffer
+	err := runAttach(context.Background(), &out, &out, sessionID, agent.AgentNameClaudeCode, attachOptions{
+		Force:        true,
+		Review:       true,
+		CheckpointID: id.CheckpointID("aabbccddeeff"),
+		CommitSHA:    strings.Repeat("deadbeef", 5),
+	})
+	if err == nil || !strings.Contains(err.Error(), "does not name a commit") {
+		t.Fatalf("expected unknown-commit rejection, got: %v", err)
+	}
+}
+
+// An explicit-target ID that already exists with OTHER content must be
+// refused, never modified: the ID is the caller's identity contract, and
+// appending to or rebuilding an existing checkpoint would corrupt it.
+func TestAttach_ExplicitTargetRefusesExistingCheckpointWithoutSession(t *testing.T) {
+	setupAttachTestRepo(t)
+
+	repoRoot := mustGetwd(t)
+	repo, err := git.PlainOpen(repoRoot)
+	if err != nil {
+		t.Fatal(err)
+	}
+	headSHA, err := repo.Head()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	targetID := id.CheckpointID("aabbccddeeff")
+	firstSession := "review-session-original"
+	setupClaudeTranscript(t, firstSession, `{"type":"user","message":{"role":"user","content":"first"},"uuid":"u1"}
+`)
+	var out bytes.Buffer
+	if err := runAttach(context.Background(), &out, &out, firstSession, agent.AgentNameClaudeCode, attachOptions{
+		Force:        true,
+		Review:       true,
+		CheckpointID: targetID,
+		CommitSHA:    headSHA.Hash().String(),
+	}); err != nil {
+		t.Fatalf("first explicit-target attach failed: %v", err)
+	}
+
+	secondSession := "review-session-collision"
+	setupClaudeTranscript(t, secondSession, `{"type":"user","message":{"role":"user","content":"second"},"uuid":"u1"}
+`)
+	out.Reset()
+	err = runAttach(context.Background(), &out, &out, secondSession, agent.AgentNameClaudeCode, attachOptions{
+		Force:        true,
+		Review:       true,
+		CheckpointID: targetID,
+		CommitSHA:    headSHA.Hash().String(),
+	})
+	if err == nil || !strings.Contains(err.Error(), "refusing to modify an existing checkpoint") {
+		t.Fatalf("expected collision refusal, got: %v", err)
+	}
+}
+
 // The explicit-target flags are a unit: both must be supplied, only with
 // --review, and the commit SHA must be a full 40-hex value.
 func TestAttach_ExplicitTargetFlagValidation(t *testing.T) {