From 824d04799d97d8739321e157dbd0f155cf62522e Mon Sep 17 00:00:00 2001 From: Andrei Hasna Date: Mon, 10 Aug 2026 02:07:09 +0300 Subject: [PATCH] fix(merge-pr): verify reviewer runtime attestation Agent: severianus --- .codewith/skills/merge-pr/SKILL.md | 8 +- .../merge-pr/references/merge-safety.md | 2 +- .../merge-pr/scripts/merge_pr_execute.py | 14 + .../merge-pr/scripts/merge_pr_preflight.py | 209 +++++++- .../merge-pr/tests/test_merge_pr_execute.py | 184 ++++++- .../merge-pr/tests/test_merge_pr_preflight.py | 478 +++++++++++++++++- 6 files changed, 860 insertions(+), 35 deletions(-) diff --git a/.codewith/skills/merge-pr/SKILL.md b/.codewith/skills/merge-pr/SKILL.md index 54691b9b7..15aed7487 100644 --- a/.codewith/skills/merge-pr/SKILL.md +++ b/.codewith/skills/merge-pr/SKILL.md @@ -22,7 +22,7 @@ Actual merge means `immediate-merge`, `auto-merge`, or `merge-queue`. 1. For actual merge, create a native Codewith goal plan when available. Do not clear or replace an active goal unless the user explicitly authorizes it. The plan must track: `preflight`, `reviewer-artifact`, `executor-recheck-merge`, and `postverify`. 2. Require one independent reviewer artifact from the fixed reviewer run tied to the exact PR head SHA. Self-review is not an acceptable fallback for actual merge. If that independent artifact cannot be obtained, stop before merge. A coordinator may deliberately fix additional reviewers before review begins, but the skill never requires more than one; every supplied artifact must have a distinct reviewer identity or run id. -3. Each reviewer artifact must include: repository, PR number, exact head SHA, reviewer identity or run id, timestamp, verdict, checked risks summary, and blocking findings. Treat missing, invalid, future, or stale artifact timestamps as blockers; the helper default staleness window is 24 hours unless `--max-artifact-age-hours` is explicitly set. +3. Each reviewer artifact must bind one receipt containing: repository, PR number, reviewed base SHA, exact head SHA, merge-result tree SHA, reviewer identity and run id, receipt id, workflow-required provider/model, runtime provider/model/thread claims, timestamp, verdict, remediation cycle, checked risks summary, and blocking findings. The helper resolves the reviewer run id through package-owned `codewith agent read --json`; that authoritative read must report the same run id, the same thread id claimed by the artifact, terminal `completed` state, an attestation for the same run, the workflow-required applied provider, and the workflow-required bound runtime model. Artifact-authored provenance fields are claims to compare, never authority. The provenance timestamp must match the review timestamp. Missing runtime state, failed reads, unavailable-provider substitution, mismatches, or missing/invalid/future/stale timestamps block merge; the helper default staleness window is 24 hours unless `--max-artifact-age-hours` is explicitly set. 4. The executor must re-fetch and re-check immediately before merge. A preflight JSON snapshot is advisory only and is not authority to merge. 5. Immediate squash merges must run through `scripts/merge_pr_execute.py`, which holds one exclusive `github-merge/OWNER/REPO/refs/heads/BASE` resource lock across authoritative recheck, exact base/head/tree validation, merge, and postverification. Pass `--lock-owner registered-agent/session-id`; the executable appends a unique invocation id so even parallel merges from one session cannot refresh the same ownership record. 6. The merge command must include `gh pr merge --match-head-commit ` and must never include `--admin`. @@ -35,7 +35,7 @@ Actual merge means `immediate-merge`, `auto-merge`, or `merge-queue`. 4. For actual merge, create or append to the native Codewith goal plan without replacing an active goal unless authorized. Record the PR head SHA in the plan. 5. Obtain one independent reviewer artifact for that exact head SHA from the reviewer fixed before review begins. Treat missing, stale, duplicate-run, self-review, or blocking-verdict artifacts as blockers. If the coordinator deliberately fixed additional reviewers before review began, validate every supplied artifact without turning the optional additions into a requirement. 6. Executor recheck immediately before merge: - - For immediate squash, invoke `scripts/merge_pr_execute.py` with the reviewed base SHA, head SHA, merge-result tree SHA, reviewer artifact, merge body file, and a unique lock owner. The executable acquires the stable repository/default-branch lock before the authoritative checks. + - For immediate squash, invoke `scripts/merge_pr_execute.py` with the reviewed base SHA, head SHA, merge-result tree SHA, reviewer artifact, workflow-declared `--required-reviewer-provider` and `--required-reviewer-model`, merge body file, and a unique lock owner. The executable refuses missing or mismatched reviewer route requirements before mutation, then acquires the stable repository/default-branch lock before the authoritative checks. - `git fetch` or equivalent remote refresh. - Re-read PR state, head SHA, base, mergeability, checks, reviews, draft/conflict state, and queue/protection behavior. - Verify the current base SHA, head SHA, and computed merge-result tree still equal the reviewed candidate. If any changed, stop and get a new reviewer artifact from the fixed reviewer set. @@ -88,5 +88,5 @@ Record the PR state, merged commit or queue state, target branch state, CI/check Validate the contract with static checks and fixtures: -- Static checks: trigger text present, `scancommitpr` non-goal present, four modes present, one independent reviewer artifact required, no self-review fallback for actual merge, optional additional artifacts remain distinct, executor recheck required, stable branch lock required for immediate squash, exact base/head/tree required, `--match-head-commit` required, `--admin` forbidden, postverify fields present. -- Fixtures: green PR with one artifact, green PR with two distinct optional artifacts, no reviewer artifacts, malformed or stale artifact, wrong repository/PR/head artifact, duplicate reviewer identity, pending checks, failed checks, requested changes, draft PR, conflict PR, merge queue passed checks, merge queue pending checks, explicit merge-when-green, head SHA changes between preflight and executor, branch protection queue, unchanged-base immediate squash, two concurrent candidates on one repository/default branch, moved-base retry refusal, and no generated command containing `--admin`. +- Static checks: trigger text present, `scancommitpr` non-goal present, four modes present, one independent reviewer artifact required, no self-review fallback for actual merge, optional additional artifacts remain distinct, workflow-declared reviewer provider/model required, authoritative `codewith agent read` validation bound to the reviewer run, thread, attestation, and receipt claims, executor recheck required, stable branch lock required for immediate squash, exact base/head/tree required, `--match-head-commit` required, `--admin` forbidden, postverify fields present. +- Fixtures: required Fable plus OpenAI rejected, required Fable plus verified Fable accepted, unavailable required provider refusing substitution, forged artifact claims rejected by authoritative runtime readback, missing/failed/nonterminal reviewer runs rejected, missing or mismatched attestation/run/thread/provider/model rejected, green PR with one artifact, green PR with two distinct optional artifacts, no reviewer artifacts, malformed or stale artifact, wrong repository/PR/head/base/tree artifact, duplicate reviewer run, pending checks, failed checks, requested changes, draft PR, conflict PR, merge queue passed checks, merge queue pending checks, explicit merge-when-green, head SHA changes between preflight and executor, branch protection queue, unchanged-base immediate squash, two concurrent candidates on one repository/default branch, moved-base retry refusal, remediation cycles 0..2 preserved, and no generated command containing `--admin`. diff --git a/.codewith/skills/merge-pr/references/merge-safety.md b/.codewith/skills/merge-pr/references/merge-safety.md index 9c58d1bda..11cc6e669 100644 --- a/.codewith/skills/merge-pr/references/merge-safety.md +++ b/.codewith/skills/merge-pr/references/merge-safety.md @@ -6,7 +6,7 @@ These rules keep merge execution tied to fresh evidence rather than stale snapsh - Check summaries are decision evidence, not navigation evidence. Project GitHub check reads at the CLI boundary to `name,state,bucket`, retain only named decision fields, and never request or persist check links, `detailsUrl`, raw `statusCheckRollup`, or other URL-bearing check fields. - Executor recheck is authoritative. Immediately before any actual merge action, re-fetch and re-read PR state, head SHA, mergeability, checks, reviews, draft/conflict state, and queue/protection behavior. - Immediate squash merges use the package-owned `merge_pr_execute.py` path. It acquires one exclusive lock keyed to `github-merge/OWNER/REPO/refs/heads/BASE` before authoritative recheck and holds it through exact base/head/tree validation, merge, and postverification. The caller supplies its registered agent/session identity and the executable appends a unique invocation id, so parallel work from one session cannot refresh the same ownership record. -- Actual merge requires one independent reviewer artifact from the reviewer fixed before review begins for the exact PR head SHA. A coordinator may deliberately fix additional reviewers before review begins, but the skill never requires more than one and every supplied artifact must identify a distinct reviewer run. Missing, duplicate-run, self-review, stale-head, blocking-verdict, or blocking-finding artifacts stop the merge. +- Actual merge requires one independent reviewer artifact from the reviewer fixed before review begins. One validated receipt binds workflow-required provider/model, reviewer run and receipt ids, claimed runtime thread/provider/model, review timestamp, exact base/head/merge-result tree, verdict, and remediation cycle. The package-owned `codewith agent read --json` result is the runtime authority: run id, thread id, completed state, attestation run id, applied provider, and bound runtime model must corroborate the artifact and workflow. Artifact-authored provenance booleans and strings are claims only. The executor fails closed on read failure, missing runtime state, unavailable-provider substitution, mismatch, duplicate runs, stale candidate identity, blocking verdict, or blocking findings. A coordinator may deliberately fix additional reviewers before review begins, but the skill never requires more than one and every supplied artifact must identify a distinct reviewer run. - Reviewer artifact freshness is part of merge safety. Missing, invalid, future, or stale timestamps are blockers; the helper default staleness window is 24 hours unless explicitly overridden for a run. - Every merge command must include `gh pr merge --match-head-commit `. This protects against merging a changed PR head after review. - `--match-head-commit` pins only the PR head. GitHub's supported merge input exposes no expected base OID, so the branch lock serializes participating merge-pr executors but cannot exclude a nonparticipating external actor. Postverify must continue comparing the merged commit's first parent and tree with the reviewed base and tree, and must report any mismatch rather than treating the merge command's success as completion. diff --git a/.codewith/skills/merge-pr/scripts/merge_pr_execute.py b/.codewith/skills/merge-pr/scripts/merge_pr_execute.py index 3c3801214..3e948b437 100755 --- a/.codewith/skills/merge-pr/scripts/merge_pr_execute.py +++ b/.codewith/skills/merge-pr/scripts/merge_pr_execute.py @@ -229,6 +229,10 @@ def preflight_command(args: argparse.Namespace) -> list[str]: ] for artifact in args.artifact: command.extend(["--artifact", artifact]) + command.extend(["--expected-base", args.expected_base]) + command.extend(["--expected-tree", args.expected_tree]) + command.extend(["--required-reviewer-provider", args.required_reviewer_provider]) + command.extend(["--required-reviewer-model", args.required_reviewer_model]) command.extend(["--max-artifact-age-hours", str(args.max_artifact_age_hours)]) return command @@ -715,6 +719,16 @@ def main() -> int: default=[], help="Independent reviewer artifact JSON; may be repeated", ) + parser.add_argument( + "--required-reviewer-provider", + required=True, + help="Workflow-declared reviewer provider required by the validated artifact", + ) + parser.add_argument( + "--required-reviewer-model", + required=True, + help="Workflow-declared reviewer model required by the validated artifact", + ) parser.add_argument("--body-file", required=True, help="Merge body ending in one Agent trailer") parser.add_argument("--subject", help="Optional squash merge subject") parser.add_argument( diff --git a/.codewith/skills/merge-pr/scripts/merge_pr_preflight.py b/.codewith/skills/merge-pr/scripts/merge_pr_preflight.py index 5ebf67a4a..77c7687a8 100644 --- a/.codewith/skills/merge-pr/scripts/merge_pr_preflight.py +++ b/.codewith/skills/merge-pr/scripts/merge_pr_preflight.py @@ -10,6 +10,7 @@ import argparse import json +import re import subprocess import sys from datetime import datetime, timedelta, timezone @@ -18,6 +19,7 @@ VERDICTS = {"mergeable", "not_mergeable", "needs_review", "pending", "unknown"} +SHA_PATTERN = re.compile(r"^[0-9a-f]{40}$") ACTUAL_MERGE_MODES = {"immediate-merge", "auto-merge", "merge-queue"} APPROVED_REVIEW_DECISIONS = {"APPROVED"} BLOCKING_REVIEW_DECISIONS = {"CHANGES_REQUESTED", "REVIEW_REQUIRED"} @@ -105,6 +107,35 @@ def parse_timestamp(value: Any) -> datetime | None: return parsed.astimezone(timezone.utc) +def normalize_requirement(value: Any) -> str | None: + if not isinstance(value, str): + return None + normalized = value.strip().casefold() + return normalized or None + + +def reviewer_run_readback(reviewer_run_id: str) -> tuple[dict[str, Any] | None, str | None]: + code, out, _ = run_command(["codewith", "agent", "read", reviewer_run_id, "--json"]) + if code != 0: + return None, "read_failed" + payload = _loads_or_none(out) + if not isinstance(payload, dict): + return None, "read_failed" + agent = payload.get("agent") if "agent" in payload else payload + if agent is None: + return None, "not_found" + if not isinstance(agent, dict): + return None, "read_failed" + return agent, None + + +def execution_context(value: Any) -> dict[str, Any]: + if not isinstance(value, dict): + return {} + context = value.get("executionContext") + return context if isinstance(context, dict) else {} + + def review_decision(review_decision_value: str | None, reviews: list[dict[str, Any]]) -> tuple[str | None, list[str], list[str]]: blockers: list[str] = [] warnings: list[str] = [] @@ -260,6 +291,10 @@ def artifact_decision( *, actual_merge_mode: bool, max_age_hours: float, + expected_base: str | None = None, + expected_tree: str | None = None, + required_provider: str | None = None, + required_model: str | None = None, ) -> tuple[list[str], list[str]]: blockers: list[str] = [] warnings: list[str] = [] @@ -270,8 +305,19 @@ def artifact_decision( target.append("no_reviewer_artifacts_provided") return blockers, warnings - identities: set[str] = set() + reviewer_runs: set[str] = set() valid_count = 0 + normalized_required_provider = normalize_requirement(required_provider) + normalized_required_model = normalize_requirement(required_model) + if actual_merge_mode: + if normalized_required_provider is None: + blockers.append("reviewer_provider_requirement_missing") + if normalized_required_model is None: + blockers.append("reviewer_model_requirement_missing") + if expected_base is None: + blockers.append("reviewed_base_requirement_missing") + if expected_tree is None: + blockers.append("merge_tree_requirement_missing") for index, artifact in enumerate(artifacts, start=1): prefix = f"artifact_{index}" artifact_repo = artifact.get("repository") or artifact.get("repo") @@ -288,11 +334,141 @@ def artifact_decision( blockers.append(f"{prefix}_missing_head_sha") elif artifact.get("head_sha") != head_sha: blockers.append(f"{prefix}_head_sha_mismatch") + reviewed_base_sha = artifact.get("reviewed_base_sha") + if not reviewed_base_sha: + blockers.append(f"{prefix}_missing_reviewed_base_sha") + elif not isinstance(reviewed_base_sha, str) or not SHA_PATTERN.fullmatch(reviewed_base_sha): + blockers.append(f"{prefix}_invalid_reviewed_base_sha") + elif expected_base and reviewed_base_sha != expected_base: + blockers.append(f"{prefix}_reviewed_base_sha_mismatch") + merge_tree_sha = artifact.get("merge_tree_sha") + if not merge_tree_sha: + blockers.append(f"{prefix}_missing_merge_tree_sha") + elif not isinstance(merge_tree_sha, str) or not SHA_PATTERN.fullmatch(merge_tree_sha): + blockers.append(f"{prefix}_invalid_merge_tree_sha") + elif expected_tree and merge_tree_sha != expected_tree: + blockers.append(f"{prefix}_merge_tree_sha_mismatch") identity = artifact.get("reviewer_identity") or artifact.get("reviewer_run_id") if not identity: blockers.append(f"{prefix}_missing_reviewer_identity") + reviewer_run_id = artifact.get("reviewer_run_id") + if not reviewer_run_id: + blockers.append(f"{prefix}_missing_reviewer_run_id") else: - identities.add(str(identity)) + reviewer_runs.add(str(reviewer_run_id)) + receipt_id = artifact.get("receipt_id") + if not receipt_id: + blockers.append(f"{prefix}_missing_receipt_id") + requirement = artifact.get("review_requirement") + if not isinstance(requirement, dict): + requirement = {} + artifact_required_provider = normalize_requirement(requirement.get("provider")) + artifact_required_model = normalize_requirement(requirement.get("model")) + if artifact_required_provider is None: + blockers.append(f"{prefix}_missing_required_provider") + elif artifact_required_provider != normalized_required_provider: + blockers.append(f"{prefix}_required_provider_mismatch") + if artifact_required_model is None: + blockers.append(f"{prefix}_missing_required_model") + elif artifact_required_model != normalized_required_model: + blockers.append(f"{prefix}_required_model_mismatch") + provenance = artifact.get("runtime_provenance") + if not isinstance(provenance, dict): + provenance = {} + runtime_provider = normalize_requirement(provenance.get("provider")) + runtime_model = normalize_requirement(provenance.get("model")) + if runtime_provider is None: + blockers.append(f"{prefix}_missing_runtime_provider") + if runtime_model is None: + blockers.append(f"{prefix}_missing_runtime_model") + if provenance.get("verified") is not True: + blockers.append(f"{prefix}_runtime_provenance_unverified") + if not provenance.get("source"): + blockers.append(f"{prefix}_missing_runtime_provenance_source") + runtime_thread_id = provenance.get("thread_id") + if not runtime_thread_id: + blockers.append(f"{prefix}_missing_runtime_thread_id") + raw_provenance_timestamp = provenance.get("observed_at") + provenance_timestamp = parse_timestamp(raw_provenance_timestamp) + if provenance_timestamp is None: + reason = "missing" if not raw_provenance_timestamp else "invalid" + blockers.append(f"{prefix}_{reason}_runtime_provenance_timestamp") + elif provenance_timestamp > now: + blockers.append(f"{prefix}_future_runtime_provenance_timestamp") + elif provenance_timestamp < freshness_cutoff: + blockers.append(f"{prefix}_stale_runtime_provenance_timestamp") + if normalized_required_provider and runtime_provider != normalized_required_provider: + blockers.append(f"{prefix}_reviewer_provider_mismatch") + if normalized_required_model and runtime_model != normalized_required_model: + blockers.append(f"{prefix}_reviewer_model_mismatch") + if reviewer_run_id: + authoritative_run, read_error = reviewer_run_readback(str(reviewer_run_id)) + if read_error == "not_found": + blockers.append(f"{prefix}_reviewer_run_not_found") + elif read_error: + blockers.append(f"{prefix}_reviewer_run_read_failed") + elif authoritative_run is not None: + authoritative_run_id = authoritative_run.get("agentId") + if authoritative_run_id != reviewer_run_id: + blockers.append(f"{prefix}_authoritative_run_id_mismatch") + if authoritative_run.get("status") != "completed": + blockers.append(f"{prefix}_reviewer_run_not_completed") + authoritative_thread_id = authoritative_run.get("threadId") + if not authoritative_thread_id: + blockers.append(f"{prefix}_missing_authoritative_thread_id") + elif runtime_thread_id and authoritative_thread_id != runtime_thread_id: + blockers.append(f"{prefix}_authoritative_thread_mismatch") + + attestation = authoritative_run.get("modelAttestation") + if not isinstance(attestation, dict): + blockers.append(f"{prefix}_missing_model_attestation") + else: + if attestation.get("agentId") != reviewer_run_id: + blockers.append(f"{prefix}_attestation_run_id_mismatch") + requested_context = execution_context(attestation.get("requestedConfiguration")) + applied_context = execution_context(attestation.get("appliedConfiguration")) + requested_provider = normalize_requirement(requested_context.get("provider")) + applied_provider = normalize_requirement(applied_context.get("provider")) + requested_model = normalize_requirement(attestation.get("requestedModel")) + applied_model = normalize_requirement(attestation.get("appliedModel")) + bound_runtime_model = normalize_requirement(attestation.get("boundRuntimeModel")) + + if requested_provider is None: + blockers.append(f"{prefix}_missing_authoritative_requested_provider") + elif ( + normalized_required_provider + and requested_provider != normalized_required_provider + ): + blockers.append(f"{prefix}_authoritative_requested_provider_mismatch") + if applied_provider is None: + blockers.append(f"{prefix}_missing_authoritative_applied_provider") + else: + if runtime_provider and applied_provider != runtime_provider: + blockers.append(f"{prefix}_runtime_provider_claim_mismatch") + if ( + normalized_required_provider + and applied_provider != normalized_required_provider + ): + blockers.append(f"{prefix}_authoritative_provider_mismatch") + + if requested_model is None: + blockers.append(f"{prefix}_missing_authoritative_requested_model") + elif normalized_required_model and requested_model != normalized_required_model: + blockers.append(f"{prefix}_authoritative_requested_model_mismatch") + if applied_model is None: + blockers.append(f"{prefix}_missing_authoritative_applied_model") + elif normalized_required_model and applied_model != normalized_required_model: + blockers.append(f"{prefix}_authoritative_applied_model_mismatch") + if bound_runtime_model is None: + blockers.append(f"{prefix}_missing_authoritative_bound_model") + else: + if runtime_model and bound_runtime_model != runtime_model: + blockers.append(f"{prefix}_runtime_model_claim_mismatch") + if ( + normalized_required_model + and bound_runtime_model != normalized_required_model + ): + blockers.append(f"{prefix}_authoritative_model_mismatch") raw_timestamp = artifact.get("timestamp") timestamp = parse_timestamp(raw_timestamp) if timestamp is None: @@ -302,6 +478,8 @@ def artifact_decision( blockers.append(f"{prefix}_future_timestamp") elif timestamp < freshness_cutoff: blockers.append(f"{prefix}_stale_timestamp") + if timestamp is not None and provenance_timestamp is not None and timestamp != provenance_timestamp: + blockers.append(f"{prefix}_provenance_timestamp_mismatch") if not artifact.get("checked_risks_summary"): blockers.append(f"{prefix}_missing_checked_risks_summary") verdict = str(artifact.get("verdict") or "").strip().lower() @@ -311,9 +489,16 @@ def artifact_decision( blockers.append(f"{prefix}_missing_blocking_findings") elif artifact.get("blocking_findings"): blockers.append(f"{prefix}_blocking_findings") + remediation_cycle = artifact.get("remediation_cycle") + if not isinstance(remediation_cycle, int) or isinstance(remediation_cycle, bool): + blockers.append(f"{prefix}_invalid_remediation_cycle") + elif remediation_cycle < 0: + blockers.append(f"{prefix}_invalid_remediation_cycle") + elif remediation_cycle > 2: + blockers.append(f"{prefix}_remediation_cycle_exceeded") valid_count += 1 - if len(identities) < valid_count: + if len(reviewer_runs) < valid_count: blockers.append("reviewer_artifacts_not_independent") return blockers, warnings @@ -400,6 +585,10 @@ def build_snapshot(args: argparse.Namespace) -> dict[str, Any]: head_sha, actual_merge_mode=args.mode in ACTUAL_MERGE_MODES, max_age_hours=args.max_artifact_age_hours, + expected_base=getattr(args, "expected_base", None), + expected_tree=getattr(args, "expected_tree", None), + required_provider=getattr(args, "required_reviewer_provider", None), + required_model=getattr(args, "required_reviewer_model", None), ) blocking_reasons.extend(merge_state_blockers) blocking_reasons.extend(review_blockers) @@ -452,6 +641,10 @@ def build_snapshot(args: argparse.Namespace) -> dict[str, Any]: "checks_error": checks_error, "reviews": pr_view.get("reviews") or [], "reviewer_artifacts": artifacts, + "review_requirement": { + "provider": getattr(args, "required_reviewer_provider", None), + "model": getattr(args, "required_reviewer_model", None), + }, "active_goal": None, "allowed_actions": allowed_actions, "blocking_reasons": blocking_reasons, @@ -480,6 +673,16 @@ def main() -> int: parser.add_argument("--mode", default="preflight", choices=["preflight", "immediate-merge", "auto-merge", "merge-queue"]) parser.add_argument("--fixture", help="Read PR/check data from fixture JSON instead of GitHub") parser.add_argument("--artifact", action="append", default=[], help="Reviewer artifact JSON file; may be repeated") + parser.add_argument("--expected-base", help="Reviewed base commit SHA required in each artifact") + parser.add_argument("--expected-tree", help="Reviewed merge-result tree SHA required in each artifact") + parser.add_argument( + "--required-reviewer-provider", + help="Workflow-declared reviewer provider; compared with verified runtime provenance", + ) + parser.add_argument( + "--required-reviewer-model", + help="Workflow-declared reviewer model; compared with verified runtime provenance", + ) parser.add_argument( "--max-artifact-age-hours", type=float, diff --git a/.codewith/skills/merge-pr/tests/test_merge_pr_execute.py b/.codewith/skills/merge-pr/tests/test_merge_pr_execute.py index 03a6d2886..fec326293 100644 --- a/.codewith/skills/merge-pr/tests/test_merge_pr_execute.py +++ b/.codewith/skills/merge-pr/tests/test_merge_pr_execute.py @@ -41,6 +41,7 @@ def __init__(self, root: Path) -> None: self.lock_dir = root / "locks" self.mutations = root / "mutations.jsonl" self.merge_calls = root / "merge-calls.jsonl" + self.agent_readbacks = root / "agent-readbacks.json" self.allow_first_merge = root / "allow-first-merge" self.allow_first_refresh = root / "allow-first-refresh" self.first_merge_entered = root / "first-merge-entered" @@ -95,6 +96,7 @@ def __init__(self, root: Path) -> None: ) self._write_executable("conversations", self._conversations_script()) self._write_executable("gh", self._gh_script()) + self._write_executable("codewith", self._codewith_script()) def _write_executable(self, name: str, content: str) -> None: path = self.bin / name @@ -327,26 +329,121 @@ def git(*git_args): """ ) - def artifact(self, pr_number: int) -> Path: + def _codewith_script(self) -> str: + return textwrap.dedent( + """\ + #!/usr/bin/env python3 + import json + import os + import sys + from pathlib import Path + + args = sys.argv[1:] + if len(args) != 4 or args[:2] != ["agent", "read"] or args[3] != "--json": + print("unexpected codewith invocation: " + " ".join(args), file=sys.stderr) + raise SystemExit(64) + run_id = args[2] + state_path = Path(os.environ["MERGE_FIXTURE_AGENT_READBACKS"]) + state = json.loads(state_path.read_text(encoding="utf-8")) + response = state.get(run_id) + if response is None: + print(json.dumps({ + "agent": None, + "statusSnapshot": None, + "executionSnapshot": None, + "pendingInteractions": [], + })) + raise SystemExit(0) + print(json.dumps(response)) + """ + ) + + def artifact( + self, + pr_number: int, + *, + provider: str = "fable", + model: str = "fable-4", + runtime_provider: str | None = None, + runtime_model: str | None = None, + ) -> Path: path = self.root / f"artifact-{pr_number}.json" + run_id = f"thread-reviewer-{pr_number}" + thread_id = run_id + authoritative_provider = runtime_provider or provider + authoritative_model = runtime_model or model + observed_at = datetime.now(timezone.utc)\ + .replace(microsecond=0)\ + .isoformat()\ + .replace("+00:00", "Z") path.write_text( json.dumps( { "repository": "hasna/codewith", "pr_number": pr_number, "head_sha": self.heads[pr_number], + "reviewed_base_sha": self.base, + "merge_tree_sha": self.trees[pr_number], "reviewer_identity": "reviewer-a", - "timestamp": datetime.now(timezone.utc) - .replace(microsecond=0) - .isoformat() - .replace("+00:00", "Z"), + "reviewer_run_id": run_id, + "receipt_id": "receipt-reviewer-a", + "review_requirement": { + "provider": "fable", + "model": "fable-4", + }, + "runtime_provenance": { + "provider": provider, + "model": model, + "verified": True, + "source": "codewith agent read", + "thread_id": thread_id, + "observed_at": observed_at, + }, + "timestamp": observed_at, "verdict": "approve", + "remediation_cycle": 0, "checked_risks_summary": "Exact candidate and merge safety gates.", "blocking_findings": [], } ), encoding="utf-8", ) + readbacks = ( + json.loads(self.agent_readbacks.read_text(encoding="utf-8")) + if self.agent_readbacks.exists() + else {} + ) + readbacks[run_id] = { + "agent": { + "agentId": run_id, + "threadId": thread_id, + "status": "completed", + "modelAttestation": { + "agentId": run_id, + "requestedModel": authoritative_model, + "appliedModel": authoritative_model, + "boundRuntimeModel": authoritative_model, + "requestedConfiguration": { + "executionContext": { + "provider": authoritative_provider, + "model": authoritative_model, + "serviceTier": "priority", + } + }, + "appliedConfiguration": { + "executionContext": { + "provider": authoritative_provider, + "model": authoritative_model, + "serviceTier": "priority", + } + }, + }, + }, + "statusSnapshot": None, + "executionSnapshot": None, + "pendingInteractions": [], + } + self.agent_readbacks.write_text(json.dumps(readbacks), encoding="utf-8") return path def body(self, pr_number: int) -> Path: @@ -369,6 +466,7 @@ def env( "PATH": f"{self.bin}:{os.environ['PATH']}", "MERGE_FIXTURE_ALLOW_FIRST": str(self.allow_first_merge), "MERGE_FIXTURE_ALLOW_FIRST_REFRESH": str(self.allow_first_refresh), + "MERGE_FIXTURE_AGENT_READBACKS": str(self.agent_readbacks), "MERGE_FIXTURE_BLOCK_FIRST": "1" if block_first else "0", "MERGE_FIXTURE_BLOCK_FIRST_REFRESH": "1" if block_first_refresh else "0", "MERGE_FIXTURE_EXTERNAL_MOVE": "1" if external_move else "0", @@ -381,7 +479,16 @@ def env( "MERGE_FIXTURE_STATE": str(self.state), } - def command(self, pr_number: int, lock_owner: str) -> list[str]: + def command( + self, + pr_number: int, + lock_owner: str, + *, + provider: str = "fable", + model: str = "fable-4", + runtime_provider: str | None = None, + runtime_model: str | None = None, + ) -> list[str]: return [ sys.executable, str(SCRIPT), @@ -395,7 +502,19 @@ def command(self, pr_number: int, lock_owner: str) -> list[str]: "--expected-tree", self.trees[pr_number], "--artifact", - str(self.artifact(pr_number)), + str( + self.artifact( + pr_number, + provider=provider, + model=model, + runtime_provider=runtime_provider, + runtime_model=runtime_model, + ) + ), + "--required-reviewer-provider", + "fable", + "--required-reviewer-model", + "fable-4", "--body-file", str(self.body(pr_number)), "--lock-owner", @@ -422,6 +541,57 @@ def merge_call_rows(self) -> list[list[str]]: class MergePrExecuteTests(unittest.TestCase): + def test_forged_fable_artifact_with_openai_readback_blocks_before_merge(self) -> None: + with TemporaryDirectory() as directory: + fixture = MergeFixture(Path(directory)) + result = subprocess.run( + fixture.command( + 1, + "severianus/session-a", + runtime_provider="openai", + runtime_model="gpt-5.6-sol", + ), + cwd=fixture.root, + env=fixture.env(), + check=False, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + ) + + self.assertEqual(result.returncode, 4, result.stderr) + output = json.loads(result.stdout) + self.assertEqual(output["status"], "preflight_blocked") + self.assertIn("artifact_1_authoritative_provider_mismatch", output["blocking_reasons"]) + self.assertIn("artifact_1_authoritative_model_mismatch", output["blocking_reasons"]) + self.assertEqual(fixture.mutation_rows(), []) + self.assertEqual(fixture.merge_call_rows(), []) + + def test_required_fable_blocks_openai_artifact_before_merge(self) -> None: + with TemporaryDirectory() as directory: + fixture = MergeFixture(Path(directory)) + result = subprocess.run( + fixture.command( + 1, + "severianus/session-a", + provider="openai", + model="gpt-5.6-sol", + ), + cwd=fixture.root, + env=fixture.env(), + check=False, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + ) + + self.assertEqual(result.returncode, 4, result.stderr) + output = json.loads(result.stdout) + self.assertEqual(output["status"], "preflight_blocked") + self.assertIn("artifact_1_reviewer_provider_mismatch", output["blocking_reasons"]) + self.assertEqual(fixture.mutation_rows(), []) + self.assertEqual(fixture.merge_call_rows(), []) + def test_unchanged_base_uses_exact_head_squash_path(self) -> None: with TemporaryDirectory() as directory: fixture = MergeFixture(Path(directory)) diff --git a/.codewith/skills/merge-pr/tests/test_merge_pr_preflight.py b/.codewith/skills/merge-pr/tests/test_merge_pr_preflight.py index b2e4deb32..7756e6e3a 100644 --- a/.codewith/skills/merge-pr/tests/test_merge_pr_preflight.py +++ b/.codewith/skills/merge-pr/tests/test_merge_pr_preflight.py @@ -4,9 +4,12 @@ import argparse import importlib.util import json +import os +import stat import subprocess import sys import tempfile +import textwrap import unittest from datetime import datetime, timezone from pathlib import Path @@ -29,17 +32,52 @@ def _load_module(): preflight = _load_module() -def run_preflight(fixture: str, mode: str = "preflight", artifacts: list[Path] | None = None) -> dict[str, object]: +def run_preflight( + fixture: str, + mode: str = "preflight", + artifacts: list[Path] | None = None, + *, + required_provider: str | None = None, + required_model: str | None = None, + expected_base: str | None = None, + expected_tree: str | None = None, + runtime_readbacks: dict[str, object] | None = None, +) -> dict[str, object]: + if mode in {"immediate-merge", "auto-merge", "merge-queue"}: + required_provider = "fable" if required_provider is None else required_provider + required_model = "fable-4" if required_model is None else required_model + expected_base = "a" * 40 if expected_base is None else expected_base + expected_tree = "b" * 40 if expected_tree is None else expected_tree command = [sys.executable, str(SCRIPT), "--fixture", str(FIXTURES / fixture), "--mode", mode] for artifact in artifacts or []: command.extend(["--artifact", str(artifact)]) - result = subprocess.run( - command, - check=True, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - text=True, - ) + if required_provider: + command.extend(["--required-reviewer-provider", required_provider]) + if required_model: + command.extend(["--required-reviewer-model", required_model]) + if expected_base: + command.extend(["--expected-base", expected_base]) + if expected_tree: + command.extend(["--expected-tree", expected_tree]) + with TemporaryDirectory() as directory: + root = Path(directory) + readbacks = runtime_readbacks + if readbacks is None: + readbacks = {} + for artifact_path in artifacts or []: + artifact = json.loads(artifact_path.read_text(encoding="utf-8")) + run_id = artifact.get("reviewer_run_id") + if isinstance(run_id, str) and run_id: + readbacks[run_id] = reviewer_run_readback(artifact) + env = fake_codewith_env(root, readbacks) + result = subprocess.run( + command, + check=True, + env=env, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + ) return json.loads(result.stdout) @@ -50,15 +88,36 @@ def current_timestamp() -> str: def reviewer_artifact( reviewer_identity: str, *, + provider: str = "fable", + model: str = "fable-4", + remediation_cycle: int = 0, include_blocking_findings: bool = True, ) -> dict[str, object]: + observed_at = current_timestamp() artifact: dict[str, object] = { "repository": "hasna/codewith", "pr_number": 215, "head_sha": "1111111111111111111111111111111111111111", + "reviewed_base_sha": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "merge_tree_sha": "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", "reviewer_identity": reviewer_identity, - "timestamp": current_timestamp(), + "reviewer_run_id": f"thread-{reviewer_identity}", + "receipt_id": f"receipt-{reviewer_identity}", + "review_requirement": { + "provider": "fable", + "model": "fable-4", + }, + "runtime_provenance": { + "provider": provider, + "model": model, + "verified": True, + "source": "codewith agent read", + "thread_id": f"thread-{reviewer_identity}", + "observed_at": observed_at, + }, + "timestamp": observed_at, "verdict": "approve", + "remediation_cycle": remediation_cycle, "checked_risks_summary": "Focused merge preflight artifact validation.", } if include_blocking_findings: @@ -66,6 +125,105 @@ def reviewer_artifact( return artifact +def reviewer_run_readback( + artifact: dict[str, object], + *, + agent_id: str | None = None, + attestation_agent_id: str | None = None, + thread_id: str | None = None, + status: str = "completed", + provider: str | None = None, + model: str | None = None, + include_attestation: bool = True, +) -> dict[str, object]: + run_id = str(artifact.get("reviewer_run_id") or "") + provenance = artifact.get("runtime_provenance") + provenance = provenance if isinstance(provenance, dict) else {} + resolved_provider = provider or str(provenance.get("provider") or "") + resolved_model = model or str(provenance.get("model") or "") + resolved_thread_id = thread_id or str(provenance.get("thread_id") or "") + agent: dict[str, object] = { + "agentId": agent_id or run_id, + "threadId": resolved_thread_id, + "status": status, + } + if include_attestation: + agent["modelAttestation"] = { + "agentId": attestation_agent_id or run_id, + "requestedModel": resolved_model, + "appliedModel": resolved_model, + "boundRuntimeModel": resolved_model, + "requestedConfiguration": { + "executionContext": { + "provider": resolved_provider, + "model": resolved_model, + "serviceTier": "priority", + } + }, + "appliedConfiguration": { + "executionContext": { + "provider": resolved_provider, + "model": resolved_model, + "serviceTier": "priority", + } + }, + } + return { + "agent": agent, + "statusSnapshot": None, + "executionSnapshot": None, + "pendingInteractions": [], + } + + +def fake_codewith_env(root: Path, readbacks: dict[str, object]) -> dict[str, str]: + bin_dir = root / "bin" + bin_dir.mkdir() + state = root / "agent-readbacks.json" + state.write_text(json.dumps(readbacks), encoding="utf-8") + executable = bin_dir / "codewith" + executable.write_text( + textwrap.dedent( + """\ + #!/usr/bin/env python3 + import json + import os + import sys + from pathlib import Path + + args = sys.argv[1:] + if len(args) != 4 or args[:2] != ["agent", "read"] or args[3] != "--json": + print("unexpected codewith invocation: " + " ".join(args), file=sys.stderr) + raise SystemExit(64) + run_id = args[2] + state = json.loads( + Path(os.environ["MERGE_FIXTURE_AGENT_READBACKS"]).read_text(encoding="utf-8") + ) + response = state.get(run_id) + if response is None: + print(json.dumps({ + "agent": None, + "statusSnapshot": None, + "executionSnapshot": None, + "pendingInteractions": [], + })) + raise SystemExit(0) + if isinstance(response, dict) and "__error__" in response: + print(str(response["__error__"]), file=sys.stderr) + raise SystemExit(int(response.get("__returncode__", 1))) + print(json.dumps(response)) + """ + ), + encoding="utf-8", + ) + executable.chmod(executable.stat().st_mode | stat.S_IXUSR) + return { + **os.environ, + "PATH": f"{bin_dir}:{os.environ['PATH']}", + "MERGE_FIXTURE_AGENT_READBACKS": str(state), + } + + def write_artifact(directory: Path, name: str, artifact: dict[str, object]) -> Path: path = directory / name path.write_text(json.dumps(artifact), encoding="utf-8") @@ -193,6 +351,291 @@ def test_one_valid_reviewer_artifact_allows_actual_merge(self) -> None: self.assertEqual(snapshot["verdict"], "mergeable") self.assertEqual(snapshot["blocking_reasons"], []) + def test_required_fable_rejects_openai_reviewer_provenance(self) -> None: + with TemporaryDirectory() as directory: + artifact = write_artifact( + Path(directory), + "openai-review.json", + reviewer_artifact("reviewer-a", provider="openai", model="gpt-5.6-sol"), + ) + snapshot = run_preflight( + "checks_success_current.json", + mode="immediate-merge", + artifacts=[artifact], + required_provider="fable", + required_model="fable-4", + expected_base="a" * 40, + expected_tree="b" * 40, + ) + + self.assertEqual(snapshot["verdict"], "not_mergeable") + self.assertIn("artifact_1_reviewer_provider_mismatch", snapshot["blocking_reasons"]) + self.assertIn("artifact_1_reviewer_model_mismatch", snapshot["blocking_reasons"]) + + def test_actual_merge_requires_declared_provider_model_base_and_tree(self) -> None: + blockers, _ = preflight.artifact_decision( + [reviewer_artifact("reviewer-a")], + "hasna/codewith", + 215, + "1" * 40, + actual_merge_mode=True, + max_age_hours=24.0, + ) + + self.assertIn("reviewer_provider_requirement_missing", blockers) + self.assertIn("reviewer_model_requirement_missing", blockers) + self.assertIn("reviewed_base_requirement_missing", blockers) + self.assertIn("merge_tree_requirement_missing", blockers) + + def test_required_fable_accepts_verified_fable_reviewer_provenance(self) -> None: + with TemporaryDirectory() as directory: + artifact = write_artifact( + Path(directory), + "fable-review.json", + reviewer_artifact("reviewer-a"), + ) + snapshot = run_preflight( + "checks_success_current.json", + mode="immediate-merge", + artifacts=[artifact], + required_provider="fable", + required_model="fable-4", + expected_base="a" * 40, + expected_tree="b" * 40, + ) + + self.assertEqual(snapshot["verdict"], "mergeable") + self.assertEqual(snapshot["blocking_reasons"], []) + + def test_distinct_run_and_thread_ids_pass_when_authoritative_read_matches(self) -> None: + with TemporaryDirectory() as directory: + artifact_data = reviewer_artifact("reviewer-a") + provenance = artifact_data["runtime_provenance"] + assert isinstance(provenance, dict) + provenance["thread_id"] = "reviewer-thread-a" + artifact = write_artifact(Path(directory), "distinct-run-thread.json", artifact_data) + snapshot = run_preflight( + "checks_success_current.json", + mode="immediate-merge", + artifacts=[artifact], + ) + + self.assertEqual(snapshot["verdict"], "mergeable") + self.assertEqual(snapshot["blocking_reasons"], []) + + def test_forged_artifact_provenance_is_rejected_by_authoritative_agent_read(self) -> None: + with TemporaryDirectory() as directory: + artifact_data = reviewer_artifact("reviewer-a") + artifact = write_artifact(Path(directory), "forged-fable-review.json", artifact_data) + runtime_readbacks = { + "thread-reviewer-a": reviewer_run_readback( + artifact_data, + provider="openai", + model="gpt-5.6-sol", + ) + } + snapshot = run_preflight( + "checks_success_current.json", + mode="immediate-merge", + artifacts=[artifact], + runtime_readbacks=runtime_readbacks, + ) + + self.assertEqual(snapshot["verdict"], "not_mergeable") + self.assertIn("artifact_1_authoritative_provider_mismatch", snapshot["blocking_reasons"]) + self.assertIn("artifact_1_authoritative_model_mismatch", snapshot["blocking_reasons"]) + + def test_authoritative_reviewer_run_failures_block(self) -> None: + with TemporaryDirectory() as directory: + artifact_data = reviewer_artifact("reviewer-a") + artifact = write_artifact(Path(directory), "review.json", artifact_data) + cases = { + "missing_run": ( + {}, + "artifact_1_reviewer_run_not_found", + ), + "failed_read": ( + { + "thread-reviewer-a": { + "__error__": "runtime read failed", + "__returncode__": 70, + } + }, + "artifact_1_reviewer_run_read_failed", + ), + "missing_attestation": ( + { + "thread-reviewer-a": reviewer_run_readback( + artifact_data, + include_attestation=False, + ) + }, + "artifact_1_missing_model_attestation", + ), + "nonterminal": ( + { + "thread-reviewer-a": reviewer_run_readback( + artifact_data, + status="running", + ) + }, + "artifact_1_reviewer_run_not_completed", + ), + "run_id_mismatch": ( + { + "thread-reviewer-a": reviewer_run_readback( + artifact_data, + agent_id="different-run", + ) + }, + "artifact_1_authoritative_run_id_mismatch", + ), + "attestation_run_id_mismatch": ( + { + "thread-reviewer-a": reviewer_run_readback( + artifact_data, + attestation_agent_id="different-run", + ) + }, + "artifact_1_attestation_run_id_mismatch", + ), + "thread_mismatch": ( + { + "thread-reviewer-a": reviewer_run_readback( + artifact_data, + thread_id="different-thread", + ) + }, + "artifact_1_authoritative_thread_mismatch", + ), + "provider_mismatch": ( + { + "thread-reviewer-a": reviewer_run_readback( + artifact_data, + provider="openai", + ) + }, + "artifact_1_authoritative_provider_mismatch", + ), + "model_mismatch": ( + { + "thread-reviewer-a": reviewer_run_readback( + artifact_data, + model="gpt-5.6-sol", + ) + }, + "artifact_1_authoritative_model_mismatch", + ), + } + for name, (runtime_readbacks, expected_blocker) in cases.items(): + with self.subTest(name=name): + snapshot = run_preflight( + "checks_success_current.json", + mode="immediate-merge", + artifacts=[artifact], + runtime_readbacks=runtime_readbacks, + ) + self.assertEqual(snapshot["verdict"], "not_mergeable") + self.assertIn(expected_blocker, snapshot["blocking_reasons"]) + + def test_unavailable_required_provider_refuses_unverified_substitution(self) -> None: + with TemporaryDirectory() as directory: + artifact_data = reviewer_artifact( + "reviewer-a", + provider="openai", + model="gpt-5.6-sol", + ) + provenance = artifact_data["runtime_provenance"] + assert isinstance(provenance, dict) + provenance["verified"] = False + provenance["substituted_for"] = "fable" + artifact = write_artifact(Path(directory), "substitute-review.json", artifact_data) + snapshot = run_preflight( + "checks_success_current.json", + mode="immediate-merge", + artifacts=[artifact], + required_provider="fable", + required_model="fable-4", + expected_base="a" * 40, + expected_tree="b" * 40, + ) + + self.assertEqual(snapshot["verdict"], "not_mergeable") + self.assertIn("artifact_1_runtime_provenance_unverified", snapshot["blocking_reasons"]) + self.assertIn("artifact_1_reviewer_provider_mismatch", snapshot["blocking_reasons"]) + + def test_receipt_binds_candidate_verdict_timestamp_and_runtime_provenance(self) -> None: + cases = { + "receipt": ({"receipt_id": ""}, "artifact_1_missing_receipt_id"), + "run": ({"reviewer_run_id": ""}, "artifact_1_missing_reviewer_run_id"), + "base": ({"reviewed_base_sha": "c" * 40}, "artifact_1_reviewed_base_sha_mismatch"), + "tree": ({"merge_tree_sha": "d" * 40}, "artifact_1_merge_tree_sha_mismatch"), + "head": ({"head_sha": "e" * 40}, "artifact_1_head_sha_mismatch"), + "timestamp": ({"timestamp": "2000-01-01T00:00:00Z"}, "artifact_1_stale_timestamp"), + "verdict": ({"verdict": "NO_GO"}, "artifact_1_non_passing_verdict"), + "provenance": ({"runtime_provenance": {}}, "artifact_1_missing_runtime_provider"), + "required_provider": ( + {"review_requirement": {"provider": "openai", "model": "fable-4"}}, + "artifact_1_required_provider_mismatch", + ), + } + with TemporaryDirectory() as directory: + for name, (changes, expected_blocker) in cases.items(): + with self.subTest(name=name): + artifact_data = reviewer_artifact("reviewer-a") + artifact_data.update(changes) + artifact = write_artifact(Path(directory), f"{name}.json", artifact_data) + snapshot = run_preflight( + "checks_success_current.json", + mode="immediate-merge", + artifacts=[artifact], + required_provider="fable", + required_model="fable-4", + expected_base="a" * 40, + expected_tree="b" * 40, + ) + + self.assertEqual(snapshot["verdict"], "not_mergeable") + self.assertIn(expected_blocker, snapshot["blocking_reasons"]) + + def test_one_reviewer_and_two_focused_remediation_cycles_are_preserved(self) -> None: + with TemporaryDirectory() as directory: + for remediation_cycle in (0, 1, 2): + with self.subTest(remediation_cycle=remediation_cycle): + artifact = write_artifact( + Path(directory), + f"cycle-{remediation_cycle}.json", + reviewer_artifact("reviewer-a", remediation_cycle=remediation_cycle), + ) + snapshot = run_preflight( + "checks_success_current.json", + mode="immediate-merge", + artifacts=[artifact], + required_provider="fable", + required_model="fable-4", + expected_base="a" * 40, + expected_tree="b" * 40, + ) + self.assertEqual(snapshot["verdict"], "mergeable") + + third_cycle = write_artifact( + Path(directory), + "cycle-3.json", + reviewer_artifact("reviewer-a", remediation_cycle=3), + ) + snapshot = run_preflight( + "checks_success_current.json", + mode="immediate-merge", + artifacts=[third_cycle], + required_provider="fable", + required_model="fable-4", + expected_base="a" * 40, + expected_tree="b" * 40, + ) + + self.assertEqual(snapshot["verdict"], "not_mergeable") + self.assertIn("artifact_1_remediation_cycle_exceeded", snapshot["blocking_reasons"]) + def test_single_invalid_reviewer_artifact_blocks_actual_merge(self) -> None: cases = { "wrong-repo": ({"repository": "hasna/not-codewith"}, "artifact_1_repo_mismatch"), @@ -240,21 +683,16 @@ def test_duplicate_reviewer_identity_blocks_optional_additional_artifact(self) - self.assertIn("reviewer_artifacts_not_independent", snapshot["blocking_reasons"]) def test_uppercase_pass_artifacts_allow_actual_merge(self) -> None: - artifact_base = { - "repository": "hasna/codewith", - "pr_number": 215, - "head_sha": "1111111111111111111111111111111111111111", - "timestamp": datetime.now(timezone.utc).replace(microsecond=0).isoformat().replace("+00:00", "Z"), - "verdict": "PASS", - "checked_risks_summary": "exact head, diff scope, checks, and secret posture reviewed", - "blocking_findings": [], - } with tempfile.TemporaryDirectory() as tmp_dir: tmp_path = Path(tmp_dir) first = tmp_path / "artifact-1.json" second = tmp_path / "artifact-2.json" - first.write_text(json.dumps({**artifact_base, "reviewer_identity": "reviewer-a"})) - second.write_text(json.dumps({**artifact_base, "reviewer_identity": "reviewer-b"})) + first_artifact = reviewer_artifact("reviewer-a") + first_artifact["verdict"] = "PASS" + second_artifact = reviewer_artifact("reviewer-b") + second_artifact["verdict"] = "PASS" + first.write_text(json.dumps(first_artifact)) + second.write_text(json.dumps(second_artifact)) snapshot = run_preflight( "checks_success_current.json",