diff --git a/internal/cli/state_commit_test.go b/internal/cli/state_commit_test.go index 85e72ef0d..0b641b6f4 100644 --- a/internal/cli/state_commit_test.go +++ b/internal/cli/state_commit_test.go @@ -122,6 +122,64 @@ func TestStateCommitHaltsOnSameEntityConflict(t *testing.T) { } } +// TestStateCommitHaltStderrCarriesRemediationAndPeerCommit pins AC-2 (D1): the +// exit-3 HALT stderr carries the FO's next-action line, the never-force/never- +// auto-resolve line, and the peer commit that survived the aborted rebase — the +// remediation the resident prose used to own, now emitted at fire time. The peer +// commit is A's pushed HEAD sha (the pull's fetch phase updates origin/{branch} +// before the rebase conflicts; abort does not touch it). +func TestStateCommitHaltStderrCarriesRemediationAndPeerCommit(t *testing.T) { + _, workflowA, workflowB, _ := twoHostStateWorkflow(t) + checkoutA := filepath.Join(workflowA, ".spacedock-state") + hostA := filepath.Dir(filepath.Dir(workflowA)) + + writeEntity(t, workflowA, "first-task", "---\nstatus: implementation\n---\n# First Task (A)\n") + if code, _, errOut := runStateCommitCmd(t, hostA, workflowA, "first-task", "-m", "A: -> implementation"); code != 0 { + t.Fatalf("A's commit should succeed (exit 0); got exit=%d stderr=%q", code, errOut) + } + peerSHA := strings.TrimSpace(git(t, checkoutA, "rev-parse", "--short", "HEAD")) + + writeEntity(t, workflowB, "first-task", "---\nstatus: review\n---\n# First Task (B)\n") + hostB := filepath.Dir(filepath.Dir(workflowB)) + code, _, errOut := runStateCommitCmd(t, hostB, workflowB, "first-task", "-m", "B: -> review") + if code != 3 { + t.Fatalf("same-entity conflict must HALT with exit 3; got exit=%d stderr=%q", code, errOut) + } + if !strings.Contains(errOut, "Peer commit: "+peerSHA) { + t.Fatalf("HALT stderr should name the peer commit %q, got:\n%s", peerSHA, errOut) + } + if !strings.Contains(errOut, "Next: HALT dispatch — do not dispatch against this state tree. Surface the conflicting path(s) and peer commit to the operator and stop.") { + t.Fatalf("HALT stderr should name the FO's next action, got:\n%s", errOut) + } + if !strings.Contains(errOut, "Never `git push --force`/`--force-with-lease`; never re-run with `-X ours`/`-X theirs`; never discard either side.") { + t.Fatalf("HALT stderr should carry the never-force/never-auto-resolve line, got:\n%s", errOut) + } +} + +// TestStateCommitHaltJSONCarriesPeerCommit pins AC-2's --json requirement: the +// halt envelope carries peer_commit alongside the existing conflicting_paths. +func TestStateCommitHaltJSONCarriesPeerCommit(t *testing.T) { + _, workflowA, workflowB, _ := twoHostStateWorkflow(t) + checkoutA := filepath.Join(workflowA, ".spacedock-state") + hostA := filepath.Dir(filepath.Dir(workflowA)) + + writeEntity(t, workflowA, "first-task", "---\nstatus: implementation\n---\n# First Task (A)\n") + if code, _, errOut := runStateCommitCmd(t, hostA, workflowA, "first-task", "-m", "A: -> implementation"); code != 0 { + t.Fatalf("A's commit should succeed (exit 0); got exit=%d stderr=%q", code, errOut) + } + peerSHA := strings.TrimSpace(git(t, checkoutA, "rev-parse", "--short", "HEAD")) + + writeEntity(t, workflowB, "first-task", "---\nstatus: review\n---\n# First Task (B)\n") + hostB := filepath.Dir(filepath.Dir(workflowB)) + code, stdout, errOut := runStateCommitCmd(t, hostB, workflowB, "first-task", "-m", "B: -> review", "--json") + if code != 3 { + t.Fatalf("same-entity conflict must HALT with exit 3; got exit=%d stderr=%q", code, errOut) + } + if !strings.Contains(stdout, `"peer_commit": "`+peerSHA+`"`) { + t.Fatalf("--json halt envelope should carry peer_commit=%q, got:\n%s", peerSHA, stdout) + } +} + // TestStateCommitIsPathScoped pins AC-2: a sibling dirty/untracked file in the // state checkout is NOT swept into the commit (the verb stages exactly the entity, // never `add -A`). This is the w4 2/3 `cd && git add -A` drift the verb deletes. diff --git a/internal/cli/state_ready_test.go b/internal/cli/state_ready_test.go index d48083a1c..b7dc3793d 100644 --- a/internal/cli/state_ready_test.go +++ b/internal/cli/state_ready_test.go @@ -85,6 +85,41 @@ func TestStateReadyHaltsOnBootConflict(t *testing.T) { } } +// TestStateReadyHaltStderrCarriesRemediationAndPeerCommit pins AC-2 (D1) for the +// boot-conflict HALT path: identical remediation to `state commit`'s exit-3 halt +// — the peer commit, the FO's next-action line, and the never-force line. +func TestStateReadyHaltStderrCarriesRemediationAndPeerCommit(t *testing.T) { + _, workflowA, workflowB, _ := twoHostStateWorkflow(t) + checkoutA := filepath.Join(workflowA, ".spacedock-state") + checkoutB := filepath.Join(workflowB, ".spacedock-state") + hostA := filepath.Dir(filepath.Dir(workflowA)) + hostB := filepath.Dir(filepath.Dir(workflowB)) + + writeEntity(t, workflowA, "first-task", "---\nstatus: implementation\n---\n# First Task (A)\n") + if code, _, errOut := runStateCommitCmd(t, hostA, workflowA, "first-task", "-m", "A: -> implementation"); code != 0 { + t.Fatalf("A's commit should succeed; exit=%d stderr=%q", code, errOut) + } + peerSHA := strings.TrimSpace(git(t, checkoutA, "rev-parse", "--short", "HEAD")) + + writeEntity(t, workflowB, "first-task", "---\nstatus: review\n---\n# First Task (B)\n") + git(t, checkoutB, "add", "first-task.md") + git(t, checkoutB, "commit", "-q", "-m", "B: -> review", "--", "first-task.md") + + code, _, errOut := runStateReadyCmd(t, hostB, workflowB) + if code != 3 { + t.Fatalf("same-entity boot conflict must HALT with exit 3; got exit=%d stderr=%q", code, errOut) + } + if !strings.Contains(errOut, "Peer commit: "+peerSHA) { + t.Fatalf("ready HALT stderr should name the peer commit %q, got:\n%s", peerSHA, errOut) + } + if !strings.Contains(errOut, "Next: HALT dispatch — do not dispatch against this state tree. Surface the conflicting path(s) and peer commit to the operator and stop.") { + t.Fatalf("ready HALT stderr should name the FO's next action, got:\n%s", errOut) + } + if !strings.Contains(errOut, "Never `git push --force`/`--force-with-lease`; never re-run with `-X ours`/`-X theirs`; never discard either side.") { + t.Fatalf("ready HALT stderr should carry the never-force/never-auto-resolve line, got:\n%s", errOut) + } +} + // TestStateReadyInlineNoOp pins AC-6 inline case: an inline workflow is a clean // no-op (exit 0, nothing to sync, no git network op). func TestStateReadyInlineNoOp(t *testing.T) { @@ -136,11 +171,17 @@ func TestStateReadyResumesAbsentCheckout(t *testing.T) { if _, err := os.Stat(freshState); !os.IsNotExist(err) { t.Fatalf("precondition: fresh clone should NOT yet have the state checkout (err=%v)", err) } - code, _, errOut := runStateReadyCmd(t, fresh, freshWorkflow) + code, stdout, errOut := runStateReadyCmd(t, fresh, freshWorkflow) if code != 0 { t.Fatalf("state ready on an absent checkout should resume it (exit 0); got exit=%d stderr=%q", code, errOut) } if _, err := os.Stat(freshState); err != nil { t.Fatalf("state ready should have resumed the absent checkout: %v", err) } + // D1(c): the resume path carries the re-boot-after-resume sequencing the + // «state.ensure-ready» prose used to own — the FO must re-invoke the boot read + // before the greet since the checkout was just linked. + if !strings.Contains(stdout, "checkout resumed — re-run `spacedock status --boot` before the greet.") { + t.Fatalf("resumed checkout should print the re-boot-before-greet line; stdout:\n%s", stdout) + } } diff --git a/internal/cli/state_sync.go b/internal/cli/state_sync.go index 4ec1298ca..962f33a62 100644 --- a/internal/cli/state_sync.go +++ b/internal/cli/state_sync.go @@ -26,6 +26,7 @@ type syncResult struct { Result string `json:"result"` StateBranch string `json:"state_branch,omitempty"` ConflictingPaths []string `json:"conflicting_paths,omitempty"` + PeerCommit string `json:"peer_commit,omitempty"` Reason string `json:"reason"` } @@ -156,6 +157,13 @@ func runStateReady(ctx context.Context, args []string, env []string, dir string, if code := runStateInit(ctx, []string{"--workflow-dir", workflowDir}, env, dir, stdout, stderr); code != 0 { return code } + // The re-boot-after-resume sequencing the «state.ensure-ready» prose used to + // own: a just-linked checkout means the boot read the FO already did (if any) + // predates the entity dir existing, so it must re-read before greeting. + // Prose-only (not --json) — it is FO guidance, not part of the result envelope. + if !jsonOut { + fmt.Fprintln(stdout, "checkout resumed — re-run `spacedock status --boot` before the greet.") + } } // No origin → ready, local-only (no network integration to do). @@ -200,6 +208,10 @@ func runStateSweep(ctx context.Context, args []string, env []string, dir string, // code is the enforcement: a caller cannot proceed on an unmerged tree. func haltOnConflict(stdout, stderr io.Writer, jsonOut bool, command, slug, branch, checkout, entityPath, rebaseOut string) int { conflicting := conflictingPaths(checkout) + // The peer commit that survived: the pull's fetch phase already updated + // origin/{branch} before the rebase conflicted, and --abort does not touch + // that ref, so this resolves network-free (spiked in ideation). + peerCommit := peerCommitSHA(checkout, branch) // Restore a clean tree so the next operation starts fresh. abort failure is // surfaced but the exit is still the halt. runGit(checkout, "rebase", "--abort") @@ -209,14 +221,36 @@ func haltOnConflict(stdout, stderr io.Writer, jsonOut bool, command, slug, branc } fmt.Fprintf(stderr, "spacedock %s: HALT — same-entity rebase conflict on %s.\n", command, branch) fmt.Fprintf(stderr, "Conflicting path(s): %s\n", strings.Join(conflicting, ", ")) - fmt.Fprintf(stderr, "The rebase was aborted (checkout left clean) and nothing was force-pushed; a peer's edit is preserved on origin. Manual intervention is required.\n") + if peerCommit != "" { + fmt.Fprintf(stderr, "Peer commit: %s (origin/%s)\n", peerCommit, branch) + } + fmt.Fprintf(stderr, "The rebase was aborted (checkout left clean) and nothing was force-pushed; a peer's edit is preserved on origin.\n") + fmt.Fprintf(stderr, "Next: HALT dispatch — do not dispatch against this state tree. Surface the conflicting path(s) and peer commit to the operator and stop.\n") + fmt.Fprintf(stderr, "Never `git push --force`/`--force-with-lease`; never re-run with `-X ours`/`-X theirs`; never discard either side.\n") return emitSync(stdout, jsonOut, syncResult{ Command: command, Slug: slug, Result: "halted", StateBranch: branch, ConflictingPaths: conflicting, + PeerCommit: peerCommit, Reason: fmt.Sprintf("HALT: same-entity rebase conflict on %s — rebase aborted, nothing force-pushed, manual intervention required.", strings.Join(conflicting, ", ")), }, 3) } +// peerCommitSHA resolves the peer's pushed commit on origin/{branch} — the edit +// preserved when this side's rebase conflicted. Runs BEFORE `rebase --abort` +// clears the conflict, though the ref itself does not depend on abort timing (the +// pull's fetch phase already updated it). Returns "" on any git failure rather +// than surfacing an error for what is purely diagnostic context. +func peerCommitSHA(checkout, branch string) string { + if branch == "" { + return "" + } + ok, out := runGit(checkout, "rev-parse", "--short", "origin/"+branch) + if !ok { + return "" + } + return strings.TrimSpace(out) +} + // commitEntityPathScoped stages and commits exactly entityPath (never `add -A`), // retrying the staging on index.lock contention. It returns (true, "") for a // clean no-op (nothing staged for the entity → success), (true, output) for a diff --git a/internal/dispatch/reconcile.go b/internal/dispatch/reconcile.go index 4f32d9172..74dad1fd5 100644 --- a/internal/dispatch/reconcile.go +++ b/internal/dispatch/reconcile.go @@ -89,20 +89,19 @@ func GhRunnerExec(prRef string) (string, error) { return ghRunnerExec(prRef) } // passes gitRunnerExec. type gitRunner func(dir string, args ...string) (string, error) -// ghRunnerExec calls `gh pr view PR --json state` and extracts the state field. +// ghRunnerExec calls `gh pr view PR --json state --jq .state` and returns the +// trimmed, uppercased state. This mirrors boot.go's PR_STATE probe invocation +// exactly (same flags, same leading-"#" trim, same --jq extraction) rather than +// parsing the `--json state` envelope as a second JSON layer — the two probes +// must agree on what "gh can answer" means, since they run against the same `gh` +// and the FO trusts both. func ghRunnerExec(prRef string) (string, error) { - cmd := exec.Command("gh", "pr", "view", prRef, "--json", "state") - out, err := cmd.Output() + pr := strings.TrimPrefix(prRef, "#") + out, err := exec.Command("gh", "pr", "view", pr, "--json", "state", "--jq", ".state").Output() if err != nil { return "", err } - var obj struct { - State string `json:"state"` - } - if err := json.Unmarshal(out, &obj); err != nil { - return "", err - } - return obj.State, nil + return strings.ToUpper(strings.TrimSpace(string(out))), nil } // gitRunnerExec runs git with the given args under -C dir and returns trimmed @@ -666,6 +665,8 @@ type sweepResult struct { StateBranch string `json:"state_branch,omitempty"` Swept []sweptEntity `json:"swept"` Reason string `json:"reason"` + Gh string `json:"gh,omitempty"` + Next string `json:"next,omitempty"` } // Sweep is the read-only `state sweep` computation: the entities whose code PR has @@ -675,6 +676,12 @@ type sweepResult struct { // injected so tests pin a deterministic merged-state; production passes // GhRunnerExec. An inline (single-root) workflow sweeps its own dir. Exit 0 the // sweep ran (set may be empty or populated); 1 setup failure (no README/state). +// +// classC itself stays best-effort — a gh error silently skips that entity, so the +// idle hook never blows up on a transient failure. Sweep wraps the injected probe +// to COUNT calls and errors around that unchanged behavior: when every PR-pending +// entity's probe errored, "0 entity(ies)" would be indistinguishable from a real +// empty sweep, so Sweep reports merge state UNKNOWN instead (D2). func Sweep(workflowDir string, gh GhRunner, jsonOut bool, stdout, stderr io.Writer) int { if info, err := os.Stat(workflowDir); err != nil || !info.IsDir() { fmt.Fprintf(stderr, "spacedock state sweep: workflow directory not found: %s\n", workflowDir) @@ -685,30 +692,74 @@ func Sweep(workflowDir string, gh GhRunner, jsonOut bool, stdout, stderr io.Writ stateRoot = workflowDir } active := loadEntityFrontmatter(activeEntityDir(stateRoot)) - drift := classC(active, gh) + + probeTotal, probeErrs := 0, 0 + counting := func(prRef string) (string, error) { + probeTotal++ + state, err := gh(prRef) + if err != nil { + probeErrs++ + } + return state, err + } + drift := classC(active, counting) swept := make([]sweptEntity, 0, len(drift)) for _, d := range drift { swept = append(swept, sweptEntity{Slug: d.Slug, PR: d.PR, Reason: d.Reason}) } branch, _ := status.StateBranch(workflowDir) - reason := fmt.Sprintf("%d entity(ies) merged but not yet terminalized.", len(swept)) + + var reason, ghField, next string + switch { + case probeTotal > 0 && probeErrs == probeTotal: + reason = "merge state UNKNOWN — gh unavailable; sweep skipped, not empty." + ghField = "unavailable" + case len(swept) > 0: + reason = fmt.Sprintf("%d entity(ies) merged but not yet terminalized.", len(swept)) + next = sweepNextStep(workflowDir) + default: + reason = fmt.Sprintf("%d entity(ies) merged but not yet terminalized.", len(swept)) + } if jsonOut { enc := json.NewEncoder(stdout) enc.SetIndent("", " ") enc.Encode(sweepResult{ - Command: "state sweep", StateBranch: branch, Swept: swept, Reason: reason, + Command: "state sweep", StateBranch: branch, Swept: swept, Reason: reason, Gh: ghField, Next: next, }) return 0 } fmt.Fprintln(stdout, reason) + if next != "" { + fmt.Fprintln(stdout, next) + } for _, s := range swept { fmt.Fprintf(stdout, " %s (PR %s): %s\n", s.Slug, s.PR, s.Reason) } return 0 } +// sweepNextStep names the FO's next action for a non-empty sweep: the registered +// startup-hook mod file(s) to advance each entity per, from the same hookPoint -> +// mod-name scan the boot MODS-REPORT uses (status.ScanMods). It points at the mod +// FILE, never a procedure — the shipped mods/pr-merge.md advances an entity +// directly while a local per-workflow pr-merge mod can delegate to sentinel + +// merge guard, and the binary has no way to know which one applies without +// picking a side. No startup mod registered falls back to a generic _mods/ +// pointer. +func sweepNextStep(workflowDir string) string { + hooks := status.ScanMods(workflowDir)["startup"] + if len(hooks) == 0 { + return "next: advance each per the workflow's startup-hook advancement (_mods/)." + } + paths := make([]string, len(hooks)) + for i, h := range hooks { + paths[i] = fmt.Sprintf("_mods/%s.md", h) + } + return fmt.Sprintf("next: advance each per the workflow's startup-hook advancement (%s).", strings.Join(paths, ", ")) +} + // classD flags worktrees whose branch HEAD is behind origin/{trunk}. The remedy // is ownership-gated: a `pull --rebase` is prescribed ONLY when the worktree's // entity slug is in `owned` — the set of slugs the CURRENT trusted roster's diff --git a/internal/dispatch/sweep_test.go b/internal/dispatch/sweep_test.go index dc24c563a..f0f5ffa64 100644 --- a/internal/dispatch/sweep_test.go +++ b/internal/dispatch/sweep_test.go @@ -4,6 +4,7 @@ package dispatch import ( "encoding/json" + "fmt" "os" "path/filepath" "strings" @@ -78,6 +79,181 @@ stages: } } +// TestSweepGhUnavailableReportsUnknown pins AC-4 (D2): with >=1 PR-pending entity +// and every gh probe erroring, Sweep must declare merge state UNKNOWN rather than +// silently reporting "0 entity(ies) merged" — a real empty sweep and a gh-outage +// sweep are indistinguishable without this. classC itself stays best-effort (it +// swallows the error); Sweep counts probes around it. +func TestSweepGhUnavailableReportsUnknown(t *testing.T) { + repoRoot := t.TempDir() + workflowDir := filepath.Join(repoRoot, "docs", "wf") + writeFile(t, filepath.Join(workflowDir, "README.md"), `--- +entity-type: task +state: .spacedock-state +stages: + states: + - name: ideation + initial: true + - name: done + terminal: true +--- +`) + stateRoot := filepath.Join(workflowDir, ".spacedock-state") + writeFile(t, filepath.Join(stateRoot, "pr-pending", "index.md"), reconcileEntityFM(map[string]string{ + "id": "id-p", "title": "pending", "slug": "pr-pending", "status": "implementation", "pr": "42", + })) + + ghUnavailable := func(string) (string, error) { return "", fmt.Errorf("gh: command not found") } + var out, errBuf strings.Builder + code := Sweep(workflowDir, ghUnavailable, true, &out, &errBuf) + if code != 0 { + t.Fatalf("Sweep should exit 0 even when gh is unavailable; got %d stderr=%q", code, errBuf.String()) + } + var res sweepResult + if err := json.Unmarshal([]byte(out.String()), &res); err != nil { + t.Fatalf("Sweep --json should emit valid JSON: %v\n%s", err, out.String()) + } + if res.Gh != "unavailable" { + t.Fatalf(`sweep JSON should carry "gh": "unavailable", got %q (full: %s)`, res.Gh, out.String()) + } + if !strings.Contains(res.Reason, "UNKNOWN") { + t.Fatalf("reason should declare merge state UNKNOWN, got %q", res.Reason) + } + if strings.Contains(res.Reason, "0 entity(ies)") { + t.Fatalf("gh-unavailable reason must NOT read as a real empty sweep, got %q", res.Reason) + } + if len(res.Swept) != 0 { + t.Fatalf("gh-unavailable sweep should report zero swept entities (all probes errored), got %v", res.Swept) + } +} + +// TestSweepGhPartiallyAvailableStillReportsNormally pins the complement: when at +// least one gh probe SUCCEEDS, the sweep is NOT gh-unavailable — a mixed +// success/failure batch still resolves to the normal count-based reason (the +// binary is reachable; a single flake should not mask real merged entities). +func TestSweepGhPartiallyAvailableStillReportsNormally(t *testing.T) { + repoRoot := t.TempDir() + workflowDir := filepath.Join(repoRoot, "docs", "wf") + writeFile(t, filepath.Join(workflowDir, "README.md"), `--- +entity-type: task +state: .spacedock-state +stages: + states: + - name: ideation + initial: true + - name: done + terminal: true +--- +`) + stateRoot := filepath.Join(workflowDir, ".spacedock-state") + writeFile(t, filepath.Join(stateRoot, "pr-merged", "index.md"), reconcileEntityFM(map[string]string{ + "id": "id-m", "title": "merged", "slug": "pr-merged", "status": "implementation", "pr": "42", + })) + writeFile(t, filepath.Join(stateRoot, "pr-erroring", "index.md"), reconcileEntityFM(map[string]string{ + "id": "id-e", "title": "erroring", "slug": "pr-erroring", "status": "implementation", "pr": "43", + })) + + gh := func(pr string) (string, error) { + if pr == "42" { + return "MERGED", nil + } + return "", fmt.Errorf("rate limited") + } + var out, errBuf strings.Builder + code := Sweep(workflowDir, gh, true, &out, &errBuf) + if code != 0 { + t.Fatalf("Sweep should exit 0; got %d stderr=%q", code, errBuf.String()) + } + var res sweepResult + if err := json.Unmarshal([]byte(out.String()), &res); err != nil { + t.Fatalf("Sweep --json should emit valid JSON: %v\n%s", err, out.String()) + } + if res.Gh == "unavailable" { + t.Fatalf("a partial gh failure must NOT report unavailable, got %q", res.Gh) + } + if !strings.Contains(res.Reason, "1 entity(ies)") { + t.Fatalf("reason should still report the one real merged entity, got %q", res.Reason) + } +} + +// TestSweepNonEmptyNamesRegisteredStartupModNextStep pins AC-4/D2's non-empty +// next-step: a workflow with a registered startup-hook mod names the mod file to +// advance per, not a hardcoded procedure — the mod file is the per-workflow +// authority (shipped pr-merge.md advances directly; this repo's local one +// delegates to sentinel+merge-guard, and the binary must not pick a side). +func TestSweepNonEmptyNamesRegisteredStartupModNextStep(t *testing.T) { + repoRoot := t.TempDir() + workflowDir := filepath.Join(repoRoot, "docs", "wf") + writeFile(t, filepath.Join(workflowDir, "README.md"), `--- +entity-type: task +state: .spacedock-state +stages: + states: + - name: ideation + initial: true + - name: done + terminal: true +--- +`) + writeFile(t, filepath.Join(workflowDir, "_mods", "pr-merge.md"), "## Hook: startup\n\nAdvance a merged PR.\n") + stateRoot := filepath.Join(workflowDir, ".spacedock-state") + writeFile(t, filepath.Join(stateRoot, "pr-merged", "index.md"), reconcileEntityFM(map[string]string{ + "id": "id-m", "title": "merged", "slug": "pr-merged", "status": "implementation", "pr": "42", + })) + + gh := func(string) (string, error) { return "MERGED", nil } + var out, errBuf strings.Builder + if code := Sweep(workflowDir, gh, true, &out, &errBuf); code != 0 { + t.Fatalf("Sweep should exit 0; got %d stderr=%q", code, errBuf.String()) + } + var res sweepResult + if err := json.Unmarshal([]byte(out.String()), &res); err != nil { + t.Fatalf("Sweep --json should emit valid JSON: %v\n%s", err, out.String()) + } + if !strings.Contains(res.Next, "_mods/pr-merge.md") { + t.Fatalf("next-step should name the registered startup mod file, got %q", res.Next) + } + if strings.Contains(res.Next, "merge guard") || strings.Contains(res.Next, "sentinel") { + t.Fatalf("next-step must point at the mod file, not prescribe a procedure, got %q", res.Next) + } +} + +// TestSweepNonEmptyNamesGenericModPointerWhenNoneRegistered pins the fallback: a +// non-empty sweep with NO startup-hook mod registered still names a next-step, +// pointing generically at _mods/ rather than a specific (nonexistent) file. +func TestSweepNonEmptyNamesGenericModPointerWhenNoneRegistered(t *testing.T) { + repoRoot := t.TempDir() + workflowDir := filepath.Join(repoRoot, "docs", "wf") + writeFile(t, filepath.Join(workflowDir, "README.md"), `--- +entity-type: task +state: .spacedock-state +stages: + states: + - name: ideation + initial: true + - name: done + terminal: true +--- +`) + stateRoot := filepath.Join(workflowDir, ".spacedock-state") + writeFile(t, filepath.Join(stateRoot, "pr-merged", "index.md"), reconcileEntityFM(map[string]string{ + "id": "id-m", "title": "merged", "slug": "pr-merged", "status": "implementation", "pr": "42", + })) + + gh := func(string) (string, error) { return "MERGED", nil } + var out, errBuf strings.Builder + if code := Sweep(workflowDir, gh, true, &out, &errBuf); code != 0 { + t.Fatalf("Sweep should exit 0; got %d stderr=%q", code, errBuf.String()) + } + var res sweepResult + if err := json.Unmarshal([]byte(out.String()), &res); err != nil { + t.Fatalf("Sweep --json should emit valid JSON: %v\n%s", err, out.String()) + } + if !strings.Contains(res.Next, "_mods/") { + t.Fatalf("next-step should still point generically at _mods/, got %q", res.Next) + } +} + // TestSweepEmptyIsEmptyArray pins the empty case: no merged-not-done entities yields // an empty swept array (never null), exit 0. func TestSweepEmptyIsEmptyArray(t *testing.T) { @@ -110,3 +286,93 @@ stages: t.Fatalf("empty sweep should emit an empty array, not null; json:\n%s", out.String()) } } + +// stubGhScript is the shared-fixture `gh` shim's exact shape (ensigncycle's +// writeStubMergedGh): `gh pr view {n} --json state --jq .state` answers bare, +// jq-extracted text ("MERGED"); every other gh invocation prints an empty line. +// It does NOT emit a `{"state":...}` JSON envelope — mirroring the flags-agnostic +// case match the live stub uses (it dispatches on $1 $2 only). +const stubGhScript = "#!/bin/sh\n" + + "case \"$1 $2\" in\n" + + " \"pr view\") echo MERGED ;;\n" + + " *) echo \"\" ;;\n" + + "esac\n" + +// writeStubGh writes stubGhScript to dir/gh and prepends dir to PATH for the +// test's duration via t.Setenv, so exec.Command("gh", ...) resolves to it. +func writeStubGh(t *testing.T) { + t.Helper() + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "gh"), []byte(stubGhScript), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) +} + +// TestGhRunnerExecParsesJqExtractedState (feedback cycle 2 regression): the +// production `GhRunnerExec` probe must parse the SAME `--jq .state` +// bare-text shape boot.go's already-proven PR_STATE probe uses — a working `gh` +// answering that way must not be treated as a probe error. Before the fix, +// GhRunnerExec called `gh pr view PR --json state` (no --jq) and JSON-unmarshaled +// the result expecting a `{"state":...}` envelope; against this stub's bare +// "MERGED" text that unmarshal fails, so a genuinely-working `gh` was +// misclassified as an error probe. +func TestGhRunnerExecParsesJqExtractedState(t *testing.T) { + writeStubGh(t) + + state, err := GhRunnerExec("#42") + if err != nil { + t.Fatalf("GhRunnerExec should not error against a working stub gh, got %v", err) + } + if state != "MERGED" { + t.Fatalf("GhRunnerExec state = %q, want MERGED", state) + } +} + +// TestSweepWithRealGhStubDoesNotReportUnknown (feedback cycle 2 regression): this +// is the exact live shallow-boot failure reproduced offline — Sweep driven with +// the REAL production GhRunnerExec (not an injected fake) against a working `gh` +// on PATH must NOT report gh-unavailable UNKNOWN for a PR-pending entity. Caught +// by PR #465's codex-live shallow-boot scenario: the boot's before-greet sweep +// reported UNKNOWN despite a working stub gh, so the FO skipped the mod's +// advancement and hand-advanced without archiving. +func TestSweepWithRealGhStubDoesNotReportUnknown(t *testing.T) { + writeStubGh(t) + + repoRoot := t.TempDir() + workflowDir := filepath.Join(repoRoot, "docs", "wf") + writeFile(t, filepath.Join(workflowDir, "README.md"), `--- +entity-type: task +state: .spacedock-state +stages: + states: + - name: ideation + initial: true + - name: done + terminal: true +--- +`) + // pr is quoted ("#42") to match the shared shallow-boot fixture's entity + // exactly (shared_fixtures_test.go's shallowBootMergedEntity) — an unquoted + // `pr: #42` would parse as a YAML comment (empty value), which reconcileEntityFM + // writes unquoted, so this entity is hand-written here instead. + stateRoot := filepath.Join(workflowDir, ".spacedock-state") + writeFile(t, filepath.Join(stateRoot, "merged-pr", "index.md"), "---\n"+ + "id: id-m\ntitle: merged\nslug: merged-pr\nstatus: implementation\npr: \"#42\"\n---\n\nEntity body.\n") + + var out, errBuf strings.Builder + code := Sweep(workflowDir, GhRunnerExec, true, &out, &errBuf) + if code != 0 { + t.Fatalf("Sweep should exit 0; got %d stderr=%q", code, errBuf.String()) + } + var res sweepResult + if err := json.Unmarshal([]byte(out.String()), &res); err != nil { + t.Fatalf("Sweep --json should emit valid JSON: %v\n%s", err, out.String()) + } + if res.Gh == "unavailable" { + t.Fatalf("a working gh must NOT report unavailable, got reason=%q", res.Reason) + } + if len(res.Swept) != 1 || res.Swept[0].Slug != "merged-pr" { + t.Fatalf("sweep should report the merged entity via the real GhRunnerExec, got %v", res.Swept) + } +} diff --git a/internal/status/archive_guard_test.go b/internal/status/archive_guard_test.go index b3c22a74f..7714bf3cf 100644 --- a/internal/status/archive_guard_test.go +++ b/internal/status/archive_guard_test.go @@ -183,7 +183,8 @@ func TestTerminalSetUnderModBlockRejected(t *testing.T) { if nCode != 1 { t.Fatalf("native exit=%d, want 1 (guard must reject)", nCode) } - wantErr := "Error: entity 010-blocked has pending mod-block (merge:pr-merge). Clear mod-block in a separate --set call, or use --force." + wantErr := "Error: entity 010-blocked has pending mod-block (merge:pr-merge). Clear mod-block in a separate --set call. " + + "(--force bypasses this guard; a refusal usually means a ceremony step was skipped — re-run merge guard 010-blocked instead.)" if !strings.Contains(nErr, wantErr) { t.Fatalf("native stderr = %q, want it to contain %q", nErr, wantErr) } diff --git a/internal/status/handlers.go b/internal/status/handlers.go index dcf0470ca..b500925d1 100644 --- a/internal/status/handlers.go +++ b/internal/status/handlers.go @@ -164,8 +164,9 @@ func runSet(roots roots, set *setUpdate, args []string, whereFilters []whereFilt reason = "mod-block transition" } return errExit(stderr, fmt.Sprintf( - "entity %s has %s. Clear mod-block in a separate --set call, or use --force.", - slug, reason)) + "entity %s has %s. Clear mod-block in a separate --set call. "+ + "(--force bypasses this guard; a refusal usually means a ceremony step was skipped — re-run merge guard %s instead.)", + slug, reason, slug)) } } @@ -205,8 +206,9 @@ func runSet(roots roots, set *setUpdate, args []string, whereFilters []whereFilt if len(mergeHooks) > 0 { return errExit(stderr, fmt.Sprintf( "entity %s cannot advance to terminal — workflow has merge hook(s) [%s] that have not run "+ - "(pr field is empty and mod-block is empty). Set mod-block=merge:%s and invoke the hook, or use --force to bypass.", - slug, strings.Join(mergeHooks, ", "), mergeHooks[0])) + "(pr field is empty and mod-block is empty). Set mod-block=merge:%s and invoke the hook. "+ + "(--force bypasses this guard; a refusal usually means a ceremony step was skipped — re-run merge guard %s instead.)", + slug, strings.Join(mergeHooks, ", "), mergeHooks[0], slug)) } } diff --git a/internal/status/merge.go b/internal/status/merge.go index 2d8b46ba6..fc0549a38 100644 --- a/internal/status/merge.go +++ b/internal/status/merge.go @@ -105,6 +105,7 @@ func MergeGuard(args []string, dir string, stdout, stderr io.Writer) int { fields := ParseFrontmatter(entityPath) modBlock := strings.TrimSpace(fields["mod-block"]) pr := strings.TrimSpace(fields["pr"]) + worktree := strings.TrimSpace(fields["worktree"]) mergeHooks := scanMods(roots.definitionDir)["merge"] hookRegistered := len(mergeHooks) > 0 @@ -115,7 +116,7 @@ func MergeGuard(args []string, dir string, stdout, stderr io.Writer) int { case verdict == "rejected": // A rejected entity never merged, so the pr-requirement is vacuous: finalize // straight through, clearing an in-flight mod-block standalone first (AC-6). - return finalize(roots, slug, modBlock, verdict, quiet, asJSON, stdout, stderr) + return finalize(roots, slug, modBlock, pr, verdict, worktree, hookRegistered, quiet, asJSON, stdout, stderr) case prIndicatesMerged(pr): // FINALIZE from a detected-MERGED state. The `pr` field carries a merge @@ -125,7 +126,17 @@ func MergeGuard(args []string, dir string, stdout, stderr io.Writer) int { // (empty mod-block) state — the stranded case a re-validation bounce leaves // behind (AC-2). The merge-hook guard is satisfied because the sentinel is a // non-empty pr that honestly records the landed merge. - return finalize(roots, slug, modBlock, verdict, quiet, asJSON, stdout, stderr) + return finalize(roots, slug, modBlock, pr, verdict, worktree, hookRegistered, quiet, asJSON, stdout, stderr) + + case modBlockNamesMissingMergeMod(modBlock, mergeHooks): + // A mod-block naming a merge mod that no longer exists under _mods/, with no + // merge sentinel recorded and a non-rejected verdict — the mid-flight state a + // deleted mod file leaves behind. The default case below would otherwise clear + // the block (standalone --set) and terminalize unguarded once no OTHER merge + // hook happens to be registered, silently finalizing without the hook ever + // having run. Refuse BEFORE any mutation — this case is checked ahead of both + // the blocked and default cases so no --set runs. + return refuseMissingMergeMod(roots.definitionDir, slug, modBlock, stderr) case pr != "": // Phase B blocked: a bare/open PR reference (e.g. #42) — the hook opened a PR @@ -151,7 +162,7 @@ func MergeGuard(args []string, dir string, stdout, stderr io.Writer) int { // the terminalize --set is unguarded and succeeds. A merge: pr with a hook // registered never reaches here with an empty mod-block — auto-arm above claims // that state — so the merge-hook guard cannot strand a finalize. - return finalize(roots, slug, modBlock, verdict, quiet, asJSON, stdout, stderr) + return finalize(roots, slug, modBlock, pr, verdict, worktree, hookRegistered, quiet, asJSON, stdout, stderr) } } @@ -198,6 +209,34 @@ func isSHALike(s string) bool { // PR landed. It mirrors localMergeSentinelPrefix for the no-PR fallback. const mergedPRSentinelPrefix = "pr-merge:" +// modBlockNamesMissingMergeMod reports whether modBlock is a merge-ceremony block +// (`merge:{name}`) naming a mod that is NOT among the currently registered merge +// hooks — a mod file deleted while its block was still in flight. An empty +// mod-block, or one naming a mod still present in mergeHooks, returns false. +func modBlockNamesMissingMergeMod(modBlock string, mergeHooks []string) bool { + name, ok := strings.CutPrefix(modBlock, "merge:") + if !ok || name == "" { + return false + } + for _, hook := range mergeHooks { + if hook == name { + return false + } + } + return true +} + +// refuseMissingMergeMod signals D5: the mod-block names a merge mod missing from +// definitionDir/_mods/, with no merge sentinel recorded — the ceremony cannot +// resolve without the mod's guidance, and clearing the block would finalize +// without the hook ever having run. Exits 1, mutating nothing. +func refuseMissingMergeMod(definitionDir, slug, modBlock string, stderr io.Writer) int { + name := strings.TrimPrefix(modBlock, "merge:") + return errExit(stderr, fmt.Sprintf( + "blocking mod %s is missing from %s/_mods/ — the entity %s is stuck. Restore the mod file, or have the operator clear the block with --force.", + name, definitionDir, slug)) +} + // arm performs Phase A: set mod-block=merge:{hook} in its own --set and signal the // FO to invoke the hook. The underlying runSet is the proven mutation path. func arm(roots roots, slug, hook string, quiet, asJSON bool, stdout, stderr io.Writer) int { @@ -205,7 +244,7 @@ func arm(roots roots, slug, hook string, quiet, asJSON bool, stdout, stderr io.W if rc := emitSet(roots, slug, []fieldUpdate{{field: "mod-block", value: modValue, hasValue: true}}, stderr); rc != 0 { return rc } - return signalArmed(slug, hook, quiet, asJSON, stdout) + return signalArmed(roots.definitionDir, slug, hook, quiet, asJSON, stdout) } // finalize performs Phase C: clear an in-flight mod-block in a STANDALONE --set, @@ -221,7 +260,7 @@ func arm(roots roots, slug, hook string, quiet, asJSON bool, stdout, stderr io.W // find it on a re-run. So finalize snapshots the entity's pre-finalize bytes and live // location before mutating, and on commit failure reverses the move and restores the // original content, returning the entity to its exact pre-finalize state. -func finalize(roots roots, slug, modBlock, verdict string, quiet, asJSON bool, stdout, stderr io.Writer) int { +func finalize(roots roots, slug, modBlock, pr, verdict, worktree string, hookRegistered bool, quiet, asJSON bool, stdout, stderr io.Writer) int { // Snapshot the pre-finalize state up front — before any mutation — so a failed // archive commit can be rolled back to exactly the state the FO would re-run // against. The live path and form resolve here while the file still sits at its @@ -264,7 +303,7 @@ func finalize(roots roots, slug, modBlock, verdict string, quiet, asJSON bool, s } return rc } - return signalFinalized(slug, terminal, verdict, quiet, asJSON, stdout) + return signalFinalized(roots.definitionDir, slug, terminal, verdict, worktree, hookRegistered, prIndicatesMerged(pr), quiet, asJSON, stdout) } // archiveSnapshot captures an entity's pre-archive state so finalize can reverse a @@ -443,8 +482,10 @@ func terminalStageName(definitionDir string) string { } // signalArmed reports the arm outcome: the mod-block is set and the FO must now -// invoke the named merge hook. -func signalArmed(slug, hook string, quiet, asJSON bool, stdout io.Writer) int { +// invoke the named merge hook. The default prose names the hook's file path and +// the re-run command — the FO's next action, carried at fire time instead of +// resident prose (D3). +func signalArmed(definitionDir, slug, hook string, quiet, asJSON bool, stdout io.Writer) int { switch { case asJSON: emitJSON(stdout, newJSONObj(). @@ -453,13 +494,16 @@ func signalArmed(slug, hook string, quiet, asJSON bool, stdout io.Writer) int { case quiet: fmt.Fprintf(stdout, "merge-guard slug=%s signal=armed action=invoke-hook hook=%s\n", slug, hook) default: - fmt.Fprintf(stdout, "armed: mod-block set to merge:%s — invoke the %s merge hook, then re-run `merge guard %s`.\n", hook, hook, slug) + fmt.Fprintf(stdout, "armed: mod-block set to merge:%s — invoke the %s merge hook (%s/_mods/%s.md; merge guard never invokes it), then re-run `merge guard %s`.\n", + hook, hook, definitionDir, hook, slug) } return 0 } // signalBlocked reports the blocked outcome: the hook opened a PR, so the verb -// leaves the mod-block intact and waits. +// leaves the mod-block intact and waits. The default prose names the never- +// finalize-on-open-PR invariant and the sentinel format that unlocks finalize +// (D3), carried at fire time instead of resident prose. func signalBlocked(slug, pr string, quiet, asJSON bool, stdout io.Writer) int { switch { case asJSON: @@ -469,14 +513,37 @@ func signalBlocked(slug, pr string, quiet, asJSON bool, stdout io.Writer) int { case quiet: fmt.Fprintf(stdout, "merge-guard slug=%s signal=blocked action=await-pr pr=%s\n", slug, pr) default: - fmt.Fprintf(stdout, "blocked: PR %s is pending — mod-block left intact. Re-run `merge guard %s` after it lands.\n", pr, slug) + fmt.Fprintf(stdout, "blocked: PR %s is pending — mod-block left intact, never finalize on an open PR. "+ + "When gh reports it MERGED, record the sentinel (pr=pr-merge:{number}) and re-run `merge guard %s`.\n", pr, slug) } return 0 } // signalFinalized reports the finalize outcome: the entity is terminal, verdict -// recorded, archived. -func signalFinalized(slug, terminal, verdict string, quiet, asJSON bool, stdout io.Writer) int { +// recorded, archived. The default prose appends the FO's next-step lines (D3), +// built from the pre-terminalize frontmatter: a recorded worktree names its own +// removal/branch-cleanup/teardown sequence; an entity finalizing with no merge +// hook registered and no merge sentinel (the default-local-merge path) also +// names the manual `--no-ff` merge onto trunk that nothing automated. +func signalFinalized(definitionDir, slug, terminal, verdict, worktree string, hookRegistered, hasSentinel bool, quiet, asJSON bool, stdout io.Writer) int { + base := fmt.Sprintf("finalized: %s -> %s (verdict %s), archived.", slug, terminal, verdict) + var next []string + if worktree != "" { + next = append(next, fmt.Sprintf( + "Next: push; remove the worktree (`git worktree remove %s`, no --force — if it refuses on untracked files, audit them with the operator before any --force); "+ + "delete the local branch (`git branch -d`); keep the remote branch while a PR references it; tear down the entity's workers per your runtime adapter.", + worktree)) + } + if !hookRegistered && !hasSentinel && verdict != "rejected" { + branch := "the stage branch" + if worktree != "" { + if ref, ok := abbrevRefHead(worktree); ok && ref != "" { + branch = ref + } + } + next = append(next, fmt.Sprintf( + "no merge hook registered — merge %s onto %s with --no-ff if not already merged.", branch, resolveMergeTrunk(definitionDir))) + } switch { case asJSON: emitJSON(stdout, newJSONObj(). @@ -485,7 +552,34 @@ func signalFinalized(slug, terminal, verdict string, quiet, asJSON bool, stdout case quiet: fmt.Fprintf(stdout, "merge-guard slug=%s signal=finalized status=%s verdict=%s\n", slug, terminal, verdict) default: - fmt.Fprintf(stdout, "finalized: %s -> %s (verdict %s), archived.\n", slug, terminal, verdict) + fmt.Fprintln(stdout, base) + for _, line := range next { + fmt.Fprintln(stdout, line) + } } return 0 } + +// resolveMergeTrunk mirrors dispatch.resolveTrunk's resolution (the top-level +// README `trunk:` key, falling back to "main"). Duplicated rather than imported — +// internal/dispatch already imports internal/status, so the reverse import would +// cycle. +func resolveMergeTrunk(definitionDir string) string { + if t := strings.TrimSpace(ParseFrontmatter(filepath.Join(definitionDir, "README.md"))["trunk"]); t != "" { + return t + } + return "main" +} + +// abbrevRefHead best-effort resolves worktree's current branch name via +// `git -C {worktree} rev-parse --abbrev-ref HEAD`. ok is false on any git failure +// (detached HEAD, a removed worktree) — the caller falls back to a branch- +// agnostic phrase rather than surfacing an error for a purely cosmetic name. +func abbrevRefHead(worktree string) (ref string, ok bool) { + out, err := runGitCmd(worktree, "rev-parse", "--abbrev-ref", "HEAD") + if err != nil { + return "", false + } + ref = strings.TrimSpace(out) + return ref, ref != "" +} diff --git a/internal/status/merge_guard_test.go b/internal/status/merge_guard_test.go index 378b16492..fdd1bb207 100644 --- a/internal/status/merge_guard_test.go +++ b/internal/status/merge_guard_test.go @@ -4,6 +4,7 @@ package status import ( "bytes" + "fmt" "os" "os/exec" "path/filepath" @@ -388,6 +389,225 @@ func TestMergeGuardRejectedClearsModBlockFirst(t *testing.T) { } } +// TestMergeGuardRefusesMissingMergeModNoSentinel (AC-5 row a, D5): a mod-block +// naming a merge mod that is no longer registered under `_mods/`, with no merge +// sentinel and a non-rejected verdict, must REFUSE — not silently finalize. This +// pins the bug found by code read (merge.go's pre-D5 default case): clearing the +// block here would archive the entity without its hook ever having run. The +// refusal exits 1, names the missing mod, and mutates nothing (status, mod-block, +// and pr all survive byte-identical). +func TestMergeGuardRefusesMissingMergeModNoSentinel(t *testing.T) { + root, out, errOut, code := driveMergeGuard(t, "merge-pr-workflow", "120-missing-mod-no-sentinel", "--verdict", "passed") + if code != 1 { + t.Fatalf("missing-mod no-sentinel must refuse (exit 1), got %d (stdout=%q)", code, out) + } + if !strings.Contains(errOut, "ghost-merge") { + t.Fatalf("stderr should name the missing mod, got %q", errOut) + } + if !strings.Contains(errOut, "is missing") { + t.Fatalf("stderr should say the mod is missing, got %q", errOut) + } + // Feedback cycle 1, T2: pin the full remediation tail, not just the mod-name + + // "is missing" fragment — a change that drops the restore/--force remedy must + // fail this test. + if !strings.Contains(errOut, "Restore the mod file, or have the operator clear the block with --force.") { + t.Fatalf("stderr should carry the full remediation tail, got %q", errOut) + } + if strings.Contains(out, "finalized") { + t.Fatalf("stdout must not claim finalized on the missing-mod refusal, got %q", out) + } + entity := filepath.Join(root, "120-missing-mod-no-sentinel.md") + if got := frontmatterField(t, entity, "status"); got != "implementation" { + t.Fatalf("refused entity must not terminalize, status=%q", got) + } + if got := frontmatterField(t, entity, "mod-block"); got != "merge:ghost-merge" { + t.Fatalf("refused entity must keep its mod-block intact, got %q", got) + } + if fileExists(filepath.Join(root, "_archive", "120-missing-mod-no-sentinel.md")) { + t.Fatal("refused entity must NOT archive") + } +} + +// TestMergeGuardRefusesMissingMergeModWhenNoHookRegisteredAtAll (feedback cycle 1, +// T1): the likeliest real-world D5 trigger — the deleted mod file WAS the +// workflow's only registered merge hook, so `mergeHooks` is EMPTY, not merely +// missing the named entry. This is a distinct shape from +// TestMergeGuardRefusesMissingMergeModNoSentinel (which fixtures a workflow with +// a DIFFERENT hook still registered): a mutant that special-cases +// modBlockNamesMissingMergeMod on `len(mergeHooks)==0` (treating "no hooks at +// all" as vacuously not-missing) passes every other D5 test but must fail here. +func TestMergeGuardRefusesMissingMergeModWhenNoHookRegisteredAtAll(t *testing.T) { + root, out, errOut, code := driveMergeGuard(t, "merge-no-hook-workflow", "030-missing-mod-no-hooks-registered", "--verdict", "passed") + if code != 1 { + t.Fatalf("missing-mod refusal must fire even with zero hooks registered (exit 1), got %d (stdout=%q)", code, out) + } + if !strings.Contains(errOut, "ghost-merge") { + t.Fatalf("stderr should name the missing mod, got %q", errOut) + } + if !strings.Contains(errOut, "is missing") { + t.Fatalf("stderr should say the mod is missing, got %q", errOut) + } + if strings.Contains(out, "finalized") { + t.Fatalf("stdout must not claim finalized — this is the exact silent-finalize shape D5 fixes, got %q", out) + } + entity := filepath.Join(root, "030-missing-mod-no-hooks-registered.md") + if got := frontmatterField(t, entity, "status"); got != "implementation" { + t.Fatalf("refused entity must not terminalize, status=%q", got) + } + if got := frontmatterField(t, entity, "mod-block"); got != "merge:ghost-merge" { + t.Fatalf("refused entity must keep its mod-block intact, got %q", got) + } + if fileExists(filepath.Join(root, "_archive", "030-missing-mod-no-hooks-registered.md")) { + t.Fatal("refused entity must NOT archive") + } +} + +// TestMergeGuardFinalizesMissingMergeModWithSentinel (AC-5 row b): a mod-block +// naming a missing merge mod does NOT refuse when a well-formed merge sentinel is +// already recorded — the sentinel honestly proves the ceremony ran before the mod +// file was deleted, so finalize proceeds exactly as it does today. +func TestMergeGuardFinalizesMissingMergeModWithSentinel(t *testing.T) { + root, out, errOut, code := driveMergeGuard(t, "merge-pr-workflow", "130-missing-mod-with-sentinel", "--verdict", "passed") + if code != 0 { + t.Fatalf("missing-mod WITH sentinel should still finalize (exit 0), got %d (stderr=%q)", code, errOut) + } + if !strings.Contains(out, "finalized") { + t.Fatalf("stdout should signal finalized, got %q", out) + } + archived := filepath.Join(root, "_archive", "130-missing-mod-with-sentinel.md") + if !fileExists(archived) { + t.Fatal("finalize should archive the entity") + } + if got := frontmatterField(t, archived, "status"); got != "done" { + t.Fatalf("archived status=%q, want done", got) + } +} + +// TestMergeGuardFinalizesMissingMergeModRejected (AC-5 row c): a rejected verdict +// finalizes even when the mod-block names a missing merge mod — the entity never +// merged, so the rejected-verdict escape takes priority over the missing-mod +// refusal, exactly as it does today. +func TestMergeGuardFinalizesMissingMergeModRejected(t *testing.T) { + root, out, errOut, code := driveMergeGuard(t, "merge-pr-workflow", "140-missing-mod-rejected", "--verdict", "rejected") + if code != 0 { + t.Fatalf("missing-mod rejected should still finalize (exit 0), got %d (stderr=%q)", code, errOut) + } + if !strings.Contains(out, "finalized") { + t.Fatalf("stdout should signal finalized, got %q", out) + } + archived := filepath.Join(root, "_archive", "140-missing-mod-rejected.md") + if got := frontmatterField(t, archived, "verdict"); got != "rejected" { + t.Fatalf("archived verdict=%q, want rejected", got) + } +} + +// TestMergeGuardArmedLineNamesNextStep (AC-3, D3): the armed phase's default +// prose names the hook's file path and the never-invoked-by-the-verb caveat, plus +// the re-run command — the FO's next action, carried at fire time. +func TestMergeGuardArmedLineNamesNextStep(t *testing.T) { + root, out, errOut, code := driveMergeGuard(t, "merge-local-workflow", "020-no-sentinel", "--verdict", "passed") + if code != 0 { + t.Fatalf("arm should exit 0, got %d (stderr=%q)", code, errOut) + } + want := fmt.Sprintf( + "armed: mod-block set to merge:local-merge — invoke the local-merge merge hook (%s/_mods/local-merge.md; merge guard never invokes it), then re-run `merge guard 020-no-sentinel`.\n", + root) + if out != want { + t.Fatalf("armed line = %q, want %q", out, want) + } +} + +// TestMergeGuardBlockedLineNamesNextStep (AC-3, D3): the blocked phase's default +// prose names the never-finalize-on-open-PR invariant and the sentinel format +// that unlocks finalize, plus the re-run command. +func TestMergeGuardBlockedLineNamesNextStep(t *testing.T) { + _, out, errOut, code := driveMergeGuard(t, "merge-pr-workflow", "070-pr-pending", "--verdict", "passed") + if code != 0 { + t.Fatalf("blocked should exit 0, got %d (stderr=%q)", code, errOut) + } + want := "blocked: PR #42 is pending — mod-block left intact, never finalize on an open PR. " + + "When gh reports it MERGED, record the sentinel (pr=pr-merge:{number}) and re-run `merge guard 070-pr-pending`.\n" + if out != want { + t.Fatalf("blocked line = %q, want %q", out, want) + } +} + +// TestMergeGuardFinalizedLineNoWorktreeNoHookClause (AC-3, D3): an entity +// finalizing via a merged sentinel with NO worktree recorded gets the base +// finalized line only — no worktree-removal clause (nothing to remove) and no +// no-merge-hook clause (a hook IS registered and a sentinel IS recorded). +func TestMergeGuardFinalizedLineNoWorktreeNoHookClause(t *testing.T) { + _, out, errOut, code := driveMergeGuard(t, "merge-pr-workflow", "080-pr-merged", "--verdict", "passed") + if code != 0 { + t.Fatalf("finalize should exit 0, got %d (stderr=%q)", code, errOut) + } + want := "finalized: 080-pr-merged -> done (verdict passed), archived.\n" + if out != want { + t.Fatalf("finalized line = %q, want %q (no worktree/no-hook clauses)", out, want) + } +} + +// TestMergeGuardFinalizedLineWithWorktree (AC-3, D3): an entity finalizing via a +// merged sentinel WITH a recorded worktree gets the worktree-removal/branch- +// cleanup/teardown next-step clause, and — since a hook IS registered and a +// sentinel IS recorded — no no-merge-hook clause. +func TestMergeGuardFinalizedLineWithWorktree(t *testing.T) { + _, out, errOut, code := driveMergeGuard(t, "merge-pr-workflow", "150-worktree-finalize", "--verdict", "passed") + if code != 0 { + t.Fatalf("finalize should exit 0, got %d (stderr=%q)", code, errOut) + } + if !strings.Contains(out, "finalized: 150-worktree-finalize -> done (verdict passed), archived.") { + t.Fatalf("stdout should carry the base finalized line, got %q", out) + } + if !strings.Contains(out, "Next: push; remove the worktree (`git worktree remove .worktrees/150-worktree-finalize`") { + t.Fatalf("stdout should carry the worktree-removal next-step clause, got %q", out) + } + if !strings.Contains(out, "delete the local branch (`git branch -d`)") { + t.Fatalf("stdout should carry the branch-cleanup clause, got %q", out) + } + if !strings.Contains(out, "keep the remote branch while a PR references it") { + t.Fatalf("stdout should carry the remote-branch-retention clause, got %q", out) + } + if !strings.Contains(out, "tear down the entity's workers per your runtime adapter") { + t.Fatalf("stdout should carry the worker-teardown clause, got %q", out) + } + if strings.Contains(out, "no merge hook registered") { + t.Fatalf("stdout must NOT carry the no-merge-hook clause (a hook IS registered and a sentinel IS recorded), got %q", out) + } +} + +// TestMergeGuardFinalizedLineNoHookRegisteredNoWorktree (AC-3, D3): under a +// workflow with NO merge hook registered at all, a Phase-C default finalize (no +// pr, no mod-block, no worktree) names the manual `--no-ff` merge onto trunk — +// nothing automated it — with no worktree-removal clause. +func TestMergeGuardFinalizedLineNoHookRegisteredNoWorktree(t *testing.T) { + _, out, errOut, code := driveMergeGuard(t, "merge-no-hook-workflow", "010-no-hook-no-worktree", "--verdict", "passed") + if code != 0 { + t.Fatalf("finalize should exit 0, got %d (stderr=%q)", code, errOut) + } + want := "finalized: 010-no-hook-no-worktree -> done (verdict passed), archived.\n" + + "no merge hook registered — merge the stage branch onto main with --no-ff if not already merged.\n" + if out != want { + t.Fatalf("finalized output = %q, want %q", out, want) + } +} + +// TestMergeGuardFinalizedLineNoHookRegisteredWithWorktree (AC-3, D3): the same +// no-hook-registered path but WITH a recorded worktree carries BOTH next-step +// clauses — the two conditions are independent. +func TestMergeGuardFinalizedLineNoHookRegisteredWithWorktree(t *testing.T) { + _, out, errOut, code := driveMergeGuard(t, "merge-no-hook-workflow", "020-no-hook-with-worktree", "--verdict", "passed") + if code != 0 { + t.Fatalf("finalize should exit 0, got %d (stderr=%q)", code, errOut) + } + if !strings.Contains(out, "Next: push; remove the worktree (`git worktree remove .worktrees/020-no-hook-with-worktree`") { + t.Fatalf("stdout should carry the worktree-removal clause, got %q", out) + } + if !strings.Contains(out, "no merge hook registered — merge the stage branch onto main with --no-ff if not already merged.") { + t.Fatalf("stdout should carry the no-merge-hook clause, got %q", out) + } +} + // TestMergeGuardRequiresSlug (usage): a guard call with no slug is a usage error. func TestMergeGuardRequiresSlug(t *testing.T) { root := stageFixture(t, "merge-local-workflow") diff --git a/internal/status/merge_policy_guard_test.go b/internal/status/merge_policy_guard_test.go index cf7faae87..d653af196 100644 --- a/internal/status/merge_policy_guard_test.go +++ b/internal/status/merge_policy_guard_test.go @@ -123,6 +123,14 @@ func TestMergePrDefaultNoSentinelStillRefuses(t *testing.T) { if !strings.Contains(errOut, "cannot advance to terminal") { t.Fatalf("stderr should name the merge-hook refusal, got %q", errOut) } + // D4: the refusal tail warns instead of inviting --force, and names the re-run + // remedy; the "cannot advance to terminal" HEAD (asserted above) is unchanged. + if !strings.Contains(errOut, "(--force bypasses this guard; a refusal usually means a ceremony step was skipped — re-run merge guard 020-no-sentinel instead.)") { + t.Fatalf("stderr should carry the D4 reworded tail, got %q", errOut) + } + if strings.Contains(errOut, "or use --force to bypass") { + t.Fatalf("stderr must not invite bare --force anymore, got %q", errOut) + } if out != "" { t.Fatalf("stdout must be empty on rejection, got %q", out) } diff --git a/internal/status/mutate.go b/internal/status/mutate.go index 4e9d6b253..f8c2eea25 100644 --- a/internal/status/mutate.go +++ b/internal/status/mutate.go @@ -428,6 +428,11 @@ func PyJoin(parts ...string) string { return result } +// ScanMods is the exported alias of scanMods for callers outside this package +// (`dispatch.Sweep`'s startup-hook mod-pointer) that need the same hookPoint -> +// mod-name scan the boot MODS-REPORT and the merge-hook guard already use. +func ScanMods(definitionDir string) map[string][]string { return scanMods(definitionDir) } + // scanMods scans definitionDir/_mods/*.md for `## Hook:` headings, returning // hookPoint -> sorted mod names. Mods are workflow definition (lifecycle hooks // declared for the workflow, kin to the README stages), so they live next to the diff --git a/internal/status/testdata/golden/archive-guard-terminal-modblock.txt b/internal/status/testdata/golden/archive-guard-terminal-modblock.txt index 56fa6d08a..cff71c4ce 100644 --- a/internal/status/testdata/golden/archive-guard-terminal-modblock.txt +++ b/internal/status/testdata/golden/archive-guard-terminal-modblock.txt @@ -3,4 +3,4 @@ ===== stdout ===== ===== stderr ===== -Error: entity 010-blocked has pending mod-block (merge:pr-merge). Clear mod-block in a separate --set call, or use --force. +Error: entity 010-blocked has pending mod-block (merge:pr-merge). Clear mod-block in a separate --set call. (--force bypasses this guard; a refusal usually means a ceremony step was skipped — re-run merge guard 010-blocked instead.) diff --git a/internal/status/testdata/golden/merge-local-combined-clear-terminalize.txt b/internal/status/testdata/golden/merge-local-combined-clear-terminalize.txt index 350d73c4e..2278fe15e 100644 --- a/internal/status/testdata/golden/merge-local-combined-clear-terminalize.txt +++ b/internal/status/testdata/golden/merge-local-combined-clear-terminalize.txt @@ -3,4 +3,4 @@ ===== stdout ===== ===== stderr ===== -Error: entity 030-pending has combined mod-block clear with terminal transition (current mod-block='merge:local-merge'). Clear mod-block in a separate --set call, or use --force. +Error: entity 030-pending has combined mod-block clear with terminal transition (current mod-block='merge:local-merge'). Clear mod-block in a separate --set call. (--force bypasses this guard; a refusal usually means a ceremony step was skipped — re-run merge guard 030-pending instead.) diff --git a/internal/status/testdata/golden/merge-local-modblock-pending.txt b/internal/status/testdata/golden/merge-local-modblock-pending.txt index 3819fda79..aa23e7316 100644 --- a/internal/status/testdata/golden/merge-local-modblock-pending.txt +++ b/internal/status/testdata/golden/merge-local-modblock-pending.txt @@ -3,4 +3,4 @@ ===== stdout ===== ===== stderr ===== -Error: entity 030-pending has pending mod-block (merge:local-merge). Clear mod-block in a separate --set call, or use --force. +Error: entity 030-pending has pending mod-block (merge:local-merge). Clear mod-block in a separate --set call. (--force bypasses this guard; a refusal usually means a ceremony step was skipped — re-run merge guard 030-pending instead.) diff --git a/internal/status/testdata/golden/merge-pr-default-nosentinel-set.txt b/internal/status/testdata/golden/merge-pr-default-nosentinel-set.txt index 7adaf11ee..573d503ca 100644 --- a/internal/status/testdata/golden/merge-pr-default-nosentinel-set.txt +++ b/internal/status/testdata/golden/merge-pr-default-nosentinel-set.txt @@ -3,4 +3,4 @@ ===== stdout ===== ===== stderr ===== -Error: entity 020-no-sentinel cannot advance to terminal — workflow has merge hook(s) [local-merge] that have not run (pr field is empty and mod-block is empty). Set mod-block=merge:local-merge and invoke the hook, or use --force to bypass. +Error: entity 020-no-sentinel cannot advance to terminal — workflow has merge hook(s) [local-merge] that have not run (pr field is empty and mod-block is empty). Set mod-block=merge:local-merge and invoke the hook. (--force bypasses this guard; a refusal usually means a ceremony step was skipped — re-run merge guard 020-no-sentinel instead.) diff --git a/internal/status/testdata/golden/native-guard-terminal-modblock.txt b/internal/status/testdata/golden/native-guard-terminal-modblock.txt index 56fa6d08a..cff71c4ce 100644 --- a/internal/status/testdata/golden/native-guard-terminal-modblock.txt +++ b/internal/status/testdata/golden/native-guard-terminal-modblock.txt @@ -3,4 +3,4 @@ ===== stdout ===== ===== stderr ===== -Error: entity 010-blocked has pending mod-block (merge:pr-merge). Clear mod-block in a separate --set call, or use --force. +Error: entity 010-blocked has pending mod-block (merge:pr-merge). Clear mod-block in a separate --set call. (--force bypasses this guard; a refusal usually means a ceremony step was skipped — re-run merge guard 010-blocked instead.) diff --git a/internal/status/testdata/merge-no-hook-workflow/010-no-hook-no-worktree.md b/internal/status/testdata/merge-no-hook-workflow/010-no-hook-no-worktree.md new file mode 100644 index 000000000..af09ed79f --- /dev/null +++ b/internal/status/testdata/merge-no-hook-workflow/010-no-hook-no-worktree.md @@ -0,0 +1,14 @@ +--- +id: "010" +title: No merge hook registered, no worktree recorded +status: implementation +score: "0.40" +source: roadmap +--- +# No merge hook registered, no worktree recorded + +No `_mods/` merge hook is registered for this workflow, no `pr` sentinel, no +`mod-block`, and no `worktree` frontmatter field. `merge guard --verdict passed` +reaches Phase C default and finalizes; the finalized line must name the manual +`--no-ff` merge onto trunk since nothing automated it, with no worktree-removal +clause (there is no worktree to remove). diff --git a/internal/status/testdata/merge-no-hook-workflow/020-no-hook-with-worktree.md b/internal/status/testdata/merge-no-hook-workflow/020-no-hook-with-worktree.md new file mode 100644 index 000000000..f1def818e --- /dev/null +++ b/internal/status/testdata/merge-no-hook-workflow/020-no-hook-with-worktree.md @@ -0,0 +1,14 @@ +--- +id: "020" +title: No merge hook registered, worktree recorded +status: implementation +worktree: .worktrees/020-no-hook-with-worktree +score: "0.40" +source: roadmap +--- +# No merge hook registered, worktree recorded + +Same as the sibling no-hook entity, but a `worktree` frontmatter field is +recorded. `merge guard --verdict passed` finalizes; the finalized line must +carry BOTH the worktree-removal next-step clause and the no-merge-hook manual +`--no-ff` clause — the two conditions are independent. diff --git a/internal/status/testdata/merge-no-hook-workflow/030-missing-mod-no-hooks-registered.md b/internal/status/testdata/merge-no-hook-workflow/030-missing-mod-no-hooks-registered.md new file mode 100644 index 000000000..8ca0c6c00 --- /dev/null +++ b/internal/status/testdata/merge-no-hook-workflow/030-missing-mod-no-hooks-registered.md @@ -0,0 +1,16 @@ +--- +id: "030" +title: Mod-block names a merge mod but NO merge hook is registered at all +status: implementation +mod-block: merge:ghost-merge +score: "0.40" +source: roadmap +--- +# Mod-block names a merge mod but NO merge hook is registered at all + +`mod-block: merge:ghost-merge`, and this workflow (unlike the sibling +merge-pr-workflow fixture) registers NO merge hook whatsoever — `_mods/` has no +`## Hook: merge` mod at all, not merely a mismatched one. This is the likeliest +real-world D5 trigger: the deleted mod file WAS the workflow's only merge hook. +`merge guard` must still refuse (not silently finalize via the len(mergeHooks)==0 +default-Phase-C path) since no sentinel records a landed merge. diff --git a/internal/status/testdata/merge-no-hook-workflow/README.md b/internal/status/testdata/merge-no-hook-workflow/README.md new file mode 100644 index 000000000..d28b5a848 --- /dev/null +++ b/internal/status/testdata/merge-no-hook-workflow/README.md @@ -0,0 +1,29 @@ +--- +entity-type: task +entity-label: task +entity-label-plural: tasks +id-style: sequential +stages: + defaults: + worktree: false + concurrency: 1 + states: + - name: backlog + initial: true + gate: true + - name: ideation + gate: true + - name: implementation + worktree: true + - name: done + terminal: true +--- + +# No-Merge-Hook Fixture Workflow + +The default-policy (`merge:` key absent) sibling with NO `_mods/` directory at +all — no merge hook is registered. `merge guard`'s Phase C default case reaches +these entities directly: the terminalize `--set` is unguarded (no merge hook to +require) and finalize proceeds without ever having invoked a hook or recorded a +merge sentinel, which is exactly the state the D3 finalized-line "no merge hook +registered" clause exists to name. diff --git a/internal/status/testdata/merge-pr-workflow/120-missing-mod-no-sentinel.md b/internal/status/testdata/merge-pr-workflow/120-missing-mod-no-sentinel.md new file mode 100644 index 000000000..82addaf16 --- /dev/null +++ b/internal/status/testdata/merge-pr-workflow/120-missing-mod-no-sentinel.md @@ -0,0 +1,16 @@ +--- +id: "120" +title: Entity blocked on a merge mod that no longer exists, no sentinel +status: implementation +verdict: passed +mod-block: merge:ghost-merge +score: "0.40" +source: roadmap +--- +# Entity blocked on a merge mod that no longer exists, no sentinel + +`mod-block: merge:ghost-merge` names a merge mod that is NOT registered under +`_mods/` (the file was deleted mid-ceremony). No `pr` sentinel records a landed +merge, and `verdict` is not `rejected`. `merge guard` must refuse rather than +silently finalize — clearing the block here would archive the entity without the +hook ever having run. diff --git a/internal/status/testdata/merge-pr-workflow/130-missing-mod-with-sentinel.md b/internal/status/testdata/merge-pr-workflow/130-missing-mod-with-sentinel.md new file mode 100644 index 000000000..b66ce2c36 --- /dev/null +++ b/internal/status/testdata/merge-pr-workflow/130-missing-mod-with-sentinel.md @@ -0,0 +1,17 @@ +--- +id: "130" +title: Entity blocked on a missing merge mod but carrying a landed-merge sentinel +status: implementation +verdict: passed +mod-block: merge:ghost-merge +pr: pr-merge:77 +score: "0.40" +source: roadmap +--- +# Entity blocked on a missing merge mod but carrying a landed-merge sentinel + +`mod-block: merge:ghost-merge` names a merge mod that is NOT registered under +`_mods/`, but `pr: pr-merge:77` is a well-formed merge sentinel — the merge +genuinely landed before the mod file was deleted. `merge guard` must finalize as +today: a recorded sentinel honestly proves the ceremony ran, so the missing mod +file is not a stuck state. diff --git a/internal/status/testdata/merge-pr-workflow/140-missing-mod-rejected.md b/internal/status/testdata/merge-pr-workflow/140-missing-mod-rejected.md new file mode 100644 index 000000000..73243f582 --- /dev/null +++ b/internal/status/testdata/merge-pr-workflow/140-missing-mod-rejected.md @@ -0,0 +1,15 @@ +--- +id: "140" +title: Rejected entity blocked on a merge mod that no longer exists +status: implementation +verdict: rejected +mod-block: merge:ghost-merge +score: "0.40" +source: roadmap +--- +# Rejected entity blocked on a merge mod that no longer exists + +`mod-block: merge:ghost-merge` names a merge mod that is NOT registered under +`_mods/`, and `verdict: rejected` — the entity never merged, so the missing mod +file cannot have stranded a landed merge. `merge guard` must finalize as today; +the rejected-verdict escape takes priority over the missing-mod refusal. diff --git a/internal/status/testdata/merge-pr-workflow/150-worktree-finalize.md b/internal/status/testdata/merge-pr-workflow/150-worktree-finalize.md new file mode 100644 index 000000000..498eda5a1 --- /dev/null +++ b/internal/status/testdata/merge-pr-workflow/150-worktree-finalize.md @@ -0,0 +1,16 @@ +--- +id: "150" +title: Entity finalizing from a merged sentinel with a worktree recorded +status: implementation +worktree: .worktrees/150-worktree-finalize +score: "0.50" +source: roadmap +pr: pr-merge:88 +--- +# Entity finalizing from a merged sentinel with a worktree recorded + +The PR landed (`pr: pr-merge:88`) and a `worktree` frontmatter field is still +recorded (the FO has not yet removed it). `merge guard` finalizes; the finalized +line must carry the worktree-removal/branch-cleanup/teardown next-step clause. +Since a merge hook IS registered (`local-merge`) and a sentinel IS recorded, the +no-merge-hook manual-merge clause must NOT appear. diff --git a/skills/first-officer/references/first-officer-shared-core.md b/skills/first-officer/references/first-officer-shared-core.md index 8ac1bcaa7..fa65c5c67 100644 --- a/skills/first-officer/references/first-officer-shared-core.md +++ b/skills/first-officer/references/first-officer-shared-core.md @@ -111,37 +111,24 @@ The FO declares state intent by invoking the prose-functions below. Each is idem - **done-when:** the boot record is in hand for the greet. - → **shipped**: `` `spacedock status --boot --json` ``. -## «state.ensure-ready»(): the split-root checkout is linked and integrated with peers before any dispatch +## «state.ensure-ready»(): split-root checkout linked & integrated pre-dispatch -- **guard:** `state_backend == split-root` (a single-root workflow is a no-op). -- **effect — halt-gate.** If `entity_dir_present == false`, the state checkout is NOT initialized (orphan branch on origin without a linked worktree — fresh clone or removed worktree); the boot table renders EMPTY yet `--validate` VALID, a silent failure. HALT dispatch, report "state not initialized," and run (or prompt the captain to run) `spacedock state init` (manual fallback: `git fetch origin && git worktree add `). Re-invoke `«state.boot»()` and proceed only after `entity_dir_present == true`. -- **effect — pull-on-boot.** Before the greet, `git -C pull --rebase origin ` to integrate peers' state (one pull at boot, NOT per-read). -- **done-when:** `entity_dir_present == true` and the boot rebase is clean. -- **block:** on `pull --rebase` CONFLICT, `«halt.rebase-conflict»(paths)` — do not dispatch against an unmerged state tree. -- → **shipped**: `` `spacedock state ready` ``. +- **guard:** `state_backend == split-root` (single-root is a no-op). +- → **shipped**: `` `spacedock state ready` `` — one pull at boot, not per-read; resumes an absent checkout; stderr/exit carry the halt remediation + resume sequencing; on exit 3 → `«halt.rebase-conflict»(paths)`. -## «state.sweep-merged»(): merged PRs reach their terminal stage at boot, before the greet +## «state.sweep-merged»(): merged PRs reach terminal at boot -- **guard:** `pr_state` has an entry whose `state == "MERGED"` and whose entity status is non-terminal. -- **effect:** for each such entry, read `_mods/pr-merge.md` and run its startup-hook advancement (clear `mod-block`, terminalize `verdict=PASSED`, archive, remove the worktree). Skip when no such entry exists — the common boot reads zero mod files. A greet-and-stop boot never enters the event loop, so a merged PR is advanced here or not at all. -- **done-when:** no `MERGED` + non-terminal entity remains. -- **block:** when `pr_state.status == "gh not available"`, the merge state is unknowable — skip the sweep (per the pr-merge mod's "warn the captain and skip PR state checks") and treat merge status as UNKNOWN in the greet, not as stale or absent. -- → **shipped**: `` `spacedock state sweep` ``. +- **guard:** an entry `state == "MERGED"` and non-terminal; skip otherwise (zero mod files read). +- → **shipped**: `` `spacedock state sweep` `` — advanced here or never (greet-and-stop skips the event loop); output distinguishes real-empty from gh-unavailable UNKNOWN, names the startup-hook mod to advance per. -## «state.commit»(slug): record one entity's change durably and concurrency-safe +## «state.commit»(slug): record an entity's change durably -- **effect:** invoke `spacedock state commit ` after each state mutation. The command resolves the split-root entity, commits only that entity path, syncs with `origin` when present, and reports local-only/no-op as needed. -- **done-when:** the command exits 0 with the entity committed, and pushed when an origin exists. -- **block:** exit 3 means a same-entity rebase conflict was aborted by `spacedock state commit`; `«halt.rebase-conflict»(paths)`. -- → **shipped**: `` `spacedock state commit ` ``. +- → **shipped**: `` `spacedock state commit ` `` — on exit 3 → `«halt.rebase-conflict»(paths)`. -## «halt.rebase-conflict»(paths): a two-writer state conflict halts dispatch — abort, surface, stop +## «halt.rebase-conflict»(paths): abort, surface, stop -Invoked on a `pull --rebase` CONFLICT on the split-root state checkout — from `«state.ensure-ready»`'s manual pull or a `«state.commit»` exit 3. - -- **block:** HALT; ensure the rebase is aborted (`git rebase --abort` when the FO holds the conflict; already done on a `«state.commit»` exit 3); surface the named conflicting `paths` and the peer commit to the captain; stop. Never `--force`/`--force-with-lease`, never `-X ours`/`-X theirs`, never silently discard either side — do not force-push or auto-resolve. -- **done-when:** the conflict is surfaced to the captain and dispatch is halted against the unmerged state tree. -- → **prose** — no binary resolves a two-writer frontmatter conflict; the FO halts and the captain reconciles. +- **block:** a `«state.ensure-ready»`/`«state.commit»` exit 3 already carries the remediation in its stderr — HALT per that output. A manual FO-held `pull --rebase` CONFLICT (code worktree, `claude-fo-dispatch.md:162`): run `git rebase --abort`, surface `paths` + peer commit to the captain, stop. Never `--force`/`--force-with-lease`, never `-X ours`/`-X theirs`, never discard either side — do not force-push or auto-resolve. +- → **prose** — no binary resolves a two-writer conflict; the FO halts and the captain reconciles. ## Mod Hook Convention diff --git a/skills/first-officer/references/fo-merge-core.md b/skills/first-officer/references/fo-merge-core.md index 4e0374db7..96f38a894 100644 --- a/skills/first-officer/references/fo-merge-core.md +++ b/skills/first-officer/references/fo-merge-core.md @@ -4,42 +4,23 @@ The terminal merge-and-cleanup ceremony, the mod-block guard that protects it, a ## Merge and Cleanup -When an entity reaches its terminal stage, `«merge.guard»(slug)` drives the terminal merge-finalize ceremony as a re-entrant envelope: the FO invokes `spacedock merge guard` once per phase, and `«merge.guard»` owns arm + clear + terminalize + archive (including the path-scoped archive commit). It reads the `pr`/`mod-block`/`verdict` state delta and signals the **armed** / **blocked** / **finalized** phase (defined below), the same under both `merge:` policies. The FO owns only the steps `«merge.guard»` cannot: invoking the hook, pushing, opening the captain-gated PR, and removing the worktree+branch. - -`«merge.guard»` never invokes the hook and never local-merges, and the clear+terminalize is always two separate `--set` calls, not one. The merge sentinel is the FINALIZE key: the FO's `pr-merge` startup/idle/sweep hook detects MERGED via `gh` and records the sentinel; `«merge.guard»` keys off it without ever talking to GitHub. +`«merge.guard»` never invokes the merge hook and never local-merges — `merge: local`'s hook itself performs the `--no-ff merge`; `merge: pr` opens the captain-gated PR. The merge sentinel is the FINALIZE key: the FO's `pr-merge` startup/idle/sweep hook detects MERGED via `gh` and records the sentinel; `«merge.guard»` keys off it without ever talking to GitHub. Each phase's next action for the FO — which hook to invoke, when to wait, what to clean up — is named in `merge guard`'s own output; see `«merge.guard»` below. **Launcher invariant — use the right binary.** `merge guard` exists only on the current-checkout / `SPACEDOCK_BIN` binary, not necessarily the brew-installed `spacedock` on `$PATH`. Invoke it via `${SPACEDOCK_BIN:-spacedock} merge guard ` (or the checkout binary directly). If you call a stale `spacedock` that predates `«merge.guard»`, the subcommand is unknown and you silently fall back to the hand ceremony — the toil `«merge.guard»` eliminates. ## «merge.guard»(slug): auto-arm → block-on-open-PR → finalize-on-merge-sentinel, then archive -- **effect:** drive the terminal merge-finalize ceremony. **auto-arm (both policies):** with an empty mod-block and a merge hook registered, set `mod-block=merge:{mod_name}` and signal the FO to invoke the hook (`«merge.guard»` does NOT invoke it). **finalize:** read the `pr`/`mod-block`/`verdict` delta — if the verdict is `rejected` OR `pr` carries a merge sentinel (`pr-merge:`/`local-merge:`), clear the mod-block in its own standalone `--set`, terminalize (`completed verdict={verdict} worktree=`), archive, and commit the archive move path-scoped; finalize works EVEN from a non-armed state. **block:** if `pr` is a bare/open reference and the verdict is not `rejected`, signal `blocked` and leave state intact — never finalize on `pr`-presence alone. `«merge.guard»` never invokes the hook and never local-merges; the mechanism-level mod-block / merge-hook guards (below) refuse any out-of-order or hook-skipping terminal transition without `--force`, and `«merge.guard»` propagates — never bypasses — that refusal. -- **done-when:** the entity is archived terminal with its mod-block cleared and `pr`/sentinel recorded, the archive move committed path-scoped (or `«merge.guard»` left it blocked on an open PR, mod-block intact and the pending state reported). -- **block:** if the hook blocks (open PR pending, captain approval pending, external wait), leave `mod-block` set and do not local-merge. `--force` is never part of the happy path — if the guard refuses, a step was skipped, not a flag forgotten. -- → **shipped**: `` `spacedock merge guard ` `` — invoke it directly per phase (via `${SPACEDOCK_BIN:-spacedock}`, per the launcher invariant above). The hand-followed sequence it automates is the steps below. - -`«merge.guard»` performs the arm/clear/terminalize/archive sequence and commits the archive move path-scoped. The FO owns only what `«merge.guard»` does NOT: +- **effect:** drive the terminal merge-finalize ceremony — auto-arm, block on an open PR, finalize on a merge sentinel, then archive (including the path-scoped archive commit) — the same under both `merge:` policies. Invoke it once per phase; its own stdout/stderr name the FO's next action (armed: which hook to invoke and where; blocked: wait for the sentinel, never finalize on `pr`-presence alone; finalized: worktree/branch/worker cleanup, or the manual merge when no hook is registered). +- **done-when:** the entity is archived terminal, or `«merge.guard»` left it armed/blocked with its next step named in its own output. +- **block:** `--force` is never part of the happy path — if the guard refuses, a step was skipped, not a flag forgotten. +- → **shipped**: `` `spacedock merge guard ` `` — invoke it directly per phase (via `${SPACEDOCK_BIN:-spacedock}`, per the launcher invariant above). -1. **Invoke the merge hook.** When `«merge.guard»` signals `armed`, run the merge hooks (`merge: local` does the local `--no-ff` merge and records a `local-merge:{sha}` sentinel; `merge: pr` opens the captain-gated PR and sets `pr`), then re-run `merge guard`. `«merge.guard»` signals; the FO invokes. When the PR is detected MERGED (the `pr-merge` startup/idle/sweep hook's `gh` check), record the `pr-merge:{number}` sentinel in `pr` and re-run `merge guard` — the sentinel is what unlocks finalize. -2. **Default local merge** when no merge hook is registered: merge `{branch}` onto the trunk (`${SPACEDOCK_BIN:-spacedock} dispatch trunk --workflow-dir {workflow_dir}`, default `main`) from the stage worktree branch. `«merge.guard»` never local-merges under any policy. -3. **Remove the worktree** (`git worktree remove {path}`, no `--force`) and delete the local branch (`git branch -d {branch}`). Do NOT delete the remote branch while a PR is pending — the reviewer needs it; remote cleanup belongs to the PR merge. -4. **Teardown workers at terminal (step 10).** At the terminal boundary, derive the entity's worker cohort and cooperatively shut each one down (best-effort, fire-and-forget — the runtime adapter supplies the shutdown call), then drop them from session memory. Mandatory whether the merge ran locally or via a PR host. The cohort-derivation rule and the cooperative-shutdown call are the adapter's. A runtime MAY add a further teardown step (e.g. a bounded team-registry teardown); the adapter declares it where it applies. This core states only the boundary obligation: cohort shutdown, then drop from session memory. +**Step 10: teardown workers at terminal.** At the terminal boundary, derive the entity's worker cohort and cooperatively shut each one down (best-effort, fire-and-forget — the runtime adapter supplies the shutdown call), then drop them from session memory. Mandatory whether the merge ran locally or via a PR host — `«merge.guard»`'s own finalized output only points here ("tear down the entity's workers per your runtime adapter"); the cohort-derivation rule and the cooperative-shutdown call are the adapter's. A runtime MAY add a further teardown step (e.g. a bounded team-registry teardown); the adapter declares it where it applies. ### Worktree removal safety -Use `git worktree remove {path}` (no `--force`). The default refuses to delete a worktree with untracked changes — that refusal is the safety net. - -If removal fails on untracked files, the FO MUST: - -1. Audit: `git -C {path} status --short` from the parent worktree. -2. Decide per file: commit to the worktree branch (audit-essential per gitignore), move to a persistent location (experiment-output outside the worktree), or explicitly confirm destruction with the captain. -3. ONLY after the audit, `--force` is permitted. - -`--force` is never default; it is an explicit captain-confirmed bypass. +`--force` is never default; audit untracked files with the operator first. ## Mod-Block Guard -The mod-block guard exists so the FO recovers on session resume and so the merge ceremony cannot skip its hook. `merge guard` owns the set→clear mechanics; the FO's standing concern is recovery: - -- **Survives session resume.** Read `mod-block` from frontmatter on boot. A non-empty `mod-block=merge:{mod_name}` means a merge hook is mid-flight — do not re-run it from scratch; check what it left (PR created? branch pushed?) and re-run `merge guard` to continue. -- **The guard refuses, it does not auto-fix.** `status --set`/`status --archive` refuse a terminal transition while `mod-block` is non-empty, refuse combining `mod-block=` with terminal fields, and refuse terminalizing with merge hooks registered while `pr` and `mod-block` are both empty (the hook provably has not run). `--force` bypasses; `merge: local` and `verdict=rejected` exempt only the pr-requirement. Do NOT `--force` merely to clear the guard — it catches exactly the mistake of skipping the hook. -- **A missing blocking mod is a captain escalation.** If `{workflow_dir}/_mods/{mod_name}.md` is missing or unreadable, report: "Blocking mod {mod_name} is missing. The entity is stuck. Options: restore the mod file, or use `--force` to clear the block and resume normal flow." Wait for direction. +A non-empty `mod-block=merge:{mod_name}` on boot means a merge is mid-flight — check what the hook left and re-run `merge guard` to continue.