# Root Completion Follows Arbitration 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:** A story's root (builder-role) task only transitions to `COMPLETED` once its own arbitration approves it — never eagerly the moment it reaches `READY`. Rejection leaves it at `READY` forever (superseded; `task.CurrentAttempt` resolves forward to whatever fix attempt gets spawned).

**Architecture:** `StoryOrchestrator.processStory` stops calling `autoAccept` on the root task. `finalizeArbitration` gains a `root *task.Task` parameter and, on approval, is the sole place that ever promotes root to `COMPLETED`; on rejection it leaves root untouched. `READY` already satisfies a dependent's `DependsOn` (`executor.depDoneStates` includes both `StateCompleted` and `StateReady`), so a rejected-but-READY root is still immediately dispatchable as the anchor for `ensureFixAttempt`'s fix-attempt task — no change needed there. Evaluator and Arbitration tasks are not `builder`-role — they are the review mechanism itself, not something reviewed — so they keep their existing immediate-auto-accept behavior unchanged.

**Tech Stack:** Go, existing `internal/scheduler` package, `internal/task`.

## Global Constraints

- This is piece 4a of `docs/superpowers/specs/2026-07-09-recursive-arbitrated-review-design.md`'s piece 4 ("Generalize `ensureEvaluators`/`ensureArbitration`/the fix-loop trigger to recurse into `builder`-role subtask trees"), split out as the smaller, safer first step: fixing the root's own completion semantics before generalizing the trigger to walk subtask trees (piece 4b, a separate future plan).
- Root cause this plan closes: today, `task.State == COMPLETED` for the story root means only "the agent's own execution finished" — arbitration is a side-annotation on `story.Status`, never touching the task's own state. That's tolerable only because `Story` has a separate status field to carry "has this been verified" independently of any task's raw state. A nested subtask in piece 4b will have no such separate wrapper — the only signal its parent's `maybeUnblockParent` can check is the subtask's own `task.State`. So `COMPLETED` must mean "verified by arbitration", uniformly, starting here at the root, before piece 4b can safely reuse the same mechanism one level down.
- No change to evaluator/arbitration/retro task auto-accept behavior — only the root's own completion semantics change.
- No change to `ensureFixAttempt`, `fixAttemptDepth`, `ensureEvaluators`, `ensureArbitration`, `rejectionReasoning`, `findEvaluators`, `findArbitration`, `taskTree`, `processRetro`, `autoAccept` itself, `maybeEmitVerdict`, `spawnRoleTask`, `dependsOnAll`, `formatAcceptanceCriteria` — verify by running the full existing test suite for this file and confirming these functions' own tests are unaffected (some of their *callers'* test fixtures do change — see Task 1 below).

---

### Task 1: Root only reaches COMPLETED via arbitration approval

**Files:**
- Modify: `internal/scheduler/story_orchestrator.go` (`processStory`, `finalizeArbitration`)
- Modify: `internal/scheduler/story_orchestrator_test.go` (fixture + several tests, detailed below)

**Interfaces:**
- Consumes: `task.CurrentAttempt` (unchanged), `o.autoAccept` (unchanged, still used for evaluators/arbitration), `o.ensureEvaluators`/`o.ensureArbitration` (unchanged), `o.arbitrationVerdict` (unchanged), `executor.depDoneStates` (unchanged, already includes both `StateCompleted` and `StateReady` — confirmed by reading `internal/executor/executor.go`, no change needed there).
- Produces: `finalizeArbitration`'s new signature `func (o *StoryOrchestrator) finalizeArbitration(st *story.Story, root, arbitration *task.Task)` — note the added `root` parameter, second positional argument.

- [ ] **Step 1: Update `processStory` to stop auto-accepting root**

In `internal/scheduler/story_orchestrator.go`, find:

