diff options
Diffstat (limited to 'docs/superpowers')
| -rw-r--r-- | docs/superpowers/plans/2026-07-09-story-fix-and-reevaluate-loop.md | 538 |
1 files changed, 538 insertions, 0 deletions
diff --git a/docs/superpowers/plans/2026-07-09-story-fix-and-reevaluate-loop.md b/docs/superpowers/plans/2026-07-09-story-fix-and-reevaluate-loop.md new file mode 100644 index 0000000..5aae010 --- /dev/null +++ b/docs/superpowers/plans/2026-07-09-story-fix-and-reevaluate-loop.md @@ -0,0 +1,538 @@ +# Story Fix-and-Re-Evaluate Loop Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Close the recursive-story-decomposition design spec's "fix-and-re-evaluate loop has to become real" gap. Today, when `finalizeArbitration` sets a story to `NEEDS_FIX`, nothing further happens automatically — a human must notice and manually intervene. This plan makes `StoryOrchestrator` automatically spawn a new builder-role fix attempt, re-point the story at it, and let the existing Builder → Evaluators → Arbitration chain run again from scratch against the new attempt — with no new pipeline, just the existing one re-entered against a new root. + +**Architecture:** A story at status `NEEDS_FIX` is handled by a new `ensureFixAttempt` stage, checked first thing in `processStory` before the existing Builder/Evaluators/Arbitration flow. It spawns one new top-level `builder`-role task (`DependsOn: [oldRootID]`, purely for structural discoverability/audit trail — the rejected root is already `COMPLETED` so this dependency is immediately satisfied), whose instructions carry the original story spec/acceptance criteria plus the arbitration's structured rejection reasoning (read via `AgentChannel.ReportVerdict`'s `KindVerdictReported` event, the same mechanism `finalizeArbitration` already reads). It then re-points `story.RootTaskID` at the new task and resets `story.Status` to `IN_PROGRESS`. On the *next* tick, `processStory` finds the new root via the identical unmodified path every other root already takes — `ensureEvaluators`/`ensureArbitration`/`finalizeArbitration` all reused completely unchanged. Idempotency is structural (mirrors every other stage in this file): `ensureFixAttempt` looks for an existing `builder`-role dependent of the rejected root before spawning a new one, safe to call on every tick and safe across a process restart. A hardcoded `maxFixAttempts = 3` safety cap (a plain, deterministic counter — not real cost/escalation tiering, which the design spec's Non-Goals explicitly deferred) stops automatic spawning once a chain of that many consecutive fix attempts has been tried, leaving the story at `NEEDS_FIX` for a human exactly as it behaves today. + +**Tech Stack:** Go, existing `internal/scheduler`/`internal/story`/`internal/task`/`internal/event` packages — no new packages, no schema changes (`story.Status` already lists `IN_PROGRESS` as a valid value; `story.RootTaskID` is already an unvalidated plain string field). + +## Global Constraints + +- The mechanism to re-point a story at a new attempt is **re-pointing `story.RootTaskID`** (not spawning the fix task as a dependent of the original root while leaving `RootTaskID` fixed forever) — this was an explicit design decision, made after comparing both approaches: re-pointing keeps every existing root-task-id-keyed structural idempotency check (`ensureEvaluators`, `ensureArbitration`) working unmodified against the new attempt, with no "find the latest attempt in the chain" logic needed anywhere. +- `maxFixAttempts = 3` is a **hardcoded safety net, not a designed cost/escalation policy** — the design spec's Non-Goals section explicitly deferred real depth/cost bounds to future work tied to cheap-model tiering. This plan's cap exists only to prevent a literal unbounded spend loop; it is not meant to be a tuned value. +- Do not touch `internal/api` (chatbot/REST MCP surface), the `builder` role's system prompt, or anything about per-node arbitrated review at depths below a story's root. Those remain separately deferred pieces of the recursive-story-decomposition design. + +--- + +## Task 1: Automatic fix-attempt spawning on `NEEDS_FIX` + +**Files:** +- Modify: `internal/scheduler/story_orchestrator.go` (`processStory`, `finalizeArbitration`, `arbitrationVerdict`; new `ensureFixAttempt`, `rejectionReasoning`, `fixAttemptDepth`, `maxFixAttempts`) +- Test: `internal/scheduler/story_orchestrator_test.go` (append) + +**Interfaces:** +- Consumes: `event.KindVerdictReported` (existing), `StoryStore.ListEvents`/`ListDependents`/`CreateTask`/`UpdateTaskState`/`UpdateStory` (all existing, already used elsewhere in this file), `spawnRoleTask`/`findEvaluators`/`findArbitration` (existing helpers in this same file). +- Produces: nothing new for later work — this closes the last "documented simplification" this plan set out to close. + +- [ ] **Step 1: Write the failing tests** + +Append to `internal/scheduler/story_orchestrator_test.go`, directly after `TestStoryOrchestrator_ArbitrationApproves_SetsReviewReady` (the last test before `TestStoryOrchestrator_DoesNothing_WhenBuilderNotComplete`): + +```go +// seedNeedsFixStory drives a story through the full Builder -> Evaluators -> +// Arbitration chain to a rejected verdict (mirrors +// TestStoryOrchestrator_ArbitrationRejects_SetsStoryNeedsFix's own setup), +// leaving it at status NEEDS_FIX with its original (rejected) root task +// still in the store -- the starting point ensureFixAttempt operates on. +// Returns the store, the story (as currently persisted), the rejected root +// task, the evaluators, and the arbitration task. +func seedNeedsFixStory(t *testing.T) (*fakeStoryStore, *story.Story, *task.Task, []*task.Task, *task.Task) { + t.Helper() + store, st, evaluators := seedStoryWithEvaluators(t, task.StateCompleted) + oldRoot, err := store.GetTask(st.RootTaskID) + if err != nil { + t.Fatalf("get root: %v", err) + } + + pool := &fakePool{} + orch := &StoryOrchestrator{Store: store, Pool: pool} + orch.Tick(context.Background()) // spawns arbitration + + arbitrations := store.dependentsWithRole(evaluators[0].ID, "planner") + if len(arbitrations) != 1 { + t.Fatalf("expected 1 arbitration task, got %d", len(arbitrations)) + } + arb := arbitrations[0] + + payload, _ := json.Marshal(struct { + Approved bool `json:"approved"` + Reasoning string `json:"reasoning"` + }{Approved: false, Reasoning: "breaks the running-log panel styling"}) + if err := store.CreateEvent(&event.Event{ + TaskID: arb.ID, + Kind: event.KindVerdictReported, + Actor: event.ActorAgent, + Payload: payload, + }); err != nil { + t.Fatalf("seed verdict event: %v", err) + } + store.setTaskState(arb.ID, task.StateCompleted) + store.setTaskSummary(arb.ID, "found a real problem") + + orch.Tick(context.Background()) // finalizeArbitration runs, sets NEEDS_FIX + + stories, _ := store.ListStories(storage.StoryFilter{}) + var got *story.Story + for _, s := range stories { + if s.ID == st.ID { + got = s + } + } + if got == nil || got.Status != "NEEDS_FIX" { + t.Fatalf("setup failed: story = %+v, want status NEEDS_FIX", got) + } + return store, got, oldRoot, evaluators, arb +} + +// TestStoryOrchestrator_NeedsFix_SpawnsFixAttemptAndRepointsRoot proves the +// core mechanism: a story at NEEDS_FIX gets a new builder-role fix-attempt +// task spawned (depending on the rejected root), and the story is re-pointed +// at it (RootTaskID updated, Status reset to IN_PROGRESS) so the very next +// tick re-enters the normal Builder->Evaluators->Arbitration flow against +// the new attempt. +func TestStoryOrchestrator_NeedsFix_SpawnsFixAttemptAndRepointsRoot(t *testing.T) { + store, st, oldRoot, _, _ := seedNeedsFixStory(t) + + pool := &fakePool{} + orch := &StoryOrchestrator{Store: store, Pool: pool} + orch.Tick(context.Background()) + + fixAttempts := store.dependentsWithRole(oldRoot.ID, "builder") + if len(fixAttempts) != 1 { + t.Fatalf("expected exactly 1 fix-attempt task depending on the rejected root, got %d", len(fixAttempts)) + } + fix := fixAttempts[0] + if fix.State != task.StateQueued { + t.Errorf("fix attempt State = %v, want QUEUED", fix.State) + } + if fix.ParentTaskID != "" { + t.Errorf("fix attempt ParentTaskID = %q, want empty (top-level, DAG sibling not delegated subtask)", fix.ParentTaskID) + } + + stories, _ := store.ListStories(storage.StoryFilter{}) + var got *story.Story + for _, s := range stories { + if s.ID == st.ID { + got = s + } + } + if got == nil { + t.Fatal("story not found") + } + if got.RootTaskID != fix.ID { + t.Errorf("story RootTaskID = %q, want the new fix-attempt task %q", got.RootTaskID, fix.ID) + } + if got.Status != "IN_PROGRESS" { + t.Errorf("story Status = %q, want IN_PROGRESS", got.Status) + } +} + +// TestStoryOrchestrator_NeedsFix_FixAttemptInstructionsIncludeRejectionReasoning +// proves the fix attempt's instructions carry the arbitration's structured +// rejection reasoning (read via the KindVerdictReported event), not just a +// generic "try again" -- the whole point of automating this loop is that the +// next attempt has something concrete to act on. +func TestStoryOrchestrator_NeedsFix_FixAttemptInstructionsIncludeRejectionReasoning(t *testing.T) { + store, _, oldRoot, _, _ := seedNeedsFixStory(t) + + pool := &fakePool{} + orch := &StoryOrchestrator{Store: store, Pool: pool} + orch.Tick(context.Background()) + + fixAttempts := store.dependentsWithRole(oldRoot.ID, "builder") + if len(fixAttempts) != 1 { + t.Fatalf("expected exactly 1 fix-attempt task, got %d", len(fixAttempts)) + } + instructions := fixAttempts[0].Agent.Instructions + if !strings.Contains(instructions, "breaks the running-log panel styling") { + t.Errorf("fix-attempt instructions do not include the arbitration's rejection reasoning:\n%s", instructions) + } + if !strings.Contains(instructions, "Test Story") { + t.Errorf("fix-attempt instructions do not include the story name:\n%s", instructions) + } +} + +// TestStoryOrchestrator_NeedsFix_IdempotentAcrossTicks proves repeated ticks +// don't spawn more than one fix-attempt task -- after the first tick the +// story's status flips to IN_PROGRESS, so the NEEDS_FIX branch simply isn't +// re-entered on subsequent ticks (the normal Builder-not-complete-yet path +// takes over instead, since the fix attempt is freshly QUEUED). +func TestStoryOrchestrator_NeedsFix_IdempotentAcrossTicks(t *testing.T) { + store, _, oldRoot, _, _ := seedNeedsFixStory(t) + + pool := &fakePool{} + orch := &StoryOrchestrator{Store: store, Pool: pool} + orch.Tick(context.Background()) + orch.Tick(context.Background()) + orch.Tick(context.Background()) + + fixAttempts := store.dependentsWithRole(oldRoot.ID, "builder") + if len(fixAttempts) != 1 { + t.Fatalf("expected exactly 1 fix-attempt task after repeated ticks, got %d", len(fixAttempts)) + } +} + +// TestStoryOrchestrator_EnsureFixAttempt_FindsExistingSpawnedAttempt proves +// ensureFixAttempt's own structural idempotency: simulating a restart +// between "fix task spawned" and "story RootTaskID/Status updated to point +// at it" -- calling ensureFixAttempt again must find the already-spawned +// builder-role dependent rather than spawning a second one, and still +// complete the re-pointing. +func TestStoryOrchestrator_EnsureFixAttempt_FindsExistingSpawnedAttempt(t *testing.T) { + store, st, oldRoot, _, _ := seedNeedsFixStory(t) + + pool := &fakePool{} + orch := &StoryOrchestrator{Store: store, Pool: pool} + + preexisting, err := orch.spawnRoleTask(context.Background(), "Fix attempt: pre-existing", "builder", []string{oldRoot.ID}, oldRoot, "fix it") + if err != nil { + t.Fatalf("seed pre-existing fix attempt: %v", err) + } + + orch.ensureFixAttempt(context.Background(), st) + + fixAttempts := store.dependentsWithRole(oldRoot.ID, "builder") + if len(fixAttempts) != 1 { + t.Fatalf("expected exactly 1 fix-attempt task (the pre-existing one, not a new one), got %d", len(fixAttempts)) + } + if fixAttempts[0].ID != preexisting.ID { + t.Errorf("expected ensureFixAttempt to reuse the pre-existing task %s, got a different one %s", preexisting.ID, fixAttempts[0].ID) + } + if st.RootTaskID != preexisting.ID { + t.Errorf("story RootTaskID = %q, want the pre-existing fix attempt %q", st.RootTaskID, preexisting.ID) + } + if st.Status != "IN_PROGRESS" { + t.Errorf("story Status = %q, want IN_PROGRESS", st.Status) + } +} + +// TestStoryOrchestrator_NeedsFix_CapsAtMaxFixAttempts proves the safety net: +// once a chain of maxFixAttempts consecutive fix attempts already exists, +// ensureFixAttempt stops spawning new ones and leaves the story at +// NEEDS_FIX, exactly like today's fully-manual behavior. +func TestStoryOrchestrator_NeedsFix_CapsAtMaxFixAttempts(t *testing.T) { + store, st, oldRoot, _, _ := seedNeedsFixStory(t) + + pool := &fakePool{} + orch := &StoryOrchestrator{Store: store, Pool: pool} + + // Build a chain of maxFixAttempts prior fix-attempt tasks, each + // depending on the one before it, ending at oldRoot -- simulating a + // story that has already exhausted its automatic fix attempts. + prev := oldRoot + for i := 0; i < maxFixAttempts; i++ { + nt, err := orch.spawnRoleTask(context.Background(), fmt.Sprintf("Fix attempt %d", i), "builder", []string{prev.ID}, oldRoot, "fix it") + if err != nil { + t.Fatalf("seed fix-attempt chain: %v", err) + } + prev = nt + } + st.RootTaskID = prev.ID + if err := store.UpdateStory(st); err != nil { + t.Fatalf("persist chained root: %v", err) + } + + orch.Tick(context.Background()) + + fixAttempts := store.dependentsWithRole(prev.ID, "builder") + if len(fixAttempts) != 0 { + t.Fatalf("expected no new fix-attempt task once maxFixAttempts is reached, got %d", len(fixAttempts)) + } + + stories, _ := store.ListStories(storage.StoryFilter{}) + var got *story.Story + for _, s := range stories { + if s.ID == st.ID { + got = s + } + } + if got.Status != "NEEDS_FIX" { + t.Errorf("story Status = %q, want NEEDS_FIX (cap reached, no further automation)", got.Status) + } +} +``` + +`internal/scheduler/story_orchestrator_test.go` already imports `"strings"` (used elsewhere in this file) — no import changes are needed for this task. + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `go test ./internal/scheduler/ -run 'TestStoryOrchestrator_(NeedsFix|EnsureFixAttempt)' -v` +Expected: compile failure — `ensureFixAttempt`, `fixAttemptDepth`, and `maxFixAttempts` don't exist yet. Paste the actual output. + +- [ ] **Step 3: Extend `arbitrationVerdict` to also return the rejection reasoning** + +In `internal/scheduler/story_orchestrator.go`, find `arbitrationVerdict`: + +```go +// arbitrationVerdict reads the arbitration task's own event stream for a +// KindVerdictReported event (AgentChannel.ReportVerdict) and returns its +// approved value. hasVerdict is false if no such event was ever recorded — +// callers must treat that as "no structured verdict was reported", not as +// an implicit rejection. +func (o *StoryOrchestrator) arbitrationVerdict(st *story.Story, arbitration *task.Task) (approved bool, hasVerdict bool) { + events, err := o.Store.ListEvents(arbitration.ID, 0) + if err != nil { + o.logf("story orchestrator: list arbitration task events", "storyID", st.ID, "taskID", arbitration.ID, "error", err) + return false, false + } + for _, e := range events { + if e.Kind != event.KindVerdictReported { + continue + } + var payload struct { + Approved bool `json:"approved"` + } + if json.Unmarshal(e.Payload, &payload) == nil { + approved = payload.Approved + hasVerdict = true + } + } + return approved, hasVerdict +} +``` + +Replace it with: + +```go +// arbitrationVerdict reads the arbitration task's own event stream for a +// KindVerdictReported event (AgentChannel.ReportVerdict) and returns its +// approved value and reasoning text. hasVerdict is false if no such event was +// ever recorded — callers must treat that as "no structured verdict was +// reported", not as an implicit rejection. reasoning feeds ensureFixAttempt's +// automatic fix-attempt instructions; finalizeArbitration itself only needs +// approved/hasVerdict and discards reasoning. +func (o *StoryOrchestrator) arbitrationVerdict(st *story.Story, arbitration *task.Task) (approved bool, reasoning string, hasVerdict bool) { + events, err := o.Store.ListEvents(arbitration.ID, 0) + if err != nil { + o.logf("story orchestrator: list arbitration task events", "storyID", st.ID, "taskID", arbitration.ID, "error", err) + return false, "", false + } + for _, e := range events { + if e.Kind != event.KindVerdictReported { + continue + } + var payload struct { + Approved bool `json:"approved"` + Reasoning string `json:"reasoning"` + } + if json.Unmarshal(e.Payload, &payload) == nil { + approved = payload.Approved + reasoning = payload.Reasoning + hasVerdict = true + } + } + return approved, reasoning, hasVerdict +} +``` + +- [ ] **Step 4: Update `finalizeArbitration`'s call site** + +In the same file, find this line inside `finalizeArbitration`: + +```go + approved, hasVerdict := o.arbitrationVerdict(st, arbitration) +``` + +Replace it with: + +```go + approved, _, hasVerdict := o.arbitrationVerdict(st, arbitration) +``` + +- [ ] **Step 5: Add the `NEEDS_FIX` branch to `processStory`** + +In the same file, find the start of `processStory`: + +```go +func (o *StoryOrchestrator) processStory(ctx context.Context, st *story.Story) { + root, err := o.Store.GetTask(st.RootTaskID) +``` + +Replace it with: + +```go +func (o *StoryOrchestrator) processStory(ctx context.Context, st *story.Story) { + if st.Status == "NEEDS_FIX" { + o.ensureFixAttempt(ctx, st) + return + } + + root, err := o.Store.GetTask(st.RootTaskID) +``` + +- [ ] **Step 6: Add `maxFixAttempts`, `ensureFixAttempt`, `rejectionReasoning`, and `fixAttemptDepth`** + +In the same file, find the `optionalBool` function (it ends right before `processRetro`'s doc comment): + +```go +// optionalBool returns a pointer to v if present is true, nil otherwise — +// used so KindArbitrationDecided's payload omits "approved" entirely +// (rather than encoding a misleading false) when no structured verdict was +// ever reported. +func optionalBool(v bool, present bool) *bool { + if !present { + return nil + } + return &v +} +``` + +Directly after it (still before `processRetro`'s doc comment), add: + +```go +// maxFixAttempts caps automatic fix-attempt spawning (see ensureFixAttempt) +// to prevent an unbounded loop if arbitration keeps rejecting a story's +// work. This is a simple, hardcoded safety net, not real cost/escalation +// tiering — the design spec's Non-Goals explicitly deferred "no depth or +// cost bound in this design... left as a distinct future piece of work". +// Once hit, the story stays at NEEDS_FIX exactly like it does today, for a +// human to intervene manually via PUT /api/stories/{id}. +const maxFixAttempts = 3 + +// ensureFixAttempt handles a story sitting at NEEDS_FIX: it spawns one new +// top-level builder-role task depending on the rejected root (purely for +// structural discoverability/audit trail — the rejected root is already +// COMPLETED, so this dependency is immediately satisfied), whose +// instructions carry the original story spec/acceptance criteria plus the +// arbitration's rejection reasoning, then re-points st.RootTaskID at the new +// task and resets st.Status to IN_PROGRESS. The very next tick re-enters +// processStory's normal Builder->Evaluators->Arbitration flow against the +// new root, completely unchanged — no new pipeline, the existing one +// re-entered. +// +// Idempotency is structural, mirroring every other stage in this file: it +// looks for an existing builder-role dependent of the rejected root before +// spawning a new one, so calling this repeatedly (or after a restart between +// "task spawned" and "story updated") never spawns duplicates. The +// maxFixAttempts cap is only checked in the "spawn a new one" branch — a +// dependent that was already committed to being spawned still gets +// re-pointed to, regardless of the cap, since refusing to do so would leave +// a task dangling with nothing tracking it. +func (o *StoryOrchestrator) ensureFixAttempt(ctx context.Context, st *story.Story) { + oldRoot, err := o.Store.GetTask(st.RootTaskID) + if err != nil { + o.logf("story orchestrator: fix attempt: get rejected root task", "storyID", st.ID, "rootTaskID", st.RootTaskID, "error", err) + return + } + + dependents, err := o.Store.ListDependents(oldRoot.ID) + if err != nil { + o.logf("story orchestrator: fix attempt: list root dependents", "storyID", st.ID, "error", err) + return + } + var fix *task.Task + for _, d := range dependents { + if d.Agent.Role == "builder" { + fix = d + break + } + } + + if fix == nil { + if depth := o.fixAttemptDepth(oldRoot); depth >= maxFixAttempts { + o.logf("story orchestrator: fix attempt: max fix attempts reached, leaving story at NEEDS_FIX for a human", "storyID", st.ID, "depth", depth) + return + } + reasoning := o.rejectionReasoning(st, oldRoot) + instructions := fmt.Sprintf( + "A previous attempt at story %q (task %s) was rejected by arbitration. Fix the issues and resubmit.\n\n"+ + "Story spec:\n%s\n\nAcceptance criteria:\n%s\n\nArbitration's rejection reasoning:\n%s", + st.Name, oldRoot.ID, st.Spec, formatAcceptanceCriteria(st.AcceptanceCriteria), reasoning) + nt, err := o.spawnRoleTask(ctx, "Fix attempt: "+st.Name, "builder", []string{oldRoot.ID}, oldRoot, instructions) + if err != nil { + o.logf("story orchestrator: fix attempt: spawn", "storyID", st.ID, "error", err) + return + } + fix = nt + } + + st.RootTaskID = fix.ID + st.Status = "IN_PROGRESS" + if err := o.Store.UpdateStory(st); err != nil { + o.logf("story orchestrator: fix attempt: repoint story root", "storyID", st.ID, "error", err) + } +} + +// rejectionReasoning finds the rejected root's evaluator/arbitration chain +// (read-only, via findEvaluators/findArbitration — the same lookups +// processRetro uses) and returns the arbitration task's structured +// KindVerdictReported reasoning, falling back to its plain-text Summary if no +// structured verdict was reported, or a placeholder if neither is available. +func (o *StoryOrchestrator) rejectionReasoning(st *story.Story, oldRoot *task.Task) string { + evaluators, ok := o.findEvaluators(oldRoot.ID) + if !ok { + return "(no arbitration reasoning found)" + } + arbitration, ok := o.findArbitration(evaluators) + if !ok { + return "(no arbitration reasoning found)" + } + _, reasoning, hasVerdict := o.arbitrationVerdict(st, arbitration) + if hasVerdict && reasoning != "" { + return reasoning + } + if arbitration.Summary != "" { + return arbitration.Summary + } + return "(no arbitration reasoning found)" +} + +// fixAttemptDepth walks backward through builder-role tasks connected by a +// single DependsOn edge — the exact shape ensureFixAttempt creates — counting +// how many fix attempts precede root. Stops as soon as it finds a task with +// anything other than exactly one DependsOn entry (the original, non-fix- +// attempt root, which was created by whatever process first submitted the +// story). Bounded by maxFixAttempts+1 iterations since callers only care +// whether the cap is hit, not the exact depth beyond that. +func (o *StoryOrchestrator) fixAttemptDepth(root *task.Task) int { + depth := 0 + current := root + for depth <= maxFixAttempts { + if len(current.DependsOn) != 1 { + break + } + prev, err := o.Store.GetTask(current.DependsOn[0]) + if err != nil { + break + } + depth++ + current = prev + } + return depth +} +``` + +- [ ] **Step 7: Run tests to verify they pass** + +Run: `go test ./internal/scheduler/ -run 'TestStoryOrchestrator_(NeedsFix|EnsureFixAttempt|Arbitration)' -v` +Expected: PASS, all of them — the new tests, plus the pre-existing `TestStoryOrchestrator_ArbitrationRejects_SetsStoryNeedsFix`, `TestStoryOrchestrator_ArbitrationApproves_SetsReviewReady`, and `TestStoryOrchestrator_ArbitrationCompletes_EmitsDecisionAndReviewReady` (unmodified, must still pass — they only exercise `arbitrationVerdict` indirectly through `finalizeArbitration`, which still compiles and behaves the same after Step 4's one-line signature-adjustment). Paste the actual output. + +- [ ] **Step 8: Run the full package suite, then the full repo suite** + +Run: `go test ./internal/scheduler/...` +Expected: PASS for everything. + +Then run: `go test ./...` (the entire repo). This must also pass — a prior plan in this repo found that task-scoped tests passing is not sufficient on its own; grep the repo for any other caller of `arbitrationVerdict` before assuming none exists (`grep -rn "arbitrationVerdict" internal/`) — it should only be called from within `story_orchestrator.go` itself (by `finalizeArbitration` and the new `rejectionReasoning`), so Step 4's signature change should not require touching any other file. Paste the actual full-repo test output. + +Note: if you see a transient one-off failure in `internal/api` under the race detector mentioning "sql: database is closed" during an unrelated webhook/websocket test's teardown, that is a known pre-existing flake unrelated to this change — simply rerun `go test ./...` once to confirm it doesn't reproduce, and proceed if it passes clean on rerun. If it fails a second time in a way connected to your actual changes, stop and call ask_user. + +- [ ] **Step 9: Commit and push to `main` directly, matching this repo's existing workflow** + +```bash +git add internal/scheduler/story_orchestrator.go internal/scheduler/story_orchestrator_test.go +git commit -m "feat(scheduler): automatically spawn and re-point a fix attempt when a story is rejected" +``` + +## Mandatory verification disclosure + +When you call report_summary, paste the actual terminal output of every test command above — literal pass/fail counts, not a claim of success, including the full-repo `go test ./...` run. If anything here is ambiguous or conflicts with what you find in the actual repo, call ask_user and describe the specific conflict — don't guess or silently decide. + +--- + +## Final Verification + +- [ ] Run `go build ./...` — passes. +- [ ] Run `go test ./...` — passes, full repo. +- [ ] Run `grep -rn "ensureFixAttempt\|fixAttemptDepth\|rejectionReasoning\|maxFixAttempts" internal/` to visually confirm the File Map was actually touched (should all be in `internal/scheduler/story_orchestrator.go` and its test file only). |