fix review attach metadata guard

8efab29→main·

pfleidi·1mo ago·2 files·+69 added/-1 removed

Check existing checkpoint sessions through metadata so review attach refuses metadata-only sessions before rewriting them.

Cover the missing-state regression with a focused attach test.

Sessions

1bb7e927da3eView transcript

[?
Fix Review Attach Metadata and Checkpoint ReconciliationCodex·GPT-5.5·1 step](/content/gh/entireio/cli/session/019e953d-3ce8-7881-a9eb-31211a16b642#timeline-1bb7e927da3e/index.html)

Changes

2

• cmd/entire/cli

• Mattach.go+25/-1

• Mattach_test.go+44

264 unmodified lines

265
266
267
268
268
269
270
271
272
273
274
275
68 unmodified lines

344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369

264 unmodified lines

// review-attach on such a session silently overwrites the existing
    // session's metadata in the checkpoint.
    if opts.Review && isExistingCheckpoint {
        if existing, readErr := store.ReadSessionContentByID(ctx, checkpointID, sessionID); readErr == nil && existing != nil {
            exists, readErr := checkpointHasSessionMetadata(ctx, store, checkpointID, sessionID)
            if readErr != nil {
                return fmt.Errorf("failed to check checkpoint %s for session %s: %w", checkpointID.String(), sessionID, readErr)
            }
            if exists {
                return fmt.Errorf(
                    "session %s is already recorded in checkpoint %s; rewriting an existing checkpoint as a review is not supported yet",
                    sessionID, checkpointID.String(),
                )
            }
        }
68 unmodified lines

return nil
}

func checkpointHasSessionMetadata(ctx context.Context, store *cpkg.GitStore, checkpointID id.CheckpointID, sessionID string) (bool, error) {
    summary, err := store.ReadCommitted(ctx, checkpointID)
    if err != nil {
        return false, fmt.Errorf("read checkpoint summary: %w", err)
    }
    if summary == nil {
        return false, nil
    }
    for i := range summary.Sessions {
        metadata, err := store.ReadSessionMetadata(ctx, checkpointID, i)
        if err != nil {
            return false, fmt.Errorf("read session %d metadata: %w", i, err)
        }
        if metadata != nil && metadata.SessionID == sessionID {
            return true, nil
        }
    }
    return false, nil
}

// getHeadCommit returns the HEAD commit object.
func getHeadCommit(repo *git.Repository) (*object.Commit, error) {
    headRef, err := repo.Head()

Mcmd/entire/cli/attach.go+25/-1

24 unmodified lines

25
26
27
28
29
30
31
1252 unmodified lines

1284
1285
1286
1287
1288
1289
1290
1291
1292
1293
1294
1295
1296
1297
1298
1299
1300
1301
1302
1303
1304
1305
1306
1307
1308
1309
1310
1311
1312
1313
1314
1315
1316
1317
1318
1319
1320
1321
1322
1323
1324
1325
1326
1327
1328
1329
1330
1331
1332

24 unmodified lines

cliReview "github.com/entireio/cli/cmd/entire/cli/review"
    "github.com/entireio/cli/cmd/entire/cli/session"
    "github.com/entireio/cli/cmd/entire/cli/settings"
    "github.com/entireio/cli/cmd/entire/cli/strategy"
    "github.com/entireio/cli/cmd/entire/cli/testutil"
    "github.com/entireio/cli/cmd/entire/cli/trailers"
    "github.com/entireio/cli/redact"
1252 unmodified lines

}
}

func TestAttach_ReviewWithExistingMetadataOnlyCheckpointErrorsEvenWithoutSessionState(t *testing.T) {
    setupAttachTestRepo(t)

repoRoot := mustGetwd(t)
    repo, err := git.PlainOpen(repoRoot)
    if err != nil {
        t.Fatal(err)
    }

sessionID := "test-attach-review-metadata-only"
    checkpointID := id.MustCheckpointID("aabbccddeeff")
    store := cpkg.NewGitStore(repo, cpkg.DefaultV1Refs())
    if err := store.WriteCommitted(context.Background(), cpkg.WriteCommittedOptions{
        CheckpointID: checkpointID,
        SessionID:    sessionID,
        Strategy:     strategy.StrategyNameManualCommit,
        Transcript:   redact.AlreadyRedacted(nil),
        Prompts:      []string{"original prompt"},
        AuthorName:   "Test",
        AuthorEmail:  "test@example.com",
        Agent:        agent.AgentTypeClaudeCode,
    }); err != nil {
        t.Fatalf("WriteCommitted: %v", err)
    }
    runGitInDir(t, repoRoot, "commit", "--amend", "--no-edit", "-m", "init\n\nEntire-Checkpoint: "+checkpointID.String())

setupClaudeTranscript(t, sessionID, `{\"type\":\"user\",\"message\":{\"role\":\"user\",\"content\":\"review again\"},\"uuid\":\"uuid-1\"}
`)

var out bytes.Buffer
    err = runAttach(context.Background(), &out, sessionID, agent.AgentNameClaudeCode, attachOptions{
        Force:                true,
        Review:               true,
        ReviewSkillsOverride: []string{"/pr-review-toolkit:review-pr"},
    })
    if err == nil {
        t.Fatal("expected error when review-attaching a metadata-only session already recorded in HEAD's checkpoint")
    }
    if !strings.Contains(err.Error(), "already recorded in checkpoint") {
        t.Errorf("error should mention 'already recorded in checkpoint'; got: %v", err)
    }
}

// Regression: attach must NOT silently attach skills from the spawn-path
// config. settings.Review[agent] is what the user would run if they used
// `entire review`, not a claim about what ran in a given manual session.