diff --git a/docs/src/content/docs/reference/cli.md b/docs/src/content/docs/reference/cli.md index e463a5717..dfe2a85a7 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. @@ -193,11 +193,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. @@ -213,7 +213,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. @@ -331,9 +331,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 @@ -347,12 +347,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. @@ -360,7 +360,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 @@ -372,6 +372,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..f3e6e7a80 --- /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. Include the rendered cached summary in the existing status telemetry fingerprint so sampled status events correspond to visible state. + +**Tech Stack:** Go, Cobra, existing `branchsync.Service`, existing CLI test helpers. + +--- + +## File structure + +- Modify: `internal/cli/status.go` - render cached evidence and fingerprint it. +- Create: `internal/cli/status_test.go` - table tests for the pure cached-state presenter and fingerprint coverage. + +### 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 aligned with rendered evidence + +**Files:** + +- Modify: `internal/cli/status.go` +- Modify: `internal/cli/status_test.go` + +- [ ] **Step 1: Add a failing fingerprint regression test** + +```go +func TestStatusFingerprintIncludesCachedSummary(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 displayed cached evidence must change the status fingerprint") } +} +``` + +- [ ] **Step 2: Verify red** + +Run: `go test ./internal/cli -run '^TestStatusFingerprintIncludesCachedSummary$'` + +Expected: compile failure until `statusFingerprint` accepts the cached summary. + +- [ ] **Step 3: Update signature and call site** + +```go +fingerprint := statusFingerprint(repo.ID, daemonState, activeRun, cachedSummary) +``` + +Build the fingerprint from repository id, daemon state, cached summary, and +the existing active-run fields. 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/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..6a5274215 --- /dev/null +++ b/docs/superpowers/specs/2026-08-26-cached-repository-state-design.md @@ -0,0 +1,46 @@ +# 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 include that rendered evidence so a +meaningful displayed state change is observable by the sampled read surface. + +## 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 to rendered cached state changes the status telemetry fingerprint. +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..bc07d01f6 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. diff --git a/internal/branchsync/sync.go b/internal/branchsync/sync.go index 52e040a13..c6c2be936 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,38 +1434,53 @@ 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 } localAnchor := custody.RecoveryRef(run.ID) _, localAnchorExists, err := git.ExactRefTarget(ctx, s.workDir(), localAnchor) if err != nil { - return false + return recoverySourceUnavailable } if localAnchorExists { anchored, err := git.Run(ctx, s.workDir(), "rev-parse", localAnchor+"^{commit}") if err != nil || anchored != run.HeadSHA { - return false + return recoverySourceUnavailable } } else if target, err := git.Run(ctx, s.workDir(), "symbolic-ref", "-q", localAnchor); err == nil && target != "" { - return false + return recoverySourceUnavailable } local := state.Local.Head preserved := run.HeadSHA @@ -1473,34 +1488,43 @@ func (s *Service) recoverySourceAvailable(ctx context.Context, state *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 recoverySourceUnavailable } } gateDir := strings.TrimSpace(s.GateDir) if gateDir == "" { - return localEligible + if localEligible { + return recoverySourceAvailable + } + return recoverySourceUnavailable } if _, err := os.Stat(gateDir); err != nil { - return localEligible + if localEligible { + return recoverySourceAvailable + } + return recoverySourceUnavailable } compatible, err := recoveryAnchorCompatible(ctx, gateDir, run.ID, preserved) 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 + return recoverySourceUnavailable } if isAncestor(ctx, gateDir, local, preserved) { - return true + return recoverySourceAvailable } - return preservedContainsLocalWork(ctx, gateDir, local, preserved) + // 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 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..b45fb853b 100644 --- a/internal/cli/status.go +++ b/internal/cli/status.go @@ -61,10 +61,10 @@ 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: "."}).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 +85,36 @@ func newStatusCmd() *cobra.Command { } } -func statusFingerprint(repoID, daemonState string, activeRun *db.Run) string { +func statusFingerprint(repoID, daemonState string, activeRun *db.Run, cachedSummary string) string { + base := repoID + "|" + daemonState + "|" + cachedSummary 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.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..68f9d8ae4 --- /dev/null +++ b/internal/cli/status_test.go @@ -0,0 +1,303 @@ +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: "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 TestStatusFingerprintIncludesCachedSummary(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 displayed cached evidence must change the status fingerprint") + } +} 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") } }