Skip to content

Commit 7cb5555

Browse files
clkaoSpike Test
andauthored
State/merge error-path remediation moves from resident contract prose into the binary's own failure output (#465)
* D4: reword mod-block/merge-hook --set refusal tails, drop bare --force invitation * D5: merge guard refuses a mod-block naming a missing merge mod (no-sentinel case) * D3: merge guard armed/blocked/finalized default prose names the FO's next step * D1: state commit/ready HALT stderr carries peer commit + FO remediation; ready resume prints re-boot line * D2: state sweep distinguishes gh-unavailable UNKNOWN from truly empty; names the startup-hook mod-pointer next step * Shrink shared-core and fo-merge-core prose per the guidance-coverage map (AC-1) * Feedback cycle 1: strengthen D5 test coverage per adversarial audit T1: dedicated test for the no-merge-hook-registered-at-all shape (the deleted mod file was the workflow's only hook) — distinct from the existing missing-mod test, which fixtures a workflow with a DIFFERENT hook still registered. Catches a mutant that special-cases modBlockNamesMissingMergeMod on len(mergeHooks)==0. T2: pin the full remediation tail on the existing missing-mod refusal test, not just the mod-name + "is missing" fragment. Both mutants verified RED locally before landing, then reverted. * Feedback cycle 2: fix ghRunnerExec to parse the --jq .state shape, not a JSON envelope Root cause: ghRunnerExec called `gh pr view PR --json state` and json.Unmarshal'd the result, while boot.go's already-proven PR_STATE probe uses `gh pr view PR --json state --jq .state` (bare, jq-extracted text). Against a `gh` whose output matches the latter shape — including the shared ensigncycle shallow-boot fixture's stub gh, which PR #465's codex-live scenario runs against — the JSON unmarshal fails and a working probe was counted as an error by D2's gh-unavailable classifier, making `state sweep` report UNKNOWN instead of advancing the merged entity. Fix: align ghRunnerExec with boot.go's proven invocation exactly (same flags, same leading-"#" trim, same --jq extraction). The two probes now agree on what "gh can answer" means. Reproduced offline against the exact shared fixture shape before fixing; regression tests pin both the direct probe (TestGhRunnerExecParsesJqExtractedState) and the end-to-end sweep (TestSweepWithRealGhStubDoesNotReportUnknown) using the real production GhRunnerExec. Both verified RED against the pre-fix ghRunnerExec, reverted, GREEN restored. --------- Co-authored-by: Spike Test <spike@example.com>
1 parent 8be4cc4 commit 7cb5555

26 files changed

Lines changed: 975 additions & 90 deletions

internal/cli/state_commit_test.go

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,64 @@ func TestStateCommitHaltsOnSameEntityConflict(t *testing.T) {
122122
}
123123
}
124124

