Problem
When reviewers discover test gaps for code authored on the current branch, those findings are classified as TODO (T-level). At medium effort, the orchestrator's effort table defers TODOs ("ticket, defer"), so the code ships without test coverage for new behavior.
#379 / #380 addressed this by propagating test criteria through AC (design-and-plan → planner → coder → reviewer). But the fix has two weaknesses:
-
The test reviewer's classification guardrail undermines the rule it supports. The clause "do not infer a test requirement where none was stated" gives the reviewer an escape hatch: it interprets "covered by tests" loosely, treats additional gaps as T rather than unmet AC (F), and those gaps get deferred.
-
The coder's test obligation is gated on AC. The coder writes tests "when a step's acceptance criteria include test coverage" — making test-writing conditional on upstream propagation rather than a standing obligation.
Context
The core insight is about authorship responsibility:
- We wrote the code (
orchestrate-dev): untested branch-authored behavior is an F — we should have written the tests.
- We didn't write the code (
orchestrate-review / standalone): untested branch-authored behavior is a T — it's the author's discretion.
The test reviewer currently has no way to distinguish these contexts. It needs a signal from the orchestrator indicating whether the code under review was authored by the pipeline.
Solution
Four targeted changes that replace the layered, AC-dependent guidance with clean standalone rules:
-
aspect-test-reviewer — rewrite the test-gap classification guidance. Replace the hedging guardrail with a clean rule conditioned on a new dispatch-time input: when told the code is pipeline-authored, untested branch-authored behavior is F; otherwise T. The reviewer should stand on its own — the input provides categorical clarity, but the classification logic is self-contained.
-
orchestrated-coder — make the test obligation unconditional. Replace "when a step's acceptance criteria include test coverage, write those tests" with a standing obligation: write tests for changed behavior as part of each implementation step. The coder already has the testing-conventions skill; it should follow it regardless of AC.
-
orchestrate/modules/review-cycle.md — add an input variable ({authored-by-pipeline}) and include a conditional line in the test reviewer's dispatch prompt when true.
-
orchestrate/SKILL.md — derive {authored-by-pipeline} from the pipeline spec (true when implementation is in the pipeline) and pass it to the review-cycle module.
Acceptance criteria
Problem
When reviewers discover test gaps for code authored on the current branch, those findings are classified as TODO (T-level). At medium effort, the orchestrator's effort table defers TODOs ("ticket, defer"), so the code ships without test coverage for new behavior.
#379 / #380 addressed this by propagating test criteria through AC (design-and-plan → planner → coder → reviewer). But the fix has two weaknesses:
The test reviewer's classification guardrail undermines the rule it supports. The clause "do not infer a test requirement where none was stated" gives the reviewer an escape hatch: it interprets "covered by tests" loosely, treats additional gaps as T rather than unmet AC (F), and those gaps get deferred.
The coder's test obligation is gated on AC. The coder writes tests "when a step's acceptance criteria include test coverage" — making test-writing conditional on upstream propagation rather than a standing obligation.
Context
The core insight is about authorship responsibility:
orchestrate-dev): untested branch-authored behavior is an F — we should have written the tests.orchestrate-review/ standalone): untested branch-authored behavior is a T — it's the author's discretion.The test reviewer currently has no way to distinguish these contexts. It needs a signal from the orchestrator indicating whether the code under review was authored by the pipeline.
Solution
Four targeted changes that replace the layered, AC-dependent guidance with clean standalone rules:
aspect-test-reviewer — rewrite the test-gap classification guidance. Replace the hedging guardrail with a clean rule conditioned on a new dispatch-time input: when told the code is pipeline-authored, untested branch-authored behavior is F; otherwise T. The reviewer should stand on its own — the input provides categorical clarity, but the classification logic is self-contained.
orchestrated-coder — make the test obligation unconditional. Replace "when a step's acceptance criteria include test coverage, write those tests" with a standing obligation: write tests for changed behavior as part of each implementation step. The coder already has the
testing-conventionsskill; it should follow it regardless of AC.orchestrate/modules/review-cycle.md — add an input variable (
{authored-by-pipeline}) and include a conditional line in the test reviewer's dispatch prompt whentrue.orchestrate/SKILL.md — derive
{authored-by-pipeline}from the pipeline spec (true whenimplementationis in the pipeline) and pass it to the review-cycle module.Acceptance criteria