diff --git a/skills/implement/SKILL.md b/skills/implement/SKILL.md index 22358f3..7077108 100644 --- a/skills/implement/SKILL.md +++ b/skills/implement/SKILL.md @@ -1,150 +1,140 @@ --- name: implement -description: Use when an implementation plan exists in docs/plans/ and is ready to execute. Dispatches one fresh subagent per task with two-stage review (spec compliance, then code quality). Continuous execution; stops only on BLOCKED status or completion. Also runs in mini-mode for bug fixes handed off by debug. +description: Use when an implementation plan exists in docs/plans/ and is ready to execute, OR when a debug RED-test is handed off for a bugfix. Dispatches the ailang-implement-orchestrator agent, which runs the entire per-task loop (implementer → spec-reviewer → quality-reviewer) on an isolated branch `iter/`, writes a per-iter journal + stats file, and returns a compressed end-report. The Boss reads the end-report, optionally rewrites the journal Summary section, appends one line to docs/journals/INDEX.md, and rebases onto main. --- -# implement — plan execution with two-stage review +# implement — plan execution via a dedicated orchestrator-agent > **Violating the letter of these rules is violating the spirit.** ## Overview -Plan execution is delegated work. The orchestrator (me) reads the -plan once, dispatches a fresh subagent per task, and reviews each -task's output through two independent checks (spec compliance, then -code quality) before moving on. Subagents have isolated context; the -orchestrator only sees their reports. This is what gives AILang's -development cycle its context budget back. +Plan execution is fully delegated. The Boss-Orchestrator dispatches +ONE subagent (`ailang-implement-orchestrator`), which runs the entire +per-task loop in its own context: implementer → spec-reviewer → +quality-reviewer, per task, with the Iron Law and re-loop limits +below. All commits land on a dedicated branch `iter/`. +The Boss sees one ≤500-token end-report and a per-iter journal file +on the branch. + +This skill body is intentionally short. The procedural details of +the per-task loop live in +`skills/implement/agents/ailang-implement-orchestrator.md`. The +**canonical discipline** (Iron Law, sub-status table, common +rationalisations) lives in this file and is read by the +orchestrator-agent every dispatch. ## When to Use / Skipping Triggers: -- A plan exists at `docs/plans/.md` and is ready to run. +- A plan exists at `docs/plans/.md` (standard mode). - A `debug` skill has produced a RED test + cause and is handing off - for the GREEN side (mini-mode, no plan file). + for the GREEN side (mini mode). **Never skipped** when there is code to ship. Trivial mechanical edits (one-line typo fix, schema rename across N files) MAY be -handled inline by the orchestrator without dispatching subagents, -per CLAUDE.md "trivial mechanical edits" carve-out — but the -review-and-commit discipline still applies. +handled inline by the Boss without dispatch, per CLAUDE.md "trivial +mechanical edits" carve-out — but no review-and-commit discipline +is shed. ## The Iron Law ``` -FRESH SUBAGENT PER TASK -TWO-STAGE REVIEW: SPEC COMPLIANCE FIRST, CODE QUALITY SECOND -NEVER START QUALITY REVIEW BEFORE SPEC COMPLIANCE IS GREEN -NEVER PUSH PAST `BLOCKED` BY HAND +ONE BRANCH PER ITER — `iter/`, created from origin/main. +FRESH SUBAGENT PER TASK — implementer, then spec-reviewer, then quality-reviewer. +TWO-STAGE REVIEW: SPEC COMPLIANCE FIRST, CODE QUALITY SECOND. +NEVER START QUALITY REVIEW BEFORE SPEC COMPLIANCE IS GREEN. +NEVER PUSH PAST `BLOCKED` BY HAND. +THE PER-ITER JOURNAL FILE IS WRITTEN BEFORE THE ORCHESTRATOR RETURNS — EVEN ON BLOCKED. ``` -## The Process — standard mode (plan-driven) +## Per-task sub-status mechanics -### Step 1 — Load plan once, extract everything +The orchestrator-agent reads and follows this table verbatim every +dispatch: -Read `docs/plans/.md` ONCE. Extract every task with full -text. Create TodoWrite entries for each task. Note any cross-task -context (shared types, file structure decisions). +| Sub-status | Orchestrator action | +|------------|---------------------| +| `DONE` | next task / next phase | +| `DONE_WITH_CONCERNS` | accumulate concern, next | +| `NEEDS_CONTEXT` (1st–2nd) | re-dispatch with expanded carrier | +| `NEEDS_CONTEXT` (3rd) | stop → `BLOCKED` to Boss, reason `context-exhausted` | +| `non_compliant` / `changes_requested` (1st–2nd) | implementer re-dispatch with the reviewer's report as repair brief | +| `non_compliant` / `changes_requested` (3rd) | stop → `BLOCKED` to Boss, reason `review-loop-exhausted` | +| `BLOCKED` (worker) | stop → `BLOCKED` to Boss, reason verbatim | +| `unclear` (spec-reviewer) | stop → `BLOCKED` to Boss, reason `spec-ambiguous` | +| Tool / infra error | stop → `BLOCKED` to Boss, reason `infra` + raw error | -The plan is NOT re-read by subagents. The orchestrator hands each -subagent the full task text plus surrounding scene-set context. +Re-loop limit: 2 retries per failure-mode per task. The 3rd is +`BLOCKED` to the Boss. `skip task K, continue` is intentionally NOT +a mode — tasks have implicit ordering dependencies and the +orchestrator does not know the dependency graph. -### Step 2 — Per-task loop +## The Process — Boss side -For each task in TodoWrite order: +### Step 1 — Dispatch the orchestrator-agent -#### 2.1 — Dispatch implementer +For a standard iteration: -Dispatch `ailang-implementer` with: -- the full task text from the plan -- scene-set: parent milestone, where this task fits, dependencies - on earlier tasks -- output expectations: build green, tests green, structured report +``` +Agent("ailang-implement-orchestrator", { + mode: "standard", + iter_id: "", // e.g. "ct.2.3", "or.1", "23.4" + plan_path: "docs/plans/.md", + task_range: [3, 8] // optional +}) +``` -#### 2.2 — Handle implementer status +For a debug-handoff (mini mode): -The implementer reports one of: +``` +Agent("ailang-implement-orchestrator", { + mode: "mini", + iter_id: "bugfix-", + red_test_path: "", + cause_summary: "<1-2 sentences from debugger>", + constraint: "minimal fix, no surrounding cleanup" +}) +``` -| Status | Action | -|--------|--------| -| `DONE` | Proceed to spec compliance review (Step 2.3). | -| `DONE_WITH_CONCERNS` | Read concerns first. If correctness/scope, address before review. If observation, note + proceed. | -| `NEEDS_CONTEXT` | Provide missing context, redispatch. Max 2 retries with the same model before escalating. | -| `BLOCKED` | Diagnose: context problem? → more context. Reasoning? → stronger model. Task too large? → split + re-plan. Plan wrong? → escalate to user. **Never** push past BLOCKED by hand. | +### Step 2 — Read the end-report -#### 2.3 — Spec compliance review +The orchestrator returns a ≤500-token plain-text report (see the +agent's "Output format — end-report" section for the fixed +structure). Read it. The end-report is the only thing that costs the +Boss-context tokens; per-task chatter has stayed inside the +orchestrator-agent. -Dispatch `ailang-spec-reviewer` with: -- `task_text`: the verbatim text from the plan (same text the - implementer received) -- `diff`: the implementer's diff (or `pre_task_sha` + `head_sha`) -- `status_from_implementer`: `DONE` or `DONE_WITH_CONCERNS` (with - the concerns quoted) +### Step 3 — Boss merge step (on DONE / PARTIAL with partial DONE-tasks) -The reviewer reports `compliant` / `non_compliant` / `unclear` / -`infra_blocked`. +1. Open the per-iter journal file on the branch: + `git show iter/:docs/journals/-iter-.md` + (or check out the branch if a closer look is needed). +2. Accept the agent's Summary section as-is, OR rewrite it with + Boss-level framing. The rest of the journal file is factual and + preserved verbatim. +3. Append one line to `docs/journals/INDEX.md`: + `- YYYY-MM-DD — iter : → -iter-.md` +4. Rebase the iter branch onto main: + `git rebase iter/ onto main` (or fast-forward if linear). +5. Optionally delete the branch: `git branch -D iter/`. +6. If trigger is done-state and the user is away, write a + `docs/WhatsNew.md` entry + `notify.sh` per CLAUDE.md's + "Done-state notifications" subsection. -If `non_compliant`: implementer (same subagent role) fixes the missing -requirements / removes the unrequested extras; re-dispatch the spec -reviewer; loop until `compliant`. +### Step 4 — Boss handling (on BLOCKED) -If `unclear`: the task text is ambiguous. Resolve at the orchestrator -level — either by re-stating the task to the implementer with the -ambiguity removed, or by bouncing back to `planner` if the spec itself -is the source of the ambiguity. - -#### 2.4 — Code quality review - -ONLY after spec compliance is `compliant`, dispatch -`ailang-quality-reviewer` with: -- `diff` (or SHAs) -- `spec_review_status`: must be `compliant` (otherwise quality review - refuses to run) -- `task_subject`: short title for context (NOT the full task text — - quality review doesn't care about spec compliance) - -The reviewer reports `approved` / `changes_requested` / `infra_blocked`. - -If `changes_requested`: implementer fixes the `Important` and `Minor` -issues; `Nit` items are advisory. Re-dispatch the quality reviewer; -loop until `approved`. - -#### 2.5 — Mark task complete - -TodoWrite update. Move to next task. - -### Step 3 — E2E coverage - -After all tasks: dispatch `ailang-tester` to add or extend E2E tests -that protect the milestone's invariants. The tester reads the spec -and the recent commits to identify what to cover. - -### Step 4 — JOURNAL entry - -Append the iteration entry: `## YYYY-MM-DD — Iteration : `, -including: -- one-paragraph summary -- per-task subjects (mirrors commit messages) -- known debt (anything deliberately not touched) - -Commit pattern matches existing JOURNAL style: -`iter <X>.<n>: <subject>` for task commits. - -## The Process — mini-mode (debug handoff) - -When invoked from `skills/debug` with a RED-test path + cause -summary: - -1. Skip plan loading (no plan file). -2. Dispatch `ailang-implementer` with: - - "make this RED test pass: `<path>`" - - cause summary from `debug` - - hard constraint: *minimal fix, no surrounding cleanup* -3. Spec compliance review: "does this fix the RED test without - touching anything outside the bug's scope?" -4. Code quality review: standard. -5. Commit pattern: `fix: <symptom>` (single commit, GREEN side). -6. JOURNAL entry: `## YYYY-MM-DD — Bug fix: <symptom>`. +1. Read the per-iter journal on the branch — `Blocked detail:` names + the failure mode. +2. Decide: + - **Repair:** adjust plan or extend context; re-dispatch the + orchestrator with the same `iter_id` and a `task_range` covering + the remaining tasks. The orchestrator picks up the existing + branch. + - **Discard:** `git branch -D iter/<iter_id>`. INDEX.md is NOT + touched (the orchestrator did not append). + - **Escalate:** ask the user via `notify.sh`. The branch sits + until the conversation resumes. ## Handoff Contract @@ -152,59 +142,53 @@ summary: | Source | Carrier | |--------|---------| -| from `planner` | path to `docs/plans/<iteration>.md` + optional task focus | +| from `planner` | path to `docs/plans/<iteration>.md` (+ optional `task_range`) | | from `debug` | RED-test path + cause summary + minimal-fix constraint | -`implement` produces: per-task commits + JOURNAL entry. No further -hand-off — `audit` runs independently at milestone close. - -## Model Selection - -Use the cheapest model that handles each role. - -| Task complexity | Model | -|-----------------|-------| -| 1-2 files, complete spec, mechanical | cheap/fast | -| Multi-file integration, judgement | standard | -| Architecture, design, review | most capable | - -Reviewer subagents typically need at least the standard model. +`implement` produces: a feature branch `iter/<iter_id>` carrying +per-task commits, a per-iter journal file (`docs/journals/<file>.md`), +and a stats file (`bench/orchestrator-stats/<file>.json`). The Boss +merges and updates `docs/journals/INDEX.md`. No further hand-off — +`audit` runs independently at milestone close. ## Common Rationalisations | Excuse | Reality | |--------|---------| -| "Implementer self-reviewed, skip spec compliance" | Self-review is intent, not outcome. Past iters showed self-review misses spec drift and quality issues that compound. | -| "Task is trivial, skip quality review" | The marginal cost of one subagent dispatch is trivial vs. catching a quality issue three iters later. | -| "Working on `main` directly is fine for this iter" | Branch first, reset main to pre-iter, cherry-pick onto feature branch. Defeats rollback discipline if not. | -| "DONE_WITH_CONCERNS, move on" | Read the concerns first. If correctness/scope, address before review. If observation, note in JOURNAL and proceed. | -| "Reviewer is the same model as implementer, dispatching twice is redundant" | Fresh context per dispatch is the point, not the model identity. The reviewer doesn't see the implementer's reasoning, only its diff. | -| "BLOCKED with 'task too large' — let me push through" | Split + re-plan. Pushing through wastes context and produces low-quality output. | +| "Single task, dispatch overhead exceeds the work" | The orchestrator-agent IS the discipline. A single dispatch is cheap; the per-task review loop, the branch isolation, and the journal file are the value. | +| "I'll merge on main directly, no need for the iter branch" | The branch is what makes parallel `/implement` activity safe and makes "discard on BLOCKED" trivial. Skipping it puts you back in the pre-2026-05-11 coordination problems. | +| "BLOCKED end-report, let me dig into the branch and continue myself" | Read the `Blocked detail:` first. The orchestrator stopped at the re-loop limit for a reason. Continuing by hand undoes the discipline. | +| "End-report says PARTIAL with 4/5 DONE — close to enough" | The 5th task may carry an invariant the earlier 4 silently depend on. Either re-dispatch for the missing task or treat the iter as incomplete. | +| "Skip the INDEX line, the journal file is enough" | INDEX.md is the only thing that makes per-iter journals navigable. Without it, future agents have to ls the directory and parse filenames. | +| "I'll let the orchestrator-agent update INDEX.md" | No. INDEX.md is Boss-only. The orchestrator-agent does not know what `<one-line title>` the Boss will pick. | ## Red Flags — STOP -- Quality review running before spec compliance is ✅ -- Two reviewers dispatched in parallel (must be sequential) -- Subagent re-reading the plan file (controller curates context) -- Direct edit to `main` without feature branch -- Skipping JOURNAL entry "because the commits are self-explanatory" -- "I'll do the E2E coverage in the next iter" +- Boss dispatching `ailang-implementer` directly (bypassing the + orchestrator-agent). +- Boss editing a per-iter journal file on `main` instead of on its + iter branch. +- Two `/implement` runs sharing one `iter_id`. +- INDEX.md modified by anything other than the Boss. +- Branch `iter/<iter_id>` pushed to remote before the merge step + decided what to do with it. +- End-report longer than ~500 tokens. ## Cross-references -- **Agents dispatched:** - - `skills/implement/agents/ailang-implementer.md` — task execution - (carries TDD discipline as an independent layer; falls back to - RED-first even when the plan template forgot) - - `skills/implement/agents/ailang-spec-reviewer.md` — Step 2.3, did - the diff match the task text? - - `skills/implement/agents/ailang-quality-reviewer.md` — Step 2.4, - is the diff well-built? - - `skills/implement/agents/ailang-tester.md` — Step 3, E2E coverage +- **Agent dispatched:** + `skills/implement/agents/ailang-implement-orchestrator.md` — + carries the per-task loop, including dispatch of: + - `skills/implement/agents/ailang-implementer.md` (Phase 2.1) + - `skills/implement/agents/ailang-spec-reviewer.md` (Phase 2.3) + - `skills/implement/agents/ailang-quality-reviewer.md` (Phase 2.4) + - `skills/implement/agents/ailang-tester.md` (Phase 3) - **Input sources:** - `skills/planner/SKILL.md` — produces the plan files this skill consumes - `skills/debug/SKILL.md` — produces the RED-test handoff for mini-mode -- **Output target:** orchestrator (me) reads JOURNAL entry and - commits; `audit` runs at milestone close. +- **Output target:** Boss reads the end-report and the per-iter + journal; `audit` runs at milestone close. +- **Documented exception:** in `skills/README.md`, this is the named + exception to "agents do not call other agents".