Skip to content

Commit 7ebca4e

Browse files
wongkclaude
andcommitted
feat(code-review): in-process run-prefix segment runner + subprocess A/B parity oracle (PLN-1229 Phase 1)
Collapse the deterministic prefix (stages 01→cache_check) from one orchestrator turn per stage into a single resumable process. Production (code_review_helpers.py): - cmd_run_prefix: reads run_plan.json + setup.json, resolves each stage's <ANGLE_BRACKET> tokens from prior-stage artifacts, dispatches cmd_* in-process with the stage's stdout redirect, honors on_failure + validation gates, and pauses at the genuine decision points — Gate A (hygiene_only), a PLN-725 singleton needs_agent, or the Phase-1 boundary (stage_17_partition → ready_for_route). Resumable: the depends_on completed set is reconstructed from artifacts on disk, so each --resume-from segment is a fresh process. - _execute_stage_inprocess mirrors the start.md Walker Contract steps 1-5; continue_with_coverage_gap emits a canonical agent-failure system finding. - Registered run-prefix in config/cli.json (46th subparser). Parity oracle (prefix_golden_harness.py + test_prefix_golden.py): - Subprocess A-side (one python3 helpers.py <stage> per stage, real > redirect) vs B-side (python3 helpers.py run-prefix per segment). A and B implement the walk wrapper independently, so a shared wrapper bug cannot hide. Byte-equal (normalized) artifacts through cache_check across all 7 P0-B fixtures, plus a pause-sequence test pinning the resumable 3-segment contract. - A failing gh stub shadows PATH so subprocess stages take the deterministic no-PR branch; env is snapshotted inside the hermetic HOME redirect. P0-C contract tests (test_code_review_helpers.py, 31 tests): token resolution, completed-set reconstruction, singleton needs_agent detection, on_failure abort/continue incl. coverage-gap emission, and cmd_run_prefix error/boundary returns. SCHEMA.md §7b documents the run-prefix result contract. Zero orchestrator changes (start.md untouched) — validated purely by the parity oracle. Route + partition (Segment 3) fold in at Phase 2. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 341c9be commit 7ebca4e

8 files changed

Lines changed: 1554 additions & 23 deletions

File tree

plugins/code-review/.claude-plugin/plugin.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"name": "code-review",
33
"description": "Code review plugin",
4-
"version": "3.1.1",
4+
"version": "3.2.0",
55
"author": {
66
"name": "ClosedLoop",
77
"email": "support@closedloop.ai"

plugins/code-review/SCHEMA.md

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -493,6 +493,49 @@ Stages from plans 01/03/05/06 are present in `run_plan.json` but marked
493493

494494
---
495495

496+
## 7b. `run-prefix` result contract (PLN-1229)
497+
498+
`run-prefix` runs the deterministic prefix (stages 01→`cache_check`) in ONE
499+
process instead of one orchestrator turn per stage. It reads `run_plan.json` +
500+
`setup.json` from `--cr-dir`, walks from `--resume-from` (default: the first
501+
plan stage), and stops at the next genuine decision point — emitting a status
502+
JSON (to stdout, or `--output <path>`) that tells the orchestrator what to do
503+
next. The runner is **resumable**: after handling a pause the orchestrator
504+
re-invokes `run-prefix --resume-from <resume_stage>`. Because each segment is a
505+
fresh process, the `depends_on` `completed` set is reconstructed from artifacts
506+
on disk (a prior stage counts as done iff its literal `expected_outputs` exist).
507+
508+
**Result fields:**
509+
510+
| Field | Type | Meaning |
511+
| --------------- | --------------- | ----------------------------------------------------------------------- |
512+
| `next_action` | string (enum) | The pause reason — authoritative (read this, not the exit code). |
513+
| `resume_stage` | string \| null | The stage id to pass as `--resume-from` on the next invocation. |
514+
| `singleton` | string \| null | `"extract_signals"` \| `"coverage_critic"` when `needs_singleton`. |
515+
| `failed_stage` | string \| null | The aborting stage id when `next_action == "error"`. |
516+
| `ran_stages` | string[] | Stage ids executed (or `continue`-failed) this segment, in order. |
517+
| `message` | string \| null | Short diagnostic on `error`, else null. |
518+
519+
**`next_action` values:**
520+
521+
| Value | Fires at | Orchestrator does next |
522+
| ------------------- | ------------------------------------- | ---------------------------------------------------------------------------- |
523+
| `needs_singleton` | `stage_11` / `stage_15` `needs_agent` | Spawn the `singleton` agent, write its output, re-invoke from `resume_stage`. |
524+
| `hygiene_exit` | Gate A (`hygiene_only` after hygiene) | Present hygiene findings and stop (no verdict/footer). |
525+
| `ready_for_route` | reaching `stage_17_partition` | Run Gate B (`route`) + partition + the rest of the walk. |
526+
| `error` | a stage aborted / a gate failed | Fall back to the per-stage walk from `failed_stage`; partials are preserved. |
527+
528+
The exit code is `0` for every well-formed result (including `error`) — the
529+
`next_action` field is the contract. Route + partition (Segment 3) fold into the
530+
runner in Phase 2, at which point `ready_for_route` becomes `ready_for_reviewers`
531+
(carrying `fast_path` + `cache_status_message`). The `on_failure` policy of each
532+
stage is honored exactly as the Walker Contract prescribes: `abort``error`;
533+
`continue` → proceed; `continue_with_coverage_gap` → proceed after writing an
534+
`agent-failure` system finding to `agent_<stage>-failed.json` (collected by
535+
`collect-findings`).
536+
537+
---
538+
496539
## 8. Determinism tiers (PLN-719 Section 8)
497540

498541
| Tier | Definition | Required reviewers may depend? |

0 commit comments

Comments
 (0)