diff --git a/plugins/review-suite/scripts/review.py b/plugins/review-suite/scripts/review.py index c3c1501..cb504f9 100644 --- a/plugins/review-suite/scripts/review.py +++ b/plugins/review-suite/scripts/review.py @@ -148,6 +148,10 @@ def build_parser() -> argparse.ArgumentParser: parser.add_argument("--reason") parser.add_argument("--cd") parser.add_argument("--base", help="Override the detected default branch ref.") + parser.add_argument( + "--review-brief", + help="Optional Markdown goal and constraints, frozen for this review cycle.", + ) parser.add_argument("--decision", choices=(DECISION_CLEAN, DECISION_FINDINGS)) parser.add_argument("--github-review", action="store_true") parser.add_argument("--github-force", action="store_true") @@ -684,7 +688,11 @@ def _load_cycle_and_state_dir( def _reject_id_creation_args(args: argparse.Namespace, state: dict[str, Any]) -> None: - sent = [name for name in ("mode", "cd", "base") if getattr(args, name) is not None] + sent = [ + name + for name in ("mode", "cd", "base", "review_brief") + if getattr(args, name) is not None + ] if not sent: return mode = dict(state.get("mode") or {}) @@ -702,6 +710,15 @@ def _reject_id_creation_args(args: argparse.Namespace, state: dict[str, Any]) -> ) +def _reject_review_brief_replacement( + state: dict[str, Any], review_brief: str | None +) -> None: + if review_brief is not None and review_brief != state.get("review_brief"): + raise ValueError( + "review brief is frozen for this cycle; start a new cycle to replace it" + ) + + def _restart_reason(args: argparse.Namespace) -> str: reason = str(args.reason or "").strip() if not reason or reason == "REASON": @@ -1151,6 +1168,10 @@ def _show_status(state: dict[str, Any], *, state_dir: Path) -> int: payload: dict[str, Any] = { "review": state.get("public_id"), "status": state.get("stage") or "unknown", + "review_brief": "available" if state.get("review_brief") else "unavailable", + "design_conformance_context": "available" + if state.get("review_brief") + else "unavailable", } if mode := _mode_label(state): payload["mode"] = mode @@ -1798,6 +1819,7 @@ def _compatible_continuation_cycle( head: str, merge_base_head: str, effective_mode: str, + review_brief: str | None = None, skip_deslop: bool = False, ) -> dict[str, Any] | None: normalized_cwd = normalize_cwd(str(review_root)) @@ -1882,8 +1904,10 @@ def _compatible_continuation_cycle( f"rerun with --id for one of: {', '.join(public_ids)}" ) selected_base_drift = selected_candidates[0][3] + selected_state = selected_candidates[0][2] + _reject_review_brief_replacement(selected_state, review_brief) resumed = _with_current_identity( - selected_candidates[0][2], + selected_state, head=head, merge_base_head=merge_base_head, base_drift=selected_base_drift, @@ -1922,6 +1946,7 @@ def _create_or_resume_cycle( head=head, merge_base_head=merge_base_head, effective_mode=resolution.effective_mode, + review_brief=args.review_brief, skip_deslop=skip_deslop, ) if continuation is not None: @@ -1939,6 +1964,7 @@ def _create_or_resume_cycle( deslop_enabled=profile_deslop_enabled and not skip_deslop, deslop_skip_source="cli" if skip_deslop else None, cycle_token="skip-deslop" if skip_deslop else None, + review_brief=args.review_brief, ) if str(base_info["requested_base"]) != base: state["identity"]["requested_base"] = str(base_info["requested_base"]) @@ -1948,6 +1974,7 @@ def _create_or_resume_cycle( state = _apply_runtime_options(state, args) existing = load_cycle_by_key(state_dir, str(state["cycle_key"])) if existing is not None: + _reject_review_brief_replacement(existing, args.review_brief) return _apply_runtime_options(existing, args) return _apply_profile_resolution(state, resolution) @@ -2064,6 +2091,7 @@ def _create_successor_cycle( else resolution.profile.deslop_enabled, deslop_skip_source=deslop_skip_source, restart_token=restart_token, + review_brief=state.get("review_brief"), ) existing = load_cycle_by_key(state_dir, str(replacement["cycle_key"])) if existing is not None: @@ -2606,6 +2634,10 @@ def _action_payload(state: dict[str, Any], *, state_dir: Path) -> dict[str, Any] def _render(state: dict[str, Any], *, state_dir: Path) -> None: payload: dict[str, Any] = { "review": state.get("public_id"), + "review_brief": "available" if state.get("review_brief") else "unavailable", + "design_conformance_context": "available" + if state.get("review_brief") + else "unavailable", } action = _action_payload(state, state_dir=state_dir) summary = _add_review_ladder_fields(payload, state, action) @@ -2710,6 +2742,7 @@ def main() -> int: or args.validation_note or args.deslop_done or args.skip_deslop + or args.review_brief or args.show_findings or args.show_status ): diff --git a/plugins/review-suite/scripts/review_suite_core/orchestrator_state.py b/plugins/review-suite/scripts/review_suite_core/orchestrator_state.py index 9ce6c0b..54445de 100644 --- a/plugins/review-suite/scripts/review_suite_core/orchestrator_state.py +++ b/plugins/review-suite/scripts/review_suite_core/orchestrator_state.py @@ -385,6 +385,7 @@ def create_cycle( deslop_skip_source: str | None = None, cycle_token: str | None = None, restart_token: str | None = None, + review_brief: str | None = None, ) -> dict[str, Any]: identity = normalize_cycle_identity( cwd=cwd, base=base, branch=branch, head=head, merge_base=merge_base @@ -412,6 +413,8 @@ def create_cycle( if fresh_token is not None and restart is not None: raise ValueError("cycle_token cannot be combined with restart_token") key_token = restart or fresh_token + if review_brief is not None and not review_brief.strip(): + raise ValueError("review_brief cannot be blank") state = { "schema_version": ORCHESTRATOR_STATE_SCHEMA_VERSION, "cycle_key": cycle_key( @@ -431,6 +434,7 @@ def create_cycle( "requested": requested_selection, "effective": resolved_selection, }, + "review_brief": review_brief, "stage": STAGE_CREATED, "pending_action": None, "deslop": { diff --git a/plugins/review-suite/tests/test_review_orchestrator_cli.py b/plugins/review-suite/tests/test_review_orchestrator_cli.py index feb9e59..4726d14 100644 --- a/plugins/review-suite/tests/test_review_orchestrator_cli.py +++ b/plugins/review-suite/tests/test_review_orchestrator_cli.py @@ -1556,6 +1556,73 @@ def test_create_with_skip_deslop_runs_review_without_sidecar( assert state["rounds"][0]["round_id"] == "phase_review-round-1" +def test_review_brief_is_frozen_and_public_output_reports_coverage( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + _stub_review(monkeypatch) + repo = tmp_path / "repo" + state_dir = tmp_path / "state" + _init_repo(repo) + _commit_file(repo, "app.txt", "base\n", "base") + + _, created = _run_review( + monkeypatch, + [ + "--mode", + "fast", + "--review-brief", + "# Goal\n\nKeep it neutral.", + "--cd", + str(repo), + "--base", + "main", + "--state-dir", + str(state_dir), + ], + ) + public_id = str(created["review"]) + + assert _cycle_payload(state_dir, public_id)["review_brief"] == ( + "# Goal\n\nKeep it neutral." + ) + assert created["review_brief"] == "available" + assert created["design_conformance_context"] == "available" + + errors: list[str] = [] + monkeypatch.setattr( + review, + "emit_error", + lambda message, **kwargs: errors.append(str(message)) or 2, + ) + monkeypatch.setattr( + sys, + "argv", + [ + "review.py", + "--mode", + "fast", + "--review-brief", + "Changed goal", + "--cd", + str(repo), + "--base", + "main", + ], + ) + + assert review.main() == 2 + assert errors[-1] == ( + "review brief is frozen for this cycle; start a new cycle to replace it" + ) + + terminal = _cycle_payload(state_dir, public_id) + terminal["stage"] = "aborted" + _write_cycle_payload(state_dir, public_id, terminal) + errors.clear() + assert review.main() == 2 + assert "review brief is frozen" in errors[-1] + + def test_skip_deslop_does_not_resume_same_head_sidecar_cycle( monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: @@ -2109,6 +2176,8 @@ def fail_deslop(*, command: list[str], cwd: Path) -> subprocess.CompletedProcess ) assert payload["rounds"] == 1 assert payload["deslop"] == "skipped-fast" + assert payload["review_brief"] == "unavailable" + assert payload["design_conformance_context"] == "unavailable" assert dict(payload["worktree"]) == { "branch": "feature/show-status", "head": str(dict(before_state["identity"])["head"])[:12], diff --git a/plugins/review-suite/tests/test_review_orchestrator_state.py b/plugins/review-suite/tests/test_review_orchestrator_state.py index b83c1d9..008158e 100644 --- a/plugins/review-suite/tests/test_review_orchestrator_state.py +++ b/plugins/review-suite/tests/test_review_orchestrator_state.py @@ -129,6 +129,31 @@ def test_create_cycle_is_compact_json_state_keyed_by_normalized_inputs( } +def test_create_cycle_preserves_exact_optional_review_brief(tmp_path: Path) -> None: + brief = "# Goal\n\nKeep the provider-neutral contract.\n" + + briefed = create_cycle( + cwd=tmp_path / "repo", + base="main", + branch="feature/brief", + head="head-1", + merge_base="base-1", + requested_mode="fast", + review_brief=brief, + ) + briefless = create_cycle( + cwd=tmp_path / "repo", + base="main", + branch="feature/brief", + head="head-1", + merge_base="base-1", + requested_mode="fast", + ) + + assert briefed["review_brief"] == brief + assert briefless["review_brief"] is None + + def test_mark_deslop_closed_disables_tracked_sidecar_and_leaves_fast_untracked( tmp_path: Path, ) -> None: