diff --git a/README.md b/README.md
index ffaec94..de7a5b9 100644
--- a/README.md
+++ b/README.md
@@ -20,9 +20,9 @@ codex plugin marketplace upgrade review-suite
| Mode | Use it for | Local review |
| --- | --- | --- |
-| `fast` | UI-only, local presentation, and other small, well-tested changes | Dual Sol medium; no deslop, Arena, or GitHub review; at most two local rounds |
-| `normal` | Everything else | Deslop sidecar, configured phase Arena rounds when enabled, then dual Sol medium until green and GitHub review |
-| `deep` | Billing, login/auth, security, business-critical systems, database integrity/migrations, concurrency, and similarly critical logic | Deslop sidecar, dual Sol medium until green, configured deep Arena rounds when enabled, dual Sol xhigh until green, then GitHub review |
+| `fast` | UI-only, local presentation, and other small, well-tested changes | Dual Sol medium until green, then bounded exact-head closure; no Arena or GitHub review; at most two local rounds |
+| `normal` | Everything else | Configured phase Arena rounds when enabled, dual Sol medium until green, bounded exact-head closure, then GitHub review |
+| `deep` | Billing, login/auth, security, business-critical systems, database integrity/migrations, concurrency, and similarly critical logic | Dual Sol medium until green, configured deep Arena rounds when enabled, dual Sol xhigh until green, bounded exact-head closure, then GitHub review |
These are risk heuristics, not permission to downgrade a UI-looking change that
crosses a trust or data-integrity boundary.
diff --git a/plugins/review-suite/references/default_config.json b/plugins/review-suite/references/default_config.json
index e3b566f..6fcbd8a 100644
--- a/plugins/review-suite/references/default_config.json
+++ b/plugins/review-suite/references/default_config.json
@@ -128,14 +128,12 @@
"profiles": {
"stable": {
"normal": {
- "deslop_enabled": true,
"steps": [
{"kind": "arena", "name": "arena-phase-review", "lane": "review_t1", "task_class": "phase_review", "pool": "arena_phase", "loop_ref": "normal_arena_loops", "enabled_ref": "arena.enabled"},
{"name": "precision-signoff", "count": 2, "model_ref": "signoff_normal_model", "rerun_on_findings": true}
]
},
"deep": {
- "deslop_enabled": true,
"steps": [
{"name": "precision-signoff", "count": 2, "model_ref": "signoff_normal_model", "rerun_on_findings": true},
{"kind": "arena", "name": "arena-pr-review", "lane": "review_t3", "task_class": "pr_review", "pool": "arena_deep", "loop_ref": "deep_arena_loops", "enabled_ref": "arena.enabled"},
@@ -143,7 +141,6 @@
]
},
"fast": {
- "deslop_enabled": false,
"steps": [
{"name": "fast-signoff", "count": 2, "model_ref": "signoff_normal_model", "max_review_rounds": 2}
]
diff --git a/plugins/review-suite/scripts/review.py b/plugins/review-suite/scripts/review.py
index 6acedea..f7e1384 100644
--- a/plugins/review-suite/scripts/review.py
+++ b/plugins/review-suite/scripts/review.py
@@ -76,7 +76,6 @@
STAGE_RETRY_REQUESTED,
STAGE_REVIEW_GREEN,
STAGE_ABORTED,
- DESLOP_STATUS_CLOSED,
DESLOP_STATUS_DONE,
DESLOP_STATUS_SKIPPED,
create_cycle,
@@ -192,7 +191,7 @@ def build_parser() -> argparse.ArgumentParser:
"--no-deslop",
dest="skip_deslop",
action="store_true",
- help="Skip the deslop sidecar when creating a review cycle.",
+ help="Skip bounded exact-head closure when creating a review cycle.",
)
parser.add_argument(
"--show-findings",
@@ -996,6 +995,15 @@ def add_round_id(round_id: str) -> None:
def _show_findings(state: dict[str, Any], *, state_dir: Path) -> int:
public_id = str(state.get("public_id") or "").strip()
+ closure_findings = str(
+ dict(state.get("deslop") or {}).get("findings") or ""
+ ).strip()
+ if closure_findings:
+ write_text(f"review: {public_id}")
+ write_text("")
+ write_text("Output:")
+ write_text(closure_findings)
+ return 0
candidates = _output_round_candidates(state)
if not candidates:
write_text(f"review: {public_id}")
@@ -1206,6 +1214,8 @@ def _show_status(state: dict[str, Any], *, state_dir: Path) -> int:
deslop = dict(state.get("deslop") or {})
if deslop_status := str(deslop.get("status") or "").strip():
payload["deslop"] = deslop_status
+ if conformance := str(deslop.get("conformance") or "").strip():
+ payload["conformance"] = conformance
if validation := _validation_summary(state):
payload["validation"] = validation
github_review = dict(state.get("github_review") or {})
@@ -1613,6 +1623,7 @@ def _resume_progress(
base_drift=base_drift,
)
next_state = _with_equivalent_base_drift_review_head(next_state, base_drift)
+ next_state["github_review"] = {"status": "unknown"}
return mark_latest_profile_step_rerun_needed(next_state, head=head)
if stage != STAGE_FIX_PENDING:
return state
@@ -1968,8 +1979,7 @@ def _create_or_resume_cycle(
resolution = resolve_orchestrator_profile(
config, mode=mode, selection=_configured_selection(config)
)
- profile_deslop_enabled = bool(resolution.profile.deslop_enabled)
- skip_deslop = bool(args.skip_deslop) and profile_deslop_enabled
+ skip_deslop = bool(args.skip_deslop)
continuation = _compatible_continuation_cycle(
state_dir=state_dir,
review_root=review_root,
@@ -1993,7 +2003,7 @@ def _create_or_resume_cycle(
effective_mode=resolution.effective_mode,
selection=resolution.requested_selection,
effective_selection=resolution.effective_selection,
- deslop_enabled=profile_deslop_enabled and not skip_deslop,
+ deslop_enabled=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,
@@ -2118,9 +2128,7 @@ def _create_successor_cycle(
effective_mode=resolution.effective_mode,
selection=resolution.requested_selection,
effective_selection=resolution.effective_selection,
- deslop_enabled=False
- if source_skipped_deslop
- else resolution.profile.deslop_enabled,
+ deslop_enabled=not source_skipped_deslop,
deslop_skip_source=deslop_skip_source,
restart_token=restart_token,
review_brief=state.get("review_brief"),
@@ -2309,6 +2317,7 @@ def _record_validation_status(
def _record_github_result(
state: dict[str, Any], args: argparse.Namespace
) -> dict[str, Any]:
+ _require_local_green_for_github_review(state)
reviewed_head = str(
dict(state.get("review_heads") or {}).get("last_reviewed_head")
or dict(state.get("identity") or {}).get("head")
@@ -2466,20 +2475,41 @@ def _github_pending_head_change_identity(
) -> dict[str, Any] | None:
if state.get("stage") not in {STAGE_REVIEW_GREEN, STAGE_LOCAL_GREEN_HANDOFF}:
return None
- github_status = _github_review_status(state)
- if (
- _mode_label(state) == "fast" and github_status == "unknown"
- ) or github_status in {GITHUB_RESULT_CLEAN, GITHUB_RESULT_WAIVED}:
- return None
try:
identity = _current_cycle_identity_if_compatible(state)
except OSError, ValueError:
return None
if not identity:
- return None
+ try:
+ _, _, _, head, current_merge_base = _current_restart_identity(
+ state, require_exact=False
+ )
+ except AttributeError, OSError, ValueError:
+ return None
+ identity = {
+ "head": head,
+ "merge_base": current_merge_base,
+ "base_drift": None,
+ }
+ head = str(identity.get("head") or "").strip()
+ current_merge_base = str(identity.get("merge_base") or "").strip()
+ deslop = dict(state.get("deslop") or {})
+ closure_head = str(
+ deslop.get("reviewed_head")
+ or dict(state.get("identity") or {}).get("head")
+ or ""
+ ).strip()
+ terminal = _github_review_status(state) in {"clean", "waived"}
+ if (
+ str(deslop.get("status") or "").strip() != DESLOP_STATUS_SKIPPED or terminal
+ ) and (
+ closure_head != head
+ or str(dict(state.get("identity") or {}).get("merge_base") or "").strip()
+ != current_merge_base
+ ):
+ return identity
if bool(dict(identity.get("base_drift") or {}).get("patch_equivalent")):
return None
- head = str(identity.get("head") or "").strip()
summary = review_ladder_summary(state, current_head=head)
if summary.get("review_ladder") != "invalidated":
return None
@@ -2505,9 +2535,10 @@ def _github_terminal_action(
def _deslop_is_open(state: dict[str, Any]) -> bool:
deslop = dict(state.get("deslop") or {})
- if not bool(deslop.get("tracked")):
- return False
- return str(deslop.get("status") or "").strip() != DESLOP_STATUS_CLOSED
+ return (
+ bool(deslop.get("tracked"))
+ and str(deslop.get("status") or "").strip() == DESLOP_STATUS_DONE
+ )
def _with_deslop_done_action(
@@ -2519,11 +2550,15 @@ def _with_deslop_done_action(
) -> dict[str, Any] | None:
if not _deslop_is_open(state):
return action
- if action is None:
- return {"cmd": _deslop_done_command(public_id, state_dir=state_dir)}
- next_action = dict(action or {})
- next_action["deslop_done"] = _deslop_done_command(public_id, state_dir=state_dir)
- return next_action
+ findings = dict(state.get("deslop") or {}).get("decision") == "findings"
+ return {
+ "cmd": _deslop_done_command(public_id, state_dir=state_dir),
+ "note": (
+ "Address or dismiss the closure findings, then acknowledge the exact-head result."
+ if findings
+ else "Acknowledge the exact-head closure before continuing."
+ ),
+ }
def _public_convergence(state: dict[str, Any]) -> dict[str, Any] | None:
@@ -2628,6 +2663,20 @@ def _action_payload(state: dict[str, Any], *, state_dir: Path) -> dict[str, Any]
action["note"] = note
return _with_deslop_done_action(state, action, public_id, state_dir=state_dir)
if stage in {STAGE_REVIEW_GREEN, STAGE_LOCAL_GREEN_HANDOFF}:
+ deslop = dict(state.get("deslop") or {})
+ if deslop.get("conformance") == "MATERIALLY_DRIFTED":
+ return {
+ "cmd": _review_command(public_id, state_dir=state_dir),
+ "note": "Revise the materially drifted implementation, then rerun this review id.",
+ }
+ if bool(deslop.get("tracked")) and str(deslop.get("status") or "") in {
+ "tracked",
+ "failed",
+ }:
+ return {
+ "cmd": _review_command(public_id, state_dir=state_dir),
+ "note": "Run the bounded exact-head closure before handoff.",
+ }
summary = review_ladder_summary(state, current_head=_identity_head(state))
if summary.get("review_ladder") == "invalidated":
if green_review_head_change_summary(state, summary=summary):
@@ -2671,6 +2720,10 @@ def _render(state: dict[str, Any], *, state_dir: Path) -> None:
github_status = str(github_review.get("status") or "").strip()
if github_status and github_status != "unknown":
payload["github_review"] = github_status
+ if conformance := str(
+ dict(state.get("deslop") or {}).get("conformance") or ""
+ ).strip():
+ payload["conformance"] = conformance
if convergence := _public_convergence(state):
payload["convergence"] = convergence
if validation := _validation_summary(state):
@@ -2701,6 +2754,8 @@ def _render_stale_decision_recovery(
def _require_local_green_for_github_review(state: dict[str, Any]) -> None:
if state.get("stage") not in {STAGE_REVIEW_GREEN, STAGE_LOCAL_GREEN_HANDOFF}:
raise ValueError("--github-review requires local green review state")
+ if bool(dict(state.get("deslop") or {}).get("tracked")):
+ raise ValueError("--github-review requires completed exact-head closure")
def _github_review_subprocess_command(
@@ -2918,6 +2973,15 @@ def main() -> int:
_render(saved, state_dir=state_dir)
return 0
if args.deslop_done:
+ resumed = _resume_progress(state, state_dir=state_dir)
+ if resumed != state:
+ saved = save_cycle(state_dir, resumed)
+ _render(saved, state_dir=state_dir)
+ return 0
+ if _current_cycle_identity_if_compatible(state) is None:
+ raise ValueError(
+ "--deslop-done requires the exact clean branch, HEAD, and merge-base"
+ )
state = mark_deslop_closed(state)
saved = save_cycle(state_dir, state)
_render(saved, state_dir=state_dir)
@@ -2971,6 +3035,15 @@ def main() -> int:
)
return 0
if has_validation_status and not args.decision:
+ resumed = (
+ _resume_progress(state, state_dir=state_dir)
+ if _github_review_status(state) in {"clean", "waived"}
+ else state
+ )
+ if resumed != state:
+ saved = save_cycle(state_dir, resumed)
+ _render(saved, state_dir=state_dir)
+ return 0
state = _record_validation_status(state, args)
elif not args.decision:
state = (
diff --git a/plugins/review-suite/scripts/review_deslop.py b/plugins/review-suite/scripts/review_deslop.py
index bea6007..4b75d2f 100644
--- a/plugins/review-suite/scripts/review_deslop.py
+++ b/plugins/review-suite/scripts/review_deslop.py
@@ -31,8 +31,7 @@
format_command,
lens_model_config,
resolve_repo_root,
- run_codex_review,
- use_unsafe_windows_wsl_fallback,
+ run_codex,
validated_linear_review_range,
write_text,
)
@@ -83,6 +82,10 @@ def build_parser() -> argparse.ArgumentParser:
parser.add_argument("--base", help="Override the detected default branch ref.")
parser.add_argument("--commit", nargs="+")
parser.add_argument("--focus")
+ parser.add_argument("--review-brief", help=argparse.SUPPRESS)
+ parser.add_argument(
+ "--conformance-only", action="store_true", help=argparse.SUPPRESS
+ )
parser.add_argument("--wsl", action="store_true")
parser.add_argument("--output-only", action="store_true", help=argparse.SUPPRESS)
return parser
@@ -112,6 +115,8 @@ def build_prompt(
commit: str | None,
commit_end: str | None,
focus: str | None,
+ review_brief: str | None = None,
+ conformance_only: bool = False,
) -> str:
focus_block = (
f"\nPay extra attention to this focus area:\n- {focus.strip()}\n"
@@ -132,19 +137,27 @@ def build_prompt(
target_block = (
f"Review the current repository changes against base branch `{base}`.\n\n"
)
+ conformance = (
+ "Compare the implementation with this frozen review brief:\n"
+ f"\n{review_brief.strip()}\n\n"
+ "Report CONFORMS unless the implementation materially changes the brief's goal or constraints; otherwise report MATERIALLY_DRIFTED.\n\n"
+ if review_brief
+ else "No frozen review brief is available; report NOT_APPLICABLE for conformance.\n\n"
+ )
+ cleanup = (
+ "Do not perform another cleanup review or report cleanup findings; this is the post-edit conformance rerun. You must still emit the required final review decision.\n"
+ if conformance_only
+ else "Inspect only for concrete redundant code, dead code, duplicate logic, and needless wrappers.\n"
+ )
return (
target_block
- + "Prefer the smallest correct shape.\n\n"
- + "Inspect for:\n"
- + "- redundant code\n"
- + "- duplicated logic\n"
- + "- dead or unused code\n"
- + "- places where a smaller or more direct implementation would work\n"
- + "- unnecessary helpers, wrappers, flags, branching, or abstraction layers\n"
- + "- overcomplicated abstractions that can be collapsed\n"
+ + conformance
+ + cleanup
+ + "Do not redesign ownership, add abstractions, broaden scope, or change behavior.\n"
+ focus_block
- + "\nReturn only concrete findings with severity, file path, and fix suggestion.\n"
- + "Skip style-only comments."
+ + "\nBegin with exactly `Conformance: CONFORMS`, `Conformance: MATERIALLY_DRIFTED`, or `Conformance: NOT_APPLICABLE`.\n"
+ + "Return only concrete cleanup findings with severity, file path, and fix suggestion. Skip style-only comments.\n"
+ + "Always finish with exactly `Review decision: clean` or `Review decision: findings`."
)
@@ -179,10 +192,10 @@ def _git_lines(
]
-def _changed_python_lines(*, review_root: Path, base: str) -> dict[str, set[int]]:
+def _changed_python_lines(*, review_root: Path, diff_range: str) -> dict[str, set[int]]:
diff = _git_output(
review_root,
- ["diff", "--unified=0", "--find-renames", f"{base}...HEAD", "--", "*.py"],
+ ["diff", "--unified=0", "--find-renames", diff_range, "--", "*.py"],
)
changed: dict[str, set[int]] = {}
current_path: str | None = None
@@ -244,9 +257,16 @@ def _start_static_cleanup_scan(
commit: str | None,
commit_end: str | None,
) -> StaticCleanupScan | None:
- if commit or commit_end or not base:
+ if commit and not commit_end:
return None
- changed_lines = _changed_python_lines(review_root=review_root, base=base)
+ diff_range = (
+ f"{commit}..{commit_end}" if commit else f"{base}...HEAD" if base else None
+ )
+ if not diff_range:
+ return None
+ changed_lines = _changed_python_lines(
+ review_root=review_root, diff_range=diff_range
+ )
if not changed_lines:
return None
command = _ensure_vulture_command()
@@ -405,9 +425,11 @@ def _with_static_cleanup_output(
body = str(result.get("final_message") or "").strip()
if not body:
return result
- if _deslop_output_clean(result):
+ if _deslop_output_clean(result) and "conformance:" not in body.lower():
body = "No reviewer findings."
- return {**result, "final_message": f"{section}\n\nDeslop Results:\n{body}"}
+ body = body.rpartition("\n")[0] if terminal_review_command(body) else body
+ body = f"{section}\n\nDeslop Results:\n{body}\nReview decision: findings"
+ return {**result, "final_message": body}
def _review_output_text(result: dict[str, object]) -> str:
@@ -475,60 +497,56 @@ def main() -> int:
if commit and args.base is not None:
raise ValueError("use either --base or --commit")
review_root = resolve_repo_root(args.cd)
- if use_unsafe_windows_wsl_fallback(review_root, bool(args.wsl)):
- print(
- "[review-deslop] WARNING: using Windows Codex fallback for a WSL UNC repo. This bypasses the Codex sandbox and is not the happy path.",
- file=sys.stderr,
- flush=True,
- )
model_config = lens_model_config("review-deslop")
if commit and commit_end:
validated_linear_review_range(
review_root,
commit,
commit_end,
- label="native commit-range deslop review",
+ label="commit-range deslop review",
)
ensure_clean_git_worktree(review_root)
- review_target = {"base": commit, "commit_end": commit_end}
prompt_base = None
elif commit:
- review_target = {"commit": commit}
prompt_base = None
else:
prompt_base = str(effective_base_ref(review_root, args.base)["base"])
ensure_clean_git_worktree(review_root)
- review_target = {"base": prompt_base}
- static_scan = _start_static_cleanup_scan(
- review_root=review_root,
- base=prompt_base,
- commit=commit,
- commit_end=commit_end,
+ static_scan = (
+ None
+ if args.conformance_only
+ else _start_static_cleanup_scan(
+ review_root=review_root,
+ base=prompt_base,
+ commit=commit,
+ commit_end=commit_end,
+ )
)
prompt = build_prompt(
base=prompt_base,
commit=commit,
commit_end=commit_end,
focus=args.focus,
+ review_brief=args.review_brief,
+ conformance_only=bool(args.conformance_only),
)
try:
- result = run_codex_review(
+ result = run_codex(
tool_name="review-deslop",
prompt=prompt,
model=model_config.model,
reasoning_effort=model_config.reasoning_effort,
service_tier=model_config.service_tier,
- title="review-deslop",
review_root=review_root,
- **review_target,
progress_interval_seconds=DEFAULT_PROGRESS_INTERVAL_SECONDS,
timeout_seconds=DEFAULT_TIMEOUT_SECONDS,
allow_unsafe_windows_wsl_fallback=bool(args.wsl),
)
result = _with_effective_returncode(result)
- static_suggestions = _collect_static_cleanup_scan(static_scan)
- static_scan = None
- result = _with_static_cleanup_output(result, static_suggestions)
+ if static_scan is not None:
+ static_suggestions = _collect_static_cleanup_scan(static_scan)
+ static_scan = None
+ result = _with_static_cleanup_output(result, static_suggestions)
finally:
if static_scan is not None:
_stop_static_cleanup_scan(static_scan)
diff --git a/plugins/review-suite/scripts/review_followup.py b/plugins/review-suite/scripts/review_followup.py
index ca84775..5b3f47c 100644
--- a/plugins/review-suite/scripts/review_followup.py
+++ b/plugins/review-suite/scripts/review_followup.py
@@ -37,7 +37,6 @@
resolve_ref,
resolve_repo_root,
run_codex_review,
- use_unsafe_windows_wsl_fallback,
validated_linear_review_range,
)
from review_suite_local import (
@@ -255,12 +254,6 @@ def main() -> int:
base_info = effective_base_ref(review_root, args.base)
branch_base = str(base_info["base"])
requested_base = str(base_info["requested_base"])
- if use_unsafe_windows_wsl_fallback(review_root, bool(args.wsl)):
- print(
- "[review-followup] WARNING: using Windows Codex fallback for a WSL UNC repo. This bypasses the Codex sandbox and is not the happy path.",
- file=sys.stderr,
- flush=True,
- )
since_head = resolve_since_head(
explicit_since=args.since,
state_dir=state_dir,
diff --git a/plugins/review-suite/scripts/review_gate.py b/plugins/review-suite/scripts/review_gate.py
index 72588b6..89a65a4 100644
--- a/plugins/review-suite/scripts/review_gate.py
+++ b/plugins/review-suite/scripts/review_gate.py
@@ -1119,6 +1119,7 @@ def _launch_gate_run(
child = launch_captured_child_process(
command=launch.command,
cwd=launch.cwd,
+ env=launch.env,
stdin_text=launch.stdin_text,
stdout_prefix=f"{gate_task_class}-{slot}-",
)
diff --git a/plugins/review-suite/scripts/review_plan.py b/plugins/review-suite/scripts/review_plan.py
index 6a60493..802dea1 100644
--- a/plugins/review-suite/scripts/review_plan.py
+++ b/plugins/review-suite/scripts/review_plan.py
@@ -30,7 +30,6 @@
resolve_cd_path,
resolve_repo_root,
run_codex,
- use_unsafe_windows_wsl_fallback,
)
_STD_INPUT_HANDLE = -10
@@ -148,12 +147,6 @@ def main() -> int:
try:
args = parser.parse_args()
review_root = resolve_review_root(args)
- if use_unsafe_windows_wsl_fallback(review_root, bool(args.wsl)):
- print(
- "[review-plan] WARNING: using Windows Codex fallback for a WSL UNC repo. This bypasses the Codex sandbox and is not the happy path.",
- file=sys.stderr,
- flush=True,
- )
plan_text, _input_source = load_plan_input(args)
model_config = lens_model_config("review-plan")
result = run_codex(
diff --git a/plugins/review-suite/scripts/review_suite_arena.py b/plugins/review-suite/scripts/review_suite_arena.py
index 44f39ed..9c818e2 100644
--- a/plugins/review-suite/scripts/review_suite_arena.py
+++ b/plugins/review-suite/scripts/review_suite_arena.py
@@ -419,7 +419,6 @@ def _validate_benchmarked_review_runtime(
codex_executable=codex_executable,
review_root=review_cwd,
allow_unsafe_windows_wsl_fallback=allow_unsafe_windows_wsl_fallback,
- unsafe_command_hint="codex exec review --dangerously-bypass-approvals-and-sandbox",
)
diff --git a/plugins/review-suite/scripts/review_suite_core/codex_runtime.py b/plugins/review-suite/scripts/review_suite_core/codex_runtime.py
index 467140d..04b8e80 100644
--- a/plugins/review-suite/scripts/review_suite_core/codex_runtime.py
+++ b/plugins/review-suite/scripts/review_suite_core/codex_runtime.py
@@ -6,7 +6,6 @@
DOCS_URL = "https://developers.openai.com/codex/windows#use-codex-cli-with-wsl"
-AUTO_UNSAFE_WINDOWS_WSL_FALLBACK_ENV = "REVIEW_SUITE_AUTO_WSL_FALLBACK"
def wrapper_launch_cwd() -> Path:
@@ -49,39 +48,27 @@ def running_in_wsl() -> bool:
return False
-def _env_flag_value(name: str) -> str:
- value = str(os.environ.get(name) or "").strip()
- if value:
- return value
- if sys.platform != "win32":
- return ""
- try:
- import winreg
-
- with winreg.OpenKey(winreg.HKEY_CURRENT_USER, "Environment") as key:
- registry_value, _ = winreg.QueryValueEx(key, name)
- return str(registry_value or "").strip()
- except Exception:
- return ""
-
-
-def _env_flag_enabled(name: str) -> bool:
- value = _env_flag_value(name).lower()
- return value in {"1", "true", "yes", "on"}
-
-
-def unsafe_windows_wsl_fallback_requested(explicit_flag: bool) -> bool:
- return bool(explicit_flag) or _env_flag_enabled(
- AUTO_UNSAFE_WINDOWS_WSL_FALLBACK_ENV
+def use_unsafe_windows_wsl_fallback(
+ review_root: Path, allow_unsafe_windows_wsl_fallback: bool
+) -> bool:
+ return bool(allow_unsafe_windows_wsl_fallback) and is_windows_wsl_unc_path(
+ review_root
)
-def use_unsafe_windows_wsl_fallback(
+def windows_wsl_codex_child_env(
review_root: Path, allow_unsafe_windows_wsl_fallback: bool
-) -> bool:
- return unsafe_windows_wsl_fallback_requested(
- allow_unsafe_windows_wsl_fallback
- ) and is_windows_wsl_unc_path(review_root)
+) -> dict[str, str] | None:
+ if not use_unsafe_windows_wsl_fallback(
+ review_root, allow_unsafe_windows_wsl_fallback
+ ):
+ return None
+ env = os.environ.copy()
+ count = int(env.get("GIT_CONFIG_COUNT", "0"))
+ env[f"GIT_CONFIG_KEY_{count}"] = "safe.directory"
+ env[f"GIT_CONFIG_VALUE_{count}"] = review_root.resolve().as_posix()
+ env["GIT_CONFIG_COUNT"] = str(count + 1)
+ return env
def _windows_unc_fallback_hint(review_root: Path) -> str | None:
@@ -98,17 +85,13 @@ def validate_codex_runtime(
codex_executable: str,
review_root: Path,
allow_unsafe_windows_wsl_fallback: bool,
- unsafe_command_hint: str,
) -> None:
if use_unsafe_windows_wsl_fallback(review_root, allow_unsafe_windows_wsl_fallback):
return
if is_windows_wsl_unc_path(review_root):
raise ValueError(
f"{tool_name} detected a WSL repo through a Windows UNC path at {review_root}. "
- f"The normal Windows Codex sandbox fails in this lane. Preferred path: use a native WSL Codex install. "
- f"If you truly need the mixed Windows lane, rerun with --wsl or set "
- f"{AUTO_UNSAFE_WINDOWS_WSL_FALLBACK_ENV}=1 to use "
- f"`{unsafe_command_hint}` from a safe Windows launch cwd. This bypasses the Codex sandbox and is not the happy path. "
+ f"Rerun with --wsl to authorize the Windows Codex launch for this exact repository. "
f"Docs: {DOCS_URL}"
)
if not running_in_wsl():
@@ -125,6 +108,6 @@ def validate_codex_runtime(
raise ValueError(
f"{tool_name} detected a native WSL run, but `codex` resolves to the Windows shim at {codex_executable}. "
f"Native WSL review is unsupported in this configuration.{fallback_text} "
- f"Durable fix: install and authenticate Codex inside WSL, then rerun from your WSL repo under /home/.... "
- f"Suggested commands: `npm i -g @openai/codex` then `codex`. Docs: {DOCS_URL}"
+ f"Use a native WSL Codex executable or rerun Windows Codex against the repository UNC path with --wsl. "
+ f"Docs: {DOCS_URL}"
)
diff --git a/plugins/review-suite/scripts/review_suite_core/lens_runtime.py b/plugins/review-suite/scripts/review_suite_core/lens_runtime.py
index 653fa74..a0c4412 100644
--- a/plugins/review-suite/scripts/review_suite_core/lens_runtime.py
+++ b/plugins/review-suite/scripts/review_suite_core/lens_runtime.py
@@ -17,6 +17,7 @@
from .codex_runtime import (
use_unsafe_windows_wsl_fallback,
validate_codex_runtime,
+ windows_wsl_codex_child_env,
wrapper_launch_cwd,
)
from .model_labels import codex_reasoning_effort
@@ -60,6 +61,7 @@ class CodexReviewLaunch:
stdin_text: str | None
final_message_path: Path | None
cwd: Path
+ env: dict[str, str] | None
effective_reasoning_effort: str
@@ -165,7 +167,6 @@ def _codex_command_prefix(
tool_name: str,
review_root: Path,
allow_unsafe_windows_wsl_fallback: bool,
- unsafe_command_hint: str,
subcommand: str,
model: str,
reasoning_effort: str,
@@ -178,31 +179,20 @@ def _codex_command_prefix(
codex_executable=codex_executable,
review_root=review_root,
allow_unsafe_windows_wsl_fallback=allow_unsafe_windows_wsl_fallback,
- unsafe_command_hint=unsafe_command_hint,
)
- unsafe_fallback = use_unsafe_windows_wsl_fallback(
- review_root, allow_unsafe_windows_wsl_fallback
- )
- command = [codex_executable]
- if subcommand == "exec":
- command.append("exec")
- command.append("--ignore-user-config")
- if unsafe_fallback:
- command.append("--dangerously-bypass-approvals-and-sandbox")
- command.extend(["-C", str(review_root)])
- if not unsafe_fallback:
- command.extend(["-s", "read-only"])
- elif subcommand == "exec-review":
- command.append("exec")
- command.append("--ignore-user-config")
- if unsafe_fallback:
- command.append("--dangerously-bypass-approvals-and-sandbox")
- command.extend(["-C", str(review_root)])
- if not unsafe_fallback:
- command.extend(["-s", "read-only"])
- command.append("review")
- else:
+ if subcommand not in {"exec", "exec-review"}:
raise ValueError(f"unsupported Codex subcommand: {subcommand}")
+ command = [
+ codex_executable,
+ "exec",
+ "--ignore-user-config",
+ "-C",
+ str(review_root),
+ "-s",
+ "read-only",
+ ]
+ if subcommand == "exec-review":
+ command.append("review")
command.extend(_isolated_runtime_user_config_args())
effective_reasoning_effort = codex_reasoning_effort(model, reasoning_effort)
command.extend(
@@ -247,7 +237,6 @@ def codex_exec_command(
tool_name=tool_name,
review_root=review_root,
allow_unsafe_windows_wsl_fallback=allow_unsafe_windows_wsl_fallback,
- unsafe_command_hint="codex exec --dangerously-bypass-approvals-and-sandbox",
subcommand="exec",
model=model,
reasoning_effort=reasoning_effort,
@@ -347,7 +336,6 @@ def codex_exec_review_command(
tool_name=tool_name,
review_root=review_root,
allow_unsafe_windows_wsl_fallback=allow_unsafe_windows_wsl_fallback,
- unsafe_command_hint="codex exec --dangerously-bypass-approvals-and-sandbox review",
subcommand="exec-review",
model=model,
reasoning_effort=reasoning_effort,
@@ -445,6 +433,7 @@ def prepare_codex_review_launch(
stdin_text=stdin_text,
final_message_path=final_message_path,
cwd=cwd,
+ env=windows_wsl_codex_child_env(review_root, allow_unsafe_windows_wsl_fallback),
effective_reasoning_effort=codex_reasoning_effort(model, reasoning_effort),
)
@@ -539,12 +528,14 @@ def _run_captured_codex_command(
timeout_seconds: int,
final_message_path: Path | None = None,
cleanup_paths: tuple[Path, ...] = (),
+ env: dict[str, str] | None = None,
) -> dict[str, object]:
child: CapturedChildProcess | None = None
try:
child = launch_captured_child_process(
command=command,
cwd=cwd,
+ env=env,
stdin_text=stdin_text,
stdout_prefix=f"{tool_name}-stdout-",
stderr_prefix=f"{tool_name}-stderr-",
@@ -642,6 +633,7 @@ def run_codex(
timeout_seconds=timeout_seconds,
final_message_path=output_path,
cleanup_paths=(output_path,),
+ env=windows_wsl_codex_child_env(review_root, allow_unsafe_windows_wsl_fallback),
)
@@ -686,6 +678,7 @@ def run_codex_review(
cleanup_paths=(launch.final_message_path,)
if launch.final_message_path is not None
else (),
+ env=launch.env,
)
diff --git a/plugins/review-suite/scripts/review_suite_core/orchestrator_profiles.py b/plugins/review-suite/scripts/review_suite_core/orchestrator_profiles.py
index 6548508..0639cfd 100644
--- a/plugins/review-suite/scripts/review_suite_core/orchestrator_profiles.py
+++ b/plugins/review-suite/scripts/review_suite_core/orchestrator_profiles.py
@@ -52,7 +52,6 @@ class OrchestratorProfileStep:
class OrchestratorProfile:
mode: str
profile: str
- deslop_enabled: bool
steps: tuple[OrchestratorProfileStep, ...]
@@ -484,6 +483,10 @@ def _normalize_profile(
) -> OrchestratorProfile:
if not isinstance(raw_profile, dict):
raise ValueError(f"orchestrator.profiles.{profile}.{mode} must be an object")
+ if "deslop_enabled" in raw_profile:
+ raise ValueError(
+ f"orchestrator.profiles.{profile}.{mode}.deslop_enabled is obsolete; use --skip-deslop"
+ )
raw_steps = raw_profile.get("steps")
if not isinstance(raw_steps, list) or not raw_steps:
raise ValueError(
@@ -498,7 +501,6 @@ def _normalize_profile(
return OrchestratorProfile(
mode=mode,
profile=profile,
- deslop_enabled=bool(raw_profile.get("deslop_enabled", True)),
steps=steps,
)
diff --git a/plugins/review-suite/scripts/review_suite_core/orchestrator_runner.py b/plugins/review-suite/scripts/review_suite_core/orchestrator_runner.py
index 3dd51da..efc573e 100644
--- a/plugins/review-suite/scripts/review_suite_core/orchestrator_runner.py
+++ b/plugins/review-suite/scripts/review_suite_core/orchestrator_runner.py
@@ -2,7 +2,6 @@
import subprocess
import sys
-from concurrent.futures import ThreadPoolExecutor
from dataclasses import dataclass
from pathlib import Path
from typing import Any, Callable
@@ -34,7 +33,6 @@
progress_heartbeat_line,
)
from .orchestrator_state import (
- DESLOP_STATUS_TRACKED,
STAGE_CREATED,
STAGE_DECISION_PENDING,
STAGE_FOLLOWUP_PENDING,
@@ -50,6 +48,7 @@
mark_recovery_resolved,
mark_followup_review_pending,
mark_gate_step_pending,
+ mark_latest_profile_step_rerun_needed,
mark_review_step_running,
mark_review_step_pending,
mark_review_step_retry,
@@ -64,6 +63,7 @@
)
from .workflow_state import (
EFFECTIVE_BASE_METADATA_KEYS,
+ current_branch,
current_head,
dirty_worktree_scope,
has_committed_diff,
@@ -120,9 +120,14 @@ def deslop_command(state: dict[str, Any]) -> list[str]:
"--output-only",
"--cd",
str(cwd),
- "--base",
- _identity_text(state, "base"),
+ "--commit",
+ _identity_text(state, "merge_base"),
+ _identity_text(state, "head"),
]
+ if brief := str(state.get("review_brief") or "").strip():
+ command.append(f"--review-brief={brief}")
+ if bool(dict(state.get("deslop") or {}).get("conformance_only")):
+ command.append("--conformance-only")
if _allow_unsafe_windows_wsl_fallback(state):
command.append("--wsl")
return command
@@ -195,9 +200,33 @@ def _process_output(proc: subprocess.CompletedProcess) -> str:
def _run_deslop_once(state: dict[str, Any]) -> OrchestratorRunnerResult:
+ cwd = cwd_path_from_normalized(_identity_text(state, "cwd"))
+ expected_head = _identity_text(state, "head")
+ expected_merge_base = _identity_text(state, "merge_base")
+ expected_branch = dict(state.get("identity") or {}).get("branch")
+ dirty = dirty_worktree_scope(cwd, "HEAD")["dirty_paths"]
+ if current_branch(cwd) != expected_branch:
+ _print_step_output(
+ label="closure",
+ status="blocked",
+ body=f"expected {expected_branch or 'detached HEAD'}",
+ )
+ return OrchestratorRunnerResult(state, ran_step=False, step="blocked")
+ if dirty:
+ _print_step_output(
+ label="closure", status="blocked", body="clean worktree required"
+ )
+ return OrchestratorRunnerResult(state, ran_step=False, step="blocked")
+ scope = _review_scope(state, cwd)
+ actual_head = str(scope["reviewed_head"])
+ actual_merge_base = str(scope["merge_base"])
+ if actual_head != expected_head or actual_merge_base != expected_merge_base:
+ state = mark_latest_profile_step_rerun_needed(state, head=actual_head)
+ state["identity"].update(head=actual_head, merge_base=actual_merge_base)
+ state["review_heads"].update(head=actual_head, merge_base=actual_merge_base)
+ return OrchestratorRunnerResult(state, ran_step=True, step="deslop-rerun")
command = deslop_command(state)
command_text = format_command(command)
- cwd = cwd_path_from_normalized(_identity_text(state, "cwd"))
try:
proc = run_deslop_subprocess(command=command, cwd=cwd)
except OSError as exc:
@@ -214,16 +243,60 @@ def _run_deslop_once(state: dict[str, Any]) -> OrchestratorRunnerResult:
ran_step=True,
step="deslop",
)
+ output = _process_output(proc)
+ if output:
+ has_brief = bool(str(state.get("review_brief") or "").strip())
+ allowed = (
+ {"CONFORMS", "MATERIALLY_DRIFTED"} if has_brief else {"NOT_APPLICABLE"}
+ )
+ lines = [line.strip() for line in output.splitlines() if line.strip()]
+ verdicts = [
+ line.removeprefix("Conformance: ")
+ for line in lines
+ if line.startswith("Conformance: ")
+ ]
+ decisions = [line for line in lines if line.startswith("Review decision: ")]
+ if (
+ int(proc.returncode) == 0
+ and len(verdicts) == len(decisions) == 1
+ and verdicts[0] in allowed
+ and lines[-1] == decisions[0]
+ and decisions[0].partition(": ")[2] in {"clean", "findings"}
+ ):
+ _print_step_output(label="review-deslop", body=output)
+ protocol_lines = {f"Conformance: {verdicts[0]}", decisions[0]}
+ output_lines = output.splitlines()
+ findings = "\n".join(
+ line for line in output_lines if line not in protocol_lines
+ )
+ return OrchestratorRunnerResult(
+ mark_deslop_done(
+ state,
+ command=command_text,
+ conformance=verdicts[0],
+ reviewed_head=actual_head,
+ decision=decisions[0].removeprefix("Review decision: "),
+ findings=findings,
+ ),
+ ran_step=True,
+ step="deslop",
+ )
if int(proc.returncode) == 0:
- _print_step_output(label="review-deslop", body=str(proc.stdout or ""))
+ _print_step_output(label="review-deslop", body=output)
return OrchestratorRunnerResult(
- mark_deslop_done(state, command=command_text), ran_step=True, step="deslop"
+ mark_deslop_failed(
+ state,
+ command=command_text,
+ returncode=0,
+ reason="deslop did not report valid conformance and a terminal decision",
+ ),
+ ran_step=True,
+ step="deslop",
)
_print_step_output(
label="review-deslop",
status="failed",
- body=_process_output(proc)
- or f"review-deslop failed with exit {int(proc.returncode)}",
+ body=output or f"review-deslop failed with exit {int(proc.returncode)}",
)
return OrchestratorRunnerResult(
mark_deslop_failed(
@@ -1428,23 +1501,6 @@ def _run_profile_step_once(
)
-def _merge_deslop_result(
- state: dict[str, Any], deslop_result: OrchestratorRunnerResult
-) -> dict[str, Any]:
- next_state = dict(state)
- next_state["deslop"] = dict(deslop_result.state.get("deslop") or {})
- review_recovery = dict(state.get("recovery") or {})
- deslop_retry_recovery = str(review_recovery.get("reason") or "").startswith(
- "deslop failed"
- )
- if (
- str(review_recovery.get("status") or "") in {"", "none"}
- or deslop_retry_recovery
- ):
- next_state["recovery"] = dict(deslop_result.state.get("recovery") or {})
- return next_state
-
-
def run_one_expensive_step(
state: dict[str, Any],
*,
@@ -1453,38 +1509,7 @@ def run_one_expensive_step(
) -> OrchestratorRunnerResult:
resolved_state_dir = state_dir or Path.home() / ".codex" / "state" / "review-suite"
if deslop_should_run(state):
- deslop_status = str(dict(state.get("deslop") or {}).get("status") or "")
- if (
- deslop_status == DESLOP_STATUS_TRACKED
- and state.get("stage") == STAGE_CREATED
- and review_profile_has_next_step(state)
- ):
- with ThreadPoolExecutor(max_workers=2) as executor:
- deslop_future = executor.submit(_run_deslop_once, state)
- review_result = _run_profile_step_once(
- state,
- state_dir=resolved_state_dir,
- persist_state=persist_state,
- )
- deslop_result = deslop_future.result()
- next_state = _merge_deslop_result(review_result.state, deslop_result)
- return OrchestratorRunnerResult(
- next_state,
- ran_step=True,
- step=review_result.step,
- )
- deslop_result = _run_deslop_once(state)
- if (
- state.get("stage") not in {STAGE_CREATED, STAGE_RETRY_REQUESTED}
- or dict(state.get("pending_action") or {}).get("kind") == "arena-blocked"
- ):
- next_state = _merge_deslop_result(state, deslop_result)
- return OrchestratorRunnerResult(
- next_state,
- ran_step=True,
- step="deslop",
- )
- return deslop_result
+ return _run_deslop_once(state)
if state.get("stage") == STAGE_RUNNING:
return _collect_running_review_once(state, state_dir=resolved_state_dir)
if (
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 b3218f4..2cb8b5f 100644
--- a/plugins/review-suite/scripts/review_suite_core/orchestrator_state.py
+++ b/plugins/review-suite/scripts/review_suite_core/orchestrator_state.py
@@ -56,20 +56,7 @@
DESLOP_STATUS_FAILED = "failed"
DESLOP_STATUS_CLOSED = "closed"
DESLOP_STATUS_SKIPPED = "skipped"
-DESLOP_STATUS_SKIPPED_FAST = "skipped-fast"
-DESLOP_RETRY_STAGES = {
- STAGE_CREATED,
- STAGE_RUNNING,
- STAGE_DECISION_PENDING,
- STAGE_FIX_PENDING,
- STAGE_FOLLOWUP_PENDING,
- STAGE_GATE_RERUN_NEEDED,
- STAGE_REVIEW_GREEN,
- STAGE_LOCAL_GREEN_HANDOFF,
- STAGE_BLOCKED,
- STAGE_RETRY_REQUESTED,
-}
-
+CONFORMANCE_VERDICTS = {"CONFORMS", "MATERIALLY_DRIFTED", "NOT_APPLICABLE"}
GATE_LANES = {"review_t2", "review_t4"}
NO_WORK_STAGES = {
STAGE_DECISION_PENDING,
@@ -408,15 +395,10 @@ def create_cycle(
resolved_selection = _normalize_selection(
effective_selection or requested_selection, field="effective_selection"
)
- deslop_tracked = (
- bool(deslop_enabled) if deslop_enabled is not None else effective != "fast"
- )
+ deslop_tracked = bool(deslop_enabled) if deslop_enabled is not None else True
if deslop_tracked:
deslop_status = DESLOP_STATUS_TRACKED
deslop_skip = None
- elif effective == "fast":
- deslop_status = DESLOP_STATUS_SKIPPED_FAST
- deslop_skip = None
else:
deslop_status = DESLOP_STATUS_SKIPPED
deslop_skip = _optional_text(deslop_skip_source) or "profile"
@@ -1294,6 +1276,23 @@ def _last_completed_profile_round_id(state: dict[str, Any]) -> str | None:
return None
+def _mark_deslop_rerun_needed_inplace(state: dict[str, Any]) -> None:
+ deslop = dict(state.get("deslop") or {})
+ if str(deslop.get("status") or "") not in {
+ DESLOP_STATUS_FAILED,
+ DESLOP_STATUS_DONE,
+ DESLOP_STATUS_CLOSED,
+ }:
+ return
+ cleanup_completed = bool(deslop.get("cleanup_completed"))
+ state["deslop"] = {
+ "tracked": True,
+ "status": DESLOP_STATUS_TRACKED,
+ "cleanup_completed": cleanup_completed,
+ "conformance_only": cleanup_completed,
+ }
+
+
def mark_latest_profile_step_rerun_needed(
state: dict[str, Any], *, head: str
) -> dict[str, Any]:
@@ -1312,6 +1311,7 @@ def mark_latest_profile_step_rerun_needed(
for key in ("focused", "full_suite", "ci"):
validation[key] = "unknown"
validation.pop("note", None)
+ _mark_deslop_rerun_needed_inplace(next_state)
action = _rewind_profile_step_action(next_state, profile_step)
_set_review_green(next_state, "unknown")
_set_stage(next_state, STAGE_CREATED, action)
@@ -1331,7 +1331,13 @@ def deslop_is_ready(state: dict[str, Any]) -> bool:
deslop = dict(state.get("deslop") or {})
if not bool(deslop.get("tracked")):
return True
- return str(deslop.get("status") or "") == DESLOP_STATUS_DONE
+ status = str(deslop.get("status") or "")
+ if status in {DESLOP_STATUS_DONE, DESLOP_STATUS_CLOSED}:
+ return True
+ return not (
+ state.get("stage") in {STAGE_REVIEW_GREEN, STAGE_LOCAL_GREEN_HANDOFF}
+ or dict(state.get("pending_action") or {}).get("kind") == "run-deslop"
+ )
def deslop_should_run(state: dict[str, Any]) -> bool:
@@ -1340,38 +1346,56 @@ def deslop_should_run(state: dict[str, Any]) -> bool:
return False
status = str(deslop.get("status") or "")
if status == DESLOP_STATUS_FAILED:
- return state.get("stage") in DESLOP_RETRY_STAGES
- if status == DESLOP_STATUS_TRACKED and state.get("stage") == STAGE_RUNNING:
- return True
- if state.get("stage") not in {
- STAGE_CREATED,
- STAGE_RETRY_REQUESTED,
- }:
- return False
- if dict(state.get("pending_action") or {}).get("kind") not in (
- None,
- "run-deslop",
- "resume-after-deslop",
- ):
- return False
- return status == DESLOP_STATUS_TRACKED
+ return (
+ state.get("stage") == STAGE_RETRY_REQUESTED
+ and dict(state.get("pending_action") or {}).get("kind") == "run-deslop"
+ )
+ return status == DESLOP_STATUS_TRACKED and state.get("stage") in {
+ STAGE_REVIEW_GREEN,
+ STAGE_LOCAL_GREEN_HANDOFF,
+ }
-def mark_deslop_done(state: dict[str, Any], *, command: str) -> dict[str, Any]:
+def mark_deslop_done(
+ state: dict[str, Any],
+ *,
+ command: str,
+ conformance: str,
+ reviewed_head: str,
+ decision: str | None = None,
+ findings: str | None = None,
+) -> dict[str, Any]:
next_state = _copy_state(state)
+ verdict = _required_text(conformance, field="conformance")
+ if verdict not in CONFORMANCE_VERDICTS:
+ raise ValueError("invalid deslop conformance verdict")
+ prior = dict(next_state.get("deslop") or {})
next_state["deslop"] = {
- **dict(next_state.get("deslop") or {}),
+ **prior,
"tracked": True,
"status": DESLOP_STATUS_DONE,
"command": _required_text(command, field="command"),
"returncode": 0,
+ "conformance": verdict,
+ "reviewed_head": _required_text(reviewed_head, field="reviewed_head"),
+ "cleanup_completed": bool(prior.get("cleanup_completed"))
+ or not bool(prior.get("conformance_only")),
}
+ next_state["deslop"].pop("findings", None)
+ if decision is not None:
+ parsed_decision = _required_text(decision, field="decision")
+ if parsed_decision not in {"clean", "findings"}:
+ raise ValueError("invalid deslop review decision")
+ next_state["deslop"]["decision"] = parsed_decision
+ if parsed_decision == "findings" and _optional_text(findings):
+ next_state["deslop"]["findings"] = _optional_text(findings)
recovery = dict(next_state.get("recovery") or {})
next_state["recovery"] = {
"status": "none",
"retry_count": int(recovery.get("retry_count") or 0),
}
- _set_stage(next_state, STAGE_CREATED, {"kind": "resume-after-deslop"})
+ next_state["pending_action"] = None
+ _set_stage(next_state, STAGE_REVIEW_GREEN)
return next_state
@@ -1380,6 +1404,10 @@ def mark_deslop_closed(state: dict[str, Any]) -> dict[str, Any]:
deslop = dict(next_state.get("deslop") or {})
if not bool(deslop.get("tracked")):
return next_state
+ if str(deslop.get("status") or "") != DESLOP_STATUS_DONE:
+ raise ValueError("cleanup can only close after the bounded closure pass")
+ if deslop.get("conformance") == "MATERIALLY_DRIFTED":
+ raise ValueError("materially drifted closure cannot close; revise the head")
next_state["deslop"] = {
**deslop,
"tracked": False,
@@ -1626,6 +1654,7 @@ def mark_fix_detected(
fix_head = _required_text(head, field="head")
active["fix_head"] = fix_head
next_state.setdefault("review_heads", {})["last_fix_head"] = fix_head
+ _mark_deslop_rerun_needed_inplace(next_state)
if _findings_use_followup(next_state):
active["status"] = STAGE_FOLLOWUP_PENDING
_set_stage(
diff --git a/plugins/review-suite/scripts/review_suite_core/process_runtime.py b/plugins/review-suite/scripts/review_suite_core/process_runtime.py
index 5ad317f..9609d3b 100644
--- a/plugins/review-suite/scripts/review_suite_core/process_runtime.py
+++ b/plugins/review-suite/scripts/review_suite_core/process_runtime.py
@@ -33,6 +33,7 @@ def launch_captured_child_process(
*,
command: list[str],
cwd: Path,
+ env: dict[str, str] | None = None,
stdin_text: str | None = None,
stdout_prefix: str,
stderr_prefix: str | None = None,
@@ -52,6 +53,7 @@ def launch_captured_child_process(
proc = subprocess.Popen(
command,
cwd=str(cwd),
+ env=env,
stdin=subprocess.PIPE if stdin_text else None,
stdout=stdout_tmp,
stderr=stderr_tmp,
diff --git a/plugins/review-suite/scripts/review_suite_local.py b/plugins/review-suite/scripts/review_suite_local.py
index 54d40ab..06a6356 100644
--- a/plugins/review-suite/scripts/review_suite_local.py
+++ b/plugins/review-suite/scripts/review_suite_local.py
@@ -47,7 +47,6 @@
prepare_codex_review_launch,
price_usage_tokens,
terminate_process_tree,
- use_unsafe_windows_wsl_fallback,
utc_now,
utc_now_iso,
validated_linear_review_range,
@@ -3601,6 +3600,7 @@ def _launch_reviewer_process(
child = launch_captured_child_process(
command=launch.command,
cwd=launch.cwd,
+ env=launch.env,
stdin_text=launch.stdin_text,
stdout_prefix=f"review-suite-{run['slot']}-",
)
@@ -3791,12 +3791,6 @@ def launch_round(
allow_unsafe_windows_wsl_fallback
)
running_payload["progress_interval_seconds"] = progress_interval_seconds
- if use_unsafe_windows_wsl_fallback(review_cwd, allow_unsafe_windows_wsl_fallback):
- print(
- "[review-suite] WARNING: using Windows Codex fallback for a WSL UNC repo. This bypasses the Codex sandbox and is not the happy path.",
- file=sys.stderr,
- flush=True,
- )
launched_count = 0
for run in round_payload["runs"]:
if _run_is_finalized(run):
diff --git a/plugins/review-suite/skills/review-deslop/SKILL.md b/plugins/review-suite/skills/review-deslop/SKILL.md
index 0c49ddc..8382ebf 100644
--- a/plugins/review-suite/skills/review-deslop/SKILL.md
+++ b/plugins/review-suite/skills/review-deslop/SKILL.md
@@ -5,7 +5,7 @@ description: "Run an explicit one-off simplification review for a completed impl
# Review Deslop
-Use only for an explicit cleanup pass. Normal `review` runs manage their own deslop step.
+Use only for an explicit cleanup pass. Normal `review` runs manage their own one-shot closure pass after correctness is clean.
```powershell
/scripts/review_deslop.py --cd
diff --git a/plugins/review-suite/skills/review/SKILL.md b/plugins/review-suite/skills/review/SKILL.md
index 151bb7a..fdf0268 100644
--- a/plugins/review-suite/skills/review/SKILL.md
+++ b/plugins/review-suite/skills/review/SKILL.md
@@ -19,13 +19,13 @@ Path rules:
Mode:
- Omit `--mode` for `normal`, the default for ordinary changes.
-- Use `--mode fast` for UI-only, local presentation, and other small, well-tested changes. It runs dual GPT-5.6 Sol medium signoff with no deslop, Arena, or GitHub review; convergence uses the same accepted-findings budget as other modes.
+- Use `--mode fast` for UI-only, local presentation, and other small, well-tested changes. It runs dual GPT-5.6 Sol medium signoff with no Arena or GitHub review; convergence and bounded closure match the other modes.
- Use `--mode deep` for billing, login/authentication, authorization/security, business-critical systems, database integrity or migrations, concurrency, and similarly critical or high-blast-radius logic.
- Treat those mappings as risk heuristics. A nominally UI-only change that crosses a trust or data-integrity boundary is not `fast`.
Rules:
- Run focused validation before dispatch; start slow full-suite/CI after dispatch and track final status.
-- Normal and deep run deslop as a sidecar; handle or dismiss its output before completion, then close it with `review.py --id --deslop-done`.
+- After correctness is clean, every mode runs one bounded closure pass for frozen-brief conformance and local cleanup. Handle or dismiss its output, then close it with `review.py --id --deslop-done`. Accepted edits rerun correctness and conformance on the new exact head; cleanup does not recurse.
- To replace an existing ladder with stricter review, use `review.py --id --restart-mode deep --reason ""` while the original repo/base/branch/head/merge-base still match and the worktree is clean; plain `--mode deep --cd ` is not a restart.
- Three distinct caller-accepted findings heads require a durable `CONTINUE`, `REPLAN`, or `RESLICE` decision; `CONTINUE` is available once for one additional fix head and its correctness decision. Report conflicts with the frozen goal, acceptance, scope, stop condition, owner, authorized behavior, or unit boundary immediately with `review.py --id --contract-conflict `.
- Review orchestration expects committed review changes. If `git diff` is non-empty but `base..HEAD` is empty, commit intended changes or stash unrelated worktree changes before rerunning.
diff --git a/plugins/review-suite/tests/test_review_deslop.py b/plugins/review-suite/tests/test_review_deslop.py
index a9454cf..d1db535 100644
--- a/plugins/review-suite/tests/test_review_deslop.py
+++ b/plugins/review-suite/tests/test_review_deslop.py
@@ -96,7 +96,7 @@ def test_normalize_repo_path_preserves_dot_prefixed_paths() -> None:
)
-def test_static_cleanup_scan_skip_commit_modes(
+def test_static_cleanup_scan_skips_single_commit(
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
) -> None:
monkeypatch.setattr(
@@ -113,18 +113,36 @@ def test_static_cleanup_scan_skip_commit_modes(
)
is None
)
+
+
+def test_static_cleanup_scan_uses_two_commit_range(
+ monkeypatch: pytest.MonkeyPatch, tmp_path: Path
+) -> None:
+ diff_ranges: list[str] = []
+ monkeypatch.setattr(
+ review_deslop,
+ "_changed_python_lines",
+ lambda **kwargs: diff_ranges.append(kwargs["diff_range"]) or {},
+ )
+
assert (
review_deslop._start_static_cleanup_scan(
review_root=tmp_path, base=None, commit="abc123", commit_end="def456"
)
is None
)
+ assert diff_ranges == ["abc123..def456"]
def test_static_cleanup_scan_skip_without_changed_python(
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
) -> None:
- monkeypatch.setattr(review_deslop, "_changed_python_lines", lambda **kwargs: {})
+ diff_ranges: list[str] = []
+ monkeypatch.setattr(
+ review_deslop,
+ "_changed_python_lines",
+ lambda **kwargs: diff_ranges.append(kwargs["diff_range"]) or {},
+ )
monkeypatch.setattr(
review_deslop.subprocess,
"Popen",
@@ -139,6 +157,7 @@ def test_static_cleanup_scan_skip_without_changed_python(
)
is None
)
+ assert diff_ranges == ["origin/main...HEAD"]
def test_static_cleanup_scan_starts_with_exact_tracked_paths(
@@ -248,19 +267,16 @@ def test_static_cleanup_output_prefixes_successful_deslop_result() -> None:
assert (
updated["final_message"]
- == "Static cleanup suggestions:\n- Low - app.py:1 - unused import 'os'. Fix: Remove the unused import.\n\nDeslop Results:\nNo reviewer findings."
+ == "Static cleanup suggestions:\n- Low - app.py:1 - unused import 'os'. Fix: Remove the unused import.\n\nDeslop Results:\nNo reviewer findings.\nReview decision: findings"
)
-def test_main_uses_native_base_deslop_review(
+def test_main_uses_generic_read_only_deslop_review(
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
) -> None:
captured: dict[str, object] = {}
monkeypatch.setattr(review_deslop, "resolve_repo_root", lambda cd: tmp_path)
- monkeypatch.setattr(
- review_deslop, "use_unsafe_windows_wsl_fallback", lambda *args, **kwargs: False
- )
monkeypatch.setattr(
review_deslop, "ensure_clean_git_worktree", lambda *args, **kwargs: None
)
@@ -288,9 +304,9 @@ def test_main_uses_native_base_deslop_review(
),
)
- def fake_run_codex_review(**kwargs):
+ def fake_run_codex(**kwargs):
order.append("review")
- captured["run_codex_review"] = kwargs
+ captured["run_codex"] = kwargs
return {
"returncode": 0,
"stdout": "",
@@ -301,7 +317,7 @@ def fake_run_codex_review(**kwargs):
"timed_out": False,
}
- monkeypatch.setattr(review_deslop, "run_codex_review", fake_run_codex_review)
+ monkeypatch.setattr(review_deslop, "run_codex", fake_run_codex)
def fake_emit_result(**kwargs):
captured["emit_result"] = kwargs
@@ -314,10 +330,9 @@ def fake_emit_result(**kwargs):
assert review_deslop.main() == 0
- assert captured["run_codex_review"]["review_root"] == tmp_path
- assert captured["run_codex_review"]["base"] == "origin/main"
- assert captured["run_codex_review"].get("commit") is None
- prompt = str(captured["run_codex_review"]["prompt"])
+ assert not hasattr(review_deslop, "run_codex_review")
+ assert captured["run_codex"]["review_root"] == tmp_path
+ prompt = str(captured["run_codex"]["prompt"])
assert "base branch `origin/main`" in prompt
assert "redundant code" in prompt
assert "=== BEGIN DIFF ===" not in prompt
@@ -337,9 +352,6 @@ def test_main_stops_static_scan_when_review_launch_fails(
)
monkeypatch.setattr(review_deslop, "resolve_repo_root", lambda cd: tmp_path)
- monkeypatch.setattr(
- review_deslop, "use_unsafe_windows_wsl_fallback", lambda *args, **kwargs: False
- )
monkeypatch.setattr(
review_deslop, "ensure_clean_git_worktree", lambda *args, **kwargs: None
)
@@ -365,7 +377,7 @@ def test_main_stops_static_scan_when_review_launch_fails(
)
monkeypatch.setattr(
review_deslop,
- "run_codex_review",
+ "run_codex",
lambda **kwargs: (_ for _ in ()).throw(ValueError("launch failed")),
)
monkeypatch.setattr(review_deslop, "emit_error", lambda *args, **kwargs: 2)
@@ -377,15 +389,12 @@ def test_main_stops_static_scan_when_review_launch_fails(
assert stopped == [scan]
-def test_main_uses_native_base_for_linear_commit_ranges(
+def test_main_uses_generic_prompt_for_linear_commit_ranges(
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
) -> None:
captured: dict[str, object] = {}
monkeypatch.setattr(review_deslop, "resolve_repo_root", lambda cd: tmp_path)
- monkeypatch.setattr(
- review_deslop, "use_unsafe_windows_wsl_fallback", lambda *args, **kwargs: False
- )
monkeypatch.setattr(
review_deslop, "ensure_clean_git_worktree", lambda *args, **kwargs: None
)
@@ -413,8 +422,8 @@ def test_main_uses_native_base_for_linear_commit_ranges(
},
)
- def fake_run_codex_review(**kwargs):
- captured["run_codex_review"] = kwargs
+ def fake_run_codex(**kwargs):
+ captured["run_codex"] = kwargs
return {
"returncode": 0,
"stdout": "",
@@ -425,7 +434,7 @@ def fake_run_codex_review(**kwargs):
"timed_out": False,
}
- monkeypatch.setattr(review_deslop, "run_codex_review", fake_run_codex_review)
+ monkeypatch.setattr(review_deslop, "run_codex", fake_run_codex)
def fake_emit_result(**kwargs):
captured["emit_result"] = kwargs
@@ -438,10 +447,7 @@ def fake_emit_result(**kwargs):
assert review_deslop.main() == 0
- assert captured["run_codex_review"]["base"] == "abc123"
- assert captured["run_codex_review"]["commit_end"] == "def456"
- assert captured["run_codex_review"].get("commit") is None
- prompt = str(captured["run_codex_review"]["prompt"])
+ prompt = str(captured["run_codex"]["prompt"])
assert "commit range `abc123..def456`" in prompt
assert captured["emit_result"]["result"]["final_message"] == "No findings."
assert "=== BEGIN DIFF ===" not in prompt
diff --git a/plugins/review-suite/tests/test_review_followup.py b/plugins/review-suite/tests/test_review_followup.py
index 1bfb79d..732105e 100644
--- a/plugins/review-suite/tests/test_review_followup.py
+++ b/plugins/review-suite/tests/test_review_followup.py
@@ -52,11 +52,6 @@ def test_main_uses_recorded_anchor_and_records_new_followup_anchor(
monkeypatch.setattr(
review_followup, "ensure_clean_git_worktree", lambda *args, **kwargs: None
)
- monkeypatch.setattr(
- review_followup,
- "use_unsafe_windows_wsl_fallback",
- lambda *args, **kwargs: False,
- )
monkeypatch.setattr(
review_followup,
"effective_base_ref",
@@ -154,11 +149,6 @@ def test_main_records_effective_branch_base_for_followup_anchor(
monkeypatch.setattr(
review_followup, "ensure_clean_git_worktree", lambda *args, **kwargs: None
)
- monkeypatch.setattr(
- review_followup,
- "use_unsafe_windows_wsl_fallback",
- lambda *args, **kwargs: False,
- )
monkeypatch.setattr(
review_followup,
"effective_base_ref",
@@ -251,11 +241,6 @@ def test_main_rejects_empty_followup_interdiff(monkeypatch, tmp_path: Path) -> N
errors: list[tuple[str, dict[str, object]]] = []
monkeypatch.setattr(review_followup, "resolve_repo_root", lambda cd: tmp_path)
- monkeypatch.setattr(
- review_followup,
- "use_unsafe_windows_wsl_fallback",
- lambda *args, **kwargs: False,
- )
monkeypatch.setattr(
review_followup,
"effective_base_ref",
diff --git a/plugins/review-suite/tests/test_review_gate.py b/plugins/review-suite/tests/test_review_gate.py
index 3043e5b..61d06bd 100644
--- a/plugins/review-suite/tests/test_review_gate.py
+++ b/plugins/review-suite/tests/test_review_gate.py
@@ -1947,10 +1947,13 @@ def fake_launch_gate_run(
def test_launch_gate_run_preserves_retry_attempts(monkeypatch, tmp_path: Path) -> None:
+ captured: dict[str, object] = {}
+
class FakeProc:
def __init__(self, *args, **kwargs) -> None:
self.pid = 123
self.stdin = None
+ captured.update(kwargs)
monkeypatch.setattr("review_gate.subprocess.Popen", FakeProc)
monkeypatch.setattr(
@@ -1958,6 +1961,7 @@ def __init__(self, *args, **kwargs) -> None:
lambda **kwargs: SimpleNamespace(
command=["codex", "review"],
cwd=tmp_path,
+ env={"GIT_CONFIG_COUNT": "1"},
stdin_text=None,
final_message_path=None,
),
@@ -1977,6 +1981,7 @@ def __init__(self, *args, **kwargs) -> None:
try:
assert run["retry_attempts"] == 1
+ assert captured["env"] == {"GIT_CONFIG_COUNT": "1"}
finally:
for key in ("stdout_path", "stderr_path"):
Path(run[key]).unlink(missing_ok=True)
diff --git a/plugins/review-suite/tests/test_review_local.py b/plugins/review-suite/tests/test_review_local.py
index 77f6503..d5e5748 100644
--- a/plugins/review-suite/tests/test_review_local.py
+++ b/plugins/review-suite/tests/test_review_local.py
@@ -713,9 +713,6 @@ def test_review_plan_forwards_skip_git_repo_check(
captured: dict[str, object] = {}
monkeypatch.setattr(review_plan, "resolve_repo_root", lambda value: tmp_path)
- monkeypatch.setattr(
- review_plan, "use_unsafe_windows_wsl_fallback", lambda *args: False
- )
monkeypatch.setattr(
review_plan,
"lens_model_config",
diff --git a/plugins/review-suite/tests/test_review_orchestrator_cli.py b/plugins/review-suite/tests/test_review_orchestrator_cli.py
index e27d297..af14afa 100644
--- a/plugins/review-suite/tests/test_review_orchestrator_cli.py
+++ b/plugins/review-suite/tests/test_review_orchestrator_cli.py
@@ -120,7 +120,12 @@ def _stub_deslop(monkeypatch: pytest.MonkeyPatch, *returncodes: int) -> list[lis
def fake_run(*, command: list[str], cwd: Path) -> subprocess.CompletedProcess:
calls.append(command)
index = min(len(calls) - 1, len(codes) - 1)
- return subprocess.CompletedProcess(command, codes[index], stdout="", stderr="")
+ return subprocess.CompletedProcess(
+ command,
+ codes[index],
+ stdout="Conformance: NOT_APPLICABLE\nReview decision: clean\n",
+ stderr="",
+ )
monkeypatch.setattr(orchestrator_runner, "run_deslop_subprocess", fake_run)
return calls
@@ -446,7 +451,6 @@ def _use_single_step_normal_profile(
) -> None:
config = deepcopy(review.load_config(state_dir))
normal = config["orchestrator"]["profiles"]["stable"]["normal"]
- normal["deslop_enabled"] = False
normal["steps"] = [
{
"name": "precision-signoff",
@@ -1190,15 +1194,12 @@ def test_create_resume_and_id_reprint_use_one_pending_action(
assert "mode" not in payload
assert "selection" not in payload
assert "grading" not in payload
- assert set(dict(payload["Action"])) == {"cmd", "alt", "deslop_done"}
+ assert set(dict(payload["Action"])) == {"cmd", "alt"}
assert f"--id {public_id}" in str(payload["Action"]["cmd"])
assert "--state-dir" not in str(payload["Action"]["cmd"])
assert str(state_dir) not in str(payload["Action"]["cmd"])
assert "--decision clean" in str(payload["Action"]["cmd"])
- assert "--deslop-done" in str(payload["Action"]["deslop_done"])
- assert "--state-dir" not in str(payload["Action"]["deslop_done"])
- assert str(state_dir) not in str(payload["Action"]["deslop_done"])
- assert len(deslop_calls) == 1
+ assert len(deslop_calls) == 0
assert not (state_dir / "orchestrator" / "state_dirs.json").exists()
state = _cycle_payload(state_dir, public_id)
assert state["selection"] == {
@@ -1207,7 +1208,7 @@ def test_create_resume_and_id_reprint_use_one_pending_action(
"reason": "auto_stable_profile",
}
assert "grading" not in state
- assert state["deslop"]["status"] == "done"
+ assert state["deslop"]["status"] == "tracked"
assert len(state["rounds"]) == 1
assert len(review_calls) == 1
@@ -1215,7 +1216,7 @@ def test_create_resume_and_id_reprint_use_one_pending_action(
assert exit_code == 0
assert resumed["review"] == public_id
assert "stage" not in resumed
- assert set(dict(resumed["Action"])) == {"cmd", "alt", "deslop_done"}
+ assert set(dict(resumed["Action"])) == {"cmd", "alt"}
assert "--decision clean" in str(resumed["Action"]["cmd"])
assert "--decision findings" in str(resumed["Action"]["alt"])
assert "--state-dir" not in str(resumed["Action"]["cmd"])
@@ -1229,7 +1230,6 @@ def test_create_resume_and_id_reprint_use_one_pending_action(
assert state["rounds"][0]["lane"] == "review_t1"
assert state["rounds"][0]["review_status"] == "completed"
assert state["rounds"][0]["output_refs"] == ["rollout://phase_review-round-1/alpha"]
- assert len(deslop_calls) == 1
assert len(review_calls) == 1
exit_code, by_id = _run_review(
@@ -1248,7 +1248,6 @@ def test_create_resume_and_id_reprint_use_one_pending_action(
assert exit_code == 0
assert first_clean["review"] == public_id
assert "stage" not in first_clean
- assert set(dict(first_clean["Action"])) == {"cmd", "deslop_done"}
assert f"--id {public_id}" in str(first_clean["Action"]["cmd"])
assert "--state-dir" not in str(first_clean["Action"]["cmd"])
assert str(state_dir) not in str(first_clean["Action"]["cmd"])
@@ -1295,6 +1294,11 @@ def test_create_resume_and_id_reprint_use_one_pending_action(
)
assert exit_code == 0
assert "stage" not in final_clean
+ _, closure = _run_review(
+ monkeypatch, ["--id", public_id, "--state-dir", str(state_dir)]
+ )
+ assert closure["conformance"] == "NOT_APPLICABLE"
+ _, final_clean = _run_review(monkeypatch, ["--id", public_id, "--deslop-done"])
_assert_github_handoff(
final_clean["Action"],
public_id=public_id,
@@ -1402,80 +1406,6 @@ def test_new_cycle_defaults_to_normal_without_mode(
assert tuple(mode_action.choices) == ("fast", "normal", "deep")
-def test_decision_retries_failed_deslop_and_advances(
- monkeypatch: pytest.MonkeyPatch, tmp_path: Path
-) -> None:
- deslop_calls = _stub_deslop(monkeypatch, 9, 0)
- review_calls = _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",
- "normal",
- "--cd",
- str(repo),
- "--base",
- "main",
- "--state-dir",
- str(state_dir),
- ],
- )
- public_id = str(created["review"])
-
- _run_review(
- monkeypatch,
- ["--id", public_id, "--decision", "clean", "--state-dir", str(state_dir)],
- )
-
- state = _cycle_payload(state_dir, public_id)
- assert state["deslop"]["status"] == "done"
- assert [decision["command"] for decision in state["decisions"]] == ["clean"]
- assert state["validation"]["review_green"] == "passed"
- assert len(deslop_calls) == 2
- assert len(review_calls) == 1
-
-
-def test_automatic_decision_waits_for_failed_deslop_retry(
- monkeypatch: pytest.MonkeyPatch, tmp_path: Path
-) -> None:
- deslop_calls = _stub_deslop(monkeypatch, 9, 0)
- review_calls = _stub_review_with_terminal(monkeypatch, "clean")
- 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",
- "normal",
- "--cd",
- str(repo),
- "--base",
- "main",
- "--state-dir",
- str(state_dir),
- ],
- )
- public_id = str(created["review"])
- assert _cycle_payload(state_dir, public_id)["decisions"] == []
-
- _run_review(monkeypatch, ["--id", public_id, "--state-dir", str(state_dir)])
-
- state = _cycle_payload(state_dir, public_id)
- assert state["deslop"]["status"] == "done"
- assert [decision["command"] for decision in state["decisions"]] == ["clean"]
- assert state["validation"]["review_green"] == "passed"
- assert len(deslop_calls) == 2
- assert len(review_calls) == 1
-
-
def test_head_change_waits_for_failed_deslop_retry(
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
) -> None:
@@ -1507,12 +1437,12 @@ def test_head_change_waits_for_failed_deslop_retry(
_run_review(monkeypatch, ["--id", public_id, "--state-dir", str(state_dir)])
state = _cycle_payload(state_dir, public_id)
- assert state["deslop"]["status"] == "done"
+ assert state["deslop"]["status"] == "tracked"
assert [decision["command"] for decision in state["decisions"]] == ["findings"]
assert state["review_heads"]["last_fix_head"] == fixed_head
assert state["pending_action"]["fix_verification"]["findings_reviewed_head"]
- assert len(deslop_calls) == 2
- assert len(review_calls) == 1
+ assert len(deslop_calls) == 0
+ assert len(review_calls) == 2
@pytest.mark.parametrize("flag", ["--skip-deslop", "--no-deslop"])
@@ -1533,7 +1463,21 @@ def test_create_with_skip_deslop_runs_review_without_sidecar(
monkeypatch,
[
"--mode",
- "normal",
+ "fast",
+ flag,
+ "--cd",
+ str(repo),
+ "--base",
+ "main",
+ "--state-dir",
+ str(state_dir),
+ ],
+ )
+ _, resumed = _run_review(
+ monkeypatch,
+ [
+ "--mode",
+ "fast",
flag,
"--cd",
str(repo),
@@ -1546,11 +1490,13 @@ def test_create_with_skip_deslop_runs_review_without_sidecar(
assert exit_code == 0
public_id = str(payload["review"])
+ assert resumed["review"] == public_id
assert set(dict(payload["Action"])) == {"cmd", "alt"}
assert "--decision clean" in str(payload["Action"]["cmd"])
assert "--decision findings" in str(payload["Action"]["alt"])
assert len(deslop_calls) == 0
assert len(review_calls) == 1
+ assert len(list((state_dir / "orchestrator" / "cycles").glob("*.json"))) == 1
state = _cycle_payload(state_dir, public_id)
assert state["deslop"] == {"tracked": False, "status": "skipped", "source": "cli"}
assert state["rounds"][0]["round_id"] == "phase_review-round-1"
@@ -1665,11 +1611,12 @@ def test_skip_deslop_does_not_resume_same_head_sidecar_cycle(
)
assert sidecar["review"] != skipped["review"]
- assert len(deslop_calls) == 1
+ assert len(deslop_calls) == 0
assert len(review_calls) == 2
assert len(list((state_dir / "orchestrator" / "cycles").glob("*.json"))) == 2
assert (
- _cycle_payload(state_dir, str(sidecar["review"]))["deslop"]["status"] == "done"
+ _cycle_payload(state_dir, str(sidecar["review"]))["deslop"]["status"]
+ == "tracked"
)
assert _cycle_payload(state_dir, str(skipped["review"]))["deslop"] == {
"tracked": False,
@@ -1720,6 +1667,8 @@ def test_id_auto_records_structured_clean_and_runs_next_step(
assert exit_code == 0
assert len(review_calls) == 2
+ _run_review(monkeypatch, ["--id", public_id, "--state-dir", str(state_dir)])
+ _, final = _run_review(monkeypatch, ["--id", public_id, "--deslop-done"])
_assert_github_handoff(
final["Action"],
public_id=public_id,
@@ -1941,7 +1890,7 @@ def test_wsl_flag_persists_to_orchestrated_steps(
review_calls = _stub_review(monkeypatch, "phase_review-round-1")
repo = tmp_path / "repo"
state_dir = tmp_path / "state"
- _use_compact_normal_profile(monkeypatch, state_dir)
+ _use_single_step_normal_profile(monkeypatch, state_dir)
_init_repo(repo)
_commit_file(repo, "app.txt", "base\n", "base")
_git(repo, "checkout", "-b", "feature/wsl-review")
@@ -1962,7 +1911,7 @@ def test_wsl_flag_persists_to_orchestrated_steps(
public_id = str(payload["review"])
assert exit_code == 0
- assert "--wsl" in deslop_calls[0]
+ assert deslop_calls == []
state = _cycle_payload(state_dir, public_id)
assert state["runtime"] == {"allow_unsafe_windows_wsl_fallback": True}
@@ -1973,6 +1922,10 @@ def test_wsl_flag_persists_to_orchestrated_steps(
assert exit_code == 0
assert review_calls[0]["allow_unsafe_windows_wsl_fallback"] is True
+ _run_review(monkeypatch, ["--id", public_id, "--decision", "clean"])
+ _run_review(monkeypatch, ["--id", public_id, "--state-dir", str(state_dir)])
+ assert "--wsl" in deslop_calls[0]
+
def test_state_dir_flag_is_rejected(
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
@@ -2030,7 +1983,7 @@ def __exit__(self, *args: object) -> None:
}
-def test_id_show_findings_reads_orchestrator_round_payload_without_running(
+def test_id_show_findings_prefers_closure_over_round_payload_without_running(
monkeypatch: pytest.MonkeyPatch,
tmp_path: Path,
capsys: pytest.CaptureFixture[str],
@@ -2122,6 +2075,13 @@ def test_id_show_findings_reads_orchestrator_round_payload_without_running(
assert "task:" not in captured.out
assert "status:" not in captured.out
assert "Alpha recovered finding" in captured.out
+
+ state["deslop"]["findings"] = "Closure recovered finding"
+ _write_cycle_payload(state_dir, public_id, state)
+ assert review.main() == 0
+ captured = capsys.readouterr()
+ assert "Closure recovered finding" in captured.out
+ assert "Alpha recovered finding" not in captured.out
assert len(review_calls) == before_calls
@@ -2175,7 +2135,7 @@ def fail_deslop(*, command: list[str], cwd: Path) -> subprocess.CompletedProcess
payload["merge_base"] == str(dict(before_state["identity"])["merge_base"])[:12]
)
assert payload["rounds"] == 1
- assert payload["deslop"] == "skipped-fast"
+ assert payload["deslop"] == "tracked"
assert payload["review_brief"] == "unavailable"
assert payload["design_conformance_context"] == "unavailable"
assert dict(payload["worktree"]) == {
@@ -2369,7 +2329,6 @@ def test_restart_mode_supersedes_cycle_and_starts_fresh_deep_ladder(
assert exit_code == 0
assert new_id != old_id
assert f"--id {new_id}" in str(restarted["Action"]["cmd"])
- assert len(deslop_calls) == 2
assert len(review_calls) == 2
assert len(list((state_dir / "orchestrator" / "cycles").glob("*.json"))) == 2
@@ -2432,7 +2391,7 @@ def test_restart_mode_supersedes_cycle_and_starts_fresh_deep_ladder(
],
)
assert retry["review"] == new_id
- assert len(deslop_calls) == 2
+ assert len(deslop_calls) == 0
assert len(review_calls) == 2
_, old_reprint = _run_review(
@@ -2690,6 +2649,7 @@ def test_fast_review_can_restart_into_deep_without_becoming_a_restart_target(
[
"--mode",
"fast",
+ "--skip-deslop",
"--cd",
str(repo),
"--base",
@@ -2717,12 +2677,17 @@ def test_fast_review_can_restart_into_deep_without_becoming_a_restart_target(
new_id = str(restarted["review"])
assert exit_code == 0
assert new_id != old_id
- assert len(deslop_calls) == 1
+ assert len(deslop_calls) == 0
assert len(review_calls) == 2
assert _cycle_payload(state_dir, old_id)["stage"] == "aborted"
new_state = _cycle_payload(state_dir, new_id)
assert new_state["mode"] == {"requested": "deep", "effective": "deep"}
assert new_state["restart"]["from_mode"] == "fast"
+ assert new_state["deslop"] == {
+ "tracked": False,
+ "status": "skipped",
+ "source": "cli",
+ }
restart_action = next(
action
@@ -2909,7 +2874,7 @@ def test_review_orchestrator_help_hides_internal_selection() -> None:
assert "--state-dir" not in help_text
-def test_deslop_done_closes_tracked_sidecar_without_rerunning_deslop(
+def test_deslop_done_rechecks_exact_head_before_closing(
monkeypatch: pytest.MonkeyPatch,
tmp_path: Path,
) -> None:
@@ -2919,6 +2884,9 @@ def test_deslop_done_closes_tracked_sidecar_without_rerunning_deslop(
state_dir = tmp_path / "state"
_init_repo(repo)
_commit_file(repo, "app.txt", "base\n", "base")
+ _git(repo, "checkout", "-b", "feature/closure-drift")
+ advanced_base = _commit_file(repo, "app.txt", "base\nstep\n", "stack step")
+ head = _commit_file(repo, "app.txt", "base\nstep\nhead\n", "feature head")
_, created = _run_review(
monkeypatch,
@@ -2934,29 +2902,26 @@ def test_deslop_done_closes_tracked_sidecar_without_rerunning_deslop(
],
)
public_id = str(created["review"])
+ _, completed = _run_review(monkeypatch, ["--id", public_id, "--decision", "clean"])
+ _, completed = _run_review(monkeypatch, ["--id", public_id])
- assert "--deslop-done" in str(created["Action"]["deslop_done"])
+ assert "--deslop-done" in str(completed["Action"]["cmd"])
assert len(deslop_calls) == 1
+ drifted = _cycle_payload(state_dir, public_id)
+ drifted["deslop"]["conformance"] = "MATERIALLY_DRIFTED"
+ assert "Revise" in review._action_payload(drifted, state_dir=state_dir)["note"]
- exit_code, closed = _run_review(
- monkeypatch,
- ["--id", public_id, "--deslop-done", "--state-dir", str(state_dir)],
- )
+ _git(repo, "branch", "-f", "main", advanced_base)
+ exit_code, resumed = _run_review(monkeypatch, ["--id", public_id, "--deslop-done"])
assert exit_code == 0
- assert closed["review"] == public_id
- assert "deslop_done" not in dict(closed["Action"])
+ assert resumed["review"] == public_id
state = _cycle_payload(state_dir, public_id)
- assert state["deslop"]["tracked"] is False
- assert state["deslop"]["status"] == "closed"
-
- exit_code, resumed = _run_review(
- monkeypatch, ["--id", public_id, "--state-dir", str(state_dir)]
- )
-
- assert exit_code == 0
- assert "--decision clean" in str(resumed["Action"]["cmd"])
- assert "deslop_done" not in dict(resumed["Action"])
+ assert state["identity"]["head"] == head
+ assert state["identity"]["merge_base"] == advanced_base
+ assert state["stage"] == "created"
+ assert state["pending_action"]["kind"] == "run-review-step"
+ assert state["deslop"]["status"] == "tracked"
assert len(deslop_calls) == 1
assert len(review_calls) == 1
@@ -2972,7 +2937,8 @@ def test_deslop_done_is_primary_action_when_no_other_action_remains() -> None:
assert action == {
"cmd": review._review_command(
"rvw_example", "--deslop-done", state_dir=state_dir
- )
+ ),
+ "note": "Acknowledge the exact-head closure before continuing.",
}
@@ -3000,48 +2966,6 @@ def test_deslop_done_requires_id_and_rejects_other_actions(
assert "--deslop-done cannot be combined" in errors[-1]
-def test_deslop_step_prints_output_once(
- monkeypatch: pytest.MonkeyPatch,
- tmp_path: Path,
- capsys: pytest.CaptureFixture[str],
-) -> None:
- calls: list[list[str]] = []
- _stub_review(monkeypatch)
-
- def fake_run(*, command: list[str], cwd: Path) -> subprocess.CompletedProcess:
- calls.append(command)
- return subprocess.CompletedProcess(
- command, 0, stdout="Remove redundant helper.\n", stderr=""
- )
-
- monkeypatch.setattr(orchestrator_runner, "run_deslop_subprocess", fake_run)
- repo = tmp_path / "repo"
- state_dir = tmp_path / "state"
- _init_repo(repo)
- _commit_file(repo, "app.txt", "base\n", "base")
-
- exit_code, payload = _run_review(
- monkeypatch,
- [
- "--mode",
- "normal",
- "--cd",
- str(repo),
- "--base",
- "main",
- "--state-dir",
- str(state_dir),
- ],
- )
- captured = capsys.readouterr()
-
- assert exit_code == 0
- assert "stage" not in payload
- assert captured.out.count("review-deslop:") == 1
- assert "Remove redundant helper." in captured.out
- assert "--output-only" in calls[0]
-
-
def test_review_step_output_is_not_reprinted_by_review_py(
monkeypatch: pytest.MonkeyPatch,
tmp_path: Path,
@@ -3180,6 +3104,8 @@ def test_github_review_rejects_cycle_before_local_green(
str(state_dir),
],
)
+ public_id = str(created["review"])
+ _run_review(monkeypatch, ["--id", public_id, "--decision", "clean"])
errors: list[tuple[str, dict[str, object]]] = []
def fake_error(message: str, **kwargs: object) -> int:
@@ -3188,7 +3114,7 @@ def fake_error(message: str, **kwargs: object) -> int:
monkeypatch.setattr(review, "emit_error", fake_error)
monkeypatch.setattr(
- sys, "argv", ["review.py", "--id", str(created["review"]), "--github-review"]
+ sys, "argv", ["review.py", "--id", public_id, "--github-review"]
)
exit_code = review.main()
@@ -3196,7 +3122,7 @@ def fake_error(message: str, **kwargs: object) -> int:
assert exit_code == 2
assert errors == [
(
- "--github-review requires local green review state",
+ "--github-review requires completed exact-head closure",
{
"status": "usage_error",
"help_items": [review._help_command()],
@@ -3209,6 +3135,7 @@ def test_github_review_runs_existing_lane_with_canonical_state_dir_and_force(
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
) -> None:
review_calls = _stub_review(monkeypatch)
+ _stub_deslop(monkeypatch)
repo = tmp_path / "repo"
state_dir = tmp_path / "default-state"
_init_repo(repo)
@@ -3233,12 +3160,17 @@ def test_github_review_runs_existing_lane_with_canonical_state_dir_and_force(
["--id", public_id, "--decision", "clean", "--state-dir", str(state_dir)],
)
assert "stage" not in clean
+ _run_review(monkeypatch, ["--id", public_id, "--state-dir", str(state_dir)])
+ _run_review(monkeypatch, ["--id", public_id, "--deslop-done"])
calls: list[list[str]] = []
+ real_subprocess_run = review.subprocess.run
def fake_subprocess_run(
- command: list[str], check: bool
+ command: list[str], check: bool, **_kwargs: object
) -> subprocess.CompletedProcess:
+ if command[0] == "git":
+ return real_subprocess_run(command, check=check, **_kwargs)
calls.append(command)
return subprocess.CompletedProcess(command, 23)
@@ -3271,7 +3203,7 @@ def test_github_result_findings_reenters_existing_cycle_for_final_signoff(
monkeypatch: pytest.MonkeyPatch,
tmp_path: Path,
) -> None:
- _stub_deslop(monkeypatch)
+ deslop_calls = _stub_deslop(monkeypatch)
review_calls = _stub_review(monkeypatch, "signoff-round-1", "signoff-round-2")
followup_calls = _stub_followup(monkeypatch, "github-followup-1")
repo = tmp_path / "repo"
@@ -3296,10 +3228,9 @@ def test_github_result_findings_reenters_existing_cycle_for_final_signoff(
],
)
public_id = str(opened["review"])
- _, green = _run_review(
- monkeypatch,
- ["--id", public_id, "--decision", "clean", "--state-dir", str(state_dir)],
- )
+ _run_review(monkeypatch, ["--id", public_id, "--decision", "clean"])
+ _run_review(monkeypatch, ["--id", public_id, "--state-dir", str(state_dir)])
+ _, green = _run_review(monkeypatch, ["--id", public_id, "--deslop-done"])
_assert_github_handoff(
green["Action"],
public_id=public_id,
@@ -3332,7 +3263,7 @@ def test_github_result_findings_reenters_existing_cycle_for_final_signoff(
assert state["active_findings"]["profile_round_id"] == "signoff-round-1"
assert state["validation"]["review_green"] == "unknown"
- _commit_file(repo, "app.txt", "feature\nfixed\n", "fix github finding")
+ fixed_head = _commit_file(repo, "app.txt", "feature\nfixed\n", "fix github finding")
_, signoff = _run_review(
monkeypatch, ["--id", public_id, "--state-dir", str(state_dir)]
)
@@ -3346,10 +3277,12 @@ def test_github_result_findings_reenters_existing_cycle_for_final_signoff(
assert state["pending_action"]["round_id"] == "signoff-round-2"
assert state["review_progress"]["completed_steps"] == []
- _, final_clean = _run_review(
- monkeypatch,
- ["--id", public_id, "--decision", "clean", "--state-dir", str(state_dir)],
- )
+ _run_review(monkeypatch, ["--id", public_id, "--decision", "clean"])
+ _run_review(monkeypatch, ["--id", public_id, "--state-dir", str(state_dir)])
+ assert len(deslop_calls) == 2
+ assert fixed_head == deslop_calls[-1][deslop_calls[-1].index("--commit") + 2]
+ assert "--conformance-only" in deslop_calls[-1]
+ _, final_clean = _run_review(monkeypatch, ["--id", public_id, "--deslop-done"])
_assert_github_handoff(
final_clean["Action"],
public_id=public_id,
@@ -3388,10 +3321,9 @@ def test_github_result_clean_and_waived_are_terminal_for_existing_cycle(
],
)
public_id = str(opened["review"])
- _run_review(
- monkeypatch,
- ["--id", public_id, "--decision", "clean", "--state-dir", str(state_dir)],
- )
+ _run_review(monkeypatch, ["--id", public_id, "--decision", "clean"])
+ _run_review(monkeypatch, ["--id", public_id, "--state-dir", str(state_dir)])
+ _run_review(monkeypatch, ["--id", public_id, "--deslop-done"])
exit_code, clean = _run_review(
monkeypatch,
@@ -3532,67 +3464,30 @@ def fake_error(message: str, **kwargs: object) -> int:
stale_head = _commit_file(
repo, "app.txt", "base\nnew work\n", "new work after github waiver"
)
- _, stale = _run_review(
- monkeypatch, ["--id", public_id, "--state-dir", str(state_dir)]
- )
- assert stale["status"] == "head_changed_after_review"
- assert stale["done"] is False
- assert stale["review_ladder"] == "head_changed_after_review"
- assert stale["next_action"] == "validation"
- assert stale["head_changed_after_review"] is True
- assert stale["reviewed_head"] == reviewed_head
- assert stale["current_head"] == stale_head
- assert stale["changed_since_review"] == ["app.txt"]
- assert "review remains green after test-only fixes" in stale["note"]
- assert "do not rerun the review" in stale["note"]
- assert "production code or intended behavior changed" in stale["note"]
- assert stale["github_review"] == "waived"
- assert stale["Action"]["blocked_by"] == ["full_suite:unknown", "ci:unknown"]
- assert "--full-suite FULL_SUITE_STATUS --ci CI_STATUS" in str(
- stale["Action"]["cmd"]
- )
-
exit_code, stale = _run_review(
monkeypatch,
[
"--id",
public_id,
"--full-suite",
- "waived",
+ "passed",
"--ci",
- "waived",
- "--validation-note",
- "Full suite and CI waived for this test-only head change",
+ "passed",
"--state-dir",
str(state_dir),
],
)
assert exit_code == 0
- assert stale["status"] == "head_changed_after_review"
assert stale["done"] is False
- assert stale["review_ladder"] == "head_changed_after_review"
- assert stale["next_action"] == "inspect_changed_since_review"
- assert stale["head_changed_after_review"] is True
- assert stale["current_head"] == stale_head
- assert stale["changed_since_review"] == ["app.txt"]
- assert "review remains green after test-only fixes" in stale["note"]
- assert "Action" not in stale
-
+ assert stale["review_ladder"] == "pending"
+ assert stale["next_action"] == "continue"
+ assert "--full-suite" not in str(stale["Action"]["cmd"])
state = _cycle_payload(state_dir, public_id)
- state["github_review"] = {"status": "unknown"}
- state["validation"]["full_suite"] = "unknown"
- state["validation"]["ci"] = "unknown"
- _write_cycle_payload(state_dir, public_id, state)
- _, stale = _run_review(
- monkeypatch, ["--id", public_id, "--state-dir", str(state_dir)]
- )
- assert stale["status"] == "stale"
- assert stale["done"] is False
- assert stale["review_ladder"] == "invalidated"
- assert stale["next_action"] == "rerun_review"
- assert stale["current_head"] == stale_head
- assert "github_review" not in stale
- assert "Action" not in stale
+ assert stale_head != reviewed_head and state["identity"]["head"] == stale_head
+ assert state["github_review"] == {"status": "unknown"}
+ assert state["validation"]["full_suite"] == "unknown"
+ assert state["validation"]["ci"] == "unknown"
+ assert state["deslop"]["status"] == "tracked"
def test_github_result_findings_does_not_auto_start_followup_when_fix_already_committed(
@@ -3647,17 +3542,7 @@ def test_github_result_findings_does_not_auto_start_followup_when_fix_already_co
)
assert exit_code == 0
- assert (
- findings["Action"]["note"]
- == "Commit/amend valid fixes, then rerun this command."
- )
assert len(followup_calls) == 0
- state = _cycle_payload(state_dir, public_id)
- assert state["stage"] == "fix-pending"
- assert (
- state["active_findings"]["reviewed_head"]
- == state["review_heads"]["last_reviewed_head"]
- )
def test_pending_github_review_after_amend_reuses_same_id_for_signoff(
@@ -3688,10 +3573,12 @@ def test_pending_github_review_after_amend_reuses_same_id_for_signoff(
],
)
public_id = str(opened["review"])
- _run_review(
+ _, final_clean = _run_review(
monkeypatch,
["--id", public_id, "--decision", "clean", "--state-dir", str(state_dir)],
)
+ _run_review(monkeypatch, ["--id", public_id, "--state-dir", str(state_dir)])
+ _run_review(monkeypatch, ["--id", public_id, "--deslop-done"])
_run_review(
monkeypatch,
[
@@ -3742,10 +3629,12 @@ def fail_github_review(*args: object, **kwargs: object) -> int:
assert state["validation"]["full_suite"] == "unknown"
assert state["validation"]["ci"] == "unknown"
- _, final_clean = _run_review(
+ _run_review(
monkeypatch,
["--id", public_id, "--decision", "clean", "--state-dir", str(state_dir)],
)
+ _run_review(monkeypatch, ["--id", public_id, "--state-dir", str(state_dir)])
+ _, final_clean = _run_review(monkeypatch, ["--id", public_id, "--deslop-done"])
_assert_github_handoff(
final_clean["Action"],
public_id=public_id,
@@ -3818,10 +3707,11 @@ def test_mode_rerun_after_pending_github_head_change_reuses_same_id_for_signoff(
assert state["github_review"]["status"] == "unknown"
-def test_mode_rerun_after_patch_equivalent_green_base_drift_keeps_handoff(
+def test_patch_equivalent_rebase_after_closed_deslop_reopens_local_closure(
monkeypatch: pytest.MonkeyPatch,
tmp_path: Path,
) -> None:
+ _stub_deslop(monkeypatch)
review_calls = _stub_review(monkeypatch, "signoff-round-1")
repo = tmp_path / "repo"
state_dir = tmp_path / "state"
@@ -3849,6 +3739,8 @@ def test_mode_rerun_after_patch_equivalent_green_base_drift_keeps_handoff(
monkeypatch,
["--id", public_id, "--decision", "clean", "--state-dir", str(state_dir)],
)
+ _run_review(monkeypatch, ["--id", public_id, "--state-dir", str(state_dir)])
+ _run_review(monkeypatch, ["--id", public_id, "--deslop-done"])
_git(repo, "checkout", "main")
current_base = _commit_file(repo, "docs/notes.md", "main notes\n", "main moves")
@@ -3872,20 +3764,19 @@ def test_mode_rerun_after_patch_equivalent_green_base_drift_keeps_handoff(
assert exit_code == 0
assert resumed["review"] == public_id
- assert resumed["next_action"] == "github_review"
- assert "--github-review" in str(resumed["Action"]["cmd"])
- assert len(review_calls) == 1
+ assert "--decision clean" in str(resumed["Action"]["cmd"])
+ assert len(review_calls) == 2
assert len(list((state_dir / "orchestrator" / "cycles").glob("*.json"))) == 1
state = _cycle_payload(state_dir, public_id)
- assert state["stage"] == "review-green"
+ assert state["stage"] == "decision-pending"
assert state["identity"]["head"] == rebased_head
assert state["identity"]["merge_base"] == current_base
- assert state["review_heads"]["last_reviewed_head"] == rebased_head
- assert state["rounds"][0]["reviewed_head"] == rebased_head
- assert state["decisions"][0]["reviewed_head"] == rebased_head
- assert (
- state["review_progress"]["completed_steps"][0]["reviewed_head"] == rebased_head
- )
+ assert state["deslop"] == {
+ "tracked": True,
+ "status": "tracked",
+ "cleanup_completed": True,
+ "conformance_only": True,
+ }
assert state["base_drift"] == {
"status": "ignored_no_path_overlap",
"recorded_merge_base": base_at_review,
@@ -3948,7 +3839,7 @@ def test_github_result_after_amend_requires_same_id_signoff(
assert state["github_review"]["status"] == "unknown"
-def test_mode_rerun_after_concurrent_deslop_amend_reuses_existing_cycle(
+def test_mode_rerun_after_pending_review_amend_reuses_existing_cycle(
monkeypatch: pytest.MonkeyPatch,
tmp_path: Path,
) -> None:
@@ -3978,9 +3869,9 @@ def test_mode_rerun_after_concurrent_deslop_amend_reuses_existing_cycle(
public_id = str(created["review"])
state = _cycle_payload(state_dir, public_id)
assert state["stage"] == "decision-pending"
- assert state["deslop"]["status"] == "done"
+ assert state["deslop"]["status"] == "tracked"
assert state["identity"]["head"] == original_head
- assert len(deslop_calls) == 1
+ assert len(deslop_calls) == 0
amended_head = _amend_file(repo, "app.txt", "feature\nfix from deslop\n")
assert amended_head != original_head
@@ -4002,7 +3893,7 @@ def test_mode_rerun_after_concurrent_deslop_amend_reuses_existing_cycle(
assert exit_code == 0
assert resumed["review"] == public_id
assert "--decision clean" in str(resumed["Action"]["cmd"])
- assert len(deslop_calls) == 1
+ assert len(deslop_calls) == 0
assert len(review_calls) == 2
assert len(list((state_dir / "orchestrator" / "cycles").glob("*.json"))) == 1
state = _cycle_payload(state_dir, public_id)
@@ -4025,7 +3916,7 @@ def test_mode_rerun_after_concurrent_deslop_amend_reuses_existing_cycle(
)
assert exit_code == 0
assert restarted["review"] != public_id
- assert len(deslop_calls) == 2
+ assert len(deslop_calls) == 0
def test_mode_rerun_allows_non_overlapping_merge_base_drift_without_rerunning_deslop(
@@ -4056,7 +3947,7 @@ def test_mode_rerun_allows_non_overlapping_merge_base_drift_without_rerunning_de
],
)
public_id = str(created["review"])
- assert len(deslop_calls) == 1
+ assert len(deslop_calls) == 0
_git(repo, "checkout", "main")
current_base = _commit_file(repo, "docs/notes.md", "main notes\n", "main moves")
@@ -4082,7 +3973,7 @@ def test_mode_rerun_allows_non_overlapping_merge_base_drift_without_rerunning_de
assert exit_code == 0
assert resumed["review"] == public_id
assert "--decision clean" in str(resumed["Action"]["cmd"])
- assert len(deslop_calls) == 1
+ assert len(deslop_calls) == 0
assert len(review_calls) == 2
assert len(list((state_dir / "orchestrator" / "cycles").glob("*.json"))) == 1
state = _cycle_payload(state_dir, public_id)
@@ -4154,7 +4045,7 @@ def test_mode_rerun_after_non_equivalent_base_drift_starts_fresh_cycle(
assert exit_code == 0
assert fresh["review"] != old_id
- assert len(deslop_calls) == 2
+ assert len(deslop_calls) == 0
state = _cycle_payload(state_dir, str(fresh["review"]))
assert state["identity"]["head"] == edited_head
assert len(list((state_dir / "orchestrator" / "cycles").glob("*.json"))) == 2
@@ -4208,7 +4099,7 @@ def test_mode_rerun_after_initial_review_commit_reuses_cycle(
assert exit_code == 0
assert fresh["review"] == old_id
- assert len(deslop_calls) == 1
+ assert len(deslop_calls) == 0
state = _cycle_payload(state_dir, old_id)
assert state["identity"]["head"] == new_head
assert len(review_calls) == 2
@@ -4267,7 +4158,7 @@ def test_mode_rerun_after_overlapping_merge_base_drift_starts_fresh_cycle(
assert exit_code == 0
assert fresh["review"] != old_id
- assert len(deslop_calls) == 2
+ assert len(deslop_calls) == 0
state = _cycle_payload(state_dir, str(fresh["review"]))
assert state["identity"]["head"] == rebased_head
assert len(list((state_dir / "orchestrator" / "cycles").glob("*.json"))) == 2
@@ -4319,7 +4210,7 @@ def test_mode_rerun_after_initial_review_reset_reuses_cycle(
assert exit_code == 0
assert fresh["review"] == old_id
- assert len(deslop_calls) == 1
+ assert len(deslop_calls) == 0
state = _cycle_payload(state_dir, old_id)
assert state["identity"]["head"] == base_head
assert len(list((state_dir / "orchestrator" / "cycles").glob("*.json"))) == 1
@@ -4765,7 +4656,7 @@ def test_clean_followup_note_does_not_leak_to_later_review_steps(
["--id", public_id, "--decision", "clean", "--state-dir", str(state_dir)],
)
- assert set(dict(final_step_ready["Action"])) == {"cmd", "deslop_done"}
+ assert set(dict(final_step_ready["Action"])) == {"cmd"}
state = _cycle_payload(state_dir, public_id)
assert state["pending_action"] == {
"kind": "run-review-step",
@@ -4881,7 +4772,7 @@ def test_validation_flags_do_not_run_expensive_resume(
assert exit_code == 0
assert "stage" not in payload
- assert len(deslop_calls) == 1
+ assert len(deslop_calls) == 0
assert len(review_calls) == 1
assert followup_calls == []
state = _cycle_payload(state_dir, public_id)
@@ -4889,15 +4780,11 @@ def test_validation_flags_do_not_run_expensive_resume(
assert state["active_findings"]["round_id"] == "phase_review-round-1"
-def test_fast_mode_skips_deslop_and_runs_review(
+def test_fast_mode_runs_same_bounded_closure_after_review(
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
) -> None:
review_calls = _stub_review(monkeypatch)
-
- def fail_run(*, command: list[str], cwd: Path) -> subprocess.CompletedProcess:
- raise AssertionError("fast mode must not run deslop")
-
- monkeypatch.setattr(orchestrator_runner, "run_deslop_subprocess", fail_run)
+ _stub_deslop(monkeypatch)
repo = tmp_path / "repo"
state_dir = tmp_path / "state"
_init_repo(repo)
@@ -4929,13 +4816,14 @@ def fail_run(*, command: list[str], cwd: Path) -> subprocess.CompletedProcess:
)
assert exit_code == 0
assert "stage" not in clean
- assert clean["status"] == "done"
- assert clean["done"] is True
- assert clean["review_ladder"] == "complete"
- assert clean["next_action"] == "none"
- assert "Action" not in clean
- assert len(review_calls) == 1
- assert _gate_signoff_decisions(state_dir) == []
+ _, clean = _run_review(
+ monkeypatch, ["--id", public_id, "--state-dir", str(state_dir)]
+ )
+ assert clean["conformance"] == "NOT_APPLICABLE"
+ assert "--deslop-done" in str(clean["Action"]["cmd"])
+
+ _, closed = _run_review(monkeypatch, ["--id", public_id, "--deslop-done"])
+ assert "Action" not in closed
exit_code, github_clean = _run_review(
monkeypatch,
@@ -4954,10 +4842,7 @@ def test_fast_manual_github_findings_keeps_re_review_action(
) -> None:
review_calls = _stub_review(monkeypatch, "signoff-round-1", "signoff-round-2")
- def fail_run(*, command: list[str], cwd: Path) -> subprocess.CompletedProcess:
- raise AssertionError("fast mode must not run deslop")
-
- monkeypatch.setattr(orchestrator_runner, "run_deslop_subprocess", fail_run)
+ _stub_deslop(monkeypatch)
repo = tmp_path / "repo"
state_dir = tmp_path / "state"
_init_repo(repo)
@@ -4983,6 +4868,8 @@ def fail_run(*, command: list[str], cwd: Path) -> subprocess.CompletedProcess:
monkeypatch,
["--id", public_id, "--decision", "clean", "--state-dir", str(state_dir)],
)
+ _run_review(monkeypatch, ["--id", public_id, "--state-dir", str(state_dir)])
+ _, clean = _run_review(monkeypatch, ["--id", public_id, "--deslop-done"])
assert "Action" not in clean
exit_code, github_findings = _run_review(
@@ -5017,6 +4904,8 @@ def fail_run(*, command: list[str], cwd: Path) -> subprocess.CompletedProcess:
monkeypatch,
["--id", public_id, "--decision", "clean", "--state-dir", str(state_dir)],
)
+ _run_review(monkeypatch, ["--id", public_id, "--state-dir", str(state_dir)])
+ _, final_clean = _run_review(monkeypatch, ["--id", public_id, "--deslop-done"])
_assert_github_handoff(
final_clean["Action"],
public_id=public_id,
@@ -5034,10 +4923,7 @@ def test_stale_decision_renders_current_action_without_mutating_cycle(
) -> None:
review_calls = _stub_review(monkeypatch)
- def fail_run(*, command: list[str], cwd: Path) -> subprocess.CompletedProcess:
- raise AssertionError("fast mode must not run deslop")
-
- monkeypatch.setattr(orchestrator_runner, "run_deslop_subprocess", fail_run)
+ _stub_deslop(monkeypatch)
repo = tmp_path / "repo"
state_dir = tmp_path / "state"
_init_repo(repo)
@@ -5061,6 +4947,8 @@ def fail_run(*, command: list[str], cwd: Path) -> subprocess.CompletedProcess:
monkeypatch,
["--id", public_id, "--decision", "clean", "--state-dir", str(state_dir)],
)
+ _run_review(monkeypatch, ["--id", public_id, "--state-dir", str(state_dir)])
+ _run_review(monkeypatch, ["--id", public_id, "--deslop-done"])
before = _cycle_payload(state_dir, public_id)
exit_code, stale = _run_review(
@@ -5105,10 +4993,7 @@ def test_stale_decision_persists_auto_resume_transition(
) -> None:
review_calls = _stub_review(monkeypatch)
- def fail_run(*, command: list[str], cwd: Path) -> subprocess.CompletedProcess:
- raise AssertionError("fast mode must not run deslop")
-
- monkeypatch.setattr(orchestrator_runner, "run_deslop_subprocess", fail_run)
+ _stub_deslop(monkeypatch)
repo = tmp_path / "repo"
state_dir = tmp_path / "state"
_init_repo(repo)
@@ -5156,10 +5041,7 @@ def test_decision_pending_with_missing_metadata_still_errors(
) -> None:
_stub_review(monkeypatch)
- def fail_run(*, command: list[str], cwd: Path) -> subprocess.CompletedProcess:
- raise AssertionError("fast mode must not run deslop")
-
- monkeypatch.setattr(orchestrator_runner, "run_deslop_subprocess", fail_run)
+ _stub_deslop(monkeypatch)
repo = tmp_path / "repo"
state_dir = tmp_path / "state"
_init_repo(repo)
diff --git a/plugins/review-suite/tests/test_review_orchestrator_profiles.py b/plugins/review-suite/tests/test_review_orchestrator_profiles.py
index 5c07815..d638290 100644
--- a/plugins/review-suite/tests/test_review_orchestrator_profiles.py
+++ b/plugins/review-suite/tests/test_review_orchestrator_profiles.py
@@ -42,7 +42,6 @@ def test_default_stable_profiles_cover_all_modes(tmp_path: Path) -> None:
assert set(profiles["stable"]) == {"normal", "deep", "fast"}
assert config["arena"]["enabled"] is False
assert config["orchestrator"]["calibration"]["auto_promotion_enabled"] is False
- assert profiles["stable"]["normal"].deslop_enabled is True
assert [_step_summary(step) for step in profiles["stable"]["normal"].steps] == [
("review", "precision-signoff", 2, "medium", True),
]
@@ -57,7 +56,6 @@ def test_default_stable_profiles_cover_all_modes(tmp_path: Path) -> None:
assert profiles["stable"]["normal"].steps[-1].rerun_on_findings is True
assert profiles["stable"]["deep"].steps[0].rerun_on_findings is True
assert profiles["stable"]["deep"].steps[-1].rerun_on_findings is True
- assert profiles["stable"]["fast"].deslop_enabled is False
assert [_step_summary(step) for step in profiles["stable"]["fast"].steps] == [
("review", "fast-signoff", 2, "medium", False)
]
@@ -65,6 +63,14 @@ def test_default_stable_profiles_cover_all_modes(tmp_path: Path) -> None:
assert set(profiles) == {"stable"}
+def test_stable_profile_rejects_obsolete_deslop_setting(tmp_path: Path) -> None:
+ config = deepcopy(load_config(tmp_path / "state"))
+ config["orchestrator"]["profiles"]["stable"]["normal"]["deslop_enabled"] = False
+
+ with pytest.raises(ValueError, match=r"deslop_enabled.*--skip-deslop"):
+ load_orchestrator_profiles(config)
+
+
def test_profile_step_kind_defaults_to_review(tmp_path: Path) -> None:
config = deepcopy(load_config(tmp_path / "state"))
config["orchestrator"]["profiles"]["stable"]["normal"]["steps"] = [
diff --git a/plugins/review-suite/tests/test_review_orchestrator_runner.py b/plugins/review-suite/tests/test_review_orchestrator_runner.py
index cc8fdbd..9ea6008 100644
--- a/plugins/review-suite/tests/test_review_orchestrator_runner.py
+++ b/plugins/review-suite/tests/test_review_orchestrator_runner.py
@@ -4,7 +4,6 @@
import subprocess
import sys
from pathlib import Path
-from threading import Barrier
import pytest
@@ -20,11 +19,9 @@
STAGE_DECISION_PENDING,
STAGE_REVIEW_GREEN,
STAGE_RETRY_REQUESTED,
- STAGE_RUNNING,
abort_cycle,
create_cycle,
mark_arena_recovery_requested,
- mark_blocked,
mark_fix_detected,
mark_review_step_pending,
mark_review_step_running,
@@ -34,6 +31,16 @@
from review_suite_local import write_round
+@pytest.fixture
+def clean_worktree(monkeypatch: pytest.MonkeyPatch) -> None:
+ monkeypatch.setattr(
+ orchestrator_runner, "current_branch", lambda cwd: "feature/orchestrator"
+ )
+ monkeypatch.setattr(
+ orchestrator_runner, "dirty_worktree_scope", lambda *_: {"dirty_paths": []}
+ )
+
+
def _cycle(
tmp_path: Path,
*,
@@ -229,63 +236,83 @@ def fake_run(**kwargs: object) -> tuple[dict[str, object], int]:
return calls
-def test_runner_executes_deslop_and_first_review_step_together(
- monkeypatch, tmp_path: Path
+def test_runner_runs_bounded_closure_after_clean_correctness(
+ monkeypatch, clean_worktree: None, tmp_path: Path
) -> None:
calls: list[tuple[list[str], Path]] = []
- both_started = Barrier(2, timeout=10)
review_calls = _stub_review(monkeypatch)
- stub_review = orchestrator_runner.run_review_step
-
- def concurrent_review(**kwargs: object) -> dict[str, object]:
- both_started.wait()
- return stub_review(**kwargs)
def fake_run(*, command: list[str], cwd: Path) -> subprocess.CompletedProcess:
- both_started.wait()
calls.append((command, cwd))
- return subprocess.CompletedProcess(command, 0, stdout="", stderr="")
+ return subprocess.CompletedProcess(
+ command,
+ 0,
+ stdout="Conformance: CONFORMS\nReview decision: clean\n",
+ stderr="",
+ )
- monkeypatch.setattr(orchestrator_runner, "run_review_step", concurrent_review)
monkeypatch.setattr(orchestrator_runner, "run_deslop_subprocess", fake_run)
- result = orchestrator_runner.run_one_expensive_step(_cycle(tmp_path))
+ reviewed = orchestrator_runner.run_one_expensive_step(_cycle(tmp_path))
+
+ assert calls == []
+
+ green = record_clean_decision(
+ reviewed.state,
+ round_id="phase_review-round-1",
+ lane="review_t1",
+ reviewed_head="head-1",
+ )
+ green["review_brief"] = "- frozen"
+ green["identity"]["branch"] = None
+ monkeypatch.setattr(orchestrator_runner, "current_branch", lambda cwd: None)
+ orchestrator_runner.run_one_expensive_step(green)
- assert result.ran_step is True
- assert result.step == "review"
- assert result.state["stage"] == STAGE_DECISION_PENDING
- assert result.state["deslop"]["status"] == "done"
- assert result.state["deslop"]["returncode"] == 0
- assert len(calls) == 1
command, cwd = calls[0]
- assert Path(command[1]).name == "review_deslop.py"
- assert "--output-only" in command
- assert command[-2:] == ["--base", "main"]
- assert cwd == tmp_path / "repo"
- assert len(review_calls) == 1
- assert result.state["pending_action"] == {
- "kind": "decision",
- "round_id": "phase_review-round-1",
- "lane": "review_t1",
- "step_index": 0,
- "step": "precision",
- }
- assert result.state["rounds"][0]["round_id"] == "phase_review-round-1"
- assert result.state["rounds"][0]["lane"] == "review_t1"
- assert result.state["rounds"][0]["kind"] == "review"
- assert result.state["rounds"][0]["review_status"] == "completed"
- assert result.state["rounds"][0]["output_refs"] == [
- "rollout://thread/gpt-5.5-medium"
- ]
- assert len(calls) == 1
- assert len(review_calls) == 1
+ assert command[-4:-1] == ["--commit", "base-1", "head-1"]
+ assert (len(review_calls), command[-1]) == (1, "--review-brief=- frozen")
+
- third = orchestrator_runner.run_one_expensive_step(
- result.state, state_dir=tmp_path / "state"
+def test_runner_blocks_stale_exact_head_before_closure(
+ monkeypatch, tmp_path: Path, capsys: pytest.CaptureFixture[str]
+) -> None:
+ green = _cycle(tmp_path)
+ green.update(stage=STAGE_RETRY_REQUESTED, pending_action={"kind": "run-deslop"})
+ green["deslop"]["status"] = "failed"
+ green["rounds"] = [{"round_id": "round-1", "profile_step": {"index": 0}}]
+ green["review_progress"]["completed_steps"] = [{"round_id": "round-1"}]
+ runner = orchestrator_runner
+ monkeypatch.setattr(runner, "current_branch", lambda cwd: "feature/orchestrator")
+ monkeypatch.setattr(runner, "dirty_worktree_scope", lambda *_: {"dirty_paths": []})
+ monkeypatch.setattr(runner, "current_head", lambda cwd: "head-2")
+ monkeypatch.setattr(runner, "merge_base", lambda cwd, base, head: "different-base")
+ calls: list[object] = []
+ monkeypatch.setattr(
+ runner, "run_deslop_subprocess", lambda **kwargs: calls.append(kwargs)
)
- assert third.ran_step is False
- assert len(review_calls) == 1
+ result = runner.run_one_expensive_step(green)
+ assert result.ran_step is True
+ assert result.step == "deslop-rerun"
+ assert result.state["stage"] == STAGE_CREATED
+ assert result.state["pending_action"]["kind"] == "run-review-step"
+ for mirror in ("identity", "review_heads"):
+ assert result.state[mirror]["head"] == "head-2"
+ assert result.state[mirror]["merge_base"] == "different-base"
+ assert result.state["deslop"]["status"] == "tracked"
+ assert calls == []
+ monkeypatch.setattr(runner, "current_branch", lambda cwd: "other")
+ assert runner.run_one_expensive_step(green).state is green
+ monkeypatch.setattr(runner, "current_head", lambda cwd: "head-1")
+ assert runner.run_one_expensive_step(green).step == "blocked"
+ assert "expected feature/orchestrator" in capsys.readouterr().out
+ monkeypatch.setattr(runner, "current_branch", lambda cwd: "feature/orchestrator")
+ monkeypatch.setattr(
+ runner, "dirty_worktree_scope", lambda *_: {"dirty_paths": ["dirty.py"]}
+ )
+ assert runner.run_one_expensive_step(green).step == "blocked"
+ assert "clean worktree required" in capsys.readouterr().out
+ assert calls == []
def test_deslop_subprocess_emits_parent_progress_without_leaking_child_stderr(
@@ -606,7 +633,6 @@ def test_runner_arena_findings_fix_advances_with_findings_context(
fixed, state_dir=tmp_path / "state"
)
- assert result.ran_step is True
assert result.step == "review"
assert arena_calls == []
assert review_calls[0]["step_name"] == "broad-discovery"
@@ -1231,23 +1257,30 @@ def test_runner_rejects_mismatched_arena_lane_and_task_class(
orchestrator_runner.run_one_expensive_step(state, state_dir=tmp_path / "state")
-def test_runner_skips_fast_deslop(monkeypatch, tmp_path: Path) -> None:
+def test_runner_fast_mode_uses_same_post_clean_closure(
+ monkeypatch, clean_worktree: None, tmp_path: Path
+) -> None:
review_calls = _stub_review(monkeypatch)
+ deslop_calls: list[list[str]] = []
- def fail_run(*, command: list[str], cwd: Path) -> subprocess.CompletedProcess:
- raise AssertionError("fast mode must not run deslop")
+ def fake_run(*, command, cwd):
+ deslop_calls.append(command)
+ return subprocess.CompletedProcess(
+ command, 1, "Conformance: NOT_APPLICABLE\nReview decision: findings", ""
+ )
- monkeypatch.setattr(orchestrator_runner, "run_deslop_subprocess", fail_run)
+ monkeypatch.setattr(orchestrator_runner, "run_deslop_subprocess", fake_run)
result = orchestrator_runner.run_one_expensive_step(
- _cycle(tmp_path, mode="fast", deslop_enabled=False),
+ _cycle(tmp_path, mode="fast"),
state_dir=tmp_path / "state",
)
assert result.ran_step is True
assert result.step == "review"
assert len(review_calls) == 1
- assert result.state["deslop"]["status"] == "skipped-fast"
+ assert result.state["deslop"]["status"] == "tracked"
+ assert deslop_calls == []
green = record_clean_decision(
result.state,
@@ -1259,132 +1292,46 @@ def fail_run(*, command: list[str], cwd: Path) -> subprocess.CompletedProcess:
assert green["stage"] == STAGE_REVIEW_GREEN
assert green["pending_action"] is None
+ closure = orchestrator_runner.run_one_expensive_step(green)
+ assert closure.state["deslop"]["status"] == "failed"
-def test_runner_marks_failed_deslop_retryable_and_retries_from_retry_stage(
- monkeypatch, tmp_path: Path
-) -> None:
- calls = 0
- output: list[str] = []
- review_calls = _stub_review(monkeypatch, "discarded-round", "accepted-round")
- def fake_run(*, command: list[str], cwd: Path) -> subprocess.CompletedProcess:
- nonlocal calls
- calls += 1
- return subprocess.CompletedProcess(
- command,
- 9 if calls == 1 else 0,
- stdout="",
- stderr="deslop stderr details" if calls == 1 else "",
- )
-
- monkeypatch.setattr(
- orchestrator_runner, "write_text", lambda text: output.append(str(text))
- )
- monkeypatch.setattr(orchestrator_runner, "run_deslop_subprocess", fake_run)
-
- failed = orchestrator_runner.run_one_expensive_step(_cycle(tmp_path))
-
- assert failed.ran_step is True
- assert failed.state["stage"] == STAGE_DECISION_PENDING
- assert failed.state["rounds"][0]["round_id"] == "discarded-round"
- assert failed.state["deslop"]["status"] == "failed"
- assert failed.state["deslop"]["returncode"] == 9
- assert failed.state["recovery"]["status"] == STAGE_RETRY_REQUESTED
- assert failed.state["recovery"]["retry_count"] == 1
- assert "deslop stderr details" in output
- assert len(review_calls) == 1
-
- retried = orchestrator_runner.run_one_expensive_step(failed.state)
-
- assert retried.ran_step is True
- assert retried.state["stage"] == STAGE_DECISION_PENDING
- assert retried.state["deslop"]["status"] == "done"
- assert retried.state["rounds"][0]["round_id"] == "discarded-round"
- assert retried.state["recovery"]["status"] == "none"
- assert len(review_calls) == 1
- assert calls == 2
-
-
-def test_runner_recovers_tracked_deslop_before_collecting_running_review(
- monkeypatch, tmp_path: Path
+def test_runner_retry_completes_closure_with_conformance(
+ monkeypatch, clean_worktree: None, tmp_path: Path
) -> None:
- monkeypatch.setattr(
- orchestrator_runner,
- "run_deslop_subprocess",
- lambda **kwargs: subprocess.CompletedProcess([], 0, stdout="", stderr=""),
- )
- running = mark_review_step_running(
- _cycle(tmp_path),
- round_id="running-round",
- lane="review_t1",
- step_index=0,
- step_name="precision",
- reviewed_head="head-1",
- round_state_dir="state/rounds",
+ _stub_review(monkeypatch)
+ outputs = iter(
+ subprocess.CompletedProcess([], 0, body, "")
+ for body in (
+ "Conformance: NOT_APPLICABLE\nConformance: NOT_APPLICABLE\nReview decision: clean",
+ "Conformance: NOT_APPLICABLE\nReview decision: clean\nReview decision: findings",
+ "Actionable: Conformance: NOT_APPLICABLE\n\n if ready:\n run()\nActionable: Review decision: findings\nConformance: NOT_APPLICABLE\nReview decision: findings",
+ )
)
-
- recovered = orchestrator_runner.run_one_expensive_step(running)
-
- assert recovered.step == "deslop"
- assert recovered.state["stage"] == STAGE_RUNNING
- assert recovered.state["deslop"]["status"] == "done"
- assert recovered.state["pending_action"] == running["pending_action"]
-
-
-def test_runner_preserves_arena_recovery_while_retrying_deslop(
- monkeypatch, tmp_path: Path
-) -> None:
monkeypatch.setattr(
- orchestrator_runner,
- "run_deslop_subprocess",
- lambda **kwargs: subprocess.CompletedProcess([], 0, stdout="", stderr=""),
+ orchestrator_runner, "run_deslop_subprocess", lambda **kwargs: next(outputs)
)
- pending = mark_review_step_pending(
- _cycle(tmp_path),
- round_id="blocked-round",
+ reviewed = orchestrator_runner.run_one_expensive_step(_cycle(tmp_path))
+ green = record_clean_decision(
+ reviewed.state,
+ round_id="phase_review-round-1",
lane="review_t1",
- step_index=0,
- step_name="precision",
reviewed_head="head-1",
)
- blocked = mark_arena_recovery_requested(
- pending,
- reason="blocked",
- round_id="blocked-round",
- lane="review_t1",
- step_index=0,
- step_name="precision",
- round_state_dir="state/rounds",
- )
- blocked["deslop"]["status"] = "failed"
-
- recovered = orchestrator_runner.run_one_expensive_step(blocked)
-
- assert recovered.state["deslop"]["status"] == "done"
- assert recovered.state["stage"] == STAGE_RETRY_REQUESTED
- assert recovered.state["pending_action"]["kind"] == "arena-blocked"
- assert recovered.state["recovery"]["round_id"] == "blocked-round"
-
+ failed = orchestrator_runner.run_one_expensive_step(green)
+ assert failed.state["deslop"]["status"] == "failed"
+ failed_again = orchestrator_runner.run_one_expensive_step(failed.state)
+ assert failed_again.state["deslop"]["status"] == "failed"
+ retried = orchestrator_runner.run_one_expensive_step(failed_again.state)
-def test_runner_preserves_gate_recovery_while_retrying_deslop(
- monkeypatch, tmp_path: Path
-) -> None:
- monkeypatch.setattr(
- orchestrator_runner,
- "run_deslop_subprocess",
- lambda **kwargs: subprocess.CompletedProcess([], 0, stdout="", stderr=""),
- )
- blocked = mark_blocked(
- _cycle(tmp_path), reason="gate failed", round_id="gate-round"
+ assert retried.state["deslop"]["status"] == "done"
+ assert retried.state["deslop"]["conformance"] == "NOT_APPLICABLE"
+ assert retried.state["deslop"]["decision"] == "findings"
+ assert retried.state["deslop"]["findings"] == (
+ "Actionable: Conformance: NOT_APPLICABLE\n\n"
+ " if ready:\n run()\n"
+ "Actionable: Review decision: findings"
)
- blocked["deslop"]["status"] = "failed"
-
- recovered = orchestrator_runner.run_one_expensive_step(blocked)
-
- assert recovered.state["deslop"]["status"] == "done"
- assert recovered.state["stage"] == "blocked"
- assert recovered.state["recovery"]["reason"] == "gate failed"
- assert recovered.state["recovery"]["round_id"] == "gate-round"
def test_runner_does_not_retry_failed_deslop_for_aborted_cycle(
diff --git a/plugins/review-suite/tests/test_review_orchestrator_state.py b/plugins/review-suite/tests/test_review_orchestrator_state.py
index 77a274b..ab1729c 100644
--- a/plugins/review-suite/tests/test_review_orchestrator_state.py
+++ b/plugins/review-suite/tests/test_review_orchestrator_state.py
@@ -26,6 +26,7 @@
mark_crashed,
mark_decision_pending,
mark_deslop_closed,
+ mark_deslop_done,
mark_deslop_failed,
mark_fix_detected,
mark_followup_review_pending,
@@ -123,7 +124,7 @@ def test_create_cycle_is_compact_json_state_keyed_by_normalized_inputs(
assert state["validation"]["review_green"] == "unknown"
assert state["validation"]["full_suite"] == "unknown"
assert fast["identity"]["branch"] is None
- assert fast["deslop"] == {"tracked": False, "status": "skipped-fast"}
+ assert fast["deslop"] == {"tracked": True, "status": "tracked"}
assert skipped["deslop"] == {
"tracked": False,
"status": "skipped",
@@ -156,48 +157,45 @@ def test_create_cycle_preserves_exact_optional_review_brief(tmp_path: Path) -> N
assert briefless["review_brief"] is None
-def test_mark_deslop_closed_disables_tracked_sidecar_and_leaves_fast_untracked(
+def test_mark_deslop_closed_requires_completed_conforming_closure(
tmp_path: Path,
) -> None:
tracked = _cycle(tmp_path)
- closed = mark_deslop_closed(tracked)
+ with pytest.raises(ValueError, match="only close after"):
+ mark_deslop_closed(tracked)
+
+ done = mark_deslop_done(
+ tracked,
+ command="review-deslop",
+ conformance="CONFORMS",
+ reviewed_head="head-1",
+ )
+ closed = mark_deslop_closed(done)
closed_again = mark_deslop_closed(closed)
- assert closed["deslop"] == {"tracked": False, "status": "closed"}
+ assert closed["deslop"]["status"] == "closed"
assert closed_again == closed
assert tracked["deslop"] == {"tracked": True, "status": "tracked"}
- fast = create_cycle(
- cwd=tmp_path / "repo",
- base="main",
- branch="HEAD",
- head="head-1",
- merge_base="base-1",
- requested_mode="fast",
- effective_mode="fast",
- selection="stable",
+ drifted = mark_deslop_done(
+ tracked,
+ command="review-deslop",
+ conformance="MATERIALLY_DRIFTED",
+ reviewed_head="head-1",
)
-
- assert mark_deslop_closed(fast)["deslop"] == {
- "tracked": False,
- "status": "skipped-fast",
- }
+ with pytest.raises(ValueError, match="materially drifted"):
+ mark_deslop_closed(drifted)
-def test_mark_deslop_closed_resumes_after_failed_sidecar(tmp_path: Path) -> None:
+def test_failed_closure_cannot_be_skipped(tmp_path: Path) -> None:
failed = mark_deslop_failed(
_cycle(tmp_path), command="review-deslop", returncode=2, reason="deslop failed"
)
- closed = mark_deslop_closed(failed)
-
assert failed["stage"] == STAGE_RETRY_REQUESTED
assert failed["pending_action"] == {"kind": "run-deslop"}
- assert closed["deslop"]["tracked"] is False
- assert closed["deslop"]["status"] == "closed"
- assert closed["stage"] == STAGE_CREATED
- assert closed["pending_action"] == {"kind": "resume-after-deslop"}
- assert closed["recovery"] == {"status": "none", "retry_count": 1}
+ with pytest.raises(ValueError, match="only close after"):
+ mark_deslop_closed(failed)
def test_wait_states_are_idle_and_transitions_are_idempotent(tmp_path: Path) -> None:
@@ -1585,8 +1583,14 @@ def test_gate_findings_require_fix_followup_clean_and_same_gate_rerun(
with pytest.raises(ValueError, match="must be one of"):
record_validation_statuses(rerun_clean, full_suite="classified")
- handoff = mark_local_green_handoff(
+ closure = mark_deslop_done(
rerun_clean,
+ command="review-deslop",
+ conformance="CONFORMS",
+ reviewed_head="head-2",
+ )
+ handoff = mark_local_green_handoff(
+ mark_deslop_closed(closure),
focused="passed",
full_suite="passed",
ci="waived",
@@ -1604,6 +1608,9 @@ def test_gate_findings_require_fix_followup_clean_and_same_gate_rerun(
assert validation_blockers(reasonless) == ["ci:waived_without_note"]
rerun = mark_latest_profile_step_rerun_needed(handoff, head="head-3")
assert "note" not in rerun["validation"]
+ assert rerun["deslop"]["status"] == "tracked"
+ assert rerun["stage"] == STAGE_CREATED
+ assert rerun["deslop"]["conformance_only"] is True
def test_non_deep_gate_findings_rerun_same_gate_without_followup(
diff --git a/plugins/review-suite/tests/test_review_suite_core_codex_runtime.py b/plugins/review-suite/tests/test_review_suite_core_codex_runtime.py
index 31a88c8..cc32396 100644
--- a/plugins/review-suite/tests/test_review_suite_core_codex_runtime.py
+++ b/plugins/review-suite/tests/test_review_suite_core_codex_runtime.py
@@ -1,4 +1,5 @@
import json
+import os
import sys
from pathlib import Path
@@ -10,10 +11,9 @@
sys.path.insert(0, str(SCRIPT_DIR))
from review_suite_core.codex_runtime import (
- AUTO_UNSAFE_WINDOWS_WSL_FALLBACK_ENV,
- unsafe_windows_wsl_fallback_requested,
use_unsafe_windows_wsl_fallback,
validate_codex_runtime,
+ windows_wsl_codex_child_env,
)
from review_suite_core.lens_runtime import (
TECHNICAL_REVIEW_DEVELOPER_INSTRUCTIONS,
@@ -278,7 +278,7 @@ def test_codex_exec_command_repasses_provider_overrides_before_review_model(
assert 'mcp_servers.node_repl.command="node_repl"' not in command
-def test_codex_exec_command_isolates_unsafe_wsl_fallback(
+def test_codex_exec_command_keeps_wsl_fallback_read_only(
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
) -> None:
monkeypatch.setattr(
@@ -307,12 +307,12 @@ def test_codex_exec_command_isolates_unsafe_wsl_fallback(
)
assert command[0:3] == ["codex", "exec", "--ignore-user-config"]
- assert "--dangerously-bypass-approvals-and-sandbox" in command
+ assert "--dangerously-bypass-approvals-and-sandbox" not in command
assert 'approval_policy="never"' in command
- assert "-s" not in command
+ assert command[command.index("-s") + 1] == "read-only"
-def test_codex_exec_review_command_isolates_unsafe_wsl_fallback(
+def test_codex_exec_review_command_keeps_wsl_fallback_read_only(
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
) -> None:
monkeypatch.setattr(
@@ -341,9 +341,9 @@ def test_codex_exec_review_command_isolates_unsafe_wsl_fallback(
)
assert command[0:3] == ["codex", "exec", "--ignore-user-config"]
- assert "--dangerously-bypass-approvals-and-sandbox" in command
+ assert "--dangerously-bypass-approvals-and-sandbox" not in command
assert 'approval_policy="never"' in command
- assert "-s" not in command
+ assert command[command.index("-s") + 1] == "read-only"
assert command[-2:] == ["--base", "origin/main"]
@@ -414,6 +414,7 @@ def test_prepare_codex_review_launch_creates_prompted_exec_without_native_target
assert launch.final_message_path.parent == state_dir / "tmp"
assert str(launch.final_message_path) in launch.command
assert launch.cwd == tmp_path
+ assert launch.env is None
finally:
if launch.final_message_path is not None:
launch.final_message_path.unlink(missing_ok=True)
@@ -872,50 +873,44 @@ def test_emit_result_classifies_only_non_timeout_windows_sharing_violations(
assert ("Retry the review" in output) is retryable
-def test_unsafe_windows_wsl_fallback_requested_honors_env(
+def test_windows_wsl_fallback_requires_explicit_authorization(
monkeypatch: pytest.MonkeyPatch,
) -> None:
- monkeypatch.setattr(
- "review_suite_core.codex_runtime._env_flag_value", lambda name: ""
- )
- assert not unsafe_windows_wsl_fallback_requested(False)
-
- monkeypatch.setattr(
- "review_suite_core.codex_runtime._env_flag_value", lambda name: "1"
- )
- assert unsafe_windows_wsl_fallback_requested(False)
-
-
-def test_unsafe_windows_wsl_fallback_requested_honors_windows_user_env(
- monkeypatch: pytest.MonkeyPatch,
-) -> None:
- monkeypatch.delenv(AUTO_UNSAFE_WINDOWS_WSL_FALLBACK_ENV, raising=False)
- monkeypatch.setattr(
- "review_suite_core.codex_runtime._env_flag_value", lambda name: "on"
- )
+ monkeypatch.setattr(sys, "platform", "win32")
+ monkeypatch.setenv("REVIEW_SUITE_AUTO_WSL_FALLBACK", "true")
+ review_root = Path("//wsl.localhost/Ubuntu/home/alice/code/repo")
- assert unsafe_windows_wsl_fallback_requested(False)
+ assert not use_unsafe_windows_wsl_fallback(review_root, False)
+ assert use_unsafe_windows_wsl_fallback(review_root, True)
-def test_use_unsafe_windows_wsl_fallback_honors_env_only_for_unc_path(
+@pytest.mark.skipif(os.name != "nt", reason="Windows UNC normalization")
+def test_windows_wsl_codex_child_env_appends_exact_safe_directory(
monkeypatch: pytest.MonkeyPatch,
) -> None:
monkeypatch.setattr(sys, "platform", "win32")
- monkeypatch.setenv(AUTO_UNSAFE_WINDOWS_WSL_FALLBACK_ENV, "true")
+ monkeypatch.setenv("GIT_CONFIG_COUNT", "1")
+ monkeypatch.setenv("GIT_CONFIG_KEY_0", "core.autocrlf")
+ monkeypatch.setenv("GIT_CONFIG_VALUE_0", "false")
+ monkeypatch.delenv("GIT_CONFIG_KEY_1", raising=False)
+ review_root = Path("//wsl.localhost/Ubuntu/home/alice/code/repo/../repo")
- assert use_unsafe_windows_wsl_fallback(
- Path("//wsl.localhost/Ubuntu/home/alice/code/repo"), False
- )
- assert not use_unsafe_windows_wsl_fallback(Path("C:/Code/repo"), False)
+ env = windows_wsl_codex_child_env(review_root, True)
+
+ assert env is not None
+ assert env["GIT_CONFIG_COUNT"] == "2"
+ assert env["GIT_CONFIG_KEY_0"] == "core.autocrlf"
+ assert env["GIT_CONFIG_VALUE_0"] == "false"
+ assert env["GIT_CONFIG_KEY_1"] == "safe.directory"
+ assert env["GIT_CONFIG_VALUE_1"] == "//wsl.localhost/Ubuntu/home/alice/code/repo"
+ assert os.environ["GIT_CONFIG_COUNT"] == "1"
+ assert "GIT_CONFIG_KEY_1" not in os.environ
-def test_validate_codex_runtime_mentions_env_opt_in_for_unc_path(
+def test_validate_codex_runtime_requires_wsl_flag_for_unc_path(
monkeypatch: pytest.MonkeyPatch,
) -> None:
monkeypatch.setattr(sys, "platform", "win32")
- monkeypatch.setattr(
- "review_suite_core.codex_runtime._env_flag_value", lambda name: ""
- )
with pytest.raises(ValueError) as excinfo:
validate_codex_runtime(
@@ -923,12 +918,12 @@ def test_validate_codex_runtime_mentions_env_opt_in_for_unc_path(
codex_executable="codex",
review_root=Path("//wsl.localhost/Ubuntu/home/alice/code/repo"),
allow_unsafe_windows_wsl_fallback=False,
- unsafe_command_hint="codex exec --dangerously-bypass-approvals-and-sandbox",
)
message = str(excinfo.value)
assert "--wsl" in message
- assert f"{AUTO_UNSAFE_WINDOWS_WSL_FALLBACK_ENV}=1" in message
+ assert "REVIEW_SUITE_AUTO_WSL_FALLBACK" not in message
+ assert "bypass" not in message
def test_validate_codex_runtime_mentions_windows_unc_workaround_for_wsl_windows_shim(
@@ -944,9 +939,9 @@ def test_validate_codex_runtime_mentions_windows_unc_workaround_for_wsl_windows_
codex_executable="/mnt/c/Users/alice/AppData/Roaming/npm/codex",
review_root=Path("/home/alice/code/repo"),
allow_unsafe_windows_wsl_fallback=False,
- unsafe_command_hint="codex exec --dangerously-bypass-approvals-and-sandbox",
)
message = str(excinfo.value)
assert "//wsl.localhost/Ubuntu/home/alice/code/repo" in message
assert "--wsl" in message
+ assert "install and authenticate" not in message
diff --git a/plugins/review-suite/tests/test_review_suite_core_process_runtime.py b/plugins/review-suite/tests/test_review_suite_core_process_runtime.py
index 76eb5f3..85b012a 100644
--- a/plugins/review-suite/tests/test_review_suite_core_process_runtime.py
+++ b/plugins/review-suite/tests/test_review_suite_core_process_runtime.py
@@ -21,6 +21,7 @@ def test_launch_captured_child_process_starts_clock_before_stdin_write(
tmp_path: Path,
) -> None:
events: list[str] = []
+ captured: dict[str, object] = {}
class FakeStdin:
def write(self, value: str) -> None:
@@ -38,6 +39,7 @@ def fake_monotonic() -> float:
def fake_popen(*args, **kwargs) -> FakeProcess:
events.append("popen")
+ captured.update(kwargs)
return FakeProcess()
monkeypatch.setattr(
@@ -50,6 +52,7 @@ def fake_popen(*args, **kwargs) -> FakeProcess:
child = launch_captured_child_process(
command=[sys.executable, "-c", "pass"],
cwd=tmp_path,
+ env={"SCOPED": "1"},
stdin_text="prompt",
stdout_prefix="child-stdout-",
stderr_prefix="child-stderr-",
@@ -57,6 +60,7 @@ def fake_popen(*args, **kwargs) -> FakeProcess:
try:
assert child.started_monotonic == 123.0
assert events[:4] == ["clock", "popen", "write:prompt", "close"]
+ assert captured["env"] == {"SCOPED": "1"}
finally:
child.stdout_path.unlink(missing_ok=True)
child.stderr_path.unlink(missing_ok=True)