diff options
| -rw-r--r-- | docs/superpowers/plans/2026-07-09-task-acceptance-criteria.md | 863 |
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. |