```go
func (o *StoryOrchestrator) processStory(ctx context.Context, st *story.Story) {
	if st.Status == "NEEDS_FIX" {
		o.ensureFixAttempt(ctx, st)
		return
	}

	root, err := task.CurrentAttempt(o.Store, st.RootTaskID)
	if err != nil {
		o.logf("story orchestrator: resolve current root attempt", "storyID", st.ID, "rootTaskID", st.RootTaskID, "error", err)
		return
	}
	root = o.autoAccept(st, root)
	if root.State != task.StateCompleted {
		return // Builder hasn't reached COMPLETED yet (still running, or not yet READY to auto-accept)
	}

	// Stage 1: Builder -> Evaluators (+ story -> VALIDATING).
	evaluators, ok := o.ensureEvaluators(ctx, st, root)
	if !ok {
		return // not all 4 could be found/created yet; retry next tick
	}
```

Replace with:

```go
func (o *StoryOrchestrator) processStory(ctx context.Context, st *story.Story) {
	if st.Status == "NEEDS_FIX" {
		o.ensureFixAttempt(ctx, st)
		return
	}

	root, err := task.CurrentAttempt(o.Store, st.RootTaskID)
	if err != nil {
		o.logf("story orchestrator: resolve current root attempt", "storyID", st.ID, "rootTaskID", st.RootTaskID, "error", err)
		return
	}
	// root is NOT auto-accepted here. Unlike evaluator/arbitration tasks,
	// root is builder-role: its own completion must mean "verified by
	// arbitration", not merely "the agent finished" -- so it stays READY
	// through the whole Evaluators/Arbitration cycle below, and only
	// finalizeArbitration's approval branch ever promotes it to COMPLETED.
	// READY already satisfies a dependent's DependsOn (see
	// executor.depDoneStates), so evaluators/arbitration can depend on a
	// still-READY root without issue.
	if root.State != task.StateReady {
		return // not yet finished (still running/blocked); or already COMPLETED, meaning a prior tick's finalizeArbitration already approved it -- nothing further to do
	}

	// Stage 1: Builder -> Evaluators (+ story -> VALIDATING).
	evaluators, ok := o.ensureEvaluators(ctx, st, root)
	if !ok {
		return // not all 4 could be found/created yet; retry next tick
	}
```

- [ ] **Step 2: Update the Stage 3 call site to pass `root` into `finalizeArbitration`**

In the same file, immediately below (still inside `processStory`), find:

```go
	// Stage 3: Arbitration -> REVIEW_READY.
	if arbitration.State == task.StateCompleted {
		o.finalizeArbitration(st, arbitration)
	}
}
```

Replace with:

```go
	// Stage 3: Arbitration -> approve (root: READY -> COMPLETED, story ->
	// REVIEW_READY) or reject (story -> NEEDS_FIX; a fix attempt is spawned
	// next tick by ensureFixAttempt, depending on the still-READY root --
	// task.CurrentAttempt resolves future lookups of this position forward
	// to it).
	if arbitration.State == task.StateCompleted {
		o.finalizeArbitration(st, root, arbitration)
	}
}
```

- [ ] **Step 3: Update `finalizeArbitration` to take `root` and promote it on approval**

In the same file, find the full function (including its doc comment):

