From 17ad3ba6ef3bf44ffce8276ee6cfa9b847e2225a Mon Sep 17 00:00:00 2001 From: Harald Hoyer Date: Thu, 7 May 2026 06:42:46 +0200 Subject: [PATCH] refactor(opencode): hoist dispatch rules into a top-level Dispatch Hygiene section MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- config/opencode/commands/workflow.md | 112 +++++++++++++++++++-------- 1 file changed, 78 insertions(+), 34 deletions(-) diff --git a/config/opencode/commands/workflow.md b/config/opencode/commands/workflow.md index f1b21ae..af99cbe 100644 --- a/config/opencode/commands/workflow.md +++ b/config/opencode/commands/workflow.md @@ -66,11 +66,72 @@ The plan should include: **Skip Test Design for:** Config-only changes, decorator swaps, import reorganization, documentation. 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 && …` | 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 -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: - `@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. -### 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 ~5–10 lines of context, function signatures of helpers `@make` will need to call, the file's relevant import block. -- **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. +- The existing code being replaced (verbatim, marked as "current state"), with ~5–10 lines of surrounding context +- 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). -### Finalized-Text Rule - -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. +Apply **Dispatch Hygiene** to each task spec before dispatch in Phase 7. --- ## Phase 6: Write Tests +Apply **Dispatch Hygiene** to each `@test` prompt before sending. + For each task from Phase 5, dispatch `@test` with: - The task spec (acceptance criteria, code context, files to modify) - The Test Design section from the plan (if provided) @@ -226,7 +283,7 @@ Rust integration tests live in a separate test crate (`tests/.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. - **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. - - **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. 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. @@ -242,26 +299,13 @@ The stub pass and the body pass each produce their own atomic commit (per Phase ## 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: -- The task spec (from Phase 5, finalized — see Finalized-Text Rule) -- Relevant code context (seam-revealing snippets only — see Code Context Anti-patterns) +- The task spec (from Phase 5, finalized per Dispatch Hygiene) +- 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-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 && …` | `@make` cannot `cd`. Rewrite to use absolute paths or `git -C ` 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: 1. Entry validation: run tests, verify RED, check failure codes match handoff 2. Implement minimal code to make tests pass (GREEN) @@ -287,7 +331,7 @@ After all tasks complete, verify overall integration: ## 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: - The original plan