From 848d7969728bdd77d88ebf06acac5e72d6357e0e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 12:43:03 +0000 Subject: [PATCH 01/48] fix(ci): restore OpenCode coverage honesty and mermaid surfaces Stacked on main after #1360 squash 17052a7c. Coverage-evidence failure publishes a COMMENT product-file review then sets the status comment to COVERAGE_BLOCKED. Mermaid labels crates/packages instead of generic Changed file (N files) and does not invent class edges. Findings say Review process instead of opencode-review.yml:1 unless that file is in the diff. Does not touch noema-review.yml and is not NIM-2h. --- .../workflows/opencode-review-dispatch.yml | 155 ++-- CHANGELOG.md | 1 + ci-review-prompt.md | 9 + ...opencode-review-surfaces-originweave-47.md | 82 ++ scripts/ci/opencode_review_comment_helpers.sh | 108 +-- scripts/ci/opencode_review_prompt_template.md | 4 +- scripts/ci/opencode_review_surfaces.py | 720 ++++++++++++++++++ tests/test_opencode_review_comment_helpers.py | 54 ++ tests/test_opencode_review_surfaces.py | 692 +++++++++++++++++ 9 files changed, 1680 insertions(+), 145 deletions(-) create mode 100644 docs/doctoring/opencode-review-surfaces-originweave-47.md create mode 100644 scripts/ci/opencode_review_surfaces.py create mode 100644 tests/test_opencode_review_comment_helpers.py create mode 100644 tests/test_opencode_review_surfaces.py diff --git a/.github/workflows/opencode-review-dispatch.yml b/.github/workflows/opencode-review-dispatch.yml index 9c7d781e2d..b7befcf0fb 100644 --- a/.github/workflows/opencode-review-dispatch.yml +++ b/.github/workflows/opencode-review-dispatch.yml @@ -4759,15 +4759,14 @@ jobs: fi { - printf '\n' - printf '## OpenCode Review Overview\n\n' - printf -- "- Head SHA: \`%s\`\n" "$HEAD_SHA" - printf -- '- Workflow run: %s\n' "$RUN_ID" - printf -- '- Workflow attempt: %s\n' "$RUN_ATTEMPT" - printf -- "- Gate result: \`%s\` (exit %s)\n\n" "${gate_result:-UNKNOWN}" "$gate_status" - cat "$comment_body_file" - append_mermaid_review_graph - append_merge_conflict_guidance + python3 scripts/ci/opencode_review_surfaces.py build-status \ + --result "${gate_result:-UNKNOWN}" \ + --head-sha "$HEAD_SHA" \ + --run-id "$RUN_ID" \ + --run-attempt "$RUN_ATTEMPT" \ + --coverage-result "${COVERAGE_EVIDENCE_RESULT:-unknown}" \ + --model-pool-outcome "${OPENCODE_MODEL_POOL_OUTCOME:-unknown}" \ + --verdict "${gate_result:-UNKNOWN}" } >"$overview_body_file" live_head="$(gh api -X GET "repos/${GH_REPOSITORY}/pulls/${PR_NUMBER}" --jq '.head.sha // empty' 2>"$gh_error_file" || true)" @@ -5393,7 +5392,7 @@ jobs: . scripts/ci/opencode_review_comment_helpers.sh update_review_overview() { - local result="$1" body="$2" + local result="$1" local gh_error_file local overview_body_file local overview_comment_id @@ -5415,17 +5414,15 @@ jobs: return 1 fi { - printf '\n' - printf '## OpenCode Review Overview\n\n' - printf -- "- Head SHA: \`%s\`\n" "$HEAD_SHA" - printf -- '- Workflow run: %s\n' "$RUN_ID" - printf -- '- Workflow attempt: %s\n' "$RUN_ATTEMPT" - printf -- "- Gate result: \`%s\` (approval step)\n\n" "$result" - printf '%s\n' "$body" - if ! grep -Fq "## Changed-File Evidence Map" <<<"$body"; then - append_mermaid_review_graph - fi - append_merge_conflict_guidance + python3 scripts/ci/opencode_review_surfaces.py build-status \ + --result "$result" \ + --head-sha "$HEAD_SHA" \ + --run-id "$RUN_ID" \ + --run-attempt "$RUN_ATTEMPT" \ + --coverage-result "${COVERAGE_EVIDENCE_RESULT:-unknown}" \ + --model-pool-outcome "${OPENCODE_MODEL_POOL_OUTCOME:-unknown}" \ + --verdict "$result" \ + --formal-review-url "${FORMAL_REVIEW_URL:-}" } >"$overview_body_file" if ! overview_comment_id="$( @@ -5523,12 +5520,19 @@ jobs: esac exit 1 fi + FORMAL_REVIEW_URL="$(jq -r --arg repo "$GH_REPOSITORY" --arg pr "$PR_NUMBER" ' + if (.id // 0) > 0 then + "https://github.com/\($repo)/pull/\($pr)#pullrequestreview-\(.id)" + else + empty + end + ' "$review_response_file")" rm -f "$gh_error_file" "$review_payload_file" "$review_response_file" if [ "$event" = "APPROVE" ]; then printf '::notice::OpenCode approve review was published for head %s; skipping non-authoritative overview comment mutation so the required approval check can finish promptly.\n' "$HEAD_SHA" return 0 fi - update_review_overview "$event" "$body" + update_review_overview "$event" } emit_review_body_to_action_log() { @@ -5735,7 +5739,7 @@ jobs: "" \ "## Findings" \ "" \ - "### 1. HIGH .github/workflows/opencode-review.yml:1 - Unresolved reviewer thread blocks automated approval" \ + "### 1. HIGH Review process - Unresolved reviewer thread blocks automated approval" \ "- Problem: OpenCode reached an APPROVE control result, but the approval step found unresolved, non-outdated human or review-agent thread evidence on the current pull request." \ "- Root cause: Reviewer and review-agent feedback can arrive after bounded model evidence is prepared, so the approval step must re-query GitHub immediately before publishing an approval." \ "- Fix: Address or resolve the listed reviewer thread(s), then re-run OpenCode on the current head." \ @@ -5764,7 +5768,7 @@ jobs: "" \ "## Findings" \ "" \ - "### 1. HIGH .github/workflows/opencode-review.yml:1 - Review thread lookup could not be read before approval" \ + "### 1. HIGH Review process - Review thread lookup could not be read before approval" \ "- Problem: GitHub reviewThreads could not be read for the current pull request immediately before approval." \ "- Root cause: OpenCode cannot safely approve without verifying whether newer unresolved reviewer or review-agent feedback exists." \ "- Fix: Re-run OpenCode after GitHub reviewThreads are readable." \ @@ -5780,40 +5784,45 @@ jobs: build_coverage_evidence_check_failure_body() { local body_file="$1" - { - printf '%s\n' \ - "## Pull request overview" \ - "" \ - "OpenCode cannot approve yet because required coverage evidence did not pass." \ - "" \ - "## Review outcome" \ - "" \ - "### 1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence" \ - "- Problem: The required coverage-evidence job result was \`${COVERAGE_EVIDENCE_RESULT:-unknown}\`, so OpenCode cannot establish approval sufficiency for this head." \ - "- Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker." \ - "- Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports \`success\` with required evidence or explicit no-source not-applicable evidence." \ - "- Regression test: Keep the approval branch checking \`needs.coverage-evidence.result == success\` before posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present." \ - "" \ - "- Result: REQUEST_CHANGES" \ - "- Reason: coverage-evidence result was \`${COVERAGE_EVIDENCE_RESULT:-unknown}\`, so required test/docstring evidence was not proven for current head \`${HEAD_SHA}\`." \ - "- Head SHA: \`${HEAD_SHA}\`" \ - "- Workflow run: ${RUN_ID}" \ - "- Workflow attempt: ${RUN_ATTEMPT}" \ - "" \ - "## Coverage evidence" \ - "" - printf '%s\n' "${COVERAGE_EVIDENCE_SUMMARY:-Coverage evidence summary was unavailable.}" | sed -n '1,240p' - } >"$body_file" + python3 scripts/ci/opencode_review_surfaces.py build-status \ + --result "COVERAGE_BLOCKED" \ + --head-sha "$HEAD_SHA" \ + --run-id "$RUN_ID" \ + --run-attempt "$RUN_ATTEMPT" \ + --coverage-result "${COVERAGE_EVIDENCE_RESULT:-unknown}" \ + --coverage-summary "${COVERAGE_EVIDENCE_SUMMARY:-Coverage evidence summary was unavailable.}" \ + >"$body_file" + } + + publish_fallback_diff_review() { + local body_file event + body_file="$(mktemp)" + event="COMMENT" + python3 scripts/ci/opencode_review_surfaces.py build-fallback-review \ + --changed-files-file "${OPENCODE_CHANGED_FILES_FILE}" \ + --source-root "${OPENCODE_SOURCE_WORKDIR}" \ + --head-sha "$HEAD_SHA" \ + --run-id "$RUN_ID" \ + --run-attempt "$RUN_ATTEMPT" \ + --coverage-result "${COVERAGE_EVIDENCE_RESULT:-unknown}" \ + >"$body_file" + printf '\n%s\n\n%s\n' "## Review outcome" "Coverage is a gate, not the review. This body reviews the changed product files." >>"$body_file" + create_pull_review "$event" "$(cat "$body_file")" + # create_pull_review COMMENT rewrites the status comment to Gate + # result: COMMENT. Restore the coverage gate so a miss never looks + # finished; next action stays "fix coverage evidence, then rerun". + request_changes_for_coverage_evidence_failure + rm -f "$body_file" } request_changes_for_coverage_evidence_failure() { local body_file body_file="$(mktemp)" build_coverage_evidence_check_failure_body "$body_file" - create_pull_review "REQUEST_CHANGES" "$(cat "$body_file")" + printf '::notice::Coverage evidence did not pass (%s); approval is blocked. A source-backed review of changed product files is still published. record coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence in the status comment and refuse APPROVE while still publishing a source-backed review of changed product files.\n' \ + "${COVERAGE_EVIDENCE_RESULT:-unknown}" + update_review_overview "COVERAGE_BLOCKED" rm -f "$body_file" - echo "::endgroup::" - exit 0 } create_pull_review_with_payload() { @@ -5845,8 +5854,15 @@ jobs: fi return 1 fi + FORMAL_REVIEW_URL="$(jq -r --arg repo "$GH_REPOSITORY" --arg pr "$PR_NUMBER" ' + if (.id // 0) > 0 then + "https://github.com/\($repo)/pull/\($pr)#pullrequestreview-\(.id)" + else + empty + end + ' "$review_response_file")" rm -f "$gh_error_file" "$review_response_file" - update_review_overview "$event" "$body" + update_review_overview "$event" } request_changes_for_gate_failure() { @@ -5859,7 +5875,7 @@ jobs: "" \ "## Findings" \ "" \ - "### 1. HIGH .github/workflows/opencode-review.yml:1 - OpenCode review evidence was missing or invalid" \ + "### 1. HIGH Review process - OpenCode review evidence was missing or invalid" \ "- Problem: OpenCode review evidence was missing or invalid." \ "- Root cause: ${reason}" \ "- Fix: Re-run the OpenCode review after the current-head evidence and control block are available." \ @@ -7635,10 +7651,6 @@ jobs: exit 0 fi - if [ "${COVERAGE_EVIDENCE_RESULT:-skipped}" != "success" ]; then - request_changes_for_coverage_evidence_failure - fi - opencode_review_outcome="${OPENCODE_MODEL_POOL_OUTCOME:-unknown}" printf 'OpenCode model-pool outcome=%s model=%s; publish stage performs no duplicate model-catalog pass.\n' \ "$opencode_review_outcome" "${OPENCODE_MODEL_POOL_MODEL:-none}" @@ -7651,6 +7663,11 @@ jobs: echo "::endgroup::" exit 0 fi + if [ "${COVERAGE_EVIDENCE_RESULT:-skipped}" != "success" ]; then + publish_fallback_diff_review + echo "::endgroup::" + exit 1 + fi stop_without_review_after_model_unavailable fi @@ -7738,7 +7755,22 @@ jobs: case "$gate_result" in APPROVE) if [ "${COVERAGE_EVIDENCE_RESULT:-skipped}" != "success" ]; then + if [ -s "$tmp_body" ]; then + model_prose_file="$(mktemp)" + python3 scripts/ci/opencode_review_surfaces.py extract-prose \ + --model-body-file "$tmp_body" >"$model_prose_file" + if [ -s "$model_prose_file" ]; then + create_pull_review "COMMENT" "$(cat "$model_prose_file")" + else + publish_fallback_diff_review + fi + rm -f "$model_prose_file" + else + publish_fallback_diff_review + fi request_changes_for_coverage_evidence_failure + echo "::endgroup::" + exit 1 fi if request_changes_for_merge_conflict_if_present; then echo "::endgroup::" @@ -8011,11 +8043,20 @@ jobs: fi elif request_changes_for_merge_conflict_if_present; then : + elif [ "${COVERAGE_EVIDENCE_RESULT:-skipped}" != "success" ]; then + publish_fallback_diff_review + echo "::endgroup::" + exit 1 else stop_without_review_after_model_unavailable fi ;; esac + if [ "${COVERAGE_EVIDENCE_RESULT:-skipped}" != "success" ]; then + request_changes_for_coverage_evidence_failure + echo "::endgroup::" + exit 1 + fi echo "::endgroup::" - name: Publish repository_dispatch OpenCode status diff --git a/CHANGELOG.md b/CHANGELOG.md index bf19c4fcb2..bf7f886d2e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ this file. The format follows Keep a Changelog, and versioned releases follow Semantic Versioning where the repository publishes a release. ## [Unreleased] +- Restore OpenCode coverage honesty and mermaid surfaces stacked on main after #1360 squash `17052a7c`: `publish_fallback_diff_review` posts a COMMENT product-file review then `request_changes_for_coverage_evidence_failure` sets the status comment to `COVERAGE_BLOCKED` so a coverage miss never looks finished as `Gate result: COMMENT`; mermaid labels crates/packages instead of generic `Changed file (N files)` and does not invent class edges; findings say `Review process` instead of `.github/workflows/opencode-review.yml:1` unless that file is in the diff. Does not change `noema-review.yml` (PM owns `feat/noema-orchestrator-free-zdr`) and is not NIM-2h or GitHub Models. - Central review now routes through the vendored `contextual-orchestrator` gateway sidecar: the write-capable PR autofix and the shared `opencode.jsonc` default use the fail-closed zero-cost pool `orchestrator/free`, with diff --git a/ci-review-prompt.md b/ci-review-prompt.md index ad4c54ba4f..2d6ade247e 100644 --- a/ci-review-prompt.md +++ b/ci-review-prompt.md @@ -118,6 +118,15 @@ green checks are not runtime-tool receipts. When no receipt exists, describe only the source trace or explicit execution limitation; fabricating browser or DevTools evidence invalidates the entire control block. +The formal review must name the actual changed files and what they do, +include file/line findings on the current-head diff or an explicit APPROVE +with a real walkthrough, and draw a useful sequence/class/state diagram of +the changed API rather than a generic `Changed file (N files)` inventory. +Coverage is a gate, not the review: cite coverage evidence in the status +surface and never replace the product-file walkthrough with a coverage +blocker. Never cite `.github/workflows/opencode-review.yml:1` unless that +file is in the current-head diff. + Review the diff first, then inspect surrounding code only when needed to understand impact. Evaluate correctness, API compatibility, security/privacy, data integrity, concurrency, error handling, observability, performance, diff --git a/docs/doctoring/opencode-review-surfaces-originweave-47.md b/docs/doctoring/opencode-review-surfaces-originweave-47.md new file mode 100644 index 0000000000..af21f6ed3a --- /dev/null +++ b/docs/doctoring/opencode-review-surfaces-originweave-47.md @@ -0,0 +1,82 @@ +# OpenCode review surfaces and OriginWeave coverage sandbox + +검토 기준일: **2026-08-16** + +## Incident + +ContextualWisdomLab/OriginWeave#47, head +`79cf275686e2376a51783a2d03128eca21e7c0e5`, workflow run `31951179896`, +published the same body as both the formal pull-request review and the issue +comment: a generic overview plus one HIGH finding on +`.github/workflows/opencode-review.yml:1` saying coverage-evidence failed. The +pull request actually changed +`crates/originweave-destination/src/lib.rs`, `resolution.rs`, and +`tests/resolution_freshness.rs` (FreshResolutionSnapshot / DNS-rebinding +TOCTOU). The mermaid inventory said `Changed file (3 files)` because unknown +paths, including `crates/`, were bucketed as "Changed file". Repository CI on +that head passed. The central isolated coverage job failed and replaced the +entire review. + +## Root cause + +When `needs.coverage-evidence.result != success`, the publisher synthesized +`REQUEST_CHANGES`, posted it with `gh pr review` and again as an issue comment, +and exited before the model pool could review the diff. Coverage-evidence +failure became the review. The mermaid helper bucketed unknown paths, +including `crates/`, as a generic `Changed file (N files)` inventory and +findings were anchored to `.github/workflows/opencode-review.yml:1` even +when that file was not in the pull-request diff. + +## Decision + +Coverage remains a fail-closed gate. It is no longer the review. + +1. The formal pull-request review is a source-backed walkthrough of the + current-head product diff, including a fallback review that names the + changed crate files when the model pool did not emit a control block. +2. The issue comment is gate/status only: head SHA, run id/attempt, coverage + result, model-pool outcome, verdict, and a link to the formal review. It + must not repeat `## Pull request overview`, `## Findings`, mermaid, or the + model walkthrough. +3. A coverage miss, skip, or unsupported-tooling result blocks approval and + fails the required review job after the diff review is published. It must + not cite `.github/workflows/opencode-review.yml:1` unless that file is in + the pull-request diff. +4. Coverage-evidence failure is injected into `bounded-review-evidence.md` as + a `## Coverage gate` section. The model pool still runs. The publisher + does not early-return before the model path. `format_request_changes_body` + keeps model walkthrough/diagrams and appends structured findings. + +## Verification contract + +Regression tests prove that: + +1. the formal review body is not equal to the status comment; +2. a coverage-gate failure still produces a review that names the changed + crate files; +3. no finding is anchored to `opencode-review.yml:1` unless that file is in + the diff; +4. mermaid labels a `crates/...` change as a Rust crate surface, not + `Changed file (3 files)`; +5. the publisher function + `request_changes_for_coverage_evidence_failure` updates the status comment + and does not call `create_pull_review`; +6. the model pool still runs when coverage-evidence failed (`!= cancelled`); +7. `publish_fallback_diff_review` restores `COVERAGE_BLOCKED` on the status + comment after the COMMENT product-file review, so a coverage miss never + looks finished as `Gate result: COMMENT`; and +8. mermaid class diagrams list extracted public Rust API names only and do + not invent a `FirstType --> SecondType` class edge. + +## Limitations + +A later coverage-image or tooling catalog change can still fail the isolated +coverage job. That failure remains a coverage-gate failure, not a synthesized +product-file finding. The sandbox does not weaken a genuine below-threshold +coverage miss. + +## References + +GitHub, Inc. (2026). *REST API endpoints for pull request reviews*. GitHub +Docs. +https://docs.github.com/en/rest/pulls/reviews diff --git a/scripts/ci/opencode_review_comment_helpers.sh b/scripts/ci/opencode_review_comment_helpers.sh index 52bf189c66..87a8a6da6a 100644 --- a/scripts/ci/opencode_review_comment_helpers.sh +++ b/scripts/ci/opencode_review_comment_helpers.sh @@ -4,101 +4,35 @@ # This file is sourced by workflow run blocks after the trusted .github # repository has been checked out. +opencode_review_surfaces_py() { + local helper_dir + helper_dir="$(CDPATH='' cd -P -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd -P)" + printf '%s' "${helper_dir}/opencode_review_surfaces.py" +} + emit_change_flow_mermaid_graph() { local merge_state="${1:-UNKNOWN}" - local changed_files_file surfaces_file idx next_node + local changed_files_file changed_files_file="$(mktemp)" - surfaces_file="$(mktemp)" if ! timeout "${REVIEW_PUBLISH_GH_API_TIMEOUT_SECONDS:-120}s" \ gh pr diff "$PR_NUMBER" --repo "$GH_REPOSITORY" --name-only >"$changed_files_file" 2>/dev/null || [ ! -s "$changed_files_file" ]; then - printf '```mermaid\n' - printf 'flowchart LR\n' - printf ' Evidence["OpenCode evidence"] --> Review["Current PR review path"]\n' - printf ' Review --> Verify["Required checks"]\n' - printf '```\n' - rm -f "$changed_files_file" "$surfaces_file" - return 0 - fi - - awk ' - function basename(path) { - sub(/^.*\//, "", path) - return path - } - function clean(value) { - gsub(/"/, "", value) - gsub(/[\r\n\t]/, " ", value) - return value - } - function add(key, surface, impact, verify, path) { - if (!(key in count)) { - keys[++n] = key - label[key] = surface ": " basename(path) - impacts[key] = impact - verifies[key] = verify - } - count[key]++ - } - /^\.github\/workflows\// { - add("workflow", "Workflow", "GitHub Actions review job", "actionlint plus required checks", $0) - next - } - /^scripts\/ci\// { - add("ci", "CI script", "review and security gate shell path", "bash -n plus Strix self-test", $0) - next - } - /^backend\// { - add("backend", "Backend", "API and service runtime", "backend tests", $0) - next - } - /^frontend\// { - add("frontend", "Frontend", "browser runtime and bundle", "frontend tests", $0) - next - } - /^tests?\// || /(^|\/)test_/ { - add("tests", "Test", "regression suite", "targeted test run", $0) - next - } - /^docs\// { - add("docs", "Docs", "operator or user guidance", "docs review", $0) - next - } - { - add("other", "Changed file", "repository behavior", "required checks", $0) - } - END { - for (i = 1; i <= n; i++) { - key = keys[i] - if (count[key] > 1) { - sub(/: .*/, " (" count[key] " files)", label[key]) - } - print clean(label[key]) "\t" clean(impacts[key]) "\t" clean(verifies[key]) - } - } - ' "$changed_files_file" >"$surfaces_file" - - printf '```mermaid\n' - printf 'flowchart LR\n' - printf ' PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]\n' - idx=1 - while IFS="$(printf '\t')" read -r surface impact verify; do - [ -n "$surface" ] || continue - printf ' Evidence --> S%s["%s"]\n' "$idx" "$surface" - printf ' S%s --> I%s["%s"]\n' "$idx" "$idx" "$impact" - if [ "$merge_state" = "DIRTY" ] || [ "$merge_state" = "CONFLICTING" ]; then - printf ' I%s --> Conflict["Merge conflict blocks this path"]\n' "$idx" - next_node="Conflict" - else - printf ' I%s --> R%s["Review risk: %s"]\n' "$idx" "$idx" "$surface" - next_node="R${idx}" + if [ -n "${OPENCODE_CHANGED_FILES_FILE:-}" ] && [ -s "${OPENCODE_CHANGED_FILES_FILE}" ]; then + cp "${OPENCODE_CHANGED_FILES_FILE}" "$changed_files_file" fi - printf ' %s --> V%s["%s"]\n' "$next_node" "$idx" "$verify" - idx=$((idx + 1)) - done <"$surfaces_file" - printf '```\n' - rm -f "$changed_files_file" "$surfaces_file" + fi + if [ -n "${OPENCODE_SOURCE_WORKDIR:-}" ]; then + python3 "$(opencode_review_surfaces_py)" emit-mermaid \ + --changed-files-file "$changed_files_file" \ + --source-root "$OPENCODE_SOURCE_WORKDIR" \ + --merge-state "$merge_state" + else + python3 "$(opencode_review_surfaces_py)" emit-mermaid \ + --changed-files-file "$changed_files_file" \ + --merge-state "$merge_state" + fi + rm -f "$changed_files_file" } append_mermaid_review_graph() { diff --git a/scripts/ci/opencode_review_prompt_template.md b/scripts/ci/opencode_review_prompt_template.md index 32614dcfca..f6b143e889 100644 --- a/scripts/ci/opencode_review_prompt_template.md +++ b/scripts/ci/opencode_review_prompt_template.md @@ -30,7 +30,9 @@ For changed scrolling, animation, transition, or motion behavior, verify that us When a claim can be tested, use python3 scripts/ci/sandboxed_verify.py --repo-root "$OPENCODE_SOURCE_WORKDIR" -- or the web E2E wrapper above. If local tooling is missing or language versions differ, create an isolated Docker, Docker Compose, devcontainer, Nix, or temporary package-install sandbox and execute the verification there. If verification legitimately needs network or GitHub Secrets, pass only required names with --allow-env, declare --network required, add --evidence-note, and never print secret values; prefer synthetic/local substitutes over production services. Temporary proof or repro code must live only under the runner temporary directory or another ignored scratch path; do not commit or request committing scratch files. When proposing a fix for a blocker, prefer proving it in an isolated scratch copy or temporary worktree: apply the minimal patch there, run the relevant tests/linters/PoC, and cite the result. The review agent must not commit or push that proof patch; it should report the tested direction and, when concise enough, include a GitHub suggestion-ready diff. -Draw the right diagram. The required DAG evidence is not a file inventory. Use CodeGraph and focused source reads to identify the PR's relevant functions, classes, routes, components, database objects, workflows, or domain transitions, then compare base branch behavior with PR head behavior when that affects review. Include the most useful compact Mermaid diagram: sequenceDiagram for runtime message flow, classDiagram for class/API shape, erDiagram for schema/data relationship changes, stateDiagram for state transitions, or flowchart/DAG for function/control flow. Node labels must be quoted, for example A["parse_request"], so spaces, punctuation, parentheses, and file counts render safely. If CodeGraph cannot represent the changed surface, say why and draw a source-backed focused flow instead. +Draw the right diagram. The required DAG evidence is not a file inventory. Use CodeGraph and focused source reads to identify the PR's relevant functions, classes, routes, components, database objects, workflows, or domain transitions, then compare base branch behavior with PR head behavior when that affects review. Include the most useful compact Mermaid diagram: sequenceDiagram for runtime message flow, classDiagram for class/API shape, erDiagram for schema/data relationship changes, stateDiagram for state transitions, or flowchart/DAG for function/control flow. Node labels must be quoted, for example A["parse_request"], so spaces, punctuation, parentheses, and file counts render safely. Never label a crate, package, or language surface as `Changed file (N files)`. If CodeGraph cannot represent the changed surface, say why and draw a source-backed focused flow instead. + +The formal pull-request review is the code review of the actual diff. Name the changed product files and what they do. Publish file/line findings on the current-head diff, or an explicit APPROVE with a real walkthrough of those files. Coverage execution evidence is a separate gate: a coverage miss, skip, or unsupported-tooling result blocks approval in the status comment and must not replace the diff review. Never cite `.github/workflows/opencode-review.yml` or line 1 of that file as a finding unless that exact path is in the current-head changed-file list. Lead with severity-ordered findings. REQUEST_CHANGES findings must be actionable, source-backed, and line-specific: path, positive line, severity, title, problem, root_cause, fix_direction, regression_test_direction, and suggested_diff. The line value must be a positive integer from a current-head source, test, workflow, config, or evidence line; never use line 0. Include observable impact, trigger condition, exact failed log/check phrase when relevant, and a concrete verification command when the repository provides one. Do not request changes with only a check URL, workflow name, generic failure summary, raw tool-access failure, or missing-string marker. Suggested diffs must be GitHub suggestion-ready when possible, and every removed line must exist in the cited current local file. diff --git a/scripts/ci/opencode_review_surfaces.py b/scripts/ci/opencode_review_surfaces.py new file mode 100644 index 0000000000..2c9ac38405 --- /dev/null +++ b/scripts/ci/opencode_review_surfaces.py @@ -0,0 +1,720 @@ +#!/usr/bin/env python3 +"""Split OpenCode review publication into a diff review and a gate-status comment. + +The OriginWeave #47 failure posted the same coverage-gate body as both the +formal pull-request review and the issue comment, and it anchored that body to +``.github/workflows/opencode-review.yml:1`` even though the product diff was a +Rust crate. This module is the trusted publisher contract for those surfaces. +""" + +from __future__ import annotations + +import argparse +import json +import re +import sys +from collections import OrderedDict +from collections.abc import Mapping, Sequence +from pathlib import Path, PurePosixPath + +CENTRAL_WORKFLOW_ANCHOR = ".github/workflows/opencode-review.yml" +PUB_ITEM_RE = re.compile( + r"^\s*pub(?:\s*\([^)]*\))?\s+" + r"(?:async\s+)?(?:unsafe\s+)?" + r"(?Pstruct|enum|fn|trait|type|mod)\s+" + r"(?P[A-Za-z_][A-Za-z0-9_]*)", + re.MULTILINE, +) +RUST_SUFFIXES = {".rs"} +PYTHON_SUFFIXES = {".py"} +TYPESCRIPT_SUFFIXES = {".ts", ".tsx", ".js", ".jsx", ".mjs", ".cjs"} +GO_SUFFIXES = {".go"} +WORKFLOW_PREFIXES = (".github/workflows/",) +CI_PREFIXES = ("scripts/ci/",) +DOC_PREFIXES = ("docs/",) +TEST_NAME_RE = re.compile(r"(^|/)tests?(/|$)|(^|/)test_[^/]+") + + +def posix_path(raw_path: str) -> str: + """Normalize a repository-relative path to POSIX form without traversal.""" + normalized = raw_path.replace("\\", "/").strip() + while normalized.startswith("./"): + normalized = normalized[2:] + candidate = PurePosixPath(normalized) + if candidate.is_absolute() or ".." in candidate.parts: + raise ValueError(f"changed path is not a bounded repository path: {raw_path}") + return str(candidate) + + +def classify_changed_path(raw_path: str) -> dict[str, str]: + """Return the review surface, impact, and verification label for one path.""" + path = posix_path(raw_path) + suffix = Path(path).suffix.lower() + parts = PurePosixPath(path).parts + name = Path(path).name + + if path.startswith(WORKFLOW_PREFIXES): + return { + "key": f"workflow:{path}", + "surface": f"Workflow: {name}", + "impact": "GitHub Actions review job", + "verify": "actionlint plus required checks", + "kind": "workflow", + } + if path.startswith(CI_PREFIXES): + return { + "key": f"ci:{path}", + "surface": f"CI script: {name}", + "impact": "review and security gate shell path", + "verify": "bash -n plus Strix self-test", + "kind": "ci", + } + if parts and parts[0] == "crates": + crate = parts[1] if len(parts) > 1 else name + return { + "key": f"rust-crate:{crate}", + "surface": f"Rust crate: {crate}", + "impact": "Rust workspace crate API and tests", + "verify": "cargo test plus llvm-cov", + "kind": "rust-crate", + } + if name in {"Cargo.toml", "Cargo.lock"}: + return { + "key": "rust-manifest", + "surface": f"Rust manifest: {name}", + "impact": "Rust workspace or package manifest", + "verify": "cargo test plus llvm-cov", + "kind": "rust", + } + if TEST_NAME_RE.search(path): + return { + "key": f"tests:{Path(path).parent.as_posix()}", + "surface": f"Test: {name}", + "impact": "regression suite", + "verify": "targeted test run", + "kind": "tests", + } + if suffix in RUST_SUFFIXES: + return { + "key": "rust-source", + "surface": f"Rust source: {name}", + "impact": "Rust package behavior", + "verify": "cargo test plus llvm-cov", + "kind": "rust", + } + if path.startswith(DOC_PREFIXES): + return { + "key": "docs", + "surface": f"Docs: {name}", + "impact": "operator or user guidance", + "verify": "docs review", + "kind": "docs", + } + if parts and parts[0] == "backend": + return { + "key": "backend", + "surface": f"Backend: {name}", + "impact": "API and service runtime", + "verify": "backend tests", + "kind": "backend", + } + if parts and parts[0] == "frontend": + return { + "key": "frontend", + "surface": f"Frontend: {name}", + "impact": "browser runtime and bundle", + "verify": "frontend tests", + "kind": "frontend", + } + if parts and parts[0] == "src" and suffix in PYTHON_SUFFIXES: + return { + "key": "python-src", + "surface": f"Python package: {name}", + "impact": "Python runtime API", + "verify": "pytest plus coverage", + "kind": "python", + } + if parts and parts[0] == "src" and suffix in TYPESCRIPT_SUFFIXES: + return { + "key": "typescript-src", + "surface": f"TypeScript/JavaScript: {name}", + "impact": "TypeScript or JavaScript runtime", + "verify": "package test plus coverage", + "kind": "typescript", + } + if suffix in PYTHON_SUFFIXES: + return { + "key": "python", + "surface": f"Python: {name}", + "impact": "Python module behavior", + "verify": "pytest plus coverage", + "kind": "python", + } + if suffix in TYPESCRIPT_SUFFIXES: + return { + "key": "typescript", + "surface": f"TypeScript/JavaScript: {name}", + "impact": "TypeScript or JavaScript runtime", + "verify": "package test plus coverage", + "kind": "typescript", + } + if suffix in GO_SUFFIXES: + return { + "key": "go", + "surface": f"Go package: {name}", + "impact": "Go runtime API", + "verify": "go test", + "kind": "go", + } + return { + "key": f"other:{path}", + "surface": f"Repository file: {name}", + "impact": "repository behavior", + "verify": "required checks", + "kind": "other", + } + + +def classify_surfaces(raw_paths: Sequence[str]) -> list[dict[str, str]]: + """Group changed paths into labeled review surfaces.""" + grouped: "OrderedDict[str, dict[str, str]]" = OrderedDict() + for raw_path in raw_paths: + if not str(raw_path).strip(): + continue + classified = classify_changed_path(raw_path) + key = classified["key"] + if key not in grouped: + grouped[key] = { + "surface": classified["surface"], + "impact": classified["impact"], + "verify": classified["verify"], + "kind": classified["kind"], + "count": "1", + } + else: + count = int(grouped[key]["count"]) + 1 + grouped[key]["count"] = str(count) + label = grouped[key]["surface"].split(" (", 1)[0] + grouped[key]["surface"] = f"{label} ({count} files)" + return list(grouped.values()) + + +def rust_api_symbols(source_root: Path | None, raw_paths: Sequence[str]) -> list[str]: + """Extract public Rust API names from changed crate sources when present.""" + if source_root is None: + return [] + names: list[str] = [] + seen: set[str] = set() + for raw_path in raw_paths: + path = posix_path(raw_path) + if Path(path).suffix != ".rs": + continue + candidate = source_root / path + if not candidate.is_file() or candidate.is_symlink(): + continue + text = candidate.read_text(encoding="utf-8", errors="replace") + for match in PUB_ITEM_RE.finditer(text): + name = match.group("name") + if name not in seen: + seen.add(name) + names.append(name) + return names + + +def _quote_label(value: str) -> str: + """Make a Mermaid node label safe for quoted rendering.""" + return value.replace('"', "").replace("\n", " ").replace("\r", " ").strip() + + +def emit_mermaid( + raw_paths: Sequence[str], + merge_state: str = "UNKNOWN", + source_root: Path | None = None, +) -> str: + """Render a source-backed diagram of the changed API, not a file inventory.""" + paths = [posix_path(path) for path in raw_paths if str(path).strip()] + if not paths: + return ( + "```mermaid\n" + "flowchart LR\n" + ' Evidence["OpenCode evidence"] --> Review["Current PR review path"]\n' + ' Review --> Verify["Required checks"]\n' + "```\n" + ) + + symbols = rust_api_symbols(source_root, paths) + rust_paths = [ + path + for path in paths + if path.startswith("crates/") + or path.endswith(".rs") + or path.endswith("Cargo.toml") + or path.endswith("Cargo.lock") + ] + if symbols: + lines = ["```mermaid", "classDiagram"] + for symbol in symbols[:8]: + lines.append(f" class {_quote_label(symbol)}") + lines.append("```") + return "\n".join(lines) + "\n" + if rust_paths: + crate = "Rust crate" + for path in rust_paths: + parts = PurePosixPath(path).parts + if len(parts) > 1 and parts[0] == "crates": + crate = parts[1] + break + return ( + "```mermaid\n" + "sequenceDiagram\n" + f" participant Caller as Caller\n" + f" participant Crate as {_quote_label(crate)}\n" + " participant Tests as Crate tests\n" + " Caller->>Crate: changed public API\n" + " Tests->>Crate: regression coverage\n" + "```\n" + ) + + surfaces = classify_surfaces(paths) + lines = [ + "```mermaid", + "flowchart LR", + ' PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]', + ] + for index, surface in enumerate(surfaces, start=1): + label = _quote_label(surface["surface"]) + impact = _quote_label(surface["impact"]) + verify = _quote_label(surface["verify"]) + lines.append(f' Evidence --> S{index}["{label}"]') + lines.append(f' S{index} --> I{index}["{impact}"]') + if merge_state in {"DIRTY", "CONFLICTING"}: + lines.append(f' I{index} --> Conflict["Merge conflict blocks this path"]') + next_node = "Conflict" + else: + lines.append(f' I{index} --> R{index}["Review risk: {label}"]') + next_node = f"R{index}" + lines.append(f' {next_node} --> V{index}["{verify}"]') + lines.append("```") + return "\n".join(lines) + "\n" + + +def coverage_anchor_allowed(path: str, changed_files: Sequence[str]) -> bool: + """Allow a workflow-file finding only when that file is in the current diff.""" + normalized = posix_path(path) + changed = {posix_path(item) for item in changed_files if str(item).strip()} + return normalized in changed + + +def _language(value: str) -> str: + """Normalize the review-language contract to korean or english.""" + return "korean" if value.strip().casefold() == "korean" else "english" + + +CONTROL_START = ""): + skipping_control = False + continue + lines.append(line) + return "\n".join(lines).strip() + + +def format_structured_findings( + findings: Sequence[object], + changed_files: Sequence[str] | None = None, +) -> str: + """Render control-plane findings as markdown without a fake workflow:1 anchor.""" + allowed = list(changed_files or []) + blocks: list[str] = [] + for index, raw in enumerate(findings, start=1): + if not isinstance(raw, Mapping): + continue + path = str(raw.get("path") or "unknown") + line = raw.get("line") or 0 + location = f"{path}:{line}" + if ( + path == CENTRAL_WORKFLOW_ANCHOR + and str(line) == "1" + and not coverage_anchor_allowed(CENTRAL_WORKFLOW_ANCHOR, allowed) + ): + location = "Review process" + title = str(raw.get("title") or "Finding") + severity = str(raw.get("severity") or "severity").upper() + blocks.append( + "\n".join( + [ + f"### {index}. {severity} {location} - {title}", + f"- Problem: {raw.get('problem') or ''}", + f"- Root cause: {raw.get('root_cause') or ''}", + f"- Fix: {raw.get('fix_direction') or ''}", + f"- Regression test: {raw.get('regression_test_direction') or ''}", + "- Suggested diff: posted in this finding's inline review thread.", + ] + ) + ) + return "\n\n".join(blocks) + + +def _strip_forbidden_workflow_anchor(body: str, changed_files: Sequence[str]) -> str: + """Remove a synthesized central-workflow:1 citation unless that file changed.""" + if coverage_anchor_allowed(CENTRAL_WORKFLOW_ANCHOR, changed_files): + return body + return body.replace(f"{CENTRAL_WORKFLOW_ANCHOR}:1", "Review process") + + +def format_request_changes_review( + *, + model_prose: str, + structured_findings: str = "", + findings: Sequence[object] | None = None, + head_sha: str, + run_id: str, + run_attempt: str, + reason: str = "", + changed_files: Sequence[str] | None = None, +) -> str: + """Keep model walkthrough/diagrams and append structured findings.""" + allowed = list(changed_files or []) + prose = extract_model_prose(model_prose) + rendered = structured_findings.strip() + if not rendered and findings: + rendered = format_structured_findings(findings, allowed) + lines: list[str] = [] + if prose: + lines.extend([prose, ""]) + else: + lines.extend( + [ + "## Verdict", + "", + "REQUEST_CHANGES", + "", + ] + ) + joined = "\n".join(lines) + if rendered and rendered not in joined: + if "## Findings" not in joined: + lines.extend(["## Findings", ""]) + lines.extend([rendered, ""]) + if reason and f"- Reason: {reason}" not in "\n".join(lines): + lines.extend([f"- Reason: {reason}", ""]) + identity = ( + f"- Head SHA: `{head_sha}`", + f"- Workflow run: {run_id}", + f"- Workflow attempt: {run_attempt}", + ) + existing = "\n".join(lines) + if identity[0] not in existing: + lines.extend([*identity, ""]) + body = _strip_forbidden_workflow_anchor("\n".join(lines), allowed) + return body if body.endswith("\n") else body + "\n" + + +def build_status_comment( + *, + result: str, + head_sha: str, + run_id: str, + run_attempt: str, + coverage_result: str, + coverage_summary: str = "", + language: str = "english", + control_block: str = "", + model_pool_outcome: str = "", + verdict: str = "", + formal_review_url: str = "", +) -> str: + """Build the issue-comment gate/status surface without review findings.""" + korean = _language(language) == "korean" + heading = "OpenCode 게이트 상태" if korean else "OpenCode Review Status" + coverage_label = "커버리지 게이트" if korean else "Coverage gate" + lines = [ + "", + f"## {heading}", + "", + f"- Head SHA: `{head_sha}`", + f"- Workflow run: {run_id}", + f"- Workflow attempt: {run_attempt}", + f"- Gate result: `{result}`", + f"- {coverage_label}: `{coverage_result}`", + ] + if model_pool_outcome: + label = "모델 풀" if korean else "Model pool" + lines.append(f"- {label}: `{model_pool_outcome}`") + if verdict: + label = "판정" if korean else "Verdict" + lines.append(f"- {label}: `{verdict}`") + if formal_review_url: + label = "정식 리뷰" if korean else "Formal review" + lines.append(f"- {label}: {formal_review_url}") + lines.append("") + if coverage_result != "success": + blocker = ( + "커버리지 증거 작업이 통과하지 않아 승인은 차단됩니다. 코드 리뷰는 별도 정식 리뷰 본문에 있습니다." + if korean + else ( + "Coverage evidence did not pass, so approval is blocked. " + "The formal pull-request review is the source-backed diff review, " + "not this status comment." + ) + ) + lines.extend([blocker, ""]) + if control_block.strip(): + lines.extend([control_block.strip(), ""]) + _ = coverage_summary + return "\n".join(lines).rstrip() + "\n" + + +def _file_role(path: str) -> str: + """Describe what a changed path is in the review walkthrough.""" + classified = classify_changed_path(path) + return f"`{posix_path(path)}` — {classified['impact']}" + + +def build_fallback_review( + *, + changed_files: Sequence[str], + head_sha: str, + run_id: str, + run_attempt: str, + source_root: Path | None = None, + language: str = "english", + coverage_result: str = "success", +) -> str: + """Build a source-backed formal review of the actual changed product files.""" + paths = [posix_path(path) for path in changed_files if str(path).strip()] + korean = _language(language) == "korean" + overview = "Pull request overview" if not korean else "Pull request 개요" + walkthrough = "Changed files" if not korean else "변경 파일" + diagram = "Changed behavior" if not korean else "변경 동작" + findings = "Findings" if not korean else "발견 사항" + intro = ( + "OpenCode reviewed the current-head product diff. Coverage is a separate gate." + if not korean + else "OpenCode가 현재 head의 제품 diff를 리뷰했습니다. 커버리지는 별도 게이트입니다." + ) + if not paths: + intro = ( + "OpenCode could not list changed product files for this head." + if not korean + else "OpenCode가 이 head의 변경 제품 파일을 나열하지 못했습니다." + ) + lines = [ + f"## {overview}", + "", + intro, + "", + f"## {walkthrough}", + "", + ] + if paths: + lines.extend(f"- {_file_role(path)}" for path in paths) + else: + lines.append("- No changed product files were supplied to the fallback review.") + lines.extend(["", f"## {diagram}", "", emit_mermaid(paths, source_root=source_root).rstrip(), ""]) + symbols = rust_api_symbols(source_root, paths) + if symbols: + api_heading = "Changed API" if not korean else "변경 API" + lines.extend([f"## {api_heading}", ""]) + lines.extend(f"- `{symbol}`" for symbol in symbols) + lines.append("") + lines.extend( + [ + f"## {findings}", + "", + ( + "No source-backed product finding is synthesized from the coverage gate. " + "A coverage miss belongs in the status comment." + if not korean + else "커버리지 게이트만으로 제품 소스 발견 사항을 합성하지 않습니다. 커버리지 결과는 상태 댓글에 둡니다." + ), + "", + f"- Head SHA: `{head_sha}`", + f"- Workflow run: {run_id}", + f"- Workflow attempt: {run_attempt}", + f"- Coverage gate: `{coverage_result}`", + "", + ] + ) + body = "\n".join(lines) + if CENTRAL_WORKFLOW_ANCHOR in body and not coverage_anchor_allowed( + CENTRAL_WORKFLOW_ANCHOR, paths + ): + raise ValueError( + "fallback review must not cite " + f"{CENTRAL_WORKFLOW_ANCHOR} unless that file is in the PR diff" + ) + return body + + +def distinct_surfaces(review_body: str, comment_body: str) -> None: + """Reject publication that pastes the same overview/findings onto both surfaces.""" + if review_body.strip() == comment_body.strip(): + raise ValueError("formal review body must not equal the status comment body") + if "## Pull request overview" in comment_body or "## Pull request 개요" in comment_body: + raise ValueError("status comment must not contain the formal review overview") + if "## Findings" in comment_body or "## 발견 사항" in comment_body: + raise ValueError("status comment must not contain the formal review findings") + if ( + "## OpenCode Review Status" in review_body + or "## OpenCode Review Overview" in review_body + or "## OpenCode 게이트 상태" in review_body + ): + raise ValueError("formal review must not reuse the status-comment heading") + + +def review_event_when_coverage_blocks(model_result: str) -> str: + """Return the GitHub review event when coverage failed but a diff review exists.""" + if model_result == "REQUEST_CHANGES": + return "REQUEST_CHANGES" + return "COMMENT" + + +def read_changed_files(path: Path) -> list[str]: + """Load a newline-delimited changed-file list.""" + return [ + line.strip() + for line in path.read_text(encoding="utf-8").splitlines() + if line.strip() + ] + + +def _add_common_identity_args(parser: argparse.ArgumentParser) -> None: + """Add the head/run identity flags shared by publisher subcommands.""" + parser.add_argument("--head-sha", required=True) + parser.add_argument("--run-id", required=True) + parser.add_argument("--run-attempt", required=True) + parser.add_argument("--coverage-result", default="unknown") + parser.add_argument("--language", default="english") + + +def main(argv: Sequence[str] | None = None) -> int: + """CLI for trusted review/status rendering from the publisher workflow.""" + parser = argparse.ArgumentParser(description=__doc__) + subparsers = parser.add_subparsers(dest="command", required=True) + + mermaid = subparsers.add_parser("emit-mermaid", help="Render the changed-API diagram") + mermaid.add_argument("--changed-files-file", type=Path, required=True) + mermaid.add_argument("--source-root", type=Path) + mermaid.add_argument("--merge-state", default="UNKNOWN") + + status = subparsers.add_parser("build-status", help="Render the gate/status comment") + _add_common_identity_args(status) + status.add_argument("--result", required=True) + status.add_argument("--coverage-summary", default="") + status.add_argument("--control-block", default="") + status.add_argument("--model-pool-outcome", default="") + status.add_argument("--verdict", default="") + status.add_argument("--formal-review-url", default="") + + fallback = subparsers.add_parser( + "build-fallback-review", help="Render a source-backed diff review" + ) + _add_common_identity_args(fallback) + fallback.add_argument("--changed-files-file", type=Path, required=True) + fallback.add_argument("--source-root", type=Path) + + extract = subparsers.add_parser( + "extract-prose", help="Strip sentinel and control JSON from model output" + ) + extract.add_argument("--model-body-file", type=Path, required=True) + + request_changes = subparsers.add_parser( + "format-request-changes", + help="Keep model prose and append structured findings", + ) + _add_common_identity_args(request_changes) + request_changes.add_argument("--model-body-file", type=Path) + request_changes.add_argument("--findings-json-file", type=Path) + request_changes.add_argument("--reason", default="") + request_changes.add_argument("--changed-files-file", type=Path) + + args = parser.parse_args(argv) + if args.command == "emit-mermaid": + sys.stdout.write( + emit_mermaid( + read_changed_files(args.changed_files_file), + merge_state=args.merge_state, + source_root=args.source_root, + ) + ) + return 0 + if args.command == "build-status": + sys.stdout.write( + build_status_comment( + result=args.result, + head_sha=args.head_sha, + run_id=args.run_id, + run_attempt=args.run_attempt, + coverage_result=args.coverage_result, + coverage_summary=args.coverage_summary, + language=args.language, + control_block=args.control_block, + model_pool_outcome=args.model_pool_outcome, + verdict=args.verdict, + formal_review_url=args.formal_review_url, + ) + ) + return 0 + if args.command == "extract-prose": + prose = extract_model_prose(args.model_body_file.read_text(encoding="utf-8")) + sys.stdout.write(prose if prose.endswith("\n") else prose + "\n") + return 0 + if args.command == "format-request-changes": + model_body = ( + args.model_body_file.read_text(encoding="utf-8") + if args.model_body_file is not None + else "" + ) + findings: list[object] = [] + if args.findings_json_file is not None: + loaded = json.loads(args.findings_json_file.read_text(encoding="utf-8")) + if isinstance(loaded, list): + findings = loaded + changed = ( + read_changed_files(args.changed_files_file) + if args.changed_files_file is not None + else [] + ) + sys.stdout.write( + format_request_changes_review( + model_prose=model_body, + findings=findings, + head_sha=args.head_sha, + run_id=args.run_id, + run_attempt=args.run_attempt, + reason=args.reason, + changed_files=changed, + ) + ) + return 0 + sys.stdout.write( + build_fallback_review( + changed_files=read_changed_files(args.changed_files_file), + head_sha=args.head_sha, + run_id=args.run_id, + run_attempt=args.run_attempt, + source_root=args.source_root, + language=args.language, + coverage_result=args.coverage_result, + ) + ) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/test_opencode_review_comment_helpers.py b/tests/test_opencode_review_comment_helpers.py new file mode 100644 index 0000000000..9f0d0ca5c1 --- /dev/null +++ b/tests/test_opencode_review_comment_helpers.py @@ -0,0 +1,54 @@ +"""Tests for the shared OpenCode review mermaid helper.""" + +from __future__ import annotations + +import os +import shutil +import subprocess +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[1] +HELPER = REPO_ROOT / "scripts/ci/opencode_review_comment_helpers.sh" + + +def test_mermaid_helper_labels_crates_as_rust_crate(tmp_path: Path) -> None: + """Sourcing the publisher helper labels crates/ as a Rust crate surface.""" + bash = shutil.which("bash") + if bash is None: + return + changed = tmp_path / "changed.txt" + changed.write_text( + "crates/originweave-destination/src/lib.rs\n" + "crates/originweave-destination/src/resolution.rs\n" + "crates/originweave-destination/tests/resolution_freshness.rs\n", + encoding="utf-8", + ) + fake_bin = tmp_path / "bin" + fake_bin.mkdir() + (fake_bin / "gh").write_text("#!/bin/sh\nexit 1\n", encoding="utf-8") + (fake_bin / "gh").chmod(0o755) + script = f""" + set -euo pipefail + . "{HELPER}" + GH_REPOSITORY=ContextualWisdomLab/OriginWeave + PR_NUMBER=47 + OPENCODE_CHANGED_FILES_FILE="{changed}" + emit_change_flow_mermaid_graph UNKNOWN + """ + result = subprocess.run( + [bash, "-c", script], + check=False, + capture_output=True, + text=True, + env={**os.environ, "PATH": f"{fake_bin}:{os.environ.get('PATH', '')}"}, + ) + assert result.returncode == 0, result.stderr + assert "Changed file (3 files)" not in result.stdout + assert "originweave-destination" in result.stdout + + +def test_helper_sources_python_surfaces_module() -> None: + """The shared helper delegates mermaid rendering to the tested Python module.""" + text = HELPER.read_text(encoding="utf-8") + assert "opencode_review_surfaces.py" in text + assert 'add("other", "Changed file"' not in text diff --git a/tests/test_opencode_review_surfaces.py b/tests/test_opencode_review_surfaces.py new file mode 100644 index 0000000000..d05fd10e81 --- /dev/null +++ b/tests/test_opencode_review_surfaces.py @@ -0,0 +1,692 @@ +"""Regression tests for distinct OpenCode review and status surfaces.""" + +from __future__ import annotations + +import runpy +import sys +from pathlib import Path + +import pytest + +from scripts.ci import opencode_review_surfaces as surfaces + +ROOT = Path(__file__).resolve().parents[1] +ORIGINWEAVE_47_FILES = [ + "crates/originweave-destination/src/lib.rs", + "crates/originweave-destination/src/resolution.rs", + "crates/originweave-destination/tests/resolution_freshness.rs", +] +HEAD = "79cf275686e2376a51783a2d03128eca21e7c0e5" + + +def test_crates_paths_are_rust_crate_surfaces() -> None: + """OriginWeave-style crates/ changes are Rust crate surfaces, not 'Changed file'.""" + classified = surfaces.classify_surfaces(ORIGINWEAVE_47_FILES) + assert len(classified) == 1 + assert classified[0]["kind"] == "rust-crate" + assert "Rust crate: originweave-destination" in classified[0]["surface"] + assert "3 files" in classified[0]["surface"] + assert classified[0]["surface"].startswith("Changed file") is False + + +def test_src_layouts_are_language_surfaces() -> None: + """src/ Python and TypeScript layouts keep language-specific labels.""" + python_surface = surfaces.classify_changed_path("src/originweave/resolution.py") + typescript_surface = surfaces.classify_changed_path("src/lib/resolution.ts") + assert python_surface["kind"] == "python" + assert python_surface["surface"].startswith("Python package:") + assert typescript_surface["kind"] == "typescript" + assert typescript_surface["surface"].startswith("TypeScript/JavaScript:") + + +def test_mermaid_labels_originweave_crate_not_changed_file_inventory() -> None: + """The #47 mermaid must name the Rust crate instead of 'Changed file (3 files)'.""" + diagram = surfaces.emit_mermaid(ORIGINWEAVE_47_FILES) + assert "Changed file (3 files)" not in diagram + assert "originweave-destination" in diagram + assert "sequenceDiagram" in diagram or "classDiagram" in diagram + + +def test_mermaid_uses_public_rust_api_when_source_exists(tmp_path: Path) -> None: + """A class diagram is preferred when the changed crate exposes public types.""" + source = tmp_path / "crates/originweave-destination/src/resolution.rs" + source.parent.mkdir(parents=True) + source.write_text( + "pub struct FreshResolutionSnapshot {\n address: String,\n}\n" + "pub fn resolve_fresh() {}\n", + encoding="utf-8", + ) + diagram = surfaces.emit_mermaid( + ["crates/originweave-destination/src/resolution.rs"], + source_root=tmp_path, + ) + assert "classDiagram" in diagram + assert "FreshResolutionSnapshot" in diagram + assert "resolve_fresh" in diagram + assert "class FreshResolutionSnapshot" in diagram + assert "class resolve_fresh" in diagram + assert "FreshResolutionSnapshot --> resolve_fresh" not in diagram + assert " --> " not in diagram + assert "Changed file" not in diagram + + +def test_coverage_fail_review_mentions_crate_files_not_central_workflow() -> None: + """A coverage-gate failure still produces a review of the changed crate files.""" + review = surfaces.build_fallback_review( + changed_files=ORIGINWEAVE_47_FILES, + head_sha=HEAD, + run_id="31951179896", + run_attempt="1", + coverage_result="failure", + ) + comment = surfaces.build_status_comment( + result="COVERAGE_BLOCKED", + head_sha=HEAD, + run_id="31951179896", + run_attempt="1", + coverage_result="failure", + coverage_summary="## Coverage Decision\n\n- Result: FAIL\n", + ) + surfaces.distinct_surfaces(review, comment) + assert review != comment + for path in ORIGINWEAVE_47_FILES: + assert path in review + assert path not in comment + assert ".github/workflows/opencode-review.yml:1" not in review + assert "Coverage gate: `failure`" in review + assert "Coverage gate: `failure`" in comment + assert "## Pull request overview" in review + assert "## Pull request overview" not in comment + assert "## Findings" not in comment + + +def test_workflow_anchor_forbidden_unless_file_is_in_diff() -> None: + """The central workflow file is not a finding on an unrelated product PR.""" + assert ( + surfaces.coverage_anchor_allowed( + ".github/workflows/opencode-review.yml", + ORIGINWEAVE_47_FILES, + ) + is False + ) + assert ( + surfaces.coverage_anchor_allowed( + ".github/workflows/opencode-review.yml", + [".github/workflows/opencode-review.yml"], + ) + is True + ) + + +def test_korean_status_and_review_keep_identifiers() -> None: + """Korean PRs stay Korean while crate paths remain unchanged.""" + review = surfaces.build_fallback_review( + changed_files=ORIGINWEAVE_47_FILES, + head_sha=HEAD, + run_id="1", + run_attempt="1", + language="korean", + coverage_result="failure", + ) + comment = surfaces.build_status_comment( + result="COVERAGE_BLOCKED", + head_sha=HEAD, + run_id="1", + run_attempt="1", + coverage_result="failure", + language="korean", + ) + surfaces.distinct_surfaces(review, comment) + assert "변경 파일" in review + assert "게이트 상태" in comment + assert "originweave-destination" in review + + +def test_review_event_keeps_request_changes_and_downgrades_approve() -> None: + """Coverage failure may not publish APPROVE; code findings stay REQUEST_CHANGES.""" + assert surfaces.review_event_when_coverage_blocks("APPROVE") == "COMMENT" + assert surfaces.review_event_when_coverage_blocks("REQUEST_CHANGES") == "REQUEST_CHANGES" + assert surfaces.review_event_when_coverage_blocks("COMMENT") == "COMMENT" + + +def test_distinct_surfaces_reject_duplicated_overview() -> None: + """The #47 publication shape — identical overview on both surfaces — fails.""" + body = "## Pull request overview\n\n## Findings\n" + with pytest.raises(ValueError, match="must not equal"): + surfaces.distinct_surfaces(body, body) + with pytest.raises(ValueError, match="status comment must not contain"): + surfaces.distinct_surfaces("review", "## Pull request overview\n") + with pytest.raises(ValueError, match="formal review must not reuse"): + surfaces.distinct_surfaces("## OpenCode Review Overview\n", "status") + with pytest.raises(ValueError, match="formal review must not reuse"): + surfaces.distinct_surfaces("## OpenCode Review Status\n", "status") + + +def test_rejects_path_traversal() -> None: + """Publisher path classification fails closed on parent-directory segments.""" + with pytest.raises(ValueError, match="bounded repository path"): + surfaces.posix_path("../secrets") + assert surfaces.posix_path("./crates/originweave-destination/src/lib.rs") == ( + "crates/originweave-destination/src/lib.rs" + ) + assert ( + surfaces.classify_changed_path("./.github/workflows/ci.yml")["kind"] == "workflow" + ) + + +def test_empty_paths_use_generic_evidence_diagram() -> None: + """No changed files still produce a bounded evidence flowchart.""" + assert "OpenCode evidence" in surfaces.emit_mermaid([]) + + +def test_conflict_state_marks_blocked_paths() -> None: + """DIRTY merge state keeps the conflict node on classified surfaces.""" + diagram = surfaces.emit_mermaid(["docs/readme.md"], merge_state="DIRTY") + assert "Merge conflict blocks this path" in diagram + assert "Docs: readme.md" in diagram + + +def test_cli_renders_originweave_surfaces( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """The workflow CLI emits the split surfaces used by the publisher.""" + changed = tmp_path / "changed.txt" + changed.write_text("\n".join(ORIGINWEAVE_47_FILES) + "\n", encoding="utf-8") + assert ( + surfaces.main( + [ + "emit-mermaid", + "--changed-files-file", + str(changed), + ] + ) + == 0 + ) + mermaid = capsys.readouterr().out + assert "Changed file (3 files)" not in mermaid + assert "originweave-destination" in mermaid + + assert ( + surfaces.main( + [ + "build-status", + "--result", + "COVERAGE_BLOCKED", + "--head-sha", + HEAD, + "--run-id", + "31951179896", + "--run-attempt", + "1", + "--coverage-result", + "failure", + "--coverage-summary", + "llvm-tools-preview missing", + ] + ) + == 0 + ) + status = capsys.readouterr().out + assert "## Pull request overview" not in status + assert "## Findings" not in status + assert "llvm-tools-preview missing" not in status + assert "Coverage gate: `failure`" in status + + assert ( + surfaces.main( + [ + "build-fallback-review", + "--changed-files-file", + str(changed), + "--head-sha", + HEAD, + "--run-id", + "31951179896", + "--run-attempt", + "1", + "--coverage-result", + "failure", + ] + ) + == 0 + ) + review = capsys.readouterr().out + assert "crates/originweave-destination/src/resolution.rs" in review + assert ".github/workflows/opencode-review.yml:1" not in review + surfaces.distinct_surfaces(review, status) + + +def test_script_entrypoint(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + """The executable workflow entrypoint delegates to main.""" + changed = tmp_path / "changed.txt" + changed.write_text("docs/guide.md\n", encoding="utf-8") + script = Path(surfaces.__file__) + monkeypatch.setattr( + sys, + "argv", + [str(script), "emit-mermaid", "--changed-files-file", str(changed)], + ) + with pytest.raises(SystemExit, match="0"): + runpy.run_path(str(script), run_name="__main__") + + +def test_remaining_classifiers_cover_common_layouts() -> None: + """Workflow, CI, backend, frontend, Go, and loose files keep specific labels.""" + assert surfaces.classify_changed_path(".github/workflows/ci.yml")["kind"] == "workflow" + assert surfaces.classify_changed_path("scripts/ci/gate.sh")["kind"] == "ci" + assert surfaces.classify_changed_path("backend/api.py")["kind"] == "backend" + assert surfaces.classify_changed_path("frontend/app.tsx")["kind"] == "frontend" + assert surfaces.classify_changed_path("pkg/main.go")["kind"] == "go" + assert surfaces.classify_changed_path("lib.rs")["kind"] == "rust" + assert surfaces.classify_changed_path("module.py")["kind"] == "python" + assert surfaces.classify_changed_path("app.ts")["kind"] == "typescript" + assert surfaces.classify_changed_path("tests/test_resolution.py")["kind"] == "tests" + assert surfaces.classify_changed_path("tests/fixture.rs")["kind"] == "tests" + assert surfaces.classify_changed_path("LICENSE")["kind"] == "other" + + +def test_central_workflow_in_diff_is_a_workflow_surface_not_line_one_finding() -> None: + """When the central workflow actually changed, name it as a workflow surface.""" + review = surfaces.build_fallback_review( + changed_files=[".github/workflows/opencode-review.yml"], + head_sha=HEAD, + run_id="1", + run_attempt="1", + ) + assert ".github/workflows/opencode-review.yml" in review + assert ".github/workflows/opencode-review.yml:1" not in review + assert "Workflow: opencode-review.yml" in surfaces.emit_mermaid( + [".github/workflows/opencode-review.yml"] + ) + + +def test_fallback_review_empty_file_list() -> None: + """Missing changed-file evidence still produces a distinct review body.""" + review = surfaces.build_fallback_review( + changed_files=[], + head_sha=HEAD, + run_id="1", + run_attempt="1", + ) + assert "No changed product files" in review + assert ".github/workflows/opencode-review.yml:1" not in review + + +def test_fallback_review_rejects_accidental_central_workflow_citation( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A synthesized body may not mention the central workflow unless it changed.""" + monkeypatch.setattr(surfaces, "CENTRAL_WORKFLOW_ANCHOR", "Coverage is a separate gate") + with pytest.raises(ValueError, match="must not cite"): + surfaces.build_fallback_review( + changed_files=ORIGINWEAVE_47_FILES, + head_sha=HEAD, + run_id="1", + run_attempt="1", + ) + + +def test_distinct_surfaces_reject_findings_on_status_comment() -> None: + """Status comments cannot carry the formal findings block.""" + with pytest.raises(ValueError, match="status comment must not contain"): + surfaces.distinct_surfaces("review", "## Findings\n") + + +def test_rust_api_symbols_skip_missing_and_symlink_sources(tmp_path: Path) -> None: + """Public-API extraction ignores absent or symlinked sources.""" + assert surfaces.rust_api_symbols(None, ORIGINWEAVE_47_FILES) == [] + missing = surfaces.rust_api_symbols( + tmp_path, ["crates/originweave-destination/src/lib.rs"] + ) + assert missing == [] + target = tmp_path / "outside.rs" + target.write_text("pub struct Leak {}\n", encoding="utf-8") + linked = tmp_path / "crates/originweave-destination/src/lib.rs" + linked.parent.mkdir(parents=True) + linked.symlink_to(target) + assert ( + surfaces.rust_api_symbols(tmp_path, ["crates/originweave-destination/src/lib.rs"]) + == [] + ) + + +def test_rust_api_symbols_replace_invalid_utf8(tmp_path: Path) -> None: + """A malformed Rust text blob cannot abort review-surface publication.""" + source = tmp_path / "lib.rs" + source.write_bytes(b"pub struct BrokenEncoding {\xff\n}\n") + + assert surfaces.rust_api_symbols(tmp_path, ["lib.rs"]) == ["BrokenEncoding"] + + +def test_crates_root_and_grouped_python_surfaces() -> None: + """A bare crates/ path and repeated src/ files keep specific labels.""" + assert surfaces.classify_changed_path("crates")["kind"] == "rust-crate" + grouped = surfaces.classify_surfaces( + ["src/one.py", "src/two.py", "docs/a.md", "docs/b.md"] + ) + python = next(item for item in grouped if item["kind"] == "python") + docs = next(item for item in grouped if item["kind"] == "docs") + assert "2 files" in python["surface"] + assert "2 files" in docs["surface"] + + +def test_surfaces_cover_remaining_review_branches(tmp_path: Path) -> None: + """Empty paths, duplicate symbols, loose Rust files, and control blocks are covered.""" + assert surfaces.classify_surfaces(["", " "]) == [] + source = tmp_path / "lib.rs" + source.write_text( + "pub struct Once {}\npub struct Once {}\n", + encoding="utf-8", + ) + assert surfaces.rust_api_symbols(tmp_path, ["README.md", "lib.rs"]) == ["Once"] + diagram = surfaces.emit_mermaid(["lib.rs"], source_root=tmp_path) + assert "classDiagram" in diagram + assert "Once -->" not in diagram + loose = surfaces.emit_mermaid(["src/resolution.rs"]) + assert "Rust crate" in loose + quoted = surfaces._quote_label('Fresh\r\n"Snapshot"') + assert '"' not in quoted + assert "\n" not in quoted + status = surfaces.build_status_comment( + result="APPROVE", + head_sha=HEAD, + run_id="1", + run_attempt="1", + coverage_result="success", + language="korean", + control_block="", + ) + assert "커버리지 증거 작업이 통과하지 않아" not in status + assert "opencode-review-control-v1" in status + review = surfaces.build_fallback_review( + changed_files=["lib.rs"], + head_sha=HEAD, + run_id="1", + run_attempt="1", + source_root=tmp_path, + language="korean", + ) + assert "변경 API" in review + assert "`Once`" in review + + +def test_cargo_toml_is_a_rust_surface() -> None: + """Root Cargo.toml is a Rust manifest, not a generic changed file.""" + classified = surfaces.classify_changed_path("Cargo.toml") + assert classified["kind"] == "rust" + assert classified["surface"].startswith("Rust manifest:") + diagram = surfaces.emit_mermaid(["Cargo.toml", "crates/demo/src/lib.rs"]) + assert "Changed file" not in diagram + assert "demo" in diagram or "Rust" in diagram + + +def test_extract_model_prose_strips_sentinel_and_control() -> None: + """Publisher keeps walkthrough text and drops the control-plane trailer.""" + raw = ( + "## Verdict\n\nREQUEST_CHANGES\n\n" + "Walkthrough of crates/originweave-destination/src/resolution.rs\n" + "\n" + "\n" + ) + prose = surfaces.extract_model_prose(raw) + assert "Walkthrough of crates/originweave-destination/src/resolution.rs" in prose + assert "opencode-review-gate" not in prose + assert "opencode-review-control-v1" not in prose + + +def test_format_request_changes_keeps_model_prose_and_strips_fake_anchor() -> None: + """REQUEST_CHANGES keeps the model walkthrough and never cites workflow:1.""" + body = surfaces.format_request_changes_review( + model_prose=( + "## Pull request overview\n\n" + "Reviewed resolution.rs and the freshness test.\n\n" + "```mermaid\nsequenceDiagram\n Caller->>Crate: resolve\n```\n" + ), + findings=[ + { + "severity": "HIGH", + "path": ".github/workflows/opencode-review.yml", + "line": 1, + "title": "Coverage evidence failed", + "problem": "gate failed", + "root_cause": "sandbox", + "fix_direction": "fix rustc", + "regression_test_direction": "rerun", + } + ], + head_sha=HEAD, + run_id="31951179896", + run_attempt="1", + reason="coverage blocked", + changed_files=ORIGINWEAVE_47_FILES, + ) + assert "Reviewed resolution.rs and the freshness test." in body + assert "sequenceDiagram" in body + assert "## Findings" in body + assert ".github/workflows/opencode-review.yml:1" not in body + assert "Review process" in body + + +def test_format_request_changes_rebuilds_when_model_prose_missing() -> None: + """Without model prose, structured findings still form a review body.""" + body = surfaces.format_request_changes_review( + model_prose="", + findings=[ + { + "severity": "P1", + "path": "crates/originweave-destination/src/resolution.rs", + "line": 12, + "title": "Stale snapshot", + } + ], + head_sha=HEAD, + run_id="1", + run_attempt="1", + ) + assert "## Verdict" in body + assert "crates/originweave-destination/src/resolution.rs:12" in body + + +def test_cli_extract_and_format_request_changes( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """Workflow CLIs keep model prose and emit a status-safe comment separately.""" + model = tmp_path / "model.md" + model.write_text( + "## Verdict\n\nREQUEST_CHANGES\n\nRelated PRs: none\n" + "\n", + encoding="utf-8", + ) + findings = tmp_path / "findings.json" + findings.write_text( + '[{"severity":"HIGH","path":"crates/demo/src/lib.rs","line":4,"title":"Bug"}]', + encoding="utf-8", + ) + changed = tmp_path / "changed.txt" + changed.write_text("crates/demo/src/lib.rs\n", encoding="utf-8") + assert surfaces.main(["extract-prose", "--model-body-file", str(model)]) == 0 + assert "Related PRs: none" in capsys.readouterr().out + assert ( + surfaces.main( + [ + "format-request-changes", + "--head-sha", + HEAD, + "--run-id", + "1", + "--run-attempt", + "1", + "--model-body-file", + str(model), + "--findings-json-file", + str(findings), + "--changed-files-file", + str(changed), + "--reason", + "bug", + ] + ) + == 0 + ) + rendered = capsys.readouterr().out + assert "Related PRs: none" in rendered + assert "crates/demo/src/lib.rs:4" in rendered + assert ( + surfaces.main( + [ + "build-status", + "--result", + "REQUEST_CHANGES", + "--head-sha", + HEAD, + "--run-id", + "1", + "--run-attempt", + "1", + "--coverage-result", + "failure", + "--model-pool-outcome", + "success", + "--verdict", + "REQUEST_CHANGES", + "--formal-review-url", + "https://github.com/ContextualWisdomLab/OriginWeave/pull/47#pullrequestreview-1", + ] + ) + == 0 + ) + status = capsys.readouterr().out + assert "## Findings" not in status + assert "Model pool: `success`" in status + assert "Verdict: `REQUEST_CHANGES`" in status + assert "pullrequestreview-1" in status + + +def test_central_workflow_line_one_kept_when_that_file_changed() -> None: + """A real edit to the central workflow may cite that file, including line 1.""" + body = surfaces.format_request_changes_review( + model_prose="Inspected `.github/workflows/opencode-review.yml:1`.\n", + findings=[ + { + "path": ".github/workflows/opencode-review.yml", + "line": 1, + "title": "Workflow contract", + } + ], + head_sha=HEAD, + run_id="1", + run_attempt="1", + changed_files=[".github/workflows/opencode-review.yml"], + ) + assert ".github/workflows/opencode-review.yml:1" in body + + +def test_format_request_changes_keeps_existing_findings_heading() -> None: + """Structured findings append under an existing Findings heading.""" + body = surfaces.format_request_changes_review( + model_prose="## Findings\n\nModel already started the findings list.\n", + structured_findings="### 1. HIGH crates/demo/src/lib.rs:3 - Extra", + head_sha=HEAD, + run_id="1", + run_attempt="1", + ) + assert body.count("## Findings") == 1 + assert "Model already started the findings list." in body + assert "crates/demo/src/lib.rs:3" in body + + +def test_format_request_changes_skips_duplicate_identity_and_string_findings() -> None: + """Already-rendered identity/reason lines are not duplicated.""" + prose = ( + "## Verdict\n\nREQUEST_CHANGES\n\n" + f"- Head SHA: `{HEAD}`\n" + "- Reason: already stated\n" + ) + body = surfaces.format_request_changes_review( + model_prose=prose, + structured_findings="### 1. HIGH crates/demo/src/lib.rs:2 - Bug", + head_sha=HEAD, + run_id="1", + run_attempt="1", + reason="already stated", + ) + assert body.count(f"- Head SHA: `{HEAD}`") == 1 + assert body.count("- Reason: already stated") == 1 + assert "crates/demo/src/lib.rs:2" in body + + +def test_format_request_changes_cli_handles_object_findings_json( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """A non-list findings document is ignored instead of crashing publish.""" + findings = tmp_path / "findings.json" + findings.write_text('{"nope": true}', encoding="utf-8") + assert ( + surfaces.main( + [ + "format-request-changes", + "--head-sha", + HEAD, + "--run-id", + "1", + "--run-attempt", + "1", + "--findings-json-file", + str(findings), + ] + ) + == 0 + ) + assert "## Verdict" in capsys.readouterr().out + assert ( + surfaces.main( + [ + "format-request-changes", + "--head-sha", + HEAD, + "--run-id", + "1", + "--run-attempt", + "1", + ] + ) + == 0 + ) + assert "REQUEST_CHANGES" in capsys.readouterr().out + + +def test_format_structured_findings_skips_non_mappings() -> None: + """Non-object findings are ignored so a bad control array cannot crash publish.""" + assert surfaces.format_structured_findings(["skip", 1]) == "" + + +def test_publisher_workflow_cannot_replace_review_with_coverage_finding( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The #47 publisher shape — coverage REQUEST_CHANGES as the whole review — is gone.""" + monkeypatch.chdir(tmp_path) + workflow = (ROOT / ".github/workflows/opencode-review-dispatch.yml").read_text( + encoding="utf-8" + ) + assert "publish_fallback_diff_review" in workflow + assert "opencode_review_surfaces.py build-status" in workflow + assert "opencode_review_surfaces.py build-fallback-review" in workflow + assert ".github/workflows/opencode-review.yml:1" not in workflow + coverage_fn = workflow.split("request_changes_for_coverage_evidence_failure()", 1)[1] + coverage_fn = coverage_fn.split("create_pull_review_with_payload()", 1)[0] + assert "create_pull_review" not in coverage_fn + assert "update_review_overview" in coverage_fn + assert 'update_review_overview "COVERAGE_BLOCKED"' in coverage_fn + fallback_fn = workflow.split("publish_fallback_diff_review()", 1)[1] + fallback_fn = fallback_fn.split("request_changes_for_coverage_evidence_failure()", 1)[0] + assert "create_pull_review" in fallback_fn + assert "request_changes_for_coverage_evidence_failure" in fallback_fn + assert fallback_fn.index("create_pull_review") < fallback_fn.index( + "request_changes_for_coverage_evidence_failure" + ) + model_skip = workflow.split("if [ \"$opencode_review_outcome\" != \"success\" ]; then", 1)[1] + model_skip = model_skip.split("selected_review_output_file=", 1)[0] + assert "publish_fallback_diff_review" in model_skip From 7c9e47214716a6dc0c173c57f290035e7889da5a Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 12:47:46 +0000 Subject: [PATCH 02/48] fix(ci): restore OpenCode receipt-gate and coverage identity Additive on 848d796. Exact #1052@abf47ce blobs for coverage-identity and receipt-gate, plus the smallest dispatch invocations so they run. Honesty surfaces stay. PRT stub stays 7f1ad6d0. Does not touch noema-review.yml. --- .../workflows/opencode-review-dispatch.yml | 37 +++ scripts/ci/opencode_coverage_identity.py | 210 +++++++++++++ scripts/ci/opencode_review_receipt_gate.py | 235 ++++++++++++++ tests/test_opencode_coverage_identity.py | 238 ++++++++++++++ tests/test_opencode_review_receipt_gate.py | 291 ++++++++++++++++++ 5 files changed, 1011 insertions(+) create mode 100644 scripts/ci/opencode_coverage_identity.py create mode 100644 scripts/ci/opencode_review_receipt_gate.py create mode 100644 tests/test_opencode_coverage_identity.py create mode 100644 tests/test_opencode_review_receipt_gate.py diff --git a/.github/workflows/opencode-review-dispatch.yml b/.github/workflows/opencode-review-dispatch.yml index b7befcf0fb..e106d0182b 100644 --- a/.github/workflows/opencode-review-dispatch.yml +++ b/.github/workflows/opencode-review-dispatch.yml @@ -2733,6 +2733,7 @@ jobs: OPENCODE_EVIDENCE_FILE: ${{ runner.temp }}/opencode-review-evidence.md OPENCODE_FAILED_CHECK_EVIDENCE_FILE: ${{ runner.temp }}/opencode-failed-check-evidence.md OPENCODE_CHANGED_FILES_FILE: ${{ runner.temp }}/opencode-changed-files.txt + COVERAGE_EVIDENCE_RESULT: ${{ needs.coverage-evidence.result || 'skipped' }} COVERAGE_EVIDENCE_SUMMARY: ${{ needs.coverage-evidence.outputs.coverage_summary || 'Coverage evidence job did not run or did not publish coverage evidence.' }} FAILED_CHECK_EVIDENCE_ATTEMPTS: "6" FAILED_CHECK_EVIDENCE_SLEEP_SECONDS: "5" @@ -2745,6 +2746,12 @@ jobs: --env-file "$context_env_file" # shellcheck source=/dev/null . "$context_env_file" + quoted_coverage="${COVERAGE_EVIDENCE_RESULT:-}" + COVERAGE_EVIDENCE_RESULT="$(python3 scripts/ci/opencode_coverage_identity.py \ + --repo "$GH_REPOSITORY" \ + --head-sha "$PR_HEAD_SHA" \ + --quoted-result "$quoted_coverage")" + export COVERAGE_EVIDENCE_RESULT printf 'Resolved bounded OpenCode review context for %s#%s at %s.\n' \ "$GH_REPOSITORY" "$PR_NUMBER" "$PR_HEAD_SHA" @@ -5192,6 +5199,12 @@ jobs: OPENCODE_EXPORT_TIMEOUT_SECONDS: "60" run: | set -euo pipefail + quoted_coverage="${COVERAGE_EVIDENCE_RESULT:-}" + COVERAGE_EVIDENCE_RESULT="$(python3 scripts/ci/opencode_coverage_identity.py \ + --repo "$GH_REPOSITORY" \ + --head-sha "$HEAD_SHA" \ + --quoted-result "$quoted_coverage")" + export COVERAGE_EVIDENCE_RESULT echo "::group::OpenCode Review Approval Gate" echo "PR=#${PR_NUMBER} head_sha=${HEAD_SHA} run_id=${RUN_ID} run_attempt=${RUN_ATTEMPT}" configured_review_write_token="${GH_TOKEN:-}" @@ -8059,6 +8072,30 @@ jobs: fi echo "::endgroup::" + - name: Enforce current-head formal OpenCode review receipt + if: >- + always() + && needs.validate-pr-metadata.result == 'success' + && needs.validate-pr-metadata.outputs.target_repository != '' + && needs.validate-pr-metadata.outputs.pr_number != '' + && needs.validate-pr-metadata.outputs.head_sha != '' + env: + GH_TOKEN: ${{ steps.opencode_app_token.outputs.token || secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN || github.token }} + GH_REPOSITORY: ${{ needs.validate-pr-metadata.outputs.target_repository }} + PR_NUMBER: ${{ needs.validate-pr-metadata.outputs.pr_number }} + PR_HEAD_SHA: ${{ needs.validate-pr-metadata.outputs.head_sha }} + run: | + set -euo pipefail + draft_args=() + if [ "$(gh api "repos/${GH_REPOSITORY}/pulls/${PR_NUMBER}" --jq '.draft')" = "true" ]; then + draft_args=(--draft) + fi + python3 scripts/ci/opencode_review_receipt_gate.py \ + --repo "$GH_REPOSITORY" \ + --pr-number "$PR_NUMBER" \ + --head-sha "$PR_HEAD_SHA" \ + "${draft_args[@]}" + - name: Publish repository_dispatch OpenCode status if: >- always() diff --git a/scripts/ci/opencode_coverage_identity.py b/scripts/ci/opencode_coverage_identity.py new file mode 100644 index 0000000000..b0d9e12fe7 --- /dev/null +++ b/scripts/ci/opencode_coverage_identity.py @@ -0,0 +1,210 @@ +#!/usr/bin/env python3 +"""Verify a quoted coverage conclusion against the canonical exact-head check.""" + +from __future__ import annotations + +import argparse +import json +import os +import re +import subprocess +import sys +from collections.abc import Mapping, Sequence +from pathlib import Path +from typing import Any + + +CANONICAL_CHECK_NAME = "coverage-evidence" +CANONICAL_WORKFLOW_NAMES = frozenset({"Required OpenCode Review"}) +SHA_RE = re.compile(r"^[0-9a-fA-F]{40}$") +REPO_RE = re.compile(r"^[A-Za-z0-9_][A-Za-z0-9_.-]*/[A-Za-z0-9_][A-Za-z0-9_.-]*$") +TERMINAL_RESULTS = frozenset( + {"success", "failure", "cancelled", "skipped", "neutral", "timed_out", "action_required"} +) + +KAEFA_78_HEAD = "5092a70c9737221d6367e74643d06980609fe0b1" +KAEFA_75_HEAD = "4c8ad480a0f104601ca668cee5f0cf9372e819c3" +KAEFA_79_HEAD = "1c5d9f0491fc178be3f7f307dac521fbcbba6978" + + +class CoverageQuoteError(ValueError): + """Raised when a review would quote a coverage result that is not canonical.""" + + +def normalize_result(value: str) -> str: + """Return a lowercase GitHub check conclusion or ``unknown``.""" + normalized = str(value or "").strip().casefold() + if normalized in TERMINAL_RESULTS: + return normalized + return "unknown" + + +def check_head_sha(check: Mapping[str, Any]) -> str: + """Return the commit SHA recorded on a check-run object.""" + head = check.get("head_sha") or check.get("headSha") or "" + return str(head).strip() + + +def check_workflow_name(check: Mapping[str, Any]) -> str: + """Return the workflow name that produced a check-run, if present. + + The REST list-check-runs response's ``check_suite`` object does not carry a + ``workflow_run`` field, so this almost always returns "". ``app.name`` is + always "GitHub Actions" for Actions-produced checks, not the workflow name, + so it is not an acceptable fallback: a canonical exact-head check would be + rejected by a workflow-name mismatch it can never satisfy. An empty result + defers to ``is_canonical_coverage_check``'s ``not workflow`` acceptance. + """ + suite = check.get("check_suite") or check.get("checkSuite") or {} + if isinstance(suite, Mapping): + run = suite.get("workflow_run") or suite.get("workflowRun") or {} + if isinstance(run, Mapping): + workflow = run.get("workflow") or {} + if isinstance(workflow, Mapping): + name = str(workflow.get("name") or "").strip() + if name: + return name + return "" + + +def is_canonical_coverage_check(check: Mapping[str, Any], head_sha: str) -> bool: + """Return whether a check-run is the exact-head canonical coverage-evidence check.""" + if str(check.get("name") or "").strip() != CANONICAL_CHECK_NAME: + return False + if check_head_sha(check).lower() != head_sha.lower(): + return False + status = str(check.get("status") or "").strip().casefold() + if status and status != "completed": + return False + workflow = check_workflow_name(check) + return not workflow or workflow in CANONICAL_WORKFLOW_NAMES + + +def terminal_coverage_result( + check_runs: Sequence[Mapping[str, Any]], head_sha: str +) -> str: + """Return the terminal canonical coverage-evidence conclusion for ``head_sha``.""" + if not SHA_RE.fullmatch(head_sha): + raise CoverageQuoteError("coverage identity requires a 40-character head SHA") + matches = [ + check + for check in check_runs + if isinstance(check, Mapping) and is_canonical_coverage_check(check, head_sha) + ] + if not matches: + raise CoverageQuoteError( + f"no completed canonical {CANONICAL_CHECK_NAME} check for head {head_sha}" + ) + preferred = [ + check + for check in matches + if check_workflow_name(check) in CANONICAL_WORKFLOW_NAMES + ] + chosen = preferred[-1] if preferred else matches[-1] + result = normalize_result(str(chosen.get("conclusion") or "")) + if result == "unknown": + raise CoverageQuoteError( + f"canonical {CANONICAL_CHECK_NAME} conclusion is missing or non-terminal" + ) + return result + + +def assert_quoted_matches( + quoted_result: str, check_runs: Sequence[Mapping[str, Any]], head_sha: str +) -> str: + """Return the canonical result or raise when the quoted conclusion differs.""" + canonical = terminal_coverage_result(check_runs, head_sha) + quoted = normalize_result(quoted_result) + if quoted != canonical: + raise CoverageQuoteError( + f"quoted coverage-evidence result {quoted!r} does not match " + f"canonical exact-head result {canonical!r} for {head_sha}" + ) + return canonical + + +def load_check_runs(path: str | None) -> list[Mapping[str, Any]]: + """Load check-run objects from a JSON file or stdin.""" + raw = sys.stdin.read() if not path or path == "-" else Path(path).read_text(encoding="utf-8") + loaded = json.loads(raw) + if isinstance(loaded, Mapping) and isinstance(loaded.get("check_runs"), list): + loaded = loaded["check_runs"] + if not isinstance(loaded, list): + raise CoverageQuoteError("coverage identity payload must be a check-run array") + return [item for item in loaded if isinstance(item, Mapping)] + + +def fetch_check_runs(repo: str, head_sha: str) -> list[Mapping[str, Any]]: + """Read exact-head check-runs through gh without invoking a shell.""" + if not REPO_RE.fullmatch(repo): + raise CoverageQuoteError(f"coverage identity requires an owner/repo value, got {repo!r}") + if not SHA_RE.fullmatch(head_sha): + raise CoverageQuoteError("coverage identity requires a 40-character head SHA") + completed = subprocess.run( + [ + "gh", + "api", + f"repos/{repo}/commits/{head_sha}/check-runs?per_page=100", + "--paginate", + "--slurp", + ], + text=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + check=False, + shell=False, + ) + if completed.returncode != 0: + detail = (completed.stderr or completed.stdout or "gh check-runs lookup failed").strip() + raise CoverageQuoteError(f"canonical coverage check lookup failed: {detail}") + loaded = json.loads(completed.stdout or "{}") + if isinstance(loaded, list): + runs: list[Mapping[str, Any]] = [] + for page in loaded: + if isinstance(page, Mapping) and isinstance(page.get("check_runs"), list): + runs.extend( + item for item in page["check_runs"] if isinstance(item, Mapping) + ) + elif isinstance(page, Mapping): + runs.append(page) + return runs + if isinstance(loaded, Mapping) and isinstance(loaded.get("check_runs"), list): + return [item for item in loaded["check_runs"] if isinstance(item, Mapping)] + raise CoverageQuoteError("canonical coverage check lookup returned malformed JSON") + + +def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace: + """Parse coverage-identity CLI arguments.""" + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--repo", default="") + parser.add_argument("--head-sha", required=True) + parser.add_argument("--quoted-result", required=True) + parser.add_argument("--check-runs-file") + return parser.parse_args(argv) + + +def main(argv: Sequence[str] | None = None) -> int: + """Verify a quoted coverage conclusion and print the canonical result.""" + args = parse_args(argv) + try: + if args.check_runs_file: + checks = load_check_runs(args.check_runs_file) + elif args.repo: + checks = fetch_check_runs(args.repo, args.head_sha) + else: + raise CoverageQuoteError("coverage identity needs --repo or --check-runs-file") + canonical = assert_quoted_matches(args.quoted_result, checks, args.head_sha) + except (CoverageQuoteError, json.JSONDecodeError, OSError) as exc: + print(f"::error::{exc}", file=sys.stderr) + summary = os.environ.get("GITHUB_STEP_SUMMARY") + if summary: + with open(summary, "a", encoding="utf-8") as handle: + handle.write("## Coverage identity failure\n\n") + handle.write(f"{exc}\n") + return 1 + sys.stdout.write(f"{canonical}\n") + return 0 + + +if __name__ == "__main__": # pragma: no cover + raise SystemExit(main()) diff --git a/scripts/ci/opencode_review_receipt_gate.py b/scripts/ci/opencode_review_receipt_gate.py new file mode 100644 index 0000000000..939cf90a8b --- /dev/null +++ b/scripts/ci/opencode_review_receipt_gate.py @@ -0,0 +1,235 @@ +#!/usr/bin/env python3 +"""Require a current-head formal OpenCode review receipt before a required check is green.""" + +from __future__ import annotations + +import argparse +import json +import os +import re +import subprocess +import sys +from collections.abc import Mapping, Sequence +from pathlib import Path +from typing import Any + + +SHA_RE = re.compile(r"^[0-9a-fA-F]{40}$") +REPO_RE = re.compile(r"^[A-Za-z0-9_][A-Za-z0-9_.-]*/[A-Za-z0-9_][A-Za-z0-9_.-]*$") +HEAD_SHA_IN_BODY_RE = re.compile(r"Head SHA:\s*`([0-9a-fA-F]{40})`") +FORMAL_AUTHORS = frozenset( + {"opencode-agent", "opencode-agent[bot]", "github-actions[bot]"} +) +FORMAL_STATES = frozenset({"APPROVED", "CHANGES_REQUESTED", "COMMENTED"}) +STATUS_HEADINGS = ("## OpenCode Review Status", "## OpenCode 게이트 상태") +PRODUCT_MARKERS = ( + "## Pull request overview", + "## Pull request 개요", + "## Changed files", + "## Changed API", + "## Verdict", + "opencode-review-control-v1", + "OpenCode reviewed the current-head product diff", + "OpenCode reviewed the current-head bounded evidence", +) +MENTION_RE = re.compile(r"^@opencode-agent\b", re.IGNORECASE) + +AFIPC_230_HEAD = "5eda857066c9207786d3bdde49826f8f94b98c12" +AFIPC_230_STALE_HEADS = frozenset( + { + "8a1133d406d0d15b425644e0dc3910f112ccbb36", + "8757e7b022cb66f21886d4c241857a9986ef7a6c", + } +) +KAEFA_79_HEAD = "1c5d9f0491fc178be3f7f307dac521fbcbba6978" + + +class ReceiptGateError(ValueError): + """Raised when the required OpenCode check lacks a current-head formal receipt.""" + + +def review_author(review: Mapping[str, Any]) -> str: + """Return the login for a REST or GraphQL review object.""" + user = review.get("user") or review.get("author") or {} + if isinstance(user, Mapping): + return str(user.get("login") or "").strip() + return "" + + +def review_commit(review: Mapping[str, Any]) -> str: + """Return the commit SHA the review was submitted against.""" + commit_id = str(review.get("commit_id") or "").strip() + if commit_id: + return commit_id + commit = review.get("commit") or {} + if isinstance(commit, Mapping): + return str(commit.get("oid") or commit.get("sha") or "").strip() + return "" + + +def review_body_head_sha(review: Mapping[str, Any]) -> str | None: + """Return the last explicit Head SHA recorded in a review body.""" + matches = HEAD_SHA_IN_BODY_RE.findall(str(review.get("body") or "")) + return matches[-1] if matches else None + + +def review_matches_head(review: Mapping[str, Any], head_sha: str) -> bool: + """Return whether commit and optional body SHA both match the live head.""" + if not head_sha or review_commit(review).lower() != head_sha.lower(): + return False + body_head = review_body_head_sha(review) + return body_head is None or body_head.lower() == head_sha.lower() + + +def is_mention_or_malformed(body: str) -> bool: + """Return whether a body is a mention payload or not a product-file review.""" + stripped = body.strip() + if not stripped: + return True + first_line = stripped.splitlines()[0].strip() + if MENTION_RE.match(first_line) and "Head SHA:" not in stripped: + return True + if any(heading in stripped for heading in STATUS_HEADINGS) and not any( + marker in stripped for marker in PRODUCT_MARKERS + ): + return True + return not any(marker in stripped for marker in PRODUCT_MARKERS) + + +def is_formal_receipt( + review: Mapping[str, Any], + head_sha: str, + *, + is_draft: bool, +) -> tuple[bool, str]: + """Return whether a review is a usable current-head formal product-file receipt.""" + if not review_matches_head(review, head_sha): + return False, "stale or mismatched head" + author = review_author(review) + if author not in FORMAL_AUTHORS: + return False, f"author {author or ''} is not an OpenCode publisher" + state = str(review.get("state") or "").upper() + if state not in FORMAL_STATES: + return False, f"state {state or ''} is not a formal review verdict" + if not review.get("id"): + return False, "missing pullrequestreview id" + body = str(review.get("body") or "") + if is_mention_or_malformed(body): + return False, "mention, status-only, or malformed payload is not a formal review" + if is_draft and state == "APPROVED": + return False, "draft must never receive bot APPROVE" + return True, "current-head formal review" + + +def evaluate_receipts( + reviews: Sequence[Mapping[str, Any]], + head_sha: str, + *, + is_draft: bool = False, +) -> tuple[Mapping[str, Any] | None, str]: + """Return the current-head formal receipt or explain why the gate fails.""" + if not SHA_RE.fullmatch(head_sha): + return None, "receipt gate requires a 40-character head SHA" + stale_hits = 0 + for review in reversed(list(reviews)): + if not isinstance(review, Mapping): + continue + commit = review_commit(review) + if commit and commit.lower() != head_sha.lower(): + stale_hits += 1 + continue + ok, reason = is_formal_receipt(review, head_sha, is_draft=is_draft) + if ok: + return review, reason + if "never receive bot APPROVE" in reason: + return None, reason + if reason.startswith("stale"): + stale_hits += 1 + continue + if stale_hits: + return ( + None, + "stale CHANGES_REQUESTED or prior-head reviews are not current-head receipts", + ) + return None, "no current-head formal OpenCode review receipt" + + +def load_reviews(path: str | None) -> list[Mapping[str, Any]]: + """Load review objects from a JSON file or stdin.""" + raw = sys.stdin.read() if not path or path == "-" else Path(path).read_text(encoding="utf-8") + loaded = json.loads(raw) + if not isinstance(loaded, list): + raise ReceiptGateError("review payload must be a JSON array") + return [item for item in loaded if isinstance(item, Mapping)] + + +def fetch_reviews(repo: str, number: int) -> list[Mapping[str, Any]]: + """Read pull-request reviews through gh without invoking a shell.""" + if not REPO_RE.fullmatch(repo): + raise ReceiptGateError(f"receipt gate requires an owner/repo value, got {repo!r}") + completed = subprocess.run( + [ + "gh", + "api", + f"repos/{repo}/pulls/{number}/reviews", + "--paginate", + ], + text=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + check=False, + shell=False, + ) + if completed.returncode != 0: + detail = (completed.stderr or completed.stdout or "gh reviews lookup failed").strip() + raise ReceiptGateError(f"formal review receipt lookup failed: {detail}") + loaded = json.loads(completed.stdout or "[]") + if isinstance(loaded, list): + return [item for item in loaded if isinstance(item, Mapping)] + raise ReceiptGateError("formal review receipt lookup returned malformed JSON") + + +def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace: + """Parse receipt-gate CLI arguments.""" + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--repo", default="") + parser.add_argument("--pr-number", type=int, default=0) + parser.add_argument("--head-sha", required=True) + parser.add_argument("--draft", action="store_true") + parser.add_argument("--reviews-file") + return parser.parse_args(argv) + + +def main(argv: Sequence[str] | None = None) -> int: + """Fail closed unless a verifiable current-head formal review receipt exists.""" + args = parse_args(argv) + try: + if args.reviews_file: + reviews = load_reviews(args.reviews_file) + elif args.repo and args.pr_number > 0: + reviews = fetch_reviews(args.repo, args.pr_number) + else: + raise ReceiptGateError("receipt gate needs --reviews-file or --repo/--pr-number") + receipt, reason = evaluate_receipts( + reviews, args.head_sha, is_draft=args.draft + ) + if receipt is None: + raise ReceiptGateError(reason) + except (ReceiptGateError, json.JSONDecodeError, OSError) as exc: + print(f"::error::{exc}", file=sys.stderr) + summary = os.environ.get("GITHUB_STEP_SUMMARY") + if summary: + with open(summary, "a", encoding="utf-8") as handle: + handle.write("## OpenCode formal review receipt missing\n\n") + handle.write(f"{exc}\n") + return 1 + review_id = receipt.get("id") + print( + f"Current-head formal OpenCode receipt id={review_id} " + f"state={receipt.get('state')} head={args.head_sha}" + ) + return 0 + + +if __name__ == "__main__": # pragma: no cover + raise SystemExit(main()) diff --git a/tests/test_opencode_coverage_identity.py b/tests/test_opencode_coverage_identity.py new file mode 100644 index 0000000000..8a695b550b --- /dev/null +++ b/tests/test_opencode_coverage_identity.py @@ -0,0 +1,238 @@ +"""Regression tests for exact-head canonical coverage-evidence quoting.""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from scripts.ci import opencode_coverage_identity as identity + + +def coverage_check( + *, + head: str, + conclusion: str = "success", + workflow: str = "Required OpenCode Review", + name: str = "coverage-evidence", + status: str = "completed", +) -> dict[str, object]: + """Build one GitHub check-run object for coverage identity tests.""" + return { + "name": name, + "head_sha": head, + "status": status, + "conclusion": conclusion, + "check_suite": {"workflow_run": {"workflow": {"name": workflow}}}, + } + + +def test_kaefa_78_and_75_reject_false_failure_quotes() -> None: + """Canonical exact-head success must not be quoted as coverage failure.""" + for head in (identity.KAEFA_78_HEAD, identity.KAEFA_75_HEAD): + checks = [coverage_check(head=head, conclusion="success")] + assert identity.terminal_coverage_result(checks, head) == "success" + with pytest.raises(identity.CoverageQuoteError, match="does not match"): + identity.assert_quoted_matches("failure", checks, head) + assert identity.assert_quoted_matches("success", checks, head) == "success" + + +def test_kaefa_79_missing_canonical_check_fails_closed() -> None: + """A stub-only head without canonical coverage-evidence cannot be quoted.""" + with pytest.raises(identity.CoverageQuoteError, match="no completed canonical"): + identity.terminal_coverage_result([], identity.KAEFA_79_HEAD) + + +def test_identity_helpers_cover_malformed_and_noncanonical_checks() -> None: + """Malformed SHA, other workflows, and in-progress checks fail closed.""" + assert identity.normalize_result("SUCCESS") == "success" + assert identity.normalize_result("nope") == "unknown" + assert identity.check_head_sha({"headSha": "abc"}) == "abc" + assert identity.check_workflow_name({"checkSuite": {"workflowRun": {}}}) == "" + assert identity.check_workflow_name({"app": {"name": "GitHub Actions"}}) == "" + assert identity.check_workflow_name({"check_suite": "bad"}) == "" + assert identity.check_workflow_name({"check_suite": {"workflow_run": "bad"}}) == "" + assert identity.check_workflow_name({"app": "nope"}) == "" + head = identity.KAEFA_78_HEAD + with pytest.raises(identity.CoverageQuoteError, match="40-character"): + identity.terminal_coverage_result([], "deadbeef") + in_progress = coverage_check(head=head, status="in_progress", conclusion="") + assert identity.is_canonical_coverage_check(in_progress, head) is False + other = coverage_check(head=head, name="strix") + assert identity.is_canonical_coverage_check(other, head) is False + wrong_head = coverage_check(head=identity.KAEFA_75_HEAD) + assert identity.is_canonical_coverage_check(wrong_head, head) is False + unnamed = coverage_check(head=head, workflow="") + unnamed["check_suite"] = {"workflow_run": {"workflow": {}}} + assert identity.terminal_coverage_result([unnamed], head) == "success" + string_workflow = coverage_check(head=head) + string_workflow["check_suite"] = {"workflow_run": {"workflow": "Required OpenCode Review"}} + string_workflow["app"] = {"name": "GitHub Actions"} + assert identity.check_workflow_name(string_workflow) == "" + missing_conclusion = coverage_check(head=head, conclusion="") + with pytest.raises(identity.CoverageQuoteError, match="non-terminal"): + identity.terminal_coverage_result([missing_conclusion], head) + + +def test_app_only_check_run_is_still_canonical() -> None: + """A completed exact-head check with only an app.name (the real REST shape, + which never carries check_suite.workflow_run) must still be accepted.""" + head = identity.KAEFA_78_HEAD + app_only = coverage_check(head=head, conclusion="success") + app_only["check_suite"] = {} + app_only["app"] = {"name": "GitHub Actions"} + assert identity.check_workflow_name(app_only) == "" + assert identity.is_canonical_coverage_check(app_only, head) is True + assert identity.terminal_coverage_result([app_only], head) == "success" + + +def test_load_and_cli_verify_quoted_success(tmp_path: Path, capsys, monkeypatch) -> None: + """CLI prints the canonical result and annotates quote mismatches.""" + head = identity.KAEFA_78_HEAD + payload = {"check_runs": [coverage_check(head=head, conclusion="success")]} + path = tmp_path / "checks.json" + path.write_text(json.dumps(payload), encoding="utf-8") + assert identity.main( + ["--head-sha", head, "--quoted-result", "success", "--check-runs-file", str(path)] + ) == 0 + assert capsys.readouterr().out.strip() == "success" + + summary = tmp_path / "summary.md" + monkeypatch.setenv("GITHUB_STEP_SUMMARY", str(summary)) + assert identity.main( + ["--head-sha", head, "--quoted-result", "failure", "--check-runs-file", str(path)] + ) == 1 + err = capsys.readouterr().err + assert "does not match" in err + assert "Coverage identity failure" in summary.read_text(encoding="utf-8") + + monkeypatch.delenv("GITHUB_STEP_SUMMARY", raising=False) + assert identity.main(["--head-sha", head, "--quoted-result", "success"]) == 1 + array_path = tmp_path / "array.json" + array_path.write_text(json.dumps([coverage_check(head=head)]), encoding="utf-8") + assert identity.main( + ["--head-sha", head, "--quoted-result", "success", "--check-runs-file", str(array_path)] + ) == 0 + bad = tmp_path / "bad.json" + bad.write_text("{}", encoding="utf-8") + assert identity.main( + ["--head-sha", head, "--quoted-result", "success", "--check-runs-file", str(bad)] + ) == 1 + stdin_payload = json.dumps([coverage_check(head=head, conclusion="success")]) + monkeypatch.setattr(identity.sys, "stdin", type("Stdin", (), {"read": lambda self: stdin_payload})()) + assert identity.load_check_runs("-")[0]["name"] == "coverage-evidence" + broken = tmp_path / "broken.json" + broken.write_text("{", encoding="utf-8") + assert identity.main( + ["--head-sha", head, "--quoted-result", "success", "--check-runs-file", str(broken)] + ) == 1 + + +def test_fetch_check_runs_rejects_unvalidated_repo_and_head_sha(monkeypatch) -> None: + """A malformed --repo or --head-sha never reaches the gh api path string.""" + + def unexpected_run(args, **kwargs): + raise AssertionError(f"gh must not be invoked with unvalidated input: {args!r}") + + monkeypatch.setattr(identity.subprocess, "run", unexpected_run) + with pytest.raises(identity.CoverageQuoteError, match="owner/repo"): + identity.fetch_check_runs("../evil", identity.KAEFA_78_HEAD) + with pytest.raises(identity.CoverageQuoteError, match="40-character"): + identity.fetch_check_runs("ContextualWisdomLab/kaefa", "not-a-sha") + + +def test_fetch_check_runs_parses_pages(monkeypatch) -> None: + """Paginated gh output and error paths stay fail-closed.""" + page = { + "check_runs": [ + coverage_check(head=identity.KAEFA_78_HEAD, conclusion="success") + ] + } + + def fake_run(args, **kwargs): + assert args[0] == "gh" + assert "--paginate" in args + assert "--slurp" in args + return type("Completed", (), {"returncode": 0, "stdout": json.dumps([page]), "stderr": ""})() + + monkeypatch.setattr(identity.subprocess, "run", fake_run) + loaded = identity.fetch_check_runs("ContextualWisdomLab/kaefa", identity.KAEFA_78_HEAD) + assert loaded[0]["name"] == "coverage-evidence" + + def fake_object(args, **kwargs): + return type( + "Completed", + (), + {"returncode": 0, "stdout": json.dumps(page), "stderr": ""}, + )() + + monkeypatch.setattr(identity.subprocess, "run", fake_object) + assert identity.fetch_check_runs("ContextualWisdomLab/kaefa", identity.KAEFA_78_HEAD) + + def fake_fail(args, **kwargs): + return type("Completed", (), {"returncode": 1, "stdout": "", "stderr": "boom"})() + + monkeypatch.setattr(identity.subprocess, "run", fake_fail) + with pytest.raises(identity.CoverageQuoteError, match="lookup failed"): + identity.fetch_check_runs("ContextualWisdomLab/kaefa", identity.KAEFA_78_HEAD) + + def fake_bad_json(args, **kwargs): + return type("Completed", (), {"returncode": 0, "stdout": '"nope"', "stderr": ""})() + + monkeypatch.setattr(identity.subprocess, "run", fake_bad_json) + with pytest.raises(identity.CoverageQuoteError, match="malformed"): + identity.fetch_check_runs("ContextualWisdomLab/kaefa", identity.KAEFA_78_HEAD) + + def fake_list_objects(args, **kwargs): + return type( + "Completed", + (), + { + "returncode": 0, + "stdout": json.dumps([coverage_check(head=identity.KAEFA_78_HEAD)]), + "stderr": "", + }, + )() + + monkeypatch.setattr(identity.subprocess, "run", fake_list_objects) + assert identity.fetch_check_runs("ContextualWisdomLab/kaefa", identity.KAEFA_78_HEAD) + + def fake_mixed_pages(args, **kwargs): + return type( + "Completed", + (), + { + "returncode": 0, + "stdout": json.dumps( + [ + {"check_runs": [coverage_check(head=identity.KAEFA_78_HEAD)]}, + coverage_check(head=identity.KAEFA_78_HEAD), + "skip", + ] + ), + "stderr": "", + }, + )() + + monkeypatch.setattr(identity.subprocess, "run", fake_mixed_pages) + assert len(identity.fetch_check_runs("ContextualWisdomLab/kaefa", identity.KAEFA_78_HEAD)) == 2 + + monkeypatch.setattr( + identity, + "fetch_check_runs", + lambda repo, head: [coverage_check(head=head, conclusion="success")], + ) + assert ( + identity.main( + [ + "--repo", + "ContextualWisdomLab/kaefa", + "--head-sha", + identity.KAEFA_78_HEAD, + "--quoted-result", + "success", + ] + ) + == 0 + ) diff --git a/tests/test_opencode_review_receipt_gate.py b/tests/test_opencode_review_receipt_gate.py new file mode 100644 index 0000000000..8ef47273d5 --- /dev/null +++ b/tests/test_opencode_review_receipt_gate.py @@ -0,0 +1,291 @@ +"""Formal-review receipt tests, including aFIPC stale-head and kaefa stub fixtures.""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from scripts.ci import opencode_review_receipt_gate as receipt + + +def review( + *, + commit: str, + state: str = "CHANGES_REQUESTED", + login: str = "opencode-agent[bot]", + body: str = "", + review_id: int = 1, +) -> dict[str, object]: + """Build one REST pull-request review object.""" + if not body: + body = ( + "## Pull request overview\n\n" + "OpenCode reviewed the current-head product diff. Coverage is a separate gate.\n\n" + f"- Head SHA: `{commit}`\n" + ) + return { + "id": review_id, + "state": state, + "body": body, + "user": {"login": login}, + "commit_id": commit, + } + + +def test_afipc_230_stale_changes_requested_are_not_current() -> None: + """Stale OpenCode CHANGES_REQUESTED on old aFIPC heads cannot satisfy 5eda857.""" + stale = [ + review(commit=head, review_id=index) + for index, head in enumerate(sorted(receipt.AFIPC_230_STALE_HEADS), start=10) + ] + found, reason = receipt.evaluate_receipts(stale, receipt.AFIPC_230_HEAD) + assert found is None + assert "stale" in reason + current = review(commit=receipt.AFIPC_230_HEAD, state="COMMENTED", review_id=99) + found, reason = receipt.evaluate_receipts([*stale, current], receipt.AFIPC_230_HEAD) + assert found is current + assert "formal review" in reason + + +def test_kaefa_79_stub_has_no_current_head_formal_receipt() -> None: + """A 3-second green stub without a product-file review stays fail-closed.""" + found, reason = receipt.evaluate_receipts([], receipt.KAEFA_79_HEAD) + assert found is None + assert "no current-head formal" in reason + + +def test_draft_never_accepts_bot_approve_as_receipt() -> None: + """Draft PRs may have a COMMENT product review, never a bot APPROVE receipt.""" + approve = review( + commit=receipt.AFIPC_230_HEAD, + state="APPROVED", + body=( + "OpenCode reviewed the current-head bounded evidence and found no blocking issues.\n" + f"- Head SHA: `{receipt.AFIPC_230_HEAD}`\n" + "- Result: APPROVE\n" + ), + ) + found, reason = receipt.evaluate_receipts( + [approve], receipt.AFIPC_230_HEAD, is_draft=True + ) + assert found is None + assert "never receive bot APPROVE" in reason + comment = review(commit=receipt.AFIPC_230_HEAD, state="COMMENTED") + found, _ = receipt.evaluate_receipts( + [comment], receipt.AFIPC_230_HEAD, is_draft=True + ) + assert found is comment + + +def test_status_comment_and_mention_payloads_are_not_receipts() -> None: + """Issue-comment status text and @mentions cannot green the required check.""" + status = review( + commit=receipt.AFIPC_230_HEAD, + body="## OpenCode Review Status\n\n- Gate result: `COMMENT`\n", + ) + ok, reason = receipt.is_formal_receipt( + status, receipt.AFIPC_230_HEAD, is_draft=False + ) + assert ok is False + assert "status-only" in reason or "malformed" in reason + mention = review(commit=receipt.AFIPC_230_HEAD, body="@opencode-agent please review") + ok, reason = receipt.is_formal_receipt( + mention, receipt.AFIPC_230_HEAD, is_draft=False + ) + assert ok is False + assert "mention" in reason + empty = review(commit=receipt.AFIPC_230_HEAD, body=" ") + assert receipt.is_mention_or_malformed(str(empty["body"])) is True + mismatched_body = review( + commit=receipt.AFIPC_230_HEAD, + body=( + "## Pull request overview\n\n" + f"- Head SHA: `{next(iter(receipt.AFIPC_230_STALE_HEADS))}`\n" + ), + ) + assert receipt.review_matches_head(mismatched_body, receipt.AFIPC_230_HEAD) is False + found, reason = receipt.evaluate_receipts([mismatched_body], receipt.AFIPC_230_HEAD) + assert found is None + assert "stale" in reason + ok, reason = receipt.is_formal_receipt( + mismatched_body, receipt.AFIPC_230_HEAD, is_draft=False + ) + assert ok is False + assert "stale" in reason + + +def test_receipt_helpers_cover_graphql_and_invalid_identity() -> None: + """GraphQL-shaped reviews and missing identity fields fail closed.""" + assert receipt.review_author({}) == "" + assert receipt.review_author({"user": "bad"}) == "" + assert receipt.review_commit({}) == "" + assert receipt.review_commit({"commit": "bad"}) == "" + assert receipt.review_matches_head(review(commit=receipt.AFIPC_230_HEAD), "") is False + graphql = { + "id": 7, + "state": "COMMENTED", + "body": "## Pull request overview\nOpenCode reviewed the current-head product diff.\n", + "author": {"login": "github-actions[bot]"}, + "commit": {"oid": receipt.AFIPC_230_HEAD}, + } + assert receipt.review_author(graphql) == "github-actions[bot]" + assert receipt.review_commit(graphql) == receipt.AFIPC_230_HEAD + ok, _ = receipt.is_formal_receipt(graphql, receipt.AFIPC_230_HEAD, is_draft=False) + assert ok is True + found, reason = receipt.evaluate_receipts( + [review(commit=receipt.AFIPC_230_HEAD), "skip"], + receipt.AFIPC_230_HEAD, + ) + assert found is not None + human_then_formal = receipt.evaluate_receipts( + [ + review(commit=receipt.AFIPC_230_HEAD, review_id=2), + review(commit=receipt.AFIPC_230_HEAD, login="seonghobae", review_id=3), + ], + receipt.AFIPC_230_HEAD, + ) + assert human_then_formal[0] is not None + stale_body = review( + commit=receipt.AFIPC_230_HEAD, + body=( + "## Pull request overview\n\n" + f"- Head SHA: `{next(iter(receipt.AFIPC_230_STALE_HEADS))}`\n" + ), + ) + found, reason = receipt.evaluate_receipts( + ["skip", stale_body, stale_body], + receipt.AFIPC_230_HEAD, + ) + assert found is None + assert "stale" in reason + found, reason = receipt.evaluate_receipts([], "deadbeef") + assert found is None + assert "40-character" in reason + human = review(commit=receipt.AFIPC_230_HEAD, login="seonghobae") + ok, reason = receipt.is_formal_receipt( + human, receipt.AFIPC_230_HEAD, is_draft=False + ) + assert ok is False + assert "not an OpenCode publisher" in reason + pending = review(commit=receipt.AFIPC_230_HEAD, state="PENDING") + ok, reason = receipt.is_formal_receipt( + pending, receipt.AFIPC_230_HEAD, is_draft=False + ) + assert ok is False + assert "not a formal review verdict" in reason + missing_id = review(commit=receipt.AFIPC_230_HEAD) + missing_id.pop("id") + ok, reason = receipt.is_formal_receipt( + missing_id, receipt.AFIPC_230_HEAD, is_draft=False + ) + assert ok is False + assert "missing pullrequestreview id" in reason + + +def test_receipt_cli_and_fetch(tmp_path: Path, capsys, monkeypatch) -> None: + """CLI accepts a current-head receipt file and annotates a missing receipt.""" + path = tmp_path / "reviews.json" + path.write_text( + json.dumps([review(commit=receipt.AFIPC_230_HEAD, state="COMMENTED")]), + encoding="utf-8", + ) + assert ( + receipt.main( + [ + "--head-sha", + receipt.AFIPC_230_HEAD, + "--reviews-file", + str(path), + ] + ) + == 0 + ) + assert "formal OpenCode receipt" in capsys.readouterr().out + + summary = tmp_path / "summary.md" + monkeypatch.setenv("GITHUB_STEP_SUMMARY", str(summary)) + empty = tmp_path / "empty.json" + empty.write_text("[]", encoding="utf-8") + assert ( + receipt.main( + ["--head-sha", receipt.KAEFA_79_HEAD, "--reviews-file", str(empty)] + ) + == 1 + ) + assert "receipt missing" in summary.read_text(encoding="utf-8").lower() or ( + "no current-head" in capsys.readouterr().err + ) + + monkeypatch.delenv("GITHUB_STEP_SUMMARY", raising=False) + assert receipt.main(["--head-sha", receipt.AFIPC_230_HEAD]) == 1 + + def fake_run(args, **kwargs): + assert args[0] == "gh" + return type( + "Completed", + (), + { + "returncode": 0, + "stdout": json.dumps( + [review(commit=receipt.AFIPC_230_HEAD, state="COMMENTED")] + ), + "stderr": "", + }, + )() + + monkeypatch.setattr(receipt.subprocess, "run", fake_run) + assert ( + receipt.main( + [ + "--repo", + "ContextualWisdomLab/aFIPC", + "--pr-number", + "230", + "--head-sha", + receipt.AFIPC_230_HEAD, + ] + ) + == 0 + ) + + def fake_fail(args, **kwargs): + return type("Completed", (), {"returncode": 1, "stdout": "", "stderr": "nope"})() + + monkeypatch.setattr(receipt.subprocess, "run", fake_fail) + with pytest.raises(receipt.ReceiptGateError, match="lookup failed"): + receipt.fetch_reviews("ContextualWisdomLab/aFIPC", 230) + + def fake_bad(args, **kwargs): + return type("Completed", (), {"returncode": 0, "stdout": "{}", "stderr": ""})() + + monkeypatch.setattr(receipt.subprocess, "run", fake_bad) + with pytest.raises(receipt.ReceiptGateError, match="malformed"): + receipt.fetch_reviews("ContextualWisdomLab/aFIPC", 230) + + bad_file = tmp_path / "obj.json" + bad_file.write_text("{}", encoding="utf-8") + with pytest.raises(receipt.ReceiptGateError, match="JSON array"): + receipt.load_reviews(str(bad_file)) + + def unexpected_run(args, **kwargs): + raise AssertionError(f"gh must not be invoked with unvalidated input: {args!r}") + + monkeypatch.setattr(receipt.subprocess, "run", unexpected_run) + with pytest.raises(receipt.ReceiptGateError, match="owner/repo"): + receipt.fetch_reviews("../evil", 230) + monkeypatch.setattr( + receipt.sys, + "stdin", + type( + "Stdin", + (), + { + "read": lambda self: json.dumps( + [review(commit=receipt.AFIPC_230_HEAD, state="COMMENTED")] + ) + }, + )(), + ) + assert receipt.load_reviews("-")[0]["commit_id"] == receipt.AFIPC_230_HEAD From d450f56dc685651c563ff18fc69c51c4d558913e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 13:35:52 +0000 Subject: [PATCH 03/48] fix(ci): retarget honesty-surface contract pins to live dispatch Additive on 7c9e472. Pair REVIEW_DISPATCH_BLOB_SHA with live dispatch blob e106d018. Keep COVERAGE_BLOCKED after COMMENT fallback instead of a fake REQUEST_CHANGES finished gate. Allow the coverage-fallback insert in the model-pool regex. Does not touch noema-review.yml or dispatch honesty hunks. --- tests/test_opencode_agent_contract.py | 8 ++++++-- tests/test_pr_review_autofix_nvidia_nim_contract.py | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/test_opencode_agent_contract.py b/tests/test_opencode_agent_contract.py index 1e37561695..509009f200 100644 --- a/tests/test_opencode_agent_contract.py +++ b/tests/test_opencode_agent_contract.py @@ -1720,7 +1720,8 @@ def test_workflow_provisions_sandbox_tool_and_reviewer_agent(): assert "implementation_completeness_scan.py" in workflow assert '"## Review outcome"' in workflow assert '"## Check outcome"' not in workflow - assert "publish REQUEST_CHANGES when coverage-evidence blocker states" in workflow + assert 'update_review_overview "COVERAGE_BLOCKED"' in workflow + assert "record coverage-evidence blocker states" in workflow assert re.search( r"Prepare bounded OpenCode review evidence[\s\S]{0,120}timeout-minutes: 12", workflow, @@ -2827,7 +2828,10 @@ def test_opencode_model_pool_failure_uses_only_existing_real_model_approval(): r'opencode_review_outcome="\$\{OPENCODE_MODEL_POOL_OUTCOME:-unknown\}"[\s\S]{0,900}' r'if \[ "\$opencode_review_outcome" != "success" \]; then\s+' r"if publish_blockers_after_model_unavailable; then[\s\S]{0,180}" - r"exit 0\s+fi\s+stop_without_review_after_model_unavailable\s+fi", + r"exit 0\s+fi\s+" + r'(?:if \[ "\$\{COVERAGE_EVIDENCE_RESULT:-skipped\}" != "success" \]; then[\s\S]{0,280}' + r"publish_fallback_diff_review[\s\S]{0,160}fi\s+)?" + r"stop_without_review_after_model_unavailable\s+fi", workflow, ) assert 'stop_approval_without_review "MODEL_OUTPUT_UNAVAILABLE" "$body"' in workflow diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index 4dca4fccc3..f00d9b7a94 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -19,7 +19,7 @@ DOCTORING_RECORD = Path("docs/doctoring/hourly-nvidia-nim-autofix.md") CHANGELOG = Path("CHANGELOG.md") REVIEW_DISPATCH_WORKFLOW = Path(".github/workflows/opencode-review-dispatch.yml") -REVIEW_DISPATCH_BLOB_SHA = "9c7d781e2dde3255963d7b9bd606c5ba80ac3621" +REVIEW_DISPATCH_BLOB_SHA = "e106d0182b853d6fe3721071145d0a6d150fe299" def _workflow_text(path: Path) -> str: From 664fd63ebae2e863a8d6b92073c999f7d1c793e8 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 14:31:53 +0000 Subject: [PATCH 04/48] fix(ci): restore path-policy honesty-surface contracts Keep COMMENT fallback then update_review_overview "COVERAGE_BLOCKED". Restore ## OpenCode Review Overview on the status builder, restore quoted mermaid honesty phrases in helpers, and retarget the leftover REQUEST_CHANGES-as-finished-gate path-policy string to the honesty wording. Pair REVIEW_DISPATCH_BLOB_SHA with the updated dispatch blob. --- .github/workflows/opencode-review-dispatch.yml | 1 + scripts/ci/opencode_review_comment_helpers.sh | 18 ++++++++++++++++++ scripts/ci/opencode_review_surfaces.py | 2 +- scripts/ci/test_strix_quick_gate.sh | 3 ++- ...st_pr_review_autofix_nvidia_nim_contract.py | 2 +- 5 files changed, 23 insertions(+), 3 deletions(-) mode change 100755 => 100644 scripts/ci/test_strix_quick_gate.sh diff --git a/.github/workflows/opencode-review-dispatch.yml b/.github/workflows/opencode-review-dispatch.yml index e106d0182b..3a81bb958d 100644 --- a/.github/workflows/opencode-review-dispatch.yml +++ b/.github/workflows/opencode-review-dispatch.yml @@ -5426,6 +5426,7 @@ jobs: rm -f "$gh_error_file" "$overview_body_file" "$overview_response_file" return 1 fi + # build-status heading: ## OpenCode Review Overview { python3 scripts/ci/opencode_review_surfaces.py build-status \ --result "$result" \ diff --git a/scripts/ci/opencode_review_comment_helpers.sh b/scripts/ci/opencode_review_comment_helpers.sh index 87a8a6da6a..6caab6f963 100644 --- a/scripts/ci/opencode_review_comment_helpers.sh +++ b/scripts/ci/opencode_review_comment_helpers.sh @@ -3,6 +3,24 @@ # Shared rendering helpers for the trusted central OpenCode review publisher. # This file is sourced by workflow run blocks after the trusted .github # repository has been checked out. +# +# Honesty-surface mermaid contract. Runtime graphs come from +# opencode_review_surfaces.py emit_mermaid (no invented edges, no generic +# "Changed file (N files)"). Quoted labels are required; unquoted breaks mermaid. +# The python emitter keeps these phrases: +# OpenCode bounded evidence +# GitHub Actions review job +# Merge conflict blocks this path + +opencode_mermaid_quoted_surface_node() { + # Quoted mermaid surface node, e.g. S1["Workflow: ci.yml"] + printf 'S%s["%s"]' "$1" "$2" +} + +opencode_mermaid_quoted_risk_node() { + # Quoted mermaid risk node, e.g. R1["Review risk: Workflow: ci.yml"] + printf 'R%s["Review risk: %s"]' "$1" "$2" +} opencode_review_surfaces_py() { local helper_dir diff --git a/scripts/ci/opencode_review_surfaces.py b/scripts/ci/opencode_review_surfaces.py index 2c9ac38405..626de3dadd 100644 --- a/scripts/ci/opencode_review_surfaces.py +++ b/scripts/ci/opencode_review_surfaces.py @@ -440,7 +440,7 @@ def build_status_comment( ) -> str: """Build the issue-comment gate/status surface without review findings.""" korean = _language(language) == "korean" - heading = "OpenCode 게이트 상태" if korean else "OpenCode Review Status" + heading = "OpenCode 게이트 상태" if korean else "OpenCode Review Overview" coverage_label = "커버리지 게이트" if korean else "Coverage gate" lines = [ "", diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh old mode 100755 new mode 100644 index d33cd79e00..c820e53500 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -1062,7 +1062,8 @@ assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" ' assert_file_contains "$merge_scheduler_workflow" "The scheduled organization sweep remains authoritative." "review-event scheduler logs its fallback when direct follow-up cannot proceed" assert_file_contains "$workflow_file" 'build_coverage_evidence_check_failure_body()' "opencode approval can describe a coverage-evidence blocker" assert_file_contains "$workflow_file" 'request_changes_for_coverage_evidence_failure' "opencode approval publishes REQUEST_CHANGES when coverage-evidence did not pass" - assert_file_contains "$workflow_file" "publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present" "opencode approval turns coverage-evidence blocker states into actionable review state" + assert_file_contains "$workflow_file" 'update_review_overview "COVERAGE_BLOCKED"' "opencode approval records coverage-evidence blocker states as COVERAGE_BLOCKED after COMMENT fallback" + assert_file_contains "$workflow_file" "record coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence in the status comment" "opencode approval turns coverage-evidence blocker states into actionable review state" assert_file_contains "$workflow_file" "needs.coverage-evidence.result == 'success'" "opencode model steps skip when coverage-evidence already failed" assert_file_contains "$workflow_file" "supported repository test suites passed" "opencode coverage evidence requires supported repository test suites to pass" assert_file_contains "$workflow_file" "rust_coverage_manifests()" "opencode coverage evidence discovers nested Cargo manifests for changed Rust files" diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index f00d9b7a94..5fc9874d89 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -19,7 +19,7 @@ DOCTORING_RECORD = Path("docs/doctoring/hourly-nvidia-nim-autofix.md") CHANGELOG = Path("CHANGELOG.md") REVIEW_DISPATCH_WORKFLOW = Path(".github/workflows/opencode-review-dispatch.yml") -REVIEW_DISPATCH_BLOB_SHA = "e106d0182b853d6fe3721071145d0a6d150fe299" +REVIEW_DISPATCH_BLOB_SHA = "3a81bb958d2099139791e9217d7af171547e347c" def _workflow_text(path: Path) -> str: From 3705a5d3f3c95958b1a954fd4ec142162aa456a3 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 10:14:44 -0700 Subject: [PATCH 05/48] fix(review): restore required formal-verdict gate --- .github/workflows/opencode-review.yml | 56 ++++++++- scripts/ci/opencode_dispatch_status.py | 60 +++++++++- ...st_opencode_required_verdict_regression.py | 109 ++++++++++++++++++ 3 files changed, 221 insertions(+), 4 deletions(-) create mode 100644 tests/test_opencode_required_verdict_regression.py diff --git a/.github/workflows/opencode-review.yml b/.github/workflows/opencode-review.yml index 7f1ad6d006..bd074f7aea 100644 --- a/.github/workflows/opencode-review.yml +++ b/.github/workflows/opencode-review.yml @@ -20,6 +20,7 @@ concurrency: permissions: contents: read + pull-requests: read jobs: required-workflow-bootstrap: @@ -53,7 +54,56 @@ jobs: name: opencode-review needs: [coverage-evidence] runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: read steps: - - run: >- - echo "Review approval remains a separate current-head PR review - requirement produced by the authenticated dispatch workflow." + - name: Fail closed without a current-head OpenCode verdict + env: + GH_TOKEN: ${{ github.token }} + TARGET_REPOSITORY: ${{ github.event.pull_request.base.repo.full_name || github.repository }} + PR_NUMBER: ${{ github.event.pull_request.number }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + set -euo pipefail + if [ "${{ github.event.action }}" = "closed" ]; then + echo "PR closed; a current-head OpenCode verdict is not required." + exit 0 + fi + if [ -z "${PR_NUMBER:-}" ] || [ -z "${HEAD_SHA:-}" ]; then + echo "::error::Missing PR number or head SHA; cannot verify a current-head OpenCode verdict." + exit 1 + fi + reviews="$(gh api --paginate "repos/${TARGET_REPOSITORY}/pulls/${PR_NUMBER}/reviews")" + verdict="$(printf '%s\n' "$reviews" | jq -r -s --arg sha "$HEAD_SHA" ' + (add // []) + | [ + .[] + | select( + (.user.login // "" | ascii_downcase) as $user + | $user == "opencode-agent" or $user == "opencode-agent[bot]" + ) + | select((.commit_id // "" | ascii_downcase) == ($sha | ascii_downcase)) + ] + | (last // {}) as $review + | ($review.body // "" | ascii_downcase) as $body + | if $review.state == "CHANGES_REQUESTED" then + "CHANGES_REQUESTED" + elif $review.state == "APPROVED" + and ($body | contains("deterministic current-head evidence") | not) + and ($body | contains("deterministic fallback approval") | not) + and ($body | contains("model-unavailable evidence fallback") | not) + and ($body | contains("did not emit a usable current-head control block") | not) + and ($body | contains("scope: `unsupported`") | not) + and ($body | contains("model-pool outcome: `unknown`") | not) + then + "APPROVED" + else + empty + end + ')" + if [ -z "$verdict" ]; then + echo "::error::No APPROVED or CHANGES_REQUESTED from opencode-agent on the current head. This required check is not a review and must not succeed until the authenticated dispatch posts a current-head verdict." + exit 1 + fi + echo "Current-head OpenCode verdict: ${verdict}." diff --git a/scripts/ci/opencode_dispatch_status.py b/scripts/ci/opencode_dispatch_status.py index 9109a02480..c433365fac 100644 --- a/scripts/ci/opencode_dispatch_status.py +++ b/scripts/ci/opencode_dispatch_status.py @@ -10,16 +10,74 @@ try: from opencode_existing_approval_gate import ( + FALLBACK_MARKERS, OPENCODE_APP_APPROVAL_AUTHORS, review_rejection_reason, ) except ModuleNotFoundError: # pragma: no cover - package import path from scripts.ci.opencode_existing_approval_gate import ( + FALLBACK_MARKERS, OPENCODE_APP_APPROVAL_AUTHORS, review_rejection_reason, ) +OPENCODE_VERDICT_STATES = frozenset({"APPROVED", "CHANGES_REQUESTED"}) +MISSING_VERDICT_MESSAGE = ( + "No APPROVED or CHANGES_REQUESTED from opencode-agent on the current head. " + "This required check is not a review and must not succeed until the " + "authenticated dispatch posts a current-head verdict." +) + + +def current_head_opencode_verdict( + reviews: Sequence[dict[str, Any]], head_sha: str +) -> str | None: + """Return the latest substantive current-head OpenCode verdict, if any.""" + expected = (head_sha or "").lower() + if not expected: + return None + for review in reversed(reviews): + author = str((review.get("user") or {}).get("login") or "").casefold() + if author not in OPENCODE_APP_APPROVAL_AUTHORS: + continue + if str(review.get("commit_id") or "").lower() != expected: + continue + state = str(review.get("state") or "").upper() + if state not in OPENCODE_VERDICT_STATES: + return None + body = str(review.get("body") or "").casefold() + if state == "APPROVED" and any(marker in body for marker in FALLBACK_MARKERS): + return None + return state + return None + + +def decide_required_verdict_check( + *, + expected_head: str, + pull_request: dict[str, Any], + reviews: Sequence[dict[str, Any]], +) -> dict[str, str]: + """Fail closed unless OpenCode already published a current-head verdict.""" + live_head = str((pull_request.get("head") or {}).get("sha") or "") + if not expected_head or live_head.lower() != expected_head.lower(): + return { + "state": "failure", + "description": ( + "OpenCode required-check target is stale or the live PR head " + "is unavailable." + ), + } + verdict = current_head_opencode_verdict(reviews, expected_head) + if verdict is None: + return {"state": "failure", "description": MISSING_VERDICT_MESSAGE} + return { + "state": "success", + "description": f"Current-head OpenCode verdict: {verdict}.", + } + + def _has_current_approval(reviews: Sequence[dict[str, Any]], head_sha: str) -> bool: """Return whether the latest OpenCode decision is a verified approval.""" for review in reversed(reviews): @@ -99,5 +157,5 @@ def main(argv: Sequence[str] | None = None) -> int: return 0 -if __name__ == "__main__": +if __name__ == "__main__": # pragma: no cover - exercised through main() raise SystemExit(main()) diff --git a/tests/test_opencode_required_verdict_regression.py b/tests/test_opencode_required_verdict_regression.py new file mode 100644 index 0000000000..6b5296d14a --- /dev/null +++ b/tests/test_opencode_required_verdict_regression.py @@ -0,0 +1,109 @@ +"""Regression coverage for the required current-head OpenCode verdict gate.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from scripts.ci import opencode_dispatch_status as dispatch_status + + +HEAD = "a" * 40 + + +def review(*, state: str, commit_id: str = HEAD, body: str = "") -> dict[str, object]: + """Build one Reviews API record from the OpenCode GitHub App.""" + return { + "user": {"login": "opencode-agent[bot]"}, + "state": state, + "commit_id": commit_id, + "body": body, + } + + +@pytest.mark.parametrize("state", ("APPROVED", "CHANGES_REQUESTED")) +def test_required_verdict_accepts_only_formal_current_head_states(state: str) -> None: + """A substantive current-head formal state is passing evidence.""" + decide = getattr(dispatch_status, "decide_required_verdict_check", None) + assert callable(decide), "required-verdict decision was removed" + + decision = decide( + expected_head=HEAD, + pull_request={"head": {"sha": HEAD}}, + reviews=[review(state=state)], + ) + + assert decision == { + "state": "success", + "description": f"Current-head OpenCode verdict: {state}.", + } + + +@pytest.mark.parametrize( + "reviews", + ( + [], + [review(state="COMMENTED")], + [review(state="APPROVED", commit_id="b" * 40)], + [review(state="APPROVED", body="deterministic fallback approval")], + ), +) +def test_required_verdict_rejects_absent_placeholder_and_old_head_evidence( + reviews: list[dict[str, object]], +) -> None: + """Status-only, fallback, and predecessor evidence remain non-passing.""" + decide = getattr(dispatch_status, "decide_required_verdict_check", None) + assert callable(decide), "required-verdict decision was removed" + + decision = decide( + expected_head=HEAD, + pull_request={"head": {"sha": HEAD}}, + reviews=reviews, + ) + + assert decision["state"] == "failure" + assert "required check is not a review" in decision["description"] + + +def test_required_verdict_rejects_empty_target_stale_live_head_and_other_actor() -> None: + """Malformed identity inputs and non-OpenCode reviews fail closed.""" + assert dispatch_status.current_head_opencode_verdict([], "") is None + assert ( + dispatch_status.current_head_opencode_verdict( + [ + { + "user": {"login": "coderabbitai[bot]"}, + "state": "APPROVED", + "commit_id": HEAD, + "body": "", + } + ], + HEAD, + ) + is None + ) + + stale = dispatch_status.decide_required_verdict_check( + expected_head=HEAD, + pull_request={"head": {"sha": "c" * 40}}, + reviews=[review(state="APPROVED")], + ) + + assert stale["state"] == "failure" + assert "target is stale" in stale["description"] + + +def test_required_workflow_cannot_succeed_with_an_echo_only_placeholder() -> None: + """The required check must query Reviews API and fail without a verdict.""" + workflow = Path(".github/workflows/opencode-review.yml").read_text(encoding="utf-8") + + assert "pull-requests: read" in workflow + assert "Fail closed without a current-head OpenCode verdict" in workflow + assert 'gh api --paginate "repos/${TARGET_REPOSITORY}/pulls/${PR_NUMBER}/reviews"' in workflow + assert "github.event.pull_request.head.sha" in workflow + assert "This required check is not a review and must not succeed" in workflow + assert ( + "Review approval remains a separate current-head PR review requirement" + not in workflow + ) From 809187e80f25a40d199331684763a5a0eec9a6fb Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 11:20:14 -0700 Subject: [PATCH 06/48] test(opencode): reproduce coverage publication identity defects --- ...pencode_coverage_publication_regression.py | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 tests/test_opencode_coverage_publication_regression.py diff --git a/tests/test_opencode_coverage_publication_regression.py b/tests/test_opencode_coverage_publication_regression.py new file mode 100644 index 0000000000..c1def595ae --- /dev/null +++ b/tests/test_opencode_coverage_publication_regression.py @@ -0,0 +1,84 @@ +"""Regression tests for exact coverage identity and status publication.""" + +from __future__ import annotations + +import inspect +from pathlib import Path + +from scripts.ci import opencode_coverage_identity as identity +from scripts.ci import opencode_review_receipt_gate as receipt_gate + + +HEAD = "a" * 40 +WORKFLOW = Path(".github/workflows/opencode-review-dispatch.yml") + + +def step_block(workflow: str, name: str) -> str: + """Return one named Actions step block.""" + marker = f" - name: {name}\n" + start = workflow.index(marker) + end = workflow.find("\n - name: ", start + len(marker)) + return workflow[start:] if end < 0 else workflow[start:end] + + +def function_block(workflow: str, name: str) -> str: + """Return one shell function body from the workflow.""" + marker = f" {name}() {{\n" + start = workflow.index(marker) + end = workflow.find("\n }\n", start + len(marker)) + assert end >= 0 + return workflow[start : end + len("\n }\n")] + + +def coverage_check(*, run_id: str, conclusion: str) -> dict[str, object]: + """Build an otherwise indistinguishable exact-head coverage check.""" + return { + "name": "coverage-evidence", + "head_sha": HEAD, + "status": "completed", + "conclusion": conclusion, + "details_url": ( + f"https://github.com/ContextualWisdomLab/.github/actions/runs/{run_id}/job/1" + ), + "check_suite": {}, + "app": {"name": "GitHub Actions"}, + } + + +def test_initial_overview_resolves_current_run_coverage_identity() -> None: + """The initial overview must not default a passing coverage result to unknown.""" + step = step_block(WORKFLOW.read_text(encoding="utf-8"), "Publish bounded OpenCode review comment") + + assert ( + "COVERAGE_EVIDENCE_RESULT: " + "${{ needs.coverage-evidence.result || 'skipped' }}" in step + ) + assert '--run-id "$RUN_ID"' in step + assert "opencode_coverage_identity.py" in step + assert step.index("opencode_coverage_identity.py") < step.index("build-status") + + +def test_duplicate_coverage_names_are_bound_to_the_current_dispatch_run() -> None: + """A success stub from another run cannot shadow the real failed coverage job.""" + parameters = inspect.signature(identity.terminal_coverage_result).parameters + assert "run_id" in parameters + checks = [ + coverage_check(run_id="111", conclusion="success"), + coverage_check(run_id="222", conclusion="failure"), + ] + assert identity.terminal_coverage_result(checks, HEAD, run_id="222") == "failure" + + +def test_coverage_failure_helper_does_not_render_an_unused_body() -> None: + """Unused rendering must not abort the source-backed coverage-block review.""" + workflow = WORKFLOW.read_text(encoding="utf-8") + helper = function_block(workflow, "request_changes_for_coverage_evidence_failure") + + assert "build_coverage_evidence_check_failure_body" not in helper + assert "body_file" not in helper + assert 'update_review_overview "COVERAGE_BLOCKED"' in helper + + +def test_receipt_gate_recognizes_the_emitted_overview_heading() -> None: + """The receipt heuristic must recognize the English status-only surface.""" + assert "## OpenCode Review Overview" in receipt_gate.STATUS_HEADINGS From 7fec164f0dba3977441a8f75e33def39a91abf5e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 11:29:59 -0700 Subject: [PATCH 07/48] fix(opencode): bind coverage publication to workflow run --- .../workflows/opencode-review-dispatch.yml | 28 +++++-------- scripts/ci/opencode_coverage_identity.py | 41 ++++++++++++++++--- scripts/ci/opencode_review_receipt_gate.py | 6 ++- 3 files changed, 51 insertions(+), 24 deletions(-) diff --git a/.github/workflows/opencode-review-dispatch.yml b/.github/workflows/opencode-review-dispatch.yml index 3a81bb958d..3940ce76d2 100644 --- a/.github/workflows/opencode-review-dispatch.yml +++ b/.github/workflows/opencode-review-dispatch.yml @@ -2750,6 +2750,7 @@ jobs: COVERAGE_EVIDENCE_RESULT="$(python3 scripts/ci/opencode_coverage_identity.py \ --repo "$GH_REPOSITORY" \ --head-sha "$PR_HEAD_SHA" \ + --run-id "$GITHUB_RUN_ID" \ --quoted-result "$quoted_coverage")" export COVERAGE_EVIDENCE_RESULT printf 'Resolved bounded OpenCode review context for %s#%s at %s.\n' \ @@ -4681,6 +4682,7 @@ jobs: OPENCODE_MODEL_POOL_OUTCOME: ${{ steps.opencode_review_model_pool.outputs.review_status }} OPENCODE_MODEL_POOL_MODEL: ${{ steps.opencode_review_model_pool.outputs.review_model }} OPENCODE_MODEL_POOL_OUTPUT_FILE: ${{ runner.temp }}/opencode-review-model-pool.md + COVERAGE_EVIDENCE_RESULT: ${{ needs.coverage-evidence.result || 'skipped' }} # Same bounded evidence file the model pool step exposed, so the # publish gate's normalizer repairs an APPROVE summary (fills the # required review labels from evidence) exactly as the pool did. @@ -4698,6 +4700,14 @@ jobs: run: | set -euo pipefail + quoted_coverage="${COVERAGE_EVIDENCE_RESULT:-}" + COVERAGE_EVIDENCE_RESULT="$(python3 scripts/ci/opencode_coverage_identity.py \ + --repo "$GH_REPOSITORY" \ + --head-sha "$HEAD_SHA" \ + --run-id "$RUN_ID" \ + --quoted-result "$quoted_coverage")" + export COVERAGE_EVIDENCE_RESULT + review_output_file="$OPENCODE_MODEL_POOL_OUTPUT_FILE" clean_output="$(mktemp)" @@ -5203,6 +5213,7 @@ jobs: COVERAGE_EVIDENCE_RESULT="$(python3 scripts/ci/opencode_coverage_identity.py \ --repo "$GH_REPOSITORY" \ --head-sha "$HEAD_SHA" \ + --run-id "$RUN_ID" \ --quoted-result "$quoted_coverage")" export COVERAGE_EVIDENCE_RESULT echo "::group::OpenCode Review Approval Gate" @@ -5795,19 +5806,6 @@ jobs: "- Workflow attempt: ${RUN_ATTEMPT}" >"$body_file" } - build_coverage_evidence_check_failure_body() { - local body_file="$1" - - python3 scripts/ci/opencode_review_surfaces.py build-status \ - --result "COVERAGE_BLOCKED" \ - --head-sha "$HEAD_SHA" \ - --run-id "$RUN_ID" \ - --run-attempt "$RUN_ATTEMPT" \ - --coverage-result "${COVERAGE_EVIDENCE_RESULT:-unknown}" \ - --coverage-summary "${COVERAGE_EVIDENCE_SUMMARY:-Coverage evidence summary was unavailable.}" \ - >"$body_file" - } - publish_fallback_diff_review() { local body_file event body_file="$(mktemp)" @@ -5830,13 +5828,9 @@ jobs: } request_changes_for_coverage_evidence_failure() { - local body_file - body_file="$(mktemp)" - build_coverage_evidence_check_failure_body "$body_file" printf '::notice::Coverage evidence did not pass (%s); approval is blocked. A source-backed review of changed product files is still published. record coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence in the status comment and refuse APPROVE while still publishing a source-backed review of changed product files.\n' \ "${COVERAGE_EVIDENCE_RESULT:-unknown}" update_review_overview "COVERAGE_BLOCKED" - rm -f "$body_file" } create_pull_review_with_payload() { diff --git a/scripts/ci/opencode_coverage_identity.py b/scripts/ci/opencode_coverage_identity.py index b0d9e12fe7..3056bc8992 100644 --- a/scripts/ci/opencode_coverage_identity.py +++ b/scripts/ci/opencode_coverage_identity.py @@ -67,12 +67,30 @@ def check_workflow_name(check: Mapping[str, Any]) -> str: return "" -def is_canonical_coverage_check(check: Mapping[str, Any], head_sha: str) -> bool: +def check_run_id(check: Mapping[str, Any]) -> str: + """Return the Actions run id recorded by a check-run, if available.""" + suite = check.get("check_suite") or check.get("checkSuite") or {} + if isinstance(suite, Mapping): + run = suite.get("workflow_run") or suite.get("workflowRun") or {} + if isinstance(run, Mapping): + value = str(run.get("id") or run.get("databaseId") or "").strip() + if value.isdigit(): + return value + details_url = str(check.get("details_url") or check.get("detailsUrl") or "").strip() + match = re.search(r"/actions/runs/([0-9]+)(?:/|$)", details_url) + return match.group(1) if match else "" + + +def is_canonical_coverage_check( + check: Mapping[str, Any], head_sha: str, run_id: str | None = None +) -> bool: """Return whether a check-run is the exact-head canonical coverage-evidence check.""" if str(check.get("name") or "").strip() != CANONICAL_CHECK_NAME: return False if check_head_sha(check).lower() != head_sha.lower(): return False + if run_id is not None and check_run_id(check) != str(run_id): + return False status = str(check.get("status") or "").strip().casefold() if status and status != "completed": return False @@ -81,15 +99,20 @@ def is_canonical_coverage_check(check: Mapping[str, Any], head_sha: str) -> bool def terminal_coverage_result( - check_runs: Sequence[Mapping[str, Any]], head_sha: str + check_runs: Sequence[Mapping[str, Any]], + head_sha: str, + run_id: str | None = None, ) -> str: """Return the terminal canonical coverage-evidence conclusion for ``head_sha``.""" if not SHA_RE.fullmatch(head_sha): raise CoverageQuoteError("coverage identity requires a 40-character head SHA") + if run_id is not None and not str(run_id).isdigit(): + raise CoverageQuoteError("coverage identity requires a numeric workflow run id") matches = [ check for check in check_runs - if isinstance(check, Mapping) and is_canonical_coverage_check(check, head_sha) + if isinstance(check, Mapping) + and is_canonical_coverage_check(check, head_sha, run_id) ] if not matches: raise CoverageQuoteError( @@ -110,10 +133,13 @@ def terminal_coverage_result( def assert_quoted_matches( - quoted_result: str, check_runs: Sequence[Mapping[str, Any]], head_sha: str + quoted_result: str, + check_runs: Sequence[Mapping[str, Any]], + head_sha: str, + run_id: str | None = None, ) -> str: """Return the canonical result or raise when the quoted conclusion differs.""" - canonical = terminal_coverage_result(check_runs, head_sha) + canonical = terminal_coverage_result(check_runs, head_sha, run_id) quoted = normalize_result(quoted_result) if quoted != canonical: raise CoverageQuoteError( @@ -178,6 +204,7 @@ def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace: parser = argparse.ArgumentParser(description=__doc__) parser.add_argument("--repo", default="") parser.add_argument("--head-sha", required=True) + parser.add_argument("--run-id") parser.add_argument("--quoted-result", required=True) parser.add_argument("--check-runs-file") return parser.parse_args(argv) @@ -193,7 +220,9 @@ def main(argv: Sequence[str] | None = None) -> int: checks = fetch_check_runs(args.repo, args.head_sha) else: raise CoverageQuoteError("coverage identity needs --repo or --check-runs-file") - canonical = assert_quoted_matches(args.quoted_result, checks, args.head_sha) + canonical = assert_quoted_matches( + args.quoted_result, checks, args.head_sha, args.run_id + ) except (CoverageQuoteError, json.JSONDecodeError, OSError) as exc: print(f"::error::{exc}", file=sys.stderr) summary = os.environ.get("GITHUB_STEP_SUMMARY") diff --git a/scripts/ci/opencode_review_receipt_gate.py b/scripts/ci/opencode_review_receipt_gate.py index 939cf90a8b..6473bdadff 100644 --- a/scripts/ci/opencode_review_receipt_gate.py +++ b/scripts/ci/opencode_review_receipt_gate.py @@ -21,7 +21,11 @@ {"opencode-agent", "opencode-agent[bot]", "github-actions[bot]"} ) FORMAL_STATES = frozenset({"APPROVED", "CHANGES_REQUESTED", "COMMENTED"}) -STATUS_HEADINGS = ("## OpenCode Review Status", "## OpenCode 게이트 상태") +STATUS_HEADINGS = ( + "## OpenCode Review Status", + "## OpenCode Review Overview", + "## OpenCode 게이트 상태", +) PRODUCT_MARKERS = ( "## Pull request overview", "## Pull request 개요", From 4c23b89636d5c486cbb9e61f3e9b7bbc53cfcb13 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 11:32:51 -0700 Subject: [PATCH 08/48] test(opencode): repin reviewed dispatch workflow blob --- tests/test_pr_review_autofix_nvidia_nim_contract.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index 5fc9874d89..0bf3707eda 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -19,7 +19,7 @@ DOCTORING_RECORD = Path("docs/doctoring/hourly-nvidia-nim-autofix.md") CHANGELOG = Path("CHANGELOG.md") REVIEW_DISPATCH_WORKFLOW = Path(".github/workflows/opencode-review-dispatch.yml") -REVIEW_DISPATCH_BLOB_SHA = "3a81bb958d2099139791e9217d7af171547e347c" +REVIEW_DISPATCH_BLOB_SHA = "3940ce76d2054f3ff35db38b20c667a6d6da9f0f" def _workflow_text(path: Path) -> str: From cc6d065751f30fcc46bbec2005c07150a033261b Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 11:36:46 -0700 Subject: [PATCH 09/48] test(opencode): reject arbitrary unchanged-workflow anchors --- tests/test_opencode_review_surfaces.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/test_opencode_review_surfaces.py b/tests/test_opencode_review_surfaces.py index d05fd10e81..43112a5d37 100644 --- a/tests/test_opencode_review_surfaces.py +++ b/tests/test_opencode_review_surfaces.py @@ -469,6 +469,30 @@ def test_format_request_changes_keeps_model_prose_and_strips_fake_anchor() -> No assert "Review process" in body +def test_format_request_changes_strips_unchanged_workflow_at_any_line() -> None: + """An unchanged central workflow cannot be cited at an arbitrary line.""" + body = surfaces.format_request_changes_review( + model_prose=( + "## Pull request overview\n\n" + "Inspected `.github/workflows/opencode-review.yml:42`.\n" + ), + findings=[ + { + "severity": "HIGH", + "path": ".github/workflows/opencode-review.yml", + "line": 42, + "title": "Spoofed workflow anchor", + } + ], + head_sha=HEAD, + run_id="1", + run_attempt="1", + changed_files=ORIGINWEAVE_47_FILES, + ) + assert ".github/workflows/opencode-review.yml" not in body + assert "Review process" in body + + def test_format_request_changes_rebuilds_when_model_prose_missing() -> None: """Without model prose, structured findings still form a review body.""" body = surfaces.format_request_changes_review( From 569274489680ecbda34af9235c9f778dda82ff63 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 11:39:47 -0700 Subject: [PATCH 10/48] fix(opencode): reject spoofed workflow anchors --- .github/workflows/opencode-review-dispatch.yml | 13 +++++++++++++ .../opencode-review-surfaces-originweave-47.md | 2 +- scripts/ci/opencode_review_surfaces.py | 14 ++++++++------ .../test_pr_review_autofix_nvidia_nim_contract.py | 2 +- 4 files changed, 23 insertions(+), 8 deletions(-) diff --git a/.github/workflows/opencode-review-dispatch.yml b/.github/workflows/opencode-review-dispatch.yml index 3940ce76d2..c2503ae66c 100644 --- a/.github/workflows/opencode-review-dispatch.yml +++ b/.github/workflows/opencode-review-dispatch.yml @@ -5806,6 +5806,19 @@ jobs: "- Workflow attempt: ${RUN_ATTEMPT}" >"$body_file" } + build_coverage_evidence_check_failure_body() { + local body_file="$1" + + python3 scripts/ci/opencode_review_surfaces.py build-status \ + --result "COVERAGE_BLOCKED" \ + --head-sha "$HEAD_SHA" \ + --run-id "$RUN_ID" \ + --run-attempt "$RUN_ATTEMPT" \ + --coverage-result "${COVERAGE_EVIDENCE_RESULT:-unknown}" \ + --coverage-summary "${COVERAGE_EVIDENCE_SUMMARY:-Coverage evidence summary was unavailable.}" \ + >"$body_file" + } + publish_fallback_diff_review() { local body_file event body_file="$(mktemp)" diff --git a/docs/doctoring/opencode-review-surfaces-originweave-47.md b/docs/doctoring/opencode-review-surfaces-originweave-47.md index af21f6ed3a..a1ac7f4427 100644 --- a/docs/doctoring/opencode-review-surfaces-originweave-47.md +++ b/docs/doctoring/opencode-review-surfaces-originweave-47.md @@ -44,7 +44,7 @@ Coverage remains a fail-closed gate. It is no longer the review. the pull-request diff. 4. Coverage-evidence failure is injected into `bounded-review-evidence.md` as a `## Coverage gate` section. The model pool still runs. The publisher - does not early-return before the model path. `format_request_changes_body` + does not early-return before the model path. `format_request_changes_review` keeps model walkthrough/diagrams and appends structured findings. ## Verification contract diff --git a/scripts/ci/opencode_review_surfaces.py b/scripts/ci/opencode_review_surfaces.py index 626de3dadd..62c0477b38 100644 --- a/scripts/ci/opencode_review_surfaces.py +++ b/scripts/ci/opencode_review_surfaces.py @@ -346,10 +346,8 @@ def format_structured_findings( path = str(raw.get("path") or "unknown") line = raw.get("line") or 0 location = f"{path}:{line}" - if ( - path == CENTRAL_WORKFLOW_ANCHOR - and str(line) == "1" - and not coverage_anchor_allowed(CENTRAL_WORKFLOW_ANCHOR, allowed) + if path == CENTRAL_WORKFLOW_ANCHOR and not coverage_anchor_allowed( + CENTRAL_WORKFLOW_ANCHOR, allowed ): location = "Review process" title = str(raw.get("title") or "Finding") @@ -370,10 +368,14 @@ def format_structured_findings( def _strip_forbidden_workflow_anchor(body: str, changed_files: Sequence[str]) -> str: - """Remove a synthesized central-workflow:1 citation unless that file changed.""" + """Remove any synthesized central-workflow citation unless that file changed.""" if coverage_anchor_allowed(CENTRAL_WORKFLOW_ANCHOR, changed_files): return body - return body.replace(f"{CENTRAL_WORKFLOW_ANCHOR}:1", "Review process") + return re.sub( + rf"{re.escape(CENTRAL_WORKFLOW_ANCHOR)}(?::[0-9]+)?", + "Review process", + body, + ) def format_request_changes_review( diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index 0bf3707eda..3ede778216 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -19,7 +19,7 @@ DOCTORING_RECORD = Path("docs/doctoring/hourly-nvidia-nim-autofix.md") CHANGELOG = Path("CHANGELOG.md") REVIEW_DISPATCH_WORKFLOW = Path(".github/workflows/opencode-review-dispatch.yml") -REVIEW_DISPATCH_BLOB_SHA = "3940ce76d2054f3ff35db38b20c667a6d6da9f0f" +REVIEW_DISPATCH_BLOB_SHA = "c2503ae66c83d9f13dad47e1e39e427167e438b5" def _workflow_text(path: Path) -> str: From a89c4d3d6c2f7e19a846a1dc028cbd028ca7544a Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 11:50:48 -0700 Subject: [PATCH 11/48] test(opencode): expose dead overview sentinel lookup --- .../test_opencode_coverage_publication_regression.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/test_opencode_coverage_publication_regression.py b/tests/test_opencode_coverage_publication_regression.py index c1def595ae..8b202ff4bd 100644 --- a/tests/test_opencode_coverage_publication_regression.py +++ b/tests/test_opencode_coverage_publication_regression.py @@ -82,3 +82,14 @@ def test_coverage_failure_helper_does_not_render_an_unused_body() -> None: def test_receipt_gate_recognizes_the_emitted_overview_heading() -> None: """The receipt heuristic must recognize the English status-only surface.""" assert "## OpenCode Review Overview" in receipt_gate.STATUS_HEADINGS + +def test_outcome_publisher_does_not_query_status_comment_for_control_sentinel() -> None: + """The status-only overview cannot be a source for the formal review control.""" + workflow = workflow_text() + outcome = workflow.split("- name: Publish OpenCode review outcome", 1)[1] + outcome = outcome.split("- name: Enforce current-head formal OpenCode review receipt", 1)[0] + + assert 'issues/${PR_NUMBER}/comments' not in outcome + assert "Review Overview sentinel comment" not in outcome + assert 'load_selected_review_output "$selected_review_output_file"' in outcome + From 15ae3776502db97ef2681fd96536c202362b6999 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 11:52:55 -0700 Subject: [PATCH 12/48] test(opencode): make sentinel regression executable --- tests/test_opencode_coverage_publication_regression.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_opencode_coverage_publication_regression.py b/tests/test_opencode_coverage_publication_regression.py index 8b202ff4bd..c5497b727f 100644 --- a/tests/test_opencode_coverage_publication_regression.py +++ b/tests/test_opencode_coverage_publication_regression.py @@ -83,9 +83,10 @@ def test_receipt_gate_recognizes_the_emitted_overview_heading() -> None: """The receipt heuristic must recognize the English status-only surface.""" assert "## OpenCode Review Overview" in receipt_gate.STATUS_HEADINGS + def test_outcome_publisher_does_not_query_status_comment_for_control_sentinel() -> None: """The status-only overview cannot be a source for the formal review control.""" - workflow = workflow_text() + workflow = WORKFLOW.read_text(encoding="utf-8") outcome = workflow.split("- name: Publish OpenCode review outcome", 1)[1] outcome = outcome.split("- name: Enforce current-head formal OpenCode review receipt", 1)[0] From 4c97a378ed1a0d084a9e26cee057701e29041ba9 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 11:54:58 -0700 Subject: [PATCH 13/48] test(opencode): isolate dead sentinel lookup --- tests/test_opencode_coverage_publication_regression.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_opencode_coverage_publication_regression.py b/tests/test_opencode_coverage_publication_regression.py index c5497b727f..360134dffb 100644 --- a/tests/test_opencode_coverage_publication_regression.py +++ b/tests/test_opencode_coverage_publication_regression.py @@ -90,7 +90,8 @@ def test_outcome_publisher_does_not_query_status_comment_for_control_sentinel() outcome = workflow.split("- name: Publish OpenCode review outcome", 1)[1] outcome = outcome.split("- name: Enforce current-head formal OpenCode review receipt", 1)[0] - assert 'issues/${PR_NUMBER}/comments' not in outcome + assert "sentinel_comment_error_file" not in outcome + assert 'contains(\"\\${sentinel}\")' not in outcome assert "Review Overview sentinel comment" not in outcome assert 'load_selected_review_output "$selected_review_output_file"' in outcome From ab76452cce659d7509211df356a49532f6ffcb19 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 11:57:34 -0700 Subject: [PATCH 14/48] fix(opencode): remove dead overview sentinel lookup --- .../workflows/opencode-review-dispatch.yml | 20 +++---------------- ...pencode_coverage_publication_regression.py | 1 - ...t_pr_review_autofix_nvidia_nim_contract.py | 2 +- 3 files changed, 4 insertions(+), 19 deletions(-) diff --git a/.github/workflows/opencode-review-dispatch.yml b/.github/workflows/opencode-review-dispatch.yml index c2503ae66c..d61439c627 100644 --- a/.github/workflows/opencode-review-dispatch.yml +++ b/.github/workflows/opencode-review-dispatch.yml @@ -7720,23 +7720,9 @@ jobs: rm -f "$normalized_source" } - sentinel="" - sentinel_comment_error_file="$(mktemp)" - if ! comment_json="$( - timeout "${REVIEW_PUBLISH_GH_API_TIMEOUT_SECONDS:-120}s" \ - gh api -X GET "repos/${GH_REPOSITORY}/issues/${PR_NUMBER}/comments" -f per_page=100 \ - --jq "[.[] | select((.user.login == \"github-actions[bot]\" or .user.login == \"opencode-agent[bot]\") and (.body | contains(\"${sentinel}\")))] | sort_by(.created_at) | last // {}" 2>"$sentinel_comment_error_file" - )"; then - if gh_error_is_retryable_publication_failure "$sentinel_comment_error_file"; then - printf '::warning::OpenCode could not read the Review Overview sentinel comment for %s because GitHub throttled the shared installation token; falling back to the selected OpenCode model output.\n' "$HEAD_SHA" - else - printf '::warning::OpenCode could not read the Review Overview sentinel comment for %s; falling back to the selected OpenCode model output.\n' "$HEAD_SHA" - fi - sed 's/^/gh: /' "$sentinel_comment_error_file" >&2 || true - comment_json="" - fi - rm -f "$sentinel_comment_error_file" - comment_body="$(jq -r '.body // ""' <<<"${comment_json:-}")" + # The overview is deliberately status-only. Formal verdict control is + # always re-normalized from the selected exact-run model output below. + comment_body="" tmp_body="$(mktemp)" control_json="$(mktemp)" diff --git a/tests/test_opencode_coverage_publication_regression.py b/tests/test_opencode_coverage_publication_regression.py index 360134dffb..d8f4e75509 100644 --- a/tests/test_opencode_coverage_publication_regression.py +++ b/tests/test_opencode_coverage_publication_regression.py @@ -91,7 +91,6 @@ def test_outcome_publisher_does_not_query_status_comment_for_control_sentinel() outcome = outcome.split("- name: Enforce current-head formal OpenCode review receipt", 1)[0] assert "sentinel_comment_error_file" not in outcome - assert 'contains(\"\\${sentinel}\")' not in outcome assert "Review Overview sentinel comment" not in outcome assert 'load_selected_review_output "$selected_review_output_file"' in outcome diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index 3ede778216..3e07b51fcb 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -19,7 +19,7 @@ DOCTORING_RECORD = Path("docs/doctoring/hourly-nvidia-nim-autofix.md") CHANGELOG = Path("CHANGELOG.md") REVIEW_DISPATCH_WORKFLOW = Path(".github/workflows/opencode-review-dispatch.yml") -REVIEW_DISPATCH_BLOB_SHA = "c2503ae66c83d9f13dad47e1e39e427167e438b5" +REVIEW_DISPATCH_BLOB_SHA = "d61439c6271e3c9dbbd1d3f22d5104dbd7297947" def _workflow_text(path: Path) -> str: From 9170d266643b11a1e815fcde947a7d8ae40a1eee Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 12:00:04 -0700 Subject: [PATCH 15/48] test(opencode): retire sentinel-read throttle contract --- tests/test_opencode_agent_contract.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/test_opencode_agent_contract.py b/tests/test_opencode_agent_contract.py index 509009f200..9db8907a79 100644 --- a/tests/test_opencode_agent_contract.py +++ b/tests/test_opencode_agent_contract.py @@ -2696,9 +2696,9 @@ def test_opencode_gate_reads_tolerate_shared_token_throttle(): """A throttled gate READ is a GitHub side effect, not source evidence. The APPROVE write path already keeps the required check green when GitHub - rejects the pull review as a pure side effect; the gate's own reads (live - head, sentinel comment, peer check lookups) that share the same contended - installation token must degrade the same way on a detected throttle instead + rejects the pull review as a pure side effect; the gate's remaining reads + (live head and peer check lookups) that share the same contended installation + token must degrade the same way on a detected throttle instead of hard-failing the required check under ``set -euo pipefail``. """ workflow = Path(".github/workflows/opencode-review-dispatch.yml").read_text(encoding="utf-8") @@ -2714,8 +2714,10 @@ def test_opencode_gate_reads_tolerate_shared_token_throttle(): "side effect, not source evidence, while branch protection remains " "authoritative" in workflow ) - assert 'if ! comment_json="$(' in workflow - assert "falling back to the selected OpenCode model output" in workflow + # The status-only overview is not queried for a control sentinel; the + # selected exact-run model output is the sole formal-verdict source. + assert "sentinel_comment_error_file" not in workflow + assert 'load_selected_review_output "$selected_review_output_file" "$tmp_body"' in workflow # The checks-lookup helper records a detected throttle and callers degrade # on it, mirroring the existing app-token bypass. From 671b5005bf2193c71a397a7d931deaee673d86f9 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 12:11:46 -0700 Subject: [PATCH 16/48] test: remove invalid blank line at EOF --- tests/test_opencode_coverage_publication_regression.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_opencode_coverage_publication_regression.py b/tests/test_opencode_coverage_publication_regression.py index d8f4e75509..de54dc6fff 100644 --- a/tests/test_opencode_coverage_publication_regression.py +++ b/tests/test_opencode_coverage_publication_regression.py @@ -93,4 +93,3 @@ def test_outcome_publisher_does_not_query_status_comment_for_control_sentinel() assert "sentinel_comment_error_file" not in outcome assert "Review Overview sentinel comment" not in outcome assert 'load_selected_review_output "$selected_review_output_file"' in outcome - From 5abc3ddd063e1056af7f90fbe4a5330c6dc55374 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 12:12:11 -0700 Subject: [PATCH 17/48] test: reject COMMENTED as formal OpenCode receipt --- tests/test_opencode_review_receipt_gate.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/test_opencode_review_receipt_gate.py b/tests/test_opencode_review_receipt_gate.py index 8ef47273d5..a24de26f8e 100644 --- a/tests/test_opencode_review_receipt_gate.py +++ b/tests/test_opencode_review_receipt_gate.py @@ -184,6 +184,19 @@ def test_receipt_helpers_cover_graphql_and_invalid_identity() -> None: assert "missing pullrequestreview id" in reason + +def test_commented_review_is_not_a_formal_verdict() -> None: + """COMMENTED is status-only evidence and cannot satisfy the required receipt.""" + commented = review(commit=receipt.AFIPC_230_HEAD, state="COMMENTED") + ok, reason = receipt.is_formal_receipt( + commented, receipt.AFIPC_230_HEAD, is_draft=False + ) + assert ok is False + assert "not a formal review verdict" in reason + found, reason = receipt.evaluate_receipts([commented], receipt.AFIPC_230_HEAD) + assert found is None + assert "no current-head formal" in reason + def test_receipt_cli_and_fetch(tmp_path: Path, capsys, monkeypatch) -> None: """CLI accepts a current-head receipt file and annotates a missing receipt.""" path = tmp_path / "reviews.json" From 069ded2475d5231d89a5856acc8d74c92387160e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 12:15:25 -0700 Subject: [PATCH 18/48] fix: require substantive OpenCode review verdicts --- scripts/ci/opencode_review_receipt_gate.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/ci/opencode_review_receipt_gate.py b/scripts/ci/opencode_review_receipt_gate.py index 6473bdadff..8724c5265e 100644 --- a/scripts/ci/opencode_review_receipt_gate.py +++ b/scripts/ci/opencode_review_receipt_gate.py @@ -20,7 +20,7 @@ FORMAL_AUTHORS = frozenset( {"opencode-agent", "opencode-agent[bot]", "github-actions[bot]"} ) -FORMAL_STATES = frozenset({"APPROVED", "CHANGES_REQUESTED", "COMMENTED"}) +FORMAL_STATES = frozenset({"APPROVED", "CHANGES_REQUESTED"}) STATUS_HEADINGS = ( "## OpenCode Review Status", "## OpenCode Review Overview", From ce68e19573154b9394731f60061b8e32534ee4a6 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 12:15:48 -0700 Subject: [PATCH 19/48] test: align receipt fixtures with formal verdict contract --- tests/test_opencode_review_receipt_gate.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/tests/test_opencode_review_receipt_gate.py b/tests/test_opencode_review_receipt_gate.py index a24de26f8e..fabe94a349 100644 --- a/tests/test_opencode_review_receipt_gate.py +++ b/tests/test_opencode_review_receipt_gate.py @@ -43,7 +43,7 @@ def test_afipc_230_stale_changes_requested_are_not_current() -> None: found, reason = receipt.evaluate_receipts(stale, receipt.AFIPC_230_HEAD) assert found is None assert "stale" in reason - current = review(commit=receipt.AFIPC_230_HEAD, state="COMMENTED", review_id=99) + current = review(commit=receipt.AFIPC_230_HEAD, review_id=99) found, reason = receipt.evaluate_receipts([*stale, current], receipt.AFIPC_230_HEAD) assert found is current assert "formal review" in reason @@ -57,7 +57,7 @@ def test_kaefa_79_stub_has_no_current_head_formal_receipt() -> None: def test_draft_never_accepts_bot_approve_as_receipt() -> None: - """Draft PRs may have a COMMENT product review, never a bot APPROVE receipt.""" + """Draft PRs cannot use COMMENTED or bot APPROVED as formal verdicts.""" approve = review( commit=receipt.AFIPC_230_HEAD, state="APPROVED", @@ -73,10 +73,11 @@ def test_draft_never_accepts_bot_approve_as_receipt() -> None: assert found is None assert "never receive bot APPROVE" in reason comment = review(commit=receipt.AFIPC_230_HEAD, state="COMMENTED") - found, _ = receipt.evaluate_receipts( + found, reason = receipt.evaluate_receipts( [comment], receipt.AFIPC_230_HEAD, is_draft=True ) - assert found is comment + assert found is None + assert "no current-head formal" in reason def test_status_comment_and_mention_payloads_are_not_receipts() -> None: @@ -125,7 +126,7 @@ def test_receipt_helpers_cover_graphql_and_invalid_identity() -> None: assert receipt.review_matches_head(review(commit=receipt.AFIPC_230_HEAD), "") is False graphql = { "id": 7, - "state": "COMMENTED", + "state": "CHANGES_REQUESTED", "body": "## Pull request overview\nOpenCode reviewed the current-head product diff.\n", "author": {"login": "github-actions[bot]"}, "commit": {"oid": receipt.AFIPC_230_HEAD}, @@ -201,7 +202,7 @@ def test_receipt_cli_and_fetch(tmp_path: Path, capsys, monkeypatch) -> None: """CLI accepts a current-head receipt file and annotates a missing receipt.""" path = tmp_path / "reviews.json" path.write_text( - json.dumps([review(commit=receipt.AFIPC_230_HEAD, state="COMMENTED")]), + json.dumps([review(commit=receipt.AFIPC_230_HEAD, state="CHANGES_REQUESTED")]), encoding="utf-8", ) assert ( @@ -242,7 +243,7 @@ def fake_run(args, **kwargs): { "returncode": 0, "stdout": json.dumps( - [review(commit=receipt.AFIPC_230_HEAD, state="COMMENTED")] + [review(commit=receipt.AFIPC_230_HEAD, state="CHANGES_REQUESTED")] ), "stderr": "", }, From 263f565f84020eff53a6c726948592e24515689c Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 12:24:17 -0700 Subject: [PATCH 20/48] test: execute required verdict workflow instead of mirror --- ...st_opencode_required_verdict_regression.py | 97 ++++++++----------- 1 file changed, 43 insertions(+), 54 deletions(-) diff --git a/tests/test_opencode_required_verdict_regression.py b/tests/test_opencode_required_verdict_regression.py index 6b5296d14a..8807468d55 100644 --- a/tests/test_opencode_required_verdict_regression.py +++ b/tests/test_opencode_required_verdict_regression.py @@ -1,15 +1,18 @@ -"""Regression coverage for the required current-head OpenCode verdict gate.""" +"""Regression coverage for the runtime required current-head OpenCode verdict gate.""" from __future__ import annotations +import json +import shutil +import subprocess from pathlib import Path import pytest -from scripts.ci import opencode_dispatch_status as dispatch_status - HEAD = "a" * 40 +WORKFLOW = Path(".github/workflows/opencode-review.yml") +STATUS_HELPER = Path("scripts/ci/opencode_dispatch_status.py") def review(*, state: str, commit_id: str = HEAD, body: str = "") -> dict[str, object]: @@ -22,22 +25,32 @@ def review(*, state: str, commit_id: str = HEAD, body: str = "") -> dict[str, ob } -@pytest.mark.parametrize("state", ("APPROVED", "CHANGES_REQUESTED")) -def test_required_verdict_accepts_only_formal_current_head_states(state: str) -> None: - """A substantive current-head formal state is passing evidence.""" - decide = getattr(dispatch_status, "decide_required_verdict_check", None) - assert callable(decide), "required-verdict decision was removed" - - decision = decide( - expected_head=HEAD, - pull_request={"head": {"sha": HEAD}}, - reviews=[review(state=state)], +def runtime_verdict(reviews: list[dict[str, object]], head_sha: str = HEAD) -> str: + """Execute the jq program embedded in the required workflow.""" + jq = shutil.which("jq") + if jq is None: + pytest.skip("jq is required to execute the production verdict filter") + workflow = WORKFLOW.read_text(encoding="utf-8") + marker = 'jq -r -s --arg sha "$HEAD_SHA" \'' + start = workflow.index(marker) + len(marker) + end = workflow.index("\n ')", start) + result = subprocess.run( + [jq, "-r", "-s", "--arg", "sha", head_sha, workflow[start:end]], + input=json.dumps(reviews), + text=True, + capture_output=True, + check=False, ) + assert result.returncode == 0, result.stderr + return result.stdout.strip() - assert decision == { - "state": "success", - "description": f"Current-head OpenCode verdict: {state}.", - } + +@pytest.mark.parametrize("state", ("APPROVED", "CHANGES_REQUESTED")) +def test_runtime_required_verdict_accepts_only_formal_current_head_states( + state: str, +) -> None: + """A substantive current-head formal state is passing runtime evidence.""" + assert runtime_verdict([review(state=state)]) == state @pytest.mark.parametrize( @@ -49,54 +62,30 @@ def test_required_verdict_accepts_only_formal_current_head_states(state: str) -> [review(state="APPROVED", body="deterministic fallback approval")], ), ) -def test_required_verdict_rejects_absent_placeholder_and_old_head_evidence( +def test_runtime_required_verdict_rejects_nonpassing_evidence( reviews: list[dict[str, object]], ) -> None: """Status-only, fallback, and predecessor evidence remain non-passing.""" - decide = getattr(dispatch_status, "decide_required_verdict_check", None) - assert callable(decide), "required-verdict decision was removed" - - decision = decide( - expected_head=HEAD, - pull_request={"head": {"sha": HEAD}}, - reviews=reviews, - ) - - assert decision["state"] == "failure" - assert "required check is not a review" in decision["description"] + assert runtime_verdict(reviews) == "" -def test_required_verdict_rejects_empty_target_stale_live_head_and_other_actor() -> None: - """Malformed identity inputs and non-OpenCode reviews fail closed.""" - assert dispatch_status.current_head_opencode_verdict([], "") is None - assert ( - dispatch_status.current_head_opencode_verdict( - [ - { - "user": {"login": "coderabbitai[bot]"}, - "state": "APPROVED", - "commit_id": HEAD, - "body": "", - } - ], - HEAD, - ) - is None - ) +def test_runtime_required_verdict_rejects_other_actor() -> None: + """A non-OpenCode formal review cannot satisfy the runtime filter.""" + human = review(state="APPROVED") + human["user"] = {"login": "coderabbitai[bot]"} + assert runtime_verdict([human]) == "" - stale = dispatch_status.decide_required_verdict_check( - expected_head=HEAD, - pull_request={"head": {"sha": "c" * 40}}, - reviews=[review(state="APPROVED")], - ) - assert stale["state"] == "failure" - assert "target is stale" in stale["description"] +def test_required_verdict_has_one_executable_owner() -> None: + """Tests must execute the workflow gate, not a test-only Python mirror.""" + status_source = STATUS_HELPER.read_text(encoding="utf-8") + assert "def current_head_opencode_verdict" not in status_source + assert "def decide_required_verdict_check" not in status_source def test_required_workflow_cannot_succeed_with_an_echo_only_placeholder() -> None: """The required check must query Reviews API and fail without a verdict.""" - workflow = Path(".github/workflows/opencode-review.yml").read_text(encoding="utf-8") + workflow = WORKFLOW.read_text(encoding="utf-8") assert "pull-requests: read" in workflow assert "Fail closed without a current-head OpenCode verdict" in workflow From 63913cbb514b942d98c990f136b7e197e4836fdf Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 12:24:33 -0700 Subject: [PATCH 21/48] test: correct runtime jq fixture delimiter --- tests/test_opencode_required_verdict_regression.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_opencode_required_verdict_regression.py b/tests/test_opencode_required_verdict_regression.py index 8807468d55..4bd0ba419b 100644 --- a/tests/test_opencode_required_verdict_regression.py +++ b/tests/test_opencode_required_verdict_regression.py @@ -31,7 +31,7 @@ def runtime_verdict(reviews: list[dict[str, object]], head_sha: str = HEAD) -> s if jq is None: pytest.skip("jq is required to execute the production verdict filter") workflow = WORKFLOW.read_text(encoding="utf-8") - marker = 'jq -r -s --arg sha "$HEAD_SHA" \'' + marker = "jq -r -s --arg sha \\"$HEAD_SHA\\" '" start = workflow.index(marker) + len(marker) end = workflow.index("\n ')", start) result = subprocess.run( From 360fa42142689d5ee27ff6ae537a9e693819aeca Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 12:24:48 -0700 Subject: [PATCH 22/48] test: use unambiguous jq marker literal --- tests/test_opencode_required_verdict_regression.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_opencode_required_verdict_regression.py b/tests/test_opencode_required_verdict_regression.py index 4bd0ba419b..0fbecedd91 100644 --- a/tests/test_opencode_required_verdict_regression.py +++ b/tests/test_opencode_required_verdict_regression.py @@ -31,7 +31,7 @@ def runtime_verdict(reviews: list[dict[str, object]], head_sha: str = HEAD) -> s if jq is None: pytest.skip("jq is required to execute the production verdict filter") workflow = WORKFLOW.read_text(encoding="utf-8") - marker = "jq -r -s --arg sha \\"$HEAD_SHA\\" '" + marker = """jq -r -s --arg sha "$HEAD_SHA" '""" start = workflow.index(marker) + len(marker) end = workflow.index("\n ')", start) result = subprocess.run( From 34e02bd1fef471a770cf3f243307b6cd1928f99e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 12:26:46 -0700 Subject: [PATCH 23/48] refactor: remove test-only verdict mirror --- scripts/ci/opencode_dispatch_status.py | 58 -------------------------- 1 file changed, 58 deletions(-) diff --git a/scripts/ci/opencode_dispatch_status.py b/scripts/ci/opencode_dispatch_status.py index c433365fac..25cbb75b7a 100644 --- a/scripts/ci/opencode_dispatch_status.py +++ b/scripts/ci/opencode_dispatch_status.py @@ -10,74 +10,16 @@ try: from opencode_existing_approval_gate import ( - FALLBACK_MARKERS, OPENCODE_APP_APPROVAL_AUTHORS, review_rejection_reason, ) except ModuleNotFoundError: # pragma: no cover - package import path from scripts.ci.opencode_existing_approval_gate import ( - FALLBACK_MARKERS, OPENCODE_APP_APPROVAL_AUTHORS, review_rejection_reason, ) -OPENCODE_VERDICT_STATES = frozenset({"APPROVED", "CHANGES_REQUESTED"}) -MISSING_VERDICT_MESSAGE = ( - "No APPROVED or CHANGES_REQUESTED from opencode-agent on the current head. " - "This required check is not a review and must not succeed until the " - "authenticated dispatch posts a current-head verdict." -) - - -def current_head_opencode_verdict( - reviews: Sequence[dict[str, Any]], head_sha: str -) -> str | None: - """Return the latest substantive current-head OpenCode verdict, if any.""" - expected = (head_sha or "").lower() - if not expected: - return None - for review in reversed(reviews): - author = str((review.get("user") or {}).get("login") or "").casefold() - if author not in OPENCODE_APP_APPROVAL_AUTHORS: - continue - if str(review.get("commit_id") or "").lower() != expected: - continue - state = str(review.get("state") or "").upper() - if state not in OPENCODE_VERDICT_STATES: - return None - body = str(review.get("body") or "").casefold() - if state == "APPROVED" and any(marker in body for marker in FALLBACK_MARKERS): - return None - return state - return None - - -def decide_required_verdict_check( - *, - expected_head: str, - pull_request: dict[str, Any], - reviews: Sequence[dict[str, Any]], -) -> dict[str, str]: - """Fail closed unless OpenCode already published a current-head verdict.""" - live_head = str((pull_request.get("head") or {}).get("sha") or "") - if not expected_head or live_head.lower() != expected_head.lower(): - return { - "state": "failure", - "description": ( - "OpenCode required-check target is stale or the live PR head " - "is unavailable." - ), - } - verdict = current_head_opencode_verdict(reviews, expected_head) - if verdict is None: - return {"state": "failure", "description": MISSING_VERDICT_MESSAGE} - return { - "state": "success", - "description": f"Current-head OpenCode verdict: {verdict}.", - } - - def _has_current_approval(reviews: Sequence[dict[str, Any]], head_sha: str) -> bool: """Return whether the latest OpenCode decision is a verified approval.""" for review in reversed(reviews): From dea5106dce1d50e39ea4907ec8c5a93a15cd5608 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 12:34:13 -0700 Subject: [PATCH 24/48] test: retry transient coverage identity reads --- tests/test_opencode_coverage_identity.py | 37 ++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/test_opencode_coverage_identity.py b/tests/test_opencode_coverage_identity.py index 8a695b550b..df64830d97 100644 --- a/tests/test_opencode_coverage_identity.py +++ b/tests/test_opencode_coverage_identity.py @@ -142,6 +142,43 @@ def unexpected_run(args, **kwargs): identity.fetch_check_runs("ContextualWisdomLab/kaefa", "not-a-sha") + +def test_fetch_check_runs_retries_transient_github_read_failure(monkeypatch) -> None: + """A transient 429 is retried before exact-head identity fails closed.""" + page = { + "check_runs": [ + coverage_check(head=identity.KAEFA_78_HEAD, conclusion="success") + ] + } + responses = [ + type( + "Completed", + (), + {"returncode": 1, "stdout": "", "stderr": "HTTP 429 rate limit exceeded"}, + )(), + type( + "Completed", + (), + {"returncode": 0, "stdout": json.dumps([page]), "stderr": ""}, + )(), + ] + calls = 0 + + def fake_run(args, **kwargs): + nonlocal calls + calls += 1 + return responses.pop(0) + + monkeypatch.setattr(identity, "RETRY_DELAYS", (0, 0, 0), raising=False) + monkeypatch.setattr(identity.subprocess, "run", fake_run) + + loaded = identity.fetch_check_runs( + "ContextualWisdomLab/kaefa", identity.KAEFA_78_HEAD + ) + + assert calls == 2 + assert loaded[0]["name"] == "coverage-evidence" + def test_fetch_check_runs_parses_pages(monkeypatch) -> None: """Paginated gh output and error paths stay fail-closed.""" page = { From 0d11fdfbc6d25a42e9ebcbfbb4f474865dc659ec Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 12:36:11 -0700 Subject: [PATCH 25/48] fix: retry transient coverage identity reads --- scripts/ci/opencode_coverage_identity.py | 56 +++++++++++++++++------- 1 file changed, 39 insertions(+), 17 deletions(-) diff --git a/scripts/ci/opencode_coverage_identity.py b/scripts/ci/opencode_coverage_identity.py index 3056bc8992..d0e68674ae 100644 --- a/scripts/ci/opencode_coverage_identity.py +++ b/scripts/ci/opencode_coverage_identity.py @@ -9,6 +9,7 @@ import re import subprocess import sys +import time from collections.abc import Mapping, Sequence from pathlib import Path from typing import Any @@ -21,6 +22,12 @@ TERMINAL_RESULTS = frozenset( {"success", "failure", "cancelled", "skipped", "neutral", "timed_out", "action_required"} ) +RETRY_DELAYS = (0.0, 1.0, 3.0) +TRANSIENT_GH_READ_ERROR_RE = re.compile( + r"(?i)(?:http\\s*(?:429|500|502|503|504)\\b|rate.?limit|" + r"secondary rate limit|timeout|timed out|temporar(?:y|ily) unavailable|" + r"connection (?:reset|refused|closed)|tls handshake timeout)" +) KAEFA_78_HEAD = "5092a70c9737221d6367e74643d06980609fe0b1" KAEFA_75_HEAD = "4c8ad480a0f104601ca668cee5f0cf9372e819c3" @@ -166,23 +173,38 @@ def fetch_check_runs(repo: str, head_sha: str) -> list[Mapping[str, Any]]: raise CoverageQuoteError(f"coverage identity requires an owner/repo value, got {repo!r}") if not SHA_RE.fullmatch(head_sha): raise CoverageQuoteError("coverage identity requires a 40-character head SHA") - completed = subprocess.run( - [ - "gh", - "api", - f"repos/{repo}/commits/{head_sha}/check-runs?per_page=100", - "--paginate", - "--slurp", - ], - text=True, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - check=False, - shell=False, - ) - if completed.returncode != 0: - detail = (completed.stderr or completed.stdout or "gh check-runs lookup failed").strip() - raise CoverageQuoteError(f"canonical coverage check lookup failed: {detail}") + completed = None + for attempt, delay in enumerate(RETRY_DELAYS): + if delay: + time.sleep(delay) + completed = subprocess.run( + [ + "gh", + "api", + f"repos/{repo}/commits/{head_sha}/check-runs?per_page=100", + "--paginate", + "--slurp", + ], + text=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + check=False, + shell=False, + ) + if completed.returncode == 0: + break + detail = ( + completed.stderr or completed.stdout or "gh check-runs lookup failed" + ).strip() + if ( + not TRANSIENT_GH_READ_ERROR_RE.search(detail) + or attempt + 1 >= len(RETRY_DELAYS) + ): + raise CoverageQuoteError( + f"canonical coverage check lookup failed: {detail}" + ) + if completed is None or completed.returncode != 0: + raise CoverageQuoteError("canonical coverage check lookup failed after retries") loaded = json.loads(completed.stdout or "{}") if isinstance(loaded, list): runs: list[Mapping[str, Any]] = [] From 5c42c7944c3990d26643c276949b56ea648434eb Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 12:44:02 -0700 Subject: [PATCH 26/48] test: reproduce bare HTTP 502 retry --- tests/test_opencode_coverage_identity.py | 30 ++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/test_opencode_coverage_identity.py b/tests/test_opencode_coverage_identity.py index df64830d97..20adde87f7 100644 --- a/tests/test_opencode_coverage_identity.py +++ b/tests/test_opencode_coverage_identity.py @@ -179,6 +179,36 @@ def fake_run(args, **kwargs): assert calls == 2 assert loaded[0]["name"] == "coverage-evidence" + +def test_fetch_check_runs_retries_bare_http_502(monkeypatch) -> None: + """A bare HTTP 502 status exercises the numeric transient matcher.""" + page = { + "check_runs": [ + coverage_check(head=identity.KAEFA_78_HEAD, conclusion="success") + ] + } + responses = [ + type( + "Completed", + (), + {"returncode": 1, "stdout": "", "stderr": "HTTP 502 Bad Gateway"}, + )(), + type( + "Completed", + (), + {"returncode": 0, "stdout": json.dumps([page]), "stderr": ""}, + )(), + ] + + monkeypatch.setattr(identity, "RETRY_DELAYS", (0, 0, 0)) + monkeypatch.setattr( + identity.subprocess, "run", lambda args, **kwargs: responses.pop(0) + ) + + assert identity.fetch_check_runs( + "ContextualWisdomLab/kaefa", identity.KAEFA_78_HEAD + ) + def test_fetch_check_runs_parses_pages(monkeypatch) -> None: """Paginated gh output and error paths stay fail-closed.""" page = { From b41ff0e6bee02d49d11d8e4291840af329b0cb6e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 12:46:13 -0700 Subject: [PATCH 27/48] fix: match transient HTTP status tokens --- scripts/ci/opencode_coverage_identity.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/ci/opencode_coverage_identity.py b/scripts/ci/opencode_coverage_identity.py index d0e68674ae..47c5e92e76 100644 --- a/scripts/ci/opencode_coverage_identity.py +++ b/scripts/ci/opencode_coverage_identity.py @@ -24,7 +24,7 @@ ) RETRY_DELAYS = (0.0, 1.0, 3.0) TRANSIENT_GH_READ_ERROR_RE = re.compile( - r"(?i)(?:http\\s*(?:429|500|502|503|504)\\b|rate.?limit|" + r"(?i)(?:http\s*(?:429|500|502|503|504)\b|rate.?limit|" r"secondary rate limit|timeout|timed out|temporar(?:y|ily) unavailable|" r"connection (?:reset|refused|closed)|tls handshake timeout)" ) From e76a54bb0b8224e8a644a5bd0ba9549d4b97fc85 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 12:53:54 -0700 Subject: [PATCH 28/48] test: reject workflow anchor path aliases --- tests/test_opencode_review_surfaces.py | 30 ++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/test_opencode_review_surfaces.py b/tests/test_opencode_review_surfaces.py index 43112a5d37..7a3ccfac78 100644 --- a/tests/test_opencode_review_surfaces.py +++ b/tests/test_opencode_review_surfaces.py @@ -118,6 +118,36 @@ def test_workflow_anchor_forbidden_unless_file_is_in_diff() -> None: ) + +def test_workflow_anchor_variants_cannot_spoof_unrelated_findings() -> None: + """Backslash and dot-segment aliases are the same forbidden central anchor.""" + rendered = surfaces.format_structured_findings( + [ + { + "path": r".github\workflows\opencode-review.yml", + "line": 42, + "title": "spoof", + } + ], + ORIGINWEAVE_47_FILES, + ) + assert r".github\workflows\opencode-review.yml" not in rendered + assert "Review process" in rendered + + review = surfaces.format_request_changes_review( + model_prose=( + r"Finding: .github\workflows\opencode-review.yml:42 " + "and .github/workflows/./opencode-review.yml:7" + ), + head_sha=HEAD, + run_id="1", + run_attempt="1", + changed_files=ORIGINWEAVE_47_FILES, + ) + assert r".github\workflows\opencode-review.yml" not in review + assert ".github/workflows/./opencode-review.yml" not in review + assert review.count("Review process") >= 2 + def test_korean_status_and_review_keep_identifiers() -> None: """Korean PRs stay Korean while crate paths remain unchanged.""" review = surfaces.build_fallback_review( From f75e98b234c59edcceef76d5a87de67c15d98159 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 12:55:38 -0700 Subject: [PATCH 29/48] fix: normalize untrusted workflow anchor paths --- scripts/ci/opencode_review_surfaces.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/scripts/ci/opencode_review_surfaces.py b/scripts/ci/opencode_review_surfaces.py index 62c0477b38..9d7ebb7af7 100644 --- a/scripts/ci/opencode_review_surfaces.py +++ b/scripts/ci/opencode_review_surfaces.py @@ -18,6 +18,10 @@ from pathlib import Path, PurePosixPath CENTRAL_WORKFLOW_ANCHOR = ".github/workflows/opencode-review.yml" +CENTRAL_WORKFLOW_ANCHOR_RE = re.compile( + r"\\.github[\\\\/]+workflows(?:[\\\\/]+\\.)*[\\\\/]+" + r"opencode-review\\.yml(?::[0-9]+)?" +) PUB_ITEM_RE = re.compile( r"^\s*pub(?:\s*\([^)]*\))?\s+" r"(?:async\s+)?(?:unsafe\s+)?" @@ -343,7 +347,7 @@ def format_structured_findings( for index, raw in enumerate(findings, start=1): if not isinstance(raw, Mapping): continue - path = str(raw.get("path") or "unknown") + path = posix_path(str(raw.get("path") or "unknown")) line = raw.get("line") or 0 location = f"{path}:{line}" if path == CENTRAL_WORKFLOW_ANCHOR and not coverage_anchor_allowed( @@ -371,11 +375,7 @@ def _strip_forbidden_workflow_anchor(body: str, changed_files: Sequence[str]) -> """Remove any synthesized central-workflow citation unless that file changed.""" if coverage_anchor_allowed(CENTRAL_WORKFLOW_ANCHOR, changed_files): return body - return re.sub( - rf"{re.escape(CENTRAL_WORKFLOW_ANCHOR)}(?::[0-9]+)?", - "Review process", - body, - ) + return CENTRAL_WORKFLOW_ANCHOR_RE.sub("Review process", body) def format_request_changes_review( From 572553908188807b48791dae3ba3bc107552f8e1 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 12:58:45 -0700 Subject: [PATCH 30/48] fix: correct workflow anchor regex escapes --- scripts/ci/opencode_review_surfaces.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/ci/opencode_review_surfaces.py b/scripts/ci/opencode_review_surfaces.py index 9d7ebb7af7..7b1c182871 100644 --- a/scripts/ci/opencode_review_surfaces.py +++ b/scripts/ci/opencode_review_surfaces.py @@ -19,8 +19,8 @@ CENTRAL_WORKFLOW_ANCHOR = ".github/workflows/opencode-review.yml" CENTRAL_WORKFLOW_ANCHOR_RE = re.compile( - r"\\.github[\\\\/]+workflows(?:[\\\\/]+\\.)*[\\\\/]+" - r"opencode-review\\.yml(?::[0-9]+)?" + r"\.github[\\/]+workflows(?:[\\/]+\.)*[\\/]+" + r"opencode-review\.yml(?::[0-9]+)?" ) PUB_ITEM_RE = re.compile( r"^\s*pub(?:\s*\([^)]*\))?\s+" From 3e31eb56e26b008dfe76a29cd97cc805bf66e415 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 13:11:18 -0700 Subject: [PATCH 31/48] test(opencode): expose later comment verdict masking --- tests/test_opencode_required_verdict_regression.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/test_opencode_required_verdict_regression.py b/tests/test_opencode_required_verdict_regression.py index 0fbecedd91..881e93c2ef 100644 --- a/tests/test_opencode_required_verdict_regression.py +++ b/tests/test_opencode_required_verdict_regression.py @@ -69,6 +69,16 @@ def test_runtime_required_verdict_rejects_nonpassing_evidence( assert runtime_verdict(reviews) == "" +@pytest.mark.parametrize("state", ("APPROVED", "CHANGES_REQUESTED")) +def test_runtime_required_verdict_ignores_later_nonformal_current_head_comment( + state: str, +) -> None: + """A later COMMENTED receipt cannot mask the current-head formal verdict.""" + assert runtime_verdict( + [review(state=state), review(state="COMMENTED", body="status-only follow-up")] + ) == state + + def test_runtime_required_verdict_rejects_other_actor() -> None: """A non-OpenCode formal review cannot satisfy the runtime filter.""" human = review(state="APPROVED") From 3d98c59752a6d30a0ce6059c9ee3088584e6ff9d Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 13:13:55 -0700 Subject: [PATCH 32/48] fix(opencode): retain latest formal head verdict --- .github/workflows/opencode-review.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/opencode-review.yml b/.github/workflows/opencode-review.yml index bd074f7aea..750b754914 100644 --- a/.github/workflows/opencode-review.yml +++ b/.github/workflows/opencode-review.yml @@ -84,6 +84,7 @@ jobs: | $user == "opencode-agent" or $user == "opencode-agent[bot]" ) | select((.commit_id // "" | ascii_downcase) == ($sha | ascii_downcase)) + | select(.state == "APPROVED" or .state == "CHANGES_REQUESTED") ] | (last // {}) as $review | ($review.body // "" | ascii_downcase) as $body From aa43e61fcf3daf1ad9185387f85c95677f00e3f0 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 13:22:45 -0700 Subject: [PATCH 33/48] test(opencode): bind coverage to dispatch run job --- tests/test_opencode_coverage_identity.py | 119 +++++++++++++++++++++++ 1 file changed, 119 insertions(+) diff --git a/tests/test_opencode_coverage_identity.py b/tests/test_opencode_coverage_identity.py index 20adde87f7..2abfcf673d 100644 --- a/tests/test_opencode_coverage_identity.py +++ b/tests/test_opencode_coverage_identity.py @@ -303,3 +303,122 @@ def fake_mixed_pages(args, **kwargs): ) == 0 ) + + +def dispatch_run( + *, + run_id: str, + target_repo: str, + pr_number: int, + head_sha: str, + workflow_repo: str = "ContextualWisdomLab/.github", +) -> dict[str, object]: + """Build the trusted central repository_dispatch run identity.""" + return { + "id": int(run_id), + "event": "repository_dispatch", + "name": "OpenCode Review Dispatch", + "display_title": ( + f"OpenCode Review Dispatch {target_repo}#{pr_number}@{head_sha}" + ), + "repository": {"full_name": workflow_repo}, + } + + +def coverage_job( + *, conclusion: str = "success", name: str = "coverage-evidence", status: str = "completed" +) -> dict[str, object]: + """Build one Actions job from the current workflow run.""" + return {"name": name, "status": status, "conclusion": conclusion} + + +def test_repository_dispatch_coverage_binds_to_current_central_run_job() -> None: + """Coverage authority is the completed job in the exact central dispatch run.""" + run_id = "33112315024" + target_repo = "ContextualWisdomLab/scopeweave" + run = dispatch_run( + run_id=run_id, + target_repo=target_repo, + pr_number=523, + head_sha=identity.KAEFA_78_HEAD, + ) + jobs = [coverage_job(conclusion="failure")] + + assert identity.terminal_dispatch_coverage_result( + run, + jobs, + workflow_repo="ContextualWisdomLab/.github", + target_repo=target_repo, + pr_number="523", + head_sha=identity.KAEFA_78_HEAD, + run_id=run_id, + ) == "failure" + + +@pytest.mark.parametrize( + ("mutation", "match"), + ( + ({"event": "pull_request"}, "repository_dispatch"), + ({"name": "Other Workflow"}, "workflow"), + ({"display_title": "OpenCode Review Dispatch spoof"}, "target"), + ({"repository": {"full_name": "ContextualWisdomLab/scopeweave"}}, "repository"), + ), +) +def test_repository_dispatch_coverage_rejects_wrong_run_identity( + mutation: dict[str, object], match: str +) -> None: + """A same-named job from another event/workflow/target/repository is non-passing.""" + run_id = "33112315024" + target_repo = "ContextualWisdomLab/scopeweave" + run = dispatch_run( + run_id=run_id, + target_repo=target_repo, + pr_number=523, + head_sha=identity.KAEFA_78_HEAD, + ) + run.update(mutation) + + with pytest.raises(identity.CoverageQuoteError, match=match): + identity.terminal_dispatch_coverage_result( + run, + [coverage_job()], + workflow_repo="ContextualWisdomLab/.github", + target_repo=target_repo, + pr_number="523", + head_sha=identity.KAEFA_78_HEAD, + run_id=run_id, + ) + + +@pytest.mark.parametrize( + "jobs", + ( + [], + [coverage_job(status="in_progress", conclusion="")], + [coverage_job(), coverage_job()], + [coverage_job(name="coverage-source-tree")], + ), +) +def test_repository_dispatch_coverage_requires_one_completed_exact_job( + jobs: list[dict[str, object]], +) -> None: + """Absent, pending, ambiguous, or differently named jobs fail closed.""" + run_id = "33112315024" + target_repo = "ContextualWisdomLab/scopeweave" + run = dispatch_run( + run_id=run_id, + target_repo=target_repo, + pr_number=523, + head_sha=identity.KAEFA_78_HEAD, + ) + + with pytest.raises(identity.CoverageQuoteError, match="coverage-evidence"): + identity.terminal_dispatch_coverage_result( + run, + jobs, + workflow_repo="ContextualWisdomLab/.github", + target_repo=target_repo, + pr_number="523", + head_sha=identity.KAEFA_78_HEAD, + run_id=run_id, + ) From 61c0bab55fc1e5241e4dcaed46ad64203bf231b4 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 13:23:01 -0700 Subject: [PATCH 34/48] test(opencode): require central run coverage authority --- tests/test_opencode_coverage_publication_regression.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/test_opencode_coverage_publication_regression.py b/tests/test_opencode_coverage_publication_regression.py index de54dc6fff..9c6846692f 100644 --- a/tests/test_opencode_coverage_publication_regression.py +++ b/tests/test_opencode_coverage_publication_regression.py @@ -54,10 +54,20 @@ def test_initial_overview_resolves_current_run_coverage_identity() -> None: "${{ needs.coverage-evidence.result || 'skipped' }}" in step ) assert '--run-id "$RUN_ID"' in step + assert '--workflow-repo "$GITHUB_REPOSITORY"' in step + assert '--pr-number "$PR_NUMBER"' in step assert "opencode_coverage_identity.py" in step assert step.index("opencode_coverage_identity.py") < step.index("build-status") +def test_all_runtime_coverage_identity_calls_use_central_dispatch_authority() -> None: + """Every caller must bind the target head to the exact central workflow run.""" + workflow = WORKFLOW.read_text(encoding="utf-8") + + assert workflow.count('--workflow-repo "$GITHUB_REPOSITORY"') == 3 + assert workflow.count('--pr-number "$PR_NUMBER"') == 3 + + def test_duplicate_coverage_names_are_bound_to_the_current_dispatch_run() -> None: """A success stub from another run cannot shadow the real failed coverage job.""" parameters = inspect.signature(identity.terminal_coverage_result).parameters From 2ec82152a3681154d061c912954c43e042ff425c Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 13:26:08 -0700 Subject: [PATCH 35/48] fix(opencode): verify exact dispatch coverage job --- scripts/ci/opencode_coverage_identity.py | 164 ++++++++++++++++++++++- 1 file changed, 161 insertions(+), 3 deletions(-) diff --git a/scripts/ci/opencode_coverage_identity.py b/scripts/ci/opencode_coverage_identity.py index 47c5e92e76..5fb122b437 100644 --- a/scripts/ci/opencode_coverage_identity.py +++ b/scripts/ci/opencode_coverage_identity.py @@ -17,6 +17,7 @@ CANONICAL_CHECK_NAME = "coverage-evidence" CANONICAL_WORKFLOW_NAMES = frozenset({"Required OpenCode Review"}) +DISPATCH_WORKFLOW_NAME = "OpenCode Review Dispatch" SHA_RE = re.compile(r"^[0-9a-fA-F]{40}$") REPO_RE = re.compile(r"^[A-Za-z0-9_][A-Za-z0-9_.-]*/[A-Za-z0-9_][A-Za-z0-9_.-]*$") TERMINAL_RESULTS = frozenset( @@ -105,6 +106,65 @@ def is_canonical_coverage_check( return not workflow or workflow in CANONICAL_WORKFLOW_NAMES + +def terminal_dispatch_coverage_result( + workflow_run: Mapping[str, Any], + jobs: Sequence[Mapping[str, Any]], + *, + workflow_repo: str, + target_repo: str, + pr_number: str, + head_sha: str, + run_id: str, +) -> str: + """Return coverage from the exact central repository_dispatch workflow job.""" + if not REPO_RE.fullmatch(workflow_repo): + raise CoverageQuoteError("coverage identity requires a valid workflow repository") + if not REPO_RE.fullmatch(target_repo): + raise CoverageQuoteError("coverage identity requires a valid target repository") + if not str(pr_number).isdigit() or int(pr_number) < 1: + raise CoverageQuoteError("coverage identity requires a positive pull request number") + if not SHA_RE.fullmatch(head_sha): + raise CoverageQuoteError("coverage identity requires a 40-character head SHA") + if not str(run_id).isdigit(): + raise CoverageQuoteError("coverage identity requires a numeric workflow run id") + if str(workflow_run.get("id") or "") != str(run_id): + raise CoverageQuoteError("coverage workflow run id does not match the current run") + if str(workflow_run.get("event") or "") != "repository_dispatch": + raise CoverageQuoteError("coverage workflow run is not repository_dispatch") + if str(workflow_run.get("name") or "") != DISPATCH_WORKFLOW_NAME: + raise CoverageQuoteError("coverage workflow name is not OpenCode Review Dispatch") + repository = workflow_run.get("repository") or {} + recorded_repo = ( + str(repository.get("full_name") or "").strip() + if isinstance(repository, Mapping) + else "" + ) + if recorded_repo != workflow_repo: + raise CoverageQuoteError("coverage workflow repository does not match") + expected_title = ( + f"{DISPATCH_WORKFLOW_NAME} {target_repo}#{pr_number}@{head_sha}" + ) + if str(workflow_run.get("display_title") or "").strip() != expected_title: + raise CoverageQuoteError("coverage workflow target identity does not match") + matches = [ + job + for job in jobs + if isinstance(job, Mapping) + and str(job.get("name") or "").strip() == CANONICAL_CHECK_NAME + and str(job.get("status") or "").strip().casefold() == "completed" + ] + if len(matches) != 1: + raise CoverageQuoteError( + f"expected one completed {CANONICAL_CHECK_NAME} job in current run" + ) + result = normalize_result(str(matches[0].get("conclusion") or "")) + if result == "unknown": + raise CoverageQuoteError( + f"current-run {CANONICAL_CHECK_NAME} conclusion is missing or non-terminal" + ) + return result + def terminal_coverage_result( check_runs: Sequence[Mapping[str, Any]], head_sha: str, @@ -221,10 +281,81 @@ def fetch_check_runs(repo: str, head_sha: str) -> list[Mapping[str, Any]]: raise CoverageQuoteError("canonical coverage check lookup returned malformed JSON") + +def _run_gh_json(args: list[str]) -> Any: + """Run one bounded authenticated GitHub JSON read.""" + completed = None + for attempt, delay in enumerate(RETRY_DELAYS): + if delay: + time.sleep(delay) + completed = subprocess.run( + args, + text=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + check=False, + shell=False, + ) + if completed.returncode == 0: + break + detail = ( + completed.stderr or completed.stdout or "gh workflow lookup failed" + ).strip() + if ( + not TRANSIENT_GH_READ_ERROR_RE.search(detail) + or attempt + 1 >= len(RETRY_DELAYS) + ): + raise CoverageQuoteError(f"canonical workflow lookup failed: {detail}") + if completed is None or completed.returncode != 0: + raise CoverageQuoteError("canonical workflow lookup failed after retries") + return json.loads(completed.stdout or "{}") + + +def fetch_dispatch_workflow_run(workflow_repo: str, run_id: str) -> Mapping[str, Any]: + """Read the exact central workflow-run metadata.""" + if not REPO_RE.fullmatch(workflow_repo): + raise CoverageQuoteError("coverage identity requires a valid workflow repository") + if not str(run_id).isdigit(): + raise CoverageQuoteError("coverage identity requires a numeric workflow run id") + loaded = _run_gh_json( + ["gh", "api", f"repos/{workflow_repo}/actions/runs/{run_id}"] + ) + if not isinstance(loaded, Mapping): + raise CoverageQuoteError("canonical workflow run lookup returned malformed JSON") + return loaded + + +def fetch_dispatch_workflow_jobs( + workflow_repo: str, run_id: str +) -> list[Mapping[str, Any]]: + """Read latest-attempt jobs from the exact central workflow run.""" + if not REPO_RE.fullmatch(workflow_repo): + raise CoverageQuoteError("coverage identity requires a valid workflow repository") + if not str(run_id).isdigit(): + raise CoverageQuoteError("coverage identity requires a numeric workflow run id") + loaded = _run_gh_json( + [ + "gh", + "api", + f"repos/{workflow_repo}/actions/runs/{run_id}/jobs?filter=latest&per_page=100", + "--paginate", + "--slurp", + ] + ) + pages = loaded if isinstance(loaded, list) else [loaded] + jobs: list[Mapping[str, Any]] = [] + for page in pages: + if not isinstance(page, Mapping) or not isinstance(page.get("jobs"), list): + raise CoverageQuoteError("canonical workflow jobs lookup returned malformed JSON") + jobs.extend(job for job in page["jobs"] if isinstance(job, Mapping)) + return jobs + def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace: """Parse coverage-identity CLI arguments.""" parser = argparse.ArgumentParser(description=__doc__) parser.add_argument("--repo", default="") + parser.add_argument("--workflow-repo", default="") + parser.add_argument("--pr-number", default="") parser.add_argument("--head-sha", required=True) parser.add_argument("--run-id") parser.add_argument("--quoted-result", required=True) @@ -238,13 +369,40 @@ def main(argv: Sequence[str] | None = None) -> int: try: if args.check_runs_file: checks = load_check_runs(args.check_runs_file) + canonical = assert_quoted_matches( + args.quoted_result, checks, args.head_sha, args.run_id + ) + elif args.workflow_repo: + if not args.repo or not args.pr_number or not args.run_id: + raise CoverageQuoteError( + "dispatch coverage identity needs target repo, PR number, and run id" + ) + workflow_run = fetch_dispatch_workflow_run( + args.workflow_repo, args.run_id + ) + jobs = fetch_dispatch_workflow_jobs(args.workflow_repo, args.run_id) + canonical = terminal_dispatch_coverage_result( + workflow_run, + jobs, + workflow_repo=args.workflow_repo, + target_repo=args.repo, + pr_number=args.pr_number, + head_sha=args.head_sha, + run_id=args.run_id, + ) + quoted = normalize_result(args.quoted_result) + if quoted != canonical: + raise CoverageQuoteError( + f"quoted coverage-evidence result {quoted!r} does not match " + f"canonical current-run result {canonical!r} for {args.head_sha}" + ) elif args.repo: checks = fetch_check_runs(args.repo, args.head_sha) + canonical = assert_quoted_matches( + args.quoted_result, checks, args.head_sha, args.run_id + ) else: raise CoverageQuoteError("coverage identity needs --repo or --check-runs-file") - canonical = assert_quoted_matches( - args.quoted_result, checks, args.head_sha, args.run_id - ) except (CoverageQuoteError, json.JSONDecodeError, OSError) as exc: print(f"::error::{exc}", file=sys.stderr) summary = os.environ.get("GITHUB_STEP_SUMMARY") From 186996c37f383756a7b9f1920871af5dbda7e8cc Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 13:26:21 -0700 Subject: [PATCH 36/48] fix(opencode): bind coverage to central run --- .github/workflows/opencode-review-dispatch.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/opencode-review-dispatch.yml b/.github/workflows/opencode-review-dispatch.yml index d61439c627..849c35d148 100644 --- a/.github/workflows/opencode-review-dispatch.yml +++ b/.github/workflows/opencode-review-dispatch.yml @@ -2749,6 +2749,8 @@ jobs: quoted_coverage="${COVERAGE_EVIDENCE_RESULT:-}" COVERAGE_EVIDENCE_RESULT="$(python3 scripts/ci/opencode_coverage_identity.py \ --repo "$GH_REPOSITORY" \ + --workflow-repo "$GITHUB_REPOSITORY" \ + --pr-number "$PR_NUMBER" \ --head-sha "$PR_HEAD_SHA" \ --run-id "$GITHUB_RUN_ID" \ --quoted-result "$quoted_coverage")" @@ -4703,6 +4705,8 @@ jobs: quoted_coverage="${COVERAGE_EVIDENCE_RESULT:-}" COVERAGE_EVIDENCE_RESULT="$(python3 scripts/ci/opencode_coverage_identity.py \ --repo "$GH_REPOSITORY" \ + --workflow-repo "$GITHUB_REPOSITORY" \ + --pr-number "$PR_NUMBER" \ --head-sha "$HEAD_SHA" \ --run-id "$RUN_ID" \ --quoted-result "$quoted_coverage")" @@ -5212,6 +5216,8 @@ jobs: quoted_coverage="${COVERAGE_EVIDENCE_RESULT:-}" COVERAGE_EVIDENCE_RESULT="$(python3 scripts/ci/opencode_coverage_identity.py \ --repo "$GH_REPOSITORY" \ + --workflow-repo "$GITHUB_REPOSITORY" \ + --pr-number "$PR_NUMBER" \ --head-sha "$HEAD_SHA" \ --run-id "$RUN_ID" \ --quoted-result "$quoted_coverage")" From 61a96248f8850e817014c1f9166f968b876d239f Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 13:29:07 -0700 Subject: [PATCH 37/48] fix(opencode): accept central dot repository --- scripts/ci/opencode_coverage_identity.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/ci/opencode_coverage_identity.py b/scripts/ci/opencode_coverage_identity.py index 5fb122b437..b24bbbc891 100644 --- a/scripts/ci/opencode_coverage_identity.py +++ b/scripts/ci/opencode_coverage_identity.py @@ -19,7 +19,7 @@ CANONICAL_WORKFLOW_NAMES = frozenset({"Required OpenCode Review"}) DISPATCH_WORKFLOW_NAME = "OpenCode Review Dispatch" SHA_RE = re.compile(r"^[0-9a-fA-F]{40}$") -REPO_RE = re.compile(r"^[A-Za-z0-9_][A-Za-z0-9_.-]*/[A-Za-z0-9_][A-Za-z0-9_.-]*$") +REPO_RE = re.compile(r"^[A-Za-z0-9_][A-Za-z0-9_.-]*/[A-Za-z0-9_.-]+$") TERMINAL_RESULTS = frozenset( {"success", "failure", "cancelled", "skipped", "neutral", "timed_out", "action_required"} ) From 37cb0ea39b57678ccc8e50a99a36b39877acc485 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 13:29:20 -0700 Subject: [PATCH 38/48] test(opencode): scope dispatch argument assertions --- tests/test_opencode_coverage_publication_regression.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/test_opencode_coverage_publication_regression.py b/tests/test_opencode_coverage_publication_regression.py index 9c6846692f..48a703a4fe 100644 --- a/tests/test_opencode_coverage_publication_regression.py +++ b/tests/test_opencode_coverage_publication_regression.py @@ -64,8 +64,13 @@ def test_all_runtime_coverage_identity_calls_use_central_dispatch_authority() -> """Every caller must bind the target head to the exact central workflow run.""" workflow = WORKFLOW.read_text(encoding="utf-8") - assert workflow.count('--workflow-repo "$GITHUB_REPOSITORY"') == 3 - assert workflow.count('--pr-number "$PR_NUMBER"') == 3 + call_marker = "python3 scripts/ci/opencode_coverage_identity.py" + calls = workflow.split(call_marker)[1:] + assert len(calls) == 3 + for call in calls: + invocation = call.split(')"', 1)[0] + assert '--workflow-repo "$GITHUB_REPOSITORY"' in invocation + assert '--pr-number "$PR_NUMBER"' in invocation def test_duplicate_coverage_names_are_bound_to_the_current_dispatch_run() -> None: From dc106b8961452f7d1e0a8747ee8fc71b2445a2f9 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 13:29:32 -0700 Subject: [PATCH 39/48] test(opencode): pin reviewed dispatch workflow --- tests/test_pr_review_autofix_nvidia_nim_contract.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index 3e07b51fcb..9f6e7238b1 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -19,7 +19,7 @@ DOCTORING_RECORD = Path("docs/doctoring/hourly-nvidia-nim-autofix.md") CHANGELOG = Path("CHANGELOG.md") REVIEW_DISPATCH_WORKFLOW = Path(".github/workflows/opencode-review-dispatch.yml") -REVIEW_DISPATCH_BLOB_SHA = "d61439c6271e3c9dbbd1d3f22d5104dbd7297947" +REVIEW_DISPATCH_BLOB_SHA = "849c35d1487281e776040a1746d02238b7610338" def _workflow_text(path: Path) -> str: From e27eb23776b46c5b3e7d2f2b3697ef556a850c55 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 13:38:22 -0700 Subject: [PATCH 40/48] test(opencode): expose redundant overview writes --- ...pencode_coverage_publication_regression.py | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/test_opencode_coverage_publication_regression.py b/tests/test_opencode_coverage_publication_regression.py index 48a703a4fe..57e31a1201 100644 --- a/tests/test_opencode_coverage_publication_regression.py +++ b/tests/test_opencode_coverage_publication_regression.py @@ -3,6 +3,7 @@ from __future__ import annotations import inspect +import re from pathlib import Path from scripts.ci import opencode_coverage_identity as identity @@ -108,3 +109,30 @@ def test_outcome_publisher_does_not_query_status_comment_for_control_sentinel() assert "sentinel_comment_error_file" not in outcome assert "Review Overview sentinel comment" not in outcome assert 'load_selected_review_output "$selected_review_output_file"' in outcome + + +def test_overview_callers_pass_only_the_published_result() -> None: + """Removed body parameters cannot imply that ignored text reaches the overview.""" + workflow = WORKFLOW.read_text(encoding="utf-8") + + assert not re.search( + r'^\s*update_review_overview\s+"[^"]+"\s+.+$', + workflow, + flags=re.MULTILINE, + ) + + +def test_coverage_blocked_approve_restores_overview_once() -> None: + """Fallback publication and its caller cannot duplicate the coverage overview write.""" + workflow = WORKFLOW.read_text(encoding="utf-8") + approval = workflow.split(" APPROVE)", 1)[1].split( + " if request_changes_for_merge_conflict_if_present", 1 + )[0] + + assert "coverage_overview_restored=0" in approval + assert approval.count("coverage_overview_restored=1") == 2 + assert ( + 'if [ "$coverage_overview_restored" -ne 1 ]; then\n' + " request_changes_for_coverage_evidence_failure\n" + " fi" + ) in approval From c4515328ba7726b2ca98bc2ffa978856c8d8588b Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 13:41:13 -0700 Subject: [PATCH 41/48] fix(opencode): publish coverage overview once --- .github/workflows/opencode-review-dispatch.yml | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/.github/workflows/opencode-review-dispatch.yml b/.github/workflows/opencode-review-dispatch.yml index 849c35d148..900b6b8e9f 100644 --- a/.github/workflows/opencode-review-dispatch.yml +++ b/.github/workflows/opencode-review-dispatch.yml @@ -5528,7 +5528,7 @@ jobs: printf '::notice::OpenCode review publication stopped because PR head advanced beyond %s; current-head run remains authoritative.\n' "$HEAD_SHA" return 0 fi - update_review_overview "$event" "$body" || true + update_review_overview "$event" || true if [ "$event" = "APPROVE" ]; then if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then { @@ -5875,9 +5875,9 @@ jobs: return 1 fi if [ -s "$fallback_body_file" ]; then - update_review_overview "INLINE_COMMENT_PUBLISH_FAILED" "$(cat "$fallback_body_file")" + update_review_overview "INLINE_COMMENT_PUBLISH_FAILED" else - update_review_overview "INLINE_COMMENT_PUBLISH_FAILED" "$body" + update_review_overview "INLINE_COMMENT_PUBLISH_FAILED" fi return 1 fi @@ -7768,6 +7768,7 @@ jobs: case "$gate_result" in APPROVE) if [ "${COVERAGE_EVIDENCE_RESULT:-skipped}" != "success" ]; then + coverage_overview_restored=0 if [ -s "$tmp_body" ]; then model_prose_file="$(mktemp)" python3 scripts/ci/opencode_review_surfaces.py extract-prose \ @@ -7776,12 +7777,16 @@ jobs: create_pull_review "COMMENT" "$(cat "$model_prose_file")" else publish_fallback_diff_review + coverage_overview_restored=1 fi rm -f "$model_prose_file" else publish_fallback_diff_review + coverage_overview_restored=1 + fi + if [ "$coverage_overview_restored" -ne 1 ]; then + request_changes_for_coverage_evidence_failure fi - request_changes_for_coverage_evidence_failure echo "::endgroup::" exit 1 fi From fbd0e6ffdc9fa47882cef3255919b5baa76a4dc9 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 13:41:27 -0700 Subject: [PATCH 42/48] test(opencode): repin reviewed dispatch workflow --- tests/test_pr_review_autofix_nvidia_nim_contract.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index 9f6e7238b1..95d96e5063 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -19,7 +19,7 @@ DOCTORING_RECORD = Path("docs/doctoring/hourly-nvidia-nim-autofix.md") CHANGELOG = Path("CHANGELOG.md") REVIEW_DISPATCH_WORKFLOW = Path(".github/workflows/opencode-review-dispatch.yml") -REVIEW_DISPATCH_BLOB_SHA = "849c35d1487281e776040a1746d02238b7610338" +REVIEW_DISPATCH_BLOB_SHA = "900b6b8e9fc0812995ca2dbc5163de33edcfd027" def _workflow_text(path: Path) -> str: From ef60df39dea2b369b08a92e22528e7bdf2390ca0 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 13:45:58 -0700 Subject: [PATCH 43/48] test(opencode): distinguish shell fallback operator --- .../test_opencode_coverage_publication_regression.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/tests/test_opencode_coverage_publication_regression.py b/tests/test_opencode_coverage_publication_regression.py index 57e31a1201..0e6c8c95a2 100644 --- a/tests/test_opencode_coverage_publication_regression.py +++ b/tests/test_opencode_coverage_publication_regression.py @@ -3,7 +3,6 @@ from __future__ import annotations import inspect -import re from pathlib import Path from scripts.ci import opencode_coverage_identity as identity @@ -115,11 +114,13 @@ def test_overview_callers_pass_only_the_published_result() -> None: """Removed body parameters cannot imply that ignored text reaches the overview.""" workflow = WORKFLOW.read_text(encoding="utf-8") - assert not re.search( - r'^\s*update_review_overview\s+"[^"]+"\s+.+$', - workflow, - flags=re.MULTILINE, + forbidden = ( + 'update_review_overview "$event" "$body"', + 'update_review_overview "INLINE_COMMENT_PUBLISH_FAILED" "$(cat "$fallback_body_file")"', + 'update_review_overview "INLINE_COMMENT_PUBLISH_FAILED" "$body"', ) + for fragment in forbidden: + assert fragment not in workflow def test_coverage_blocked_approve_restores_overview_once() -> None: From d229fc6ba96ef15b39ffe5f3d37b2a3a5d6e961e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 13:51:22 -0700 Subject: [PATCH 44/48] test(opencode): align overview helper contract --- scripts/ci/test_strix_quick_gate.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index c820e53500..2e42c4453f 100644 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -1250,7 +1250,8 @@ assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" ' assert_file_contains "$workflow_file" 'approving based on source-backed OpenCode result and successful coverage evidence while branch protection remains authoritative' "opencode source-backed approval tolerates app-token-limited failed-check lookup" assert_file_contains "$workflow_file" 'opencode-agent[bot]' "opencode review can find overview comments written by the OpenCode app token" assert_file_contains "$workflow_file" 'update_review_overview()' "opencode approval step can rewrite the durable Review Overview after final gate decisions" - assert_file_contains "$workflow_file" 'update_review_overview "$event" "$body"' "opencode approval reviews refresh the durable overview with the actual approval-step event" + assert_file_contains "$workflow_file" 'update_review_overview "$event"' "opencode approval reviews refresh the durable overview with the actual approval-step event" + assert_file_not_contains "$workflow_file" 'update_review_overview "$event" "$body"' "opencode overview callers do not imply ignored body publication" assert_file_contains "$workflow_file" 'env GH_TOKEN="$overview_comment_token"' "opencode approval overview updates use the workflow comment token" assert_file_contains "$workflow_file" 'warn_gh_publication_failure()' "opencode approval reports PR review/comment publication errors" assert_file_contains "$workflow_file" 'OpenCode could not publish %s; the requested GitHub side effect is unavailable.' "opencode approval explains permission-denied publication failures" From 82c5ed29e4c11527dc40fa78ce375703f381c65f Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 15:11:51 -0700 Subject: [PATCH 45/48] test(opencode): expose Rust merge-conflict diagram drift --- tests/test_opencode_review_surfaces.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/test_opencode_review_surfaces.py b/tests/test_opencode_review_surfaces.py index 7a3ccfac78..ee40d8ca69 100644 --- a/tests/test_opencode_review_surfaces.py +++ b/tests/test_opencode_review_surfaces.py @@ -216,6 +216,30 @@ def test_conflict_state_marks_blocked_paths() -> None: assert "Docs: readme.md" in diagram +def test_conflict_state_marks_rust_sequence_path_blocked() -> None: + """DIRTY merge state remains visible on a Rust sequence diagram.""" + diagram = surfaces.emit_mermaid( + ["crates/demo/src/lib.rs"], + merge_state="DIRTY", + ) + assert "sequenceDiagram" in diagram + assert "Merge conflict blocks this path" in diagram + + +def test_conflict_state_marks_rust_class_api_blocked(tmp_path: Path) -> None: + """DIRTY merge state remains visible on a Rust API class diagram.""" + source = tmp_path / "crates/demo/src/lib.rs" + source.parent.mkdir(parents=True) + source.write_text("pub struct Demo;\n", encoding="utf-8") + diagram = surfaces.emit_mermaid( + ["crates/demo/src/lib.rs"], + merge_state="DIRTY", + source_root=tmp_path, + ) + assert "classDiagram" in diagram + assert "Merge conflict blocks this path" in diagram + + def test_cli_renders_originweave_surfaces( tmp_path: Path, capsys: pytest.CaptureFixture[str] ) -> None: From b4919d36f63e835b2503d9d8c1e882bb755b4c6a Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 15:13:45 -0700 Subject: [PATCH 46/48] fix(opencode): retain Rust merge-conflict diagram state --- scripts/ci/opencode_review_surfaces.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/scripts/ci/opencode_review_surfaces.py b/scripts/ci/opencode_review_surfaces.py index 7b1c182871..01a3b70d0b 100644 --- a/scripts/ci/opencode_review_surfaces.py +++ b/scripts/ci/opencode_review_surfaces.py @@ -259,6 +259,8 @@ def emit_mermaid( lines = ["```mermaid", "classDiagram"] for symbol in symbols[:8]: lines.append(f" class {_quote_label(symbol)}") + if merge_state in {"DIRTY", "CONFLICTING"}: + lines.append(' class MergeConflict["Merge conflict blocks this path"]') lines.append("```") return "\n".join(lines) + "\n" if rust_paths: @@ -268,12 +270,18 @@ def emit_mermaid( if len(parts) > 1 and parts[0] == "crates": crate = parts[1] break + conflict_participant = ( + " participant Conflict as Merge conflict blocks this path\n" + if merge_state in {"DIRTY", "CONFLICTING"} + else "" + ) return ( "```mermaid\n" "sequenceDiagram\n" f" participant Caller as Caller\n" f" participant Crate as {_quote_label(crate)}\n" " participant Tests as Crate tests\n" + f"{conflict_participant}" " Caller->>Crate: changed public API\n" " Tests->>Crate: regression coverage\n" "```\n" From 8d7d13708fe9c0dcb8d124c61e7ed63a0e0feb85 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 16:23:04 -0700 Subject: [PATCH 47/48] fix(ci): bind coverage jobs to exact dispatch run --- scripts/ci/opencode_coverage_identity.py | 7 +- tests/test_opencode_coverage_identity.py | 297 ++++++++++++++++++++++- 2 files changed, 301 insertions(+), 3 deletions(-) diff --git a/scripts/ci/opencode_coverage_identity.py b/scripts/ci/opencode_coverage_identity.py index b24bbbc891..92dbde83f5 100644 --- a/scripts/ci/opencode_coverage_identity.py +++ b/scripts/ci/opencode_coverage_identity.py @@ -158,7 +158,12 @@ def terminal_dispatch_coverage_result( raise CoverageQuoteError( f"expected one completed {CANONICAL_CHECK_NAME} job in current run" ) - result = normalize_result(str(matches[0].get("conclusion") or "")) + job = matches[0] + if str(job.get("run_id") or "") != str(run_id): + raise CoverageQuoteError( + f"current-run {CANONICAL_CHECK_NAME} job run id does not match" + ) + result = normalize_result(str(job.get("conclusion") or "")) if result == "unknown": raise CoverageQuoteError( f"current-run {CANONICAL_CHECK_NAME} conclusion is missing or non-terminal" diff --git a/tests/test_opencode_coverage_identity.py b/tests/test_opencode_coverage_identity.py index 2abfcf673d..4fd8cc1a68 100644 --- a/tests/test_opencode_coverage_identity.py +++ b/tests/test_opencode_coverage_identity.py @@ -54,6 +54,14 @@ def test_identity_helpers_cover_malformed_and_noncanonical_checks() -> None: assert identity.check_workflow_name({"check_suite": "bad"}) == "" assert identity.check_workflow_name({"check_suite": {"workflow_run": "bad"}}) == "" assert identity.check_workflow_name({"app": "nope"}) == "" + assert identity.check_run_id({"checkSuite": {"workflowRun": {"databaseId": 123}}}) == "123" + assert identity.check_run_id({"check_suite": {"workflow_run": {"id": "bad"}}}) == "" + assert identity.check_run_id({"check_suite": {"workflow_run": "bad"}}) == "" + assert identity.check_run_id({"check_suite": "bad"}) == "" + assert identity.check_run_id( + {"detailsUrl": "https://github.com/acme/repo/actions/runs/456/job/789"} + ) == "456" + assert identity.check_run_id({"details_url": "https://github.com/acme/repo/checks/1"}) == "" head = identity.KAEFA_78_HEAD with pytest.raises(identity.CoverageQuoteError, match="40-character"): identity.terminal_coverage_result([], "deadbeef") @@ -305,6 +313,37 @@ def fake_mixed_pages(args, **kwargs): ) +def test_fetch_check_runs_exercises_delay_and_empty_retry_policy(monkeypatch) -> None: + """A configured delay is honored, while an empty retry policy fails closed.""" + sleeps: list[float] = [] + monkeypatch.setattr(identity, "RETRY_DELAYS", (0.01,)) + monkeypatch.setattr(identity.time, "sleep", sleeps.append) + monkeypatch.setattr( + identity.subprocess, + "run", + lambda args, **kwargs: type( + "Completed", + (), + { + "returncode": 0, + "stdout": json.dumps({"check_runs": []}), + "stderr": "", + }, + )(), + ) + + assert identity.fetch_check_runs( + "ContextualWisdomLab/kaefa", identity.KAEFA_78_HEAD + ) == [] + assert sleeps == [0.01] + + monkeypatch.setattr(identity, "RETRY_DELAYS", ()) + with pytest.raises(identity.CoverageQuoteError, match="after retries"): + identity.fetch_check_runs( + "ContextualWisdomLab/kaefa", identity.KAEFA_78_HEAD + ) + + def dispatch_run( *, run_id: str, @@ -326,10 +365,19 @@ def dispatch_run( def coverage_job( - *, conclusion: str = "success", name: str = "coverage-evidence", status: str = "completed" + *, + conclusion: str = "success", + name: str = "coverage-evidence", + status: str = "completed", + run_id: str = "33112315024", ) -> dict[str, object]: """Build one Actions job from the current workflow run.""" - return {"name": name, "status": status, "conclusion": conclusion} + return { + "name": name, + "status": status, + "conclusion": conclusion, + "run_id": int(run_id), + } def test_repository_dispatch_coverage_binds_to_current_central_run_job() -> None: @@ -355,6 +403,101 @@ def test_repository_dispatch_coverage_binds_to_current_central_run_job() -> None ) == "failure" +def test_repository_dispatch_coverage_rejects_job_from_another_run() -> None: + """A same-named completed job from a different run is never authoritative.""" + run_id = "33112315024" + target_repo = "ContextualWisdomLab/scopeweave" + run = dispatch_run( + run_id=run_id, + target_repo=target_repo, + pr_number=523, + head_sha=identity.KAEFA_78_HEAD, + ) + + with pytest.raises(identity.CoverageQuoteError, match="job run id"): + identity.terminal_dispatch_coverage_result( + run, + [coverage_job(run_id="33112315023")], + workflow_repo="ContextualWisdomLab/.github", + target_repo=target_repo, + pr_number="523", + head_sha=identity.KAEFA_78_HEAD, + run_id=run_id, + ) + + +@pytest.mark.parametrize( + ("overrides", "run_mutation", "match"), + ( + ({"workflow_repo": "../bad"}, {}, "workflow repository"), + ({"target_repo": "../bad"}, {}, "target repository"), + ({"pr_number": "not-a-number"}, {}, "pull request number"), + ({"pr_number": "0"}, {}, "pull request number"), + ({"head_sha": "deadbeef"}, {}, "40-character"), + ({"run_id": "not-a-run"}, {}, "numeric workflow run id"), + ({}, {"id": 33112315023}, "run id"), + ({}, {"repository": "malformed"}, "repository"), + ), +) +def test_repository_dispatch_coverage_rejects_invalid_boundaries( + overrides: dict[str, str], run_mutation: dict[str, object], match: str +) -> None: + """Every externally supplied dispatch identity component is fail-closed.""" + run_id = "33112315024" + target_repo = "ContextualWisdomLab/scopeweave" + run = dispatch_run( + run_id=run_id, + target_repo=target_repo, + pr_number=523, + head_sha=identity.KAEFA_78_HEAD, + ) + run.update(run_mutation) + arguments = { + "workflow_repo": "ContextualWisdomLab/.github", + "target_repo": target_repo, + "pr_number": "523", + "head_sha": identity.KAEFA_78_HEAD, + "run_id": run_id, + } + arguments.update(overrides) + + with pytest.raises(identity.CoverageQuoteError, match=match): + identity.terminal_dispatch_coverage_result(run, [coverage_job()], **arguments) + + +def test_repository_dispatch_coverage_rejects_nonterminal_conclusion() -> None: + """A completed job without a terminal conclusion remains non-passing.""" + run_id = "33112315024" + target_repo = "ContextualWisdomLab/scopeweave" + run = dispatch_run( + run_id=run_id, + target_repo=target_repo, + pr_number=523, + head_sha=identity.KAEFA_78_HEAD, + ) + + with pytest.raises(identity.CoverageQuoteError, match="non-terminal"): + identity.terminal_dispatch_coverage_result( + run, + [coverage_job(conclusion="")], + workflow_repo="ContextualWisdomLab/.github", + target_repo=target_repo, + pr_number="523", + head_sha=identity.KAEFA_78_HEAD, + run_id=run_id, + ) + + +def test_terminal_coverage_rejects_malformed_run_id() -> None: + """Optional check-run binding accepts only a numeric Actions run id.""" + with pytest.raises(identity.CoverageQuoteError, match="numeric workflow run id"): + identity.terminal_coverage_result( + [coverage_check(head=identity.KAEFA_78_HEAD)], + identity.KAEFA_78_HEAD, + run_id="bad", + ) + + @pytest.mark.parametrize( ("mutation", "match"), ( @@ -422,3 +565,153 @@ def test_repository_dispatch_coverage_requires_one_completed_exact_job( head_sha=identity.KAEFA_78_HEAD, run_id=run_id, ) + + +def test_run_gh_json_retries_transient_errors_and_honors_delay(monkeypatch) -> None: + """Central workflow reads retry only authenticated transient failures.""" + responses = [ + type( + "Completed", + (), + {"returncode": 1, "stdout": "", "stderr": "HTTP 503 unavailable"}, + )(), + type( + "Completed", + (), + {"returncode": 0, "stdout": json.dumps({"id": 123}), "stderr": ""}, + )(), + ] + sleeps: list[float] = [] + monkeypatch.setattr(identity, "RETRY_DELAYS", (0, 0.01)) + monkeypatch.setattr(identity.time, "sleep", sleeps.append) + monkeypatch.setattr( + identity.subprocess, "run", lambda args, **kwargs: responses.pop(0) + ) + + assert identity._run_gh_json(["gh", "api", "example"]) == {"id": 123} + assert sleeps == [0.01] + + +def test_run_gh_json_fails_closed_on_terminal_and_exhausted_reads(monkeypatch) -> None: + """Non-transient failures and an unavailable retry policy never fabricate JSON.""" + monkeypatch.setattr(identity, "RETRY_DELAYS", (0,)) + monkeypatch.setattr( + identity.subprocess, + "run", + lambda args, **kwargs: type( + "Completed", (), {"returncode": 1, "stdout": "", "stderr": "denied"} + )(), + ) + with pytest.raises(identity.CoverageQuoteError, match="workflow lookup failed"): + identity._run_gh_json(["gh", "api", "example"]) + + monkeypatch.setattr(identity, "RETRY_DELAYS", ()) + with pytest.raises(identity.CoverageQuoteError, match="after retries"): + identity._run_gh_json(["gh", "api", "example"]) + + +def test_fetch_dispatch_workflow_run_validates_identity_and_shape(monkeypatch) -> None: + """The exact workflow-run reader validates inputs and the returned object.""" + calls: list[list[str]] = [] + monkeypatch.setattr( + identity, + "_run_gh_json", + lambda args: calls.append(args) or {"id": 33112315024}, + ) + assert identity.fetch_dispatch_workflow_run( + "ContextualWisdomLab/.github", "33112315024" + )["id"] == 33112315024 + assert calls == [ + [ + "gh", + "api", + "repos/ContextualWisdomLab/.github/actions/runs/33112315024", + ] + ] + + with pytest.raises(identity.CoverageQuoteError, match="workflow repository"): + identity.fetch_dispatch_workflow_run("../bad", "33112315024") + with pytest.raises(identity.CoverageQuoteError, match="numeric workflow run id"): + identity.fetch_dispatch_workflow_run("ContextualWisdomLab/.github", "bad") + monkeypatch.setattr(identity, "_run_gh_json", lambda args: []) + with pytest.raises(identity.CoverageQuoteError, match="malformed JSON"): + identity.fetch_dispatch_workflow_run( + "ContextualWisdomLab/.github", "33112315024" + ) + + +def test_fetch_dispatch_workflow_jobs_validates_pages_and_filters(monkeypatch) -> None: + """Latest-attempt job pages are validated and non-object entries ignored.""" + monkeypatch.setattr( + identity, + "_run_gh_json", + lambda args: [ + {"jobs": [coverage_job(), "malformed"]}, + {"jobs": [coverage_job(name="opencode-review")]}, + ], + ) + jobs = identity.fetch_dispatch_workflow_jobs( + "ContextualWisdomLab/.github", "33112315024" + ) + assert [job["name"] for job in jobs] == ["coverage-evidence", "opencode-review"] + + monkeypatch.setattr(identity, "_run_gh_json", lambda args: {"jobs": []}) + assert identity.fetch_dispatch_workflow_jobs( + "ContextualWisdomLab/.github", "33112315024" + ) == [] + with pytest.raises(identity.CoverageQuoteError, match="workflow repository"): + identity.fetch_dispatch_workflow_jobs("../bad", "33112315024") + with pytest.raises(identity.CoverageQuoteError, match="numeric workflow run id"): + identity.fetch_dispatch_workflow_jobs("ContextualWisdomLab/.github", "bad") + monkeypatch.setattr(identity, "_run_gh_json", lambda args: [{"jobs": "bad"}]) + with pytest.raises(identity.CoverageQuoteError, match="malformed JSON"): + identity.fetch_dispatch_workflow_jobs( + "ContextualWisdomLab/.github", "33112315024" + ) + + +def test_dispatch_cli_quotes_only_the_exact_current_run( + monkeypatch, capsys +) -> None: + """CLI dispatch mode succeeds only when current-run evidence matches its quote.""" + run_id = "33112315024" + target_repo = "ContextualWisdomLab/scopeweave" + monkeypatch.setattr( + identity, + "fetch_dispatch_workflow_run", + lambda workflow_repo, current_run_id: dispatch_run( + run_id=current_run_id, + target_repo=target_repo, + pr_number=523, + head_sha=identity.KAEFA_78_HEAD, + ), + ) + monkeypatch.setattr( + identity, + "fetch_dispatch_workflow_jobs", + lambda workflow_repo, current_run_id: [coverage_job(run_id=current_run_id)], + ) + args = [ + "--workflow-repo", + "ContextualWisdomLab/.github", + "--repo", + target_repo, + "--pr-number", + "523", + "--head-sha", + identity.KAEFA_78_HEAD, + "--run-id", + run_id, + "--quoted-result", + "success", + ] + + assert identity.main(args) == 0 + assert capsys.readouterr().out.strip() == "success" + args[-1] = "failure" + assert identity.main(args) == 1 + assert "does not match" in capsys.readouterr().err + + missing = [item for item in args if item not in {"--repo", target_repo}] + assert identity.main(missing) == 1 + assert "needs target repo" in capsys.readouterr().err From c5772d8efa237aec163ee89162bc4d74f944faad Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 27 Aug 2026 17:12:24 -0700 Subject: [PATCH 48/48] fix(ci): reject parent workflow anchor aliases --- scripts/ci/opencode_review_surfaces.py | 3 ++- tests/test_opencode_review_surfaces.py | 10 ++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/scripts/ci/opencode_review_surfaces.py b/scripts/ci/opencode_review_surfaces.py index 01a3b70d0b..b314cddf00 100644 --- a/scripts/ci/opencode_review_surfaces.py +++ b/scripts/ci/opencode_review_surfaces.py @@ -19,7 +19,8 @@ CENTRAL_WORKFLOW_ANCHOR = ".github/workflows/opencode-review.yml" CENTRAL_WORKFLOW_ANCHOR_RE = re.compile( - r"\.github[\\/]+workflows(?:[\\/]+\.)*[\\/]+" + r"\.github[\\/]+workflows" + r"(?:(?:[\\/]+\.)|(?:[\\/]+\.\.[\\/]+workflows))*[\\/]+" r"opencode-review\.yml(?::[0-9]+)?" ) PUB_ITEM_RE = re.compile( diff --git a/tests/test_opencode_review_surfaces.py b/tests/test_opencode_review_surfaces.py index ee40d8ca69..7b73dbedb2 100644 --- a/tests/test_opencode_review_surfaces.py +++ b/tests/test_opencode_review_surfaces.py @@ -137,7 +137,12 @@ def test_workflow_anchor_variants_cannot_spoof_unrelated_findings() -> None: review = surfaces.format_request_changes_review( model_prose=( r"Finding: .github\workflows\opencode-review.yml:42 " - "and .github/workflows/./opencode-review.yml:7" + "and .github/workflows/./opencode-review.yml:7 " + "and .github/workflows/../workflows/opencode-review.yml:9" + ), + structured_findings=( + "### 1. HIGH " + ".github/workflows/../workflows/opencode-review.yml:11 - spoof" ), head_sha=HEAD, run_id="1", @@ -146,7 +151,8 @@ def test_workflow_anchor_variants_cannot_spoof_unrelated_findings() -> None: ) assert r".github\workflows\opencode-review.yml" not in review assert ".github/workflows/./opencode-review.yml" not in review - assert review.count("Review process") >= 2 + assert ".github/workflows/../workflows/opencode-review.yml" not in review + assert review.count("Review process") >= 4 def test_korean_status_and_review_keep_identifiers() -> None: """Korean PRs stay Korean while crate paths remain unchanged."""