refactor(opencode): hoist dispatch rules into a top-level Dispatch Hygiene section

A workflow run on GAL-38 dispatched a plan to @check that contained a
self-contradicting "Wait, the movement should be direct position
assignment, not delta… Let me reconsider…" passage with two versions
of the same move_enemies code, plus drop-in cargo-pasted match arms /
function bodies (plan-as-implementation). The rules added in 832306c
caught these patterns when @make was the recipient but did not cover
the plan itself, plan-review dispatches, test-author dispatches, or
final-review dispatches.

Hoists the Finalized-Text Rule and Pre-Dispatch Validation table out
of Phase 5/7 into a new top-level "Dispatch Hygiene" section between
Phase 3 and Phase 4, and adds an explicit "No-Implementation-in-Plan-
or-Spec Rule" that bans drop-in code blocks > ~5 lines, full function
bodies, and stage-by-stage transformations from plans and specs alike.

Phases 3, 4, 5, 6, 7, 8 each gain a one-line pointer requiring the
orchestrator to apply Dispatch Hygiene before sending. Phase 5's
former "Code Context Anti-patterns" becomes "Code Context — what to
include" with positive framing, deferring the negative list to the
hoisted rules. The Phase 6 stub-first section's stale anti-pattern
reference is updated to point at Dispatch Hygiene as well.
This commit is contained in:
Harald Hoyer 2026-05-07 06:42:46 +02:00
parent d47bb6e15b
commit 17ad3ba6ef

View file

