From d8e88f96157659c23c9635daa2c0f3c6e9544469 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 16 Aug 2026 16:17:21 +0000 Subject: [PATCH 1/3] fix(opencode): keep product-file review when coverage gate fails OriginWeave #47 posted the same coverage-gate body as both the formal review and the issue comment, citing opencode-review.yml:1 while the diff was a Rust crate. Split those surfaces, still review changed product files when coverage-evidence fails, label crates/ as a Rust surface, and install the declared rustup channel plus llvm-tools in the isolated coverage image. Co-authored-by: Seongho Bae --- .../workflows/opencode-review-dispatch.yml | 189 +++++-- CHANGELOG.md | 4 + ci-review-prompt.md | 9 + ...opencode-review-surfaces-originweave-47.md | 92 +++ scripts/ci/materialize_base_rust_toolchain.py | 235 ++++++++ scripts/ci/opencode_review_comment_helpers.sh | 108 +--- scripts/ci/opencode_review_prompt_template.md | 4 +- scripts/ci/opencode_review_surfaces.py | 532 ++++++++++++++++++ scripts/ci/test_strix_quick_gate.sh | 14 +- tests/test_materialize_base_rust_toolchain.py | 323 +++++++++++ tests/test_opencode_agent_contract.py | 24 +- tests/test_opencode_review_comment_helpers.py | 54 ++ tests/test_opencode_review_surfaces.py | 369 ++++++++++++ 13 files changed, 1808 insertions(+), 149 deletions(-) create mode 100644 docs/doctoring/opencode-review-surfaces-originweave-47.md create mode 100644 scripts/ci/materialize_base_rust_toolchain.py create mode 100644 scripts/ci/opencode_review_surfaces.py create mode 100644 tests/test_materialize_base_rust_toolchain.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 83f6830d5..703b63c05 100644 --- a/.github/workflows/opencode-review-dispatch.yml +++ b/.github/workflows/opencode-review-dispatch.yml @@ -633,6 +633,9 @@ jobs: --base-sha "$PR_BASE_SHA" \ --head-sha "$PR_HEAD_SHA" \ --output-dir "$coverage_build_dir/base-javascript-packages" + python3 -I "$GITHUB_WORKSPACE/scripts/ci/materialize_base_rust_toolchain.py" \ + --repo-root "$COVERAGE_SOURCE_WORKDIR" \ + --output-dir "$coverage_build_dir/base-rust" cat >"$coverage_build_dir/Dockerfile" <<'DOCKERFILE' FROM docker.io/library/python:3.14-slim@sha256:b877e50bd90de10af8d82c57a022fc2e0dc731c5320d762a27986facfc3355c1 ENV DEBIAN_FRONTEND=noninteractive @@ -646,6 +649,7 @@ jobs: libcurl4-openssl-dev \ libssl-dev \ libxml2-dev \ + llvm \ mesa-vulkan-drivers \ libvulkan1 \ pkg-config \ @@ -727,6 +731,39 @@ jobs: --requirements-root /tmp/base-python-requirements \ && rm -rf /tmp/base-python-requirements \ && rm -f /usr/local/libexec/install-base-python-locks.py + COPY base-rust /tmp/base-rust + RUN set -eu; \ + if [ ! -f /tmp/base-rust/manifest.json ]; then \ + echo "Rust coverage manifest.json must exist in the trusted build context." >&2; \ + exit 1; \ + fi; \ + channel="$(jq -r '.rustup_channel // empty' /tmp/base-rust/manifest.json)"; \ + if [ -n "$channel" ]; then \ + curl --proto '=https' --tlsv1.2 -fsSLo /tmp/rustup-init \ + https://static.rust-lang.org/rustup/archive/1.28.2/x86_64-unknown-linux-gnu/rustup-init; \ + echo '20a06e644b0d9bd2fbdbfd52d42540bdde820ea7df86e92e533c073da0cdd43c /tmp/rustup-init' | sha256sum -c -; \ + chmod 0755 /tmp/rustup-init; \ + RUSTUP_HOME=/opt/rustup CARGO_HOME=/opt/cargo \ + /tmp/rustup-init -y --no-modify-path --profile minimal --default-toolchain none; \ + RUSTUP_HOME=/opt/rustup CARGO_HOME=/opt/cargo \ + /opt/cargo/bin/rustup toolchain install "$channel" --component llvm-tools-preview --profile minimal; \ + RUSTUP_HOME=/opt/rustup CARGO_HOME=/opt/cargo \ + /opt/cargo/bin/rustup default "$channel"; \ + chmod -R a+rX /opt/rustup /opt/cargo; \ + rm -f /tmp/rustup-init; \ + fi; \ + if [ -f /tmp/base-rust/Cargo.toml ] && [ -f /tmp/base-rust/Cargo.lock ]; then \ + mkdir -p /opt/cargo; \ + if [ -x /opt/cargo/bin/cargo ]; then \ + RUSTUP_HOME=/opt/rustup CARGO_HOME=/opt/cargo \ + /opt/cargo/bin/cargo fetch --locked --manifest-path /tmp/base-rust/Cargo.toml; \ + else \ + CARGO_HOME=/opt/cargo \ + cargo fetch --locked --manifest-path /tmp/base-rust/Cargo.toml; \ + fi; \ + chmod -R a+rX /opt/cargo; \ + fi; \ + rm -rf /tmp/base-rust DOCKERFILE if ! docker build --pull --no-cache --network=default \ --tag "$coverage_tool_image" \ @@ -881,7 +918,9 @@ jobs: HOME=/work/.opencode-sandbox-home \ XDG_CACHE_HOME=/work/.opencode-sandbox-cache \ CARGO_HOME=/work/.opencode-sandbox-home/.cargo \ - PATH="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" \ + RUSTUP_HOME=/opt/rustup \ + CARGO_NET_OFFLINE=true \ + PATH="/opt/cargo/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" \ "$@" >"$log_file" 2>&1 local rc=$? set -e @@ -1708,7 +1747,23 @@ jobs: fi } + prepare_writable_cargo_home() { + if [ -d /opt/cargo ] && [ ! -L /opt/cargo ]; then + mkdir -p /work/.opencode-sandbox-home/.cargo + cp -R /opt/cargo/. /work/.opencode-sandbox-home/.cargo/ + fi + mkdir -p /work/.opencode-sandbox-home/.cargo + chown -R --no-dereference \ + "$OPENCODE_SANDBOX_UID:$OPENCODE_SANDBOX_GID" \ + /work/.opencode-sandbox-home/.cargo + chmod -R u+rwX,go-rwx /work/.opencode-sandbox-home/.cargo + } + ensure_rust_toolchain() { + if [ -x /opt/cargo/bin/cargo ]; then + export PATH="/opt/cargo/bin:${PATH}" + export RUSTUP_HOME=/opt/rustup + fi if ! command -v cargo >/dev/null 2>&1; then append "### Rust coverage toolchain" append "" @@ -1729,6 +1784,17 @@ jobs: failures=$((failures + 1)) return 1 fi + if [ ! -x /opt/cargo/bin/cargo ] && [ ! -x /usr/bin/llvm-cov ]; then + append "### Rust coverage toolchain" + append "" + append "- Result: FAIL" + append "- Reason: llvm-tools-preview is unavailable and /usr/bin/llvm-cov is missing, so cargo llvm-cov cannot measure coverage." + append "- Fix: rebuild the trusted coverage image with rustup llvm-tools-preview or the distribution llvm package." + append "" + failures=$((failures + 1)) + return 1 + fi + prepare_writable_cargo_home ensure_rust_gpu_adapter ensure_rust_desktop_deps } @@ -1807,10 +1873,10 @@ jobs: fi if [ "$manifest" = "Cargo.toml" ]; then run_and_capture "Rust coverage with missing-line report (${manifest})" \ - cargo llvm-cov --workspace --all-features --fail-under-lines "$threshold" --show-missing-lines + cargo llvm-cov --offline --locked --workspace --all-features --fail-under-lines "$threshold" --show-missing-lines else run_and_capture "Rust coverage with missing-line report (${manifest})" \ - cargo llvm-cov --manifest-path "$manifest" --all-features --fail-under-lines "$threshold" --show-missing-lines + cargo llvm-cov --offline --locked --manifest-path "$manifest" --all-features --fail-under-lines "$threshold" --show-missing-lines fi done <<<"$manifests" else @@ -2294,7 +2360,7 @@ jobs: - name: Detect central review-process scope id: central_review_process_fallback_scope - if: needs.coverage-evidence.result == 'success' + if: needs.coverage-evidence.result != 'cancelled' env: GH_TOKEN: ${{ secrets.OPENCODE_APPROVE_TOKEN || github.token }} GH_REPOSITORY: ${{ needs.validate-pr-metadata.outputs.target_repository }} @@ -4275,7 +4341,7 @@ jobs: - name: Run OpenCode PR Review model pool id: opencode_review_model_pool - if: needs.coverage-evidence.result == 'success' + if: needs.coverage-evidence.result != 'cancelled' timeout-minutes: 205 continue-on-error: true env: @@ -4495,6 +4561,8 @@ jobs: OPENCODE_SOURCE_WORKDIR: ${{ runner.temp }}/opencode-pr-head PR_BASE_SHA: ${{ needs.validate-pr-metadata.outputs.base_sha }} PR_HEAD_SHA: ${{ needs.validate-pr-metadata.outputs.head_sha }} + 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.' }} run: | set -euo pipefail @@ -4566,14 +4634,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 + 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}" \ + --coverage-summary "${COVERAGE_EVIDENCE_SUMMARY:-}" \ + --control-block "$(cat "$comment_body_file")" append_merge_conflict_guidance } >"$overview_body_file" @@ -5222,16 +5290,13 @@ 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 + 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}" \ + --coverage-summary "${COVERAGE_EVIDENCE_SUMMARY:-}" append_merge_conflict_guidance } >"$overview_body_file" @@ -5542,7 +5607,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." \ @@ -5571,7 +5636,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." \ @@ -5587,40 +5652,41 @@ 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")" + 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() { @@ -5666,7 +5732,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." \ @@ -7458,6 +7524,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 @@ -7545,7 +7616,10 @@ jobs: case "$gate_result" in APPROVE) if [ "${COVERAGE_EVIDENCE_RESULT:-skipped}" != "success" ]; then + publish_fallback_diff_review request_changes_for_coverage_evidence_failure + echo "::endgroup::" + exit 1 fi if request_changes_for_merge_conflict_if_present; then echo "::endgroup::" @@ -7818,11 +7892,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 1de9130a5..4ae5b51e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ Semantic Versioning where the repository publishes a release. ### Changed +- Split central OpenCode publication into distinct surfaces: the formal pull-request review is a source-backed walkthrough of the actual diff, and the issue comment is gate/status only (head SHA, run id, coverage/check results). Coverage-evidence failure no longer replaces the review or cites `.github/workflows/opencode-review.yml:1` on a product repository that did not change that file. - Require the hourly repair worker to establish an exact-head root cause, enumerate the smallest remediation candidates, and prove writer authority, sealed-path scope, credentials, dependency order, verifiability, and causal effect before editing; infeasible or external blockers leave the tree unchanged while the broader loop continues with another eligible PR or buyer-visible product gap. - Run the bounded Clearfolio PR review-feedback repair caller at minute 23 of every hour while keeping the shared scheduler free of product-specific timers and repository names for modular reuse by naruon, contextual-orchestrator, Inkspan, and other CWL services. - Run the bounded DiskSage repair heartbeat at minute 37 of every hour, dispatch no more than one exact-head repair, and wait two hours before redispatching an unchanged head so legitimate OpenCode or NVIDIA NIM latency does not create duplicate writers. @@ -26,6 +27,8 @@ Semantic Versioning where the repository publishes a release. ### Fixed +- Stopped the OriginWeave-style coverage sandbox false blocker: the isolated image now installs the repository rustup channel plus `llvm-tools-preview`, prefetches `Cargo.lock` crates, and runs `cargo llvm-cov --offline --locked` instead of Debian rustc 1.85 without llvm-tools (ContextualWisdomLab/OriginWeave#47). +- Taught the OpenCode mermaid helper `crates/`, `src/`, and Rust/Python/TypeScript layouts so a three-file crate change is labeled as that crate, not `Changed file (3 files)`. - Materialized base Python locks only when every package line is an exact SHA-256 pin or a bounded relative `-r`/`--requirement` include. A lone `--require-hashes` directive, a dotted include such as `./lock.txt`, or `-r other-hashes.txt` no longer enters the trusted build context. - Refused a conflict-scope repository root whose immediate parent is a symbolic link, so a swapped parent cannot redirect the canonical worktree after the last-component check (CWE-367). - Bounded the Strix quality self-test's deterministic timeout fixtures to 3-second process and 5-second fake-sleep budgets so exact-head policy evidence completes inside the existing job limit without changing production Strix scanner timeouts, providers, credentials, or review semantics. @@ -55,6 +58,7 @@ Semantic Versioning where the repository publishes a release. ### Documentation +- Added an APA 7 doctoring record for the OriginWeave #47 OpenCode publication failure: coverage-as-review, duplicate formal-review/issue-comment bodies, generic mermaid inventory, and the rustc 1.85 / missing llvm-tools sandbox RCA. - Added an APA 7 doctoring record for conflict-control evidence isolation, including the Strix-reported trust-boundary failure, test-first remediation, canonical-path rule, operator contract, rollback, MITRE CWE-22, and current GitHub Actions secure-use guidance. - Added operator and APA 7 doctoring records for the hourly cadence, immutable source identity, NVIDIA NIM provider and secret boundary, high-reasoning Mistral Small 4 writer, model-process credential isolation, modular MSA ownership, product-specific caller activation, verification contract, and rollback. - Added DiskSage operational documentation for the hourly RCA loop, bounded retry cadence, permission model, standalone and MSA reuse, verification, rollback, and APA 7 references. diff --git a/ci-review-prompt.md b/ci-review-prompt.md index ad4c54ba4..2d6ade247 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 000000000..9831cc898 --- /dev/null +++ b/docs/doctoring/opencode-review-surfaces-originweave-47.md @@ -0,0 +1,92 @@ +# 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. The coverage sandbox +false blocker was Debian rustc 1.85 plus cargo-llvm-cov 0.8.7 without +`llvm-tools-preview`, in a `--network=none` image, against a workspace that +declares `rust-version = "1.97"` and `edition = "2024"`. The job log was +`failed to find llvm-tools-preview`. The default 100% line threshold was not +the false blocker: OriginWeave also requires 100% in repository CI. + +## 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, coverage/check + results, and the hidden approve-gate control block. It must not repeat + `## Pull request overview` or `## Findings`. +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. The trusted coverage image materializes bounded `Cargo.toml` / + `Cargo.lock` / `rust-toolchain.toml` / workspace member manifests, installs + the declared rustup channel with `llvm-tools-preview` when it is newer than + Debian rustc 1.85, prefetches the lockfile, and runs + `cargo llvm-cov --offline --locked`. A real coverage miss still fails. + +Read-only review-agent permissions, NVIDIA NIM-first routing +(`NVIDIA_NIM_API_KEY` bound into `NVIDIA_API_KEY`), OpenCode CLI 1.17.13, and +the existing review-bot identity are unchanged. `COPILOT_GITHUB_TOKEN` is not +introduced. + +## 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`); + and +7. bounded Rust toolchain materialization copies manifests only, selects + rustup 1.97 for OriginWeave-style workspaces, and rejects parent-directory + members and symlinks. + +## Limitations + +A later rustup or llvm-tools catalog change can still fail the image build. +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 + +Rust Project Developers. (2026). *The rustup book*. Rust Project. +https://rust-lang.github.io/rustup/ + +Taiki Endo. (2026). *cargo-llvm-cov*. GitHub. +https://github.com/taiki-e/cargo-llvm-cov diff --git a/scripts/ci/materialize_base_rust_toolchain.py b/scripts/ci/materialize_base_rust_toolchain.py new file mode 100644 index 000000000..4a7d6a0fd --- /dev/null +++ b/scripts/ci/materialize_base_rust_toolchain.py @@ -0,0 +1,235 @@ +#!/usr/bin/env python3 +"""Copy bounded Rust workspace inputs into the trusted coverage image context. + +The isolated coverage sandbox is networkless and previously used Debian rustc +1.85 without ``llvm-tools-preview``. OriginWeave-style workspaces declare +``rust-version = "1.97"`` and ``edition = "2024"``, so the image must install +the repository toolchain plus llvm-tools and prefetch ``Cargo.lock`` crates +before the sandbox starts. +""" + +from __future__ import annotations + +import argparse +import json +import re +import shutil +import subprocess +import sys +from pathlib import Path, PurePosixPath +from typing import Any + +try: + import tomllib +except ModuleNotFoundError: # pragma: no cover - exercised by Python 3.10 CI. + import tomli as tomllib + +DEBIAN_RUSTC = (1, 85, 0) +CHANNEL_RE = re.compile(r"^[A-Za-z0-9._+-]+$") +VERSION_RE = re.compile(r"^(\d+)\.(\d+)(?:\.(\d+))?$") +RUST_INPUT_NAMES = ("rust-toolchain.toml", "rust-toolchain", "Cargo.toml", "Cargo.lock") + + +def _git(repo_root: Path, *args: str) -> bytes: + """Run one read-only git command in the materialized merge tree.""" + completed = subprocess.run( + ["git", "-C", str(repo_root), *args], + check=False, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + if completed.returncode != 0: + stderr = completed.stderr.decode("utf-8", errors="replace").strip() + raise RuntimeError(f"git {args[0]} failed: {stderr}") + return completed.stdout + + +def parse_rust_version(value: str) -> tuple[int, int, int] | None: + """Parse a rust-version or toolchain channel into a comparable triple.""" + match = VERSION_RE.fullmatch(value.strip()) + if match is None: + return None + return (int(match.group(1)), int(match.group(2)), int(match.group(3) or 0)) + + +def _nested(document: dict[str, Any], path: str) -> Any: + """Return a dotted TOML value, or None when any segment is absent.""" + value: Any = document + for segment in path.split("."): + if not isinstance(value, dict) or segment not in value: + return None + value = value[segment] + return value + + +def read_toml(path: Path) -> dict[str, Any]: + """Load one TOML document as a mapping.""" + document = tomllib.loads(path.read_text(encoding="utf-8")) + if not isinstance(document, dict): + raise ValueError(f"{path} must contain a TOML table") + return document + + +def toolchain_channel(repo_root: Path) -> str | None: + """Return the rustup channel declared by rust-toolchain files, if any.""" + toml_path = repo_root / "rust-toolchain.toml" + if toml_path.is_file() and not toml_path.is_symlink(): + channel = _nested(read_toml(toml_path), "toolchain.channel") + if isinstance(channel, str) and CHANNEL_RE.fullmatch(channel): + return channel + legacy = repo_root / "rust-toolchain" + if legacy.is_file() and not legacy.is_symlink(): + channel = legacy.read_text(encoding="utf-8").strip().splitlines()[0].strip() + if CHANNEL_RE.fullmatch(channel): + return channel + return None + + +def declared_rust_version(repo_root: Path) -> str | None: + """Return package or workspace rust-version from the root Cargo.toml.""" + manifest = repo_root / "Cargo.toml" + if not manifest.is_file() or manifest.is_symlink(): + return None + document = read_toml(manifest) + for path in ("package.rust-version", "workspace.package.rust-version"): + value = _nested(document, path) + if isinstance(value, str) and value.strip(): + return value.strip() + return None + + +def rustup_channel(repo_root: Path) -> str | None: + """Choose the rustup toolchain the coverage image must install.""" + channel = toolchain_channel(repo_root) + if channel is not None: + return channel + rust_version = declared_rust_version(repo_root) + if rust_version is None: + return None + parsed = parse_rust_version(rust_version) + if parsed is None or parsed <= DEBIAN_RUSTC: + return None + return rust_version + + +def _bounded_member_path(member: str) -> PurePosixPath: + """Reject absolute or parent-directory workspace member paths.""" + relative = PurePosixPath(member) + if relative.is_absolute() or ".." in relative.parts: + raise ValueError(f"workspace member is not a bounded path: {member}") + return relative + + +def expand_workspace_member(repo_root: Path, member: str) -> list[str]: + """Expand one workspace member or a single trailing ``dir/*`` glob.""" + if any(marker in member for marker in ("?", "[", "**")): + raise ValueError(f"unsupported workspace member glob: {member}") + if member.endswith("/*"): + parent = _bounded_member_path(member[:-2]) + directory = repo_root / parent + if directory.is_symlink() or not directory.is_dir(): + return [] + paths: list[str] = [] + for child in sorted(directory.iterdir()): + if child.is_symlink() or not child.is_dir(): + continue + manifest = child / "Cargo.toml" + if manifest.is_file() and not manifest.is_symlink(): + paths.append(f"{parent.as_posix()}/{child.name}/Cargo.toml") + return paths + if "*" in member: + raise ValueError(f"unsupported workspace member glob: {member}") + relative = _bounded_member_path(member) + member_manifest = f"{relative.as_posix()}/Cargo.toml" + candidate = repo_root / member_manifest + if candidate.is_file() and not candidate.is_symlink(): + return [member_manifest] + return [] + + +def workspace_member_manifests(repo_root: Path) -> list[str]: + """Return bounded workspace member Cargo.toml paths from the root manifest.""" + manifest = repo_root / "Cargo.toml" + if not manifest.is_file() or manifest.is_symlink(): + return [] + members = _nested(read_toml(manifest), "workspace.members") + if not isinstance(members, list): + return [] + paths: list[str] = [] + for member in members: + if not isinstance(member, str): + continue + paths.extend(expand_workspace_member(repo_root, member)) + return list(dict.fromkeys(paths)) + + +def tracked_paths(repo_root: Path) -> set[str]: + """Return tracked repository paths from the materialized merge tree.""" + listed = _git(repo_root, "ls-files", "-z").split(b"\0") + paths: set[str] = set() + for raw in listed: + if not raw: + continue + path = raw.decode("utf-8", errors="surrogateescape") + candidate = PurePosixPath(path) + if not candidate.is_absolute() and ".." not in candidate.parts: + paths.add(path) + return paths + + +def tracked_rust_inputs(repo_root: Path) -> list[str]: + """List root and workspace Rust manifests that may enter the image context.""" + if not (repo_root / "Cargo.toml").is_file(): + return [] + tracked = tracked_paths(repo_root) + paths = [name for name in RUST_INPUT_NAMES if name in tracked] + paths.extend(member for member in workspace_member_manifests(repo_root) if member in tracked) + return list(dict.fromkeys(paths)) + + +def copy_bounded_file(repo_root: Path, relative: str, output_dir: Path) -> None: + """Copy one regular, non-symlink repository file into the build context.""" + source = repo_root / relative + if source.is_symlink() or not source.is_file(): + raise ValueError(f"refusing to materialize non-regular Rust input: {relative}") + destination = output_dir / relative + destination.parent.mkdir(parents=True, exist_ok=True) + shutil.copyfile(source, destination, follow_symlinks=False) + destination.chmod(0o444) + + +def materialize(repo_root: Path, output_dir: Path) -> dict[str, Any]: + """Write bounded Rust toolchain inputs and a machine-readable manifest.""" + repo_root = repo_root.resolve() + output_dir.mkdir(parents=True, exist_ok=True) + inputs = tracked_rust_inputs(repo_root) + for relative in inputs: + copy_bounded_file(repo_root, relative, output_dir) + payload = { + "rustup_channel": rustup_channel(repo_root) if inputs else None, + "has_lock": "Cargo.lock" in inputs, + "has_manifest": "Cargo.toml" in inputs, + "inputs": inputs, + } + manifest = output_dir / "manifest.json" + manifest.write_text(json.dumps(payload, indent=2) + "\n", encoding="utf-8") + manifest.chmod(0o444) + return payload + + +def main(argv: list[str] | None = None) -> int: + """Copy Rust coverage inputs from the merge tree into the image build context.""" + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--repo-root", type=Path, required=True) + parser.add_argument("--output-dir", type=Path, required=True) + args = parser.parse_args(argv) + try: + payload = materialize(args.repo_root, args.output_dir) + except (OSError, RuntimeError, ValueError, tomllib.TOMLDecodeError) as exc: + parser.error(str(exc)) + print(json.dumps(payload, sort_keys=True)) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/ci/opencode_review_comment_helpers.sh b/scripts/ci/opencode_review_comment_helpers.sh index 52bf189c6..87a8a6da6 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 32614dcfc..f6b143e88 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 000000000..c1643fb1a --- /dev/null +++ b/scripts/ci/opencode_review_surfaces.py @@ -0,0 +1,532 @@ +#!/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 re +import sys +from collections import OrderedDict +from pathlib import Path, PurePosixPath +from typing import Sequence + +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 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 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 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") + 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")] + if symbols: + lines = ["```mermaid", "classDiagram"] + for symbol in symbols[:8]: + lines.append(f" class {_quote_label(symbol)}") + if len(symbols) >= 2: + lines.append(f" {_quote_label(symbols[0])} --> {_quote_label(symbols[1])}") + 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" + + +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 = "", +) -> 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 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, ""]) + summary = coverage_summary.strip() + if summary: + lines.extend( + [ + "## Coverage evidence", + "", + summary, + "", + ] + ) + if control_block.strip(): + lines.extend([control_block.strip(), ""]) + 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 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="") + + 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) + + 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, + ) + ) + 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/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index 7343c06ac..20f014747 100755 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -746,7 +746,7 @@ assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" ' assert_file_contains "$workflow_file" 'OPENCODE_POOL_MAX_CYCLES: "1"' "opencode model pool exhausts each candidate only once before bounded fallback" assert_file_not_contains "$workflow_file" 'opencode-exhausted-retry:' "opencode model exhaustion retries stay owned by the least-privilege central scheduler" assert_file_not_contains "$workflow_file" 'RETRY_DISPATCH_TOKEN' "opencode does not retain a recursive write-token dispatch path" - assert_file_contains "$workflow_file" "needs.coverage-evidence.result == 'success'" "opencode model pool only runs after coverage evidence passed" + assert_file_contains "$workflow_file" "needs.coverage-evidence.result != 'cancelled'" "opencode model pool still runs when coverage evidence failed so the diff can be reviewed" assert_file_contains "$workflow_file" "id: opencode_review_model_pool" "opencode DeepSeek V3 fallback still runs after a primary model timeout or step failure when coverage evidence passed" assert_file_contains "$workflow_file" "always()" "opencode fallback chain uses always() so failed model steps cannot skip every fallback" assert_file_contains "$workflow_file" 'OPENCODE_MODEL_ATTEMPTS: "1"' "opencode fallback tries the catalog promptly instead of spending the entire review on one model" @@ -1021,12 +1021,13 @@ assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" ' assert_file_contains "$merge_scheduler_workflow" 'repos/${GITHUB_REPOSITORY}/commits/${REVIEW_HEAD_SHA}/check-runs?per_page=100' "review-event scheduler reads exact-head OpenCode completion evidence" 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" "needs.coverage-evidence.result == 'success'" "opencode model steps skip when coverage-evidence already failed" + assert_file_contains "$workflow_file" 'request_changes_for_coverage_evidence_failure' "opencode approval records coverage-evidence failure on the status comment without replacing the diff review" + 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 and refuse APPROVE while still publishing a source-backed review of changed product files" "opencode approval turns coverage-evidence blocker states into a status-comment gate" + assert_file_contains "$workflow_file" "needs.coverage-evidence.result == 'success'" "opencode fast approval still requires coverage evidence success" + assert_file_contains "$workflow_file" "publish_fallback_diff_review" "opencode still publishes a source-backed product-file review when coverage-evidence 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" - assert_file_contains "$workflow_file" 'cargo llvm-cov --manifest-path "$manifest"' "opencode coverage evidence runs Rust coverage against nested Cargo packages" + assert_file_contains "$workflow_file" 'cargo llvm-cov --offline --locked --manifest-path "$manifest"' "opencode coverage evidence runs offline locked Rust coverage against nested Cargo packages" assert_file_contains "$workflow_file" "ensure_tauri_frontend_dist()" "opencode coverage evidence prepares local Tauri frontendDist assets before Rust coverage" assert_file_contains "$workflow_file" "Tauri frontendDist build" "opencode coverage evidence labels Tauri frontend build logs before cargo coverage" assert_file_contains "$workflow_file" 'npm run build --workspace "$package_name"' "opencode coverage evidence builds npm workspace Tauri frontends before cargo coverage" @@ -1182,7 +1183,8 @@ assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" ' assert_file_contains "$workflow_file" 'select((.status // "") != "COMPLETED")' "opencode approval treats incomplete check runs as approval blockers" assert_file_contains "$workflow_file" '["PENDING","EXPECTED"]' "opencode approval treats pending status contexts as approval blockers" assert_file_contains "$workflow_file" "" "opencode review publishes a durable Review Overview marker" - assert_file_contains "$workflow_file" "## OpenCode Review Overview" "opencode review publishes a visible Review Overview heading" + assert_file_contains "$workflow_file" "opencode_review_surfaces.py build-status" "opencode review publishes a gate-status comment instead of pasting the formal review body" + assert_file_contains "$REPO_ROOT/scripts/ci/opencode_review_surfaces.py" "## OpenCode Review Status" "opencode status comment uses a distinct heading from the formal review" assert_file_contains "$workflow_file" 'gh api -X PATCH "repos/${GH_REPOSITORY}/issues/comments/${overview_comment_id}"' "opencode review updates an existing Review Overview comment instead of duplicating it" assert_file_contains "$workflow_file" "Exchange OpenCode app token for review writes" "opencode review obtains an app token before publishing review writes" assert_file_contains "$workflow_file" 'OPENCODE_APP_TOKEN_EXCHANGE_TIMEOUT_SECONDS: "20"' "opencode app-token exchange has a bounded network timeout" diff --git a/tests/test_materialize_base_rust_toolchain.py b/tests/test_materialize_base_rust_toolchain.py new file mode 100644 index 000000000..ceb9090ee --- /dev/null +++ b/tests/test_materialize_base_rust_toolchain.py @@ -0,0 +1,323 @@ +"""Tests for bounded Rust toolchain materialization into the coverage image.""" + +from __future__ import annotations + +import json +import runpy +import subprocess +import sys +from pathlib import Path + +import pytest + +from scripts.ci import materialize_base_rust_toolchain as materializer + + +def git(repo: Path, *args: str) -> str: + """Run git in a temporary fixture repository.""" + return subprocess.run( + ["git", "-C", str(repo), *args], + check=True, + capture_output=True, + text=True, + ).stdout.strip() + + +def rust_workspace(tmp_path: Path) -> Path: + """Create an OriginWeave-style virtual workspace with a pinned toolchain.""" + repo = tmp_path / "repo" + repo.mkdir() + git(repo, "init") + git(repo, "config", "user.name", "Test") + git(repo, "config", "user.email", "test@example.invalid") + (repo / "Cargo.toml").write_text( + "[workspace]\n" + 'members = ["crates/originweave-destination", "crates/originweave-core"]\n' + 'resolver = "3"\n\n' + "[workspace.package]\n" + 'edition = "2024"\n' + 'rust-version = "1.97"\n', + encoding="utf-8", + ) + (repo / "Cargo.lock").write_text("# lock\n", encoding="utf-8") + (repo / "rust-toolchain.toml").write_text( + '[toolchain]\nchannel = "1.97.1"\n', + encoding="utf-8", + ) + destination = repo / "crates/originweave-destination" + destination.mkdir(parents=True) + (destination / "Cargo.toml").write_text( + "[package]\nname = \"originweave-destination\"\nversion = \"0.1.0\"\n", + encoding="utf-8", + ) + (destination / "src").mkdir() + (destination / "src/lib.rs").write_text("pub fn ok() {}\n", encoding="utf-8") + core = repo / "crates/originweave-core" + core.mkdir(parents=True) + (core / "Cargo.toml").write_text( + "[package]\nname = \"originweave-core\"\nversion = \"0.1.0\"\n", + encoding="utf-8", + ) + git(repo, "add", ".") + git(repo, "commit", "-m", "workspace") + return repo + + +def test_originweave_workspace_selects_rustup_1_97(tmp_path: Path) -> None: + """A 1.97 rust-toolchain.toml is a rustup install, not Debian rustc 1.85.""" + repo = rust_workspace(tmp_path) + output = tmp_path / "base-rust" + payload = materializer.materialize(repo, output) + assert payload["rustup_channel"] == "1.97.1" + assert payload["has_lock"] is True + assert (output / "rust-toolchain.toml").is_file() + assert (output / "crates/originweave-destination/Cargo.toml").is_file() + assert not (output / "crates/originweave-destination/src/lib.rs").exists() + manifest = json.loads((output / "manifest.json").read_text(encoding="utf-8")) + assert manifest["rustup_channel"] == "1.97.1" + + +def test_rust_version_newer_than_debian_selects_rustup(tmp_path: Path) -> None: + """A rust-version newer than Debian rustc 1.85 selects rustup without a pin file.""" + repo = tmp_path / "repo" + repo.mkdir() + git(repo, "init") + git(repo, "config", "user.name", "Test") + git(repo, "config", "user.email", "test@example.invalid") + (repo / "Cargo.toml").write_text( + "[package]\nname = \"newer\"\nversion = \"0.1.0\"\nrust-version = \"1.97\"\n", + encoding="utf-8", + ) + git(repo, "add", ".") + git(repo, "commit", "-m", "newer") + assert materializer.declared_rust_version(repo) == "1.97" + assert materializer.rustup_channel(repo) == "1.97" + + +def test_old_rust_version_keeps_debian_toolchain(tmp_path: Path) -> None: + """A crate that Debian rustc 1.85 can build does not force rustup.""" + repo = tmp_path / "repo" + repo.mkdir() + git(repo, "init") + git(repo, "config", "user.name", "Test") + git(repo, "config", "user.email", "test@example.invalid") + (repo / "Cargo.toml").write_text( + "[package]\nname = \"legacy\"\nversion = \"0.1.0\"\nrust-version = \"1.80\"\n", + encoding="utf-8", + ) + git(repo, "add", ".") + git(repo, "commit", "-m", "legacy") + assert materializer.rustup_channel(repo) is None + + +def test_legacy_rust_toolchain_file(tmp_path: Path) -> None: + """A one-line rust-toolchain file is accepted as the rustup channel.""" + repo = tmp_path / "repo" + repo.mkdir() + git(repo, "init") + git(repo, "config", "user.name", "Test") + git(repo, "config", "user.email", "test@example.invalid") + (repo / "Cargo.toml").write_text("[package]\nname = \"x\"\nversion = \"0.1.0\"\n", encoding="utf-8") + (repo / "rust-toolchain").write_text("nightly-2026-08-01\n", encoding="utf-8") + git(repo, "add", ".") + git(repo, "commit", "-m", "nightly") + assert materializer.toolchain_channel(repo) == "nightly-2026-08-01" + assert materializer.rustup_channel(repo) == "nightly-2026-08-01" + + +def test_no_cargo_toml_writes_empty_manifest(tmp_path: Path) -> None: + """Python-only trees do not install a Rust toolchain.""" + repo = tmp_path / "repo" + repo.mkdir() + payload = materializer.materialize(repo, tmp_path / "out") + assert payload["rustup_channel"] is None + assert payload["has_manifest"] is False + assert payload["inputs"] == [] + + +def test_rejects_parent_directory_workspace_member(tmp_path: Path) -> None: + """Workspace members cannot escape the repository root.""" + repo = tmp_path / "repo" + repo.mkdir() + (repo / "Cargo.toml").write_text( + "[workspace]\nmembers = [\"../escape\"]\n", + encoding="utf-8", + ) + with pytest.raises(ValueError, match="bounded path"): + materializer.workspace_member_manifests(repo) + + +def test_rejects_symlink_input(tmp_path: Path) -> None: + """Symlinked Cargo inputs cannot enter the trusted image context.""" + repo = rust_workspace(tmp_path) + target = tmp_path / "outside.toml" + target.write_text("[package]\n", encoding="utf-8") + (repo / "Cargo.lock").unlink() + (repo / "Cargo.lock").symlink_to(target) + git(repo, "add", "-A") + git(repo, "commit", "-m", "symlink") + with pytest.raises(ValueError, match="non-regular"): + materializer.materialize(repo, tmp_path / "out") + + +def test_cli_and_entrypoint( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """The workflow CLI prints the manifest and the script entrypoint succeeds.""" + repo = rust_workspace(tmp_path) + output = tmp_path / "cli-out" + assert materializer.main(["--repo-root", str(repo), "--output-dir", str(output)]) == 0 + printed = json.loads(capsys.readouterr().out) + assert printed["rustup_channel"] == "1.97.1" + + monkeypatch.setattr( + sys, + "argv", + [materializer.__file__, "--repo-root", str(repo), "--output-dir", str(tmp_path / "entry")], + ) + with pytest.raises(SystemExit, match="0"): + runpy.run_path(materializer.__file__, run_name="__main__") + + +def test_parse_rust_version_helpers() -> None: + """Version parsing distinguishes Debian rustc from newer rust-version pins.""" + assert materializer.parse_rust_version("1.97") == (1, 97, 0) + assert materializer.parse_rust_version("1.85.0") == (1, 85, 0) + assert materializer.parse_rust_version("nightly") is None + assert materializer.parse_rust_version("1.97") > materializer.DEBIAN_RUSTC + + +def test_workspace_glob_members_copy_crate_manifests(tmp_path: Path) -> None: + """A trailing crates/* glob copies each member Cargo.toml without sources.""" + repo = tmp_path / "repo" + repo.mkdir() + git(repo, "init") + git(repo, "config", "user.name", "Test") + git(repo, "config", "user.email", "test@example.invalid") + (repo / "Cargo.toml").write_text( + '[workspace]\nmembers = ["crates/*"]\n', + encoding="utf-8", + ) + crate = repo / "crates/originweave-destination" + crate.mkdir(parents=True) + (crate / "Cargo.toml").write_text( + "[package]\nname = \"originweave-destination\"\nversion = \"0.1.0\"\n", + encoding="utf-8", + ) + (crate / "src").mkdir() + (crate / "src/lib.rs").write_text("pub fn ok() {}\n", encoding="utf-8") + git(repo, "add", ".") + git(repo, "commit", "-m", "glob") + output = tmp_path / "out" + payload = materializer.materialize(repo, output) + assert "crates/originweave-destination/Cargo.toml" in payload["inputs"] + assert (output / "crates/originweave-destination/Cargo.toml").is_file() + assert not (output / "crates/originweave-destination/src/lib.rs").exists() + + +def test_unsupported_workspace_glob_fails_closed(tmp_path: Path) -> None: + """Recursive or in-segment globs are not trusted image inputs.""" + repo = tmp_path / "repo" + repo.mkdir() + (repo / "Cargo.toml").write_text( + '[workspace]\nmembers = ["crates/**"]\n', + encoding="utf-8", + ) + with pytest.raises(ValueError, match="unsupported workspace member glob"): + materializer.workspace_member_manifests(repo) + (repo / "Cargo.toml").write_text( + '[workspace]\nmembers = ["cr*tes/foo"]\n', + encoding="utf-8", + ) + with pytest.raises(ValueError, match="unsupported workspace member glob"): + materializer.workspace_member_manifests(repo) + + +def test_non_git_repo_with_cargo_toml_fails_closed(tmp_path: Path) -> None: + """Materialization requires a readable git tree for tracked-path evidence.""" + repo = tmp_path / "repo" + repo.mkdir() + (repo / "Cargo.toml").write_text("[package]\nname = \"x\"\nversion = \"0.1.0\"\n", encoding="utf-8") + with pytest.raises(SystemExit): + materializer.main(["--repo-root", str(repo), "--output-dir", str(tmp_path / "out")]) + + +def test_read_toml_requires_a_table(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + """A TOML document that is not a table cannot describe a Cargo workspace.""" + path = tmp_path / "Cargo.toml" + path.write_text("[package]\nname = \"x\"\n", encoding="utf-8") + monkeypatch.setattr(materializer.tomllib, "loads", lambda _text: ["not-a-table"]) + with pytest.raises(ValueError, match="TOML table"): + materializer.read_toml(path) + + +def test_invalid_toml_and_channel(tmp_path: Path) -> None: + """Non-table manifests and unsafe toolchain channels fail closed.""" + repo = tmp_path / "repo" + repo.mkdir() + (repo / "Cargo.toml").write_text("[]\n", encoding="utf-8") + with pytest.raises(materializer.tomllib.TOMLDecodeError): + materializer.read_toml(repo / "Cargo.toml") + (repo / "Cargo.toml").write_text("[workspace]\nmembers = [1]\n", encoding="utf-8") + assert materializer.workspace_member_manifests(repo) == [] + (repo / "rust-toolchain.toml").write_text( + '[toolchain]\nchannel = "../evil"\n', + encoding="utf-8", + ) + assert materializer.toolchain_channel(repo) is None + (repo / "rust-toolchain").write_text("not a channel!\n", encoding="utf-8") + assert materializer.toolchain_channel(repo) is None + + +def test_empty_glob_directory_and_missing_member(tmp_path: Path) -> None: + """Missing glob parents and absent member directories yield no manifests.""" + repo = tmp_path / "repo" + repo.mkdir() + (repo / "Cargo.toml").write_text( + '[workspace]\nmembers = ["crates/*", "missing-crate"]\n', + encoding="utf-8", + ) + assert materializer.workspace_member_manifests(repo) == [] + + +def test_symlink_glob_parent_is_ignored(tmp_path: Path) -> None: + """A symlinked crates/ directory cannot expand workspace members.""" + repo = tmp_path / "repo" + repo.mkdir() + outside = tmp_path / "outside" + outside.mkdir() + (outside / "crate").mkdir() + (outside / "crate/Cargo.toml").write_text("[package]\nname = \"x\"\nversion = \"0.1.0\"\n", encoding="utf-8") + (repo / "crates").symlink_to(outside) + assert materializer.expand_workspace_member(repo, "crates/*") == [] + + +def test_non_numeric_rust_version_does_not_select_rustup(tmp_path: Path) -> None: + """A rust-version channel name is not treated as newer than Debian rustc.""" + repo = tmp_path / "repo" + repo.mkdir() + git(repo, "init") + git(repo, "config", "user.name", "Test") + git(repo, "config", "user.email", "test@example.invalid") + (repo / "Cargo.toml").write_text( + "[package]\nname = \"stable\"\nversion = \"0.1.0\"\nrust-version = \"stable\"\n", + encoding="utf-8", + ) + git(repo, "add", ".") + git(repo, "commit", "-m", "stable") + assert materializer.declared_rust_version(repo) == "stable" + assert materializer.rustup_channel(repo) is None + + +def test_invalid_toml_decode_fails_cli(tmp_path: Path) -> None: + """Corrupt Cargo.toml fails the materializer CLI instead of building an image.""" + repo = tmp_path / "repo" + repo.mkdir() + git(repo, "init") + git(repo, "config", "user.name", "Test") + git(repo, "config", "user.email", "test@example.invalid") + (repo / "Cargo.toml").write_text("this is not toml [[[\n", encoding="utf-8") + git(repo, "add", ".") + git(repo, "commit", "-m", "corrupt") + with pytest.raises(SystemExit): + materializer.main(["--repo-root", str(repo), "--output-dir", str(tmp_path / "out")]) diff --git a/tests/test_opencode_agent_contract.py b/tests/test_opencode_agent_contract.py index 5316b67d9..096b2654a 100644 --- a/tests/test_opencode_agent_contract.py +++ b/tests/test_opencode_agent_contract.py @@ -718,6 +718,11 @@ def test_opencode_target_coverage_materializes_only_after_authorized_dispatch(): in measure_step ) assert "CARGO_HOME=/work/.opencode-sandbox-home/.cargo" in measure_step + assert "materialize_base_rust_toolchain.py" in measure_step + assert "RUSTUP_HOME=/opt/rustup" in measure_step + assert "CARGO_NET_OFFLINE=true" in measure_step + assert 'PATH="/opt/cargo/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"' in measure_step + assert "llvm-tools-preview" in measure_step assert "docker run --rm --init --network=none" in measure_step sandbox_runtime = measure_step.split( " export OPENCODE_SANDBOX_UID=65532", 1 @@ -1197,6 +1202,8 @@ def test_code_reviewer_prompt_preserves_review_only_policy(): ) assert "full-screen blocking layer" in ci_prompt_normalized assert "formerly blank sections receive real data" in ci_prompt_normalized + assert "Coverage is a gate, not the review" in ci_prompt + assert "Never cite `.github/workflows/opencode-review.yml:1`" in ci_prompt assert "deliberate empty states" in ci_prompt assert "demo/visual-QA mode is isolated" in ci_prompt_normalized assert "production API behavior" in ci_prompt @@ -1468,7 +1475,14 @@ 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 "record coverage-evidence blocker states" in workflow + 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 "materialize_base_rust_toolchain.py" in workflow + assert "llvm-tools-preview" in workflow + assert "cargo llvm-cov --offline --locked" in workflow + assert ".github/workflows/opencode-review.yml:1" not in workflow assert re.search( r"Prepare bounded OpenCode review evidence[\s\S]{0,120}timeout-minutes: 12", workflow, @@ -1796,6 +1810,9 @@ def test_workflow_provisions_sandbox_tool_and_reviewer_agent(): ) assert "prefers-reduced-motion: reduce" in prompt_template assert "forced smooth scrolling" in prompt_template + assert "Coverage execution evidence is a separate gate" in prompt_template + assert "Never cite `.github/workflows/opencode-review.yml`" in prompt_template + assert "Never label a crate, package, or language surface as `Changed file (N files)`" in prompt_template def test_opencode_excludes_queue_self_check_from_every_failed_check_path(): @@ -2550,7 +2567,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,240}' + r"publish_fallback_diff_review[\s\S]{0,180}" + 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_opencode_review_comment_helpers.py b/tests/test_opencode_review_comment_helpers.py new file mode 100644 index 000000000..9f0d0ca5c --- /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 000000000..d4b1df05e --- /dev/null +++ b/tests/test_opencode_review_surfaces.py @@ -0,0 +1,369 @@ +"""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 + +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 "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") + + +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 "llvm-tools-preview missing" 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("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_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_publisher_workflow_cannot_replace_review_with_coverage_finding() -> None: + """The #47 publisher shape — coverage REQUEST_CHANGES as the whole review — is gone.""" + workflow = Path(".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 + 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 29e40f1164986e9b77ae2ba7c005fb6484d063d6 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 16 Aug 2026 16:28:26 +0000 Subject: [PATCH 2/3] test(opencode): pin dispatch blob and close surface coverage gaps Update the independent-reviewer workflow hash after the publisher split, and add the remaining OriginWeave-style rustup/mermaid branches so scripts/ci stays at 100% coverage. Co-authored-by: Seongho Bae --- tests/test_materialize_base_rust_toolchain.py | 52 +++++++++++++++++++ tests/test_opencode_review_surfaces.py | 40 ++++++++++++++ ...t_pr_review_autofix_nvidia_nim_contract.py | 2 +- 3 files changed, 93 insertions(+), 1 deletion(-) diff --git a/tests/test_materialize_base_rust_toolchain.py b/tests/test_materialize_base_rust_toolchain.py index ceb9090ee..d9d46207e 100644 --- a/tests/test_materialize_base_rust_toolchain.py +++ b/tests/test_materialize_base_rust_toolchain.py @@ -309,6 +309,58 @@ def test_non_numeric_rust_version_does_not_select_rustup(tmp_path: Path) -> None assert materializer.rustup_channel(repo) is None +def test_symlink_manifest_and_non_list_members(tmp_path: Path) -> None: + """Symlinked manifests and non-list workspace members yield no rustup inputs.""" + repo = tmp_path / "repo" + repo.mkdir() + target = tmp_path / "outside.toml" + target.write_text("[package]\nname = \"x\"\nversion = \"0.1.0\"\n", encoding="utf-8") + (repo / "Cargo.toml").symlink_to(target) + assert materializer.declared_rust_version(repo) is None + assert materializer.workspace_member_manifests(repo) == [] + (repo / "Cargo.toml").unlink() + (repo / "Cargo.toml").write_text("[workspace]\nmembers = \"crates/*\"\n", encoding="utf-8") + assert materializer.workspace_member_manifests(repo) == [] + + +def test_glob_skips_non_crate_children_and_unsafe_git_paths(tmp_path: Path) -> None: + """Glob expansion ignores files, symlinked crates, and unsafe git paths.""" + repo = tmp_path / "repo" + repo.mkdir() + git(repo, "init") + git(repo, "config", "user.name", "Test") + git(repo, "config", "user.email", "test@example.invalid") + crates = repo / "crates" + crates.mkdir() + (crates / "README").write_text("not a crate\n", encoding="utf-8") + linked = tmp_path / "linked-crate" + linked.mkdir() + (linked / "Cargo.toml").write_text("[package]\nname = \"x\"\nversion = \"0.1.0\"\n", encoding="utf-8") + (crates / "linked").symlink_to(linked) + empty = crates / "empty" + empty.mkdir() + (empty / "Cargo.toml").symlink_to(linked / "Cargo.toml") + (repo / "Cargo.toml").write_text('[workspace]\nmembers = ["crates/*"]\n', encoding="utf-8") + git(repo, "add", ".") + git(repo, "commit", "-m", "glob-skips") + assert materializer.expand_workspace_member(repo, "crates/*") == [] + listed = materializer.tracked_paths(repo) + assert all(".." not in path and not path.startswith("/") for path in listed) + + +def test_declared_version_without_manifest_and_unsafe_tracked_paths( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """Missing manifests and git paths with traversal do not enter the image.""" + assert materializer.declared_rust_version(tmp_path) is None + monkeypatch.setattr( + materializer, + "_git", + lambda *_args, **_kwargs: b"../escape\0/abs/Cargo.toml\0Cargo.toml\0", + ) + assert materializer.tracked_paths(tmp_path) == {"Cargo.toml"} + + def test_invalid_toml_decode_fails_cli(tmp_path: Path) -> None: """Corrupt Cargo.toml fails the materializer CLI instead of building an image.""" repo = tmp_path / "repo" diff --git a/tests/test_opencode_review_surfaces.py b/tests/test_opencode_review_surfaces.py index d4b1df05e..add6ef9b3 100644 --- a/tests/test_opencode_review_surfaces.py +++ b/tests/test_opencode_review_surfaces.py @@ -351,6 +351,46 @@ def test_crates_root_and_grouped_python_surfaces() -> None: 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_publisher_workflow_cannot_replace_review_with_coverage_finding() -> None: """The #47 publisher shape — coverage REQUEST_CHANGES as the whole review — is gone.""" workflow = Path(".github/workflows/opencode-review-dispatch.yml").read_text( diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index 1bbd98750..6c64bbb6d 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -20,7 +20,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 = "83f6830d5c21a324b4dbcd4e5c21a07968994b81" +REVIEW_DISPATCH_BLOB_SHA = "703b63c05c2ca24ba04a6f53863914289ae59421" def _workflow_text(path: Path) -> str: From 53ef7b58963bee5b82ba399cc13b905dce46ec19 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 16 Aug 2026 16:40:36 +0000 Subject: [PATCH 3/3] fix(opencode): restore coverage-blocked status after fallback review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit create_pull_review COMMENT rewrites the issue comment to Gate result: COMMENT. On the OriginWeave #47 path — coverage miss plus unavailable model — that left the status surface looking finished. Restore COVERAGE_BLOCKED after the product-file review, and list extracted Rust API names without inventing a classDiagram edge. Co-authored-by: Seongho Bae --- .../workflows/opencode-review-dispatch.yml | 2 +- CHANGELOG.md | 2 +- ...opencode-review-surfaces-originweave-47.md | 35 +++++++++++++++---- scripts/ci/opencode_review_surfaces.py | 2 -- tests/test_opencode_review_surfaces.py | 6 ++++ ...t_pr_review_autofix_nvidia_nim_contract.py | 2 +- 6 files changed, 37 insertions(+), 12 deletions(-) diff --git a/.github/workflows/opencode-review-dispatch.yml b/.github/workflows/opencode-review-dispatch.yml index 703b63c05..1dec64063 100644 --- a/.github/workflows/opencode-review-dispatch.yml +++ b/.github/workflows/opencode-review-dispatch.yml @@ -5676,6 +5676,7 @@ jobs: >"$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")" + request_changes_for_coverage_evidence_failure rm -f "$body_file" } @@ -7617,7 +7618,6 @@ jobs: APPROVE) if [ "${COVERAGE_EVIDENCE_RESULT:-skipped}" != "success" ]; then publish_fallback_diff_review - request_changes_for_coverage_evidence_failure echo "::endgroup::" exit 1 fi diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ae5b51e9..7c572e4f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,7 +17,7 @@ Semantic Versioning where the repository publishes a release. ### Changed -- Split central OpenCode publication into distinct surfaces: the formal pull-request review is a source-backed walkthrough of the actual diff, and the issue comment is gate/status only (head SHA, run id, coverage/check results). Coverage-evidence failure no longer replaces the review or cites `.github/workflows/opencode-review.yml:1` on a product repository that did not change that file. +- Split central OpenCode publication into distinct surfaces: the formal pull-request review is a source-backed walkthrough of the actual diff, and the issue comment is gate/status only (head SHA, run id, coverage/check results). Coverage-evidence failure no longer replaces the review or cites `.github/workflows/opencode-review.yml:1` on a product repository that did not change that file. After a fallback COMMENT review, restore the status comment to `COVERAGE_BLOCKED` so a model-unavailable coverage miss cannot look like a finished comment-only review, and list extracted Rust API names without inventing a class relationship. - Require the hourly repair worker to establish an exact-head root cause, enumerate the smallest remediation candidates, and prove writer authority, sealed-path scope, credentials, dependency order, verifiability, and causal effect before editing; infeasible or external blockers leave the tree unchanged while the broader loop continues with another eligible PR or buyer-visible product gap. - Run the bounded Clearfolio PR review-feedback repair caller at minute 23 of every hour while keeping the shared scheduler free of product-specific timers and repository names for modular reuse by naruon, contextual-orchestrator, Inkspan, and other CWL services. - Run the bounded DiskSage repair heartbeat at minute 37 of every hour, dispatch no more than one exact-head repair, and wait two hours before redispatching an unchanged head so legitimate OpenCode or NVIDIA NIM latency does not create duplicate writers. diff --git a/docs/doctoring/opencode-review-surfaces-originweave-47.md b/docs/doctoring/opencode-review-surfaces-originweave-47.md index 9831cc898..5e60e914f 100644 --- a/docs/doctoring/opencode-review-surfaces-originweave-47.md +++ b/docs/doctoring/opencode-review-surfaces-originweave-47.md @@ -68,8 +68,12 @@ Regression tests prove that: `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`); - and -7. bounded Rust toolchain materialization copies manifests only, selects +7. `publish_fallback_diff_review` restores `COVERAGE_BLOCKED` after the + COMMENT review so `create_pull_review` cannot leave `Gate result: COMMENT` + on a coverage miss; +8. the class diagram lists public items and does not invent + `FirstType --> SecondType`; and +9. bounded Rust toolchain materialization copies manifests only, selects rustup 1.97 for OriginWeave-style workspaces, and rejects parent-directory members and symlinks. @@ -79,14 +83,31 @@ A later rustup or llvm-tools catalog change can still fail the image build. That failure remains a coverage-gate failure, not a synthesized product-file finding. The sandbox does not weaken a genuine below-threshold coverage miss. +Publishing the fallback review uses `create_pull_review COMMENT`, which also +rewrites the issue comment to `Gate result: COMMENT`. The publisher must then +restore `COVERAGE_BLOCKED` on that status surface so a model-unavailable +coverage miss does not look like a completed comment-only review. The fallback +class diagram lists extracted public items and does not invent a relationship +between the first two names. + ## References -GitHub, Inc. (2026). *REST API endpoints for pull request reviews*. GitHub -Docs. +GitHub, Inc. (n.d.). *REST API endpoints for pull request reviews*. GitHub +Docs. Retrieved August 16, 2026, from https://docs.github.com/en/rest/pulls/reviews -Rust Project Developers. (2026). *The rustup book*. Rust Project. -https://rust-lang.github.io/rustup/ +International Organization for Standardization. (2023). *Systems and software +engineering — Systems and software Quality Requirements and Evaluation +(SQuaRE) — Product quality model* (ISO/IEC 25010:2023). +https://www.iso.org/standard/78176.html + +National Institute of Standards and Technology. (2022). *Secure software +development framework (SSDF) version 1.1: Recommendations for mitigating the +risk of software vulnerabilities* (NIST Special Publication 800-218). +https://doi.org/10.6028/NIST.SP.800-218 + +Rust Project Developers. (n.d.). *The rustup book*. Retrieved August 16, 2026, +from https://rust-lang.github.io/rustup/ -Taiki Endo. (2026). *cargo-llvm-cov*. GitHub. +Taiki Endo. (2026). *cargo-llvm-cov* (Version 0.8.7) [Computer software]. https://github.com/taiki-e/cargo-llvm-cov diff --git a/scripts/ci/opencode_review_surfaces.py b/scripts/ci/opencode_review_surfaces.py index c1643fb1a..b74fed0cd 100644 --- a/scripts/ci/opencode_review_surfaces.py +++ b/scripts/ci/opencode_review_surfaces.py @@ -239,8 +239,6 @@ def emit_mermaid( lines = ["```mermaid", "classDiagram"] for symbol in symbols[:8]: lines.append(f" class {_quote_label(symbol)}") - if len(symbols) >= 2: - lines.append(f" {_quote_label(symbols[0])} --> {_quote_label(symbols[1])}") lines.append("```") return "\n".join(lines) + "\n" if rust_paths: diff --git a/tests/test_opencode_review_surfaces.py b/tests/test_opencode_review_surfaces.py index add6ef9b3..4e436f4cc 100644 --- a/tests/test_opencode_review_surfaces.py +++ b/tests/test_opencode_review_surfaces.py @@ -61,6 +61,8 @@ def test_mermaid_uses_public_rust_api_when_source_exists(tmp_path: Path) -> None ) assert "classDiagram" in diagram assert "FreshResolutionSnapshot" in diagram + assert "resolve_fresh" in diagram + assert "FreshResolutionSnapshot --> resolve_fresh" not in diagram assert "Changed file" not in diagram @@ -404,6 +406,10 @@ def test_publisher_workflow_cannot_replace_review_with_coverage_finding() -> Non 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 + 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 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 diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index 6c64bbb6d..fab39becb 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -20,7 +20,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 = "703b63c05c2ca24ba04a6f53863914289ae59421" +REVIEW_DISPATCH_BLOB_SHA = "1dec640637c07b4da431a8ebe596652312e7b20e" def _workflow_text(path: Path) -> str: