Address PR feedback: guaranteed enqueue, cleanup parity

4501fc9→main·

pfleidi·1w ago·3 files·+43 added/-3 removed

MigrateBranchToRefs promised refs are queued for push, but setRef's enqueue is best-effort — a failed enqueue left migrated refs written locally and silently never pushed. The migration now enqueues explicitly and fails on error; duplicates collapse on Drain.

PushQueuedCheckpointRefs now runs post-push shadow-branch cleanup after a successful flush, matching the pre-push paths.

Sessions

01KWX19DEHT2AEBYYGZQY33HV7View transcript

[?
Fix Merge Conflicts and Checkpoint MigrationClaude Code·Fable 5·2 steps](/content/gh/entireio/cli/session/bd0430d2-6e5a-41d2-a42e-bce6f4b55711#timeline-01KWX19DEHT2AEBYYGZQY33HV7/index.html)

Changes

3

• cmd/entire/cli

• checkpoint

• Mmigrate.go+17/-2

• Mmigrate_test.go+20

• strategy

• Mmanual_commit_push.go+6/-1

40 unmodified lines

41
42
43
44
45
44
45
46
47
48
49
9 unmodified lines

59
60
61
62
63
64
65
66
67
68
37 unmodified lines

106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121

40 unmodified lines

// a re-run after more branch activity fast-forwards the ref (parenting on the
// existing commit).
//
// New and advanced refs are enqueued for push; this function does not push.
// When dryRun is true it reports what would change without writing refs.
// New and advanced refs are enqueued for push — a failed enqueue is an error,
// not best-effort. This function does not push. When dryRun is true it reports
// what would change without writing refs.
func MigrateBranchToRefs(ctx context.Context, repo *git.Repository, dryRun bool) (MigrateResult, error) {
    var result MigrateResult

9 unmodified lines

refsStore := newGitRefsStore(repo)
    authorName, authorEmail := GetGitAuthorFromRepo(repo)
    queue, err := PushQueueForRepo(ctx, repo)
    if err != nil {
        return result, fmt.Errorf("resolve push queue: %w", err)
    }

walkErr := WalkCheckpointShards(ctx, repo, tree, func(cid id.CheckpointID, cpTreeHash plumbing.Hash) error {
        if err := ctx.Err(); err != nil {
37 unmodified lines

if err := refsStore.setRef(ctx, cid, commitHash); err != nil {
            return fmt.Errorf("set ref for checkpoint %s: %w", cid, err)
        }
        // setRef's own enqueue is best-effort (a condensation write must not
        // fail on it); the migration's queued-for-push contract needs a
        // guaranteed one. Duplicates collapse on Drain.
        refName, err := RefName(cid)
        if err != nil {
            return fmt.Errorf("ref name for checkpoint %s: %w", cid, err)
        }
        if err := queue.Enqueue(refName); err != nil {
            return fmt.Errorf("enqueue checkpoint %s for push: %w", cid, err)
        }
        result.Migrated = append(result.Migrated, cid)
        return nil
    })

Mcmd/entire/cli/checkpoint/migrate.go+17/-2

2 unmodified lines

3
4
5
6
7
8
9
291 unmodified lines

301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325

2 unmodified lines

import (
    "context"
    "encoding/json"
    "os"
    "testing"

git "github.com/go-git/go-git/v6"
291 unmodified lines

assert.Empty(t, commit.ParentHashes, "unreadable ref must not become the parent")
}

func TestMigrateBranchToRefs_EnqueueFailureIsError(t *testing.T) {
    t.Parallel()
    repo, _ := setupBranchTestRepo(t)
    ctx := context.Background()
    branch := NewGitStore(repo, DefaultV1Refs())
    cid := id.MustCheckpointID("a1b2c3d4e5f6")
    seedBranchCheckpoint(t, branch, cid, "s1")

// Occupy the queue file path with a directory so appending fails: the
    // queued-for-push contract must surface this, not leave the migrated ref
    // silently unpushed.
    queue, err := PushQueueForRepo(ctx, repo)
    require.NoError(t, err)
    require.NoError(t, os.MkdirAll(queue.queuePath(), 0o755))

_, err = MigrateBranchToRefs(ctx, repo, false)
    require.Error(t, err, "a failed enqueue must fail the migration")
}

func TestMigrateBranchToRefs_NoBranchIsNoop(t *testing.T) {
    t.Parallel()
    repo, _ := setupBranchTestRepo(t) // initial commit only; no v1 checkpoint branch yet

Mcmd/entire/cli/checkpoint/migrate_test.go+20

185 unmodified lines

186
187
188
189
189
190
191
192
193
194
195
196
197

185 unmodified lines

if !checkpointPolicyAllowsGitHook(ctx, repo) {
        return 0, errors.New("checkpoint policy does not allow pushing checkpoint refs; refs stay queued")
    }
    return flushCheckpointRefsQueue(ctx, repo, ps.pushTarget())
pushed, err := flushCheckpointRefsQueue(ctx, repo, ps.pushTarget())
if err != nil {
    return pushed, err
}
    cleanupPushedShadowBranches(ctx)
    return pushed, nil
}

// flushCheckpointRefsQueue drains the push-discovery queue and batch-pushes the