diff --git a/CHANGELOG.md b/CHANGELOG.md index bf192f6a9e..6af8a028df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +### Manual Strix runs no longer replace required scheduler evidence + +- The scheduler now binds Strix CheckRun identity to workflow, job, and Actions event; caller-selected Strix `workflow_dispatch` runs cannot hide, fail, park, or become the rerun target for required `pull_request_target` or `repository_dispatch` evidence. Both paginated GraphQL query shapes retain `WorkflowRun.event`, while missing event data remains fail-closed. Proposed in ContextualWisdomLab/.github#1061. + ### Failed-check finding names the Strix sandbox instead of the gateway - `opencode-review-dispatch.yml`'s `emit_strix_provider_failure_finding` rendered one fixed finding for every `STRIX_PROVIDER_UNAVAILABLE` line, whose Root cause read "The contextual-orchestrator gateway or its discovered provider pool was unavailable for this run". `#1953` had just given the Strix sandbox bootstrap failure its own second verdict token (`STRIX_SANDBOX_UNAVAILABLE`) precisely because that attribution is wrong for it -- the sandbox container never reaches its Caido proxy, so the run dies before the gateway serves anything -- and this consumer re-applied the wrong attribution one step downstream, into the review findings and the failure census. The emitter now branches on the second token: a sandbox verdict gets a finding that names Strix's sandbox, says the verdict does not name the gateway, and tells the reader not to change gateway or provider configuration on its strength. A `STRIX_PROVIDER_UNAVAILABLE` line without the token keeps its existing text verbatim, so the gateway class has no regression surface. No test covered this finding text at all before (`gateway or its discovered provider pool` matched nothing under `tests/`); `tests/test_opencode_dispatch_strix_sandbox_finding.py` now runs the production emitter from the published run block and pins both directions plus the no-signal case. Refs #1953, #1935. diff --git a/docs/doctoring/strix-manual-dispatch-not-merge-evidence.md b/docs/doctoring/strix-manual-dispatch-not-merge-evidence.md new file mode 100644 index 0000000000..b24de93240 --- /dev/null +++ b/docs/doctoring/strix-manual-dispatch-not-merge-evidence.md @@ -0,0 +1,47 @@ +# Manual Strix workflow dispatch is not scheduler evidence + +검토 기준일: **2026-09-07** + +## Problem + +A caller-selected `workflow_dispatch` run can use the same workflow and job +display names as the required Strix run. If the scheduler deduplicates only by +those names, a newer manual Deep run can hide, block, or become the rerun target +for required `pull_request_target` or `repository_dispatch` evidence. + +## Decision + +The central scheduler reads `checkSuite.workflowRun.event` in every paginated +GraphQL context page. CheckRun rerun identity is +`(workflow name, job name, event)`. A Strix `workflow_dispatch` CheckRun is excluded +from Strix evidence, failed-check collection, action-required collection, job +selection, and active-run suppression. + +A classic successful `strix` commit status remains a bounded reviewer signal +for the self-modifying base-branch catch-up case. It does not replace GitHub's +required CheckRun at merge time. + +Missing event data is not classified as manual and therefore remains +authoritative/fail-closed. The repair does not weaken a required failure and +does not synthesize success. + +## Verification contract + +`tests/test_strix_manual_dispatch_isolation.py` proves that a newer manual run +cannot deduplicate away an older required failure, cannot become a rerun target, +cannot create ACTION_REQUIRED debt, and cannot suppress the required dispatch. +Hosted exact-head checks remain mandatory. + +## Status and rollback + +Status: **Proposed** in ContextualWisdomLab/.github#1061. Protected `main` +remains the release authority. Roll back only if GitHub stops exposing +`WorkflowRun.event`; absence must continue to fail closed as non-manual. + +## References + +GitHub. (n.d.). *Manually running a workflow*. GitHub Docs. Retrieved September +7, 2026, from https://docs.github.com/en/actions/how-tos/manage-workflow-runs/manually-run-a-workflow + +GitHub. (n.d.). *Objects: WorkflowRun*. GitHub GraphQL API. Retrieved September +7, 2026, from https://docs.github.com/en/graphql/reference/objects#workflowrun diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 1cc9e20313..d376a2e729 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -7,6 +7,12 @@ 이 문서는 제품·기술·운영 Gap을 현재 문서와 현재 GitHub 상태에 묶어 두는 기준선이다. 새 작업은 먼저 이 문서의 Gap ID를 PR 설명과 테스트 증거에 연결하고, PR의 정확한 exact HEAD·Checks·리뷰를 다시 수집한 뒤 구현한다. 표의 상태는 작성 시점의 관측값이므로, 병합 판단에는 재사용하지 않는다. 이 인벤토리는 스냅샷이며 merge authorization이 아니다. +### 2026-09-07 Strix manual-dispatch authority amendment + +- **Gap:** workflow/job display-name-only deduplication lets a newer caller-selected `workflow_dispatch` run displace required Strix evidence. +- **Action:** ContextualWisdomLab/.github#1061 binds CheckRun identity to the Actions event and excludes manual Strix CheckRuns from scheduler authority while preserving fail-closed missing-event behavior. +- **Status:** Proposed; exact-head hosted Checks, independent review, ordinary protected integration, and post-merge current-main verification remain required. + ## 1. 근거와 범위 ### 1.1 우선순위가 높은 근거 diff --git a/scripts/ci/pr_review_merge_scheduler_core.py b/scripts/ci/pr_review_merge_scheduler_core.py index 4df4dac3de..09e6678ba2 100644 --- a/scripts/ci/pr_review_merge_scheduler_core.py +++ b/scripts/ci/pr_review_merge_scheduler_core.py @@ -234,6 +234,7 @@ def live_dispatch_head_matches(repo: str, pr: dict[str, Any]) -> bool: checkSuite { createdAt workflowRun { + event workflow { name } } } @@ -307,7 +308,7 @@ def live_dispatch_head_matches(repo: str, pr: dict[str, Any]) -> bool: __typename ... on CheckRun { name status conclusion startedAt detailsUrl - checkSuite { createdAt workflowRun { workflow { name } } } + checkSuite { createdAt workflowRun { event workflow { name } } } } ... on StatusContext { context state } } @@ -1650,9 +1651,38 @@ def is_opencode_context(node: dict[str, Any]) -> bool: return node.get("context") == "opencode-review" +def workflow_run_event(node: dict[str, Any]) -> str: + """Return the GitHub Actions event that created one check run, if present.""" + workflow_run = ((node.get("checkSuite") or {}).get("workflowRun") or {}) + return str(workflow_run.get("event") or "").strip() + + +def is_manual_workflow_dispatch(node: dict[str, Any]) -> bool: + """Return whether a check run came from caller-selected workflow_dispatch.""" + return ( + node.get("__typename") == "CheckRun" + and workflow_run_event(node) == "workflow_dispatch" + ) + + +def is_manual_strix_workflow_dispatch(node: dict[str, Any]) -> bool: + """Return whether a check run is a caller-selected manual Strix run.""" + if not is_manual_workflow_dispatch(node): + return False + workflow = ( + ((node.get("checkSuite") or {}).get("workflowRun") or {}).get("workflow") + or {} + ) + return workflow.get("name") in {"Strix Security Scan", "Strix"} or ( + node.get("name") == "strix" + ) + + def is_strix_context(node: dict[str, Any]) -> bool: - """Return whether a check or status context belongs to Strix evidence.""" + """Return whether a context is authoritative Strix scheduler evidence.""" if node.get("__typename") == "CheckRun": + if is_manual_strix_workflow_dispatch(node): + return False workflow = ( ((node.get("checkSuite") or {}).get("workflowRun") or {}).get("workflow") or {} @@ -1735,7 +1765,7 @@ def check_run_recency_key( """Return a single comparable recency key for one same-purpose check run. Ranking a sequence of same-purpose check runs (either the reruns sharing - one (workflow, name) key in ``latest_check_runs``, or the + one (workflow, name, event) key in ``latest_check_runs``, or the coverage-evidence runs ``latest_coverage_evidence_index`` compares across workflow names) down to the single newest one used to be done by folding a pairwise "does B supersede A" predicate left-to-right across the @@ -1803,26 +1833,27 @@ def check_run_recency_key( def _newest_check_run_per_identity( indexed_check_runs: Sequence[tuple[int, dict[str, Any]]] ) -> list[tuple[int, dict[str, Any]]]: - """Return the newest CheckRun per (workflow, name) identity, index-tagged. + """Return the newest CheckRun per (workflow, name, event) identity, index-tagged. Shared core for ``latest_check_runs`` (which keeps only CheckRun nodes) and ``latest_check_run_attempts`` (which also passes non-CheckRun nodes through unchanged): both resolve CheckRun reruns sharing one - (workflow, name) identity down to the single newest attempt, and both - must rank candidates with the identical ``check_run_recency_key`` signal + (workflow, name, event) identity down to the single newest attempt. The + event keeps manual and required executions distinct even when their display + names match. Both must rank candidates with the identical ``check_run_recency_key`` signal so they cannot silently diverge again the way ``latest_check_run_attempts`` once did with its own ``startedAt``-only comparison. Each input ``(index, node)`` pair's original position is preserved in the return value so callers can restore overall document order after merging back any non-CheckRun nodes. """ - latest: dict[tuple[str, str], tuple[tuple[int, datetime, int], int, dict[str, Any]]] = {} + latest: dict[tuple[str, str, str], tuple[tuple[int, datetime, int], int, dict[str, Any]]] = {} for index, node in indexed_check_runs: workflow = ( (((node.get("checkSuite") or {}).get("workflowRun") or {}).get("workflow") or {}).get("name") or "" ) - key = (workflow, node.get("name") or "check-run") + key = (workflow, node.get("name") or "check-run", workflow_run_event(node)) started_at = parse_github_datetime(node.get("startedAt")) recency_key = check_run_recency_key(node, started_at, index) previous = latest.get(key) @@ -1832,7 +1863,7 @@ def _newest_check_run_per_identity( def latest_check_runs(pr: dict[str, Any]) -> list[dict[str, Any]]: - """Return the newest check run for each workflow and check-name pair.""" + """Return the newest check run for each workflow, check-name, and event identity.""" indexed_check_runs = [ (index, node) for index, node in enumerate(context_nodes(pr)) @@ -1913,7 +1944,7 @@ def has_in_flight_check_runs(pr: dict[str, Any]) -> bool: def latest_check_run_attempts(nodes: list[dict[str, Any]]) -> list[dict[str, Any]]: - """Return each CheckRun's most recent attempt per (workflow, name) identity. + """Return each CheckRun's latest attempt per (workflow, name, event) identity. A rerun leaves every earlier attempt's CheckRun node in the rollup alongside the latest one, so callers that walk ``nodes`` directly can see @@ -2537,6 +2568,8 @@ def failed_status_checks( if (node.get("state") or "").upper() == "SUCCESS" } for index, node in enumerate(check_runs): + if is_manual_strix_workflow_dispatch(node): + continue if is_non_authoritative_coverage_check_run(node): continue conclusion = (node.get("conclusion") or "").upper() @@ -2565,6 +2598,8 @@ def action_required_checks(pr: dict[str, Any]) -> list[str]: for node in context_nodes(pr): if node.get("__typename") != "CheckRun": continue + if is_manual_strix_workflow_dispatch(node): + continue conclusion = (node.get("conclusion") or "").upper() if conclusion in ACTION_REQUIRED_CONCLUSIONS: required.append(node.get("name") or "check-run") @@ -3290,6 +3325,14 @@ def active_review_run_refs( continue (current if dispatched_head == head else stale).append(run_ref) continue + if ( + run_data.get("event") == "workflow_dispatch" + and any( + candidate in {"Strix Security Scan", "Strix"} + for candidate in (workflow, run_title, *workflow_aliases) + ) + ): + continue if centralized_dispatch: continue run_head = str(run_data.get("head_sha") or "").lower() diff --git a/tests/test_strix_manual_dispatch_isolation.py b/tests/test_strix_manual_dispatch_isolation.py new file mode 100644 index 0000000000..20a9fd9b5d --- /dev/null +++ b/tests/test_strix_manual_dispatch_isolation.py @@ -0,0 +1,179 @@ +"""Regression contracts for manual Strix dispatch isolation.""" + +from __future__ import annotations + +from typing import Any + +from scripts.ci import pr_review_merge_scheduler_core as scheduler + + +def _check( + *, + event: str, + conclusion: str = "SUCCESS", + status: str = "COMPLETED", + created_at: str = "2026-09-07T00:00:00Z", + details_url: str | None = None, +) -> dict[str, Any]: + """Build one Strix CheckRun with an explicit Actions trigger.""" + node: dict[str, Any] = { + "__typename": "CheckRun", + "name": "strix", + "status": status, + "conclusion": conclusion, + "startedAt": created_at, + "checkSuite": { + "createdAt": created_at, + "workflowRun": { + "event": event, + "workflow": {"name": "Strix Security Scan"}, + }, + }, + } + if details_url is not None: + node["detailsUrl"] = details_url + return node + + +def _pull_request(*nodes: dict[str, Any]) -> dict[str, Any]: + """Build one current-head PR rollup.""" + return { + "number": 1061, + "headRefOid": "a" * 40, + "statusCheckRollup": {"contexts": {"nodes": list(nodes)}}, + } + + +def test_graphql_and_helpers_preserve_workflow_event() -> None: + """Both paginated query shapes retain the event used for authority.""" + assert "workflowRun {\n event" in scheduler.PULL_REQUEST_FIELDS_FRAGMENT + assert "workflowRun { event workflow { name } }" in scheduler.PR_CONTEXTS_PAGE_QUERY + assert scheduler.workflow_run_event({}) == "" + manual = _check(event=" workflow_dispatch ") + assert scheduler.workflow_run_event(manual) == "workflow_dispatch" + assert scheduler.is_manual_workflow_dispatch(manual) + assert not scheduler.is_strix_context(manual) + assert scheduler.is_strix_context(_check(event="pull_request_target")) + + +def test_newer_manual_run_cannot_hide_required_failure() -> None: + """A newer manual run stays distinct from required Strix evidence.""" + required = _check( + event="pull_request_target", + conclusion="FAILURE", + created_at="2026-09-07T00:00:00Z", + ) + manual = _check( + event="workflow_dispatch", + created_at="2026-09-07T00:01:00Z", + ) + pull_request = _pull_request(required, manual) + + assert len(scheduler.latest_check_runs(pull_request)) == 2 + assert scheduler.strix_evidence_state(pull_request) == "failed" + assert scheduler.failed_status_checks(pull_request) == ["strix"] + + +def test_manual_action_required_and_job_are_not_scheduler_authority() -> None: + """Manual Deep runs cannot block or become the required rerun target.""" + manual = _check( + event="workflow_dispatch", + conclusion="ACTION_REQUIRED", + details_url="https://github.com/o/r/actions/runs/1/job/11", + ) + required = _check( + event="repository_dispatch", + details_url="https://github.com/o/r/actions/runs/2/job/22", + ) + pull_request = _pull_request(manual, required) + + assert scheduler.action_required_checks(pull_request) == [] + assert ( + scheduler.matching_actions_job_id( + pull_request, + scheduler.is_strix_context, + ) + == "22" + ) + + +def test_active_review_runs_ignore_manual_dispatch(monkeypatch: Any) -> None: + """A same-head manual run cannot suppress the required Strix dispatch.""" + manual_run = { + "id": 9500, + "name": "Strix Security Scan", + "event": "workflow_dispatch", + "head_sha": "a" * 40, + "pull_requests": [{"number": 1061}], + } + monkeypatch.setattr( + scheduler, + "active_workflow_runs", + lambda repository, statuses=("queued", "in_progress"): [manual_run], + ) + monkeypatch.delenv("SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY", raising=False) + + current, stale = scheduler.active_review_run_refs( + "ContextualWisdomLab/.github", + "Strix Security Scan", + _pull_request(), + run_title="Strix Security Scan", + workflow_aliases=frozenset({"Strix"}), + ) + + assert current == [] + assert stale == [] + +def test_manual_non_strix_checks_remain_scheduler_authority() -> None: + """Manual non-Strix failures remain visible to the central scheduler.""" + failed = { + "__typename": "CheckRun", + "name": "dependency-review", + "status": "COMPLETED", + "conclusion": "FAILURE", + "startedAt": "2026-09-07T00:00:00Z", + "checkSuite": { + "createdAt": "2026-09-07T00:00:00Z", + "workflowRun": { + "event": "workflow_dispatch", + "workflow": {"name": "Security Scan"}, + }, + }, + } + blocked = { + **failed, + "name": "release-approval", + "conclusion": "ACTION_REQUIRED", + } + pull_request = _pull_request(failed, blocked) + + assert scheduler.failed_status_checks(pull_request) == ["dependency-review"] + assert scheduler.action_required_checks(pull_request) == ["release-approval"] + + +def test_manual_non_strix_run_remains_active(monkeypatch: Any) -> None: + """Manual OpenCode activity is not silently reclassified as Strix.""" + manual_run = { + "id": 9600, + "name": "Required OpenCode Review", + "event": "workflow_dispatch", + "head_sha": "a" * 40, + "pull_requests": [{"number": 1061}], + } + monkeypatch.setattr( + scheduler, + "active_workflow_runs", + lambda repository, statuses=("queued", "in_progress"): [manual_run], + ) + monkeypatch.delenv("SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY", raising=False) + + current, stale = scheduler.active_review_run_refs( + "ContextualWisdomLab/.github", + "Required OpenCode Review", + _pull_request(), + run_title="Required OpenCode Review", + workflow_aliases=frozenset(scheduler.OPENCODE_WORKFLOW_NAMES), + ) + + assert current == [("ContextualWisdomLab/.github", "9600")] + assert stale == []