diff --git a/docs/src/content/docs/guides/agents.md b/docs/src/content/docs/guides/agents.md index 9e410f0a7..f70b0ed5a 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 b6e656c85..32ab9c0a0 100644 --- a/docs/src/content/docs/reference/cli.md +++ b/docs/src/content/docs/reference/cli.md @@ -216,6 +216,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/branchsync/recover_test.go b/internal/branchsync/recover_test.go index 99481251e..0361c9e66 100644 --- a/internal/branchsync/recover_test.go +++ b/internal/branchsync/recover_test.go @@ -451,7 +451,7 @@ func TestRecoverDivergedRefusesButKeepLocalReturnsCustody(t *testing.T) { } kept := f.service.Recover(f.ctx, true) - if !kept.Recovered || kept.Changed { + if !kept.Recovered || kept.Changed || kept.State != StateCustodyReturned || kept.Safety != "custody_returned" { t.Fatalf("keep-local recover = %#v", kept) } if got := mustRun(t, f.local, "rev-parse", "HEAD"); got != divergedHead { @@ -468,6 +468,57 @@ func TestRecoverDivergedRefusesButKeepLocalReturnsCustody(t *testing.T) { } } +// TestRecoverKeepLocalMissingPreservedHeadDoesNotClaimRecovery pins the +// missing-head boundary: --keep-local may return custody only after the +// preserved head is verified. A dead run with no recoverable object must stay +// manual-only instead of reporting recovered=true and stranding ownership. +func TestRecoverKeepLocalMissingPreservedHeadDoesNotClaimRecovery(t *testing.T) { + t.Parallel() + + f := newRecoverFixture(t, types.RunFailed) + missing := strings.Repeat("f", 40) + if err := f.db.UpdateRunStatusWithVerifiedHead(f.run.ID, types.RunFailed, missing); err != nil { + t.Fatal(err) + } + + state := f.service.Recover(f.ctx, true) + if state.Recovered || state.Changed || state.State != StatePipelineOwned || state.Safety != "blocked_recover_preserved_head_missing" { + t.Fatalf("missing-head keep-local recovery = %#v", state) + } + if f.custodyReturned() { + t.Fatal("missing-head recovery stamped custody") + } +} + +// TestNormalizeRecoveredStateDoesNotLeavePipelineOwnership proves the durable +// custody stamp wins over a stale pre-recovery classification. This is the +// state returned by the live recovery loop: recovered=true must not send the +// caller back to recover_custody forever. +func TestNormalizeRecoveredStateDoesNotLeavePipelineOwnership(t *testing.T) { + t.Parallel() + + f := newRecoverFixture(t, types.RunFailed) + if err := f.db.SetRunCustodyReturned(f.run.ID); err != nil { + t.Fatal(err) + } + run, err := f.db.GetRun(f.run.ID) + if err != nil { + t.Fatal(err) + } + state := State{ + State: StatePipelineOwned, + Local: LocalState{Branch: "feature/recover", Head: f.submitted, Clean: true}, + Pipeline: PipelineState{CurrentHead: f.preserved}, + } + f.service.normalizeRecoveredState(f.ctx, &state, run) + if state.State != StateCustodyReturned || state.Safety != "custody_returned" { + t.Fatalf("normalized recovered state = %#v", state) + } + if state.NextAction == nil || state.NextAction.Code != "run_pipeline" { + t.Fatalf("normalized recovered next action = %#v", state.NextAction) + } +} + // TestRecoverKeepLocalDirtyBehindReturnsCustodyWithoutTouchingWorktree covers // the explicit keep-local choice on a dirty worktree: no worktree mutation is // needed, so dirtiness must not block it, and the gate follows the kept head. diff --git a/internal/branchsync/sync.go b/internal/branchsync/sync.go index 52e040a13..ffcdb901e 100644 --- a/internal/branchsync/sync.go +++ b/internal/branchsync/sync.go @@ -567,6 +567,7 @@ func (s *Service) Recover(ctx context.Context, keepLocal bool) State { } state, run, _ := s.inspect(ctx) if run != nil && run.CustodyReturnedAt != nil { + s.normalizeRecoveredState(ctx, &state, run) state.Recovered = true state.Changed = false return state @@ -995,12 +996,33 @@ func (s *Service) finishRecover(ctx context.Context, run *db.Run, changed bool) state.NextAction = nil return state } - state, _, _ := s.inspect(ctx) + state, freshRun, ok := s.inspect(ctx) + // The stamp is the custody transition. If inspection raced with the + // terminal run's old classification, never report a successful recovery + // while continuing to advertise pipeline ownership. + if ok { + s.normalizeRecoveredState(ctx, &state, freshRun) + } state.Recovered = true state.Changed = changed return state } +// normalizeRecoveredState applies the durable custody transition to a state +// snapshot that was classified before or during the recovery stamp. A stale +// pipeline_owned result must never be returned with recovered=true: callers +// use that state to decide whether another recovery is required. +func (s *Service) normalizeRecoveredState(ctx context.Context, state *State, run *db.Run) { + if state == nil || run == nil || run.CustodyReturnedAt == nil || state.State != StatePipelineOwned { + return + } + if run.LastPushedSHA == nil { + s.classifyCustodyReturned(ctx, state) + return + } + s.classifyRelation(ctx, state, ptr(run.LastPushedSHA), run.BaseSHA, false) +} + func recoverAnchorRef(runID string) string { return custody.RecoveryRef(runID) } 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/daemon/manager.go b/internal/daemon/manager.go index 01febf298..349d1cd95 100644 --- a/internal/daemon/manager.go +++ b/internal/daemon/manager.go @@ -684,6 +684,10 @@ func (m *RunManager) HandlePushReceived(ctx context.Context, params *ipc.PushRec return "", err } + if !samePath(params.Gate, m.paths.RepoDir(repoID)) { + return "", fmt.Errorf("home/gate mismatch: gate %q does not belong to daemon home %q", params.Gate, m.paths.RepoDir(repoID)) + } + repo, err := m.db.GetRepo(repoID) if err != nil { return "", fmt.Errorf("get repo: %w", err) diff --git a/internal/daemon/manager_test.go b/internal/daemon/manager_test.go index f39deebfe..88da29461 100644 --- a/internal/daemon/manager_test.go +++ b/internal/daemon/manager_test.go @@ -112,6 +112,33 @@ func TestPushReceivedTracksRunTelemetry(t *testing.T) { } } +func TestPushReceivedRejectsGateFromDifferentHome(t *testing.T) { + p, d := startTestDaemonWithSteps(t, func() []pipeline.Step { + return []pipeline.Step{&mockPassStep{name: types.StepReview}} + }) + + const repoID = "cross-home-repo" + _, headSHA := setupTestGitRepo(t, p, d, repoID) + wrongGate := filepath.Join(t.TempDir(), "repos", repoID+".git") + + client, err := ipc.Dial(p.Socket()) + if err != nil { + t.Fatal(err) + } + defer client.Close() + + var result ipc.PushReceivedResult + err = client.Call(ipc.MethodPushReceived, &ipc.PushReceivedParams{ + Gate: wrongGate, + Ref: "refs/heads/main", + Old: "0000000000000000000000000000000000000000", + New: headSHA, + }, &result) + if err == nil || !strings.Contains(err.Error(), "home/gate mismatch") { + t.Fatalf("push from different home error = %v, want explicit home/gate mismatch", err) + } +} + func TestPushReceivedSkipStepsConfiguresExecutor(t *testing.T) { review := &mockPassStep{name: types.StepReview} testStep := &mockPassStep{name: types.StepTest} diff --git a/internal/e2e/docs_only_pipeline_test.go b/internal/e2e/docs_only_pipeline_test.go new file mode 100644 index 000000000..c96145ba5 --- /dev/null +++ b/internal/e2e/docs_only_pipeline_test.go @@ -0,0 +1,80 @@ +//go:build e2e + +package e2e + +import ( + "context" + "os" + "path/filepath" + "testing" + "time" + + "github.com/kunchenguid/no-mistakes/internal/types" +) + +// TestDocsOnlyBranchRunsValidation proves that a new branch whose only +// committed changes are project instruction markdown still goes through the +// ordinary validation and delivery gates. In particular, document analysis +// being a no-op must not be confused with an empty branch diff. +func TestDocsOnlyBranchRunsValidation(t *testing.T) { + h := NewHarness(t, SetupOpts{Agent: "claude", Scenario: cleanReviewScenario(t)}) + branch := "feature/docs-only" + h.Checkout("main") + if out, err := h.runGit(context.Background(), h.WorkDir, "checkout", "-b", branch); err != nil { + t.Fatalf("create docs-only branch: %v\n%s", err, out) + } + docsWorktree := h.AddWorktree(branch) + if out, err := h.RunInDir(docsWorktree, "init"); err != nil { + t.Fatalf("init: %v\n%s", err, out) + } + + for name, content := range map[string]string{ + "AGENTS.md": "# Agent guidance\n\nKeep the project instructions current.\n", + "CLAUDE.md": "@AGENTS.md\n", + } { + if err := os.WriteFile(filepath.Join(docsWorktree, name), []byte(content), 0o644); err != nil { + t.Fatalf("write %s: %v", name, err) + } + } + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + if out, err := h.runGit(ctx, docsWorktree, "add", "AGENTS.md", "CLAUDE.md"); err != nil { + t.Fatalf("stage docs-only change: %v\n%s", err, out) + } + if out, err := h.runGit(ctx, docsWorktree, "commit", "-m", "update agent guidance"); err != nil { + t.Fatalf("commit docs-only change: %v\n%s", err, out) + } + + if out, err := h.runGit(ctx, docsWorktree, "push", "no-mistakes", branch); err != nil { + t.Fatalf("push docs-only branch: %v\n%s", err, out) + } + run := h.WaitForRun(branch, 60*time.Second) + if run.Status != types.RunCompleted { + t.Fatalf("docs-only run did not complete: status=%s error=%v", run.Status, deref(run.Error)) + } + for _, name := range []types.StepName{ + types.StepRebase, + types.StepReview, + types.StepTest, + types.StepDocument, + types.StepLint, + types.StepPush, + } { + step, ok := findStep(run.Steps, name) + if !ok { + t.Fatalf("docs-only run missing %s step", name) + } + if step.Status != types.StepStatusCompleted { + t.Fatalf("docs-only run skipped %s; got status %s", name, step.Status) + } + } + for _, name := range []types.StepName{types.StepPR, types.StepCI} { + step, ok := findStep(run.Steps, name) + if !ok { + t.Fatalf("docs-only run missing %s step", name) + } + if step.Status != types.StepStatusSkipped { + t.Fatalf("file-backed docs-only fixture should skip unavailable %s, got %s", name, step.Status) + } + } +} diff --git a/internal/git/hook.go b/internal/git/hook.go index 2a0fd6192..8d159b6ff 100644 --- a/internal/git/hook.go +++ b/internal/git/hook.go @@ -48,7 +48,22 @@ case "$GATE_DIR" in GATE_DIR=$(cd "$HOOK_DIR/.." 2>/dev/null && (/bin/pwd -P 2>/dev/null || pwd -P) || :) ;; esac -out=$(NM_HOOK_HELPER=1 "$NM_BIN" daemon admit-push --gate "$GATE_DIR" 2>&1) +case "$GATE_DIR" in + /*) ;; + *) + printf 'no-mistakes: cannot resolve gate directory\n' >&2 + exit 1 + ;; +esac +GATE_HOME=$(cd "$GATE_DIR/../.." 2>/dev/null && (/bin/pwd -P 2>/dev/null || pwd -P) || :) +case "$GATE_HOME" in + /*) ;; + *) + printf 'no-mistakes: cannot derive gate home from %s\n' "$GATE_DIR" >&2 + exit 1 + ;; +esac +out=$(NM_HOOK_HELPER=1 NM_HOME="$GATE_HOME" "$NM_BIN" daemon admit-push --gate "$GATE_DIR" 2>&1) status=$? if [ $status -ne 0 ]; then printf 'no-mistakes: gate push refused before ref mutation:\n%s\n' "$out" >&2 @@ -164,6 +179,28 @@ case "$GATE_DIR" in esac LOG="$GATE_DIR/notify-push.log" nm_ts() { date '+%Y-%m-%dT%H:%M:%S' 2>/dev/null || echo unknown; } +case "$GATE_DIR" in + /*) ;; + *) + [ -n "$GATE_DIR" ] || LOG="./notify-push.log" + { + printf '[%s] notify-push failed: cannot resolve gate directory\n\n' "$(nm_ts)" + } >> "$LOG" + printf 'no-mistakes: notify-push failed: cannot resolve gate directory\n' >&2 + exit 0 + ;; +esac +GATE_HOME=$(cd "$GATE_DIR/../.." 2>/dev/null && (/bin/pwd -P 2>/dev/null || pwd -P) || :) +case "$GATE_HOME" in + /*) ;; + *) + { + printf '[%s] notify-push failed: cannot derive gate home from %s\n\n' "$(nm_ts)" "$GATE_DIR" + } >> "$LOG" + printf 'no-mistakes: notify-push failed: cannot derive gate home from %s\n' "$GATE_DIR" >&2 + exit 0 + ;; +esac notify_failed=0 while read oldrev newrev refname; do set -- --gate "$GATE_DIR" \ @@ -176,7 +213,7 @@ while read oldrev newrev refname; do set -- "$@" --push-option "$opt" i=$((i + 1)) done - out=$(NM_HOOK_HELPER=1 "$NM_BIN" daemon notify-push "$@" 2>&1) + out=$(NM_HOOK_HELPER=1 NM_HOME="$GATE_HOME" "$NM_BIN" daemon notify-push "$@" 2>&1) status=$? if [ $status -ne 0 ]; then notify_failed=1 diff --git a/internal/git/hook_test.go b/internal/git/hook_test.go index c961fd463..95d583f10 100644 --- a/internal/git/hook_test.go +++ b/internal/git/hook_test.go @@ -16,6 +16,7 @@ func TestPreReceiveHookScript(t *testing.T) { "#!/bin/sh", "NM_BIN='/opt/No Mistakes/no-mistakes'", "git rev-parse --absolute-git-dir", + "NM_HOME=\"$GATE_HOME\" \"$NM_BIN\" daemon admit-push", "daemon admit-push --gate \"$GATE_DIR\"", "gate push refused before ref mutation", preservedPreReceiveHook, @@ -111,6 +112,9 @@ func TestPostReceiveHookScript(t *testing.T) { if !strings.Contains(script, "daemon notify-push") { t.Fatal("hook should invoke the CLI notify subcommand") } + if !strings.Contains(script, "NM_HOME=\"$GATE_HOME\" \"$NM_BIN\" daemon notify-push") { + t.Fatal("hook should bind notify-push to the gate's owning home") + } if !strings.Contains(script, "GIT_PUSH_OPTION_COUNT") { t.Fatal("hook should forward git push options to notify-push") } @@ -476,6 +480,85 @@ func TestPostReceiveHook_FallsBackToHookLocationForGateDir(t *testing.T) { } } +func TestReceiveHooksRouteToGateHome(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("receive hooks are /bin/sh-only") + } + + for _, inheritedHome := range []struct { + name string + home string + }{ + {name: "opposite home", home: "opposite"}, + {name: "unset home", home: ""}, + } { + t.Run(inheritedHome.name, func(t *testing.T) { + ctx := context.Background() + base := t.TempDir() + gateHome := filepath.Join(base, "home-b") + oppositeHome := filepath.Join(base, "home-a") + const repoID = "cross-home-hook" + bare := filepath.Join(gateHome, "repos", repoID+".git") + if err := InitBare(ctx, bare); err != nil { + t.Fatal(err) + } + + work := filepath.Join(base, "work") + runGitOrFatal(t, base, "init", work) + runGitOrFatal(t, work, "config", "user.email", "test@test.com") + runGitOrFatal(t, work, "config", "user.name", "Test") + runGitOrFatal(t, work, "commit", "--allow-empty", "-m", "initial") + runGitOrFatal(t, work, "remote", "add", "gate", bare) + + fakeBin := filepath.Join(base, "no-mistakes") + marker := filepath.Join(gateHome, "repos", repoID+".git", "socket-received") + fakeScript := "#!/bin/sh\n" + + "if [ \"$1\" = daemon ] && [ \"$2\" = notify-push ]; then\n" + + " mkdir -p \"$NM_HOME/repos/" + repoID + ".git\"\n" + + " printf '%s\\n' \"$NM_HOME\" > \"$NM_HOME/repos/" + repoID + ".git/socket-received\"\n" + + "fi\n" + + "exit 0\n" + if err := os.WriteFile(fakeBin, []byte(fakeScript), 0o755); err != nil { + t.Fatal(err) + } + hooksDir := filepath.Join(bare, "hooks") + if err := os.WriteFile(filepath.Join(hooksDir, "pre-receive"), []byte(preReceiveHookScript(fakeBin)), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(hooksDir, "post-receive"), []byte(postReceiveHookScript(fakeBin)), 0o755); err != nil { + t.Fatal(err) + } + + env := make([]string, 0, len(os.Environ())+1) + for _, value := range os.Environ() { + if strings.HasPrefix(value, "NM_HOME=") { + continue + } + env = append(env, value) + } + if inheritedHome.home == "opposite" { + env = append(env, "NM_HOME="+oppositeHome) + } + cmd := exec.Command("git", "-C", work, "push", "gate", "HEAD:refs/heads/main") + cmd.Env = env + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("push: %v: %s", err, out) + } + + got, err := os.ReadFile(marker) + if err != nil { + t.Fatalf("gate-owned notification socket was not reached: %v", err) + } + if strings.TrimSpace(string(got)) != gateHome { + t.Fatalf("notification home = %q, want gate home %q", strings.TrimSpace(string(got)), gateHome) + } + if _, err := os.Stat(filepath.Join(oppositeHome, "repos", repoID+".git", "socket-received")); !os.IsNotExist(err) { + t.Fatalf("notification reached opposite home, stat error: %v", err) + } + }) + } +} + // gateArgFromArgv returns the value following the first "--gate" token in a // newline-separated argv dump, or "" if absent. func gateArgFromArgv(argv string) string { diff --git a/internal/pipeline/steps/rebase.go b/internal/pipeline/steps/rebase.go index f6bd39760..3e30fac57 100644 --- a/internal/pipeline/steps/rebase.go +++ b/internal/pipeline/steps/rebase.go @@ -530,7 +530,7 @@ func updateHeadSHA(ctx context.Context, sctx *pipeline.StepContext) (*pipeline.S defaultBranch := effectivePRBaseBranch(sctx) baseSHA := resolveBranchBaseSHA(ctx, sctx.WorkDir, sctx.Run.BaseSHA, defaultBranch) diff, err := git.Diff(ctx, sctx.WorkDir, baseSHA, "HEAD") - if err == nil && strings.TrimSpace(diff) == "" { + if err == nil && strings.TrimSpace(diff) == "" && !freshDocumentationPush(ctx, sctx, "HEAD") { sctx.Log("empty diff after rebase, skipping remaining steps") return &pipeline.StepOutcome{SkipRemaining: true}, nil } @@ -538,6 +538,55 @@ func updateHeadSHA(ctx context.Context, sctx *pipeline.StepContext) (*pipeline.S return &pipeline.StepOutcome{}, nil } +// freshDocumentationPush identifies the one empty-diff case that must still +// be validated: a new branch push whose only commit is markdown. A stale or +// cross-home default-branch mirror can make the merge-base equal HEAD, which +// otherwise makes the empty-diff shortcut skip every downstream gate. Existing +// branches retain the normal already-merged shortcut. +func freshDocumentationPush(ctx context.Context, sctx *pipeline.StepContext, head string) bool { + if sctx == nil || sctx.Run == nil || !git.IsZeroSHA(sctx.Run.BaseSHA) { + return false + } + base, err := freshPushBase(ctx, sctx.WorkDir, sctx.Run.Branch, head) + if err != nil { + return false + } + files, err := git.DiffNameOnly(ctx, sctx.WorkDir, base, head) + if err != nil || len(files) == 0 { + return false + } + for _, file := range files { + ext := strings.ToLower(filepath.Ext(file)) + base := strings.ToLower(filepath.Base(file)) + if ext != ".md" && ext != ".mdx" && ext != ".rst" && ext != ".adoc" && + !strings.HasPrefix(base, "readme") && !strings.HasPrefix(base, "changelog") && + !strings.HasPrefix(base, "contributing") { + return false + } + } + return true +} + +// freshPushBase returns the oldest recorded tip of a branch. A fresh branch's +// reflog records its creation point, which lets this check cover all commits +// in a documentation-only push, including an empty tip commit. Falling back +// to HEAD^ keeps missing reflogs conservative and preserves the old shortcut +// for the common single-commit case. +func freshPushBase(ctx context.Context, workDir, branch, head string) (string, error) { + branch = strings.TrimPrefix(strings.TrimSpace(branch), "refs/heads/") + if branch != "" && branch != "HEAD" { + entries, err := git.Run(ctx, workDir, "reflog", "show", "--format=%H", "refs/heads/"+branch) + if err == nil { + lines := strings.Fields(entries) + if len(lines) > 0 && strings.TrimSpace(lines[0]) != "" { + return strings.TrimSpace(lines[len(lines)-1]), nil + } + } + } + parent, err := git.Run(ctx, workDir, "rev-parse", head+"^") + return strings.TrimSpace(parent), err +} + func shortSHA(sha string) string { if len(sha) <= 12 { return sha diff --git a/internal/pipeline/steps/rebase_test.go b/internal/pipeline/steps/rebase_test.go index 1f63d5b98..f7ff5450d 100644 --- a/internal/pipeline/steps/rebase_test.go +++ b/internal/pipeline/steps/rebase_test.go @@ -156,6 +156,39 @@ func TestRebaseStep_UsesConfiguredPRBaseBranch(t *testing.T) { } } +// TestUpdateHeadSHA_DocsOnlyFreshPushDoesNotUseEmptyDiffShortcut reproduces +// the fresh-lane failure: the zero old SHA lets a stale default-branch mirror +// make merge-base resolve to the pushed documentation head, so the aggregate +// diff is empty even though the new push contains markdown changes. +func TestUpdateHeadSHA_DocsOnlyFreshPushDoesNotUseEmptyDiffShortcut(t *testing.T) { + t.Parallel() + dir, _, _ := setupGitRepo(t) + gitCmd(t, dir, "checkout", "main") + if err := os.WriteFile(filepath.Join(dir, "AGENTS.md"), []byte("# Guidance\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "CLAUDE.md"), []byte("@AGENTS.md\n"), 0o644); err != nil { + t.Fatal(err) + } + gitCmd(t, dir, "add", "AGENTS.md", "CLAUDE.md") + gitCmd(t, dir, "commit", "-m", "update agent guidance") + gitCmd(t, dir, "commit", "--allow-empty", "-m", "pipeline bookkeeping") + headSHA := gitCmd(t, dir, "rev-parse", "HEAD") + // Model the default-branch mirror already pointing at this fresh head. + gitCmd(t, dir, "update-ref", "refs/remotes/origin/main", headSHA) + + zeroSHA := strings.Repeat("0", 40) + sctx := newTestContextWithDBRecords(t, &mockAgent{name: "test"}, dir, zeroSHA, headSHA, config.Commands{}) + sctx.Run.Branch = "refs/heads/main" + outcome, err := updateHeadSHA(context.Background(), sctx) + if err != nil { + t.Fatal(err) + } + if outcome.SkipRemaining { + t.Fatal("docs-only fresh push was treated as an empty branch diff") + } +} + func TestRebaseStep_FixModeCallsAgent(t *testing.T) { t.Parallel() upstream := t.TempDir() diff --git a/internal/skill/skill.go b/internal/skill/skill.go index cf3843976..1a9d4a807 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 fe3c82c49..d2f413a31 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.