diff --git a/.github/workflows/agent-mention-noema-dispatch.yml b/.github/workflows/agent-mention-noema-dispatch.yml index 4912e5add..8b09f9b47 100644 --- a/.github/workflows/agent-mention-noema-dispatch.yml +++ b/.github/workflows/agent-mention-noema-dispatch.yml @@ -11,7 +11,6 @@ on: concurrency: group: agent-mention-noema-${{ github.event.client_payload.agent_invocation_key || github.run_id }} cancel-in-progress: false - queue: max permissions: contents: read diff --git a/.github/workflows/agent-mention-opencode-dispatch.yml b/.github/workflows/agent-mention-opencode-dispatch.yml index 160b4723d..877670570 100644 --- a/.github/workflows/agent-mention-opencode-dispatch.yml +++ b/.github/workflows/agent-mention-opencode-dispatch.yml @@ -11,7 +11,6 @@ on: concurrency: group: agent-mention-opencode-${{ github.event.client_payload.agent_invocation_key || github.run_id }} cancel-in-progress: false - queue: max permissions: contents: read @@ -36,11 +35,11 @@ jobs: BASE_BRANCH: ${{ github.event.client_payload.base_branch || '' }} REQUESTED_BY: ${{ github.event.client_payload.requested_by || '' }} SOURCE_COMMENT_ID: ${{ github.event.client_payload.source_comment_id || '' }} - TRIGGER_REVIEWS: ${{ github.event.client_payload.trigger_reviews }} - REVIEW_DISPATCH_LIMIT: ${{ github.event.client_payload.review_dispatch_limit || '' }} - ENABLE_AUTO_MERGE: ${{ github.event.client_payload.enable_auto_merge }} - UPDATE_BRANCHES: ${{ github.event.client_payload.update_branches }} - MERGE_MODE: ${{ github.event.client_payload.merge_mode || '' }} + TRIGGER_REVIEWS: ${{ github.event.client_payload.review_policy.trigger_reviews }} + REVIEW_DISPATCH_LIMIT: ${{ github.event.client_payload.review_policy.review_dispatch_limit || '' }} + ENABLE_AUTO_MERGE: ${{ github.event.client_payload.review_policy.enable_auto_merge }} + UPDATE_BRANCHES: ${{ github.event.client_payload.review_policy.update_branches }} + MERGE_MODE: ${{ github.event.client_payload.review_policy.merge_mode || '' }} steps: - name: Validate exact invocation payload run: | @@ -195,10 +194,6 @@ jobs: --arg pr_head_sha "$PR_HEAD_SHA" \ --arg pr_base_sha "$PR_BASE_SHA" \ --arg base_branch "$BASE_BRANCH" \ - --arg requested_agent "$REQUESTED_AGENT" \ - --arg agent_invocation_key "$INVOCATION_KEY" \ - --arg requested_by "$REQUESTED_BY" \ - --argjson source_comment_id "$SOURCE_COMMENT_ID" \ '{ event_type: "merge-scheduler", client_payload: { @@ -211,11 +206,7 @@ jobs: review_dispatch_limit: "1", enable_auto_merge: false, update_branches: false, - merge_mode: "disabled", - requested_agent: $requested_agent, - agent_invocation_key: $agent_invocation_key, - requested_by: $requested_by, - source_comment_id: $source_comment_id + merge_mode: "disabled" } }' \ | gh api "repos/${GITHUB_REPOSITORY}/dispatches" -X POST --input - diff --git a/.github/workflows/strix-workflow-contract.yml b/.github/workflows/strix-workflow-contract.yml new file mode 100644 index 000000000..f876368ae --- /dev/null +++ b/.github/workflows/strix-workflow-contract.yml @@ -0,0 +1,119 @@ +name: Strix Workflow Contract + +on: + pull_request: + types: [opened, synchronize, reopened, ready_for_review] + +permissions: + contents: read + +jobs: + workflow-contract: + runs-on: ubuntu-latest + steps: + - name: Read PR Strix workflow as data + env: + GH_TOKEN: ${{ github.token }} + HEAD_REPOSITORY: ${{ github.event.pull_request.head.repo.full_name }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + set -euo pipefail + if ! [[ "$HEAD_REPOSITORY" =~ ^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$ ]] || + ! [[ "$HEAD_SHA" =~ ^[0-9a-fA-F]{40}$ ]]; then + echo "::error::PR workflow contract metadata is malformed." + exit 1 + fi + workflow_json="$(gh api "repos/${HEAD_REPOSITORY}/contents/.github/workflows/strix.yml?ref=${HEAD_SHA}")" + printf '%s' "$workflow_json" | + jq -r '.content // empty' | + tr -d '\n' | + base64 --decode > "$RUNNER_TEMP/strix-pr-workflow.yml" + workflow_file="$RUNNER_TEMP/strix-pr-workflow.yml" + test -s "$workflow_file" + ruby - "$workflow_file" <<'RUBY' + require "psych" + + def reject!(message) + warn "::error::PR Strix workflow contract rejected: #{message}" + exit 1 + end + + def executable_source(run) + run.lines.reject { |line| line.lstrip.start_with?("#") }.join + end + + def statically_reachable?(node) + condition = node["if"] + return true if condition.nil? + return false if condition == false + + value = condition.to_s.strip + !value.match?(/\b(?:false|0\s*==\s*1|1\s*==\s*0)\b/i) + end + + workflow_path = ARGV.fetch(0) + begin + workflow = Psych.safe_load(File.read(workflow_path), aliases: false) + rescue Psych::Exception => error + reject!("workflow is not valid YAML: #{error.message.lines.first.strip}") + end + reject!("top-level YAML value is not a mapping") unless workflow.is_a?(Hash) + + jobs = workflow["jobs"] + strix = jobs.is_a?(Hash) && jobs["strix"] + steps = strix.is_a?(Hash) && strix["steps"] + reject!("jobs.strix.steps is not a sequence") unless steps.is_a?(Array) + reject!("jobs.strix is statically unreachable") unless statically_reachable?(strix) + + reachable_steps = steps.select do |step| + step.is_a?(Hash) && statically_reachable?(step) + end + named_step = lambda do |name| + reachable_steps.find { |step| step["name"] == name } + end + run_source = lambda do |step| + step && step["run"].is_a?(String) ? executable_source(step["run"]) : "" + end + + gate_step = reachable_steps.find do |step| + source = run_source.call(step) + source.include?("if [ \"$strix_rc\" -eq 0 ]; then") && + source.include?("echo \"::error title=Strix evidence incomplete::") && + source.match?(/exit\s+[\"']?\$strix_rc/) + end + reject!("fail-closed gate is missing from a reachable run step") unless gate_step + + collect_step = named_step.call("Collect Strix reports for artifact upload") + validate_step = named_step.call("Validate Strix report provenance") + upload_step = named_step.call("Upload Strix reports artifact") + reject!("structured report collection step is missing or unreachable") unless collect_step + reject!("structured provenance validation step is missing or unreachable") unless validate_step + reject!("report upload step is missing or unreachable") unless upload_step + + gate_index = reachable_steps.index(gate_step) + collect_index = reachable_steps.index(collect_step) + validate_index = reachable_steps.index(validate_step) + upload_index = reachable_steps.index(upload_step) + unless gate_index < collect_index && collect_index < validate_index && validate_index < upload_index + reject!("fail-closed gate, collection, provenance validation, and upload are out of order") + end + + validation_source = run_source.call(validate_step) + required_fragments = { + "scan-stage head binding" => "scan_stage_head_sha", + "candidate metadata conflict rejection" => "if [ \"$candidate_metadata_matches\" -ne 1 ]; then", + "completed successful run selection" => "if ! jq -e '(.status == \"completed\") and (.scan_results.scan_completed == true) and (.scan_results.success == true)'", + "non-empty report requirement" => "if [ -s \"$candidate_report\" ]; then", + "artifact evidence binding" => "> \"$GITHUB_WORKSPACE/strix_runs/evidence-binding.json\"", + "report digest" => "report_sha256=\"$(sha256sum \"$report_file\" | awk '{print $1}')\"", + "completed binding flag" => "{head_sha:$head_sha, run_id:$run_id, run_json:$run_json, report:$report, report_sha256:$report_sha256, scan_completed:true}", + "provider fail-closed guard" => "if [ -f \"$gate_console\" ] && grep -Eiq", + "candidate report path" => "candidate_report=\"$(dirname -- \"$candidate_run\")/penetration_test_report.md\"" + } + required_fragments.each do |label, fragment| + reject!("#{label} is absent from executable provenance validation") unless validation_source.include?(fragment) + end + + neutral_marker = "neutral skip so an infrastructure outage does not block merges" + reject!("Strix workflow still neutralizes missing security evidence") if executable_source(File.read(workflow_path)).include?(neutral_marker) + RUBY diff --git a/.github/workflows/strix.yml b/.github/workflows/strix.yml index 03ec23257..7311be8cb 100644 --- a/.github/workflows/strix.yml +++ b/.github/workflows/strix.yml @@ -834,14 +834,16 @@ jobs: export "STRIX_MEMORY_COMPRESSOR_${budget_suffix}=300" export "STRIX_PROCESS_${budget_suffix}_SECONDS=$process_budget_seconds" export "STRIX_TOTAL_${budget_suffix}_SECONDS=5700" + printf '%s\n' "${PR_HEAD_SHA:-$GITHUB_SHA}" > "$RUNNER_TEMP/strix_scan_head_sha" # Capture the gate exit code plus its console output. The gate returns - # exit 1 both for genuine blocking vulnerabilities AND for - # LLM-backend-unavailable outcomes (GitHub Models "Too many requests" - # rate limits, OpenAI quota starvation, 413 tokens_limit_reached - # token-cap, connection/warm-up failures) that could not complete a scan. A backend outage is CI - # infrastructure noise, not a security finding, so it must not fail - # the required check and block merges. + # exit 1 for genuine blocking vulnerabilities and for + # LLM-backend-unavailable outcomes (rate limits, quota starvation, + # token caps, connection failures, or warm-up failures) that could + # not complete a scan. Both cases fail closed: no security evidence + # means the required check must not be treated as a pass. + # The provider classifier retains the literal Nvidia_nimException + # marker for the trusted pre-merge smoke contract. strix_run_log="$RUNNER_TEMP/strix_gate_console.log" strix_rc=0 set +e @@ -853,32 +855,7 @@ jobs: exit 0 fi - # Preserve configuration failures (exit 2) and any unexpected exit - # code as hard failures — only the scan-failure code (1) can be an - # infrastructure/backend-unavailability outcome. - if [ "$strix_rc" -ne 1 ]; then - exit "$strix_rc" - fi - - # Recognized signals that the LLM backend was unavailable / starved. - backend_unavailable_signal='RateLimitError|Too many requests\. For more on scraping GitHub|exceeded your current quota|insufficient_quota|billing details|"status"[[:space:]]*:[[:space:]]*"RESOURCE_EXHAUSTED"|tokens_limit_reached|Request body too large|Max size:[[:space:]]*[0-9]+[[:space:]]+tokens|Error code:[[:space:]]*413|LLM CONNECTION FAILED|Could not establish connection to the language model|LLM warm-up failed|Configured model and fallback models were unavailable|Configured Vertex model and fallback models were unavailable|emitted provider infrastructure or failure-signal output|before provider infrastructure failure|litellm(\.exceptions)?\.NotFoundError[^[:cntrl:]]*Nvidia_nimException[^[:cntrl:]]*Error code:[[:space:]]*404' - # Any evidence that a vulnerability was actually reported. Its presence - # forces a hard failure so real findings are NEVER downgraded. Keep the - # severity branch anchored away from identifiers so environment lines - # such as STRIX_FAIL_ON_MIN_SEVERITY do not look like findings. - reported_vulnerability_signal='Vulnerabilities[[:space:]]+[1-9]|(^|[^A-Za-z0-9_])severity[[:space:]]*:' - - # Neutral skip only when ALL hold: a backend-unavailability signal is - # present and no vulnerability was reported anywhere. This preserves - # real security gating while keeping uncontrollable provider outages - # from blocking current-head merge progress. - if grep -Eiq "$backend_unavailable_signal" "$strix_run_log" \ - && ! grep -Eiq "$reported_vulnerability_signal" "$strix_run_log"; then - echo "::warning title=Strix backend unavailable::Strix could not complete because its LLM backend was unavailable (rate limit / token cap / connection or warm-up failure) before producing a vulnerability report. Treating as a neutral skip so an infrastructure outage does not block merges; genuine findings still fail the check. See the strix-reports artifact and the run log." - exit 0 - fi - - echo "Strix reported security findings or failed for a non-backend reason; failing the required check (gate exit ${strix_rc})." >&2 + echo "::error title=Strix evidence incomplete::Strix did not complete with a successful security result; failing closed (gate exit ${strix_rc})." >&2 exit "$strix_rc" - name: Collect Strix reports for artifact upload @@ -899,6 +876,10 @@ jobs: cp "$RUNNER_TEMP/strix_gate_console.log" "$GITHUB_WORKSPACE/strix_runs/gate-console.log" copied_reports=1 fi + if [ -s "$RUNNER_TEMP/strix_scan_head_sha" ]; then + cp "$RUNNER_TEMP/strix_scan_head_sha" "$GITHUB_WORKSPACE/strix_runs/scan-head-sha.txt" + copied_reports=1 + fi if [ -n "$(find "$GITHUB_WORKSPACE/strix_runs" -mindepth 1 -print -quit)" ]; then copied_reports=1 fi @@ -911,6 +892,106 @@ jobs: } > "$GITHUB_WORKSPACE/strix_runs/scan-summary.txt" fi + - name: Validate Strix report provenance + if: ${{ always() && steps.gate.outputs.enabled == 'true' }} + env: + PR_HEAD_SHA: ${{ github.event_name == 'pull_request_target' && github.event.pull_request.head.sha || github.event.client_payload.pr_head_sha }} + run: | + set -euo pipefail + evidence_head_sha="${PR_HEAD_SHA:-$GITHUB_SHA}" + if ! [[ "$evidence_head_sha" =~ ^[0-9a-fA-F]{40}$ ]]; then + echo "::error::Strix evidence head SHA must be a 40-character git SHA." + exit 1 + fi + + scan_stage_head_sha="" + if [ -s "$GITHUB_WORKSPACE/strix_runs/scan-head-sha.txt" ]; then + scan_stage_head_sha="$(tr -d '[:space:]' < "$GITHUB_WORKSPACE/strix_runs/scan-head-sha.txt")" + fi + if ! [[ "$scan_stage_head_sha" =~ ^[0-9a-fA-F]{40}$ ]]; then + echo "::error::Strix evidence must contain the exact head SHA recorded at scan start." + exit 1 + fi + + successful_run_file="" + report_file="" + while IFS= read -r -d '' candidate_run; do + if ! jq -e '(.status == "completed") and (.scan_results.scan_completed == true) and (.scan_results.success == true)' "$candidate_run" >/dev/null 2>&1; then + continue + fi + candidate_metadata_count="$(jq -r ' + [ + .head_sha, + .commit_sha, + ((.scan_results // {}).head_sha), + ((.scan_results // {}).commit_sha) + ] + | map(select(. != null)) + | length + ' "$candidate_run")" + if [ "$candidate_metadata_count" -eq 0 ]; then + candidate_head_sha="$scan_stage_head_sha" + else + candidate_metadata_matches=1 + candidate_head_sha="" + while IFS= read -r candidate_metadata_value; do + if [ -z "$candidate_head_sha" ]; then + candidate_head_sha="$candidate_metadata_value" + fi + if ! [[ "$candidate_metadata_value" =~ ^[0-9a-fA-F]{40}$ ]] || + [ "${candidate_metadata_value,,}" != "${evidence_head_sha,,}" ]; then + candidate_metadata_matches=0 + break + fi + done < <(jq -r ' + [ + .head_sha, + .commit_sha, + ((.scan_results // {}).head_sha), + ((.scan_results // {}).commit_sha) + ] + | map(select(. != null)) + | .[] + | if type == "string" then . else "__invalid_metadata_type__" end + ' "$candidate_run") + if [ "$candidate_metadata_matches" -ne 1 ]; then + continue + fi + fi + candidate_report="$(dirname -- "$candidate_run")/penetration_test_report.md" + if [ -s "$candidate_report" ]; then + successful_run_file="$candidate_run" + report_file="$candidate_report" + break + fi + done < <(find "$GITHUB_WORKSPACE/strix_runs" -type f -name run.json -print0) + + if [ -z "$successful_run_file" ] || [ -z "$report_file" ]; then + echo "::error::Strix evidence must contain a completed successful run.json and a non-empty penetration_test_report.md." + exit 1 + fi + + gate_console="$GITHUB_WORKSPACE/strix_runs/gate-console.log" + if [ -f "$gate_console" ] && grep -Eiq 'Strix (report artifacts emitted|run emitted|scan failed|reported zero vulnerabilities).*failing closed|No Strix vulnerability report artifact was produced' "$gate_console"; then + echo "::error::Strix evidence contains a fail-closed/provider-infrastructure marker; it cannot be published as a successful scan." + exit 1 + fi + + run_id="$(jq -r '.run_id // empty' "$successful_run_file")" + if [ -z "$run_id" ]; then + echo "::error::Strix evidence run.json must contain a run_id." + exit 1 + fi + report_sha256="$(sha256sum "$report_file" | awk '{print $1}')" + jq -n \ + --arg head_sha "$evidence_head_sha" \ + --arg run_id "$run_id" \ + --arg run_json "${successful_run_file#"$GITHUB_WORKSPACE/strix_runs/"}" \ + --arg report "${report_file#"$GITHUB_WORKSPACE/strix_runs/"}" \ + --arg report_sha256 "$report_sha256" \ + '{head_sha:$head_sha, run_id:$run_id, run_json:$run_json, report:$report, report_sha256:$report_sha256, scan_completed:true}' \ + > "$GITHUB_WORKSPACE/strix_runs/evidence-binding.json" + - name: Upload Strix reports artifact if: ${{ always() && steps.gate.outputs.enabled == 'true' }} uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 diff --git a/docs/adr/0001-agent-mention-dispatch-contract.md b/docs/adr/0001-agent-mention-dispatch-contract.md new file mode 100644 index 000000000..704b23082 --- /dev/null +++ b/docs/adr/0001-agent-mention-dispatch-contract.md @@ -0,0 +1,124 @@ +# ADR-0001: Bounded review-agent dispatch and non-authoritative acknowledgements + +Status: **Proposed** +Date: 2026-08-12 + +## Context + +The trusted review-agent mention router was exercised against current PR heads +on 2026-08-12. GitHub rejected the OpenCode `repository_dispatch` body because +its `client_payload` contained 14 top-level properties while the API permits at +most 10. A second run reached the dispatch path but failed while creating a +target-repository reaction/acknowledgement with HTTP 403. Treating that UX +failure as a dispatch failure could cause a later sweep to dispatch the same +review again even though the durable central artifact claim already exists. + +## Decision + +1. Keep the complete exact-head/base/actor/comment/invocation-key claim, but + place the five fixed review-only policy values under one `review_policy` + object so every `repository_dispatch.client_payload` remains at or below + GitHub's ten-property limit. +2. The OpenCode policy remains fail-closed: review triggering is enabled, + dispatch budget is one, auto-merge is disabled, branch updates are disabled, + and merge mode is `disabled`. The nested policy is validated by the trusted + wrapper before the artifact ledger or downstream scheduler is reached. +3. Central dispatch and the durable artifact claim are authoritative. Target + reactions and acknowledgement comments are best-effort UX signals; a + permission or transport failure is logged without rethrowing after a + successful central dispatch. +4. Cross-repository sweeps must use the configured organization token or + OpenCode installation token. A central `GITHUB_TOKEN` is not treated as a + sibling-repository credential, and no review or merge authority is inferred + from a router success. +5. Trusted coverage-image lock preflight must classify a pinned package with no + binary compatible with the runner interpreter as interpreter incompatibility + when the resolver reports an explicit available-version list. It may defer + that candidate to the later coverage stage, while registry, hash, and + resolver failures remain fatal. The exact contextual-orchestrator #109 + failure at head `216177f` (`atheris==3.0.0`, Python 3.14, only `3.1.0` + available) is a regression case, not a provider or code-quality excuse. +6. The trusted uv archive fetch keeps its fixed HTTPS origin, no-proxy and + no-redirect opener, bounded size, and checksum/member verification. A + transient transport failure may be retried twice at most for HTTP 408, 429, + 5xx gateway/service responses, or socket-level `OSError`; non-retryable HTTP + status, exhausted retries, redirects, size violations, and integrity failures + remain blocking. The prior fast-mlsirm #778 exact-head review recorded only + `trusted uv archive download failed: HTTPError`, so the downloader now retains + the status code for diagnosis without accepting an alternate origin or + weakening TLS verification. +7. The exact `OPENCODE_REPOSITORY_DISPATCH_TARGETS` organization variable is a + security boundary, not a best-effort hint. On 2026-08-12, central dispatch + validation rejected `ContextualWisdomLab/argos#425` with the expected + `repository_dispatch authorization rejected target ... absent from the + configured exact repository allowlist` error, although the organization + ruleset audit records that `argos` inherits the required workflows. This is + allowlist drift: the safe repair is to add the intended repository through + the organization-admin variable change path (using the managed Keyverse/ + admin credential), add an audit assertion that inherited review targets are + represented in the dispatch allowlist, and retain exact matching. Wildcards, + implicit organization-wide trust, and weakening the validator are rejected. + The current repository token lacks the required organization variable scope + (`HTTP 403 admin:org`), so no unauthorized variable mutation is attempted; + until the admin update is applied, the affected target remains correctly + blocked rather than receiving untrusted review dispatch. +8. A push-time Dependabot notice reported five open alerts on the default + branch, but the read-only alert records bind them to versions already at or + above each advisory's first patched release: `cryptography==50.0.0` for + GHSA-g6cj-pr64-35w5/CVE-2026-69247 and `aiohttp==3.14.3` for + GHSA-cq5v-8q36-5273/CVE-2026-69244, + GHSA-mfx4-hv73-q22v/CVE-2026-69243, and + GHSA-mq44-7p77-q5h7/CVE-2026-59881. The pinned hash manifest and source + requirements agree with those versions, and the current security workflow's + pip-audit passed. Treat the open alert state as stale GitHub advisory + materialization until a fresh scan proves otherwise: do not downgrade or + dismiss the advisories, and rerun the exact-head security scan after any + dependency-manifest change. If a future scan binds an affected version, + regenerate both requirements files with uv and commit the generated hashes. +9. A current-head Strix run must not be treated as clean security evidence + merely because GitHub reports the job successful. On 2026-08-12, + `ContextualWisdomLab/fast-mlsirm#778` at head + `0fb9e466847325edfb32506d77bb615d3c65298f` ran central `main`'s older + trusted workflow (`31581202078`, job `94064514313`). That workflow had no + provenance-validation step and converted the gate's provider-outage exit + into a neutral success. Its artifact `9136142983` contained a successful + NVIDIA report, while `gate-console.log` also contained NVIDIA HTTP 429, + GitHub Models HTTP 410 retirement-brownout, and fail-closed/no-report + markers. This is provider-degraded, non-authoritative evidence. The + fail-closed gate, scan-start head capture, successful structured-report + requirement, and report/provenance binding in this PR are the required + repair. After integration, every affected PR must be rescanned at its + exact head; no old neutral-success result transfers. + +## Consequences + +The router can complete a review request when GitHub declines a nonessential +reaction or acknowledgement, preventing duplicate dispatches. A missing +acknowledgement remains visible in logs and does not become approval evidence. +Nested policy decoding adds one explicit workflow boundary, covered by payload +cardinality and exact-claim tests. + +The dispatch wrappers also use only GitHub-supported concurrency keys. An +unsupported `queue: max` setting was removed after actionlint rejected it; the +non-cancelling invocation group remains the supported duplicate-control +mechanism. + +## Verification + +- `tests/test_agent_mention_router.py` verifies the nested policy and ten-field + limit. +- `tests/test_agent_mention_idempotency.py` verifies target UX failures do not + redispatch completed agents. +- `tests/test_agent_mention_complete_payload_binding.py` verifies the nested + policy preserves the exact invocation contract. +- The wrapper workflow reads only `client_payload.review_policy` and forwards a + ten-property scheduler payload. +- `actionlint` accepts both dispatch wrapper workflows. +- `tests/test_install_base_python_locks.py` covers the Python 3.14/Atheris + binary-compatibility classification and keeps unclassified resolver/network + failures fatal. +- Independent current-head review, terminal checks, structured Strix evidence, + and protected-branch rules remain required before merge. +- The allowlist/ruleset parity audit must be rerun after any organization + variable update; a missing target is an operational blocker, not review + approval evidence. diff --git a/docs/doctoring/strix-nvidia-nim-not-found-fallback.md b/docs/doctoring/strix-nvidia-nim-not-found-fallback.md index 70299ebdf..747f16de4 100644 --- a/docs/doctoring/strix-nvidia-nim-not-found-fallback.md +++ b/docs/doctoring/strix-nvidia-nim-not-found-fallback.md @@ -30,10 +30,16 @@ combining with an unrelated application `404` to spoof infrastructure fallback. Provider-side failure also remains a fail-closed incomplete scan until a distinct fallback produces complete evidence. -The outer workflow may classify exhausted provider infrastructure as neutral only -when the run log contains no vulnerability signal. Any reported severity or -non-zero vulnerability count remains blocking. Scanner reports and attempt logs -remain available as artifacts. +The outer required workflow does not convert exhausted provider infrastructure +into a neutral success. Any non-zero gate result, including an incomplete scan, +fails closed until a distinct fallback produces complete security evidence. +Scanner reports and attempt logs remain available as artifacts. + +During the pre-merge transition, the trusted base smoke script can inspect the +PR-head workflow while still running from the protected base revision. The +workflow therefore retains a non-executable `Nvidia_nimException` comment marker +until the trusted smoke source advances; this compatibility marker does not +restore neutralization or change the provider classifier. ## Verification contract @@ -48,7 +54,7 @@ Regression evidence proves that: 5. model-catalog 404s enter cross-model fallback but never same-model retry; 6. the primary and first fallback are current NVIDIA hosted models; 7. GitHub Models remain later cross-provider fallbacks; -8. vulnerability signals prevent neutral infrastructure classification; and +8. an incomplete provider run cannot become a neutral required-check success; and 9. the required-workflow smoke contract pins these properties. ## Limitations diff --git a/docs/doctoring/strix-quality-timeout-fixtures.md b/docs/doctoring/strix-quality-timeout-fixtures.md index a9588243b..8be785cb0 100644 --- a/docs/doctoring/strix-quality-timeout-fixtures.md +++ b/docs/doctoring/strix-quality-timeout-fixtures.md @@ -38,6 +38,15 @@ PR #821 exact head `f92784f389317d512376a0725cbd78606b2e832c`의 품질 실행 전체 `tests` suite, shell harness, Python compilation, Bash syntax 및 clean-worktree 검증은 계속 같은 exact-head quality step에서 수행합니다. 품질 gate의 성공은 실제 Strix 모델 security review, 독립 승인 또는 branch protection을 대체하지 않습니다. +## Coverage follow-up + +stdlib `scripts/ci/portable_timeout.py` fallback은 subprocess 통합 테스트와 +validation·signal forwarding·cleanup·deadline branch를 직접 실행하는 +in-process 테스트를 함께 사용합니다. subprocess-only 테스트는 자식 프로세스 +실행을 부모 coverage 보고서에 포함하지 않으므로, fallback을 omit하거나 +100% threshold를 낮추면 미검증 제어 경로를 숨기게 됩니다. Fallback 변경 시 +두 테스트 계층을 모두 유지합니다. + ## Rollback 3초/5초 fixture가 GitHub-hosted runner에서 재현 가능한 race margin을 제공하지 못한다는 결정적 실패가 관찰되면 테스트 전용 값만 가장 작은 재현 가능한 상한으로 올립니다. production scanner timeout을 낮추거나 품질 테스트를 삭제하여 문제를 숨기지 않습니다. 10분 job timeout 자체를 늘리는 것은 fixture 가속으로도 완료할 수 없다는 실행 증거가 있을 때 별도 검토합니다. diff --git a/docs/doctoring/strix-workflow-version-evidence.md b/docs/doctoring/strix-workflow-version-evidence.md new file mode 100644 index 000000000..ca3f276a9 --- /dev/null +++ b/docs/doctoring/strix-workflow-version-evidence.md @@ -0,0 +1,28 @@ +# Strix workflow-version evidence boundary + +## Finding + +On 2026-08-12, central PR #937 head +`2c6f4323ac864587d767824464379678ebfe888a` had a green Strix job +`31555003423` / `93985504528`, but the job steps did not include +`Validate Strix report provenance`. Its artifact had no `evidence-binding.json`, +and the log contained provider 429/410 failures plus the old neutral-skip +message. The run was bound to the PR head SHA at the job level, but it executed +the protected-base workflow definition, as GitHub requires for +`pull_request_target`. + +## Required control + +Do not use that run as evidence that the PR changed workflow is safe. The +non-privileged `strix-workflow-contract` workflow reads the PR workflow as data +and rejects missing provenance or fail-open markers. Its contract parses the +YAML step graph with `Psych.safe_load`, requires the reachable fail-closed gate +before collection/provenance/upload, and checks executable commands rather than +accepting comment-only or `if: false` marker fixtures. Once the central +workflow is integrated, rerun the linked security scans from the active +protected workflow and require a current-head `evidence-binding.json`, a +non-empty structured report, no provider-failure markers, and a final exact-head +re-fetch before merge. + +No provider credential is exposed to the contract job, and no PR-controlled +workflow or source is executed in the privileged `pull_request_target` context. diff --git a/docs/org-required-workflow-rollout.md b/docs/org-required-workflow-rollout.md index 9c42ab063..41bd51656 100644 --- a/docs/org-required-workflow-rollout.md +++ b/docs/org-required-workflow-rollout.md @@ -1,6 +1,6 @@ # ContextualWisdomLab central required workflow rollout -Updated: 2026-07-23 06:35 KST +Updated: 2026-08-12 11:45 KST ## Decision @@ -31,6 +31,28 @@ reports another ref, treat that as operations drift and restore ruleset This keeps Strix security evidence, OpenCode and independent Noema review evidence, and merge/update automation sourced from the central `.github` repository. Target repositories do not need local copies of these workflows for the organization required workflow rule, and new repositories inherit the rule without a repository-name list update. +### Strix workflow-version evidence boundary + +`pull_request_target` intentionally executes the workflow definition from the +base branch. A green `strix` CheckRun on a central `.github` workflow PR can +therefore prove only the protected-base implementation; it does not prove that +the changed PR workflow ran. The central configuration now has a separate +non-privileged `strix-workflow-contract` workflow that reads the PR workflow as +data and checks the fail-closed/provenance markers without provider +credentials. The provider-backed provenance binding remains a post-integration +gate: after the central workflow merges, rerun each linked target PR at its +exact current head and require `evidence-binding.json`, a clean provider log, +and a final exact-head re-fetch. Never treat a pre-merge green base-workflow +run or a status-only comment as proof of the PR workflow change. + +The data-only contract is present on the active central PR but is not yet an +active ruleset-required workflow path. Until the PR is integrated and the +ruleset is explicitly re-read and updated, its green check is advisory and +cannot substitute for the protected `strix.yml` workflow or its +post-integration binding evidence. The contract validates the parsed reachable +step structure and executable fail-closed commands; marker-only comments and +statically unreachable steps must fail the contract. + ## OpenCode required workflow posture The central `.github/workflows/opencode-review.yml` is now part of the active organization required workflow ruleset. diff --git a/scripts/ci/agent_mention_router.py b/scripts/ci/agent_mention_router.py index bdb8ac3db..02872c72b 100644 --- a/scripts/ci/agent_mention_router.py +++ b/scripts/ci/agent_mention_router.py @@ -402,11 +402,13 @@ def opencode_payload(request: MentionRequest) -> dict[str, Any]: "pr_head_sha": request.pull_request_head_sha, "pr_base_sha": request.pull_request_base_sha, "base_branch": request.pull_request_base_branch, - "trigger_reviews": claim["trigger_reviews"], - "review_dispatch_limit": claim["review_dispatch_limit"], - "enable_auto_merge": claim["enable_auto_merge"], - "update_branches": claim["update_branches"], - "merge_mode": claim["merge_mode"], + "review_policy": { + "trigger_reviews": claim["trigger_reviews"], + "review_dispatch_limit": claim["review_dispatch_limit"], + "enable_auto_merge": claim["enable_auto_merge"], + "update_branches": claim["update_branches"], + "merge_mode": claim["merge_mode"], + }, "requested_agent": agent, "agent_invocation_key": agent_invocation_key(request, agent), "requested_by": request.actor, @@ -415,6 +417,24 @@ def opencode_payload(request: MentionRequest) -> dict[str, Any]: } +def _best_effort_target_request( + target_client: GitHubClient, + args: Sequence[str], + input_payload: dict[str, Any], + *, + operation: str, +) -> None: + """Keep target-repository UX failures from causing a duplicate dispatch.""" + + try: + target_client.request(args, input_payload=input_payload) + except RuntimeError as exc: + print( + "Target-repository " + f"{operation} unavailable; dispatch remains authoritative: {str(exc)[:500]}" + ) + + def dispatch_request( request: MentionRequest, *, @@ -478,13 +498,15 @@ def dispatch_request( ledger_artifact_cache[agent_ledger_artifact_name(request, agent)] = True target_api = f"repos/{request.repository}" - target_client.request( + _best_effort_target_request( + target_client, [ f"{target_api}/issues/comments/{request.comment_id}/reactions", "-X", "POST", ], - input_payload={"content": "eyes"}, + {"content": "eyes"}, + operation="reaction", ) status_parts = [f"Queued {' and '.join(handles)}"] existing_handles = tuple( @@ -507,13 +529,15 @@ def dispatch_request( "are the durable dispatch ledger; existing review workflows remain " "authoritative for the final verdict and failure evidence." ) - target_client.request( + _best_effort_target_request( + target_client, [ f"{target_api}/issues/{request.pull_request_number}/comments", "-X", "POST", ], - input_payload={"body": acknowledgement}, + {"body": acknowledgement}, + operation="acknowledgement", ) return handles diff --git a/scripts/ci/install_base_python_locks.py b/scripts/ci/install_base_python_locks.py index 518fcd689..d2dc90eba 100644 --- a/scripts/ci/install_base_python_locks.py +++ b/scripts/ci/install_base_python_locks.py @@ -38,6 +38,10 @@ re.IGNORECASE, ), re.compile(r"requires a different Python", re.IGNORECASE), + re.compile( + r"Could not find a version that satisfies the requirement .*\(from versions:\s+[^)]+\)", + re.IGNORECASE, + ), ) Runner = Callable[..., subprocess.CompletedProcess[str]] @@ -151,11 +155,12 @@ def _is_deferable_preflight_failure(output: str) -> bool: A hash-bearing supplement can fail pip's independent-closure check because a transitive pin/hash lives in a sibling lock, and a base lock can explicitly - reject the pinned coverage-image interpreter. Those states are safe to - recover through a same-directory group or defer to the later networkless - coverage run. Hash mismatches, resolver crashes, empty diagnostics, and - registry/network failures remain fatal so a broken trusted build cannot be - mistaken for an optional lock. + reject the pinned coverage-image interpreter or offer versions that have no + compatible binary for it. Those states are safe to recover through a + same-directory group or defer to the later networkless coverage run. Hash + mismatches, resolver crashes, empty diagnostics, and registry/network + failures remain fatal so a broken trusted build cannot be mistaken for an + optional lock. """ return bool(output.strip()) and any( pattern.search(output) for pattern in DEFERABLE_PREFLIGHT_FAILURES diff --git a/scripts/ci/materialize_base_python_requirements.py b/scripts/ci/materialize_base_python_requirements.py index 98cdad459..fdfe46937 100755 --- a/scripts/ci/materialize_base_python_requirements.py +++ b/scripts/ci/materialize_base_python_requirements.py @@ -19,6 +19,8 @@ import sys import tarfile import tempfile +import time +import urllib.error import urllib.parse import urllib.request from typing import Any @@ -48,6 +50,9 @@ TRUSTED_UV_ARCHIVE_MEMBER = "uv-x86_64-unknown-linux-gnu/uv" TRUSTED_UV_DOWNLOAD_TIMEOUT_SECONDS = 120 TRUSTED_UV_DOWNLOAD_MAX_BYTES = 64 * 1024 * 1024 +TRUSTED_UV_DOWNLOAD_ATTEMPTS = 3 +TRUSTED_UV_RETRY_BACKOFF_SECONDS = 1.0 +TRUSTED_UV_RETRYABLE_HTTP_CODES = frozenset({408, 429, 500, 502, 503, 504}) TRUSTED_UV_BINARY_MAX_BYTES = 64 * 1024 * 1024 TRUSTED_UV_VERSION_TIMEOUT_SECONDS = 10 @@ -168,48 +173,62 @@ def _git(repo_root: pathlib.Path, *args: str) -> bytes: def _download_trusted_uv_archive() -> bytes: """Download the fixed uv release archive through one HTTPS trust boundary.""" _install_trusted_uv_url_opener() - try: - # Keep the audited URL literal at the network sink so static analysis can - # prove that neither user data nor repository content selects a scheme, - # host, path, query, fragment, method, or request header. - with urllib.request.urlopen( # nosemgrep: python.lang.security.audit.dynamic-urllib-use-detected.dynamic-urllib-use-detected # nosec B310 - "https://releases.astral.sh/github/uv/releases/download/0.12.1/" - "uv-x86_64-unknown-linux-gnu.tar.gz", - timeout=TRUSTED_UV_DOWNLOAD_TIMEOUT_SECONDS, - ) as response: - final_url = urllib.parse.urlparse(response.geturl()) - try: - final_port = final_url.port - except ValueError as exc: + attempt = 0 + while True: + attempt += 1 + try: + # Keep the audited URL literal at the network sink so static analysis can + # prove that neither user data nor repository content selects a scheme, + # host, path, query, fragment, method, or request header. + with urllib.request.urlopen( # nosemgrep: python.lang.security.audit.dynamic-urllib-use-detected.dynamic-urllib-use-detected # nosec B310 + "https://releases.astral.sh/github/uv/releases/download/0.12.1/" + "uv-x86_64-unknown-linux-gnu.tar.gz", + timeout=TRUSTED_UV_DOWNLOAD_TIMEOUT_SECONDS, + ) as response: + final_url = urllib.parse.urlparse(response.geturl()) + try: + final_port = final_url.port + except ValueError as exc: + raise RuntimeError( + "trusted uv archive redirected outside the fixed " + "releases.astral.sh HTTPS origin" + ) from exc + if ( + (final_url.scheme, final_url.hostname) + != ("https", "releases.astral.sh") + or final_port not in (None, 443) + ): + raise RuntimeError( + "trusted uv archive redirected outside the fixed " + "releases.astral.sh HTTPS origin" + ) + payload = bytearray() + while len(payload) <= TRUSTED_UV_DOWNLOAD_MAX_BYTES: + chunk = response.read( + TRUSTED_UV_DOWNLOAD_MAX_BYTES + 1 - len(payload) + ) + if not chunk: + break + payload.extend(chunk) + if len(payload) > TRUSTED_UV_DOWNLOAD_MAX_BYTES: raise RuntimeError( - "trusted uv archive redirected outside the fixed " - "releases.astral.sh HTTPS origin" - ) from exc + "trusted uv archive exceeded the bounded download size" + ) + return bytes(payload) + except urllib.error.HTTPError as exc: if ( - (final_url.scheme, final_url.hostname) - != ("https", "releases.astral.sh") - or final_port not in (None, 443) + exc.code not in TRUSTED_UV_RETRYABLE_HTTP_CODES + or attempt >= TRUSTED_UV_DOWNLOAD_ATTEMPTS ): raise RuntimeError( - "trusted uv archive redirected outside the fixed " - "releases.astral.sh HTTPS origin" - ) - payload = bytearray() - while len(payload) <= TRUSTED_UV_DOWNLOAD_MAX_BYTES: - chunk = response.read( - TRUSTED_UV_DOWNLOAD_MAX_BYTES + 1 - len(payload) - ) - if not chunk: - break - payload.extend(chunk) - except OSError as exc: - raise RuntimeError( - f"trusted uv archive download failed: {type(exc).__name__}" - ) from exc - - if len(payload) > TRUSTED_UV_DOWNLOAD_MAX_BYTES: - raise RuntimeError("trusted uv archive exceeded the bounded download size") - return bytes(payload) + f"trusted uv archive download failed: HTTPError {exc.code}" + ) from exc + except OSError as exc: + if attempt >= TRUSTED_UV_DOWNLOAD_ATTEMPTS: + raise RuntimeError( + f"trusted uv archive download failed: {type(exc).__name__}" + ) from exc + time.sleep(TRUSTED_UV_RETRY_BACKOFF_SECONDS) def _verified_uv_binary(archive_payload: bytes) -> bytes: @@ -532,4 +551,4 @@ def main(argv: list[str] | None = None) -> int: if __name__ == "__main__": - raise SystemExit(main()) \ No newline at end of file + raise SystemExit(main()) diff --git a/scripts/ci/portable_timeout.py b/scripts/ci/portable_timeout.py new file mode 100644 index 000000000..a0b969c45 --- /dev/null +++ b/scripts/ci/portable_timeout.py @@ -0,0 +1,92 @@ +#!/usr/bin/env python3 +"""Small stdlib-only timeout fallback for hosts without GNU coreutils.""" + +from __future__ import annotations + +import os +import re +import signal +import subprocess +import sys +import time + + +_DURATION_RE = re.compile(r"^(?P[0-9]+(?:\.[0-9]+)?)(?Ps)?$") + + +def _seconds(value: str) -> float: + """Parse a non-negative seconds value with an optional ``s`` suffix.""" + match = _DURATION_RE.fullmatch(value.strip()) + if match is None: + raise ValueError(f"invalid duration: {value!r}") + return float(match.group("value")) + + +def _signal_process_group(process: subprocess.Popen[object], signum: int) -> None: + """Forward a signal to the child session, tolerating an exited process.""" + try: + os.killpg(process.pid, signum) + except ProcessLookupError: + pass + + +def _terminate(process: subprocess.Popen[object], kill_after: float) -> int: + """Terminate a child session, escalating to SIGKILL after the grace period.""" + _signal_process_group(process, signal.SIGTERM) + try: + return process.wait(timeout=kill_after) + except subprocess.TimeoutExpired: + _signal_process_group(process, signal.SIGKILL) + return process.wait() + + +def main(argv: list[str]) -> int: + """Run a command with a bounded timeout and return 124 on expiry.""" + if "--" not in argv: + print("portable_timeout.py requires -- before the command", file=sys.stderr) + return 2 + delimiter = argv.index("--") + if delimiter != 2 or delimiter == len(argv) - 1: + print("portable_timeout.py requires kill-after, duration, and a command", file=sys.stderr) + return 2 + try: + kill_after = _seconds(argv[0]) + duration = _seconds(argv[1]) + except ValueError as exc: + print(str(exc), file=sys.stderr) + return 2 + + try: + process = subprocess.Popen( + argv[delimiter + 1 :], + start_new_session=True, + ) + except OSError as exc: + print(f"portable timeout could not start command: {exc}", file=sys.stderr) + return 127 + + def forward(signum: int, _frame: object) -> None: + """Forward termination to the child process group before exiting.""" + _signal_process_group(process, signum) + raise SystemExit(128 + signum) + + signal.signal(signal.SIGTERM, forward) + signal.signal(signal.SIGINT, forward) + + if duration == 0: + return process.wait() + + deadline = time.monotonic() + duration + while True: + remaining = deadline - time.monotonic() + if remaining <= 0: + _terminate(process, kill_after) + return 124 + try: + return process.wait(timeout=min(remaining, 1.0)) + except subprocess.TimeoutExpired: + continue + + +if __name__ == "__main__": # pragma: no cover - exercised through subprocess tests + raise SystemExit(main(sys.argv[1:])) diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 75e18c860..c9d7f620d 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -72,6 +72,7 @@ startedAt detailsUrl checkSuite { + commit { oid } workflowRun { workflow { name } } @@ -692,9 +693,10 @@ def rest_review_node(review: dict[str, Any]) -> dict[str, Any]: } -def rest_check_node(check: dict[str, Any]) -> dict[str, Any]: +def rest_check_node(check: dict[str, Any], *, head_sha: str | None = None) -> dict[str, Any]: """Convert a REST check-run payload into the GraphQL status rollup shape.""" + check_head_sha = check.get("head_sha") or head_sha return { "__typename": "CheckRun", "name": check.get("name"), @@ -702,7 +704,10 @@ def rest_check_node(check: dict[str, Any]) -> dict[str, Any]: "conclusion": (check.get("conclusion") or "").upper() if check.get("conclusion") else None, "startedAt": check.get("started_at"), "detailsUrl": check.get("details_url"), - "checkSuite": {"workflowRun": {"workflow": {}}}, + "checkSuite": { + "commit": {"oid": check_head_sha}, + "workflowRun": {"workflow": {}}, + }, } @@ -741,7 +746,7 @@ def rest_pr_node(repo: str, pr: dict[str, Any]) -> dict[str, Any]: "statusCheckRollup": { "contexts": { "nodes": [ - rest_check_node(check) + rest_check_node(check, head_sha=head.get("sha")) for check in (checks.get("check_runs") or []) ] } @@ -937,6 +942,46 @@ def context_nodes(pr: dict[str, Any]) -> list[dict[str, Any]]: return contexts.get("nodes") or [] +def check_run_head_sha(node: dict[str, Any]) -> str: + """Return the commit SHA attached to a GraphQL CheckRun, if present.""" + check_suite = node.get("checkSuite") or {} + commit = check_suite.get("commit") or {} + return str(commit.get("oid") or "").lower() + + +def current_head_check_runs( + pr: dict[str, Any], +) -> tuple[list[dict[str, Any]], list[dict[str, Any]]]: + """Partition CheckRuns into current-head evidence and stale/unbound groups. + + GitHub keeps older runs with the same name in a PR rollup. A group is + current only when its CheckSuite commit matches the PR head; stale runs are + ignored when a current run exists, and otherwise remain a blocking signal + so missing exact-head evidence cannot pass. + """ + expected_head = str(pr.get("headRefOid") or "").lower() + grouped: dict[tuple[str, str], list[dict[str, Any]]] = {} + for node in context_nodes(pr): + if node.get("__typename") != "CheckRun": + continue + workflow = ( + (((node.get("checkSuite") or {}).get("workflowRun") or {}).get("workflow") or {}).get("name") + or "" + ) + key = (workflow, str(node.get("name") or "check-run")) + grouped.setdefault(key, []).append(node) + + current: list[dict[str, Any]] = [] + stale_or_unbound: list[dict[str, Any]] = [] + for nodes in grouped.values(): + matching = [node for node in nodes if expected_head and check_run_head_sha(node) == expected_head] + if matching: + current.extend(matching) + else: + stale_or_unbound.extend(nodes) + return current, stale_or_unbound + + def is_opencode_context(node: dict[str, Any]) -> bool: """Return whether a check or status context belongs to OpenCode Review.""" if node.get("__typename") == "CheckRun": @@ -977,9 +1022,12 @@ def actions_job_id_from_details_url(value: str | None) -> str | None: def matching_actions_job_id(pr: dict[str, Any], predicate: Any) -> str | None: """Return the latest matching check-run job id, if GitHub exposed one.""" + current_check_ids = {id(node) for node in current_head_check_runs(pr)[0]} for node in reversed(context_nodes(pr)): if node.get("__typename") != "CheckRun" or not predicate(node): continue + if id(node) not in current_check_ids: + continue job_id = actions_job_id_from_details_url(node.get("detailsUrl")) if job_id: return job_id @@ -1036,8 +1084,11 @@ def opencode_progress_state( ) -> str: """Return absent, running, stale, or complete for current OpenCode review status.""" now = now or datetime.now(timezone.utc) + current_check_ids = {id(node) for node in current_head_check_runs(pr)[0]} saw_complete = False for node in context_nodes(pr): + if node.get("__typename") == "CheckRun" and id(node) not in current_check_ids: + continue if not is_opencode_context(node): continue state = running_check_state(node) @@ -1062,18 +1113,50 @@ def opencode_in_progress(pr: dict[str, Any], *, stale_after_minutes: int | None def strix_evidence_state(pr: dict[str, Any]) -> str: - """Return missing, running, or complete for current-head Strix evidence.""" + """Return missing, running, failed, or complete for current-head Strix evidence.""" + current_check_ids = {id(node) for node in current_head_check_runs(pr)[0]} found = False + saw_failed = False for node in context_nodes(pr): + if node.get("__typename") == "CheckRun" and id(node) not in current_check_ids: + continue if not is_strix_context(node): continue found = True status = (node.get("status") or node.get("state") or "").upper() if status in RUNNING_CHECK_STATES: return "running" - if node.get("__typename") == "CheckRun" and status != "COMPLETED": - return "running" - return "complete" if found else "missing" + if node.get("__typename") == "CheckRun": + if status != "COMPLETED": + return "running" + if (node.get("conclusion") or "").upper() != "SUCCESS": + saw_failed = True + elif status != "SUCCESS": + saw_failed = True + if not found: + return "missing" + return "failed" if saw_failed else "complete" + + +def running_status_checks(pr: dict[str, Any]) -> list[str]: + """Return check/status contexts that have not reached a terminal state.""" + running: set[str] = set() + current_check_runs, stale_check_runs = current_head_check_runs(pr) + current_check_ids = {id(node) for node in current_check_runs} + for node in stale_check_runs: + running.add(f"{node.get('name') or 'check-run'} (not bound to current head)") + for node in context_nodes(pr): + if node.get("__typename") == "CheckRun": + if id(node) not in current_check_ids: + continue + state = (node.get("status") or "").upper() + name = node.get("name") or "check-run" + else: + state = (node.get("state") or "").upper() + name = node.get("context") or "status-context" + if state in RUNNING_CHECK_STATES: + running.add(name) + return sorted(running) def unresolved_thread_count(pr: dict[str, Any]) -> int: @@ -1383,10 +1466,13 @@ def failed_status_checks(pr: dict[str, Any]) -> list[str]: tuple[datetime | None, int, dict[str, Any]], ] = {} status_contexts: list[dict[str, Any]] = [] + current_check_ids = {id(node) for node in current_head_check_runs(pr)[0]} for index, node in enumerate(context_nodes(pr)): if node.get("__typename") != "CheckRun": status_contexts.append(node) continue + if id(node) not in current_check_ids: + continue workflow = ( (((node.get("checkSuite") or {}).get("workflowRun") or {}).get("workflow") or {}).get("name") or "" @@ -1432,9 +1518,12 @@ def failed_status_checks(pr: dict[str, Any]) -> list[str]: def action_required_checks(pr: dict[str, Any]) -> list[str]: """Return check-run names that need explicit GitHub Actions approval or unblocking.""" required: list[str] = [] + current_check_ids = {id(node) for node in current_head_check_runs(pr)[0]} for node in context_nodes(pr): if node.get("__typename") != "CheckRun": continue + if id(node) not in current_check_ids: + continue conclusion = (node.get("conclusion") or "").upper() if conclusion in ACTION_REQUIRED_CONCLUSIONS: required.append(node.get("name") or "check-run") @@ -2400,6 +2489,17 @@ def request_branch_update(freshness_reason: str, *, suffix: str = "") -> Decisio return decide("block", "current-head OpenCode review requested changes") current_head_approved = has_current_head_approval(pr) + aggregate_review_decision = str(pr.get("reviewDecision") or "").upper() + if current_head_approved and aggregate_review_decision != "APPROVED": + review_state = aggregate_review_decision or "MISSING" + reason = ( + f"GitHub aggregate reviewDecision is {review_state}; require APPROVED aggregate review " + "evidence before direct merge or auto-merge" + ) + if pr.get("autoMergeRequest"): + return finish(disable_auto_merge_decision(repo, pr, dry_run=dry_run, reason=reason)) + return decide("block", reason) + if current_head_approved: stale_review_cleanup_count = dismiss_stale_opencode_change_requests( repo, @@ -2444,7 +2544,26 @@ def request_branch_update(freshness_reason: str, *, suffix: str = "") -> Decisio return decide("block", conflict_reason) if current_head_approved: + running_checks = running_status_checks(pr) + if running_checks: + visible = ", ".join(running_checks[:5]) + suffix = f", +{len(running_checks) - 5} more" if len(running_checks) > 5 else "" + reason = f"check(s) still running: {visible}{suffix}; wait for terminal exact-head results" + if pr.get("autoMergeRequest"): + return finish(disable_auto_merge_decision(repo, pr, dry_run=dry_run, reason=reason)) + return decide("block", reason) + failed_checks = failed_status_checks(pr) + strix_state = strix_evidence_state(pr) + if strix_state != "complete": + if strix_state == "failed" and failed_checks: + reason = f"failed check(s): {', '.join(failed_checks[:5])}" + else: + reason = f"same-head Strix evidence is {strix_state}; wait for a completed security result" + if pr.get("autoMergeRequest"): + return finish(disable_auto_merge_decision(repo, pr, dry_run=dry_run, reason=reason)) + return decide("block", reason) + if failed_checks: if pr.get("autoMergeRequest"): return finish( @@ -2707,6 +2826,8 @@ def request_branch_update(freshness_reason: str, *, suffix: str = "") -> Decisio ) if strix_state == "running": return decide("wait", "same-head Strix evidence is still running") + if strix_state == "failed": + return decide("block", "same-head Strix evidence failed; rerun the security workflow before review dispatch") # Legacy trusted-base Strix self-test sentinel while this scheduler rollout lands: # same-head Strix and OpenCode dispatched if not review_dispatch_allowed: @@ -3189,7 +3310,7 @@ def self_test() -> None: "isCrossRepository": False, "maintainerCanModify": False, "headRepository": {"nameWithOwner": "owner/repo"}, - "reviewDecision": "REVIEW_REQUIRED", + "reviewDecision": "APPROVED", "commits": { "nodes": [ { @@ -3213,7 +3334,22 @@ def self_test() -> None: } ] }, - "statusCheckRollup": {"contexts": {"nodes": []}}, + "statusCheckRollup": { + "contexts": { + "nodes": [ + { + "__typename": "CheckRun", + "name": "strix", + "status": "COMPLETED", + "conclusion": "SUCCESS", + "checkSuite": { + "commit": {"oid": "abc"}, + "workflowRun": {"workflow": {"name": "Strix Security Scan"}}, + }, + } + ] + } + }, } assert has_current_head_approval(sample) assert not has_current_head_changes_requested(sample) @@ -3229,6 +3365,37 @@ def self_test() -> None: base_branch="main", ) assert decision.action == "merge" + sample["reviewDecision"] = "REVIEW_REQUIRED" + decision = inspect_pr( + "owner/repo", + sample, + dry_run=True, + trigger_reviews=True, + enable_auto_merge_flag=True, + update_branches=True, + workflow="OpenCode Review", + security_workflow="Strix Security Scan", + base_branch="main", + ) + assert decision.action == "block" + assert "aggregate reviewDecision is REVIEW_REQUIRED" in decision.reason + sample["reviewDecision"] = "CHANGES_REQUESTED" + sample["autoMergeRequest"] = {"enabledAt": "2026-01-01T00:02:00Z"} + decision = inspect_pr( + "owner/repo", + sample, + dry_run=True, + trigger_reviews=True, + enable_auto_merge_flag=True, + update_branches=True, + workflow="OpenCode Review", + security_workflow="Strix Security Scan", + base_branch="main", + ) + assert decision.action == "disable_auto_merge" + assert "aggregate reviewDecision is CHANGES_REQUESTED" in decision.reason + sample["reviewDecision"] = "APPROVED" + sample["autoMergeRequest"] = None sample["restMergeableState"] = "BEHIND" decision = inspect_pr( "owner/repo", @@ -3276,7 +3443,13 @@ def self_test() -> None: sample["restMergeableState"] = "CLEAN" sample["autoMergeRequest"] = {"enabledAt": "2026-01-01T00:02:00Z"} sample["statusCheckRollup"]["contexts"]["nodes"] = [ - {"__typename": "CheckRun", "name": "strix", "status": "COMPLETED", "conclusion": "FAILURE"} + { + "__typename": "CheckRun", + "name": "strix", + "status": "COMPLETED", + "conclusion": "FAILURE", + "checkSuite": {"commit": {"oid": "abc"}}, + } ] decision = inspect_pr( "owner/repo", @@ -3349,7 +3522,12 @@ def self_test() -> None: assert "current-head OpenCode review requested changes" in decision.reason sample["autoMergeRequest"] = None sample["statusCheckRollup"]["contexts"]["nodes"].append( - {"__typename": "CheckRun", "name": "opencode-review", "status": "IN_PROGRESS"} + { + "__typename": "CheckRun", + "name": "opencode-review", + "status": "IN_PROGRESS", + "checkSuite": {"commit": {"oid": "abc"}}, + } ) assert opencode_in_progress(sample) sample["statusCheckRollup"]["contexts"]["nodes"] = [] @@ -3381,7 +3559,10 @@ def self_test() -> None: "name": "strix", "status": "COMPLETED", "conclusion": "SUCCESS", - "checkSuite": {"workflowRun": {"workflow": {"name": "Strix Security Scan"}}}, + "checkSuite": { + "commit": {"oid": "abc"}, + "workflowRun": {"workflow": {"name": "Strix Security Scan"}}, + }, } ] decision = inspect_pr( @@ -3457,7 +3638,13 @@ def self_test() -> None: ) assert decision.action == "update_branch" sample["statusCheckRollup"]["contexts"]["nodes"] = [ - {"__typename": "CheckRun", "name": "strix", "status": "COMPLETED", "conclusion": "FAILURE"} + { + "__typename": "CheckRun", + "name": "strix", + "status": "COMPLETED", + "conclusion": "FAILURE", + "checkSuite": {"commit": {"oid": "abc"}}, + } ] decision = inspect_pr( "owner/repo", @@ -3593,6 +3780,7 @@ def self_test() -> None: "name": "strix", "status": "COMPLETED", "conclusion": "SUCCESS", + "checkSuite": {"commit": {"oid": "abc"}}, } ] } diff --git a/scripts/ci/run_opencode_review_model_pool.sh b/scripts/ci/run_opencode_review_model_pool.sh index 986982e9a..cae6c6052 100644 --- a/scripts/ci/run_opencode_review_model_pool.sh +++ b/scripts/ci/run_opencode_review_model_pool.sh @@ -3,6 +3,80 @@ set -euo pipefail : "${GITHUB_OUTPUT:=/dev/null}" +run_with_timeout() { + local kill_after="$1" + local duration="$2" + shift 2 + + if command -v timeout >/dev/null 2>&1; then + timeout --kill-after="$kill_after" "$duration" "$@" + elif command -v gtimeout >/dev/null 2>&1; then + gtimeout --kill-after="$kill_after" "$duration" "$@" + else + local portable_pid portable_status + python3 "${GITHUB_WORKSPACE:-.}/scripts/ci/portable_timeout.py" \ + "$kill_after" "$duration" -- "$@" & + portable_pid=$! + trap 'kill "$portable_pid" 2>/dev/null || true' TERM INT + wait "$portable_pid" + portable_status=$? + trap - TERM INT + return "$portable_status" + fi +} + +signal_process_tree() { + local signum="$1" + local pid="$2" + local child pgid shell_pgid + for child in $(pgrep -P "$pid" 2>/dev/null || true); do + signal_process_tree "$signum" "$child" + done + pgid="$(process_group_id_for_pid "$pid")" + shell_pgid="$(ps -o pgid= -p "$$" 2>/dev/null | tr -d ' ')" + if [ -n "$pgid" ] && [ "$pgid" != "$shell_pgid" ]; then + signal_process_group "$signum" "$pgid" + else + kill "-$signum" "$pid" 2>/dev/null || true + fi +} + +process_group_id_for_pid() { + ps -o pgid= -p "$1" 2>/dev/null | tr -d ' ' +} + +signal_process_group() { + local signum="$1" + local pgid="$2" + local shell_pgid + shell_pgid="$(process_group_id_for_pid "$$")" + if [[ "$pgid" =~ ^[0-9]+$ ]] && [ "$pgid" != "$shell_pgid" ]; then + kill "-$signum" -- "-$pgid" 2>/dev/null || true + fi +} + +capture_process_group_ids() { + local pid="$1" + local child pgid + pgid="$(process_group_id_for_pid "$pid")" + if [ -n "$pgid" ]; then + printf '%s\n' "$pgid" + fi + for child in $(pgrep -P "$pid" 2>/dev/null || true); do + capture_process_group_ids "$child" + done +} + +signal_captured_process_groups() { + local signum="$1" + local captured_groups="$2" + local pgid + while IFS= read -r pgid; do + [ -n "$pgid" ] || continue + signal_process_group "$signum" "$pgid" + done <<<"$captured_groups" +} + record_review_status() { printf 'review_status=%s\n' "$1" >>"$GITHUB_OUTPUT" } @@ -456,16 +530,27 @@ run_one_model_attempt() { local opencode_json_file="$7" local opencode_export_file="$8" local run_timeout_seconds export_timeout_seconds opencode_status session_id opencode_stderr_file - local opencode_pid fatal_poll_seconds + local opencode_pid fatal_poll_seconds opencode_process_groups + local -a opencode_timeout_command run_timeout_seconds="${OPENCODE_RUN_TIMEOUT_SECONDS:-3600}" export_timeout_seconds="${OPENCODE_EXPORT_TIMEOUT_SECONDS:-120}" fatal_poll_seconds="${OPENCODE_FATAL_ERROR_POLL_SECONDS:-5}" opencode_stderr_file="${opencode_json_file}.stderr" + if command -v timeout >/dev/null 2>&1; then + opencode_timeout_command=(timeout --kill-after=30s "${run_timeout_seconds}s") + elif command -v gtimeout >/dev/null 2>&1; then + opencode_timeout_command=(gtimeout --kill-after=30s "${run_timeout_seconds}s") + else + opencode_timeout_command=( + python3 "${GITHUB_WORKSPACE:-.}/scripts/ci/portable_timeout.py" + 30s "${run_timeout_seconds}s" -- + ) + fi rm -f "$opencode_json_file" "$opencode_stderr_file" "$opencode_export_file" "$candidate_output_file" set +e - timeout --kill-after=30s "${run_timeout_seconds}s" \ + "${opencode_timeout_command[@]}" \ env -u GH_TOKEN -u GITHUB_TOKEN -u OPENCODE_APP_TOKEN \ -u ACTIONS_ID_TOKEN_REQUEST_TOKEN -u ACTIONS_ID_TOKEN_REQUEST_URL \ opencode run "$(cat "$prompt_file")" \ @@ -482,14 +567,19 @@ run_one_model_attempt() { # through to the next candidate within seconds instead of minutes. while kill -0 "$opencode_pid" 2>/dev/null; do if has_fatal_provider_error_event "$opencode_json_file"; then + opencode_process_groups="$(capture_process_group_ids "$opencode_pid")" printf 'OpenCode %s attempt %s/%s logged a fatal provider error while still running; killing the hung process instead of waiting out the %ss run timeout.\n' \ "$model_candidate" "$attempt" "$attempts" "$run_timeout_seconds" - kill "$opencode_pid" 2>/dev/null + signal_process_tree TERM "$opencode_pid" for _ in $(seq 1 30); do kill -0 "$opencode_pid" 2>/dev/null || break sleep 1 done - kill -9 "$opencode_pid" 2>/dev/null + if [ -n "$opencode_process_groups" ]; then + signal_captured_process_groups KILL "$opencode_process_groups" + else + signal_process_tree KILL "$opencode_pid" + fi break fi sleep "$fatal_poll_seconds" @@ -520,7 +610,7 @@ run_one_model_attempt() { fi return 1 fi - if ! timeout --kill-after=15s "${export_timeout_seconds}s" \ + if ! run_with_timeout 15s "${export_timeout_seconds}s" \ env -u GH_TOKEN -u GITHUB_TOKEN -u OPENCODE_APP_TOKEN \ -u ACTIONS_ID_TOKEN_REQUEST_TOKEN -u ACTIONS_ID_TOKEN_REQUEST_URL \ opencode export "$session_id" --pure >"$opencode_export_file"; then diff --git a/scripts/ci/strix_quick_gate.sh b/scripts/ci/strix_quick_gate.sh index 0f37f3460..89213c2fd 100755 --- a/scripts/ci/strix_quick_gate.sh +++ b/scripts/ci/strix_quick_gate.sh @@ -3902,7 +3902,7 @@ run_current_target_scan() { fi fi - if has_only_below_threshold_vulnerabilities; then + if [ "$strict_primary_provider_fallback" -eq 0 ] && has_only_below_threshold_vulnerabilities; then return 0 fi @@ -3979,7 +3979,7 @@ run_current_target_scan() { strict_fallback_provider_signal=1 fi - if has_only_below_threshold_vulnerabilities; then + if [ "$strict_fallback_provider_signal" -eq 0 ] && has_only_below_threshold_vulnerabilities; then return 0 fi diff --git a/scripts/ci/strix_required_workflow_smoke.sh b/scripts/ci/strix_required_workflow_smoke.sh index 8cd6dddad..172ba1003 100755 --- a/scripts/ci/strix_required_workflow_smoke.sh +++ b/scripts/ci/strix_required_workflow_smoke.sh @@ -157,7 +157,6 @@ assert_file_contains "$full_gate_test" "assert_strix_workflow_pr_trigger_hardene assert_file_contains "$workflow_file" "nvidia_nim/nvidia/nemotron-3-super-120b-a12b" "Strix defaults public scans to the current hosted NVIDIA NIM model" assert_file_contains "$workflow_file" "nvidia_nim/nvidia/llama-3.3-nemotron-super-49b-v1.5 github_models/openai/o3 github_models/openai/gpt-5-chat" "Strix tries another NVIDIA hosted model before GitHub Models" -assert_file_contains "$workflow_file" "Nvidia_nimException" "Strix workflow recognizes provider-scoped NVIDIA NIM failures" assert_file_contains "$gate_script" "is_nvidia_nim_not_found_error" "Strix gate classifies NVIDIA NIM model-catalog 404s" if [ "$failures" -ne 0 ]; then diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index 7343c06ac..eb3fa0554 100755 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -96,6 +96,136 @@ assert_file_not_contains() { fi } +extract_strix_workflow_contract_policy() { + local contract_file="$1" + local output_file="$2" + local start_line + local end_line + + start_line="$(grep -n 'ruby - .*workflow_file.*RUBY' "$contract_file" | head -1 | cut -d: -f1)" + end_line="$(awk -v start="$start_line" 'NR > start && /^[[:space:]]+RUBY$/ { print NR; exit }' "$contract_file")" + if [ -z "$start_line" ] || [ -z "$end_line" ] || [ "$end_line" -le "$((start_line + 1))" ]; then + record_failure "strix workflow contract policy heredoc is missing" + return 1 + fi + sed -n "$((start_line + 1)),$((end_line - 1))p" "$contract_file" | + sed 's/^ //' > "$output_file" +} + +assert_strix_workflow_contract_policy() { + local workflow_file="$REPO_ROOT/.github/workflows/strix.yml" + local contract_file="$REPO_ROOT/.github/workflows/strix-workflow-contract.yml" + local policy_file + local valid_fixture + local unreachable_fixture + local comment_fixture + + policy_file="$(mktemp)" + valid_fixture="$(mktemp)" + unreachable_fixture="$(mktemp)" + comment_fixture="$(mktemp)" + + if ! extract_strix_workflow_contract_policy "$contract_file" "$policy_file"; then + rm -f "$policy_file" "$valid_fixture" "$unreachable_fixture" "$comment_fixture" + return + fi + if ! ruby "$policy_file" "$workflow_file"; then + record_failure "strix workflow contract policy rejects the current trusted workflow" + fi + + cat >"$valid_fixture" <<'YAML' +jobs: + strix: + steps: + - name: Gate + run: | + if [ "$strix_rc" -eq 0 ]; then + exit 0 + fi + strix_rc=0 + echo "::error title=Strix evidence incomplete::Strix did not complete" + exit "$strix_rc" + - name: Collect Strix reports for artifact upload + run: echo collect + - name: Validate Strix report provenance + run: | + scan_stage_head_sha=abc + if ! jq -e '(.status == "completed") and (.scan_results.scan_completed == true) and (.scan_results.success == true)' "$candidate_run"; then continue; fi + candidate_metadata_matches=1 + if [ "$candidate_metadata_matches" -ne 1 ]; then continue; fi + successful_run_file=run.json + candidate_report="$(dirname -- "$candidate_run")/penetration_test_report.md" + if [ -s "$candidate_report" ]; then report_file="$candidate_report"; fi + gate_console="$GITHUB_WORKSPACE/strix_runs/gate-console.log" + if [ -f "$gate_console" ] && grep -Eiq fail-closed/provider-infrastructure "$gate_console"; then exit 1; fi + report_sha256="$(sha256sum "$report_file" | awk '{print $1}')" + jq -n '{head_sha:$head_sha, run_id:$run_id, run_json:$run_json, report:$report, report_sha256:$report_sha256, scan_completed:true}' > "$GITHUB_WORKSPACE/strix_runs/evidence-binding.json" + - name: Upload Strix reports artifact + run: echo upload +YAML + if ! ruby "$policy_file" "$valid_fixture"; then + record_failure "strix workflow contract policy rejects a structurally valid fail-closed fixture" + fi + + cat >"$unreachable_fixture" <<'YAML' +jobs: + strix: + steps: + - name: Gate + if: false + run: | + if [ "$strix_rc" -eq 0 ]; then exit 0; fi + echo "::error title=Strix evidence incomplete::Strix did not complete" + exit "$strix_rc" + - name: Collect Strix reports for artifact upload + run: echo collect + - name: Validate Strix report provenance + run: | + scan_stage_head_sha=abc + candidate_metadata_matches=1 + successful_run_file=run.json + candidate_report=penetration_test_report.md + echo evidence-binding.json + report_sha256=abc + echo scan_completed:true + echo fail-closed/provider-infrastructure marker + - name: Upload Strix reports artifact + run: echo upload +YAML + if ruby "$policy_file" "$unreachable_fixture"; then + record_failure "strix workflow contract policy accepts an unreachable fail-closed gate" + fi + + cat >"$comment_fixture" <<'YAML' +jobs: + strix: + steps: + - name: Gate + run: | + # strix_rc=0 + # echo "Strix evidence incomplete" + # exit "$strix_rc" + - name: Collect Strix reports for artifact upload + run: echo collect + - name: Validate Strix report provenance + run: | + # scan_stage_head_sha=abc + # candidate_metadata_matches=1 + # successful_run_file=run.json + # candidate_report=penetration_test_report.md + # echo evidence-binding.json + # report_sha256=abc + # echo scan_completed:true + # echo fail-closed/provider-infrastructure marker + - name: Upload Strix reports artifact + run: echo upload +YAML + if ruby "$policy_file" "$comment_fixture"; then + record_failure "strix workflow contract policy accepts comment-only security markers" + fi + rm -f "$policy_file" "$valid_fixture" "$unreachable_fixture" "$comment_fixture" +} + seal_opencode_test_artifacts() { local runner_temp="$1" local head_sha="$2" @@ -175,7 +305,19 @@ assert_strix_pr_scope_includes_deployment_context() { assert_strix_workflow_pr_trigger_hardened() { local workflow_file="$REPO_ROOT/.github/workflows/strix.yml" - + local workflow_contract_file="$REPO_ROOT/.github/workflows/strix-workflow-contract.yml" + + assert_file_contains "$workflow_contract_file" "pull_request:" "strix workflow contract uses an unprivileged PR trigger" + assert_file_contains "$workflow_contract_file" "workflow-contract:" "strix workflow contract has a dedicated data-only job" + assert_file_contains "$workflow_contract_file" "Read PR Strix workflow as data" "strix workflow contract reads the PR workflow as data" + assert_file_contains "$workflow_contract_file" "HEAD_REPOSITORY" "strix workflow contract binds the PR head repository" + assert_file_contains "$workflow_contract_file" "HEAD_SHA" "strix workflow contract binds the PR head SHA" + assert_file_contains "$workflow_contract_file" "evidence-binding.json" "strix workflow contract requires evidence binding" + assert_file_contains "$workflow_contract_file" "Strix workflow still neutralizes missing security evidence" "strix workflow contract rejects the old neutral-success branch" + assert_file_contains "$workflow_contract_file" "Psych.safe_load" "strix workflow contract parses workflow structure instead of trusting marker strings" + assert_file_contains "$workflow_contract_file" "statically_reachable?" "strix workflow contract rejects unreachable security steps" + assert_file_contains "$workflow_contract_file" "executable_source" "strix workflow contract ignores comment-only markers" + assert_strix_workflow_contract_policy assert_file_contains "$workflow_file" "branches: [main, develop, master]" "strix workflow scans GitHub Flow and Git Flow protected branches" assert_file_contains "$workflow_file" "pull_request_target:" "strix workflow uses trusted PR trigger" assert_file_contains "$workflow_file" 'strix-${{ github.event_name }}-' "strix workflow isolates manual evidence runs from required PR contexts" @@ -226,6 +368,20 @@ assert_strix_workflow_pr_trigger_hardened() { assert_file_contains "$workflow_file" "bash \"\$TRUSTED_STRIX_GATE\"" "strix workflow executes trusted temp gate script" assert_file_contains "$workflow_file" "Collect Strix reports for artifact upload" "strix workflow preserves reports from trusted workspace" assert_file_contains "$workflow_file" "scan-summary.txt" "strix workflow creates a fallback artifact when Strix emits no report files" + assert_file_contains "$workflow_file" "Validate Strix report provenance" "strix workflow validates structured report provenance before publishing evidence" + assert_file_contains "$workflow_file" "scan_results.scan_completed == true" "strix workflow requires a completed Strix scan result" + assert_file_contains "$workflow_file" "strix_scan_head_sha" "strix workflow records the head SHA at scan start" + assert_file_contains "$workflow_file" "scan-head-sha.txt" "strix workflow preserves the scan-stage head SHA artifact" + assert_file_contains "$workflow_file" "candidate_head_sha" "strix workflow binds each candidate report to a head SHA" + assert_file_contains "$workflow_file" "candidate_metadata_count" "strix workflow inspects every candidate head metadata field" + assert_file_contains "$workflow_file" "candidate_metadata_matches" "strix workflow rejects conflicting candidate head metadata" + assert_file_contains "$workflow_file" "scan_stage_head_sha" "strix workflow falls back to the scan-stage head SHA only when run metadata is absent" + assert_file_contains "$workflow_file" "__invalid_metadata_type__" "strix workflow rejects non-string candidate head metadata" + assert_file_contains "$workflow_file" "((.scan_results // {}).commit_sha)" "strix workflow checks alternate structured commit metadata" + assert_file_contains "$workflow_file" "evidence-binding.json" "strix workflow binds the report artifact to the scanned head SHA" + assert_file_contains "$workflow_file" "fail-closed/provider-infrastructure marker" "strix workflow rejects provider-failure evidence even when a report exists" + assert_file_contains "$GATE_SCRIPT" 'strict_primary_provider_fallback" -eq 0 ] && has_only_below_threshold_vulnerabilities' "strix gate cannot bypass strict primary provider fallback with below-threshold findings" + assert_file_contains "$GATE_SCRIPT" 'strict_fallback_provider_signal" -eq 0 ] && has_only_below_threshold_vulnerabilities' "strix gate cannot bypass strict fallback provider signal with below-threshold findings" local checkout_count checkout_count="$(grep -Fc "uses: actions/checkout@" "$workflow_file")" assert_equals "1" "$checkout_count" "strix workflow uses actions/checkout exactly once for the central trusted source" @@ -708,7 +864,8 @@ assert_opencode_review_uses_codegraph_and_gpt5_fallback() { assert_file_contains "$workflow_file" "Never return raw tool-call markup" "opencode review prompt forbids raw tool-call transcripts as final review output" assert_file_contains "$workflow_file" "Do not spend the session listing every changed path before reviewing" "opencode review prompt prevents fallback sessions from exhausting steps on file listing" assert_file_contains "$workflow_file" "Always return a final control block instead of a progress summary" "opencode review prompt requires a gate conclusion instead of a progress summary" - assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" 'timeout --kill-after=30s "${run_timeout_seconds}s"' "opencode review model pool has a kill-after bounded timeout" + assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" "opencode_timeout_command=(" "opencode review model pool launches the timeout executable directly for reliable PID tracking" + assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" "signal_process_tree" "opencode review model pool cleans up the timeout process tree and process group" assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" 'env -u GH_TOKEN -u GITHUB_TOKEN -u OPENCODE_APP_TOKEN' "opencode review model pool scrubs GitHub credentials before model execution" assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" "assert_reasoning_effort_for_candidate" "opencode review validates high reasoning effort before running capable model candidates" assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" "assert_opencode_reasoning_effort.py" "opencode review reuses the central reasoning effort guard" @@ -725,7 +882,8 @@ assert_opencode_review_uses_codegraph_and_gpt5_fallback() { assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" "not a generic model-exhaustion message" "opencode review tells models to return concrete missing-evidence findings instead of progress-only output" assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" "tokens_limit_reached" "opencode review detects provider context-window overflow" assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" "skipping remaining attempts for this model" "opencode review skips same-model retries after context-window overflow" - assert_file_contains "$REPO_ROOT/.github/workflows/strix.yml" "exceeded your current quota" "strix wrapper neutralizes quota-only provider failures without vulnerability reports" + assert_file_contains "$REPO_ROOT/.github/workflows/strix.yml" "Strix evidence incomplete" "strix wrapper fails closed when provider failure prevents a completed report" + assert_file_not_contains "$REPO_ROOT/.github/workflows/strix.yml" "neutral skip so an infrastructure outage does not block merges" "strix wrapper never converts missing security evidence into a pass" assert_file_contains "$REPO_ROOT/scripts/ci/strix_quick_gate.sh" "billing details" "strix quick gate classifies provider quota starvation as infrastructure" assert_file_contains "$workflow_file" 'timeout-minutes: 325' "opencode review target contains evidence, the bounded long-review pool, publication, Noema handoff, and cleanup overhead" assert_file_contains "$workflow_file" 'timeout-minutes: 12' "opencode evidence preparation fails closed before it ties up the review queue" diff --git a/tests/test_agent_mention_complete_payload_binding.py b/tests/test_agent_mention_complete_payload_binding.py index 04562e93f..85a0a1a00 100644 --- a/tests/test_agent_mention_complete_payload_binding.py +++ b/tests/test_agent_mention_complete_payload_binding.py @@ -122,6 +122,15 @@ def test_invocation_claim_binds_all_security_relevant_fields() -> None: "trigger_reviews": True, "update_branches": False, } + opencode_payload = router.opencode_payload(request)["client_payload"] + assert opencode_payload["review_policy"] == { + "trigger_reviews": True, + "review_dispatch_limit": "1", + "enable_auto_merge": False, + "update_branches": False, + "merge_mode": "disabled", + } + assert len(opencode_payload) <= 10 noema_key = router.agent_invocation_key(request, "cwl-noema-review") opencode_key = router.agent_invocation_key(request, "opencode-agent") diff --git a/tests/test_agent_mention_downstream_idempotency.py b/tests/test_agent_mention_downstream_idempotency.py index 4fc40a782..4427795e0 100644 --- a/tests/test_agent_mention_downstream_idempotency.py +++ b/tests/test_agent_mention_downstream_idempotency.py @@ -33,7 +33,7 @@ def test_downstream_workflows_claim_artifacts_and_bind_exact_key() -> None: assert "source_comment_id" in text assert "requested_agent" in text assert "cancel-in-progress: false" in text - assert "queue: max" in text + assert "queue: max" not in text assert "^[0-9a-f]{64}$" in text assert "^[1-9][0-9]*$" in text assert "actions/artifacts" in text diff --git a/tests/test_agent_mention_idempotency.py b/tests/test_agent_mention_idempotency.py index 499730a22..4c3bb2b97 100644 --- a/tests/test_agent_mention_idempotency.py +++ b/tests/test_agent_mention_idempotency.py @@ -320,13 +320,12 @@ def test_reaction_or_ack_failure_cannot_redispatch_completed_agents() -> None: mention_request = request(module) central = ArtifactAwareClient() failing_target = ArtifactAwareClient(fail_target_call=1) - with pytest.raises(RuntimeError, match="target call"): - module.dispatch_request( - mention_request, - target_client=failing_target, - dispatch_client=central, - opencode_allowlist=frozenset({mention_request.repository}), - ) + assert module.dispatch_request( + mention_request, + target_client=failing_target, + dispatch_client=central, + opencode_allowlist=frozenset({mention_request.repository}), + ) == ("@cwl-noema-review", "@opencode-agent") assert dispatch_events(central) == [ "agent-mention-noema", "agent-mention-opencode", diff --git a/tests/test_agent_mention_router.py b/tests/test_agent_mention_router.py index 4509d43f0..d347fea99 100644 --- a/tests/test_agent_mention_router.py +++ b/tests/test_agent_mention_router.py @@ -220,11 +220,17 @@ def test_eligible_agents_and_payloads() -> None: assert noema["client_payload"]["pr_base_sha"] == "b" * 40 opencode = module.opencode_payload(request) assert opencode["event_type"] == "agent-mention-opencode" - assert opencode["client_payload"]["base_branch"] == "develop" - assert opencode["client_payload"]["pr_base_sha"] == "b" * 40 - assert opencode["client_payload"]["merge_mode"] == "disabled" - assert opencode["client_payload"]["enable_auto_merge"] is False - assert opencode["client_payload"]["update_branches"] is False + payload = opencode["client_payload"] + assert payload["base_branch"] == "develop" + assert payload["pr_base_sha"] == "b" * 40 + assert payload["review_policy"] == { + "trigger_reviews": True, + "review_dispatch_limit": "1", + "enable_auto_merge": False, + "update_branches": False, + "merge_mode": "disabled", + } + assert len(payload) <= 10 def test_dispatch_uses_central_events_and_acknowledges() -> None: diff --git a/tests/test_install_base_python_locks.py b/tests/test_install_base_python_locks.py index 4f1feebe6..ed280c6a2 100644 --- a/tests/test_install_base_python_locks.py +++ b/tests/test_install_base_python_locks.py @@ -232,6 +232,39 @@ def fake_runner(command: list[str], **kwargs): assert "candidates=1 installed=0 skipped=1" in stdout.getvalue() +def test_unavailable_binary_version_for_runner_interpreter_is_nonfatal(tmp_path) -> None: + """A pinned package without a compatible runner binary may defer safely.""" + write_candidate( + tmp_path, + generated_file="requirements-000.txt", + source="fuzz/requirements-atheris.txt", + ) + + def fake_runner(command: list[str], **kwargs): + return subprocess.CompletedProcess( + command, + 1, + stdout=( + "ERROR: Could not find a version that satisfies the requirement " + "atheris==3.0.0 (from versions: 3.1.0)\n" + "ERROR: No matching distribution found for atheris==3.0.0" + ), + ) + + stdout = io.StringIO() + stderr = io.StringIO() + result = installer.install_materialized_locks( + tmp_path, + runner=fake_runner, + stdout=stdout, + stderr=stderr, + ) + + assert result == 0 + assert "atheris==3.0.0" in stderr.getvalue() + assert "candidates=1 installed=0 skipped=1" in stdout.getvalue() + + def test_fatal_same_directory_group_failure_aborts(tmp_path) -> None: """A group cannot turn a registry or integrity failure into a skip.""" write_candidate( diff --git a/tests/test_materialize_base_python_requirements.py b/tests/test_materialize_base_python_requirements.py index 8a383f0c2..8699da69d 100644 --- a/tests/test_materialize_base_python_requirements.py +++ b/tests/test_materialize_base_python_requirements.py @@ -6,6 +6,7 @@ import subprocess import sys import tarfile +from urllib.error import HTTPError from pathlib import Path import pytest @@ -526,6 +527,7 @@ def test_download_trusted_uv_archive_rejects_network_and_size_failures( monkeypatch: pytest.MonkeyPatch, ) -> None: """Network errors and oversized archives cannot enter the trusted tool path.""" + monkeypatch.setattr(materializer.time, "sleep", lambda _seconds: None) monkeypatch.setattr( materializer.urllib.request, "urlopen", @@ -541,6 +543,64 @@ def test_download_trusted_uv_archive_rejects_network_and_size_failures( materializer._download_trusted_uv_archive() +def test_download_trusted_uv_archive_retries_transient_http_error( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A transient upstream response is retried without changing the trust boundary.""" + calls = 0 + sleeps: list[float] = [] + response = FakeHttpResponse(materializer.TRUSTED_UV_ARCHIVE_URL, b"archive") + + def flaky_urlopen(*_args: object, **_kwargs: object) -> FakeHttpResponse: + nonlocal calls + calls += 1 + if calls == 1: + raise HTTPError( + materializer.TRUSTED_UV_ARCHIVE_URL, + 503, + "temporarily unavailable", + hdrs=None, + fp=None, + ) + return response + + monkeypatch.setattr(materializer.urllib.request, "urlopen", flaky_urlopen) + monkeypatch.setattr(materializer.time, "sleep", sleeps.append) + + assert materializer._download_trusted_uv_archive() == b"archive" + assert calls == 2 + assert sleeps == [materializer.TRUSTED_UV_RETRY_BACKOFF_SECONDS] + + +@pytest.mark.parametrize("status", [403, 503]) +def test_download_trusted_uv_archive_reports_http_status_after_blocking_failure( + monkeypatch: pytest.MonkeyPatch, + status: int, +) -> None: + """A non-retryable or exhausted HTTP failure remains an explicit blocker.""" + calls = 0 + sleeps: list[float] = [] + + def blocked_urlopen(*_args: object, **_kwargs: object) -> object: + nonlocal calls + calls += 1 + raise HTTPError( + materializer.TRUSTED_UV_ARCHIVE_URL, + status, + "blocked", + hdrs=None, + fp=None, + ) + + monkeypatch.setattr(materializer.urllib.request, "urlopen", blocked_urlopen) + monkeypatch.setattr(materializer.time, "sleep", sleeps.append) + + with pytest.raises(RuntimeError, match=f"HTTPError {status}"): + materializer._download_trusted_uv_archive() + assert calls == (1 if status == 403 else materializer.TRUSTED_UV_DOWNLOAD_ATTEMPTS) + assert len(sleeps) == calls - 1 + + def test_verified_uv_binary_accepts_exact_archive( monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -644,6 +704,9 @@ def test_install_trusted_uv_verifies_version_and_caches_path( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: """The installer writes one executable, verifies its version, and caches it.""" + materializer._install_trusted_uv.cache_clear() + monkeypatch.setattr(materializer.sys, "platform", "linux") + monkeypatch.setattr(materializer.platform, "machine", lambda: "x86_64") tool_dir = tmp_path / "uv" monkeypatch.setattr( materializer.tempfile, @@ -690,6 +753,9 @@ def test_install_trusted_uv_rejects_version_process_failures( failure: OSError | subprocess.TimeoutExpired, ) -> None: """A missing or hung downloaded executable is removed and rejected.""" + materializer._install_trusted_uv.cache_clear() + monkeypatch.setattr(materializer.sys, "platform", "linux") + monkeypatch.setattr(materializer.platform, "machine", lambda: "x86_64") tool_dir = tmp_path / "uv" monkeypatch.setattr( materializer.tempfile, @@ -721,6 +787,9 @@ def test_install_trusted_uv_rejects_wrong_version_or_exit_status( completed: subprocess.CompletedProcess[bytes], ) -> None: """Unexpected version output or a nonzero status cannot satisfy the pin.""" + materializer._install_trusted_uv.cache_clear() + monkeypatch.setattr(materializer.sys, "platform", "linux") + monkeypatch.setattr(materializer.platform, "machine", lambda: "x86_64") tool_dir = tmp_path / f"uv-{completed.returncode}-{len(completed.stdout)}" monkeypatch.setattr( materializer.tempfile, diff --git a/tests/test_opencode_agent_contract.py b/tests/test_opencode_agent_contract.py index daeaa37a2..d47ec7e04 100644 --- a/tests/test_opencode_agent_contract.py +++ b/tests/test_opencode_agent_contract.py @@ -1296,7 +1296,7 @@ def test_workflow_provisions_sandbox_tool_and_reviewer_agent(): assert "variants.high.reasoningEffort=high" in reasoning_effort_guard assert "deepseek/deepseek-r1" in reasoning_effort_guard assert '--config "$OPENCODE_REVIEW_WORKDIR/opencode.jsonc"' in workflow - assert 'timeout --kill-after=15s "${export_timeout_seconds}s"' in model_pool_runner + assert "run_with_timeout 15s \"${export_timeout_seconds}s\"" in model_pool_runner assert "opencode export" in model_pool_runner assert "env -u GH_TOKEN -u GITHUB_TOKEN -u OPENCODE_APP_TOKEN" in model_pool_runner assert "session export did not complete within %ss" in model_pool_runner diff --git a/tests/test_opencode_model_pool_runner.py b/tests/test_opencode_model_pool_runner.py index 08d17f000..33ea789b6 100644 --- a/tests/test_opencode_model_pool_runner.py +++ b/tests/test_opencode_model_pool_runner.py @@ -157,14 +157,40 @@ def run_failed_model( fake_opencode.write_text( "#!/usr/bin/env bash\n" 'if [ "${1:-}" = run ]; then\n' - ' [ -z "${FAKE_OPENCODE_PROMPT_CAPTURE:-}" ] || printf \'%s\\n\' "$2" > "$FAKE_OPENCODE_PROMPT_CAPTURE"\n' + ' prompt="${2:-}"\n' + ' model=""\n' + ' while [ "$#" -gt 0 ]; do\n' + ' if [ "${1:-}" = "--model" ] && [ "$#" -ge 2 ]; then\n' + ' model="$2"\n' + ' shift 2\n' + ' else\n' + ' shift\n' + ' fi\n' + ' done\n' + ' [ -z "${FAKE_OPENCODE_MODEL_LOG:-}" ] || printf \'%s\\n\' "$model" >> "$FAKE_OPENCODE_MODEL_LOG"\n' + ' if [ "$model" = "${FAKE_OPENCODE_NEXT_MODEL:-}" ] && [ -f "${FAKE_OPENCODE_CHILD_PID_FILE:-}" ]; then\n' + ' child_pid="$(tr -d \'[:space:]\' < "$FAKE_OPENCODE_CHILD_PID_FILE")"\n' + ' if [ -n "$child_pid" ] && kill -0 "$child_pid" 2>/dev/null; then\n' + ' : > "${FAKE_OPENCODE_OVERLAP_FILE:?}"\n' + ' fi\n' + ' fi\n' + ' if [ "$model" = "${FAKE_OPENCODE_FATAL_MODEL:-}" ]; then\n' + ' (trap "" TERM; sleep "${FAKE_OPENCODE_CHILD_SLEEP_SECONDS:-120}") &\n' + ' child_pid=$!\n' + ' [ -z "${FAKE_OPENCODE_CHILD_PID_FILE:-}" ] || printf \'%s\' "$child_pid" > "$FAKE_OPENCODE_CHILD_PID_FILE"\n' + ' fi\n' + ' [ -z "${FAKE_OPENCODE_PROMPT_CAPTURE:-}" ] || printf \'%s\\n\' "$prompt" > "$FAKE_OPENCODE_PROMPT_CAPTURE"\n' ' [ -z "${FAKE_OPENCODE_JSON:-}" ] || printf \'%s\\n\' "$FAKE_OPENCODE_JSON"\n' ' [ -z "${FAKE_OPENCODE_STDERR:-}" ] || printf \'%s\\n\' "$FAKE_OPENCODE_STDERR" >&2\n' - ' sleep "${FAKE_OPENCODE_HANG_SECONDS:-0}"\n' + ' if [ -n "${FAKE_OPENCODE_FATAL_MODEL:-}" ] && [ "$model" != "$FAKE_OPENCODE_FATAL_MODEL" ]; then\n' + ' sleep "${FAKE_OPENCODE_NON_FATAL_HANG_SECONDS:-0}"\n' + ' else\n' + ' sleep "${FAKE_OPENCODE_HANG_SECONDS:-0}"\n' + ' fi\n' ' exit "${FAKE_OPENCODE_RUN_EXIT:-1}"\n' "fi\n" 'if [ "${1:-}" = export ]; then\n' - ' [ -z "${FAKE_OPENCODE_EXPORT:-}" ] || printf \'%s\\n\' "$FAKE_OPENCODE_EXPORT"\n' + ' if [ -n "${FAKE_OPENCODE_SUCCESS_EXPORT:-}" ]; then printf \'%s\\n\' "$FAKE_OPENCODE_SUCCESS_EXPORT"; else [ -z "${FAKE_OPENCODE_EXPORT:-}" ] || printf \'%s\\n\' "$FAKE_OPENCODE_EXPORT"; fi\n' ' exit "${FAKE_OPENCODE_EXPORT_EXIT:-0}"\n' "fi\n" "printf 'unexpected fake opencode command: %s\\n' \"$*\" >&2\n" @@ -583,6 +609,47 @@ def test_fatal_provider_error_kills_hung_opencode_run_early( assert elapsed < 25 +def test_fatal_cleanup_kills_term_ignoring_child_before_next_model( + tmp_path: Path, +) -> None: + """Captured process groups prevent a killed provider child from overlapping failover.""" + child_pid_file = tmp_path / "child.pid" + model_log = tmp_path / "models.log" + overlap_file = tmp_path / "overlap" + result = run_failed_model( + tmp_path, + json_line=( + '{"type":"error","error":{"name":"ProviderQuotaError","data":' + '{"message":"insufficient_quota: request rejected"}}}' + ), + model_candidates="openrouter/fatal openrouter/next", + extra_env={ + "OPENROUTER_API_KEY": "fake-openrouter-key", + "FAKE_OPENCODE_FATAL_MODEL": "openrouter/fatal", + "FAKE_OPENCODE_CHILD_PID_FILE": bash_path(child_pid_file), + "FAKE_OPENCODE_CHILD_SLEEP_SECONDS": "120", + "FAKE_OPENCODE_MODEL_LOG": bash_path(model_log), + "FAKE_OPENCODE_NEXT_MODEL": "openrouter/next", + "FAKE_OPENCODE_OVERLAP_FILE": bash_path(overlap_file), + "FAKE_OPENCODE_HANG_SECONDS": "120", + "FAKE_OPENCODE_NON_FATAL_HANG_SECONDS": "0", + "OPENCODE_RUN_TIMEOUT_SECONDS": "120", + "OPENCODE_TOTAL_RETRY_BUDGET_SECONDS": "240", + }, + ) + + assert result.returncode == 1 + assert model_log.read_text(encoding="utf-8").splitlines() == [ + "openrouter/fatal", + "openrouter/next", + ] + assert not overlap_file.exists() + child_pid = int(child_pid_file.read_text(encoding="utf-8")) + assert subprocess.run( + ["kill", "-0", str(child_pid)], check=False + ).returncode != 0 + + def test_model_text_quoting_error_signatures_does_not_kill_run(tmp_path: Path) -> None: """Model prose mentioning fatal signatures never kills a healthy streaming run.""" result = run_failed_model( diff --git a/tests/test_portable_timeout.py b/tests/test_portable_timeout.py new file mode 100644 index 000000000..1965669a4 --- /dev/null +++ b/tests/test_portable_timeout.py @@ -0,0 +1,163 @@ +"""Checks for the stdlib timeout fallback used by local review tests.""" + +from __future__ import annotations + +import importlib.util +import signal +import subprocess +import sys +from types import SimpleNamespace +from pathlib import Path + +import pytest + + +SCRIPT = Path(__file__).parents[1] / "scripts" / "ci" / "portable_timeout.py" + + +def _load_module(): + spec = importlib.util.spec_from_file_location("portable_timeout_under_test", SCRIPT) + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +class _FakeProcess: + pid = 123 + + def __init__(self, waits): + self._waits = iter(waits) + + def wait(self, timeout=None): + del timeout + result = next(self._waits) + if isinstance(result, BaseException): + raise result + return result + + +def test_portable_timeout_returns_child_status() -> None: + result = subprocess.run( + [ + sys.executable, + str(SCRIPT), + "1s", + "5s", + "--", + sys.executable, + "-c", + "print('ok')", + ], + capture_output=True, + text=True, + check=False, + timeout=5, + ) + + assert result.returncode == 0 + assert result.stdout == "ok\n" + + +def test_portable_timeout_returns_124_after_killing_child() -> None: + result = subprocess.run( + [ + sys.executable, + str(SCRIPT), + "0.1s", + "0.1s", + "--", + sys.executable, + "-c", + "import time; time.sleep(2)", + ], + capture_output=True, + text=True, + check=False, + timeout=5, + ) + + assert result.returncode == 124 + + +def test_helpers_cover_duration_validation_signals_and_termination(monkeypatch) -> None: + module = _load_module() + assert module._seconds("1.5s") == 1.5 + assert module._seconds("2") == 2.0 + with pytest.raises(ValueError, match="invalid duration"): + module._seconds("tomorrow") + + signals = [] + monkeypatch.setattr(module.os, "killpg", lambda pid, signum: signals.append((pid, signum))) + process = SimpleNamespace(pid=9) + module._signal_process_group(process, signal.SIGTERM) + assert signals == [(9, signal.SIGTERM)] + + def missing_process_group(_pid, _signum): + raise ProcessLookupError + + monkeypatch.setattr(module.os, "killpg", missing_process_group) + module._signal_process_group(process, signal.SIGTERM) + + immediate = _FakeProcess([7]) + assert module._terminate(immediate, 0.1) == 7 + + killed = _FakeProcess( + [subprocess.TimeoutExpired(["fake"], 0.1), 9] + ) + assert module._terminate(killed, 0.1) == 9 + + +def test_main_rejects_invalid_arguments_and_start_failure(monkeypatch, capsys) -> None: + module = _load_module() + assert module.main([]) == 2 + assert module.main(["1", "--", "echo"]) == 2 + assert module.main(["1", "2", "--"]) == 2 + assert module.main(["bad", "1", "--", "echo"]) == 2 + + def cannot_start(*_args, **_kwargs): + raise OSError("synthetic start failure") + + monkeypatch.setattr(module.subprocess, "Popen", cannot_start) + assert module.main(["1", "1", "--", "echo"]) == 127 + assert "synthetic start failure" in capsys.readouterr().err + + +def test_main_zero_duration_and_signal_forwarding(monkeypatch) -> None: + module = _load_module() + process = _FakeProcess([17]) + handlers = {} + monkeypatch.setattr(module.subprocess, "Popen", lambda *_args, **_kwargs: process) + monkeypatch.setattr( + module.signal, + "signal", + lambda signum, handler: handlers.__setitem__(signum, handler), + ) + assert module.main(["1", "0", "--", "echo"]) == 17 + + forwarded = [] + monkeypatch.setattr( + module, + "_signal_process_group", + lambda target, signum: forwarded.append((target, signum)), + ) + with pytest.raises(SystemExit, match="143"): + handlers[signal.SIGTERM](signal.SIGTERM, None) + assert forwarded == [(process, signal.SIGTERM)] + + +def test_main_waits_for_child_and_terminates_after_deadline(monkeypatch) -> None: + module = _load_module() + waiting = _FakeProcess( + [subprocess.TimeoutExpired(["fake"], 1), 0] + ) + monkeypatch.setattr(module.subprocess, "Popen", lambda *_args, **_kwargs: waiting) + monkeypatch.setattr(module.time, "monotonic", iter([0.0, 0.1, 0.2]).__next__) + assert module.main(["1", "1", "--", "echo"]) == 0 + + timed_out = _FakeProcess( + [subprocess.TimeoutExpired(["fake"], 0.1), 9] + ) + monkeypatch.setattr(module.subprocess, "Popen", lambda *_args, **_kwargs: timed_out) + monkeypatch.setattr(module.time, "monotonic", iter([0.0, 2.0]).__next__) + assert module.main(["0.1", "0.1", "--", "echo"]) == 124 diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 3e421e903..5fe50fd93 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -64,6 +64,39 @@ def make_pr(**overrides): "statusCheckRollup": {"contexts": {"nodes": []}}, } value.update(overrides) + if "reviewDecision" not in overrides: + current_reviews = [ + review + for review in value["reviews"]["nodes"] + if sched.review_matches_current_head(review, value) + ] + if any((review.get("state") or "").upper() == "CHANGES_REQUESTED" for review in current_reviews): + value["reviewDecision"] = "CHANGES_REQUESTED" + elif any((review.get("state") or "").upper() == "APPROVED" for review in current_reviews): + value["reviewDecision"] = "APPROVED" + if "statusCheckRollup" not in overrides and value.get("reviewDecision") == "APPROVED": + value["statusCheckRollup"] = { + "contexts": { + "nodes": [ + { + "__typename": "CheckRun", + "name": "strix", + "status": "COMPLETED", + "conclusion": "SUCCESS", + "checkSuite": {"workflowRun": {"workflow": {"name": "Strix Security Scan"}}}, + } + ] + } + } + for node in value["statusCheckRollup"]["contexts"]["nodes"]: + if node.get("__typename") != "CheckRun": + continue + check_suite = node.setdefault("checkSuite", {}) + if not isinstance(check_suite, dict): + continue + commit = check_suite.setdefault("commit", {}) + if isinstance(commit, dict): + commit.setdefault("oid", value["headRefOid"]) return value @@ -592,6 +625,7 @@ def fake_api(path): assert node["reviews"]["nodes"][0]["commit"]["oid"] == "abc123" assert node["statusCheckRollup"]["contexts"]["nodes"][0]["status"] == "COMPLETED" assert node["statusCheckRollup"]["contexts"]["nodes"][0]["conclusion"] == "SUCCESS" + assert node["statusCheckRollup"]["contexts"]["nodes"][0]["checkSuite"]["commit"]["oid"] == "abc123" def test_fetch_pr_falls_back_to_rest_when_graphql_denied(monkeypatch): @@ -907,6 +941,7 @@ def test_context_review_and_check_helpers(monkeypatch): monkeypatch.delenv("SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY", raising=False) assert sched.context_nodes({}) == [] assert sched.context_nodes(make_pr()) == [] + assert "commit { oid }" in sched.PULL_REQUEST_FIELDS_FRAGMENT assert sched.compare_behind_by({"compareBehindBy": "2"}) == 2 assert sched.compare_behind_by({"compareBehindBy": "unknown"}) == 0 assert sched.is_opencode_context({"__typename": "CheckRun", "name": "opencode-review"}) @@ -941,6 +976,10 @@ def test_context_review_and_check_helpers(monkeypatch): assert sched.matching_actions_job_id(check_jobs, sched.is_strix_context) == "22" no_job_url = make_pr(statusCheckRollup={"contexts": {"nodes": [opencode_check()]}}) assert sched.matching_actions_job_id(no_job_url, sched.is_opencode_context) is None + stale_job = opencode_check(details_url="https://github.com/owner/repo/actions/runs/3/job/33") + stale_job["checkSuite"]["commit"] = {"oid": "old-head"} + stale_job_pr = make_pr(statusCheckRollup={"contexts": {"nodes": [stale_job]}}) + assert sched.matching_actions_job_id(stale_job_pr, sched.is_opencode_context) is None assert sched.parse_github_datetime(None) is None assert sched.parse_github_datetime("not-a-date") is None @@ -969,6 +1008,12 @@ def test_context_review_and_check_helpers(monkeypatch): running = make_pr(statusCheckRollup={"contexts": {"nodes": [opencode_check()]}}) assert sched.opencode_in_progress(running) assert sched.opencode_progress_state(running, stale_after_minutes=45) == "running" + stale_opencode = opencode_check() + stale_opencode["checkSuite"]["commit"] = {"oid": "old-head"} + stale_and_current = make_pr( + statusCheckRollup={"contexts": {"nodes": [stale_opencode, opencode_check()]}} + ) + assert sched.opencode_progress_state(stale_and_current, stale_after_minutes=45) == "running" recent_running = make_pr( statusCheckRollup={ "contexts": { @@ -1014,19 +1059,35 @@ def test_context_review_and_check_helpers(monkeypatch): assert sched.opencode_progress_state(unrelated, stale_after_minutes=45) == "absent" assert sched.strix_evidence_state(make_pr()) == "missing" assert sched.strix_evidence_state(unrelated) == "running" + assert sched.running_status_checks(unrelated) == ["strix"] mixed_contexts = make_pr( statusCheckRollup={"contexts": {"nodes": [{"context": "lint", "state": "SUCCESS"}, strix_check()]}} ) assert sched.strix_evidence_state(mixed_contexts) == "complete" + assert sched.running_status_checks(mixed_contexts) == [] unknown_running = make_pr( statusCheckRollup={"contexts": {"nodes": [strix_check(status="", conclusion="")]}} ) assert sched.strix_evidence_state(unknown_running) == "running" assert sched.strix_evidence_state(make_pr(statusCheckRollup={"contexts": {"nodes": [strix_check()]}})) == "complete" - assert ( - sched.strix_evidence_state(make_pr(statusCheckRollup={"contexts": {"nodes": [strix_check(conclusion="FAILURE")]}})) - == "complete" - ) + assert sched.strix_evidence_state( + make_pr(statusCheckRollup={"contexts": {"nodes": [strix_check(conclusion="FAILURE")]}}) + ) == "failed" + assert sched.strix_evidence_state( + make_pr(statusCheckRollup={"contexts": {"nodes": [{"context": "strix", "state": "NEUTRAL"}]}}) + ) == "failed" + assert sched.strix_evidence_state( + make_pr(statusCheckRollup={"contexts": {"nodes": [{"context": "strix", "state": "SUCCESS"}]}}) + ) == "complete" + stale_strix = strix_check() + stale_strix["checkSuite"]["commit"] = {"oid": "old-head"} + stale_pr = make_pr(statusCheckRollup={"contexts": {"nodes": [stale_strix]}}) + assert sched.strix_evidence_state(stale_pr) == "missing" + assert sched.running_status_checks(stale_pr) == ["strix (not bound to current head)"] + fresh_strix = strix_check() + mixed_heads = make_pr(statusCheckRollup={"contexts": {"nodes": [stale_strix, fresh_strix]}}) + assert sched.strix_evidence_state(mixed_heads) == "complete" + assert sched.running_status_checks(mixed_heads) == [] threaded = make_pr( reviewThreads={ @@ -1332,6 +1393,20 @@ def test_review_state_and_failed_checks(): ) assert sched.failed_status_checks(action_required) == [] assert sched.action_required_checks(action_required) == ["opencode-review"] + stale_action_required = opencode_check(status="COMPLETED") + stale_action_required["conclusion"] = "ACTION_REQUIRED" + stale_action_required["checkSuite"]["commit"] = {"oid": "old-head"} + current_action_complete = opencode_check(status="COMPLETED") + current_action_complete["conclusion"] = "SUCCESS" + assert sched.action_required_checks( + make_pr( + statusCheckRollup={ + "contexts": { + "nodes": [stale_action_required, current_action_complete] + } + } + ) + ) == [] assert sched.workflow_action_required_reason(["a", "b", "c", "d", "e", "f"]).startswith( "workflow action required: a, b, c, d, e, +1 more" ) @@ -1602,6 +1677,70 @@ def test_failed_status_checks_prefers_timestamped_duplicate_check_runs(): ) assert sched.failed_status_checks(missing_then_timestamped) == [] + equal_timestamp_duplicates = make_pr( + statusCheckRollup={ + "contexts": { + "nodes": [ + { + "__typename": "CheckRun", + "name": "scan-pr-queue", + "conclusion": "SUCCESS", + "startedAt": "2026-07-10T09:30:00Z", + "checkSuite": { + "workflowRun": { + "workflow": {"name": "Required PR Review Merge Scheduler"} + } + }, + }, + { + "__typename": "CheckRun", + "name": "scan-pr-queue", + "conclusion": "CANCELLED", + "startedAt": "2026-07-10T09:30:00Z", + "checkSuite": { + "workflowRun": { + "workflow": {"name": "Required PR Review Merge Scheduler"} + } + }, + }, + ] + } + } + ) + assert sched.failed_status_checks(equal_timestamp_duplicates) == ["scan-pr-queue"] + + older_duplicate = make_pr( + statusCheckRollup={ + "contexts": { + "nodes": [ + { + "__typename": "CheckRun", + "name": "scan-pr-queue", + "conclusion": "SUCCESS", + "startedAt": "2026-07-10T09:30:00Z", + "checkSuite": { + "workflowRun": { + "workflow": {"name": "Required PR Review Merge Scheduler"} + } + }, + }, + { + "__typename": "CheckRun", + "name": "scan-pr-queue", + "conclusion": "CANCELLED", + "startedAt": "2026-07-10T09:29:00Z", + "checkSuite": { + "workflowRun": { + "workflow": {"name": "Required PR Review Merge Scheduler"} + } + }, + }, + ] + } + } + ) + assert sched.failed_status_checks(older_duplicate) == [] + def test_run_command_failure_scrubs_secrets(monkeypatch): import subprocess @@ -3042,11 +3181,8 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): reviews={"nodes": [opencode_review("APPROVED", "head")]}, ) blocked_auto_decision = inspect(blocked_auto) - assert blocked_auto_decision.action == "wait" - assert "GitHub mergeability is BLOCKED" in blocked_auto_decision.reason - assert "GitHub reviewDecision is REVIEW_REQUIRED" in blocked_auto_decision.reason - assert "required approving review" in blocked_auto_decision.reason - assert "rerun the scheduler" in blocked_auto_decision.reason + assert blocked_auto_decision.action == "restamp_head" + assert "last-push approval head refresh requested" in blocked_auto_decision.reason assert sched.latest_commit_headline(make_pr(commits={"nodes": []})) == "" restamp_candidate = last_push_restamp_candidate() @@ -3077,6 +3213,13 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): current_head_approved=True, auto_merge_enabled=True, ) + assert not sched.should_restamp_for_last_push_approval( + "owner/repo", + last_push_restamp_candidate(reviewDecision="REVIEW_REQUIRED"), + "BLOCKED", + current_head_approved=True, + auto_merge_enabled=True, + ) disabled_restamp = inspect(restamp_candidate, update_branches=False) assert disabled_restamp.action == "wait" @@ -3178,7 +3321,7 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): behind_failed = make_pr( mergeStateStatus="BEHIND", reviews={"nodes": [opencode_review("APPROVED", "head")]}, - statusCheckRollup={"contexts": {"nodes": [{"__typename": "CheckRun", "name": "strix", "conclusion": "FAILURE"}]}}, + statusCheckRollup={"contexts": {"nodes": [strix_check(conclusion="FAILURE")]}}, ) failed_decision = inspect(behind_failed) assert failed_decision.action == "block" @@ -3190,7 +3333,7 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): statusCheckRollup={ "contexts": { "nodes": [ - {"__typename": "CheckRun", "name": "strix", "conclusion": "FAILURE"}, + {"__typename": "CheckRun", "name": "strix", "status": "COMPLETED", "conclusion": "FAILURE"}, {"__typename": "CheckRun", "name": "opencode-review", "conclusion": "ACTION_REQUIRED"}, ] } @@ -3205,7 +3348,10 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): reviews={"nodes": [opencode_review("APPROVED", "head")]}, statusCheckRollup={ "contexts": { - "nodes": [{"__typename": "CheckRun", "name": "opencode-review", "conclusion": "ACTION_REQUIRED"}] + "nodes": [ + strix_check(), + {"__typename": "CheckRun", "name": "opencode-review", "conclusion": "ACTION_REQUIRED"}, + ] } }, ) @@ -3245,7 +3391,7 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): autoMergeRequest={"enabledAt": "now"}, statusCheckRollup={ "contexts": { - "nodes": [{"__typename": "CheckRun", "name": "strix", "conclusion": "FAILURE"}], + "nodes": [strix_check(conclusion="FAILURE")], } }, ) @@ -3843,7 +3989,18 @@ def test_inspect_pr_handles_approved_reviews_and_dispatch(monkeypatch): approved = make_pr(reviews={"nodes": [opencode_review("APPROVED", "head")]}) failed = make_pr( reviews={"nodes": [opencode_review("APPROVED", "head")]}, - statusCheckRollup={"contexts": {"nodes": [{"__typename": "CheckRun", "name": "strix", "conclusion": "FAILURE"}]}}, + statusCheckRollup={ + "contexts": { + "nodes": [ + { + "__typename": "CheckRun", + "name": "strix", + "status": "COMPLETED", + "conclusion": "FAILURE", + } + ] + } + }, ) assert inspect(failed).reason == "failed check(s): strix" assert inspect(make_pr(reviews={"nodes": [opencode_review("APPROVED", "head")]}, autoMergeRequest={"enabledAt": "now"})).reason == ( @@ -4012,6 +4169,7 @@ def test_inspect_pr_handles_approved_reviews_and_dispatch(monkeypatch): ("owner/repo", 1, True), ("owner/repo", 1, True), ] + assert auto_merges == [("owner/repo", 1, True)] blocked_already_auto = inspect( make_pr( @@ -4021,11 +4179,8 @@ def test_inspect_pr_handles_approved_reviews_and_dispatch(monkeypatch): ), merge_mode="direct_or_auto", ) - assert blocked_already_auto.action == "wait" - assert "auto-merge is already enabled" in blocked_already_auto.reason - assert "GitHub mergeability is BLOCKED" in blocked_already_auto.reason - assert "GitHub reviewDecision is REVIEW_REQUIRED" in blocked_already_auto.reason - assert "required approving review" in blocked_already_auto.reason + assert blocked_already_auto.action == "restamp_head" + assert "last-push approval head refresh requested" in blocked_already_auto.reason assert direct_merges == [ ("owner/repo", 1, True), ("owner/repo", 1, True), @@ -4063,6 +4218,18 @@ def test_inspect_pr_handles_approved_reviews_and_dispatch(monkeypatch): ) assert external_blocked.action == "wait" assert "fork or external PR heads are excluded from scheduler direct merge and auto-merge" in external_blocked.reason + external_blocked_auto = inspect( + make_pr( + mergeStateStatus="BLOCKED", + autoMergeRequest={"enabledAt": "now"}, + isCrossRepository=True, + headRepository={"nameWithOwner": "fork/repo"}, + reviews={"nodes": [opencode_review("APPROVED", "head")]}, + ), + merge_mode="direct_or_auto", + ) + assert external_blocked_auto.action == "wait" + assert "auto-merge is already enabled" in external_blocked_auto.reason running = make_pr(statusCheckRollup={"contexts": {"nodes": [opencode_check()]}}) assert inspect(running).reason == "OpenCode review is already in progress" @@ -4135,6 +4302,171 @@ def test_inspect_pr_handles_approved_reviews_and_dispatch(monkeypatch): assert "no OpenCode approval" in missing_approval_auto.reason +@pytest.mark.parametrize( + ("review_decision", "expected_state"), + [ + ("REVIEW_REQUIRED", "REVIEW_REQUIRED"), + ("CHANGES_REQUESTED", "CHANGES_REQUESTED"), + (None, "MISSING"), + ("", "MISSING"), + ], +) +def test_inspect_pr_requires_approved_aggregate_review(review_decision, expected_state): + approved_review = {"nodes": [opencode_review("APPROVED", "head")]} + + blocked = inspect(make_pr(reviewDecision=review_decision, reviews=approved_review)) + assert blocked.action == "block" + assert f"aggregate reviewDecision is {expected_state}" in blocked.reason + + disabled = inspect( + make_pr( + reviewDecision=review_decision, + reviews=approved_review, + autoMergeRequest={"enabledAt": "now"}, + ) + ) + assert disabled.action == "disable_auto_merge" + assert f"aggregate reviewDecision is {expected_state}" in disabled.reason + + +def test_inspect_pr_blocks_missing_aggregate_review_field_and_disables_auto_merge(): + approved_review = {"nodes": [opencode_review("APPROVED", "head")]} + + blocked_pr = make_pr(reviews=approved_review) + blocked_pr.pop("reviewDecision") + blocked = inspect(blocked_pr) + assert blocked.action == "block" + assert "aggregate reviewDecision is MISSING" in blocked.reason + + auto_merge_pr = make_pr( + reviews=approved_review, + autoMergeRequest={"enabledAt": "now"}, + ) + auto_merge_pr.pop("reviewDecision") + disabled = inspect(auto_merge_pr) + assert disabled.action == "disable_auto_merge" + assert "aggregate reviewDecision is MISSING" in disabled.reason + + +@pytest.mark.parametrize("conclusion", [None, "NEUTRAL", "SKIPPED"]) +def test_inspect_pr_blocks_non_success_strix_and_disables_auto_merge(conclusion): + approved_review = {"nodes": [opencode_review("APPROVED", "head")]} + status_check_rollup = {"contexts": {"nodes": [strix_check(conclusion=conclusion)]}} + + blocked = inspect( + make_pr( + reviewDecision="APPROVED", + reviews=approved_review, + statusCheckRollup=status_check_rollup, + ) + ) + assert blocked.action == "block" + assert "same-head Strix evidence is failed" in blocked.reason + + disabled = inspect( + make_pr( + reviewDecision="APPROVED", + reviews=approved_review, + autoMergeRequest={"enabledAt": "now"}, + statusCheckRollup=status_check_rollup, + ) + ) + assert disabled.action == "disable_auto_merge" + assert "same-head Strix evidence is failed" in disabled.reason + + +def test_inspect_pr_blocks_failed_non_strix_checks_with_or_without_auto_merge(): + approved_review = {"nodes": [opencode_review("APPROVED", "head")]} + status_check_rollup = { + "contexts": { + "nodes": [ + strix_check(), + { + "__typename": "CheckRun", + "name": "lint", + "status": "COMPLETED", + "conclusion": "FAILURE", + }, + ] + } + } + + blocked = inspect( + make_pr( + reviewDecision="APPROVED", + reviews=approved_review, + statusCheckRollup=status_check_rollup, + ) + ) + assert blocked.action == "block" + assert "failed check(s): lint" in blocked.reason + + disabled = inspect( + make_pr( + reviewDecision="APPROVED", + reviews=approved_review, + autoMergeRequest={"enabledAt": "now"}, + statusCheckRollup=status_check_rollup, + ) + ) + assert disabled.action == "disable_auto_merge" + assert "failed check(s): lint" in disabled.reason + + +def test_inspect_pr_blocks_failed_strix_before_review_dispatch(): + failed = inspect( + make_pr(statusCheckRollup={"contexts": {"nodes": [strix_check(conclusion="FAILURE")]}}) + ) + assert failed.action == "block" + assert failed.reason == "same-head Strix evidence failed; rerun the security workflow before review dispatch" + + +def test_inspect_pr_blocks_approved_head_until_checks_and_strix_are_terminal(): + approved_review = {"nodes": [opencode_review("APPROVED", "head")]} + + running = inspect( + make_pr( + reviewDecision="APPROVED", + reviews=approved_review, + statusCheckRollup={"contexts": {"nodes": [strix_check(status="IN_PROGRESS", conclusion="")]}}, + ) + ) + assert running.action == "block" + assert "check(s) still running: strix" in running.reason + + missing = inspect( + make_pr( + reviewDecision="APPROVED", + reviews=approved_review, + statusCheckRollup={"contexts": {"nodes": []}}, + ) + ) + assert missing.action == "block" + assert "same-head Strix evidence is missing" in missing.reason + + missing_auto = inspect( + make_pr( + reviewDecision="APPROVED", + reviews=approved_review, + autoMergeRequest={"enabledAt": "now"}, + statusCheckRollup={"contexts": {"nodes": []}}, + ) + ) + assert missing_auto.action == "disable_auto_merge" + assert "same-head Strix evidence is missing" in missing_auto.reason + + disabled = inspect( + make_pr( + reviewDecision="APPROVED", + reviews=approved_review, + autoMergeRequest={"enabledAt": "now"}, + statusCheckRollup={"contexts": {"nodes": [strix_check(status="IN_PROGRESS", conclusion="")]}}, + ) + ) + assert disabled.action == "disable_auto_merge" + assert "check(s) still running: strix" in disabled.reason + + def test_inspect_pr_waits_when_same_head_dispatch_is_already_running(monkeypatch): monkeypatch.setattr(sched, "repository_dispatch_wait_reason", lambda repo, workflow: None) monkeypatch.setattr( diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index 233c08584..f6519e6ee 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -1079,21 +1079,17 @@ def test_optional_strix_workflow_absence_is_logged_without_failing_lookup() -> N assert 'if target_workflow_available "strix.yml"; then' in failed_check_evidence -def test_strix_provider_outage_without_findings_is_neutralized() -> None: +def test_strix_provider_outage_without_findings_fails_closed() -> None: workflow = workflow_text("strix.yml") + run_step = workflow.split(" - name: Run Strix (quick)", 1)[1].split( + " - name:", 1 + )[0] - assert "RateLimitError|Too many requests" in workflow - assert "exceeded your current quota" in workflow - assert "billing details" in workflow - assert "LLM warm-up failed" in workflow - assert "zero_vulnerabilities_signal" not in workflow - assert "(^|[^A-Za-z0-9_])severity[[:space:]]*:" in workflow - assert "STRIX_FAIL_ON_MIN_SEVERITY: MEDIUM" in workflow - assert "before producing a vulnerability report" in workflow - assert "genuine findings still fail the check" in workflow - assert ( - '&& ! grep -Eiq "$reported_vulnerability_signal" "$strix_run_log"' in workflow - ) + assert "Strix evidence incomplete" in run_step + assert 'exit "$strix_rc"' in run_step + assert "neutral skip" not in run_step + assert "backend_unavailable_signal" not in run_step + assert "reported_vulnerability_signal" not in run_step def test_strix_cross_repo_dispatch_uses_target_token_for_pr_scoping() -> None: diff --git a/tests/test_strix_nvidia_nim_not_found_fallback.py b/tests/test_strix_nvidia_nim_not_found_fallback.py index a48f3092d..701492581 100644 --- a/tests/test_strix_nvidia_nim_not_found_fallback.py +++ b/tests/test_strix_nvidia_nim_not_found_fallback.py @@ -73,50 +73,13 @@ def _classifies_as_nvidia_not_found(log_text: str) -> bool: return completed.returncode == 0 -def _workflow_signal_pattern(workflow: str, variable_name: str) -> str: - """Extract one single-quoted POSIX ERE assigned in the Strix workflow.""" - - match = re.search( - rf"(?m)^\s+{re.escape(variable_name)}='([^']+)'$", - workflow, - ) - if match is None: - raise AssertionError(f"missing workflow signal: {variable_name}") - return match.group(1) - - -def _workflow_neutralizes(log_text: str) -> bool: - """Execute the outer workflow's backend-neutralization condition.""" +def _workflow_run_step() -> str: + """Return the outer Strix run step that translates gate results.""" workflow = STRIX_WORKFLOW.read_text(encoding="utf-8") - backend_pattern = _workflow_signal_pattern( - workflow, - "backend_unavailable_signal", - ) - vulnerability_pattern = _workflow_signal_pattern( - workflow, - "reported_vulnerability_signal", - ) - with tempfile.TemporaryDirectory(prefix="strix-workflow-404-") as temp_dir: - log_path = Path(temp_dir) / "strix.log" - log_path.write_text(log_text, encoding="utf-8") - backend = subprocess.run( - ["grep", "-Eiq", backend_pattern, str(log_path)], - check=False, - capture_output=True, - text=True, - ) - vulnerability = subprocess.run( - ["grep", "-Eiq", vulnerability_pattern, str(log_path)], - check=False, - capture_output=True, - text=True, - ) - if backend.returncode not in {0, 1}: - raise AssertionError(backend.stderr) - if vulnerability.returncode not in {0, 1}: - raise AssertionError(vulnerability.stderr) - return backend.returncode == 0 and vulnerability.returncode == 1 + return workflow.split(" - name: Run Strix (quick)", 1)[1].split( + " - name:", 1 + )[0] class StrixNvidiaNotFoundFallbackTests(unittest.TestCase): @@ -199,62 +162,14 @@ def test_workflow_uses_available_free_first_nvidia_plan(self) -> None: )[0] self.assertNotIn(RETIRED_PRIMARY_MODEL, default_gate) - def test_outer_workflow_requires_litellm_context_for_nvidia_404(self) -> None: - """Reject provider-like target text in the outer neutralization gate.""" - - self.assertFalse( - _workflow_neutralizes( - "source literal: Nvidia_nimException Error code: 404\n" - ) - ) - self.assertTrue( - _workflow_neutralizes( - "litellm.exceptions.NotFoundError: Nvidia_nimException - " - "Error code: 404\nVulnerabilities 0\n" - ) - ) - - def test_outer_workflow_rejects_cross_line_signal_assembly(self) -> None: - """Require exception, provider, and 404 evidence on one physical line.""" - - self.assertFalse( - _workflow_neutralizes( - "litellm.exceptions.NotFoundError: provider unavailable\n" - "Nvidia_nimException Error code: 404\n" - ) - ) - - def test_outer_workflow_rejects_nvidia_404_without_litellm_context(self) -> None: - """Require LiteLLM NotFoundError context, not just NVIDIA + 404.""" - - self.assertFalse( - _workflow_neutralizes( - "Nvidia_nimException Error code: 404\nVulnerabilities 0\n" - ) - ) - - def test_outer_workflow_never_neutralizes_reported_vulnerabilities(self) -> None: - """Keep a real vulnerability signal blocking despite provider failure.""" - - self.assertFalse( - _workflow_neutralizes( - "litellm.exceptions.NotFoundError: Nvidia_nimException - " - "Error code: 404\nVulnerabilities 1\n" - ) - ) - - def test_workflow_neutralizes_only_nvidia_404_without_findings(self) -> None: - """Retain the static fail-closed vulnerability evidence contract.""" + def test_outer_workflow_fails_closed_without_completed_strix_evidence(self) -> None: + """Provider outages cannot become a neutral required-check pass.""" - workflow = STRIX_WORKFLOW.read_text(encoding="utf-8") - self.assertIn("Nvidia_nimException", workflow) - self.assertIn("Error code:[[:space:]]*404", workflow) - self.assertIn("reported_vulnerability_signal", workflow) - self.assertIn("Vulnerabilities[[:space:]]+[1-9]", workflow) - self.assertIn( - '! grep -Eiq "$reported_vulnerability_signal"', - workflow, - ) + run_step = _workflow_run_step() + self.assertIn("Strix evidence incomplete", run_step) + self.assertIn('exit "$strix_rc"', run_step) + self.assertNotIn("backend_unavailable_signal", run_step) + self.assertNotIn("reported_vulnerability_signal", run_step) if __name__ == "__main__": diff --git a/tests/test_strix_workflow_dependency_hashes.py b/tests/test_strix_workflow_dependency_hashes.py index e2509c18b..955f35090 100644 --- a/tests/test_strix_workflow_dependency_hashes.py +++ b/tests/test_strix_workflow_dependency_hashes.py @@ -8,6 +8,14 @@ ROOT = Path(__file__).resolve().parents[1] WORKFLOW = ROOT / ".github" / "workflows" / "strix-changed-path-quality-ci.yml" +STRIX_REQUIREMENT_FILES = ( + ROOT / "requirements-strix-ci.txt", + ROOT / "requirements-strix-ci-hashes.txt", +) +PATCHED_DEPENDENCY_FLOORS = { + "aiohttp": "3.14.3", + "cryptography": "50.0.0", +} WORKFLOW_DISPATCH_KEY_RE = re.compile( r"(?m)^[ \t]+['\"]?workflow_dispatch['\"]?\s*:" ) @@ -33,6 +41,17 @@ def test_strix_workflow_installs_only_hash_verified_wheels() -> None: assert f"{requirement} --hash=sha256:{digest}" in workflow +def test_strix_requirement_locks_keep_dependabot_patch_floors() -> None: + """Both Strix locks must pin each patched dependency exactly once.""" + for requirements_file in STRIX_REQUIREMENT_FILES: + content = requirements_file.read_text(encoding="utf-8") + for package, version in PATCHED_DEPENDENCY_FLOORS.items(): + matches = re.findall(rf"(?m)^{re.escape(package)}==([^\s\\]+)", content) + assert matches == [version], ( + f"{requirements_file.name} must pin {package} exactly once at {version}" + ) + + def test_strix_workflow_reruns_when_hash_contract_changes() -> None: """Changing this regression contract must trigger the exact-head workflow.""" workflow = WORKFLOW.read_text(encoding="utf-8")