125+
// TestStateCommitHaltStderrCarriesRemediationAndPeerCommit pins AC-2 (D1): the
126+
// exit-3 HALT stderr carries the FO's next-action line, the never-force/never-
127+
// auto-resolve line, and the peer commit that survived the aborted rebase — the
128+
// remediation the resident prose used to own, now emitted at fire time. The peer
129+
// commit is A's pushed HEAD sha (the pull's fetch phase updates origin/{branch}
130+
// before the rebase conflicts; abort does not touch it).
131+
func TestStateCommitHaltStderrCarriesRemediationAndPeerCommit(t *testing.T) {
132+
_, workflowA, workflowB, _ := twoHostStateWorkflow(t)
133+
checkoutA := filepath.Join(workflowA, ".spacedock-state")
134+
hostA := filepath.Dir(filepath.Dir(workflowA))
135+
136+
writeEntity(t, workflowA, "first-task", "---\nstatus: implementation\n---\n# First Task (A)\n")
137+
if code, _, errOut := runStateCommitCmd(t, hostA, workflowA, "first-task", "-m", "A: -> implementation"); code != 0 {
138+
t.Fatalf("A's commit should succeed (exit 0); got exit=%d stderr=%q", code, errOut)
139+
}
140+
peerSHA := strings.TrimSpace(git(t, checkoutA, "rev-parse", "--short", "HEAD"))
141+
142+
writeEntity(t, workflowB, "first-task", "---\nstatus: review\n---\n# First Task (B)\n")
143+
hostB := filepath.Dir(filepath.Dir(workflowB))
144+
code, _, errOut := runStateCommitCmd(t, hostB, workflowB, "first-task", "-m", "B: -> review")
145+
if code != 3 {
146+
t.Fatalf("same-entity conflict must HALT with exit 3; got exit=%d stderr=%q", code, errOut)
147+
}
148+
if !strings.Contains(errOut, "Peer commit: "+peerSHA) {
149+
t.Fatalf("HALT stderr should name the peer commit %q, got:\n%s", peerSHA, errOut)
150+
}
151+
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.") {
152+
t.Fatalf("HALT stderr should name the FO's next action, got:\n%s", errOut)
153+
}
154+
if !strings.Contains(errOut, "Never `git push --force`/`--force-with-lease`; never re-run with `-X ours`/`-X theirs`; never discard either side.") {
155+
t.Fatalf("HALT stderr should carry the never-force/never-auto-resolve line, got:\n%s", errOut)
156+
}
157+
}
158+
159+
// TestStateCommitHaltJSONCarriesPeerCommit pins AC-2's --json requirement: the
160+
// halt envelope carries peer_commit alongside the existing conflicting_paths.
161+
func TestStateCommitHaltJSONCarriesPeerCommit(t *testing.T) {
162+
_, workflowA, workflowB, _ := twoHostStateWorkflow(t)
163+
checkoutA := filepath.Join(workflowA, ".spacedock-state")
164+
hostA := filepath.Dir(filepath.Dir(workflowA))
165+
166+
writeEntity(t, workflowA, "first-task", "---\nstatus: implementation\n---\n# First Task (A)\n")
167+
if code, _, errOut := runStateCommitCmd(t, hostA, workflowA, "first-task", "-m", "A: -> implementation"); code != 0 {
168+
t.Fatalf("A's commit should succeed (exit 0); got exit=%d stderr=%q", code, errOut)
169+
}
170+
peerSHA := strings.TrimSpace(git(t, checkoutA, "rev-parse", "--short", "HEAD"))
171+
172+
writeEntity(t, workflowB, "first-task", "---\nstatus: review\n---\n# First Task (B)\n")
173+
hostB := filepath.Dir(filepath.Dir(workflowB))
174+
code, stdout, errOut := runStateCommitCmd(t, hostB, workflowB, "first-task", "-m", "B: -> review", "--json")
175+
if code != 3 {
176+
t.Fatalf("same-entity conflict must HALT with exit 3; got exit=%d stderr=%q", code, errOut)
177+
}
178+
if !strings.Contains(stdout, `"peer_commit": "`+peerSHA+`"`) {
179+
t.Fatalf("--json halt envelope should carry peer_commit=%q, got:\n%s", peerSHA, stdout)
180+
}
181+
}
182+
125183
// TestStateCommitIsPathScoped pins AC-2: a sibling dirty/untracked file in the
126184
// state checkout is NOT swept into the commit (the verb stages exactly the entity,
127185
// never `add -A`). This is the w4 2/3 `cd && git add -A` drift the verb deletes.

internal/cli/state_ready_test.go

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,41 @@ func TestStateReadyHaltsOnBootConflict(t *testing.T) {
8585
}
8686
}
8787

