diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index c48db831f..d5b0c6a64 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -104,13 +104,17 @@ sequenceDiagram OC->>SV: PoC command in isolated copy SV-->>OC: redacted stdout/stderr + command metadata OC-->>PR: APPROVE or request changes - MS->>PR: merge only on current-head approval + green checks + MS->>PR: merge only on current-head approval + required Strix evidence ``` ## Trust boundaries - Required review workflows execute **base-branch** scripts. A PR that edits those workflows cannot widen its own `pull_request_target` token. +- Merge evidence for Strix is `pull_request_target` and + `repository_dispatch` `strix-scan` only. A caller-selected + `workflow_dispatch` check run (including Deep) must not park, fail, or + satisfy the merge scheduler. - Reviewer agents stay `edit: deny`. They judge; they do not implement. - Sandbox helpers copy the workspace, drop secret environment values unless explicitly allowlisted by **name**, and run subprocesses with `shell=False`. @@ -145,6 +149,8 @@ trusted `uv` exporter is downloaded from the literal GitHub Releases URL for contract. - [`docs/doctoring/hourly-nvidia-nim-autofix.md`](docs/doctoring/hourly-nvidia-nim-autofix.md) — current increment's repair-worker decision and APA 7th citations. +- [`docs/doctoring/strix-manual-dispatch-not-merge-evidence.md`](docs/doctoring/strix-manual-dispatch-not-merge-evidence.md) + — manual Deep `workflow_dispatch` is not merge evidence. - [`docs/doctoring/fast-mlsirm-hourly-review-caller.md`](docs/doctoring/fast-mlsirm-hourly-review-caller.md) — product-specific psychometric repair heartbeat and scientific gates. - [`docs/doctoring/exact-artifact-sbom-attestation.md`](docs/doctoring/exact-artifact-sbom-attestation.md) diff --git a/CHANGELOG.md b/CHANGELOG.md index f4903c2f3..4723f0d73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,8 @@ Semantic Versioning where the repository publishes a release. - Download the pinned `uv` 0.12.1 exporter from the official GitHub Releases URL instead of `releases.astral.sh`, which now returns HTTP 403 and blocks org-wide OpenCode `coverage-evidence`. The SHA-256 pin is unchanged. The opener may follow one hop onto `release-assets.githubusercontent.com` or `objects.githubusercontent.com` and still rejects every other host, userinfo, non-HTTPS scheme, and nondefault port (ContextualWisdomLab/.github#1109). - Compared the trusted `uv` executable's post-install `--version` output against the real GitHub Releases build's full string, `uv 0.12.1 (x86_64-unknown-linux-gnu)`, instead of the bare `uv 0.12.1` the prior check required; the genuine release binary always prints the target triple, so every installation was failing the pin check immediately after the archive download itself was fixed (ContextualWisdomLab/.github#1109). - Excluded relative `-r` and `--requirement` referrers from generated flat base-lock publication while retaining bounded include syntax diagnostics and discovering independently complete direct `.txt` children of `requirements` directories. +- Ignore caller-selected `workflow_dispatch` Strix check runs in merge evidence so a manual Deep scan cannot park, fail, or satisfy the scheduler for up to six hours. Required evidence remains `pull_request_target` and `repository_dispatch` `strix-scan`; a successful `strix` commit status can still supersede a failed required check. +- Materialized base Python locks only when every package line is an exact SHA-256 pin or a bounded relative `-r`/`--requirement` include. A lone `--require-hashes` directive, a dotted include such as `./lock.txt`, or `-r other-hashes.txt` no longer enters the trusted build context. - Refused a conflict-scope repository root whose immediate parent is a symbolic link, so a swapped parent cannot redirect the canonical worktree after the last-component check (CWE-367). - Materialized base Python locks only when every package line is an exact SHA-256 pin or a bounded relative `-r`/`--requirement` include. A lone `--require-hashes` directive, a dotted include such as `./lock.txt`, or `-r other-hashes.txt` no longer enters the trusted build context. - Bounded the Strix quality self-test's deterministic timeout fixtures to 3-second process and 5-second fake-sleep budgets so exact-head policy evidence completes inside the existing job limit without changing production Strix scanner timeouts, providers, credentials, or review semantics. diff --git a/CLAUDE.md b/CLAUDE.md index 02d6b3d84..2fa091f84 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -123,8 +123,9 @@ repeatable compile command. on the worker, never `COPILOT_GITHUB_TOKEN`. - **`pull_request_target` trust boundary.** The required review workflows run the *base branch's* trusted scripts. A PR that edits the trusted review workflows can fail its own checks until the - base branch catches up; a same-head manual `workflow_dispatch` Strix run may supply review evidence - but does not replace required PR checks. Do not widen a `pull_request_target` job token to + base branch catches up; a same-head manual `workflow_dispatch` Strix run may + inform a reviewer but must not park, fail, or satisfy merge evidence, and it + does not replace required PR checks. Do not widen a `pull_request_target` job token to repository-write permission. - **Review output must go through the Python normalizer** (`scripts/ci/opencode_review_normalize_output.py`) — it escapes `<`, `>`, `&` when embedding JSON in HTML comments to prevent Markdown-comment diff --git a/README.md b/README.md index 94e91fe4d..af07a827a 100644 --- a/README.md +++ b/README.md @@ -45,6 +45,26 @@ reconciliation do not require naruon to be present, imported, or running. Every CWL component is standalone and also composable. For this repository that means: + +- `naruon`: approved PRs can become `BEHIND`; the scheduler treats that as an + update request, not as a merge signal. GitHub Actions updates the branch with + `expected_head_sha`, then the new head is reviewed again. +- `pg-erd-cloud`: successful bot merges used current-head evidence and + `--match-head-commit`; the centralized path keeps that head-SHA guard. +- `.github`: PRs that edit trusted review workflows can fail because + `pull_request_target` runs the base branch's trusted scripts. A same-head + manual `workflow_dispatch` Strix run may inform a reviewer, but it does not + park, fail, or satisfy merge evidence. Required PR checks stay + `pull_request_target` and `repository_dispatch` `strix-scan` until the + trusted base branch catches up. +- `naruon#745`: new OpenCode review-flow work improves Mermaid output by + replacing generic risk sketches with changed-file flow DAGs. The central + workflow carries that review contract while keeping the self-test drift fix. +- Cross-repo DX/UX: helpful sibling-repo patterns should be adopted when they + reduce maintainer, reviewer, CI-operator, contributor, user, or reader + friction. Noisy automation, repeated waiting, false failures, misleading + statuses, and URL-only diagnostics are treated as review-experience defects. + | Mode | What happens | | --- | --- | | **따로 (this repo alone)** | Clone, test, and operate `.github` as the org profile and workflow source. Local quality gates, Cloudflare dry-run, and this repository's own PRs do not depend on naruon or any sibling product checkout. | 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 000000000..c6ffe5a24 --- /dev/null +++ b/docs/doctoring/strix-manual-dispatch-not-merge-evidence.md @@ -0,0 +1,78 @@ +# Manual Strix `workflow_dispatch` is not merge evidence + +검토 기준일: **2026-08-16** + +## Incident + +The merge scheduler treated every `Strix Security Scan` / `strix` check run as +required current-head evidence. A caller-selected `workflow_dispatch` run +publishes that same check name. Official Deep mode can occupy the GitHub-hosted +360-minute ceiling. An in-progress or failed Deep job on a pull-request head +therefore parked `strix_evidence_state()`, could replace a later +`failed_status_checks()` winner for the same workflow/name key, and could +suppress `repository_dispatch` `strix-scan` retry when the scheduler was not in +centralized-dispatch mode. + +GitHub's Actions UI and `gh workflow run --ref` let a writer choose the +workflow revision (GitHub, n.d.-a). That revision supplies the workflow +definition before any trusted-source checkout. Manual Deep remains a reviewer +tool, not a merge gate. + +## Decision + +Required Strix merge evidence is only: + +- `pull_request_target` check runs, and +- `repository_dispatch` type `strix-scan` check runs. + +The scheduler now: + +1. reads `checkSuite.workflowRun.event` on each check run; +2. ignores `workflow_dispatch` check runs in `is_strix_context`, + `strix_evidence_state`, `failed_status_checks`, `action_required_checks`, + and `matching_actions_job_id`; +3. treats a completed required Strix check run as complete even when a later + pending `strix` commit status exists; +4. skips `workflow_dispatch` workflow runs in non-central `active_review_run_refs` + so a same-head Deep job cannot return `already_running` and block + `strix-scan`. + +A successful `strix` commit status can still supersede a failed required check +run. That path informs a reviewer and remains the documented exception for +base-branch catch-up. It does not let a Deep check run park merge. + +Do not fold this change into ContextualWisdomLab/.github#1054. That pull +request owns the official `quick|standard|deep` mapping. This record owns +merge-evidence isolation so #1054 can land without a six-hour merge stall. + +## Verification contract + +`tests/test_pr_review_merge_scheduler.py` fails if: + +1. `PULL_REQUEST_FIELDS_FRAGMENT` drops `workflowRun.event`; +2. a running or failed `workflow_dispatch` Strix check run changes + `strix_evidence_state` or `failed_status_checks` when a required check run + is present; +3. a same-head `workflow_dispatch` workflow run is classified as current + evidence in non-central `active_review_run_refs`; +4. `matching_actions_job_id` returns a Deep job over a required job. + +## Rollback + +Roll back only if a required `pull_request_target` or `strix-scan` check run is +observed ignored because GitHub omitted `event` and the run was mis-labeled +`workflow_dispatch`. Missing `event` stays required evidence. + +## References (APA 7th) + +GitHub. (n.d.-a). *Manually running a workflow*. GitHub Docs. Retrieved +August 16, 2026, from +https://docs.github.com/en/actions/how-tos/manage-workflow-runs/manually-run-a-workflow + +GitHub. (n.d.-b). *Workflow syntax for GitHub Actions*. GitHub Docs. Retrieved +August 16, 2026, from +https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax + +GitHub. (n.d.-c). *Objects: WorkflowRun* (`event`). GitHub GraphQL API. +Retrieved August 16, 2026, from +https://docs.github.com/en/graphql/reference/objects#workflowrun diff --git a/requirements-pip-audit-ci-hashes.txt b/requirements-pip-audit-ci-hashes.txt index ade197a49..0ae099d8f 100644 --- a/requirements-pip-audit-ci-hashes.txt +++ b/requirements-pip-audit-ci-hashes.txt @@ -213,9 +213,9 @@ packaging==26.2 \ # via # pip-audit # pip-requirements-parser -pip==26.1.2 \ - --hash=sha256:382ff9f685ee3bc25864f820aa50505825f10f5458ffff07e30a6d96e5715cab \ - --hash=sha256:f49cd134c61cf2fd75e0ce2676db03e4054504a5a4986d00f8299ae632dc4605 +pip==26.2.1 \ + --hash=sha256:71138adf1f4ca900cdb7d289c21b7494329f2332b6d85f0e1c42108c0384ed3e \ + --hash=sha256:f6ad667e89a1fe78046c8f13232b247200f5258d7828f3f7883d660878e0813f # via pip-api pip-api==0.0.34 \ --hash=sha256:8b2d7d7c37f2447373aa2cf8b1f60a2f2b27a84e1e9e0294a3f6ef10eb3ba6bb \ diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 118d0d903..de2768cd2 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -73,6 +73,7 @@ detailsUrl checkSuite { workflowRun { + event workflow { name } } } @@ -953,9 +954,28 @@ 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 this 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 was created by caller-selected workflow_dispatch.""" + return workflow_run_event(node) == "workflow_dispatch" + + def is_strix_context(node: dict[str, Any]) -> bool: - """Return whether a check or status context belongs to Strix evidence.""" + """Return whether a check or status context is required Strix merge evidence. + + Manual ``workflow_dispatch`` check runs can publish the same job name for + up to six hours (Deep). They may inform a reviewer but must not park, + fail, or satisfy the merge scheduler. A successful ``strix`` commit status + can still supersede a failed required check run. + """ if node.get("__typename") == "CheckRun": + if is_manual_workflow_dispatch(node): + return False workflow = ( ((node.get("checkSuite") or {}).get("workflowRun") or {}).get("workflow") or {} @@ -1062,17 +1082,31 @@ 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, or complete for current-head Strix evidence. + + A completed required Strix check run is enough. A later pending + ``strix`` commit status from a manual Deep dispatch must not park merge. + """ found = False + completed_required_check_run = False + running_required = False for node in context_nodes(pr): if not is_strix_context(node): continue found = True status = (node.get("status") or node.get("state") or "").upper() - if status in RUNNING_CHECK_STATES: - return "running" - if node.get("__typename") == "CheckRun" and status != "COMPLETED": - return "running" + is_check_run = node.get("__typename") == "CheckRun" + if status in RUNNING_CHECK_STATES or ( + is_check_run and status != "COMPLETED" + ): + running_required = True + continue + if is_check_run: + completed_required_check_run = True + if completed_required_check_run: + return "complete" + if running_required: + return "running" return "complete" if found else "missing" @@ -1387,6 +1421,8 @@ def failed_status_checks(pr: dict[str, Any]) -> list[str]: if node.get("__typename") != "CheckRun": status_contexts.append(node) continue + if is_manual_workflow_dispatch(node): + continue workflow = ( (((node.get("checkSuite") or {}).get("workflowRun") or {}).get("workflow") or {}).get("name") or "" @@ -1435,6 +1471,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_workflow_dispatch(node): + continue conclusion = (node.get("conclusion") or "").upper() if conclusion in ACTION_REQUIRED_CONCLUSIONS: required.append(node.get("name") or "check-run") @@ -1947,6 +1985,8 @@ def active_review_run_refs( continue if centralized_dispatch: continue + if run_data.get("event") == "workflow_dispatch": + continue run_head = str(run_data.get("head_sha") or "").lower() pull_requests = run_data.get("pull_requests") or [] if run_head == head: diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index f2dd25813..6fc2df292 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -81,13 +81,22 @@ def opencode_review( } -def strix_check(status="COMPLETED", conclusion="SUCCESS", workflow="Strix Security Scan", details_url=None): +def strix_check( + status="COMPLETED", + conclusion="SUCCESS", + workflow="Strix Security Scan", + details_url=None, + event=None, +): + workflow_run = {"workflow": {"name": workflow}} + if event is not None: + workflow_run["event"] = event value = { "__typename": "CheckRun", "name": "strix", "status": status, "conclusion": conclusion, - "checkSuite": {"workflowRun": {"workflow": {"name": workflow}}}, + "checkSuite": {"workflowRun": workflow_run}, } if details_url: value["detailsUrl"] = details_url @@ -1027,6 +1036,92 @@ def test_context_review_and_check_helpers(monkeypatch): sched.strix_evidence_state(make_pr(statusCheckRollup={"contexts": {"nodes": [strix_check(conclusion="FAILURE")]}})) == "complete" ) + assert "event\n workflow { name }" in sched.PULL_REQUEST_FIELDS_FRAGMENT + assert sched.workflow_run_event({}) == "" + assert sched.workflow_run_event({"checkSuite": {}}) == "" + assert sched.workflow_run_event({"checkSuite": {"workflowRun": {}}}) == "" + assert ( + sched.workflow_run_event( + {"checkSuite": {"workflowRun": {"event": " workflow_dispatch "}}} + ) + == "workflow_dispatch" + ) + assert sched.is_manual_workflow_dispatch(strix_check(event="workflow_dispatch")) + assert not sched.is_manual_workflow_dispatch(strix_check(event="pull_request_target")) + assert not sched.is_strix_context(strix_check(event="workflow_dispatch")) + assert sched.is_strix_context(strix_check(event="pull_request_target")) + assert sched.is_strix_context(strix_check(event="repository_dispatch")) + running_manual_with_required = make_pr( + statusCheckRollup={ + "contexts": { + "nodes": [ + strix_check(status="IN_PROGRESS", event="workflow_dispatch"), + strix_check(event="pull_request_target"), + {"context": "strix", "state": "PENDING"}, + ] + } + } + ) + assert sched.strix_evidence_state(running_manual_with_required) == "complete" + assert sched.strix_evidence_state( + make_pr( + statusCheckRollup={ + "contexts": { + "nodes": [strix_check(status="IN_PROGRESS", event="workflow_dispatch")] + } + } + ) + ) == "missing" + assert sched.strix_evidence_state( + make_pr(statusCheckRollup={"contexts": {"nodes": [strix_check(event="workflow_dispatch")]}}) + ) == "missing" + assert sched.strix_evidence_state( + make_pr(statusCheckRollup={"contexts": {"nodes": [{"context": "strix", "state": "SUCCESS"}]}}) + ) == "complete" + failed_manual = make_pr( + statusCheckRollup={ + "contexts": { + "nodes": [ + strix_check(conclusion="FAILURE", event="workflow_dispatch"), + strix_check(event="pull_request_target"), + ] + } + } + ) + assert sched.failed_status_checks(failed_manual) == [] + assert sched.action_required_checks( + make_pr( + statusCheckRollup={ + "contexts": { + "nodes": [ + strix_check( + conclusion="ACTION_REQUIRED", + event="workflow_dispatch", + ) + ] + } + } + ) + ) == [] + assert sched.matching_actions_job_id( + make_pr( + statusCheckRollup={ + "contexts": { + "nodes": [ + strix_check( + event="workflow_dispatch", + details_url="https://github.com/owner/repo/actions/runs/2/job/99", + ), + strix_check( + event="pull_request_target", + details_url="https://github.com/owner/repo/actions/runs/2/job/22", + ), + ] + } + } + ), + sched.is_strix_context, + ) == "22" threaded = make_pr( reviewThreads={ @@ -2285,6 +2380,37 @@ def fake_run(args, stdin=None): ) +def test_active_review_run_refs_ignores_manual_strix_dispatch(monkeypatch): + """A same-head Deep workflow_dispatch must not suppress required strix-scan.""" + head_sha = "a" * 40 + runs = [ + { + "id": 9500, + "name": "Strix Security Scan", + "event": "workflow_dispatch", + "head_sha": head_sha, + "pull_requests": [{"number": 1}], + }, + ] + monkeypatch.setattr( + sched, + "active_workflow_runs", + lambda repo, statuses=("queued", "in_progress"): runs, + ) + monkeypatch.delenv("SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY", raising=False) + + current, stale = sched.active_review_run_refs( + "owner/repo", + "Strix Security Scan", + make_pr(headRefOid=head_sha), + run_title="Strix Security Scan", + workflow_aliases=frozenset({"Strix Security Scan"}), + ) + + assert current == [] + assert stale == [] + + def test_central_run_filter_ignores_malformed_and_non_dispatch_titles(monkeypatch): head_sha = "a" * 40 central_runs = [