summaryrefslogtreecommitdiff
path: root/docs
diff options
context:
space:
mode:
Diffstat (limited to 'docs')
-rw-r--r--docs/superpowers/plans/2026-07-09-task-acceptance-criteria.md863
1 files changed, 863 insertions, 0 deletions
diff --git a/docs/superpowers/plans/2026-07-09-task-acceptance-criteria.md b/docs/superpowers/plans/2026-07-09-task-acceptance-criteria.md
new file mode 100644
index 0000000..967185f
--- /dev/null
+++ b/docs/superpowers/plans/2026-07-09-task-acceptance-criteria.md
@@ -0,0 +1,863 @@
+# Task AcceptanceCriteria 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:** Add `AcceptanceCriteria []string` to `task.Task`, mirroring `story.Story`'s existing pattern exactly, and expose it as a `spawn_subtask` parameter so a decomposing builder can state what each subtask must satisfy. This is piece 1 of `docs/superpowers/specs/2026-07-09-recursive-arbitrated-review-design.md`'s implementation order — additive, no behavior change to anything existing.
+
+**Architecture:** `task.Task` gains one new field, stored as `acceptance_criteria_json` (same column-naming/JSON-marshal convention as `depends_on_json`/`tags_json`). Touching this column means touching every one of the 5 places `internal/storage/db.go` repeats the tasks table's `SELECT` column list verbatim — this plan extracts a `taskSelectColumns()` helper (mirroring the already-established `storySelectColumns()` in `internal/storage/story.go`) at the same time, so a future column addition only requires one edit, not five kept in sync by hand.
+
+**Tech Stack:** Go, SQLite (additive `ALTER TABLE`, no data migration needed — new column defaults to `'[]'`).
+
+## Global Constraints
+
+- `AcceptanceCriteria` is **only settable at subtask creation** via `spawn_subtask` — it is intentionally not added to `storage.TaskUpdate`/`UpdateTask` (which already excludes several other `Task` fields, e.g. `ElaborationInput`, `ParentTaskID` — it is a curated subset for the "edit and reset to PENDING" REST use case, not an exhaustive field list).
+- Do not touch anything about arbitrated review's trigger, `maybeUnblockParent`, or the story-level fix loop's `RootTaskID` mechanism — those are separate, later pieces of the same design spec's implementation order.
+
+---
+
+## Task 1: Storage layer
+
+**Files:**
+- Modify: `internal/task/task.go` (`Task` struct)
+- Modify: `internal/storage/db.go` (schema, migration, new `taskSelectColumns()` helper, `CreateTask`, `GetTask`, `ListTasks`, `ListSubtasks`, `ListDependents`, `ResetTaskForRetry`, `scanTask`)
+- Test: `internal/storage/db_test.go` (append)
+
+**Interfaces:**
+- Produces: `task.Task.AcceptanceCriteria []string`, round-tripped through `storage.DB.CreateTask`/`GetTask` (and every other read path). Task 2 consumes this field directly when wiring `spawn_subtask`.
+
+- [ ] **Step 1: Add the field to `task.Task`**
+
+In `internal/task/task.go`, find the `Task` struct's `DependsOn` field:
+
+```go
+ DependsOn []string `yaml:"depends_on" json:"depends_on"`
+```
+
+Add directly after it:
+
+```go
+ DependsOn []string `yaml:"depends_on" json:"depends_on"`
+ // AcceptanceCriteria, when non-empty, states what this task's work must
+ // satisfy — set by a decomposing parent via spawn_subtask's
+ // acceptance_criteria parameter (mirrors story.Story.AcceptanceCriteria
+ // exactly). Unused by execution itself; a later phase's arbitrated-review
+ // generalization reads it when evaluating this task's work, falling back
+ // to the enclosing story's own AcceptanceCriteria when empty.
+ AcceptanceCriteria []string `yaml:"acceptance_criteria" json:"acceptance_criteria"`
+```
+
+- [ ] **Step 2: Write the failing storage test**
+
+Append to `internal/storage/db_test.go`, directly after `TestCreateTask_Project_RoundTrip`:
+
+```go
+func TestCreateTask_AcceptanceCriteria_RoundTrip(t *testing.T) {
+ db := testDB(t)
+ now := time.Now().UTC().Truncate(time.Second)
+
+ tk := &task.Task{
+ ID: "ac-1",
+ Name: "Task With Criteria",
+ Agent: task.AgentConfig{Type: "claude", Instructions: "do it"},
+ Priority: task.PriorityNormal,
+ Tags: []string{},
+ DependsOn: []string{},
+ AcceptanceCriteria: []string{"tests pass", "no new lint warnings"},
+ Retry: task.RetryConfig{MaxAttempts: 1, Backoff: "linear"},
+ State: task.StatePending,
+ CreatedAt: now,
+ UpdatedAt: now,
+ }
+ if err := db.CreateTask(tk); err != nil {
+ t.Fatalf("creating task: %v", err)
+ }
+
+ got, err := db.GetTask("ac-1")
+ if err != nil {
+ t.Fatalf("getting task: %v", err)
+ }
+ if len(got.AcceptanceCriteria) != 2 || got.AcceptanceCriteria[0] != "tests pass" || got.AcceptanceCriteria[1] != "no new lint warnings" {
+ t.Errorf("AcceptanceCriteria: want [tests pass, no new lint warnings], got %+v", got.AcceptanceCriteria)
+ }
+}
+
+// TestCreateTask_AcceptanceCriteria_DefaultsEmpty proves a task created
+// without AcceptanceCriteria round-trips as an empty (not nil) slice, the
+// same "never null" convention story.Story.AcceptanceCriteria already uses.
+func TestCreateTask_AcceptanceCriteria_DefaultsEmpty(t *testing.T) {
+ db := testDB(t)
+ now := time.Now().UTC().Truncate(time.Second)
+
+ tk := &task.Task{
+ ID: "ac-2",
+ Name: "Task Without Criteria",
+ Agent: task.AgentConfig{Type: "claude", Instructions: "do it"},
+ Priority: task.PriorityNormal,
+ Tags: []string{},
+ DependsOn: []string{},
+ Retry: task.RetryConfig{MaxAttempts: 1, Backoff: "linear"},
+ State: task.StatePending,
+ CreatedAt: now,
+ UpdatedAt: now,
+ }
+ if err := db.CreateTask(tk); err != nil {
+ t.Fatalf("creating task: %v", err)
+ }
+
+ got, err := db.GetTask("ac-2")
+ if err != nil {
+ t.Fatalf("getting task: %v", err)
+ }
+ if got.AcceptanceCriteria == nil {
+ t.Error("AcceptanceCriteria: want empty slice, got nil")
+ }
+ if len(got.AcceptanceCriteria) != 0 {
+ t.Errorf("AcceptanceCriteria: want empty, got %+v", got.AcceptanceCriteria)
+ }
+}
+```
+
+- [ ] **Step 3: Run tests to verify they fail**
+
+Run: `go test ./internal/storage/ -run TestCreateTask_AcceptanceCriteria -v`
+Expected: compile failure — `task.Task` has no `AcceptanceCriteria` field yet (Step 1 alone doesn't make this pass; the storage layer doesn't read/write it yet). Paste the actual output.
+
+- [ ] **Step 4: Add the column to the schema and migrations**
+
+In `internal/storage/db.go`, find the `CREATE TABLE IF NOT EXISTS tasks` block:
+
+```go
+ CREATE TABLE IF NOT EXISTS tasks (
+ id TEXT PRIMARY KEY,
+ name TEXT NOT NULL,
+ description TEXT,
+ config_json TEXT NOT NULL,
+ priority TEXT NOT NULL DEFAULT 'normal',
+ timeout_ns INTEGER NOT NULL DEFAULT 0,
+ retry_json TEXT NOT NULL DEFAULT '{}',
+ tags_json TEXT NOT NULL DEFAULT '[]',
+ depends_on_json TEXT NOT NULL DEFAULT '[]',
+ parent_task_id TEXT,
+ state TEXT NOT NULL DEFAULT 'PENDING',
+ created_at DATETIME NOT NULL,
+ updated_at DATETIME NOT NULL
+ );
+```
+
+Replace it with:
+
+```go
+ CREATE TABLE IF NOT EXISTS tasks (
+ id TEXT PRIMARY KEY,
+ name TEXT NOT NULL,
+ description TEXT,
+ config_json TEXT NOT NULL,
+ priority TEXT NOT NULL DEFAULT 'normal',
+ timeout_ns INTEGER NOT NULL DEFAULT 0,
+ retry_json TEXT NOT NULL DEFAULT '{}',
+ tags_json TEXT NOT NULL DEFAULT '[]',
+ depends_on_json TEXT NOT NULL DEFAULT '[]',
+ acceptance_criteria_json TEXT NOT NULL DEFAULT '[]',
+ parent_task_id TEXT,
+ state TEXT NOT NULL DEFAULT 'PENDING',
+ created_at DATETIME NOT NULL,
+ updated_at DATETIME NOT NULL
+ );
+```
+
+Find the last entry in the `migrations` slice:
+
+```go
+ `ALTER TABLE tasks ADD COLUMN needs_review BOOLEAN NOT NULL DEFAULT 0`,
+ }
+```
+
+Replace it with:
+
+```go
+ `ALTER TABLE tasks ADD COLUMN needs_review BOOLEAN NOT NULL DEFAULT 0`,
+ `ALTER TABLE tasks ADD COLUMN acceptance_criteria_json TEXT NOT NULL DEFAULT '[]'`,
+ }
+```
+
+- [ ] **Step 5: Add `taskSelectColumns()` and replace all 5 duplicated SELECT column lists**
+
+In `internal/storage/db.go`, find `GetTask`:
+
+```go
+// GetTask retrieves a task by ID.
+func (s *DB) GetTask(id string) (*task.Task, error) {
+ row := s.db.QueryRow(`SELECT id, name, description, elaboration_input, project, repository_url, config_json, priority, timeout_ns, retry_json, tags_json, depends_on_json, parent_task_id, state, created_at, updated_at, rejection_comment, question_json, summary, interactions_json, needs_review FROM tasks WHERE id = ?`, id)
+ return scanTask(row)
+}
+```
+
+Replace it with (adding the new helper directly above `GetTask`):
+
+```go
+// taskSelectColumns is the shared SELECT column list every task-reading
+// query in this file uses — extracted so a future column addition is one
+// edit instead of five kept in sync by hand. Mirrors storySelectColumns in
+// internal/storage/story.go.
+func taskSelectColumns() string {
+ return `SELECT id, name, description, elaboration_input, project, repository_url, config_json, priority, timeout_ns, retry_json, tags_json, depends_on_json, acceptance_criteria_json, parent_task_id, state, created_at, updated_at, rejection_comment, question_json, summary, interactions_json, needs_review`
+}
+
+// GetTask retrieves a task by ID.
+func (s *DB) GetTask(id string) (*task.Task, error) {
+ row := s.db.QueryRow(taskSelectColumns()+` FROM tasks WHERE id = ?`, id)
+ return scanTask(row)
+}
+```
+
+In the same file, find `ListTasks`'s query assignment:
+
+```go
+ query := `SELECT id, name, description, elaboration_input, project, repository_url, config_json, priority, timeout_ns, retry_json, tags_json, depends_on_json, parent_task_id, state, created_at, updated_at, rejection_comment, question_json, summary, interactions_json, needs_review FROM tasks WHERE 1=1`
+```
+
+Replace it with:
+
+```go
+ query := taskSelectColumns() + ` FROM tasks WHERE 1=1`
+```
+
+Find `ListSubtasks`:
+
+```go
+// ListSubtasks returns all tasks whose parent_task_id matches the given ID.
+func (s *DB) ListSubtasks(parentID string) ([]*task.Task, error) {
+ rows, err := s.db.Query(`SELECT id, name, description, elaboration_input, project, repository_url, config_json, priority, timeout_ns, retry_json, tags_json, depends_on_json, parent_task_id, state, created_at, updated_at, rejection_comment, question_json, summary, interactions_json, needs_review FROM tasks WHERE parent_task_id = ? ORDER BY created_at ASC`, parentID)
+```
+
+Replace the query line with:
+
+```go
+ rows, err := s.db.Query(taskSelectColumns()+` FROM tasks WHERE parent_task_id = ? ORDER BY created_at ASC`, parentID)
+```
+
+Find `ListDependents`:
+
+```go
+ rows, err := s.db.Query(`SELECT id, name, description, elaboration_input, project, repository_url, config_json, priority, timeout_ns, retry_json, tags_json, depends_on_json, parent_task_id, state, created_at, updated_at, rejection_comment, question_json, summary, interactions_json, needs_review FROM tasks`)
+```
+
+Replace it with:
+
+```go
+ rows, err := s.db.Query(taskSelectColumns() + ` FROM tasks`)
+```
+
+Find `ResetTaskForRetry`:
+
+```go
+ t, err := scanTask(tx.QueryRow(`SELECT id, name, description, elaboration_input, project, repository_url, config_json, priority, timeout_ns, retry_json, tags_json, depends_on_json, parent_task_id, state, created_at, updated_at, rejection_comment, question_json, summary, interactions_json, needs_review FROM tasks WHERE id = ?`, id))
+```
+
+Replace it with:
+
+```go
+ t, err := scanTask(tx.QueryRow(taskSelectColumns()+` FROM tasks WHERE id = ?`, id))
+```
+
+- [ ] **Step 6: Update `CreateTask` to write the new column**
+
+In the same file, find `CreateTask`:
+
+```go
+func (s *DB) CreateTask(t *task.Task) error {
+ configJSON, err := json.Marshal(t.Agent)
+ if err != nil {
+ return fmt.Errorf("marshaling config: %w", err)
+ }
+ retryJSON, err := json.Marshal(t.Retry)
+ if err != nil {
+ return fmt.Errorf("marshaling retry: %w", err)
+ }
+ tagsJSON, err := json.Marshal(t.Tags)
+ if err != nil {
+ return fmt.Errorf("marshaling tags: %w", err)
+ }
+ depsJSON, err := json.Marshal(t.DependsOn)
+ if err != nil {
+ return fmt.Errorf("marshaling depends_on: %w", err)
+ }
+
+ tx, err := s.db.Begin()
+ if err != nil {
+ return err
+ }
+ defer tx.Rollback() //nolint:errcheck
+
+ if _, err = tx.Exec(`
+ INSERT INTO tasks (id, name, description, elaboration_input, project, repository_url, config_json, priority, timeout_ns, retry_json, tags_json, depends_on_json, parent_task_id, state, created_at, updated_at)
+ VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
+ t.ID, t.Name, t.Description, t.ElaborationInput, t.Project, t.RepositoryURL, string(configJSON), string(t.Priority),
+ t.Timeout.Duration.Nanoseconds(), string(retryJSON), string(tagsJSON), string(depsJSON),
+ t.ParentTaskID, string(t.State), t.CreatedAt.UTC(), t.UpdatedAt.UTC(),
+ ); err != nil {
+ return err
+ }
+```
+
+Replace it with:
+
+```go
+func (s *DB) CreateTask(t *task.Task) error {
+ configJSON, err := json.Marshal(t.Agent)
+ if err != nil {
+ return fmt.Errorf("marshaling config: %w", err)
+ }
+ retryJSON, err := json.Marshal(t.Retry)
+ if err != nil {
+ return fmt.Errorf("marshaling retry: %w", err)
+ }
+ tagsJSON, err := json.Marshal(t.Tags)
+ if err != nil {
+ return fmt.Errorf("marshaling tags: %w", err)
+ }
+ depsJSON, err := json.Marshal(t.DependsOn)
+ if err != nil {
+ return fmt.Errorf("marshaling depends_on: %w", err)
+ }
+ acceptanceCriteria := t.AcceptanceCriteria
+ if acceptanceCriteria == nil {
+ acceptanceCriteria = []string{}
+ }
+ acceptanceCriteriaJSON, err := json.Marshal(acceptanceCriteria)
+ if err != nil {
+ return fmt.Errorf("marshaling acceptance_criteria: %w", err)
+ }
+
+ tx, err := s.db.Begin()
+ if err != nil {
+ return err
+ }
+ defer tx.Rollback() //nolint:errcheck
+
+ if _, err = tx.Exec(`
+ INSERT INTO tasks (id, name, description, elaboration_input, project, repository_url, config_json, priority, timeout_ns, retry_json, tags_json, depends_on_json, acceptance_criteria_json, parent_task_id, state, created_at, updated_at)
+ VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
+ t.ID, t.Name, t.Description, t.ElaborationInput, t.Project, t.RepositoryURL, string(configJSON), string(t.Priority),
+ t.Timeout.Duration.Nanoseconds(), string(retryJSON), string(tagsJSON), string(depsJSON), string(acceptanceCriteriaJSON),
+ t.ParentTaskID, string(t.State), t.CreatedAt.UTC(), t.UpdatedAt.UTC(),
+ ); err != nil {
+ return err
+ }
+```
+
+- [ ] **Step 7: Update `scanTask` to read the new column**
+
+In the same file, find `scanTask`:
+
+```go
+func scanTask(row scanner) (*task.Task, error) {
+ var (
+ t task.Task
+ configJSON string
+ retryJSON string
+ tagsJSON string
+ depsJSON string
+ state string
+ priority string
+ timeoutNS int64
+ parentTaskID sql.NullString
+ elaborationInput sql.NullString
+ project sql.NullString
+ repositoryURL sql.NullString
+ rejectionComment sql.NullString
+ questionJSON sql.NullString
+ summary sql.NullString
+ interactionsJSON sql.NullString
+ )
+ err := row.Scan(
+ &t.ID, &t.Name, &t.Description, &elaborationInput, &project, &repositoryURL,
+ &configJSON, &priority, &timeoutNS, &retryJSON, &tagsJSON, &depsJSON,
+ &parentTaskID, &state, &t.CreatedAt, &t.UpdatedAt,
+ &rejectionComment, &questionJSON, &summary, &interactionsJSON, &t.NeedsReview,
+ )
+ t.ParentTaskID = parentTaskID.String
+ t.ElaborationInput = elaborationInput.String
+ t.Project = project.String
+ t.RepositoryURL = repositoryURL.String
+ t.RejectionComment = rejectionComment.String
+ t.QuestionJSON = questionJSON.String
+ t.Summary = summary.String
+ if err != nil {
+ return nil, err
+ }
+ t.State = task.State(state)
+ t.Priority = task.Priority(priority)
+ t.Timeout.Duration = time.Duration(timeoutNS)
+ if err := json.Unmarshal([]byte(configJSON), &t.Agent); err != nil {
+ return nil, fmt.Errorf("unmarshaling agent config: %w", err)
+ }
+ if err := json.Unmarshal([]byte(retryJSON), &t.Retry); err != nil {
+ return nil, fmt.Errorf("unmarshaling retry: %w", err)
+ }
+ if err := json.Unmarshal([]byte(tagsJSON), &t.Tags); err != nil {
+ return nil, fmt.Errorf("unmarshaling tags: %w", err)
+ }
+ if err := json.Unmarshal([]byte(depsJSON), &t.DependsOn); err != nil {
+ return nil, fmt.Errorf("unmarshaling depends_on: %w", err)
+ }
+ raw := interactionsJSON.String
+ if raw == "" {
+ raw = "[]"
+ }
+ if err := json.Unmarshal([]byte(raw), &t.Interactions); err != nil {
+ return nil, fmt.Errorf("unmarshaling interactions: %w", err)
+ }
+ return &t, nil
+}
+```
+
+Replace it with:
+
+```go
+func scanTask(row scanner) (*task.Task, error) {
+ var (
+ t task.Task
+ configJSON string
+ retryJSON string
+ tagsJSON string
+ depsJSON string
+ acceptanceCriteriaJSON string
+ state string
+ priority string
+ timeoutNS int64
+ parentTaskID sql.NullString
+ elaborationInput sql.NullString
+ project sql.NullString
+ repositoryURL sql.NullString
+ rejectionComment sql.NullString
+ questionJSON sql.NullString
+ summary sql.NullString
+ interactionsJSON sql.NullString
+ )
+ err := row.Scan(
+ &t.ID, &t.Name, &t.Description, &elaborationInput, &project, &repositoryURL,
+ &configJSON, &priority, &timeoutNS, &retryJSON, &tagsJSON, &depsJSON, &acceptanceCriteriaJSON,
+ &parentTaskID, &state, &t.CreatedAt, &t.UpdatedAt,
+ &rejectionComment, &questionJSON, &summary, &interactionsJSON, &t.NeedsReview,
+ )
+ t.ParentTaskID = parentTaskID.String
+ t.ElaborationInput = elaborationInput.String
+ t.Project = project.String
+ t.RepositoryURL = repositoryURL.String
+ t.RejectionComment = rejectionComment.String
+ t.QuestionJSON = questionJSON.String
+ t.Summary = summary.String
+ if err != nil {
+ return nil, err
+ }
+ t.State = task.State(state)
+ t.Priority = task.Priority(priority)
+ t.Timeout.Duration = time.Duration(timeoutNS)
+ if err := json.Unmarshal([]byte(configJSON), &t.Agent); err != nil {
+ return nil, fmt.Errorf("unmarshaling agent config: %w", err)
+ }
+ if err := json.Unmarshal([]byte(retryJSON), &t.Retry); err != nil {
+ return nil, fmt.Errorf("unmarshaling retry: %w", err)
+ }
+ if err := json.Unmarshal([]byte(tagsJSON), &t.Tags); err != nil {
+ return nil, fmt.Errorf("unmarshaling tags: %w", err)
+ }
+ if err := json.Unmarshal([]byte(depsJSON), &t.DependsOn); err != nil {
+ return nil, fmt.Errorf("unmarshaling depends_on: %w", err)
+ }
+ acRaw := acceptanceCriteriaJSON
+ if acRaw == "" {
+ acRaw = "[]"
+ }
+ if err := json.Unmarshal([]byte(acRaw), &t.AcceptanceCriteria); err != nil {
+ return nil, fmt.Errorf("unmarshaling acceptance_criteria: %w", err)
+ }
+ raw := interactionsJSON.String
+ if raw == "" {
+ raw = "[]"
+ }
+ if err := json.Unmarshal([]byte(raw), &t.Interactions); err != nil {
+ return nil, fmt.Errorf("unmarshaling interactions: %w", err)
+ }
+ return &t, nil
+}
+```
+
+- [ ] **Step 8: Run tests to verify they pass**
+
+Run: `go test ./internal/storage/ -run TestCreateTask -v`
+Expected: PASS, all `TestCreateTask_*` tests, including the two new ones and every pre-existing one (`TestCreateTask_AndGetTask`, `TestCreateTask_Project_RoundTrip`, `TestCreateTask_Subtask_EmitsSubtaskSpawnedOnParent`, `TestCreateTask_TopLevel_EmitsNoEvent`) — this proves `taskSelectColumns()`'s column order matches `scanTask`'s scan order exactly, and that no existing behavior broke. Paste the actual output.
+
+- [ ] **Step 9: Run the full package suite, then the full repo suite**
+
+Run: `go test ./internal/storage/...`
+Expected: PASS for everything — `ListTasks`/`ListSubtasks`/`ListDependents`/`ResetTaskForRetry` all have their own existing tests elsewhere in this package; if `taskSelectColumns()`'s column list or `scanTask`'s scan order is wrong, one of them will fail with a scan-type-mismatch error, not silently return wrong data.
+
+Then run: `go test ./...` (the entire repo). This must also 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.
+
+- [ ] **Step 10: Commit and push to `main` directly, matching this repo's existing workflow**
+
+```bash
+git add internal/task/task.go internal/storage/db.go internal/storage/db_test.go
+git commit -m "feat(task,storage): add Task.AcceptanceCriteria, mirroring story.Story's pattern"
+```
+
+## 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.
+
+---
+
+## Task 2: Expose `acceptance_criteria` on `spawn_subtask`
+
+**Files:**
+- Modify: `internal/agentchannel/agentchannel.go` (`SubtaskSpec`)
+- Modify: `internal/executor/channel.go` (`storeChannel.SpawnSubtask`)
+- Modify: `internal/agentloop/tools.go` (schema + case handler)
+- Modify: `internal/executor/agentmcp.go` (input struct + MCP tool)
+- Test: `internal/executor/channel_test.go` (append)
+
+**Interfaces:**
+- Consumes: `task.Task.AcceptanceCriteria` (Task 1).
+- Produces: nothing new for later work — this closes the `spawn_subtask` half of this plan's scope. The arbitrated-review generalization that actually *reads* a task's `AcceptanceCriteria` when evaluating it is a later, separate piece of the design spec's implementation order.
+
+- [ ] **Step 1: Add `AcceptanceCriteria` to `SubtaskSpec`**
+
+In `internal/agentchannel/agentchannel.go`, find the end of `SubtaskSpec`:
+
+```go
+ // DependsOn, when non-empty, names sibling task IDs (returned by a prior
+ // SpawnSubtask call in the same decomposition) this new subtask must wait
+ // for — set directly on the created child's task.DependsOn, so the
+ // existing dispatch-gating and cascade-fail-on-dependency-failure
+ // machinery (already built for top-level tasks) applies unchanged. Empty
+ // (the default) preserves today's behavior: every spawned subtask is
+ // structurally independent/parallel.
+ DependsOn []string
+}
+```
+
+Replace it with:
+
+```go
+ // DependsOn, when non-empty, names sibling task IDs (returned by a prior
+ // SpawnSubtask call in the same decomposition) this new subtask must wait
+ // for — set directly on the created child's task.DependsOn, so the
+ // existing dispatch-gating and cascade-fail-on-dependency-failure
+ // machinery (already built for top-level tasks) applies unchanged. Empty
+ // (the default) preserves today's behavior: every spawned subtask is
+ // structurally independent/parallel.
+ DependsOn []string
+ // AcceptanceCriteria, when non-empty, states what this subtask's work
+ // must satisfy — set directly on the created child's
+ // task.Task.AcceptanceCriteria. Empty (the default) leaves the child with
+ // no criteria of its own; a later phase's arbitrated-review
+ // generalization falls back to the enclosing story's AcceptanceCriteria
+ // in that case.
+ AcceptanceCriteria []string
+}
+```
+
+- [ ] **Step 2: Wire it through `storeChannel.SpawnSubtask`**
+
+In `internal/executor/channel.go`, find:
+
+```go
+ child := &task.Task{
+ ID: uuid.NewString(),
+ Name: spec.Name,
+ ParentTaskID: c.taskID,
+ Agent: agent,
+ Priority: task.PriorityNormal,
+ DependsOn: spec.DependsOn,
+ State: task.StatePending,
+ CreatedAt: now,
+ UpdatedAt: now,
+ }
+```
+
+Replace it with:
+
+```go
+ child := &task.Task{
+ ID: uuid.NewString(),
+ Name: spec.Name,
+ ParentTaskID: c.taskID,
+ Agent: agent,
+ Priority: task.PriorityNormal,
+ DependsOn: spec.DependsOn,
+ AcceptanceCriteria: spec.AcceptanceCriteria,
+ State: task.StatePending,
+ CreatedAt: now,
+ UpdatedAt: now,
+ }
+```
+
+- [ ] **Step 3: Write the failing tests**
+
+Append to `internal/executor/channel_test.go`, directly after `TestStoreChannel_SpawnSubtask_DependsOnDefaultsEmpty`:
+
+```go
+// TestStoreChannel_SpawnSubtask_SetsAcceptanceCriteria proves a caller can
+// state what a spawned subtask's work must satisfy.
+func TestStoreChannel_SpawnSubtask_SetsAcceptanceCriteria(t *testing.T) {
+ store := &fakeChannelStore{}
+ ch := newStoreChannel(store, "parent-task-1")
+
+ id, err := ch.SpawnSubtask(context.Background(), agentchannel.SubtaskSpec{
+ Name: "step 1",
+ Instructions: "do the thing",
+ AcceptanceCriteria: []string{"tests pass", "no new lint warnings"},
+ })
+ if err != nil {
+ t.Fatalf("spawn subtask: %v", err)
+ }
+
+ var got *task.Task
+ for _, ct := range store.createdTasks {
+ if ct.ID == id {
+ got = ct
+ }
+ }
+ if got == nil {
+ t.Fatal("subtask not found in store.createdTasks")
+ }
+ if len(got.AcceptanceCriteria) != 2 || got.AcceptanceCriteria[0] != "tests pass" || got.AcceptanceCriteria[1] != "no new lint warnings" {
+ t.Errorf("AcceptanceCriteria: want [tests pass, no new lint warnings], got %+v", got.AcceptanceCriteria)
+ }
+}
+
+// TestStoreChannel_SpawnSubtask_AcceptanceCriteriaDefaultsEmpty proves the
+// backward-compatible default: a caller that never sets AcceptanceCriteria
+// (every pre-existing caller) gets a child task with none, exactly as before
+// this change.
+func TestStoreChannel_SpawnSubtask_AcceptanceCriteriaDefaultsEmpty(t *testing.T) {
+ store := &fakeChannelStore{}
+ ch := newStoreChannel(store, "parent-task-1")
+
+ id, err := ch.SpawnSubtask(context.Background(), agentchannel.SubtaskSpec{
+ Name: "solo step",
+ Instructions: "do the thing",
+ })
+ if err != nil {
+ t.Fatalf("spawn subtask: %v", err)
+ }
+
+ var got *task.Task
+ for _, ct := range store.createdTasks {
+ if ct.ID == id {
+ got = ct
+ }
+ }
+ if got == nil {
+ t.Fatal("subtask not found in store.createdTasks")
+ }
+ if len(got.AcceptanceCriteria) != 0 {
+ t.Errorf("expected empty AcceptanceCriteria by default, got %v", got.AcceptanceCriteria)
+ }
+}
+```
+
+- [ ] **Step 4: Run tests to verify they fail, then implement, then verify they pass**
+
+Run: `go test ./internal/executor/ -run TestStoreChannel_SpawnSubtask_.*AcceptanceCriteria -v`
+Expected: FAIL (compile error, `SubtaskSpec` has no `AcceptanceCriteria` field) — this is the point where you actually make Steps 1-2's edits above if you haven't yet, then re-run and expect PASS. Paste both the failing and passing output.
+
+- [ ] **Step 5: Add the tool definition and case handler to the native tool-use loop**
+
+In `internal/agentloop/tools.go`, find the `spawn_subtask` tool definition:
+
+```go
+ {
+ Name: "spawn_subtask",
+ Description: "Create a child task to be executed separately. Use this to break large work into focused pieces.",
+ ParametersJSONSchema: map[string]any{
+ "type": "object",
+ "properties": map[string]any{
+ "name": strProp("short descriptive name for the subtask"),
+ "instructions": strProp("complete instructions for the subtask agent"),
+ "model": strProp("optional model override"),
+ "max_budget_usd": map[string]any{"type": "number", "description": "optional budget cap in USD"},
+ "role": strProp("optional role name to dispatch the subtask through instead of a fixed model (e.g. an evaluator role); when set, model is ignored and the role's escalation ladder picks the provider/model"),
+ "depends_on": map[string]any{"type": "array", "items": map[string]any{"type": "string"}, "description": "optional list of sibling subtask IDs (returned by prior spawn_subtask calls in this same decomposition) this subtask must wait for before it can run"},
+ },
+ "required": []string{"name", "instructions"},
+ },
+ },
+```
+
+Replace it with:
+
+```go
+ {
+ Name: "spawn_subtask",
+ Description: "Create a child task to be executed separately. Use this to break large work into focused pieces.",
+ ParametersJSONSchema: map[string]any{
+ "type": "object",
+ "properties": map[string]any{
+ "name": strProp("short descriptive name for the subtask"),
+ "instructions": strProp("complete instructions for the subtask agent"),
+ "model": strProp("optional model override"),
+ "max_budget_usd": map[string]any{"type": "number", "description": "optional budget cap in USD"},
+ "role": strProp("optional role name to dispatch the subtask through instead of a fixed model (e.g. an evaluator role); when set, model is ignored and the role's escalation ladder picks the provider/model"),
+ "depends_on": map[string]any{"type": "array", "items": map[string]any{"type": "string"}, "description": "optional list of sibling subtask IDs (returned by prior spawn_subtask calls in this same decomposition) this subtask must wait for before it can run"},
+ "acceptance_criteria": map[string]any{"type": "array", "items": map[string]any{"type": "string"}, "description": "optional list of concrete criteria this subtask's work must satisfy"},
+ },
+ "required": []string{"name", "instructions"},
+ },
+ },
+```
+
+In the same file, find the `spawn_subtask` case handler:
+
+```go
+ case "spawn_subtask":
+ var a struct {
+ Name string `json:"name"`
+ Instructions string `json:"instructions"`
+ Model string `json:"model"`
+ MaxBudgetUSD float64 `json:"max_budget_usd"`
+ Role string `json:"role"`
+ DependsOn []string `json:"depends_on"`
+ }
+ _ = json.Unmarshal([]byte(argsJSON), &a)
+ id, ssErr := l.Channel.SpawnSubtask(ctx, agentchannel.SubtaskSpec{
+ Name: a.Name,
+ Instructions: a.Instructions,
+ Model: a.Model,
+ MaxBudgetUSD: a.MaxBudgetUSD,
+ Role: a.Role,
+ DependsOn: a.DependsOn,
+ })
+```
+
+Replace it with:
+
+```go
+ case "spawn_subtask":
+ var a struct {
+ Name string `json:"name"`
+ Instructions string `json:"instructions"`
+ Model string `json:"model"`
+ MaxBudgetUSD float64 `json:"max_budget_usd"`
+ Role string `json:"role"`
+ DependsOn []string `json:"depends_on"`
+ AcceptanceCriteria []string `json:"acceptance_criteria"`
+ }
+ _ = json.Unmarshal([]byte(argsJSON), &a)
+ id, ssErr := l.Channel.SpawnSubtask(ctx, agentchannel.SubtaskSpec{
+ Name: a.Name,
+ Instructions: a.Instructions,
+ Model: a.Model,
+ MaxBudgetUSD: a.MaxBudgetUSD,
+ Role: a.Role,
+ DependsOn: a.DependsOn,
+ AcceptanceCriteria: a.AcceptanceCriteria,
+ })
+```
+
+- [ ] **Step 6: Add the MCP input field and wire the registration**
+
+In `internal/executor/agentmcp.go`, find `spawnSubtaskInput`:
+
+```go
+type spawnSubtaskInput struct {
+ Name string `json:"name" jsonschema:"short descriptive name for the subtask"`
+ Instructions string `json:"instructions" jsonschema:"complete instructions for the subtask agent"`
+ Model string `json:"model,omitempty" jsonschema:"optional model override, e.g. sonnet or opus"`
+ MaxBudgetUSD float64 `json:"max_budget_usd,omitempty" jsonschema:"optional budget cap in USD"`
+ Role string `json:"role,omitempty" jsonschema:"optional role name to dispatch the subtask through instead of a fixed model (e.g. an evaluator role); when set, model is ignored and the role's escalation ladder picks the provider/model"`
+ DependsOn []string `json:"depends_on,omitempty" jsonschema:"optional list of sibling subtask IDs (returned by prior spawn_subtask calls in this same decomposition) this subtask must wait for before it can run"`
+}
+```
+
+Replace it with:
+
+```go
+type spawnSubtaskInput struct {
+ Name string `json:"name" jsonschema:"short descriptive name for the subtask"`
+ Instructions string `json:"instructions" jsonschema:"complete instructions for the subtask agent"`
+ Model string `json:"model,omitempty" jsonschema:"optional model override, e.g. sonnet or opus"`
+ MaxBudgetUSD float64 `json:"max_budget_usd,omitempty" jsonschema:"optional budget cap in USD"`
+ Role string `json:"role,omitempty" jsonschema:"optional role name to dispatch the subtask through instead of a fixed model (e.g. an evaluator role); when set, model is ignored and the role's escalation ladder picks the provider/model"`
+ DependsOn []string `json:"depends_on,omitempty" jsonschema:"optional list of sibling subtask IDs (returned by prior spawn_subtask calls in this same decomposition) this subtask must wait for before it can run"`
+ AcceptanceCriteria []string `json:"acceptance_criteria,omitempty" jsonschema:"optional list of concrete criteria this subtask's work must satisfy"`
+}
+```
+
+In the same file, find the `spawn_subtask` `mcp.AddTool` call:
+
+```go
+ mcp.AddTool(s, &mcp.Tool{
+ Name: "spawn_subtask",
+ Description: "Create a child task to be executed separately. Use this to break large work into focused pieces, then finish your turn.",
+ }, func(ctx context.Context, _ *mcp.CallToolRequest, in spawnSubtaskInput) (*mcp.CallToolResult, any, error) {
+ id, err := ch.SpawnSubtask(ctx, SubtaskSpec{
+ Name: in.Name,
+ Instructions: in.Instructions,
+ Model: in.Model,
+ MaxBudgetUSD: in.MaxBudgetUSD,
+ Role: in.Role,
+ DependsOn: in.DependsOn,
+ })
+ if err != nil {
+ return nil, nil, err
+ }
+ return textResult("Created subtask " + id), nil, nil
+ })
+```
+
+Replace it with:
+
+```go
+ mcp.AddTool(s, &mcp.Tool{
+ Name: "spawn_subtask",
+ Description: "Create a child task to be executed separately. Use this to break large work into focused pieces, then finish your turn.",
+ }, func(ctx context.Context, _ *mcp.CallToolRequest, in spawnSubtaskInput) (*mcp.CallToolResult, any, error) {
+ id, err := ch.SpawnSubtask(ctx, SubtaskSpec{
+ Name: in.Name,
+ Instructions: in.Instructions,
+ Model: in.Model,
+ MaxBudgetUSD: in.MaxBudgetUSD,
+ Role: in.Role,
+ DependsOn: in.DependsOn,
+ AcceptanceCriteria: in.AcceptanceCriteria,
+ })
+ if err != nil {
+ return nil, nil, err
+ }
+ return textResult("Created subtask " + id), nil, nil
+ })
+```
+
+- [ ] **Step 7: Run tests to verify they pass**
+
+Run: `go test ./internal/agentloop/... ./internal/executor/...`
+Expected: PASS for everything, no new tests needed for `tools.go`/`agentmcp.go` themselves (the underlying `SpawnSubtask` behavior is already covered by Task 2's own `channel_test.go` tests — this mirrors how `depends_on`'s exposure on both transports needed no new tests of its own in the earlier plan that added it).
+
+- [ ] **Step 8: Run the full package suite, then the full repo suite**
+
+Run: `go test ./internal/agentchannel/... ./internal/executor/... ./internal/agentloop/...`
+Expected: PASS for everything.
+
+Then run: `go test ./...` (the entire repo). This must also pass before you commit — 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.
+
+- [ ] **Step 9: Commit and push to `main` directly, matching this repo's existing workflow**
+
+```bash
+git add internal/agentchannel/agentchannel.go internal/executor/channel.go internal/executor/channel_test.go internal/agentloop/tools.go internal/executor/agentmcp.go
+git commit -m "feat(agentchannel,executor,agentloop): expose acceptance_criteria on spawn_subtask"
+```
+
+## 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 "AcceptanceCriteria" internal/` to visually confirm every file in both tasks' File Maps was actually touched.
generated by cgit v1.2.3 (git 2.39.1) at 2026-07-16 09:05:39 +0000