Skip to content

Commit f5dadcc

Browse files
dipreeclaude
andcommitted
feat(attach): explicit-target review attach bound to a commit SHA
Adds an explicit-target mode to 'entire session attach --review': --checkpoint-id <id> --commit-sha <sha> creates the checkpoint under a caller-supplied ID bound to the given commit via a new commit_sha metadata field, instead of resolving HEAD's Entire-Checkpoint trailer and amending the commit. This unblocks platform review provenance for commits that cannot carry a trailer (merge commits, squash merges, human pushes): the reviewed commit is immutable once pushed, so the trailer-based linkage requires a force push that is never allowed. The platform mints the checkpoint ID and passes the reviewed commit's SHA; the binding lives in checkpoint metadata, not the commit message. - commit_sha (omitempty) on Metadata, CheckpointSummary and WriteOptions, plumbed through the session and summary writers. - Explicit-target mode skips trailer resolution, the session-has-checkpoint refusal (the reviewer's own eagerly-condensed checkpoint is irrelevant to the new checkpoint), and never amends HEAD. - Retried attaches with the same target are idempotent no-op successes so the platform can safely retry the publication script. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent c4cf088 commit f5dadcc

4 files changed

Lines changed: 271 additions & 7 deletions

File tree

api/checkpoint/metadata.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,12 @@ type WriteOptions struct {
149149
// Kind identifies the session purpose (e.g., "agent_review"). Empty for normal sessions.
150150
Kind string
151151

152+
// CommitSHA is the full SHA of the user-code commit this checkpoint is
153+
// bound to, for checkpoints attached to a commit without an
154+
// Entire-Checkpoint trailer (e.g. platform-published review provenance
155+
// keyed by the reviewed commit). Empty for trailer-linked checkpoints.
156+
CommitSHA string
157+
152158
// ReviewSkills is the snapshot of skills used (only meaningful when Kind is a review kind).
153159
// May be empty when a review is attached post-hoc without declared skills.
154160
ReviewSkills []string
@@ -393,6 +399,12 @@ type Metadata struct {
393399
// Kind identifies the session purpose (e.g., "agent_review"). Empty for normal sessions.
394400
Kind string `json:"kind,omitempty"`
395401

402+
// CommitSHA is the full SHA of the user-code commit this checkpoint is
403+
// bound to, for checkpoints attached without an Entire-Checkpoint
404+
// trailer (e.g. platform-published review provenance). Empty for
405+
// trailer-linked checkpoints.
406+
CommitSHA string `json:"commit_sha,omitempty"`
407+
396408
// ReviewSkills lists the review skills that were run (only set when Kind is a review kind).
397409
// May be empty when a review was attached post-hoc without declared skills.
398410
ReviewSkills []string `json:"review_skills,omitempty"`
@@ -503,6 +515,12 @@ type CheckpointSummary struct {
503515
// agent history (a session with Kind == "imported"): read-only and
504516
// commit-less.
505517
Imported bool `json:"imported,omitempty"`
518+
519+
// CommitSHA is the full SHA of the user-code commit this checkpoint is
520+
// bound to, for checkpoints attached without an Entire-Checkpoint
521+
// trailer (e.g. platform-published review provenance). Empty for
522+
// trailer-linked checkpoints.
523+
CommitSHA string `json:"commit_sha,omitempty"`
506524
}
507525

508526
// SessionMetrics contains hook-provided session metrics from agents that report

cmd/entire/cli/attach.go

Lines changed: 86 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"log/slog"
99
"os"
1010
"os/exec"
11+
"regexp"
1112
"strings"
1213
"time"
1314

@@ -35,6 +36,9 @@ import (
3536
"github.com/spf13/cobra"
3637
)
3738

39+
// fullCommitSHARegex matches a full 40-hex git commit SHA (lowercased input).
40+
var fullCommitSHARegex = regexp.MustCompile(`^[0-9a-f]{40}$`)
41+
3842
// attachOptions carries optional flags for runAttach. Force is the original
3943
// flag; Review opts the attach into recording the session as an
4044
// agent_review in the checkpoint metadata.
@@ -54,6 +58,22 @@ type attachOptions struct {
5458
// transcript's first user prompt. Set from a pending-review marker when
5559
// `entire attach --review` adopts the prompt the user was asked to run.
5660
ReviewPromptOverride string
61+
// CheckpointID, when non-empty, is the explicit target checkpoint ID to
62+
// create instead of resolving one from HEAD's Entire-Checkpoint trailer.
63+
// Used by platform review-provenance publication, where the server mints
64+
// the ID. Requires Review and CommitSHA; the explicit-target mode never
65+
// amends HEAD (the binding is the recorded CommitSHA, not a trailer).
66+
CheckpointID id.CheckpointID
67+
// CommitSHA, when non-empty, is the full SHA of the user-code commit this
68+
// attach is bound to. Recorded in the checkpoint metadata (commit_sha) in
69+
// place of a commit-message trailer. Requires Review and CheckpointID.
70+
CommitSHA string
71+
}
72+
73+
// explicitTarget reports whether this attach targets a caller-supplied
74+
// checkpoint ID bound to a commit SHA (no trailer resolution, no HEAD amend).
75+
func (opts attachOptions) explicitTarget() bool {
76+
return !opts.CheckpointID.IsEmpty()
5777
}
5878

5979
// committedRefs resolves the committed metadata topology.
@@ -73,10 +93,12 @@ func openAttachStore(ctx context.Context, repo *git.Repository, refs cpkg.Persis
7393

7494
func newAttachCmd() *cobra.Command {
7595
var (
76-
force bool
77-
agentFlag string
78-
reviewFlag bool
79-
skillsFlag []string
96+
force bool
97+
agentFlag string
98+
reviewFlag bool
99+
skillsFlag []string
100+
checkpointIDFlag string
101+
commitSHAFlag string
80102
)
81103
cmd := &cobra.Command{
82104
Use: "attach <session-id>",
@@ -115,6 +137,27 @@ the transcript and prints the detected agent name.`,
115137
Review: reviewFlag,
116138
ReviewSkillsOverride: skillsFlag,
117139
}
140+
// Explicit-target mode: a server-minted checkpoint ID bound to a
141+
// commit SHA, with no trailer resolution and no HEAD amend. Both
142+
// flags travel together and only make sense for reviews.
143+
if checkpointIDFlag != "" || commitSHAFlag != "" {
144+
if checkpointIDFlag == "" || commitSHAFlag == "" {
145+
return errors.New("--checkpoint-id and --commit-sha must be used together")
146+
}
147+
if !reviewFlag {
148+
return errors.New("--checkpoint-id/--commit-sha require --review")
149+
}
150+
cpID, idErr := id.NewCheckpointID(checkpointIDFlag)
151+
if idErr != nil {
152+
return fmt.Errorf("invalid --checkpoint-id: %w", idErr)
153+
}
154+
normalizedSHA := strings.ToLower(commitSHAFlag)
155+
if !fullCommitSHARegex.MatchString(normalizedSHA) {
156+
return fmt.Errorf("invalid --commit-sha (want full 40-hex SHA): %s", commitSHAFlag)
157+
}
158+
opts.CheckpointID = cpID
159+
opts.CommitSHA = normalizedSHA
160+
}
118161
// When tagging as a review, consume any pending-review marker left
119162
// by `entire review` for an agent it could not launch itself: adopt
120163
// its agent / skills / prompt so the manual attach matches what the
@@ -149,6 +192,8 @@ the transcript and prints the detected agent name.`,
149192
cmd.Flags().StringVarP(&agentFlag, "agent", "a", string(agent.DefaultAgentName), "Agent that created the session (see 'entire agent list' for registered agents, including external)")
150193
cmd.Flags().BoolVar(&reviewFlag, "review", false, "Tag the attached session as an agent review")
151194
cmd.Flags().StringSliceVar(&skillsFlag, "skills", nil, "Optional: declare which review skills were run in this session. Only used with --review")
195+
cmd.Flags().StringVar(&checkpointIDFlag, "checkpoint-id", "", "Explicit checkpoint ID to create for this attach (requires --review and --commit-sha; skips trailer resolution and never amends HEAD)")
196+
cmd.Flags().StringVar(&commitSHAFlag, "commit-sha", "", "Full SHA of the commit this attach is bound to, recorded as commit_sha in the checkpoint metadata (requires --review and --checkpoint-id)")
152197
return cmd
153198
}
154199

@@ -237,7 +282,10 @@ func runAttach(ctx context.Context, w, errW io.Writer, sessionID string, agentNa
237282
}
238283

239284
// If session already has a checkpoint, just offer to link it.
240-
if existingState != nil && !existingState.LastCheckpointID.IsEmpty() {
285+
// Explicit-target mode is exempt: it authors a separate new checkpoint
286+
// under the caller-supplied ID, so the session's own checkpoint (e.g.
287+
// from the session-stop hook's eager condense) is irrelevant.
288+
if existingState != nil && !existingState.LastCheckpointID.IsEmpty() && !opts.explicitTarget() {
241289
// Review-upgrade isn't supported yet: the existing checkpoint's
242290
// metadata tree would need to be rewritten with Kind/ReviewSkills/
243291
// ReviewPrompt set, and a new commit pushed onto entire/checkpoints/v1.
@@ -288,8 +336,16 @@ func runAttach(ctx context.Context, w, errW io.Writer, sessionID string, agentNa
288336
meta := extractTranscriptMetadata(transcriptData)
289337
warnEmptyTranscriptMetadata(errW, ag.Name(), meta, opts)
290338

291-
// Determine checkpoint ID: reuse from HEAD if one exists, otherwise generate new.
292-
checkpointID, isExistingCheckpoint := resolveCheckpointID(ctx, headCommit)
339+
// Determine checkpoint ID: an explicit target uses the caller-supplied ID
340+
// (new by construction, bound to opts.CommitSHA); otherwise reuse HEAD's
341+
// trailer if present or generate a fresh ID.
342+
var checkpointID id.CheckpointID
343+
var isExistingCheckpoint bool
344+
if opts.explicitTarget() {
345+
checkpointID, isExistingCheckpoint = opts.CheckpointID, false
346+
} else {
347+
checkpointID, isExistingCheckpoint = resolveCheckpointID(ctx, headCommit)
348+
}
293349

294350
// If HEAD references an existing checkpoint, make sure we have it locally
295351
// before writing — otherwise we'd create a fresh session 0 under the same
@@ -335,6 +391,20 @@ func runAttach(ctx context.Context, w, errW io.Writer, sessionID string, agentNa
335391
}
336392
}
337393

394+
// Explicit-target idempotency: a retried attach with the same server-minted
395+
// checkpoint ID and session is a no-op success, not an error — the caller
396+
// (platform provenance publication) retries the whole script.
397+
if opts.explicitTarget() {
398+
exists, readErr := checkpointHasSessionMetadata(ctx, repo, refs, checkpointID, sessionID)
399+
if readErr != nil {
400+
return fmt.Errorf("failed to check checkpoint %s for session %s: %w", checkpointID.String(), sessionID, readErr)
401+
}
402+
if exists {
403+
fmt.Fprintf(w, "Session %s already attached to checkpoint %s\n", sessionID, checkpointID.String())
404+
return nil
405+
}
406+
}
407+
338408
author, err := GetGitAuthor(ctx)
339409
if err != nil {
340410
return fmt.Errorf("failed to get git author: %w", err)
@@ -368,6 +438,9 @@ func runAttach(ctx context.Context, w, errW io.Writer, sessionID string, agentNa
368438
writeOpts.ReviewPrompt = reviewPromptForAttach(meta, opts)
369439
writeOpts.HasReview = true
370440
}
441+
if opts.explicitTarget() {
442+
writeOpts.CommitSHA = opts.CommitSHA
443+
}
371444

372445
if err := store.Write(ctx, cpkg.Session(writeOpts)); err != nil {
373446
return fmt.Errorf("failed to write checkpoint: %w", err)
@@ -386,6 +459,12 @@ func runAttach(ctx context.Context, w, errW io.Writer, sessionID string, agentNa
386459
}
387460

388461
fmt.Fprintf(w, " Created checkpoint %s\n", checkpointID)
462+
// Explicit-target attaches are bound to their commit via the recorded
463+
// commit_sha, not a commit-message trailer — never amend HEAD.
464+
if opts.explicitTarget() {
465+
fmt.Fprintf(w, " Bound to commit %s\n", opts.CommitSHA)
466+
return nil
467+
}
389468
amendOrPrintTrailer(logCtx, w, errW, headCommit, checkpointID.String(), opts.Force)
390469

391470
return nil

cmd/entire/cli/attach_test.go

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1165,6 +1165,167 @@ func TestAttach_ReviewWithExistingCheckpointErrors(t *testing.T) {
11651165
}
11661166
}
11671167

1168+
// Explicit-target mode: the platform review-provenance flow supplies a
1169+
// server-minted checkpoint ID and the reviewed commit's SHA. The attach must
1170+
// create that checkpoint bound to the commit via metadata (commit_sha), even
1171+
// when HEAD carries no Entire-Checkpoint trailer, and must never amend HEAD.
1172+
func TestAttach_ReviewExplicitTargetCreatesShaBoundCheckpoint(t *testing.T) {
1173+
setupAttachTestRepo(t)
1174+
1175+
repoRoot := mustGetwd(t)
1176+
repo, err := git.PlainOpen(repoRoot)
1177+
if err != nil {
1178+
t.Fatal(err)
1179+
}
1180+
headRef, err := repo.Head()
1181+
if err != nil {
1182+
t.Fatal(err)
1183+
}
1184+
headSHA := headRef.Hash().String()
1185+
headCommit, err := repo.CommitObject(headRef.Hash())
1186+
if err != nil {
1187+
t.Fatal(err)
1188+
}
1189+
if len(trailers.ParseAllCheckpoints(headCommit.Message)) != 0 {
1190+
t.Fatal("test requires a HEAD without an Entire-Checkpoint trailer")
1191+
}
1192+
originalMessage := headCommit.Message
1193+
1194+
sessionID := "review-session-explicit-target"
1195+
setupClaudeTranscript(t, sessionID, `{"type":"user","message":{"role":"user","content":"please review"},"uuid":"u1"}
1196+
`)
1197+
// Simulate the session-stop hook's eager condense: the reviewer session
1198+
// already holds its own checkpoint. Explicit-target mode must not refuse.
1199+
ownCheckpointID, err := cpkg.GenerateCheckpointID(context.Background())
1200+
if err != nil {
1201+
t.Fatal(err)
1202+
}
1203+
stateStore, err := session.NewStateStore(context.Background())
1204+
if err != nil {
1205+
t.Fatal(err)
1206+
}
1207+
if err := stateStore.Save(context.Background(), &session.State{
1208+
SessionID: sessionID,
1209+
StartedAt: time.Now(),
1210+
WorktreePath: repoRoot,
1211+
AgentType: agent.AgentTypeClaudeCode,
1212+
LastCheckpointID: ownCheckpointID,
1213+
}); err != nil {
1214+
t.Fatalf("seed session state: %v", err)
1215+
}
1216+
1217+
targetID := id.CheckpointID("aabbccddeeff")
1218+
opts := attachOptions{
1219+
Force: true,
1220+
Review: true,
1221+
ReviewSkillsOverride: []string{"/review"},
1222+
CheckpointID: targetID,
1223+
CommitSHA: headSHA,
1224+
}
1225+
var out bytes.Buffer
1226+
if err := runAttach(context.Background(), &out, &out, sessionID, agent.AgentNameClaudeCode, opts); err != nil {
1227+
t.Fatalf("explicit-target review attach failed: %v", err)
1228+
}
1229+
1230+
// Checkpoint exists at the supplied ID with the commit binding recorded.
1231+
store := cpkg.NewGitStore(repo, cpkg.DefaultV1Refs())
1232+
summary, err := store.Read(context.Background(), targetID)
1233+
if err != nil {
1234+
t.Fatalf("Read(%s): %v", targetID, err)
1235+
}
1236+
if summary == nil {
1237+
t.Fatalf("checkpoint %s not created", targetID)
1238+
}
1239+
if summary.CommitSHA != headSHA {
1240+
t.Errorf("summary.CommitSHA = %q, want %q", summary.CommitSHA, headSHA)
1241+
}
1242+
if !summary.HasReview {
1243+
t.Error("summary.HasReview should be true")
1244+
}
1245+
if len(summary.Sessions) != 1 {
1246+
t.Fatalf("checkpoint has %d sessions, want 1", len(summary.Sessions))
1247+
}
1248+
metadata, err := store.ReadSessionMetadata(context.Background(), targetID, 0)
1249+
if err != nil {
1250+
t.Fatalf("ReadSessionMetadata: %v", err)
1251+
}
1252+
if metadata.SessionID != sessionID {
1253+
t.Errorf("session metadata SessionID = %q, want %q", metadata.SessionID, sessionID)
1254+
}
1255+
if metadata.CommitSHA != headSHA {
1256+
t.Errorf("session metadata CommitSHA = %q, want %q", metadata.CommitSHA, headSHA)
1257+
}
1258+
if metadata.Kind != string(session.KindAgentReview) {
1259+
t.Errorf("session metadata Kind = %q, want agent_review", metadata.Kind)
1260+
}
1261+
1262+
// HEAD must be untouched: same commit, no trailer appended.
1263+
newHeadRef, err := repo.Head()
1264+
if err != nil {
1265+
t.Fatal(err)
1266+
}
1267+
if newHeadRef.Hash().String() != headSHA {
1268+
t.Errorf("HEAD moved from %s to %s; explicit-target attach must not amend", headSHA, newHeadRef.Hash())
1269+
}
1270+
newHeadCommit, err := repo.CommitObject(newHeadRef.Hash())
1271+
if err != nil {
1272+
t.Fatal(err)
1273+
}
1274+
if newHeadCommit.Message != originalMessage {
1275+
t.Error("HEAD commit message changed; explicit-target attach must not amend")
1276+
}
1277+
1278+
// Retried attach with the same target is an idempotent no-op success.
1279+
out.Reset()
1280+
if err := runAttach(context.Background(), &out, &out, sessionID, agent.AgentNameClaudeCode, opts); err != nil {
1281+
t.Fatalf("retried explicit-target attach should be a no-op success: %v", err)
1282+
}
1283+
if !strings.Contains(out.String(), "already attached") {
1284+
t.Errorf("expected idempotent no-op message, got: %q", out.String())
1285+
}
1286+
summary, err = store.Read(context.Background(), targetID)
1287+
if err != nil {
1288+
t.Fatal(err)
1289+
}
1290+
if len(summary.Sessions) != 1 {
1291+
t.Errorf("retry duplicated the session: %d sessions, want 1", len(summary.Sessions))
1292+
}
1293+
}
1294+
1295+
// The explicit-target flags are a unit: both must be supplied, only with
1296+
// --review, and the commit SHA must be a full 40-hex value.
1297+
func TestAttach_ExplicitTargetFlagValidation(t *testing.T) {
1298+
setupAttachTestRepo(t)
1299+
1300+
run := func(args ...string) error {
1301+
cmd := newAttachCmd()
1302+
var out bytes.Buffer
1303+
cmd.SetOut(&out)
1304+
cmd.SetErr(&out)
1305+
cmd.SetArgs(args)
1306+
return cmd.Execute()
1307+
}
1308+
1309+
fullSHA := strings.Repeat("ab", 20)
1310+
cases := []struct {
1311+
name string
1312+
args []string
1313+
want string
1314+
}{
1315+
{"checkpoint-id without commit-sha", []string{"s1", "--review", "--checkpoint-id", "aabbccddeeff"}, "must be used together"},
1316+
{"commit-sha without checkpoint-id", []string{"s1", "--review", "--commit-sha", fullSHA}, "must be used together"},
1317+
{"without review", []string{"s1", "--checkpoint-id", "aabbccddeeff", "--commit-sha", fullSHA}, "require --review"},
1318+
{"bad checkpoint id", []string{"s1", "--review", "--checkpoint-id", "not/valid", "--commit-sha", fullSHA}, "invalid --checkpoint-id"},
1319+
{"short commit sha", []string{"s1", "--review", "--checkpoint-id", "aabbccddeeff", "--commit-sha", "abc123"}, "invalid --commit-sha"},
1320+
}
1321+
for _, tc := range cases {
1322+
err := run(tc.args...)
1323+
if err == nil || !strings.Contains(err.Error(), tc.want) {
1324+
t.Errorf("%s: got %v, want error containing %q", tc.name, err, tc.want)
1325+
}
1326+
}
1327+
}
1328+
11681329
// Regression for the second "review-attach overwrote the session on the
11691330
// checkpoint" report: a DIFFERENT session ID (not present in the existing
11701331
// checkpoint) must APPEND at the next-available index, not overwrite

0 commit comments

Comments
 (0)