```go
// finalizeArbitration handles the Arbitration task reaching COMPLETED: it
// emits KindArbitrationDecided and moves the story to REVIEW_READY or
// NEEDS_FIX, depending on whether the arbitration task recorded a structured
// KindVerdictReported event (AgentChannel.ReportVerdict). An arbitration
// task that never calls report_verdict defaults to REVIEW_READY, preserving
// the prior behavior for agents that don't report a structured verdict.
//
// Gated on st.Status == "VALIDATING" so repeated ticks (or a story a human
// already advanced past REVIEW_READY) don't re-emit the event or re-write the
// status — this is the one place in the orchestrator where the story's own
// status field, not a structural dependents check, is the idempotency guard,
// because by this stage there's nothing further to check structurally: the
// Arbitration task is the last task in the chain, so "does a subsequent task
// exist" isn't an available signal.
func (o *StoryOrchestrator) finalizeArbitration(st *story.Story, arbitration *task.Task) {
	if st.Status != "VALIDATING" {
		return
	}

	approved, _, hasVerdict := o.arbitrationVerdict(st, arbitration)

	payload, _ := json.Marshal(struct {
		TaskID   string `json:"task_id"`
		Summary  string `json:"summary"`
		Approved *bool  `json:"approved,omitempty"`
	}{TaskID: arbitration.ID, Summary: arbitration.Summary, Approved: optionalBool(approved, hasVerdict)})
	if err := o.Store.CreateEvent(&event.Event{
		TaskID:  st.ID,
		Kind:    event.KindArbitrationDecided,
		Actor:   event.ActorSystem,
		Payload: payload,
	}); err != nil {
		o.logf("story orchestrator: emit arbitration_decided", "storyID", st.ID, "error", err)
	}

	// Documented simplification (Phase 7b) closed: a structured
	// KindVerdictReported event (AgentChannel.ReportVerdict) on the
	// arbitration task, if present, now decides REVIEW_READY vs NEEDS_FIX.
	// An arbitration task that never calls report_verdict (hasVerdict ==
	// false) preserves the prior unconditional REVIEW_READY behavior — a
	// human or chatbot can still manually set NEEDS_FIX via the existing
	// PUT /api/stories/{id} for a story arbitrated by an agent that doesn't
	// report a structured verdict.
	if hasVerdict && !approved {
		st.Status = "NEEDS_FIX"
	} else {
		st.Status = "REVIEW_READY"
	}
	if err := o.Store.UpdateStory(st); err != nil {
		o.logf("story orchestrator: update story status after arbitration", "storyID", st.ID, "status", st.Status, "error", err)
	}
}
```

Replace with:

```go
// finalizeArbitration handles the Arbitration task reaching COMPLETED: it
// emits KindArbitrationDecided, then either approves or rejects root based on
// whether the arbitration task recorded a structured KindVerdictReported
// event (AgentChannel.ReportVerdict). An arbitration task that never calls
// report_verdict defaults to approval, preserving the prior behavior for
// agents that don't report a structured verdict.
//
// On approval, root is promoted READY -> COMPLETED *here* — not eagerly the
// moment it first reached READY — so COMPLETED means "verified by
// arbitration", uniformly, the same rule this design applies at every depth
// of a builder-role task tree (see
// docs/superpowers/specs/2026-07-09-recursive-arbitrated-review-design.md).
// On rejection, root is left untouched at READY: it has been superseded, and
// task.CurrentAttempt will resolve future lookups of this position forward
// to whatever fix-attempt task ensureFixAttempt spawns once the story lands
// at NEEDS_FIX below — READY already satisfies a dependent's DependsOn (see
// executor.depDoneStates), so the fix attempt is immediately dispatchable
// without root ever becoming COMPLETED.
//
// Gated on st.Status == "VALIDATING" so repeated ticks (or a story a human
// already advanced past REVIEW_READY) don't re-emit the event, re-promote
// root, or re-write the status — this is the one place in the orchestrator
// where the story's own status field, not a structural dependents check, is
// the idempotency guard, because by this stage there's nothing further to
// check structurally: the Arbitration task is the last task in the chain, so
// "does a subsequent task exist" isn't an available signal.
func (o *StoryOrchestrator) finalizeArbitration(st *story.Story, root, arbitration *task.Task) {
	if st.Status != "VALIDATING" {
		return
	}

	approved, _, hasVerdict := o.arbitrationVerdict(st, arbitration)

	payload, _ := json.Marshal(struct {
		TaskID   string `json:"task_id"`
		Summary  string `json:"summary"`
		Approved *bool  `json:"approved,omitempty"`
	}{TaskID: arbitration.ID, Summary: arbitration.Summary, Approved: optionalBool(approved, hasVerdict)})
	if err := o.Store.CreateEvent(&event.Event{
		TaskID:  st.ID,
		Kind:    event.KindArbitrationDecided,
		Actor:   event.ActorSystem,
		Payload: payload,
	}); err != nil {
		o.logf("story orchestrator: emit arbitration_decided", "storyID", st.ID, "error", err)
	}

	if hasVerdict && !approved {
		st.Status = "NEEDS_FIX"
	} else {
		// Approved (or no structured verdict reported -- preserves the
		// prior unconditional-approve default). root only becomes COMPLETED
		// here, after arbitration.
		if err := o.Store.UpdateTaskState(root.ID, task.StateCompleted); err != nil {
			o.logf("story orchestrator: finalize arbitration: promote root to completed", "storyID", st.ID, "taskID", root.ID, "error", err)
			return
		}
		st.Status = "REVIEW_READY"
	}
	if err := o.Store.UpdateStory(st); err != nil {
		o.logf("story orchestrator: update story status after arbitration", "storyID", st.ID, "status", st.Status, "error", err)
	}
}
```

