From 68490976b0db4a39e2c9c2520bc9872475077a27 Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Fri, 4 Sep 2026 17:16:57 -0500 Subject: [PATCH 1/6] engine: resolve bisection from durable ordered evidence Persist and validate exact bisection results, pin base anchors, safely invalidate stale work, and persist complete queue state. --- internal/checkpoint/checkpoint.go | 6 + .../migrations/003_format_version.sql | 10 + .../postgres/migrations/004_snapshot_blob.sql | 17 + internal/checkpoint/postgres/postgres.go | 50 +- internal/checkpoint/postgres/postgres_test.go | 117 ++- internal/engine/burnin_test.go | 11 +- internal/engine/checkpoint.go | 239 ++++- internal/engine/engine.go | 834 +++++++++++++++- internal/engine/engine_test.go | 908 +++++++++++++++++- internal/engine/frontier_model_test.go | 368 +++++++ internal/engine/transitions.go | 136 +++ internal/engine/tree_liveness_test.go | 146 +++ internal/forge/client.go | 24 + internal/forge/client_test.go | 24 + internal/gitops/git.go | 10 +- mq/checkpoint/checkpoint.go | 154 ++- mq/checkpoint/checkpoint_test.go | 16 + mq/mq.go | 37 +- 18 files changed, 3016 insertions(+), 91 deletions(-) create mode 100644 internal/checkpoint/postgres/migrations/003_format_version.sql create mode 100644 internal/checkpoint/postgres/migrations/004_snapshot_blob.sql create mode 100644 internal/engine/frontier_model_test.go create mode 100644 internal/engine/transitions.go create mode 100644 internal/engine/tree_liveness_test.go create mode 100644 mq/checkpoint/checkpoint_test.go diff --git a/internal/checkpoint/checkpoint.go b/internal/checkpoint/checkpoint.go index b0ce553..b811e29 100644 --- a/internal/checkpoint/checkpoint.go +++ b/internal/checkpoint/checkpoint.go @@ -8,10 +8,16 @@ import ( pub "github.com/rbtr/shunt/mq/checkpoint" ) +const CurrentFormatVersion = pub.CurrentFormatVersion + type QueueKey = pub.QueueKey type QueueSnapshot = pub.QueueSnapshot type ActiveBatchSnapshot = pub.ActiveBatchSnapshot type PullRequestSnapshot = pub.PullRequestSnapshot +type PendingNodeSnapshot = pub.PendingNodeSnapshot +type BisectionTreeSnapshot = pub.BisectionTreeSnapshot +type HeldLeafSnapshot = pub.HeldLeafSnapshot +type OutboxTransitionSnapshot = pub.OutboxTransitionSnapshot // Store is the public mq/checkpoint.Store (kept here for internal imports). type Store = pub.Store diff --git a/internal/checkpoint/postgres/migrations/003_format_version.sql b/internal/checkpoint/postgres/migrations/003_format_version.sql new file mode 100644 index 0000000..848706f --- /dev/null +++ b/internal/checkpoint/postgres/migrations/003_format_version.sql @@ -0,0 +1,10 @@ +-- Add the checkpoint format version to the Postgres queue-state store. +-- +-- The store persists the snapshot as individual columns, not a JSON blob, so +-- QueueSnapshot.FormatVersion was silently dropped on save and always read +-- back as 0 — unlike the bolt store, which json.Marshals the whole struct. +-- Existing rows default to 0; the engine treats a below-current version with +-- in-flight work as a legacy checkpoint it cannot resume exactly, discards it, +-- and re-derives the queue from the forge. +ALTER TABLE shunt_queue_state + ADD COLUMN IF NOT EXISTS format_version integer NOT NULL DEFAULT 0; diff --git a/internal/checkpoint/postgres/migrations/004_snapshot_blob.sql b/internal/checkpoint/postgres/migrations/004_snapshot_blob.sql new file mode 100644 index 0000000..1c55916 --- /dev/null +++ b/internal/checkpoint/postgres/migrations/004_snapshot_blob.sql @@ -0,0 +1,17 @@ +-- Store the whole QueueSnapshot as one JSON document. +-- +-- The store was written as a fixed set of columns (pending, active, +-- linger_since, base_generation, staging_sequence, format_version). Every +-- field added to QueueSnapshot since — the bisection Trees, PendingNodes, +-- the TransitionOutbox, and the per-batch RunID / BaseAnchor / ExactKey / +-- LineagePath — was silently dropped on save and read back as zero, because +-- nothing here knew about it. The bolt and in-memory stores json.Marshal the +-- whole struct and were unaffected; production runs on Postgres. +-- +-- From now on SaveQueue writes the full snapshot into `snapshot` and LoadQueue +-- reads from it. The individual columns are still written (external queries and +-- the updated_at index depend on them) but are no longer the source of truth. +-- Rows written before this migration have snapshot IS NULL; LoadQueue falls +-- back to reconstructing what it can from the columns for those. +ALTER TABLE shunt_queue_state + ADD COLUMN IF NOT EXISTS snapshot jsonb; diff --git a/internal/checkpoint/postgres/postgres.go b/internal/checkpoint/postgres/postgres.go index 97f7652..09e356a 100644 --- a/internal/checkpoint/postgres/postgres.go +++ b/internal/checkpoint/postgres/postgres.go @@ -24,6 +24,17 @@ var PostgresMigrationV1 string //go:embed migrations/002_queue_leases.sql var PostgresMigrationV2 string +// PostgresMigrationV3 adds the checkpoint format version column. +// +//go:embed migrations/003_format_version.sql +var PostgresMigrationV3 string + +// PostgresMigrationV4 adds the full-snapshot JSON column so no QueueSnapshot +// field can be silently dropped by a store that predates it. +// +//go:embed migrations/004_snapshot_blob.sql +var PostgresMigrationV4 string + type rowScanner interface { Scan(dest ...any) error } @@ -102,7 +113,7 @@ func (p *Store) ApplyMigrations(ctx context.Context) error { if err := p.ready(); err != nil { return err } - for _, migration := range []string{PostgresMigrationV1, PostgresMigrationV2} { + for _, migration := range []string{PostgresMigrationV1, PostgresMigrationV2, PostgresMigrationV3, PostgresMigrationV4} { m := strings.ReplaceAll(migration, "shunt_queue_state", p.stateTbl) m = strings.ReplaceAll(m, "shunt_queue_leases", p.leaseTbl) if _, err := p.db.ExecContext(ctx, m); err != nil { @@ -166,22 +177,31 @@ func (p *Store) SaveQueue(ctx context.Context, snapshot checkpoint.QueueSnapshot if err != nil { return fmt.Errorf("state: marshal active batches: %w", err) } + // The whole snapshot, verbatim. This is the source of truth on load; the + // individual columns above are kept for external queries and the + // updated_at index but never lose a field the way hand-picked columns do. + full, err := json.Marshal(snapshot) + if err != nil { + return fmt.Errorf("state: marshal snapshot: %w", err) + } var linger any if !snapshot.LingerSince.IsZero() { linger = snapshot.LingerSince.UTC() } _, err = p.db.ExecContext(ctx, fmt.Sprintf(` INSERT INTO %s ( - owner, repo, base, pending, active, linger_since, base_generation, staging_sequence, updated_at -) VALUES ($1, $2, $3, $4::jsonb, $5::jsonb, $6, $7, $8, now()) + owner, repo, base, pending, active, linger_since, base_generation, staging_sequence, format_version, snapshot, updated_at +) VALUES ($1, $2, $3, $4::jsonb, $5::jsonb, $6, $7, $8, $9, $10::jsonb, now()) ON CONFLICT (owner, repo, base) DO UPDATE SET pending = EXCLUDED.pending, active = EXCLUDED.active, linger_since = EXCLUDED.linger_since, base_generation = EXCLUDED.base_generation, staging_sequence = EXCLUDED.staging_sequence, + format_version = EXCLUDED.format_version, + snapshot = EXCLUDED.snapshot, updated_at = now() -`, p.stateTbl), snapshot.Key.Owner, snapshot.Key.Repo, snapshot.Key.Base, string(pending), string(active), linger, snapshot.BaseGeneration, snapshot.StagingSequence) +`, p.stateTbl), snapshot.Key.Owner, snapshot.Key.Repo, snapshot.Key.Base, string(pending), string(active), linger, snapshot.BaseGeneration, snapshot.StagingSequence, snapshot.FormatVersion, string(full)) if err != nil { return fmt.Errorf("state: save queue %s/%s@%s: %w", snapshot.Key.Owner, snapshot.Key.Repo, snapshot.Key.Base, err) } @@ -197,20 +217,33 @@ func (p *Store) LoadQueue(ctx context.Context, key checkpoint.QueueKey) (checkpo if err := key.Validate(); err != nil { return checkpoint.QueueSnapshot{}, false, err } - var pendingRaw, activeRaw []byte + var pendingRaw, activeRaw, snapshotRaw []byte var linger sql.NullTime - var baseGeneration, stagingSequence int + var baseGeneration, stagingSequence, formatVersion int err := p.db.QueryRowContext(ctx, fmt.Sprintf(` -SELECT pending, active, linger_since, base_generation, staging_sequence +SELECT pending, active, linger_since, base_generation, staging_sequence, format_version, snapshot FROM %s WHERE owner = $1 AND repo = $2 AND base = $3 -`, p.stateTbl), key.Owner, key.Repo, key.Base).Scan(&pendingRaw, &activeRaw, &linger, &baseGeneration, &stagingSequence) +`, p.stateTbl), key.Owner, key.Repo, key.Base).Scan(&pendingRaw, &activeRaw, &linger, &baseGeneration, &stagingSequence, &formatVersion, &snapshotRaw) if errors.Is(err, sql.ErrNoRows) { return checkpoint.QueueSnapshot{}, false, nil } if err != nil { return checkpoint.QueueSnapshot{}, false, fmt.Errorf("state: load queue %s/%s@%s: %w", key.Owner, key.Repo, key.Base, err) } + // Preferred path: the row carries the whole snapshot. Rows written before + // migration 004 have snapshot IS NULL and fall through to the columns. + if len(snapshotRaw) > 0 { + var snapshot checkpoint.QueueSnapshot + if err := json.Unmarshal(snapshotRaw, &snapshot); err != nil { + return checkpoint.QueueSnapshot{}, false, fmt.Errorf("state: decode snapshot: %w", err) + } + snapshot.Key = key + if err := snapshot.Validate(); err != nil { + return checkpoint.QueueSnapshot{}, false, err + } + return snapshot.Clone(), true, nil + } var pending [][]int if err := json.Unmarshal(pendingRaw, &pending); err != nil { return checkpoint.QueueSnapshot{}, false, fmt.Errorf("state: decode pending queue: %w", err) @@ -220,6 +253,7 @@ WHERE owner = $1 AND repo = $2 AND base = $3 return checkpoint.QueueSnapshot{}, false, fmt.Errorf("state: decode active batches: %w", err) } snapshot := checkpoint.QueueSnapshot{ + FormatVersion: formatVersion, Key: key, Pending: pending, Active: activeFromJSON(activeJSON), diff --git a/internal/checkpoint/postgres/postgres_test.go b/internal/checkpoint/postgres/postgres_test.go index 90bdf98..839adf8 100644 --- a/internal/checkpoint/postgres/postgres_test.go +++ b/internal/checkpoint/postgres/postgres_test.go @@ -29,6 +29,7 @@ func TestPostgresSaveQueueUpsertsSnapshot(t *testing.T) { LingerSince: linger, BaseGeneration: 2, StagingSequence: 7, + FormatVersion: checkpoint.CurrentFormatVersion, } if err := store.SaveQueue(context.Background(), snapshot); err != nil { @@ -65,6 +66,100 @@ func TestPostgresSaveQueueUpsertsSnapshot(t *testing.T) { if got := exec.args[6:8]; !reflect.DeepEqual(got, []any{2, 7}) { t.Fatalf("generation args = %#v", got) } + if got := exec.args[8]; got != checkpoint.CurrentFormatVersion { + t.Fatalf("format_version arg = %#v, want %d", got, checkpoint.CurrentFormatVersion) + } + if !strings.Contains(exec.query, "format_version") { + t.Fatalf("upsert does not write format_version:\n%s", exec.query) + } + // $10 is the whole snapshot, verbatim — the field nothing can drop. + if !strings.Contains(exec.query, "snapshot") { + t.Fatalf("upsert does not write the snapshot column:\n%s", exec.query) + } + var full checkpoint.QueueSnapshot + if err := json.Unmarshal([]byte(exec.args[9].(string)), &full); err != nil { + t.Fatalf("snapshot json: %v", err) + } + if !reflect.DeepEqual(full.Pending, snapshot.Pending) || + !reflect.DeepEqual(full.Active, snapshot.Active) || + full.FormatVersion != snapshot.FormatVersion || + full.StagingSequence != snapshot.StagingSequence || + !full.LingerSince.Equal(snapshot.LingerSince) { + t.Fatalf("snapshot blob did not round-trip: %#v", full) + } +} + +func TestPostgresRoundTripsEveryField(t *testing.T) { + // A snapshot that exercises the fields the column layout never persisted: + // the bisection Trees (with anchor + results cache + held leaves), the + // PendingNodes lineage, the TransitionOutbox, and the per-batch RunID / + // BaseAnchor / ExactKey / LineagePath. + snapshot := checkpoint.QueueSnapshot{ + Key: checkpoint.QueueKey{Owner: "octo", Repo: "app", Base: "main"}, + Pending: [][]int{{9}}, + PendingNodes: []checkpoint.PendingNodeSnapshot{ + {PRs: []int{9}, RunID: "run-9", Path: "L"}, + }, + Active: []checkpoint.ActiveBatchSnapshot{{ + PRs: []checkpoint.PullRequestSnapshot{{Number: 4, HeadSHA: "abc"}}, + StagingBranch: "mq/main/staging-run-7-L", + StagingSHA: "stage7", + RunID: "run-7", + LineagePath: "L", + ExactKey: "v2|anchorsha|merge|4", + BaseAnchor: "anchorsha", + }}, + Trees: []checkpoint.BisectionTreeSnapshot{{ + RunID: "run-7", + Anchor: "anchorsha", + Open: 1, + Cursor: 0, + Accepted: []checkpoint.PullRequestSnapshot{{Number: 3, HeadSHA: "cccc"}}, + Results: map[string]string{"v2|anchorsha|merge|3": "success"}, + Held: []checkpoint.HeldLeafSnapshot{{ + Batch: checkpoint.ActiveBatchSnapshot{ + PRs: []checkpoint.PullRequestSnapshot{{Number: 5, HeadSHA: "eeee"}}, + StagingBranch: "mq/main/staging-run-7-LR", + StagingSHA: "stage7lr", + RunID: "run-7", + LineagePath: "LR", + }, + Outcome: "success", + }}, + }}, + TransitionOutbox: []checkpoint.OutboxTransitionSnapshot{{ + Kind: "landed", PRs: []int{3}, RunID: "run-7", EventID: "run-7|landed|3", Attempts: 2, + }}, + BaseGeneration: 1, + StagingSequence: 4, + FormatVersion: checkpoint.CurrentFormatVersion, + } + + db := &fakeDB{} + store := &Store{db: db, stateTbl: "shunt_queue_state", leaseTbl: "shunt_queue_leases"} + if err := store.SaveQueue(context.Background(), snapshot); err != nil { + t.Fatalf("SaveQueue: %v", err) + } + blob := db.execs[0].args[9].(string) + + load := &fakeDB{rows: []fakeRow{{scan: func(dest ...any) error { + *dest[0].(*[]byte) = []byte(`[]`) + *dest[1].(*[]byte) = []byte(`[]`) + *dest[2].(*sql.NullTime) = sql.NullTime{} + *dest[3].(*int) = 0 + *dest[4].(*int) = 0 + *dest[5].(*int) = checkpoint.CurrentFormatVersion + *dest[6].(*[]byte) = []byte(blob) + return nil + }}}} + loadStore := &Store{db: load, stateTbl: "shunt_queue_state", leaseTbl: "shunt_queue_leases"} + got, ok, err := loadStore.LoadQueue(context.Background(), snapshot.Key) + if err != nil || !ok { + t.Fatalf("LoadQueue: ok=%v err=%v", ok, err) + } + if !reflect.DeepEqual(got, snapshot.Clone()) { + t.Fatalf("round trip lost data:\n got %#v\n want %#v", got, snapshot.Clone()) + } } func TestPostgresLoadQueueDecodesSnapshot(t *testing.T) { @@ -76,6 +171,7 @@ func TestPostgresLoadQueueDecodesSnapshot(t *testing.T) { *dest[2].(*sql.NullTime) = sql.NullTime{Time: linger, Valid: true} *dest[3].(*int) = 2 *dest[4].(*int) = 7 + *dest[5].(*int) = checkpoint.CurrentFormatVersion return nil }, }}} @@ -104,7 +200,10 @@ func TestPostgresLoadQueueDecodesSnapshot(t *testing.T) { if !snapshot.LingerSince.Equal(linger) || snapshot.BaseGeneration != 2 || snapshot.StagingSequence != 7 { t.Fatalf("snapshot metadata = %#v", snapshot) } - if len(db.queries) != 1 || !strings.Contains(db.queries[0].query, "SELECT pending, active") { + if snapshot.FormatVersion != checkpoint.CurrentFormatVersion { + t.Fatalf("format version = %d, want %d", snapshot.FormatVersion, checkpoint.CurrentFormatVersion) + } + if len(db.queries) != 1 || !strings.Contains(db.queries[0].query, "format_version") { t.Fatalf("queries = %#v", db.queries) } } @@ -132,8 +231,8 @@ func TestPostgresApplyMigrationsAndDelete(t *testing.T) { if err := store.DeleteQueue(context.Background(), checkpoint.QueueKey{Owner: "octo", Repo: "app", Base: "main"}); err != nil { t.Fatalf("DeleteQueue: %v", err) } - if len(db.execs) != 3 { - t.Fatalf("execs = %d, want 3", len(db.execs)) + if len(db.execs) != 5 { + t.Fatalf("execs = %d, want 5", len(db.execs)) } if !strings.Contains(db.execs[0].query, "CREATE TABLE IF NOT EXISTS shunt_queue_state") { t.Fatalf("first migration query = %q", db.execs[0].query) @@ -141,10 +240,16 @@ func TestPostgresApplyMigrationsAndDelete(t *testing.T) { if !strings.Contains(db.execs[1].query, "CREATE TABLE IF NOT EXISTS shunt_queue_leases") { t.Fatalf("second migration query = %q", db.execs[1].query) } - if !strings.Contains(db.execs[2].query, "DELETE FROM shunt_queue_state") { - t.Fatalf("delete query = %q", db.execs[2].query) + if !strings.Contains(db.execs[2].query, "ADD COLUMN IF NOT EXISTS format_version") { + t.Fatalf("third migration query = %q", db.execs[2].query) + } + if !strings.Contains(db.execs[3].query, "ADD COLUMN IF NOT EXISTS snapshot") { + t.Fatalf("fourth migration query = %q", db.execs[3].query) + } + if !strings.Contains(db.execs[4].query, "DELETE FROM shunt_queue_state") { + t.Fatalf("delete query = %q", db.execs[4].query) } - if got := db.execs[2].args; !reflect.DeepEqual(got, []any{"octo", "app", "main"}) { + if got := db.execs[4].args; !reflect.DeepEqual(got, []any{"octo", "app", "main"}) { t.Fatalf("delete args = %#v", got) } } diff --git a/internal/engine/burnin_test.go b/internal/engine/burnin_test.go index 71d0df8..00bb608 100644 --- a/internal/engine/burnin_test.go +++ b/internal/engine/burnin_test.go @@ -37,7 +37,10 @@ func TestBurnInBisectionWithRealGitStaging(t *testing.T) { BisectFanout: 1, }, burn, gitops.NewStager(repoDir, "bot", "unused-token", "bot", "bot@example.invalid")) - for i := 0; i < 12 && (len(burn.merged) < 2 || !burn.bounced[2]); i++ { + // Root finalization waits for every leaf before beginning ordered native + // merges, so this proof needs room for the held-leaf barrier as well as + // bisection and Forge completion. + for i := 0; i < 20 && (len(burn.merged) < 2 || !burn.bounced[2]); i++ { if err := eng.Reconcile(ctx); err != nil { t.Fatalf("reconcile %d: %v", i, err) } @@ -130,6 +133,12 @@ func (b *burnInForge) GetPR(_ context.Context, _, _ string, index int) (forge.Pu return *b.prs[index], nil } +func (b *burnInForge) BranchHead(_ context.Context, _, _, _ string) (string, error) { + // The burn-in drives the real git stager against a scratch repo; return no + // anchor so staging checks out the live base branch instead of a fake SHA. + return "", nil +} + func (b *burnInForge) AutomergeState(_ context.Context, _, _ string, index int) (forge.AutomergeState, error) { return forge.AutomergeState{Scheduled: b.automerge[index]}, nil } diff --git a/internal/engine/checkpoint.go b/internal/engine/checkpoint.go index 513c9e9..48236ce 100644 --- a/internal/engine/checkpoint.go +++ b/internal/engine/checkpoint.go @@ -69,7 +69,8 @@ func (e *Engine) saveCheckpoint(ctx context.Context) error { } func (e *Engine) emptyCheckpoint() bool { - return len(e.pending) == 0 && len(e.active) == 0 && e.lingerSince.IsZero() + return len(e.pending) == 0 && len(e.active) == 0 && len(e.trees) == 0 && + len(e.outbox) == 0 && e.lingerSince.IsZero() } func (e *Engine) queueKey() checkpoint.QueueKey { @@ -79,28 +80,136 @@ func (e *Engine) queueKey() checkpoint.QueueKey { func (e *Engine) snapshot() checkpoint.QueueSnapshot { active := make([]checkpoint.ActiveBatchSnapshot, len(e.active)) for i, a := range e.active { - prs := make([]checkpoint.PullRequestSnapshot, len(a.prs)) - for j, pr := range a.prs { - prs[j] = checkpoint.PullRequestSnapshot{Number: pr.Number, HeadSHA: pr.Head.Sha} + active[i] = e.snapshotBatch(a) + } + pendingNodes := make([]checkpoint.PendingNodeSnapshot, len(e.pending)) + for i, candidate := range e.pending { + pendingNodes[i] = checkpoint.PendingNodeSnapshot{ + PRs: append([]int(nil), candidate...), + RunID: e.lineageRunID[candidate[0]], + Path: e.lineage[candidate[0]], + } + } + trees := make([]checkpoint.BisectionTreeSnapshot, 0, len(e.trees)) + for runID, tree := range e.trees { + ts := checkpoint.BisectionTreeSnapshot{ + RunID: runID, + Anchor: e.rootAnchor[runID], + // Open is derived on load from the restored active/pending nodes + // (treeHasUnresolvedNode); it is persisted only as an at-a-glance + // count for anyone inspecting the checkpoint. + Open: e.unresolvedNodeCount(runID), + Cursor: tree.cursor, + Accepted: snapshotPRs(tree.accepted), + Results: tree.results, } - active[i] = checkpoint.ActiveBatchSnapshot{ - PRs: prs, - StagingBranch: a.stagingBranch, - StagingSHA: a.stagingSHA, - BaseGeneration: a.baseGen, - Outcome: a.outcome, - PhaseSince: a.phaseSince, - MissingGateRetries: a.missingGateRetries, + for _, leaf := range tree.held { + ts.Held = append(ts.Held, checkpoint.HeldLeafSnapshot{ + Batch: e.snapshotBatch(leaf.batch), + Outcome: leaf.outcome, + }) + } + trees = append(trees, ts) + } + outbox := make([]checkpoint.OutboxTransitionSnapshot, len(e.outbox)) + for i, entry := range e.outbox { + outbox[i] = checkpoint.OutboxTransitionSnapshot{ + Kind: entry.t.Kind, + PRs: append([]int(nil), entry.t.PRs...), + StagingBranch: entry.t.StagingBranch, + RunID: entry.t.RunID, + LineagePath: entry.t.LineagePath, + Reason: entry.t.Reason, + EventID: entry.t.EventID, + Attempts: entry.attempts, } } return checkpoint.QueueSnapshot{ - Key: e.queueKey(), - Pending: clonePending(e.pending), - Active: active, - LingerSince: e.lingerSince, - BaseGeneration: e.baseGen, - StagingSequence: e.stagingSeq, + FormatVersion: checkpoint.CurrentFormatVersion, + Key: e.queueKey(), + Pending: clonePending(e.pending), + PendingNodes: pendingNodes, + Active: active, + LingerSince: e.lingerSince, + BaseGeneration: e.baseGen, + StagingSequence: e.stagingSeq, + Trees: trees, + TransitionOutbox: outbox, + } +} + +// snapshotBatch is the single place an in-memory activeBatch is projected into +// its durable shape — used for both the live active list and a tree's held +// leaves, so a new field is added in exactly one spot. +func (e *Engine) snapshotBatch(a *activeBatch) checkpoint.ActiveBatchSnapshot { + return checkpoint.ActiveBatchSnapshot{ + PRs: snapshotPRs(a.prs), + StagingBranch: a.stagingBranch, + StagingSHA: a.stagingSHA, + BaseGeneration: a.baseGen, + Outcome: a.outcome, + PhaseSince: a.phaseSince, + MissingGateRetries: a.missingGateRetries, + RunID: a.runID, + LineagePath: a.lineagePath, + ExactKey: a.exactKey, + BaseAnchor: e.rootAnchor[a.runID], + DebugURL: a.debugURL, + Speculative: a.speculative, + } +} + +// restoreBatch is the inverse of snapshotBatch: the field mapping only, with +// the caller supplying the re-fetched PRs. +func (e *Engine) restoreBatch(snap checkpoint.ActiveBatchSnapshot, prs []forge.PullRequest) *activeBatch { + return &activeBatch{ + prs: prs, + stagingBranch: snap.StagingBranch, + stagingSHA: snap.StagingSHA, + baseGen: snap.BaseGeneration, + outcome: snap.Outcome, + phase: phaseForOutcome(snap.Outcome), + phaseSince: orNow(snap.PhaseSince, e.now()), + missingGateRetries: snap.MissingGateRetries, + runID: snap.RunID, + lineagePath: snap.LineagePath, + exactKey: snap.ExactKey, + debugURL: snap.DebugURL, + speculative: snap.Speculative, + } +} + +func snapshotPRs(prs []forge.PullRequest) []checkpoint.PullRequestSnapshot { + out := make([]checkpoint.PullRequestSnapshot, len(prs)) + for i, pr := range prs { + out[i] = checkpoint.PullRequestSnapshot{Number: pr.Number, HeadSHA: pr.Head.Sha} + } + return out +} + +// refetchPRs re-reads each persisted PR so a resumed batch carries a full +// PullRequest. With strict=false (a live active batch) a PR that closed or +// merged during the downtime is dropped — land() observes the merge. With +// strict=true (accepted / held-leaf evidence) any state or head change is an +// error: the evidence was gathered against a PR that no longer exists as +// tested, and this snapshot cannot be safely resumed. +func (e *Engine) refetchPRs(ctx context.Context, snaps []checkpoint.PullRequestSnapshot, strict bool, what string) ([]forge.PullRequest, error) { + out := make([]forge.PullRequest, 0, len(snaps)) + for _, ps := range snaps { + pr, err := e.fc.GetPR(ctx, e.cfg.Owner, e.cfg.Repo, ps.Number) + if err != nil { + return nil, fmt.Errorf("resume %s PR #%d: %w", what, ps.Number, err) + } + if strict { + if pr.State != "open" || pr.Merged || pr.Head.Sha != ps.HeadSHA { + return nil, fmt.Errorf("resume %s PR #%d changed", what, ps.Number) + } + } else if pr.State != "open" || pr.Merged { + continue + } + out = append(out, pr) } + return out, nil } // applySnapshot restores queue state from a durable snapshot. Unlike the @@ -112,23 +221,42 @@ func (e *Engine) snapshot() checkpoint.QueueSnapshot { // that closed or merged during the downtime are dropped (a merged PR is // observed in land()), and a fully-drained branch is deleted. func (e *Engine) applySnapshot(ctx context.Context, snapshot checkpoint.QueueSnapshot) error { + if snapshot.FormatVersion > checkpoint.CurrentFormatVersion { + // A newer engine wrote this. Refuse rather than mis-read it (rollback + // protection). Validate() normally catches this first. + return fmt.Errorf("queue checkpoint format %d is newer than this engine (%d)", snapshot.FormatVersion, checkpoint.CurrentFormatVersion) + } + if snapshot.FormatVersion < checkpoint.CurrentFormatVersion && (len(snapshot.Pending) > 0 || len(snapshot.Active) > 0) { + // A legacy checkpoint lacks the base anchor, accepted accumulator and + // lineage the ordered frontier needs to resume a partly-tested queue + // exactly. Do not error forever (that wedges the queue): discard the + // in-flight state and re-derive from the forge. Staged branches from + // the old attempt orphan and are cleaned up by the stale-branch sweep. + e.logger.Warn("discarding a legacy queue checkpoint with in-flight work; re-deriving the queue from the forge", + "format_version", snapshot.FormatVersion, "current", checkpoint.CurrentFormatVersion, + "pending", len(snapshot.Pending), "active", len(snapshot.Active)) + snapshot = checkpoint.QueueSnapshot{FormatVersion: checkpoint.CurrentFormatVersion, Key: snapshot.Key} + } e.pending = clonePending(snapshot.Pending) + e.lineage = map[int]string{} + e.lineageRunID = map[int]string{} + e.rootAnchor = map[string]string{} + for _, node := range snapshot.PendingNodes { + if node.RunID == "" { + continue + } + e.lineage[node.PRs[0]] = node.Path + e.lineageRunID[node.PRs[0]] = node.RunID + } e.lingerSince = snapshot.LingerSince e.baseGen = snapshot.BaseGeneration e.stagingSeq = snapshot.StagingSequence active := make([]*activeBatch, 0, len(snapshot.Active)) for _, snap := range snapshot.Active { - prs := make([]forge.PullRequest, 0, len(snap.PRs)) - for _, ps := range snap.PRs { - pr, err := e.fc.GetPR(ctx, e.cfg.Owner, e.cfg.Repo, ps.Number) - if err != nil { - return fmt.Errorf("resume staged batch: re-fetch PR #%d: %w", ps.Number, err) - } - if pr.State != "open" || pr.Merged { - continue // merged or closed during downtime - } - prs = append(prs, pr) + prs, err := e.refetchPRs(ctx, snap.PRs, false, "staged batch") + if err != nil { + return err } if len(prs) == 0 { // Every PR landed during the downtime — remove the empty branch. @@ -137,18 +265,53 @@ func (e *Engine) applySnapshot(ctx context.Context, snapshot checkpoint.QueueSna } continue } - active = append(active, &activeBatch{ - prs: prs, - stagingBranch: snap.StagingBranch, - stagingSHA: snap.StagingSHA, - baseGen: snap.BaseGeneration, - outcome: snap.Outcome, - phase: phaseForOutcome(snap.Outcome), - phaseSince: orNow(snap.PhaseSince, e.now()), - missingGateRetries: snap.MissingGateRetries, - }) + active = append(active, e.restoreBatch(snap, prs)) + if snap.RunID != "" && snap.BaseAnchor != "" { + e.rootAnchor[snap.RunID] = snap.BaseAnchor + } } e.active = active + + e.trees = map[string]*bisectionTree{} + for _, ts := range snapshot.Trees { + results := make(map[string]string, len(ts.Results)) + for key, outcome := range ts.Results { + results[key] = outcome + } + tree := &bisectionTree{cursor: ts.Cursor, results: results} + accepted, err := e.refetchPRs(ctx, ts.Accepted, true, "accepted") + if err != nil { + return err + } + tree.accepted = accepted + for _, held := range ts.Held { + prs, err := e.refetchPRs(ctx, held.Batch.PRs, true, "held leaf") + if err != nil { + return err + } + tree.held = append(tree.held, heldLeaf{batch: e.restoreBatch(held.Batch, prs), outcome: held.Outcome}) + } + e.trees[ts.RunID] = tree + if ts.Anchor != "" { + e.rootAnchor[ts.RunID] = ts.Anchor + } + } + + e.outbox = e.outbox[:0] + for _, ob := range snapshot.TransitionOutbox { + e.outbox = append(e.outbox, outboxEntry{ + t: Transition{ + Kind: ob.Kind, + PRs: append([]int(nil), ob.PRs...), + StagingBranch: ob.StagingBranch, + RunID: ob.RunID, + LineagePath: ob.LineagePath, + Reason: ob.Reason, + EventID: ob.EventID, + }, + attempts: ob.Attempts, + }) + } return nil } diff --git a/internal/engine/engine.go b/internal/engine/engine.go index d10784d..2f6967e 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -12,6 +12,7 @@ import ( "fmt" "log/slog" "sort" + "strconv" "strings" "time" @@ -47,6 +48,18 @@ type Config struct { Logger *slog.Logger } +type heldLeaf struct { + batch *activeBatch + outcome string +} + +type bisectionTree struct { + accepted []forge.PullRequest + results map[string]string + held []heldLeaf + cursor int +} + type activeBatch struct { prs []forge.PullRequest stagingBranch string @@ -59,6 +72,10 @@ type activeBatch struct { phase string // "waiting_gate", "waiting_merge", or "bisecting" phaseSince time.Time // when the current phase started missingGateRetries int + exactKey string + runID string // the root batch this was bisected out of + lineagePath string // "r", "r0", "r01", … ; parent is this minus one char + speculative bool } // ForgeAPI is the subset of the forge client the engine needs (interface so the @@ -69,6 +86,7 @@ type ForgeAPI interface { AutomergeState(ctx context.Context, owner, repo string, index int) (forge.AutomergeState, error) ListReviews(ctx context.Context, owner, repo string, index int) ([]forge.Review, error) ProtectedBranch(ctx context.Context, owner, repo, branch string) (forge.BranchProtection, error) + BranchHead(ctx context.Context, owner, repo, branch string) (string, error) LatestCommitStatus(ctx context.Context, owner, repo, sha, statusContext string) (forge.CommitStatus, bool, error) RunStatus(ctx context.Context, owner, repo, sha, branch string) (string, error) RunTargetURL(ctx context.Context, owner, repo, sha, branch string) (string, error) @@ -103,8 +121,11 @@ var ( ) // Stager builds an integration ("staging") branch from a base + PR head refs. +// baseAnchor, when non-empty, is an immutable commit SHA to build on instead +// of the live tip of base (see bisection-tree-finalization.md, +// "Immutable-base requirement"). type Stager interface { - BuildStaging(ctx context.Context, base, stagingBranch string, refs []gitops.MergedRef) (sha string, conflictPR int, err error) + BuildStaging(ctx context.Context, base, baseAnchor, stagingBranch string, refs []gitops.MergedRef) (sha string, conflictPR int, err error) } type Engine struct { @@ -121,6 +142,34 @@ type Engine struct { baseGen int stagingSeq int + // Bisection lineage. A staging branch name carries where in the tree it + // sits, so the tree can be read off the names without a side channel: + // + // --r root batch + // --r0 first half of the root + // --r01 second half of that first half + // + // A branch's parent is its name with the last character removed, so the + // whole ancestry is in the name and siblings sort adjacent. The previous + // scheme was -: the timestamp was taken per staging + // operation so it was unique to each attempt, and seq counted over the + // engine's lifetime. Neither encoded lineage, and under BisectFanout > 1 + // siblings stage at effectively the same instant, so nothing about the + // name distinguished them. + // + // runID identifies one root batch and everything bisected out of it. + // lineage maps a pending candidate to its path, keyed by the candidate's + // first PR number — the same idiom as bisectOrigins and requeueStates. + runID string + lineage map[int]string + lineageRunID map[int]string + trees map[string]*bisectionTree + // rootAnchor pins the base-branch commit SHA read when each root runID was + // opened. Every staged integration and every exact test key under that root + // is anchored to it, so main moving between two nodes cannot silently + // change what a cached outcome means. + rootAnchor map[string]string + // bisectOrigins tracks the first PR number of each pending candidate that was // produced by bisection. Checked in startNext to set phase = "bisecting". bisectOrigins map[int]bool @@ -135,8 +184,32 @@ type Engine struct { checkpointExists bool leaseHeld bool durableLease bool + + // transitions accumulates single-shot lifecycle records for the current + // Reconcile call; see transitions.go. + transitions []Transition + // outbox holds terminal transitions (those with an EventID, driving an + // irreversible side effect) until they have been re-sent enough times to + // survive a dropped reconcile response. It is checkpointed, so it also + // survives a restart. Redelivery is safe because the consumer de-dups by + // EventID. + outbox []outboxEntry + // pendingAcks are EventIDs the consumer confirmed on the reconcile request + // but that could not be applied yet because the outbox is loaded from the + // checkpoint inside Reconcile. Applied right after loadCheckpoint. + pendingAcks []string +} + +type outboxEntry struct { + t Transition + attempts int } +// outboxMaxAttempts bounds how many reconcile responses re-carry a terminal +// transition. Seven gives generous coverage for a dropped response without +// letting the outbox grow without bound. +const outboxMaxAttempts = 7 + func New(cfg Config, fc ForgeAPI, st Stager) *Engine { durableLease := cfg.Lease != nil if cfg.Lease == nil { @@ -162,12 +235,17 @@ func New(cfg Config, fc ForgeAPI, st Stager) *Engine { terminalQueueComments: map[int]string{}, requeueStates: map[int]string{}, mergeStrikes: map[string]int{}, + lineage: map[int]string{}, + lineageRunID: map[int]string{}, + trees: map[string]*bisectionTree{}, + rootAnchor: map[string]string{}, durableLease: durableLease, } } // Reconcile advances the queue by one step. Safe to call on a fixed interval. func (e *Engine) Reconcile(ctx context.Context) error { + e.transitions = nil if e.durableLease { var cancel context.CancelFunc ctx, cancel = context.WithTimeout(ctx, e.cfg.LeaseTTL/2) @@ -205,6 +283,12 @@ func (e *Engine) Reconcile(ctx context.Context) error { e.observeQueue() return err } + // The outbox only exists once the checkpoint is loaded, so consumer acks + // and the retransmit-attempt count are applied here, not at the top of the + // call — otherwise a fresh process would drop neither and redeliver every + // terminal transition forever. + e.applyPendingAcks() + e.ageOutbox() if err := e.reconcileContextErr(ctx); err != nil { return err } @@ -291,10 +375,13 @@ func (e *Engine) resetVolatileQueueState() { e.bisectOrigins = map[int]bool{} e.baseGen = 0 e.stagingSeq = 0 + e.runID = "" + e.lineage = nil e.queueComments = map[int]string{} e.terminalQueueComments = map[int]string{} e.requeueStates = map[int]string{} e.mergeStrikes = map[string]int{} + e.outbox = nil e.checkpointLoaded = false e.checkpointExists = false } @@ -511,15 +598,48 @@ func (e *Engine) startNext(ctx context.Context) (bool, error) { return false, err } if len(prs) == 0 { + // Every PR of this candidate withdrew before it could be staged. If it + // was a bisection node, consume its lineage and record it so the root + // it belonged to is not left waiting on a node that will never run. + if len(cand) > 0 { + if runID := e.lineageRunID[cand[0]]; runID != "" { + path := e.lineage[cand[0]] + delete(e.lineage, cand[0]) + delete(e.lineageRunID, cand[0]) + delete(e.bisectOrigins, cand[0]) + e.recordTransition(Transition{Kind: "bisected", RunID: runID, LineagePath: path}) + e.logger.Info("bisection node dropped: all its candidates withdrew before staging", "prs", cand, "runID", runID) + } + } return false, nil } - refs := make([]gitops.MergedRef, len(prs)) - for i, p := range prs { + runID, lineagePath := e.lineageFor(cand) + anchor, err := e.ensureRootAnchor(ctx, runID) + if err != nil { + return false, err + } + stagedPRs := prs + if tree, ok := e.trees[runID]; ok { + stagedPRs = append(append([]forge.PullRequest(nil), tree.accepted...), prs...) + } + refs := make([]gitops.MergedRef, len(stagedPRs)) + for i, p := range stagedPRs { refs[i] = gitops.MergedRef{PR: p.Number, Ref: fmt.Sprintf("refs/pull/%d/head", p.Number)} } - stagingBranch := e.stagingBranch() - sha, conflictPR, err := e.st.BuildStaging(ctx, e.cfg.Base, stagingBranch, refs) + exactKey := e.exactKey(anchor, stagedPRs) + if tree, ok := e.trees[runID]; ok { + if outcome, cached := tree.results[exactKey]; cached { + e.active = append(e.active, &activeBatch{prs: prs, stagingBranch: e.stagingBranchFor(runID, lineagePath), outcome: outcome, phase: phaseForOutcome(outcome), phaseSince: e.now(), runID: runID, lineagePath: lineagePath, exactKey: exactKey}) + return true, nil + } + } + stagingBranch := e.stagingBranchFor(runID, lineagePath) + // Every integration under a root is built on that root's immutable anchor, + // not the live base tip, so main moving mid-tree cannot change what a + // staged result means. Detecting and acting on such a move (root + // invalidation) is a separate change. + sha, conflictPR, err := e.st.BuildStaging(ctx, e.cfg.Base, anchor, stagingBranch, refs) if err != nil { if conflictPR > 0 { if e.cfg.Metrics != nil { @@ -541,12 +661,19 @@ func (e *Engine) startNext(ctx context.Context) (bool, error) { baseGen: e.baseGen, phase: phase, phaseSince: e.now(), + runID: runID, + lineagePath: lineagePath, + exactKey: exactKey, } e.active = append(e.active, a) if e.cfg.Metrics != nil { e.cfg.Metrics.IncBatchesStarted(e.metricLabels()) } e.logger.Info("testing batch", "prs", numbersOf(prs), "stagingBranch", a.stagingBranch, "sha", short(sha)) + e.recordTransition(Transition{ + Kind: "staged", PRs: numbersOf(prs), StagingBranch: a.stagingBranch, + RunID: a.runID, LineagePath: a.lineagePath, + }) return true, nil } @@ -585,6 +712,15 @@ func missingGateGrace(retries int) time.Duration { } func (e *Engine) checkActive(ctx context.Context) (bool, error) { + if resolved, err := e.invalidateAdvancedRoots(ctx); resolved || err != nil { + return resolved, err + } + if resolved, err := e.invalidateRootsOnCandidateChange(ctx); resolved || err != nil { + return resolved, err + } + if resolved, err := e.finalizeReadyTree(ctx); resolved || err != nil { + return resolved, err + } for _, a := range e.active { if a.outcome == "" { changed, err := e.requeueActiveIfHeadChanged(ctx, a) @@ -613,9 +749,19 @@ func (e *Engine) checkActive(ctx context.Context) (bool, error) { continue } if a.missingGateRetries >= missingGateMaxRetries { + // The design (bisection-tree-finalization.md, outcome + // table): a node whose gate never runs, after the retry + // budget, aborts the *root* with no source-PR mutation — + // an infra failure must not be published as a PR rejection. + // Tear the root down and re-queue every candidate fresh. + if e.requeuedTreeNode(ctx, a, "staging gate produced no result after retries", + "re-queued: the staging gate produced no result") { + e.logger.Error("bisection root aborted: a node's gate never ran", "prs", numbersOf(a.prs), "runID", a.runID) + return true, nil + } e.cleanupBatch(ctx, a) for _, staged := range a.prs { - e.bounce(ctx, staged.Number, staged.Head.Sha, "no CI gate run after 3 retries", "error", a.debugURL) + e.bounceFrom(ctx, a.runID, a.stagingBranch, staged.Number, staged.Head.Sha, "no CI gate run after 3 retries", "error", "") } e.logger.Error("staging batch abandoned after missing gate retries", "prs", numbersOf(a.prs)) return true, nil @@ -627,8 +773,12 @@ func (e *Engine) checkActive(ctx context.Context) (bool, error) { for i, pr := range prs { refs[i] = gitops.MergedRef{PR: pr.Number, Ref: fmt.Sprintf("refs/pull/%d/head", pr.Number)} } - branch := e.stagingBranch() - sha, conflictPR, err := e.st.BuildStaging(ctx, e.cfg.Base, branch, refs) + // A missing-gate retry restages the SAME batch at the same + // point in the tree, so it keeps its run and path rather than + // becoming a new node. cleanupBatch above deleted the previous + // branch, so the name is free to reuse. + branch := e.stagingBranchFor(a.runID, a.lineagePath) + sha, conflictPR, err := e.st.BuildStaging(ctx, e.cfg.Base, e.rootAnchor[a.runID], branch, refs) if err != nil { if conflictPR > 0 { return false, e.handleStagingConflict(ctx, prs, conflictPR) @@ -638,12 +788,15 @@ func (e *Engine) checkActive(ctx context.Context) (bool, error) { retry := a.missingGateRetries + 1 e.active = append(e.active, &activeBatch{ prs: prs, + runID: a.runID, + lineagePath: a.lineagePath, stagingBranch: branch, stagingSHA: sha, baseGen: e.baseGen, phase: "waiting_gate", phaseSince: e.now(), missingGateRetries: retry, + exactKey: a.exactKey, }) e.logger.Warn("staging batch re-staged after no gate result", "prs", numbersOf(prs), "stagingBranch", branch, "retry", retry) @@ -666,6 +819,9 @@ func (e *Engine) checkActive(ctx context.Context) (bool, error) { } a.outcome = status a.debugURL = debugURL + if tree, ok := e.trees[a.runID]; ok { + tree.results[a.exactKey] = status + } if e.cfg.Metrics != nil { e.cfg.Metrics.IncGateOutcome(e.metricLabels(), status) e.cfg.Metrics.ObserveGateDuration(e.metricLabels(), status, e.now().Sub(a.phaseSince)) @@ -673,6 +829,10 @@ func (e *Engine) checkActive(ctx context.Context) (bool, error) { if status == "success" { a.phase = "waiting_merge" a.phaseSince = e.now() + e.recordTransition(Transition{ + Kind: "gate_success", PRs: numbersOf(a.prs), StagingBranch: a.stagingBranch, + RunID: a.runID, LineagePath: a.lineagePath, + }) } default: // running, waiting, blocked -> keep waiting changed, err := e.invalidateStaleActive(ctx, a) @@ -692,8 +852,27 @@ func (e *Engine) checkActive(ctx context.Context) (bool, error) { e.requeueStaleActive(ctx, a) return true, nil } + // Fanout can stage a bisection node speculatively, before its left + // siblings have resolved. Its gate ran against an assumed accumulator; + // the result is authoritative only if its exact key still equals the + // key the resolved frontier now asks for. If a left sibling has since + // been accepted, the key differs and this node is superseded and + // re-staged on the real baseline (bisection-tree-finalization.md, + // "Speculative fanout"). + if a.exactKey != "" { + if tree, ok := e.trees[a.runID]; ok { + staged := append(append([]forge.PullRequest(nil), tree.accepted...), a.prs...) + if want := e.exactKey(e.rootAnchor[a.runID], staged); want != a.exactKey { + e.supersedeSpeculative(ctx, a) + return true, nil + } + } + } switch a.outcome { case "success": + if e.holdTreeLeaf(ctx, a, "success") { + return true, nil + } resolved, merged, err := e.land(ctx, a) if merged > 0 { e.baseGen++ @@ -765,6 +944,11 @@ func (e *Engine) invalidateStaleActive(ctx context.Context, a *activeBatch) (boo } func (e *Engine) discardIneligibleActive(ctx context.Context, a *activeBatch, pr int, reason string) { + if e.requeuedTreeNode(ctx, a, "a pinned candidate became ineligible", "re-queued: a pinned candidate became ineligible mid-test") { + e.observeQueueExit(pr, "ineligible") + e.logger.Info("bisection root torn down after a candidate became ineligible", "pr", pr, "reason", reason) + return + } remaining := removeNum(numbersOf(a.prs), pr) e.cleanupBatch(ctx, a) e.enqueueWithState("requeued after another PR became ineligible", remaining) @@ -849,7 +1033,7 @@ func (e *Engine) land(ctx context.Context, a *activeBatch) (resolved bool, merge e.logger.Warn("PR bounced: auto-merge repeatedly cancelled", "pr", staged.Number, "head", short(staged.Head.Sha)) e.requeueActiveRemainder("requeued after an earlier PR failed to merge", a.prs[1:]) e.cleanupBatch(ctx, a) - e.bounce(ctx, staged.Number, staged.Head.Sha, "auto-merge was repeatedly cancelled before the merge completed", "error", a.debugURL) + e.bounceFrom(ctx, a.runID, a.stagingBranch, staged.Number, staged.Head.Sha, "auto-merge was repeatedly cancelled before the merge completed", "error", a.debugURL) return true, merged, nil } e.skipLand(ctx, staged, current, "auto-merge is no longer scheduled", len(a.prs) > 1, a.debugURL, true) @@ -885,7 +1069,7 @@ func (e *Engine) land(ctx context.Context, a *activeBatch) (resolved bool, merge e.logger.Warn("PR bounced: auto-merge repeatedly cancelled", "pr", staged.Number, "head", short(staged.Head.Sha)) e.requeueActiveRemainder("requeued after an earlier PR failed to merge", a.prs[1:]) e.cleanupBatch(ctx, a) - e.bounce(ctx, staged.Number, staged.Head.Sha, "auto-merge was repeatedly cancelled before the merge completed", "error", a.debugURL) + e.bounceFrom(ctx, a.runID, a.stagingBranch, staged.Number, staged.Head.Sha, "auto-merge was repeatedly cancelled before the merge completed", "error", a.debugURL) return true, merged, nil } e.skipLand(ctx, staged, current, "auto-merge is no longer scheduled", len(a.prs) > 1, a.debugURL, true) @@ -914,7 +1098,7 @@ func (e *Engine) land(ctx context.Context, a *activeBatch) (resolved bool, merge e.logger.Warn("PR bounced: auto-merge repeatedly cancelled during recovery", "pr", staged.Number, "head", short(staged.Head.Sha)) e.requeueActiveRemainder("requeued after an earlier PR failed to merge", a.prs[1:]) e.cleanupBatch(ctx, a) - e.bounce(ctx, staged.Number, staged.Head.Sha, "auto-merge was repeatedly cancelled before the merge completed", "error", a.debugURL) + e.bounceFrom(ctx, a.runID, a.stagingBranch, staged.Number, staged.Head.Sha, "auto-merge was repeatedly cancelled before the merge completed", "error", a.debugURL) return true, merged, nil } e.logger.Info("PR skipped during native merge recovery", "pr", staged.Number, "reason", "auto-merge is no longer scheduled") @@ -943,7 +1127,7 @@ func (e *Engine) land(ctx context.Context, a *activeBatch) (resolved bool, merge e.logger.Warn("PR bounced: native merge repeatedly did not complete", "pr", staged.Number, "head", short(staged.Head.Sha)) e.requeueActiveRemainder("requeued after an earlier PR failed to merge", a.prs[1:]) e.cleanupBatch(ctx, a) - e.bounce(ctx, staged.Number, staged.Head.Sha, + e.bounceFrom(ctx, a.runID, a.stagingBranch, staged.Number, staged.Head.Sha, "the forge did not complete its scheduled merge after repeated attempts", "error", a.debugURL) return true, merged, nil } @@ -997,7 +1181,7 @@ func (e *Engine) land(ctx context.Context, a *activeBatch) (resolved bool, merge e.logger.Warn("PR bounced: auto-merge repeatedly cancelled", "pr", staged.Number, "head", short(staged.Head.Sha)) e.requeueActiveRemainder("requeued after an earlier PR failed to merge", a.prs[1:]) e.cleanupBatch(ctx, a) - e.bounce(ctx, staged.Number, staged.Head.Sha, "auto-merge was repeatedly cancelled before the merge completed", "error", a.debugURL) + e.bounceFrom(ctx, a.runID, a.stagingBranch, staged.Number, staged.Head.Sha, "auto-merge was repeatedly cancelled before the merge completed", "error", a.debugURL) return true, merged, nil } e.skipLand(ctx, staged, current, "auto-merge is no longer scheduled", len(a.prs) > 1, a.debugURL, true) @@ -1135,6 +1319,11 @@ func (e *Engine) recordLanded(ctx context.Context, a *activeBatch, staged forge. true, ) e.logger.Info("PR merged", "pr", staged.Number) + e.recordTransition(Transition{ + Kind: "landed", PRs: []int{staged.Number}, StagingBranch: a.stagingBranch, + RunID: a.runID, LineagePath: a.lineagePath, + EventID: terminalEventID(a.runID, "landed", []int{staged.Number}), + }) } func (e *Engine) releasedByShunt(ctx context.Context, a *activeBatch, staged forge.PullRequest) (bool, error) { @@ -1162,7 +1351,7 @@ func (e *Engine) handleStagingConflict(ctx context.Context, prs []forge.PullRequ return fmt.Errorf("stager reported conflict on PR #%d outside candidate %v", conflictPR, nums) } if idx == 0 { - e.bounce(ctx, conflictPR, prs[idx].Head.Sha, "merge conflict while staging the PR", "error", "") + e.bounce(ctx, "", conflictPR, prs[idx].Head.Sha, "merge conflict while staging the PR", "error", "") if len(nums) > 1 { rest := append([]int(nil), nums[1:]...) e.enqueueWithState("retrying after staging conflict; testing a smaller batch", rest) @@ -1193,20 +1382,178 @@ func (e *Engine) skipLand(ctx context.Context, staged, current forge.PullRequest // bisectOrBounce: a size-1 failing batch bounces the culprit; a larger batch is // split in half, with the first half tested next (the good half lands, the // recursion isolates the bad PR(s)). +// holdTreeLeaf moves a terminal-gate node of a bisection tree out of the +// active list without publishing a source-PR decision; the decision is made +// in queue order by finalizeReadyTree once every descendant is terminal. +func (e *Engine) holdTreeLeaf(ctx context.Context, a *activeBatch, outcome string) bool { + tree, ok := e.trees[a.runID] + if !ok { + return false + } + e.cleanupBatch(ctx, a) + if outcome == "success" { + tree.accepted = append(tree.accepted, a.prs...) + } + tree.held = append(tree.held, heldLeaf{batch: a, outcome: outcome}) + sort.Slice(tree.held, func(i, j int) bool { + return tree.held[i].batch.prs[0].Number < tree.held[j].batch.prs[0].Number + }) + e.recordTransition(Transition{ + Kind: "held", PRs: numbersOf(a.prs), StagingBranch: a.stagingBranch, + RunID: a.runID, LineagePath: a.lineagePath, Reason: outcome, + }) + return true +} + +// treeHasUnresolvedNode reports whether any candidate under runID is still +// awaiting a gate result — staged in e.active, or waiting in e.pending. A tree +// is ready to finalize exactly when this is false. It is derived on every read +// rather than tracked as a counter so that no batch-removal path (a missing +// gate, a requeue, a supersede) can strand a root by forgetting to decrement. +func (e *Engine) treeHasUnresolvedNode(runID string) bool { + return e.unresolvedNodeCount(runID) > 0 +} + +func (e *Engine) unresolvedNodeCount(runID string) int { + n := 0 + for _, a := range e.active { + if a.runID == runID { + n++ + } + } + for _, node := range e.pending { + if len(node) > 0 && e.lineageRunID[node[0]] == runID { + n++ + } + } + return n +} + +// finalizeReadyTree performs one ordered source-PR decision only after every +// descendant has produced terminal gate evidence. +func (e *Engine) finalizeReadyTree(ctx context.Context) (bool, error) { + for runID, tree := range e.trees { + if e.treeHasUnresolvedNode(runID) { + continue + } + if tree.cursor == len(tree.held) { + delete(e.trees, runID) + delete(e.rootAnchor, runID) + return true, nil + } + // An external base advance while a ready root has not yet landed + // anything means every not-yet-performed decision used stale evidence. + // A bounce does not move main, so if we have only bounced so far and + // the anchor no longer matches, someone else landed a change; re-root + // the unperformed suffix on current main. (An external advance + // interleaved with our own landings is a narrower case left for later: + // once a success has landed, main legitimately moved and the + // acknowledged actions are irreversible facts.) + if anchor := e.rootAnchor[runID]; anchor != "" && !treeFinalizedASuccess(tree) { + head, err := e.fc.BranchHead(ctx, e.cfg.Owner, e.cfg.Repo, e.cfg.Base) + if err != nil { + return false, err + } + if head != anchor { + e.reRootFinalizationSuffix(ctx, runID, tree) + return true, nil + } + } + leaf := tree.held[tree.cursor] + if leaf.outcome == "success" { + resolved, _, err := e.land(ctx, leaf.batch) + if err != nil { + return false, err + } + if !resolved { + return true, nil + } + } else { + staged := leaf.batch.prs[0] + e.bounceFrom(ctx, leaf.batch.runID, leaf.batch.stagingBranch, staged.Number, staged.Head.Sha, + fmt.Sprintf("merge-queue gate **%s**", leaf.outcome), gateOutcomeStatus(leaf.outcome), leaf.batch.debugURL) + } + tree.cursor++ + return true, nil + } + return false, nil +} + +// treeFinalizedASuccess reports whether any leaf up to the finalization cursor +// was a success (and so legitimately moved the base branch). +func treeFinalizedASuccess(tree *bisectionTree) bool { + for _, leaf := range tree.held[:tree.cursor] { + if leaf.outcome == "success" { + return true + } + } + return false +} + +// reRootFinalizationSuffix handles an external base advance during a ready +// root's finalization before any success has landed: the already-bounced +// leaves stay bounced (irreversible), and every held leaf from the cursor +// onward is re-queued as a fresh root on current main because its evidence +// used the now-stale anchor. +func (e *Engine) reRootFinalizationSuffix(ctx context.Context, runID string, tree *bisectionTree) { + suffix := map[int]bool{} + for _, leaf := range tree.held[tree.cursor:] { + for _, pr := range leaf.batch.prs { + suffix[pr.Number] = true + } + if leaf.batch.stagingBranch != "" { + if err := e.fc.DeleteBranch(ctx, e.cfg.Owner, e.cfg.Repo, leaf.batch.stagingBranch); err != nil { + e.logger.Warn("re-root: failed to delete held staging branch", "branch", leaf.batch.stagingBranch, "error", err) + } + } + e.recordTransition(Transition{Kind: "bisected", StagingBranch: leaf.batch.stagingBranch, RunID: runID, LineagePath: leaf.batch.lineagePath}) + } + nums := make([]int, 0, len(suffix)) + for n := range suffix { + nums = append(nums, n) + } + sort.Ints(nums) + + delete(e.trees, runID) + delete(e.rootAnchor, runID) + e.recordTransition(Transition{ + Kind: "root_invalidated", PRs: nums, RunID: runID, + Reason: "base branch advanced during finalization", + EventID: terminalEventID(runID, "root_invalidated", nums), + }) + e.logger.Warn("root finalization aborted: base advanced before any decision landed; re-queuing the suffix", + "runID", runID, "suffix", nums) + if len(nums) > 0 { + e.enqueueWithState("re-queued: base advanced during finalization", nums) + } +} + func (e *Engine) bisectOrBounce(ctx context.Context, a *activeBatch, status string) (bool, error) { nums := numbersOf(a.prs) e.cleanupBatch(ctx, a) if len(nums) == 1 { - bounced := e.bounce(ctx, nums[0], a.prs[0].Head.Sha, fmt.Sprintf("merge-queue gate **%s**", status), gateOutcomeStatus(status), a.debugURL) + if e.holdTreeLeaf(ctx, a, status) { + return true, nil + } + bounced := e.bounceFrom(ctx, a.runID, a.stagingBranch, nums[0], a.prs[0].Head.Sha, fmt.Sprintf("merge-queue gate **%s**", status), gateOutcomeStatus(status), a.debugURL) return bounced, nil } mid := len(nums) / 2 first := append([]int(nil), nums[:mid]...) second := append([]int(nil), nums[mid:]...) + if _, ok := e.trees[a.runID]; !ok { + e.trees[a.runID] = &bisectionTree{results: map[string]string{a.exactKey: status}} + } e.markBisectOrigins(first[0], second[0]) + e.markLineage(a.runID, a.lineagePath, first, second) e.enqueueWithState("retrying after gate "+status+"; isolating the batch", first, second) - e.logger.Info("batch failed; bisecting", "prs", nums, "status", status, "first", first, "second", second) + e.logger.Info("batch failed; bisecting", "prs", nums, "status", status, "first", first, "second", second, + "stagingBranch", a.stagingBranch, "firstPath", e.lineage[first[0]], "secondPath", e.lineage[second[0]]) + e.recordTransition(Transition{ + Kind: "bisected", PRs: nums, StagingBranch: a.stagingBranch, + RunID: a.runID, LineagePath: a.lineagePath, + }) return true, nil } @@ -1228,7 +1575,14 @@ func gateOutcomeStatus(status string) string { return "error" } -func (e *Engine) bounce(ctx context.Context, num int, expectedHeadSHA, reason, statusState, debugURL string) bool { +func (e *Engine) bounce(ctx context.Context, runID string, num int, expectedHeadSHA, reason, statusState, debugURL string) bool { + return e.bounceFrom(ctx, runID, "", num, expectedHeadSHA, reason, statusState, debugURL) +} + +// bounceFrom is bounce with the batch's staging branch attached to the +// resulting Transition, when the caller has one (every caller does, except +// the missing-gate-retries path which has already torn its batch down). +func (e *Engine) bounceFrom(ctx context.Context, runID, stagingBranch string, num int, expectedHeadSHA, reason, statusState, debugURL string) bool { if pr, err := e.fc.GetPR(ctx, e.cfg.Owner, e.cfg.Repo, num); err == nil && pr.State == "open" && !pr.Merged { if expectedHeadSHA != "" && pr.Head.Sha != expectedHeadSHA { e.enqueueWithState("requeued after PR head changed", []int{num}) @@ -1247,6 +1601,15 @@ func (e *Engine) bounce(ctx context.Context, num int, expectedHeadSHA, reason, s } } e.logger.Info("PR bounced", "pr", num, "reason", reason) + t := Transition{Kind: "bounced", PRs: []int{num}, StagingBranch: stagingBranch, Reason: reason, RunID: runID} + if runID != "" { + // Only a root-scoped bounce gets a stable de-dup id. The rootless + // staging-conflict bounce has no unique key (the same PR can conflict + // in successive attempts), so it stays non-idempotent — acceptable + // because it drives no merge counter. + t.EventID = terminalEventID(runID, "bounced", []int{num}) + } + e.recordTransition(t) return true } @@ -1257,9 +1620,92 @@ func (e *Engine) activeLimit() int { return 1 } -func (e *Engine) stagingBranch() string { +// frontierKeyVersion is bumped whenever the meaning of an exact key changes, +// so a key written by an older engine can never be mistaken for a current one. +// v1 was PR head SHAs only; v2 adds the base anchor and merge style. +const frontierKeyVersion = 2 + +// exactKey is the identity of one gate question: the base anchor, the merge +// style, and the ordered pinned PR heads of (accepted baseline + candidate). +// Different keys have no logical relationship even if their PR sets overlap +// (bisection-tree-finalization.md, "Formal model"). +func (e *Engine) exactKey(anchor string, prs []forge.PullRequest) string { + parts := make([]string, 0, len(prs)+3) + parts = append(parts, "v"+strconv.Itoa(frontierKeyVersion), anchor, e.cfg.MergeStyle) + for _, pr := range prs { + parts = append(parts, pr.Head.Sha) + } + return strings.Join(parts, "\x1f") +} + +// ensureRootAnchor pins, once per root, the base branch's commit SHA. Every +// staged integration and exact key under that root is scoped to it. +func (e *Engine) ensureRootAnchor(ctx context.Context, runID string) (string, error) { + if a := e.rootAnchor[runID]; a != "" { + return a, nil + } + head, err := e.fc.BranchHead(ctx, e.cfg.Owner, e.cfg.Repo, e.cfg.Base) + if err != nil { + return "", fmt.Errorf("read base anchor for %q: %w", e.cfg.Base, err) + } + if e.rootAnchor == nil { + e.rootAnchor = map[string]string{} + } + e.rootAnchor[runID] = head + return head, nil +} + +// stagingBranchFor names a staging branch for a batch at a known point in the +// bisection tree. Callers pass the run this batch belongs to and its path. + +func (e *Engine) stagingBranchFor(runID, path string) string { e.stagingSeq++ - return fmt.Sprintf("%s-%d-%d", e.cfg.StagingBranch, e.now().UnixNano(), e.stagingSeq) + return fmt.Sprintf("%s-%s-%s", e.cfg.StagingBranch, runID, path) +} + +// lineageFor returns the run and path for a candidate about to be staged, +// consuming the entry recorded when it was split out of its parent. A +// candidate with no recorded lineage is a new root: it starts a new run. +func (e *Engine) lineageFor(cand []int) (string, string) { + if len(cand) > 0 { + if path, ok := e.lineage[cand[0]]; ok { + runID := e.lineageRunID[cand[0]] + delete(e.lineage, cand[0]) + delete(e.lineageRunID, cand[0]) + return runID, path + } + } + // The run id must be unique per root attempt, not merely per instant: a + // batch restacked because a PR head changed is a fresh root, and if it + // reused the name the gate status for the previous branch would still be + // attached to it. stagingSeq is monotonic over the engine's lifetime, so + // pairing it with the clock keeps roots distinct even when two are staged + // within the same nanosecond — which is exactly what happens under + // BisectFanout > 1, and in tests with a frozen clock. + e.runID = fmt.Sprintf("%d-%d", e.now().UnixNano(), e.stagingSeq) + return e.runID, "r" +} + +// markLineage records the path each half of a split will stage under, keyed by +// its first PR number so startNext can find it. Called before the halves are +// enqueued. +func (e *Engine) markLineage(runID, parentPath string, halves ...[]int) { + if e.lineage == nil { + e.lineage = map[int]string{} + } + if e.lineageRunID == nil { + e.lineageRunID = map[int]string{} + } + if parentPath == "" { + parentPath = "r" + } + for i, half := range halves { + if len(half) == 0 { + continue + } + e.lineage[half[0]] = fmt.Sprintf("%s%d", parentPath, i) + e.lineageRunID[half[0]] = runID + } } func (e *Engine) enqueue(cands ...[]int) { @@ -1303,6 +1749,23 @@ func (e *Engine) readyToResolve(a *activeBatch) bool { return true } +// requeuedTreeNode intercepts a requeue/eviction that targets a live bisection +// node. Detaching a node from its tree without accounting for it would leave +// the root waiting forever on a slot nothing will fill. A candidate whose +// evidence is now in doubt invalidates the whole immutable root, so tear it +// down and re-queue every candidate as a fresh root. Returns true when it +// handled the batch. +func (e *Engine) requeuedTreeNode(ctx context.Context, a *activeBatch, reason, state string) bool { + if a.runID == "" { + return false + } + if _, ok := e.trees[a.runID]; !ok { + return false + } + e.tearDownAndRequeueRoot(ctx, a.runID, reason, state) + return true +} + func (e *Engine) freeSlotForEarlierPending(ctx context.Context) { if len(e.pending) == 0 || len(e.active) < e.activeLimit() { return @@ -1311,6 +1774,11 @@ func (e *Engine) freeSlotForEarlierPending(ctx context.Context) { idx := -1 latest := -1 for i, a := range e.active { + // A live bisection node must finish; do not evict it for a + // newly-arrived earlier candidate. + if _, inTree := e.trees[a.runID]; inTree && a.runID != "" { + continue + } if first := firstPR(a.prs); first > earliestPending && first > latest { idx = i latest = first @@ -1326,17 +1794,48 @@ func (e *Engine) freeSlotForEarlierPending(ctx context.Context) { } func (e *Engine) requeueStaleActive(ctx context.Context, a *activeBatch) { + if e.requeuedTreeNode(ctx, a, "base branch advanced", "re-queued: base branch advanced mid-test") { + return + } e.cleanupBatch(ctx, a) e.enqueueWithState("requeued after base branch advanced", numbersOf(a.prs)) e.logger.Info("stale speculative batch requeued after base advanced", "prs", numbersOf(a.prs)) } func (e *Engine) requeueChangedActive(ctx context.Context, a *activeBatch) { + if e.requeuedTreeNode(ctx, a, "a pinned candidate changed", "re-queued: a pinned candidate changed mid-test") { + return + } e.cleanupBatch(ctx, a) e.enqueueWithState("requeued after PR head changed", numbersOf(a.prs)) e.logger.Info("active batch requeued after PR head changed", "prs", numbersOf(a.prs)) } +// supersedeSpeculative discards a fanout-staged bisection node whose gate ran +// against an accumulator the resolved frontier no longer matches, and re-queues +// the same node — keeping its run id and lineage path — so it re-stages on the +// correct baseline. tree.open is untouched: the node still exists, unresolved. +// A matching exact-key result already in tree.results is reused by startNext. +func (e *Engine) supersedeSpeculative(ctx context.Context, a *activeBatch) { + first := a.prs[0].Number + e.recordTransition(Transition{ + Kind: "node_superseded", StagingBranch: a.stagingBranch, PRs: numbersOf(a.prs), + RunID: a.runID, LineagePath: a.lineagePath, + }) + e.cleanupBatch(ctx, a) + if e.lineage == nil { + e.lineage = map[int]string{} + } + if e.lineageRunID == nil { + e.lineageRunID = map[int]string{} + } + e.lineage[first] = a.lineagePath + e.lineageRunID[first] = a.runID + e.enqueueWithState("re-staged on the resolved frontier baseline", numbersOf(a.prs)) + e.logger.Info("speculative batch superseded; re-staging on resolved baseline", + "prs", numbersOf(a.prs), "path", a.lineagePath) +} + func (e *Engine) requeueStaleActives(ctx context.Context) { for _, a := range append([]*activeBatch(nil), e.active...) { if a.baseGen != e.baseGen { @@ -1345,6 +1844,300 @@ func (e *Engine) requeueStaleActives(ctx context.Context) { } } +// invalidateAdvancedRoots re-reads the base branch head once per reconcile. +// If it has moved since a still-testing root pinned its anchor, that root's +// evidence was gathered against a base that no longer exists as tested, so the +// whole root is torn down and its candidates are re-queued as one fresh root +// on the new base. The design forbids inheriting any decision prefix across a +// base-anchor change, and no source-PR status, auto-merge release, or +// cancellation is published (bisection-tree-finalization.md, "Base and +// candidate invalidation"). Trees already in finalization (no unresolved node) +// are skipped: their base moves are the engine's own merges. +func (e *Engine) invalidateAdvancedRoots(ctx context.Context) (bool, error) { + testing := map[string]bool{} + for runID := range e.trees { + if e.treeHasUnresolvedNode(runID) { + testing[runID] = true + } + } + for _, a := range e.active { + if a.runID != "" && e.trees[a.runID] == nil { + testing[a.runID] = true + } + } + if len(testing) == 0 { + return false, nil + } + head, err := e.fc.BranchHead(ctx, e.cfg.Owner, e.cfg.Repo, e.cfg.Base) + if err != nil { + return false, err + } + for runID := range testing { + anchor := e.rootAnchor[runID] + if anchor == "" || anchor == head { + continue + } + e.tearDownAndRequeueRoot(ctx, runID, "base branch advanced", + "re-queued: base branch advanced mid-test") + return true, nil // one root per tick; re-observe on the next reconcile + } + return false, nil +} + +// invalidateRootsOnCandidateChange checks every pinned candidate of a +// still-testing root — accepted, held, AND the ones currently staged in an +// active bisection node — for a head change, close, or merge. Any such change +// invalidates the immutable root's evidence; reRootPreservingPrefix carries the +// longest independently evidenced prefix into a successor, or the whole root is +// torn down. Covering the active nodes here is what keeps a mid-test push from +// falling through to requeueChangedActive, which would strand the tree. +// +// The GetPR sweep costs one call per distinct pinned candidate per reconcile. +// That is the design's "revalidate every pinned root candidate on each +// reconcile"; a root with no held leaves and one active node sweeps one PR. +func (e *Engine) invalidateRootsOnCandidateChange(ctx context.Context) (bool, error) { + for runID := range e.trees { + if !e.treeHasUnresolvedNode(runID) { + continue // finalizing: acknowledged actions are irreversible facts + } + tree := e.trees[runID] + pinned := map[int]string{} + for _, pr := range tree.accepted { + pinned[pr.Number] = pr.Head.Sha + } + for _, leaf := range tree.held { + for _, pr := range leaf.batch.prs { + pinned[pr.Number] = pr.Head.Sha + } + } + for _, a := range e.active { + if a.runID == runID { + for _, pr := range a.prs { + pinned[pr.Number] = pr.Head.Sha + } + } + } + // Lowest changed PR number wins: the preserved prefix is everything + // strictly to its left. + changed := 0 + for num, sha := range pinned { + cur, err := e.fc.GetPR(ctx, e.cfg.Owner, e.cfg.Repo, num) + if err != nil { + return false, err + } + if cur.State != "open" || cur.Merged || cur.Head.Sha != sha { + if changed == 0 || num < changed { + changed = num + } + } + } + if changed == 0 { + continue + } + e.logger.Warn("root invalidated: a pinned candidate changed", + "runID", runID, "pr", changed) + if !e.reRootPreservingPrefix(ctx, runID, changed) { + e.tearDownAndRequeueRoot(ctx, runID, "a pinned candidate changed", + "re-queued: a pinned candidate changed mid-test") + } + return true, nil + } + return false, nil +} + +// reRootPreservingPrefix handles a pinned-candidate change by carrying the +// longest queue-order prefix of held decisions that is provably independent of +// the changed candidate into a successor root, and re-resolving only the suffix +// from the changed candidate onward. A held leaf is independent iff every PR in +// it is strictly left of `changed` — a successful group is indivisible, so a +// change inside one discards the whole group. Returns false when nothing can be +// preserved, leaving the caller to tear the whole root down. +// +// The successor keeps the predecessor's base anchor (a candidate change is not +// a base change) and its own held/accepted evidence, but starts a fresh +// outcome cache and owns finalization for the whole resulting queue. Inherited +// leaves keep their original run id for audit. +func (e *Engine) reRootPreservingPrefix(ctx context.Context, oldRunID string, changed int) bool { + tree := e.trees[oldRunID] + if tree == nil { + return false + } + maxPR := func(prs []forge.PullRequest) int { + m := 0 + for _, pr := range prs { + if pr.Number > m { + m = pr.Number + } + } + return m + } + + var preserved []heldLeaf + for _, leaf := range tree.held { // tree.held is kept sorted by first PR + if maxPR(leaf.batch.prs) < changed { + preserved = append(preserved, leaf) + continue + } + break + } + if len(preserved) == 0 { + return false + } + + // Suffix: every still-pinned PR from the changed candidate onward, plus + // active batches and pending nodes of this root. resolve() drops the ones + // that are no longer eligible (including a withdrawn `changed`). + suffix := map[int]bool{} + var preservedAccepted []forge.PullRequest + for _, leaf := range preserved { + if leaf.outcome == "success" { + preservedAccepted = append(preservedAccepted, leaf.batch.prs...) + } + } + for _, leaf := range tree.held[len(preserved):] { + for _, pr := range leaf.batch.prs { + suffix[pr.Number] = true + } + if leaf.batch.stagingBranch != "" { + if err := e.fc.DeleteBranch(ctx, e.cfg.Owner, e.cfg.Repo, leaf.batch.stagingBranch); err != nil { + e.logger.Warn("re-root: failed to delete discarded held branch", "branch", leaf.batch.stagingBranch, "error", err) + } + } + e.recordTransition(Transition{Kind: "bisected", StagingBranch: leaf.batch.stagingBranch, RunID: oldRunID, LineagePath: leaf.batch.lineagePath}) + } + for _, a := range append([]*activeBatch(nil), e.active...) { + if a.runID == oldRunID { + for _, pr := range a.prs { + suffix[pr.Number] = true + } + e.recordTransition(Transition{Kind: "bisected", StagingBranch: a.stagingBranch, RunID: oldRunID, LineagePath: a.lineagePath}) + e.cleanupBatch(ctx, a) + } + } + var keptPending [][]int + for _, node := range e.pending { + if len(node) > 0 && e.lineageRunID[node[0]] == oldRunID { + for _, n := range node { + suffix[n] = true + } + delete(e.lineage, node[0]) + delete(e.lineageRunID, node[0]) + delete(e.bisectOrigins, node[0]) + continue + } + keptPending = append(keptPending, node) + } + e.pending = keptPending + + suffixNums := make([]int, 0, len(suffix)) + for n := range suffix { + suffixNums = append(suffixNums, n) + } + sort.Ints(suffixNums) + + anchor := e.rootAnchor[oldRunID] // a candidate change does not move the base + newRunID := fmt.Sprintf("%d-%d-s", e.now().UnixNano(), e.stagingSeq) + delete(e.trees, oldRunID) + delete(e.rootAnchor, oldRunID) + e.trees[newRunID] = &bisectionTree{ + accepted: preservedAccepted, + held: preserved, + results: map[string]string{}, + } + e.rootAnchor[newRunID] = anchor + + if e.lineage == nil { + e.lineage = map[int]string{} + } + if e.lineageRunID == nil { + e.lineageRunID = map[int]string{} + } + if len(suffixNums) > 0 { + e.lineage[suffixNums[0]] = "r" + e.lineageRunID[suffixNums[0]] = newRunID + e.enqueueWithState("re-resolved suffix after a pinned candidate changed", suffixNums) + } + // With no suffix the successor has no unresolved node, so + // treeHasUnresolvedNode reports it ready and finalizeReadyTree runs it out + // on the next pass. + + e.recordTransition(Transition{ + Kind: "root_invalidated", PRs: suffixNums, RunID: oldRunID, + Reason: "a pinned candidate changed; prefix preserved", + EventID: terminalEventID(oldRunID, "root_invalidated", suffixNums), + }) + e.logger.Info("root re-rooted with preserved prefix", + "oldRunID", oldRunID, "newRunID", newRunID, + "preservedLeaves", len(preserved), "suffix", suffixNums) + return true +} + +func (e *Engine) tearDownAndRequeueRoot(ctx context.Context, runID, reason, requeueState string) { + var nums []int + seen := map[int]bool{} + add := func(ns ...int) { + for _, n := range ns { + if !seen[n] { + seen[n] = true + nums = append(nums, n) + } + } + } + + var toClean []*activeBatch + for _, a := range e.active { + if a.runID == runID { + toClean = append(toClean, a) + add(numbersOf(a.prs)...) + } + } + if tree, ok := e.trees[runID]; ok { + for _, pr := range tree.accepted { + add(pr.Number) + } + for _, leaf := range tree.held { + add(numbersOf(leaf.batch.prs)...) + e.recordTransition(Transition{Kind: "bisected", StagingBranch: leaf.batch.stagingBranch, RunID: runID, LineagePath: leaf.batch.lineagePath}) + if leaf.batch.stagingBranch != "" { + if err := e.fc.DeleteBranch(ctx, e.cfg.Owner, e.cfg.Repo, leaf.batch.stagingBranch); err != nil { + e.logger.Warn("re-root: failed to delete held staging branch", "branch", leaf.batch.stagingBranch, "error", err) + } + } + } + delete(e.trees, runID) + } + var keptPending [][]int + for _, node := range e.pending { + if len(node) > 0 && e.lineageRunID[node[0]] == runID { + add(node...) + delete(e.lineage, node[0]) + delete(e.lineageRunID, node[0]) + delete(e.bisectOrigins, node[0]) + continue + } + keptPending = append(keptPending, node) + } + e.pending = keptPending + + for _, a := range toClean { + e.recordTransition(Transition{Kind: "bisected", StagingBranch: a.stagingBranch, RunID: runID, LineagePath: a.lineagePath}) + e.cleanupBatch(ctx, a) + } + + delete(e.rootAnchor, runID) + + sort.Ints(nums) + e.logger.Warn("root invalidated; re-queuing its candidates as a fresh root", + "runID", runID, "reason", reason, "candidates", nums) + e.recordTransition(Transition{ + Kind: "root_invalidated", PRs: nums, RunID: runID, Reason: reason, + EventID: terminalEventID(runID, "root_invalidated", nums), + }) + if len(nums) > 0 { + e.enqueueWithState(requeueState, nums) + } +} + func (e *Engine) removeActive(a *activeBatch) { for i, candidate := range e.active { if candidate == a { @@ -1358,6 +2151,9 @@ func (e *Engine) removeActive(a *activeBatch) { // branch. Called on every path that finishes with a batch (land, skip, bounce). func (e *Engine) cleanupBatch(ctx context.Context, a *activeBatch) { e.removeActive(a) + if a.stagingBranch == "" { + return + } if err := e.fc.DeleteBranch(ctx, e.cfg.Owner, e.cfg.Repo, a.stagingBranch); err != nil { e.logger.Warn("failed to delete staging branch", "branch", a.stagingBranch, "error", err) diff --git a/internal/engine/engine_test.go b/internal/engine/engine_test.go index 89b2475..89bd395 100644 --- a/internal/engine/engine_test.go +++ b/internal/engine/engine_test.go @@ -69,6 +69,14 @@ type mock struct { listErr error runStatusErr error upsertErr error + branchHead string // base-branch head SHA the mock reports (default "anchor-") + branchHeads map[string]string // per-branch override, checked before branchHead + stagedAnchors []string // baseAnchor passed to BuildStaging, per call + // gateOracle, when set, decides a batch's gate outcome from its exact + // ordered PR set (base anchor + accepted baseline + candidate, as the + // engine stages it). It lets a test express an arbitrary interacting CI + // oracle, unlike badPR which is a single candidate-independent failure. + gateOracle func(prNums []int) string } func newMock(badPR int, prNums ...int) *mock { @@ -120,6 +128,15 @@ func (m *mock) GetPR(_ context.Context, _, _ string, n int) (forge.PullRequest, } return *m.prs[n], nil } +func (m *mock) BranchHead(_ context.Context, _, _, branch string) (string, error) { + if m.branchHeads[branch] != "" { + return m.branchHeads[branch], nil + } + if m.branchHead != "" { + return m.branchHead, nil + } + return "anchor-" + branch, nil +} func (m *mock) AutomergeState(_ context.Context, _, _ string, n int) (forge.AutomergeState, error) { if m.beforeAutomergeState != nil { m.beforeAutomergeState(n) @@ -200,6 +217,9 @@ func (m *mock) RunStatus(_ context.Context, _, _, sha, _ string) (string, error) if m.runStatusSet || m.runStatus != "" { return m.runStatus, nil } + if m.gateOracle != nil { + return m.gateOracle(m.batchOf[sha]), nil + } for _, n := range m.batchOf[sha] { if n == m.badPR { return "failure", nil @@ -280,13 +300,14 @@ func (m *mock) advanceNative() { } } -func (m *mock) BuildStaging(_ context.Context, _, stagingBranch string, refs []gitops.MergedRef) (string, int, error) { +func (m *mock) BuildStaging(_ context.Context, _, baseAnchor, stagingBranch string, refs []gitops.MergedRef) (string, int, error) { var nums []int for _, r := range refs { nums = append(nums, r.PR) } m.staged = append(m.staged, append([]int(nil), nums...)) m.stagingBranches = append(m.stagingBranches, stagingBranch) + m.stagedAnchors = append(m.stagedAnchors, baseAnchor) baseMerged := m.conflictBasePR > 0 && m.prs[m.conflictBasePR].Merged if idx := indexOfNum(nums, m.conflictPR); idx > 0 || (idx == 0 && (m.conflictFirst || baseMerged)) { return "", m.conflictPR, fmt.Errorf("staging conflict") @@ -724,6 +745,578 @@ func TestBatchLingerResetsAfterBatchStarts(t *testing.T) { } // A 4-PR batch with one bad PR must land the 3 good PRs and isolate the bad one. +func TestBisectionHoldsFailedLeafUntilSiblingTerminal(t *testing.T) { + m := newMock(1, 1, 2) + e := New(Config{Owner: "o", Repo: "r", Base: "main", StatusCtx: "merge-queue", StagingBranch: "mq/main/staging"}, m, m) + + // Stage root, split its failed gate, stage left leaf, then observe its failure. + for range 4 { + if err := e.Reconcile(context.Background()); err != nil { + t.Fatal(err) + } + } + if !m.automerge[1] { + t.Fatal("failed leaf mutated Forge before sibling leaf became terminal") + } + if len(m.statuses) != 0 { + t.Fatalf("source statuses = %v, want none before root terminal", m.statuses) + } +} + +func TestBisectionStagesRightLeafOnAcceptedLeftBaseline(t *testing.T) { + m := newMock(2, 1, 2) + e := New(Config{Owner: "o", Repo: "r", Base: "main", StatusCtx: "merge-queue", StagingBranch: "mq/main/staging"}, m, m) + for range 5 { + if err := e.Reconcile(context.Background()); err != nil { + t.Fatal(err) + } + } + if got := fmt.Sprint(m.staged); got != "[[1 2] [1]]" { + t.Fatalf("staged refs = %s, want root and left only; right reuses root exact key", got) + } +} + +func TestBisectionRestagesRightAfterRejectedLeft(t *testing.T) { + m := newMock(2, 1, 2, 3) + e := New(Config{Owner: "o", Repo: "r", Base: "main", StatusCtx: "merge-queue", StagingBranch: "mq/main/staging"}, m, m) + for range 9 { + if err := e.Reconcile(context.Background()); err != nil { + t.Fatal(err) + } + } + if got := fmt.Sprint(m.staged); got != "[[1 2 3] [1] [1 2] [1 3]]" { + t.Fatalf("staged refs = %s, want cumulative right frontier", got) + } +} + +// TestEngineFrontierWorkedExample drives the real engine through the A..G +// scenario from bisection-tree-finalization.md and asserts its trace matches +// the reference model: ABC fails, A passes, the A+BC key is a cache hit (no +// re-stage), B and C each fail on the A baseline, DEFG passes on the A +// baseline. Six distinct staging calls; accept A,D,E,F,G; reject B,C; every +// source decision deferred until the whole root is terminal, then applied in +// queue order. +func TestEngineFrontierWorkedExample(t *testing.T) { + m := newMock(-1, 1, 2, 3, 4, 5, 6, 7) + pass := map[string]bool{ + "[1]": true, "[1 4 5 6 7]": true, + } + m.gateOracle = func(prNums []int) string { + if pass[fmt.Sprint(prNums)] { + return "success" + } + return "failure" + } + e := New(Config{Owner: "o", Repo: "r", Base: "main", StatusCtx: "merge-queue", StagingBranch: "mq/main/staging"}, m, m) + drive(e, 40) + + if got := fmt.Sprint(m.staged); got != "[[1 2 3 4 5 6 7] [1 2 3] [1] [1 2] [1 3] [1 4 5 6 7]]" { + t.Fatalf("staged = %s\nwant root, ABC, A, A+B, A+C, A+DEFG (ABC reused for A+BC)", got) + } + sort.Ints(m.merged) + if got := fmt.Sprint(m.merged); got != "[1 4 5 6 7]" { + t.Fatalf("merged = %s, want A,D,E,F,G", got) + } + if !m.bounced[2] || !m.bounced[3] { + t.Fatalf("bounced = %v, want B and C rejected", m.bounced) + } +} + +// TestFrontierExactKeyScopedToAnchorAndMergeStyle proves an exact key is not a +// bare list of PR heads: the base anchor and merge style are part of it, so a +// cached outcome can never be reused across a re-rooted queue or a config +// change (bisection-tree-finalization.md, "Formal model"). +func TestFrontierExactKeyScopedToAnchorAndMergeStyle(t *testing.T) { + prs := []forge.PullRequest{{Number: 1}, {Number: 2}} + prs[0].Head.Sha = "h1" + prs[1].Head.Sha = "h2" + + a := New(Config{Owner: "o", Repo: "r", Base: "main", MergeStyle: "squash"}, newMock(-1), newMock(-1)) + b := New(Config{Owner: "o", Repo: "r", Base: "main", MergeStyle: "rebase"}, newMock(-1), newMock(-1)) + + if a.exactKey("anchorX", prs) == a.exactKey("anchorY", prs) { + t.Fatal("exact key ignored the base anchor") + } + if a.exactKey("anchorX", prs) == b.exactKey("anchorX", prs) { + t.Fatal("exact key ignored the merge style") + } + if !strings.Contains(a.exactKey("anchorX", prs), "h1") || !strings.Contains(a.exactKey("anchorX", prs), "h2") { + t.Fatal("exact key dropped a pinned PR head") + } +} + +// TestBisectionAnchorPinnedAndDurable checks the engine reads the base anchor +// once when a root opens, keys every node under it, and restores it verbatim +// on restart so the exact-outcome cache still matches. +func TestBisectionAnchorPinnedAndDurable(t *testing.T) { + m := newMock(1, 1, 2) + m.branchHead = "deadbeefanchor" + store := &memoryCheckpointStore{} + cfg := Config{Owner: "o", Repo: "r", Base: "main", StatusCtx: "merge-queue", StagingBranch: "mq/main/staging", Checkpoint: store} + e := New(cfg, m, m) + for range 4 { + if err := e.Reconcile(context.Background()); err != nil { + t.Fatal(err) + } + } + if len(store.saved.Trees) != 1 || store.saved.Trees[0].Anchor != "deadbeefanchor" { + t.Fatalf("tree anchor = %#v, want the pinned base head", store.saved.Trees) + } + for i, a := range m.stagedAnchors { + if a != "deadbeefanchor" { + t.Fatalf("staging call %d built on %q, want the pinned anchor", i, a) + } + } + for key := range store.saved.Trees[0].Results { + if !strings.Contains(key, "deadbeefanchor") { + t.Fatalf("cached key %q is not scoped to the anchor", key) + } + } + + restarted := New(cfg, m, m) + if err := restarted.Reconcile(context.Background()); err != nil { + t.Fatal(err) + } + if got := restarted.rootAnchor[store.saved.Trees[0].RunID]; got != "deadbeefanchor" { + t.Fatalf("restored rootAnchor = %q, want deadbeefanchor", got) + } +} + +// TestRootInvalidatedWhenBaseAdvancesMidTest: an external advance of the base +// branch during testing tears the whole root down with no source-PR decision +// and re-queues its candidates as one fresh root on the new base +// (bisection-tree-finalization.md, "Base and candidate invalidation"). +// TestFinalizationAbortsWhenBaseAdvancesBeforeAnyLanding: a ready root that has +// only bounced so far (bounces do not move main) sees the base advance under +// it; the already-bounced PRs stay bounced and the unperformed suffix is +// re-queued on current main (bisection-tree-finalization.md, "Base and +// candidate invalidation"). +func TestFinalizationAbortsWhenBaseAdvancesBeforeAnyLanding(t *testing.T) { + m := newMock(-1, 1, 2, 3) + m.branchHead = "base-v1" + // 1 and 2 fail; 3 passes on its own. + m.gateOracle = func(prNums []int) string { + if len(prNums) == 1 && prNums[0] == 3 { + return "success" + } + return "failure" + } + e := New(Config{Owner: "o", Repo: "r", Base: "main", StatusCtx: "merge-queue", StagingBranch: "mq/main/staging"}, m, m) + + // Drive until the tree is ready ([1]✗ [2]✗ [3]✓, no unresolved node) and + // finalization has bounced both failures but not yet landed 3. + var tree *bisectionTree + var ready bool + for i := 0; i < 40; i++ { + _ = e.Reconcile(context.Background()) + m.advanceNative() + tree, ready = nil, false + for id, tr := range e.trees { + tree = tr + ready = !e.treeHasUnresolvedNode(id) + } + if tree != nil && ready && tree.cursor == 2 { + break + } + } + if tree == nil || !ready || tree.cursor != 2 { + t.Fatalf("setup: want a ready tree with 2 bounced leaves, got %#v", tree) + } + if !m.bounced[1] || !m.bounced[2] { + t.Fatalf("PRs 1 and 2 should already be bounced: %v", m.bounced) + } + + m.branchHead = "base-v2" // someone lands a change out-of-band + + if err := e.Reconcile(context.Background()); err != nil { + t.Fatal(err) + } + if len(e.trees) != 0 { + t.Fatalf("tree survived a base advance during finalization: %#v", e.trees) + } + if len(m.merged) != 0 { + t.Fatalf("PR 3 merged on stale evidence: %v", m.merged) + } + sawAbort := false + for _, tr := range e.Transitions() { + if tr.Kind == "root_invalidated" && tr.Reason == "base branch advanced during finalization" { + sawAbort = true + } + } + if !sawAbort { + t.Fatal("no finalization-abort transition emitted") + } + + // PR 3 re-resolves on the new base and lands. + for i := 0; i < 20; i++ { + _ = e.Reconcile(context.Background()) + m.advanceNative() + } + if got := fmt.Sprint(m.merged); got != "[3]" { + t.Fatalf("merged = %s, want [3] after re-resolving the suffix", got) + } +} + +func TestRootInvalidatedWhenBaseAdvancesMidTest(t *testing.T) { + m := newMock(2, 1, 2, 3) + m.branchHead = "base-v1" + e := New(Config{Owner: "o", Repo: "r", Base: "main", StatusCtx: "merge-queue", StagingBranch: "mq/main/staging"}, m, m) + + // Root [1 2 3] fails its gate and bisects; the left leaf [1] stages. + for range 3 { + if err := e.Reconcile(context.Background()); err != nil { + t.Fatal(err) + } + } + if len(e.trees) != 1 { + t.Fatalf("expected one bisection tree mid-test, got %d", len(e.trees)) + } + stagedBefore := len(m.staged) + + m.branchHead = "base-v2" // main advances underneath the queue + if err := e.Reconcile(context.Background()); err != nil { + t.Fatal(err) + } + + if len(e.trees) != 0 { + t.Fatalf("tree survived a base advance: %#v", e.trees) + } + if len(m.statuses) != 0 { + t.Fatalf("source gate statuses written during invalidation: %v", m.statuses) + } + for _, c := range m.calls { + if strings.HasPrefix(c, "cancel:") { + t.Fatalf("auto-merge cancelled during invalidation: %s", c) + } + } + if len(m.bounced) != 0 { + t.Fatalf("PRs bounced during invalidation: %v", m.bounced) + } + invalidated := false + for _, tr := range e.Transitions() { + if tr.Kind == "root_invalidated" { + invalidated = true + } + } + if !invalidated { + t.Fatal("no root_invalidated transition emitted") + } + + // The candidates re-stage as one fresh root on the new base. + for range 3 { + if err := e.Reconcile(context.Background()); err != nil { + t.Fatal(err) + } + } + if len(m.staged) <= stagedBefore { + t.Fatal("candidates were not re-staged after re-rooting") + } + if got := m.stagedAnchors[len(m.stagedAnchors)-1]; got != "base-v2" { + t.Fatalf("re-rooted staging built on %q, want base-v2", got) + } + for id, a := range e.rootAnchor { + if a != "base-v2" { + t.Fatalf("rootAnchor[%s] = %q, want the new base head", id, a) + } + } +} + +// TestSpeculativeFanoutResultSupersededWhenAccumulatorChanges: with fanout the +// right subtree stages speculatively on an empty accumulator. When the left +// subtree accepts a PR, the speculative gate result is against the wrong key +// and must not be trusted — the node is re-staged on the real baseline. Here +// main+3+4 passes but main+2+3+4 fails (an interaction), so trusting the +// speculative pass would wrongly merge PR 4. +func TestSpeculativeFanoutResultSupersededWhenAccumulatorChanges(t *testing.T) { + m := newMock(-1, 1, 2, 3, 4) + pass := map[string]bool{"[2]": true, "[3 4]": true, "[2 3]": true} + m.gateOracle = func(prNums []int) string { + if pass[fmt.Sprint(prNums)] { + return "success" + } + return "failure" + } + e := New(Config{Owner: "o", Repo: "r", Base: "main", StatusCtx: "merge-queue", StagingBranch: "mq/main/staging", BisectFanout: 2}, m, m) + drive(e, 60) + + sort.Ints(m.merged) + if got := fmt.Sprint(m.merged); got != "[2 3]" { + t.Fatalf("merged = %s, want [2 3] (speculative main+3+4 pass must not merge PR 4)", got) + } + if !m.bounced[1] || !m.bounced[4] { + t.Fatalf("bounced = %v, want PRs 1 and 4 rejected", m.bounced) + } + var spec, authoritative bool + for _, s := range m.staged { + switch fmt.Sprint(s) { + case "[3 4]": + spec = true // speculative: empty accumulator + case "[2 3 4]": + authoritative = true // re-staged on the resolved [2] baseline + } + } + if !spec || !authoritative { + t.Fatalf("staged = %v; want both the speculative [3 4] and the re-staged [2 3 4]", m.staged) + } +} + +// TestRootInvalidatedWhenAcceptedCandidateHeadChanges: a PR that was already +// held-success (in the accumulator, not currently active) gets a new head +// mid-test. Its evidence — and every later key built on it — is now stale, so +// the whole root is torn down and re-queued with nothing merged. +func TestRootInvalidatedWhenAcceptedCandidateHeadChanges(t *testing.T) { + m := newMock(2, 1, 2, 3) + e := New(Config{Owner: "o", Repo: "r", Base: "main", StatusCtx: "merge-queue", StagingBranch: "mq/main/staging"}, m, m) + + // Root [1 2 3] fails, splits, left leaf [1] passes and is held-success. + for range 4 { + if err := e.Reconcile(context.Background()); err != nil { + t.Fatal(err) + } + } + var tree *bisectionTree + for _, tr := range e.trees { + tree = tr + } + if tree == nil || len(tree.accepted) != 1 || tree.accepted[0].Number != 1 { + t.Fatalf("expected PR 1 held-success in the accumulator, got %#v", e.trees) + } + + m.prs[1].Head.Sha = "head-1-v2" // PR 1 is force-pushed while held + + if err := e.Reconcile(context.Background()); err != nil { + t.Fatal(err) + } + if len(e.trees) != 0 { + t.Fatalf("tree survived an accepted-candidate head change: %#v", e.trees) + } + if len(m.merged) != 0 { + t.Fatalf("merged %v on stale evidence", m.merged) + } + got := "" + for _, tr := range e.Transitions() { + if tr.Kind == "root_invalidated" { + got = tr.Reason + } + } + if got == "" { + t.Fatal("no root_invalidated transition for the candidate change") + } +} + +// TestSuccessorRootPreservesEvidencedPrefix: when a held candidate that is not +// the first changes, the successor root carries the held decisions strictly to +// its left and re-resolves only the suffix (bisection-tree-finalization.md, +// "Successor roots and preserved prefixes"). +func TestSuccessorRootPreservesEvidencedPrefix(t *testing.T) { + m := newMock(-1, 1, 2, 3, 4, 5) + // [1 2] passes as a group; everything with 3, 4, or 5 fails on that + // baseline — until PR 4 gets a new head, after which [1 2]+[4 5] passes. + m.gateOracle = func(prNums []int) string { + s := fmt.Sprint(prNums) + if s == "[1 2]" { + return "success" + } + if s == "[1 2 4 5]" && m.prs[4].Head.Sha == "head-4-v2" { + return "success" + } + return "failure" + } + e := New(Config{Owner: "o", Repo: "r", Base: "main", StatusCtx: "merge-queue", StagingBranch: "mq/main/staging"}, m, m) + + var tree *bisectionTree + for i := 0; i < 40; i++ { + _ = e.Reconcile(context.Background()) + m.advanceNative() + tree = nil + for _, tr := range e.trees { + tree = tr + } + if tree != nil && len(tree.held) == 3 { // [1 2]✓, [3]✗, [4]✗ + break + } + } + if tree == nil || len(tree.held) != 3 { + t.Fatalf("setup: want 3 held leaves, got %#v", tree) + } + oldRunID := "" + for id := range e.trees { + oldRunID = id + } + + m.prs[4].Head.Sha = "head-4-v2" // PR 4 force-pushed while held-failure + + if err := e.Reconcile(context.Background()); err != nil { + t.Fatal(err) + } + + var succ *bisectionTree + for id, tr := range e.trees { + if id == oldRunID { + t.Fatalf("predecessor root %s survived", id) + } + succ = tr + } + if succ == nil { + t.Fatal("no successor root created — the [1 2] and [3] prefix should carry over") + } + if len(succ.held) != 2 { + t.Fatalf("successor preserved held = %+v, want [1 2]✓ and [3]✗", succ.held) + } + if fmt.Sprint(numbersOf(succ.accepted)) != "[1 2]" { + t.Fatalf("successor accepted = %v, want [1 2]", numbersOf(succ.accepted)) + } + if len(m.merged) != 0 { + t.Fatalf("merged %v before the successor finalized", m.merged) + } + + for i := 0; i < 60; i++ { + _ = e.Reconcile(context.Background()) + m.advanceNative() + if len(e.trees) == 0 { + break + } + } + sort.Ints(m.merged) + if got := fmt.Sprint(m.merged); got != "[1 2 4 5]" { + t.Fatalf("merged = %s, want [1 2 4 5] after re-resolving the suffix", got) + } + if !m.bounced[3] { + t.Fatalf("PR 3 should remain bounced, bounced=%v", m.bounced) + } +} + +// TestTerminalTransitionsCarryStableEventID: landed and bounced transitions +// carry a deterministic event_id derived only from the durable run id, the +// action, and the PR — so a redelivered reconcile response applies the +// irreversible side effects exactly once. +func TestTerminalTransitionsCarryStableEventID(t *testing.T) { + if got := terminalEventID("run-7", "landed", []int{3, 1, 2}); got != "run-7|landed|1,2,3" { + t.Fatalf("terminalEventID = %q, want sorted, run-scoped", got) + } + + m := newMock(2, 1, 2, 3) // PR 2 is bad: 1 and 3 land, 2 bounces + e := New(Config{Owner: "o", Repo: "r", Base: "main", StatusCtx: "merge-queue", StagingBranch: "mq/main/staging"}, m, m) + + seen := map[string]int{} + for range 30 { + _ = e.Reconcile(context.Background()) + for _, tr := range e.Transitions() { + if tr.Kind == "landed" || tr.Kind == "bounced" { + if tr.EventID == "" { + t.Fatalf("%s transition for PR %v has no event_id", tr.Kind, tr.PRs) + } + seen[tr.EventID]++ + } + } + m.advanceNative() + } + if len(seen) != 3 { + t.Fatalf("distinct terminal event ids = %d (%v), want 3 (land 1, land 3, bounce 2)", len(seen), seen) + } +} + +// TestTransitionOutboxSurvivesRestart: a terminal transition that has not yet +// exhausted its retransmits is written to the checkpoint and re-emitted by a +// freshly restarted engine, so a reconcile response dropped right before a +// crash still reaches the consumer. +func TestTransitionOutboxSurvivesRestart(t *testing.T) { + m := newMock(-1, 1) // PR 1 lands cleanly + store := &memoryCheckpointStore{} + cfg := Config{Owner: "o", Repo: "r", Base: "main", StatusCtx: "merge-queue", StagingBranch: "mq/main/staging", Checkpoint: store} + e := New(cfg, m, m) + + var landedSeen bool + for i := 0; i < 8; i++ { + _ = e.Reconcile(context.Background()) + for _, tr := range e.Transitions() { + if tr.Kind == "landed" { + landedSeen = true + } + } + m.advanceNative() + if landedSeen && len(store.saved.TransitionOutbox) > 0 { + break + } + } + if len(m.merged) != 1 { + t.Fatalf("PR 1 did not land: merged=%v", m.merged) + } + if len(store.saved.TransitionOutbox) == 0 { + t.Fatal("landed transition was not persisted to the outbox") + } + + restarted := New(cfg, m, m) + if err := restarted.Reconcile(context.Background()); err != nil { + t.Fatal(err) + } + found := false + for _, tr := range restarted.Transitions() { + if tr.Kind == "landed" && tr.EventID != "" { + found = true + } + } + if !found { + t.Fatal("restarted engine did not re-emit the pending landed transition") + } +} + +// TestAckTransitionsStopsRetransmit: once the consumer acknowledges a terminal +// transition's event id, the engine drops it from the outbox and stops +// re-carrying it instead of running out its full retransmit budget. +func TestAckTransitionsStopsRetransmit(t *testing.T) { + m := newMock(-1, 1) + e := New(Config{Owner: "o", Repo: "r", Base: "main", StatusCtx: "merge-queue", StagingBranch: "mq/main/staging"}, m, m) + + var landedID string + for i := 0; i < 8 && landedID == ""; i++ { + _ = e.Reconcile(context.Background()) + for _, tr := range e.Transitions() { + if tr.Kind == "landed" { + landedID = tr.EventID + } + } + m.advanceNative() + } + if landedID == "" { + t.Fatal("no landed transition observed") + } + + e.AckTransitions([]string{landedID}) + _ = e.Reconcile(context.Background()) + for _, tr := range e.Transitions() { + if tr.EventID == landedID { + t.Fatalf("acked transition %q was still re-sent", landedID) + } + } +} + +func TestCheckpointRestoresHeldBisectionLeaf(t *testing.T) { + m := newMock(1, 1, 2) + store := &memoryCheckpointStore{} + cfg := Config{Owner: "o", Repo: "r", Base: "main", StatusCtx: "merge-queue", StagingBranch: "mq/main/staging", Checkpoint: store} + e := New(cfg, m, m) + for range 4 { + if err := e.Reconcile(context.Background()); err != nil { + t.Fatal(err) + } + } + if store.saved == nil || len(store.saved.Trees) != 1 || len(store.saved.Trees[0].Held) != 1 { + t.Fatalf("checkpoint trees = %#v, want held leaf", store.saved) + } + + restarted := New(cfg, m, m) + if err := restarted.Reconcile(context.Background()); err != nil { + t.Fatal(err) + } + if got := m.stagingBranches[len(m.stagingBranches)-1]; !strings.HasSuffix(got, "-r1") { + t.Fatalf("resumed sibling branch = %q, want r1", got) + } + if !m.automerge[1] { + t.Fatal("restart finalized held leaf before sibling terminal") + } +} + func TestBisectionIsolatesBadPR(t *testing.T) { m := newMock(3, 1, 2, 3, 4) e := New(Config{Owner: "o", Repo: "r", Base: "main", StatusCtx: "merge-queue", StagingBranch: "mq/main/staging"}, m, m) @@ -747,6 +1340,111 @@ func TestBisectionIsolatesBadPR(t *testing.T) { } } +// TestTransitionsRecordFullBisectionLifecycle drives the same fixture as +// TestBisectionIsolatesBadPR (PR 3 fails, gets isolated by bisection, 1/2/4 +// land) but asserts on the structured Transitions a caller would persist, +// instead of the mock's side effects — this is the RFC-0032 write path's +// entire data source, so its shape and completeness matter independently of +// whether the engine's existing behavior (already covered above) is +// otherwise correct. +func TestTransitionsRecordFullBisectionLifecycle(t *testing.T) { + m := newMock(3, 1, 2, 3, 4) + e := New(Config{Owner: "o", Repo: "r", Base: "main", StatusCtx: "merge-queue", StagingBranch: "mq/main/staging"}, m, m) + + var all []Transition + // Terminal transitions (those with an EventID) are re-sent for several + // reconciles so a dropped response cannot lose them; the consumer de-dups + // by EventID, so do the same here. + seenEvent := map[string]bool{} + for i := 0; i < 30; i++ { + _ = e.Reconcile(context.Background()) + for _, tr := range e.Transitions() { + if tr.EventID != "" { + if seenEvent[tr.EventID] { + continue + } + seenEvent[tr.EventID] = true + } + all = append(all, tr) + } + if mk, ok := e.fc.(*mock); ok { + mk.advanceNative() + } + } + + byKind := map[string][]Transition{} + for _, tr := range all { + byKind[tr.Kind] = append(byKind[tr.Kind], tr) + } + + if len(byKind["staged"]) < 2 { + t.Fatalf("want at least 2 'staged' transitions (root batch + a bisected child), got %d: %+v", len(byKind["staged"]), byKind["staged"]) + } + for _, tr := range byKind["staged"] { + if tr.StagingBranch == "" { + t.Errorf("staged transition missing StagingBranch: %+v", tr) + } + if tr.LineagePath == "" { + t.Errorf("staged transition missing LineagePath: %+v", tr) + } + } + + // [1 2 3 4] fails and splits into [1 2] (passes, lands) and [3 4] (fails + // again and splits into [3] bounced, [4] lands) -- two bisections, not + // one, to fully isolate the single bad PR. + if len(byKind["bisected"]) != 2 { + t.Fatalf("want exactly 2 'bisected' transitions, got %d: %+v", len(byKind["bisected"]), byKind["bisected"]) + } + gotPRSets := map[string]bool{} + for _, tr := range byKind["bisected"] { + sort.Ints(tr.PRs) + gotPRSets[fmt.Sprint(tr.PRs)] = true + if tr.StagingBranch == "" || tr.LineagePath == "" { + t.Errorf("bisected transition missing StagingBranch/LineagePath: %+v", tr) + } + } + for _, want := range []string{"[1 2 3 4]", "[3 4]"} { + if !gotPRSets[want] { + t.Errorf("want a bisected transition for PRs %s, got sets %v", want, gotPRSets) + } + } + + if len(byKind["bounced"]) != 1 { + t.Fatalf("want exactly 1 'bounced' transition (PR 3), got %d: %+v", len(byKind["bounced"]), byKind["bounced"]) + } + if got := byKind["bounced"][0]; len(got.PRs) != 1 || got.PRs[0] != 3 { + t.Errorf("bounced transition PRs = %v, want [3]", got.PRs) + } else if got.StagingBranch == "" { + t.Errorf("bounced transition missing StagingBranch: %+v", got) + } else if got.Reason == "" { + t.Errorf("bounced transition missing Reason: %+v", got) + } + + landedPRs := map[int]bool{} + for _, tr := range byKind["landed"] { + if len(tr.PRs) != 1 { + t.Errorf("landed transition should carry exactly one PR, got %+v", tr) + continue + } + landedPRs[tr.PRs[0]] = true + if tr.StagingBranch == "" { + t.Errorf("landed transition missing StagingBranch: %+v", tr) + } + } + for _, pr := range []int{1, 2, 4} { + if !landedPRs[pr] { + t.Errorf("want a 'landed' transition for PR %d, got landed=%v", pr, landedPRs) + } + } + if landedPRs[3] { + t.Error("PR 3 (the bounced culprit) must not also have a 'landed' transition") + } + + if len(byKind["gate_success"]) == 0 { + t.Error("want at least one 'gate_success' transition for a batch that went on to land") + } +} + func TestTerminalGateOutcomeStatusState(t *testing.T) { for _, tc := range []struct { outcome string @@ -819,6 +1517,9 @@ func TestCheckpointRestoresActiveBatchByRestaging(t *testing.T) { if store.saved == nil || len(store.saved.Active) != 1 { t.Fatalf("checkpoint active batches = %v, want 1 active batch", store.saved) } + if got := store.saved.FormatVersion; got != checkpoint.CurrentFormatVersion { + t.Fatalf("checkpoint format version = %d, want %d", got, checkpoint.CurrentFormatVersion) + } restarted := New(cfg, m, m) if err := restarted.Reconcile(context.Background()); err != nil { @@ -834,8 +1535,12 @@ func TestCheckpointRestoresActiveBatchByRestaging(t *testing.T) { if got := fmt.Sprint(m.merged); got != "[1 2]" { t.Errorf("merged after restore = %s, want [1 2]", got) } + // The transition outbox keeps the checkpoint alive for a few more ticks so + // a dropped final response cannot lose the landed records; it drains and + // the checkpoint is then deleted. + drive(restarted, outboxMaxAttempts+2) if !store.deleted { - t.Error("empty queue should delete checkpoint after restored batch lands") + t.Error("empty queue should delete checkpoint once the transition outbox drains") } } @@ -891,8 +1596,8 @@ func TestCheckpointRestoresPendingBisectionFrontier(t *testing.T) { restarted := New(cfg, m, m) drive(restarted, 30) - if got := fmt.Sprint(m.staged); got != "[[1 2 3 4] [1 2] [3 4] [3] [4]]" { - t.Errorf("staged after restoring frontier = %s, want resumed bisection without root rerun", got) + if got := fmt.Sprint(m.staged); got != "[[1 2 3 4] [1 2] [1 2 3] [1 2 4]]" { + t.Errorf("staged after restoring frontier = %s, want cumulative resumed bisection without root rerun", got) } sort.Ints(m.merged) if got := fmt.Sprint(m.merged); got != "[1 2 4]" { @@ -918,6 +1623,55 @@ func TestAllGreenBatchLandsInOneRun(t *testing.T) { } } +func TestCheckpointRejectsFutureFormatBeforeQueueActions(t *testing.T) { + m := newMock(-1, 1) + store := &memoryCheckpointStore{saved: &checkpoint.QueueSnapshot{ + FormatVersion: checkpoint.CurrentFormatVersion + 1, + Key: checkpoint.QueueKey{Owner: "o", Repo: "r", Base: "main"}, + Pending: [][]int{{1}}, + }} + e := New(Config{Owner: "o", Repo: "r", Base: "main", StagingBranch: "mq/main/staging", Checkpoint: store}, m, m) + + if err := e.Reconcile(context.Background()); err == nil { + t.Fatal("future checkpoint format reconciled successfully") + } + if got := len(m.staged); got != 0 { + t.Fatalf("staged batches = %d, want 0", got) + } +} + +// A legacy (below-current, e.g. Postgres pre-versioning "0" or v1) checkpoint +// with in-flight work cannot be resumed exactly, but it must not wedge the +// queue forever: the engine discards the stale state and re-derives from the +// forge, then persists a current-version checkpoint. +func TestCheckpointDiscardsLegacyInFlightAndReDerives(t *testing.T) { + m := newMock(-1, 1, 2) + store := &memoryCheckpointStore{saved: &checkpoint.QueueSnapshot{ + FormatVersion: 0, // Postgres store before it persisted a version + Key: checkpoint.QueueKey{Owner: "o", Repo: "r", Base: "main"}, + Pending: [][]int{{9}}, // a stale PR that isn't even queued anymore + }} + e := New(Config{Owner: "o", Repo: "r", Base: "main", StatusCtx: "merge-queue", StagingBranch: "mq/main/staging", Checkpoint: store}, m, m) + + if err := e.Reconcile(context.Background()); err != nil { + t.Fatalf("legacy checkpoint wedged the queue: %v", err) + } + // The stale [9] is gone; the queue re-derived the real ready PRs. + for _, s := range m.staged { + for _, n := range s { + if n == 9 { + t.Fatalf("stale legacy candidate 9 was staged: %v", m.staged) + } + } + } + if len(m.staged) == 0 { + t.Fatal("queue did not re-derive after discarding the legacy checkpoint") + } + if store.saved == nil || store.saved.FormatVersion != checkpoint.CurrentFormatVersion { + t.Fatalf("re-saved checkpoint version = %v, want current", store.saved) + } +} + func TestCheckpointPreservesMissingGateRetryMetadata(t *testing.T) { m := newMock(-1, 1) e := New(Config{Owner: "o", Repo: "r", Base: "main", StagingBranch: "mq/main/staging"}, m, m) @@ -2335,8 +3089,9 @@ func TestLeaseContentionSkipsQueueActions(t *testing.T) { func TestLeaseReacquisitionResetsVolatileStateAndReloadsCheckpoint(t *testing.T) { m := newMock(-1, 1, 2) store := &memoryCheckpointStore{saved: &checkpoint.QueueSnapshot{ - Key: checkpoint.QueueKey{Owner: "o", Repo: "r", Base: "main"}, - Pending: [][]int{{1}}, + FormatVersion: checkpoint.CurrentFormatVersion, + Key: checkpoint.QueueKey{Owner: "o", Repo: "r", Base: "main"}, + Pending: [][]int{{1}}, }} lease := &testQueueLease{held: []bool{false, true}} e := New(Config{ @@ -2781,7 +3536,7 @@ type contextBlockingStager struct { hasDeadline bool } -func (s *contextBlockingStager) BuildStaging(ctx context.Context, _ string, _ string, _ []gitops.MergedRef) (string, int, error) { +func (s *contextBlockingStager) BuildStaging(ctx context.Context, _, _, _ string, _ []gitops.MergedRef) (string, int, error) { s.deadline, s.hasDeadline = ctx.Deadline() <-ctx.Done() return "", 0, ctx.Err() @@ -2832,3 +3587,142 @@ func (s *memoryCheckpointStore) DeleteQueue(_ context.Context, _ checkpoint.Queu s.deleted = true return nil } + +// Staging branch names encode bisection lineage. +// +// The name is `--`, where every batch bisected out of one +// root shares the runID and the path records the splits: "r" for the root, +// "r0"/"r1" for its halves, "r01" for the second half of the first, and so on. +// A branch's parent is its path minus the last character. +// +// The previous scheme, `-`, encoded nothing: the timestamp was +// taken per staging operation so it was unique to each attempt, and seq counted +// over the engine's lifetime. Under BisectFanout > 1 siblings stage at +// effectively the same instant, so nothing in the name distinguished them and a +// tree could not be drawn from the branches alone. +func TestStagingBranchNamesEncodeBisectionLineage(t *testing.T) { + m := newMock(3, 1, 2, 3, 4) + e := New(Config{Owner: "o", Repo: "r", Base: "main", StatusCtx: "merge-queue", StagingBranch: "mq/main/staging"}, m, m) + drive(e, 30) + + if len(m.stagingBranches) < 2 { + t.Fatalf("expected a bisection, got branches: %v", m.stagingBranches) + } + + paths := map[string]bool{} + runs := map[string]bool{} + for _, branch := range m.stagingBranches { + rest := strings.TrimPrefix(branch, "mq/main/staging-") + if rest == branch { + t.Fatalf("branch %q lost the staging prefix", branch) + } + cut := strings.LastIndex(rest, "-") + if cut < 0 { + t.Fatalf("branch %q has no lineage path segment", branch) + } + run, path := rest[:cut], rest[cut+1:] + runs[run] = true + if !strings.HasPrefix(path, "r") { + t.Fatalf("branch %q path %q does not start at a root", branch, path) + } + for _, c := range path[1:] { + if c != '0' && c != '1' { + t.Fatalf("branch %q path %q is not a binary bisection path", branch, path) + } + } + if paths[run+"/"+path] { + t.Fatalf("staging branch reused for run %s path %s", run, path) + } + paths[run+"/"+path] = true + } + + // The bisection of one root shares its run, so the tree groups by name. + if len(runs) != 1 { + t.Fatalf("one root batch should yield one run id, got %d: %v", len(runs), runs) + } + + // Exact-key cache hits may resolve a node without staging its branch, so an + // actually staged grandchild can legitimately have an unstaged parent path. + + // A real split must have happened, or this proves nothing. + if len(paths) < 2 { + t.Fatalf("expected at least one split, got paths: %v", paths) + } +} + +// A batch restacked because a PR head changed is a fresh root, not a retry of +// the same tree position: it must not reuse the previous branch name, or the +// gate status attached to the old branch would still be there. Guards the +// clock-only run id that broke TestStagingBranchesAreUniquePerAttempt. +func TestRestackedBatchGetsANewRunEvenWithAFrozenClock(t *testing.T) { + m := newMock(-1, 1) + m.runStatus = "running" + e := New(Config{Owner: "o", Repo: "r", Base: "main", StagingBranch: "mq/main/staging"}, m, m) + e.now = func() time.Time { return time.Unix(100, 0) } + + if err := e.Reconcile(context.Background()); err != nil { + t.Fatalf("start batch: %v", err) + } + m.prs[1].Head.Sha = "head-1-updated" + if err := e.Reconcile(context.Background()); err != nil { + t.Fatalf("restack updated head: %v", err) + } + + if len(m.stagingBranches) != 2 { + t.Fatalf("staging branches = %d, want 2", len(m.stagingBranches)) + } + if m.stagingBranches[0] == m.stagingBranches[1] { + t.Fatalf("staging branch reused across restack: %q", m.stagingBranches[0]) + } + for _, b := range m.stagingBranches { + if !strings.HasSuffix(b, "-r") { + t.Errorf("restack should stage a root path, got %q", b) + } + } +} + +// A consumer that acks a terminal transition on the SAME tick a fresh process +// starts: the ack arrives before Reconcile restores the outbox from the +// checkpoint. Regression — the ack used to be applied against an empty outbox +// and lost, so the entry was redelivered forever and the checkpoint never +// drained. +func TestAckAppliedAfterCheckpointRestoreOnFreshProcess(t *testing.T) { + m := newMock(-1, 1) + store := &memoryCheckpointStore{} + cfg := Config{Owner: "o", Repo: "r", Base: "main", StatusCtx: "merge-queue", StagingBranch: "mq/main/staging", Checkpoint: store} + e := New(cfg, m, m) + + var landedID string + for i := 0; i < 8; i++ { + _ = e.Reconcile(context.Background()) + for _, tr := range e.Transitions() { + if tr.Kind == "landed" { + landedID = tr.EventID + } + } + m.advanceNative() + if landedID != "" && store.saved != nil && len(store.saved.TransitionOutbox) > 0 { + break + } + } + if landedID == "" || store.saved == nil || len(store.saved.TransitionOutbox) == 0 { + t.Fatalf("setup: landedID=%q outbox persisted=%v", landedID, store.saved != nil && len(store.saved.TransitionOutbox) > 0) + } + + // Fresh process. Ack comes in on the reconcile request, before the outbox + // is restored. + restarted := New(cfg, m, m) + restarted.AckTransitions([]string{landedID}) + if err := restarted.Reconcile(context.Background()); err != nil { + t.Fatal(err) + } + for _, tr := range restarted.Transitions() { + if tr.EventID == landedID { + t.Fatalf("acked transition %q was redelivered by the fresh process", landedID) + } + } + // The only outbox entry was acked, so the checkpoint drains entirely. + if store.saved != nil && len(store.saved.TransitionOutbox) != 0 { + t.Fatalf("outbox not drained after ack: %d entries", len(store.saved.TransitionOutbox)) + } +} diff --git a/internal/engine/frontier_model_test.go b/internal/engine/frontier_model_test.go new file mode 100644 index 0000000..ed9a04f --- /dev/null +++ b/internal/engine/frontier_model_test.go @@ -0,0 +1,368 @@ +package engine + +import ( + "fmt" + "math" + "sort" + "strconv" + "strings" + "testing" +) + +// This file is the Go port of docs/architecture/bisection-frontier-model.py: +// an exhaustive, deterministic reference model for the ordered bisection +// frontier. It is executable design documentation. It explores every reachable +// Boolean-oracle path for queues of up to seven candidates and checks the +// invariants stated in docs/architecture/bisection-tree-finalization.md, so the +// engine's own traces can later be compared against a proven oracle without +// adding Python to the build. +// +// Candidates are identified by their queue index (0 = "A", 1 = "B", …). An +// exact test key is the ordered accepted accumulator followed by the candidate +// under test, encoded as a comma-joined string of indices. + +func frontierKeyOf(nums []int) string { + parts := make([]string, len(nums)) + for i, n := range nums { + parts[i] = strconv.Itoa(n) + } + return strings.Join(parts, ",") +} + +// errUnknownFrontierOutcome is raised (as a value) by the model when the oracle +// has no answer for an exact key, so the exhaustive walker can fork both +// answers. +type errUnknownFrontierOutcome struct{ key string } + +func (e errUnknownFrontierOutcome) Error() string { return "unknown outcome for key " + e.key } + +type frontierEvent struct { + outcome string // "accept" or "reject" + baseline []int + candidate []int + key string +} + +type frontierResult struct { + accepted []int + rejected []int + events []frontierEvent + lookups []string +} + +// resolveFrontier resolves one queue of the given size using exact-key caching +// and left-first depth-first recursion. It is a direct port of resolve_queue. +func resolveFrontier(size int, outcomes map[string]bool) (frontierResult, error) { + var ( + accepted []int + rejected []int + events []frontierEvent + lookups []string + unknown *errUnknownFrontierOutcome + ) + + test := func(candidate []int) bool { + key := frontierKeyOf(append(append([]int(nil), accepted...), candidate...)) + lookups = append(lookups, key) + out, ok := outcomes[key] + if !ok { + unknown = &errUnknownFrontierOutcome{key: key} + panic(unknown) + } + return out + } + + var resolve func(candidate []int) + resolve = func(candidate []int) { + baseline := append([]int(nil), accepted...) + key := frontierKeyOf(append(append([]int(nil), baseline...), candidate...)) + if test(candidate) { + events = append(events, frontierEvent{"accept", baseline, append([]int(nil), candidate...), key}) + accepted = append(accepted, candidate...) + return + } + if len(candidate) == 1 { + events = append(events, frontierEvent{"reject", baseline, append([]int(nil), candidate...), key}) + rejected = append(rejected, candidate...) + return + } + middle := len(candidate) / 2 + resolve(candidate[:middle]) + resolve(candidate[middle:]) + } + + full := make([]int, size) + for i := range full { + full[i] = i + } + + err := func() (err error) { + defer func() { + if r := recover(); r != nil { + if unknown != nil { + err = *unknown + return + } + panic(r) + } + }() + resolve(full) + return nil + }() + if err != nil { + return frontierResult{}, err + } + return frontierResult{accepted, rejected, events, lookups}, nil +} + +// preservedFrontierPrefix returns only whole held decisions strictly before a +// changed candidate. Port of preserved_prefix. +func preservedFrontierPrefix(events []frontierEvent, changed int) []frontierEvent { + var prefix []frontierEvent + for _, ev := range events { + stop := false + for _, n := range ev.candidate { + if n >= changed { + stop = true + break + } + } + if stop { + break + } + prefix = append(prefix, ev) + } + return prefix +} + +func assertFrontierInvariants(t *testing.T, size int, outcomes map[string]bool, r frontierResult) { + t.Helper() + + union := append(append([]int(nil), r.accepted...), r.rejected...) + sort.Ints(union) + want := make([]int, size) + for i := range want { + want[i] = i + } + if frontierKeyOf(union) != frontierKeyOf(want) { + t.Fatalf("accepted+rejected = %v, want every candidate 0..%d exactly once", union, size-1) + } + if !sort.IntsAreSorted(r.accepted) { + t.Fatalf("accepted %v is not an ordered subsequence", r.accepted) + } + if !sort.IntsAreSorted(r.rejected) { + t.Fatalf("rejected %v is not ordered", r.rejected) + } + + var accumulated []int + for _, ev := range r.events { + if frontierKeyOf(ev.baseline) != frontierKeyOf(accumulated) { + t.Fatalf("event baseline %v != running accumulator %v", ev.baseline, accumulated) + } + if ev.key != frontierKeyOf(append(append([]int(nil), ev.baseline...), ev.candidate...)) { + t.Fatalf("event key %q != baseline+candidate", ev.key) + } + if outcomes[ev.key] != (ev.outcome == "accept") { + t.Fatalf("event %q outcome %q disagrees with oracle %v", ev.key, ev.outcome, outcomes[ev.key]) + } + if ev.outcome == "accept" { + accumulated = append(accumulated, ev.candidate...) + } else if len(ev.candidate) != 1 { + t.Fatalf("rejected a non-singleton candidate %v", ev.candidate) + } + } + if frontierKeyOf(accumulated) != frontierKeyOf(r.accepted) { + t.Fatalf("event accumulator %v != accepted %v", accumulated, r.accepted) + } + if len(r.accepted) > 0 && !outcomes[frontierKeyOf(r.accepted)] { + t.Fatalf("final accepted accumulator %v has no passing proof", r.accepted) + } + + // Successor-root prefix inheritance: whatever crosses into a successor when + // candidate `changed` is repinned must be whole decisions strictly to its + // left, and the boundary decision must contain or start after `changed`. + for changed := 0; changed < size; changed++ { + inherited := preservedFrontierPrefix(r.events, changed) + for _, ev := range inherited { + for _, c := range ev.candidate { + if c >= changed { + t.Fatalf("inherited decision %v crosses changed candidate %d", ev.candidate, changed) + } + } + } + if len(inherited) < len(r.events) { + boundary := r.events[len(inherited)].candidate + maxc := boundary[0] + for _, c := range boundary { + if c > maxc { + maxc = c + } + } + if maxc < changed { + t.Fatalf("boundary decision %v is entirely left of changed %d but was not inherited", boundary, changed) + } + } + } +} + +// TestFrontierModelExhaustive walks every deterministic Boolean-oracle path for +// n = 1..7 and checks the invariants plus the CI-run bounds from the design. +func TestFrontierModelExhaustive(t *testing.T) { + for size := 1; size <= 7; size++ { + frontier := []map[string]bool{{}} + var completed []frontierResult + + for len(frontier) > 0 { + outcomes := frontier[len(frontier)-1] + frontier = frontier[:len(frontier)-1] + + r, err := resolveFrontier(size, outcomes) + if err != nil { + u := err.(errUnknownFrontierOutcome) + for _, answer := range []bool{false, true} { + branch := make(map[string]bool, len(outcomes)+1) + for k, v := range outcomes { + branch[k] = v + } + branch[u.key] = answer + frontier = append(frontier, branch) + } + continue + } + assertFrontierInvariants(t, size, outcomes, r) + completed = append(completed, r) + } + + acceptedSets := map[string]bool{} + minRuns, maxRuns := math.MaxInt, 0 + for _, r := range completed { + acceptedSets[frontierKeyOf(r.accepted)] = true + distinct := map[string]bool{} + for _, k := range r.lookups { + distinct[k] = true + } + if len(distinct) < minRuns { + minRuns = len(distinct) + } + if len(distinct) > maxRuns { + maxRuns = len(distinct) + } + } + + if len(completed) != 1< 0 { + ok := true + for _, n := range prefix { + if n == bad { + ok = false + } + } + outcomes[frontierKeyOf(prefix)] = ok + } + for i := start; i < size; i++ { + build(append(prefix, i), i+1) + } + } + build(nil, 0) + + r, err := resolveFrontier(size, outcomes) + if err != nil { + t.Fatalf("bad=%d resolve: %v", bad, err) + } + var wantAccepted []int + for n := 0; n < size; n++ { + if n != bad { + wantAccepted = append(wantAccepted, n) + } + } + if frontierKeyOf(r.accepted) != frontierKeyOf(wantAccepted) { + t.Fatalf("bad=%d accepted = %v, want %v", bad, r.accepted, wantAccepted) + } + if frontierKeyOf(r.rejected) != strconv.Itoa(bad) { + t.Fatalf("bad=%d rejected = %v, want [%d]", bad, r.rejected, bad) + } + distinct := map[string]bool{} + for _, k := range r.lookups { + distinct[k] = true + } + if len(distinct) != want[bad] { + t.Fatalf("bad=%d distinct CI runs = %d, want %d", bad, len(distinct), want[bad]) + } + } +} diff --git a/internal/engine/transitions.go b/internal/engine/transitions.go new file mode 100644 index 0000000..2bf461c --- /dev/null +++ b/internal/engine/transitions.go @@ -0,0 +1,136 @@ +package engine + +import ( + "sort" + "strconv" + "strings" +) + +// Transition is a structured record of one merge-queue lifecycle event that +// happened during a single Reconcile call: a batch was staged, its gate +// passed, it was bisected, a PR bounced, or a PR landed. It exists so a +// caller can persist what actually happened without parsing log text. +// +// Additive to the engine's existing logger.Info/Warn calls at the same +// sites — recording a Transition never changes what gets logged. +type Transition struct { + // Kind is one of "staged", "gate_success", "bisected", "bounced", "landed", + // "held" (a leaf reached a terminal gate result but its source decision is + // deferred until the whole bisection root is terminal — Reason carries the + // held outcome, "success" or "failure"/"error"), "root_invalidated" (a + // whole bisection root was torn down after its base branch advanced or a + // pinned candidate changed mid-test; its candidates are re-queued as a + // fresh root and no source decision is published), or "node_superseded" (a + // speculatively-staged bisection node whose gate ran against an accumulator + // the resolved frontier no longer matches; re-staged on the correct + // baseline, no source decision). + Kind string `json:"kind"` + // PRs is the batch's PR set for "staged"/"gate_success"/"bisected"/"held", + // or the single terminated/landed PR (as a length-1 slice) for + // "bounced"/"landed". + PRs []int `json:"prs"` + StagingBranch string `json:"staging_branch"` + // RunID and LineagePath identify this batch's position in a bisection + // tree (see stagingBranchFor/markLineage) — a "staged" transition's + // LineagePath minus its last character is its parent's LineagePath, + // which is how a caller resolves parentage without re-parsing branch + // name strings from scratch. + RunID string `json:"run_id"` + LineagePath string `json:"lineage_path"` + // Reason is set for "bounced" (why the PR was rejected) and for "held" + // (the held gate outcome: "success", "failure", or "error"). + Reason string `json:"reason,omitempty"` + // EventID is a deterministic key for transitions that drive an + // irreversible side effect (a merge counter, a bounce notification): the + // consumer records it and skips a redelivery carrying the same id. It is + // stable across restarts because it is derived only from the durable root + // run id, the action, and the PR(s) — not from a clock or a slice index. + // Empty for transitions whose application is already idempotent by branch + // name ("staged", "gate_success", "bisected"). + EventID string `json:"event_id,omitempty"` +} + +// Transitions returns the lifecycle records the caller should persist for the +// most recently completed Reconcile call: this tick's single-shot records plus +// every still-pending terminal record in the outbox (re-sent until a dropped +// reconcile response can no longer have lost it). Redelivery is safe — the +// consumer de-dups terminal records by EventID. +func (e *Engine) Transitions() []Transition { + out := append([]Transition(nil), e.transitions...) + for _, entry := range e.outbox { + out = append(out, entry.t) + } + return out +} + +// ageOutbox runs at the start of every Reconcile: it counts one more send +// against each pending terminal transition and drops the ones that have had +// their full run of retransmits. outboxMaxAttempts is a safety net for a +// consumer that never acks — the primary removal is AckTransitions. +func (e *Engine) ageOutbox() { + kept := e.outbox[:0] + for _, entry := range e.outbox { + entry.attempts++ + if entry.attempts < outboxMaxAttempts { + kept = append(kept, entry) + } + } + e.outbox = kept +} + +// AckTransitions records the EventIDs the consumer confirmed it persisted, to +// be dropped from the outbox. Called before Reconcile with the ids carried on +// the reconcile request. The drop is deferred to applyPendingAcks (run just +// after the checkpoint is loaded) because on a fresh process the outbox does +// not exist yet — it is restored from the checkpoint inside Reconcile, and an +// ack applied before that would be silently lost and the entry redelivered +// forever. Unknown ids are ignored. +func (e *Engine) AckTransitions(ids []string) { + e.pendingAcks = append(e.pendingAcks, ids...) +} + +// applyPendingAcks drops every outbox entry whose EventID the consumer has +// acknowledged. Run once per Reconcile, immediately after loadCheckpoint. +func (e *Engine) applyPendingAcks() { + if len(e.pendingAcks) == 0 { + return + } + acked := make(map[string]bool, len(e.pendingAcks)) + for _, id := range e.pendingAcks { + acked[id] = true + } + e.pendingAcks = e.pendingAcks[:0] + if len(e.outbox) == 0 { + return + } + kept := e.outbox[:0] + for _, entry := range e.outbox { + if !acked[entry.t.EventID] { + kept = append(kept, entry) + } + } + e.outbox = kept +} + +func (e *Engine) recordTransition(t Transition) { + if t.EventID != "" { + e.outbox = append(e.outbox, outboxEntry{t: t}) + return + } + e.transitions = append(e.transitions, t) +} + +// terminalEventID builds the deterministic de-dup key for a transition that +// drives an irreversible side effect. Finalization performs each action once +// (the persisted cursor guarantees it), so runID + kind + the ordered PR list +// uniquely names it regardless of how many times the reconcile response is +// redelivered. +func terminalEventID(runID, kind string, prs []int) string { + sorted := append([]int(nil), prs...) + sort.Ints(sorted) + parts := make([]string, len(sorted)) + for i, n := range sorted { + parts[i] = strconv.Itoa(n) + } + return runID + "|" + kind + "|" + strings.Join(parts, ",") +} diff --git a/internal/engine/tree_liveness_test.go b/internal/engine/tree_liveness_test.go new file mode 100644 index 0000000..04d9c94 --- /dev/null +++ b/internal/engine/tree_liveness_test.go @@ -0,0 +1,146 @@ +package engine + +import ( + "context" + "testing" + "time" + + "github.com/rbtr/shunt/internal/forge" +) + +// A bisection root has one held success and one node still under test. The open +// node never gets a gate result and is abandoned after the missing-gate +// retries. Regression: the abandon used to bounce the node's PR and drop the +// batch without accounting for it, so the root waited forever on a slot +// nothing would fill and its held success was stranded. Now the whole root is +// torn down and its candidates are re-queued — no wedge, no wrongful bounce. +func TestMissingGateAbandonDoesNotStrandRoot(t *testing.T) { + m := newMock(-1, 1, 2) + m.runStatusSet = true + m.runStatus = "" // no gate result for anything: the missing-gate condition + e := New(Config{Owner: "o", Repo: "r", Base: "main", StatusCtx: "merge-queue", StagingBranch: "mq/main/staging"}, m, m) + + clock := time.Now() + e.now = func() time.Time { return clock } + e.checkpointLoaded = true + + runID := "run-liveness" + e.rootAnchor = map[string]string{runID: "anchor-main"} // matches mock BranchHead + + held := &activeBatch{ + prs: []forge.PullRequest{*m.prs[1]}, + stagingBranch: "mq/main/staging-run-liveness-r0", + stagingSHA: "sha-held", + runID: runID, + lineagePath: "r0", + outcome: "success", + } + open := &activeBatch{ + prs: []forge.PullRequest{*m.prs[2]}, + stagingBranch: "mq/main/staging-run-liveness-r1", + stagingSHA: "sha-open", + runID: runID, + lineagePath: "r1", + phase: "waiting_gate", + phaseSince: clock, + missingGateRetries: missingGateMaxRetries, + } + e.trees[runID] = &bisectionTree{ + accepted: []forge.PullRequest{*m.prs[1]}, + results: map[string]string{}, + held: []heldLeaf{{batch: held, outcome: "success"}}, + } + e.active = []*activeBatch{open} + clock = clock.Add(2 * time.Hour) + + for i := 0; i < 40; i++ { + if _, err := e.checkActive(context.Background()); err != nil { + t.Fatalf("checkActive: %v", err) + } + m.advanceNative() + clock = clock.Add(10 * time.Minute) + } + + if _, ok := e.trees[runID]; ok { + t.Fatalf("root %s was not torn down after its open node's gate never ran", runID) + } + // Infra failure must not be published as a PR rejection. + if m.bounced[1] || m.bounced[2] { + t.Fatalf("no PR should be bounced for a missing gate, bounced=%v", m.bounced) + } + // Both candidates go back to pending as a fresh root. + if len(e.pending) == 0 { + t.Fatalf("candidates were not re-queued: pending=%v", e.pending) + } +} + +// Same shape, but the open node's PR head changes mid-test. Regression: the +// head-change requeue used to detach the node from its tree under a fresh run +// id, stranding the root and its held prefix. Now +// invalidateRootsOnCandidateChange sees the active node's PR and re-roots (or +// tears down) the whole tree — nothing is left waiting. +func TestActiveNodeHeadChangeReRootsInsteadOfStranding(t *testing.T) { + m := newMock(-1, 1, 2) + m.runStatusSet = true + m.runStatus = "" + e := New(Config{Owner: "o", Repo: "r", Base: "main", StatusCtx: "merge-queue", StagingBranch: "mq/main/staging"}, m, m) + clock := time.Now() + e.now = func() time.Time { return clock } + e.checkpointLoaded = true + + runID := "run-liveness2" + e.rootAnchor = map[string]string{runID: "anchor-main"} + held := &activeBatch{ + prs: []forge.PullRequest{*m.prs[1]}, stagingBranch: "b-r0", stagingSHA: "s0", + runID: runID, lineagePath: "r0", outcome: "success", + } + open := &activeBatch{ + prs: []forge.PullRequest{*m.prs[2]}, stagingBranch: "b-r1", stagingSHA: "s1", + runID: runID, lineagePath: "r1", phase: "waiting_gate", phaseSince: clock, + } + e.trees[runID] = &bisectionTree{ + accepted: []forge.PullRequest{*m.prs[1]}, + results: map[string]string{}, held: []heldLeaf{{batch: held, outcome: "success"}}, + } + e.active = []*activeBatch{open} + + m.prs[2].Head.Sha = "head-2-v2" // someone pushes to PR #2 + + for i := 0; i < 30; i++ { + if _, err := e.checkActive(context.Background()); err != nil { + t.Fatalf("checkActive: %v", err) + } + clock = clock.Add(30 * time.Minute) + } + if _, ok := e.trees[runID]; ok { + t.Fatalf("root %s survived a mid-test head change on an active node", runID) + } + // Both candidates are re-queued as a fresh root; nothing is stranded. + if got := len(e.pending); got == 0 && len(e.active) == 0 { + t.Fatalf("candidates were dropped, not re-queued: pending=%v active=%d", e.pending, len(e.active)) + } +} + +// treeHasUnresolvedNode is the finalization gate; make sure it counts both +// staged and pending nodes and nothing else. +func TestTreeHasUnresolvedNode(t *testing.T) { + e := New(Config{Owner: "o", Repo: "r", Base: "main"}, newMock(-1), nil) + e.trees["R"] = &bisectionTree{results: map[string]string{}} + if e.treeHasUnresolvedNode("R") { + t.Fatal("empty tree should be ready") + } + e.active = []*activeBatch{{prs: []forge.PullRequest{{Number: 1}}, runID: "R"}} + if !e.treeHasUnresolvedNode("R") { + t.Fatal("an active node makes the tree unresolved") + } + e.active = nil + e.pending = [][]int{{2}} + e.lineageRunID = map[int]string{2: "R"} + if !e.treeHasUnresolvedNode("R") { + t.Fatal("a pending node with matching lineage makes the tree unresolved") + } + e.lineageRunID[2] = "OTHER" + if e.treeHasUnresolvedNode("R") { + t.Fatal("a pending node for a different root must not count") + } +} diff --git a/internal/forge/client.go b/internal/forge/client.go index e6d4be7..fb314b5 100644 --- a/internal/forge/client.go +++ b/internal/forge/client.go @@ -551,6 +551,30 @@ func (c *Client) ProtectedBranch(ctx context.Context, owner, repo, branch string return p, nil } +// BranchHead returns the commit SHA at the tip of branch. The merge queue +// pins this once when it opens a bisection root so every staged integration +// and every exact test key is anchored to the same base, and so a later +// external advance of the live branch can be detected as invalidation. +func (c *Client) BranchHead(ctx context.Context, owner, repo, branch string) (string, error) { + data, err := c.doRaw(ctx, http.MethodGet, + fmt.Sprintf("/repos/%s/branches/%s", repoPath(owner, repo), url.PathEscape(branch)), nil) + if err != nil { + return "", err + } + var b struct { + Commit struct { + ID string `json:"id"` + } `json:"commit"` + } + if err := json.Unmarshal(data, &b); err != nil { + return "", err + } + if b.Commit.ID == "" { + return "", fmt.Errorf("branch %q has no commit id", branch) + } + return b.Commit.ID, nil +} + // CancelAutomerge reports whether a live scheduled merge was removed. func (c *Client) CancelAutomerge(ctx context.Context, owner, repo string, index int) (bool, error) { _, err := c.doRaw(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/pulls/%d/merge", repoPath(owner, repo), index), nil) diff --git a/internal/forge/client_test.go b/internal/forge/client_test.go index 9b6f6d0..aede3a4 100644 --- a/internal/forge/client_test.go +++ b/internal/forge/client_test.go @@ -111,6 +111,30 @@ func TestListReviewsParsesReviewFlags(t *testing.T) { } } +func TestBranchHeadReturnsTipCommit(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/api/v1/repos/o/r/branches/main" { + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `{"name":"main","commit":{"id":"abc123def456","message":"x"}}`) + return + } + http.NotFound(w, r) + })) + defer srv.Close() + + c := New(srv.URL, "token") + sha, err := c.BranchHead(context.Background(), "o", "r", "main") + if err != nil { + t.Fatalf("BranchHead: %v", err) + } + if sha != "abc123def456" { + t.Fatalf("BranchHead = %q, want abc123def456", sha) + } + if _, err := c.BranchHead(context.Background(), "o", "r", "gone"); err == nil { + t.Fatal("BranchHead on a missing branch returned no error") + } +} + func TestProtectedBranchParsesRequirementsAnd404MeansNoRule(t *testing.T) { var path string srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/internal/gitops/git.go b/internal/gitops/git.go index 8ed706c..a17b5f1 100644 --- a/internal/gitops/git.go +++ b/internal/gitops/git.go @@ -52,7 +52,7 @@ type gitAuth struct { // it, and returns the resulting SHA. The caller must pass a fresh branch name for // each attempt. On a merge conflict it returns the offending PR number // (conflictPR > 0) with an error. -func (s *Stager) BuildStaging(ctx context.Context, base, stagingBranch string, refs []MergedRef) (sha string, conflictPR int, err error) { +func (s *Stager) BuildStaging(ctx context.Context, base, baseAnchor, stagingBranch string, refs []MergedRef) (sha string, conflictPR int, err error) { parent, err := os.MkdirTemp("", "shunt-stage-") if err != nil { return "", 0, err @@ -93,8 +93,12 @@ func (s *Stager) BuildStaging(ctx context.Context, base, stagingBranch string, r if _, err := run("config", "user.email", s.authorEmail); err != nil { return "", 0, err } - if out, err := run("checkout", "-B", stagingBranch, "origin/"+base); err != nil { - return "", 0, fmt.Errorf("checkout base %q: %v: %s", base, err, out) + startPoint := "origin/" + base + if baseAnchor != "" { + startPoint = baseAnchor // pin to the root's immutable base commit + } + if out, err := run("checkout", "-B", stagingBranch, startPoint); err != nil { + return "", 0, fmt.Errorf("checkout base %q: %v: %s", startPoint, err, out) } for _, r := range refs { if out, err := run("fetch", "--quiet", "origin", r.Ref); err != nil { diff --git a/mq/checkpoint/checkpoint.go b/mq/checkpoint/checkpoint.go index a2504d0..2e5697e 100644 --- a/mq/checkpoint/checkpoint.go +++ b/mq/checkpoint/checkpoint.go @@ -19,14 +19,65 @@ type QueueKey struct { Base string `json:"Base"` } +// CurrentFormatVersion is written by the current engine. Version 1 was the +// compatibility release; version 2 carries durable bisection-tree state. +const CurrentFormatVersion = 2 + // QueueSnapshot is the durable shape of queue state. type QueueSnapshot struct { - Key QueueKey `json:"Key"` - Pending [][]int `json:"Pending"` - Active []ActiveBatchSnapshot `json:"Active"` - LingerSince time.Time `json:"LingerSince"` - BaseGeneration int `json:"BaseGeneration"` - StagingSequence int `json:"StagingSequence"` + FormatVersion int `json:"FormatVersion"` + Key QueueKey `json:"Key"` + Pending [][]int `json:"Pending"` + PendingNodes []PendingNodeSnapshot `json:"PendingNodes"` + Active []ActiveBatchSnapshot `json:"Active"` + LingerSince time.Time `json:"LingerSince"` + BaseGeneration int `json:"BaseGeneration"` + StagingSequence int `json:"StagingSequence"` + Trees []BisectionTreeSnapshot `json:"Trees"` + // TransitionOutbox carries terminal lifecycle records (a merge counter, a + // bounce notification) that have not yet been re-sent enough times to be + // certain a dropped reconcile response did not lose them. Redelivery is + // safe: the consumer de-dups by EventID. + TransitionOutbox []OutboxTransitionSnapshot `json:"TransitionOutbox,omitempty"` +} + +// OutboxTransitionSnapshot is one pending terminal lifecycle record plus how +// many reconcile responses have already carried it. +type OutboxTransitionSnapshot struct { + Kind string `json:"Kind"` + PRs []int `json:"PRs"` + StagingBranch string `json:"StagingBranch"` + RunID string `json:"RunID"` + LineagePath string `json:"LineagePath"` + Reason string `json:"Reason"` + EventID string `json:"EventID"` + Attempts int `json:"Attempts"` +} + +// PendingNodeSnapshot preserves bisection lineage before a node is staged. +type PendingNodeSnapshot struct { + PRs []int `json:"PRs"` + RunID string `json:"RunID"` + Path string `json:"Path"` +} + +// BisectionTreeSnapshot persists a root's held leaves and finalization cursor. +type BisectionTreeSnapshot struct { + RunID string `json:"RunID"` + // Anchor is the base-branch commit SHA pinned when this root opened. Every + // exact key in Results is scoped to it, so it must be restored verbatim. + Anchor string `json:"Anchor"` + Open int `json:"Open"` + Cursor int `json:"Cursor"` + Accepted []PullRequestSnapshot `json:"Accepted"` + Results map[string]string `json:"Results"` + Held []HeldLeafSnapshot `json:"Held"` +} + +// HeldLeafSnapshot is a gate-terminal leaf whose source action is deferred. +type HeldLeafSnapshot struct { + Batch ActiveBatchSnapshot `json:"Batch"` + Outcome string `json:"Outcome"` } // ActiveBatchSnapshot records a staging branch currently waiting on its gate. @@ -40,6 +91,18 @@ type ActiveBatchSnapshot struct { Outcome string `json:"Outcome"` PhaseSince time.Time `json:"PhaseSince"` MissingGateRetries int `json:"MissingGateRetries"` + RunID string `json:"RunID"` + LineagePath string `json:"LineagePath"` + ExactKey string `json:"ExactKey"` + // BaseAnchor is the root's pinned base-branch SHA, carried on a root batch + // that has not split yet (and so has no BisectionTreeSnapshot). + BaseAnchor string `json:"BaseAnchor"` + // DebugURL is the staging gate's run link, shown in a later bounce comment; + // Speculative marks a fanout node staged ahead of the frontier. Both are + // restored so a checkpoint reload between the gate result and the bounce / + // promotion keeps the link and the metric accurate. + DebugURL string `json:"DebugURL,omitempty"` + Speculative bool `json:"Speculative,omitempty"` } // PullRequestSnapshot is the PR identity needed to re-queue a batch. @@ -62,6 +125,9 @@ type Store interface { // Validate checks the snapshot shape so a host can reject malformed persisted // data before handing it to the engine (a bad snapshot must not wedge it). func (s QueueSnapshot) Validate() error { + if s.FormatVersion != 0 && s.FormatVersion != 1 && s.FormatVersion != CurrentFormatVersion { + return fmt.Errorf("unsupported queue checkpoint format version %d", s.FormatVersion) + } if err := s.Key.Validate(); err != nil { return err } @@ -81,6 +147,16 @@ func (s QueueSnapshot) Validate() error { } } } + for i, node := range s.PendingNodes { + if len(node.PRs) == 0 || (node.RunID == "") != (node.Path == "") { + return fmt.Errorf("queue checkpoint pending node %d is invalid", i) + } + for _, n := range node.PRs { + if n <= 0 { + return fmt.Errorf("queue checkpoint pending node %d has invalid PR number %d", i, n) + } + } + } for i, active := range s.Active { if active.StagingBranch == "" { return fmt.Errorf("queue checkpoint active batch %d missing staging branch", i) @@ -94,6 +170,9 @@ func (s QueueSnapshot) Validate() error { if active.MissingGateRetries < 0 { return fmt.Errorf("queue checkpoint active batch %d has negative missing-gate retries", i) } + if (active.RunID == "") != (active.LineagePath == "") { + return fmt.Errorf("queue checkpoint active batch %d has incomplete lineage", i) + } if active.Outcome != "" && active.Outcome != "success" && active.Outcome != "failure" && active.Outcome != "cancelled" && active.Outcome != "error" { return fmt.Errorf("queue checkpoint active batch %d has invalid outcome %q", i, active.Outcome) } @@ -109,6 +188,46 @@ func (s QueueSnapshot) Validate() error { } } } + for i, tree := range s.Trees { + if tree.RunID == "" || tree.Open < 0 || tree.Cursor < 0 || tree.Cursor > len(tree.Held) { + return fmt.Errorf("queue checkpoint tree %d is invalid", i) + } + for key, outcome := range tree.Results { + if key == "" || (outcome != "success" && outcome != "failure" && outcome != "cancelled" && outcome != "error") { + return fmt.Errorf("queue checkpoint tree %d has invalid cached outcome", i) + } + } + for j, accepted := range tree.Accepted { + if accepted.Number <= 0 || accepted.HeadSHA == "" { + return fmt.Errorf("queue checkpoint tree %d accepted PR %d is invalid", i, j) + } + } + for j, leaf := range tree.Held { + if leaf.Outcome != "success" && leaf.Outcome != "failure" && leaf.Outcome != "cancelled" && leaf.Outcome != "error" { + return fmt.Errorf("queue checkpoint tree %d leaf %d has invalid outcome %q", i, j, leaf.Outcome) + } + if err := validActive(leaf.Batch); err != nil { + return fmt.Errorf("queue checkpoint tree %d leaf %d: %w", i, j, err) + } + } + } + for i, ob := range s.TransitionOutbox { + if ob.Kind == "" || ob.EventID == "" || ob.Attempts < 0 { + return fmt.Errorf("queue checkpoint transition outbox entry %d is invalid", i) + } + } + return nil +} + +func validActive(active ActiveBatchSnapshot) error { + if active.StagingBranch == "" || active.StagingSHA == "" || active.BaseGeneration < 0 || active.MissingGateRetries < 0 || len(active.PRs) == 0 { + return fmt.Errorf("invalid active batch") + } + for _, pr := range active.PRs { + if pr.Number <= 0 || pr.HeadSHA == "" { + return fmt.Errorf("invalid active batch PR") + } + } return nil } @@ -130,11 +249,34 @@ func (k QueueKey) Validate() error { func (s QueueSnapshot) Clone() QueueSnapshot { out := s out.Pending = clonePending(s.Pending) + out.PendingNodes = make([]PendingNodeSnapshot, len(s.PendingNodes)) + for i, node := range s.PendingNodes { + out.PendingNodes[i] = node + out.PendingNodes[i].PRs = append([]int(nil), node.PRs...) + } out.Active = make([]ActiveBatchSnapshot, len(s.Active)) for i, active := range s.Active { out.Active[i] = active out.Active[i].PRs = append([]PullRequestSnapshot(nil), active.PRs...) } + out.Trees = make([]BisectionTreeSnapshot, len(s.Trees)) + for i, tree := range s.Trees { + out.Trees[i] = tree + out.Trees[i].Accepted = append([]PullRequestSnapshot(nil), tree.Accepted...) + out.Trees[i].Results = make(map[string]string, len(tree.Results)) + for key, outcome := range tree.Results { + out.Trees[i].Results[key] = outcome + } + out.Trees[i].Held = append([]HeldLeafSnapshot(nil), tree.Held...) + for j := range out.Trees[i].Held { + out.Trees[i].Held[j].Batch.PRs = append([]PullRequestSnapshot(nil), tree.Held[j].Batch.PRs...) + } + } + out.TransitionOutbox = make([]OutboxTransitionSnapshot, len(s.TransitionOutbox)) + for i, ob := range s.TransitionOutbox { + out.TransitionOutbox[i] = ob + out.TransitionOutbox[i].PRs = append([]int(nil), ob.PRs...) + } return out } diff --git a/mq/checkpoint/checkpoint_test.go b/mq/checkpoint/checkpoint_test.go new file mode 100644 index 0000000..e453bba --- /dev/null +++ b/mq/checkpoint/checkpoint_test.go @@ -0,0 +1,16 @@ +package checkpoint + +import "testing" + +func TestQueueSnapshotFormatVersion(t *testing.T) { + key := QueueKey{Owner: "o", Repo: "r", Base: "main"} + + for _, version := range []int{0, CurrentFormatVersion} { + if err := (QueueSnapshot{Key: key, FormatVersion: version}).Validate(); err != nil { + t.Fatalf("format version %d: %v", version, err) + } + } + if err := (QueueSnapshot{Key: key, FormatVersion: CurrentFormatVersion + 1}).Validate(); err == nil { + t.Fatal("future format version passed validation") + } +} diff --git a/mq/mq.go b/mq/mq.go index 121f381..8c391b6 100644 --- a/mq/mq.go +++ b/mq/mq.go @@ -196,6 +196,27 @@ func (eng *Engine) Reconcile(ctx context.Context) error { return eng.e.Reconcile(ctx) } +// Transition is a structured record of one merge-queue lifecycle event +// (a batch staged, its gate passed, it bisected, a PR bounced, or a PR +// landed) recorded during a Reconcile call. See LastTransitions. +type Transition = engine.Transition + +// LastTransitions returns the transitions recorded during the most +// recently completed Reconcile call, so a caller can persist what actually +// happened without parsing log text. Safe to call after Reconcile returns; +// the slice is replaced, not appended to, on the next call. +func (eng *Engine) LastTransitions() []Transition { + return eng.e.Transitions() +} + +// AckTransitions tells the engine which terminal lifecycle records the caller +// has durably persisted, so it stops re-carrying them on the reconcile +// response. Call before Reconcile with the event ids from the last response +// the caller fully applied. +func (eng *Engine) AckTransitions(eventIDs []string) { + eng.e.AckTransitions(eventIDs) +} + // ForgeClient is the interface that a forge client must satisfy to be used // with mq.New. type ForgeClient interface { @@ -204,6 +225,7 @@ type ForgeClient interface { AutomergeState(ctx context.Context, owner, repo string, index int) (AutomergeState, error) ListReviews(ctx context.Context, owner, repo string, index int) ([]Review, error) ProtectedBranch(ctx context.Context, owner, repo, branch string) (BranchProtection, error) + BranchHead(ctx context.Context, owner, repo, branch string) (string, error) LatestCommitStatus(ctx context.Context, owner, repo, sha, statusContext string) (CommitStatus, bool, error) RunStatus(ctx context.Context, owner, repo, sha, branch string) (string, error) RunTargetURL(ctx context.Context, owner, repo, sha, branch string) (string, error) @@ -216,8 +238,13 @@ type ForgeClient interface { // Stager is the interface that a staging implementation must satisfy to be used // with mq.New. +// +// baseAnchor, when non-empty, is an immutable commit SHA the integration must +// be built on instead of the live tip of base. The merge queue pins it once +// per bisection root so main moving mid-tree cannot change what a staged +// result means. type Stager interface { - BuildStaging(ctx context.Context, base, stagingBranch string, refs []MergedRef) (sha string, conflictPR int, err error) + BuildStaging(ctx context.Context, base, baseAnchor, stagingBranch string, refs []MergedRef) (sha string, conflictPR int, err error) } // adapter wraps a mq.ForgeClient to satisfy engine.ForgeAPI. @@ -276,6 +303,10 @@ func (a *adapter) ProtectedBranch(ctx context.Context, owner, repo, branch strin }, nil } +func (a *adapter) BranchHead(ctx context.Context, owner, repo, branch string) (string, error) { + return a.fc.BranchHead(ctx, owner, repo, branch) +} + func (a *adapter) GetPR(ctx context.Context, owner, repo string, index int) (forge.PullRequest, error) { pr, err := a.fc.GetPR(ctx, owner, repo, index) if err != nil { @@ -358,10 +389,10 @@ type stagerAdapter struct { st Stager } -func (s *stagerAdapter) BuildStaging(ctx context.Context, base, stagingBranch string, refs []gitops.MergedRef) (string, int, error) { +func (s *stagerAdapter) BuildStaging(ctx context.Context, base, baseAnchor, stagingBranch string, refs []gitops.MergedRef) (string, int, error) { refs2 := make([]MergedRef, len(refs)) for i, r := range refs { refs2[i] = MergedRef{PR: r.PR, Ref: r.Ref} } - return s.st.BuildStaging(ctx, base, stagingBranch, refs2) + return s.st.BuildStaging(ctx, base, baseAnchor, stagingBranch, refs2) } From 5ceb95d98792b91e03c6a7a804818b82c5c9ab12 Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Fri, 4 Sep 2026 20:11:43 -0500 Subject: [PATCH 2/6] ci: use patched Go toolchain --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index eaa4e6a..2f89499 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,7 +14,7 @@ jobs: - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 - uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff with: - go-version: "1.25.12" + go-version: "1.25.13" - name: gofmt run: test -z "$(gofmt -l .)" || { echo 'gofmt needed:'; gofmt -l .; exit 1; } - name: vet From 00615024ba4f5c8dfc980af98e8bd4569ed8c32f Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Sat, 5 Sep 2026 00:30:03 -0500 Subject: [PATCH 3/6] docs: describe ordered frontier without history references --- docs/design.md | 31 +++++++++++++------------- internal/engine/engine.go | 21 +++++------------ internal/engine/frontier_model_test.go | 6 ++--- 3 files changed, 23 insertions(+), 35 deletions(-) diff --git a/docs/design.md b/docs/design.md index 78165b9..c51ab75 100644 --- a/docs/design.md +++ b/docs/design.md @@ -78,22 +78,21 @@ State, per `(repo, base)`: - `lingerSince` — when the idle engine first saw ready PRs while the optional batch-accumulation window was active. -The engine has a checkpoint boundary around that state: when configured with its -consumer-side `CheckpointStore`, it loads one queue snapshot before the first -reconcile tick, saves after each tick that leaves queue work in progress, and -deletes the snapshot once the queue is idle. The production command can use -bbolt with `SHUNT_STATE_PATH` or Postgres with `SHUNT_POSTGRES_DSN`; otherwise -releases keep the historical process-local state. - -When Postgres is configured, each `(owner, repo, base)` must first acquire its -durable queue lease before loading a checkpoint or calling the forge. The lease -is renewed once per `Reconcile()` call, and that call receives a deadline at -half the configured lease TTL so no holder keeps mutating after its lease can -expire. A replica that cannot acquire it does nothing for that queue; one that -takes it over drops process-local queue and comment caches, then reloads the -durable checkpoint. Restored active batches are re-staged, as on process -restart. bbolt and in-memory state are single-process options and do not -coordinate replicas. +The engine saves queue state through `CheckpointStore`. It loads one snapshot +before the first reconcile. It saves work after each reconcile. It deletes the +snapshot when the queue is idle. + +The production command uses bbolt when `SHUNT_STATE_PATH` is set. This is the +default durable store. Shunt can use Postgres when `SHUNT_POSTGRES_DSN` is set. +Postgres is an optional store for replicas that need a shared queue lease. +Without either setting, queue state exists only in the process. + +A Postgres replica acquires its queue lease before it loads state or calls the +forge. Each `Reconcile()` call renews the lease. The call ends before half of +the lease period expires. A replica that does not hold the lease does not act. +A replica that acquires the lease clears its local queue and comment state. It +then loads the durable checkpoint. bbolt and in-memory state support one +process only. Each `Reconcile()` tick advances one step. Ticks are driven by relevant Forgejo/Gitea webhooks when available, with `SHUNT_POLL_INTERVAL` as the diff --git a/internal/engine/engine.go b/internal/engine/engine.go index 2f6967e..d762397 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -142,24 +142,15 @@ type Engine struct { baseGen int stagingSeq int - // Bisection lineage. A staging branch name carries where in the tree it - // sits, so the tree can be read off the names without a side channel: + // A staging branch records its position in one bisection tree. // // --r root batch - // --r0 first half of the root - // --r01 second half of that first half + // --r0 first child + // --r01 second child of the first child // - // A branch's parent is its name with the last character removed, so the - // whole ancestry is in the name and siblings sort adjacent. The previous - // scheme was -: the timestamp was taken per staging - // operation so it was unique to each attempt, and seq counted over the - // engine's lifetime. Neither encoded lineage, and under BisectFanout > 1 - // siblings stage at effectively the same instant, so nothing about the - // name distinguished them. - // - // runID identifies one root batch and everything bisected out of it. - // lineage maps a pending candidate to its path, keyed by the candidate's - // first PR number — the same idiom as bisectOrigins and requeueStates. + // The path identifies the parent and the child order. + // runID identifies one root batch and its child batches. + // lineage records the path for each pending candidate. runID string lineage map[int]string lineageRunID map[int]string diff --git a/internal/engine/frontier_model_test.go b/internal/engine/frontier_model_test.go index ed9a04f..d7e5524 100644 --- a/internal/engine/frontier_model_test.go +++ b/internal/engine/frontier_model_test.go @@ -50,8 +50,7 @@ type frontierResult struct { lookups []string } -// resolveFrontier resolves one queue of the given size using exact-key caching -// and left-first depth-first recursion. It is a direct port of resolve_queue. +// resolveFrontier resolves one queue with exact keys and left-first recursion. func resolveFrontier(size int, outcomes map[string]bool) (frontierResult, error) { var ( accepted []int @@ -115,8 +114,7 @@ func resolveFrontier(size int, outcomes map[string]bool) (frontierResult, error) return frontierResult{accepted, rejected, events, lookups}, nil } -// preservedFrontierPrefix returns only whole held decisions strictly before a -// changed candidate. Port of preserved_prefix. +// preservedFrontierPrefix returns held decisions before a changed candidate. func preservedFrontierPrefix(events []frontierEvent, changed int) []frontierEvent { var prefix []frontierEvent for _, ev := range events { From a0a0c13ea8ec1e271b71ca0d10231a0f331d47fd Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Sat, 5 Sep 2026 00:32:23 -0500 Subject: [PATCH 4/6] test: cover ordered frontier snapshots in bolt --- internal/checkpoint/bolt/bolt_test.go | 29 +++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/internal/checkpoint/bolt/bolt_test.go b/internal/checkpoint/bolt/bolt_test.go index 393d01a..5d6293e 100644 --- a/internal/checkpoint/bolt/bolt_test.go +++ b/internal/checkpoint/bolt/bolt_test.go @@ -19,23 +19,48 @@ func TestStoreSavesLoadsAndDeletesQueue(t *testing.T) { defer store.Close() snapshot := checkpoint.QueueSnapshot{ - Key: checkpoint.QueueKey{Owner: "o", Repo: "r", Base: "main"}, + FormatVersion: checkpoint.CurrentFormatVersion, + Key: checkpoint.QueueKey{Owner: "o", Repo: "r", Base: "main"}, Pending: [][]int{ {1, 2}, {3}, }, + PendingNodes: []checkpoint.PendingNodeSnapshot{{PRs: []int{3}, RunID: "run-1", Path: "r1"}}, Active: []checkpoint.ActiveBatchSnapshot{{ PRs: []checkpoint.PullRequestSnapshot{ {Number: 4, HeadSHA: "head-4"}, }, - StagingBranch: "mq/main/staging", + StagingBranch: "mq/main/staging-4", StagingSHA: "stage-4", BaseGeneration: 2, Outcome: "failure", + PhaseSince: time.Date(2026, 6, 24, 10, 1, 0, 0, time.UTC), + RunID: "run-1", + LineagePath: "r0", + ExactKey: "base-1|head-4", + BaseAnchor: "base-1", + DebugURL: "https://forge.example/runs/4", + Speculative: true, }}, LingerSince: time.Date(2026, 6, 24, 10, 0, 0, 0, time.UTC), BaseGeneration: 2, StagingSequence: 7, + Trees: []checkpoint.BisectionTreeSnapshot{{ + RunID: "run-1", + Anchor: "base-1", + Open: 1, + Accepted: []checkpoint.PullRequestSnapshot{{Number: 1, HeadSHA: "head-1"}}, + Results: map[string]string{"base-1|head-1": "success"}, + Held: []checkpoint.HeldLeafSnapshot{{ + Batch: checkpoint.ActiveBatchSnapshot{ + PRs: []checkpoint.PullRequestSnapshot{{Number: 2, HeadSHA: "head-2"}}, + StagingBranch: "mq/main/staging-2", + StagingSHA: "stage-2", + }, + Outcome: "failure", + }}, + }}, + TransitionOutbox: []checkpoint.OutboxTransitionSnapshot{{Kind: "replaced", PRs: []int{4}, EventID: "event-4"}}, } if err := store.SaveQueue(context.Background(), snapshot); err != nil { From d36abb48badaf2072be4381b6afade88b8c25c99 Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Sat, 5 Sep 2026 00:33:10 -0500 Subject: [PATCH 5/6] test: describe the local frontier model --- internal/engine/frontier_model_test.go | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/internal/engine/frontier_model_test.go b/internal/engine/frontier_model_test.go index d7e5524..e82bb93 100644 --- a/internal/engine/frontier_model_test.go +++ b/internal/engine/frontier_model_test.go @@ -9,17 +9,12 @@ import ( "testing" ) -// This file is the Go port of docs/architecture/bisection-frontier-model.py: -// an exhaustive, deterministic reference model for the ordered bisection -// frontier. It is executable design documentation. It explores every reachable -// Boolean-oracle path for queues of up to seven candidates and checks the -// invariants stated in docs/architecture/bisection-tree-finalization.md, so the -// engine's own traces can later be compared against a proven oracle without -// adding Python to the build. +// This file contains an exhaustive model of the ordered frontier. +// It checks all Boolean gate outcomes for queues of up to seven candidates. +// The tests check that the model preserves the order of queue decisions. // -// Candidates are identified by their queue index (0 = "A", 1 = "B", …). An -// exact test key is the ordered accepted accumulator followed by the candidate -// under test, encoded as a comma-joined string of indices. +// Candidates use their queue index. An exact key contains the accepted prefix +// and the candidate under test. The key uses comma-separated indexes. func frontierKeyOf(nums []int) string { parts := make([]string, len(nums)) From 80b60673bb949ef3a7130a483d34317634f65ce1 Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Sat, 5 Sep 2026 00:54:46 -0500 Subject: [PATCH 6/6] engine: reject stale checkpoint evidence --- docs/design.md | 50 ++++++++-------- internal/engine/checkpoint.go | 63 ++++++++++++++++---- internal/engine/engine.go | 80 ++++++++++--------------- internal/engine/engine_test.go | 81 ++++++++++++++++++-------- internal/engine/frontier_model_test.go | 6 +- 5 files changed, 163 insertions(+), 117 deletions(-) diff --git a/docs/design.md b/docs/design.md index c51ab75..66ea7c2 100644 --- a/docs/design.md +++ b/docs/design.md @@ -82,10 +82,10 @@ The engine saves queue state through `CheckpointStore`. It loads one snapshot before the first reconcile. It saves work after each reconcile. It deletes the snapshot when the queue is idle. -The production command uses bbolt when `SHUNT_STATE_PATH` is set. This is the -default durable store. Shunt can use Postgres when `SHUNT_POSTGRES_DSN` is set. -Postgres is an optional store for replicas that need a shared queue lease. -Without either setting, queue state exists only in the process. +bbolt is the recommended local durable store. Set `SHUNT_STATE_PATH` to use +bbolt. Set `SHUNT_POSTGRES_DSN` to use Postgres. Postgres is a supported store +for replicas that need a shared queue lease. Without either setting, queue +state exists only in the process. A Postgres replica acquires its queue lease before it loads state or calls the forge. Each `Reconcile()` call renews the lease. The call ends before half of @@ -199,29 +199,25 @@ else: mid = len/2 pending.push_front(nums[:mid], nums[mid:]) # test first half next ``` -Because candidates are just lists of PR numbers and staging is always rebuilt -from the *current* base tip, a successful sub-batch that advances the base is -handled safely: any later speculative staging run from the old base generation is -abandoned and re-queued, then re-staged before it can land. - -The same preflight protects active batches from PR updates. While a gate is -running, shunt rechecks every open PR head before accepting the result. During -landing, it rechecks each PR immediately before release. A changed head is -re-queued for fresh staging. A pull-request webhook wakes this path promptly; -polling remains the backstop for missed webhook deliveries. - -The neutral `checkpoint` package defines the snapshot DTOs. The engine owns the -consumer-side store interface, and the concrete bbolt implementation lives in -the more specific `checkpoint/bolt` package. Restored active batches are -conservatively re-queued for fresh staging instead of resuming an old staging -branch/run, so shunt does not release additional PRs from a result that may now -be stale. A PR released before the restart may still finish through the forge; -it was released only after a passing batch, and the remaining PRs are re-staged -on the resulting base. The production command wires the default bbolt -implementation when `SHUNT_STATE_PATH` is set. That store persists one snapshot -per `(owner, repo, base)` and keeps the binary static/CGO-free; -operators should place the database on persistent storage if they want queue -state to survive pod replacement or host reboots. +Each bisection root uses one fixed base commit. Shunt builds each root child +on this base commit. The exact gate key includes the base commit, merge style, +and ordered PR heads. Shunt discards a speculative result when its exact key no +longer matches the ordered frontier. + +Shunt checks each active PR head before it accepts a gate result. Shunt checks +again before it releases a PR. A changed head starts fresh staging. A pull +request webhook starts this check early. Polling starts the check if a webhook +is missed. + +The `checkpoint` package defines the snapshot types. The engine owns the store +interface. The `checkpoint/bolt` package provides the bbolt store. On restart, +Shunt resumes an active staging attempt only when its stored evidence is valid. +Otherwise, Shunt removes the staging branch and derives fresh queue work. A PR +that Shunt released before restart can still merge through the forge. + +The bbolt store saves one snapshot for each `(owner, repo, base)` queue. The +store keeps the binary static and free of CGO. Use persistent storage when queue +state must survive host or pod replacement. ### Worked example diff --git a/internal/engine/checkpoint.go b/internal/engine/checkpoint.go index 48236ce..245e4ae 100644 --- a/internal/engine/checkpoint.go +++ b/internal/engine/checkpoint.go @@ -227,14 +227,17 @@ func (e *Engine) applySnapshot(ctx context.Context, snapshot checkpoint.QueueSna return fmt.Errorf("queue checkpoint format %d is newer than this engine (%d)", snapshot.FormatVersion, checkpoint.CurrentFormatVersion) } if snapshot.FormatVersion < checkpoint.CurrentFormatVersion && (len(snapshot.Pending) > 0 || len(snapshot.Active) > 0) { - // A legacy checkpoint lacks the base anchor, accepted accumulator and - // lineage the ordered frontier needs to resume a partly-tested queue - // exactly. Do not error forever (that wedges the queue): discard the - // in-flight state and re-derive from the forge. Staged branches from - // the old attempt orphan and are cleaned up by the stale-branch sweep. - e.logger.Warn("discarding a legacy queue checkpoint with in-flight work; re-deriving the queue from the forge", - "format_version", snapshot.FormatVersion, "current", checkpoint.CurrentFormatVersion, - "pending", len(snapshot.Pending), "active", len(snapshot.Active)) + e.logger.Warn("discarding a legacy queue checkpoint with in-flight work", "format_version", snapshot.FormatVersion) + e.discardSnapshotBranches(ctx, snapshot) + snapshot = checkpoint.QueueSnapshot{FormatVersion: checkpoint.CurrentFormatVersion, Key: snapshot.Key} + } + stale, err := e.snapshotActiveStale(ctx, snapshot.Active) + if err != nil { + return err + } + if stale { + e.logger.Warn("discarding checkpoint with stale active evidence") + e.discardSnapshotBranches(ctx, snapshot) snapshot = checkpoint.QueueSnapshot{FormatVersion: checkpoint.CurrentFormatVersion, Key: snapshot.Key} } e.pending = clonePending(snapshot.Pending) @@ -259,10 +262,7 @@ func (e *Engine) applySnapshot(ctx context.Context, snapshot checkpoint.QueueSna return err } if len(prs) == 0 { - // Every PR landed during the downtime — remove the empty branch. - if err := e.fc.DeleteBranch(ctx, e.cfg.Owner, e.cfg.Repo, snap.StagingBranch); err != nil { - e.logger.Warn("resume: failed to delete drained staging branch", "branch", snap.StagingBranch, "error", err) - } + e.deleteStagingBranch(ctx, snap.StagingBranch) continue } active = append(active, e.restoreBatch(snap, prs)) @@ -317,6 +317,45 @@ func (e *Engine) applySnapshot(ctx context.Context, snapshot checkpoint.QueueSna // phaseForOutcome derives the batch phase from a persisted gate outcome so a // resumed batch flows through checkActive correctly. +func (e *Engine) snapshotActiveStale(ctx context.Context, active []checkpoint.ActiveBatchSnapshot) (bool, error) { + for _, batch := range active { + if !e.ownsStagingBranch(batch.StagingBranch) { + return true, nil + } + for _, saved := range batch.PRs { + pr, err := e.fc.GetPR(ctx, e.cfg.Owner, e.cfg.Repo, saved.Number) + if err != nil { + return false, fmt.Errorf("check checkpoint PR #%d: %w", saved.Number, err) + } + if pr.Merged { + continue + } + if pr.State != "open" || pr.Head.Sha != saved.HeadSHA { + return true, nil + } + state, err := e.fc.AutomergeState(ctx, e.cfg.Owner, e.cfg.Repo, saved.Number) + if err != nil { + return false, fmt.Errorf("check checkpoint PR #%d auto-merge: %w", saved.Number, err) + } + if !state.Scheduled { + return true, nil + } + } + } + return false, nil +} + +func (e *Engine) discardSnapshotBranches(ctx context.Context, snapshot checkpoint.QueueSnapshot) { + for _, batch := range snapshot.Active { + e.deleteStagingBranch(ctx, batch.StagingBranch) + } + for _, tree := range snapshot.Trees { + for _, leaf := range tree.Held { + e.deleteStagingBranch(ctx, leaf.Batch.StagingBranch) + } + } +} + func orNow(t, fallback time.Time) time.Time { if t.IsZero() { return fallback diff --git a/internal/engine/engine.go b/internal/engine/engine.go index d762397..13ed1d6 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -120,10 +120,8 @@ var ( nativeMergePoll = time.Second ) -// Stager builds an integration ("staging") branch from a base + PR head refs. -// baseAnchor, when non-empty, is an immutable commit SHA to build on instead -// of the live tip of base (see bisection-tree-finalization.md, -// "Immutable-base requirement"). +// Stager builds a staging branch from a base and PR head references. +// baseAnchor selects a fixed base commit when it is not empty. type Stager interface { BuildStaging(ctx context.Context, base, baseAnchor, stagingBranch string, refs []gitops.MergedRef) (sha string, conflictPR int, err error) } @@ -740,11 +738,8 @@ func (e *Engine) checkActive(ctx context.Context) (bool, error) { continue } if a.missingGateRetries >= missingGateMaxRetries { - // The design (bisection-tree-finalization.md, outcome - // table): a node whose gate never runs, after the retry - // budget, aborts the *root* with no source-PR mutation — - // an infra failure must not be published as a PR rejection. - // Tear the root down and re-queue every candidate fresh. + // A missing gate result cannot decide a source PR. + // Requeue the root when the retry limit is reached. if e.requeuedTreeNode(ctx, a, "staging gate produced no result after retries", "re-queued: the staging gate produced no result") { e.logger.Error("bisection root aborted: a node's gate never ran", "prs", numbersOf(a.prs), "runID", a.runID) @@ -843,13 +838,8 @@ func (e *Engine) checkActive(ctx context.Context) (bool, error) { e.requeueStaleActive(ctx, a) return true, nil } - // Fanout can stage a bisection node speculatively, before its left - // siblings have resolved. Its gate ran against an assumed accumulator; - // the result is authoritative only if its exact key still equals the - // key the resolved frontier now asks for. If a left sibling has since - // been accepted, the key differs and this node is superseded and - // re-staged on the real baseline (bisection-tree-finalization.md, - // "Speculative fanout"). + // A speculative result is valid only when its exact key still matches + // the resolved frontier. Re-stage the node when the keys differ. if a.exactKey != "" { if tree, ok := e.trees[a.runID]; ok { staged := append(append([]forge.PullRequest(nil), tree.accepted...), a.prs...) @@ -1492,11 +1482,7 @@ func (e *Engine) reRootFinalizationSuffix(ctx context.Context, runID string, tre for _, pr := range leaf.batch.prs { suffix[pr.Number] = true } - if leaf.batch.stagingBranch != "" { - if err := e.fc.DeleteBranch(ctx, e.cfg.Owner, e.cfg.Repo, leaf.batch.stagingBranch); err != nil { - e.logger.Warn("re-root: failed to delete held staging branch", "branch", leaf.batch.stagingBranch, "error", err) - } - } + e.deleteStagingBranch(ctx, leaf.batch.stagingBranch) e.recordTransition(Transition{Kind: "bisected", StagingBranch: leaf.batch.stagingBranch, RunID: runID, LineagePath: leaf.batch.lineagePath}) } nums := make([]int, 0, len(suffix)) @@ -1616,10 +1602,9 @@ func (e *Engine) activeLimit() int { // v1 was PR head SHAs only; v2 adds the base anchor and merge style. const frontierKeyVersion = 2 -// exactKey is the identity of one gate question: the base anchor, the merge -// style, and the ordered pinned PR heads of (accepted baseline + candidate). -// Different keys have no logical relationship even if their PR sets overlap -// (bisection-tree-finalization.md, "Formal model"). +// exactKey identifies one gate result. +// It includes the base anchor, merge style, and ordered PR heads. +// Results with different keys are independent. func (e *Engine) exactKey(anchor string, prs []forge.PullRequest) string { parts := make([]string, 0, len(prs)+3) parts = append(parts, "v"+strconv.Itoa(frontierKeyVersion), anchor, e.cfg.MergeStyle) @@ -1835,15 +1820,11 @@ func (e *Engine) requeueStaleActives(ctx context.Context) { } } -// invalidateAdvancedRoots re-reads the base branch head once per reconcile. -// If it has moved since a still-testing root pinned its anchor, that root's -// evidence was gathered against a base that no longer exists as tested, so the -// whole root is torn down and its candidates are re-queued as one fresh root -// on the new base. The design forbids inheriting any decision prefix across a -// base-anchor change, and no source-PR status, auto-merge release, or -// cancellation is published (bisection-tree-finalization.md, "Base and -// candidate invalidation"). Trees already in finalization (no unresolved node) -// are skipped: their base moves are the engine's own merges. +// invalidateAdvancedRoots checks each testing root against its base anchor. +// Shunt discards a root when the base changed. It requeues the root candidates. +// Shunt does not publish a source decision during this operation. +// A root in finalization has no unresolved nodes. Its base can move after a +// Shunt merge. func (e *Engine) invalidateAdvancedRoots(ctx context.Context) (bool, error) { testing := map[string]bool{} for runID := range e.trees { @@ -1989,11 +1970,7 @@ func (e *Engine) reRootPreservingPrefix(ctx context.Context, oldRunID string, ch for _, pr := range leaf.batch.prs { suffix[pr.Number] = true } - if leaf.batch.stagingBranch != "" { - if err := e.fc.DeleteBranch(ctx, e.cfg.Owner, e.cfg.Repo, leaf.batch.stagingBranch); err != nil { - e.logger.Warn("re-root: failed to delete discarded held branch", "branch", leaf.batch.stagingBranch, "error", err) - } - } + e.deleteStagingBranch(ctx, leaf.batch.stagingBranch) e.recordTransition(Transition{Kind: "bisected", StagingBranch: leaf.batch.stagingBranch, RunID: oldRunID, LineagePath: leaf.batch.lineagePath}) } for _, a := range append([]*activeBatch(nil), e.active...) { @@ -2089,11 +2066,7 @@ func (e *Engine) tearDownAndRequeueRoot(ctx context.Context, runID, reason, requ for _, leaf := range tree.held { add(numbersOf(leaf.batch.prs)...) e.recordTransition(Transition{Kind: "bisected", StagingBranch: leaf.batch.stagingBranch, RunID: runID, LineagePath: leaf.batch.lineagePath}) - if leaf.batch.stagingBranch != "" { - if err := e.fc.DeleteBranch(ctx, e.cfg.Owner, e.cfg.Repo, leaf.batch.stagingBranch); err != nil { - e.logger.Warn("re-root: failed to delete held staging branch", "branch", leaf.batch.stagingBranch, "error", err) - } - } + e.deleteStagingBranch(ctx, leaf.batch.stagingBranch) } delete(e.trees, runID) } @@ -2138,19 +2111,26 @@ func (e *Engine) removeActive(a *activeBatch) { } } -// cleanupBatch removes the batch from the active list and deletes its staging -// branch. Called on every path that finishes with a batch (land, skip, bounce). +// cleanupBatch removes a batch and its Shunt-owned staging branch. func (e *Engine) cleanupBatch(ctx context.Context, a *activeBatch) { e.removeActive(a) - if a.stagingBranch == "" { + e.deleteStagingBranch(ctx, a.stagingBranch) +} + +func (e *Engine) deleteStagingBranch(ctx context.Context, branch string) { + if !e.ownsStagingBranch(branch) { + e.logger.Warn("not deleting a branch outside the staging namespace", "branch", branch) return } - if err := e.fc.DeleteBranch(ctx, e.cfg.Owner, e.cfg.Repo, a.stagingBranch); err != nil { - e.logger.Warn("failed to delete staging branch", - "branch", a.stagingBranch, "error", err) + if err := e.fc.DeleteBranch(ctx, e.cfg.Owner, e.cfg.Repo, branch); err != nil { + e.logger.Warn("failed to delete staging branch", "branch", branch, "error", err) } } +func (e *Engine) ownsStagingBranch(branch string) bool { + return e.cfg.StagingBranch != "" && strings.HasPrefix(branch, e.cfg.StagingBranch+"-") +} + func firstPR(prs []forge.PullRequest) int { if len(prs) == 0 { return 0 diff --git a/internal/engine/engine_test.go b/internal/engine/engine_test.go index 89bd395..8508fde 100644 --- a/internal/engine/engine_test.go +++ b/internal/engine/engine_test.go @@ -789,13 +789,8 @@ func TestBisectionRestagesRightAfterRejectedLeft(t *testing.T) { } } -// TestEngineFrontierWorkedExample drives the real engine through the A..G -// scenario from bisection-tree-finalization.md and asserts its trace matches -// the reference model: ABC fails, A passes, the A+BC key is a cache hit (no -// re-stage), B and C each fail on the A baseline, DEFG passes on the A -// baseline. Six distinct staging calls; accept A,D,E,F,G; reject B,C; every -// source decision deferred until the whole root is terminal, then applied in -// queue order. +// TestEngineFrontierWorkedExample checks a seven-PR ordered frontier. +// The test checks cached results, ordered decisions, and held leaves. func TestEngineFrontierWorkedExample(t *testing.T) { m := newMock(-1, 1, 2, 3, 4, 5, 6, 7) pass := map[string]bool{ @@ -822,10 +817,8 @@ func TestEngineFrontierWorkedExample(t *testing.T) { } } -// TestFrontierExactKeyScopedToAnchorAndMergeStyle proves an exact key is not a -// bare list of PR heads: the base anchor and merge style are part of it, so a -// cached outcome can never be reused across a re-rooted queue or a config -// change (bisection-tree-finalization.md, "Formal model"). +// TestFrontierExactKeyScopedToAnchorAndMergeStyle checks exact-key inputs. +// A cached result cannot cross a base anchor or merge-style change. func TestFrontierExactKeyScopedToAnchorAndMergeStyle(t *testing.T) { prs := []forge.PullRequest{{Number: 1}, {Number: 2}} prs[0].Head.Sha = "h1" @@ -882,15 +875,8 @@ func TestBisectionAnchorPinnedAndDurable(t *testing.T) { } } -// TestRootInvalidatedWhenBaseAdvancesMidTest: an external advance of the base -// branch during testing tears the whole root down with no source-PR decision -// and re-queues its candidates as one fresh root on the new base -// (bisection-tree-finalization.md, "Base and candidate invalidation"). -// TestFinalizationAbortsWhenBaseAdvancesBeforeAnyLanding: a ready root that has -// only bounced so far (bounces do not move main) sees the base advance under -// it; the already-bounced PRs stay bounced and the unperformed suffix is -// re-queued on current main (bisection-tree-finalization.md, "Base and -// candidate invalidation"). +// TestFinalizationAbortsWhenBaseAdvancesBeforeAnyLanding checks base changes. +// Shunt keeps completed bounces. Shunt requeues the unresolved suffix. func TestFinalizationAbortsWhenBaseAdvancesBeforeAnyLanding(t *testing.T) { m := newMock(-1, 1, 2, 3) m.branchHead = "base-v1" @@ -1104,10 +1090,8 @@ func TestRootInvalidatedWhenAcceptedCandidateHeadChanges(t *testing.T) { } } -// TestSuccessorRootPreservesEvidencedPrefix: when a held candidate that is not -// the first changes, the successor root carries the held decisions strictly to -// its left and re-resolves only the suffix (bisection-tree-finalization.md, -// "Successor roots and preserved prefixes"). +// TestSuccessorRootPreservesEvidencedPrefix checks a changed held candidate. +// The successor root keeps prior evidence and rechecks the later suffix. func TestSuccessorRootPreservesEvidencedPrefix(t *testing.T) { m := newMock(-1, 1, 2, 3, 4, 5) // [1 2] passes as a group; everything with 3, 4, or 5 fails on that @@ -1544,6 +1528,55 @@ func TestCheckpointRestoresActiveBatchByRestaging(t *testing.T) { } } +func TestCheckpointDiscardsChangedActiveEvidence(t *testing.T) { + m := newMock(-1, 1, 2) + store := &memoryCheckpointStore{} + cfg := Config{Owner: "o", Repo: "r", Base: "main", StatusCtx: "merge-queue", StagingBranch: "mq/main/staging", Checkpoint: store} + e := New(cfg, m, m) + if err := e.Reconcile(context.Background()); err != nil { + t.Fatalf("start batch: %v", err) + } + staleBranch := m.stagingBranches[0] + m.prs[1].Head.Sha = "head-1-new" + + restarted := New(cfg, m, m) + if err := restarted.Reconcile(context.Background()); err != nil { + t.Fatalf("restore changed batch: %v", err) + } + if got := fmt.Sprint(m.staged); got != "[[1 2] [1 2]]" { + t.Fatalf("staged = %s, want fresh staging after the head changed", got) + } + if got := fmt.Sprint(m.calls); !strings.Contains(got, "delete:"+staleBranch) { + t.Fatalf("calls = %s, want stale staging branch deleted", got) + } + if got := fmt.Sprint(m.statuses); got != "[]" { + t.Fatalf("statuses = %s, want no release from stale evidence", got) + } +} + +func TestCheckpointDoesNotDeleteBranchOutsideStagingNamespace(t *testing.T) { + m := newMock(-1, 1) + store := &memoryCheckpointStore{saved: &checkpoint.QueueSnapshot{ + FormatVersion: checkpoint.CurrentFormatVersion, + Key: checkpoint.QueueKey{Owner: "o", Repo: "r", Base: "main"}, + Active: []checkpoint.ActiveBatchSnapshot{{ + PRs: []checkpoint.PullRequestSnapshot{{Number: 1, HeadSHA: "head-1"}}, + StagingBranch: "main", + StagingSHA: "stage-1", + }}, + }} + cfg := Config{Owner: "o", Repo: "r", Base: "main", StatusCtx: "merge-queue", StagingBranch: "mq/main/staging", Checkpoint: store} + if err := New(cfg, m, m).Reconcile(context.Background()); err != nil { + t.Fatalf("restore queue: %v", err) + } + if got := fmt.Sprint(m.calls); strings.Contains(got, "delete:main") { + t.Fatalf("calls = %s, must not delete a branch outside the staging namespace", got) + } + if got := fmt.Sprint(m.staged); got != "[[1]]" { + t.Fatalf("staged = %s, want fresh staging", got) + } +} + func TestCheckpointRestartResumesRemainderAfterReleasedPRMerges(t *testing.T) { m := newMock(-1, 1, 2) store := &memoryCheckpointStore{} diff --git a/internal/engine/frontier_model_test.go b/internal/engine/frontier_model_test.go index e82bb93..e2de35b 100644 --- a/internal/engine/frontier_model_test.go +++ b/internal/engine/frontier_model_test.go @@ -258,10 +258,8 @@ func TestFrontierModelExhaustive(t *testing.T) { } } -// TestFrontierModelWorkedExample is the A…G scenario from -// bisection-tree-finalization.md: ABC fails, A passes, B and C each fail on the -// A baseline, DEFG passes on the A baseline. Exactly six distinct CI runs, and -// the ABC key is looked up twice (once tested, once as the A+BC cache hit). +// TestFrontierModelWorkedExample checks a seven-candidate frontier. +// It checks gate results, exact-key reuse, and the number of gate runs. func TestFrontierModelWorkedExample(t *testing.T) { outcomes := map[string]bool{ "0,1,2,3,4,5,6": false,