From c4efe8e660a0976979c5ea8cbab3ba8800d6fd5b Mon Sep 17 00:00:00 2001 From: Zach Date: Fri, 28 Aug 2026 00:29:56 +0800 Subject: [PATCH 1/7] fix(cli): unblock fresh runs after missing custody head --- docs/src/content/docs/guides/agents.md | 1 + docs/src/content/docs/reference/cli.md | 1 + internal/cli/axi_drive.go | 57 +++++++++++ internal/cli/axi_guidance.go | 2 +- internal/cli/axi_missing_head_test.go | 125 +++++++++++++++++++++++++ internal/skill/skill.go | 1 + skills/no-mistakes/SKILL.md | 1 + 7 files changed, 187 insertions(+), 1 deletion(-) create mode 100644 internal/cli/axi_missing_head_test.go diff --git a/docs/src/content/docs/guides/agents.md b/docs/src/content/docs/guides/agents.md index 4942af8e6..8e99fef88 100644 --- a/docs/src/content/docs/guides/agents.md +++ b/docs/src/content/docs/guides/agents.md @@ -179,6 +179,7 @@ Before any post-pipeline local commit or fresh run, read `branch_sync`. Only when its structured `next_action.code` is `sync`, run `no-mistakes axi sync` first. When `next_action.code` is `recover_custody` - a terminal run left unpublished pipeline commits preserved in the local gate - run `no-mistakes axi sync --recover` to return custody, or `no-mistakes rerun` to resume validating the preserved head. A `branch_sync.state` of `user_owned` means the run went terminal before changing the submitted head and cancellation released the branch: it is immediately usable and needs no sync action. +A terminal `pipeline_owned` state with `safety: blocked_recover_preserved_head_missing` may still require manual reconciliation for sync or recovery; `no-mistakes axi run` rechecks that the recorded head and recovery evidence are truly absent and can start unrelated fresh work only in that case. Any surviving commit or recovery anchor keeps custody blocked. When `next_action.code` is `continue_active_run`, run the reported command and keep driving the active run. If synchronization is blocked, process that state instead of improvising reset, stash, merge, rebase, force, or branch replacement. Then commit follow-up work on top so every pipeline fix commit remains in the branch. diff --git a/docs/src/content/docs/reference/cli.md b/docs/src/content/docs/reference/cli.md index f7621dc17..df17185ef 100644 --- a/docs/src/content/docs/reference/cli.md +++ b/docs/src/content/docs/reference/cli.md @@ -224,6 +224,7 @@ Run `axi sync` only when structured output offers `next_action.code: sync`; proc A run that goes terminal (cancelled, failed, or completed without a push stage) after moving the pipeline head leaves the branch `pipeline_owned`. Status offers `next_action.code: recover_custody` only when recovery can establish the same eligibility it will enforce: an equal or ahead local head proves the source locally and can create the local anchor when the gate is unavailable, but any existing gate recovery ref must still match the recorded head; importing a missing preserved head requires an exact run-specific gate anchor (or legacy commit evidence that can be anchored), a clean worktree, and either local ancestry or the content-preservation proof described below. The eligible state reports `safety: blocked_pipeline_owned_recoverable`, the run's terminal `pipeline.status`, and the exact `submitted_head`/`current_head`/`relation` ownership facts. A run whose terminalization verifies that the managed worktree head never changed from the submitted head releases the branch instead: the terminal outcome, including cancellation, ends ownership; status reports `state: user_owned` with the same exact ownership facts and no `next_action`; the branch and head are immediately usable for any separately authorized delivery; and nothing blocks a direct push or PR. Without positive evidence that the submitted head stayed unchanged, custody is not guessed away. Missing or conflicting evidence, and import cases with a dirty worktree or genuinely divergent history, require manual reconciliation instead of advertising a recovery that will refuse. +If a terminal moved head is genuinely absent from both the invoking worktree and local gate, and no recovery anchor survives, sync and recovery remain manual-only but that unrecoverable historical record does not block `axi run` from attempting unrelated fresh work; the fresh path rechecks the absence and ordinary gate push safety still applies. While a run is still active, it reports `state: pipeline_owned`, the exact submitted/current heads and their relation, and `next_action.code: continue_active_run` with `no-mistakes axi status`, even when its head has not moved yet. `--recover` verifies the run is terminal, anchors the preserved head under `refs/no-mistakes/recover/` in the invoking repository, and stamps custody returned so a fresh run can start. For equal or ahead worktrees where the preserved head is already locally reachable, recovery writes that anchor locally without requiring gate access. If the gate is available, an existing symbolic, non-commit, or mismatched recovery ref is conflicting evidence and recovery refuses without overwriting it. diff --git a/internal/cli/axi_drive.go b/internal/cli/axi_drive.go index 0805b0708..dd9855aec 100644 --- a/internal/cli/axi_drive.go +++ b/internal/cli/axi_drive.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "io" + "os" "strings" "time" @@ -12,6 +13,7 @@ import ( "github.com/kunchenguid/no-mistakes/internal/branchsync" "github.com/kunchenguid/no-mistakes/internal/cimonitor" + "github.com/kunchenguid/no-mistakes/internal/custody" "github.com/kunchenguid/no-mistakes/internal/daemon" "github.com/kunchenguid/no-mistakes/internal/db" "github.com/kunchenguid/no-mistakes/internal/gate" @@ -277,6 +279,14 @@ func freshRunBranchOwnershipState(ctx context.Context, env *axiEnv) *branchsync. if branchsync.RunHeadUnmoved(state) { return nil } + // A terminal run can retain a moved head in its record after that + // commit and all recovery evidence have disappeared. Keep the + // branch-sync state manual-only, but do not let an unrecoverable + // historical record block unrelated fresh work forever. Any surviving + // commit or recovery anchor remains a custody block. + if state.Safety == "blocked_recover_preserved_head_missing" && freshRunHasNoPipelineEvidence(ctx, env, state) { + return nil + } return &state case branchsync.StatePushInProgress: return &state @@ -285,6 +295,53 @@ func freshRunBranchOwnershipState(ctx context.Context, env *axiEnv) *branchsync. } } +// freshRunHasNoPipelineEvidence permits a fresh delivery only after positive +// local evidence shows that a terminal run's recorded moved head and its +// recovery anchors are gone. Unreadable repositories or conflicting anchors +// fail closed so this exception cannot discard recoverable pipeline work. +func freshRunHasNoPipelineEvidence(ctx context.Context, env *axiEnv, state branchsync.State) bool { + if env == nil || env.p == nil || env.repo == nil || strings.TrimSpace(state.Pipeline.RunID) == "" || strings.TrimSpace(state.Pipeline.CurrentHead) == "" { + return false + } + if _, err := git.Run(ctx, ".", "cat-file", "-e", state.Pipeline.CurrentHead+"^{commit}"); err == nil { + return false + } + if recoveryEvidencePresent(ctx, ".", state.Pipeline.RunID) { + return false + } + + gateDir := env.p.RepoDir(env.repo.ID) + info, err := os.Stat(gateDir) + if err != nil { + return os.IsNotExist(err) + } + if !info.IsDir() { + return false + } + if recoveryEvidencePresent(ctx, gateDir, state.Pipeline.RunID) { + return false + } + if _, err := git.Run(ctx, gateDir, "cat-file", "-e", state.Pipeline.CurrentHead+"^{commit}"); err == nil { + return false + } + // A present but unreadable gate is not proof that the pipeline head is + // gone. Treat it as evidence we cannot safely inspect. + bare, err := git.Run(ctx, gateDir, "rev-parse", "--is-bare-repository") + if err != nil || strings.TrimSpace(bare) != "true" { + return false + } + return true +} + +func recoveryEvidencePresent(ctx context.Context, dir, runID string) bool { + ref := custody.RecoveryRef(runID) + if target, err := git.Run(ctx, dir, "symbolic-ref", "-q", ref); err == nil && strings.TrimSpace(target) != "" { + return true + } + _, exists, err := git.ExactRefTarget(ctx, dir, ref) + return err != nil || exists +} + // triggerRun starts a fresh run for branch: it pushes the current HEAD through // the gate to trigger a pipeline, and falls back to a rerun when the push was a // no-op (the gate already had this commit). Callers must check for an existing diff --git a/internal/cli/axi_guidance.go b/internal/cli/axi_guidance.go index 178b913ba..2cbb66845 100644 --- a/internal/cli/axi_guidance.go +++ b/internal/cli/axi_guidance.go @@ -26,4 +26,4 @@ const preserveGateFixCommitsGuidance = "Commit post-pipeline follow-up work on t // branchSyncAgentGuidance is emitted only when a relevant branch_sync object // is present. Keeping it conditional avoids flooding ordinary runs whose local // and pipeline heads never differed. -const branchSyncAgentGuidance = "Before a post-pipeline local commit or fresh run, follow the structured `branch_sync.next_action`. Run `no-mistakes axi sync` only when its code is `sync`; that guarded sync may be a strict fast-forward or a content-equivalent diverged advance that anchors the pre-sync head before moving the branch with reset semantics. Run `no-mistakes axi sync --recover` only when its code is `recover_custody` (a terminal run left unpublished pipeline commits preserved in the local gate). A `user_owned` state means cancellation released the branch before changing the submitted head: the exact branch and head are yours, immediately usable, and no sync action is needed. Process blocked or pipeline-owned states instead of improvising reset, stash, merge, rebase, force, or branch replacement." +const branchSyncAgentGuidance = "Before a post-pipeline local commit or fresh run, follow the structured `branch_sync.next_action`. Run `no-mistakes axi sync` only when its code is `sync`; that guarded sync may be a strict fast-forward or a content-equivalent diverged advance that anchors the pre-sync head before moving the branch with reset semantics. Run `no-mistakes axi sync --recover` only when its code is `recover_custody` (a terminal run left unpublished pipeline commits preserved in the local gate). A `user_owned` state means cancellation released the branch before changing the submitted head: the exact branch and head are yours, immediately usable, and no sync action is needed. A terminal `pipeline_owned` state with `safety: blocked_recover_preserved_head_missing` may still require manual reconciliation for sync or recovery; `no-mistakes axi run` rechecks that the recorded head and recovery evidence are truly absent and can start unrelated fresh work only in that case. Any surviving commit or recovery anchor keeps custody blocked. Process other blocked or pipeline-owned states instead of improvising reset, stash, merge, rebase, force, or branch replacement." diff --git a/internal/cli/axi_missing_head_test.go b/internal/cli/axi_missing_head_test.go new file mode 100644 index 000000000..27430a45a --- /dev/null +++ b/internal/cli/axi_missing_head_test.go @@ -0,0 +1,125 @@ +package cli + +import ( + "context" + "os" + "strings" + "testing" + + "github.com/kunchenguid/no-mistakes/internal/config" + "github.com/kunchenguid/no-mistakes/internal/custody" + "github.com/kunchenguid/no-mistakes/internal/types" +) + +// TestFreshRunBranchOwnershipDistinguishesMissingTerminalHead reproduces the +// custody deadlock caused by a terminal run whose moved head is no longer +// available. That state must remain manual-only for recovery, while it must +// not keep unrelated fresh work behind a custody claim for commits that no +// longer exist. +func TestFreshRunBranchOwnershipDistinguishesMissingTerminalHead(t *testing.T) { + for _, tc := range []struct { + name string + recordedHead func(t *testing.T, submitted string) string + advanceWorktree bool + gatePresent bool + survivingAnchor bool + verifiedHead bool + wantFreshBlocked bool + wantSafety string + }{ + { + name: "terminal unmoved releases branch", + recordedHead: func(_ *testing.T, submitted string) string { + return submitted + }, + verifiedHead: true, + wantFreshBlocked: false, + wantSafety: "user_owned", + }, + { + name: "terminal recoverable moved head keeps custody", + recordedHead: func(_ *testing.T, submitted string) string { + return submitted + }, + advanceWorktree: true, + verifiedHead: true, + wantFreshBlocked: true, + wantSafety: "blocked_pipeline_owned_recoverable", + }, + { + name: "terminal missing moved head releases fresh path", + recordedHead: func(_ *testing.T, _ string) string { + return strings.Repeat("f", 40) + }, + gatePresent: true, + wantFreshBlocked: false, + wantSafety: "blocked_recover_preserved_head_missing", + }, + { + name: "missing head with recovery evidence keeps custody", + recordedHead: func(_ *testing.T, _ string) string { + return strings.Repeat("f", 40) + }, + gatePresent: true, + survivingAnchor: true, + wantFreshBlocked: true, + wantSafety: "blocked_recover_preserved_head_missing", + }, + } { + t.Run(tc.name, func(t *testing.T) { + repoDir, paths, database, repo := setupAxiQueryRepo(t) + cliGit(t, repoDir, "checkout", "-b", "feature/missing-head") + chdir(t, repoDir) + + submitted := cliGit(t, repoDir, "rev-parse", "HEAD") + recorded := tc.recordedHead(t, submitted) + if tc.advanceWorktree { + cliGit(t, repoDir, "commit", "--allow-empty", "-m", "pipeline fix") + recorded = cliGit(t, repoDir, "rev-parse", "HEAD") + } + if tc.gatePresent { + gateDir := paths.RepoDir(repo.ID) + if err := os.MkdirAll(gateDir, 0o755); err != nil { + t.Fatalf("create gate directory: %v", err) + } + cliGit(t, gateDir, "init", "--bare") + cliGit(t, repoDir, "push", gateDir, "HEAD:refs/heads/feature/missing-head") + } + + pipelineRun, err := database.InsertRun(repo.ID, "feature/missing-head", submitted, submitted) + if err != nil { + t.Fatalf("insert pipeline run: %v", err) + } + if err := database.UpdateRunHeadSHA(pipelineRun.ID, recorded); err != nil { + t.Fatalf("record pipeline head: %v", err) + } + if tc.verifiedHead { + if err := database.UpdateRunStatusWithVerifiedHead(pipelineRun.ID, types.RunCancelled, recorded); err != nil { + t.Fatalf("terminalize pipeline run: %v", err) + } + } else if err := database.UpdateRunStatus(pipelineRun.ID, types.RunCancelled); err != nil { + t.Fatalf("terminalize pipeline run: %v", err) + } + if tc.survivingAnchor { + cliGit(t, paths.RepoDir(repo.ID), "update-ref", custody.RecoveryRef(pipelineRun.ID), submitted) + } + + env := &axiEnv{p: paths, d: database, repo: repo, cfg: config.DefaultGlobalConfig()} + state := inspectAxiBranchSync(context.Background(), env) + if state.Safety != tc.wantSafety { + t.Fatalf("branch ownership state = %s, want %s: %#v", state.Safety, tc.wantSafety, state) + } + blocked := freshRunBranchOwnershipState(context.Background(), env) + if (blocked != nil) != tc.wantFreshBlocked { + t.Fatalf("fresh-run ownership = %#v, blocked = %t, want blocked = %t", blocked, blocked != nil, tc.wantFreshBlocked) + } + if tc.wantFreshBlocked && blocked.NextAction == nil { + t.Fatal("recoverable terminal head lost its custody guidance") + } + if tc.wantSafety == "blocked_recover_preserved_head_missing" && + (state.NextAction == nil || state.NextAction.Code != "inspect_and_reconcile_manually") { + t.Fatalf("missing-head state lost manual reconciliation guidance: %#v", state) + } + }) + } +} diff --git a/internal/skill/skill.go b/internal/skill/skill.go index f3fda1497..5ecdc7b3e 100644 --- a/internal/skill/skill.go +++ b/internal/skill/skill.go @@ -241,6 +241,7 @@ When ` + "`next_action.code`" + ` is ` + "`recover_custody`" + `, a terminal run Recovery takes that head by fast-forward, or by adopting a diverged preserved head proven to carry every local change - the ordinary result of the pipeline rebasing your commits onto a newer base - after anchoring your pre-recovery head under ` + "`refs/no-mistakes/recover-local/`" + `. That proof is deliberately narrow, so a rebase whose fix rounds also rewrote your own lines refuses instead of being adopted: when nothing can tell a deliberate pipeline fix from a dropped change, the decision is yours. A ` + "`branch_sync.state`" + ` of ` + "`user_owned`" + ` means the run went terminal before changing the submitted head and cancellation released the branch: the exact branch and head are yours and immediately usable for whichever delivery path is authorized - no sync action is needed, and a repeated ` + "`--recover`" + ` there is a harmless no-op. +A terminal ` + "`pipeline_owned`" + ` state with ` + "`safety: blocked_recover_preserved_head_missing`" + ` may still require manual reconciliation for sync or recovery; ` + "`no-mistakes axi run`" + ` rechecks that the recorded head and recovery evidence are truly absent and can start unrelated fresh work only in that case. Any surviving commit or recovery anchor keeps custody blocked. A dirty worktree, or divergence that cannot be proven contained, makes the recovery refuse with explicit choices; ` + "`--keep-local`" + ` keeps your current head while the preserved commits stay anchored under ` + "`refs/no-mistakes/recover/`" + `. If synchronization is blocked, process that structured state instead of improvising reset, stash, merge, rebase, force, or branch replacement. After synchronization, commit the follow-up on top and re-run ` + "`no-mistakes axi run --intent \"...\"`" + ` with the original user intent. diff --git a/skills/no-mistakes/SKILL.md b/skills/no-mistakes/SKILL.md index 36f3b973f..8ec81a6ee 100644 --- a/skills/no-mistakes/SKILL.md +++ b/skills/no-mistakes/SKILL.md @@ -241,6 +241,7 @@ When `next_action.code` is `recover_custody`, a terminal run left unpublished pi Recovery takes that head by fast-forward, or by adopting a diverged preserved head proven to carry every local change - the ordinary result of the pipeline rebasing your commits onto a newer base - after anchoring your pre-recovery head under `refs/no-mistakes/recover-local/`. That proof is deliberately narrow, so a rebase whose fix rounds also rewrote your own lines refuses instead of being adopted: when nothing can tell a deliberate pipeline fix from a dropped change, the decision is yours. A `branch_sync.state` of `user_owned` means the run went terminal before changing the submitted head and cancellation released the branch: the exact branch and head are yours and immediately usable for whichever delivery path is authorized - no sync action is needed, and a repeated `--recover` there is a harmless no-op. +A terminal `pipeline_owned` state with `safety: blocked_recover_preserved_head_missing` may still require manual reconciliation for sync or recovery; `no-mistakes axi run` rechecks that the recorded head and recovery evidence are truly absent and can start unrelated fresh work only in that case. Any surviving commit or recovery anchor keeps custody blocked. A dirty worktree, or divergence that cannot be proven contained, makes the recovery refuse with explicit choices; `--keep-local` keeps your current head while the preserved commits stay anchored under `refs/no-mistakes/recover/`. If synchronization is blocked, process that structured state instead of improvising reset, stash, merge, rebase, force, or branch replacement. After synchronization, commit the follow-up on top and re-run `no-mistakes axi run --intent "..."` with the original user intent. From 269f08a3924816964d316c3392f59c0df72624ea Mon Sep 17 00:00:00 2001 From: Zach Date: Fri, 28 Aug 2026 01:30:20 +0800 Subject: [PATCH 2/7] fix(git): fail closed on object lookup errors --- internal/cli/axi_drive.go | 13 ++++++------- internal/cli/axi_missing_head_test.go | 21 +++++++++++++++++++++ internal/git/git.go | 12 ++++++++++-- 3 files changed, 37 insertions(+), 9 deletions(-) diff --git a/internal/cli/axi_drive.go b/internal/cli/axi_drive.go index dd9855aec..cf1d0d998 100644 --- a/internal/cli/axi_drive.go +++ b/internal/cli/axi_drive.go @@ -303,7 +303,8 @@ func freshRunHasNoPipelineEvidence(ctx context.Context, env *axiEnv, state branc if env == nil || env.p == nil || env.repo == nil || strings.TrimSpace(state.Pipeline.RunID) == "" || strings.TrimSpace(state.Pipeline.CurrentHead) == "" { return false } - if _, err := git.Run(ctx, ".", "cat-file", "-e", state.Pipeline.CurrentHead+"^{commit}"); err == nil { + localHeadExists, err := git.RefExists(ctx, ".", state.Pipeline.CurrentHead) + if err != nil || localHeadExists { return false } if recoveryEvidencePresent(ctx, ".", state.Pipeline.RunID) { @@ -318,16 +319,14 @@ func freshRunHasNoPipelineEvidence(ctx context.Context, env *axiEnv, state branc if !info.IsDir() { return false } - if recoveryEvidencePresent(ctx, gateDir, state.Pipeline.RunID) { + if err := git.ValidateBareRepository(ctx, gateDir); err != nil { return false } - if _, err := git.Run(ctx, gateDir, "cat-file", "-e", state.Pipeline.CurrentHead+"^{commit}"); err == nil { + if recoveryEvidencePresent(ctx, gateDir, state.Pipeline.RunID) { return false } - // A present but unreadable gate is not proof that the pipeline head is - // gone. Treat it as evidence we cannot safely inspect. - bare, err := git.Run(ctx, gateDir, "rev-parse", "--is-bare-repository") - if err != nil || strings.TrimSpace(bare) != "true" { + gateHeadExists, err := git.RefExists(ctx, gateDir, state.Pipeline.CurrentHead) + if err != nil || gateHeadExists { return false } return true diff --git a/internal/cli/axi_missing_head_test.go b/internal/cli/axi_missing_head_test.go index 27430a45a..9ac7e600f 100644 --- a/internal/cli/axi_missing_head_test.go +++ b/internal/cli/axi_missing_head_test.go @@ -3,6 +3,7 @@ package cli import ( "context" "os" + "path/filepath" "strings" "testing" @@ -24,6 +25,7 @@ func TestFreshRunBranchOwnershipDistinguishesMissingTerminalHead(t *testing.T) { gatePresent bool survivingAnchor bool verifiedHead bool + objectReadError bool wantFreshBlocked bool wantSafety string }{ @@ -65,6 +67,16 @@ func TestFreshRunBranchOwnershipDistinguishesMissingTerminalHead(t *testing.T) { wantFreshBlocked: true, wantSafety: "blocked_recover_preserved_head_missing", }, + { + name: "object read failure keeps custody", + recordedHead: func(_ *testing.T, _ string) string { + return strings.Repeat("f", 40) + }, + gatePresent: true, + objectReadError: true, + wantFreshBlocked: true, + wantSafety: "blocked_recover_preserved_head_missing", + }, } { t.Run(tc.name, func(t *testing.T) { repoDir, paths, database, repo := setupAxiQueryRepo(t) @@ -84,6 +96,15 @@ func TestFreshRunBranchOwnershipDistinguishesMissingTerminalHead(t *testing.T) { } cliGit(t, gateDir, "init", "--bare") cliGit(t, repoDir, "push", gateDir, "HEAD:refs/heads/feature/missing-head") + if tc.objectReadError { + objectPath := filepath.Join(gateDir, "objects", recorded[:2], recorded[2:]) + if err := os.MkdirAll(filepath.Dir(objectPath), 0o755); err != nil { + t.Fatalf("create corrupt object directory: %v", err) + } + if err := os.WriteFile(objectPath, []byte("corrupt object"), 0o644); err != nil { + t.Fatalf("write corrupt object: %v", err) + } + } } pipelineRun, err := database.InsertRun(repo.ID, "feature/missing-head", submitted, submitted) diff --git a/internal/git/git.go b/internal/git/git.go index 91468dbe1..a0a0783c7 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -679,9 +679,17 @@ func ExactRefTarget(ctx context.Context, dir, ref string) (string, bool, error) // RefExists reports whether the given ref resolves to a commit. It uses // `git rev-parse --verify --quiet` so a missing ref is a clean (nil, false) -// result rather than a loud error. +// result rather than a loud error. Bare repositories are addressed with an +// explicit --git-dir so callers can distinguish missing objects without +// relying on cwd-based repository discovery. func RefExists(ctx context.Context, dir, ref string) (bool, error) { - cmd := exec.CommandContext(ctx, "git", "-C", dir, "rev-parse", "--verify", "--quiet", ref+"^{commit}") + args := []string{"rev-parse", "--verify", "--quiet", ref + "^{commit}"} + if isBareGitDir(dir) { + args = append([]string{"--git-dir=" + dir}, args...) + } else { + args = append([]string{"-C", dir}, args...) + } + cmd := exec.CommandContext(ctx, "git", args...) cmd.Env = nonInteractiveEnvForContext(ctx, dir) winproc.Harden(cmd) if err := cmd.Run(); err != nil { From 8571448f42327dd46285485f9b97b96a33dd45d1 Mon Sep 17 00:00:00 2001 From: Zach Date: Fri, 28 Aug 2026 03:25:43 +0800 Subject: [PATCH 3/7] no-mistakes(review): Handle no-op fresh runs after missing custody heads --- internal/cli/attach.go | 47 ++++++++++++- internal/cli/axi_drive.go | 80 +++++++++++++++++----- internal/cli/axi_missing_head_test.go | 99 +++++++++++++++++++++++++++ internal/cli/root_test.go | 7 +- internal/cli/wizard.go | 6 ++ internal/daemon/daemon.go | 15 ++++ internal/daemon/manager.go | 49 +++++++++++++ internal/ipc/protocol.go | 13 ++++ internal/ipc/protocol_test.go | 1 + 9 files changed, 295 insertions(+), 22 deletions(-) diff --git a/internal/cli/attach.go b/internal/cli/attach.go index edb3dd0dc..5536c0f44 100644 --- a/internal/cli/attach.go +++ b/internal/cli/attach.go @@ -9,6 +9,7 @@ import ( "github.com/kunchenguid/no-mistakes/internal/daemon" "github.com/kunchenguid/no-mistakes/internal/db" + "github.com/kunchenguid/no-mistakes/internal/git" "github.com/kunchenguid/no-mistakes/internal/ipc" "github.com/kunchenguid/no-mistakes/internal/telemetry" "github.com/kunchenguid/no-mistakes/internal/tui" @@ -74,6 +75,9 @@ func attachRun(ctx context.Context, w io.Writer, runID string, rootDefault bool, if err != nil { return err } + state.beforePush = func(ctx context.Context, workDir string) error { + return guardFreshRun(ctx, p, d, repo, workDir) + } // Skip the active-run check entirely when the state clearly calls // for the wizard (detached HEAD, or default branch with pending @@ -101,7 +105,7 @@ func attachRun(ctx context.Context, w io.Writer, runID string, rootDefault bool, // has the run registered, so the handoff to runTUI below is // seamless rather than flashing the pre-wizard terminal. waitFn := func(ctx context.Context, branch string) error { - return awaitDaemonRunRegistration(ctx, client, repo.ID, branch, 5*time.Second) + return awaitDaemonRunRegistrationOrStartFresh(ctx, client, p, d, repo, state.workDir, branch, skipSteps) } var res wizard.Result var wErr error @@ -118,10 +122,19 @@ func attachRun(ctx context.Context, w io.Writer, runID string, rootDefault bool, return wErr } if res.Success { - run, err = waitForActiveRun(ctx, client, repo.ID, res.TargetBranch, 5*time.Second) + run, err = waitForActiveRun(ctx, client, repo.ID, res.TargetBranch, triggerWaitTimeout) if err != nil { return fmt.Errorf("wait for active run: %w", err) } + if run == nil { + if err := awaitDaemonRunRegistrationOrStartFresh(ctx, client, p, d, repo, state.workDir, res.TargetBranch, skipSteps); err != nil { + return err + } + run, err = waitForActiveRun(ctx, client, repo.ID, res.TargetBranch, triggerWaitTimeout) + if err != nil { + return fmt.Errorf("wait for fresh run: %w", err) + } + } if autoYes && run == nil { return fmt.Errorf("no active run appeared after pushing %q", res.TargetBranch) } @@ -150,6 +163,36 @@ func attachRun(ctx context.Context, w io.Writer, runID string, rootDefault bool, return runTUI(p.Socket(), client, run, update.CachedLatestVersion()) } +func awaitDaemonRunRegistrationOrStartFresh(ctx context.Context, client *ipc.Client, p *paths.Paths, d *db.DB, repo *db.Repo, workDir, branch string, skipSteps []types.StepName) error { + run, err := waitForActiveRun(ctx, client, repo.ID, branch, triggerWaitTimeout) + if err != nil { + return err + } + if run != nil { + return nil + } + + headSHA, err := git.Run(ctx, workDir, "rev-parse", "HEAD") + if err != nil { + return fmt.Errorf("resolve current HEAD for %q: %w", branch, err) + } + if err := guardFreshRun(ctx, p, d, repo, workDir); err != nil { + return err + } + if _, err := startFreshRun(ctx, client, repo.ID, branch, headSHA, skipSteps, ""); err != nil { + return fmt.Errorf("start fresh run for %q: %w", branch, err) + } + return nil +} + +func guardFreshRun(ctx context.Context, p *paths.Paths, d *db.DB, repo *db.Repo, workDir string) error { + env := &axiEnv{p: p, d: d, repo: repo} + if state := freshRunBranchOwnershipStateAt(ctx, env, workDir); state != nil { + return &branchOwnershipError{state: *state} + } + return nil +} + func attachEntrypoint(rootDefault bool, runID string) string { if rootDefault && runID == "" { return "root" diff --git a/internal/cli/axi_drive.go b/internal/cli/axi_drive.go index cf1d0d998..40fcbce71 100644 --- a/internal/cli/axi_drive.go +++ b/internal/cli/axi_drive.go @@ -26,8 +26,8 @@ import ( ) // triggerWaitTimeout bounds how long we wait for the daemon to register a run -// after pushing to the gate before falling back to a rerun. -const triggerWaitTimeout = 5 * time.Second +// after pushing to the gate before requesting a fresh run. +var triggerWaitTimeout = 5 * time.Second // abortStateWaitTimeout bounds the post-cancel wait for the executor to // persist its terminal state before AXI renders refreshed custody guidance. @@ -255,19 +255,34 @@ func emitBranchOwnershipError(cmd *cobra.Command, ownershipErr *branchOwnershipE } func inspectAxiBranchSync(ctx context.Context, env *axiEnv) branchsync.State { + return inspectAxiBranchSyncAt(ctx, env, ".") +} + +func inspectAxiBranchSyncAt(ctx context.Context, env *axiEnv, workDir string) branchsync.State { service := &branchsync.Service{ DB: env.d, Repo: env.repo, - WorkDir: ".", + WorkDir: workDir, GateDir: env.p.RepoDir(env.repo.ID), Paths: env.p, - RemoteTimeout: env.cfg.BranchSyncRemoteTimeout, + RemoteTimeout: branchSyncRemoteTimeout(env), } return service.InspectCached(ctx) } +func branchSyncRemoteTimeout(env *axiEnv) time.Duration { + if env == nil || env.cfg == nil { + return 0 + } + return env.cfg.BranchSyncRemoteTimeout +} + func freshRunBranchOwnershipState(ctx context.Context, env *axiEnv) *branchsync.State { - state := inspectAxiBranchSync(ctx, env) + return freshRunBranchOwnershipStateAt(ctx, env, ".") +} + +func freshRunBranchOwnershipStateAt(ctx context.Context, env *axiEnv, workDir string) *branchsync.State { + state := inspectAxiBranchSyncAt(ctx, env, workDir) switch state.State { case branchsync.StatePipelineOwned: // The ownership block exists to keep a fresh push from discarding @@ -284,7 +299,7 @@ func freshRunBranchOwnershipState(ctx context.Context, env *axiEnv) *branchsync. // branch-sync state manual-only, but do not let an unrecoverable // historical record block unrelated fresh work forever. Any surviving // commit or recovery anchor remains a custody block. - if state.Safety == "blocked_recover_preserved_head_missing" && freshRunHasNoPipelineEvidence(ctx, env, state) { + if state.Safety == "blocked_recover_preserved_head_missing" && freshRunHasNoPipelineEvidence(ctx, env, state, workDir) { return nil } return &state @@ -299,15 +314,15 @@ func freshRunBranchOwnershipState(ctx context.Context, env *axiEnv) *branchsync. // local evidence shows that a terminal run's recorded moved head and its // recovery anchors are gone. Unreadable repositories or conflicting anchors // fail closed so this exception cannot discard recoverable pipeline work. -func freshRunHasNoPipelineEvidence(ctx context.Context, env *axiEnv, state branchsync.State) bool { +func freshRunHasNoPipelineEvidence(ctx context.Context, env *axiEnv, state branchsync.State, workDir string) bool { if env == nil || env.p == nil || env.repo == nil || strings.TrimSpace(state.Pipeline.RunID) == "" || strings.TrimSpace(state.Pipeline.CurrentHead) == "" { return false } - localHeadExists, err := git.RefExists(ctx, ".", state.Pipeline.CurrentHead) + localHeadExists, err := git.RefExists(ctx, workDir, state.Pipeline.CurrentHead) if err != nil || localHeadExists { return false } - if recoveryEvidencePresent(ctx, ".", state.Pipeline.RunID) { + if recoveryEvidencePresent(ctx, workDir, state.Pipeline.RunID) { return false } @@ -341,10 +356,9 @@ func recoveryEvidencePresent(ctx context.Context, dir, runID string) bool { return err != nil || exists } -// triggerRun starts a fresh run for branch: it pushes the current HEAD through -// the gate to trigger a pipeline, and falls back to a rerun when the push was a -// no-op (the gate already had this commit). Callers must check for an existing -// active run first (see activeRunID) and apply pre-flight guards. +// triggerRun starts a fresh run for branch by pushing the current HEAD through +// the gate. Callers must check for an existing active run first (see +// activeRunID) and apply pre-flight guards. func triggerRun(ctx context.Context, env *axiEnv, branch, headSHA string, skipSteps []types.StepName, intent string) (string, error) { pushOptions := formatSkipPushOptions(skipSteps) if opt := formatIntentPushOption(intent); opt != "" { @@ -359,6 +373,8 @@ func triggerRun(ctx context.Context, env *axiEnv, branch, headSHA string, skipSt if state := freshRunBranchOwnershipState(ctx, env); state != nil { return "", &branchOwnershipError{state: *state} } + gateHeadBefore, gateHeadErr := git.Run(ctx, env.p.RepoDir(env.repo.ID), "rev-parse", "refs/heads/"+branch+"^{commit}") + gateWasCurrent := gateHeadErr == nil && gateHeadBefore == headSHA pushErr := git.PushWithOptions(ctx, ".", gate.RemoteName, "refs/heads/"+branch, "", false, pushOptions) if pushErr != nil { // Close the inspection-to-push race: if the pipeline advanced ownership @@ -368,6 +384,16 @@ func triggerRun(ctx context.Context, env *axiEnv, branch, headSHA string, skipSt return "", &branchOwnershipError{state: *state} } } + if pushErr == nil && gateWasCurrent { + if state := freshRunBranchOwnershipState(ctx, env); state != nil { + return "", &branchOwnershipError{state: *state} + } + runID, err := startFreshRun(ctx, env.client, env.repo.ID, branch, headSHA, skipSteps, intent) + if err != nil { + return "", fmt.Errorf("start fresh run for %q: %v", branch, err) + } + return runID, nil + } if run, _ := waitForTriggeredRunForHead(ctx, env.client, env.repo.ID, branch, headSHA, priorRunIDs, triggerWaitTimeout); run != nil { return run.ID, nil @@ -376,13 +402,31 @@ func triggerRun(ctx context.Context, env *axiEnv, branch, headSHA string, skipSt return "", fmt.Errorf("push %q to gate: %v", branch, pushErr) } - // No run appeared: the push was likely up-to-date. Rerun the latest gate - // head so `axi run` is still useful when there are no new commits. - var rr ipc.RerunResult - if err := env.client.Call(ipc.MethodRerun, rerunParams(env.repo.ID, branch, skipSteps, intent), &rr); err != nil { + if state := freshRunBranchOwnershipState(ctx, env); state != nil { + return "", &branchOwnershipError{state: *state} + } + runID, err := startFreshRun(ctx, env.client, env.repo.ID, branch, headSHA, skipSteps, intent) + if err != nil { return "", fmt.Errorf("no run started for %q: %v", branch, err) } - return rr.RunID, nil + return runID, nil +} + +func startFreshRun(ctx context.Context, client *ipc.Client, repoID, branch, headSHA string, skipSteps []types.StepName, intent string) (string, error) { + var result ipc.StartFreshRunResult + if err := client.Call(ipc.MethodStartFreshRun, &ipc.StartFreshRunParams{ + RepoID: repoID, + Branch: branch, + HeadSHA: headSHA, + SkipSteps: skipSteps, + Intent: intent, + }, &result); err != nil { + return "", err + } + if strings.TrimSpace(result.RunID) == "" { + return "", fmt.Errorf("daemon returned an empty run ID") + } + return result.RunID, nil } // runIDsForHead snapshots the run IDs already present for a repo's exact branch diff --git a/internal/cli/axi_missing_head_test.go b/internal/cli/axi_missing_head_test.go index 9ac7e600f..83a6016f7 100644 --- a/internal/cli/axi_missing_head_test.go +++ b/internal/cli/axi_missing_head_test.go @@ -6,9 +6,14 @@ import ( "path/filepath" "strings" "testing" + "time" "github.com/kunchenguid/no-mistakes/internal/config" "github.com/kunchenguid/no-mistakes/internal/custody" + "github.com/kunchenguid/no-mistakes/internal/db" + "github.com/kunchenguid/no-mistakes/internal/gate" + "github.com/kunchenguid/no-mistakes/internal/ipc" + "github.com/kunchenguid/no-mistakes/internal/paths" "github.com/kunchenguid/no-mistakes/internal/types" ) @@ -144,3 +149,97 @@ func TestFreshRunBranchOwnershipDistinguishesMissingTerminalHead(t *testing.T) { }) } } + +type missingHeadFreshRunFixture struct { + repoDir string + paths *paths.Paths + d *db.DB + repo *db.Repo + branch string + head string +} + +func newMissingHeadFreshRunFixture(t *testing.T) missingHeadFreshRunFixture { + t.Helper() + repoDir := setupTestRepo(t) + p := paths.WithRoot(os.Getenv("NM_HOME")) + d, err := db.Open(p.DB()) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = d.Close() }) + repo, _, err := gate.Init(context.Background(), d, p, ".") + if err != nil { + t.Fatal(err) + } + + branch := "feature/missing-head" + run(t, repoDir, "git", "checkout", "-b", branch) + run(t, repoDir, "git", "push", gate.RemoteName, "HEAD:refs/heads/"+branch) + head := gitOutput(t, repoDir, "rev-parse", "HEAD") + missing := strings.Repeat("f", 40) + pipelineRun, err := d.InsertRun(repo.ID, branch, head, head) + if err != nil { + t.Fatal(err) + } + if err := d.UpdateRunStatusWithVerifiedHead(pipelineRun.ID, types.RunCancelled, missing); err != nil { + t.Fatal(err) + } + + mockClaude := writeMockClaude(t, t.TempDir()) + configYAML := "agent: claude\nagent_path_override:\n claude: " + mockClaude + "\n" + if err := os.WriteFile(p.ConfigFile(), []byte(configYAML), 0o644); err != nil { + t.Fatal(err) + } + startTestDaemon(t, p, d) + + return missingHeadFreshRunFixture{repoDir: repoDir, paths: p, d: d, repo: repo, branch: branch, head: head} +} + +func TestTriggerRunStartsFreshRunWhenNoopGateHasMissingTerminalHead(t *testing.T) { + f := newMissingHeadFreshRunFixture(t) + client, err := ipc.Dial(f.paths.Socket()) + if err != nil { + t.Fatal(err) + } + defer client.Close() + + env := &axiEnv{p: f.paths, d: f.d, repo: f.repo, cfg: config.DefaultGlobalConfig(), client: client} + runID, err := triggerRun(context.Background(), env, f.branch, f.head, nil, "fresh delivery") + if err != nil { + t.Fatalf("triggerRun() error = %v", err) + } + got, err := f.d.GetRun(runID) + if err != nil { + t.Fatal(err) + } + if got == nil || got.HeadSHA != f.head { + t.Fatalf("fresh run = %#v, want head %s", got, f.head) + } +} + +func TestRootWizardStartsFreshRunWhenNoopGateHasMissingTerminalHead(t *testing.T) { + f := newMissingHeadFreshRunFixture(t) + + previousInteractive := terminalInteractive + terminalInteractive = func() bool { return false } + defer func() { terminalInteractive = previousInteractive }() + previousTimeout := triggerWaitTimeout + triggerWaitTimeout = 100 * time.Millisecond + defer func() { triggerWaitTimeout = previousTimeout }() + + previousRunTUI := runTUI + var attached *ipc.RunInfo + runTUI = func(_ string, _ *ipc.Client, run *ipc.RunInfo, _ string) error { + attached = run + return nil + } + defer func() { runTUI = previousRunTUI }() + + if _, err := executeCmd("-y"); err != nil { + t.Fatalf("executeCmd(-y) error = %v", err) + } + if attached == nil || attached.HeadSHA != f.head { + t.Fatalf("wizard attached run = %#v, want head %s", attached, f.head) + } +} diff --git a/internal/cli/root_test.go b/internal/cli/root_test.go index a9e6914ea..7fed556c8 100644 --- a/internal/cli/root_test.go +++ b/internal/cli/root_test.go @@ -196,6 +196,9 @@ func TestRootYesUsesVisibleWizardWhenInteractive(t *testing.T) { func TestRootYesFailsWhenWizardPushProducesNoRun(t *testing.T) { setupTestRepo(t) + previousTimeout := triggerWaitTimeout + triggerWaitTimeout = 100 * time.Millisecond + defer func() { triggerWaitTimeout = previousTimeout }() nmHome := makeSocketSafeTempDir(t) t.Setenv("NM_HOME", nmHome) p := paths.WithRoot(nmHome) @@ -222,8 +225,8 @@ func TestRootYesFailsWhenWizardPushProducesNoRun(t *testing.T) { if err == nil { t.Fatal("expected -y to fail when no active run appears after push") } - if !strings.Contains(err.Error(), "no active run") { - t.Fatalf("error should mention missing active run, got %v", err) + if !strings.Contains(err.Error(), "resolve gate head") { + t.Fatalf("error should explain that the pushed branch is unavailable, got %v", err) } } diff --git a/internal/cli/wizard.go b/internal/cli/wizard.go index 2f6020b6a..21592d095 100644 --- a/internal/cli/wizard.go +++ b/internal/cli/wizard.go @@ -137,6 +137,7 @@ type repoState struct { defaultBranch string detached bool dirty bool + beforePush func(context.Context, string) error } // needsBranch reports whether the user has no feature branch to work on — @@ -240,6 +241,11 @@ func runWizardWithMode(ctx context.Context, p *paths.Paths, state *repoState, sk return git.CommitAll(ctx, workDir, msg) }, Push: func(ctx context.Context, branch string) error { + if state.beforePush != nil { + if err := state.beforePush(ctx, workDir); err != nil { + return err + } + } return git.PushWithOptions(ctx, workDir, gate.RemoteName, "refs/heads/"+branch, "", false, formatSkipPushOptions(skipSteps)) }, SuggestBranch: func(ctx context.Context) (string, error) { diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index 42e5414e1..525d61b81 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -1201,6 +1201,21 @@ func registerHandlers(srv *ipc.Server, mgr *RunManager, d *db.DB, shutdown func( return &ipc.RerunResult{RunID: runID}, nil }) + srv.Handle(ipc.MethodStartFreshRun, func(ctx context.Context, params json.RawMessage) (interface{}, error) { + if err := refuseNested(ctx, false); err != nil { + return nil, err + } + var p ipc.StartFreshRunParams + if err := json.Unmarshal(params, &p); err != nil { + return nil, fmt.Errorf("invalid params: %w", err) + } + runID, err := mgr.HandleStartFreshRun(ctx, p.RepoID, p.Branch, p.HeadSHA, p.SkipSteps, p.Intent) + if err != nil { + return nil, err + } + return &ipc.StartFreshRunResult{RunID: runID}, nil + }) + srv.Handle(ipc.MethodPushReceived, func(ctx context.Context, params json.RawMessage) (interface{}, error) { // Hooks execute in a managed bare gate by definition, so only the // authenticated peer ancestry is meaningful at this ingress. diff --git a/internal/daemon/manager.go b/internal/daemon/manager.go index b5a9d5b25..8c02c9818 100644 --- a/internal/daemon/manager.go +++ b/internal/daemon/manager.go @@ -791,6 +791,55 @@ func (m *RunManager) HandleRerun(ctx context.Context, repoID, branch, previousRu return m.startRunWithIntentSource(ctx, repo, branch, headSHA, baseSHA, "rerun", skipSteps, intent, intentSource) } +func (m *RunManager) HandleStartFreshRun(ctx context.Context, repoID, branch, headSHA string, skipSteps []types.StepName, intent string) (string, error) { + if strings.TrimSpace(branch) == "" || strings.TrimSpace(headSHA) == "" { + return "", fmt.Errorf("fresh run requires a branch and head") + } + repo, err := m.db.GetRepo(repoID) + if err != nil { + return "", fmt.Errorf("get repo: %w", err) + } + if repo == nil { + return "", fmt.Errorf("unknown repo %s", repoID) + } + + gateDir := m.paths.RepoDir(repo.ID) + gateHead, err := git.ResolveRef(ctx, gateDir, "refs/heads/"+branch) + if err != nil { + return "", fmt.Errorf("resolve gate head: %w", err) + } + if gateHead != headSHA { + return "", fmt.Errorf("gate head for %q changed from %s to %s; retry the fresh run", branch, headSHA, gateHead) + } + + runs, err := m.db.GetRunsByRepo(repoID) + if err != nil { + return "", fmt.Errorf("get runs: %w", err) + } + baseSHA := gateHead + for _, run := range runs { + if run.Branch != branch { + continue + } + if run.Status == types.RunPending || run.Status == types.RunRunning { + return "", fmt.Errorf("an active run already owns branch %s", branch) + } + if baseSHA == gateHead { + baseSHA = run.BaseSHA + } + if run.HeadSHA == gateHead { + baseSHA = run.BaseSHA + break + } + } + + source := "" + if strings.TrimSpace(intent) != "" { + source = db.RunIntentSourceAgent + } + return m.startRunWithIntentSource(ctx, repo, branch, gateHead, baseSHA, "fresh", skipSteps, intent, source) +} + func resolveRerunHead(ctx context.Context, gateDir, branch string, latest *db.Run) (string, error) { gateHead, err := git.Run(ctx, gateDir, "rev-parse", "refs/heads/"+branch+"^{commit}") if err != nil { diff --git a/internal/ipc/protocol.go b/internal/ipc/protocol.go index e1970e7ce..2a564eada 100644 --- a/internal/ipc/protocol.go +++ b/internal/ipc/protocol.go @@ -15,6 +15,7 @@ const ( MethodGetRuns = "get_runs" MethodGetRunsForHead = "get_runs_for_head" MethodGetActiveRun = "get_active_run" + MethodStartFreshRun = "start_fresh_run" MethodRerun = "rerun" MethodSubscribe = "subscribe" MethodRespond = "respond" @@ -132,6 +133,18 @@ type RerunParams struct { Intent string `json:"intent,omitempty"` } +type StartFreshRunParams struct { + RepoID string `json:"repo_id"` + Branch string `json:"branch"` + HeadSHA string `json:"head_sha"` + SkipSteps []types.StepName `json:"skip_steps,omitempty"` + Intent string `json:"intent,omitempty"` +} + +type StartFreshRunResult struct { + RunID string `json:"run_id"` +} + // SubscribeParams starts an event stream for a run. type SubscribeParams struct { RunID string `json:"run_id"` diff --git a/internal/ipc/protocol_test.go b/internal/ipc/protocol_test.go index dc5baad48..92fe88991 100644 --- a/internal/ipc/protocol_test.go +++ b/internal/ipc/protocol_test.go @@ -405,6 +405,7 @@ func TestMethodConstants(t *testing.T) { MethodGetRuns, MethodGetRunsForHead, MethodGetActiveRun, + MethodStartFreshRun, MethodRerun, MethodSubscribe, MethodRespond, From f64b53951b631597a587866351699918c9b56fe8 Mon Sep 17 00:00:00 2001 From: Zach Date: Fri, 28 Aug 2026 09:08:26 +0800 Subject: [PATCH 4/7] no-mistakes(review): Centralized fail-closed custody and preserved fresh-run identity --- internal/branchsync/sync.go | 87 ++++++++++++++++ internal/cli/attach.go | 54 +++++++--- internal/cli/axi_drive.go | 100 ++++-------------- internal/cli/axi_missing_head_test.go | 143 +++++++++++++++++++++++++- internal/daemon/daemon.go | 2 +- internal/daemon/manager.go | 59 +++++++---- internal/ipc/protocol.go | 1 + internal/ipc/protocol_test.go | 4 +- 8 files changed, 327 insertions(+), 123 deletions(-) diff --git a/internal/branchsync/sync.go b/internal/branchsync/sync.go index 52e040a13..1f9c99e94 100644 --- a/internal/branchsync/sync.go +++ b/internal/branchsync/sync.go @@ -263,6 +263,93 @@ func (s *Service) InspectCached(ctx context.Context) State { return state } +func (s *Service) FreshRunOwnershipState(ctx context.Context, expectedBranch, expectedHead string) *State { + state := s.InspectCached(ctx) + if state.Local.Branch == "" { + return &state + } + if expectedBranch != "" && state.Local.Branch != expectedBranch { + state.State = StateAmbiguousContext + state.Safety = "blocked_fresh_context_mismatch" + state.Error = "the fresh run request does not match the checked-out branch" + return &state + } + if expectedHead != "" && state.Local.Head != expectedHead { + state.State = StateAmbiguousContext + state.Safety = "blocked_fresh_context_mismatch" + state.Error = "the fresh run request does not match the checked-out HEAD" + return &state + } + if state.State == StateAmbiguousContext && state.Safety != "blocked_wrong_branch" { + return &state + } + + switch state.State { + case StatePipelineOwned: + if RunHeadUnmoved(state) { + return nil + } + if state.Safety == "blocked_recover_preserved_head_missing" && s.hasNoPipelineEvidence(ctx, state.Local.Branch) { + return nil + } + return &state + case StatePushInProgress: + return &state + default: + return nil + } +} + +func (s *Service) hasNoPipelineEvidence(ctx context.Context, branch string) bool { + if s == nil || s.DB == nil || s.Repo == nil || strings.TrimSpace(branch) == "" { + return false + } + runs, err := s.DB.GetRunsByRepo(s.Repo.ID) + if err != nil { + return false + } + seen := false + for _, run := range runs { + if run.Branch != branch || !unpublishedPipelineHead(run) { + continue + } + seen = true + if !run.Status.Terminal() || run.TerminalHeadVerifiedAt == nil || !s.pipelineEvidenceAbsent(ctx, run) { + return false + } + } + return seen +} + +func (s *Service) pipelineEvidenceAbsent(ctx context.Context, run *db.Run) bool { + if run == nil || strings.TrimSpace(run.HeadSHA) == "" { + return false + } + localHeadExists, err := git.RefExists(ctx, s.workDir(), run.HeadSHA) + if err != nil || localHeadExists || recoveryEvidencePresent(ctx, s.workDir(), run.ID) { + return false + } + + gateDir := strings.TrimSpace(s.GateDir) + if gateDir == "" || git.ValidateBareRepository(ctx, gateDir) != nil { + return false + } + if recoveryEvidencePresent(ctx, gateDir, run.ID) { + return false + } + gateHeadExists, err := git.RefExists(ctx, gateDir, run.HeadSHA) + return err == nil && !gateHeadExists +} + +func recoveryEvidencePresent(ctx context.Context, dir, runID string) bool { + ref := custody.RecoveryRef(runID) + if target, err := git.Run(ctx, dir, "symbolic-ref", "-q", ref); err == nil && strings.TrimSpace(target) != "" { + return true + } + _, exists, err := git.ExactRefTarget(ctx, dir, ref) + return err != nil || exists +} + // Refresh explicitly verifies the exact configured push ref into a private // no-mistakes ref. It never updates an ordinary remote-tracking ref. func (s *Service) Refresh(ctx context.Context) State { diff --git a/internal/cli/attach.go b/internal/cli/attach.go index 5536c0f44..f0123db8a 100644 --- a/internal/cli/attach.go +++ b/internal/cli/attach.go @@ -101,11 +101,14 @@ func attachRun(ctx context.Context, w io.Writer, runID string, rootDefault bool, interactive := terminalInteractive() if rootDefault && runID == "" && repo != nil && state != nil && (autoYes || interactive) { startedViaWizard = true + var wizardRunID string // waitFn blocks inside the wizard's alt screen until the daemon // has the run registered, so the handoff to runTUI below is // seamless rather than flashing the pre-wizard terminal. waitFn := func(ctx context.Context, branch string) error { - return awaitDaemonRunRegistrationOrStartFresh(ctx, client, p, d, repo, state.workDir, branch, skipSteps) + var err error + wizardRunID, err = awaitDaemonRunRegistrationOrStartFresh(ctx, client, p, d, repo, state.workDir, branch, skipSteps) + return err } var res wizard.Result var wErr error @@ -122,17 +125,33 @@ func attachRun(ctx context.Context, w io.Writer, runID string, rootDefault bool, return wErr } if res.Success { - run, err = waitForActiveRun(ctx, client, repo.ID, res.TargetBranch, triggerWaitTimeout) - if err != nil { - return fmt.Errorf("wait for active run: %w", err) - } - if run == nil { - if err := awaitDaemonRunRegistrationOrStartFresh(ctx, client, p, d, repo, state.workDir, res.TargetBranch, skipSteps); err != nil { - return err + if wizardRunID != "" { + var result ipc.GetRunResult + if err := client.Call(ipc.MethodGetRun, &ipc.GetRunParams{RunID: wizardRunID}, &result); err != nil { + return fmt.Errorf("get wizard run: %w", err) + } + run = result.Run + if run == nil { + return fmt.Errorf("wizard run %s no longer exists", wizardRunID) } + } else { run, err = waitForActiveRun(ctx, client, repo.ID, res.TargetBranch, triggerWaitTimeout) if err != nil { - return fmt.Errorf("wait for fresh run: %w", err) + return fmt.Errorf("wait for active run: %w", err) + } + if run == nil { + wizardRunID, err = awaitDaemonRunRegistrationOrStartFresh(ctx, client, p, d, repo, state.workDir, res.TargetBranch, skipSteps) + if err != nil { + return err + } + var result ipc.GetRunResult + if err := client.Call(ipc.MethodGetRun, &ipc.GetRunParams{RunID: wizardRunID}, &result); err != nil { + return fmt.Errorf("get fresh run: %w", err) + } + run = result.Run + if run == nil { + return fmt.Errorf("fresh run %s no longer exists", wizardRunID) + } } } if autoYes && run == nil { @@ -163,26 +182,27 @@ func attachRun(ctx context.Context, w io.Writer, runID string, rootDefault bool, return runTUI(p.Socket(), client, run, update.CachedLatestVersion()) } -func awaitDaemonRunRegistrationOrStartFresh(ctx context.Context, client *ipc.Client, p *paths.Paths, d *db.DB, repo *db.Repo, workDir, branch string, skipSteps []types.StepName) error { +func awaitDaemonRunRegistrationOrStartFresh(ctx context.Context, client *ipc.Client, p *paths.Paths, d *db.DB, repo *db.Repo, workDir, branch string, skipSteps []types.StepName) (string, error) { run, err := waitForActiveRun(ctx, client, repo.ID, branch, triggerWaitTimeout) if err != nil { - return err + return "", err } if run != nil { - return nil + return run.ID, nil } headSHA, err := git.Run(ctx, workDir, "rev-parse", "HEAD") if err != nil { - return fmt.Errorf("resolve current HEAD for %q: %w", branch, err) + return "", fmt.Errorf("resolve current HEAD for %q: %w", branch, err) } if err := guardFreshRun(ctx, p, d, repo, workDir); err != nil { - return err + return "", err } - if _, err := startFreshRun(ctx, client, repo.ID, branch, headSHA, skipSteps, ""); err != nil { - return fmt.Errorf("start fresh run for %q: %w", branch, err) + runID, err := startFreshRun(ctx, client, repo.ID, branch, headSHA, workDir, skipSteps, "") + if err != nil { + return "", fmt.Errorf("start fresh run for %q: %w", branch, err) } - return nil + return runID, nil } func guardFreshRun(ctx context.Context, p *paths.Paths, d *db.DB, repo *db.Repo, workDir string) error { diff --git a/internal/cli/axi_drive.go b/internal/cli/axi_drive.go index 40fcbce71..01abe6baf 100644 --- a/internal/cli/axi_drive.go +++ b/internal/cli/axi_drive.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "io" - "os" "strings" "time" @@ -13,7 +12,6 @@ import ( "github.com/kunchenguid/no-mistakes/internal/branchsync" "github.com/kunchenguid/no-mistakes/internal/cimonitor" - "github.com/kunchenguid/no-mistakes/internal/custody" "github.com/kunchenguid/no-mistakes/internal/daemon" "github.com/kunchenguid/no-mistakes/internal/db" "github.com/kunchenguid/no-mistakes/internal/gate" @@ -282,84 +280,25 @@ func freshRunBranchOwnershipState(ctx context.Context, env *axiEnv) *branchsync. } func freshRunBranchOwnershipStateAt(ctx context.Context, env *axiEnv, workDir string) *branchsync.State { - state := inspectAxiBranchSyncAt(ctx, env, workDir) - switch state.State { - case branchsync.StatePipelineOwned: - // The ownership block exists to keep a fresh push from discarding - // pipeline commits that live only in the gate. An ACTIVE run whose - // head has not moved yet holds none, so the pre-existing supersede - // flow (push new commits over an in-flight run) stays available; a - // terminal unmoved run never reaches here because cancellation - // releases the branch as user_owned. - if branchsync.RunHeadUnmoved(state) { - return nil - } - // A terminal run can retain a moved head in its record after that - // commit and all recovery evidence have disappeared. Keep the - // branch-sync state manual-only, but do not let an unrecoverable - // historical record block unrelated fresh work forever. Any surviving - // commit or recovery anchor remains a custody block. - if state.Safety == "blocked_recover_preserved_head_missing" && freshRunHasNoPipelineEvidence(ctx, env, state, workDir) { - return nil - } - return &state - case branchsync.StatePushInProgress: - return &state - default: - return nil - } -} - -// freshRunHasNoPipelineEvidence permits a fresh delivery only after positive -// local evidence shows that a terminal run's recorded moved head and its -// recovery anchors are gone. Unreadable repositories or conflicting anchors -// fail closed so this exception cannot discard recoverable pipeline work. -func freshRunHasNoPipelineEvidence(ctx context.Context, env *axiEnv, state branchsync.State, workDir string) bool { - if env == nil || env.p == nil || env.repo == nil || strings.TrimSpace(state.Pipeline.RunID) == "" || strings.TrimSpace(state.Pipeline.CurrentHead) == "" { - return false - } - localHeadExists, err := git.RefExists(ctx, workDir, state.Pipeline.CurrentHead) - if err != nil || localHeadExists { - return false - } - if recoveryEvidencePresent(ctx, workDir, state.Pipeline.RunID) { - return false - } - - gateDir := env.p.RepoDir(env.repo.ID) - info, err := os.Stat(gateDir) - if err != nil { - return os.IsNotExist(err) - } - if !info.IsDir() { - return false - } - if err := git.ValidateBareRepository(ctx, gateDir); err != nil { - return false - } - if recoveryEvidencePresent(ctx, gateDir, state.Pipeline.RunID) { - return false - } - gateHeadExists, err := git.RefExists(ctx, gateDir, state.Pipeline.CurrentHead) - if err != nil || gateHeadExists { - return false - } - return true -} - -func recoveryEvidencePresent(ctx context.Context, dir, runID string) bool { - ref := custody.RecoveryRef(runID) - if target, err := git.Run(ctx, dir, "symbolic-ref", "-q", ref); err == nil && strings.TrimSpace(target) != "" { - return true + service := &branchsync.Service{ + DB: env.d, + Repo: env.repo, + WorkDir: workDir, + GateDir: env.p.RepoDir(env.repo.ID), + Paths: env.p, + RemoteTimeout: branchSyncRemoteTimeout(env), } - _, exists, err := git.ExactRefTarget(ctx, dir, ref) - return err != nil || exists + return service.FreshRunOwnershipState(ctx, "", "") } // triggerRun starts a fresh run for branch by pushing the current HEAD through // the gate. Callers must check for an existing active run first (see // activeRunID) and apply pre-flight guards. func triggerRun(ctx context.Context, env *axiEnv, branch, headSHA string, skipSteps []types.StepName, intent string) (string, error) { + workDir, err := git.FindGitRoot(".") + if err != nil { + return "", fmt.Errorf("resolve current worktree: %w", err) + } pushOptions := formatSkipPushOptions(skipSteps) if opt := formatIntentPushOption(intent); opt != "" { pushOptions = append(pushOptions, opt) @@ -370,7 +309,7 @@ func triggerRun(ctx context.Context, env *axiEnv, branch, headSHA string, skipSt // a matching terminal run may predate this push, so do not attach to it. priorRunIDs = nil } - if state := freshRunBranchOwnershipState(ctx, env); state != nil { + if state := freshRunBranchOwnershipStateAt(ctx, env, workDir); state != nil { return "", &branchOwnershipError{state: *state} } gateHeadBefore, gateHeadErr := git.Run(ctx, env.p.RepoDir(env.repo.ID), "rev-parse", "refs/heads/"+branch+"^{commit}") @@ -380,15 +319,15 @@ func triggerRun(ctx context.Context, env *axiEnv, branch, headSHA string, skipSt // Close the inspection-to-push race: if the pipeline advanced ownership // after the pre-push check, preserve the structured branch-sync refusal // instead of leaking the resulting Git non-fast-forward. - if state := freshRunBranchOwnershipState(ctx, env); state != nil { + if state := freshRunBranchOwnershipStateAt(ctx, env, workDir); state != nil { return "", &branchOwnershipError{state: *state} } } if pushErr == nil && gateWasCurrent { - if state := freshRunBranchOwnershipState(ctx, env); state != nil { + if state := freshRunBranchOwnershipStateAt(ctx, env, workDir); state != nil { return "", &branchOwnershipError{state: *state} } - runID, err := startFreshRun(ctx, env.client, env.repo.ID, branch, headSHA, skipSteps, intent) + runID, err := startFreshRun(ctx, env.client, env.repo.ID, branch, headSHA, workDir, skipSteps, intent) if err != nil { return "", fmt.Errorf("start fresh run for %q: %v", branch, err) } @@ -402,22 +341,23 @@ func triggerRun(ctx context.Context, env *axiEnv, branch, headSHA string, skipSt return "", fmt.Errorf("push %q to gate: %v", branch, pushErr) } - if state := freshRunBranchOwnershipState(ctx, env); state != nil { + if state := freshRunBranchOwnershipStateAt(ctx, env, workDir); state != nil { return "", &branchOwnershipError{state: *state} } - runID, err := startFreshRun(ctx, env.client, env.repo.ID, branch, headSHA, skipSteps, intent) + runID, err := startFreshRun(ctx, env.client, env.repo.ID, branch, headSHA, workDir, skipSteps, intent) if err != nil { return "", fmt.Errorf("no run started for %q: %v", branch, err) } return runID, nil } -func startFreshRun(ctx context.Context, client *ipc.Client, repoID, branch, headSHA string, skipSteps []types.StepName, intent string) (string, error) { +func startFreshRun(ctx context.Context, client *ipc.Client, repoID, branch, headSHA, workDir string, skipSteps []types.StepName, intent string) (string, error) { var result ipc.StartFreshRunResult if err := client.Call(ipc.MethodStartFreshRun, &ipc.StartFreshRunParams{ RepoID: repoID, Branch: branch, HeadSHA: headSHA, + WorkDir: workDir, SkipSteps: skipSteps, Intent: intent, }, &result); err != nil { diff --git a/internal/cli/axi_missing_head_test.go b/internal/cli/axi_missing_head_test.go index 83a6016f7..295f8e98e 100644 --- a/internal/cli/axi_missing_head_test.go +++ b/internal/cli/axi_missing_head_test.go @@ -15,6 +15,7 @@ import ( "github.com/kunchenguid/no-mistakes/internal/ipc" "github.com/kunchenguid/no-mistakes/internal/paths" "github.com/kunchenguid/no-mistakes/internal/types" + "github.com/kunchenguid/no-mistakes/internal/wizard" ) // TestFreshRunBranchOwnershipDistinguishesMissingTerminalHead reproduces the @@ -28,6 +29,7 @@ func TestFreshRunBranchOwnershipDistinguishesMissingTerminalHead(t *testing.T) { recordedHead func(t *testing.T, submitted string) string advanceWorktree bool gatePresent bool + olderRecoverable bool survivingAnchor bool verifiedHead bool objectReadError bool @@ -62,6 +64,24 @@ func TestFreshRunBranchOwnershipDistinguishesMissingTerminalHead(t *testing.T) { wantFreshBlocked: false, wantSafety: "blocked_recover_preserved_head_missing", }, + { + name: "missing gate keeps fresh path blocked", + recordedHead: func(_ *testing.T, _ string) string { + return strings.Repeat("f", 40) + }, + wantFreshBlocked: true, + wantSafety: "blocked_recover_preserved_head_missing", + }, + { + name: "unverified missing head keeps fresh path blocked", + recordedHead: func(_ *testing.T, _ string) string { + return strings.Repeat("f", 40) + }, + gatePresent: true, + verifiedHead: false, + wantFreshBlocked: true, + wantSafety: "blocked_recover_preserved_head_missing", + }, { name: "missing head with recovery evidence keeps custody", recordedHead: func(_ *testing.T, _ string) string { @@ -72,6 +92,16 @@ func TestFreshRunBranchOwnershipDistinguishesMissingTerminalHead(t *testing.T) { wantFreshBlocked: true, wantSafety: "blocked_recover_preserved_head_missing", }, + { + name: "older recoverable head keeps custody", + recordedHead: func(_ *testing.T, _ string) string { + return strings.Repeat("f", 40) + }, + gatePresent: true, + olderRecoverable: true, + wantFreshBlocked: true, + wantSafety: "blocked_recover_preserved_head_missing", + }, { name: "object read failure keeps custody", recordedHead: func(_ *testing.T, _ string) string { @@ -90,16 +120,31 @@ func TestFreshRunBranchOwnershipDistinguishesMissingTerminalHead(t *testing.T) { submitted := cliGit(t, repoDir, "rev-parse", "HEAD") recorded := tc.recordedHead(t, submitted) - if tc.advanceWorktree { - cliGit(t, repoDir, "commit", "--allow-empty", "-m", "pipeline fix") - recorded = cliGit(t, repoDir, "rev-parse", "HEAD") - } + var gateDir string if tc.gatePresent { - gateDir := paths.RepoDir(repo.ID) + gateDir = paths.RepoDir(repo.ID) if err := os.MkdirAll(gateDir, 0o755); err != nil { t.Fatalf("create gate directory: %v", err) } cliGit(t, gateDir, "init", "--bare") + } + if tc.olderRecoverable { + cliGit(t, repoDir, "commit", "--allow-empty", "-m", "older pipeline fix") + olderHead := cliGit(t, repoDir, "rev-parse", "HEAD") + cliGit(t, repoDir, "push", gateDir, "HEAD:refs/heads/feature/missing-head") + olderRun, err := database.InsertRun(repo.ID, "feature/missing-head", submitted, submitted) + if err != nil { + t.Fatalf("insert older pipeline run: %v", err) + } + if err := database.UpdateRunStatusWithVerifiedHead(olderRun.ID, types.RunCancelled, olderHead); err != nil { + t.Fatalf("terminalize older pipeline run: %v", err) + } + } + if tc.advanceWorktree { + cliGit(t, repoDir, "commit", "--allow-empty", "-m", "pipeline fix") + recorded = cliGit(t, repoDir, "rev-parse", "HEAD") + } + if tc.gatePresent && !tc.olderRecoverable { cliGit(t, repoDir, "push", gateDir, "HEAD:refs/heads/feature/missing-head") if tc.objectReadError { objectPath := filepath.Join(gateDir, "objects", recorded[:2], recorded[2:]) @@ -243,3 +288,91 @@ func TestRootWizardStartsFreshRunWhenNoopGateHasMissingTerminalHead(t *testing.T t.Fatalf("wizard attached run = %#v, want head %s", attached, f.head) } } + +func TestRootWizardKeepsRunIdentityAfterTerminalWait(t *testing.T) { + f := newMissingHeadFreshRunFixture(t) + + previousInteractive := terminalInteractive + terminalInteractive = func() bool { return true } + defer func() { terminalInteractive = previousInteractive }() + previousTimeout := triggerWaitTimeout + triggerWaitTimeout = 100 * time.Millisecond + defer func() { triggerWaitTimeout = previousTimeout }() + + previousWizardRun := wizardRun + var startedRunID string + wizardRun = func(cfg wizard.Config) (wizard.Result, error) { + if err := cfg.WaitForRun(context.Background(), f.branch); err != nil { + return wizard.Result{}, err + } + runs, err := f.d.GetRunsByRepo(f.repo.ID) + if err != nil { + return wizard.Result{}, err + } + startedRunID = runs[0].ID + if err := f.d.UpdateRunStatus(startedRunID, types.RunCompleted); err != nil { + return wizard.Result{}, err + } + return wizard.Result{Success: true, Pushed: true, TargetBranch: f.branch}, nil + } + defer func() { wizardRun = previousWizardRun }() + + previousRunTUI := runTUI + var attached *ipc.RunInfo + runTUI = func(_ string, _ *ipc.Client, run *ipc.RunInfo, _ string) error { + attached = run + return nil + } + defer func() { runTUI = previousRunTUI }() + + if _, err := executeCmd(); err != nil { + t.Fatalf("executeCmd() error = %v", err) + } + if attached == nil || attached.ID != startedRunID { + t.Fatalf("wizard attached run = %#v, want run %s", attached, startedRunID) + } + runs, err := f.d.GetRunsByRepo(f.repo.ID) + if err != nil { + t.Fatal(err) + } + if len(runs) != 2 { + t.Fatalf("runs after terminal wizard handoff = %d, want original plus one fresh run", len(runs)) + } +} + +func TestDaemonFreshRunRechecksPipelineCustody(t *testing.T) { + f := newMissingHeadFreshRunFixture(t) + cliGit(t, f.repoDir, "commit", "--allow-empty", "-m", "recoverable pipeline fix") + movedHead := cliGit(t, f.repoDir, "rev-parse", "HEAD") + cliGit(t, f.repoDir, "push", f.paths.RepoDir(f.repo.ID), "HEAD:refs/heads/"+f.branch) + recoverable, err := f.d.InsertRun(f.repo.ID, f.branch, f.head, f.head) + if err != nil { + t.Fatal(err) + } + if err := f.d.UpdateRunStatusWithVerifiedHead(recoverable.ID, types.RunCancelled, movedHead); err != nil { + t.Fatal(err) + } + + client, err := ipc.Dial(f.paths.Socket()) + if err != nil { + t.Fatal(err) + } + defer client.Close() + var result ipc.StartFreshRunResult + err = client.Call(ipc.MethodStartFreshRun, &ipc.StartFreshRunParams{ + RepoID: f.repo.ID, + Branch: f.branch, + HeadSHA: movedHead, + WorkDir: f.repoDir, + }, &result) + if err == nil || !strings.Contains(err.Error(), "fresh run blocked") { + t.Fatalf("fresh run IPC error = %v, want custody refusal", err) + } + runs, err := f.d.GetRunsByRepo(f.repo.ID) + if err != nil { + t.Fatal(err) + } + if len(runs) != 2 { + t.Fatalf("runs after custody refusal = %d, want 2", len(runs)) + } +} diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index 525d61b81..e8e313a4c 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -1209,7 +1209,7 @@ func registerHandlers(srv *ipc.Server, mgr *RunManager, d *db.DB, shutdown func( if err := json.Unmarshal(params, &p); err != nil { return nil, fmt.Errorf("invalid params: %w", err) } - runID, err := mgr.HandleStartFreshRun(ctx, p.RepoID, p.Branch, p.HeadSHA, p.SkipSteps, p.Intent) + runID, err := mgr.HandleStartFreshRun(ctx, p.RepoID, p.Branch, p.HeadSHA, p.WorkDir, p.SkipSteps, p.Intent) if err != nil { return nil, err } diff --git a/internal/daemon/manager.go b/internal/daemon/manager.go index 8c02c9818..f25c6245d 100644 --- a/internal/daemon/manager.go +++ b/internal/daemon/manager.go @@ -13,6 +13,7 @@ import ( "time" "github.com/kunchenguid/no-mistakes/internal/agent" + "github.com/kunchenguid/no-mistakes/internal/branchsync" "github.com/kunchenguid/no-mistakes/internal/config" "github.com/kunchenguid/no-mistakes/internal/custody" "github.com/kunchenguid/no-mistakes/internal/db" @@ -791,10 +792,14 @@ func (m *RunManager) HandleRerun(ctx context.Context, repoID, branch, previousRu return m.startRunWithIntentSource(ctx, repo, branch, headSHA, baseSHA, "rerun", skipSteps, intent, intentSource) } -func (m *RunManager) HandleStartFreshRun(ctx context.Context, repoID, branch, headSHA string, skipSteps []types.StepName, intent string) (string, error) { - if strings.TrimSpace(branch) == "" || strings.TrimSpace(headSHA) == "" { - return "", fmt.Errorf("fresh run requires a branch and head") +func (m *RunManager) HandleStartFreshRun(ctx context.Context, repoID, branch, headSHA, workDir string, skipSteps []types.StepName, intent string) (string, error) { + if strings.TrimSpace(branch) == "" || strings.TrimSpace(headSHA) == "" || strings.TrimSpace(workDir) == "" { + return "", fmt.Errorf("fresh run requires a branch, head, and worktree") } + branchMu := m.branchLock(repoID, branch) + branchMu.Lock() + defer branchMu.Unlock() + repo, err := m.db.GetRepo(repoID) if err != nil { return "", fmt.Errorf("get repo: %w", err) @@ -802,6 +807,13 @@ func (m *RunManager) HandleStartFreshRun(ctx context.Context, repoID, branch, he if repo == nil { return "", fmt.Errorf("unknown repo %s", repoID) } + active, err := m.db.GetActiveRun(repoID, branch) + if err != nil { + return "", fmt.Errorf("get active run: %w", err) + } + if active != nil { + return "", fmt.Errorf("an active run already owns branch %s", branch) + } gateDir := m.paths.RepoDir(repo.ID) gateHead, err := git.ResolveRef(ctx, gateDir, "refs/heads/"+branch) @@ -811,6 +823,18 @@ func (m *RunManager) HandleStartFreshRun(ctx context.Context, repoID, branch, he if gateHead != headSHA { return "", fmt.Errorf("gate head for %q changed from %s to %s; retry the fresh run", branch, headSHA, gateHead) } + if state := (&branchsync.Service{ + DB: m.db, + Repo: repo, + WorkDir: workDir, + GateDir: gateDir, + Paths: m.paths, + }).FreshRunOwnershipState(ctx, branch, headSHA); state != nil { + if state.Error != "" { + return "", fmt.Errorf("fresh run blocked: %s", state.Error) + } + return "", fmt.Errorf("fresh run blocked: pipeline custody is unresolved") + } runs, err := m.db.GetRunsByRepo(repoID) if err != nil { @@ -821,9 +845,6 @@ func (m *RunManager) HandleStartFreshRun(ctx context.Context, repoID, branch, he if run.Branch != branch { continue } - if run.Status == types.RunPending || run.Status == types.RunRunning { - return "", fmt.Errorf("an active run already owns branch %s", branch) - } if baseSHA == gateHead { baseSHA = run.BaseSHA } @@ -837,7 +858,7 @@ func (m *RunManager) HandleStartFreshRun(ctx context.Context, repoID, branch, he if strings.TrimSpace(intent) != "" { source = db.RunIntentSourceAgent } - return m.startRunWithIntentSource(ctx, repo, branch, gateHead, baseSHA, "fresh", skipSteps, intent, source) + return m.startRunWithIntentSourceLocked(ctx, repo, branch, gateHead, baseSHA, "fresh", skipSteps, intent, source) } func resolveRerunHead(ctx context.Context, gateDir, branch string, latest *db.Run) (string, error) { @@ -902,10 +923,20 @@ func (m *RunManager) startRun(ctx context.Context, repo *db.Repo, branch, headSH return m.startRunWithIntentSource(ctx, repo, branch, headSHA, baseSHA, trigger, skipSteps, intent, db.RunIntentSourceAgent) } -// startRunWithIntentSource is the common run-creation path. source is empty -// when no intent is supplied, RunIntentSourceAgent for a new explicit -// override, and RunIntentSourceRerun for inherited explicit intent. +// branchLock returns the serializer for one repository branch. +func (m *RunManager) branchLock(repoID, branch string) *sync.Mutex { + lockVal, _ := m.branchLocks.LoadOrStore(repoID+"/"+branch, &sync.Mutex{}) + return lockVal.(*sync.Mutex) +} + func (m *RunManager) startRunWithIntentSource(ctx context.Context, repo *db.Repo, branch, headSHA, baseSHA, trigger string, skipSteps []types.StepName, intent, source string) (string, error) { + branchMu := m.branchLock(repo.ID, branch) + branchMu.Lock() + defer branchMu.Unlock() + return m.startRunWithIntentSourceLocked(ctx, repo, branch, headSHA, baseSHA, trigger, skipSteps, intent, source) +} + +func (m *RunManager) startRunWithIntentSourceLocked(ctx context.Context, repo *db.Repo, branch, headSHA, baseSHA, trigger string, skipSteps []types.StepName, intent, source string) (string, error) { branchRole := telemetryBranchRole(branch, repo.DefaultBranch) trackStartFailure := func(stage string) { telemetry.Track("run", telemetry.Fields{ @@ -921,14 +952,6 @@ func (m *RunManager) startRunWithIntentSource(ctx context.Context, repo *db.Repo return "", fmt.Errorf("daemon is shutting down") } - // Serialize per repo+branch to prevent two concurrent pushes from both - // passing cancelActiveRuns and creating duplicate pipelines. - lockKey := repo.ID + "/" + branch - lockVal, _ := m.branchLocks.LoadOrStore(lockKey, &sync.Mutex{}) - branchMu := lockVal.(*sync.Mutex) - branchMu.Lock() - defer branchMu.Unlock() - // Best-effort only: a clone's remotes may change after init. Refresh the // registered URLs before constructing any run-owned Git operation, but keep // the exact prior repo value and continue when discovery, validation, or the diff --git a/internal/ipc/protocol.go b/internal/ipc/protocol.go index 2a564eada..89809cbe8 100644 --- a/internal/ipc/protocol.go +++ b/internal/ipc/protocol.go @@ -137,6 +137,7 @@ type StartFreshRunParams struct { RepoID string `json:"repo_id"` Branch string `json:"branch"` HeadSHA string `json:"head_sha"` + WorkDir string `json:"work_dir"` SkipSteps []types.StepName `json:"skip_steps,omitempty"` Intent string `json:"intent,omitempty"` } diff --git a/internal/ipc/protocol_test.go b/internal/ipc/protocol_test.go index 92fe88991..dddb4a283 100644 --- a/internal/ipc/protocol_test.go +++ b/internal/ipc/protocol_test.go @@ -425,8 +425,8 @@ func TestMethodConstants(t *testing.T) { } seen[m] = true } - if len(methods) != 14 { - t.Errorf("expected 14 methods, got %d", len(methods)) + if len(methods) != 15 { + t.Errorf("expected 15 methods, got %d", len(methods)) } } From 56b6ca954828e4453d691a71343276a466399f9a Mon Sep 17 00:00:00 2001 From: Zach Date: Fri, 28 Aug 2026 11:08:06 +0800 Subject: [PATCH 5/7] test(cli): repair missing-head validation fixture --- internal/cli/attach.go | 1 + internal/cli/axi_missing_head_test.go | 15 ++++++++------- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/internal/cli/attach.go b/internal/cli/attach.go index f0123db8a..e3cb415ff 100644 --- a/internal/cli/attach.go +++ b/internal/cli/attach.go @@ -11,6 +11,7 @@ import ( "github.com/kunchenguid/no-mistakes/internal/db" "github.com/kunchenguid/no-mistakes/internal/git" "github.com/kunchenguid/no-mistakes/internal/ipc" + "github.com/kunchenguid/no-mistakes/internal/paths" "github.com/kunchenguid/no-mistakes/internal/telemetry" "github.com/kunchenguid/no-mistakes/internal/tui" "github.com/kunchenguid/no-mistakes/internal/types" diff --git a/internal/cli/axi_missing_head_test.go b/internal/cli/axi_missing_head_test.go index 295f8e98e..e2165e1aa 100644 --- a/internal/cli/axi_missing_head_test.go +++ b/internal/cli/axi_missing_head_test.go @@ -61,6 +61,7 @@ func TestFreshRunBranchOwnershipDistinguishesMissingTerminalHead(t *testing.T) { return strings.Repeat("f", 40) }, gatePresent: true, + verifiedHead: true, wantFreshBlocked: false, wantSafety: "blocked_recover_preserved_head_missing", }, @@ -198,10 +199,10 @@ func TestFreshRunBranchOwnershipDistinguishesMissingTerminalHead(t *testing.T) { type missingHeadFreshRunFixture struct { repoDir string paths *paths.Paths - d *db.DB - repo *db.Repo - branch string - head string + d *db.DB + repo *db.Repo + branch string + head string } func newMissingHeadFreshRunFixture(t *testing.T) missingHeadFreshRunFixture { @@ -220,8 +221,8 @@ func newMissingHeadFreshRunFixture(t *testing.T) missingHeadFreshRunFixture { branch := "feature/missing-head" run(t, repoDir, "git", "checkout", "-b", branch) - run(t, repoDir, "git", "push", gate.RemoteName, "HEAD:refs/heads/"+branch) - head := gitOutput(t, repoDir, "rev-parse", "HEAD") + head := cliGit(t, repoDir, "rev-parse", "HEAD") + cliGit(t, p.RepoDir(repo.ID), "fetch", repoDir, "HEAD:refs/heads/"+branch) missing := strings.Repeat("f", 40) pipelineRun, err := d.InsertRun(repo.ID, branch, head, head) if err != nil { @@ -344,7 +345,7 @@ func TestDaemonFreshRunRechecksPipelineCustody(t *testing.T) { f := newMissingHeadFreshRunFixture(t) cliGit(t, f.repoDir, "commit", "--allow-empty", "-m", "recoverable pipeline fix") movedHead := cliGit(t, f.repoDir, "rev-parse", "HEAD") - cliGit(t, f.repoDir, "push", f.paths.RepoDir(f.repo.ID), "HEAD:refs/heads/"+f.branch) + cliGit(t, f.paths.RepoDir(f.repo.ID), "fetch", f.repoDir, "HEAD:refs/heads/"+f.branch) recoverable, err := f.d.InsertRun(f.repo.ID, f.branch, f.head, f.head) if err != nil { t.Fatal(err) From 7729058a8009e48cef486227cf2018861f070f0d Mon Sep 17 00:00:00 2001 From: Zach Date: Fri, 28 Aug 2026 16:46:31 +0800 Subject: [PATCH 6/7] no-mistakes(review): Hardened fresh-run custody and exact run handoff --- internal/branchsync/sync.go | 2 +- internal/cli/attach.go | 50 ++++++---- internal/cli/axi_drive.go | 126 +++++++++++++++++++------- internal/cli/axi_missing_head_test.go | 91 +++++++++++++++++++ internal/cli/root_test.go | 4 +- internal/cli/wizard.go | 4 +- internal/daemon/daemon.go | 2 +- internal/daemon/manager.go | 42 +++++++-- internal/ipc/protocol.go | 13 +-- 9 files changed, 261 insertions(+), 73 deletions(-) diff --git a/internal/branchsync/sync.go b/internal/branchsync/sync.go index 1f9c99e94..18ec80a2e 100644 --- a/internal/branchsync/sync.go +++ b/internal/branchsync/sync.go @@ -286,7 +286,7 @@ func (s *Service) FreshRunOwnershipState(ctx context.Context, expectedBranch, ex switch state.State { case StatePipelineOwned: - if RunHeadUnmoved(state) { + if RunHeadUnmoved(state) && (state.Pipeline.Status == string(types.RunPending) || state.Pipeline.Status == string(types.RunRunning)) { return nil } if state.Safety == "blocked_recover_preserved_head_missing" && s.hasNoPipelineEvidence(ctx, state.Local.Branch) { diff --git a/internal/cli/attach.go b/internal/cli/attach.go index e3cb415ff..f257547a4 100644 --- a/internal/cli/attach.go +++ b/internal/cli/attach.go @@ -9,7 +9,6 @@ import ( "github.com/kunchenguid/no-mistakes/internal/daemon" "github.com/kunchenguid/no-mistakes/internal/db" - "github.com/kunchenguid/no-mistakes/internal/git" "github.com/kunchenguid/no-mistakes/internal/ipc" "github.com/kunchenguid/no-mistakes/internal/paths" "github.com/kunchenguid/no-mistakes/internal/telemetry" @@ -76,8 +75,17 @@ func attachRun(ctx context.Context, w io.Writer, runID string, rootDefault bool, if err != nil { return err } - state.beforePush = func(ctx context.Context, workDir string) error { - return guardFreshRun(ctx, p, d, repo, workDir) + var wizardIdentity *freshRunIdentity + state.beforePush = func(ctx context.Context, workDir, branch string) error { + identity, err := captureFreshRunIdentity(ctx, client, repo.ID, workDir, branch) + if err != nil { + return err + } + if err := guardFreshRun(ctx, p, d, repo, workDir); err != nil { + return err + } + wizardIdentity = identity + return nil } // Skip the active-run check entirely when the state clearly calls @@ -108,7 +116,7 @@ func attachRun(ctx context.Context, w io.Writer, runID string, rootDefault bool, // seamless rather than flashing the pre-wizard terminal. waitFn := func(ctx context.Context, branch string) error { var err error - wizardRunID, err = awaitDaemonRunRegistrationOrStartFresh(ctx, client, p, d, repo, state.workDir, branch, skipSteps) + wizardRunID, err = awaitDaemonRunRegistrationOrStartFresh(ctx, client, p, d, repo, state.workDir, branch, wizardIdentity, skipSteps) return err } var res wizard.Result @@ -136,12 +144,8 @@ func attachRun(ctx context.Context, w io.Writer, runID string, rootDefault bool, return fmt.Errorf("wizard run %s no longer exists", wizardRunID) } } else { - run, err = waitForActiveRun(ctx, client, repo.ID, res.TargetBranch, triggerWaitTimeout) - if err != nil { - return fmt.Errorf("wait for active run: %w", err) - } - if run == nil { - wizardRunID, err = awaitDaemonRunRegistrationOrStartFresh(ctx, client, p, d, repo, state.workDir, res.TargetBranch, skipSteps) + if wizardIdentity != nil { + wizardRunID, err = awaitDaemonRunRegistrationOrStartFresh(ctx, client, p, d, repo, state.workDir, res.TargetBranch, wizardIdentity, skipSteps) if err != nil { return err } @@ -153,6 +157,14 @@ func attachRun(ctx context.Context, w io.Writer, runID string, rootDefault bool, if run == nil { return fmt.Errorf("fresh run %s no longer exists", wizardRunID) } + } else { + run, err = waitForActiveRun(ctx, client, repo.ID, res.TargetBranch, triggerWaitTimeout) + if err != nil { + return fmt.Errorf("wait for active run: %w", err) + } + if run == nil { + return fmt.Errorf("wizard did not record a pushed branch and HEAD for %q", res.TargetBranch) + } } } if autoYes && run == nil { @@ -183,23 +195,27 @@ func attachRun(ctx context.Context, w io.Writer, runID string, rootDefault bool, return runTUI(p.Socket(), client, run, update.CachedLatestVersion()) } -func awaitDaemonRunRegistrationOrStartFresh(ctx context.Context, client *ipc.Client, p *paths.Paths, d *db.DB, repo *db.Repo, workDir, branch string, skipSteps []types.StepName) (string, error) { - run, err := waitForActiveRun(ctx, client, repo.ID, branch, triggerWaitTimeout) +func awaitDaemonRunRegistrationOrStartFresh(ctx context.Context, client *ipc.Client, p *paths.Paths, d *db.DB, repo *db.Repo, workDir, branch string, identity *freshRunIdentity, skipSteps []types.StepName) (string, error) { + if identity == nil || identity.branch != branch || identity.headSHA == "" || identity.priorRunIDs == nil { + return "", fmt.Errorf("cannot identify the run created for %q: pre-push branch, HEAD, or run baseline is unavailable", branch) + } + run, err := waitForTriggeredRunForHead(ctx, client, repo.ID, branch, identity.headSHA, identity.priorRunIDs, triggerWaitTimeout) if err != nil { return "", err } if run != nil { + if err := validateFreshRunContext(ctx, workDir, identity.branch, identity.headSHA); err != nil { + return "", err + } return run.ID, nil } - - headSHA, err := git.Run(ctx, workDir, "rev-parse", "HEAD") - if err != nil { - return "", fmt.Errorf("resolve current HEAD for %q: %w", branch, err) + if err := validateFreshRunContext(ctx, workDir, identity.branch, identity.headSHA); err != nil { + return "", err } if err := guardFreshRun(ctx, p, d, repo, workDir); err != nil { return "", err } - runID, err := startFreshRun(ctx, client, repo.ID, branch, headSHA, workDir, skipSteps, "") + runID, err := startFreshRun(ctx, client, repo.ID, branch, identity.headSHA, workDir, identity.priorRunIDs, skipSteps, "") if err != nil { return "", fmt.Errorf("start fresh run for %q: %w", branch, err) } diff --git a/internal/cli/axi_drive.go b/internal/cli/axi_drive.go index 01abe6baf..5d55bdd38 100644 --- a/internal/cli/axi_drive.go +++ b/internal/cli/axi_drive.go @@ -291,6 +291,45 @@ func freshRunBranchOwnershipStateAt(ctx context.Context, env *axiEnv, workDir st return service.FreshRunOwnershipState(ctx, "", "") } +type freshRunIdentity struct { + branch string + headSHA string + priorRunIDs map[string]struct{} +} + +func captureFreshRunIdentity(ctx context.Context, client *ipc.Client, repoID, workDir, branch string) (*freshRunIdentity, error) { + headSHA, err := git.HeadSHA(ctx, workDir) + if err != nil { + return nil, fmt.Errorf("resolve current HEAD for %q: %w", branch, err) + } + if err := validateFreshRunContext(ctx, workDir, branch, headSHA); err != nil { + return nil, err + } + priorRunIDs, err := runIDsForHead(client, repoID, branch, headSHA) + if err != nil { + return nil, fmt.Errorf("snapshot runs for %q at %s: %w", branch, headSHA, err) + } + return &freshRunIdentity{branch: branch, headSHA: headSHA, priorRunIDs: priorRunIDs}, nil +} + +func validateFreshRunContext(ctx context.Context, workDir, branch, headSHA string) error { + currentBranch, err := git.CurrentBranch(ctx, workDir) + if err != nil { + return fmt.Errorf("recheck current branch for %q: %w", branch, err) + } + if currentBranch != branch { + return fmt.Errorf("fresh run context changed from branch %q to %q", branch, currentBranch) + } + currentHead, err := git.HeadSHA(ctx, workDir) + if err != nil { + return fmt.Errorf("recheck current HEAD for %q: %w", branch, err) + } + if currentHead != headSHA { + return fmt.Errorf("fresh run context changed from HEAD %s to %s", headSHA, currentHead) + } + return nil +} + // triggerRun starts a fresh run for branch by pushing the current HEAD through // the gate. Callers must check for an existing active run first (see // activeRunID) and apply pre-flight guards. @@ -299,21 +338,20 @@ func triggerRun(ctx context.Context, env *axiEnv, branch, headSHA string, skipSt if err != nil { return "", fmt.Errorf("resolve current worktree: %w", err) } + if err := validateFreshRunContext(ctx, workDir, branch, headSHA); err != nil { + return "", err + } pushOptions := formatSkipPushOptions(skipSteps) if opt := formatIntentPushOption(intent); opt != "" { pushOptions = append(pushOptions, opt) } priorRunIDs, err := runIDsForHead(env.client, env.repo.ID, branch, headSHA) if err != nil { - // An active run can still be found below. Without a baseline, however, - // a matching terminal run may predate this push, so do not attach to it. - priorRunIDs = nil + return "", fmt.Errorf("snapshot runs for %q at %s: %w", branch, headSHA, err) } if state := freshRunBranchOwnershipStateAt(ctx, env, workDir); state != nil { return "", &branchOwnershipError{state: *state} } - gateHeadBefore, gateHeadErr := git.Run(ctx, env.p.RepoDir(env.repo.ID), "rev-parse", "refs/heads/"+branch+"^{commit}") - gateWasCurrent := gateHeadErr == nil && gateHeadBefore == headSHA pushErr := git.PushWithOptions(ctx, ".", gate.RemoteName, "refs/heads/"+branch, "", false, pushOptions) if pushErr != nil { // Close the inspection-to-push race: if the pipeline advanced ownership @@ -323,18 +361,14 @@ func triggerRun(ctx context.Context, env *axiEnv, branch, headSHA string, skipSt return "", &branchOwnershipError{state: *state} } } - if pushErr == nil && gateWasCurrent { - if state := freshRunBranchOwnershipStateAt(ctx, env, workDir); state != nil { - return "", &branchOwnershipError{state: *state} - } - runID, err := startFreshRun(ctx, env.client, env.repo.ID, branch, headSHA, workDir, skipSteps, intent) - if err != nil { - return "", fmt.Errorf("start fresh run for %q: %v", branch, err) - } - return runID, nil + run, waitErr := waitForTriggeredRunForHead(ctx, env.client, env.repo.ID, branch, headSHA, priorRunIDs, triggerWaitTimeout) + if waitErr != nil { + return "", fmt.Errorf("wait for triggered run for %q: %w", branch, waitErr) } - - if run, _ := waitForTriggeredRunForHead(ctx, env.client, env.repo.ID, branch, headSHA, priorRunIDs, triggerWaitTimeout); run != nil { + if run != nil { + if err := validateFreshRunContext(ctx, workDir, branch, headSHA); err != nil { + return "", err + } return run.ID, nil } if !shouldRerunAfterNoActiveRun(pushErr) { @@ -344,22 +378,33 @@ func triggerRun(ctx context.Context, env *axiEnv, branch, headSHA string, skipSt if state := freshRunBranchOwnershipStateAt(ctx, env, workDir); state != nil { return "", &branchOwnershipError{state: *state} } - runID, err := startFreshRun(ctx, env.client, env.repo.ID, branch, headSHA, workDir, skipSteps, intent) + runID, err := startFreshRun(ctx, env.client, env.repo.ID, branch, headSHA, workDir, priorRunIDs, skipSteps, intent) if err != nil { return "", fmt.Errorf("no run started for %q: %v", branch, err) } return runID, nil } -func startFreshRun(ctx context.Context, client *ipc.Client, repoID, branch, headSHA, workDir string, skipSteps []types.StepName, intent string) (string, error) { +func startFreshRun(ctx context.Context, client *ipc.Client, repoID, branch, headSHA, workDir string, priorRunIDs map[string]struct{}, skipSteps []types.StepName, intent string) (string, error) { + if priorRunIDs == nil { + return "", fmt.Errorf("fresh run requires a pre-push run identity baseline") + } + ids := make([]string, 0, len(priorRunIDs)) + for id := range priorRunIDs { + if strings.TrimSpace(id) == "" { + return "", fmt.Errorf("fresh run received an empty pre-push run identity") + } + ids = append(ids, id) + } var result ipc.StartFreshRunResult if err := client.Call(ipc.MethodStartFreshRun, &ipc.StartFreshRunParams{ - RepoID: repoID, - Branch: branch, - HeadSHA: headSHA, - WorkDir: workDir, - SkipSteps: skipSteps, - Intent: intent, + RepoID: repoID, + Branch: branch, + HeadSHA: headSHA, + WorkDir: workDir, + PriorRunIDs: ids, + SkipSteps: skipSteps, + Intent: intent, }, &result); err != nil { return "", err } @@ -381,6 +426,9 @@ func runIDsForHead(client *ipc.Client, repoID, branch, headSHA string) (map[stri } ids := make(map[string]struct{}, len(runs)) for _, run := range runs { + if strings.TrimSpace(run.ID) == "" { + return nil, fmt.Errorf("exact-head run has an empty ID") + } ids[run.ID] = struct{}{} } return ids, nil @@ -399,6 +447,9 @@ func runsForHead(client *ipc.Client, repoID, branch, headSHA string) ([]ipc.RunI // that fails before it can be observed as active. priorRunIDs prevents an // up-to-date push from attaching to a terminal run created by an earlier one. func waitForTriggeredRunForHead(ctx context.Context, client *ipc.Client, repoID, branch, headSHA string, priorRunIDs map[string]struct{}, timeout time.Duration) (*ipc.RunInfo, error) { + if priorRunIDs == nil { + return nil, fmt.Errorf("run identity baseline is unavailable") + } deadline := time.NewTimer(timeout) defer deadline.Stop() @@ -414,19 +465,24 @@ func waitForTriggeredRunForHead(ctx context.Context, client *ipc.Client, repoID, return nil, err } if run := activeRunInfoForHead(result.Run, headSHA); run != nil { - return run, nil + if strings.TrimSpace(run.ID) == "" { + return nil, fmt.Errorf("active exact-head run has an empty ID") + } + if _, existed := priorRunIDs[run.ID]; !existed { + return run, nil + } } - if priorRunIDs != nil { - runs, err := runsForHead(client, repoID, branch, headSHA) - if err != nil { - return nil, err + runs, err := runsForHead(client, repoID, branch, headSHA) + if err != nil { + return nil, err + } + for i := range runs { + run := &runs[i] + if strings.TrimSpace(run.ID) == "" { + return nil, fmt.Errorf("exact-head run has an empty ID") } - for i := range runs { - run := &runs[i] - if _, existed := priorRunIDs[run.ID]; !existed { - return run, nil - } - break + if _, existed := priorRunIDs[run.ID]; !existed { + return run, nil } } select { diff --git a/internal/cli/axi_missing_head_test.go b/internal/cli/axi_missing_head_test.go index e2165e1aa..7152b6dcc 100644 --- a/internal/cli/axi_missing_head_test.go +++ b/internal/cli/axi_missing_head_test.go @@ -45,6 +45,15 @@ func TestFreshRunBranchOwnershipDistinguishesMissingTerminalHead(t *testing.T) { wantFreshBlocked: false, wantSafety: "user_owned", }, + { + name: "terminal unmoved without verification keeps custody", + recordedHead: func(_ *testing.T, submitted string) string { + return submitted + }, + verifiedHead: false, + wantFreshBlocked: true, + wantSafety: "blocked_pipeline_owned_recoverable", + }, { name: "terminal recoverable moved head keeps custody", recordedHead: func(_ *testing.T, submitted string) string { @@ -303,6 +312,9 @@ func TestRootWizardKeepsRunIdentityAfterTerminalWait(t *testing.T) { previousWizardRun := wizardRun var startedRunID string wizardRun = func(cfg wizard.Config) (wizard.Result, error) { + if err := cfg.Push(context.Background(), f.branch); err != nil { + return wizard.Result{}, err + } if err := cfg.WaitForRun(context.Background(), f.branch); err != nil { return wizard.Result{}, err } @@ -377,3 +389,82 @@ func TestDaemonFreshRunRechecksPipelineCustody(t *testing.T) { t.Fatalf("runs after custody refusal = %d, want 2", len(runs)) } } + +func TestDaemonFreshRunReturnsNewExactHeadRunInsteadOfDuplicating(t *testing.T) { + f := newMissingHeadFreshRunFixture(t) + runs, err := f.d.GetRunsByRepo(f.repo.ID) + if err != nil { + t.Fatal(err) + } + priorRunIDs := make([]string, 0, len(runs)) + for _, run := range runs { + priorRunIDs = append(priorRunIDs, run.ID) + } + existing, err := f.d.InsertRun(f.repo.ID, f.branch, f.head, f.head) + if err != nil { + t.Fatal(err) + } + if err := f.d.UpdateRunStatusWithVerifiedHead(existing.ID, types.RunCompleted, f.head); err != nil { + t.Fatal(err) + } + + client, err := ipc.Dial(f.paths.Socket()) + if err != nil { + t.Fatal(err) + } + defer client.Close() + var result ipc.StartFreshRunResult + err = client.Call(ipc.MethodStartFreshRun, &ipc.StartFreshRunParams{ + RepoID: f.repo.ID, + Branch: f.branch, + HeadSHA: f.head, + WorkDir: f.repoDir, + PriorRunIDs: priorRunIDs, + }, &result) + if err != nil { + t.Fatalf("fresh run IPC error = %v", err) + } + if result.RunID != existing.ID { + t.Fatalf("fresh run ID = %s, want existing exact-head run %s", result.RunID, existing.ID) + } + runs, err = f.d.GetRunsByRepo(f.repo.ID) + if err != nil { + t.Fatal(err) + } + if len(runs) != len(priorRunIDs)+1 { + t.Fatalf("runs after exact-head handoff = %d, want %d", len(runs), len(priorRunIDs)+1) + } +} + +func TestWizardFreshRunRefusesContextDriftAfterPush(t *testing.T) { + f := newMissingHeadFreshRunFixture(t) + runs, err := f.d.GetRunsByRepo(f.repo.ID) + if err != nil { + t.Fatal(err) + } + priorRunIDs := make(map[string]struct{}, len(runs)) + for _, run := range runs { + priorRunIDs[run.ID] = struct{}{} + } + identity := &freshRunIdentity{branch: f.branch, headSHA: f.head, priorRunIDs: priorRunIDs} + + cliGit(t, f.repoDir, "checkout", "-b", "feature/context-drift") + previousTimeout := triggerWaitTimeout + triggerWaitTimeout = 100 * time.Millisecond + defer func() { triggerWaitTimeout = previousTimeout }() + client, err := ipc.Dial(f.paths.Socket()) + if err != nil { + t.Fatal(err) + } + defer client.Close() + if _, err := awaitDaemonRunRegistrationOrStartFresh(context.Background(), client, f.paths, f.d, f.repo, f.repoDir, f.branch, identity, nil); err == nil || !strings.Contains(err.Error(), "context changed") { + t.Fatalf("context-drift handoff error = %v, want context refusal", err) + } + runs, err = f.d.GetRunsByRepo(f.repo.ID) + if err != nil { + t.Fatal(err) + } + if len(runs) != len(priorRunIDs) { + t.Fatalf("runs after context-drift refusal = %d, want %d", len(runs), len(priorRunIDs)) + } +} diff --git a/internal/cli/root_test.go b/internal/cli/root_test.go index 7fed556c8..f85a81f10 100644 --- a/internal/cli/root_test.go +++ b/internal/cli/root_test.go @@ -225,8 +225,8 @@ func TestRootYesFailsWhenWizardPushProducesNoRun(t *testing.T) { if err == nil { t.Fatal("expected -y to fail when no active run appears after push") } - if !strings.Contains(err.Error(), "resolve gate head") { - t.Fatalf("error should explain that the pushed branch is unavailable, got %v", err) + if !strings.Contains(err.Error(), "record a pushed branch") { + t.Fatalf("error should explain that the pushed branch identity is unavailable, got %v", err) } } diff --git a/internal/cli/wizard.go b/internal/cli/wizard.go index 21592d095..85e44c6b1 100644 --- a/internal/cli/wizard.go +++ b/internal/cli/wizard.go @@ -137,7 +137,7 @@ type repoState struct { defaultBranch string detached bool dirty bool - beforePush func(context.Context, string) error + beforePush func(context.Context, string, string) error } // needsBranch reports whether the user has no feature branch to work on — @@ -242,7 +242,7 @@ func runWizardWithMode(ctx context.Context, p *paths.Paths, state *repoState, sk }, Push: func(ctx context.Context, branch string) error { if state.beforePush != nil { - if err := state.beforePush(ctx, workDir); err != nil { + if err := state.beforePush(ctx, workDir, branch); err != nil { return err } } diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index e8e313a4c..38bad7d4f 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -1209,7 +1209,7 @@ func registerHandlers(srv *ipc.Server, mgr *RunManager, d *db.DB, shutdown func( if err := json.Unmarshal(params, &p); err != nil { return nil, fmt.Errorf("invalid params: %w", err) } - runID, err := mgr.HandleStartFreshRun(ctx, p.RepoID, p.Branch, p.HeadSHA, p.WorkDir, p.SkipSteps, p.Intent) + runID, err := mgr.HandleStartFreshRun(ctx, p.RepoID, p.Branch, p.HeadSHA, p.WorkDir, p.PriorRunIDs, p.SkipSteps, p.Intent) if err != nil { return nil, err } diff --git a/internal/daemon/manager.go b/internal/daemon/manager.go index f25c6245d..2c89a952d 100644 --- a/internal/daemon/manager.go +++ b/internal/daemon/manager.go @@ -792,7 +792,7 @@ func (m *RunManager) HandleRerun(ctx context.Context, repoID, branch, previousRu return m.startRunWithIntentSource(ctx, repo, branch, headSHA, baseSHA, "rerun", skipSteps, intent, intentSource) } -func (m *RunManager) HandleStartFreshRun(ctx context.Context, repoID, branch, headSHA, workDir string, skipSteps []types.StepName, intent string) (string, error) { +func (m *RunManager) HandleStartFreshRun(ctx context.Context, repoID, branch, headSHA, workDir string, priorRunIDs []string, skipSteps []types.StepName, intent string) (string, error) { if strings.TrimSpace(branch) == "" || strings.TrimSpace(headSHA) == "" || strings.TrimSpace(workDir) == "" { return "", fmt.Errorf("fresh run requires a branch, head, and worktree") } @@ -807,14 +807,6 @@ func (m *RunManager) HandleStartFreshRun(ctx context.Context, repoID, branch, he if repo == nil { return "", fmt.Errorf("unknown repo %s", repoID) } - active, err := m.db.GetActiveRun(repoID, branch) - if err != nil { - return "", fmt.Errorf("get active run: %w", err) - } - if active != nil { - return "", fmt.Errorf("an active run already owns branch %s", branch) - } - gateDir := m.paths.RepoDir(repo.ID) gateHead, err := git.ResolveRef(ctx, gateDir, "refs/heads/"+branch) if err != nil { @@ -823,6 +815,35 @@ func (m *RunManager) HandleStartFreshRun(ctx context.Context, repoID, branch, he if gateHead != headSHA { return "", fmt.Errorf("gate head for %q changed from %s to %s; retry the fresh run", branch, headSHA, gateHead) } + var prior map[string]struct{} + if priorRunIDs != nil { + prior = make(map[string]struct{}, len(priorRunIDs)) + for _, runID := range priorRunIDs { + if strings.TrimSpace(runID) == "" { + return "", fmt.Errorf("fresh run received an empty pre-push run identity") + } + prior[runID] = struct{}{} + } + runsForHead, err := m.db.GetRunsByRepoHead(repoID, branch, headSHA) + if err != nil { + return "", fmt.Errorf("get exact-head runs: %w", err) + } + for _, run := range runsForHead { + if strings.TrimSpace(run.ID) == "" { + return "", fmt.Errorf("exact-head run has an empty ID") + } + if _, existed := prior[run.ID]; !existed { + return run.ID, nil + } + } + } + active, err := m.db.GetActiveRun(repoID, branch) + if err != nil { + return "", fmt.Errorf("get active run: %w", err) + } + if active != nil { + return "", fmt.Errorf("an active run already owns branch %s", branch) + } if state := (&branchsync.Service{ DB: m.db, Repo: repo, @@ -835,6 +856,9 @@ func (m *RunManager) HandleStartFreshRun(ctx context.Context, repoID, branch, he } return "", fmt.Errorf("fresh run blocked: pipeline custody is unresolved") } + if priorRunIDs == nil { + return "", fmt.Errorf("fresh run requires a pre-push run identity baseline") + } runs, err := m.db.GetRunsByRepo(repoID) if err != nil { diff --git a/internal/ipc/protocol.go b/internal/ipc/protocol.go index 89809cbe8..4a97f4b91 100644 --- a/internal/ipc/protocol.go +++ b/internal/ipc/protocol.go @@ -134,12 +134,13 @@ type RerunParams struct { } type StartFreshRunParams struct { - RepoID string `json:"repo_id"` - Branch string `json:"branch"` - HeadSHA string `json:"head_sha"` - WorkDir string `json:"work_dir"` - SkipSteps []types.StepName `json:"skip_steps,omitempty"` - Intent string `json:"intent,omitempty"` + RepoID string `json:"repo_id"` + Branch string `json:"branch"` + HeadSHA string `json:"head_sha"` + WorkDir string `json:"work_dir"` + PriorRunIDs []string `json:"prior_run_ids"` + SkipSteps []types.StepName `json:"skip_steps,omitempty"` + Intent string `json:"intent,omitempty"` } type StartFreshRunResult struct { From 6394245de5fa596e33d3c9af48a496955efd0cee Mon Sep 17 00:00:00 2001 From: Zach Date: Fri, 28 Aug 2026 17:09:11 +0800 Subject: [PATCH 7/7] test(cli): repair fresh-run handoff fixture --- internal/cli/attach.go | 2 +- internal/cli/axi_missing_head_test.go | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/internal/cli/attach.go b/internal/cli/attach.go index f257547a4..307dd62cc 100644 --- a/internal/cli/attach.go +++ b/internal/cli/attach.go @@ -57,6 +57,7 @@ func attachRun(ctx context.Context, w io.Writer, runID string, rootDefault bool, var run *ipc.RunInfo var repoID string var state *repoState + var wizardIdentity *freshRunIdentity startedViaWizard := false if runID != "" { @@ -75,7 +76,6 @@ func attachRun(ctx context.Context, w io.Writer, runID string, rootDefault bool, if err != nil { return err } - var wizardIdentity *freshRunIdentity state.beforePush = func(ctx context.Context, workDir, branch string) error { identity, err := captureFreshRunIdentity(ctx, client, repo.ID, workDir, branch) if err != nil { diff --git a/internal/cli/axi_missing_head_test.go b/internal/cli/axi_missing_head_test.go index 7152b6dcc..fd39fae10 100644 --- a/internal/cli/axi_missing_head_test.go +++ b/internal/cli/axi_missing_head_test.go @@ -217,6 +217,11 @@ type missingHeadFreshRunFixture struct { func newMissingHeadFreshRunFixture(t *testing.T) missingHeadFreshRunFixture { t.Helper() repoDir := setupTestRepo(t) + // Fresh-run admission reads the registered default branch to load trusted + // project settings. Seed the fixture's otherwise-empty origin so that the + // wizard handoff exercises identity matching instead of failing on missing + // test infrastructure. + cliGit(t, repoDir, "push", "origin", "HEAD:refs/heads/main") p := paths.WithRoot(os.Getenv("NM_HOME")) d, err := db.Open(p.DB()) if err != nil {