- [ ] **Step 4: Update `seedStoryWithEvaluators` to seed root at READY, not COMPLETED**

In `internal/scheduler/story_orchestrator_test.go`, find:

```go
func seedStoryWithEvaluators(t *testing.T, evalState task.State) (*fakeStoryStore, *story.Story, []*task.Task) {
	t.Helper()
	store := newFakeStoryStore()
	root := builderTask("builder-1", task.StateCompleted)
	store.tasks[root.ID] = root
	st := newStoryWithRoot("story-1", root.ID, "VALIDATING")
	store.stories[st.ID] = st
```

Replace with:

```go
func seedStoryWithEvaluators(t *testing.T, evalState task.State) (*fakeStoryStore, *story.Story, []*task.Task) {
	t.Helper()
	store := newFakeStoryStore()
	// root is READY, not COMPLETED: under the new rule, root only reaches
	// COMPLETED once its own arbitration approves it (see
	// finalizeArbitration), and every caller of this helper is simulating a
	// point in the pipeline before that approval has happened yet.
	// seedDoneStory (below) explicitly promotes root to COMPLETED itself for
	// its own already-approved narrative.
	root := builderTask("builder-1", task.StateReady)
	store.tasks[root.ID] = root
	st := newStoryWithRoot("story-1", root.ID, "VALIDATING")
	store.stories[st.ID] = st
```

- [ ] **Step 5: Update `seedDoneStory` to explicitly promote root to COMPLETED**

In the same file, find:

```go
func seedDoneStory(t *testing.T) (*fakeStoryStore, *story.Story, *task.Task, []*task.Task, *task.Task) {
	t.Helper()
	store, st, evaluators := seedStoryWithEvaluators(t, task.StateCompleted)
	st.Status = "DONE"

	ids := make([]string, len(evaluators))
	for i, ev := range evaluators {
		ids[i] = ev.ID
	}
	arb := &task.Task{
		ID:        "arbitration-1",
		Name:      "Arbitration",
		Agent:     task.AgentConfig{Role: arbitrationRole},
		DependsOn: ids,
		State:     task.StateCompleted,
		Summary:   "ship it",
	}
	store.tasks[arb.ID] = arb

	root, err := store.GetTask(st.RootTaskID)
	if err != nil {
		t.Fatal(err)
	}
	return store, st, root, evaluators, arb
}
```

Replace with:

```go
func seedDoneStory(t *testing.T) (*fakeStoryStore, *story.Story, *task.Task, []*task.Task, *task.Task) {
	t.Helper()
	store, st, evaluators := seedStoryWithEvaluators(t, task.StateCompleted)
	st.Status = "DONE"

	ids := make([]string, len(evaluators))
	for i, ev := range evaluators {
		ids[i] = ev.ID
	}
	arb := &task.Task{
		ID:        "arbitration-1",
		Name:      "Arbitration",
		Agent:     task.AgentConfig{Role: arbitrationRole},
		DependsOn: ids,
		State:     task.StateCompleted,
		Summary:   "ship it",
	}
	store.tasks[arb.ID] = arb

	// By the time a story reaches DONE, its root has already been through a
	// real (or simulated) approval and is COMPLETED. seedStoryWithEvaluators
	// seeds root at READY (the pre-arbitration state every other caller of
	// that helper needs), so promote it here to match this fixture's own
	// DONE narrative.
	if err := store.UpdateTaskState(st.RootTaskID, task.StateCompleted); err != nil {
		t.Fatalf("promote root to completed for DONE fixture: %v", err)
	}

	root, err := store.GetTask(st.RootTaskID)
	if err != nil {
		t.Fatal(err)
	}
	return store, st, root, evaluators, arb
}
```

