From 9ad793a6da0cd29d4d8c4336add8e45ce58ab24b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=88=98=E5=8D=87=E9=BE=99?= Date: Wed, 9 Sep 2026 01:55:01 +0800 Subject: [PATCH 01/18] fix(review): admit native host review provenance --- scripts/orchestration/review_runtime.py | 78 +++++++++- scripts/work-bundle/reviewer_workspace.py | 178 ++++++++++++++++++++-- tests/test_orchestration_reviews.py | 82 ++++++++++ tests/test_reviewer_workspace.py | 64 +++++++- 4 files changed, 380 insertions(+), 22 deletions(-) diff --git a/scripts/orchestration/review_runtime.py b/scripts/orchestration/review_runtime.py index 38a82cb..48cf1d9 100644 --- a/scripts/orchestration/review_runtime.py +++ b/scripts/orchestration/review_runtime.py @@ -3,9 +3,11 @@ import argparse import hashlib +import importlib.util import json import re import subprocess +import sys from dataclasses import dataclass from datetime import datetime, timezone from pathlib import Path @@ -512,6 +514,70 @@ def _known_task_execution_ids(root: Path, task_id: str) -> set[str]: return ids +def _native_reviewer_module(): + path = Path(__file__).resolve().parents[1] / "work-bundle/reviewer_workspace.py" + existing = sys.modules.get("reviewer_workspace") + if existing is not None: + if Path(existing.__file__).resolve() != path: + raise ReviewContractError("native reviewer module collision") + return existing + spec = importlib.util.spec_from_file_location("reviewer_workspace", path) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def _validate_native_run_proof(receipt, packet, result, path, immutable_file, canonical): + """Re-derive observed host identity/result from immutable actual run input/output.""" + runtime = _native_reviewer_module() + try: + stdout = immutable_file(path.with_suffix(".stdout.jsonl")) + stderr = immutable_file(path.with_suffix(".stderr.txt")) + request_bytes = immutable_file(path.with_suffix(".request.json")) + request = json.loads(request_bytes) + launch = json.loads(immutable_file(path.with_suffix(".launch.json"))) + argv = launch["argv"] + host_id, worker = runtime.parse_native_reviewer_transcript(stdout.decode(), stderr.decode()) + if (receipt.get("host_run_id") != host_id or receipt.get("isolation") != runtime.NATIVE_ISOLATION + or receipt.get("stdout_sha256") != hashlib.sha256(stdout).hexdigest() + or receipt.get("stderr_sha256") != hashlib.sha256(stderr).hexdigest() + or receipt.get("request_sha256") != hashlib.sha256(request_bytes).hexdigest() + or receipt.get("argv_sha256") != canonical(argv) + or receipt.get("executable_sha256") != launch.get("executable_sha256") + or not SHA256_RE.fullmatch(str(receipt.get("executable_sha256", ""))) + or not Path(argv[0]).is_absolute() + or argv != runtime._native_reviewer_argv(Path(argv[0]), Path(argv[9]), argv[11]) + or set(request) != {"instructions", "review_input", "evidence"} + or request["review_input"] != runtime._native_review_input(packet) or not isinstance(request["instructions"], str) + or not request["instructions"].strip()): + raise ValueError("native launch/input mismatch") + artifacts = packet["artifacts"] + evidence = request["evidence"] + if len(evidence) != len(artifacts): + raise ValueError("native input evidence mismatch") + for expected, actual in zip(artifacts, evidence): + if (set(actual) != {*expected, "content"} or any(actual[key] != value for key, value in expected.items()) + or hashlib.sha256(actual["content"].encode()).hexdigest() != expected["sha256"]): + raise ValueError("native input evidence mismatch") + key = "task_review_context" if result.get("review_target_kind", "stage") == "task" else "stage_review_context" + context = {**packet[key], "agent_id": host_id, "execution_id": host_id} + compact = key == "task_review_context" or (context.get("stage") == "integrated_implementation" and "task_review" in worker) + if compact: + observed = runtime._task_product_judgment_review( + worker, review_id=receipt["review_id"], context=context, packet=packet, + started_at=receipt["started_at"], completed_at=receipt["completed_at"], + previous_review=result.get("previous_review"), integrated_stage=key == "stage_review_context") + else: + observed = runtime._stage_product_judgment_review( + worker, review_id=receipt["review_id"], context=context, packet=packet, + started_at=receipt["started_at"], completed_at=receipt["completed_at"]) + if observed != result or receipt.get("review_result") != result or receipt.get(key) != context: + raise ValueError("native judgment/result mismatch") + return context + except (ValueError, TypeError, KeyError, IndexError, AttributeError, runtime.ReviewerWorkspaceError) as error: + raise ReviewContractError("native reviewer-run provenance does not bind this accepted review") from error + + def _validate_reviewer_run(root: Path, review: Mapping[str, Any]) -> None: reference = review.get("reviewer_run") if not isinstance(reference, dict) or set(reference) != {"run_id", "sha256"}: @@ -541,6 +607,10 @@ def canonical(value: Any) -> str: kind = str(review.get("review_target_kind") or "stage") context_key = "task_review_context" if kind == "task" else "stage_review_context" context = _mapping(receipt.get(context_key, {}), "reviewer-run provenance context") + native = receipt.get("schema") == "reviewer-native-receipt-v1" + packet_context = packet.get(context_key) + if native: + packet_context = _validate_native_run_proof(receipt, packet, result, path, immutable_file, canonical) mode = "direct_source" if review["evidence"]["mode"] == "direct" else review["evidence"]["mode"] review_context = { "review_mode": review.get("review_mode", "initial"), @@ -554,19 +624,19 @@ def canonical(value: Any) -> str: "repair_frontier": context.get("repair_frontier"), "review_reset": context.get("review_reset"), } - if (receipt.get("schema") != "reviewer-process-receipt-v1" or receipt.get("run_id") != run_id + if (receipt.get("schema") not in {"reviewer-process-receipt-v1", "reviewer-native-receipt-v1"} or receipt.get("run_id") != run_id or receipt.get("review_id") != review["review_id"] or receipt.get("status") != "passed" or receipt.get("exit_code") != 0 or receipt.get("review_result_sha256") != canonical(result) - or receipt.get("packet_sha256") != canonical(packet) or packet.get(context_key) != context + or receipt.get("packet_sha256") != canonical(packet) or packet_context != context or context.get("target_identity") != review["target_identity"] or (kind == "stage" and context.get("stage") != review["stage"]) or context.get("agent_id") != review["reviewer"]["agent_id"] or context.get("evidence_mode") != review["reviewer"]["context_origin"] or context.get("capability") != review["reviewer"]["capability"] or context.get("evidence_mode") != mode or review_context != receipt_context - or receipt.get("isolation") != {"mechanism": "sandbox-exec", "network": "denied", "write_scope": "scratch"} + or (not native and receipt.get("isolation") != {"mechanism": "sandbox-exec", "network": "denied", "write_scope": "scratch"}) or not context.get("execution_id") - or receipt.get("sandbox_profile_sha256") != hashlib.sha256(immutable_file(path.with_suffix(".profile.sb"))).hexdigest() + or (not native and receipt.get("sandbox_profile_sha256") != hashlib.sha256(immutable_file(path.with_suffix(".profile.sb"))).hexdigest()) or receipt.get("event_log_sha256") != hashlib.sha256(immutable_file(path.with_suffix(".events.jsonl"))).hexdigest()): raise ReviewContractError("reviewer-run provenance does not bind this accepted review") known = ( diff --git a/scripts/work-bundle/reviewer_workspace.py b/scripts/work-bundle/reviewer_workspace.py index fcaa2cd..802b8d7 100644 --- a/scripts/work-bundle/reviewer_workspace.py +++ b/scripts/work-bundle/reviewer_workspace.py @@ -21,6 +21,108 @@ NETWORK_STATES = frozenset({"denied"}) VALIDATOR_KINDS = frozenset({"json", "sha256", "command"}) TERMINAL_VERDICTS = frozenset({"accepted", "repair", "blocked"}) +NATIVE_DISABLED_FEATURES = ( + "shell_tool", "unified_exec", "code_mode", "code_mode_host", "apps", "hooks", + "browser_use", "browser_use_external", "browser_use_full_cdp_access", "computer_use", + "in_app_browser", "image_generation", "multi_agent", "view_image", "workspace_dependencies", + "tool_suggest", "skill_search", "sleep_tool", "goals", "memories", "remote_plugin", "recommended_plugins", +) +NATIVE_ISOLATION = { + "mechanism": "native-host-read-only", "network": "model-transport", + "write_scope": "read-only", "context": "fresh-bounded-input", + "tools": "disabled-and-no-observed-activity", "os_process_isolation": False, +} + + +def parse_native_reviewer_transcript(raw: str, stderr: str = "") -> tuple[str, dict[str, object]]: + """Accept one completed fresh host turn, never a supplied verdict or tool run.""" + thread_id = None + phase = "new" + messages = [] + try: + # The host can report failed tool dispatch only on stderr, with no JSONL + # tool item. Unknown diagnostics are inadmissible, not evidence of silence. + if any(line.strip() not in {"", "Reading additional input from stdin..."} for line in stderr.splitlines()): + raise ValueError("unexpected host diagnostic") + for line in raw.splitlines(): + event = json.loads(line) + kind = event["type"] + if kind == "thread.started" and phase == "new": + thread_id = str(uuid.UUID(event["thread_id"])) + phase = "ready" + elif kind == "turn.started" and phase == "ready": + phase = "running" + elif kind == "item.completed" and phase in {"ready", "running"}: + item = event["item"] + if phase == "ready" and item["type"] == "error" and ( + str(item.get("message", "")).startswith("Under-development features enabled: skip_host_skill_discovery.") + or str(item.get("message", "")).startswith("Code Mode is unavailable because code-mode host is disabled.") + ): + continue + if phase != "running" or item["type"] not in {"agent_message", "reasoning"}: + raise ValueError("unexpected host activity") + if item["type"] == "agent_message": + messages.append(item["text"]) + elif kind == "turn.completed" and phase == "running" and messages: + phase = "complete" + else: + raise ValueError("unexpected host activity") + if phase != "complete" or not thread_id: + raise ValueError("incomplete host run") + result = json.loads(messages[-1]) + if not isinstance(result, dict): + raise ValueError("judgment must be an object") + return thread_id, result + except (ValueError, TypeError, KeyError, AttributeError) as error: + raise ReviewerWorkspaceError("WB_REVIEW_NATIVE_TRANSCRIPT_INVALID") from error + + +def _native_reviewer_argv(executable: Path, workspace: Path, model: str) -> list[str]: + return [str(executable), "exec", "--ignore-user-config", "--sandbox", "read-only", "--ephemeral", + "--json", "--skip-git-repo-check", "-C", str(workspace), "-m", model, + "-c", 'model_reasoning_effort="medium"', "-c", "project_doc_max_bytes=0", + "-c", 'web_search="disabled"', "--enable", "skip_host_skill_discovery", + *[part for feature in NATIVE_DISABLED_FEATURES for part in ("--disable", feature)], "-"] + + +def _run_native_process(workspace: Path, argv: list[str], request: str) -> subprocess.CompletedProcess[str]: + # Desktop transport/session variables would reconnect the reviewer to author + # capabilities even when native user config is suppressed. Never inherit them. + environment = {"PATH": "/usr/bin:/bin:/usr/sbin:/sbin", "HOME": str(Path.home()), + "TMPDIR": str(workspace / "scratch")} + if os.environ.get("CODEX_HOME"): + environment["CODEX_HOME"] = os.environ["CODEX_HOME"] + return subprocess.run(argv, cwd=workspace, env=environment, input=request, text=True, + capture_output=True, check=False, timeout=1800) + + +def _native_review_input(packet: dict[str, object]) -> dict[str, object]: + key = "task_review_context" if "task_review_context" in packet else "stage_review_context" + context = packet[key] + if key == "task_review_context" or context.get("stage") == "integrated_implementation": + return {"target_identity": context["target_identity"], "artifacts": packet["artifacts"]} + return {"stage": context["stage"], "target_identity": context["target_identity"], "artifacts": packet["artifacts"]} + + +def run_native_reviewer(workspace: Path, executable: Path, *, model: str, review_instructions: str) -> dict[str, object]: + """Run a fresh native host judgment over explicit evidence; no plugin required. + + Native read-only policy is not the legacy OS process sandbox. The host may + use its authentication/model transport; no author thread transport propagates, + and any observed tool activity makes the result inadmissible. + """ + executable = executable.expanduser().resolve() + workspace = workspace.expanduser().resolve() + if not executable.is_file() or not os.access(executable, os.X_OK) or not model or not review_instructions.strip(): + raise ReviewerWorkspaceError("WB_REVIEW_COMMAND_INVALID") + packet, _ = _load_workspace(workspace) + if not ("stage_review_context" in packet or "task_review_context" in packet): + raise ReviewerWorkspaceError("WB_REVIEW_NATIVE_CONTEXT_REQUIRED") + evidence = [{**item, "content": _evidence_path(workspace, item["locator"]).read_text(encoding="utf-8")} + for item in packet["artifacts"]] + request = {"instructions": review_instructions, "review_input": _native_review_input(packet), "evidence": evidence} + argv = _native_reviewer_argv(executable, workspace, model) + return _run_reviewer(workspace, argv, native_request=request) def _review_runtime(): @@ -796,8 +898,37 @@ def _task_product_judgment_review( return result +def _stage_product_judgment_review(judgment, *, review_id, context, packet, started_at, completed_at): + """Compose native stage authority without asking the reviewer to invent it.""" + if not isinstance(judgment, dict) or set(judgment) != {"stage_review"}: + raise ReviewerWorkspaceError("WB_REVIEW_STAGE_OUTPUT_INVALID") + product = judgment["stage_review"] + if (not isinstance(product, dict) or set(product) != {"target_identity", "verdict", "findings"} + or product["target_identity"] != context["target_identity"] + or product["verdict"] not in TERMINAL_VERDICTS or not isinstance(product["findings"], list)): + raise ReviewerWorkspaceError("WB_REVIEW_STAGE_OUTPUT_INVALID") + return { + "review_id": review_id, "stage": context["stage"], "target_identity": context["target_identity"], + "review_mode": context.get("review_mode", "initial"), "review_target_kind": "stage", + "repair_frontier": context.get("repair_frontier"), "review_reset": context.get("review_reset"), + "reviewer": {"agent_id": context["agent_id"], "capability": context["capability"], + "authorship": "none", "repair_participation": "none", "decision_participation": "none", + "deliberation_participation": "none", "context_origin": context["evidence_mode"]}, + "evidence": {"mode": context["evidence_mode"], "capabilities": ["product review judgment"], + "unavailable_evidence": [], "commands": [], + "artifacts": [{"path": item["locator"], "sha256": item["sha256"]} for item in packet["artifacts"]]}, + "verdict": product["verdict"], "findings": product["findings"], + "started_at": started_at, "completed_at": completed_at, + "staleness": {"is_stale": False, "reason": None, "supersedes": None}, + } + + def run_sandboxed_reviewer(workspace: Path, argv: list[str]) -> dict[str, object]: """Launch the entire reviewer under the frozen deny-default profile.""" + return _run_reviewer(workspace, argv) + + +def _run_reviewer(workspace: Path, argv: list[str], *, native_request: dict[str, object] | None = None) -> dict[str, object]: workspace = workspace.expanduser().resolve() runtime_root, review_id, state = _runtime_identity(workspace) packet, _ = _load_workspace(workspace) @@ -813,10 +944,22 @@ def run_sandboxed_reviewer(workspace: Path, argv: list[str]) -> dict[str, object except (ValueError, OSError, SystemExit) as error: raise ReviewerWorkspaceError("WB_REVIEW_STAGE_EVIDENCE_INCOMPLETE") from error started_at = datetime.now(timezone.utc).isoformat().replace("+00:00", "Z") - completed = _run_sandboxed_process(workspace, argv) + native = native_request is not None + request_bytes = json.dumps(native_request, sort_keys=True, ensure_ascii=False).encode() if native else b"" + executable_digest = _sha256_bytes(Path(argv[0]).read_bytes()) if native else None + completed = (_run_native_process(workspace, argv, request_bytes.decode()) if native + else _run_sandboxed_process(workspace, argv)) + host_run_id = None + native_worker_output = None + if native: + if completed.returncode != 0: + raise ReviewerWorkspaceError("WB_REVIEW_NATIVE_PROCESS_FAILED", {"exit_code": completed.returncode}) + if _sha256_bytes(Path(argv[0]).read_bytes()) != executable_digest: + raise ReviewerWorkspaceError("WB_REVIEW_NATIVE_EXECUTABLE_MUTATED") + host_run_id, native_worker_output = parse_native_reviewer_transcript(completed.stdout, completed.stderr) if _artifact_digest(workspace, packet) != state.get("evidence_digest"): raise ReviewerWorkspaceError("WB_REVIEW_EVIDENCE_MUTATED") - denied = _sandbox_denied(completed) + denied = not native and _sandbox_denied(completed) if denied: _append_denial_event( workspace, @@ -827,7 +970,7 @@ def run_sandboxed_reviewer(workspace: Path, argv: list[str]) -> dict[str, object sandbox_state = state.get("sandbox") if isinstance(state.get("sandbox"), dict) else {} run_id = f"reviewer-run-{uuid.uuid4()}" receipt = { - "schema": "reviewer-process-receipt-v1", + "schema": "reviewer-native-receipt-v1" if native else "reviewer-process-receipt-v1", "run_id": run_id, "review_id": review_id, "status": "denied" if denied else ("passed" if completed.returncode == 0 else "failed"), @@ -842,6 +985,9 @@ def run_sandboxed_reviewer(workspace: Path, argv: list[str]) -> dict[str, object "started_at": started_at, "completed_at": datetime.now(timezone.utc).isoformat().replace("+00:00", "Z"), } + if native: + receipt.update({"host_run_id": host_run_id, "request_sha256": _sha256_bytes(request_bytes), + "executable_sha256": executable_digest, "isolation": dict(NATIVE_ISOLATION)}) context_key = "stage_review_context" if "stage_review_context" in packet else ( "task_review_context" if "task_review_context" in packet else None ) @@ -852,7 +998,9 @@ def run_sandboxed_reviewer(workspace: Path, argv: list[str]) -> dict[str, object else _validate_task_context(packet[context_key]) ) try: - worker_output = json.loads(completed.stdout) + worker_output = native_worker_output if native else json.loads(completed.stdout) + if native: + context = {**context, "agent_id": host_run_id, "execution_id": host_run_id} compact_integrated = ( context_key == "stage_review_context" and context.get("stage") == "integrated_implementation" @@ -866,7 +1014,10 @@ def run_sandboxed_reviewer(workspace: Path, argv: list[str]) -> dict[str, object integrated_stage=compact_integrated, ) if context_key == "task_review_context" or compact_integrated - else worker_output + else (_stage_product_judgment_review( + worker_output, review_id=review_id, context=context, packet=packet, + started_at=started_at, completed_at=receipt["completed_at"]) + if native else worker_output) ) validated = ( _review_runtime().validate_stage_review(review) @@ -904,17 +1055,24 @@ def run_sandboxed_reviewer(workspace: Path, argv: list[str]) -> dict[str, object raise ReviewerWorkspaceError("WB_REVIEW_STAGE_EVIDENCE_INCOMPLETE") from error receipt[context_key] = context receipt["review_result_sha256"] = _canonical_digest(review) - if context_key == "task_review_context" or compact_integrated: + if native or context_key == "task_review_context" or compact_integrated: receipt["review_result"] = review - receipt["isolation"] = {"mechanism": "sandbox-exec", "network": "denied", "write_scope": "scratch"} + if not native: + receipt["isolation"] = {"mechanism": "sandbox-exec", "network": "denied", "write_scope": "scratch"} receipt_path = (runtime_root / "receipts" / "reviewer-process" / f"{run_id}.json").resolve(strict=False) if not _inside(runtime_root, receipt_path): raise ReviewerWorkspaceError("WB_REVIEW_RUNTIME_PATH_ESCAPE") receipt_path.parent.mkdir(parents=True, exist_ok=True) # Retain immutable run-scoped evidence after workspace cleanup or later runs. - for suffix, content in (("packet.json", json.dumps(packet, sort_keys=True).encode()), - ("profile.sb", (workspace / "sandbox.sb").read_bytes()), - ("events.jsonl", Path(str(sealed["event_log_path"])).read_bytes())): + retained_items = [("packet.json", json.dumps(packet, sort_keys=True).encode()), + ("events.jsonl", Path(str(sealed["event_log_path"])).read_bytes())] + if native: + retained_items.extend([("request.json", request_bytes), ("stdout.jsonl", completed.stdout.encode()), + ("stderr.txt", completed.stderr.encode()), + ("launch.json", json.dumps({"argv": argv, "executable_sha256": executable_digest}, sort_keys=True).encode())]) + else: + retained_items.append(("profile.sb", (workspace / "sandbox.sb").read_bytes())) + for suffix, content in retained_items: retained = receipt_path.with_suffix(f".{suffix}") with retained.open("xb") as stream: stream.write(content) diff --git a/tests/test_orchestration_reviews.py b/tests/test_orchestration_reviews.py index 68113e3..39da0ad 100644 --- a/tests/test_orchestration_reviews.py +++ b/tests/test_orchestration_reviews.py @@ -846,6 +846,88 @@ def test_old_packet_bytes_cannot_be_relabelled_as_current_target(tmp_path): reviewer_workspace.create_reviewer_workspace(runtime, "relabelled", packet) +def _native_spec_receipt(tmp_path, monkeypatch): + import reviewer_workspace + spec, _, reviews = _reviewed_plan_fixture(tmp_path) + record = json.loads((reviews / "specification.json").read_text()) + record.pop("reviewer_run") + workspace = review_runtime.reviewer_runtime_root(tmp_path) / "reviews" / record["review_id"] + host_id = "01a0821d-f359-7d60-a9bd-90dd0e006166" + events = [ + {"type": "thread.started", "thread_id": host_id}, {"type": "turn.started"}, + {"type": "item.completed", "item": {"id": "1", "type": "agent_message", "text": json.dumps({ + "stage_review": {key: record[key] for key in ("target_identity", "verdict", "findings")}})}}, + {"type": "turn.completed", "usage": {}}, + ] + monkeypatch.setattr(reviewer_workspace, "_run_native_process", lambda *_: + subprocess.CompletedProcess([], 0, "\n".join(json.dumps(event) for event in events), "")) + receipt = reviewer_workspace.run_native_reviewer(workspace, Path(sys.executable), model="test-model", + review_instructions="Assess supplied specification and return its stage judgment.") + result = {**receipt["review_result"], "reviewer_run": receipt["reviewer_run"]} + return spec, receipt, result + + +def test_plugin_absent_native_review_publishes_and_consumes_actual_host_identity(tmp_path, monkeypatch): + spec, receipt, result = _native_spec_receipt(tmp_path, monkeypatch) + assert result["reviewer"]["agent_id"] == receipt["host_run_id"] + assert receipt["isolation"]["mechanism"] == "native-host-read-only" + assert receipt["isolation"]["os_process_isolation"] is False + request = json.loads(Path(receipt["receipt_path"]).with_suffix(".request.json").read_text()) + assert set(request["review_input"]) == {"stage", "target_identity", "artifacts"} + assert "execution_id" not in json.dumps(request["review_input"]) + current = review_runtime.artifact_review_identity(spec) + reference = publish_review(tmp_path, result, current_target_identity=current) + loaded, accepted = review_runtime.load_stored_review(tmp_path, reference, current_target_identity=current) + assert loaded == result + assert accepted.verdict == "accepted" + spec.write_text(spec.read_text().replace("Original body", "Changed obligation")) + with pytest.raises(ReviewContractError, match="current"): + publish_review(tmp_path, result, current_target_identity=review_runtime.artifact_review_identity(spec)) + + +@pytest.mark.parametrize("change", ["isolation", "host_id", "result", "raw_result", "request", "stderr", "argv"]) +def test_native_receipt_rejects_resealed_false_provenance(tmp_path, monkeypatch, change): + import hashlib + _, receipt, result = _native_spec_receipt(tmp_path, monkeypatch) + path = Path(receipt["receipt_path"]) + saved = json.loads(path.read_text()) + if change == "isolation": + saved["isolation"] = {"mechanism": "sandbox-exec", "network": "denied", "write_scope": "scratch"} + elif change == "host_id": + saved["host_run_id"] = "author-alias" + elif change == "result": + result["verdict"] = "blocked" + saved["review_result"] = {key: value for key, value in result.items() if key != "reviewer_run"} + saved["review_result_sha256"] = hashlib.sha256(json.dumps(saved["review_result"], sort_keys=True, separators=(",", ":"), ensure_ascii=False).encode()).hexdigest() + else: + suffix = {"raw_result": ".stdout.jsonl", "request": ".request.json", "stderr": ".stderr.txt", "argv": ".launch.json"}[change] + proof = path.with_suffix(suffix) + proof.chmod(0o600) + if change == "raw_result": + proof.write_text(json.dumps({"verdict": "accepted"})) + saved["stdout_sha256"] = hashlib.sha256(proof.read_bytes()).hexdigest() + elif change == "request": + value = json.loads(proof.read_text()) + value["evidence"][0]["content"] += "changed" + proof.write_text(json.dumps(value)) + saved["request_sha256"] = hashlib.sha256(proof.read_bytes()).hexdigest() + elif change == "stderr": + proof.write_text("ERROR codex_core::tools::router: error=code-mode host is disabled\n") + saved["stderr_sha256"] = hashlib.sha256(proof.read_bytes()).hexdigest() + else: + value = json.loads(proof.read_text()) + value["argv"].remove("--ignore-user-config") + proof.write_text(json.dumps(value)) + saved["argv_sha256"] = hashlib.sha256(json.dumps(value["argv"], sort_keys=True, separators=(",", ":"), ensure_ascii=False).encode()).hexdigest() + proof.chmod(0o400) + path.chmod(0o600) + path.write_text(json.dumps(saved)) + path.chmod(0o400) + result["reviewer_run"]["sha256"] = hashlib.sha256(path.read_bytes()).hexdigest() + with pytest.raises(ReviewContractError, match="provenance"): + review_runtime._validate_reviewer_run(tmp_path, result) + + @pytest.mark.parametrize("change", ["review_id", "target", "capability", "context_origin", "failed"]) def test_launcher_does_not_publish_acceptance_for_unbound_worker_output(tmp_path, monkeypatch, change): import reviewer_workspace diff --git a/tests/test_reviewer_workspace.py b/tests/test_reviewer_workspace.py index 4ef03be..12f1998 100644 --- a/tests/test_reviewer_workspace.py +++ b/tests/test_reviewer_workspace.py @@ -28,6 +28,46 @@ import reviewer_workspace # noqa: E402 +def native_events(result, *, thread_id="01a0821d-f359-7d60-a9bd-90dd0e006166"): + return "\n".join(json.dumps(event) for event in [ + {"type": "thread.started", "thread_id": thread_id}, + {"type": "turn.started"}, + {"type": "item.completed", "item": {"id": "item-1", "type": "agent_message", "text": json.dumps(result)}}, + {"type": "turn.completed", "usage": {"input_tokens": 10, "output_tokens": 10}}, + ]) + + +def test_native_transcript_requires_one_actual_fresh_completed_judgment(): + run, result = reviewer_workspace.parse_native_reviewer_transcript(native_events({"verdict": "repair"})) + assert run == "01a0821d-f359-7d60-a9bd-90dd0e006166" + assert result == {"verdict": "repair"} + for forged in [json.dumps({"verdict": "accept"}), native_events({}) + "\n" + native_events({}), + native_events({}).rsplit("\n", 1)[0]]: + with pytest.raises(ReviewerWorkspaceError, match="NATIVE_TRANSCRIPT"): + reviewer_workspace.parse_native_reviewer_transcript(forged) + + +@pytest.mark.parametrize("kind", ["command_execution", "mcp_tool_call", "collab_tool_call", "error", "file_change"]) +def test_native_transcript_rejects_all_observed_tool_or_failure_activity(kind): + events = native_events({}).splitlines() + events.insert(2, json.dumps({"type": "item.completed", "item": {"id": "tool", "type": kind}})) + with pytest.raises(ReviewerWorkspaceError, match="NATIVE_TRANSCRIPT"): + reviewer_workspace.parse_native_reviewer_transcript("\n".join(events)) + + +def test_native_process_does_not_inherit_author_transport_or_config(tmp_path, monkeypatch): + monkeypatch.setenv("CODEX_APP_TOOLS_PIPE_PATH", "caller-transport") + monkeypatch.setenv("CODEX_THREAD_ID", "author-thread") + monkeypatch.setenv("CODEX_SESSION_ID", "author-session") + monkeypatch.setenv("BASH_ENV", "/host/instructions") + with patch.object(reviewer_workspace.subprocess, "run", return_value=subprocess.CompletedProcess([], 0, "", "")) as launch: + reviewer_workspace._run_native_process(tmp_path, ["/bin/codex"], "bounded packet") + kwargs = launch.call_args.kwargs + assert set(kwargs["env"]) <= {"PATH", "HOME", "TMPDIR", "CODEX_HOME"} + assert kwargs["input"] == "bounded packet" + assert kwargs["timeout"] > 0 + + def sha256(path: Path) -> str: return hashlib.sha256(path.read_bytes()).hexdigest() @@ -116,8 +156,9 @@ def test_workspace_contains_copied_direct_evidence_and_declares_network_denied( assert "control_root" not in json.dumps(state) +@pytest.mark.parametrize("transport", ["sandbox", "native"]) def test_task_review_worker_output_receives_native_bound_receipt( - review_roots: tuple[Path, Path, Path] + review_roots: tuple[Path, Path, Path], transport: str ) -> None: source, control, runtime = review_roots subprocess.run(["git", "init", "-q", str(source)], check=True) @@ -153,17 +194,24 @@ def test_task_review_worker_output_receives_native_bound_receipt( "reviewed_head": head, "verdict": "accept", "findings": [], }} - with patch.object( - reviewer_workspace, - "_run_sandboxed_process", - return_value=subprocess.CompletedProcess(["reviewer"], 0, json.dumps(judgment), ""), - ): - receipt = reviewer_workspace.run_sandboxed_reviewer(Path(str(created["workspace_path"])), ["reviewer"]) + if transport == "native": + with patch.object(reviewer_workspace, "_run_native_process", + return_value=subprocess.CompletedProcess([], 0, native_events(judgment), "")): + receipt = reviewer_workspace.run_native_reviewer(Path(str(created["workspace_path"])), Path(sys.executable), + model="test-model", review_instructions="Assess the accepted product requirements against the source.") + request = json.loads(Path(receipt["receipt_path"]).with_suffix(".request.json").read_text()) + assert set(request["review_input"]) == {"target_identity", "artifacts"} + assert "task_review_context" not in json.dumps(request) + else: + with patch.object(reviewer_workspace, "_run_sandboxed_process", + return_value=subprocess.CompletedProcess(["reviewer"], 0, json.dumps(judgment), "")): + receipt = reviewer_workspace.run_sandboxed_reviewer(Path(str(created["workspace_path"])), ["reviewer"]) assert receipt["status"] == "passed" assert receipt["task_review_context"]["target_identity"] == identity assert set(receipt["reviewer_run"]) == {"run_id", "sha256"} - assert receipt["review_result"]["reviewer"]["agent_id"] == "reviewer-task" + assert receipt["review_result"]["reviewer"]["agent_id"] == ( + receipt["host_run_id"] if transport == "native" else "reviewer-task") assert receipt["review_result"]["verdict"] == "accept" From 00ca518fa3a1e63d53316592ba807136e8995a98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=88=98=E5=8D=87=E9=BE=99?= Date: Wed, 9 Sep 2026 02:01:48 +0800 Subject: [PATCH 02/18] fix(review): retain rejected native run diagnostics --- scripts/work-bundle/reviewer_workspace.py | 50 ++++++++++++++++++++--- tests/test_orchestration_reviews.py | 24 ++++++++++- tests/test_reviewer_workspace.py | 17 ++++++++ 3 files changed, 85 insertions(+), 6 deletions(-) diff --git a/scripts/work-bundle/reviewer_workspace.py b/scripts/work-bundle/reviewer_workspace.py index 802b8d7..db1b84c 100644 --- a/scripts/work-bundle/reviewer_workspace.py +++ b/scripts/work-bundle/reviewer_workspace.py @@ -29,7 +29,8 @@ ) NATIVE_ISOLATION = { "mechanism": "native-host-read-only", "network": "model-transport", - "write_scope": "read-only", "context": "fresh-bounded-input", + "write_scope": "read-only", "context": "fresh-native-host-context-with-explicit-evidence", + "host_skill_catalog": "may-be-present", "tools": "disabled-and-no-observed-activity", "os_process_isolation": False, } @@ -39,6 +40,7 @@ def parse_native_reviewer_transcript(raw: str, stderr: str = "") -> tuple[str, d thread_id = None phase = "new" messages = [] + model_activity = False try: # The host can report failed tool dispatch only on stderr, with no JSONL # tool item. Unknown diagnostics are inadmissible, not evidence of silence. @@ -59,8 +61,14 @@ def parse_native_reviewer_transcript(raw: str, stderr: str = "") -> tuple[str, d or str(item.get("message", "")).startswith("Code Mode is unavailable because code-mode host is disabled.") ): continue + if (phase == "running" and not model_activity and item["type"] == "error" + and item.get("message") == "Skill descriptions were shortened to fit the skills context budget. Codex can still see every skill, but some descriptions are shorter. Disable unused skills or plugins to leave more room for the rest."): + # Observed host initialization notice, not an attempted tool + # or model failure. Native catalog metadata may be present. + continue if phase != "running" or item["type"] not in {"agent_message", "reasoning"}: raise ValueError("unexpected host activity") + model_activity = True if item["type"] == "agent_message": messages.append(item["text"]) elif kind == "turn.completed" and phase == "running" and messages: @@ -96,6 +104,32 @@ def _run_native_process(workspace: Path, argv: list[str], request: str) -> subpr capture_output=True, check=False, timeout=1800) +def _retain_native_diagnostics(runtime_root, run_id, review_id, argv, request_bytes, executable_digest, completed): + """Keep actual transport evidence before admission; this is never a receipt.""" + directory = runtime_root / "diagnostics/reviewer-native" / run_id + if not _inside(runtime_root, directory): + raise ReviewerWorkspaceError("WB_REVIEW_RUNTIME_PATH_ESCAPE") + directory.mkdir(parents=True, exist_ok=False) + items = { + "request.json": request_bytes, + "stdout.jsonl": completed.stdout.encode(), + "stderr.txt": completed.stderr.encode(), + "launch.json": json.dumps({"argv": argv, "executable_sha256": executable_digest}, sort_keys=True).encode(), + } + items["capture.json"] = json.dumps({ + "schema": "reviewer-native-diagnostic-v1", "status": "unadmitted", + "run_id": run_id, "review_id": review_id, "exit_code": completed.returncode, + "captured_at": datetime.now(timezone.utc).isoformat().replace("+00:00", "Z"), + "artifacts": {name: _sha256_bytes(content) for name, content in items.items()}, + }, sort_keys=True).encode() + for name, content in items.items(): + target = directory / name + with target.open("xb") as stream: + stream.write(content) + target.chmod(0o400) + return str(directory) + + def _native_review_input(packet: dict[str, object]) -> dict[str, object]: key = "task_review_context" if "task_review_context" in packet else "stage_review_context" context = packet[key] @@ -944,6 +978,7 @@ def _run_reviewer(workspace: Path, argv: list[str], *, native_request: dict[str, except (ValueError, OSError, SystemExit) as error: raise ReviewerWorkspaceError("WB_REVIEW_STAGE_EVIDENCE_INCOMPLETE") from error started_at = datetime.now(timezone.utc).isoformat().replace("+00:00", "Z") + run_id = f"reviewer-run-{uuid.uuid4()}" native = native_request is not None request_bytes = json.dumps(native_request, sort_keys=True, ensure_ascii=False).encode() if native else b"" executable_digest = _sha256_bytes(Path(argv[0]).read_bytes()) if native else None @@ -952,11 +987,17 @@ def _run_reviewer(workspace: Path, argv: list[str], *, native_request: dict[str, host_run_id = None native_worker_output = None if native: + diagnostic_path = _retain_native_diagnostics( + runtime_root, run_id, review_id, argv, request_bytes, executable_digest, completed) if completed.returncode != 0: - raise ReviewerWorkspaceError("WB_REVIEW_NATIVE_PROCESS_FAILED", {"exit_code": completed.returncode}) + raise ReviewerWorkspaceError("WB_REVIEW_NATIVE_PROCESS_FAILED", { + "exit_code": completed.returncode, "diagnostic_path": diagnostic_path}) if _sha256_bytes(Path(argv[0]).read_bytes()) != executable_digest: - raise ReviewerWorkspaceError("WB_REVIEW_NATIVE_EXECUTABLE_MUTATED") - host_run_id, native_worker_output = parse_native_reviewer_transcript(completed.stdout, completed.stderr) + raise ReviewerWorkspaceError("WB_REVIEW_NATIVE_EXECUTABLE_MUTATED", {"diagnostic_path": diagnostic_path}) + try: + host_run_id, native_worker_output = parse_native_reviewer_transcript(completed.stdout, completed.stderr) + except ReviewerWorkspaceError as error: + raise ReviewerWorkspaceError(error.code, {**error.result, "diagnostic_path": diagnostic_path}) from error if _artifact_digest(workspace, packet) != state.get("evidence_digest"): raise ReviewerWorkspaceError("WB_REVIEW_EVIDENCE_MUTATED") denied = not native and _sandbox_denied(completed) @@ -968,7 +1009,6 @@ def _run_reviewer(workspace: Path, argv: list[str], *, native_request: dict[str, ) sealed = _seal_event_log(runtime_root, review_id) sandbox_state = state.get("sandbox") if isinstance(state.get("sandbox"), dict) else {} - run_id = f"reviewer-run-{uuid.uuid4()}" receipt = { "schema": "reviewer-native-receipt-v1" if native else "reviewer-process-receipt-v1", "run_id": run_id, diff --git a/tests/test_orchestration_reviews.py b/tests/test_orchestration_reviews.py index 39da0ad..9fe73a0 100644 --- a/tests/test_orchestration_reviews.py +++ b/tests/test_orchestration_reviews.py @@ -846,7 +846,7 @@ def test_old_packet_bytes_cannot_be_relabelled_as_current_target(tmp_path): reviewer_workspace.create_reviewer_workspace(runtime, "relabelled", packet) -def _native_spec_receipt(tmp_path, monkeypatch): +def _native_spec_receipt(tmp_path, monkeypatch, *, observed_events=None): import reviewer_workspace spec, _, reviews = _reviewed_plan_fixture(tmp_path) record = json.loads((reviews / "specification.json").read_text()) @@ -859,6 +859,8 @@ def _native_spec_receipt(tmp_path, monkeypatch): "stage_review": {key: record[key] for key in ("target_identity", "verdict", "findings")}})}}, {"type": "turn.completed", "usage": {}}, ] + if observed_events is not None: + events = observed_events monkeypatch.setattr(reviewer_workspace, "_run_native_process", lambda *_: subprocess.CompletedProcess([], 0, "\n".join(json.dumps(event) for event in events), "")) receipt = reviewer_workspace.run_native_reviewer(workspace, Path(sys.executable), model="test-model", @@ -867,6 +869,26 @@ def _native_spec_receipt(tmp_path, monkeypatch): return spec, receipt, result +def test_failed_native_admission_retains_actual_unadmitted_diagnostics(tmp_path, monkeypatch): + import reviewer_workspace + events = [{"type": "thread.started", "thread_id": "01a0821d-f359-7d60-a9bd-90dd0e006166"}, + {"type": "turn.started"}, + {"type": "item.completed", "item": {"id": "tool", "type": "command_execution"}}] + with pytest.raises(reviewer_workspace.ReviewerWorkspaceError, match="NATIVE_TRANSCRIPT") as failed: + _native_spec_receipt(tmp_path, monkeypatch, observed_events=events) + diagnostic = Path(failed.value.result["diagnostic_path"]) + assert diagnostic.is_relative_to(review_runtime.reviewer_runtime_root(tmp_path) / "diagnostics") + assert [json.loads(line) for line in (diagnostic / "stdout.jsonl").read_text().splitlines()] == events + assert (diagnostic / "request.json").is_file() + assert (diagnostic / "stderr.txt").is_file() + assert (diagnostic / "launch.json").is_file() + metadata = json.loads((diagnostic / "capture.json").read_text()) + assert metadata["status"] == "unadmitted" + assert "review_result" not in metadata and "reviewer_run" not in metadata + assert all(not item.stat().st_mode & 0o222 for item in diagnostic.iterdir()) + assert not (review_runtime.reviewer_runtime_root(tmp_path) / "receipts/reviewer-process" / (metadata["run_id"] + ".json")).exists() + + def test_plugin_absent_native_review_publishes_and_consumes_actual_host_identity(tmp_path, monkeypatch): spec, receipt, result = _native_spec_receipt(tmp_path, monkeypatch) assert result["reviewer"]["agent_id"] == receipt["host_run_id"] diff --git a/tests/test_reviewer_workspace.py b/tests/test_reviewer_workspace.py index 12f1998..6e587b5 100644 --- a/tests/test_reviewer_workspace.py +++ b/tests/test_reviewer_workspace.py @@ -47,6 +47,23 @@ def test_native_transcript_requires_one_actual_fresh_completed_judgment(): reviewer_workspace.parse_native_reviewer_transcript(forged) +def test_native_observed_catalog_notice_is_initialization_only(): + notice = {"type": "item.completed", "item": {"id": "warning", "type": "error", "message": + "Skill descriptions were shortened to fit the skills context budget. Codex can still see every skill, but some descriptions are shorter. Disable unused skills or plugins to leave more room for the rest."}} + events = native_events({"verdict": "repair"}).splitlines() + events.insert(2, json.dumps(notice)) + assert reviewer_workspace.parse_native_reviewer_transcript("\n".join(events))[1] == {"verdict": "repair"} + events.pop(2) + events.insert(3, json.dumps(notice)) + with pytest.raises(ReviewerWorkspaceError, match="NATIVE_TRANSCRIPT"): + reviewer_workspace.parse_native_reviewer_transcript("\n".join(events)) + events.pop(3) + notice["item"]["message"] += " A tool also failed." + events.insert(2, json.dumps(notice)) + with pytest.raises(ReviewerWorkspaceError, match="NATIVE_TRANSCRIPT"): + reviewer_workspace.parse_native_reviewer_transcript("\n".join(events)) + + @pytest.mark.parametrize("kind", ["command_execution", "mcp_tool_call", "collab_tool_call", "error", "file_change"]) def test_native_transcript_rejects_all_observed_tool_or_failure_activity(kind): events = native_events({}).splitlines() From 1806b087807c09b8278f6fa002b940691c76e41a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=88=98=E5=8D=87=E9=BE=99?= Date: Wed, 9 Sep 2026 02:06:13 +0800 Subject: [PATCH 03/18] fix(review): preserve frozen evidence line endings --- scripts/work-bundle/reviewer_workspace.py | 10 +++++++-- tests/test_orchestration_reviews.py | 25 ++++++++++++++++++++++- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/scripts/work-bundle/reviewer_workspace.py b/scripts/work-bundle/reviewer_workspace.py index db1b84c..0354cd1 100644 --- a/scripts/work-bundle/reviewer_workspace.py +++ b/scripts/work-bundle/reviewer_workspace.py @@ -152,8 +152,14 @@ def run_native_reviewer(workspace: Path, executable: Path, *, model: str, review packet, _ = _load_workspace(workspace) if not ("stage_review_context" in packet or "task_review_context" in packet): raise ReviewerWorkspaceError("WB_REVIEW_NATIVE_CONTEXT_REQUIRED") - evidence = [{**item, "content": _evidence_path(workspace, item["locator"]).read_text(encoding="utf-8")} - for item in packet["artifacts"]] + evidence = [] + for item in packet["artifacts"]: + # Text-mode reads normalize CRLF. The model input must preserve the exact + # frozen bytes whose digest will be revalidated during publication. + content = _evidence_path(workspace, item["locator"]).read_bytes().decode("utf-8") + if _sha256_bytes(content.encode("utf-8")) != item["sha256"]: + raise ReviewerWorkspaceError("WB_REVIEW_EVIDENCE_MUTATED") + evidence.append({**item, "content": content}) request = {"instructions": review_instructions, "review_input": _native_review_input(packet), "evidence": evidence} argv = _native_reviewer_argv(executable, workspace, model) return _run_reviewer(workspace, argv, native_request=request) diff --git a/tests/test_orchestration_reviews.py b/tests/test_orchestration_reviews.py index 9fe73a0..fb93d76 100644 --- a/tests/test_orchestration_reviews.py +++ b/tests/test_orchestration_reviews.py @@ -846,12 +846,22 @@ def test_old_packet_bytes_cannot_be_relabelled_as_current_target(tmp_path): reviewer_workspace.create_reviewer_workspace(runtime, "relabelled", packet) -def _native_spec_receipt(tmp_path, monkeypatch, *, observed_events=None): +def _native_spec_receipt(tmp_path, monkeypatch, *, observed_events=None, crlf=False): import reviewer_workspace spec, _, reviews = _reviewed_plan_fixture(tmp_path) record = json.loads((reviews / "specification.json").read_text()) record.pop("reviewer_run") workspace = review_runtime.reviewer_runtime_root(tmp_path) / "reviews" / record["review_id"] + if crlf: + spec.write_bytes(spec.read_bytes().replace(b"\n", b"\r\n")) + previous_packet = json.loads((workspace / "packet.json").read_text()) + packet = reviewer_workspace.build_direct_evidence_packet( + source_root=tmp_path, control_root=tmp_path, protected_roots=[tmp_path / ".work-bundle/protected-test"], + artifacts=[item["locator"] for item in previous_packet["artifacts"]], search_roots=[], validators=[], + sentinels=[], network_state="denied", stage_review_context=previous_packet["stage_review_context"]) + record["review_id"] += "-crlf" + created = reviewer_workspace.create_reviewer_workspace(review_runtime.reviewer_runtime_root(tmp_path), record["review_id"], packet) + workspace = Path(created["workspace_path"]) host_id = "01a0821d-f359-7d60-a9bd-90dd0e006166" events = [ {"type": "thread.started", "thread_id": host_id}, {"type": "turn.started"}, @@ -869,6 +879,19 @@ def _native_spec_receipt(tmp_path, monkeypatch, *, observed_events=None): return spec, receipt, result +def test_native_crlf_evidence_preserves_exact_bytes_through_publication(tmp_path, monkeypatch): + spec, receipt, result = _native_spec_receipt(tmp_path, monkeypatch, crlf=True) + assert b"\r\n" in spec.read_bytes() + request = json.loads(Path(receipt["receipt_path"]).with_suffix(".request.json").read_text()) + locator = "control:" + spec.relative_to(tmp_path).as_posix() + supplied = next(item for item in request["evidence"] if item["locator"] == locator) + assert supplied["content"].encode("utf-8") == spec.read_bytes() + current = review_runtime.artifact_review_identity(spec) + reference = publish_review(tmp_path, result, current_target_identity=current) + loaded, accepted = review_runtime.load_stored_review(tmp_path, reference, current_target_identity=current) + assert loaded == result and accepted.verdict == "accepted" + + def test_failed_native_admission_retains_actual_unadmitted_diagnostics(tmp_path, monkeypatch): import reviewer_workspace events = [{"type": "thread.started", "thread_id": "01a0821d-f359-7d60-a9bd-90dd0e006166"}, From b4de393fe7cd3b5d68a2fa99ace00b7355c7726f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=88=98=E5=8D=87=E9=BE=99?= Date: Wed, 9 Sep 2026 09:39:37 +0800 Subject: [PATCH 04/18] feat(review): join native publication to task acceptance --- scripts/orchestration/execution_context.py | 86 +++- scripts/work-bundle/reviewer_workspace.py | 12 +- tests/test_native_review_integration.py | 533 +++++++++++++++++++++ 3 files changed, 629 insertions(+), 2 deletions(-) create mode 100644 tests/test_native_review_integration.py diff --git a/scripts/orchestration/execution_context.py b/scripts/orchestration/execution_context.py index f49c524..7f283fd 100644 --- a/scripts/orchestration/execution_context.py +++ b/scripts/orchestration/execution_context.py @@ -1896,10 +1896,94 @@ def materialize_accepted_task_review( *, accepted_at: str | None = None, validation_evidence_ids: Sequence[str] | None = None, + executor_handoff: Mapping[str, Any] | None = None, + validated_executor_result: Mapping[str, Any] | None = None, ) -> dict[str, Any]: - """Compose prior executor authority with one standalone current review.""" + """Compose executor authority with one published current task review.""" root = control_root.expanduser().resolve() + initial_acceptance = ( + executor_handoff is not None or validated_executor_result is not None + ) + if initial_acceptance: + expected_initial = { + "causal_class": "initial_acceptance", + "affected_task": str(task.get("task_id") or ""), + "authorized_lifecycle_action": "materialize_accepted_result", + } + if ( + not isinstance(executor_handoff, Mapping) + or not isinstance(validated_executor_result, Mapping) + or not isinstance(causal_classification, Mapping) + or dict(causal_classification) != expected_initial + or validation_evidence_ids is not None + ): + raise SystemExit( + "initial accepted task review requires exact executor result authority" + ) + binding = load_task_execution_binding( + root, str(task.get("plan_id") or ""), str(task.get("task_id") or "") + ) + try: + from review_runtime import ( + ReviewContractError, + load_stored_review, + stored_review_target_identity, + ) + + target_identity = stored_review_target_identity(root, review_reference) + review, validated_review = load_stored_review( + root, + review_reference, + current_target_identity=target_identity, + ) + except (ReviewContractError, KeyError, TypeError, ValueError) as error: + raise SystemExit(f"Accepted initial task review is invalid: {error}") from error + task_id = str(task.get("task_id") or "") + if ( + task.get("review_required") is not True + or validated_review.verdict != "accepted" + or validated_review.review_mode != "initial" + or validated_review.repair_frontier is not None + or validated_review.review_reset is not None + or validated_review.target_identity.get("artifact_id") != task_id + ): + raise SystemExit("accepted initial task review must bind the exact current task") + owner = validated_executor_result.get("task_ownership") + if not isinstance(owner, Mapping): + raise SystemExit("accepted initial task review requires validated executor ownership") + if validated_review.reviewer.get("agent_id") == owner.get("agent_id"): + raise SystemExit("accepted initial task review must be independent from the executor owner") + execution_path = Path(str(binding.get("execution_path") or "")).expanduser().resolve() + try: + evidence = capture_repository_evidence(execution_path) + except RuntimeError as error: + raise SystemExit("accepted initial task review Git identity is unavailable") from error + identity = validated_review.target_identity + if ( + evidence.get("status") != "clean" + or evidence.get("entries") + or evidence.get("head") != identity.get("revision") + or evidence.get("tree") != identity.get("source_tree") + or review.get("reviewed_head") != identity.get("revision") + ): + raise SystemExit( + "accepted initial task review does not match the clean exact source identity" + ) + handoff_with_review = dict(executor_handoff) + handoff_with_review["acceptance_review"] = review + accepted = build_accepted_task_result( + task, + binding, + handoff_with_review, + validated_executor_result, + accepted_at=accepted_at, + ) + updated = dict(binding) + updated["accepted_result"] = accepted + _persist_binding(updated, root) + return accepted + expected_classification = { "causal_class", "affected_task", "authorized_lifecycle_action", } diff --git a/scripts/work-bundle/reviewer_workspace.py b/scripts/work-bundle/reviewer_workspace.py index 0354cd1..095f0a0 100644 --- a/scripts/work-bundle/reviewer_workspace.py +++ b/scripts/work-bundle/reviewer_workspace.py @@ -147,7 +147,17 @@ def run_native_reviewer(workspace: Path, executable: Path, *, model: str, review """ executable = executable.expanduser().resolve() workspace = workspace.expanduser().resolve() - if not executable.is_file() or not os.access(executable, os.X_OK) or not model or not review_instructions.strip(): + if not executable.is_file() or not os.access(executable, os.X_OK): + raise ReviewerWorkspaceError( + "WB_REVIEW_NATIVE_CAPABILITY_UNAVAILABLE", + {"capability": "native_reviewer_executable"}, + ) + if not model: + raise ReviewerWorkspaceError( + "WB_REVIEW_NATIVE_CAPABILITY_UNAVAILABLE", + {"capability": "native_reviewer_model"}, + ) + if not review_instructions.strip(): raise ReviewerWorkspaceError("WB_REVIEW_COMMAND_INVALID") packet, _ = _load_workspace(workspace) if not ("stage_review_context" in packet or "task_review_context" in packet): diff --git a/tests/test_native_review_integration.py b/tests/test_native_review_integration.py new file mode 100644 index 0000000..fee2547 --- /dev/null +++ b/tests/test_native_review_integration.py @@ -0,0 +1,533 @@ +from __future__ import annotations + +from copy import deepcopy +import hashlib +import json +import os +from pathlib import Path +import shutil +import subprocess +import sys + +import pytest + + +REPO_ROOT = Path(__file__).resolve().parents[1] +ORCHESTRATION = REPO_ROOT / "scripts" / "orchestration" +WORK_BUNDLE = REPO_ROOT / "scripts" / "work-bundle" +for module_root in (WORK_BUNDLE, ORCHESTRATION): + if str(module_root) not in sys.path: + sys.path.insert(0, str(module_root)) + +import execution_context # noqa: E402 +import review_runtime # noqa: E402 +import reviewer_workspace # noqa: E402 + + +def _git(root: Path, *arguments: str) -> str: + completed = subprocess.run( + ["git", *arguments], cwd=root, check=True, capture_output=True, text=True + ) + return completed.stdout.strip() + + +def _native_events(result: dict[str, object]) -> str: + events = [ + { + "type": "thread.started", + "thread_id": "01a0821d-f359-7d60-a9bd-90dd0e006166", + }, + {"type": "turn.started"}, + { + "type": "item.completed", + "item": { + "id": "judgment", + "type": "agent_message", + "text": json.dumps(result), + }, + }, + {"type": "turn.completed", "usage": {}}, + ] + return "\n".join(json.dumps(event) for event in events) + + +def test_plugin_absent_native_review_publishes_once_then_materializes_initial_acceptance( + tmp_path: Path, monkeypatch, +) -> None: + source = tmp_path / "source" + control = tmp_path / "control" + source.mkdir() + control.mkdir() + (source / "product.py").write_text("VALUE = 1\n", encoding="utf-8") + _git(source, "init", "-q") + _git(source, "add", "product.py") + _git( + source, + "-c", + "user.name=Test", + "-c", + "user.email=test@example.invalid", + "commit", + "-qm", + "fixture", + ) + head = _git(source, "rev-parse", "HEAD") + tree = _git(source, "rev-parse", "HEAD^{tree}") + + task = { + "plan_id": "plan-native", + "task_id": "task-native", + "source_ids": ["REQ-NATIVE"], + "truth_basis": {"decision_authority": []}, + "files": {"read": ["product.py"], "write": ["product.py"], "forbidden": []}, + "validation": [ + { + "id": "VAL-NATIVE", + "kind": "process", + "command": "python -m pytest tests/product.py -q", + "invariant_ids": ["INV-NATIVE"], + } + ], + "review_required": True, + "workspace": {"root": str(control)}, + } + binding = { + "plan_id": "plan-native", + "task_id": "task-native", + "workspace_id": "workspace-native", + "execution_id": "executor-run", + "repository_id": "repo-native", + "execution_path": str(source), + "control_root": str(control), + "git_identity": {"branch_ref": "refs/heads/main"}, + "baseline": {"head": head, "tree": tree}, + "ownership": { + "binding_id": "binding:plan-native:task-native", + "state": "active", + "current_owner": "task-native", + "history": [{"event": "created"}], + }, + } + original_handoff = { + "type": "executor-result", + "related": {"plan": "plan-native", "task": "task-native"}, + "result": {"state": "completed", "summary": "Implemented native fixture."}, + "changes": {"files": [{"path": "product.py", "change": "updated"}]}, + "task_fit_check": {"task": "task-native", "result": "clean"}, + "knowledge_disposition": { + "action": "none", + "reason": "No durable authority changed.", + "affected_authority": [], + }, + "acceptance_review": {"required": True, "verdict": "pending"}, + "delegation_evidence": { + "delegated": True, + "owner_kind": "subagent", + "agent_id": "executor-agent", + "run_id": "executor-run", + "mechanism": "host-native", + }, + "validation": { + "commands": [ + { + "command": "python -m pytest tests/product.py -q", + "result": "passed", + } + ] + }, + } + validated = { + "result_state": "completed", + "knowledge_disposition": original_handoff["knowledge_disposition"], + "task_ownership": original_handoff["delegation_evidence"], + "observed_validation": [ + { + "id": "VAL-NATIVE", + "observation_id": "observation-native", + "result": "passed", + } + ], + } + handoff_before_review = deepcopy(original_handoff) + target_identity = { + "artifact_id": "task-native", + "revision": head, + "sha256": hashlib.sha256(json.dumps(task, sort_keys=True).encode()).hexdigest(), + "source_tree": tree, + } + task_review_context = { + "target_identity": target_identity, + "agent_id": "unbound-native-reviewer", + "capability": "judgment", + "execution_id": "unbound-native-reviewer", + "evidence_mode": "reproducible_snapshot", + "review_mode": "initial", + "review_target_kind": "task", + "repair_frontier": None, + "review_reset": None, + } + protected = control / ".protected" + protected.mkdir() + packet = reviewer_workspace.build_direct_evidence_packet( + source_root=source, + control_root=control, + protected_roots=[protected], + artifacts=["source:product.py"], + search_roots=[], + validators=[], + sentinels=[], + network_state="denied", + task_review_context=task_review_context, + ) + created = reviewer_workspace.create_reviewer_workspace( + review_runtime.reviewer_runtime_root(control), "review-native-initial", packet + ) + judgment = { + "task_review": {"reviewed_head": head, "verdict": "accept", "findings": []} + } + native_calls = 0 + + def run_native(*_args): + nonlocal native_calls + native_calls += 1 + return subprocess.CompletedProcess([], 0, _native_events(judgment), "") + + monkeypatch.setattr(reviewer_workspace, "_run_native_process", run_native) + monkeypatch.setattr( + execution_context, + "load_task_execution_binding", + lambda *_args: binding, + ) + persisted: dict[str, object] = {} + + def persist(value, _root): + persisted.update(value) + binding.clear() + binding.update(value) + + monkeypatch.setattr( + execution_context, + "_persist_binding", + persist, + ) + + receipt = reviewer_workspace.run_native_reviewer( + Path(str(created["workspace_path"])), + Path(sys.executable), + model="test-model", + review_instructions="Review the supplied task source and return a task judgment.", + ) + review = {**receipt["review_result"], "reviewer_run": receipt["reviewer_run"]} + reference = review_runtime.publish_review( + control, review, current_target_identity=target_identity + ) + assert review_runtime.publish_review( + control, review, current_target_identity=target_identity + ) == reference + + monkeypatch.setattr( + execution_context, + "_claim_bound_validation_observations", + lambda *_args, **_kwargs: pytest.fail("current validation must not be replayed"), + ) + accepted = execution_context.materialize_accepted_task_review( + control, + task, + reference, + { + "causal_class": "initial_acceptance", + "affected_task": "task-native", + "authorized_lifecycle_action": "materialize_accepted_result", + }, + executor_handoff=original_handoff, + validated_executor_result=validated, + ) + _, consumed = execution_context.load_current_accepted_task_result(control, task) + + assert native_calls == 1 + assert original_handoff == handoff_before_review + assert accepted == consumed == persisted["accepted_result"] + assert accepted["baseline_identity"] == {"head": head, "tree": tree} + assert accepted["review_id"] == review["review_id"] + assert accepted["validation_evidence_ids"] == ["observation-native"] + assert accepted["owner_identity"]["agent_id"] == "executor-agent" + assert review["reviewer"]["agent_id"] != accepted["owner_identity"]["agent_id"] + request = json.loads(Path(receipt["receipt_path"]).with_suffix(".request.json").read_text()) + assert "execution-flow" not in json.dumps(request).lower() + + task["validation"][0]["command"] = "python -m pytest tests/changed.py -q" + with pytest.raises(SystemExit, match="validation authority changed"): + execution_context.load_current_accepted_task_result(control, task) + + +def test_missing_native_host_capability_fails_before_review_dispatch( + tmp_path: Path, monkeypatch, +) -> None: + dispatched = False + + def unexpected_dispatch(*_args): + nonlocal dispatched + dispatched = True + raise AssertionError("review dispatch must not occur") + + monkeypatch.setattr(reviewer_workspace, "_run_native_process", unexpected_dispatch) + missing = tmp_path / "missing-native-host" + with pytest.raises( + reviewer_workspace.ReviewerWorkspaceError, + match="WB_REVIEW_NATIVE_CAPABILITY_UNAVAILABLE", + ) as failure: + reviewer_workspace.run_native_reviewer( + tmp_path, + missing, + model="test-model", + review_instructions="Review the supplied task.", + ) + + assert failure.value.result == {"capability": "native_reviewer_executable"} + assert dispatched is False + + +@pytest.mark.skipif( + os.environ.get("WB_NATIVE_REVIEW_INTEGRATION") != "1", + reason="set WB_NATIVE_REVIEW_INTEGRATION=1 for the genuine native host observation", +) +def test_live_plugin_absent_native_execution_review_publication_and_acceptance( + tmp_path: Path, monkeypatch, +) -> None: + executable_text = os.environ.get("WB_NATIVE_REVIEW_EXECUTABLE") or shutil.which("codex") + if not executable_text: + pytest.fail("native_executor_executable capability is unavailable") + executable = Path(executable_text).expanduser().resolve() + model = os.environ.get("WB_NATIVE_REVIEW_MODEL", "gpt-6-astra") + source = tmp_path / "source" + control = tmp_path / "control" + scratch = tmp_path / "scratch" + source.mkdir() + control.mkdir() + scratch.mkdir() + (source / "README.md").write_text("native integration fixture\n", encoding="utf-8") + _git(source, "init", "-q") + _git(source, "add", "README.md") + _git( + source, + "-c", + "user.name=Test", + "-c", + "user.email=test@example.invalid", + "commit", + "-qm", + "baseline", + ) + baseline_head = _git(source, "rev-parse", "HEAD") + baseline_tree = _git(source, "rev-parse", "HEAD^{tree}") + + executor_argv = [ + str(executable), + "exec", + "--ignore-user-config", + "--sandbox", + "workspace-write", + "--ephemeral", + "--json", + "--skip-git-repo-check", + "-C", + str(source), + "-m", + model, + "-c", + 'model_reasoning_effort="medium"', + "-c", + "project_doc_max_bytes=0", + "-c", + 'web_search="disabled"', + "--enable", + "skip_host_skill_discovery", + "--disable", + "remote_plugin", + "--disable", + "recommended_plugins", + "--disable", + "apps", + "--disable", + "multi_agent", + "-", + ] + environment = { + "PATH": os.environ.get("PATH", "/usr/bin:/bin:/usr/sbin:/sbin"), + "HOME": str(Path.home()), + "TMPDIR": str(scratch), + } + if os.environ.get("CODEX_HOME"): + environment["CODEX_HOME"] = os.environ["CODEX_HOME"] + executor = subprocess.run( + executor_argv, + cwd=source, + env=environment, + input=( + "Create product.py in this workspace with exactly this UTF-8 content: " + "VALUE = 1 followed by one newline. Do not use network access. " + "Finish only after verifying the file exists." + ), + text=True, + capture_output=True, + check=False, + timeout=1800, + ) + assert executor.returncode == 0, executor.stderr + assert (source / "product.py").read_bytes() == b"VALUE = 1\n" + executor_events = [json.loads(line) for line in executor.stdout.splitlines()] + executor_ids = [ + event["thread_id"] + for event in executor_events + if event.get("type") == "thread.started" + ] + assert len(executor_ids) == 1 + assert sum(event.get("type") == "turn.completed" for event in executor_events) == 1 + _git(source, "add", "product.py") + _git( + source, + "-c", + "user.name=Test", + "-c", + "user.email=test@example.invalid", + "commit", + "-qm", + "native executor result", + ) + head = _git(source, "rev-parse", "HEAD") + tree = _git(source, "rev-parse", "HEAD^{tree}") + + task = { + "plan_id": "plan-native-live", + "task_id": "task-native-live", + "source_ids": ["REQ-NATIVE"], + "truth_basis": {"decision_authority": []}, + "files": {"read": ["product.py"], "write": ["product.py"], "forbidden": []}, + "validation": [], + "review_required": True, + "workspace": {"root": str(control)}, + } + binding = { + "plan_id": "plan-native-live", + "task_id": "task-native-live", + "workspace_id": "workspace-native-live", + "execution_id": executor_ids[0], + "repository_id": "repo-native-live", + "execution_path": str(source), + "control_root": str(control), + "git_identity": {"branch_ref": "refs/heads/main"}, + "baseline": {"head": baseline_head, "tree": baseline_tree}, + "ownership": { + "binding_id": "binding:plan-native-live:task-native-live", + "state": "active", + "current_owner": "task-native-live", + "history": [{"event": "created"}], + }, + } + handoff = { + "type": "executor-result", + "related": {"plan": "plan-native-live", "task": "task-native-live"}, + "result": {"state": "completed", "summary": "Created product.py."}, + "changes": {"files": [{"path": "product.py", "change": "created"}]}, + "task_fit_check": {"task": "task-native-live", "result": "clean"}, + "knowledge_disposition": { + "action": "none", + "reason": "No durable authority changed.", + "affected_authority": [], + }, + "acceptance_review": {"required": True, "verdict": "pending"}, + "delegation_evidence": { + "delegated": True, + "owner_kind": "subagent", + "agent_id": executor_ids[0], + "run_id": executor_ids[0], + "mechanism": "host-native", + }, + "validation": {"commands": []}, + } + validated = { + "result_state": "completed", + "knowledge_disposition": handoff["knowledge_disposition"], + "task_ownership": handoff["delegation_evidence"], + "observed_validation": [], + } + target_identity = { + "artifact_id": "task-native-live", + "revision": head, + "sha256": hashlib.sha256(json.dumps(task, sort_keys=True).encode()).hexdigest(), + "source_tree": tree, + } + context = { + "target_identity": target_identity, + "agent_id": "unbound-native-reviewer", + "capability": "judgment", + "execution_id": "unbound-native-reviewer", + "evidence_mode": "reproducible_snapshot", + "review_mode": "initial", + "review_target_kind": "task", + "repair_frontier": None, + "review_reset": None, + } + protected = control / ".protected" + protected.mkdir() + packet = reviewer_workspace.build_direct_evidence_packet( + source_root=source, + control_root=control, + protected_roots=[protected], + artifacts=["source:product.py"], + search_roots=[], + validators=[], + sentinels=[], + network_state="denied", + task_review_context=context, + ) + created = reviewer_workspace.create_reviewer_workspace( + review_runtime.reviewer_runtime_root(control), "review-native-live", packet + ) + receipt = reviewer_workspace.run_native_reviewer( + Path(str(created["workspace_path"])), + executable, + model=model, + review_instructions=( + "Independently review the supplied task source. Return only a final JSON object " + f'with exactly this shape: {{"task_review":{{"reviewed_head":"{head}",' + '"verdict":"accept","findings":[]}}}. Accept only if the evidence satisfies the task.' + ), + ) + review = {**receipt["review_result"], "reviewer_run": receipt["reviewer_run"]} + assert review["reviewer"]["agent_id"] != executor_ids[0] + reference = review_runtime.publish_review( + control, review, current_target_identity=target_identity + ) + assert review_runtime.publish_review( + control, review, current_target_identity=target_identity + ) == reference + + monkeypatch.setattr( + execution_context, "load_task_execution_binding", lambda *_args: binding + ) + + def persist(value, _root): + binding.clear() + binding.update(value) + + monkeypatch.setattr(execution_context, "_persist_binding", persist) + accepted = execution_context.materialize_accepted_task_review( + control, + task, + reference, + { + "causal_class": "initial_acceptance", + "affected_task": "task-native-live", + "authorized_lifecycle_action": "materialize_accepted_result", + }, + executor_handoff=handoff, + validated_executor_result=validated, + ) + _, consumed = execution_context.load_current_accepted_task_result(control, task) + assert consumed == accepted + assert accepted["review_id"] == review["review_id"] + assert accepted["baseline_identity"] == { + "head": baseline_head, + "tree": baseline_tree, + } From 099ab79d2c64e83741d70f4d76131890a0f5a6c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=88=98=E5=8D=87=E9=BE=99?= Date: Wed, 9 Sep 2026 09:50:02 +0800 Subject: [PATCH 05/18] docs(orchestration): converge native review contracts --- .../assets/orchestration/contract/task-v1.md | 2 +- references/assets/orchestration/workflow.md | 25 +++++++---- rules/orchestration/orch-review-completion.md | 3 +- skills/orch-execute-plan/SKILL.md | 2 +- skills/orch-review-plan/SKILL.md | 4 +- .../test_orchestration_workflow_contracts.py | 41 +++++++++++++++++++ 6 files changed, 66 insertions(+), 11 deletions(-) diff --git a/references/assets/orchestration/contract/task-v1.md b/references/assets/orchestration/contract/task-v1.md index 0276069..654fa12 100644 --- a/references/assets/orchestration/contract/task-v1.md +++ b/references/assets/orchestration/contract/task-v1.md @@ -150,7 +150,7 @@ A legacy 3-column `Command or inspection | Proves | Expected` row without YAML ` - When `task_fit_check.result` is `repaired`, completion requires `review_mode: repair`, the native review envelope fields, and exactly one `previous_review`. The closed `repair_frontier` binds that predecessor's review ID, blocking finding IDs, previous and repaired target identities, affected boundaries, and frozen evidence identity. Whole review history is not embedded or reacquired. - Both the current and previous task-review records retain `required: true`, `reviewer_independent: true`, and `review_target_kind: task`; the adapter does not infer or overwrite those ownership facts. The accepted repaired target names the completed task and its `source_tree` plus `reviewed_head` must equal the helper-observed Git tree and head. - Material redesign or changed authority, scope, acceptance, decomposition, or validation allocation requires a fresh `initial` review with `review_reset` bound to the prior review, classified reason, and current target and evidence. The reviewer may reuse the same agent identity when judgment-capable and independent by authorship/repair/decision/deliberation participation and review provenance; identity rotation is not a freshness requirement. -- Task and stage review results are first-class review-store records. Lifecycle admission takes only `{review_id, sha256}` plus the expected current target; it revalidates the native immutable reviewer-run receipt before exposing a verdict or selecting a finding. Bare output, receipt, or finding objects are non-authoritative. +- Task and stage review results are first-class review-store records. Lifecycle admission takes only `{review_id, sha256}` plus the expected current target; it revalidates the provider-specific reviewer-run receipt before exposing a verdict or selecting a finding. Bare output, receipt, or finding objects are non-authoritative. - The task-review product candidate contains accepted product requirements/boundaries, exact product source/diff identity, normalized harness-owned validation observations, and unresolved product concerns. Handoff, knowledge disposition, reviewer history, and publication/status/archive bookkeeping remain controller-owned and are excluded from reviewer judgment. Controller/orchestration code remains product when allocated by the task. - A stored post-execution task repair review may recompute the compact accepted result while preserving its executor-result digest, validation evidence identities, owner, baseline, and knowledge disposition. This path performs no executor redispatch, replacement handoff, validation rerun, or review-history embedding. diff --git a/references/assets/orchestration/workflow.md b/references/assets/orchestration/workflow.md index 24c1a62..beb5706 100644 --- a/references/assets/orchestration/workflow.md +++ b/references/assets/orchestration/workflow.md @@ -103,7 +103,9 @@ retained for blocked/repair evidence, never sole acceptance. Accepted review req direct-source or reproducible-snapshot context and no unavailable claim-relevant evidence. Snapshot access additionally requires explicit snapshot artifact digests. The record describes evidence access; lifecycle acceptance additionally requires -`reviewer_run: {run_id, sha256}` referencing a native `reviewer-process-receipt-v1`. +`reviewer_run: {run_id, sha256}` referencing a provider-specific reviewer-run receipt: +`reviewer-native-receipt-v1` for native host runs or +`reviewer-process-receipt-v1` for legacy sandboxed process runs. Envelope validation alone (including historical records without that reference) is not lifecycle admission. The gate resolves the controller-owned store through `reviewer_runtime_root(workspace_root)` under `~/.work-bundle/reviewer-runtime/workspaces/`; @@ -116,8 +118,11 @@ For task review it instead adds native `task_review_context`, binding the task t review mode/frontier or reset, reviewer identity/capability, execution identity, and evidence mode. Workspace creation admits it only when the source checkout is clean and its exact HEAD/tree still equal that task target. -The current sandbox denies live source/control access, so its packet builder derives -`evidence_mode`; requesting `direct_source` does not grant it. A mechanically complete +The frozen packet builder derives `evidence_mode` from available evidence; requesting +`direct_source` does not grant it. The legacy process sandbox denies live source/control +access. The ordinary native host path consumes the same explicit frozen evidence, +suppresses author transport and user configuration, disables tools, and rejects observed +tool activity; native host read-only policy is not OS process isolation. A mechanically complete `stage-evidence-manifest-v1` yields `reproducible_snapshot`; missing evidence yields `packet_only`, which cannot grant acceptance, even with `unavailable_evidence: []`. The manifest binds stage/target identity, required locators, roles, artifact digests, @@ -146,8 +151,9 @@ recomputing packet/receipt hashes cannot turn partial evidence into complete evi `stage_target_identity` computes the target from current source artifacts, and workspace creation checks it again. Complete stage evidence is checked before any -reviewer process launch. Run the worker with `reviewer-process-run` using that runtime -root. Specification and plan workers retain the stage-review contract; task and +reviewer launch. Use `run_native_reviewer` for the ordinary plugin-independent native path; +`reviewer-process-run` remains the legacy sandboxed process runner. Specification and +plan workers retain the stage-review contract; task and integrated-implementation product workers return the compact `task_review` judgment defined by `dev-code-review`. The controller constructs the native envelope from frozen target, independence, and evidence context, then binds its canonical digest into the receipt. The controller then attaches the run @@ -157,8 +163,11 @@ named-finding routing resolve only that stored reference and recheck its receipt current target; bare stdout, unattached receipts, and bare findings remain observations. The lifecycle gate verifies review ID, exact result/target/profile, successful -completion, sandbox/network/write boundary, and immutable packet/profile/event -digests. Run-scoped evidence remains available after workspace cleanup; full traces +completion, the provider-specific execution boundary, and immutable packet/profile/event +digests. Native receipts bind the executable, request, actual host run identity, sanitized +context, read-only policy, and absence of observed tool activity. Legacy process receipts +bind the sandbox, denied network, and scratch-only write boundary. Run-scoped evidence +remains available after workspace cleanup; full traces are never embedded into the stage envelope. Missing, altered, failed, mutable, or mismatched provenance cannot grant acceptance. Known execution IDs are obtained from artifact `execution_id`, `author_execution_id(s)`, `repair_execution_id(s)` and @@ -271,6 +280,8 @@ accepted manifests into a live source inventory. Final review aggregates accepted task dispositions from execution and task-review evidence. Any accepted `update`, `supersede`, or `reclassify` promotes durable closure to `required` even when the specification's upstream Knowledge Base Update state was `not-needed`; accepted `none` does not. Rejected task dispositions do not trigger closure. Archive is allowed only after required optional reviews are accepted, declared plan-level/integration acceptance is recorded, validation and handoffs are coherent, barriers converged, the resulting Knowledge Base Update disposition is `completed` or `not-needed`, approved `ks-*` return evidence exists when required, and allowed commit/CodeGraph/metadata/archive/index mechanics complete or are explicitly inapplicable. Missing review verdicts are not a blocker when no task set `acceptance_review.required: true`. +Knowledge closure gates final completion and archive; it never precedes specification, plan, task, or integrated-implementation review. + Only approved keep-summarizing owners write durable knowledge. Final orchestration review owns approved persistence delegation and may invoke that owner, then validate returned paths or an evidence-backed no-write result; executors and orchestration itself must not write knowledge directly. Specification authoring materializes `impact_decisions` from bounded current-state evidence about the requested surface, upstream/downstream relations, validation surfaces, and relevant dirty work. A relation is material only when its disposition could change a requirement, constraint, acceptance criterion, user-observable or contractual outcome, architectural boundary, measurable quality target, validation target, or declared boundary. Each material relation is `accepted | excluded | blocking`: accepted relations use `projects_to` for stable specification IDs, excluded relations require evidence, and blocking relations prevent verification. Stop when further exploration could change none of those surfaces and record the reason; a greenfield result may use `none_relevant` only with the searched boundary, reason, and `stopping_reason`. Targeted Git history, prior work artifacts, execution evidence, or durable knowledge is an escalation for contradiction, unresolved ownership, material regression/causality, or suspected governing legacy decisions—not mandatory full-history archaeology or broad knowledge retrieval. This impact-decision view is compared by semantic convergence; repository traversal remains owned by specification authoring. diff --git a/rules/orchestration/orch-review-completion.md b/rules/orchestration/orch-review-completion.md index 10a4508..b5a9134 100644 --- a/rules/orchestration/orch-review-completion.md +++ b/rules/orchestration/orch-review-completion.md @@ -17,7 +17,7 @@ Keep final review focused on whether the WorkBundle workflow completed correctly ## Must - Confirm each required task review judged the accepted product requirements/boundaries, exact product source/diff, normalized validation observations, and unresolved product concerns before accepting the task. -- Admit a task-or-stage verdict or route a finding only from its immutable review-store reference after native reviewer-run receipt and exact current-target validation. Treat bare reviewer output, unattached receipts, and bare findings as observations only. +- Admit a task-or-stage verdict or route a finding only from its immutable review-store reference after provider-specific reviewer-run receipt and exact current-target validation. Treat bare reviewer output, unattached receipts, and bare findings as observations only. - Keep product review candidates limited to accepted product requirements/boundaries, exact source/diff identity, harness-owned normalized observations, and unresolved product concerns. Handoff, knowledge, reviewer-history, and publication/archive bookkeeping remain controller audit concerns and cannot become review inputs. Controller/orchestration code remains reviewable product when allocated by the task. - On reviewer infrastructure or provider failure, preserve the immutable candidate and repair the first broken preparation/provider owner. A still-independent capable reviewer may be reused; infrastructure failure does not itself require identity rotation, source change, validation rerun, or another product review after a completed judgment. - For a finding-scoped repair review under unchanged authority, carry exactly the previous finding/evidence frontier and review only the repaired identity and affected boundaries. Reset to an initial frontier only after a material authority, scope, acceptance, decomposition, or validation-allocation change. @@ -28,6 +28,7 @@ Keep final review focused on whether the WorkBundle workflow completed correctly - Keep this pre-closure oracle-capability check distinct from `RuntimeVerificationClassificationV1`. WOR-59 G9 remains the unchanged post-execution classifier and may use this map only as evidence when triggered. Mechanical helpers validate IDs, completeness, provenance, and observed results; agents own semantic capability judgment and must not impose a universal browser, E2E, production, or runtime gate. - Missing `acceptance_review.verdict` blocks only a task that explicitly required independent review. Do not require universal task-review evidence. - Keep approved `ks-*` persistence delegation review-owned; executor disposition evidence never authorizes knowledge retrieval or writes. +- Knowledge closure gates final completion and archive; it never precedes specification, plan, task, or integrated-implementation review. - Audit spec, plan, phase, task, handoff, and required optional-review status coherence. - Require fresh planned validation evidence and an `accept` task-review verdict wherever review is explicitly required. diff --git a/skills/orch-execute-plan/SKILL.md b/skills/orch-execute-plan/SKILL.md index db9c36c..2c67f2b 100644 --- a/skills/orch-execute-plan/SKILL.md +++ b/skills/orch-execute-plan/SKILL.md @@ -56,7 +56,7 @@ python3 scripts/orch.py build-review-package \ Use `--head worktree` for pre-commit review; the compiler includes tracked, staged, unstaged, and untracked changes, assigns a stable worktree identity, and withholds protected-path content. -The reviewer uses only that bounded product candidate and `dev-code-review`, returning compact `accept|repair` product judgment. Invalid or incomplete input is a controller input/runner failure outside the product verdict. The controller composes the native envelope, verifies independent provenance, and publishes it. A task or stage verdict becomes lifecycle authority only after exact result and native immutable reviewer-run receipt are stored and validated against controller-authorized target identity. +The reviewer uses only that bounded product candidate and `dev-code-review`, returning compact `accept|repair` product judgment. Invalid or incomplete input is a controller input/runner failure outside the product verdict. The controller composes the native envelope, verifies independent provenance, and publishes it. A task or stage verdict becomes lifecycle authority only after the exact result and its provider-specific reviewer-run receipt are stored and validated against controller-authorized target identity. If reviewer infrastructure or provider failure prevents a verdict, preserve the immutable package and repair the first broken runner/provider owner. A capable independent reviewer may be reused. Do not change source, rerun validation, reslice, or require identity rotation for provider availability. Publication retry after a completed judgment reuses the exact result and receipt. diff --git a/skills/orch-review-plan/SKILL.md b/skills/orch-review-plan/SKILL.md index bc8ce49..92d194d 100644 --- a/skills/orch-review-plan/SKILL.md +++ b/skills/orch-review-plan/SKILL.md @@ -30,7 +30,7 @@ Verify: - approved `ks-*` return evidence exists when durable knowledge was required; - allowed commit, applicable CodeGraph sync, metadata update, archive, and index refresh completed or are explicitly not applicable. - dependency, finalization, resume, and archive decisions consume compact accepted results and current harness observations without replaying transient evidence or historical handoff chains. -- required task and stage verdicts are admitted from immutable review-store references with native reviewer-run receipts and still-current targets; bare output, unattached receipts, and bare findings are not lifecycle authority. +- required task and stage verdicts are admitted from immutable review-store references with provider-specific reviewer-run receipts and still-current targets; bare output, unattached receipts, and bare findings are not lifecycle authority. - product findings concern accepted product requirements/boundaries, exact product source/diff, normalized harness observations, and unresolved product concerns; handoff, knowledge disposition, reviewer history, and publication/status/archive bookkeeping stay with controller audit. ## Evidence capability correspondence @@ -92,6 +92,8 @@ Do not create a repair specification for every failed gate. When the upstream disposition or aggregate accepted task dispositions make closure `required`, invoke the approved keep-summarizing owner with accepted implementation, validation, handoff, review, and decision evidence. Review owns approved persistence delegation; executor disposition evidence never invokes a `ks-*` skill. Validate structural-value result, written or updated durable paths or evidence-backed no-write rationale, index rebuild status, blockers, and completion state. Resume only from that return evidence. Orchestration does not directly create, edit, promote, delete, or index durable knowledge, and archive remains blocked until the validated return resolves required closure. +Knowledge closure gates final completion and archive; it never precedes specification, plan, task, or integrated-implementation review. + ## Finalization Keep audit judgment and deterministic finalization together in this skill for now; do not create `orch-finalize-plan`. After every audit gate passes, invoke the smallest existing helper for allowed commit, CodeGraph sync, project metadata update, archive, and index refresh. Clean only a WorkBundle-owned execution workspace when policy and proven Git identity allow it. diff --git a/tests/test_orchestration_workflow_contracts.py b/tests/test_orchestration_workflow_contracts.py index 82e5baa..34f4adb 100644 --- a/tests/test_orchestration_workflow_contracts.py +++ b/tests/test_orchestration_workflow_contracts.py @@ -1929,6 +1929,47 @@ def test_review_required_task_fails_closed_until_independent_accept() -> None: assert validated["result_state"] == "completed" +def test_workflow_distinguishes_native_and_legacy_process_review_provenance() -> None: + workflow = read("references/assets/orchestration/workflow.md") + + for token in [ + "`reviewer-native-receipt-v1` for native host runs", + "`reviewer-process-receipt-v1` for legacy sandboxed process runs", + "`run_native_reviewer` for the ordinary plugin-independent native path", + "native host read-only policy is not OS process isolation", + "provider-specific execution boundary", + ]: + assert token in workflow + for process_only_claim in [ + "referencing a native `reviewer-process-receipt-v1`", + "Run the worker with `reviewer-process-run` using that runtime root", + "completion, sandbox/network/write boundary, and immutable packet/profile/event", + ]: + assert process_only_claim not in workflow + + +def test_review_contract_owners_use_common_provenance_and_final_knowledge_gate() -> None: + provenance_owners = [ + "skills/orch-execute-plan/SKILL.md", + "skills/orch-review-plan/SKILL.md", + "references/assets/orchestration/contract/task-v1.md", + "rules/orchestration/orch-review-completion.md", + "references/assets/orchestration/workflow.md", + ] + for owner in provenance_owners: + assert "provider-specific reviewer-run receipt" in read(owner), owner + + final_gate_owners = [ + "skills/orch-review-plan/SKILL.md", + "rules/orchestration/orch-review-completion.md", + "references/assets/orchestration/workflow.md", + ] + for owner in final_gate_owners: + text = read(owner) + assert "Knowledge closure gates final completion and archive" in text, owner + assert "never precedes specification, plan, task, or integrated-implementation review" in text, owner + + def test_overlapping_writes_are_not_parallelizable() -> None: execute = read("skills/orch-execute-plan/SKILL.md") create = read("skills/orch-create-implementation-plan/SKILL.md") From 082f02ff9a198df259f27c921532ea77e6fa38a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=88=98=E5=8D=87=E9=BE=99?= Date: Wed, 9 Sep 2026 10:53:14 +0800 Subject: [PATCH 06/18] fix(orchestration): consume live validation once --- scripts/orchestration/execution_context.py | 84 ++++++++++++- tests/test_orchestration_execution_context.py | 115 +++++++++++++++++- 2 files changed, 192 insertions(+), 7 deletions(-) diff --git a/scripts/orchestration/execution_context.py b/scripts/orchestration/execution_context.py index 7f283fd..804d213 100644 --- a/scripts/orchestration/execution_context.py +++ b/scripts/orchestration/execution_context.py @@ -3687,6 +3687,14 @@ def _observe_completed_validation( from review_runtime import require_plan_reviews require_plan_reviews(control_root, _find_plan(control_root, str(task["plan_id"]))[0]) binding = load_task_execution_binding(control_root, str(task["plan_id"]), str(task["task_id"])) + fit = handoff.get("task_fit_check") if isinstance(handoff.get("task_fit_check"), Mapping) else {} + if ( + isinstance(binding.get("accepted_result"), Mapping) + and fit.get("result") != "repaired" + ): + raise SystemExit( + "accepted task result already exists; consume it without rerunning validation" + ) if workspace_id and str(binding.get("workspace_id") or "") != str(workspace_id): raise SystemExit("Task execution binding workspace_id mismatch") if execution_id and str(binding.get("execution_id") or "") != str(execution_id): @@ -3714,10 +3722,36 @@ def _observe_completed_validation( accepted_dependency_paths=accepted_paths, ) for item in required_items: + policy = _completion_provenance_module().validation_reuse_policy(item) + observed_item = item + finalization_id = None + if policy["max_age_seconds"] == 0: + # A live check cannot be reusable by later lifecycle consumers, but + # initial acceptance still needs one immutable producer-to-consumer + # observation. Keep it recoverable for the atomic acceptance call; + # the stable finalization claim and accepted-result guard prevent a + # later validation dispatch from treating it as reusable evidence. + observed_item = dict(item) + observed_item["evidence_reuse"] = { + **policy, + "max_age_seconds": 86400, + } + finalization_identity = semantic_digest( + { + "command": str(item.get("command") or "").strip(), + "repair": fit.get("result") == "repaired", + "source": pre_batch, + } + ) + finalization_id = ( + f"initial-acceptance:{task['plan_id']}:{task['task_id']}:" + f"{finalization_identity}" + ) observed = _completion_provenance_module().observe_validation( - binding, task, item, pre_batch, + binding, task, observed_item, pre_batch, lambda receipt: _observe_validation_item(item, execution_root, task, receipt), lambda: capture_repository_evidence(execution_root), + finalization_id=finalization_id, ) command = str(item.get("command")).strip() reported_item = reported_commands[command] if reported_commands is not None else None @@ -4072,6 +4106,45 @@ def validate_executor_result_for_task( accepted_dependency_deltas=accepted_dependency_deltas, ) + task_ownership = None + fit = handoff.get("task_fit_check") if isinstance(handoff.get("task_fit_check"), dict) else {} + if ( + state == "completed" + and required_items + and observe + and fit.get("result") == "repaired" + and acceptance_review_sequence != "initial-reset" + ): + if mutation_events is None: + raise AcceptanceOwnershipError( + "review-blocked: completed task requires harness-owned mutation_events evidence" + ) + runtime_mutation_events = list(mutation_events) + if any(not isinstance(event, Mapping) for event in runtime_mutation_events): + raise AcceptanceOwnershipError("Runtime mutation_events must contain mappings") + try: + task_ownership = validate_task_acceptance_ownership( + delegation_evidence=( + handoff.get("delegation_evidence") + if isinstance(handoff.get("delegation_evidence"), dict) + else None + ), + mutation_events=runtime_mutation_events, + write_scope=_as_list(task_files.get("write")), + validations_passed=True, + operation="repair", + ) + _validate_repair_acceptance_continuity( + task=task, + handoff=handoff, + current_ownership=task_ownership, + prior_ownership=prior_ownership, + repair_continuity=repair_continuity, + authorized_replacements=authorized_replacements, + ) + except OwnershipBlocker as error: + raise AcceptanceOwnershipError(str(error)) from error + if state == "completed" and required_items and observe: observed_validation = _observe_completed_validation( handoff, @@ -4092,7 +4165,6 @@ def validate_executor_result_for_task( raise SystemExit( "evidence-closure-blocked: completed mapped invariants require produced harness observations and passed evidence closure" ) - task_ownership = None if state == "completed": if mutation_events is None: raise AcceptanceOwnershipError( @@ -4101,7 +4173,6 @@ def validate_executor_result_for_task( runtime_mutation_events = list(mutation_events) if any(not isinstance(event, Mapping) for event in runtime_mutation_events): raise AcceptanceOwnershipError("Runtime mutation_events must contain mappings") - fit = handoff.get("task_fit_check") if isinstance(handoff.get("task_fit_check"), dict) else {} operation = "repair" if fit.get("result") == "repaired" else "implementation" try: task_ownership = validate_task_acceptance_ownership( @@ -5329,6 +5400,13 @@ def cmd_build_review_package(args: argparse.Namespace) -> None: def cmd_observe_task_validation(args: argparse.Namespace) -> None: _, brief_document = _compile_task_brief(args) task = brief_document["task_brief"] + for item in task["validation"]: + policy = _completion_provenance_module().validation_reuse_policy(item) + if policy["max_age_seconds"] == 0: + raise SystemExit( + "non-reusable validation must use validate-executor-result so its " + "single observation is captured by initial acceptance" + ) runtime = _observation_kwargs(args) observed = _observe_completed_validation( {}, diff --git a/tests/test_orchestration_execution_context.py b/tests/test_orchestration_execution_context.py index 5644fe7..1d6e837 100644 --- a/tests/test_orchestration_execution_context.py +++ b/tests/test_orchestration_execution_context.py @@ -94,6 +94,108 @@ def test_observe_task_validation_records_before_handoff(tmp_path: Path, capsys) assert counter.read_text() == "1" +def test_live_validation_is_captured_once_by_atomic_initial_acceptance( + tmp_path: Path, capsys, +) -> None: + root, task, brief, handoff, counter = _counted_validation( + tmp_path, reuse_seconds=0 + ) + + with pytest.raises(SystemExit, match="non-reusable.*validate-executor-result"): + execution_context.cmd_observe_task_validation(args(root, task)) + assert not counter.exists() + + validated = _validate_observed(handoff, brief) + observed = validated["observed_validation"][0] + assert observed["result"] == "passed" + assert observed["observation_id"].startswith("observation-") + assert counter.read_text() == "1" + accepted = execution_context.materialize_accepted_task_result( + root, brief, handoff, validated + ) + _, consumed = execution_context.load_current_accepted_task_result(root, brief) + assert consumed == accepted + assert accepted["validation_evidence_ids"] == [observed["observation_id"]] + assert counter.read_text() == "1" + + store = root / ".work-bundle/runtime/completion-provenance/completion-provenance-v1.json" + provenance = json.loads(store.read_text()) + saved = next( + item + for item in provenance["observations"] + if item["observation_id"] == observed["observation_id"] + ) + assert set(saved["result"]) == { + "exit_code", "stdout_digest", "stderr_digest", "started_at", "completed_at" + } + assert provenance["consumptions"][observed["observation_id"]].startswith( + "initial-acceptance:" + ) + with pytest.raises(SystemExit, match="already exists.*without rerunning"): + _validate_observed(handoff, brief) + assert counter.read_text() == "1" + + +def test_live_validation_allows_one_continuity_checked_source_repair( + tmp_path: Path, +) -> None: + root, _, brief, handoff, counter = _counted_validation( + tmp_path, reuse_seconds=0 + ) + validated = _validate_observed(handoff, brief) + execution_context.materialize_accepted_task_result(root, brief, handoff, validated) + execution_context.load_current_accepted_task_result(root, brief) + assert counter.read_text() == "1" + + source = root / WRITE_SCOPE_FILE + source.write_text(source.read_text() + "\n# scoped repair\n") + repaired = deepcopy(handoff) + repaired["task_fit_check"]["result"] = "repaired" + repaired["acceptance_review"] = { + "required": False, + "repair_frontier": { + "prior_review_id": "review-prior", + "frozen_evidence_reference": "evidence-prior", + }, + } + binding = execution_context.load_task_execution_binding( + root, str(brief["plan_id"]), str(brief["task_id"]) + ) + continuity = { + "binding_id": binding["ownership"]["binding_id"], + "baseline_identity": execution_context.semantic_digest(binding["baseline"]), + "evidence_identity": "evidence-prior", + "previous_review_identity": "review-prior", + } + with pytest.raises( + execution_context.AcceptanceOwnershipError, match="repair.*continuity" + ): + execution_context.validate_executor_result_for_task( + repaired, + brief, + observe=True, + mutation_events=[{"actor_kind": "subagent", "paths": [WRITE_SCOPE_FILE]}], + prior_ownership={str(brief["task_id"]): handoff["delegation_evidence"]}, + repair_continuity=None, + ) + assert counter.read_text() == "1" + + repaired_result = execution_context.validate_executor_result_for_task( + repaired, + brief, + observe=True, + mutation_events=[{"actor_kind": "subagent", "paths": [WRITE_SCOPE_FILE]}], + prior_ownership={str(brief["task_id"]): handoff["delegation_evidence"]}, + repair_continuity={str(brief["task_id"]): continuity}, + ) + + assert repaired_result["result_state"] == "completed" + assert repaired_result["observed_validation"][0]["observation_id"] != validated[ + "observed_validation" + ][0]["observation_id"] + assert counter.read_text() == "2" + + @pytest.mark.parametrize("change", ["source", "oracle", "environment"]) def test_handoff_validation_invalidates_changed_inputs(tmp_path: Path, monkeypatch, change: str) -> None: root, _, brief, handoff, counter = _counted_validation(tmp_path) @@ -109,11 +211,16 @@ def test_handoff_validation_invalidates_changed_inputs(tmp_path: Path, monkeypat assert counter.read_text() == "2" -def test_handoff_validation_live_checks_do_not_reuse(tmp_path: Path) -> None: +def test_handoff_validation_live_check_retry_consumes_same_initial_observation( + tmp_path: Path, +) -> None: _, _, brief, handoff, counter = _counted_validation(tmp_path, reuse_seconds=0) - _validate_observed(handoff, brief) - _validate_observed(handoff, brief) - assert counter.read_text() == "2" + first = _validate_observed(handoff, brief) + second = _validate_observed(handoff, brief) + assert counter.read_text() == "1" + assert first["observed_validation"][0]["observation_id"] == second[ + "observed_validation" + ][0]["reuse_of"] def test_handoff_validation_expiry_requires_new_observation(tmp_path: Path, monkeypatch) -> None: From 7515be7c4adc3b10a01d4dd388103bc49f2d07ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=88=98=E5=8D=87=E9=BE=99?= Date: Wed, 9 Sep 2026 11:19:49 +0800 Subject: [PATCH 07/18] fix(review): bound integrated native input --- scripts/orchestration/review_runtime.py | 2 +- scripts/work-bundle/reviewer_workspace.py | 169 +++++++++++++++++++++- tests/test_orchestration_reviews.py | 122 ++++++++++++++++ 3 files changed, 291 insertions(+), 2 deletions(-) diff --git a/scripts/orchestration/review_runtime.py b/scripts/orchestration/review_runtime.py index 48cf1d9..7616f45 100644 --- a/scripts/orchestration/review_runtime.py +++ b/scripts/orchestration/review_runtime.py @@ -551,8 +551,8 @@ def _validate_native_run_proof(receipt, packet, result, path, immutable_file, ca or request["review_input"] != runtime._native_review_input(packet) or not isinstance(request["instructions"], str) or not request["instructions"].strip()): raise ValueError("native launch/input mismatch") - artifacts = packet["artifacts"] evidence = request["evidence"] + artifacts = runtime._native_review_artifacts(packet, evidence) if len(evidence) != len(artifacts): raise ValueError("native input evidence mismatch") for expected, actual in zip(artifacts, evidence): diff --git a/scripts/work-bundle/reviewer_workspace.py b/scripts/work-bundle/reviewer_workspace.py index 095f0a0..f2bf699 100644 --- a/scripts/work-bundle/reviewer_workspace.py +++ b/scripts/work-bundle/reviewer_workspace.py @@ -33,6 +33,147 @@ "host_skill_catalog": "may-be-present", "tools": "disabled-and-no-observed-activity", "os_process_isolation": False, } +NATIVE_REVIEW_REQUEST_MAX_CHARS = 1_048_576 + + +def _structured_evidence(content: str) -> dict[str, object] | None: + try: + value = json.loads(content) + except json.JSONDecodeError: + try: + value = _review_runtime().parse_yaml_subset(content) + except (SystemExit, ValueError, TypeError): + return None + return value if isinstance(value, dict) else None + + +def _integrated_change_manifest( + packet: dict[str, object], evidence: list[dict[str, object]], +) -> tuple[dict[str, object], set[str]] | None: + context = packet.get("stage_review_context") + if not isinstance(context, dict) or context.get("stage") != "integrated_implementation": + return None + target = context.get("target_identity") + target_tree = target.get("source_tree") if isinstance(target, dict) else None + candidates: list[tuple[dict[str, object], set[str]]] = [] + for item in evidence: + if not str(item.get("locator") or "").startswith("control:"): + continue + content = item.get("content") + if not isinstance(content, str): + continue + value = _structured_evidence(content) + if not isinstance(value, dict): + continue + baseline = value.get("baseline") + endpoint = value.get("endpoint") + comparison = value.get("comparison") + if not all(isinstance(part, dict) for part in (baseline, endpoint, comparison)): + continue + paths = comparison.get("paths") + if endpoint.get("tree") != target_tree or not isinstance(paths, list): + continue + locators: set[str] = set() + valid = True + for raw in paths: + if not isinstance(raw, dict) or set(raw) != {"status", "path"}: + valid = False + break + path = Path(str(raw.get("path") or "")) + if ( + raw.get("status") not in {"added", "modified", "deleted"} + or path.is_absolute() + or not path.parts + or ".." in path.parts + ): + valid = False + break + if raw["status"] != "deleted": + locators.add("source:" + path.as_posix()) + if valid: + candidates.append((value, locators)) + if len(candidates) > 1: + raise ReviewerWorkspaceError("WB_REVIEW_CHANGE_MANIFEST_AMBIGUOUS") + return candidates[0] if candidates else None + + +def _native_review_artifacts( + packet: dict[str, object], evidence: list[dict[str, object]], +) -> list[dict[str, object]]: + manifest = _integrated_change_manifest(packet, evidence) + artifacts = packet.get("artifacts") + if not isinstance(artifacts, list) or manifest is None: + return [item for item in artifacts or [] if isinstance(item, dict)] + _, changed = manifest + return [ + item + for item in artifacts + if isinstance(item, dict) + and ( + not str(item.get("locator") or "").startswith("source:") + or item.get("locator") in changed + ) + ] + + +def _validate_integrated_change_manifest( + source_root: Path, packet: dict[str, object], evidence: list[dict[str, object]], +) -> None: + selected = _integrated_change_manifest(packet, evidence) + if selected is None: + return + manifest, changed = selected + baseline = manifest["baseline"] + endpoint = manifest["endpoint"] + comparison = manifest["comparison"] + baseline_head = str(baseline.get("head") or "") + endpoint_head = str(endpoint.get("head") or "") + expected_command = f"git diff --name-status {baseline_head}..{endpoint_head}" + if comparison.get("command") != expected_command: + raise ReviewerWorkspaceError("WB_REVIEW_CHANGE_MANIFEST_INVALID") + head = subprocess.run( + ["git", "-C", str(source_root), "rev-parse", "HEAD"], capture_output=True, text=True + ) + tree = subprocess.run( + ["git", "-C", str(source_root), "rev-parse", f"{endpoint_head}^{{tree}}"], + capture_output=True, + text=True, + ) + ancestor = subprocess.run( + ["git", "-C", str(source_root), "merge-base", "--is-ancestor", baseline_head, endpoint_head], + capture_output=True, + ) + diff = subprocess.run( + ["git", "-C", str(source_root), "diff", "--name-status", baseline_head, endpoint_head], + capture_output=True, + text=True, + ) + status_names = {"A": "added", "M": "modified", "D": "deleted"} + actual: list[dict[str, str]] = [] + if not diff.returncode: + for row in diff.stdout.splitlines(): + columns = row.split("\t") + if len(columns) != 2 or columns[0] not in status_names: + raise ReviewerWorkspaceError("WB_REVIEW_CHANGE_MANIFEST_INVALID") + actual.append({"status": status_names[columns[0]], "path": columns[1]}) + paths = comparison.get("paths") + packet_locators = { + str(item.get("locator") or "") + for item in packet.get("artifacts", []) + if isinstance(item, dict) + } + if ( + head.returncode + or head.stdout.strip() != endpoint_head + or tree.returncode + or tree.stdout.strip() != endpoint.get("tree") + or ancestor.returncode + or diff.returncode + or comparison.get("path_count") != len(actual) + or paths != actual + or not changed.issubset(packet_locators) + ): + raise ReviewerWorkspaceError("WB_REVIEW_CHANGE_MANIFEST_INVALID") def parse_native_reviewer_transcript(raw: str, stderr: str = "") -> tuple[str, dict[str, object]]: @@ -162,8 +303,14 @@ def run_native_reviewer(workspace: Path, executable: Path, *, model: str, review packet, _ = _load_workspace(workspace) if not ("stage_review_context" in packet or "task_review_context" in packet): raise ReviewerWorkspaceError("WB_REVIEW_NATIVE_CONTEXT_REQUIRED") - evidence = [] + control_evidence = [] for item in packet["artifacts"]: + if str(item.get("locator") or "").startswith("control:"): + content = _evidence_path(workspace, item["locator"]).read_bytes().decode("utf-8") + control_evidence.append({**item, "content": content}) + selected_artifacts = _native_review_artifacts(packet, control_evidence) + evidence = [] + for item in selected_artifacts: # Text-mode reads normalize CRLF. The model input must preserve the exact # frozen bytes whose digest will be revalidated during publication. content = _evidence_path(workspace, item["locator"]).read_bytes().decode("utf-8") @@ -171,6 +318,11 @@ def run_native_reviewer(workspace: Path, executable: Path, *, model: str, review raise ReviewerWorkspaceError("WB_REVIEW_EVIDENCE_MUTATED") evidence.append({**item, "content": content}) request = {"instructions": review_instructions, "review_input": _native_review_input(packet), "evidence": evidence} + if len(json.dumps(request, sort_keys=True, ensure_ascii=False)) > NATIVE_REVIEW_REQUEST_MAX_CHARS: + raise ReviewerWorkspaceError( + "WB_REVIEW_NATIVE_INPUT_TOO_LARGE", + {"max_chars": NATIVE_REVIEW_REQUEST_MAX_CHARS}, + ) argv = _native_reviewer_argv(executable, workspace, model) return _run_reviewer(workspace, argv, native_request=request) @@ -626,6 +778,21 @@ def create_reviewer_workspace( mode = "packet_only" if manifest["missing"] else "reproducible_snapshot" if packet.get("stage_evidence_manifest") != manifest or context["evidence_mode"] != mode: raise ReviewerWorkspaceError("WB_REVIEW_STAGE_EVIDENCE_MISMATCH") + control_evidence = [] + for raw in artifacts: + if not isinstance(raw, dict) or not str(raw.get("locator") or "").startswith("control:"): + continue + try: + content = base64.b64decode(str(raw.get("content_base64") or ""), validate=True).decode("utf-8") + except (ValueError, UnicodeDecodeError): + raise ReviewerWorkspaceError("WB_REVIEW_PACKET_INVALID") from None + control_evidence.append( + { + **{key: value for key, value in raw.items() if key != "content_base64"}, + "content": content, + } + ) + _validate_integrated_change_manifest(effective_source, public_packet, control_evidence) elif "task_review_context" in packet: context = _validate_task_context(packet["task_review_context"]) _validate_task_source_identity(effective_source, context) diff --git a/tests/test_orchestration_reviews.py b/tests/test_orchestration_reviews.py index fb93d76..d9889f1 100644 --- a/tests/test_orchestration_reviews.py +++ b/tests/test_orchestration_reviews.py @@ -680,6 +680,128 @@ def test_integrated_snapshot_uses_compact_acceptance_not_handoff_history(tmp_pat assert not any("handoff" in locator for locator in required) +def test_native_integrated_review_bounds_large_unchanged_tree_to_exact_change_manifest( + tmp_path: Path, monkeypatch, +) -> None: + import reviewer_workspace + + _, plan, _ = _reviewed_plan_fixture(tmp_path, provenance=False) + task = _write_stage_task(plan) + protected = tmp_path / ".work-bundle/protected-test" + protected.mkdir(parents=True, exist_ok=True) + (tmp_path / ".gitignore").write_text(".work-bundle/\n", encoding="utf-8") + (tmp_path / "unchanged-large.txt").write_text("x" * 1_100_000, encoding="utf-8") + (tmp_path / "source.txt").write_text("before\n", encoding="utf-8") + for arguments in ( + ["init", "-q"], + ["config", "user.name", "Test"], + ["config", "user.email", "test@example.invalid"], + ["add", "."], + ["commit", "-qm", "baseline"], + ): + subprocess.run(["git", "-C", str(tmp_path), *arguments], check=True) + baseline = subprocess.check_output( + ["git", "-C", str(tmp_path), "rev-parse", "HEAD"], text=True + ).strip() + (tmp_path / "source.txt").write_text("after\n", encoding="utf-8") + subprocess.run(["git", "-C", str(tmp_path), "add", "source.txt"], check=True) + subprocess.run( + ["git", "-C", str(tmp_path), "commit", "-qm", "claim change"], check=True + ) + endpoint = subprocess.check_output( + ["git", "-C", str(tmp_path), "rev-parse", "HEAD"], text=True + ).strip() + tree = subprocess.check_output( + ["git", "-C", str(tmp_path), "rev-parse", "HEAD^{tree}"], text=True + ).strip() + _write_compact_accepted_result(tmp_path, task=task) + change_manifest = tmp_path / ".work-bundle/runtime/change-manifest.json" + change_manifest.parent.mkdir(parents=True, exist_ok=True) + change_manifest.write_text( + json.dumps( + { + "baseline": {"head": baseline}, + "endpoint": {"head": endpoint, "tree": tree}, + "comparison": { + "command": f"git diff --name-status {baseline}..{endpoint}", + "path_count": 1, + "paths": [{"status": "modified", "path": "source.txt"}], + }, + } + ), + encoding="utf-8", + ) + identity = review_runtime.stage_target_identity( + tmp_path, "integrated_implementation", plan, source_root=tmp_path + ) + locator = "control:" + plan.relative_to(tmp_path).as_posix() + required, missing = review_runtime.stage_evidence_requirements( + tmp_path, "integrated_implementation", plan + ) + assert missing == [] + required.update( + {entry["locator"]: "source_tree" for entry in review_runtime.source_snapshot_entries(tmp_path)} + ) + required["control:" + change_manifest.relative_to(tmp_path).as_posix()] = "change_manifest" + packet = reviewer_workspace.build_direct_evidence_packet( + source_root=tmp_path, + control_root=tmp_path, + protected_roots=[protected], + artifacts=list(required), + search_roots=[], + validators=[], + sentinels=[], + network_state="denied", + stage_review_context={ + "stage": "integrated_implementation", + "target_locator": locator, + "target_identity": identity, + "agent_id": "reviewer-large-tree", + "capability": "judgment", + "execution_id": "reviewer-large-tree-run", + "evidence_mode": "direct_source", + }, + ) + created = reviewer_workspace.create_reviewer_workspace( + review_runtime.reviewer_runtime_root(tmp_path), "review-large-tree", packet + ) + captured: dict[str, str] = {} + + def native_process(_workspace, _argv, request): + captured["request"] = request + events = [ + {"type": "thread.started", "thread_id": "01a0821d-f359-7d60-a9bd-90dd0e006166"}, + {"type": "turn.started"}, + {"type": "item.completed", "item": {"id": "judgment", "type": "agent_message", "text": json.dumps({ + "stage_review": {"target_identity": identity, "verdict": "accepted", "findings": []} + })}}, + {"type": "turn.completed", "usage": {}}, + ] + return subprocess.CompletedProcess([], 0, "\n".join(json.dumps(event) for event in events), "") + + monkeypatch.setattr(reviewer_workspace, "_run_native_process", native_process) + receipt = reviewer_workspace.run_native_reviewer( + Path(str(created["workspace_path"])), + Path(sys.executable), + model="test-model", + review_instructions="Assess the accepted requirements and exact changed source.", + ) + request = json.loads(captured["request"]) + supplied = {item["locator"] for item in request["evidence"]} + assert len(captured["request"]) <= reviewer_workspace.NATIVE_REVIEW_REQUEST_MAX_CHARS + assert "source:source.txt" in supplied + assert "source:unchanged-large.txt" not in supplied + assert request["review_input"]["target_identity"]["source_tree"] == tree + assert any( + item["locator"] == "source:unchanged-large.txt" + for item in request["review_input"]["artifacts"] + ) + review_runtime._validate_reviewer_run( + tmp_path, + {**receipt["review_result"], "reviewer_run": receipt["reviewer_run"]}, + ) + + def test_integrated_snapshot_includes_native_review_when_present_and_rejects_invalid_compact_authority(tmp_path, monkeypatch): _, plan, _ = _reviewed_plan_fixture(tmp_path, provenance=False) task = _write_stage_task(plan, review_required=True) From d300eece474872e913060dd61b93fbfbdb5a9095 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=88=98=E5=8D=87=E9=BE=99?= Date: Wed, 9 Sep 2026 11:33:49 +0800 Subject: [PATCH 08/18] fix(review): compact integrated input with exact diff --- scripts/work-bundle/reviewer_workspace.py | 52 ++++++++++++++++++++--- tests/test_orchestration_reviews.py | 44 ++++++++++++++++++- 2 files changed, 89 insertions(+), 7 deletions(-) diff --git a/scripts/work-bundle/reviewer_workspace.py b/scripts/work-bundle/reviewer_workspace.py index f2bf699..1ee141e 100644 --- a/scripts/work-bundle/reviewer_workspace.py +++ b/scripts/work-bundle/reviewer_workspace.py @@ -49,13 +49,13 @@ def _structured_evidence(content: str) -> dict[str, object] | None: def _integrated_change_manifest( packet: dict[str, object], evidence: list[dict[str, object]], -) -> tuple[dict[str, object], set[str]] | None: +) -> tuple[dict[str, object], set[str], dict[str, str] | None] | None: context = packet.get("stage_review_context") if not isinstance(context, dict) or context.get("stage") != "integrated_implementation": return None target = context.get("target_identity") target_tree = target.get("source_tree") if isinstance(target, dict) else None - candidates: list[tuple[dict[str, object], set[str]]] = [] + candidates: list[tuple[dict[str, object], set[str], dict[str, str] | None]] = [] for item in evidence: if not str(item.get("locator") or "").startswith("control:"): continue @@ -73,6 +73,22 @@ def _integrated_change_manifest( paths = comparison.get("paths") if endpoint.get("tree") != target_tree or not isinstance(paths, list): continue + exact_diff = comparison.get("exact_diff") + normalized_diff = None + if exact_diff is not None: + if ( + not isinstance(exact_diff, dict) + or set(exact_diff) != {"command", "locator", "sha256"} + or not str(exact_diff.get("locator") or "").startswith("control:") + or not re.fullmatch(r"[0-9a-f]{64}", str(exact_diff.get("sha256") or "")) + or not isinstance(exact_diff.get("command"), str) + ): + raise ReviewerWorkspaceError("WB_REVIEW_CHANGE_MANIFEST_INVALID") + normalized_diff = { + "command": exact_diff["command"], + "locator": exact_diff["locator"], + "sha256": exact_diff["sha256"], + } locators: set[str] = set() valid = True for raw in paths: @@ -91,7 +107,7 @@ def _integrated_change_manifest( if raw["status"] != "deleted": locators.add("source:" + path.as_posix()) if valid: - candidates.append((value, locators)) + candidates.append((value, locators, normalized_diff)) if len(candidates) > 1: raise ReviewerWorkspaceError("WB_REVIEW_CHANGE_MANIFEST_AMBIGUOUS") return candidates[0] if candidates else None @@ -104,7 +120,16 @@ def _native_review_artifacts( artifacts = packet.get("artifacts") if not isinstance(artifacts, list) or manifest is None: return [item for item in artifacts or [] if isinstance(item, dict)] - _, changed = manifest + _, changed, exact_diff = manifest + if exact_diff is not None: + matching = [ + item + for item in artifacts + if isinstance(item, dict) and item.get("locator") == exact_diff["locator"] + ] + if len(matching) != 1 or matching[0].get("sha256") != exact_diff["sha256"]: + raise ReviewerWorkspaceError("WB_REVIEW_CHANGE_MANIFEST_INVALID") + changed = set() return [ item for item in artifacts @@ -122,7 +147,7 @@ def _validate_integrated_change_manifest( selected = _integrated_change_manifest(packet, evidence) if selected is None: return - manifest, changed = selected + manifest, changed, exact_diff = selected baseline = manifest["baseline"] endpoint = manifest["endpoint"] comparison = manifest["comparison"] @@ -148,6 +173,10 @@ def _validate_integrated_change_manifest( capture_output=True, text=True, ) + binary_diff = subprocess.run( + ["git", "-C", str(source_root), "diff", "--binary", baseline_head, endpoint_head], + capture_output=True, + ) status_names = {"A": "added", "M": "modified", "D": "deleted"} actual: list[dict[str, str]] = [] if not diff.returncode: @@ -162,6 +191,18 @@ def _validate_integrated_change_manifest( for item in packet.get("artifacts", []) if isinstance(item, dict) } + exact_diff_valid = True + if exact_diff is not None: + supplied = [item for item in evidence if item.get("locator") == exact_diff["locator"]] + expected_binary_command = f"git diff --binary {baseline_head}..{endpoint_head}" + exact_diff_valid = ( + len(supplied) == 1 + and exact_diff["command"] == expected_binary_command + and not binary_diff.returncode + and supplied[0].get("sha256") == exact_diff["sha256"] + and hashlib.sha256(binary_diff.stdout).hexdigest() == exact_diff["sha256"] + and str(supplied[0].get("content") or "").encode("utf-8") == binary_diff.stdout + ) if ( head.returncode or head.stdout.strip() != endpoint_head @@ -172,6 +213,7 @@ def _validate_integrated_change_manifest( or comparison.get("path_count") != len(actual) or paths != actual or not changed.issubset(packet_locators) + or not exact_diff_valid ): raise ReviewerWorkspaceError("WB_REVIEW_CHANGE_MANIFEST_INVALID") diff --git a/tests/test_orchestration_reviews.py b/tests/test_orchestration_reviews.py index d9889f1..6e0487d 100644 --- a/tests/test_orchestration_reviews.py +++ b/tests/test_orchestration_reviews.py @@ -1,5 +1,7 @@ from __future__ import annotations +import base64 +import hashlib import json import subprocess import sys @@ -703,7 +705,7 @@ def test_native_integrated_review_bounds_large_unchanged_tree_to_exact_change_ma baseline = subprocess.check_output( ["git", "-C", str(tmp_path), "rev-parse", "HEAD"], text=True ).strip() - (tmp_path / "source.txt").write_text("after\n", encoding="utf-8") + (tmp_path / "source.txt").write_text("after\n" * 100_000, encoding="utf-8") subprocess.run(["git", "-C", str(tmp_path), "add", "source.txt"], check=True) subprocess.run( ["git", "-C", str(tmp_path), "commit", "-qm", "claim change"], check=True @@ -715,6 +717,13 @@ def test_native_integrated_review_bounds_large_unchanged_tree_to_exact_change_ma ["git", "-C", str(tmp_path), "rev-parse", "HEAD^{tree}"], text=True ).strip() _write_compact_accepted_result(tmp_path, task=task) + exact_diff = tmp_path / ".work-bundle/runtime/integrated-source.diff" + exact_diff.parent.mkdir(parents=True, exist_ok=True) + exact_diff.write_bytes( + subprocess.check_output( + ["git", "-C", str(tmp_path), "diff", "--binary", baseline, endpoint] + ) + ) change_manifest = tmp_path / ".work-bundle/runtime/change-manifest.json" change_manifest.parent.mkdir(parents=True, exist_ok=True) change_manifest.write_text( @@ -726,6 +735,11 @@ def test_native_integrated_review_bounds_large_unchanged_tree_to_exact_change_ma "command": f"git diff --name-status {baseline}..{endpoint}", "path_count": 1, "paths": [{"status": "modified", "path": "source.txt"}], + "exact_diff": { + "command": f"git diff --binary {baseline}..{endpoint}", + "locator": "control:.work-bundle/runtime/integrated-source.diff", + "sha256": hashlib.sha256(exact_diff.read_bytes()).hexdigest(), + }, }, } ), @@ -743,6 +757,7 @@ def test_native_integrated_review_bounds_large_unchanged_tree_to_exact_change_ma {entry["locator"]: "source_tree" for entry in review_runtime.source_snapshot_entries(tmp_path)} ) required["control:" + change_manifest.relative_to(tmp_path).as_posix()] = "change_manifest" + required["control:" + exact_diff.relative_to(tmp_path).as_posix()] = "exact_diff" packet = reviewer_workspace.build_direct_evidence_packet( source_root=tmp_path, control_root=tmp_path, @@ -762,6 +777,24 @@ def test_native_integrated_review_bounds_large_unchanged_tree_to_exact_change_ma "evidence_mode": "direct_source", }, ) + exact_bytes = exact_diff.read_bytes() + tampered = deepcopy(packet) + tampered_bytes = b"controller supplied the wrong diff\n" + exact_diff.write_bytes(tampered_bytes) + tampered_artifact = next( + item + for item in tampered["artifacts"] + if item["locator"] == "control:.work-bundle/runtime/integrated-source.diff" + ) + tampered_artifact["sha256"] = hashlib.sha256(tampered_bytes).hexdigest() + tampered_artifact["content_base64"] = base64.b64encode(tampered_bytes).decode("ascii") + with pytest.raises( + reviewer_workspace.ReviewerWorkspaceError, match="CHANGE_MANIFEST_INVALID" + ): + reviewer_workspace.create_reviewer_workspace( + review_runtime.reviewer_runtime_root(tmp_path), "review-wrong-diff", tampered + ) + exact_diff.write_bytes(exact_bytes) created = reviewer_workspace.create_reviewer_workspace( review_runtime.reviewer_runtime_root(tmp_path), "review-large-tree", packet ) @@ -789,8 +822,15 @@ def native_process(_workspace, _argv, request): request = json.loads(captured["request"]) supplied = {item["locator"] for item in request["evidence"]} assert len(captured["request"]) <= reviewer_workspace.NATIVE_REVIEW_REQUEST_MAX_CHARS - assert "source:source.txt" in supplied + assert "source:source.txt" not in supplied assert "source:unchanged-large.txt" not in supplied + assert "control:.work-bundle/runtime/integrated-source.diff" in supplied + supplied_diff = next( + item["content"] + for item in request["evidence"] + if item["locator"] == "control:.work-bundle/runtime/integrated-source.diff" + ) + assert supplied_diff.encode("utf-8") == exact_diff.read_bytes() assert request["review_input"]["target_identity"]["source_tree"] == tree assert any( item["locator"] == "source:unchanged-large.txt" From ccaa1a452d67962bb815bc3eb9e623de5072ed53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=88=98=E5=8D=87=E9=BE=99?= Date: Wed, 9 Sep 2026 11:55:18 +0800 Subject: [PATCH 09/18] fix(review): separate product evidence from controller state --- scripts/orchestration/review_runtime.py | 31 +++- scripts/work-bundle/reviewer_workspace.py | 158 +++++++++++++++++- tests/test_native_review_integration.py | 191 +++++++++++++++++----- tests/test_orchestration_reviews.py | 70 +++++++- 4 files changed, 404 insertions(+), 46 deletions(-) diff --git a/scripts/orchestration/review_runtime.py b/scripts/orchestration/review_runtime.py index 7616f45..ef2f825 100644 --- a/scripts/orchestration/review_runtime.py +++ b/scripts/orchestration/review_runtime.py @@ -535,6 +535,14 @@ def _validate_native_run_proof(receipt, packet, result, path, immutable_file, ca stderr = immutable_file(path.with_suffix(".stderr.txt")) request_bytes = immutable_file(path.with_suffix(".request.json")) request = json.loads(request_bytes) + controller_path = path.with_suffix(".controller.json") + controller_evidence = ( + json.loads(immutable_file(controller_path)) if controller_path.exists() else None + ) + combined_evidence = [ + *request.get("evidence", []), + *(controller_evidence or []), + ] launch = json.loads(immutable_file(path.with_suffix(".launch.json"))) argv = launch["argv"] host_id, worker = runtime.parse_native_reviewer_transcript(stdout.decode(), stderr.decode()) @@ -548,17 +556,36 @@ def _validate_native_run_proof(receipt, packet, result, path, immutable_file, ca or not Path(argv[0]).is_absolute() or argv != runtime._native_reviewer_argv(Path(argv[0]), Path(argv[9]), argv[11]) or set(request) != {"instructions", "review_input", "evidence"} - or request["review_input"] != runtime._native_review_input(packet) or not isinstance(request["instructions"], str) + or request["review_input"] != runtime._native_review_input( + packet, combined_evidence if controller_evidence is not None else None + ) or not isinstance(request["instructions"], str) or not request["instructions"].strip()): raise ValueError("native launch/input mismatch") evidence = request["evidence"] - artifacts = runtime._native_review_artifacts(packet, evidence) + artifacts = runtime._native_review_artifacts(packet, combined_evidence) if len(evidence) != len(artifacts): raise ValueError("native input evidence mismatch") for expected, actual in zip(artifacts, evidence): if (set(actual) != {*expected, "content"} or any(actual[key] != value for key, value in expected.items()) or hashlib.sha256(actual["content"].encode()).hexdigest() != expected["sha256"]): raise ValueError("native input evidence mismatch") + if controller_evidence is not None: + controller_locators = runtime._controller_only_artifact_locators( + packet, combined_evidence + ) + controller_artifacts = [ + item for item in packet["artifacts"] + if item.get("locator") in controller_locators + ] + if len(controller_evidence) != len(controller_artifacts): + raise ValueError("native controller evidence mismatch") + for expected, actual in zip(controller_artifacts, controller_evidence): + if ( + set(actual) != {*expected, "content"} + or any(actual[key] != value for key, value in expected.items()) + or hashlib.sha256(actual["content"].encode()).hexdigest() != expected["sha256"] + ): + raise ValueError("native controller evidence mismatch") key = "task_review_context" if result.get("review_target_kind", "stage") == "task" else "stage_review_context" context = {**packet[key], "agent_id": host_id, "execution_id": host_id} compact = key == "task_review_context" or (context.get("stage") == "integrated_implementation" and "task_review" in worker) diff --git a/scripts/work-bundle/reviewer_workspace.py b/scripts/work-bundle/reviewer_workspace.py index 1ee141e..1664efd 100644 --- a/scripts/work-bundle/reviewer_workspace.py +++ b/scripts/work-bundle/reviewer_workspace.py @@ -113,6 +113,110 @@ def _integrated_change_manifest( return candidates[0] if candidates else None +def _stage_evidence_roles(packet: dict[str, object]) -> dict[str, str]: + manifest = packet.get("stage_evidence_manifest") + entries = manifest.get("entries") if isinstance(manifest, dict) else [] + return { + str(item.get("locator") or ""): str(item.get("role") or "") + for item in entries + if isinstance(item, dict) + } + + +def _controller_only_artifact_locators( + packet: dict[str, object], evidence: list[dict[str, object]], +) -> set[str]: + context = packet.get("stage_review_context") + if not isinstance(context, dict) or context.get("stage") != "integrated_implementation": + return set() + roles = _stage_evidence_roles(packet) + result: set[str] = set() + for item in evidence: + locator = str(item.get("locator") or "") + if roles.get(locator) in {"accepted_task_result", "validation_observation"}: + result.add(locator) + continue + if "/handoff/" in locator: + result.add(locator) + continue + content = item.get("content") + value = _structured_evidence(content) if isinstance(content, str) else None + schema = str(value.get("schema") or "") if isinstance(value, dict) else "" + if ( + isinstance(value, dict) + and ( + value.get("type") == "executor-result" + or "lifecycle" in schema + or {"execution_id", "ownership", "accepted_result"}.issubset(value) + ) + ): + result.add(locator) + return result + + +def _integrated_product_evidence( + packet: dict[str, object], evidence: list[dict[str, object]], +) -> dict[str, object]: + roles = _stage_evidence_roles(packet) + accepted_results: list[dict[str, object]] = [] + accepted_observation_ids: set[str] = set() + observation_stores: list[dict[str, object]] = [] + unresolved: list[object] = [] + for item in evidence: + locator = str(item.get("locator") or "") + content = item.get("content") + value = _structured_evidence(content) if isinstance(content, str) else None + if not isinstance(value, dict): + continue + if roles.get(locator) == "accepted_task_result": + accepted = value.get("accepted_result") + if not isinstance(accepted, dict): + continue + ids = [str(value) for value in accepted.get("validation_evidence_ids", [])] + accepted_observation_ids.update(ids) + accepted_results.append( + { + "task_id": accepted.get("task_id"), + "accepted_source": accepted.get("accepted_source"), + "validation_evidence_ids": ids, + "review_id": accepted.get("review_id"), + "invalidation": accepted.get("invalidation"), + } + ) + elif roles.get(locator) == "validation_observation": + observation_stores.append(value) + elif locator not in _controller_only_artifact_locators(packet, [item]): + unresolved.extend(value.get("unresolved", []) if isinstance(value.get("unresolved"), list) else []) + observations = [] + for store in observation_stores: + for item in store.get("observations", []): + if not isinstance(item, dict) or item.get("observation_id") not in accepted_observation_ids: + continue + result = item.get("result") if isinstance(item.get("result"), dict) else {} + observations.append( + { + "observation_id": item.get("observation_id"), + "product_tree": item.get("product_tree"), + "command_digest": item.get("command_digest"), + "oracle_digest": item.get("oracle_digest"), + "result": { + key: result.get(key) + for key in ( + "exit_code", "stdout_digest", "stderr_digest", + "started_at", "completed_at", + ) + }, + } + ) + return { + "accepted_results": sorted(accepted_results, key=lambda item: str(item.get("task_id") or "")), + "validation_observations": sorted( + observations, key=lambda item: str(item.get("observation_id") or "") + ), + "unresolved_product_concerns": unresolved, + } + + def _native_review_artifacts( packet: dict[str, object], evidence: list[dict[str, object]], ) -> list[dict[str, object]]: @@ -130,10 +234,12 @@ def _native_review_artifacts( if len(matching) != 1 or matching[0].get("sha256") != exact_diff["sha256"]: raise ReviewerWorkspaceError("WB_REVIEW_CHANGE_MANIFEST_INVALID") changed = set() + controller_only = _controller_only_artifact_locators(packet, evidence) return [ item for item in artifacts if isinstance(item, dict) + and item.get("locator") not in controller_only and ( not str(item.get("locator") or "").startswith("source:") or item.get("locator") in changed @@ -313,12 +419,28 @@ def _retain_native_diagnostics(runtime_root, run_id, review_id, argv, request_by return str(directory) -def _native_review_input(packet: dict[str, object]) -> dict[str, object]: +def _native_review_input( + packet: dict[str, object], evidence: list[dict[str, object]] | None = None, +) -> dict[str, object]: key = "task_review_context" if "task_review_context" in packet else "stage_review_context" context = packet[key] + artifacts = packet["artifacts"] + integrated = key == "stage_review_context" and context.get("stage") == "integrated_implementation" + if integrated and evidence is not None: + controller_only = _controller_only_artifact_locators(packet, evidence) + artifacts = [ + item + for item in artifacts + if isinstance(item, dict) and item.get("locator") not in controller_only + ] + return { + "target_identity": context["target_identity"], + "artifacts": artifacts, + "product_evidence": _integrated_product_evidence(packet, evidence), + } if key == "task_review_context" or context.get("stage") == "integrated_implementation": - return {"target_identity": context["target_identity"], "artifacts": packet["artifacts"]} - return {"stage": context["stage"], "target_identity": context["target_identity"], "artifacts": packet["artifacts"]} + return {"target_identity": context["target_identity"], "artifacts": artifacts} + return {"stage": context["stage"], "target_identity": context["target_identity"], "artifacts": artifacts} def run_native_reviewer(workspace: Path, executable: Path, *, model: str, review_instructions: str) -> dict[str, object]: @@ -350,6 +472,10 @@ def run_native_reviewer(workspace: Path, executable: Path, *, model: str, review if str(item.get("locator") or "").startswith("control:"): content = _evidence_path(workspace, item["locator"]).read_bytes().decode("utf-8") control_evidence.append({**item, "content": content}) + controller_locators = _controller_only_artifact_locators(packet, control_evidence) + controller_evidence = [ + item for item in control_evidence if item.get("locator") in controller_locators + ] selected_artifacts = _native_review_artifacts(packet, control_evidence) evidence = [] for item in selected_artifacts: @@ -359,14 +485,21 @@ def run_native_reviewer(workspace: Path, executable: Path, *, model: str, review if _sha256_bytes(content.encode("utf-8")) != item["sha256"]: raise ReviewerWorkspaceError("WB_REVIEW_EVIDENCE_MUTATED") evidence.append({**item, "content": content}) - request = {"instructions": review_instructions, "review_input": _native_review_input(packet), "evidence": evidence} + request = { + "instructions": review_instructions, + "review_input": _native_review_input(packet, control_evidence), + "evidence": evidence, + } if len(json.dumps(request, sort_keys=True, ensure_ascii=False)) > NATIVE_REVIEW_REQUEST_MAX_CHARS: raise ReviewerWorkspaceError( "WB_REVIEW_NATIVE_INPUT_TOO_LARGE", {"max_chars": NATIVE_REVIEW_REQUEST_MAX_CHARS}, ) argv = _native_reviewer_argv(executable, workspace, model) - return _run_reviewer(workspace, argv, native_request=request) + return _run_reviewer( + workspace, argv, native_request=request, + native_controller_evidence=controller_evidence, + ) def _review_runtime(): @@ -1187,7 +1320,13 @@ def run_sandboxed_reviewer(workspace: Path, argv: list[str]) -> dict[str, object return _run_reviewer(workspace, argv) -def _run_reviewer(workspace: Path, argv: list[str], *, native_request: dict[str, object] | None = None) -> dict[str, object]: +def _run_reviewer( + workspace: Path, + argv: list[str], + *, + native_request: dict[str, object] | None = None, + native_controller_evidence: list[dict[str, object]] | None = None, +) -> dict[str, object]: workspace = workspace.expanduser().resolve() runtime_root, review_id, state = _runtime_identity(workspace) packet, _ = _load_workspace(workspace) @@ -1335,6 +1474,13 @@ def _run_reviewer(workspace: Path, argv: list[str], *, native_request: dict[str, retained_items.extend([("request.json", request_bytes), ("stdout.jsonl", completed.stdout.encode()), ("stderr.txt", completed.stderr.encode()), ("launch.json", json.dumps({"argv": argv, "executable_sha256": executable_digest}, sort_keys=True).encode())]) + if native_controller_evidence is not None: + retained_items.append( + ( + "controller.json", + json.dumps(native_controller_evidence, sort_keys=True, ensure_ascii=False).encode(), + ) + ) else: retained_items.append(("profile.sb", (workspace / "sandbox.sb").read_bytes())) for suffix, content in retained_items: diff --git a/tests/test_native_review_integration.py b/tests/test_native_review_integration.py index fee2547..329ef4a 100644 --- a/tests/test_native_review_integration.py +++ b/tests/test_native_review_integration.py @@ -20,6 +20,8 @@ sys.path.insert(0, str(module_root)) import execution_context # noqa: E402 +import completion_provenance # noqa: E402 +import execution_workspace # noqa: E402 import review_runtime # noqa: E402 import reviewer_workspace # noqa: E402 @@ -31,6 +33,55 @@ def _git(root: Path, *arguments: str) -> str: return completed.stdout.strip() +def _persist_production_binding( + control: Path, + source: Path, + task: dict[str, object], + *, + execution_id: str, + baseline: dict[str, str], +) -> dict[str, object]: + plan_id = str(task["plan_id"]) + task_id = str(task["task_id"]) + workspace_id = "workspace-native" + repository_id = "repo-native" + runtime_root = control / "execution-workspaces" + registered = execution_workspace.register_existing( + source, + workspace_id=workspace_id, + execution_id=execution_id, + repository_id=repository_id, + created_for=task_id, + owner="harness", + runtime_root=runtime_root, + ) + ownership = completion_provenance.execution_binding_ownership( + control / ".work-bundle/runtime/completion-provenance", + binding_id=f"binding:{plan_id}:{task_id}", + target_kind="git_backed", + owner=task_id, + ) + binding = { + "plan_id": plan_id, + "task_id": task_id, + "workspace_id": workspace_id, + "execution_id": execution_id, + "repository_id": repository_id, + "runtime_root": str(runtime_root), + "execution_path": str(source.resolve()), + "control_root": str(control.resolve()), + "state_path": registered["state_path"], + "git_identity": registered["git_identity"], + "write_scope": list(task.get("files", {}).get("write", [])), + "forbidden_scope": list(task.get("files", {}).get("forbidden", [])), + "ownership": ownership, + "mutating": True, + "baseline": baseline, + } + execution_context._persist_binding(binding, control) + return execution_context.load_task_execution_binding(control, plan_id, task_id) + + def _native_events(result: dict[str, object]) -> str: events = [ { @@ -287,6 +338,82 @@ def unexpected_dispatch(*_args): assert dispatched is False +def test_incorrect_executor_result_cannot_reach_acceptance_by_matching_review_shape( + tmp_path: Path, +) -> None: + source = tmp_path / "source" + control = tmp_path / "control" + source.mkdir() + control.mkdir() + (source / "product.py").write_text("VALUE = 0\n", encoding="utf-8") + _git(source, "init", "-q") + _git(source, "add", "product.py") + _git( + source, + "-c", + "user.name=Test", + "-c", + "user.email=test@example.invalid", + "commit", + "-qm", + "incorrect executor output", + ) + head = _git(source, "rev-parse", "HEAD") + tree = _git(source, "rev-parse", "HEAD^{tree}") + task = { + "plan_id": "plan-negative", + "task_id": "task-negative", + "source_ids": ["REQ-VALUE"], + "goal": "Produce the required product value", + "requirements": ["product.py must define VALUE with the integer value 1."], + "constraints": [], + "truth_basis": {"decision_authority": ["REQ-VALUE is authoritative."]}, + "files": {"read": ["product.py"], "write": ["product.py"], "forbidden": []}, + "validation": [], + "review_required": True, + "workspace": {"root": str(control)}, + } + _persist_production_binding( + control, + source, + task, + execution_id="executor-negative", + baseline={"head": head, "tree": tree}, + ) + handoff = { + "type": "executor-result", + "related": {"plan": "plan-negative", "task": "task-negative"}, + "result": {"state": "completed", "summary": "Produced the requested value."}, + "changes": {"files": [{"path": "outside-scope.py", "change": "created"}]}, + "task_fit_check": {"task": "task-negative", "result": "clean"}, + "knowledge_disposition": { + "action": "none", "reason": "No durable authority changed.", "affected_authority": [], + }, + "acceptance_review": {"required": True, "verdict": "pending"}, + "delegation_evidence": { + "delegated": True, + "owner_kind": "subagent", + "agent_id": "executor-negative", + "run_id": "executor-negative", + "mechanism": "host-native", + }, + "validation": {"commands": []}, + } + + with pytest.raises(SystemExit, match="outside task write scope"): + execution_context.validate_executor_result_for_task( + handoff, + task, + observe=True, + mutation_events=[{"actor_kind": "subagent", "paths": ["outside-scope.py"]}], + preparing_review=True, + ) + stored = execution_context.load_task_execution_binding( + control, "plan-negative", "task-negative" + ) + assert "accepted_result" not in stored + + @pytest.mark.skipif( os.environ.get("WB_NATIVE_REVIEW_INTEGRATION") != "1", reason="set WB_NATIVE_REVIEW_INTEGRATION=1 for the genuine native host observation", @@ -401,29 +528,24 @@ def test_live_plugin_absent_native_execution_review_publication_and_acceptance( "plan_id": "plan-native-live", "task_id": "task-native-live", "source_ids": ["REQ-NATIVE"], - "truth_basis": {"decision_authority": []}, + "goal": "Create the requested product constant", + "requirements": ["product.py must define VALUE with the integer value 1."], + "constraints": ["The implementation must remain within product.py."], + "truth_basis": { + "decision_authority": ["The requested VALUE behavior is authoritative."], + }, "files": {"read": ["product.py"], "write": ["product.py"], "forbidden": []}, "validation": [], "review_required": True, "workspace": {"root": str(control)}, } - binding = { - "plan_id": "plan-native-live", - "task_id": "task-native-live", - "workspace_id": "workspace-native-live", - "execution_id": executor_ids[0], - "repository_id": "repo-native-live", - "execution_path": str(source), - "control_root": str(control), - "git_identity": {"branch_ref": "refs/heads/main"}, - "baseline": {"head": baseline_head, "tree": baseline_tree}, - "ownership": { - "binding_id": "binding:plan-native-live:task-native-live", - "state": "active", - "current_owner": "task-native-live", - "history": [{"event": "created"}], - }, - } + binding = _persist_production_binding( + control, + source, + task, + execution_id=executor_ids[0], + baseline={"head": baseline_head, "tree": baseline_tree}, + ) handoff = { "type": "executor-result", "related": {"plan": "plan-native-live", "task": "task-native-live"}, @@ -445,12 +567,13 @@ def test_live_plugin_absent_native_execution_review_publication_and_acceptance( }, "validation": {"commands": []}, } - validated = { - "result_state": "completed", - "knowledge_disposition": handoff["knowledge_disposition"], - "task_ownership": handoff["delegation_evidence"], - "observed_validation": [], - } + validated = execution_context.validate_executor_result_for_task( + handoff, + task, + observe=True, + mutation_events=[{"actor_kind": "subagent", "paths": ["product.py"]}], + preparing_review=True, + ) target_identity = { "artifact_id": "task-native-live", "revision": head, @@ -470,11 +593,13 @@ def test_live_plugin_absent_native_execution_review_publication_and_acceptance( } protected = control / ".protected" protected.mkdir() + authority = control / "task-authority.json" + authority.write_text(json.dumps(task, sort_keys=True), encoding="utf-8") packet = reviewer_workspace.build_direct_evidence_packet( source_root=source, control_root=control, protected_roots=[protected], - artifacts=["source:product.py"], + artifacts=["source:product.py", "control:task-authority.json"], search_roots=[], validators=[], sentinels=[], @@ -489,9 +614,10 @@ def test_live_plugin_absent_native_execution_review_publication_and_acceptance( executable, model=model, review_instructions=( - "Independently review the supplied task source. Return only a final JSON object " - f'with exactly this shape: {{"task_review":{{"reviewed_head":"{head}",' - '"verdict":"accept","findings":[]}}}. Accept only if the evidence satisfies the task.' + "Independently judge the supplied task source against task-authority.json. Return a " + "task_review JSON object for the supplied reviewed_head. Set verdict to accept only " + "when every requirement is satisfied; otherwise set it to repair and report concrete " + "findings using the required review contract." ), ) review = {**receipt["review_result"], "reviewer_run": receipt["reviewer_run"]} @@ -503,15 +629,6 @@ def test_live_plugin_absent_native_execution_review_publication_and_acceptance( control, review, current_target_identity=target_identity ) == reference - monkeypatch.setattr( - execution_context, "load_task_execution_binding", lambda *_args: binding - ) - - def persist(value, _root): - binding.clear() - binding.update(value) - - monkeypatch.setattr(execution_context, "_persist_binding", persist) accepted = execution_context.materialize_accepted_task_review( control, task, diff --git a/tests/test_orchestration_reviews.py b/tests/test_orchestration_reviews.py index 6e0487d..f4d4049 100644 --- a/tests/test_orchestration_reviews.py +++ b/tests/test_orchestration_reviews.py @@ -716,7 +716,49 @@ def test_native_integrated_review_bounds_large_unchanged_tree_to_exact_change_ma tree = subprocess.check_output( ["git", "-C", str(tmp_path), "rev-parse", "HEAD^{tree}"], text=True ).strip() - _write_compact_accepted_result(tmp_path, task=task) + binding = _write_compact_accepted_result(tmp_path, task=task) + provenance = tmp_path / ".work-bundle/runtime/completion-provenance/completion-provenance-v1.json" + provenance.parent.mkdir(parents=True, exist_ok=True) + provenance.write_text( + json.dumps( + { + "schema": "completion-provenance-v1", + "observations": [ + { + "observation_id": "observation-val-1", + "product_tree": tree, + "command_digest": "1" * 64, + "oracle_digest": "2" * 64, + "result": { + "exit_code": 0, + "stdout_digest": "3" * 64, + "stderr_digest": "4" * 64, + "started_at": "2026-09-09T00:00:00Z", + "completed_at": "2026-09-09T00:00:01Z", + }, + "controller_history": "relevant-raw-history-must-not-reach-model", + }, + { + "observation_id": "observation-unrelated", + "result": {"exit_code": 1}, + "controller_history": "unrelated-history-must-not-reach-model", + }, + ], + } + ), + encoding="utf-8", + ) + handoff = tmp_path / ".work-bundle/orchestration/handoff/executor/active/history.json" + handoff.parent.mkdir(parents=True, exist_ok=True) + handoff.write_text( + json.dumps({"type": "executor-result", "internal": "handoff-must-not-reach-model"}), + encoding="utf-8", + ) + lifecycle = tmp_path / ".work-bundle/runtime/task-lifecycle.json" + lifecycle.write_text( + json.dumps({"schema": "task-lifecycle-v1", "internal": "lifecycle-must-not-reach-model"}), + encoding="utf-8", + ) exact_diff = tmp_path / ".work-bundle/runtime/integrated-source.diff" exact_diff.parent.mkdir(parents=True, exist_ok=True) exact_diff.write_bytes( @@ -758,6 +800,8 @@ def test_native_integrated_review_bounds_large_unchanged_tree_to_exact_change_ma ) required["control:" + change_manifest.relative_to(tmp_path).as_posix()] = "change_manifest" required["control:" + exact_diff.relative_to(tmp_path).as_posix()] = "exact_diff" + required["control:" + handoff.relative_to(tmp_path).as_posix()] = "handoff" + required["control:" + lifecycle.relative_to(tmp_path).as_posix()] = "lifecycle" packet = reviewer_workspace.build_direct_evidence_packet( source_root=tmp_path, control_root=tmp_path, @@ -821,7 +865,15 @@ def native_process(_workspace, _argv, request): ) request = json.loads(captured["request"]) supplied = {item["locator"] for item in request["evidence"]} + metadata = {item["locator"] for item in request["review_input"]["artifacts"]} + product_authority = { + item["locator"] + for item in packet["stage_evidence_manifest"]["entries"] + if item["role"] in {"target", "plan_member", "verified_specification"} + } assert len(captured["request"]) <= reviewer_workspace.NATIVE_REVIEW_REQUEST_MAX_CHARS + assert product_authority + assert product_authority <= supplied assert "source:source.txt" not in supplied assert "source:unchanged-large.txt" not in supplied assert "control:.work-bundle/runtime/integrated-source.diff" in supplied @@ -831,6 +883,22 @@ def native_process(_workspace, _argv, request): if item["locator"] == "control:.work-bundle/runtime/integrated-source.diff" ) assert supplied_diff.encode("utf-8") == exact_diff.read_bytes() + binding_locator = "control:" + binding.relative_to(tmp_path).as_posix() + provenance_locator = "control:" + provenance.relative_to(tmp_path).as_posix() + assert binding_locator not in supplied | metadata + assert provenance_locator not in supplied | metadata + assert "control:" + handoff.relative_to(tmp_path).as_posix() not in supplied | metadata + assert "control:" + lifecycle.relative_to(tmp_path).as_posix() not in supplied | metadata + encoded_request = captured["request"] + assert "relevant-raw-history-must-not-reach-model" not in encoded_request + assert "unrelated-history-must-not-reach-model" not in encoded_request + assert "handoff-must-not-reach-model" not in encoded_request + assert "lifecycle-must-not-reach-model" not in encoded_request + product_evidence = request["review_input"]["product_evidence"] + assert [item["task_id"] for item in product_evidence["accepted_results"]] == ["task-test"] + assert [ + item["observation_id"] for item in product_evidence["validation_observations"] + ] == ["observation-val-1"] assert request["review_input"]["target_identity"]["source_tree"] == tree assert any( item["locator"] == "source:unchanged-large.txt" From b92ed61a932055ca8330beaba2080b73cbd5be44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=88=98=E5=8D=87=E9=BE=99?= Date: Wed, 9 Sep 2026 12:19:06 +0800 Subject: [PATCH 10/18] fix(review): preserve repair and validation continuity --- scripts/orchestration/review_runtime.py | 12 +- scripts/work-bundle/reviewer_workspace.py | 86 +++-- tests/test_native_review_integration.py | 368 +++++++++++++++++----- tests/test_reviewer_workspace.py | 123 ++++++++ 4 files changed, 488 insertions(+), 101 deletions(-) diff --git a/scripts/orchestration/review_runtime.py b/scripts/orchestration/review_runtime.py index ef2f825..fcf5c7b 100644 --- a/scripts/orchestration/review_runtime.py +++ b/scripts/orchestration/review_runtime.py @@ -597,7 +597,8 @@ def _validate_native_run_proof(receipt, packet, result, path, immutable_file, ca else: observed = runtime._stage_product_judgment_review( worker, review_id=receipt["review_id"], context=context, packet=packet, - started_at=receipt["started_at"], completed_at=receipt["completed_at"]) + started_at=receipt["started_at"], completed_at=receipt["completed_at"], + previous_review=result.get("previous_review")) if observed != result or receipt.get("review_result") != result or receipt.get(key) != context: raise ValueError("native judgment/result mismatch") return context @@ -1271,21 +1272,22 @@ def _validated_review_envelope(value: Mapping[str, Any]) -> StageReviewV1: if value.get("review_target_kind") == "task": return validate_task_acceptance_review(value) - current = validate_stage_review(value) previous = value.get("previous_review") + envelope = {key: item for key, item in value.items() if key != "previous_review"} + current = validate_stage_review(envelope) if current.review_mode == "repair": if not isinstance(previous, Mapping) or "previous_review" in previous: raise ReviewContractError("stage repair review requires exactly one previous_review") - return validate_review_sequence(value, previous_review=previous) + return validate_review_sequence(envelope, previous_review=previous) if current.review_reset is not None: if not isinstance(previous, Mapping) or "previous_review" in previous: raise ReviewContractError("stage reset review requires exactly one previous_review") return validate_review_sequence( - value, + envelope, previous_review=previous, material_change=str(current.review_reset["reason_class"]), ) - return validate_review_sequence(value) + return validate_review_sequence(envelope) def _review_store_path(root: Path, review_id: str) -> Path: diff --git a/scripts/work-bundle/reviewer_workspace.py b/scripts/work-bundle/reviewer_workspace.py index 1664efd..50290b4 100644 --- a/scripts/work-bundle/reviewer_workspace.py +++ b/scripts/work-bundle/reviewer_workspace.py @@ -433,14 +433,24 @@ def _native_review_input( for item in artifacts if isinstance(item, dict) and item.get("locator") not in controller_only ] - return { + result = { "target_identity": context["target_identity"], "artifacts": artifacts, "product_evidence": _integrated_product_evidence(packet, evidence), } + if context.get("repair_frontier") is not None: + result["repair_frontier"] = context["repair_frontier"] + return result if key == "task_review_context" or context.get("stage") == "integrated_implementation": return {"target_identity": context["target_identity"], "artifacts": artifacts} - return {"stage": context["stage"], "target_identity": context["target_identity"], "artifacts": artifacts} + result = { + "stage": context["stage"], + "target_identity": context["target_identity"], + "artifacts": artifacts, + } + if context.get("repair_frontier") is not None: + result["repair_frontier"] = context["repair_frontier"] + return result def run_native_reviewer(workspace: Path, executable: Path, *, model: str, review_instructions: str) -> dict[str, object]: @@ -524,10 +534,17 @@ def _review_runtime(): return module -def _validate_stage_context(context: object) -> dict[str, object]: +def _validate_stage_context( + context: object, *, require_re_review_predecessor: bool = False, +) -> dict[str, object]: fields = {"stage", "target_identity", "target_locator", "agent_id", "capability", "execution_id", "evidence_mode"} repair_fields = {"review_mode", "review_target_kind", "repair_frontier", "review_reset"} - if not isinstance(context, dict) or frozenset(context) not in {frozenset(fields), frozenset(fields | repair_fields)}: + allowed = { + frozenset(fields), + frozenset(fields | repair_fields), + frozenset(fields | repair_fields | {"previous_review"}), + } + if not isinstance(context, dict) or frozenset(context) not in allowed: raise ReviewerWorkspaceError("WB_REVIEW_STAGE_CONTEXT_INVALID") if (context["stage"] not in {"specification", "plan", "integrated_implementation"} or context["capability"] not in {"standard", "judgment"} @@ -545,6 +562,15 @@ def _validate_stage_context(context: object) -> dict[str, object]: raise ValueError("repair reset") elif context["repair_frontier"] is not None: raise ValueError("initial frontier") + re_review = mode == "repair" or context["review_reset"] is not None + if "previous_review" in context: + previous = context.get("previous_review") + if not isinstance(previous, dict) or "previous_review" in previous: + raise ValueError("re-review predecessor") + if require_re_review_predecessor and re_review and "previous_review" not in context: + raise ValueError("missing re-review predecessor") + if not re_review and "previous_review" in context: + raise ValueError("unexpected predecessor") except (ValueError, TypeError): raise ReviewerWorkspaceError("WB_REVIEW_STAGE_CONTEXT_INVALID") from None return context @@ -755,7 +781,11 @@ def build_direct_evidence_packet( raise ReviewerWorkspaceError("WB_REVIEW_CONTEXT_AMBIGUOUS") stage_fields = {} if stage_review_context is not None: - context = dict(_validate_stage_context(stage_review_context)) + context = dict( + _validate_stage_context( + stage_review_context, require_re_review_predecessor=True + ) + ) manifest = _review_runtime().stage_evidence_manifest(control_root, source_root, context, records) # This runtime denies live source access. Only a complete frozen closure # is a reproducible snapshot; a caller's direct-source label grants nothing. @@ -803,11 +833,12 @@ def _public_packet(packet: dict[str, object]) -> dict[str, object]: if isinstance(item, dict) ], } - context = result.get("task_review_context") - if isinstance(context, dict) and "previous_review" in context: - result["task_review_context"] = { - key: value for key, value in context.items() if key != "previous_review" - } + for context_key in ("stage_review_context", "task_review_context"): + context = result.get(context_key) + if isinstance(context, dict) and "previous_review" in context: + result[context_key] = { + key: value for key, value in context.items() if key != "previous_review" + } return result @@ -1005,6 +1036,14 @@ def create_reviewer_workspace( sentinel_digest = _sentinel_digest( effective_source, effective_control, effective_protected, list(public_packet.get("sentinels", [])) ) + previous_review_state = {} + for context_key, state_key in ( + ("task_review_context", "task_review_previous_review"), + ("stage_review_context", "stage_review_previous_review"), + ): + context = packet.get(context_key) + if isinstance(context, dict) and "previous_review" in context: + previous_review_state[state_key] = context["previous_review"] state = { "schema": "reviewer-workspace-state-v1", "owner": "work-bundle", @@ -1021,9 +1060,7 @@ def create_reviewer_workspace( effective_source, effective_control, effective_protected ), "status": "active", - **({"task_review_previous_review": packet["task_review_context"]["previous_review"]} - if isinstance(packet.get("task_review_context"), dict) - and "previous_review" in packet["task_review_context"] else {}), + **previous_review_state, } state_path.parent.mkdir(parents=True, exist_ok=True) state_path.write_text(json.dumps(state, indent=2, sort_keys=True) + "\n", encoding="utf-8") @@ -1290,7 +1327,10 @@ def _task_product_judgment_review( return result -def _stage_product_judgment_review(judgment, *, review_id, context, packet, started_at, completed_at): +def _stage_product_judgment_review( + judgment, *, review_id, context, packet, started_at, completed_at, + previous_review=None, +): """Compose native stage authority without asking the reviewer to invent it.""" if not isinstance(judgment, dict) or set(judgment) != {"stage_review"}: raise ReviewerWorkspaceError("WB_REVIEW_STAGE_OUTPUT_INVALID") @@ -1299,7 +1339,7 @@ def _stage_product_judgment_review(judgment, *, review_id, context, packet, star or product["target_identity"] != context["target_identity"] or product["verdict"] not in TERMINAL_VERDICTS or not isinstance(product["findings"], list)): raise ReviewerWorkspaceError("WB_REVIEW_STAGE_OUTPUT_INVALID") - return { + result = { "review_id": review_id, "stage": context["stage"], "target_identity": context["target_identity"], "review_mode": context.get("review_mode", "initial"), "review_target_kind": "stage", "repair_frontier": context.get("repair_frontier"), "review_reset": context.get("review_reset"), @@ -1313,6 +1353,11 @@ def _stage_product_judgment_review(judgment, *, review_id, context, packet, star "started_at": started_at, "completed_at": completed_at, "staleness": {"is_stale": False, "reason": None, "supersedes": None}, } + if context.get("review_mode", "initial") == "repair" or context.get("review_reset") is not None: + if not isinstance(previous_review, dict): + raise ReviewerWorkspaceError("WB_REVIEW_STAGE_CONTROL_INPUT_INVALID") + result["previous_review"] = previous_review + return result def run_sandboxed_reviewer(workspace: Path, argv: list[str]) -> dict[str, object]: @@ -1414,17 +1459,22 @@ def _run_reviewer( _task_product_judgment_review( worker_output, review_id=review_id, context=context, packet=packet, started_at=started_at, completed_at=receipt["completed_at"], - previous_review=state.get("task_review_previous_review"), + previous_review=state.get( + "task_review_previous_review" + if context_key == "task_review_context" + else "stage_review_previous_review" + ), integrated_stage=compact_integrated, ) if context_key == "task_review_context" or compact_integrated else (_stage_product_judgment_review( worker_output, review_id=review_id, context=context, packet=packet, - started_at=started_at, completed_at=receipt["completed_at"]) + started_at=started_at, completed_at=receipt["completed_at"], + previous_review=state.get("stage_review_previous_review")) if native else worker_output) ) validated = ( - _review_runtime().validate_stage_review(review) + _review_runtime()._validated_review_envelope(review) if context_key == "stage_review_context" else _review_runtime().validate_task_acceptance_review(review) ) diff --git a/tests/test_native_review_integration.py b/tests/test_native_review_integration.py index 329ef4a..4c9bb49 100644 --- a/tests/test_native_review_integration.py +++ b/tests/test_native_review_integration.py @@ -5,11 +5,13 @@ import json import os from pathlib import Path +import shlex import shutil import subprocess import sys import pytest +from reviewer_run_fixtures import bind_review_receipt REPO_ROOT = Path(__file__).resolve().parents[1] @@ -82,6 +84,124 @@ def _persist_production_binding( return execution_context.load_task_execution_binding(control, plan_id, task_id) +def _establish_reviewed_plan_authority(control: Path, plan_id: str) -> None: + orchestration = control / ".work-bundle/orchestration" + metadata = control / ".work-bundle/project.yaml" + metadata.parent.mkdir(parents=True, exist_ok=True) + metadata.write_text( + f"metadata_version: 3\nworkspace_root: {control}\nworkspace_mode: single-repository\n", + encoding="utf-8", + ) + specification = orchestration / "spec/active/spec.md" + plan = orchestration / "plan/active/plan.md" + specification.parent.mkdir(parents=True, exist_ok=True) + plan.parent.mkdir(parents=True, exist_ok=True) + specification.write_text( + "---\nid: spec-native\nstatus: verified\n" + "requirements: [{id: REQ-NATIVE, requirement: product.py defines VALUE as 1.}]\n" + "---\n- **REQ-NATIVE**: product.py defines VALUE as 1.\n", + encoding="utf-8", + ) + plan.write_text( + f"---\nid: {plan_id}\nstatus: Planned\nsource_spec: [spec-native]\n---\nNative test plan.\n", + encoding="utf-8", + ) + reviews = orchestration / "reviews" + reviews.mkdir(parents=True, exist_ok=True) + for stage, identity in ( + ("specification", review_runtime.artifact_review_identity(specification)), + ("plan", review_runtime.plan_review_identity(control, plan)), + ): + record = { + "review_id": f"review-{stage}-{plan_id}", + "stage": stage, + "target_identity": identity, + "review_mode": "initial", + "review_target_kind": "stage", + "repair_frontier": None, + "review_reset": None, + "reviewer": { + "agent_id": f"reviewer-{stage}", "capability": "judgment", + "authorship": "none", "repair_participation": "none", + "decision_participation": "none", "deliberation_participation": "none", + "context_origin": "direct_source", + }, + "evidence": { + "mode": "direct", "capabilities": ["source inspection"], + "unavailable_evidence": [], "commands": [], "artifacts": [], + }, + "verdict": "accepted", "findings": [], + "started_at": "2026-09-09T00:00:00Z", + "completed_at": "2026-09-09T00:01:00Z", + "staleness": {"is_stale": False, "reason": None, "supersedes": None}, + } + (reviews / f"{stage}.json").write_text( + json.dumps(bind_review_receipt(control, record)), encoding="utf-8" + ) + + +def _harness_validation(control: Path, source: Path) -> tuple[dict[str, object], Path]: + counter = control / "harness-validation-count.txt" + program = control / "validate-product.py" + program.write_text( + "from pathlib import Path\n" + "import sys\n" + "counter = Path(sys.argv[2])\n" + "count = int(counter.read_text()) if counter.exists() else 0\n" + "counter.write_text(str(count + 1))\n" + "namespace = {}\n" + "exec(Path(sys.argv[1]).read_text(), namespace)\n" + "raise SystemExit(0 if namespace.get('VALUE') == 1 else 1)\n", + encoding="utf-8", + ) + command = shlex.join( + [sys.executable, str(program), str(source / "product.py"), str(counter)] + ) + return { + "id": "VAL-NATIVE", + "kind": "process", + "command": command, + "invariant_ids": ["INV-NATIVE"], + "capability_reason": "The harness process directly checks the required product value.", + "proves": "REQ-NATIVE", + "expected": "passed", + "evidence_reuse": {"mode": "deterministic", "max_age_seconds": 3600}, + }, counter + + +def _validation_capability(task_id: str) -> dict[str, object]: + return { + "result": "mapped", + "reason": "The harness command directly falsifies an incorrect product value.", + "invariants": [{ + "id": "INV-NATIVE", + "source_ids": ["REQ-NATIVE"], + "invariant": "product.py defines VALUE as 1.", + "boundary": "product.py", + "oracle": "VAL-NATIVE", + "capability_reason": "The harness imports and checks the exact value.", + "freshness": "current_task_batch", + "task_id": task_id, + "evidence_ids": ["VAL-NATIVE"], + "closure_result": "pending", + }], + } + + +def _invocation_counts( + executor_events: list[dict[str, object]], receipt: dict[str, object], counter: Path, +) -> tuple[int, int, int]: + review_events = [ + json.loads(line) + for line in Path(str(receipt["receipt_path"])).with_suffix(".stdout.jsonl").read_text().splitlines() + ] + return ( + sum(event.get("type") == "thread.started" for event in executor_events), + sum(event.get("type") == "thread.started" for event in review_events), + int(counter.read_text(encoding="utf-8")), + ) + + def _native_events(result: dict[str, object]) -> str: events = [ { @@ -124,41 +244,30 @@ def test_plugin_absent_native_review_publishes_once_then_materializes_initial_ac ) head = _git(source, "rev-parse", "HEAD") tree = _git(source, "rev-parse", "HEAD^{tree}") + _establish_reviewed_plan_authority(control, "plan-native") + validation, validation_counter = _harness_validation(control, source) task = { "plan_id": "plan-native", "task_id": "task-native", "source_ids": ["REQ-NATIVE"], + "goal": "Create the requested product constant", + "requirements": ["product.py must define VALUE with the integer value 1."], + "constraints": ["The implementation must remain within product.py."], "truth_basis": {"decision_authority": []}, "files": {"read": ["product.py"], "write": ["product.py"], "forbidden": []}, - "validation": [ - { - "id": "VAL-NATIVE", - "kind": "process", - "command": "python -m pytest tests/product.py -q", - "invariant_ids": ["INV-NATIVE"], - } - ], + "validation": [validation], + "evidence_capability": _validation_capability("task-native"), "review_required": True, "workspace": {"root": str(control)}, } - binding = { - "plan_id": "plan-native", - "task_id": "task-native", - "workspace_id": "workspace-native", - "execution_id": "executor-run", - "repository_id": "repo-native", - "execution_path": str(source), - "control_root": str(control), - "git_identity": {"branch_ref": "refs/heads/main"}, - "baseline": {"head": head, "tree": tree}, - "ownership": { - "binding_id": "binding:plan-native:task-native", - "state": "active", - "current_owner": "task-native", - "history": [{"event": "created"}], - }, - } + _persist_production_binding( + control, + source, + task, + execution_id="executor-run", + baseline={"head": head, "tree": tree}, + ) original_handoff = { "type": "executor-result", "related": {"plan": "plan-native", "task": "task-native"}, @@ -178,27 +287,48 @@ def test_plugin_absent_native_review_publishes_once_then_materializes_initial_ac "run_id": "executor-run", "mechanism": "host-native", }, + "repository": [{ + "root": str(source.resolve()), + "target_kind": "git-backed", + "preflight_kind": "git-clean-worktree", + "baseline": "initial", + "status": "clean", + }], + "codegraph": [{ + "root": str(source.resolve()), + "applicable": False, + "up_to_date": False, + "reason": "no-index", + }], + "evidence_closure": { + "result": "passed", + "invariants": [{ + "id": "INV-NATIVE", + "boundary": "product.py", + "freshness": "current_task_batch", + "evidence_ids": ["VAL-NATIVE"], + "closure_result": "passed", + "repair_owner": None, + }], + }, "validation": { "commands": [ - { - "command": "python -m pytest tests/product.py -q", - "result": "passed", + { + "id": "VAL-NATIVE", + "command": validation["command"], + "invariant_ids": ["INV-NATIVE"], + "result": "passed", } ] }, } - validated = { - "result_state": "completed", - "knowledge_disposition": original_handoff["knowledge_disposition"], - "task_ownership": original_handoff["delegation_evidence"], - "observed_validation": [ - { - "id": "VAL-NATIVE", - "observation_id": "observation-native", - "result": "passed", - } - ], - } + validated = execution_context.validate_executor_result_for_task( + original_handoff, + task, + observe=True, + mutation_events=[{"actor_kind": "subagent", "paths": ["product.py"]}], + preparing_review=True, + ) handoff_before_review = deepcopy(original_handoff) target_identity = { "artifact_id": "task-native", @@ -219,11 +349,17 @@ def test_plugin_absent_native_review_publishes_once_then_materializes_initial_ac } protected = control / ".protected" protected.mkdir() + authority = control / "task-authority.json" + authority.write_text(json.dumps(task, sort_keys=True), encoding="utf-8") packet = reviewer_workspace.build_direct_evidence_packet( source_root=source, control_root=control, protected_roots=[protected], - artifacts=["source:product.py"], + artifacts=[ + "source:product.py", + "control:task-authority.json", + "control:.work-bundle/runtime/completion-provenance/completion-provenance-v1.json", + ], search_roots=[], validators=[], sentinels=[], @@ -244,42 +380,29 @@ def run_native(*_args): return subprocess.CompletedProcess([], 0, _native_events(judgment), "") monkeypatch.setattr(reviewer_workspace, "_run_native_process", run_native) - monkeypatch.setattr( - execution_context, - "load_task_execution_binding", - lambda *_args: binding, - ) - persisted: dict[str, object] = {} - - def persist(value, _root): - persisted.update(value) - binding.clear() - binding.update(value) - - monkeypatch.setattr( - execution_context, - "_persist_binding", - persist, - ) receipt = reviewer_workspace.run_native_reviewer( Path(str(created["workspace_path"])), Path(sys.executable), model="test-model", - review_instructions="Review the supplied task source and return a task judgment.", + review_instructions=( + "Judge the task authority, source, and harness-owned validation evidence. Return repair " + "for unmet requirements; otherwise return accept." + ), ) review = {**receipt["review_result"], "reviewer_run": receipt["reviewer_run"]} reference = review_runtime.publish_review( control, review, current_target_identity=target_identity ) + fixture_executor_events = [{"type": "thread.started", "thread_id": "executor-run"}] + counts_after_publication = _invocation_counts( + fixture_executor_events, receipt, validation_counter + ) assert review_runtime.publish_review( control, review, current_target_identity=target_identity ) == reference - - monkeypatch.setattr( - execution_context, - "_claim_bound_validation_observations", - lambda *_args, **_kwargs: pytest.fail("current validation must not be replayed"), + counts_after_publication_retry = _invocation_counts( + fixture_executor_events, receipt, validation_counter ) accepted = execution_context.materialize_accepted_task_review( control, @@ -294,13 +417,22 @@ def persist(value, _root): validated_executor_result=validated, ) _, consumed = execution_context.load_current_accepted_task_result(control, task) + counts_after_consumption = _invocation_counts( + fixture_executor_events, receipt, validation_counter + ) + stored = execution_context.load_task_execution_binding(control, "plan-native", "task-native") assert native_calls == 1 + assert counts_after_publication == counts_after_publication_retry == counts_after_consumption == ( + 1, 1, 1 + ) assert original_handoff == handoff_before_review - assert accepted == consumed == persisted["accepted_result"] + assert accepted == consumed == stored["accepted_result"] assert accepted["baseline_identity"] == {"head": head, "tree": tree} assert accepted["review_id"] == review["review_id"] - assert accepted["validation_evidence_ids"] == ["observation-native"] + assert accepted["validation_evidence_ids"] == [ + validated["observed_validation"][0]["observation_id"] + ] assert accepted["owner_identity"]["agent_id"] == "executor-agent" assert review["reviewer"]["agent_id"] != accepted["owner_identity"]["agent_id"] request = json.loads(Path(receipt["receipt_path"]).with_suffix(".request.json").read_text()) @@ -339,7 +471,7 @@ def unexpected_dispatch(*_args): def test_incorrect_executor_result_cannot_reach_acceptance_by_matching_review_shape( - tmp_path: Path, + tmp_path: Path, monkeypatch, ) -> None: source = tmp_path / "source" control = tmp_path / "control" @@ -360,16 +492,19 @@ def test_incorrect_executor_result_cannot_reach_acceptance_by_matching_review_sh ) head = _git(source, "rev-parse", "HEAD") tree = _git(source, "rev-parse", "HEAD^{tree}") + _establish_reviewed_plan_authority(control, "plan-negative") + validation, validation_counter = _harness_validation(control, source) task = { "plan_id": "plan-negative", "task_id": "task-negative", - "source_ids": ["REQ-VALUE"], + "source_ids": ["REQ-NATIVE"], "goal": "Produce the required product value", "requirements": ["product.py must define VALUE with the integer value 1."], "constraints": [], - "truth_basis": {"decision_authority": ["REQ-VALUE is authoritative."]}, + "truth_basis": {"decision_authority": ["REQ-NATIVE is authoritative."]}, "files": {"read": ["product.py"], "write": ["product.py"], "forbidden": []}, - "validation": [], + "validation": [validation], + "evidence_capability": _validation_capability("task-negative"), "review_required": True, "workspace": {"root": str(control)}, } @@ -384,7 +519,7 @@ def test_incorrect_executor_result_cannot_reach_acceptance_by_matching_review_sh "type": "executor-result", "related": {"plan": "plan-negative", "task": "task-negative"}, "result": {"state": "completed", "summary": "Produced the requested value."}, - "changes": {"files": [{"path": "outside-scope.py", "change": "created"}]}, + "changes": {"files": [{"path": "product.py", "change": "updated"}]}, "task_fit_check": {"task": "task-negative", "result": "clean"}, "knowledge_disposition": { "action": "none", "reason": "No durable authority changed.", "affected_authority": [], @@ -397,20 +532,49 @@ def test_incorrect_executor_result_cannot_reach_acceptance_by_matching_review_sh "run_id": "executor-negative", "mechanism": "host-native", }, - "validation": {"commands": []}, + "repository": [{ + "root": str(source.resolve()), "target_kind": "git-backed", + "preflight_kind": "git-clean-worktree", "baseline": "initial", "status": "clean", + }], + "codegraph": [{ + "root": str(source.resolve()), "applicable": False, + "up_to_date": False, "reason": "no-index", + }], + "evidence_closure": { + "result": "passed", + "invariants": [{ + "id": "INV-NATIVE", "boundary": "product.py", + "freshness": "current_task_batch", "evidence_ids": ["VAL-NATIVE"], + "closure_result": "passed", "repair_owner": None, + }], + }, + "validation": {"commands": [{ + "id": "VAL-NATIVE", "command": validation["command"], + "invariant_ids": ["INV-NATIVE"], "result": "passed", + }]}, } + review_calls = 0 + + def unexpected_review(*_args): + nonlocal review_calls + review_calls += 1 + raise AssertionError("failed harness validation must block review dispatch") - with pytest.raises(SystemExit, match="outside task write scope"): + monkeypatch.setattr(reviewer_workspace, "_run_native_process", unexpected_review) + + with pytest.raises(SystemExit, match="does not match observed failed"): execution_context.validate_executor_result_for_task( handoff, task, observe=True, - mutation_events=[{"actor_kind": "subagent", "paths": ["outside-scope.py"]}], + mutation_events=[{"actor_kind": "subagent", "paths": ["product.py"]}], preparing_review=True, ) stored = execution_context.load_task_execution_binding( control, "plan-negative", "task-negative" ) + assert validation_counter.read_text(encoding="utf-8") == "1" + assert review_calls == 0 assert "accepted_result" not in stored @@ -523,6 +687,8 @@ def test_live_plugin_absent_native_execution_review_publication_and_acceptance( ) head = _git(source, "rev-parse", "HEAD") tree = _git(source, "rev-parse", "HEAD^{tree}") + _establish_reviewed_plan_authority(control, "plan-native-live") + validation, validation_counter = _harness_validation(control, source) task = { "plan_id": "plan-native-live", @@ -535,11 +701,12 @@ def test_live_plugin_absent_native_execution_review_publication_and_acceptance( "decision_authority": ["The requested VALUE behavior is authoritative."], }, "files": {"read": ["product.py"], "write": ["product.py"], "forbidden": []}, - "validation": [], + "validation": [validation], + "evidence_capability": _validation_capability("task-native-live"), "review_required": True, "workspace": {"root": str(control)}, } - binding = _persist_production_binding( + _persist_production_binding( control, source, task, @@ -565,7 +732,36 @@ def test_live_plugin_absent_native_execution_review_publication_and_acceptance( "run_id": executor_ids[0], "mechanism": "host-native", }, - "validation": {"commands": []}, + "repository": [{ + "root": str(source.resolve()), + "target_kind": "git-backed", + "preflight_kind": "git-clean-worktree", + "baseline": "initial", + "status": "clean", + }], + "codegraph": [{ + "root": str(source.resolve()), + "applicable": False, + "up_to_date": False, + "reason": "no-index", + }], + "evidence_closure": { + "result": "passed", + "invariants": [{ + "id": "INV-NATIVE", + "boundary": "product.py", + "freshness": "current_task_batch", + "evidence_ids": ["VAL-NATIVE"], + "closure_result": "passed", + "repair_owner": None, + }], + }, + "validation": {"commands": [{ + "id": "VAL-NATIVE", + "command": validation["command"], + "invariant_ids": ["INV-NATIVE"], + "result": "passed", + }]}, } validated = execution_context.validate_executor_result_for_task( handoff, @@ -599,7 +795,11 @@ def test_live_plugin_absent_native_execution_review_publication_and_acceptance( source_root=source, control_root=control, protected_roots=[protected], - artifacts=["source:product.py", "control:task-authority.json"], + artifacts=[ + "source:product.py", + "control:task-authority.json", + "control:.work-bundle/runtime/completion-provenance/completion-provenance-v1.json", + ], search_roots=[], validators=[], sentinels=[], @@ -625,9 +825,15 @@ def test_live_plugin_absent_native_execution_review_publication_and_acceptance( reference = review_runtime.publish_review( control, review, current_target_identity=target_identity ) + counts_after_publication = _invocation_counts( + executor_events, receipt, validation_counter + ) assert review_runtime.publish_review( control, review, current_target_identity=target_identity ) == reference + counts_after_publication_retry = _invocation_counts( + executor_events, receipt, validation_counter + ) accepted = execution_context.materialize_accepted_task_review( control, @@ -642,6 +848,12 @@ def test_live_plugin_absent_native_execution_review_publication_and_acceptance( validated_executor_result=validated, ) _, consumed = execution_context.load_current_accepted_task_result(control, task) + counts_after_consumption = _invocation_counts( + executor_events, receipt, validation_counter + ) + assert counts_after_publication == counts_after_publication_retry == counts_after_consumption == ( + 1, 1, 1 + ) assert consumed == accepted assert accepted["review_id"] == review["review_id"] assert accepted["baseline_identity"] == { diff --git a/tests/test_reviewer_workspace.py b/tests/test_reviewer_workspace.py index 6e587b5..3c86491 100644 --- a/tests/test_reviewer_workspace.py +++ b/tests/test_reviewer_workspace.py @@ -297,6 +297,129 @@ def test_compact_integrated_product_judgment_gets_controller_owned_stage_envelop assert validated.verdict == "accepted" +def test_native_compact_integrated_repair_keeps_exact_predecessor_controller_only( + review_roots: tuple[Path, Path, Path], +) -> None: + source, control, _ = review_roots + runtime = reviewer_workspace._review_runtime() + subprocess.run(["git", "init", "-q", str(source)], check=True) + subprocess.run(["git", "-C", str(source), "add", "."], check=True) + subprocess.run( + [ + "git", "-C", str(source), "-c", "user.name=Test", + "-c", "user.email=test@example.invalid", "commit", "-qm", "fixture", + ], + check=True, + ) + tree = subprocess.check_output( + ["git", "-C", str(source), "rev-parse", "HEAD^{tree}"], text=True + ).strip() + specification = control / ".work-bundle" / "orchestration" / "spec" / "verified" / "spec.md" + specification.parent.mkdir(parents=True) + specification.write_text( + "---\nid: spec-repair\nstatus: verified\n---\n# Specification\n", + encoding="utf-8", + ) + target = control / ".work-bundle" / "orchestration" / "plan" / "active" / "plan.md" + target.parent.mkdir(parents=True) + target.write_text( + "---\nid: plan-repair\nstatus: active\nsource_spec: [spec-repair]\n---\n# Plan\n", + encoding="utf-8", + ) + current_identity = runtime.stage_target_identity( + control, "integrated_implementation", target, source_root=source + ) + previous_identity = {**current_identity, "source_tree": "a" * 40} + previous_context = { + "stage": "integrated_implementation", + "target_identity": previous_identity, + "target_locator": "control:.work-bundle/orchestration/plan/active/plan.md", + "agent_id": "reviewer-previous", + "capability": "judgment", + "execution_id": "reviewer-previous-run", + "evidence_mode": "reproducible_snapshot", + "review_mode": "initial", + "review_target_kind": "stage", + "repair_frontier": None, + "review_reset": None, + } + previous = reviewer_workspace._task_product_judgment_review( + {"task_review": { + "reviewed_head": previous_identity["source_tree"], + "verdict": "repair", + "findings": [{ + "finding_id": "finding-live-oracle", + "severity": "blocking", + "requirement_id": "AC-009", + "boundary": "tests/test_native_review_integration.py", + "evidence": "validation invocation is not observed", + "expected": "one harness-owned validation", + "observed": "zero harness-owned validations", + "owner": "task_owner", + }], + }}, + review_id="review-integrated-previous", + context=previous_context, + packet={"artifacts": []}, + started_at="2026-09-09T00:00:00Z", + completed_at="2026-09-09T00:01:00Z", + integrated_stage=True, + ) + frontier = { + "prior_review_id": previous["review_id"], + "blocking_finding_ids": ["finding-live-oracle"], + "previous_reviewed_identity": previous_identity, + "repaired_identity": current_identity, + "affected_boundaries": ["tests/test_native_review_integration.py"], + "frozen_evidence_reference": runtime.review_evidence_identity(previous), + } + context = { + **previous_context, + "target_identity": current_identity, + "agent_id": "reviewer-current", + "execution_id": "reviewer-current-run", + "review_mode": "repair", + "repair_frontier": frontier, + "previous_review": previous, + } + packet = build_direct_evidence_packet( + source_root=source, + control_root=control, + protected_roots=[control / "credentials"], + artifacts=["control:.work-bundle/orchestration/plan/active/plan.md"], + search_roots=[], validators=[], sentinels=[], network_state="denied", + stage_review_context=context, + ) + created = create_reviewer_workspace( + runtime.reviewer_runtime_root(control), "review-integrated-repair", packet + ) + judgment = {"task_review": { + "reviewed_head": current_identity["source_tree"], + "verdict": "accept", + "findings": [], + }} + with patch.object( + reviewer_workspace, + "_run_native_process", + return_value=subprocess.CompletedProcess([], 0, native_events(judgment), ""), + ): + receipt = reviewer_workspace.run_native_reviewer( + Path(str(created["workspace_path"])), Path(sys.executable), + model="test-model", review_instructions="Review only the repaired product frontier.", + ) + request = json.loads(Path(receipt["receipt_path"]).with_suffix(".request.json").read_text()) + assert "previous_review" not in request["review_input"] + assert request["review_input"]["repair_frontier"] == frontier + review = {**receipt["review_result"], "reviewer_run": receipt["reviewer_run"]} + assert review["previous_review"] == previous + assert review["repair_frontier"] == frontier + reference = runtime.publish_review(control, review, current_target_identity=current_identity) + stored, _ = runtime.load_stored_review( + control, reference, current_target_identity=current_identity + ) + assert stored["previous_review"] == previous + + def test_incomplete_stage_snapshot_fails_before_reviewer_process_launch( review_roots: tuple[Path, Path, Path] ) -> None: From d9121fd93a6ca7c0bc2e930a39adc53cf7d1ca2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=88=98=E5=8D=87=E9=BE=99?= Date: Wed, 9 Sep 2026 12:27:29 +0800 Subject: [PATCH 11/18] fix(review): consume current stage repair envelopes --- scripts/orchestration/review_runtime.py | 14 +++++++++----- tests/test_reviewer_workspace.py | 17 +++++++++++++++++ 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/scripts/orchestration/review_runtime.py b/scripts/orchestration/review_runtime.py index fcf5c7b..09f00aa 100644 --- a/scripts/orchestration/review_runtime.py +++ b/scripts/orchestration/review_runtime.py @@ -836,21 +836,25 @@ def _require_current_review(root: Path, stage: str, identity: Mapping[str, Any]) # valid replacement review for the actual current artifact. if value["stage"] != stage or value.get("target_identity") != identity: continue - record = validate_stage_review(value) + record = _validated_review_envelope(value) if record.review_mode == "repair": frontier = record.repair_frontier assert frontier is not None prior = historical.get(str(frontier["prior_review_id"])) if prior is None: raise ReviewContractError("repair review predecessor is missing") - record = validate_review_sequence(value, previous_review=prior) + if value.get("previous_review") != prior: + raise ReviewContractError( + "repair review does not carry the exact stored predecessor" + ) elif record.review_reset is not None: prior = historical.get(str(record.review_reset["prior_review_id"])) if prior is None: raise ReviewContractError("initial review reset predecessor is missing") - record = validate_review_sequence( - value, previous_review=prior, material_change=str(record.review_reset["reason_class"]) - ) + if value.get("previous_review") != prior: + raise ReviewContractError( + "initial review reset does not carry the exact stored predecessor" + ) if record.review_id in review_ids: raise ReviewContractError("stage review IDs must be globally unique") review_ids.add(record.review_id) diff --git a/tests/test_reviewer_workspace.py b/tests/test_reviewer_workspace.py index 3c86491..687d980 100644 --- a/tests/test_reviewer_workspace.py +++ b/tests/test_reviewer_workspace.py @@ -413,11 +413,28 @@ def test_native_compact_integrated_repair_keeps_exact_predecessor_controller_onl review = {**receipt["review_result"], "reviewer_run": receipt["reviewer_run"]} assert review["previous_review"] == previous assert review["repair_frontier"] == frontier + history = control / ".work-bundle" / "orchestration" / "reviews" + history.mkdir(parents=True, exist_ok=True) + (history / f"{previous['review_id']}.json").write_text( + json.dumps(previous), encoding="utf-8" + ) reference = runtime.publish_review(control, review, current_target_identity=current_identity) stored, _ = runtime.load_stored_review( control, reference, current_target_identity=current_identity ) assert stored["previous_review"] == previous + runtime._require_current_review(control, "integrated_implementation", current_identity) + + malformed = { + **review, + "review_id": "review-integrated-extra-history", + "previous_review": {**previous, "previous_review": previous}, + } + (history / "review-integrated-extra-history.json").write_text( + json.dumps(malformed), encoding="utf-8" + ) + with pytest.raises(SystemExit, match="exactly one previous_review"): + runtime._require_current_review(control, "integrated_implementation", current_identity) def test_incomplete_stage_snapshot_fails_before_reviewer_process_launch( From d592bc63bfc6e7437a1a422eae36c33ea64aaa22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=88=98=E5=8D=87=E9=BE=99?= Date: Wed, 9 Sep 2026 12:42:43 +0800 Subject: [PATCH 12/18] fix(review): validate bounded stage predecessor chains --- scripts/orchestration/review_runtime.py | 92 ++++++++++++++---- tests/test_reviewer_workspace.py | 119 +++++++++++++++++++++++- 2 files changed, 189 insertions(+), 22 deletions(-) diff --git a/scripts/orchestration/review_runtime.py b/scripts/orchestration/review_runtime.py index 09f00aa..77d9713 100644 --- a/scripts/orchestration/review_runtime.py +++ b/scripts/orchestration/review_runtime.py @@ -836,25 +836,7 @@ def _require_current_review(root: Path, stage: str, identity: Mapping[str, Any]) # valid replacement review for the actual current artifact. if value["stage"] != stage or value.get("target_identity") != identity: continue - record = _validated_review_envelope(value) - if record.review_mode == "repair": - frontier = record.repair_frontier - assert frontier is not None - prior = historical.get(str(frontier["prior_review_id"])) - if prior is None: - raise ReviewContractError("repair review predecessor is missing") - if value.get("previous_review") != prior: - raise ReviewContractError( - "repair review does not carry the exact stored predecessor" - ) - elif record.review_reset is not None: - prior = historical.get(str(record.review_reset["prior_review_id"])) - if prior is None: - raise ReviewContractError("initial review reset predecessor is missing") - if value.get("previous_review") != prior: - raise ReviewContractError( - "initial review reset does not carry the exact stored predecessor" - ) + record = _validate_stored_stage_chain(value, historical) if record.review_id in review_ids: raise ReviewContractError("stage review IDs must be globally unique") review_ids.add(record.review_id) @@ -1294,6 +1276,70 @@ def _validated_review_envelope(value: Mapping[str, Any]) -> StageReviewV1: return validate_review_sequence(envelope) +def _bounded_stage_predecessor(value: Mapping[str, Any]) -> dict[str, Any]: + """Project one stored predecessor without recursively embedding its history.""" + + return {key: item for key, item in value.items() if key != "previous_review"} + + +def _stage_review_predecessor_id(record: StageReviewV1) -> str | None: + if record.review_mode == "repair": + assert record.repair_frontier is not None + return str(record.repair_frontier["prior_review_id"]) + if record.review_reset is not None: + return str(record.review_reset["prior_review_id"]) + return None + + +def _validate_stored_stage_chain( + value: Mapping[str, Any], + historical: Mapping[str, Mapping[str, Any]], + *, + visiting: frozenset[str] = frozenset(), +) -> StageReviewV1: + """Validate bounded predecessor projections against complete stored history.""" + + record = _validated_review_envelope(value) + review_id = record.review_id + if review_id in visiting: + raise ReviewContractError("stage review predecessor chain contains a cycle") + predecessor_id = _stage_review_predecessor_id(record) + if predecessor_id is None: + return record + predecessor = historical.get(predecessor_id) + if predecessor is None: + raise ReviewContractError("stage re-review predecessor is missing") + _validate_stored_stage_chain( + predecessor, + historical, + visiting=visiting | {review_id}, + ) + supplied = value.get("previous_review") + if supplied != _bounded_stage_predecessor(predecessor): + raise ReviewContractError( + "stage re-review predecessor projection does not match stored predecessor" + ) + return record + + +def _stored_stage_history(root: Path, stage: str) -> dict[str, Mapping[str, Any]]: + review_root = root.expanduser().resolve() / ".work-bundle/orchestration/reviews" + historical: dict[str, Mapping[str, Any]] = {} + for path in sorted(review_root.rglob("*")): + if path.suffix not in {".json", ".yaml", ".yml"} or not path.is_file(): + continue + if not path.resolve().is_relative_to(review_root.resolve()): + raise ReviewContractError("review record escapes review store") + value = _read_document(path) + if not isinstance(value, dict) or value.get("stage") != stage or "review_id" not in value: + continue + review_id = str(value["review_id"]) + if review_id in historical: + raise ReviewContractError("stage review IDs must be globally unique") + historical[review_id] = value + return historical + + def _review_store_path(root: Path, review_id: str) -> Path: store = root.expanduser().resolve() / ".work-bundle/orchestration/reviews" path = (store / f"{_identifier(review_id, 'review_id')}.json").resolve(strict=False) @@ -1312,6 +1358,10 @@ def publish_review( record = dict(_mapping(review, "review publication")) validated = _validated_review_envelope(record) + if record.get("review_target_kind", "stage") == "stage" and _stage_review_predecessor_id(validated): + validated = _validate_stored_stage_chain( + record, _stored_stage_history(root, validated.stage) + ) current = dict(_target_identity(current_target_identity, "current_target_identity")) if validated.target_identity != current: raise ReviewContractError("review publication target is not current") @@ -1352,6 +1402,10 @@ def load_stored_review( raise ReviewContractError("stored review digest mismatch") record = dict(_mapping(json.loads(raw), "stored review")) validated = _validated_review_envelope(record) + if record.get("review_target_kind", "stage") == "stage" and _stage_review_predecessor_id(validated): + validated = _validate_stored_stage_chain( + record, _stored_stage_history(root, validated.stage) + ) current = dict(_target_identity(current_target_identity, "current_target_identity")) if validated.target_identity != current: raise ReviewContractError("stored review target is not current") diff --git a/tests/test_reviewer_workspace.py b/tests/test_reviewer_workspace.py index 687d980..e055682 100644 --- a/tests/test_reviewer_workspace.py +++ b/tests/test_reviewer_workspace.py @@ -425,16 +425,129 @@ def test_native_compact_integrated_repair_keeps_exact_predecessor_controller_onl assert stored["previous_review"] == previous runtime._require_current_review(control, "integrated_implementation", current_identity) + (source / "src" / "target.py").write_text( + "def target():\n return 2\n", encoding="utf-8" + ) + subprocess.run(["git", "-C", str(source), "add", "."], check=True) + subprocess.run( + [ + "git", "-C", str(source), "-c", "user.name=Test", + "-c", "user.email=test@example.invalid", "commit", "-qm", "accepted reset", + ], + check=True, + ) + reset_identity = runtime.stage_target_identity( + control, "integrated_implementation", target, source_root=source + ) + reset_context = { + **previous_context, + "target_identity": reset_identity, + "agent_id": "reviewer-reset", + "execution_id": "reviewer-reset-run", + "review_mode": "initial", + "repair_frontier": None, + "review_reset": { + "prior_review_id": review["review_id"], + "reason_class": "acceptance", + "reason": "Accepted source identity changed.", + }, + "previous_review": runtime._bounded_stage_predecessor(review), + } + nested_context = {**reset_context, "previous_review": review} + with pytest.raises(ReviewerWorkspaceError, match="STAGE_CONTEXT_INVALID"): + build_direct_evidence_packet( + source_root=source, + control_root=control, + protected_roots=[control / "credentials"], + artifacts=["control:.work-bundle/orchestration/plan/active/plan.md"], + search_roots=[], validators=[], sentinels=[], network_state="denied", + stage_review_context=nested_context, + ) + required, missing = runtime.stage_evidence_requirements( + control, "integrated_implementation", target + ) + assert missing == [] + required.update({ + item["locator"]: "source_tree" + for item in runtime.source_snapshot_entries(source) + }) + reset_packet = build_direct_evidence_packet( + source_root=source, + control_root=control, + protected_roots=[control / "credentials"], + artifacts=list(required), + search_roots=[], validators=[], sentinels=[], network_state="denied", + stage_review_context=reset_context, + ) + reset_created = create_reviewer_workspace( + runtime.reviewer_runtime_root(control), "review-integrated-reset", reset_packet + ) + reset_judgment = {"task_review": { + "reviewed_head": reset_identity["source_tree"], + "verdict": "accept", + "findings": [], + }} + with patch.object( + reviewer_workspace, + "_run_native_process", + return_value=subprocess.CompletedProcess([], 0, native_events(reset_judgment), ""), + ): + reset_receipt = reviewer_workspace.run_native_reviewer( + Path(str(reset_created["workspace_path"])), Path(sys.executable), + model="test-model", review_instructions="Review the current accepted product evidence.", + ) + reset_request = json.loads( + Path(reset_receipt["receipt_path"]).with_suffix(".request.json").read_text() + ) + assert "previous_review" not in reset_request["review_input"] + reset_review = { + **reset_receipt["review_result"], + "reviewer_run": reset_receipt["reviewer_run"], + } + assert reset_review["previous_review"] == runtime._bounded_stage_predecessor(review) + reset_reference = runtime.publish_review( + control, reset_review, current_target_identity=reset_identity + ) + runtime.load_stored_review( + control, reset_reference, current_target_identity=reset_identity + ) + runtime._require_current_review( + control, "integrated_implementation", reset_identity + ) + + chain = { + previous["review_id"]: previous, + review["review_id"]: review, + reset_review["review_id"]: reset_review, + } + forged = json.loads(json.dumps(reset_review)) + forged["previous_review"]["reviewer"]["agent_id"] = "forged-reviewer" + with pytest.raises( + runtime.ReviewContractError, match="projection does not match" + ): + runtime._validate_stored_stage_chain(forged, chain) + with pytest.raises(runtime.ReviewContractError, match="predecessor is missing"): + runtime._validate_stored_stage_chain( + reset_review, {previous["review_id"]: previous} + ) + cyclic = json.loads(json.dumps(reset_review)) + cyclic["review_reset"]["prior_review_id"] = cyclic["review_id"] + cyclic["previous_review"] = runtime._bounded_stage_predecessor(cyclic) + with pytest.raises(runtime.ReviewContractError, match="contains a cycle"): + runtime._validate_stored_stage_chain( + cyclic, {cyclic["review_id"]: cyclic} + ) + malformed = { - **review, + **reset_review, "review_id": "review-integrated-extra-history", - "previous_review": {**previous, "previous_review": previous}, + "previous_review": {**review, "previous_review": previous}, } (history / "review-integrated-extra-history.json").write_text( json.dumps(malformed), encoding="utf-8" ) with pytest.raises(SystemExit, match="exactly one previous_review"): - runtime._require_current_review(control, "integrated_implementation", current_identity) + runtime._require_current_review(control, "integrated_implementation", reset_identity) def test_incomplete_stage_snapshot_fails_before_reviewer_process_launch( From b8484283ccd3d0d94705cd217b58de6c7d1e22a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=88=98=E5=8D=87=E9=BE=99?= Date: Wed, 9 Sep 2026 12:54:29 +0800 Subject: [PATCH 13/18] fix(orchestration): preserve plan identity across knowledge closure --- scripts/orchestration/plans.py | 9 ++- scripts/orchestration/review_runtime.py | 27 +++++++- ...orchestration_accepted_result_lifecycle.py | 31 +++++++++ ...st_orchestration_semantic_plan_identity.py | 65 +++++++++++++++++++ 4 files changed, 129 insertions(+), 3 deletions(-) diff --git a/scripts/orchestration/plans.py b/scripts/orchestration/plans.py index 2295a6a..d3dfb13 100644 --- a/scripts/orchestration/plans.py +++ b/scripts/orchestration/plans.py @@ -36,13 +36,18 @@ def _plan_knowledge_field(body: str, label: str) -> str | None: section = re.search( - r"^##\s+2\.1\s+Knowledge Base Update Carry Forward\s*$([\s\S]*?)(?=^##\s|\Z)", + r"^##\s+(?:2\.1\s+)?Knowledge Base Update Carry Forward\s*$([\s\S]*?)(?=^##\s|\Z)", body, re.MULTILINE, ) if not section: return None - match = re.search(rf"^-\s+\*\*{re.escape(label)}\*\*:\s*([^\s]+)\s*$", section.group(1), re.MULTILINE) + rendered_label = re.escape(label) + match = re.search( + rf"^-\s+(?:\*\*{rendered_label}\*\*|{rendered_label}):[ \t]*([^\s]+)[ \t]*$", + section.group(1), + re.MULTILINE, + ) return match.group(1) if match else None diff --git a/scripts/orchestration/review_runtime.py b/scripts/orchestration/review_runtime.py index 77d9713..2b4196f 100644 --- a/scripts/orchestration/review_runtime.py +++ b/scripts/orchestration/review_runtime.py @@ -743,6 +743,28 @@ def _semantic_plan_value(value: Any, *, top_level: bool = False) -> Any: return value +def _semantic_plan_body(body: str) -> str: + """Remove lifecycle closure state while retaining knowledge authority text.""" + + section_pattern = re.compile( + r"^##\s+(?:2\.1\s+)?Knowledge Base Update Carry Forward\s*$" + r"[\s\S]*?(?=^##\s|\Z)", + re.MULTILINE, + ) + closure_pattern = re.compile( + r"^(?P-\s+(?:\*\*Closure\ return\*\*|Closure\ return):[ \t]*)" + r"(?:missing|completed|not-needed|blocked)(?P[ \t]*)$", + re.MULTILINE, + ) + + def normalize_closure(match: re.Match[str]) -> str: + return closure_pattern.sub( + r"\gmissing\g", match.group(0) + ) + + return section_pattern.sub(normalize_closure, body) + + def _semantic_plan_artifact(path: Path, *, content: str | None = None) -> dict[str, Any]: text = path.read_text(encoding="utf-8") if content is None else content.rstrip() + "\n" if not text.startswith("---\n") or "\n---\n" not in text[4:]: @@ -751,7 +773,10 @@ def _semantic_plan_artifact(path: Path, *, content: str | None = None) -> dict[s metadata = parse_yaml_subset(raw) if not isinstance(metadata, dict) or not metadata.get("id"): raise SystemExit(f"stage review: missing artifact identity: {path}") - return {"metadata": _semantic_plan_value(metadata, top_level=True), "body": body} + return { + "metadata": _semantic_plan_value(metadata, top_level=True), + "body": _semantic_plan_body(body), + } def _semantic_plan_artifact_digest(projection: Mapping[str, Any]) -> str: diff --git a/tests/test_orchestration_accepted_result_lifecycle.py b/tests/test_orchestration_accepted_result_lifecycle.py index 3d1afa3..b2dc062 100644 --- a/tests/test_orchestration_accepted_result_lifecycle.py +++ b/tests/test_orchestration_accepted_result_lifecycle.py @@ -270,6 +270,37 @@ def test_archive_knowledge_gate_aggregates_new_results_and_bounds_legacy_bridge( plans._assert_archive_knowledge_gate(args, "plan-001", plan, [(legacy, brief)]) +@pytest.mark.parametrize( + "section", + [ + "## 2.1 Knowledge Base Update Carry Forward\n\n" + "- **Disposition**: required\n- **Closure return**: completed\n", + "## Knowledge Base Update Carry Forward\n\n" + "- Disposition: required\n- Closure return: completed\n", + ], +) +def test_plan_knowledge_fields_accept_settled_numbered_and_plain_syntax(section: str) -> None: + assert plans._plan_knowledge_field(section, "Disposition") == "required" + assert plans._plan_knowledge_field(section, "Closure return") == "completed" + + +def test_archive_knowledge_gate_consumes_plain_completed_closure(tmp_path: Path) -> None: + plan = tmp_path / "plan.md" + plan.write_text( + "---\nid: plan-001\n---\n\n## Knowledge Base Update Carry Forward\n\n" + "- Disposition: required\n- Closure return: completed\n", + encoding="utf-8", + ) + legacy = {"schema": "accepted-task-result-v1", "task_id": "task-001"} + + plans._assert_archive_knowledge_gate( + argparse.Namespace(), + "plan-001", + plan, + [(legacy, {"task_id": "task-001", "review_required": True})], + ) + + def test_declared_integration_commands_follow_table_headers() -> None: five_columns = ( "## 7. Tests\n\n" diff --git a/tests/test_orchestration_semantic_plan_identity.py b/tests/test_orchestration_semantic_plan_identity.py index 97f3f13..d1a621e 100644 --- a/tests/test_orchestration_semantic_plan_identity.py +++ b/tests/test_orchestration_semantic_plan_identity.py @@ -84,6 +84,19 @@ def test_semantic_projector_preserves_accepted_legacy_baseline_identity(tmp_path assert review_runtime.plan_review_identity(tmp_path, plan) == _legacy_plan_identity(tmp_path, plan) +def test_missing_knowledge_closure_preserves_accepted_legacy_identity(tmp_path: Path) -> None: + plan, _phase, _task = _plan_graph(tmp_path) + plan.write_text( + plan.read_text() + + "\n## Knowledge Base Update Carry Forward\n\n" + "- Disposition: required\n- Closure return: missing\n" + "- Source: accepted specification\n- Review Gate: resolve before archive\n", + encoding="utf-8", + ) + + assert review_runtime.plan_review_identity(tmp_path, plan) == _legacy_plan_identity(tmp_path, plan) + + def test_active_to_archived_rotation_preserves_semantic_plan_identity(tmp_path: Path) -> None: plan, _phase, _task = _plan_graph(tmp_path) original = review_runtime.plan_review_identity(tmp_path, plan) @@ -130,6 +143,58 @@ def test_progress_and_append_only_evidence_do_not_change_semantic_plan_identity( assert review_runtime.plan_review_identity(tmp_path, plan) == original +@pytest.mark.parametrize( + ("heading", "label"), + [ + ("## 2.1 Knowledge Base Update Carry Forward", "**Closure return**"), + ("## Knowledge Base Update Carry Forward", "Closure return"), + ], +) +def test_knowledge_closure_only_change_preserves_plan_review_identity( + tmp_path: Path, heading: str, label: str +) -> None: + plan, _phase, _task = _plan_graph(tmp_path) + plan.write_text( + plan.read_text() + + f"\n{heading}\n\n- **Disposition**: required\n- {label}: missing\n", + encoding="utf-8", + ) + original = review_runtime.plan_review_identity(tmp_path, plan) + + plan.write_text(plan.read_text().replace(f"{label}: missing", f"{label}: completed")) + + assert review_runtime.plan_review_identity(tmp_path, plan) == original + + +@pytest.mark.parametrize( + ("before", "after"), + [ + ("Disposition: not-needed", "Disposition: required"), + ("Source: no durable update", "Source: accepted task findings"), + ("Review Gate: no follow-up", "Review Gate: persist accepted findings"), + ], +) +def test_substantive_knowledge_change_invalidates_plan_review_identity( + tmp_path: Path, before: str, after: str +) -> None: + plan, _phase, _task = _plan_graph(tmp_path) + plan.write_text( + plan.read_text() + + "\n## Knowledge Base Update Carry Forward\n\n" + "- Disposition: not-needed\n- Closure return: missing\n" + "- Source: no durable update\n- Review Gate: no follow-up\n", + encoding="utf-8", + ) + original = review_runtime.plan_review_identity(tmp_path, plan) + + plan.write_text( + plan.read_text().replace(before, after), + encoding="utf-8", + ) + + assert review_runtime.plan_review_identity(tmp_path, plan) != original + + @pytest.mark.parametrize( ("field", "before", "after"), [ From c9096002b85c65275daee4450d589f8f44efb765 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=88=98=E5=8D=87=E9=BE=99?= Date: Wed, 9 Sep 2026 13:19:29 +0800 Subject: [PATCH 14/18] fix(orchestration): finalize direct task layouts --- scripts/orchestration/plans.py | 19 ++- scripts/orchestration/review_runtime.py | 76 +++++++++- ...orchestration_accepted_result_lifecycle.py | 134 ++++++++++++++++++ tests/test_reviewer_workspace.py | 124 ++++++++++++++++ 4 files changed, 346 insertions(+), 7 deletions(-) diff --git a/scripts/orchestration/plans.py b/scripts/orchestration/plans.py index d3dfb13..6f26644 100644 --- a/scripts/orchestration/plans.py +++ b/scripts/orchestration/plans.py @@ -635,16 +635,29 @@ def index_plans(args: argparse.Namespace) -> list[dict[str, object]]: "updated_at": fm.get("last_updated", ""), } ) - for path in sorted(root.glob("active/*/phase-*/*.md")) + sorted(root.glob("archived/*/phase-*/*.md")): + nested_task_paths = list(root.glob("active/*/phase-*/*.md")) + list( + root.glob("archived/*/phase-*/*.md") + ) + direct_task_paths = list(root.glob("active/*/task-*.md")) + list( + root.glob("archived/*/task-*.md") + ) + for path in sorted({*nested_task_paths, *direct_task_paths}): fm, _ = read_front_matter(path) if not fm: continue + direct_layout = path.parent.parent.name in {"active", "archived"} + if direct_layout and ( + fm.get("plan_id") != path.parent.name or not fm.get("phase_id") + ): + raise SystemExit(f"Invalid direct task identity: {rel(path, args)}") + inferred_plan_id = path.parent.name if direct_layout else path.parents[1].name + inferred_phase_id = "" if direct_layout else path.parent.name rows.append( { "type": "task", "id": fm.get("id", path.stem), - "plan_id": fm.get("plan_id", path.parents[1].name), - "phase_id": fm.get("phase_id", path.parent.name), + "plan_id": fm.get("plan_id", inferred_plan_id), + "phase_id": fm.get("phase_id", inferred_phase_id), "title": fm.get("name", fm.get("title", path.stem)), "status": fm.get("status", "Planned"), "path": rel(path, args), diff --git a/scripts/orchestration/review_runtime.py b/scripts/orchestration/review_runtime.py index 2b4196f..d938246 100644 --- a/scripts/orchestration/review_runtime.py +++ b/scripts/orchestration/review_runtime.py @@ -426,7 +426,10 @@ def stage_evidence_manifest(root: Path, source_root: Path, context: Mapping[str, continue entry = {"locator": key, "role": role, "sha256": available[key]["sha256"]} if role in {"target", "plan_member", "verified_specification"}: - entry["identity"] = artifact_review_identity(root / key[8:]) + path = root / key[8:] + entry["identity"] = _stage_authority_identity( + path, stage=str(context["stage"]), role=role + ) entries.append(entry) return {"schema": "stage-evidence-manifest-v1", "stage": context["stage"], "target_identity": context["target_identity"], "entries": entries, @@ -435,7 +438,29 @@ def stage_evidence_manifest(root: Path, source_root: Path, context: Mapping[str, if context.get("review_mode") == "repair" else {})} -def validate_stage_evidence(root: Path, context: Mapping[str, Any], packet: Mapping[str, Any]) -> None: +def _stage_authority_identity( + path: Path, + *, + stage: str, + role: str, + content: str | None = None, +) -> dict[str, Any]: + if stage != "specification" and role in {"target", "plan_member"}: + identity = artifact_review_identity(path, content=content) + identity["sha256"] = _semantic_plan_artifact_digest( + _semantic_plan_artifact(path, content=content) + ) + return identity + return artifact_review_identity(path, content=content) + + +def validate_stage_evidence( + root: Path, + context: Mapping[str, Any], + packet: Mapping[str, Any], + *, + frozen_control_evidence: Mapping[str, str] | None = None, +) -> None: manifest = packet.get("stage_evidence_manifest") if (not isinstance(manifest, dict) or manifest.get("schema") != "stage-evidence-manifest-v1" or manifest.get("stage") != context["stage"] or manifest.get("target_identity") != context["target_identity"] @@ -470,7 +495,31 @@ def validate_stage_evidence(root: Path, context: Mapping[str, Any], packet: Mapp if entry.get("role") != role or locator not in artifacts or entry.get("sha256") != artifacts[locator].get("sha256"): raise ReviewContractError("stage evidence artifact binding mismatch") if role in {"target", "plan_member", "verified_specification"}: - if entry.get("identity") != artifact_review_identity(root / locator[8:]): + path = root / locator[8:] + current_identity = _stage_authority_identity( + path, stage=str(context["stage"]), role=role + ) + if entry.get("identity") == current_identity: + continue + frozen_content = (frozen_control_evidence or {}).get(locator) + if frozen_content is None: + raise ReviewContractError("stage evidence authority identity changed") + if ( + hashlib.sha256(frozen_content.encode()).hexdigest() + != artifacts[locator].get("sha256") + ): + raise ReviewContractError("stage evidence artifact binding mismatch") + frozen_raw_identity = artifact_review_identity(path, content=frozen_content) + frozen_semantic_identity = _stage_authority_identity( + path, + stage=str(context["stage"]), + role=role, + content=frozen_content, + ) + if ( + entry.get("identity") != frozen_raw_identity + or frozen_semantic_identity != current_identity + ): raise ReviewContractError("stage evidence authority identity changed") @@ -637,8 +686,22 @@ def canonical(value: Any) -> str: context = _mapping(receipt.get(context_key, {}), "reviewer-run provenance context") native = receipt.get("schema") == "reviewer-native-receipt-v1" packet_context = packet.get(context_key) + frozen_control_evidence: dict[str, str] = {} if native: packet_context = _validate_native_run_proof(receipt, packet, result, path, immutable_file, canonical) + controller_path = path.with_suffix(".controller.json") + if controller_path.exists(): + controller_evidence = json.loads(immutable_file(controller_path)) + if not isinstance(controller_evidence, list): + raise ReviewContractError("native reviewer-run controller evidence is invalid") + for item in controller_evidence: + if ( + not isinstance(item, dict) + or not isinstance(item.get("locator"), str) + or not isinstance(item.get("content"), str) + ): + raise ReviewContractError("native reviewer-run controller evidence is invalid") + frozen_control_evidence[item["locator"]] = item["content"] mode = "direct_source" if review["evidence"]["mode"] == "direct" else review["evidence"]["mode"] review_context = { "review_mode": review.get("review_mode", "initial"), @@ -673,7 +736,12 @@ def canonical(value: Any) -> str: else _known_execution_ids(root, str(review["stage"]), review["target_identity"]) ) if kind == "stage": - validate_stage_evidence(root, context, packet) + validate_stage_evidence( + root, + context, + packet, + frozen_control_evidence=frozen_control_evidence, + ) if run_id in known or context["execution_id"] in known: raise ReviewContractError("reviewer-run provenance overlaps author/repair execution") diff --git a/tests/test_orchestration_accepted_result_lifecycle.py b/tests/test_orchestration_accepted_result_lifecycle.py index b2dc062..2460000 100644 --- a/tests/test_orchestration_accepted_result_lifecycle.py +++ b/tests/test_orchestration_accepted_result_lifecycle.py @@ -301,6 +301,140 @@ def test_archive_knowledge_gate_consumes_plain_completed_closure(tmp_path: Path) ) +def _write_mixed_layout_plan(root: Path) -> tuple[Path, Path, Path, Path, Path]: + active = root / ".work-bundle/orchestration/plan/active" + plan = active / "plan-direct.md" + plan_dir = active / "plan-direct" + phase_direct = plan_dir / "phase-001.md" + task_direct = plan_dir / "task-001.md" + phase_nested = plan_dir / "phase-002.md" + task_nested = plan_dir / "phase-002/task-002.md" + task_nested.parent.mkdir(parents=True) + plan.write_text("---\nid: plan-direct\nstatus: Completed\n---\n", encoding="utf-8") + phase_direct.write_text( + "---\nid: phase-001\nplan_id: plan-direct\nstatus: Completed\n---\n", + encoding="utf-8", + ) + phase_nested.write_text( + "---\nid: phase-002\nplan_id: plan-direct\nstatus: Completed\n---\n", + encoding="utf-8", + ) + task_direct.write_text( + "---\nid: task-001\nplan_id: plan-direct\nphase_id: phase-001\n" + "status: Planned\ndepends_on: []\n---\n", + encoding="utf-8", + ) + task_nested.write_text( + "---\nid: task-002\nplan_id: plan-direct\nphase_id: phase-002\n" + "status: Completed\ndepends_on: []\n---\n", + encoding="utf-8", + ) + return plan, phase_direct, task_direct, phase_nested, task_nested + + +def test_plan_index_and_status_support_direct_and_nested_task_layouts( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + _plan, _phase_direct, task_direct, _phase_nested, task_nested = _write_mixed_layout_plan( + tmp_path + ) + args = argparse.Namespace(project_root=str(tmp_path)) + + rows = plans.index_plans(args) + tasks = [row for row in rows if row["type"] == "task"] + + assert sorted((row["id"], row["plan_id"], row["phase_id"]) for row in tasks) == [ + ("task-001", "plan-direct", "phase-001"), + ("task-002", "plan-direct", "phase-002"), + ] + assert {Path(str(row["path"])).name for row in tasks} == { + task_direct.name, + task_nested.name, + } + monkeypatch.setattr(plans, "_assert_task_dependencies_current", lambda *_args: None) + monkeypatch.setattr(plans, "_assert_completed_task_authority", lambda *_args: {}) + monkeypatch.setattr(plans, "_release_completed_task_binding", lambda *_args: {}) + plans.cmd_set_plan_status( + argparse.Namespace( + project_root=str(tmp_path), + id="task-001", + plan_id="plan-direct", + kind="task", + status="Completed", + ) + ) + assert "status: Completed" in task_direct.read_text(encoding="utf-8") + + +@pytest.mark.parametrize( + "identity", + [ + "plan_id: other-plan\nphase_id: phase-001\n", + "plan_id: plan-direct\n", + ], +) +def test_direct_task_index_rejects_ambiguous_identity(tmp_path: Path, identity: str) -> None: + task = tmp_path / ".work-bundle/orchestration/plan/active/plan-direct/task-001.md" + task.parent.mkdir(parents=True) + task.write_text(f"---\nid: task-001\n{identity}---\n", encoding="utf-8") + + with pytest.raises(SystemExit, match="Invalid direct task identity"): + plans.index_plans(argparse.Namespace(project_root=str(tmp_path))) + + +def test_phase_and_archive_consumers_include_direct_tasks( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + _plan, _phase_direct, task_direct, _phase_nested, _task_nested = _write_mixed_layout_plan( + tmp_path + ) + args = argparse.Namespace(project_root=str(tmp_path)) + with pytest.raises(SystemExit, match="task task-001 is not completed"): + plans._assert_phase_tasks_accepted(args, "phase-001", "plan-direct") + + task_direct.write_text( + task_direct.read_text(encoding="utf-8").replace("status: Planned", "status: Completed"), + encoding="utf-8", + ) + accepted: list[str] = [] + monkeypatch.setattr( + plans, + "_load_current_task_acceptance", + lambda _args, path: accepted.append(path.name) or ({}, {}), + ) + plans._assert_phase_tasks_accepted(args, "phase-001", "plan-direct") + assert accepted == ["task-001.md"] + + accepted.clear() + monkeypatch.setattr( + plans, + "_task_brief_at", + lambda _args, path: {"task_id": path.stem}, + ) + results = plans._accepted_plan_task_results(args, "plan-direct") + assert {brief["task_id"] for _result, brief in results} == {"task-001", "task-002"} + assert set(accepted) == {"task-001.md", "task-002.md"} + + monkeypatch.setattr(plans, "require_plan_reviews", lambda *_args, **_kwargs: None) + monkeypatch.setattr(plans, "_plan_uses_accepted_result_authority", lambda *_args: False) + monkeypatch.setattr(plans, "_validated_plan_task_handoffs", lambda *_args: []) + monkeypatch.setattr(plans, "_assert_archive_knowledge_gate", lambda *_args: None) + monkeypatch.setattr(plans, "_assert_archive_plan_acceptance", lambda *_args: None) + plans.cmd_archive_plan(argparse.Namespace(project_root=str(tmp_path), id="plan-direct")) + + archived_rows = plans.index_plans(args) + assert { + (row["id"], row["type"]) + for row in archived_rows + if row.get("plan_id") == "plan-direct" + } == { + ("phase-001", "phase"), + ("phase-002", "phase"), + ("task-001", "task"), + ("task-002", "task"), + } + + def test_declared_integration_commands_follow_table_headers() -> None: five_columns = ( "## 7. Tests\n\n" diff --git a/tests/test_reviewer_workspace.py b/tests/test_reviewer_workspace.py index e055682..0f74c6f 100644 --- a/tests/test_reviewer_workspace.py +++ b/tests/test_reviewer_workspace.py @@ -598,6 +598,130 @@ def test_incomplete_stage_snapshot_fails_before_reviewer_process_launch( launch.assert_not_called() +def test_stage_evidence_survives_plan_lifecycle_markers_but_not_product_changes( + review_roots: tuple[Path, Path, Path] +) -> None: + source, control, _runtime = review_roots + subprocess.run(["git", "init", "-q", str(source)], check=True) + subprocess.run(["git", "-C", str(source), "add", "."], check=True) + subprocess.run( + [ + "git", "-C", str(source), "-c", "user.name=Test", + "-c", "user.email=test@example.invalid", "commit", "-qm", "fixture", + ], + check=True, + ) + plan = control / ".work-bundle/orchestration/plan/active/plan-lifecycle.md" + task = control / ".work-bundle/orchestration/plan/active/plan-lifecycle/task-001.md" + spec = control / ".work-bundle/orchestration/spec/active/spec-lifecycle.md" + task.parent.mkdir(parents=True) + spec.parent.mkdir(parents=True) + spec.write_text( + "---\nid: spec-lifecycle\nstatus: verified\n---\n\n# Specification\n", + encoding="utf-8", + ) + plan.write_text( + "---\nid: plan-lifecycle\nstatus: In progress\n" + "accepted_results: [result-task-001]\n" + "source_spec: [.work-bundle/orchestration/spec/active/spec-lifecycle.md]\n" + "---\n\n# Plan\n\n" + "## Knowledge Base Update Carry Forward\n\n" + "- Disposition: required\n- Closure return: missing\n" + "- Source: accepted specification\n- Review Gate: resolve before archive\n", + encoding="utf-8", + ) + task.write_text( + "---\nid: task-001\nplan_id: plan-lifecycle\nphase_id: phase-001\n" + "status: In progress\n---\n\n# Task\n", + encoding="utf-8", + ) + runtime = reviewer_workspace._review_runtime() + identity = runtime.stage_target_identity( + control, "integrated_implementation", plan, source_root=source + ) + locator = "control:" + plan.relative_to(control).as_posix() + context = { + "stage": "integrated_implementation", + "target_identity": identity, + "target_locator": locator, + "agent_id": "reviewer-lifecycle", + "capability": "judgment", + "execution_id": "reviewer-lifecycle-run", + "evidence_mode": "direct_source", + } + required, missing = runtime.stage_evidence_requirements( + control, "integrated_implementation", plan + ) + assert missing == [] + required.update( + {item["locator"]: "source_tree" for item in runtime.source_snapshot_entries(source)} + ) + packet = build_direct_evidence_packet( + source_root=source, + control_root=control, + protected_roots=[control / "credentials"], + artifacts=list(required), + search_roots=[], + validators=[], + sentinels=[], + network_state="denied", + stage_review_context=context, + ) + frozen_control_evidence = { + item["locator"]: (control / item["locator"].removeprefix("control:")).read_text( + encoding="utf-8" + ) + for item in packet["artifacts"] + if item["locator"].startswith("control:") + } + entries = packet["stage_evidence_manifest"]["entries"] + for entry in entries: + if entry["role"] in {"target", "plan_member"}: + entry["identity"] = runtime.artifact_review_identity( + control / entry["locator"].removeprefix("control:") + ) + runtime.validate_stage_evidence( + control, + packet["stage_review_context"], + packet, + frozen_control_evidence=frozen_control_evidence, + ) + + plan.write_text( + plan.read_text(encoding="utf-8") + .replace("status: In progress", "status: Completed") + .replace("Closure return: missing", "Closure return: completed"), + encoding="utf-8", + ) + task.write_text( + task.read_text(encoding="utf-8").replace("status: In progress", "status: Completed"), + encoding="utf-8", + ) + assert runtime.stage_target_identity( + control, "integrated_implementation", plan, source_root=source + ) == identity + runtime.validate_stage_evidence( + control, + packet["stage_review_context"], + packet, + frozen_control_evidence=frozen_control_evidence, + ) + + plan.write_text( + plan.read_text(encoding="utf-8").replace( + "Review Gate: resolve before archive", "Review Gate: bypass product review" + ), + encoding="utf-8", + ) + with pytest.raises(runtime.ReviewContractError, match="authority identity changed"): + runtime.validate_stage_evidence( + control, + packet["stage_review_context"], + packet, + frozen_control_evidence=frozen_control_evidence, + ) + + def test_bounded_read_search_and_validators_are_allowed(review_roots: tuple[Path, Path, Path]) -> None: source, control, runtime = review_roots created = create_reviewer_workspace(runtime, "review-002", packet(source, control)) From 91fadf0df6d6e31a2b9286943d56a9a3c5eca906 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=88=98=E5=8D=87=E9=BE=99?= Date: Wed, 9 Sep 2026 13:32:42 +0800 Subject: [PATCH 15/18] fix(review): retain frozen native authority evidence --- scripts/orchestration/review_runtime.py | 28 +++-- scripts/work-bundle/reviewer_workspace.py | 2 + tests/test_reviewer_workspace.py | 132 +++++++++++++++------- 3 files changed, 104 insertions(+), 58 deletions(-) diff --git a/scripts/orchestration/review_runtime.py b/scripts/orchestration/review_runtime.py index d938246..ed89367 100644 --- a/scripts/orchestration/review_runtime.py +++ b/scripts/orchestration/review_runtime.py @@ -650,7 +650,16 @@ def _validate_native_run_proof(receipt, packet, result, path, immutable_file, ca previous_review=result.get("previous_review")) if observed != result or receipt.get("review_result") != result or receipt.get(key) != context: raise ValueError("native judgment/result mismatch") - return context + frozen_control_evidence: dict[str, str] = {} + for item in combined_evidence: + locator = item.get("locator") + content = item.get("content") + if not isinstance(locator, str) or not locator.startswith("control:"): + continue + if not isinstance(content, str) or locator in frozen_control_evidence: + raise ValueError("native control evidence is invalid") + frozen_control_evidence[locator] = content + return context, frozen_control_evidence except (ValueError, TypeError, KeyError, IndexError, AttributeError, runtime.ReviewerWorkspaceError) as error: raise ReviewContractError("native reviewer-run provenance does not bind this accepted review") from error @@ -688,20 +697,9 @@ def canonical(value: Any) -> str: packet_context = packet.get(context_key) frozen_control_evidence: dict[str, str] = {} if native: - packet_context = _validate_native_run_proof(receipt, packet, result, path, immutable_file, canonical) - controller_path = path.with_suffix(".controller.json") - if controller_path.exists(): - controller_evidence = json.loads(immutable_file(controller_path)) - if not isinstance(controller_evidence, list): - raise ReviewContractError("native reviewer-run controller evidence is invalid") - for item in controller_evidence: - if ( - not isinstance(item, dict) - or not isinstance(item.get("locator"), str) - or not isinstance(item.get("content"), str) - ): - raise ReviewContractError("native reviewer-run controller evidence is invalid") - frozen_control_evidence[item["locator"]] = item["content"] + packet_context, frozen_control_evidence = _validate_native_run_proof( + receipt, packet, result, path, immutable_file, canonical + ) mode = "direct_source" if review["evidence"]["mode"] == "direct" else review["evidence"]["mode"] review_context = { "review_mode": review.get("review_mode", "initial"), diff --git a/scripts/work-bundle/reviewer_workspace.py b/scripts/work-bundle/reviewer_workspace.py index 50290b4..37078cb 100644 --- a/scripts/work-bundle/reviewer_workspace.py +++ b/scripts/work-bundle/reviewer_workspace.py @@ -1262,6 +1262,8 @@ def _task_product_judgment_review( or not isinstance(product["findings"], list) ): raise ReviewerWorkspaceError("WB_REVIEW_TASK_OUTPUT_INVALID") + if product["verdict"] == "repair" and not product["findings"]: + raise ReviewerWorkspaceError("WB_REVIEW_TASK_OUTPUT_INVALID") findings = [] for item in product["findings"]: expected = {"finding_id", "severity", "requirement_id", "boundary", "evidence", "expected", "observed", "owner"} diff --git a/tests/test_reviewer_workspace.py b/tests/test_reviewer_workspace.py index 0f74c6f..7295b62 100644 --- a/tests/test_reviewer_workspace.py +++ b/tests/test_reviewer_workspace.py @@ -177,7 +177,9 @@ def test_workspace_contains_copied_direct_evidence_and_declares_network_denied( def test_task_review_worker_output_receives_native_bound_receipt( review_roots: tuple[Path, Path, Path], transport: str ) -> None: - source, control, runtime = review_roots + source, control, _runtime = review_roots + review_runtime = reviewer_workspace._review_runtime() + runtime = review_runtime.reviewer_runtime_root(control) subprocess.run(["git", "init", "-q", str(source)], check=True) subprocess.run(["git", "-C", str(source), "add", "src/target.py", ".wor105-review-sentinel"], check=True) subprocess.run( @@ -209,7 +211,17 @@ def test_task_review_worker_output_receives_native_bound_receipt( created = create_reviewer_workspace(runtime, "review-task-native", direct) judgment = {"task_review": { "reviewed_head": head, - "verdict": "accept", "findings": [], + "verdict": "repair", + "findings": [{ + "finding_id": "finding-product-value", + "severity": "blocking", + "requirement_id": "REQ-VALUE", + "boundary": "src/target.py:target", + "evidence": "The returned value violates the requirement.", + "expected": "Return the accepted value.", + "observed": "A different value is returned.", + "owner": "task_owner", + }], }} if transport == "native": with patch.object(reviewer_workspace, "_run_native_process", @@ -229,7 +241,35 @@ def test_task_review_worker_output_receives_native_bound_receipt( assert set(receipt["reviewer_run"]) == {"run_id", "sha256"} assert receipt["review_result"]["reviewer"]["agent_id"] == ( receipt["host_run_id"] if transport == "native" else "reviewer-task") - assert receipt["review_result"]["verdict"] == "accept" + assert receipt["review_result"]["verdict"] == "repair" + finding = receipt["review_result"]["findings"][0] + assert finding["finding_id"] == "finding-product-value" + assert finding["evidence"][0]["locator"] == "src/target.py:target" + assert "The returned value violates the requirement." in finding["evidence"][0]["observation"] + reference = review_runtime.publish_review( + control, receipt["review_result"] | {"reviewer_run": receipt["reviewer_run"]}, + current_target_identity=identity, + ) + stored, validated = review_runtime.load_stored_review( + control, reference, current_target_identity=identity + ) + assert stored["findings"] == receipt["review_result"]["findings"] + assert validated.findings[0].finding_id == "finding-product-value" + + empty_repair = {"task_review": { + "reviewed_head": head, + "verdict": "repair", + "findings": [], + }} + with pytest.raises(ReviewerWorkspaceError, match="WB_REVIEW_TASK_OUTPUT_INVALID"): + reviewer_workspace._task_product_judgment_review( + empty_repair, + review_id="review-empty-repair", + context=context, + packet=direct, + started_at="2026-09-09T00:00:00Z", + completed_at="2026-09-09T00:01:00Z", + ) def test_compact_task_judgment_composes_repair_and_reset_predecessors() -> None: @@ -656,36 +696,52 @@ def test_stage_evidence_survives_plan_lifecycle_markers_but_not_product_changes( required.update( {item["locator"]: "source_tree" for item in runtime.source_snapshot_entries(source)} ) - packet = build_direct_evidence_packet( - source_root=source, - control_root=control, - protected_roots=[control / "credentials"], - artifacts=list(required), - search_roots=[], - validators=[], - sentinels=[], - network_state="denied", - stage_review_context=context, - ) - frozen_control_evidence = { - item["locator"]: (control / item["locator"].removeprefix("control:")).read_text( - encoding="utf-8" + def legacy_stage_identity(path: Path, **kwargs: object) -> dict[str, object]: + return runtime.artifact_review_identity(path, content=kwargs.get("content")) + + with patch.object(runtime, "_stage_authority_identity", side_effect=legacy_stage_identity): + packet = build_direct_evidence_packet( + source_root=source, + control_root=control, + protected_roots=[control / "credentials"], + artifacts=list(required), + search_roots=[], + validators=[], + sentinels=[], + network_state="denied", + stage_review_context=context, ) - for item in packet["artifacts"] - if item["locator"].startswith("control:") - } - entries = packet["stage_evidence_manifest"]["entries"] - for entry in entries: - if entry["role"] in {"target", "plan_member"}: - entry["identity"] = runtime.artifact_review_identity( - control / entry["locator"].removeprefix("control:") + created = create_reviewer_workspace( + runtime.reviewer_runtime_root(control), "review-lifecycle", packet + ) + judgment = { + "task_review": { + "reviewed_head": identity["source_tree"], + "verdict": "accept", + "findings": [], + } + } + with patch.object( + reviewer_workspace, + "_run_native_process", + return_value=subprocess.CompletedProcess([], 0, native_events(judgment), ""), + ): + receipt = reviewer_workspace.run_native_reviewer( + Path(str(created["workspace_path"])), + Path(sys.executable), + model="test-model", + review_instructions="Review the frozen product evidence.", ) - runtime.validate_stage_evidence( - control, - packet["stage_review_context"], - packet, - frozen_control_evidence=frozen_control_evidence, + review = {**receipt["review_result"], "reviewer_run": receipt["reviewer_run"]} + runtime.publish_review(control, review, current_target_identity=identity) + + request = json.loads( + Path(receipt["receipt_path"]).with_suffix(".request.json").read_text() ) + assert locator in {item["locator"] for item in request["evidence"]} + controller_path = Path(receipt["receipt_path"]).with_suffix(".controller.json") + controller_evidence = json.loads(controller_path.read_text()) + assert locator not in {item["locator"] for item in controller_evidence} plan.write_text( plan.read_text(encoding="utf-8") @@ -700,12 +756,7 @@ def test_stage_evidence_survives_plan_lifecycle_markers_but_not_product_changes( assert runtime.stage_target_identity( control, "integrated_implementation", plan, source_root=source ) == identity - runtime.validate_stage_evidence( - control, - packet["stage_review_context"], - packet, - frozen_control_evidence=frozen_control_evidence, - ) + runtime._require_current_review(control, "integrated_implementation", identity) plan.write_text( plan.read_text(encoding="utf-8").replace( @@ -713,13 +764,8 @@ def test_stage_evidence_survives_plan_lifecycle_markers_but_not_product_changes( ), encoding="utf-8", ) - with pytest.raises(runtime.ReviewContractError, match="authority identity changed"): - runtime.validate_stage_evidence( - control, - packet["stage_review_context"], - packet, - frozen_control_evidence=frozen_control_evidence, - ) + with pytest.raises(SystemExit, match="authority identity changed"): + runtime._require_current_review(control, "integrated_implementation", identity) def test_bounded_read_search_and_validators_are_allowed(review_roots: tuple[Path, Path, Path]) -> None: From 88a1e71d38abaad879ff32361f7303db3566efe9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=88=98=E5=8D=87=E9=BE=99?= Date: Wed, 9 Sep 2026 13:41:31 +0800 Subject: [PATCH 16/18] fix(orchestration): decode quoted plan index scalars --- scripts/orchestration/plans.py | 26 +++++++++++++++++++ ...orchestration_accepted_result_lifecycle.py | 18 ++++++++----- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/scripts/orchestration/plans.py b/scripts/orchestration/plans.py index 6f26644..1fb5dfd 100644 --- a/scripts/orchestration/plans.py +++ b/scripts/orchestration/plans.py @@ -599,6 +599,29 @@ def _assert_archive_plan_acceptance( _assert_archive_command_state_neutral(command, workspace) +def _index_front_matter_scalars( + front_matter: dict[str, object], path: Path, args: argparse.Namespace +) -> dict[str, object]: + """Decode quoted scalar values only for the flat plan index projection.""" + + normalized = dict(front_matter) + for key, value in front_matter.items(): + if ( + not isinstance(value, str) + or len(value) < 2 + or value[0] != value[-1] + or value[0] not in {"'", '"'} + ): + continue + parsed = _parse_scalar(value) + if not isinstance(parsed, str): + raise SystemExit( + f"Invalid quoted plan index scalar {key}: {rel(path, args)}" + ) + normalized[key] = parsed + return normalized + + def index_plans(args: argparse.Namespace) -> list[dict[str, object]]: root = orchestration_root(args) / "plan" rows = [] @@ -606,6 +629,7 @@ def index_plans(args: argparse.Namespace) -> list[dict[str, object]]: fm, _ = read_front_matter(path) if not fm: continue + fm = _index_front_matter_scalars(fm, path, args) rows.append( { "type": "plan", @@ -623,6 +647,7 @@ def index_plans(args: argparse.Namespace) -> list[dict[str, object]]: fm, _ = read_front_matter(path) if not fm: continue + fm = _index_front_matter_scalars(fm, path, args) rows.append( { "type": "phase", @@ -645,6 +670,7 @@ def index_plans(args: argparse.Namespace) -> list[dict[str, object]]: fm, _ = read_front_matter(path) if not fm: continue + fm = _index_front_matter_scalars(fm, path, args) direct_layout = path.parent.parent.name in {"active", "archived"} if direct_layout and ( fm.get("plan_id") != path.parent.name or not fm.get("phase_id") diff --git a/tests/test_orchestration_accepted_result_lifecycle.py b/tests/test_orchestration_accepted_result_lifecycle.py index 2460000..5942de7 100644 --- a/tests/test_orchestration_accepted_result_lifecycle.py +++ b/tests/test_orchestration_accepted_result_lifecycle.py @@ -310,9 +310,11 @@ def _write_mixed_layout_plan(root: Path) -> tuple[Path, Path, Path, Path, Path]: phase_nested = plan_dir / "phase-002.md" task_nested = plan_dir / "phase-002/task-002.md" task_nested.parent.mkdir(parents=True) - plan.write_text("---\nid: plan-direct\nstatus: Completed\n---\n", encoding="utf-8") + plan.write_text( + '---\nid: "plan-direct"\nstatus: "Completed"\n---\n', encoding="utf-8" + ) phase_direct.write_text( - "---\nid: phase-001\nplan_id: plan-direct\nstatus: Completed\n---\n", + "---\nid: 'phase-001'\nplan_id: \"plan-direct\"\nstatus: 'Completed'\n---\n", encoding="utf-8", ) phase_nested.write_text( @@ -320,13 +322,13 @@ def _write_mixed_layout_plan(root: Path) -> tuple[Path, Path, Path, Path, Path]: encoding="utf-8", ) task_direct.write_text( - "---\nid: task-001\nplan_id: plan-direct\nphase_id: phase-001\n" - "status: Planned\ndepends_on: []\n---\n", + '---\nid: "task-001"\nplan_id: "plan-direct"\nphase_id: "phase-001"\n' + 'status: "Planned"\ndepends_on: []\n---\n', encoding="utf-8", ) task_nested.write_text( - "---\nid: task-002\nplan_id: plan-direct\nphase_id: phase-002\n" - "status: Completed\ndepends_on: []\n---\n", + "---\nid: 'task-002'\nplan_id: 'plan-direct'\nphase_id: 'phase-002'\n" + "status: 'Completed'\ndepends_on: []\n---\n", encoding="utf-8", ) return plan, phase_direct, task_direct, phase_nested, task_nested @@ -393,7 +395,9 @@ def test_phase_and_archive_consumers_include_direct_tasks( plans._assert_phase_tasks_accepted(args, "phase-001", "plan-direct") task_direct.write_text( - task_direct.read_text(encoding="utf-8").replace("status: Planned", "status: Completed"), + task_direct.read_text(encoding="utf-8").replace( + 'status: "Planned"', 'status: "Completed"' + ), encoding="utf-8", ) accepted: list[str] = [] From 2b479eaca6b6da7f3a190470f1e3c1ab7ba536fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=88=98=E5=8D=87=E9=BE=99?= Date: Wed, 9 Sep 2026 13:56:16 +0800 Subject: [PATCH 17/18] fix(orchestration): resolve final plan repository authority --- scripts/orchestration/plans.py | 127 +++++++++++++-- ...orchestration_accepted_result_lifecycle.py | 147 ++++++++++++++++++ 2 files changed, 264 insertions(+), 10 deletions(-) diff --git a/scripts/orchestration/plans.py b/scripts/orchestration/plans.py index 1fb5dfd..8a2ba90 100644 --- a/scripts/orchestration/plans.py +++ b/scripts/orchestration/plans.py @@ -29,7 +29,11 @@ release_completion_binding, ) from handoffs import _read_compact_yaml_metadata -from repository_preflight import capture_repository_evidence, task_caused_paths +from repository_preflight import ( + _metadata_repository_entries, + capture_repository_evidence, + task_caused_paths, +) from specs import load_index, replace_front_matter_value from review_runtime import require_plan_reviews @@ -318,7 +322,7 @@ def _material_repository_root( handoff_has_provenance = True if not handoff_has_provenance: try: - fallback = _resolve_final_plan_workspace(args) + fallback = _resolve_final_plan_workspace(args, plan_id) except SystemExit as error: raise SystemExit( "acceptance-blocked: material handoff repository provenance is unavailable" @@ -398,15 +402,112 @@ def _handoff_has_material_changes(handoff: dict[str, object], brief: dict[str, o return bool(write) if isinstance(write, list) else False -def _resolve_final_plan_workspace(args: argparse.Namespace) -> Path: +def _accepted_plan_repository_bindings( + args: argparse.Namespace, plan_id: str +) -> list[dict[str, object]]: + bindings: list[dict[str, object]] = [] + control_root = resolve_workspace_root(args) + for row in index_plans(args): + if row.get("type") != "task" or row.get("plan_id") != plan_id: + continue + task_id = str(row.get("id") or "") + if not has_persisted_accepted_task_result(control_root, plan_id, task_id): + continue + binding, _accepted = _load_current_task_acceptance( + args, artifact_path_from_row(row, args) + ) + bindings.append(binding) + return bindings + + +def _registered_repository_roots(workspace: Path) -> dict[str, Path]: + member_roots = set(_member_roots(workspace)) + registered: dict[str, Path] = {} + for entry in _metadata_repository_entries(workspace): + repository_id = str(entry.get("id") or "").strip() + raw_root = str(entry.get("project_root") or entry.get("path") or "").strip() + if not repository_id or not raw_root: + continue + candidate = Path(raw_root).expanduser() + candidate = (workspace / candidate).resolve() if not candidate.is_absolute() else candidate.resolve() + # Only device/local member roots are eligible. Remote/origin locators never + # appear in this intersection. + if candidate not in member_roots: + continue + if repository_id in registered and registered[repository_id] != candidate: + raise SystemExit("acceptance-blocked: registered repository identity is ambiguous") + registered[repository_id] = candidate + return registered + + +def _validate_final_workspace_selectors( + args: argparse.Namespace, bindings: list[dict[str, object]] +) -> None: + selectors = { + "workspace_id": getattr(args, "workspace_id", None), + "execution_id": getattr(args, "execution_id", None), + "runtime_root": getattr(args, "execution_runtime_root", None), + } + for field, supplied in selectors.items(): + if not supplied: + continue + if field == "runtime_root": + expected = Path(str(supplied)).expanduser().resolve() + matches = [ + binding + for binding in bindings + if Path(str(binding.get(field) or "")).expanduser().resolve() == expected + ] + else: + matches = [binding for binding in bindings if str(binding.get(field) or "") == str(supplied)] + if not matches: + raise SystemExit( + f"acceptance-blocked: {field.replace('_', ' ')} selector conflicts with accepted task authority" + ) + + +def _resolve_final_plan_workspace( + args: argparse.Namespace, plan_id: str | None = None +) -> Path: workspace = resolve_workspace_root(args) try: members = _member_roots(workspace) except OSError: members = [] - if len(members) > 1: + if len(members) <= 1: + target = members[0] if members else workspace + if not target.is_dir(): + raise SystemExit("acceptance-blocked: final plan workspace is missing") + return target + + bindings = _accepted_plan_repository_bindings(args, plan_id) if plan_id else [] + _validate_final_workspace_selectors(args, bindings) + accepted_repository_ids = { + str(binding.get("repository_id") or "").strip() for binding in bindings + } + if "" in accepted_repository_ids: + raise SystemExit("acceptance-blocked: accepted task repository authority is missing") + if len(accepted_repository_ids) > 1: + raise SystemExit("acceptance-blocked: accepted task repository authority disagrees") + + explicit_repository_id = str(getattr(args, "repository_id", None) or "").strip() + accepted_repository_id = next(iter(accepted_repository_ids), "") + if ( + explicit_repository_id + and accepted_repository_id + and explicit_repository_id != accepted_repository_id + ): + raise SystemExit( + "acceptance-blocked: repository selector conflicts with accepted task authority" + ) + repository_id = explicit_repository_id or accepted_repository_id + if not repository_id: raise SystemExit("acceptance-blocked: final plan workspace is ambiguous") - target = members[0] if members else workspace + target = _registered_repository_roots(workspace).get(repository_id) + if target is None: + raise SystemExit( + "acceptance-blocked: authorized final plan repository is not a registered local member" + ) if not target.is_dir(): raise SystemExit("acceptance-blocked: final plan workspace is missing") return target @@ -594,7 +695,7 @@ def _assert_archive_plan_acceptance( # gate obtains one fresh state-neutral observation below instead. if control_root is not None: return - workspace = git_root if material else _resolve_final_plan_workspace(args) + workspace = git_root if material else _resolve_final_plan_workspace(args, plan_id) for command in commands: _assert_archive_command_state_neutral(command, workspace) @@ -713,7 +814,7 @@ def cmd_write_plan(args: argparse.Namespace) -> None: effective_status = parse_yaml_subset(content.split("---", 2)[1]).get("status") if effective_status in {"In progress", "Completed"} or args.status in {"In progress", "Completed"}: require_plan_reviews(project_root(args), target, content=content, - source_root=_resolve_final_plan_workspace(args) if "Completed" in {effective_status, args.status} else None) + source_root=_resolve_final_plan_workspace(args, pid) if "Completed" in {effective_status, args.status} else None) write_text_safely(target, content, args) index_plans(args) print(rel(target, args)) @@ -946,7 +1047,7 @@ def cmd_set_plan_status(args: argparse.Namespace) -> None: _assert_phase_tasks_accepted(args, str(row["id"]), str(row["plan_id"])) if row.get("type") == "plan" and args.status in {"In progress", "Completed"}: require_plan_reviews(project_root(args), path, - source_root=_resolve_final_plan_workspace(args) if args.status == "Completed" else None) + source_root=_resolve_final_plan_workspace(args, str(row["id"])) if args.status == "Completed" else None) if args.status == "Completed": _accepted_plan_task_results(args, str(row["id"])) if args.status == "Completed" and row.get("type") == "task": @@ -973,7 +1074,10 @@ def cmd_archive_plan(args: argparse.Namespace) -> None: moved = [] root_path = artifact_path_from_row(root_match, args) - require_plan_reviews(project_root(args), root_path, source_root=_resolve_final_plan_workspace(args)) + require_plan_reviews( + project_root(args), root_path, + source_root=_resolve_final_plan_workspace(args, args.id), + ) if _plan_uses_accepted_result_authority(args, args.id): validated = _accepted_plan_task_results(args, args.id) else: @@ -982,7 +1086,10 @@ def cmd_archive_plan(args: argparse.Namespace) -> None: validated = _validated_plan_task_handoffs(args, args.id) _assert_archive_knowledge_gate(args, args.id, root_path, validated) _assert_archive_plan_acceptance(args, args.id, root_path, validated) - require_plan_reviews(project_root(args), root_path, source_root=_resolve_final_plan_workspace(args)) + require_plan_reviews( + project_root(args), root_path, + source_root=_resolve_final_plan_workspace(args, args.id), + ) if is_relative_to(root_path, active_root): replace_front_matter_value(root_path, "status", "Completed") moved.append(move_to_archive(root_path, active_root, archived_root)) diff --git a/tests/test_orchestration_accepted_result_lifecycle.py b/tests/test_orchestration_accepted_result_lifecycle.py index 5942de7..4bd1e64 100644 --- a/tests/test_orchestration_accepted_result_lifecycle.py +++ b/tests/test_orchestration_accepted_result_lifecycle.py @@ -226,6 +226,153 @@ def test_archive_switches_irreversibly_to_accepted_results_without_handoff_repla assert (tmp_path / ".work-bundle/orchestration/plan/archived/plan.md").is_file() +def _write_multi_repository_workspace(root: Path, members: dict[str, Path]) -> None: + metadata = root / ".work-bundle/project.yaml" + metadata.parent.mkdir(parents=True, exist_ok=True) + lines = [ + "metadata_version: 3", + f"workspace_root: {root}", + "workspace_mode: multi-repository", + "source_repositories:", + ] + for repository_id, project_root in members.items(): + project_root.mkdir(parents=True, exist_ok=True) + lines.extend( + [ + f" - id: {repository_id}", + f" project_root: {project_root}", + f" origin: /origin/{repository_id}.git", + ] + ) + metadata.write_text("\n".join(lines) + "\n", encoding="utf-8") + + +def test_final_plan_workspace_uses_unanimous_accepted_repository_authority( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + members = { + "work-bundle-main": tmp_path / "work-bundle-main", + "execution-flow": tmp_path / "execution-flow", + "work-bundle-mcp": tmp_path / "work-bundle-mcp", + } + _write_multi_repository_workspace(tmp_path, members) + rows = [ + { + "type": "task", + "id": f"task-00{number}", + "plan_id": "plan-001", + "status": "Completed", + "path": f"task-00{number}.md", + } + for number in range(1, 4) + ] + monkeypatch.setattr(plans, "index_plans", lambda _args: rows) + monkeypatch.setattr(plans, "has_persisted_accepted_task_result", lambda *_args: True) + monkeypatch.setattr( + plans, + "artifact_path_from_row", + lambda row, _args: tmp_path / str(row["path"]), + ) + monkeypatch.setattr( + plans, + "_load_current_task_acceptance", + lambda _args, path: ( + { + "workspace_id": "workspace-001", + "execution_id": f"exec-{path.stem}", + "repository_id": "work-bundle-main", + "runtime_root": str(tmp_path / "runtime"), + }, + {"schema": "accepted-task-result-v1"}, + ), + ) + + selected = plans._resolve_final_plan_workspace( + argparse.Namespace( + project_root=str(tmp_path), + workspace_id="workspace-001", + execution_id="exec-task-003", + repository_id=None, + execution_runtime_root=str(tmp_path / "runtime"), + ), + "plan-001", + ) + + assert selected == members["work-bundle-main"].resolve() + assert selected != Path("/origin/work-bundle-main.git") + + +@pytest.mark.parametrize( + ("repository_ids", "selector", "message"), + [ + ( + ["work-bundle-main", "execution-flow"], + None, + "accepted task repository authority disagrees", + ), + ( + ["work-bundle-main", "work-bundle-main"], + "execution-flow", + "repository selector conflicts with accepted task authority", + ), + ( + ["missing-member", "missing-member"], + None, + "authorized final plan repository is not a registered local member", + ), + ], +) +def test_final_plan_workspace_rejects_conflict_or_missing_member( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + repository_ids: list[str], + selector: str | None, + message: str, +) -> None: + members = { + "work-bundle-main": tmp_path / "work-bundle-main", + "execution-flow": tmp_path / "execution-flow", + "work-bundle-mcp": tmp_path / "work-bundle-mcp", + } + _write_multi_repository_workspace(tmp_path, members) + rows = [ + { + "type": "task", + "id": f"task-00{number}", + "plan_id": "plan-001", + "status": "Completed", + "path": f"task-00{number}.md", + } + for number in range(1, 3) + ] + monkeypatch.setattr(plans, "index_plans", lambda _args: rows) + monkeypatch.setattr(plans, "has_persisted_accepted_task_result", lambda *_args: True) + monkeypatch.setattr( + plans, + "artifact_path_from_row", + lambda row, _args: tmp_path / str(row["path"]), + ) + monkeypatch.setattr( + plans, + "_load_current_task_acceptance", + lambda _args, path: ( + { + "workspace_id": "workspace-001", + "execution_id": f"exec-{path.stem}", + "repository_id": repository_ids[int(path.stem[-1]) - 1], + "runtime_root": str(tmp_path / "runtime"), + }, + {"schema": "accepted-task-result-v1"}, + ), + ) + + with pytest.raises(SystemExit, match=message): + plans._resolve_final_plan_workspace( + argparse.Namespace(project_root=str(tmp_path), repository_id=selector), + "plan-001", + ) + + def test_archive_knowledge_gate_aggregates_new_results_and_bounds_legacy_bridge( tmp_path: Path, ) -> None: From e99ba1c94bf856a035385b5cf76f36fae7070944 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=88=98=E5=8D=87=E9=BE=99?= Date: Wed, 9 Sep 2026 14:02:52 +0800 Subject: [PATCH 18/18] fix(orchestration): enforce final selector coherence --- scripts/orchestration/plans.py | 46 ++++++----- ...orchestration_accepted_result_lifecycle.py | 76 +++++++++++++++++++ 2 files changed, 104 insertions(+), 18 deletions(-) diff --git a/scripts/orchestration/plans.py b/scripts/orchestration/plans.py index 8a2ba90..a55e08b 100644 --- a/scripts/orchestration/plans.py +++ b/scripts/orchestration/plans.py @@ -434,8 +434,8 @@ def _registered_repository_roots(workspace: Path) -> dict[str, Path]: # appear in this intersection. if candidate not in member_roots: continue - if repository_id in registered and registered[repository_id] != candidate: - raise SystemExit("acceptance-blocked: registered repository identity is ambiguous") + if repository_id in registered: + raise SystemExit("acceptance-blocked: registered repository identity is duplicated") registered[repository_id] = candidate return registered @@ -448,22 +448,32 @@ def _validate_final_workspace_selectors( "execution_id": getattr(args, "execution_id", None), "runtime_root": getattr(args, "execution_runtime_root", None), } - for field, supplied in selectors.items(): - if not supplied: - continue - if field == "runtime_root": - expected = Path(str(supplied)).expanduser().resolve() - matches = [ - binding - for binding in bindings - if Path(str(binding.get(field) or "")).expanduser().resolve() == expected - ] - else: - matches = [binding for binding in bindings if str(binding.get(field) or "") == str(supplied)] - if not matches: - raise SystemExit( - f"acceptance-blocked: {field.replace('_', ' ')} selector conflicts with accepted task authority" - ) + supplied = {field: value for field, value in selectors.items() if value} + if not supplied: + return + + def matches(binding: dict[str, object]) -> bool: + for field, value in supplied.items(): + actual = binding.get(field) + if field == "runtime_root": + if Path(str(actual or "")).expanduser().resolve() != Path( + str(value) + ).expanduser().resolve(): + return False + elif str(actual or "") != str(value): + return False + return True + + if any(matches(binding) for binding in bindings): + return + if len(supplied) == 1: + field = next(iter(supplied)) + raise SystemExit( + f"acceptance-blocked: {field.replace('_', ' ')} selector conflicts with accepted task authority" + ) + raise SystemExit( + "acceptance-blocked: selector tuple conflicts with accepted task authority" + ) def _resolve_final_plan_workspace( diff --git a/tests/test_orchestration_accepted_result_lifecycle.py b/tests/test_orchestration_accepted_result_lifecycle.py index 4bd1e64..11a97c9 100644 --- a/tests/test_orchestration_accepted_result_lifecycle.py +++ b/tests/test_orchestration_accepted_result_lifecycle.py @@ -373,6 +373,82 @@ def test_final_plan_workspace_rejects_conflict_or_missing_member( ) +def test_final_plan_workspace_rejects_selectors_split_across_accepted_bindings( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + members = { + "work-bundle-main": tmp_path / "work-bundle-main", + "execution-flow": tmp_path / "execution-flow", + "work-bundle-mcp": tmp_path / "work-bundle-mcp", + } + _write_multi_repository_workspace(tmp_path, members) + rows = [ + { + "type": "task", + "id": f"task-00{number}", + "plan_id": "plan-001", + "status": "Completed", + "path": f"task-00{number}.md", + } + for number in range(1, 3) + ] + monkeypatch.setattr(plans, "index_plans", lambda _args: rows) + monkeypatch.setattr(plans, "has_persisted_accepted_task_result", lambda *_args: True) + monkeypatch.setattr( + plans, + "artifact_path_from_row", + lambda row, _args: tmp_path / str(row["path"]), + ) + monkeypatch.setattr( + plans, + "_load_current_task_acceptance", + lambda _args, path: ( + { + "workspace_id": "workspace-A" if path.stem.endswith("1") else "workspace-B", + "execution_id": "execution-A" if path.stem.endswith("1") else "execution-B", + "repository_id": "work-bundle-main", + "runtime_root": str( + tmp_path / ("runtime-A" if path.stem.endswith("1") else "runtime-B") + ), + }, + {"schema": "accepted-task-result-v1"}, + ), + ) + + with pytest.raises(SystemExit, match="selector tuple conflicts"): + plans._resolve_final_plan_workspace( + argparse.Namespace( + project_root=str(tmp_path), + workspace_id="workspace-A", + execution_id="execution-B", + repository_id=None, + execution_runtime_root=str(tmp_path / "runtime-A"), + ), + "plan-001", + ) + + +def test_registered_repository_roots_rejects_duplicate_identity_at_same_root( + tmp_path: Path, +) -> None: + shared = tmp_path / "work-bundle-main" + members = { + "work-bundle-main": shared, + "execution-flow": tmp_path / "execution-flow", + } + _write_multi_repository_workspace(tmp_path, members) + metadata = tmp_path / ".work-bundle/project.yaml" + metadata.write_text( + metadata.read_text(encoding="utf-8") + + " - id: work-bundle-main\n" + + f" project_root: {shared}\n", + encoding="utf-8", + ) + + with pytest.raises(SystemExit, match="registered repository identity is duplicated"): + plans._registered_repository_roots(tmp_path) + + def test_archive_knowledge_gate_aggregates_new_results_and_bounds_legacy_bridge( tmp_path: Path, ) -> None: