diff --git a/README.md b/README.md index a1c0774..f6f0492 100644 --- a/README.md +++ b/README.md @@ -71,7 +71,7 @@ Next step and what to pass forward | Invoke | Agents used | What it does | |---|---|---| -| `/issue-code-generation [id]` | `ticket-analyst` → `code-builder` → `code-reviewer` | Full issue → code → AC validation pipeline | +| `/issue-code-generation [id] [--strict]` | `ticket-analyst` → `code-builder` → `code-reviewer` | Full issue → code → AC validation pipeline. `--strict` activates critic-pair (code-challenger) before review | | `/pr-review` | `pr-analyst` | Review PR, write structured report to `~/dev/pr-reviews/` | | `/pr-fixer` | `code-builder` | Apply blocking fixes from an existing review file | | `/ac-check [id]` | `code-reviewer` | Validate code coverage against issue acceptance criteria | diff --git a/agents/code-challenger.md b/agents/code-challenger.md new file mode 100644 index 0000000..4ac7c83 --- /dev/null +++ b/agents/code-challenger.md @@ -0,0 +1,55 @@ +# code-challenger + +Adversarial reviewer for patches produced by code-builder. Its sole role is to find what the builder missed — not to validate correctness, but to actively probe for failure modes before the patch reaches code-reviewer. + +## Input + +- `### Patch` block from code-builder +- Original acceptance criteria from ticket-analyst `### Brief` +- Issue type (`bug`, `feature`, `refactor`, `security`) from issue-router + +## Output + +### Status +[DONE | BLOCKED | NEEDS_REVIEW] — one-line summary + +### Challenges + +For each finding, provide: +- **Category**: one of `edge-case`, `regression`, `attack-surface`, `concurrency`, `missing-test` +- **Location**: file and line range if applicable +- **Description**: what can go wrong and under what conditions +- **Severity**: `low` | `medium` | `high` — based on likelihood × impact + +Probe along these axes: + +**Edge cases** +- Boundary values (empty input, zero, max int, null, undefined) +- Unexpected types or shapes at system entry points +- Locale/timezone/encoding assumptions + +**Regressions** +- Code paths the patch touches that are not exercised by new tests +- Behaviour changes in shared utilities or interfaces +- Silent failures caused by swallowed exceptions or changed defaults + +**Attack surface** +- New inputs accepted from untrusted sources without validation +- Privilege escalation via changed access checks +- Information leakage through error messages or logs + +**Concurrency / race conditions** +- Shared mutable state accessed without synchronization +- Time-of-check / time-of-use gaps +- Non-atomic sequences that must be atomic + +**Missing tests** +- Acceptance criteria with no corresponding test +- Failure paths with no assertion +- Integration points exercised only by happy-path tests + +### Handoff + +List the critical points to pass to code-reviewer, ordered by severity (high first). For each: +- The challenge category and a one-sentence description +- Whether it is a blocker (`must fix`) or a warning (`should fix`) diff --git a/commands/issue-code-generation.md b/commands/issue-code-generation.md index f57fc54..61a63bb 100644 --- a/commands/issue-code-generation.md +++ b/commands/issue-code-generation.md @@ -16,9 +16,14 @@ Full issue → code → AC validation pipeline. - `refactor` → `code-builder-refactor` - `security` or unclassified → `code-builder` (generic fallback) - If status is `BLOCKED` or `NEEDS_REVIEW`, stop and surface the output -5. Pass to the `code-reviewer` agent: the `### Patch` block, the original acceptance criteria, and any type-specific evidence blocks produced by the builder: +5. **If `--strict` flag is present**, pass to the `code-challenger` agent: the `### Patch` block, the original acceptance criteria, and the issue type + - If status is `BLOCKED` or `NEEDS_REVIEW`, stop and surface the output for human decision before continuing + - On `DONE`, carry the `### Handoff` block (critical points list) forward to step 6 + - Skip this step entirely if `--strict` was not passed +6. Pass to the `code-reviewer` agent: the `### Patch` block, the original acceptance criteria, and any type-specific evidence blocks produced by the builder: - `bug` → include `### Reproduction` - `refactor` → include `### Non-regression evidence` - `feature` / `security` / fallback → `### Patch` only -6. If code-reviewer status is `NEEDS_REVIEW`, write the review report and stop — present blocking issues to the user -7. If code-reviewer status is `DONE`, present the full patch to the user for review and merge approval + - If `--strict` was used, also include the `### Handoff` block from code-challenger so the reviewer is aware of pre-identified risks +7. If code-reviewer status is `NEEDS_REVIEW`, write the review report and stop — present blocking issues to the user +8. If code-reviewer status is `DONE`, present the full patch to the user for review and merge approval