- [ ] **Step 6: Rename and update `TestStoryOrchestrator_SpawnsEvaluators_WhenBuilderCompletes`**

In the same file, find the full test (including its doc comment):

```go
// TestStoryOrchestrator_SpawnsEvaluators_WhenBuilderCompletes is verification
// item (a): a builder task reaching COMPLETED for a story spawns exactly 4
// evaluator tasks with correct roles/depends_on, moves the story to
// VALIDATING.
func TestStoryOrchestrator_SpawnsEvaluators_WhenBuilderCompletes(t *testing.T) {
	store := newFakeStoryStore()
	root := builderTask("builder-1", task.StateCompleted)
	store.tasks[root.ID] = root
	st := newStoryWithRoot("story-1", root.ID, "IN_PROGRESS")
	store.stories[st.ID] = st
```

Replace with:

```go
// TestStoryOrchestrator_SpawnsEvaluators_WhenBuilderReady is verification
// item (a): a builder task reaching READY for a story spawns exactly 4
// evaluator tasks with correct roles/depends_on, moves the story to
// VALIDATING. The builder itself stays READY (not COMPLETED) — see
// TestStoryOrchestrator_ReadyBuilder_SpawnsEvaluators_StaysReadyUntilApproved
// for that specific assertion.
func TestStoryOrchestrator_SpawnsEvaluators_WhenBuilderReady(t *testing.T) {
	store := newFakeStoryStore()
	root := builderTask("builder-1", task.StateReady)
	store.tasks[root.ID] = root
	st := newStoryWithRoot("story-1", root.ID, "IN_PROGRESS")
	store.stories[st.ID] = st
```

