diff --git a/internal/branchsync/recover_test.go b/internal/branchsync/recover_test.go index 99481251e..9f3e1a17c 100644 --- a/internal/branchsync/recover_test.go +++ b/internal/branchsync/recover_test.go @@ -1494,6 +1494,252 @@ func (f *recoverFixture) localAnchorRef() string { return "refs/no-mistakes/recover-local/" + f.run.ID } +func newTerminalEqualTreeRewriteFixture(t *testing.T) (*recoverFixture, string) { + t.Helper() + f := newRebasedRecoverFixture(t, types.RunFailed) + pipeline := filepath.Join(filepath.Dir(f.local), "terminal-rewrite") + mustRun(t, filepath.Dir(f.local), "-c", "core.autocrlf=false", "clone", f.gate, pipeline) + configureIdentity(t, pipeline) + mustRun(t, pipeline, "checkout", "--detach", f.submitted) + mustWrite(t, filepath.Join(pipeline, "feature.txt"), "pipeline reviewed replacement\n") + mustRun(t, pipeline, "commit", "-am", "reviewed pipeline result") + recorded := mustRun(t, pipeline, "rev-parse", "HEAD") + tree := mustRun(t, pipeline, "rev-parse", recorded+"^{tree}") + live := mustRun(t, pipeline, "commit-tree", tree, "-p", "origin/main", "-m", "terminal equal-tree rewrite") + mustRun(t, pipeline, "push", "origin", recorded+":refs/no-mistakes/test-recorded") + mustRun(t, pipeline, "push", "--force", "origin", live+":refs/heads/feature/recover") + mustRun(t, f.gate, "update-ref", "-d", "refs/no-mistakes/test-recorded") + if err := f.db.UpdateRunHeadSHA(f.run.ID, recorded); err != nil { + t.Fatal(err) + } + if err := f.db.UpdateRunReviewApprovedHeadSHA(f.run.ID, recorded); err != nil { + t.Fatal(err) + } + if err := f.db.UpdateRunErrorStatus(f.run.ID, "terminal worker lost", types.RunFailed); err != nil { + t.Fatal(err) + } + f.preserved = live + f.run, _ = f.db.GetRun(f.run.ID) + return f, recorded +} + +func TestRecoverTerminalUnverifiedEqualTreeRewriteAdoptsLiveGateHead(t *testing.T) { + t.Parallel() + + f, recorded := newTerminalEqualTreeRewriteFixture(t) + if f.run.TerminalHeadVerifiedAt != nil { + t.Fatal("fixture terminal head is already verified") + } + if isAncestor(f.ctx, f.gate, recorded, f.preserved) || isAncestor(f.ctx, f.gate, f.preserved, recorded) { + t.Fatal("fixture heads are not non-ancestral") + } + if got, want := mustRun(t, f.gate, "rev-parse", recorded+"^{tree}"), mustRun(t, f.gate, "rev-parse", f.preserved+"^{tree}"); got != want { + t.Fatalf("fixture trees differ: %s != %s", got, want) + } + if !isAncestor(f.ctx, f.gate, f.submitted, recorded) { + t.Fatal("recorded reviewed head does not descend from submitted local head") + } + if preservedContainsLocalWork(f.ctx, f.gate, f.submitted, f.preserved) { + t.Fatal("fixture does not reproduce the direct live-head containment conflict") + } + if f.run.ReviewApprovedHeadSHA == nil || *f.run.ReviewApprovedHeadSHA != recorded { + t.Fatalf("review authority = %#v, want %s", f.run.ReviewApprovedHeadSHA, recorded) + } + runsBefore := len(mustRuns(t, f.db, f.repo.ID)) + remoteBefore := mustRun(t, f.remote, "for-each-ref", "--format=%(refname) %(objectname)", "refs/heads") + + state := f.service.Recover(f.ctx, false) + if !state.Recovered || !state.Changed || state.State != StateCustodyReturned { + t.Fatalf("equal-tree terminal recovery = %#v", state) + } + if got := mustRun(t, f.local, "rev-parse", "HEAD"); got != f.preserved { + t.Fatalf("HEAD = %s, want live gate head %s", got, f.preserved) + } + if got := mustRun(t, f.gate, "rev-parse", f.anchorRef()); got != f.preserved { + t.Fatalf("gate recovery anchor = %s, want %s", got, f.preserved) + } + if got := mustRun(t, f.local, "rev-parse", f.anchorRef()); got != f.preserved { + t.Fatalf("local recovery anchor = %s, want %s", got, f.preserved) + } + if got := mustRun(t, f.local, "rev-parse", f.localAnchorRef()); got != f.submitted { + t.Fatalf("local pre-recovery anchor = %s, want %s", got, f.submitted) + } + run, err := f.db.GetRun(f.run.ID) + if err != nil || run.HeadSHA != f.preserved || run.TerminalHeadVerifiedAt == nil || run.CustodyReturnedAt == nil || run.UpdatedAt < f.run.UpdatedAt { + t.Fatalf("recovered run = %#v, err %v", run, err) + } + if clean, reason := worktreeClean(f.ctx, f.local); !clean { + t.Fatalf("worktree not clean after recovery: %s", reason) + } + if second := f.service.Recover(f.ctx, false); !second.Recovered || second.Changed { + t.Fatalf("repeated recovery = %#v", second) + } + if got := len(mustRuns(t, f.db, f.repo.ID)); got != runsBefore { + t.Fatalf("recovery changed run count from %d to %d", runsBefore, got) + } + if got := mustRun(t, f.remote, "for-each-ref", "--format=%(refname) %(objectname)", "refs/heads"); got != remoteBefore { + t.Fatalf("recovery pushed upstream refs: %q != %q", got, remoteBefore) + } +} + +func mustRuns(t *testing.T, database *db.DB, repoID string) []*db.Run { + t.Helper() + runs, err := database.GetRunsByRepo(repoID) + if err != nil { + t.Fatal(err) + } + return runs +} + +func pushTerminalRewrite(t *testing.T, f *recoverFixture, mutate func(string)) string { + t.Helper() + pipeline := filepath.Join(t.TempDir(), "rewrite") + mustRun(t, filepath.Dir(pipeline), "-c", "core.autocrlf=false", "clone", f.gate, pipeline) + configureIdentity(t, pipeline) + mustRun(t, pipeline, "checkout", "feature/recover") + mutate(pipeline) + mustRun(t, pipeline, "add", "-A") + mustRun(t, pipeline, "commit", "-m", "terminal changed-tree rewrite") + live := mustRun(t, pipeline, "rev-parse", "HEAD") + mustRun(t, pipeline, "push", "--force", "origin", "HEAD:refs/heads/feature/recover") + f.preserved = live + return live +} + +func assertUnverifiedRecoveryRefusalNoMutation(t *testing.T, f *recoverFixture) State { + t.Helper() + localRefs := mustRun(t, f.local, "for-each-ref", "--format=%(refname) %(objectname)", "refs/no-mistakes") + gateRefs := mustRun(t, f.gate, "for-each-ref", "--format=%(refname) %(objectname)", "refs/no-mistakes") + runBefore, err := f.db.GetRun(f.run.ID) + if err != nil { + t.Fatal(err) + } + state := f.service.Recover(f.ctx, false) + if state.Recovered || state.Changed { + t.Fatalf("unsafe recovery succeeded: %#v", state) + } + if got := mustRun(t, f.local, "for-each-ref", "--format=%(refname) %(objectname)", "refs/no-mistakes"); got != localRefs { + t.Fatalf("refusal changed local recovery refs: %q != %q", got, localRefs) + } + if got := mustRun(t, f.gate, "for-each-ref", "--format=%(refname) %(objectname)", "refs/no-mistakes"); got != gateRefs { + t.Fatalf("refusal changed gate recovery refs: %q != %q", got, gateRefs) + } + runAfter, err := f.db.GetRun(f.run.ID) + if err != nil || runAfter.HeadSHA != runBefore.HeadSHA || ptr(runAfter.ReviewApprovedHeadSHA) != ptr(runBefore.ReviewApprovedHeadSHA) || runAfter.TerminalHeadVerifiedAt != nil || runAfter.CustodyReturnedAt != nil || runAfter.UpdatedAt != runBefore.UpdatedAt { + t.Fatalf("refusal changed run: before %#v after %#v err %v", runBefore, runAfter, err) + } + return state +} + +func TestRecoverTerminalUnverifiedRewriteNegativeControls(t *testing.T) { + t.Parallel() + + t.Run("different tree", func(t *testing.T) { + f, _ := newTerminalEqualTreeRewriteFixture(t) + pushTerminalRewrite(t, f, func(dir string) { mustWrite(t, filepath.Join(dir, "pipeline.txt"), "different tree\n") }) + if state := assertUnverifiedRecoveryRefusalNoMutation(t, f); state.Safety != "blocked_recover_unverified_head" { + t.Fatalf("different-tree refusal = %s", state.Safety) + } + if got := mustRun(t, f.local, "rev-parse", "HEAD"); got != f.submitted { + t.Fatalf("different-tree refusal moved HEAD to %s", got) + } + }) + t.Run("drops local hunk", func(t *testing.T) { + f, _ := newTerminalEqualTreeRewriteFixture(t) + mustRun(t, f.gate, "update-ref", "refs/heads/feature/recover", mustRun(t, f.gate, "rev-parse", "refs/heads/main")) + if state := assertUnverifiedRecoveryRefusalNoMutation(t, f); state.Safety != "blocked_recover_unverified_head" { + t.Fatalf("dropped-work refusal = %s", state.Safety) + } + }) + t.Run("rewrites operator line", func(t *testing.T) { + f, _ := newTerminalEqualTreeRewriteFixture(t) + pushTerminalRewrite(t, f, func(dir string) { mustWrite(t, filepath.Join(dir, "feature.txt"), "pipeline replacement\n") }) + if state := assertUnverifiedRecoveryRefusalNoMutation(t, f); state.Safety != "blocked_recover_unverified_head" { + t.Fatalf("rewritten-line refusal = %s", state.Safety) + } + }) + t.Run("conflicting create-only anchor", func(t *testing.T) { + f, recorded := newTerminalEqualTreeRewriteFixture(t) + mustRun(t, f.gate, "update-ref", f.anchorRef(), recorded) + if state := assertUnverifiedRecoveryRefusalNoMutation(t, f); state.Safety != "blocked_recover_anchor_mismatch" { + t.Fatalf("anchor-conflict refusal = %s", state.Safety) + } + }) + t.Run("untrusted recorded head", func(t *testing.T) { + f, _ := newTerminalEqualTreeRewriteFixture(t) + if err := f.db.UpdateRunReviewApprovedHeadSHA(f.run.ID, f.submitted); err != nil { + t.Fatal(err) + } + if state := assertUnverifiedRecoveryRefusalNoMutation(t, f); state.Safety != "blocked_recover_unverified_head" { + t.Fatalf("untrusted-head refusal = %s", state.Safety) + } + }) + t.Run("recorded head does not preserve local", func(t *testing.T) { + f, _ := newTerminalEqualTreeRewriteFixture(t) + pipeline := filepath.Join(t.TempDir(), "uncontained-recorded") + mustRun(t, filepath.Dir(pipeline), "-c", "core.autocrlf=false", "clone", f.gate, pipeline) + configureIdentity(t, pipeline) + mustRun(t, pipeline, "checkout", "--detach", "origin/main") + mustWrite(t, filepath.Join(pipeline, "feature.txt"), "recorded without local work\n") + mustRun(t, pipeline, "add", "feature.txt") + mustRun(t, pipeline, "commit", "-m", "uncontained recorded result") + recorded := mustRun(t, pipeline, "rev-parse", "HEAD") + tree := mustRun(t, pipeline, "rev-parse", "HEAD^{tree}") + live := mustRun(t, pipeline, "commit-tree", tree, "-p", "origin/main", "-m", "uncontained live rewrite") + mustRun(t, pipeline, "push", "origin", recorded+":refs/no-mistakes/test-recorded") + mustRun(t, pipeline, "push", "--force", "origin", live+":refs/heads/feature/recover") + mustRun(t, f.gate, "update-ref", "-d", "refs/no-mistakes/test-recorded") + if err := f.db.UpdateRunHeadSHA(f.run.ID, recorded); err != nil { + t.Fatal(err) + } + if err := f.db.UpdateRunReviewApprovedHeadSHA(f.run.ID, recorded); err != nil { + t.Fatal(err) + } + if state := assertUnverifiedRecoveryRefusalNoMutation(t, f); state.Safety != "blocked_recover_unverified_head" { + t.Fatalf("uncontained-recorded refusal = %s", state.Safety) + } + }) + t.Run("symbolic live branch", func(t *testing.T) { + f, _ := newTerminalEqualTreeRewriteFixture(t) + mustRun(t, f.gate, "update-ref", "refs/no-mistakes/symbolic-live", f.preserved) + mustRun(t, f.gate, "symbolic-ref", "refs/heads/feature/recover", "refs/no-mistakes/symbolic-live") + if state := assertUnverifiedRecoveryRefusalNoMutation(t, f); state.Safety != "blocked_recover_unverified_head" { + t.Fatalf("symbolic-live refusal = %s", state.Safety) + } + }) + t.Run("active run", func(t *testing.T) { + f, _ := newTerminalEqualTreeRewriteFixture(t) + if err := f.db.UpdateRunStatus(f.run.ID, types.RunRunning); err != nil { + t.Fatal(err) + } + if state := assertUnverifiedRecoveryRefusalNoMutation(t, f); state.Safety != "blocked_recover_run_active" { + t.Fatalf("active-run refusal = %s", state.Safety) + } + }) + t.Run("dirty worktree", func(t *testing.T) { + f, _ := newTerminalEqualTreeRewriteFixture(t) + mustWrite(t, filepath.Join(f.local, "feature.txt"), "dirty local edit\n") + if state := assertUnverifiedRecoveryRefusalNoMutation(t, f); state.Safety != "blocked_recover_unverified_head" { + t.Fatalf("dirty-worktree refusal = %s", state.Safety) + } + if got := readOptional(t, filepath.Join(f.local, "feature.txt")); got != "dirty local edit\n" { + t.Fatalf("dirty worktree changed: %q", got) + } + }) + t.Run("racing local branch", func(t *testing.T) { + f, _ := newTerminalEqualTreeRewriteFixture(t) + f.service.beforeRecoverTerminalHeadPreserve = func() { + mustRun(t, f.local, "checkout", "-b", "racing-branch", f.submitted) + } + if state := assertUnverifiedRecoveryRefusalNoMutation(t, f); state.Safety != "blocked_recover_assumptions_changed" { + t.Fatalf("branch-race refusal = %s", state.Safety) + } + if got := mustRun(t, f.local, "branch", "--show-current"); got != "racing-branch" { + t.Fatalf("racing branch was changed to %s", got) + } + }) +} + // TestRecoverRebasedPreservedHeadAdoptsWithoutEscalating is the regression for // the over-escalating custody return: a cancelled validation whose preserved // pipeline head is the operator's own work rebased onto a newer base loses diff --git a/internal/branchsync/sync.go b/internal/branchsync/sync.go index 52e040a13..d1d9a292e 100644 --- a/internal/branchsync/sync.go +++ b/internal/branchsync/sync.go @@ -154,11 +154,12 @@ type Service struct { lsRemote func(context.Context, string, string, string) (string, error) fetchRemote func(context.Context, string, string, string, string) error - beforeApply func() - beforeGateReset func() - beforeRecoverWorktreeMove func() - beforeRecoverBranchMove func() - afterRecoverBranchMove func() + beforeApply func() + beforeGateReset func() + beforeRecoverTerminalHeadPreserve func() + beforeRecoverWorktreeMove func() + beforeRecoverBranchMove func() + afterRecoverBranchMove func() } // remoteTimeout returns the bounded deadline budget for one remote @@ -544,7 +545,12 @@ func (s *Service) Apply(ctx context.Context) State { // gate is available; otherwise the preserved head is verified through the // gate's run-specific recovery ref and fetched into that anchor. Legacy terminal // heads that still exist as unreferenced gate objects are anchored before -// recovery continues. The branch ref may independently lag or advance. +// recovery continues. A non-descendant live gate head is accepted only when +// the recorded head is the exact reviewed result, that reviewed result +// preserves the local work, and both commits have the same final tree. The +// live rewrite is not merged with local again because its content has +// already been proven identical to the reviewed result. The branch ref may +// independently lag or advance. // - The only possible worktree mutation is a guarded move of a clean checked-out // branch: a strict fast-forward, or an anchored move to a proven-containing // head performed by Git operations that refuse on their own rather than by a @@ -595,13 +601,56 @@ func (s *Service) Recover(ctx context.Context, keepLocal bool) State { return blockedPlan(state, StatePipelineOwned, "blocked_recover_unverified_head", "the terminal run has no verified head and the preserved gate head could not be read; no files or refs were changed") } if gateHead != run.HeadSHA { + equalTreeRewrite := false if !isAncestor(ctx, s.GateDir, run.HeadSHA, gateHead) { - return blockedPlan(state, StatePipelineOwned, "blocked_recover_unverified_head", "the terminal run has no verified head and the gate head does not descend from the recorded head; no files or refs were changed") + recordedTree, recordedTreeErr := git.Run(ctx, s.GateDir, "rev-parse", run.HeadSHA+"^{tree}") + gateTree, gateTreeErr := git.Run(ctx, s.GateDir, "rev-parse", gateHead+"^{tree}") + recordedPreservesLocal := isAncestor(ctx, s.GateDir, state.Local.Head, run.HeadSHA) || preservedContainsLocalWork(ctx, s.GateDir, state.Local.Head, run.HeadSHA) + if recordedTreeErr != nil || gateTreeErr != nil || recordedTree != gateTree || !state.Local.Clean || + run.ReviewApprovedHeadSHA == nil || *run.ReviewApprovedHeadSHA != run.HeadSHA || !recordedPreservesLocal { + return blockedPlan(state, StatePipelineOwned, "blocked_recover_unverified_head", "the terminal run has no verified head and the gate head does not descend from the recorded reviewed head with identical final content; no files or refs were changed") + } + if symbolic, err := git.Run(ctx, s.GateDir, "symbolic-ref", "-q", "refs/heads/"+branch); err == nil && symbolic != "" { + return blockedPlan(state, StatePipelineOwned, "blocked_recover_unverified_head", "the live gate branch is symbolic, so its terminal head cannot be verified; no files or refs were changed") + } + equalTreeRewrite = true + anchorRef := custody.RecoveryRef(run.ID) + if symbolic, err := git.Run(ctx, s.GateDir, "symbolic-ref", "-q", anchorRef); err == nil && symbolic != "" { + return blockedPlan(state, StatePipelineOwned, "blocked_recover_anchor_mismatch", "the run recovery ref in the local gate conflicts with the live gate head; no files or refs were changed") + } + anchored, anchorExists, anchorErr := git.ExactRefTarget(ctx, s.GateDir, anchorRef) + if anchorErr != nil || (anchorExists && anchored != gateHead) { + return blockedPlan(state, StatePipelineOwned, "blocked_recover_anchor_mismatch", "the run recovery ref in the local gate conflicts with the live gate head; no files or refs were changed") + } + if s.beforeRecoverTerminalHeadPreserve != nil { + s.beforeRecoverTerminalHeadPreserve() + } + branchNow, branchErr := git.CurrentBranch(ctx, s.workDir()) + headNow, headErr := git.HeadSHA(ctx, s.workDir()) + cleanNow, _ := worktreeClean(ctx, s.workDir()) + if branchErr != nil || branchNow != state.Local.Branch || headErr != nil || headNow != state.Local.Head || !cleanNow { + return blockedPlan(state, StatePipelineOwned, "blocked_recover_assumptions_changed", "the local branch or worktree changed while the terminal head was being verified; no files or refs were changed") + } + anchorCommand := fmt.Sprintf("create %s %s\n", anchorRef, gateHead) + if anchorExists { + anchorCommand = fmt.Sprintf("verify %s %s\n", anchorRef, gateHead) + } + transaction := fmt.Sprintf("start\nverify refs/heads/%s %s\n%sprepare\ncommit\n", branch, gateHead, anchorCommand) + if _, err := git.RunWithInput(ctx, s.GateDir, transaction, "update-ref", "--stdin", "--no-deref"); err != nil { + return blockedPlan(state, StatePipelineOwned, "blocked_recover_assumptions_changed", "the live gate head or its create-only recovery ref changed while the terminal head was being verified; no files or refs were changed") + } } - if err := s.DB.UpdateRunHeadSHA(run.ID, gateHead); err != nil { - return blockedPlan(state, StatePipelineOwned, "blocked_recover_unverified_head", "the verified gate head could not be preserved; no files or refs were changed") + if equalTreeRewrite { + updated, err := s.DB.VerifyTerminalRunHeadRewrite(run.ID, run.Status, run.HeadSHA, gateHead) + if err != nil || !updated { + return blockedPlan(state, StatePipelineOwned, "blocked_recover_assumptions_changed", "the terminal run or its recorded review authority changed while the live gate head was being verified; the preserved recovery ref remains available and custody was not returned") + } + } else if err := s.DB.UpdateRunStatusWithVerifiedHead(run.ID, run.Status, gateHead); err != nil { + return blockedPlan(state, StatePipelineOwned, "blocked_recover_unverified_head", "the verified gate head could not be recorded; the preserved recovery ref remains available and custody was not returned") } run.HeadSHA = gateHead + now := time.Now().Unix() + run.TerminalHeadVerifiedAt = &now state.Pipeline.CurrentHead = gateHead state.Relation = relationBetween(ctx, s.workDir(), state.Local.Head, gateHead) } @@ -611,6 +660,8 @@ func (s *Service) Recover(ctx context.Context, keepLocal bool) State { branch := state.Local.Branch local := state.Local.Head preserved := run.HeadSHA + trustedEqualTreeRewrite := run.TerminalHeadVerifiedAt != nil && run.ReviewApprovedHeadSHA != nil && *run.ReviewApprovedHeadSHA != preserved && + reviewedHeadProvesEquivalentTarget(ctx, s.GateDir, local, *run.ReviewApprovedHeadSHA, preserved) anchorRef := custody.RecoveryRef(run.ID) localAnchor := custody.RecoveryLocalRef(run.ID) gateDir := strings.TrimSpace(s.GateDir) @@ -708,14 +759,14 @@ func (s *Service) Recover(ctx context.Context, keepLocal bool) State { } return s.recoverKeepLocal(ctx, run, state, gateHead) } - if preservedContainsLocalWork(ctx, wd, local, preserved) { + if trustedEqualTreeRewrite || preservedContainsLocalWork(ctx, wd, local, preserved) { if !state.Local.Clean { state.Relation = RelationDiverged blocked := blockedPlan(state, StatePipelineOwned, "blocked_recover_dirty", fmt.Sprintf("the invoking worktree is not clean (%s); commit or stash first and re-run the recovery, or use --keep-local to return custody at the current head without moving the worktree; no files or refs were changed", state.Local.Reason)) blocked.NextAction = &NextAction{Code: "inspect_worktree", Command: "git status"} return blocked } - return s.recoverAdoptPreserved(ctx, run, state, preserved) + return s.recoverAdoptPreserved(ctx, run, state, preserved, trustedEqualTreeRewrite) } state.Relation = RelationDiverged blocked := blockedPlan(state, StatePipelineOwned, "blocked_recover_diverged", fmt.Sprintf("the local branch and the preserved pipeline head have diverged; the preserved commits are anchored at %s - reconcile manually and re-run the recovery, run `no-mistakes rerun` to resume validating the preserved head, or use --keep-local to keep the current head; no files or refs were changed", anchorRef)) @@ -848,6 +899,13 @@ func preservedContainsLocalWork(ctx context.Context, dir, local, preserved strin return mergeTreePreservesFinalHead(ctx, dir, base, local, preserved) } +func reviewedHeadProvesEquivalentTarget(ctx context.Context, dir, local, reviewed, target string) bool { + reviewedTree, reviewedErr := git.Run(ctx, dir, "rev-parse", reviewed+"^{tree}") + targetTree, targetErr := git.Run(ctx, dir, "rev-parse", target+"^{tree}") + return reviewedErr == nil && targetErr == nil && reviewedTree == targetTree && + (isAncestor(ctx, dir, local, reviewed) || preservedContainsLocalWork(ctx, dir, local, reviewed)) +} + // recoverAdoptPreserved returns custody for a preserved pipeline head that // already carries every local change. The local commits are represented in the // preserved head, but their exact SHAs are not reachable from it, so the move is @@ -874,7 +932,7 @@ func preservedContainsLocalWork(ctx context.Context, dir, local, preserved strin // uncommitted changes and loses nothing: containment was proven before the move // and the pre-recovery head stays anchored. Custody is stamped only after the // whole move is verified. -func (s *Service) recoverAdoptPreserved(ctx context.Context, run *db.Run, state State, preserved string) State { +func (s *Service) recoverAdoptPreserved(ctx context.Context, run *db.Run, state State, preserved string, containmentProven bool) State { if s.beforeRecoverWorktreeMove != nil { s.beforeRecoverWorktreeMove() } @@ -887,7 +945,7 @@ func (s *Service) recoverAdoptPreserved(ctx context.Context, run *db.Run, state } // The containment proof runs before the anchor and the move so that no // slow work sits between the last guard and the mutation. - if !preservedContainsLocalWork(ctx, wd, head, preserved) { + if !containmentProven && !preservedContainsLocalWork(ctx, wd, head, preserved) { return blockedPlan(state, StatePipelineOwned, "blocked_recover_assumptions_changed", "the containment proof changed while custody was being returned; no files or refs were changed") } localAnchor := recoverLocalAnchorRef(run.ID) diff --git a/internal/db/run.go b/internal/db/run.go index 46f65cc08..639bab47b 100644 --- a/internal/db/run.go +++ b/internal/db/run.go @@ -654,6 +654,16 @@ func (d *DB) UpdateRunStatusWithVerifiedHead(id string, status types.RunStatus, return nil } +func (d *DB) VerifyTerminalRunHeadRewrite(id string, status types.RunStatus, recordedHead, liveHead string) (bool, error) { + ts := now() + result, err := d.sql.Exec(`UPDATE runs SET head_sha = ?, push_active = 0, terminal_head_verified_at = ?, updated_at = ? WHERE id = ? AND status = ? AND head_sha = ? AND review_approved_head_sha = ? AND terminal_head_verified_at IS NULL AND custody_returned_at IS NULL`, liveHead, ts, ts, id, status, recordedHead, recordedHead) + if err != nil { + return false, fmt.Errorf("verify terminal run head rewrite: %w", err) + } + updated, err := result.RowsAffected() + return updated == 1, err +} + // RecordRunTerminalHeadEvidence records a managed worktree head that was // verified immediately before crash recovery makes the run terminal. The // subsequent stale-run status transition deliberately preserves this stamp. diff --git a/internal/db/run_test.go b/internal/db/run_test.go index c19133715..17b56fa95 100644 --- a/internal/db/run_test.go +++ b/internal/db/run_test.go @@ -560,6 +560,44 @@ func TestVerifiedHeadAndTerminalStatusPersistAtomically(t *testing.T) { } } +func TestVerifyTerminalRunHeadRewriteUsesRecordedReviewCAS(t *testing.T) { + d := openTestDB(t) + repo, err := d.InsertRepo("/tmp/terminal-rewrite-cas", "https://example.com/terminal-rewrite-cas", "main") + if err != nil { + t.Fatal(err) + } + run, err := d.InsertRun(repo.ID, "feature", "submitted", "base") + if err != nil { + t.Fatal(err) + } + if err := d.UpdateRunHeadSHA(run.ID, "recorded"); err != nil { + t.Fatal(err) + } + if err := d.UpdateRunReviewApprovedHeadSHA(run.ID, "other"); err != nil { + t.Fatal(err) + } + if err := d.UpdateRunStatus(run.ID, types.RunFailed); err != nil { + t.Fatal(err) + } + if updated, err := d.VerifyTerminalRunHeadRewrite(run.ID, types.RunFailed, "recorded", "live"); err != nil || updated { + t.Fatalf("mismatched review CAS = %t, %v", updated, err) + } + got, err := d.GetRun(run.ID) + if err != nil || got.HeadSHA != "recorded" || got.TerminalHeadVerifiedAt != nil { + t.Fatalf("failed CAS changed run = %#v, %v", got, err) + } + if err := d.UpdateRunReviewApprovedHeadSHA(run.ID, "recorded"); err != nil { + t.Fatal(err) + } + if updated, err := d.VerifyTerminalRunHeadRewrite(run.ID, types.RunFailed, "recorded", "live"); err != nil || !updated { + t.Fatalf("matching review CAS = %t, %v", updated, err) + } + got, err = d.GetRun(run.ID) + if err != nil || got.HeadSHA != "live" || got.TerminalHeadVerifiedAt == nil { + t.Fatalf("successful CAS did not verify live head = %#v, %v", got, err) + } +} + func TestRunPushBindingIsForwardOnlyAndLegacyRowsStayNullable(t *testing.T) { d := openTestDB(t) repo, _ := d.InsertRepo("/tmp/repo-sync-binding", "https://example.com/repo.git", "main") diff --git a/internal/e2e/axi_journey_test.go b/internal/e2e/axi_journey_test.go index f1212302b..d4f31008f 100644 --- a/internal/e2e/axi_journey_test.go +++ b/internal/e2e/axi_journey_test.go @@ -12,7 +12,9 @@ import ( toon "github.com/toon-format/toon-go" + "github.com/kunchenguid/no-mistakes/internal/db" "github.com/kunchenguid/no-mistakes/internal/ipc" + "github.com/kunchenguid/no-mistakes/internal/paths" "github.com/kunchenguid/no-mistakes/internal/types" ) @@ -472,6 +474,103 @@ func TestAxiCustodyRecoveryJourney(t *testing.T) { t.Logf("end-user fresh-run result after custody return:\n%s", freshOut) } +func TestAxiTerminalEqualTreeRecoveryJourney(t *testing.T) { + h := NewHarness(t, SetupOpts{Agent: "claude"}) + h.CommitChange("init-terminal-equal-tree", "seed.txt", "seed\n", "seed terminal recovery") + initWorktree := h.AddWorktree("init-terminal-equal-tree") + if out, err := h.RunInDir(initWorktree, "init"); err != nil { + t.Fatalf("init: %v\n%s", err, out) + } + + branch := "feature/terminal-equal-tree" + submitted := h.CommitChange(branch, "feature.txt", "operator work\n", "operator work") + operator := h.AddWorktree(branch) + gateDir := filepath.Join(h.NMHome, "repos", h.repoID()+".git") + if out, err := h.runGit(context.Background(), operator, "push", gateDir, "HEAD:refs/heads/"+branch); err != nil { + t.Fatalf("seed gate branch: %v\n%s", err, out) + } + mainBytes, err := h.runGit(context.Background(), gateDir, "rev-parse", submitted+"^") + if err != nil { + t.Fatalf("gate base: %v\n%s", err, mainBytes) + } + if err := os.WriteFile(filepath.Join(operator, "feature.txt"), []byte("pipeline reviewed replacement\n"), 0o644); err != nil { + t.Fatal(err) + } + if out, gitErr := h.runGit(context.Background(), operator, "commit", "-am", "recorded reviewed pipeline result"); gitErr != nil { + t.Fatalf("record reviewed result: %v\n%s", gitErr, out) + } + recorded := strings.TrimSpace(h.WorktreeRefSHA(branch)) + if out, gitErr := h.runGit(context.Background(), gateDir, "fetch", operator, recorded+":refs/no-mistakes/test-recorded"); gitErr != nil { + t.Fatalf("import recorded result: %v\n%s", gitErr, out) + } + treeBytes, err := h.runGit(context.Background(), gateDir, "rev-parse", recorded+"^{tree}") + if err != nil { + t.Fatalf("recorded tree: %v\n%s", err, treeBytes) + } + liveBytes, err := h.runGit(context.Background(), gateDir, "-c", "user.name=No Mistakes E2E", "-c", "user.email=e2e@example.com", "commit-tree", strings.TrimSpace(string(treeBytes)), "-p", strings.TrimSpace(string(mainBytes)), "-m", "live equal-tree rewrite") + if err != nil { + t.Fatalf("commit live rewrite: %v\n%s", err, liveBytes) + } + live := strings.TrimSpace(string(liveBytes)) + if out, gitErr := h.runGit(context.Background(), gateDir, "update-ref", "refs/heads/"+branch, live, submitted); gitErr != nil { + t.Fatalf("install live gate rewrite: %v\n%s", gitErr, out) + } + if out, gitErr := h.runGit(context.Background(), operator, "reset", "--hard", submitted); gitErr != nil { + t.Fatalf("restore submitted operator head: %v\n%s", gitErr, out) + } + if out, gitErr := h.runGit(context.Background(), gateDir, "merge-tree", "--write-tree", "--merge-base", strings.TrimSpace(string(mainBytes)), live, submitted); gitErr == nil { + t.Fatalf("fixture did not reproduce direct live/local merge conflict: %s", out) + } + if out, gitErr := h.runGit(context.Background(), gateDir, "update-ref", "-d", "refs/no-mistakes/test-recorded"); gitErr != nil { + t.Fatalf("remove fixture ref: %v\n%s", gitErr, out) + } + + database, err := db.Open(paths.WithRoot(h.NMHome).DB()) + if err != nil { + t.Fatal(err) + } + repo, err := database.GetRepo(h.repoID()) + if err != nil || repo == nil { + database.Close() + t.Fatalf("registered repo = %#v, err %v", repo, err) + } + run, err := database.InsertRun(repo.ID, branch, submitted, strings.TrimSpace(string(mainBytes))) + if err == nil { + err = database.UpdateRunHeadSHA(run.ID, recorded) + } + if err == nil { + err = database.UpdateRunReviewApprovedHeadSHA(run.ID, recorded) + } + if err == nil { + err = database.UpdateRunErrorStatus(run.ID, "terminal worker lost", types.RunFailed) + } + if closeErr := database.Close(); err != nil || closeErr != nil { + t.Fatalf("record terminal run: %v, close %v", err, closeErr) + } + + recoverOut, err := h.RunInDir(operator, "axi", "sync", "--recover") + if err != nil { + t.Fatalf("public equal-tree recovery: %v\n%s", err, recoverOut) + } + for _, want := range []string{"recovered: true", "changed: true", "state: custody_returned"} { + if !strings.Contains(recoverOut, want) { + t.Errorf("recover output missing %q:\n%s", want, recoverOut) + } + } + if got := strings.TrimSpace(h.WorktreeRefSHA(branch)); got != live { + t.Fatalf("operator HEAD = %s, want live gate head %s", got, live) + } + for _, ref := range []string{"refs/no-mistakes/recover/" + run.ID, "refs/no-mistakes/recover-local/" + run.ID} { + if out, gitErr := h.runGit(context.Background(), operator, "rev-parse", ref); gitErr != nil { + t.Fatalf("missing recovery ref %s: %v\n%s", ref, gitErr, out) + } + } + second, err := h.RunInDir(operator, "axi", "sync", "--recover") + if err != nil || !strings.Contains(second, "recovered: true") || !strings.Contains(second, "changed: false") { + t.Fatalf("idempotent public recovery: %v\n%s", err, second) + } +} + // rebaseCustodyScenario differs from branchSyncScenario in exactly one way that // matters here: its fix round ADDS a file instead of rewriting the operator's // own line. Both shapes advance the gate branch, but only this one leaves the diff --git a/internal/git/git.go b/internal/git/git.go index 91468dbe1..14321cc3f 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -48,6 +48,15 @@ func RunRaw(ctx context.Context, dir string, args ...string) ([]byte, error) { return runInDirWithEnvRaw(ctx, dir, nil, args...) } +// RunWithInput executes a git command with exact standard input and returns +// trimmed stdout. It carries the same bare-repository handling as Run. +func RunWithInput(ctx context.Context, dir, input string, args ...string) (string, error) { + if isBareGitDir(dir) { + return runInDirWithEnvAndInput(ctx, dir, nil, input, append([]string{"--git-dir=" + dir}, args...)...) + } + return runInDirWithEnvAndInput(ctx, dir, nil, input, args...) +} + // RunBare executes Git against exactly bareDir. Unlike Run, it never falls // back to cwd-based repository discovery when bareDir is malformed. Gate // recovery uses this after structural validation so an invalid directory under @@ -77,14 +86,25 @@ func runInDir(ctx context.Context, dir string, args ...string) (string, error) { } func runInDirWithEnv(ctx context.Context, dir string, extraEnv []string, args ...string) (string, error) { - out, err := runInDirWithEnvRaw(ctx, dir, extraEnv, args...) + return runInDirWithEnvAndInput(ctx, dir, extraEnv, "", args...) +} + +func runInDirWithEnvAndInput(ctx context.Context, dir string, extraEnv []string, input string, args ...string) (string, error) { + out, err := runInDirWithEnvAndInputRaw(ctx, dir, extraEnv, input, args...) return strings.TrimSpace(string(out)), err } func runInDirWithEnvRaw(ctx context.Context, dir string, extraEnv []string, args ...string) ([]byte, error) { + return runInDirWithEnvAndInputRaw(ctx, dir, extraEnv, "", args...) +} + +func runInDirWithEnvAndInputRaw(ctx context.Context, dir string, extraEnv []string, input string, args ...string) ([]byte, error) { cmd := exec.CommandContext(ctx, "git", args...) cmd.Dir = dir cmd.Env = append(nonInteractiveEnvForContext(ctx, dir), extraEnv...) + if input != "" { + cmd.Stdin = strings.NewReader(input) + } winproc.Harden(cmd) // OutputShellCommand captures stdout only, so unlike cmd.Output it never // fills ExitError.Stderr. Capture stderr explicitly or the git error text