From 6749c78a0397b551d63e2a716579f3a2239148c5 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 01:30:27 +0900 Subject: [PATCH 01/22] fix(scheduler): require approved aggregate review state --- scripts/ci/pr_review_merge_scheduler.py | 44 ++++++++++++++++++++++++- tests/test_pr_review_merge_scheduler.py | 34 ++++++++++++++++--- 2 files changed, 73 insertions(+), 5 deletions(-) diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 75e18c860..408dfb903 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -2400,6 +2400,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, @@ -3189,7 +3200,7 @@ def self_test() -> None: "isCrossRepository": False, "maintainerCanModify": False, "headRepository": {"nameWithOwner": "owner/repo"}, - "reviewDecision": "REVIEW_REQUIRED", + "reviewDecision": "APPROVED", "commits": { "nodes": [ { @@ -3229,6 +3240,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", diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 3e421e903..612a86851 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -64,6 +64,16 @@ 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" return value @@ -3044,8 +3054,6 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): 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 sched.latest_commit_headline(make_pr(commits={"nodes": []})) == "" @@ -4012,6 +4020,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( @@ -4024,8 +4033,6 @@ def test_inspect_pr_handles_approved_reviews_and_dispatch(monkeypatch): 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 direct_merges == [ ("owner/repo", 1, True), ("owner/repo", 1, True), @@ -4135,6 +4142,25 @@ def test_inspect_pr_handles_approved_reviews_and_dispatch(monkeypatch): assert "no OpenCode approval" in missing_approval_auto.reason +@pytest.mark.parametrize("review_decision", ["REVIEW_REQUIRED", "CHANGES_REQUESTED"]) +def test_inspect_pr_requires_approved_aggregate_review(review_decision): + 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 {review_decision}" 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 {review_decision}" 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( From dd8f59d50ab14dc1ffc27c99ac1b3ca463e79ae1 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 01:37:43 +0900 Subject: [PATCH 02/22] test(scheduler): cover missing aggregate review state --- tests/test_pr_review_merge_scheduler.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 612a86851..addf0713b 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -4142,13 +4142,20 @@ def test_inspect_pr_handles_approved_reviews_and_dispatch(monkeypatch): assert "no OpenCode approval" in missing_approval_auto.reason -@pytest.mark.parametrize("review_decision", ["REVIEW_REQUIRED", "CHANGES_REQUESTED"]) -def test_inspect_pr_requires_approved_aggregate_review(review_decision): +@pytest.mark.parametrize( + ("review_decision", "expected_state"), + [ + ("REVIEW_REQUIRED", "REVIEW_REQUIRED"), + ("CHANGES_REQUESTED", "CHANGES_REQUESTED"), + (None, "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 {review_decision}" in blocked.reason + assert f"aggregate reviewDecision is {expected_state}" in blocked.reason disabled = inspect( make_pr( @@ -4158,7 +4165,7 @@ def test_inspect_pr_requires_approved_aggregate_review(review_decision): ) ) assert disabled.action == "disable_auto_merge" - assert f"aggregate reviewDecision is {review_decision}" in disabled.reason + assert f"aggregate reviewDecision is {expected_state}" in disabled.reason def test_inspect_pr_waits_when_same_head_dispatch_is_already_running(monkeypatch): From 12e3d1f59cdd7659012b164ed6e75c1b16ae893e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 02:23:07 +0900 Subject: [PATCH 03/22] test(scheduler): cover empty aggregate review state --- tests/test_pr_review_merge_scheduler.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index addf0713b..20823c3ee 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -4148,6 +4148,7 @@ def test_inspect_pr_handles_approved_reviews_and_dispatch(monkeypatch): ("REVIEW_REQUIRED", "REVIEW_REQUIRED"), ("CHANGES_REQUESTED", "CHANGES_REQUESTED"), (None, "MISSING"), + ("", "MISSING"), ], ) def test_inspect_pr_requires_approved_aggregate_review(review_decision, expected_state): From 2756cd3bc930d55ca6993992c30715da297f7520 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 02:49:23 +0900 Subject: [PATCH 04/22] fix(scheduler): block merge on running checks --- scripts/ci/pr_review_merge_scheduler.py | 45 ++++++++++++- tests/test_pr_review_merge_scheduler.py | 85 +++++++++++++++++++++---- 2 files changed, 118 insertions(+), 12 deletions(-) diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 408dfb903..344d05b60 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -1076,6 +1076,21 @@ def strix_evidence_state(pr: dict[str, Any]) -> str: return "complete" if found else "missing" +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() + for node in context_nodes(pr): + if node.get("__typename") == "CheckRun": + 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: """Count active, non-outdated unresolved review threads on a PR.""" threads = ((pr.get("reviewThreads") or {}).get("nodes") or []) @@ -2455,6 +2470,22 @@ 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) + + strix_state = strix_evidence_state(pr) + if strix_state != "complete": + 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) + failed_checks = failed_status_checks(pr) if failed_checks: if pr.get("autoMergeRequest"): @@ -3224,7 +3255,19 @@ def self_test() -> None: } ] }, - "statusCheckRollup": {"contexts": {"nodes": []}}, + "statusCheckRollup": { + "contexts": { + "nodes": [ + { + "__typename": "CheckRun", + "name": "strix", + "status": "COMPLETED", + "conclusion": "SUCCESS", + "checkSuite": {"workflowRun": {"workflow": {"name": "Strix Security Scan"}}}, + } + ] + } + }, } assert has_current_head_approval(sample) assert not has_current_head_changes_requested(sample) diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 20823c3ee..89c63ef64 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -74,6 +74,20 @@ def make_pr(**overrides): 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"}}}, + } + ] + } + } return value @@ -1024,10 +1038,12 @@ 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="")]}} ) @@ -3052,9 +3068,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 "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() @@ -3186,7 +3201,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" @@ -3198,7 +3213,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"}, ] } @@ -3213,7 +3228,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"}, + ] } }, ) @@ -3253,7 +3271,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")], } }, ) @@ -3851,7 +3869,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 == ( @@ -4030,9 +4059,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 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), @@ -4169,6 +4197,41 @@ def test_inspect_pr_requires_approved_aggregate_review(review_decision, expected assert f"aggregate reviewDecision is {expected_state}" in disabled.reason +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 + + 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( From 58561518e486d3230874c346220be96ca0a41e30 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 03:02:16 +0900 Subject: [PATCH 05/22] fix(strix): fail closed on missing evidence --- .github/workflows/strix.yml | 38 +----- scripts/ci/strix_required_workflow_smoke.sh | 1 - scripts/ci/test_strix_quick_gate.sh | 3 +- .../test_required_workflow_queue_contract.py | 22 ++-- ...est_strix_nvidia_nim_not_found_fallback.py | 109 ++---------------- 5 files changed, 29 insertions(+), 144 deletions(-) diff --git a/.github/workflows/strix.yml b/.github/workflows/strix.yml index 03ec23257..c385436d8 100644 --- a/.github/workflows/strix.yml +++ b/.github/workflows/strix.yml @@ -836,12 +836,11 @@ jobs: export "STRIX_TOTAL_${budget_suffix}_SECONDS=5700" # 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. strix_run_log="$RUNNER_TEMP/strix_gate_console.log" strix_rc=0 set +e @@ -853,32 +852,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 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..eda569722 100755 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -725,7 +725,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_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__": From 9a05f03f964133b3d8da4df30baa28c78cdefbee Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 03:39:14 +0900 Subject: [PATCH 06/22] fix(scheduler): bind check evidence to current head --- scripts/ci/pr_review_merge_scheduler.py | 106 ++++++++++++++++++++++-- tests/test_pr_review_merge_scheduler.py | 20 +++++ 2 files changed, 118 insertions(+), 8 deletions(-) diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 344d05b60..517f88c6a 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) @@ -1063,8 +1114,11 @@ 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.""" + current_check_ids = {id(node) for node in current_head_check_runs(pr)[0]} found = 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 @@ -1079,8 +1133,14 @@ def strix_evidence_state(pr: dict[str, Any]) -> str: 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: @@ -1398,10 +1458,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 "" @@ -1447,9 +1510,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") @@ -3263,7 +3329,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"}}, + }, } ] } @@ -3361,7 +3430,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", @@ -3434,7 +3509,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"] = [] @@ -3466,7 +3546,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( @@ -3542,7 +3625,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", @@ -3678,6 +3767,7 @@ def self_test() -> None: "name": "strix", "status": "COMPLETED", "conclusion": "SUCCESS", + "checkSuite": {"commit": {"oid": "abc"}}, } ] } diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 89c63ef64..fea5c660b 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -88,6 +88,15 @@ def make_pr(**overrides): ] } } + 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 @@ -616,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): @@ -931,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"}) @@ -1053,6 +1064,15 @@ def test_context_review_and_check_helpers(monkeypatch): sched.strix_evidence_state(make_pr(statusCheckRollup={"contexts": {"nodes": [strix_check(conclusion="FAILURE")]}})) == "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={ From 170b98e453292dd0eb63be5ec99160504252ed21 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 04:03:00 +0900 Subject: [PATCH 07/22] fix(ci): make review model pool portable --- scripts/ci/portable_timeout.py | 87 +++++++++++++++++++ scripts/ci/run_opencode_review_model_pool.sh | 26 +++++- scripts/ci/test_strix_quick_gate.sh | 2 +- ...st_materialize_base_python_requirements.py | 9 ++ tests/test_opencode_agent_contract.py | 2 +- tests/test_portable_timeout.py | 53 +++++++++++ 6 files changed, 175 insertions(+), 4 deletions(-) create mode 100644 scripts/ci/portable_timeout.py create mode 100644 tests/test_portable_timeout.py diff --git a/scripts/ci/portable_timeout.py b/scripts/ci/portable_timeout.py new file mode 100644 index 000000000..a80a58266 --- /dev/null +++ b/scripts/ci/portable_timeout.py @@ -0,0 +1,87 @@ +#!/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: + 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: + try: + os.killpg(process.pid, signum) + except ProcessLookupError: + pass + + +def _terminate(process: subprocess.Popen[object], kill_after: float) -> int: + _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: + 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: + _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__": + raise SystemExit(main(sys.argv[1:])) diff --git a/scripts/ci/run_opencode_review_model_pool.sh b/scripts/ci/run_opencode_review_model_pool.sh index 986982e9a..cc847cb3a 100644 --- a/scripts/ci/run_opencode_review_model_pool.sh +++ b/scripts/ci/run_opencode_review_model_pool.sh @@ -3,6 +3,28 @@ 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 +} + record_review_status() { printf 'review_status=%s\n' "$1" >>"$GITHUB_OUTPUT" } @@ -465,7 +487,7 @@ run_one_model_attempt() { rm -f "$opencode_json_file" "$opencode_stderr_file" "$opencode_export_file" "$candidate_output_file" set +e - timeout --kill-after=30s "${run_timeout_seconds}s" \ + run_with_timeout 30s "${run_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 run "$(cat "$prompt_file")" \ @@ -520,7 +542,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/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index eda569722..ace23d56e 100755 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -708,7 +708,7 @@ 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" 'run_with_timeout 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" '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" diff --git a/tests/test_materialize_base_python_requirements.py b/tests/test_materialize_base_python_requirements.py index 8a383f0c2..04f125b97 100644 --- a/tests/test_materialize_base_python_requirements.py +++ b/tests/test_materialize_base_python_requirements.py @@ -644,6 +644,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 +693,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 +727,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_portable_timeout.py b/tests/test_portable_timeout.py new file mode 100644 index 000000000..3c6b3be83 --- /dev/null +++ b/tests/test_portable_timeout.py @@ -0,0 +1,53 @@ +"""Checks for the stdlib timeout fallback used by local review tests.""" + +from __future__ import annotations + +import subprocess +import sys +from pathlib import Path + + +SCRIPT = Path(__file__).parents[1] / "scripts" / "ci" / "portable_timeout.py" + + +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 From ed666c7c96e1954f6c7c627e690b6080de0b2c27 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 04:13:59 +0900 Subject: [PATCH 08/22] test(security): pin Strix dependency floors --- .../test_strix_workflow_dependency_hashes.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/test_strix_workflow_dependency_hashes.py b/tests/test_strix_workflow_dependency_hashes.py index e2509c18b..204d4bce5 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 retain versions at or above the patched advisories.""" + 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") From 775025378b6685ab71e737e6cda1ee5ff36a7eca Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 05:12:47 +0900 Subject: [PATCH 09/22] fix(ci): close coverage and Strix smoke gaps --- .github/workflows/strix.yml | 2 + .../strix-nvidia-nim-not-found-fallback.md | 16 ++- .../strix-quality-timeout-fixtures.md | 9 ++ scripts/ci/portable_timeout.py | 2 +- tests/test_portable_timeout.py | 110 ++++++++++++++++++ tests/test_pr_review_merge_scheduler.py | 86 ++++++++++++++ 6 files changed, 219 insertions(+), 6 deletions(-) diff --git a/.github/workflows/strix.yml b/.github/workflows/strix.yml index c385436d8..73b6deb0a 100644 --- a/.github/workflows/strix.yml +++ b/.github/workflows/strix.yml @@ -841,6 +841,8 @@ jobs: # 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 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/scripts/ci/portable_timeout.py b/scripts/ci/portable_timeout.py index a80a58266..e928b1de8 100644 --- a/scripts/ci/portable_timeout.py +++ b/scripts/ci/portable_timeout.py @@ -83,5 +83,5 @@ def forward(signum: int, _frame: object) -> None: continue -if __name__ == "__main__": +if __name__ == "__main__": # pragma: no cover - exercised through subprocess tests raise SystemExit(main(sys.argv[1:])) diff --git a/tests/test_portable_timeout.py b/tests/test_portable_timeout.py index 3c6b3be83..1965669a4 100644 --- a/tests/test_portable_timeout.py +++ b/tests/test_portable_timeout.py @@ -2,14 +2,41 @@ 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( [ @@ -51,3 +78,86 @@ def test_portable_timeout_returns_124_after_killing_child() -> None: ) 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 fea5c660b..fa40deafd 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -976,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 @@ -1004,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": { @@ -1378,6 +1388,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" ) @@ -1648,6 +1672,38 @@ 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": "CANCELLED", + "startedAt": "2026-07-10T09:30:00Z", + "checkSuite": { + "workflowRun": { + "workflow": {"name": "Required PR Review Merge Scheduler"} + } + }, + }, + { + "__typename": "CheckRun", + "name": "scan-pr-queue", + "conclusion": "SUCCESS", + "startedAt": "2026-07-10T09:29:00Z", + "checkSuite": { + "workflowRun": { + "workflow": {"name": "Required PR Review Merge Scheduler"} + } + }, + }, + ] + } + } + ) + assert sched.failed_status_checks(equal_timestamp_duplicates) == ["scan-pr-queue"] + def test_run_command_failure_scrubs_secrets(monkeypatch): import subprocess @@ -3120,6 +3176,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" @@ -4118,6 +4181,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" @@ -4240,6 +4315,17 @@ def test_inspect_pr_blocks_approved_head_until_checks_and_strix_are_terminal(): 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", From 249ba9864e9aa2309f40f947ba61fd1d126d31af Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 05:51:51 +0900 Subject: [PATCH 10/22] fix(ci): close scheduler evidence gaps --- scripts/ci/portable_timeout.py | 4 + scripts/ci/pr_review_merge_scheduler.py | 25 +++- scripts/ci/run_opencode_review_model_pool.sh | 33 ++++- scripts/ci/test_strix_quick_gate.sh | 3 +- tests/test_pr_review_merge_scheduler.py | 139 +++++++++++++++++- .../test_strix_workflow_dependency_hashes.py | 2 +- 6 files changed, 190 insertions(+), 16 deletions(-) diff --git a/scripts/ci/portable_timeout.py b/scripts/ci/portable_timeout.py index e928b1de8..5b65e12ef 100644 --- a/scripts/ci/portable_timeout.py +++ b/scripts/ci/portable_timeout.py @@ -15,6 +15,7 @@ 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}") @@ -22,6 +23,7 @@ def _seconds(value: str) -> float: 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: @@ -29,6 +31,7 @@ def _signal_process_group(process: subprocess.Popen[object], signum: int) -> Non 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) @@ -38,6 +41,7 @@ def _terminate(process: subprocess.Popen[object], kill_after: float) -> int: 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 diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 517f88c6a..c9d7f620d 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -1113,9 +1113,10 @@ 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 @@ -1125,9 +1126,16 @@ def strix_evidence_state(pr: dict[str, Any]) -> str: 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]: @@ -2545,14 +2553,17 @@ def request_branch_update(freshness_reason: str, *, suffix: str = "") -> Decisio 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": - reason = f"same-head Strix evidence is {strix_state}; wait for a completed security result" + 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) - failed_checks = failed_status_checks(pr) if failed_checks: if pr.get("autoMergeRequest"): return finish( @@ -2815,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: diff --git a/scripts/ci/run_opencode_review_model_pool.sh b/scripts/ci/run_opencode_review_model_pool.sh index cc847cb3a..ad9212352 100644 --- a/scripts/ci/run_opencode_review_model_pool.sh +++ b/scripts/ci/run_opencode_review_model_pool.sh @@ -25,6 +25,22 @@ run_with_timeout() { 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="$(ps -o pgid= -p "$pid" 2>/dev/null | tr -d ' ')" + shell_pgid="$(ps -o pgid= -p "$$" 2>/dev/null | tr -d ' ')" + if [ -n "$pgid" ] && [ "$pgid" != "$shell_pgid" ]; then + kill "-$signum" -- "-$pgid" 2>/dev/null || true + else + kill "-$signum" "$pid" 2>/dev/null || true + fi +} + record_review_status() { printf 'review_status=%s\n' "$1" >>"$GITHUB_OUTPUT" } @@ -479,15 +495,26 @@ run_one_model_attempt() { 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 -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 - run_with_timeout 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")" \ @@ -506,12 +533,12 @@ run_one_model_attempt() { if has_fatal_provider_error_event "$opencode_json_file"; then 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 + signal_process_tree KILL "$opencode_pid" break fi sleep "$fatal_poll_seconds" diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index ace23d56e..64e1a5893 100755 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -708,7 +708,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" 'run_with_timeout 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" diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index fa40deafd..5fe50fd93 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -1070,10 +1070,15 @@ def test_context_review_and_check_helpers(monkeypatch): ) 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]}}) @@ -1676,6 +1681,17 @@ def test_failed_status_checks_prefers_timestamped_duplicate_check_runs(): 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", @@ -1687,10 +1703,31 @@ def test_failed_status_checks_prefers_timestamped_duplicate_check_runs(): } }, }, + ] + } + } + ) + 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": { @@ -1702,7 +1739,7 @@ def test_failed_status_checks_prefers_timestamped_duplicate_check_runs(): } } ) - assert sched.failed_status_checks(equal_timestamp_duplicates) == ["scan-pr-queue"] + assert sched.failed_status_checks(older_duplicate) == [] def test_run_command_failure_scrubs_secrets(monkeypatch): @@ -4292,6 +4329,98 @@ def test_inspect_pr_requires_approved_aggregate_review(review_decision, expected 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")]} diff --git a/tests/test_strix_workflow_dependency_hashes.py b/tests/test_strix_workflow_dependency_hashes.py index 204d4bce5..89c66c5c9 100644 --- a/tests/test_strix_workflow_dependency_hashes.py +++ b/tests/test_strix_workflow_dependency_hashes.py @@ -42,7 +42,7 @@ def test_strix_workflow_installs_only_hash_verified_wheels() -> None: def test_strix_requirement_locks_keep_dependabot_patch_floors() -> None: - """Both Strix locks must retain versions at or above the patched advisories.""" + """Both Strix locks must retain the exact versions used by the hash contract.""" for requirements_file in STRIX_REQUIREMENT_FILES: content = requirements_file.read_text(encoding="utf-8") for package, version in PATCHED_DEPENDENCY_FLOORS.items(): From ab2a1ae75a4275ec799d9b53a2bea78a5340e7a1 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 09:39:07 +0900 Subject: [PATCH 11/22] fix(ci): require trustworthy Strix evidence --- .github/workflows/strix.yml | 52 +++++++++++++++++++++++++++++ scripts/ci/portable_timeout.py | 1 + scripts/ci/strix_quick_gate.sh | 4 +-- scripts/ci/test_strix_quick_gate.sh | 6 ++++ 4 files changed, 61 insertions(+), 2 deletions(-) diff --git a/.github/workflows/strix.yml b/.github/workflows/strix.yml index 73b6deb0a..f9b11f68c 100644 --- a/.github/workflows/strix.yml +++ b/.github/workflows/strix.yml @@ -887,6 +887,58 @@ 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 + + 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_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/scripts/ci/portable_timeout.py b/scripts/ci/portable_timeout.py index 5b65e12ef..a0b969c45 100644 --- a/scripts/ci/portable_timeout.py +++ b/scripts/ci/portable_timeout.py @@ -66,6 +66,7 @@ def main(argv: list[str]) -> int: 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) 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/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index 64e1a5893..e7b7f1ed3 100755 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -226,6 +226,12 @@ 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" "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" From e6c6d12808a6afbeecf5e1cae51d5e5cc542471e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 10:35:14 +0900 Subject: [PATCH 12/22] fix(ci): bind scan reports and kill captured groups --- .github/workflows/strix.yml | 31 +++++++++ scripts/ci/run_opencode_review_model_pool.sh | 49 +++++++++++-- scripts/ci/test_strix_quick_gate.sh | 5 ++ tests/test_opencode_model_pool_runner.py | 73 +++++++++++++++++++- 4 files changed, 151 insertions(+), 7 deletions(-) diff --git a/.github/workflows/strix.yml b/.github/workflows/strix.yml index f9b11f68c..fc8b82ead 100644 --- a/.github/workflows/strix.yml +++ b/.github/workflows/strix.yml @@ -834,6 +834,7 @@ 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 for genuine blocking vulnerabilities and for @@ -875,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 @@ -899,12 +904,38 @@ jobs: 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_head_sha="$(jq -r ' + [ + .head_sha, + .commit_sha, + .scan_results.head_sha, + .scan_results.commit_sha + ] + | map(select(type == "string" and . != "")) + | .[0] // empty + ' "$candidate_run")" + if [ -z "$candidate_head_sha" ]; then + candidate_head_sha="$scan_stage_head_sha" + fi + if ! [[ "$candidate_head_sha" =~ ^[0-9a-fA-F]{40}$ ]] || + [ "${candidate_head_sha,,}" != "${evidence_head_sha,,}" ]; then + continue + fi candidate_report="$(dirname -- "$candidate_run")/penetration_test_report.md" if [ -s "$candidate_report" ]; then successful_run_file="$candidate_run" diff --git a/scripts/ci/run_opencode_review_model_pool.sh b/scripts/ci/run_opencode_review_model_pool.sh index ad9212352..cae6c6052 100644 --- a/scripts/ci/run_opencode_review_model_pool.sh +++ b/scripts/ci/run_opencode_review_model_pool.sh @@ -32,15 +32,51 @@ signal_process_tree() { for child in $(pgrep -P "$pid" 2>/dev/null || true); do signal_process_tree "$signum" "$child" done - pgid="$(ps -o pgid= -p "$pid" 2>/dev/null | tr -d ' ')" + 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 - kill "-$signum" -- "-$pgid" 2>/dev/null || true + 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" } @@ -494,7 +530,7 @@ 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}" @@ -531,6 +567,7 @@ 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" signal_process_tree TERM "$opencode_pid" @@ -538,7 +575,11 @@ run_one_model_attempt() { kill -0 "$opencode_pid" 2>/dev/null || break sleep 1 done - signal_process_tree KILL "$opencode_pid" + 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" diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index e7b7f1ed3..c55bb0ddb 100755 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -228,6 +228,11 @@ assert_strix_workflow_pr_trigger_hardened() { 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" "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" ".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" 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( From 2c6f4323ac864587d767824464379678ebfe888a Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 10:51:43 +0900 Subject: [PATCH 13/22] fix(ci): reject conflicting scan metadata --- .github/workflows/strix.yml | 42 ++++++++++++++++++++++------- scripts/ci/test_strix_quick_gate.sh | 5 +++- 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/.github/workflows/strix.yml b/.github/workflows/strix.yml index fc8b82ead..7311be8cb 100644 --- a/.github/workflows/strix.yml +++ b/.github/workflows/strix.yml @@ -919,22 +919,44 @@ jobs: 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_head_sha="$(jq -r ' + candidate_metadata_count="$(jq -r ' [ .head_sha, .commit_sha, - .scan_results.head_sha, - .scan_results.commit_sha + ((.scan_results // {}).head_sha), + ((.scan_results // {}).commit_sha) ] - | map(select(type == "string" and . != "")) - | .[0] // empty + | map(select(. != null)) + | length ' "$candidate_run")" - if [ -z "$candidate_head_sha" ]; then + if [ "$candidate_metadata_count" -eq 0 ]; then candidate_head_sha="$scan_stage_head_sha" - fi - if ! [[ "$candidate_head_sha" =~ ^[0-9a-fA-F]{40}$ ]] || - [ "${candidate_head_sha,,}" != "${evidence_head_sha,,}" ]; then - continue + 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 diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index c55bb0ddb..c7302f2d7 100755 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -231,8 +231,11 @@ assert_strix_workflow_pr_trigger_hardened() { 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" ".scan_results.commit_sha" "strix workflow checks alternate structured commit metadata" + 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" From 8726df151e64eecb89d91a4c029e809a785ee126 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 11:31:16 +0900 Subject: [PATCH 14/22] fix(ci): verify PR Strix workflow version --- .github/workflows/strix-workflow-contract.yml | 40 +++++++++++++++++++ .../strix-workflow-version-evidence.md | 25 ++++++++++++ docs/org-required-workflow-rollout.md | 16 +++++++- scripts/ci/test_strix_quick_gate.sh | 10 ++++- 4 files changed, 89 insertions(+), 2 deletions(-) create mode 100644 .github/workflows/strix-workflow-contract.yml create mode 100644 docs/doctoring/strix-workflow-version-evidence.md diff --git a/.github/workflows/strix-workflow-contract.yml b/.github/workflows/strix-workflow-contract.yml new file mode 100644 index 000000000..0c6c01875 --- /dev/null +++ b/.github/workflows/strix-workflow-contract.yml @@ -0,0 +1,40 @@ +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" + grep -Fq "Validate Strix report provenance" "$workflow_file" + grep -Fq "evidence-binding.json" "$workflow_file" + grep -Fq "scan-head-sha.txt" "$workflow_file" + grep -Fq "Strix evidence incomplete" "$workflow_file" + if grep -Fq "neutral skip so an infrastructure outage does not block merges" "$workflow_file"; then + echo "::error::PR Strix workflow still neutralizes missing security evidence." + exit 1 + fi diff --git a/docs/doctoring/strix-workflow-version-evidence.md b/docs/doctoring/strix-workflow-version-evidence.md new file mode 100644 index 000000000..3627be439 --- /dev/null +++ b/docs/doctoring/strix-workflow-version-evidence.md @@ -0,0 +1,25 @@ +# 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. 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..83b1bc35c 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,20 @@ 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. + ## 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/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index c7302f2d7..93dca2f33 100755 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -175,7 +175,15 @@ 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_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" From 67d834f510fe044dd9d53cd4f4b9783353e303bd Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 12:15:29 +0900 Subject: [PATCH 15/22] fix(ci): validate executable Strix workflow policy --- .github/workflows/strix-workflow-contract.yml | 93 +++++++++++- .../strix-workflow-version-evidence.md | 5 +- docs/org-required-workflow-rollout.md | 8 ++ scripts/ci/test_strix_quick_gate.sh | 134 ++++++++++++++++++ 4 files changed, 232 insertions(+), 8 deletions(-) diff --git a/.github/workflows/strix-workflow-contract.yml b/.github/workflows/strix-workflow-contract.yml index 0c6c01875..f876368ae 100644 --- a/.github/workflows/strix-workflow-contract.yml +++ b/.github/workflows/strix-workflow-contract.yml @@ -30,11 +30,90 @@ jobs: base64 --decode > "$RUNNER_TEMP/strix-pr-workflow.yml" workflow_file="$RUNNER_TEMP/strix-pr-workflow.yml" test -s "$workflow_file" - grep -Fq "Validate Strix report provenance" "$workflow_file" - grep -Fq "evidence-binding.json" "$workflow_file" - grep -Fq "scan-head-sha.txt" "$workflow_file" - grep -Fq "Strix evidence incomplete" "$workflow_file" - if grep -Fq "neutral skip so an infrastructure outage does not block merges" "$workflow_file"; then - echo "::error::PR Strix workflow still neutralizes missing security evidence." + ruby - "$workflow_file" <<'RUBY' + require "psych" + + def reject!(message) + warn "::error::PR Strix workflow contract rejected: #{message}" exit 1 - fi + 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/docs/doctoring/strix-workflow-version-evidence.md b/docs/doctoring/strix-workflow-version-evidence.md index 3627be439..ca3f276a9 100644 --- a/docs/doctoring/strix-workflow-version-evidence.md +++ b/docs/doctoring/strix-workflow-version-evidence.md @@ -15,7 +15,10 @@ the protected-base workflow definition, as GitHub requires for 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. Once the central +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 diff --git a/docs/org-required-workflow-rollout.md b/docs/org-required-workflow-rollout.md index 83b1bc35c..41bd51656 100644 --- a/docs/org-required-workflow-rollout.md +++ b/docs/org-required-workflow-rollout.md @@ -45,6 +45,14 @@ 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/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index 93dca2f33..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" @@ -184,6 +314,10 @@ assert_strix_workflow_pr_trigger_hardened() { 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" From 19ced8936bf8bd726ece1968d60e7b5f3d8d620f Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 16:29:57 +0900 Subject: [PATCH 16/22] fix(ci): bound review dispatch payloads and UX failures --- .../agent-mention-noema-dispatch.yml | 1 - .../agent-mention-opencode-dispatch.yml | 21 ++----- .../0001-agent-mention-dispatch-contract.md | 60 +++++++++++++++++++ scripts/ci/agent_mention_router.py | 42 ++++++++++--- ..._agent_mention_complete_payload_binding.py | 9 +++ ...st_agent_mention_downstream_idempotency.py | 2 +- tests/test_agent_mention_idempotency.py | 13 ++-- tests/test_agent_mention_router.py | 16 +++-- 8 files changed, 126 insertions(+), 38 deletions(-) create mode 100644 docs/adr/0001-agent-mention-dispatch-contract.md 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/docs/adr/0001-agent-mention-dispatch-contract.md b/docs/adr/0001-agent-mention-dispatch-contract.md new file mode 100644 index 000000000..d65a0dc42 --- /dev/null +++ b/docs/adr/0001-agent-mention-dispatch-contract.md @@ -0,0 +1,60 @@ +# 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. + +## 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. +- Independent current-head review, terminal checks, structured Strix evidence, + and protected-branch rules remain required before merge. 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/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: From d6b9b3a642d477abea161f5b1e15f158e5b5e95a Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 16:42:51 +0900 Subject: [PATCH 17/22] fix(ci): defer interpreter-incompatible lock candidates --- .../0001-agent-mention-dispatch-contract.md | 10 ++++++ scripts/ci/install_base_python_locks.py | 15 ++++++--- tests/test_install_base_python_locks.py | 33 +++++++++++++++++++ 3 files changed, 53 insertions(+), 5 deletions(-) diff --git a/docs/adr/0001-agent-mention-dispatch-contract.md b/docs/adr/0001-agent-mention-dispatch-contract.md index d65a0dc42..65850d229 100644 --- a/docs/adr/0001-agent-mention-dispatch-contract.md +++ b/docs/adr/0001-agent-mention-dispatch-contract.md @@ -31,6 +31,13 @@ review again even though the durable central artifact claim already exists. 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. ## Consequences @@ -56,5 +63,8 @@ mechanism. - 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. 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/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( From 2fe2bba9f613844ed71e2223977a411e318e53f0 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 17:23:50 +0900 Subject: [PATCH 18/22] fix(ci): retry transient trusted uv downloads --- .../0001-agent-mention-dispatch-contract.md | 9 ++ .../materialize_base_python_requirements.py | 97 +++++++++++-------- ...st_materialize_base_python_requirements.py | 60 ++++++++++++ 3 files changed, 127 insertions(+), 39 deletions(-) diff --git a/docs/adr/0001-agent-mention-dispatch-contract.md b/docs/adr/0001-agent-mention-dispatch-contract.md index 65850d229..fd2780808 100644 --- a/docs/adr/0001-agent-mention-dispatch-contract.md +++ b/docs/adr/0001-agent-mention-dispatch-contract.md @@ -38,6 +38,15 @@ review again even though the durable central artifact claim already exists. 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. ## Consequences 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/tests/test_materialize_base_python_requirements.py b/tests/test_materialize_base_python_requirements.py index 04f125b97..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: From 9644f9fb148483a333789b34bf58f62c97d4f842 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 18:11:38 +0900 Subject: [PATCH 19/22] docs(adr): record dispatch allowlist drift --- .../0001-agent-mention-dispatch-contract.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/docs/adr/0001-agent-mention-dispatch-contract.md b/docs/adr/0001-agent-mention-dispatch-contract.md index fd2780808..6cadca72e 100644 --- a/docs/adr/0001-agent-mention-dispatch-contract.md +++ b/docs/adr/0001-agent-mention-dispatch-contract.md @@ -47,6 +47,21 @@ review again even though the durable central artifact claim already exists. `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. ## Consequences @@ -77,3 +92,6 @@ mechanism. 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. From 0cc98ac5b68aba378676f7c964f46e7517d77014 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 18:13:54 +0900 Subject: [PATCH 20/22] docs(adr): record stale dependency alerts --- docs/adr/0001-agent-mention-dispatch-contract.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/adr/0001-agent-mention-dispatch-contract.md b/docs/adr/0001-agent-mention-dispatch-contract.md index 6cadca72e..e88d60617 100644 --- a/docs/adr/0001-agent-mention-dispatch-contract.md +++ b/docs/adr/0001-agent-mention-dispatch-contract.md @@ -62,6 +62,19 @@ review again even though the durable central artifact claim already exists. (`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. ## Consequences From 5aafbb2d84fcf2e70f6a3e6c57ac068b27dffa9f Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 18:47:02 +0900 Subject: [PATCH 21/22] docs(adr): record trusted Strix outage evidence --- docs/adr/0001-agent-mention-dispatch-contract.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/docs/adr/0001-agent-mention-dispatch-contract.md b/docs/adr/0001-agent-mention-dispatch-contract.md index e88d60617..704b23082 100644 --- a/docs/adr/0001-agent-mention-dispatch-contract.md +++ b/docs/adr/0001-agent-mention-dispatch-contract.md @@ -75,6 +75,20 @@ review again even though the durable central artifact claim already exists. 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 From 65d4b0812792a3e1fca4d534cfe2e5b7e4075137 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 18:55:47 +0900 Subject: [PATCH 22/22] test(ci): align Strix lock contract wording --- tests/test_strix_workflow_dependency_hashes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_strix_workflow_dependency_hashes.py b/tests/test_strix_workflow_dependency_hashes.py index 89c66c5c9..955f35090 100644 --- a/tests/test_strix_workflow_dependency_hashes.py +++ b/tests/test_strix_workflow_dependency_hashes.py @@ -42,7 +42,7 @@ def test_strix_workflow_installs_only_hash_verified_wheels() -> None: def test_strix_requirement_locks_keep_dependabot_patch_floors() -> None: - """Both Strix locks must retain the exact versions used by the hash contract.""" + """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():