From e4087a7dc133fe8c8523ca585b1841ff2b0be2d9 Mon Sep 17 00:00:00 2001 From: Claude Sonnet 5 Date: Sat, 4 Jul 2026 04:08:41 +0000 Subject: feat(story): add StoryOrchestrator -- Builder->Evaluators->Arbitration->accept (Phase 7b) A deterministic, poll-based watcher (internal/scheduler.StoryOrchestrator, sibling to the Phase 5 Scheduler) that drives a story.Story through its execution pipeline, rather than relying on an LLM agent to correctly orchestrate its own fan-out via tool calls. Mechanism: polling, not a handleRunResult hook. Every task the orchestrator watches (a story's root/Builder task, 4 Evaluators, Arbitration) is top-level (no ParentTaskID), and executor.Pool.handleRunResult only ever lands a top-level task at READY or BLOCKED -- never COMPLETED directly, since that transition normally requires a human/chatbot POST /api/tasks/{id}/accept in a different package. A handleRunResult hook would never observe it; polling doesn't care how/whether a task reached a given state. Stages: Builder COMPLETED -> spawn 4 role-typed Evaluator tasks (evaluator_quality/security/correctness/performance, DependsOn: [builder], no ParentTaskID -- true DAG siblings, not delegated subtasks) + story -> VALIDATING. Each Evaluator COMPLETED -> emit KindEvalVerdict (attached to the story's ID, so one GET /api/stories/{id}/events call surfaces every verdict). All 4 Evaluators COMPLETED -> spawn 1 Arbitration task (role: planner, DependsOn: all 4 evaluator IDs). Arbitration COMPLETED -> emit KindArbitrationDecided, story -> REVIEW_READY. POST /api/stories/{id}/accept (mirrors handleAcceptTask) -> DONE, emits KindHumanAccepted. Fixes a gap caught before merging: since none of Builder/Evaluators/ Arbitration have a ParentTaskID, none of them auto-complete -- each would otherwise need a separate manual /api/tasks/{id}/accept, meaning 6 human clicks per story before ever reaching the intended single story-level gate. StoryOrchestrator.autoAccept now transitions each of these specific tasks READY->COMPLETED itself (via the same validated Store.UpdateTaskState path acceptTask uses), scoped only to tasks already established as part of a story's pipeline (root task, or role-matched dependents from ensureEvaluators/ensureArbitration) -- never a blanket sweep of unrelated READY tasks. This makes POST /api/stories/{id}/accept the system's only required human touchpoint for the whole chain, matching the design goal that story (not task/subtask) is the human-interaction atom. Idempotency: structural for task-creation stages (ensureEvaluators/ ensureArbitration check ListDependents for already-existing role-matched tasks before creating -- crash/restart-safe); story.Status=="VALIDATING" gates the Arbitration->REVIEW_READY write (nothing further downstream to check structurally there); an in-memory handledVerdicts set (mirrors Scheduler.handled) dedupes per-evaluator KindEvalVerdict emission across poll ticks, resetting harmlessly on restart. Documented simplification: finalizeArbitration never parses the Arbitration summary for approve/reject -- always routes to REVIEW_READY; NEEDS_FIX is manually settable via PUT /api/stories/{id}. A later phase could close this with a dedicated verdict-reporting AgentChannel method instead of parsing free text. go build/vet/test -race -count=1 all pass, full suite (20 packages). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01V1moSNCJRcP6kykA4tyUSs --- CLAUDE.md | 169 +++++++++++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 150 insertions(+), 19 deletions(-) (limited to 'CLAUDE.md') diff --git a/CLAUDE.md b/CLAUDE.md index 90356e5..32bca69 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -78,8 +78,8 @@ Config defaults to `~/.claudomator/config.toml`. Data is stored in `~/.claudomat | `internal/agentloop` | Provider-neutral tool-use control-flow loop shared by all `NativeRunner`s | | `internal/sandbox` | `Sandbox` interface (`HostSandbox`/`DockerSandbox`) + pre-tool-use guardrail hooks | | `internal/role` | `RoleConfig`/`Tier`/`Rung` — per-role system prompt + provider/model escalation ladder | -| `internal/scheduler` | `Scheduler` — polls role-typed FAILED tasks and retries/escalates them per their role's ladder | -| `internal/story` | `Epic`/`Story` — planning layer above the flat task tree; data model only (Phase 7a), no orchestration | +| `internal/scheduler` | `Scheduler` — polls role-typed FAILED tasks and retries/escalates them per their role's ladder; `StoryOrchestrator` (Phase 7b) — polls stories and drives Builder → Evaluators → Arbitration → REVIEW_READY | +| `internal/story` | `Epic`/`Story` — planning layer above the flat task tree; data model + CRUD only (see `internal/scheduler.StoryOrchestrator` for the orchestration that drives it) | | `web` | Embedded static UI (`embed.go`) | ### Key Data Flows @@ -108,8 +108,8 @@ PENDING ──→ QUEUED ──→ RUNNING ──→ READY ──→ COMPLETED ``` - **BLOCKED**: Parent task completed but has subtasks that are not yet COMPLETED, OR the agent called `ask_user`. Unblocked by `maybeUnblockParent()` or a user/chatbot answer via `/api/tasks/{id}/answer` (or the chatbot MCP `answer_question` tool). -- **READY**: Execution succeeded; awaits manual accept/reject via `/api/tasks/{id}/accept` or `/api/tasks/{id}/reject`. -- **COMPLETED**: Terminal — entered only via user accept (top-level) or automatic subtask completion. +- **READY**: Execution succeeded; awaits manual accept/reject via `/api/tasks/{id}/accept` or `/api/tasks/{id}/reject`. Exception: a Builder/Evaluator/Arbitration task belonging to a story's pipeline is auto-accepted straight to `COMPLETED` by `internal/scheduler.StoryOrchestrator` (Phase 7b) instead of waiting for a manual accept — see "Story orchestrator" below. +- **COMPLETED**: Terminal — entered via user accept (top-level), `StoryOrchestrator` auto-accept (story-pipeline top-level tasks), or automatic subtask completion. - `FAILED/TIMED_OUT/CANCELLED/BUDGET_EXCEEDED` all re-enter at `QUEUED` for retry/resume. **WebSocket:** `Hub` fans out task completion events to all connected clients. `Server.StartHub()` must be called before `ListenAndServe`. @@ -324,15 +324,19 @@ Two prerequisites for fan-out patterns (e.g. a Builder task with several role-ty - **Auto-cascade-fail** (`internal/executor.Pool.cascadeFail`): the moment a task lands in a terminal failure state (`FAILED`/`TIMED_OUT`/`CANCELLED`/`BUDGET_EXCEEDED` — checked in `handleRunResult` plus the two direct-cancel paths in `execute()` for budget-gate and dependency-failure), the pool looks up `Store.ListDependents(taskID)` (a full-table scan over `depends_on_json` — no reverse index, same tradeoff already accepted elsewhere for JSON-blob columns) and cancels every direct dependent still waiting (`PENDING`/`QUEUED`), recording a `CANCELLED` execution whose `error_msg` references the upstream task and writing the ordinary `KindStateChange` event via `UpdateTaskState` (no new event kind). It recurses into each cancelled dependent's own dependents (visited-set guarded against a pathological dependency cycle — task creation does not itself reject cycles), so a multi-level chain cascades all the way down. `execute()`'s dependency-wait requeue loop re-checks the task's fresh DB state before dispatching so a task cascade-cancelled out from under it is never actually run. - **Role-typed subtask spawning** (`agentchannel.SubtaskSpec.Role`, threaded through `storeChannel.SpawnSubtask` in `internal/executor/channel.go`): when a spawning agent sets `role` (available as an optional `spawn_subtask` tool parameter on both transports — `internal/agentloop/tools.go` for the native tool-use loop, `internal/executor/agentmcp.go` for the MCP transport), the child task gets `Agent.Role` set and `Agent.Type`/`Agent.Model` left empty, so Phase 5's role-based dispatch resolves them from the role's escalation ladder on the child's own first dispatch. Omitting `role` (every pre-Phase-6 caller) preserves the exact prior behavior (`Agent.Type: "claude"`, `Agent.Model` from the `model` argument). -### Planning layer: epics & stories (data model only) +### Planning layer: epics & stories `internal/story` defines `Epic` and `Story` — a planning layer above the flat -task tree (`epics`/`stories` tables, see Storage Schema above). This is -Phase 7a: **pure data model + CRUD**. Nothing creates, transitions, or reads -these rows automatically yet — no watcher spawns Builder/Evaluator/ -Arbitration tasks from a story, no deploy-gating, no epic-proposal agent -tooling. A later phase builds that orchestration on top of what's stored -here. +task tree (`epics`/`stories` tables, see Storage Schema above). Phase 7a +built pure data model + CRUD. Phase 7b (`internal/scheduler.StoryOrchestrator`) +adds the first slice of automated orchestration on top of it: driving a story +through Builder → 4 parallel Evaluators → Arbitration → a single human +accept-gate **at the story level**. Every task along the way (Builder, +Evaluators, Arbitration) is auto-accepted by the orchestrator itself — see +"Story orchestrator" below — so `POST /api/stories/{id}/accept` is the +*only* manual accept a human/chatbot ever has to make for the whole chain. +Planner→Builder chain automation, epic-proposal tooling, and deploy-gating +are still out of scope — see that type's non-goals. - **Epic** (`epics` table): a loosely-scoped initiative that decomposes into Stories. `status` is `OPEN`/`CLOSED`, unvalidated pass-through (`UpdateEpic` @@ -342,9 +346,11 @@ here. `root_task_id` are loose references (plain strings, no FK enforcement) — same tolerance the codebase already has for `tasks.project`. `status` is one of `DISCOVERY|FRAMING|BACKLOG|PRIORITIZED|IN_PROGRESS|SHIPPABLE| - DEPLOYED|VALIDATING|REVIEW_READY|NEEDS_FIX|DONE|CANCELLED`, also - unvalidated pass-through in this phase (no `task.ValidTransition`-style - enforcement — there's no orchestrator yet to make transitions meaningful). + DEPLOYED|VALIDATING|REVIEW_READY|NEEDS_FIX|DONE|CANCELLED`, still + unvalidated pass-through at the storage layer (no `task.ValidTransition`- + style enforcement in `UpdateStory`/`PUT /api/stories/{id}`) — `StoryOrchestrator` + itself only ever writes `VALIDATING` and `REVIEW_READY`, and only after + checking the story's current status/dependents first (see below). - **`GET /api/stories/{id}/task-tree`** walks the task graph realizing a story: starting at `root_task_id`, it follows both `parent_task_id` children (`ListSubtasks`) and `depends_on_json` edges in either direction @@ -354,15 +360,115 @@ here. state, agent_type, role, parent_task_id, depends_on}]}`); clients reconstruct the graph from each node's `parent_task_id`/`depends_on`. An unset `root_task_id` returns an empty node list, not an error. -- New `event.Kind` constants for the ceremony this layer will eventually - drive (`epic_proposed`, `discovery_proposed`, `framing_decided`, - `groomed`, `prioritized`, `eval_verdict`, `arbitration_decided`, - `retro_captured`, `human_accepted`) are defined in `internal/event` but - nothing emits them yet. - REST endpoints are not gated by `api_token`, consistent with `/api/projects`/`/api/tasks` (only chatbot MCP, agent MCP, and WebSocket require it). +#### Story orchestrator (Phase 7b) + +`internal/scheduler.StoryOrchestrator` (same file group as `Scheduler`, see +`internal/scheduler/story_orchestrator.go`) is a poll-based watcher +(`Tick`/`Run`, `DefaultStoryPollInterval` = 15s), not a hook into +`executor.Pool.handleRunResult`. This is a deliberate choice, not the default: +every task the orchestrator reacts to (the story's Builder/root task, the 4 +Evaluators, the Arbitration task) is a **top-level** task +(`parent_task_id == ""`), and per the task state machine above, the *only* +way one of them would ever reach `COMPLETED` on its own is via a +human/chatbot `POST /api/tasks/{id}/accept` (`READY → COMPLETED`, in +`internal/api`'s `acceptTask`) — `handleRunResult` only ever lands a +top-level task at `READY` or `BLOCKED`, never `COMPLETED` directly. Rather +than requiring a human to make that call 6 times per story (builder + 4 +evaluators + arbitration), **the orchestrator auto-accepts these specific +tasks itself** (`autoAccept`, see below) — so the *only* manual accept +anywhere in this chain is the final story-level +`POST /api/stories/{id}/accept`. A `handleRunResult` hook would never observe +either transition (the human-driven one or this orchestrator's own +auto-accept) cleanly, since both happen outside `executor`; polling +sidesteps the question of *how*/*by whom* a task reached `COMPLETED` +entirely — the same way `Scheduler` doesn't care how a task got to `FAILED`. + +Stage-by-stage: + +1. **Builder → Evaluators**: once a story's `root_task_id` task is `READY`, + the orchestrator auto-accepts it to `COMPLETED` itself (see "Auto-accept" + below); once it's `COMPLETED`, spawns 4 new top-level tasks + (`Store.CreateTask`, not `SpawnSubtask` — no `parent_task_id`, + `depends_on: [root_task_id]`) with `agent.role` = + `evaluator_quality`/`evaluator_security`/`evaluator_correctness`/ + `evaluator_performance`, queues and submits each to the pool, and sets the + story's `status` to `VALIDATING`. Idempotency is **structural**, not a + marker: it inspects `Store.ListDependents(root_task_id)` for existing + tasks whose role already matches one of the four, and only creates the + missing ones — safe to call every tick forever, and safe across a process + restart (unlike an in-memory guard would be). +2. **Per-evaluator verdicts**: each Evaluator task is likewise auto-accepted + from `READY` to `COMPLETED`. As each is observed `COMPLETED`, emits + `event.KindEvalVerdict` — payload `{task_id, role, summary}` — attached to + the **story's** ID (not the evaluator task's ID), via + `GET /api/stories/{id}/events` (new in this phase, mirrors + `GET /api/tasks/{id}/events` but resolves existence via `GetStory`). + Attaching to the story ID means one call surfaces every verdict for a + story; `events.task_id` has no enforced FK, which is exactly the tolerance + 7a's `event.Kind` doc comments anticipated. De-duplication here is an + in-memory, per-process "already emitted" set keyed by evaluator task ID + (mirrors `Scheduler.handled` exactly) — the only place in this type that + isn't structurally idempotent, because there's no persisted marker for + "this specific verdict was already emitted"; a restart can produce at most + one duplicate `eval_verdict` event per evaluator, never more. +3. **Evaluators → Arbitration**: once all 4 Evaluators for a story are + `COMPLETED`, spawns one more top-level task, `agent.role: "planner"`, + `depends_on` = all 4 evaluator task IDs. Idempotency is again structural: + looks for an existing `planner`-role dependent of the first evaluator task + whose `depends_on` already contains all 4 IDs, rather than trusting + `story.status` (a human can freely rewrite it via `PUT /api/stories/{id}`). +4. **Arbitration → REVIEW_READY**: the Arbitration task is likewise + auto-accepted from `READY` to `COMPLETED`. When it reaches `COMPLETED`, + emits `event.KindArbitrationDecided` (payload `{task_id, summary}`, + attached to the story ID) and sets `status` to `REVIEW_READY`. **Gated on + `status == "VALIDATING"`** — the one place this type's idempotency check + is the story's own status field rather than a structural "does a + downstream task exist" check, because the Arbitration task is the last + task in the chain, so there's no further task existence to check against. + **Documented simplification:** this does **not** parse the Arbitration + task's summary for an approve/reject verdict — it always routes to + `REVIEW_READY`. A human or chatbot who reads the summary and disagrees + sets `NEEDS_FIX` manually via `PUT /api/stories/{id}`. A later phase could + close this gap with a dedicated verdict-reporting tool for the Arbitration + task (a new `AgentChannel` method) instead of parsing free text. +5. **Human accept-gate (the only one)**: `POST /api/stories/{id}/accept` + (new in this phase) mirrors `handleAcceptTask`'s pattern: only valid from + `REVIEW_READY` (409 otherwise), transitions to `DONE`, emits + `event.KindHumanAccepted` (payload `{story_id, from, to}`, attached to the + story ID). Because of auto-accept (below), this is the *only* + story-status transition — indeed the only task/story state transition at + all — in this whole chain that requires a human/chatbot to act. + +**Auto-accept** (`StoryOrchestrator.autoAccept`): on every tick, for the +story's root/Builder task and each structurally-discovered +Evaluator/Arbitration dependent (i.e. only tasks the orchestrator has already +established are part of *this* story's pipeline — never a blanket "auto- +accept every READY task" sweep), if that task is `READY`, the orchestrator +transitions it straight to `COMPLETED` itself, using the exact same +state-machine-respecting write `internal/api`'s `acceptTask` uses +(`Store.UpdateTaskState`, which wraps `storage.DB.UpdateTaskStateBy` — +validates `task.ValidTransition` and writes the `state_change` event +atomically, not a raw/unchecked write). This is what makes the story-level +accept-gate the system's only required human/chatbot touchpoint: without it, +a human would have to separately `POST /api/tasks/{id}/accept` the Builder, +each of the 4 Evaluators, and the Arbitration task, since none of those +top-level tasks can reach `COMPLETED` any other way. `FAILED`/`TIMED_OUT`/ +`CANCELLED`/`BUDGET_EXCEEDED` outcomes for these same tasks are untouched by +this — they still go through `Scheduler`'s existing retry-then-escalate path +exactly as before; auto-accept only ever fires on the `READY` (success) +path. + +`evaluator_quality`/`evaluator_security`/`evaluator_correctness`/ +`evaluator_performance`/`planner`/`builder` `role_configs` rows are assumed to +be seeded separately (this phase doesn't create them); a role-typed task +spawned here with no matching active `role_configs` row dispatches in the +same degraded (no role resolution) mode `Pool.execute()` already logs a +warning and falls back to for any other role-typed task. + --- @@ -382,6 +488,29 @@ Same pattern as `task.Priority`/`RetryConfig` below: `internal/role.RoleConfig.T `Scheduler.handled` (keyed by execution ID) prevents re-emitting a `final: true` `KindEscalated` event on every poll tick once a role-typed task's ladder is exhausted, but it's per-process and resets on restart. A restart can produce one extra "reconsideration" (and, if still exhausted/denied, one more `KindEscalated` event) — not an infinite loop, just not persisted. A future phase could persist this via a `tasks` column if that turns out to matter. +### StoryOrchestrator's Arbitration outcome always routes to REVIEW_READY + +`internal/scheduler.StoryOrchestrator.finalizeArbitration` does not parse the +Arbitration task's summary for an approve/reject verdict — every completed +Arbitration moves the story to `REVIEW_READY`, never directly to `NEEDS_FIX`. +A human or chatbot who reads the Arbitration summary and disagrees must +manually set the story to `NEEDS_FIX` via `PUT /api/stories/{id}`. A later +phase could close this by giving the Arbitration task a dedicated +verdict-reporting tool (a new `AgentChannel` method) instead of parsing free +text. + +### StoryOrchestrator's per-evaluator verdict de-dup is in-memory only + +`StoryOrchestrator.handledVerdicts` (keyed by evaluator task ID) prevents +re-emitting `KindEvalVerdict` on every poll tick while sibling evaluators are +still running, but — like `Scheduler.handled` above — it's per-process and +resets on restart. A restart can produce at most one duplicate `eval_verdict` +event per evaluator; the structural idempotency checks that actually prevent +duplicate task creation and duplicate story-status transitions +(`ensureEvaluators`/`ensureArbitration`'s dependents-based checks, and +`finalizeArbitration`'s `status == "VALIDATING"` gate) are unaffected by a +restart. + ### Deprecated task columns not yet dropped (Phase 8 follow-up) `tasks.question_json`, `summary`, `interactions_json`, `elaboration_input` are @@ -472,6 +601,8 @@ In `executor.go`, `withFailureHistory` creates a copy of the task struct (`copy | GET | `/api/stories/{id}` | Get story | | PUT | `/api/stories/{id}` | Update story (all fields but id/created_at; unvalidated status pass-through) | | GET | `/api/stories/{id}/task-tree` | Walk the task graph realizing a story (parent_task_id + depends_on edges) | +| GET | `/api/stories/{id}/events` | Story observability event stream (`?since_seq=N`); surfaces `eval_verdict`/`arbitration_decided`/`human_accepted` events the story orchestrator/accept-gate attach to the story's own ID | +| POST | `/api/stories/{id}/accept` | Accept a `REVIEW_READY` story → `DONE`; emits `KindHumanAccepted` | --- -- cgit v1.2.3