88+
// TestStateReadyHaltStderrCarriesRemediationAndPeerCommit pins AC-2 (D1) for the
89+
// boot-conflict HALT path: identical remediation to `state commit`'s exit-3 halt
90+
// — the peer commit, the FO's next-action line, and the never-force line.
91+
func TestStateReadyHaltStderrCarriesRemediationAndPeerCommit(t *testing.T) {
92+
_, workflowA, workflowB, _ := twoHostStateWorkflow(t)
93+
checkoutA := filepath.Join(workflowA, ".spacedock-state")
94+
checkoutB := filepath.Join(workflowB, ".spacedock-state")
95+
hostA := filepath.Dir(filepath.Dir(workflowA))
96+
hostB := filepath.Dir(filepath.Dir(workflowB))
97+
98+
writeEntity(t, workflowA, "first-task", "---\nstatus: implementation\n---\n# First Task (A)\n")
99+
if code, _, errOut := runStateCommitCmd(t, hostA, workflowA, "first-task", "-m", "A: -> implementation"); code != 0 {
100+
t.Fatalf("A's commit should succeed; exit=%d stderr=%q", code, errOut)
101+
}
102+
peerSHA := strings.TrimSpace(git(t, checkoutA, "rev-parse", "--short", "HEAD"))
103+
104+
writeEntity(t, workflowB, "first-task", "---\nstatus: review\n---\n# First Task (B)\n")
105+
git(t, checkoutB, "add", "first-task.md")
106+
git(t, checkoutB, "commit", "-q", "-m", "B: -> review", "--", "first-task.md")
107+
108+
code, _, errOut := runStateReadyCmd(t, hostB, workflowB)
109+
if code != 3 {
110+
t.Fatalf("same-entity boot conflict must HALT with exit 3; got exit=%d stderr=%q", code, errOut)
111+
}
112+
if !strings.Contains(errOut, "Peer commit: "+peerSHA) {
113+
t.Fatalf("ready HALT stderr should name the peer commit %q, got:\n%s", peerSHA, errOut)
114+
}
115+
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.") {
116+
t.Fatalf("ready HALT stderr should name the FO's next action, got:\n%s", errOut)
117+
}
118+
if !strings.Contains(errOut, "Never `git push --force`/`--force-with-lease`; never re-run with `-X ours`/`-X theirs`; never discard either side.") {
119+
t.Fatalf("ready HALT stderr should carry the never-force/never-auto-resolve line, got:\n%s", errOut)
120+
}
121+
}
122+
88123
// TestStateReadyInlineNoOp pins AC-6 inline case: an inline workflow is a clean
89124
// no-op (exit 0, nothing to sync, no git network op).
90125
func TestStateReadyInlineNoOp(t *testing.T) {
@@ -136,11 +171,17 @@ func TestStateReadyResumesAbsentCheckout(t *testing.T) {
136171
if _, err := os.Stat(freshState); !os.IsNotExist(err) {
137172
t.Fatalf("precondition: fresh clone should NOT yet have the state checkout (err=%v)", err)
138173
}
139-
code, _, errOut := runStateReadyCmd(t, fresh, freshWorkflow)
174+
code, stdout, errOut := runStateReadyCmd(t, fresh, freshWorkflow)
140175
if code != 0 {
141176
t.Fatalf("state ready on an absent checkout should resume it (exit 0); got exit=%d stderr=%q", code, errOut)
142177
}
143178
if _, err := os.Stat(freshState); err != nil {
144179
t.Fatalf("state ready should have resumed the absent checkout: %v", err)
145180
}
181+
// D1(c): the resume path carries the re-boot-after-resume sequencing the
182+
// «state.ensure-ready» prose used to own — the FO must re-invoke the boot read
183+
// before the greet since the checkout was just linked.
184+
if !strings.Contains(stdout, "checkout resumed — re-run `spacedock status --boot` before the greet.") {
185+
t.Fatalf("resumed checkout should print the re-boot-before-greet line; stdout:\n%s", stdout)
186+
}
146187
}

internal/cli/state_sync.go

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ type syncResult struct {
2626
Result string `json:"result"`
2727
StateBranch string `json:"state_branch,omitempty"`
2828
ConflictingPaths []string `json:"conflicting_paths,omitempty"`
29+
PeerCommit string `json:"peer_commit,omitempty"`
2930
Reason string `json:"reason"`
3031
}
3132

@@ -156,6 +157,13 @@ func runStateReady(ctx context.Context, args []string, env []string, dir string,
156157
if code := runStateInit(ctx, []string{"--workflow-dir", workflowDir}, env, dir, stdout, stderr); code != 0 {
157158
return code
158159
}
160+
// The re-boot-after-resume sequencing the «state.ensure-ready» prose used to
161+
// own: a just-linked checkout means the boot read the FO already did (if any)
162+
// predates the entity dir existing, so it must re-read before greeting.
163+
// Prose-only (not --json) — it is FO guidance, not part of the result envelope.
164+
if !jsonOut {
165+
fmt.Fprintln(stdout, "checkout resumed — re-run `spacedock status --boot` before the greet.")
166+
}
159167
}
160168

161169
// 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,
200208
// code is the enforcement: a caller cannot proceed on an unmerged tree.
201209
func haltOnConflict(stdout, stderr io.Writer, jsonOut bool, command, slug, branch, checkout, entityPath, rebaseOut string) int {
202210
conflicting := conflictingPaths(checkout)
211+
// The peer commit that survived: the pull's fetch phase already updated
212+
// origin/{branch} before the rebase conflicted, and --abort does not touch
213+
// that ref, so this resolves network-free (spiked in ideation).
214+
peerCommit := peerCommitSHA(checkout, branch)
203215
// Restore a clean tree so the next operation starts fresh. abort failure is
204216
// surfaced but the exit is still the halt.
205217
runGit(checkout, "rebase", "--abort")
@@ -209,14 +221,36 @@ func haltOnConflict(stdout, stderr io.Writer, jsonOut bool, command, slug, branc
209221
}
210222
fmt.Fprintf(stderr, "spacedock %s: HALT — same-entity rebase conflict on %s.\n", command, branch)
211223
fmt.Fprintf(stderr, "Conflicting path(s): %s\n", strings.Join(conflicting, ", "))
212-
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")
224+
if peerCommit != "" {
225+
fmt.Fprintf(stderr, "Peer commit: %s (origin/%s)\n", peerCommit, branch)
226+
}
227+
fmt.Fprintf(stderr, "The rebase was aborted (checkout left clean) and nothing was force-pushed; a peer's edit is preserved on origin.\n")
228+
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")
229+
fmt.Fprintf(stderr, "Never `git push --force`/`--force-with-lease`; never re-run with `-X ours`/`-X theirs`; never discard either side.\n")
213230
return emitSync(stdout, jsonOut, syncResult{
214231
Command: command, Slug: slug, Result: "halted", StateBranch: branch,
215232
ConflictingPaths: conflicting,
233+
PeerCommit: peerCommit,
216234
Reason: fmt.Sprintf("HALT: same-entity rebase conflict on %s — rebase aborted, nothing force-pushed, manual intervention required.", strings.Join(conflicting, ", ")),
217235
}, 3)
218236
}
219237

238+
// peerCommitSHA resolves the peer's pushed commit on origin/{branch} — the edit
239+
// preserved when this side's rebase conflicted. Runs BEFORE `rebase --abort`
240+
// clears the conflict, though the ref itself does not depend on abort timing (the
241+
// pull's fetch phase already updated it). Returns "" on any git failure rather
242+
// than surfacing an error for what is purely diagnostic context.
243+
func peerCommitSHA(checkout, branch string) string {
244+
if branch == "" {
245+
return ""
246+
}
247+
ok, out := runGit(checkout, "rev-parse", "--short", "origin/"+branch)
248+
if !ok {
249+
return ""
250+
}
251+
return strings.TrimSpace(out)
252+
}
253+
220254
// commitEntityPathScoped stages and commits exactly entityPath (never `add -A`),
221255
// retrying the staging on index.lock contention. It returns (true, "") for a
222256
// clean no-op (nothing staged for the entity → success), (true, output) for a

internal/dispatch/reconcile.go

Lines changed: 64 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -89,20 +89,19 @@ func GhRunnerExec(prRef string) (string, error) { return ghRunnerExec(prRef) }
8989
// passes gitRunnerExec.
9090
type gitRunner func(dir string, args ...string) (string, error)
9191

92-
// ghRunnerExec calls `gh pr view PR --json state` and extracts the state field.
92+
// ghRunnerExec calls `gh pr view PR --json state --jq .state` and returns the
93+
// trimmed, uppercased state. This mirrors boot.go's PR_STATE probe invocation
94+
// exactly (same flags, same leading-"#" trim, same --jq extraction) rather than
95+
// parsing the `--json state` envelope as a second JSON layer — the two probes
96+
// must agree on what "gh can answer" means, since they run against the same `gh`
97+
// and the FO trusts both.
9398
func ghRunnerExec(prRef string) (string, error) {
94-
cmd := exec.Command("gh", "pr", "view", prRef, "--json", "state")
95-
out, err := cmd.Output()
99+
pr := strings.TrimPrefix(prRef, "#")
100+
out, err := exec.Command("gh", "pr", "view", pr, "--json", "state", "--jq", ".state").Output()
96101
if err != nil {
97102
return "", err
98103
}
99-
var obj struct {
100-
State string `json:"state"`
101-
}
102-
if err := json.Unmarshal(out, &obj); err != nil {
103-
return "", err
104-
}
105-
return obj.State, nil
104+
return strings.ToUpper(strings.TrimSpace(string(out))), nil
106105
}
107106

108107
// gitRunnerExec runs git with the given args under -C dir and returns trimmed
@@ -666,6 +665,8 @@ type sweepResult struct {
666665
StateBranch string `json:"state_branch,omitempty"`
667666
Swept []sweptEntity `json:"swept"`
668667
Reason string `json:"reason"`
668+
Gh string `json:"gh,omitempty"`
669+
Next string `json:"next,omitempty"`
669670
}
670671

671672
// Sweep is the read-only `state sweep` computation: the entities whose code PR has
@@ -675,6 +676,12 @@ type sweepResult struct {
675676
// injected so tests pin a deterministic merged-state; production passes
676677
// GhRunnerExec. An inline (single-root) workflow sweeps its own dir. Exit 0 the
677678
// sweep ran (set may be empty or populated); 1 setup failure (no README/state).
679+
//
680+
// classC itself stays best-effort — a gh error silently skips that entity, so the
681+
// idle hook never blows up on a transient failure. Sweep wraps the injected probe
682+
// to COUNT calls and errors around that unchanged behavior: when every PR-pending
683+
// entity's probe errored, "0 entity(ies)" would be indistinguishable from a real
684+
// empty sweep, so Sweep reports merge state UNKNOWN instead (D2).
678685
func Sweep(workflowDir string, gh GhRunner, jsonOut bool, stdout, stderr io.Writer) int {
679686
if info, err := os.Stat(workflowDir); err != nil || !info.IsDir() {
680687
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
685692
stateRoot = workflowDir
686693
}
687694
active := loadEntityFrontmatter(activeEntityDir(stateRoot))
688-
drift := classC(active, gh)
695+
696+
probeTotal, probeErrs := 0, 0
697+
counting := func(prRef string) (string, error) {
698+
probeTotal++
699+
state, err := gh(prRef)
700+
if err != nil {
701+
probeErrs++
702+
}
703+
return state, err
704+
}
705+
drift := classC(active, counting)
689706

690707
swept := make([]sweptEntity, 0, len(drift))
691708
for _, d := range drift {
692709
swept = append(swept, sweptEntity{Slug: d.Slug, PR: d.PR, Reason: d.Reason})
693710
}
694711
branch, _ := status.StateBranch(workflowDir)
695-
reason := fmt.Sprintf("%d entity(ies) merged but not yet terminalized.", len(swept))
712+
713+
var reason, ghField, next string
714+
switch {
715+
case probeTotal > 0 && probeErrs == probeTotal:
716+
reason = "merge state UNKNOWN — gh unavailable; sweep skipped, not empty."
717+
ghField = "unavailable"
718+
case len(swept) > 0:
719+
reason = fmt.Sprintf("%d entity(ies) merged but not yet terminalized.", len(swept))
720+
next = sweepNextStep(workflowDir)
721+
default:
722+
reason = fmt.Sprintf("%d entity(ies) merged but not yet terminalized.", len(swept))
723+
}
696724

697725
if jsonOut {
698726
enc := json.NewEncoder(stdout)
699727
enc.SetIndent("", " ")
700728
enc.Encode(sweepResult{
701-
Command: "state sweep", StateBranch: branch, Swept: swept, Reason: reason,
729+
Command: "state sweep", StateBranch: branch, Swept: swept, Reason: reason, Gh: ghField, Next: next,
702730
})
703731
return 0
704732
}
705733
fmt.Fprintln(stdout, reason)
734+
if next != "" {
735+
fmt.Fprintln(stdout, next)
736+
}
706737
for _, s := range swept {
707738
fmt.Fprintf(stdout, " %s (PR %s): %s\n", s.Slug, s.PR, s.Reason)
708739
}
709740
return 0
710741
}
711742

743+
// sweepNextStep names the FO's next action for a non-empty sweep: the registered
744+
// startup-hook mod file(s) to advance each entity per, from the same hookPoint ->
745+
// mod-name scan the boot MODS-REPORT uses (status.ScanMods). It points at the mod
746+
// FILE, never a procedure — the shipped mods/pr-merge.md advances an entity
747+
// directly while a local per-workflow pr-merge mod can delegate to sentinel +
748+
// merge guard, and the binary has no way to know which one applies without
749+
// picking a side. No startup mod registered falls back to a generic _mods/
750+
// pointer.
751+
func sweepNextStep(workflowDir string) string {
752+
hooks := status.ScanMods(workflowDir)["startup"]
753+
if len(hooks) == 0 {
754+
return "next: advance each per the workflow's startup-hook advancement (_mods/)."
755+
}
756+
paths := make([]string, len(hooks))
757+
for i, h := range hooks {
758+
paths[i] = fmt.Sprintf("_mods/%s.md", h)
759+
}
760+
return fmt.Sprintf("next: advance each per the workflow's startup-hook advancement (%s).", strings.Join(paths, ", "))
761+
}
762+
712763
// classD flags worktrees whose branch HEAD is behind origin/{trunk}. The remedy
713764
// is ownership-gated: a `pull --rebase` is prescribed ONLY when the worktree's
714765
// entity slug is in `owned` — the set of slugs the CURRENT trusted roster's

0 commit comments

Comments
 (0)