(The rest of this test's body — from `pool := &fakePool{}` through the final closing brace — is unchanged. Only the doc comment, function name, and the `builderTask` state argument change.)

- [ ] **Step 7: Update `TestStoryOrchestrator_DoesNotDuplicateEvaluators`'s root seed**

In the same file, find:

```go
func TestStoryOrchestrator_DoesNotDuplicateEvaluators(t *testing.T) {
	store := newFakeStoryStore()
	root := builderTask("builder-1", task.StateCompleted)
	store.tasks[root.ID] = root
	st := newStoryWithRoot("story-1", root.ID, "IN_PROGRESS")
	store.stories[st.ID] = st
```

Replace with:

```go
func TestStoryOrchestrator_DoesNotDuplicateEvaluators(t *testing.T) {
	store := newFakeStoryStore()
	root := builderTask("builder-1", task.StateReady)
	store.tasks[root.ID] = root
	st := newStoryWithRoot("story-1", root.ID, "IN_PROGRESS")
	store.stories[st.ID] = st
```

(Rest of the test body unchanged.)

- [ ] **Step 8: Replace `TestStoryOrchestrator_AutoAcceptsReadyBuilder`**

In the same file, find the full test (including its doc comment):

```go
// TestStoryOrchestrator_AutoAcceptsReadyBuilder is the core regression test
// for the auto-accept fix: a builder task sitting at READY (execution
// succeeded, awaiting what would otherwise be a manual
// POST /api/tasks/{id}/accept) is transitioned to COMPLETED by the
// orchestrator itself — with no external accept call — and, because that
// unblocks Stage 1 in the same tick, the 4 evaluators are spawned
// immediately too.
func TestStoryOrchestrator_AutoAcceptsReadyBuilder(t *testing.T) {
	store := newFakeStoryStore()
	root := builderTask("builder-1", task.StateReady)
	store.tasks[root.ID] = root
	st := newStoryWithRoot("story-1", root.ID, "IN_PROGRESS")
	store.stories[st.ID] = st

	pool := &fakePool{}
	orch := &StoryOrchestrator{Store: store, Pool: pool}
	orch.Tick(context.Background())

	got, err := store.GetTask(root.ID)
	if err != nil {
		t.Fatal(err)
	}
	if got.State != task.StateCompleted {
		t.Fatalf("builder should be auto-accepted to COMPLETED, got %v", got.State)
	}

	deps, _ := store.ListDependents(root.ID)
	if len(deps) != 4 {
		t.Fatalf("expected 4 evaluator tasks spawned in the same tick the builder auto-accepts, got %d", len(deps))
	}
}
```

Replace with:

```go
// TestStoryOrchestrator_ReadyBuilder_SpawnsEvaluators_StaysReadyUntilApproved
// replaces the old "auto-accept the builder immediately" regression test: a
// builder task sitting at READY (execution succeeded, awaiting what would
// otherwise be a manual POST /api/tasks/{id}/accept) triggers the 4
// evaluators to spawn in the same tick — but, unlike evaluators/arbitration
// tasks, the builder itself is NOT auto-accepted to COMPLETED here. It stays
// READY (which already satisfies a dependent's DependsOn — see
// executor.depDoneStates) until its own arbitration approves it (see
// finalizeArbitration). This is what makes "COMPLETED" mean "verified",
// uniformly — the same rule a nested builder subtask will need once
// arbitrated review recurses into subtask trees.
func TestStoryOrchestrator_ReadyBuilder_SpawnsEvaluators_StaysReadyUntilApproved(t *testing.T) {
	store := newFakeStoryStore()
	root := builderTask("builder-1", task.StateReady)
	store.tasks[root.ID] = root
	st := newStoryWithRoot("story-1", root.ID, "IN_PROGRESS")
	store.stories[st.ID] = st

	pool := &fakePool{}
	orch := &StoryOrchestrator{Store: store, Pool: pool}
	orch.Tick(context.Background())

	got, err := store.GetTask(root.ID)
	if err != nil {
		t.Fatal(err)
	}
	if got.State != task.StateReady {
		t.Fatalf("builder must stay READY until arbitration approves it, got %v", got.State)
	}

	deps, _ := store.ListDependents(root.ID)
	if len(deps) != 4 {
		t.Fatalf("expected 4 evaluator tasks spawned in the same tick the builder reaches READY, got %d", len(deps))
	}
}
```

- [ ] **Step 9: Update `TestStoryOrchestrator_AutoAccept_DoesNotTouchUnrelatedReadyTask`'s root seed**

In the same file, find:

```go
func TestStoryOrchestrator_AutoAccept_DoesNotTouchUnrelatedReadyTask(t *testing.T) {
	store := newFakeStoryStore()
	root := builderTask("builder-1", task.StateCompleted)
	store.tasks[root.ID] = root
	st := newStoryWithRoot("story-1", root.ID, "IN_PROGRESS")
	store.stories[st.ID] = st
```

Replace with:

```go
func TestStoryOrchestrator_AutoAccept_DoesNotTouchUnrelatedReadyTask(t *testing.T) {
	store := newFakeStoryStore()
	root := builderTask("builder-1", task.StateReady)
	store.tasks[root.ID] = root
	st := newStoryWithRoot("story-1", root.ID, "IN_PROGRESS")
	store.stories[st.ID] = st
```

(Rest of the test body unchanged. `TestStoryOrchestrator_SkipsTerminalStories`, a few lines further down, keeps its root seeded at `task.StateCompleted` unchanged — its own doc comment already says "even if (hypothetically) their root task is COMPLETED", and `Tick`'s own top-level DONE/CANCELLED skip means `processStory` is never reached for it regardless of root's state, so it is unaffected by this plan and must NOT be modified.)

- [ ] **Step 10: Update `TestStoryOrchestrator_EndToEnd`**

In the same file, find:

```go
	// Builder's execution succeeds (RUNNING -> READY, exactly like
	// handleRunResult) — no POST /api/tasks/{id}/accept call here.
	store.setTaskState(root.ID, task.StateReady)
	orch.Tick(context.Background())

	rootAfter, err := store.GetTask(root.ID)
	if err != nil {
		t.Fatal(err)
	}
	if rootAfter.State != task.StateCompleted {
		t.Fatalf("builder should be auto-accepted to COMPLETED without an accept call, got %v", rootAfter.State)
	}
```

Replace with:

```go
	// Builder's execution succeeds (RUNNING -> READY, exactly like
	// handleRunResult) — no POST /api/tasks/{id}/accept call here.
	store.setTaskState(root.ID, task.StateReady)
	orch.Tick(context.Background())

	rootAfter, err := store.GetTask(root.ID)
	if err != nil {
		t.Fatal(err)
	}
	if rootAfter.State != task.StateReady {
		t.Fatalf("builder must stay READY until arbitration approves it (not auto-accepted eagerly), got %v", rootAfter.State)
	}
```

Then, further down in the same test, find:

```go
	// Arbitration's execution succeeds (READY, not COMPLETED).
	store.setTaskState(arb.ID, task.StateReady)
	store.setTaskSummary(arb.ID, "approved")
	orch.Tick(context.Background())

	arbAfter, err := store.GetTask(arb.ID)
	if err != nil {
		t.Fatal(err)
	}
	if arbAfter.State != task.StateCompleted {
		t.Fatalf("arbitration should be auto-accepted to COMPLETED, got %v", arbAfter.State)
	}

	if statusOf() != "REVIEW_READY" {
		t.Fatalf("expected REVIEW_READY after arbitration completes, got %q", statusOf())
	}
	decided := store.eventsOfKind(event.KindArbitrationDecided)
	if len(decided) != 1 {
		t.Fatalf("expected exactly 1 arbitration_decided event, got %d", len(decided))
	}
}
```

Replace with:

```go
	// Arbitration's execution succeeds (READY, not COMPLETED).
	store.setTaskState(arb.ID, task.StateReady)
	store.setTaskSummary(arb.ID, "approved")
	orch.Tick(context.Background())

	arbAfter, err := store.GetTask(arb.ID)
	if err != nil {
		t.Fatal(err)
	}
	if arbAfter.State != task.StateCompleted {
		t.Fatalf("arbitration should be auto-accepted to COMPLETED, got %v", arbAfter.State)
	}

	rootFinal, err := store.GetTask(root.ID)
	if err != nil {
		t.Fatal(err)
	}
	if rootFinal.State != task.StateCompleted {
		t.Fatalf("builder should finally be promoted to COMPLETED now that arbitration approved it, got %v", rootFinal.State)
	}

	if statusOf() != "REVIEW_READY" {
		t.Fatalf("expected REVIEW_READY after arbitration completes, got %q", statusOf())
	}
	decided := store.eventsOfKind(event.KindArbitrationDecided)
	if len(decided) != 1 {
		t.Fatalf("expected exactly 1 arbitration_decided event, got %d", len(decided))
	}
}
```

Also update this test's doc comment. Find:

```go
// TestStoryOrchestrator_EndToEnd drives a story through the full chain —
// builder complete -> evaluators -> arbitration -> REVIEW_READY — using only
// the fake store/pool, proving the whole ceremony holds together end to end
// at the orchestrator level (verification item 5, the higher-level test).
//
// Every task in the chain is driven to READY (never directly to COMPLETED),
// mirroring exactly what executor.Pool.handleRunResult does for a real
// top-level task whose execution succeeds — proving the orchestrator's own
// auto-accept (not an external POST /api/tasks/{id}/accept call, which this
// test never makes) is what carries each task the rest of the way to
// COMPLETED and advances the story. The only accept call anywhere in this
// flow is the story-level one, which isn't part of this test — it's covered
// separately in internal/api's story accept-gate tests.
func TestStoryOrchestrator_EndToEnd(t *testing.T) {
```

Replace with:

```go
// TestStoryOrchestrator_EndToEnd drives a story through the full chain —
// builder ready -> evaluators -> arbitration -> REVIEW_READY — using only
// the fake store/pool, proving the whole ceremony holds together end to end
// at the orchestrator level (verification item 5, the higher-level test).
//
// Every task in the chain is driven to READY (never directly to COMPLETED),
// mirroring exactly what executor.Pool.handleRunResult does for a real
// top-level task whose execution succeeds. Evaluators and arbitration are
// still carried the rest of the way to COMPLETED by the orchestrator's own
// auto-accept (not an external POST /api/tasks/{id}/accept call, which this
// test never makes) — but the builder itself is the one exception: it stays
// READY through the whole cycle and is only promoted to COMPLETED once
// arbitration actually approves it, proving "COMPLETED means verified" holds
// end to end, not just in isolated unit tests. The only accept call anywhere
// in this flow is the story-level one, which isn't part of this test — it's
// covered separately in internal/api's story accept-gate tests.
func TestStoryOrchestrator_EndToEnd(t *testing.T) {
```

- [ ] **Step 11: Add a new test proving rejection leaves root at READY, never COMPLETED**

In the same file, immediately after `seedNeedsFixStory`'s closing brace (before `TestStoryOrchestrator_NeedsFix_SpawnsFixAttemptAndRepointsRoot` or whatever the next test in the file is), add:

```go
// TestStoryOrchestrator_ArbitrationRejects_LeavesRootReadyNotCompleted proves
// the core "verified before complete" invariant this plan adds: a rejected
// root is left at READY — never promoted to COMPLETED. Only
// finalizeArbitration's approval branch ever writes root to COMPLETED; a
// rejection must never look "done" to anything that trusts task.State
// directly, the same gate a nested subtask's parent will rely on once
// arbitrated review recurses into subtask trees (a future plan).
func TestStoryOrchestrator_ArbitrationRejects_LeavesRootReadyNotCompleted(t *testing.T) {
	store, _, oldRoot, _, _ := seedNeedsFixStory(t)

	got, err := store.GetTask(oldRoot.ID)
	if err != nil {
		t.Fatal(err)
	}
	if got.State != task.StateReady {
		t.Errorf("rejected root State = %v, want READY (must never be promoted to COMPLETED on rejection)", got.State)
	}
}
```

- [ ] **Step 12: Run the full scheduler package test suite**

Run: `go test ./internal/scheduler/... -v -run 'TestStoryOrchestrator'`
Expected: PASS for every test in the file, including the renamed/rewritten ones above, the 3 unmodified fix-loop tests (`TestStoryOrchestrator_NeedsFix_SpawnsFixAttemptAndRepointsRoot`, `TestStoryOrchestrator_EnsureFixAttempt_FindsExistingSpawnedAttempt`, `TestStoryOrchestrator_NeedsFix_CapsAtMaxFixAttempts` — none of these assert on root's exact state before rejection, only on the fix-attempt task's own shape and the story's status/RootTaskID, so they are unaffected), the retro test cluster (unaffected — `seedDoneStory` now explicitly promotes root to COMPLETED to match its own narrative), and the two arbitration-verdict tests (`TestStoryOrchestrator_ArbitrationRejects_SetsStoryNeedsFix`, `TestStoryOrchestrator_ArbitrationApproves_SetsReviewReady` — unaffected, since they only assert on story.Status, not root.State). Paste the actual output.

- [ ] **Step 13: Run the full repo test suite**

Run: `go build ./...` then `go test ./...` (the entire repo). Both must pass — paste the actual output. If `internal/api` flakes under the race detector on a one-off "sql: database is closed" teardown error unrelated to this task's actual changes, rerun once before treating it as real.

Also run `gofmt -l internal/scheduler/story_orchestrator.go internal/scheduler/story_orchestrator_test.go` before committing, and `gofmt -w` any file it flags that wasn't already gofmt-dirty before your changes (check with `git show HEAD:<path> | gofmt -l -` if unsure).

- [ ] **Step 14: Commit (do NOT attempt to push — see note below)**

Run these exact commands, in order, confirming each succeeds:

```bash
git add -A
git commit -m "refactor(scheduler): promote story root to COMPLETED only after arbitration approves it, not eagerly on READY"
git status
git log --oneline -1
```

**Do not run `git push`.** This repo's own bare git remote is not mounted inside this container — a `git push origin main` here will either fail outright or silently no-op; do not treat that as an error and do not spend turns retrying it. Per this repo's own CLAUDE.md ("After a successful run, new commits are pushed from the workspace"), claudomator's ContainerRunner pushes your commits externally once this run finishes successfully with a clean working tree. Your only job is to commit locally and end your turn with `git status` showing a clean tree — do this as soon as all edits (Steps 1-11) are done, before spending time on Steps 12/13's verification, so your work is safe even if you run low on turns during verification. Paste the `git status`/`git log` output in your summary regardless.