diff --git a/docs/src/content/docs/reference/cli.md b/docs/src/content/docs/reference/cli.md index f7621dc17..191256ea6 100644 --- a/docs/src/content/docs/reference/cli.md +++ b/docs/src/content/docs/reference/cli.md @@ -35,7 +35,7 @@ no-mistakes init --worktree-root ~/work/my-repo-runs | Flag | Type | Default | Description | | ----------------- | -------- | ------- | ------------------------------------------------------------------------------------------------ | -| `--fork-url` | `string` | (none) | GitHub fork remote URL to push branches to while opening PRs against `origin` | +| `--fork-url` | `string` | (none) | GitHub fork remote URL to push branches to while opening PRs against `origin` | | `--worktree-root` | `string` | (none) | Directory to create this repository's run worktrees in; prints the `worktree_roots` entry to add | Creates or refreshes a local bare repo, installs the managed pre-receive admission and post-receive notification hooks, best-effort isolates the gate repo's hook path from shared git config changes when Git supports `config --worktree`, adds or repairs the `no-mistakes` git remote, detects the default branch, records or updates the repo in SQLite, installs the `/no-mistakes` agent skill at user level into `~/.claude/skills/no-mistakes/SKILL.md` and `~/.agents/skills/no-mistakes/SKILL.md`, and ensures the daemon is running, installing the managed service when available and falling back to a detached daemon otherwise. @@ -201,11 +201,11 @@ no-mistakes axi sync --recover no-mistakes axi sync --recover --keep-local ``` -| Flag | Type | Default | Description | -| -------------- | ------ | ------- | ---------------------------------------------------------------------------- | -| `--check` | `bool` | `false` | Verify the live target and exact plan without changing `HEAD` | +| Flag | Type | Default | Description | +| -------------- | ------ | ------- | ----------------------------------------------------------------------------------------------------------------------------------------------- | +| `--check` | `bool` | `false` | Verify the live target and exact plan without changing `HEAD` | | `--recover` | `bool` | `false` | Return custody of a branch stranded by a terminal run with unpublished pipeline commits (a no-op when cancellation already released the branch) | -| `--keep-local` | `bool` | `false` | With `--recover`: keep the current local head; never touches the worktree | +| `--keep-local` | `bool` | `false` | With `--recover`: keep the current local head; never touches the worktree | The default command is an explicit non-interactive apply request and never prompts. All modes return the complete `branch_sync` object as TOON. @@ -221,7 +221,7 @@ Run `axi sync` only when structured output offers `next_action.code: sync`; proc ### Custody recovery -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 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 with verified source evidence: 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 local ancestry. A clean non-ancestral local head whose comparison requires the gate reports `safety: blocked_recover_explicit_verification_required`: cached status deliberately defers the write-capable content-preservation proof to explicit `--recover`. The ordinary 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. 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. @@ -342,9 +342,9 @@ cancels it before starting over. Treat rerun as a between-runs action after a failed or cancelled outcome, or after you have committed a separate fix outside an active run; do not use it to bypass a gate. -| Flag | Type | Default | Description | -| ---- | ---- | ------- | ----------- | -| `--intent` | `string` | (none) | Explicit intent overriding inherited intent or fresh inference | +| Flag | Type | Default | Description | +| ---------- | -------- | ------- | -------------------------------------------------------------- | +| `--intent` | `string` | (none) | Explicit intent overriding inherited intent or fresh inference | ## no-mistakes sync @@ -358,12 +358,12 @@ no-mistakes sync --recover no-mistakes sync --recover --keep-local ``` -| Flag | Type | Default | Description | -| -------------- | ------ | ------- | --------------------------------------------------------------- | -| `--check` | `bool` | `false` | Verify and print the fresh plan without changing `HEAD` | -| `-y`, `--yes` | `bool` | `false` | Apply an eligible guarded synchronization without an interactive prompt | +| Flag | Type | Default | Description | +| -------------- | ------ | ------- | ----------------------------------------------------------------------------------------------------------------------------------------------- | +| `--check` | `bool` | `false` | Verify and print the fresh plan without changing `HEAD` | +| `-y`, `--yes` | `bool` | `false` | Apply an eligible guarded synchronization without an interactive prompt | | `--recover` | `bool` | `false` | Return custody of a branch stranded by a terminal run with unpublished pipeline commits (a no-op when cancellation already released the branch) | -| `--keep-local` | `bool` | `false` | With `--recover`: keep the current local head; never touches the worktree | +| `--keep-local` | `bool` | `false` | With `--recover`: keep the current local head; never touches the worktree | Without `--yes`, apply prints the exact full-SHA plan and requires TTY confirmation; `--recover` prompts the same way before returning custody. A non-TTY apply or recovery refuses with a direct `--yes` hint. @@ -371,7 +371,7 @@ The command uses the same service and safety contract as `no-mistakes axi sync`, ## no-mistakes status -Show repo, daemon, active run, and relevant cached local-branch synchronization status. +Show repo, daemon, active run, and cached local-branch synchronization status. ```sh no-mistakes status @@ -383,6 +383,16 @@ Displays: - Gate path - Daemon status (running/stopped, PID) - Active run details: ID, branch, status, head SHA, start time +- Cached local repository state: branch, short `HEAD`, cleanliness, and the + locally recorded synchronization guidance + +The cached local-state line is always present once a repository is registered. +It is local evidence only: its Git inspection does not fetch or query a Git +remote, and it does not claim that the remote branch is currently fresh. That +inspection does not mutate either the invoking repository or local gate Git +object database, refs, index, or worktree; a Git-status failure is labelled as unavailable rather than as +confirmed dirtiness. `status` may still record its normal local command +telemetry separately. ## no-mistakes runs diff --git a/docs/superpowers/plans/2026-08-26-cached-repository-state.md b/docs/superpowers/plans/2026-08-26-cached-repository-state.md new file mode 100644 index 000000000..adde59e13 --- /dev/null +++ b/docs/superpowers/plans/2026-08-26-cached-repository-state.md @@ -0,0 +1,194 @@ +# Cached Repository State Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make `no-mistakes status` always render cached, local-only repository branch evidence without adding a pipeline or network operation. + +**Architecture:** `internal/cli/status.go` already obtains `branchsync.State` through `InspectCached`. Add one small presenter for that state and render it unconditionally after repository discovery. Deliberately exclude the rendered cached summary from the existing status telemetry fingerprint so changes to local-only evidence do not increase sampled command telemetry. + +**Tech Stack:** Go, Cobra, existing `branchsync.Service`, existing CLI test helpers. + +--- + +## File structure + +- Modify: `internal/cli/status.go` - render cached evidence without fingerprinting it. +- Create: `internal/cli/status_test.go` - table tests for the pure cached-state presenter and fingerprint stability. + +### Task 1: Write the failing presenter test + +**Files:** + +- Create: `internal/cli/status_test.go` + +- [ ] **Step 1: Define clean, dirty, and unavailable cases** + +```go +func TestCachedBranchSummary(t *testing.T) { + tests := []struct { name string; state branchsync.State; want string }{ + {"clean branch", branchsync.State{State: branchsync.StateSynchronized, Local: branchsync.LocalState{Branch: "feature/state", Head: "0123456789abcdef", Clean: true}}, "cached: feature/state 01234567 (clean; already synchronized with the pipeline-pushed head)"}, + {"dirty branch", branchsync.State{State: branchsync.StateDirty, Local: branchsync.LocalState{Branch: "feature/state", Head: "fedcba9876543210", Reason: "uncommitted changes"}}, "cached: feature/state fedcba98 (dirty: uncommitted changes; dirty)"}, + {"unavailable", branchsync.State{State: branchsync.StateAmbiguousContext}, "cached: unavailable (ambiguous context)"}, + } + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + if got := cachedBranchSummary(tt.state); got != tt.want { t.Fatalf("cachedBranchSummary() = %q, want %q", got, tt.want) } + }) } +} +``` + +- [ ] **Step 2: Verify red** + +Run: `go test ./internal/cli -run '^TestCachedBranchSummary$'` + +Expected: compile failure because `cachedBranchSummary` does not exist. + +### Task 2: Add the local-only presenter and render it + +**Files:** + +- Modify: `internal/cli/status.go` +- Test: `internal/cli/status_test.go` + +- [ ] **Step 1: Implement the presenter** + +```go +func cachedBranchSummary(state branchsync.State) string { + summary := humanSyncSummary(state) + if state.Local.Branch == "" || state.Local.Head == "" { return "cached: unavailable (" + summary + ")" } + head := state.Local.Head[:minLen(len(state.Local.Head), 8)] + cleanliness := "clean" + if !state.Local.Clean { cleanliness = "dirty"; if state.Local.Reason != "" { cleanliness += ": " + state.Local.Reason } } + return fmt.Sprintf("cached: %s %s (%s; %s)", state.Local.Branch, head, cleanliness, summary) +} +``` + +- [ ] **Step 2: Replace the conditional cached-state rendering** + +```go +syncState := (&branchsync.Service{DB: d, Repo: repo, WorkDir: "."}).InspectCached(cmd.Context()) +cachedSummary := cachedBranchSummary(syncState) +fmt.Fprintf(w, "\n %s %s\n", sDim.Render("local state:"), cachedSummary) +``` + +- [ ] **Step 3: Verify green** + +Run: `go test ./internal/cli -run '^TestCachedBranchSummary$'` + +Expected: PASS. + +### Task 3: Keep telemetry stable when cached evidence changes + +**Files:** + +- Modify: `internal/cli/status.go` +- Modify: `internal/cli/status_test.go` + +- [ ] **Step 1: Add a failing fingerprint regression test** + +```go +func TestStatusFingerprintIgnoresCachedSummary(t *testing.T) { + run := &db.Run{ID: "run-1", Branch: "feature/test", Status: "running", HeadSHA: "head-one"} + before := statusFingerprint("repo", "running", run, "cached: main 01234567 (clean; synchronized)") + after := statusFingerprint("repo", "running", run, "cached: main 89abcdef (dirty; dirty)") + if before != after { t.Fatal("changing only cached local evidence must not change the command telemetry fingerprint") } +} +``` + +- [ ] **Step 2: Verify red** + +Run: `go test ./internal/cli -run '^TestStatusFingerprintIgnoresCachedSummary$'` + +Expected: compile failure until `statusFingerprint` accepts the cached summary argument. + +- [ ] **Step 3: Update signature and call site** + +```go +fingerprint := statusFingerprint(repo.ID, daemonState, activeRun, cachedSummary) +``` + +Keep the fingerprint based on repository id, daemon state, and the existing +active-run fields. Accept but deliberately ignore the cached summary argument, +and update the existing active-run-head test with the fourth argument. + +- [ ] **Step 4: Verify focused tests** + +Run: `go test ./internal/cli -run 'Test(CachedBranchSummary|StatusFingerprint)'` + +Expected: PASS. + +### Task 4: Validate and prepare review + +**Files:** + +- Modify: `internal/cli/status.go` +- Create: `internal/cli/status_test.go` + +- [ ] **Step 1: Add command-level status coverage** + +Use `setupTestRepo`, `executeCmd("init")`, and `executeCmd("status")` to +assert the clean and a newly dirty worktree both retain `repo`, `daemon`, and +`no active run` output and always include `local state: cached:`. The clean +case must include `(clean;`; the dirty case must include `(dirty:`. + +Run: `go test ./internal/cli -run '^TestStatusAlwaysRendersCachedLocalState$' -count=1` + +Expected: PASS. + +- [ ] **Step 2: Format and run full gates** + +Run: `gofmt -w internal/cli/status.go internal/cli/status_test.go && make lint && go test -race ./... && go build -o ./bin/no-mistakes ./cmd/no-mistakes` + +Expected: each command exits 0. + +- [ ] **Step 3: Inspect the review diff** + +Run: `git diff --check && git diff -- internal/cli/status.go internal/cli/status_test.go` + +Expected: no whitespace errors and no source file outside the planned scope. + +- [ ] **Step 4: Commit after fresh evidence** + +```bash +git add internal/cli/status.go internal/cli/status_test.go +git commit -m "feat(status): show cached repository state" +``` + +### Task 5: Review-remediation safety proof + +**Files:** + +- Modify: `internal/cli/status_test.go` +- Modify: `docs/src/content/docs/reference/cli.md` + +- [x] **Step 1: Add a command-level cached-only safety regression** + +Run `status` after `init` through a test-local Git wrapper that records any +`fetch` or `ls-remote` invocation. Snapshot `.git/FETCH_HEAD`, `.git/index`, +refs, worktree state, and the gate database before and after the command. +Assert that the command renders cached state, makes no remote Git call, and +does not change any snapshot. Also exercise a locally available diverged head +and snapshot `.git/objects`: cached inspection must conservatively report +`blocked_diverged` without constructing a merge tree. Cover the terminal +recovery shape where both divergent heads exist only in the local gate and +snapshot that bare repository's `objects` directory: cached inspection must +defer semantic verification to explicit recovery. + +- [x] **Step 2: State the user-facing freshness boundary** + +Document that the always-rendered cached local-state line's Git inspection is +local evidence, does not fetch or query a Git remote, does not mutate local +Git state, and does not assert remote freshness. Existing command telemetry +is a separate concern. A Git-status failure must render cleanliness as +unavailable, never as a confirmed dirty worktree. + +- [x] **Step 3: Run focused and full verification** + +Run: + +```bash +gofmt -w internal/cli/status_test.go +go test ./internal/cli -run 'TestStatus' -count=1 +make lint && go test -race ./... && go build -o ./bin/no-mistakes ./cmd/no-mistakes && git diff --check +``` + +Expected: each command exits 0 before updating the PR. diff --git a/docs/superpowers/plans/2026-08-27-fork-main-reconciliation.md b/docs/superpowers/plans/2026-08-27-fork-main-reconciliation.md index 653ee9f00..1af9e5a96 100644 --- a/docs/superpowers/plans/2026-08-27-fork-main-reconciliation.md +++ b/docs/superpowers/plans/2026-08-27-fork-main-reconciliation.md @@ -34,7 +34,7 @@ merge-tree`. | `a445c18` | Infisical workflow | Reject as unsafe | The workflow installs through an unpinned network script, exports secrets into a process environment, and uploads `.env` on failure. | | `3ac4741` | Mergify rules | Reject as invalid/stale | It requests nonexistent `phenotype/core`, uses obsolete merge-message fields, assumes check names not provided by the current repository, and automatically closes stale work. | | `ecb1479` | Generic Trunk configuration | Reject as mismatched | It enables Rust, Python, Node, Docker, and other linters for a Go repository without corresponding project configuration. | -| `1055270` | CircleCI matrix | Reject as mismatched | It installs and runs Rust, Python, and Node pipelines that are not part of this repository and contains permissive `|| true` gates. | +| `1055270` | CircleCI matrix | Reject as mismatched | It installs and runs Rust, Python, and Node pipelines that are not part of this repository and contains permissive \|\| true gates. | | `4b723aa` | OpenSSF Scorecard workflow | Replace separately if desired | The preserved version contains an invalid job-level `security` key and stale action pins; supply-chain scoring should be reintroduced as a current, independently validated change. | | `ba63e17`, `59aa0b8` | Trunk/Prettier workflow | Superseded | Fork PR #2 already documents why the original Trunk action was broken. The replacement is a generic Prettier-only policy, not a Go correctness gate, and should not be required by this reconciliation. | | `e26ace7` | Renovate template | Reject as mismatched | It contains Rust, Python, Node, and Docker package rules absent from this repository and grants broad dependency auto-merge behavior without current governance review. | diff --git a/docs/superpowers/specs/2026-08-26-cached-repository-state-design.md b/docs/superpowers/specs/2026-08-26-cached-repository-state-design.md new file mode 100644 index 000000000..302af5475 --- /dev/null +++ b/docs/superpowers/specs/2026-08-26-cached-repository-state-design.md @@ -0,0 +1,48 @@ +# Cached Repository State Design + +## Intent + +Rebuild the legitimate portion of the old fork's diagnostics intent on current +`upstream/main`: make `no-mistakes status` always show the already-available +local branch evidence that an agent needs before starting work. + +## Scope + +`status` will call the existing `branchsync.Service.InspectCached` once and +render a clearly labelled cached summary containing the local branch, a short +HEAD, clean/dirty state, any local reason, and the existing human branch-sync +summary. Its telemetry fingerprint will deliberately exclude that rendered +evidence so changes to local-only evidence do not increase sampled command +telemetry. + +## Safety contract + +`InspectCached` is the data source because it explicitly does not fetch, +contact a remote, alter either the invoking repository or local gate Git object +database, alter refs, alter the index, alter the worktree, create a pipeline +run, or mutate the database. It must +also report unavailable cleanliness as unavailable rather than as confirmed +dirtiness. Semantic equivalence for diverged histories is deferred to an +explicit refresh or recovery, because Git's merge-tree proof can write an +object. The +output must say `cached`; it must not claim current remote freshness. + +## Non-goals + +- No new pipeline step, daemon behaviour, schema, gate, worktree inventory, + remote inspection, or direct process execution. +- No recovery or synchronization action from `status`. +- No recovery of the fork's unregistered pipeline experiment or tracked binary. + +## Acceptance criteria + +1. A registered repository's `status` output always includes cached local + branch evidence, including an explicit unavailable form when Git evidence is + absent. +2. A clean and a dirty local state render distinguishable, actionable text. +3. A change only to rendered cached state leaves the status telemetry + fingerprint unchanged. +4. Existing status behaviour for repo identity, daemon, and active-run display + remains intact. +5. `gofmt -w .`, `make lint`, `go test -race ./...`, and + `go build -o ./bin/no-mistakes ./cmd/no-mistakes` succeed before review. diff --git a/internal/branchsync/recover_test.go b/internal/branchsync/recover_test.go index 99481251e..64998a466 100644 --- a/internal/branchsync/recover_test.go +++ b/internal/branchsync/recover_test.go @@ -5,6 +5,7 @@ import ( "errors" "os" "path/filepath" + "reflect" "strings" "testing" "time" @@ -192,6 +193,61 @@ func TestTerminalPrePushRunSurfacesGuardedCustodyRecovery(t *testing.T) { } } +func TestInspectCachedGateOnlyDivergenceDefersRecoveryWithoutGateMutation(t *testing.T) { + t.Parallel() + + f := newRecoverFixture(t, types.RunCancelled) + mustWrite(t, filepath.Join(f.local, "rescope.txt"), "operator rescope\n") + mustRun(t, f.local, "add", "rescope.txt") + mustRun(t, f.local, "commit", "-m", "operator rescope") + diverged := mustRun(t, f.local, "rev-parse", "HEAD") + mustRun(t, f.gate, "fetch", f.local, diverged) + if !objectExists(f.ctx, f.gate, diverged) { + t.Fatal("test setup did not make the local diverged head available in the gate") + } + + before := bareGitObjectFiles(t, f.gate) + state := f.service.InspectCached(f.ctx) + after := bareGitObjectFiles(t, f.gate) + if !reflect.DeepEqual(before, after) { + t.Fatal("cached inspection wrote objects into the local gate") + } + if state.State != StatePipelineOwned || state.Safety != "blocked_recover_explicit_verification_required" { + t.Fatalf("state = %#v", state) + } + if state.NextAction == nil || state.NextAction.Code != "recover_custody" { + t.Fatalf("next action = %#v", state.NextAction) + } +} + +func bareGitObjectFiles(t *testing.T, bareRepo string) map[string][]byte { + t.Helper() + objects := filepath.Join(bareRepo, "objects") + files := make(map[string][]byte) + err := filepath.WalkDir(objects, func(path string, entry os.DirEntry, err error) error { + if err != nil { + return err + } + if entry.IsDir() { + return nil + } + contents, err := os.ReadFile(path) + if err != nil { + return err + } + rel, err := filepath.Rel(objects, path) + if err != nil { + return err + } + files[rel] = contents + return nil + }) + if err != nil { + t.Fatalf("snapshot gate objects: %v", err) + } + return files +} + // TestActivePrePushRunStaysBlockedWithoutRecovery pins the other half of the // class split: while the run is still active the pre-push block is correct and // no custody-return action may be offered. @@ -617,6 +673,23 @@ func TestRecoverRejectsSymbolicGateAnchorWithoutOverwritingIt(t *testing.T) { } } +func TestInspectRejectsSymbolicLocalRecoveryAnchor(t *testing.T) { + t.Parallel() + + f := newRecoverFixture(t, types.RunCancelled) + mustRun(t, f.local, "fetch", f.gate, f.preserved) + mustRun(t, f.local, "update-ref", "refs/heads/recovery-anchor-target", f.preserved) + mustRun(t, f.local, "symbolic-ref", f.anchorRef(), "refs/heads/recovery-anchor-target") + + state := f.service.InspectCached(f.ctx) + if state.NextAction != nil && state.NextAction.Code == "recover_custody" { + t.Fatalf("inspect advertised recovery despite symbolic local evidence = %#v", state) + } + if got := mustRun(t, f.local, "symbolic-ref", f.anchorRef()); got != "refs/heads/recovery-anchor-target" { + t.Fatalf("symbolic local recovery anchor = %s, want refs/heads/recovery-anchor-target", got) + } +} + func TestRecoverKeepLocalAnchorsIndependentlyMovedGateHead(t *testing.T) { t.Parallel() diff --git a/internal/branchsync/sync.go b/internal/branchsync/sync.go index 52e040a13..f59abd35c 100644 --- a/internal/branchsync/sync.go +++ b/internal/branchsync/sync.go @@ -255,9 +255,10 @@ func displayTarget(raw string) string { return safeurl.Redact(raw) } -// InspectCached reads local Git, persisted provenance, and read-only gate -// ancestry evidence without fetching or mutating refs, the index, or the -// worktree. +// InspectCached reads local Git and persisted provenance without fetching or +// mutating the Git object database, refs, index, or worktree. Semantic +// equivalence of diverged histories is intentionally deferred to Refresh, +// whose explicit operation may construct a temporary merge tree. func (s *Service) InspectCached(ctx context.Context) State { state, _, _ := s.inspect(ctx) return state @@ -1206,14 +1207,13 @@ func (s *Service) classifyRelation(ctx context.Context, state *State, pushed, ba state.NextAction = &NextAction{Code: "run_pipeline", Command: `no-mistakes axi run --intent ""`} return default: - if equivalentDivergence(ctx, s.workDir(), state.Local.Head, pushed, base) { + // Equivalence uses `git merge-tree --write-tree`, which can add an + // object even though no ref or worktree changes. A cached inspection + // promises no local mutation, so reserve that proof for Refresh. + if live && equivalentDivergence(ctx, s.workDir(), state.Local.Head, pushed, base) { state.State = StateDiverged state.Relation = RelationDiverged - if live { - state.Safety = SafetySafeEquivalentAdvance - } else { - state.Safety = "refresh_required" - } + state.Safety = SafetySafeEquivalentAdvance state.NextAction = &NextAction{Code: "sync", Command: "no-mistakes axi sync"} state.Error = "" return @@ -1434,73 +1434,112 @@ func (s *Service) classifyPipelineOwned(ctx context.Context, state *State, run * state.Pipeline.Phase = "pre_push" state.Relation = relationBetween(ctx, s.workDir(), state.Local.Head, run.HeadSHA) if terminalRunStatus(run.Status) { - if !s.recoverySourceAvailable(ctx, state, run) { + switch s.recoverySourceState(ctx, state, run) { + case recoverySourceAvailable: + state.Safety = "blocked_pipeline_owned_recoverable" + state.Error = "the run finished " + string(run.Status) + " with unpublished pipeline commits preserved in the local gate; recover custody before any local follow-up commit" + state.NextAction = &NextAction{Code: "recover_custody", Command: "no-mistakes axi sync --recover"} + return + case recoverySourceExplicitVerification: + state.Safety = "blocked_recover_explicit_verification_required" + state.Error = "the recorded pipeline and local heads are available for gate comparison but have diverged; cached inspection did not perform the semantic merge required to prove custody recovery, so run explicit recovery or reconcile manually" + state.NextAction = &NextAction{Code: "recover_custody", Command: "no-mistakes axi sync --recover"} + return + default: state.Safety = "blocked_recover_preserved_head_missing" state.Error = "the run finished " + string(run.Status) + " but its recorded pipeline head is not available in the invoking worktree or local gate; inspect and reconcile the recorded and live heads manually" state.NextAction = &NextAction{Code: "inspect_and_reconcile_manually", Command: "no-mistakes axi status"} return } - state.Safety = "blocked_pipeline_owned_recoverable" - state.Error = "the run finished " + string(run.Status) + " with unpublished pipeline commits preserved in the local gate; recover custody before any local follow-up commit" - state.NextAction = &NextAction{Code: "recover_custody", Command: "no-mistakes axi sync --recover"} - return } state.Safety = "blocked_pipeline_owned" state.Error = activeMessage state.NextAction = &NextAction{Code: "continue_active_run", Command: "no-mistakes axi status"} } -func (s *Service) recoverySourceAvailable(ctx context.Context, state *State, run *db.Run) bool { +type recoverySourceState uint8 + +const ( + recoverySourceUnavailable recoverySourceState = iota + recoverySourceAvailable + recoverySourceExplicitVerification +) + +func (s *Service) recoverySourceState(ctx context.Context, state *State, run *db.Run) recoverySourceState { if state == nil || run == nil || strings.TrimSpace(run.HeadSHA) == "" { - return false + return recoverySourceUnavailable + } + if !validLocalRecoveryAnchor(ctx, s.workDir(), run) { + return recoverySourceUnavailable } + + localEligible := localRecoveryEligible(ctx, s.workDir(), state, run) + if !validLocalRecoveryState(ctx, s.workDir(), state, run, localEligible) { + return recoverySourceUnavailable + } + return s.gateRecoverySourceState(ctx, state, run, localEligible) +} + +func validLocalRecoveryAnchor(ctx context.Context, workDir string, run *db.Run) bool { localAnchor := custody.RecoveryRef(run.ID) - _, localAnchorExists, err := git.ExactRefTarget(ctx, s.workDir(), localAnchor) + if target, err := git.Run(ctx, workDir, "symbolic-ref", "-q", localAnchor); err == nil && target != "" { + return false + } + _, localAnchorExists, err := git.ExactRefTarget(ctx, workDir, localAnchor) if err != nil { return false } if localAnchorExists { - anchored, err := git.Run(ctx, s.workDir(), "rev-parse", localAnchor+"^{commit}") - if err != nil || anchored != run.HeadSHA { - return false - } - } else if target, err := git.Run(ctx, s.workDir(), "symbolic-ref", "-q", localAnchor); err == nil && target != "" { - return false + anchored, err := git.Run(ctx, workDir, "rev-parse", localAnchor+"^{commit}") + return err == nil && anchored == run.HeadSHA } - local := state.Local.Head - preserved := run.HeadSHA - localEligible := localRecoveryEligible(ctx, s.workDir(), state, run) - if localEligible { - localPreRecovery := custody.RecoveryLocalRef(run.ID) - if anchored, err := git.Run(ctx, s.workDir(), "rev-parse", "--verify", localPreRecovery+"^{commit}"); err == nil && anchored != preserved && local == preserved && !state.Local.Clean { - return false - } + return true +} + +func validLocalRecoveryState(ctx context.Context, workDir string, state *State, run *db.Run, localEligible bool) bool { + if !localEligible { + return true } + localPreRecovery := custody.RecoveryLocalRef(run.ID) + anchored, err := git.Run(ctx, workDir, "rev-parse", "--verify", localPreRecovery+"^{commit}") + return err != nil || anchored == run.HeadSHA || state.Local.Head != run.HeadSHA || state.Local.Clean +} +func (s *Service) gateRecoverySourceState(ctx context.Context, state *State, run *db.Run, localEligible bool) recoverySourceState { gateDir := strings.TrimSpace(s.GateDir) if gateDir == "" { - return localEligible + return localRecoverySourceState(localEligible) } if _, err := os.Stat(gateDir); err != nil { - return localEligible + return localRecoverySourceState(localEligible) } - compatible, err := recoveryAnchorCompatible(ctx, gateDir, run.ID, preserved) + compatible, err := recoveryAnchorCompatible(ctx, gateDir, run.ID, run.HeadSHA) if err != nil || !compatible { - return false + return recoverySourceUnavailable } if localEligible { - return true + return recoverySourceAvailable } if !objectExists(ctx, gateDir, run.HeadSHA) { - return false + return recoverySourceUnavailable } - if !state.Local.Clean || !objectExists(ctx, gateDir, local) { - return false + if !state.Local.Clean || !objectExists(ctx, gateDir, state.Local.Head) { + return recoverySourceUnavailable } - if isAncestor(ctx, gateDir, local, preserved) { - return true + if isAncestor(ctx, gateDir, state.Local.Head, run.HeadSHA) { + return recoverySourceAvailable + } + // Proving gate-only divergence uses merge-tree --write-tree, which creates + // a Git object. Cached inspection must not mutate either repository, so the + // explicit Recover operation owns this proof and any resulting write. + return recoverySourceExplicitVerification +} + +func localRecoverySourceState(localEligible bool) recoverySourceState { + if localEligible { + return recoverySourceAvailable } - return preservedContainsLocalWork(ctx, gateDir, local, preserved) + return recoverySourceUnavailable } func localRecoveryEligible(ctx context.Context, wd string, state *State, run *db.Run) bool { diff --git a/internal/branchsync/sync_test.go b/internal/branchsync/sync_test.go index 941b168ea..27524a26d 100644 --- a/internal/branchsync/sync_test.go +++ b/internal/branchsync/sync_test.go @@ -4,6 +4,7 @@ import ( "context" "os" "path/filepath" + "reflect" "strings" "testing" "time" @@ -232,6 +233,54 @@ func TestInspectCachedBehindPerformsNoFetchOrMutation(t *testing.T) { } } +func TestInspectCachedDivergedDoesNotWriteGitObjects(t *testing.T) { + t.Parallel() + + f := newSyncFixture(t) + rebuildPipelineHead(t, f, []pipelineCommit{ + {message: "pipeline doc", files: map[string]string{"doc.txt": "pipeline doc\n"}}, + }) + mustRun(t, f.local, "fetch", f.remote, "refs/heads/feature/sync:refs/remotes/origin/feature/sync") + + before := gitObjectFiles(t, f.local) + state := f.service.InspectCached(f.ctx) + after := gitObjectFiles(t, f.local) + if state.State != StateDiverged || state.Relation != RelationDiverged || state.Safety != "blocked_diverged" { + t.Fatalf("state = %#v", state) + } + if !reflect.DeepEqual(before, after) { + t.Fatalf("cached inspection wrote Git objects: before=%v after=%v", before, after) + } +} + +func gitObjectFiles(t *testing.T, repoDir string) map[string]string { + t.Helper() + objects := filepath.Join(repoDir, ".git", "objects") + files := make(map[string]string) + err := filepath.WalkDir(objects, func(path string, entry os.DirEntry, err error) error { + if err != nil { + return err + } + if entry.IsDir() { + return nil + } + contents, err := os.ReadFile(path) + if err != nil { + return err + } + rel, err := filepath.Rel(objects, path) + if err != nil { + return err + } + files[rel] = string(contents) + return nil + }) + if err != nil { + t.Fatalf("snapshot Git objects: %v", err) + } + return files +} + func TestApplyCleanStrictBehindFastForwardsExactBoundHead(t *testing.T) { t.Parallel() diff --git a/internal/cli/status.go b/internal/cli/status.go index df8ecd3d1..6d0b1e167 100644 --- a/internal/cli/status.go +++ b/internal/cli/status.go @@ -61,10 +61,16 @@ func newStatusCmd() *cobra.Command { if err != nil { return "", "", fmt.Errorf("check active run: %w", err) } - fingerprint := statusFingerprint(repo.ID, daemonState, activeRun) - if syncState := (&branchsync.Service{DB: d, Repo: repo, WorkDir: "."}).InspectCached(cmd.Context()); relevantCachedSyncState(syncState) { - fmt.Fprintf(w, "\n %s %s\n", sDim.Render("local branch:"), humanSyncSummary(syncState)) - } + syncState := (&branchsync.Service{ + DB: d, + Repo: repo, + WorkDir: ".", + GateDir: p.RepoDir(repo.ID), + Paths: p, + }).InspectCached(cmd.Context()) + cachedSummary := cachedBranchSummary(syncState) + fingerprint := statusFingerprint(repo.ID, daemonState, activeRun, cachedSummary) + fmt.Fprintf(w, "\n %s %s\n", sDim.Render("local state:"), cachedSummary) if activeRun != nil { fmt.Fprintln(w) fmt.Fprintf(w, " %s\n", sCyan.Render("Active run")) @@ -85,11 +91,39 @@ func newStatusCmd() *cobra.Command { } } -func statusFingerprint(repoID, daemonState string, activeRun *db.Run) string { +func statusFingerprint(repoID, daemonState string, activeRun *db.Run, _ string) string { + base := repoID + "|" + daemonState if activeRun == nil { - return repoID + "|" + daemonState + "|idle" + return base + "|idle" + } + return fmt.Sprintf("%s|%s:%s:%s:%s", base, activeRun.ID, activeRun.Branch, activeRun.Status, activeRun.HeadSHA) +} + +func cachedBranchSummary(state branchsync.State) string { + summary := humanSyncSummary(state) + if state.Safety == "blocked_recover_explicit_verification_required" && state.NextAction != nil && state.NextAction.Code == "recover_custody" { + summary = "recovery requires explicit verification: run `" + state.NextAction.Command + "`" + } + if state.Local.Branch == "" || state.Local.Head == "" { + return "cached: unavailable (" + summary + ")" } - return fmt.Sprintf("%s|%s|%s:%s:%s:%s", repoID, daemonState, activeRun.ID, activeRun.Branch, activeRun.Status, activeRun.HeadSHA) + + head := state.Local.Head[:minLen(len(state.Local.Head), 8)] + if state.Local.Reason == "status_unavailable" { + if state.State == branchsync.StateDirty { + summary = "local branch status is unavailable" + } + return fmt.Sprintf("cached: %s %s (cleanliness unavailable: run `git status`; %s)", state.Local.Branch, head, summary) + } + cleanliness := "clean" + if !state.Local.Clean { + cleanliness = "dirty" + if state.Local.Reason != "" { + cleanliness += ": " + state.Local.Reason + } + } + + return fmt.Sprintf("cached: %s %s (%s; %s)", state.Local.Branch, head, cleanliness, summary) } func minLen(a, b int) int { diff --git a/internal/cli/status_test.go b/internal/cli/status_test.go new file mode 100644 index 000000000..503263eec --- /dev/null +++ b/internal/cli/status_test.go @@ -0,0 +1,316 @@ +package cli + +import ( + "bytes" + "fmt" + "os" + "os/exec" + "path/filepath" + "reflect" + "runtime" + "strings" + "testing" + + "github.com/kunchenguid/no-mistakes/internal/branchsync" + "github.com/kunchenguid/no-mistakes/internal/daemon" + "github.com/kunchenguid/no-mistakes/internal/db" + "github.com/kunchenguid/no-mistakes/internal/paths" +) + +func TestStatusAlwaysRendersCachedLocalState(t *testing.T) { + setupTestRepo(t) + if _, err := executeCmd("init"); err != nil { + t.Fatalf("init: %v", err) + } + + clean, err := executeCmd("status") + if err != nil { + t.Fatalf("clean status: %v", err) + } + for _, want := range []string{"repo:", "daemon:", "local state: cached:", "(clean;", "no active run"} { + if !strings.Contains(clean, want) { + t.Fatalf("clean status missing %q:\n%s", want, clean) + } + } + + if err := os.WriteFile("uncommitted.txt", []byte("dirty\n"), 0o644); err != nil { + t.Fatalf("make worktree dirty: %v", err) + } + + dirty, err := executeCmd("status") + if err != nil { + t.Fatalf("dirty status: %v", err) + } + for _, want := range []string{"repo:", "daemon:", "local state: cached:", "(dirty:", "no active run"} { + if !strings.Contains(dirty, want) { + t.Fatalf("dirty status missing %q:\n%s", want, dirty) + } + } +} + +func TestStatusUsesCachedStateWithoutRemoteGitOrMutation(t *testing.T) { + repoDir := setupTestRepo(t) + if _, err := executeCmd("init"); err != nil { + t.Fatalf("init: %v", err) + } + + p, err := paths.New() + if err != nil { + t.Fatalf("paths: %v", err) + } + // Keep the status command as the only process that can touch the local DB + // while this test compares the read surface before and after it. + if err := daemon.Stop(p); err != nil { + t.Fatalf("stop daemon: %v", err) + } + + gitBin, err := exec.LookPath("git") + if err != nil { + t.Fatalf("find git: %v", err) + } + callsPath := filepath.Join(t.TempDir(), "remote-git-calls") + wrapperDir := t.TempDir() + wrapperPath := filepath.Join(wrapperDir, "git") + wrapper := fmt.Sprintf(`#!/bin/sh +case "$1" in +fetch|ls-remote) printf '%%s\\n' "$1" >> "$NM_STATUS_GIT_CALLS"; exit 97 ;; +esac +exec %q "$@" +`, gitBin) + if runtime.GOOS == "windows" { + wrapperPath += ".cmd" + wrapper = fmt.Sprintf(`@echo off +if /I "%%~1"=="fetch" goto remote +if /I "%%~1"=="ls-remote" goto remote +%q %%* +exit /b %%ERRORLEVEL%% +:remote +echo %%~1>> "%%NM_STATUS_GIT_CALLS%%" +exit /b 97 +`, gitBin) + } + if err := os.WriteFile(wrapperPath, []byte(wrapper), 0o755); err != nil { + t.Fatalf("write git wrapper: %v", err) + } + t.Setenv("NM_STATUS_GIT_CALLS", callsPath) + t.Setenv("PATH", wrapperDir+string(os.PathListSeparator)+os.Getenv("PATH")) + + fetchHead := filepath.Join(repoDir, ".git", "FETCH_HEAD") + if err := os.WriteFile(fetchHead, []byte("cached-only-sentinel\\n"), 0o644); err != nil { + t.Fatalf("seed FETCH_HEAD: %v", err) + } + before := snapshotStatusReadSurface(t, repoDir, p.DB()) + + out, err := executeCmd("status") + if err != nil { + t.Fatalf("status: %v", err) + } + if !strings.Contains(out, "local state: cached:") { + t.Fatalf("status did not render cached state:\n%s", out) + } + if calls, err := os.ReadFile(callsPath); err == nil && len(calls) > 0 { + t.Fatalf("status must not call remote git operations; got %q", calls) + } else if err != nil && !os.IsNotExist(err) { + t.Fatalf("read remote git call log: %v", err) + } + + after := snapshotStatusReadSurface(t, repoDir, p.DB()) + if !before.equal(after) { + t.Fatalf("status mutated cached-only state:\n%s", before.diff(after)) + } +} + +type statusReadSurface struct { + fetchHead fileSnapshot + index fileSnapshot + database fileSnapshot + dbWAL fileSnapshot + refs []byte + worktree []byte + objects map[string][]byte +} + +type fileSnapshot struct { + present bool + data []byte +} + +func snapshotStatusReadSurface(t *testing.T, repoDir, databasePath string) statusReadSurface { + t.Helper() + return statusReadSurface{ + fetchHead: snapshotFile(t, filepath.Join(repoDir, ".git", "FETCH_HEAD")), + index: snapshotFile(t, filepath.Join(repoDir, ".git", "index")), + database: snapshotFile(t, databasePath), + dbWAL: snapshotFile(t, databasePath+"-wal"), + refs: gitReadSurface(t, repoDir, "show-ref", "--head"), + worktree: gitReadSurface(t, repoDir, "status", "--porcelain=v1", "--untracked-files=all"), + objects: snapshotDirectoryFiles(t, filepath.Join(repoDir, ".git", "objects")), + } +} + +func snapshotDirectoryFiles(t *testing.T, root string) map[string][]byte { + t.Helper() + files := make(map[string][]byte) + err := filepath.WalkDir(root, func(path string, entry os.DirEntry, err error) error { + if err != nil { + return err + } + if entry.IsDir() { + return nil + } + contents, err := os.ReadFile(path) + if err != nil { + return err + } + rel, err := filepath.Rel(root, path) + if err != nil { + return err + } + files[rel] = contents + return nil + }) + if err != nil { + t.Fatalf("snapshot directory %s: %v", root, err) + } + return files +} + +func snapshotFile(t *testing.T, path string) fileSnapshot { + t.Helper() + data, err := os.ReadFile(path) + if err == nil { + return fileSnapshot{present: true, data: data} + } + if os.IsNotExist(err) { + return fileSnapshot{} + } + t.Fatalf("read %s: %v", path, err) + return fileSnapshot{} +} + +func gitReadSurface(t *testing.T, repoDir string, args ...string) []byte { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = repoDir + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + return out +} + +func (s statusReadSurface) equal(other statusReadSurface) bool { + return s.fetchHead.equal(other.fetchHead) && + s.index.equal(other.index) && + s.database.equal(other.database) && + s.dbWAL.equal(other.dbWAL) && + bytes.Equal(s.refs, other.refs) && + bytes.Equal(s.worktree, other.worktree) && + reflect.DeepEqual(s.objects, other.objects) +} + +func (s statusReadSurface) diff(other statusReadSurface) string { + var changed []string + if !s.fetchHead.equal(other.fetchHead) { + changed = append(changed, "FETCH_HEAD") + } + if !s.index.equal(other.index) { + changed = append(changed, "index") + } + if !s.database.equal(other.database) { + changed = append(changed, "gate database") + } + if !s.dbWAL.equal(other.dbWAL) { + changed = append(changed, "gate database WAL") + } + if !bytes.Equal(s.refs, other.refs) { + changed = append(changed, "refs") + } + if !bytes.Equal(s.worktree, other.worktree) { + changed = append(changed, "worktree") + } + if !reflect.DeepEqual(s.objects, other.objects) { + changed = append(changed, "Git objects") + } + return strings.Join(changed, ", ") +} + +func (s fileSnapshot) equal(other fileSnapshot) bool { + return s.present == other.present && bytes.Equal(s.data, other.data) +} + +func TestCachedBranchSummary(t *testing.T) { + tests := []struct { + name string + state branchsync.State + want string + }{ + { + name: "clean branch", + state: branchsync.State{ + State: branchsync.StateSynchronized, + Local: branchsync.LocalState{Branch: "feature/state", Head: "0123456789abcdef", Clean: true}, + }, + want: "cached: feature/state 01234567 (clean; already synchronized with the pipeline-pushed head)", + }, + { + name: "dirty branch", + state: branchsync.State{ + State: branchsync.StateDirty, + Local: branchsync.LocalState{Branch: "feature/state", Head: "fedcba9876543210", Reason: "uncommitted changes"}, + }, + want: "cached: feature/state fedcba98 (dirty: uncommitted changes; dirty)", + }, + { + name: "cleanliness unavailable", + state: branchsync.State{ + State: branchsync.StateDirty, + Local: branchsync.LocalState{Branch: "feature/state", Head: "0123456789abcdef", Reason: "status_unavailable"}, + }, + want: "cached: feature/state 01234567 (cleanliness unavailable: run `git status`; local branch status is unavailable)", + }, + { + name: "unavailable cleanliness retains pipeline guidance", + state: branchsync.State{ + State: branchsync.StatePipelineOwned, + Local: branchsync.LocalState{Branch: "feature/state", Head: "0123456789abcdef", Reason: "status_unavailable"}, + }, + want: "cached: feature/state 01234567 (cleanliness unavailable: run `git status`; pipeline fix is not pushed yet; do not make local follow-up commits)", + }, + { + name: "explicit verification recovery retains exact command", + state: branchsync.State{ + State: branchsync.StatePipelineOwned, + Safety: "blocked_recover_explicit_verification_required", + Local: branchsync.LocalState{Branch: "feature/state", Head: "0123456789abcdef", Clean: true}, + NextAction: &branchsync.NextAction{ + Code: "recover_custody", + Command: "no-mistakes axi sync --recover", + }, + }, + want: "cached: feature/state 01234567 (clean; recovery requires explicit verification: run `no-mistakes axi sync --recover`)", + }, + { + name: "unavailable", + state: branchsync.State{State: branchsync.StateAmbiguousContext}, + want: "cached: unavailable (ambiguous context)", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := cachedBranchSummary(tt.state); got != tt.want { + t.Fatalf("cachedBranchSummary() = %q, want %q", got, tt.want) + } + }) + } +} + +func TestStatusFingerprintIgnoresCachedSummary(t *testing.T) { + run := &db.Run{ID: "run-1", Branch: "feature/test", Status: "running", HeadSHA: "head-one"} + before := statusFingerprint("repo", "running", run, "cached: main 01234567 (clean; synchronized)") + after := statusFingerprint("repo", "running", run, "cached: main 89abcdef (dirty; dirty)") + if before != after { + t.Fatal("changing only cached local evidence must not change the command telemetry fingerprint") + } +} diff --git a/internal/cli/sync.go b/internal/cli/sync.go index 2d4d1bc68..a2a96efdd 100644 --- a/internal/cli/sync.go +++ b/internal/cli/sync.go @@ -274,7 +274,7 @@ func printHumanSyncState(cmd *cobra.Command, state branchsync.State) { func humanSyncSummary(state branchsync.State) string { switch state.State { case branchsync.StatePipelineOwned: - if state.Safety == "blocked_pipeline_owned_recoverable" { + if state.Safety == "blocked_pipeline_owned_recoverable" || state.Safety == "blocked_recover_explicit_verification_required" { return "run ended without publishing its pipeline commits; recover custody with `no-mistakes sync --recover` (or `no-mistakes rerun` to resume validation)" } return "pipeline fix is not pushed yet; do not make local follow-up commits" diff --git a/internal/cli/sync_test.go b/internal/cli/sync_test.go index 41b5cbfb0..4415e8226 100644 --- a/internal/cli/sync_test.go +++ b/internal/cli/sync_test.go @@ -24,6 +24,17 @@ type cliSyncFixture struct { local, remote, base, old, pushed, runID string } +func TestHumanSyncSummaryIncludesExplicitRecoveryGuidance(t *testing.T) { + state := branchsync.State{ + State: branchsync.StatePipelineOwned, + Safety: "blocked_recover_explicit_verification_required", + } + got := humanSyncSummary(state) + if !strings.Contains(got, "recover custody") || !strings.Contains(got, "no-mistakes sync --recover") { + t.Fatalf("humanSyncSummary() = %q, want guarded recovery guidance", got) + } +} + func newCLISyncFixture(t *testing.T) cliSyncFixture { t.Helper() nmHome := filepath.Join(t.TempDir(), "nm-home") diff --git a/internal/cli/telemetry_test.go b/internal/cli/telemetry_test.go index a42b2600c..31252f851 100644 --- a/internal/cli/telemetry_test.go +++ b/internal/cli/telemetry_test.go @@ -151,9 +151,9 @@ func TestReadSurfaceTelemetryOptOutDoesNotPersistGate(t *testing.T) { func TestStatusFingerprintIncludesDisplayedRunHead(t *testing.T) { run := &db.Run{ID: "run-1", Branch: "feature/test", Status: "running", HeadSHA: "head-one"} - before := statusFingerprint("repo", "running", run) + before := statusFingerprint("repo", "running", run, "cached: main 01234567 (clean; synchronized)") run.HeadSHA = "head-two" - if after := statusFingerprint("repo", "running", run); before == after { + if after := statusFingerprint("repo", "running", run, "cached: main 01234567 (clean; synchronized)"); before == after { t.Fatal("changing the displayed head must change the status fingerprint") } } diff --git a/internal/tui/branch_sync.go b/internal/tui/branch_sync.go index d96fc8707..0a2f2d258 100644 --- a/internal/tui/branch_sync.go +++ b/internal/tui/branch_sync.go @@ -103,7 +103,8 @@ func boundedTUISyncValue(value string) string { // recoverableBranchSync reports whether the state is the stranded terminal // pipeline_owned custody state that the guarded recovery action can end. func recoverableBranchSync(state *branchsync.State) bool { - return state != nil && state.State == branchsync.StatePipelineOwned && state.Safety == "blocked_pipeline_owned_recoverable" + return state != nil && state.State == branchsync.StatePipelineOwned && state.NextAction != nil && state.NextAction.Code == "recover_custody" && + (state.Safety == "blocked_pipeline_owned_recoverable" || state.Safety == "blocked_recover_explicit_verification_required") } func renderRecoverConfirmation(state branchsync.State, width int) string { diff --git a/internal/tui/branch_sync_test.go b/internal/tui/branch_sync_test.go index f5608a82c..2a096b381 100644 --- a/internal/tui/branch_sync_test.go +++ b/internal/tui/branch_sync_test.go @@ -171,8 +171,9 @@ func TestRecoverableCustodyActionFlowsThroughConfirmationAndRecoverService(t *te m := NewModel("socket", nil, run) stranded := branchsync.State{ State: branchsync.StatePipelineOwned, Relation: branchsync.RelationUnknown, Safety: "blocked_pipeline_owned_recoverable", - Local: branchsync.LocalState{Branch: "feature", Head: strings.Repeat("a", 40), Clean: true}, - Pipeline: branchsync.PipelineState{RunID: "run-1", Status: "cancelled", Phase: "pre_push", CurrentHead: strings.Repeat("c", 40)}, + Local: branchsync.LocalState{Branch: "feature", Head: strings.Repeat("a", 40), Clean: true}, + Pipeline: branchsync.PipelineState{RunID: "run-1", Status: "cancelled", Phase: "pre_push", CurrentHead: strings.Repeat("c", 40)}, + NextAction: &branchsync.NextAction{Code: "recover_custody", Command: "no-mistakes axi sync --recover"}, } m.branchSync = &stranded @@ -228,6 +229,45 @@ func TestRecoverableCustodyActionFlowsThroughConfirmationAndRecoverService(t *te } } +// TestExplicitVerificationCustodyActionOffersGuardedRecovery keeps the explicit +// verification state in the same confirmation-only recovery path. Inspecting +// that state never proves an adoption, so the TUI must expose the existing +// guarded control rather than allowing a follow-up commit or hiding recovery. +func TestExplicitVerificationCustodyActionOffersGuardedRecovery(t *testing.T) { + m := NewModel("socket", nil, &ipc.RunInfo{ID: "run-1", Branch: "feature", Status: types.RunCancelled}) + m.branchSync = &branchsync.State{ + State: branchsync.StatePipelineOwned, Safety: "blocked_recover_explicit_verification_required", + Local: branchsync.LocalState{Branch: "feature", Head: strings.Repeat("a", 40), Clean: true}, + Pipeline: branchsync.PipelineState{RunID: "run-1", Status: "cancelled", Phase: "pre_push", CurrentHead: strings.Repeat("c", 40)}, + NextAction: &branchsync.NextAction{Code: "recover_custody", Command: "no-mistakes axi sync --recover"}, + } + m.syncRecover = func() branchsync.State { + t.Fatal("recovery must remain guarded by confirmation") + return branchsync.State{} + } + + view := stripANSI(renderLocalBranchStatus(m.branchSync, false, 80)) + if !strings.Contains(view, "u recover custody") { + t.Fatalf("explicit-verification status hid recovery control:\n%s", view) + } + + next, cmd := m.handleKey(keyMsg("u")) + m = next.(Model) + if cmd != nil || !m.recoverConfirm { + t.Fatalf("u must open guarded recovery confirmation: command=%v confirm=%v", cmd != nil, m.recoverConfirm) + } +} + +func TestRecoveryStateWithoutRecoveryActionRemainsBlocked(t *testing.T) { + state := &branchsync.State{ + State: branchsync.StatePipelineOwned, + Safety: "blocked_recover_explicit_verification_required", + } + if recoverableBranchSync(state) { + t.Fatal("a state without recover_custody action exposed guarded recovery") + } +} + // TestActivePipelineOwnedStateOffersNoRecoveryAction pins that the recovery // affordance never appears while the owning run is still active. func TestActivePipelineOwnedStateOffersNoRecoveryAction(t *testing.T) { diff --git a/scripts/e2e.sh b/scripts/e2e.sh index 7f0fd51bd..29c4999a6 100755 --- a/scripts/e2e.sh +++ b/scripts/e2e.sh @@ -61,7 +61,11 @@ if [[ "$#" -eq 0 ]]; then # The user-journey matrix runs native backends serially because each case # owns process-wide environment. Four backends plus the remaining package # journeys no longer fit the historical five-minute package budget. - set -- -tags=e2e -count=1 -timeout 480s ./internal/e2e/... ./internal/pipeline/steps/... + # The complete serial-native journey matrix measured 477-532 seconds on the + # supported macOS runner and can exceed 600 seconds under host contention. + # Keep a bounded 15-minute package budget so the package deadline detects + # hangs rather than preempting healthy tests near the end of the matrix. + set -- -tags=e2e -count=1 -timeout 15m ./internal/e2e/... ./internal/pipeline/steps/... fi go test "$@"