@ -66,11 +66,72 @@ The plan should include:
**Skip Test Design for:** Config-only changes, decorator swaps, import reorganization, documentation. **Skip Test Design for:** Config-only changes, decorator swaps, import reorganization, documentation.
When skipped, `@test` derives test cases directly from acceptance criteria. When skipped, `@test` derives test cases directly from acceptance criteria.
After drafting, apply **Dispatch Hygiene** (below) to the plan — it is a dispatch artifact and gets sent to `@check`/`@simplify` in Phase 4.
---
## Dispatch Hygiene
This applies to **every** subagent dispatch (Phases 4, 6, 7, 8) **and** to artifacts that will be dispatched (the plan from Phase 3, the task specs from Phase 5). Apply these checks before sending — fix the artifact, then re-check.
### Finalized-Text Rule
The artifact must be **finalized** — single-author text, no contradictions, no open questions. Forbidden:
- "Actually, that's wrong — let me correct…"
- "Wait, let me reconsider…"
- Two versions of the same code block, one labelled "corrected" or appearing after a revision pass
- Open questions or ambiguities the orchestrator hasn't resolved
- Mid-text revisions visible to the recipient
If you find yourself revising while writing, stop, redo the artifact from scratch with the corrected understanding, and only then dispatch. Subagents are fresh-context — they cannot reliably resolve which of two contradictory drafts is canonical, and reviewers cannot give a clean verdict on a self-contradicting plan.
### No-Implementation-in-Plan-or-Spec Rule
Plans (Phase 3) and task specs (Phase 5) are **not** the place to write the answer. They describe what to do; `@make` writes how.
Provide:
- Approach with rationale
- Files to modify with brief descriptions
- Function signatures, type declarations, data shapes (structure, not logic)
- Constraints, invariants, integration contracts
- Risks and edge cases
Do **not** provide:
- Drop-in code blocks longer than ~5 lines that constitute "the answer"
- Full function bodies for the changes being planned
- Complete `match` arms / branch logic / loop bodies for new behavior
- Pre-written test bodies (those come from `@test`, or from `@make` for Rust unit-only)
- Stage-by-stage code transformations spelled out as ready-to-commit diffs
If you've already written the implementation in the plan or spec, the artifact has overstepped. Convert finished code into structural description (signature + intent) and let `@make` produce the body.
**Allowed in plans/specs:**
- Existing code being replaced, marked as "current state"
- Function signatures and type/struct/enum declarations (data, not logic)
- Tiny inline constants (`pub const FOO: f32 = 30.0;`)
- Test specifications as one-line behavior descriptions ("input X → expect Y")
### Pre-Dispatch Validation (MANDATORY)
Scan the artifact and reject (revise, retry) if any of the following are present:
| Check | Why it matters |
|---|---|
| `bash -c`, `sh -c`, `zsh -c`, `fish -c` (anywhere, including inside `nix develop --command bash -c …`) | `@make`/`@test` sandboxes deny all `*-c` shell invocations and any nested `bash` would bypass the per-command allowlist. Replace with one direct command per line: `nix develop -c cargo check`, etc. |
| `nix develop --command bash` / `nix develop -c bash` / `nix develop -c sh` | Same — inner shell escapes the sandbox. Wrap each toolchain command directly. |
| Any `cd <path> && …` | Subagents cannot `cd`. Rewrite to use absolute paths. |
| Code blocks longer than ~5 lines that draft the answer | Violates No-Implementation-in-Plan-or-Spec. Trim to structure (signature + "current state" only). |
| Two versions of the same code, "actually let me correct…", or open questions | Violates the Finalized-Text Rule. Redo the artifact. |
| Test bodies inside `@make` specs when tests are coming from `@test` | Duplicates the TDD handoff. |
If any check trips, **do not dispatch.** Fix and re-validate. Repeated trips on a single task signal a Phase 5 split problem — go back and split.
--- ---
## Phase 4: Review Plan ## Phase 4: Review Plan
Dispatch `@check` and `@simplify` in parallel to review the plan. Apply **Dispatch Hygiene** to the plan and to each reviewer prompt before sending. Dispatch `@check` and `@simplify` in parallel to review the plan.
Reviewers should evaluate testability: Reviewers should evaluate testability:
- `@check`: Is the design testable? Are the right behaviors identified? (Review Framework §8) - `@check`: Is the design testable? Are the right behaviors identified? (Review Framework §8)
@ -140,30 +201,26 @@ When a task fails the heuristic, split into:
Each split is dispatched separately to `@make` and verified before the next. Each split is dispatched separately to `@make` and verified before the next.
### Code Context Anti-patterns ### Code Context — what to include
The **Code Context** field exists so `@make` can find the seam to modify, not so it can read off a finished answer. Strictly follow: The **Code Context** field exists so `@make` can find the seam to modify. Provide:
- **Provide:** the existing code being replaced (verbatim), the surrounding ~510 lines of context, function signatures of helpers `@make` will need to call, the file's relevant import block. - The existing code being replaced (verbatim, marked as "current state"), with ~510 lines of surrounding context
- **Do NOT provide:** a complete drop-in replacement, the new function bodies, the test bodies (those come from `@test` or — for unit-only Rust — from `@make` itself per Phase 6), or any "here is what to write" code block longer than ~5 lines. - Function signatures of helpers `@make` will need to call
- The file's relevant import block
For everything you must **not** include — drop-in replacements, full function bodies, pre-written test bodies, "here is what to write" — see **Dispatch Hygiene → No-Implementation-in-Plan-or-Spec Rule** above.
If the task is so well-specified that you've already written the implementation, the task is too small for `@make` (apply it directly) or you've over-determined the design (revisit Phase 3). If the task is so well-specified that you've already written the implementation, the task is too small for `@make` (apply it directly) or you've over-determined the design (revisit Phase 3).
### Finalized-Text Rule Apply **Dispatch Hygiene** to each task spec before dispatch in Phase 7.
Each task spec must be **finalized** before dispatch — single-author text with no contradictions. **Forbidden in dispatch prompts:**
- "Actually, that's wrong — let me correct…"
- "Wait, let me revise…"
- Two versions of the same code block with one labelled "corrected"
- Open questions or ambiguities the orchestrator hasn't resolved
If you find yourself revising while writing the spec, stop, redo the spec from scratch with the corrected understanding, and only then dispatch. `@make` is a fresh-context implementer; it cannot reliably resolve which of two contradictory drafts is canonical.
--- ---
## Phase 6: Write Tests ## Phase 6: Write Tests
Apply **Dispatch Hygiene** to each `@test` prompt before sending.
For each task from Phase 5, dispatch `@test` with: For each task from Phase 5, dispatch `@test` with:
- The task spec (acceptance criteria, code context, files to modify) - The task spec (acceptance criteria, code context, files to modify)
- The Test Design section from the plan (if provided) - The Test Design section from the plan (if provided)
@ -226,7 +283,7 @@ Rust integration tests live in a separate test crate (`tests/<feature>.rs`) that
- **Stubs only:** every function body is exactly `todo!()`. Every method body is exactly `todo!()`. Public structs may use `pub struct Foo;` or `pub struct Foo { /* fields TBD */ }` — but no logic. - **Stubs only:** every function body is exactly `todo!()`. Every method body is exactly `todo!()`. Public structs may use `pub struct Foo;` or `pub struct Foo { /* fields TBD */ }` — but no logic.
- **Signatures must match the planned final API exactly** (return types, lifetimes, generics) — otherwise the integration test will mismatch later. Lift signatures from the Phase 3 plan / Phase 5 task spec. - **Signatures must match the planned final API exactly** (return types, lifetimes, generics) — otherwise the integration test will mismatch later. Lift signatures from the Phase 3 plan / Phase 5 task spec.
- **Acceptance criteria:** `cargo check` (wrapped in `nix develop -c …` if the project has a devshell) passes; no test command is run. - **Acceptance criteria:** `cargo check` (wrapped in `nix develop -c …` if the project has a devshell) passes; no test command is run.
- **Code Context Anti-patterns still apply:** the stub pass is small and finalized — no draft bodies, no contradictory signatures. - **Dispatch Hygiene still applies:** the stub pass is small and finalized — no draft bodies, no contradictory signatures.
2. Verify `cargo check` passed in `@make`'s output. If not, fix and re-dispatch the stub pass before continuing. 2. Verify `cargo check` passed in `@make`'s output. If not, fix and re-dispatch the stub pass before continuing.
3. Dispatch `@test` as normal. The integration test now compiles; running it panics on `todo!()` at runtime, which is a clean `MISSING_BEHAVIOR` RED with a stack trace — far better than the build-error-RED form. 3. Dispatch `@test` as normal. The integration test now compiles; running it panics on `todo!()` at runtime, which is a clean `MISSING_BEHAVIOR` RED with a stack trace — far better than the build-error-RED form.
4. Continue to Phase 7's body pass (`@make` in TDD mode), where the same files are revisited and the `todo!()` bodies are replaced. 4. Continue to Phase 7's body pass (`@make` in TDD mode), where the same files are revisited and the `todo!()` bodies are replaced.
@ -242,26 +299,13 @@ The stub pass and the body pass each produce their own atomic commit (per Phase
## Phase 7: Implement ## Phase 7: Implement
Apply **Dispatch Hygiene** to each `@make` spec before sending. Repeated trips on a single task signal a Phase 5 split problem — go back and split.
Execute each task by dispatching `@make` with: Execute each task by dispatching `@make` with:
- The task spec (from Phase 5, finalized — see Finalized-Text Rule) - The task spec (from Phase 5, finalized per Dispatch Hygiene)
- Relevant code context (seam-revealing snippets only — see Code Context Anti-patterns) - Relevant code context (seam-revealing snippets only — see Phase 5 "Code Context — what to include")
- **Pre-written failing tests and handoff from `@test` (if TESTS_READY)** - **Pre-written failing tests and handoff from `@test` (if TESTS_READY)**
### Pre-Dispatch Validation (MANDATORY)
Before sending the spec to `@make`, scan it and reject (revise, then retry) if any of the following are present:
| Check | Why it matters |
|---|---|
| `bash -c`, `sh -c`, `zsh -c`, `fish -c` (anywhere, including inside `nix develop --command bash -c …`) | `@make`'s sandbox denies all `*-c` shell invocations and any nested `bash` would bypass the per-command allowlist. Replace with one direct command per line: `nix develop -c cargo check`, `nix develop -c cargo test`, etc. |
| `nix develop --command bash` / `nix develop -c bash` / `nix develop -c sh` | Same — the inner shell escapes the sandbox. Wrap each toolchain command directly. |
| Any `cd <path> && …` | `@make` cannot `cd`. Rewrite to use absolute paths or `git -C <path>` for git operations (and `@make` doesn't run git anyway). |
| Code blocks longer than ~5 lines under "Code Context" or labelled as the answer | Violates Code Context Anti-patterns. Trim to the seam. |
| Two versions of the same code, "actually let me correct…", or open questions | Violates the Finalized-Text Rule. Redo the spec. |
| Test bodies inside the `@make` spec when tests are coming from `@test` | The TDD handoff already provides them; duplicating creates conflict. |
If any check trips, **do not dispatch.** Fix the spec and re-validate. Repeated trips on the same task signal a Phase 5 split problem — go back and split.
`@make` runs in TDD mode when tests are provided: `@make` runs in TDD mode when tests are provided:
1. Entry validation: run tests, verify RED, check failure codes match handoff 1. Entry validation: run tests, verify RED, check failure codes match handoff
2. Implement minimal code to make tests pass (GREEN) 2. Implement minimal code to make tests pass (GREEN)
@ -287,7 +331,7 @@ After all tasks complete, verify overall integration:
## Phase 8: Final Review ## Phase 8: Final Review
Dispatch `@check` and `@simplify` in parallel to review the full implementation (all changes across all files). Apply **Dispatch Hygiene** to each reviewer prompt before sending. Dispatch `@check` and `@simplify` in parallel to review the full implementation (all changes across all files).
Provide reviewers with: Provide reviewers with:
- The original plan - The original plan