From 23a480bbad68f3e9c9c860a1e9a93464e9bc2b2c Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 26 Jun 2026 04:06:08 +0900 Subject: [PATCH 1/4] Handle workflow action-required PR checks --- PR_GOVERNANCE_AUDIT.md | 58 ++++++----- scripts/ci/pr_review_merge_scheduler.py | 100 ++++++++++++++++++- tests/test_pr_review_merge_scheduler.py | 125 +++++++++++++++++++++++- 3 files changed, 253 insertions(+), 30 deletions(-) diff --git a/PR_GOVERNANCE_AUDIT.md b/PR_GOVERNANCE_AUDIT.md index 462c67d62..f7c807aa2 100644 --- a/PR_GOVERNANCE_AUDIT.md +++ b/PR_GOVERNANCE_AUDIT.md @@ -1,6 +1,6 @@ # PR Governance Audit -Live check: 2026-06-25 10:49 KST, GitHub API via `gh` as `seonghobae`. +Live check: 2026-06-26 KST, GitHub API via `gh` as `seonghobae`. ## Canonical Policy @@ -16,6 +16,7 @@ OpenCode decides; GitHub Actions mutates. https://docs.github.com/en/rest/pulls/pulls#merge-a-pull-request, 2026-06-25 check). Do not widen `contents` just to support `update-branch`. - Old approvals and old checks are not merge evidence after a head SHA changes. +- OpenCode review evidence must be internally same-head as well as GitHub-attached same-head. If the review body includes `Gate evidence` with `Head SHA: `, that SHA must match the PR current `headRefOid`; otherwise the review is stale evidence even when GitHub attaches the review to the current commit. - Merge uses one path: current-head OpenCode approval, no unresolved review threads, required checks green or native auto-merge waiting on them, mergeable head, and no policy blocker. - Prefer `gh pr merge --auto --merge --match-head-commit ` when native auto-merge is enabled. - Use direct `gh pr merge --merge --match-head-commit ` only when the repo policy already allows immediate merge. @@ -47,29 +48,29 @@ them to a local defect. ## Live Repository Inventory -Live generated: 2026-06-25 15:12 KST via GitHub REST/GraphQL APIs. PR #28 post-merge refresh: 2026-06-23 16:05 KST. PR #37 post-merge refresh: 2026-06-23 21:50 KST. clearfolio PR #13 post-merge refresh: 2026-06-24 04:48 KST. Non-actionable Findings refresh: 2026-06-25 KST. PR #58 post-merge refresh: 2026-06-25 15:03 KST. +Live generated: 2026-06-26 KST via GitHub REST/GraphQL APIs. PR #28 post-merge refresh: 2026-06-23 16:05 KST. PR #37 post-merge refresh: 2026-06-23 21:50 KST. clearfolio PR #13 post-merge refresh: 2026-06-24 04:48 KST. Non-actionable Findings refresh: 2026-06-25 KST. PR #58, #65, #66, and #68 post-merge refreshes: 2026-06-25 KST. The current public non-fork inventory is 12 repositories; `VibeSec` is not in the current public non-fork set, and `appguardrail` is. | Repo | Flow | Default | Auto | Rulesets | Required checks | Stale dismissal | Open PRs | Workflows | Recent merged actor | |---|---:|---:|---:|---|---|---:|---:|---|---| -| `ContextualWisdomLab/.github` | GitHub Flow | `main` | on | `Lock default branch` | none | ruleset true | 25 | OpenCode Review; PR Review Merge Scheduler; Strix Security Scan | #58 `github-actions`; #53 `github-actions`; #51 `seonghobae` | -| `ContextualWisdomLab/bandscope` | Git Flow | `develop` | on | `Lock default branch` | `ci / build-and-test`, `dependency-review`, `security-audit`, `CodeQL`, `sbom`, `release-preflight`, `gate / build / windows`, `gate / build / macos`, `trivy-fs-scan` | ruleset true; classic false | 81 | OpenCode Review; PR Review Merge Scheduler | #427 `github-actions`; #408 `seonghobae`; #405 `seonghobae` | +| `ContextualWisdomLab/.github` | GitHub Flow | `main` | on | `Lock default branch` | none | ruleset true | 27 | OpenCode Review; PR Review Merge Scheduler; Strix Security Scan | mixed: `seonghobae`, `github-actions` | +| `ContextualWisdomLab/bandscope` | Git Flow | `develop` | on | `Lock default branch` | `CodeQL`, `ci / build-and-test`, `dependency-review`, `gate / build / macos`, `gate / build / windows`, `release-preflight`, `sbom`, `security-audit`, `trivy-fs-scan` | ruleset true; classic false | 79 | OpenCode Review; PR Review Merge Scheduler | mixed: `seonghobae`, `github-actions` | | `ContextualWisdomLab/clearfolio` | GitHub Flow | `main` | off | `PR` | none | ruleset false | 13 | OpenCode Review; PR Review Merge Scheduler; Strix Security Scan | #13 `seonghobae`; #9 `seonghobae`; #8 `seonghobae` | -| `ContextualWisdomLab/codec-carver` | GitHub Flow | `main` | on | `Lock default branch` | none | ruleset true | 8 | OpenCode Review; PR Review Merge Scheduler; Strix Security Scan | #103 `github-actions`; #98 `seonghobae`; #97 `opencode-agent` | +| `ContextualWisdomLab/codec-carver` | GitHub Flow | `main` | on | `Lock default branch` | none | ruleset true | 9 | OpenCode Review; PR Review Merge Scheduler; Strix Security Scan | mixed: `github-actions`, `opencode-agent`, `seonghobae` | | `ContextualWisdomLab/contextual-orchestrator` | GitHub Flow | `main` | off | none | none | none | 0 | none matched | none | -| `ContextualWisdomLab/ContextualWisdomLab.github.io` | GitHub Flow | `main` | on | `Lock default branch` | none | ruleset true | 6 | OpenCode Review; PR Review Merge Scheduler; Strix Security Scan | #15 `seonghobae`; #14 `seonghobae`; #13 `github-actions` | +| `ContextualWisdomLab/ContextualWisdomLab.github.io` | GitHub Flow | `main` | on | `Lock default branch` | none | ruleset true | 9 | OpenCode Review; PR Review Merge Scheduler; Strix Security Scan | mixed: `github-actions`, `seonghobae` | | `ContextualWisdomLab/hyosung-itx-slogan-brief` | GitHub Flow | `main` | off | `Do not delete any branches` | none | none | 0 | OpenCode Review; PR Review Merge Scheduler | #3 `seonghobae`; #2 `seonghobae`; #1 `seonghobae` | -| `ContextualWisdomLab/naruon` | Git Flow | `develop` | on | `Lock default branch`, `PR` | `strix`, `opencode-review` | ruleset true; classic true | 2 | OpenCode Review; PR Governance; PR Review Merge Scheduler; Strix Gate Self-Test; Strix Security Scan | #758 `seonghobae`; #757 `seonghobae`; #756 `seonghobae` | -| `ContextualWisdomLab/newsdom-api` | Git Flow | `develop` | on | `Lock default branch`, `mirror-classic-protection-main-develop` | `pytest`, `scorecard`, `codeql (python, actions)`, `dependency-review`, `quality-gate` | ruleset true | 4 | OpenCode Review; PR Review Merge Scheduler; Strix Security Scan | #207 `seonghobae`; #204 `seonghobae`; #202 `seonghobae` | -| `ContextualWisdomLab/pg-erd-cloud` | GitHub Flow | `main` | on | `Lock default branch` | none | ruleset true | 10 | OpenCode Review; PR Review Autofix; PR Review Fix Scheduler; PR Review Merge Scheduler; Strix Security Scan | #247 `github-actions`; #246 `github-actions`; #239 `github-actions` | -| `ContextualWisdomLab/scopeweave` | Git Flow | `develop` | on | `Lock default branch` | none | ruleset true | 7 | OpenCode Review; PR Review Merge Scheduler; Strix Gate Self-Test; Strix Security Scan | #124 `seonghobae`; #123 `seonghobae`; #118 `seonghobae` | -| `ContextualWisdomLab/VibeSec` | Git Flow | `develop` | on | `Lock default branch`, `PR` | none | mixed true/false | 4 | OpenCode Review; PR Review Merge Scheduler; Strix Security Scan | #126 `github-actions`; #122 `seonghobae`; #121 `seonghobae` | +| `ContextualWisdomLab/naruon` | Git Flow | `develop` | on | `Lock default branch`, `PR` | `opencode-review`, `strix` | ruleset true; classic true | 3 | OpenCode Review; PR Governance; PR Review Merge Scheduler; Strix Gate Self-Test; Strix Security Scan | `seonghobae` | +| `ContextualWisdomLab/newsdom-api` | Git Flow | `develop` | on | `Lock default branch`, `mirror-classic-protection-main-develop` | `codeql (python, actions)`, `dependency-review`, `pytest`, `quality-gate`, `scorecard` | ruleset true | 0 | OpenCode Review; PR Review Merge Scheduler; Strix Security Scan | `seonghobae` | +| `ContextualWisdomLab/pg-erd-cloud` | GitHub Flow | `main` | on | `Lock default branch` | none | ruleset true | 12 | OpenCode Review; PR Review Autofix; PR Review Fix Scheduler; PR Review Merge Scheduler; Strix Security Scan | mixed: `github-actions`, `seonghobae` | +| `ContextualWisdomLab/scopeweave` | Git Flow | `develop` | on | `Lock default branch` | none | ruleset true | 8 | OpenCode Review; PR Review Merge Scheduler; Strix Gate Self-Test; Strix Security Scan | `seonghobae` | +| `ContextualWisdomLab/appguardrail` | Git Flow | `develop` | on | `Lock default branch`, `PR` | none | ruleset true | 0 | OpenCode Review; PR Review Merge Scheduler; Strix Security Scan | `seonghobae` | ## Current Gaps By Repo | Repo | Gap | |---|---| -| `.github` | PR #37, #38, #41, #42, #49, and #58 are merged. PR #49 is the central proof that generic failed-check deflections are rejected before publication. PR #58 extends that contract so pending checks, check-rollup lookup failures, failed-check diagnosis gaps, conflict repair guidance, update-branch explanations, and scheduler decisions stay tool states or Actions Summary output instead of becoming user-facing Findings. Current dry-run still leaves most old PRs blocked by unresolved review threads or current-head `CHANGES_REQUESTED`; PR #44 is the only current `wait` case because auto-merge is already enabled. | -| `bandscope` | Required checks are repo-specific and broad; keep GitHub native auto-merge as the check interpreter. Live dry-run `28134181171` proved the repo-local scheduler still waited on an already enabled auto-merge request instead of updating a `BEHIND` approved PR, so PR #450 syncs the scheduler script, adds explicit update-branch workflow control, writes scheduler decisions to Actions Summary, and prevents failed-check mapping failures from being published as Findings. The current PR #450 head is `BEHIND` with `REVIEW_REQUIRED`, a failed `opencode-review`, and queued macOS jobs, so it is not merge/update proof. PR #378 remains the partial update-branch fixture: `github-actions[bot]` enabled auto-merge, but the current head is still `BEHIND` while OpenCode is in progress. The default branch currently has OpenCode Review and PR Review Merge Scheduler, but no Strix workflow. | +| `.github` | PR #37, #38, #41, #42, #49, #58, #65, #66, and #68 are merged. PR #49 is the central proof that generic failed-check deflections are rejected before publication. PR #58 extends that contract so pending checks, check-rollup lookup failures, failed-check diagnosis gaps, conflict repair guidance, update-branch explanations, and scheduler decisions stay tool states or Actions Summary output instead of becoming user-facing Findings. PR #65 adds explicit conflict guidance and workflow-token `update-branch`; PR #66 requires exact current-head approval by commit OID; PR #68 adds REST mergeability because GraphQL `mergeStateStatus` stayed stale after live updates. | +| `bandscope` | Required checks are repo-specific and broad; keep GitHub native auto-merge as the check interpreter. PR #459 merged the REST mergeability guard downstream. Scheduler run `28192186833` proved two current contracts: PR #450 emitted concrete conflict repair guidance instead of retrying `update-branch`, and PR #451/#446 requested `update-branch` with the workflow `GITHUB_TOKEN`, producing new heads authored by `github-actions[bot]`. That run also exposed a post-update `ACTION_REQUIRED` state with no jobs, so the scheduler must report workflow approval/policy wait rather than a source failure when it recurs. | | `clearfolio` | PR #13 is merged at `4bc17c6` after same-head manual Strix run `28051319530`, same-head manual OpenCode run `28051665082`, unresolved review threads `0`, and guarded merge against head `5fe1791`. Auto-merge remains off, so direct guarded merge is the repo path. | | `codec-carver` | PR #98 replaced the legacy scheduler with the central GitHub Actions path. Keep #94 as the historical negative sample because it used `opencode-agent` as a merge actor. | | `contextual-orchestrator` | No matching rulesets or review workflows; either opt in deliberately or mark unmanaged. | @@ -78,18 +79,18 @@ Live generated: 2026-06-25 15:12 KST via GitHub REST/GraphQL APIs. PR #28 post-m | `newsdom-api` | Ruleset-required checks must stay GitHub-interpreted. PR #207 has merged, so it is no longer an update-branch proof candidate. The remaining open PRs #187, #203, #205, and #206 currently block because the current head has no OpenCode approval. | | `pg-erd-cloud` | Good GitHub Actions merge samples; keep autofix workflows repo-local. | | `scopeweave` | PR #127 is the current representative trace. Dry-run `28147098767` selected `auto_merge`, but live run `28147157319` failed with `GraphQL: Resource not accessible by integration (mergePullRequest)` because merge through GitHub Actions requires a contents-write mutation surface. Commit `6601953` proved the tempting fix, but Scorecard immediately opened a Token-Permissions review thread against job-level `contents: write`; follow-up commit `c5c5530` restores `contents: read` and keeps update-branch on the lower-privilege PR-write path. Current head `c5c5530` is clean, approved, and green; it remains unmerged because Actions-based merge is an explicit repo policy exception, not the default rollout. | -| `VibeSec` | Actor history is mixed; central scheduler should make GitHub Actions or native auto-merge the only mechanical path. | +| `appguardrail` | Public non-fork repo discovered in the 2026-06-26 refresh. It follows Git Flow on `develop`, has the central review/merge workflow names, and has no open PRs at the snapshot, so it is a clean onboarding target for the central contract rather than a proof fixture. | ## Representative Evidence | Repo | Live evidence | Adopt | Reject | |---|---|---|---| | `naruon` | `develop`, strict required checks `opencode-review` and `strix`, stale review dismissal enabled. Open PRs show `BEHIND`, `DIRTY`, and `CHANGES_REQUESTED` cases. | Strict current-head evidence and stale-dismissal awareness. | Treating `BEHIND` as merge-ready. | -| `bandscope` | Workflow dry-run `28134181171` used the repo-local scheduler and reported `PR #367: wait: current head is approved; auto-merge already enabled`, while the central scheduler dry-run selected `update_branch` for the same `BEHIND` + current-head-approved class. PR #450 is the corrective rollout. Its first head also reproduced the bad generic failed-check Findings text; the second head patches that path and the stale review was dismissed. | Keep broad repo-specific required checks delegated to GitHub, but update outdated same-repo PR heads before relying on native auto-merge. Treat failed-check mapping failure as a review-tool state unless a source-backed finding exists. | Assuming an enabled auto-merge request means the PR branch is current enough to merge, or converting missing evidence into a PR Finding. | +| `bandscope` | Workflow dry-run `28134181171` used the repo-local scheduler and reported `PR #367: wait: current head is approved; auto-merge already enabled`, while the central scheduler dry-run selected `update_branch` for the same `BEHIND` + current-head-approved class. PR #459 is the downstream REST-mergeability rollout. Run `28192186833` then produced conflict guidance for #450 and `github-actions[bot]` branch updates for #451/#446, but those updated heads exposed `ACTION_REQUIRED` check runs with no jobs. | Keep broad repo-specific required checks delegated to GitHub, update outdated same-repo PR heads before relying on native auto-merge, and treat `ACTION_REQUIRED` as workflow approval/policy wait. | Assuming an enabled auto-merge request means the PR branch is current enough to merge, converting missing evidence into a PR Finding, or treating `ACTION_REQUIRED` as a failed source check. | | `.github` | PR #28 head `811446d` reached current-head approval after manual Strix run `28007326148` published a successful `strix` status and manual OpenCode run `28008174977` approved the same head; it was merged by `seonghobae` with merge commit `a025be1`. PR #49 then merged the explicit ban on generic failed-check deflections, and PR #58 removes remaining fallback/pending/check-lookup paths that could turn review-tool states into PR review Findings. | Same-head manual evidence for self-modifying trusted workflow changes, current-head OpenCode approval, unresolved thread check, `--match-head-commit` guarded merge, and non-actionable Findings rejection. | Treating stale PR-target failure logs as merge blockers after newer same-head evidence exists, or posting an evidence-mapping failure as a user-facing Finding. | | `pg-erd-cloud` | Recent PRs #236, #237, #239 were merged by `app/github-actions`. | GitHub Actions as mechanical merge actor with head guard. | Human-only queue draining. | | `codec-carver` | Recent PR #94 was merged by `app/opencode-agent`, and the repo still has legacy `Scheduled PR Review Merge`. | Native auto-merge path for current-head approved PRs. | OpenCode app as merge actor. | -| `VibeSec` | PR #108 had native auto-merge enabled; #106 merged by `app/github-actions`; #109 merged by human. | Keep native auto-merge as preferred waiting path. | Repo-by-repo actor inconsistency. | +| `appguardrail` | Current public non-fork repo with default `develop`, central workflow names, and no open PRs at snapshot time. | Use as a clean onboarding/control repo after central changes stabilize. | Treating zero open PRs as proof that the workflow behavior is already correct. | ## DX/UX Transfer Decisions @@ -103,7 +104,7 @@ both separately; a change can improve one while harming the other. | `.github` | Same-head manual evidence and `--match-head-commit` make self-modifying workflow changes reviewable without pretending stale base-branch checks are current. | Stale `pull_request_target` failures, long polling review runs, and cancelled helper checks can become misleading review noise. | Serialize Strix before OpenCode, bound approval runtime, and require failed-check explanations instead of URL-only comments. | | `naruon` | Strict required checks, stale review dismissal, changed-file Mermaid flow DAGs, and current-head evidence make review evidence easier to audit. | The repo-local scheduler is stale: it has no update-branch path, no Strix-before-OpenCode sequencing, and no failed-check interpretation from the central script. Run `28073490721` also showed that an auto-merge permission failure can stop the whole queue before later PRs are inspected. PR #756 additionally showed that broadening workflow permissions is a tempting DX shortcut, but it degrades review trust and triggers Scorecard/governance failures. | Sync the central scheduler into the repo, re-review every updated head, require an exact changed-file evidence path plus a Change Flow DAG before approval, keep `actions: read`/`contents: read` unless a separate privileged workflow is deliberately introduced, and record action failures per PR instead of aborting the scan. | | `pg-erd-cloud` | GitHub Actions bot merges with head guards give a clear mechanical actor for merges. | Repo-local autofix workflows are useful there, but centralizing autofix would widen mutation scope too far. | Keep GitHub Actions as the merge actor and leave autofix workflows repo-local. | -| `VibeSec` | Native auto-merge examples show a lower-friction waiting path after current-head approval. | Mixed human, OpenCode, and GitHub Actions merge actors make audit trails harder to interpret. | Prefer native auto-merge or GitHub Actions mutation; do not let OpenCode merge directly. | +| `appguardrail` | Security-review subject matter makes it a useful place to verify that review automation distinguishes policy failure, tool failure, and source-code failure. | With no open PRs in the snapshot, it cannot yet prove update-branch or merge behavior. | Onboard central scheduler changes deliberately, then use the next real PR as a low-noise policy-vs-source review fixture. | | `bandscope` | Broad required checks encode repo-specific release, build, SBOM, and security expectations. | A central script would be noisy if it tried to reinterpret every required check itself. | Let GitHub native auto-merge and rulesets interpret required checks. | | `newsdom-api` | Required quality gates and security checks give API changes stronger release evidence. | Central review comments that only point at failing check URLs do not help an API maintainer fix the failure. | Require failed-check root cause, source location when available, fix direction, and rerun command. | | `scopeweave` | Strix self-test and the central scheduler are useful rollout fixtures. Live scheduler run `28147157319` proved `action_error` is reported per PR instead of aborting the queue, and follow-up `c5c5530` shows the safer rollback when Scorecard rejects a broad token. | The scheduler could identify #127 as merge-ready, but enabling GitHub Actions merge by adding job-level `contents: write` triggered a Scorecard Token-Permissions thread. | Keep update-branch on `pull-requests: write` with `contents: read`; keep OpenCode read-only; require an explicit repo-level exception before letting the scheduler perform merge or auto-merge with `contents: write`. | @@ -122,6 +123,8 @@ The checked-in scheduler already does the minimal central path: - blocks unresolved review threads; - blocks current-head OpenCode `CHANGES_REQUESTED`; - blocks current-head failed check runs or status contexts before enabling auto-merge; +- waits on `ACTION_REQUIRED` check runs as workflow approval or repository-policy states, not as source-code failures; failed checks still take precedence for current-head-approved PRs, so `ACTION_REQUIRED` cannot mask a real failed `strix`, lint, build, or required-check result; +- rejects OpenCode reviews whose GitHub review commit matches the PR head but whose review-body `Gate evidence` names a different `Head SHA`; this prevents stale review evidence from becoming current-head approval by attachment alone; - updates `BEHIND` only when OpenCode approved the exact current head and no current-head failed check is present, using `expected_head_sha` from the scheduler workflow `GITHUB_TOKEN` so the mechanical branch update is performed by `github-actions[bot]` inside GitHub Actions instead of an OpenCode or maintainer-local credential; this path needs `pull-requests: write`, not `contents: write`; - enables native auto-merge only for current-head OpenCode approval; - dispatches same-head Strix evidence first when the current head has no completed Strix evidence; @@ -130,7 +133,7 @@ The checked-in scheduler already does the minimal central path: - dispatches OpenCode only after same-head Strix evidence is complete, including failed Strix evidence that OpenCode must explain from logs. - records mutation failures as `action_error` for the affected PR and continues scanning later PRs, so a permission failure on one merge/update action does not hide the rest of the queue. - writes the same per-PR decisions to the GitHub Actions step summary, so conflict repair and update-branch decisions are visible without opening raw logs. -- prints a machine-readable `pr-review-merge-scheduler/v1` JSON contract with every inspected PR, the scheduler action, the bounded decision value (`UPDATE_BRANCH`, `WAIT`, `REQUEST_CHANGES`, or `NO_ACTION`), and structured `guidance` for states that need action: `merge_conflict_repair` includes the base/head branches, repair steps, and merge-or-rebase commands; `github_actions_update_branch` names `github-actions[bot]`, the workflow `GITHUB_TOKEN`, `pull-requests: write`, `expected_head_sha`, and the new-head evidence required before merge. +- prints a machine-readable `pr-review-merge-scheduler/v2` JSON contract with every inspected PR, the scheduler action, the bounded decision value (`UPDATE_BRANCH`, `WAIT`, `REQUEST_CHANGES`, or `NO_ACTION`), and structured `guidance` for states that need action: `merge_conflict_repair` includes the base/head branches, repair steps, and merge-or-rebase commands; `github_actions_update_branch` names `github-actions[bot]`, the workflow `GITHUB_TOKEN`, `pull-requests: write`, `expected_head_sha`, and the new-head evidence required before merge; `workflow_action_required` names the affected check runs and requires GitHub Actions approval or policy unblock before rerunning the scheduler. - caps each GraphQL PR page at 25 nodes, so large queues can be scanned without hitting GitHub's query resource limit. - excludes OpenCode Review's own `opencode-review` check from peer failed-check evidence, so a cancelled or stale OpenCode run cannot become a source-code `REQUEST_CHANGES` review against the same head. @@ -144,23 +147,23 @@ $ python3 scripts/ci/pr_review_merge_scheduler.py --repo ContextualWisdomLab/sco PR #119: block: merge conflict: DIRTY; base=develop, head=bolt/perf-format-number-1301647661105713430; run `gh pr checkout 119`, `git fetch origin develop`, then `git merge --no-ff origin/develop` or `git rebase origin/develop`; use `git status --short` to find conflicted files, resolve conflict markers in the PR branch, rerun focused checks, and push the same bolt/perf-format-number-1301647661105713430 branch (use `git push --force-with-lease` only if rebased) ... PR #127: wait: current head is approved; auto-merge disabled by scheduler inputs -{"base_branch": "develop", "counts": {"block": 6, "wait": 1}, "decisions": [...], "dry_run": true, "inspected": 7, "project_flow": "git-flow", "schema_version": "pr-review-merge-scheduler/v1"} +{"base_branch": "develop", "counts": {"block": 6, "wait": 1}, "decisions": [...], "dry_run": true, "inspected": 7, "project_flow": "git-flow", "schema_version": "pr-review-merge-scheduler/v2"} $ python3 scripts/ci/pr_review_merge_scheduler.py --repo ContextualWisdomLab/.github --base-branch main --project-flow github-flow --dry-run --max-prs 40 --no-trigger-reviews --no-enable-auto-merge PR #44: wait: current head is approved; auto-merge already enabled ... -{"base_branch": "main", "counts": {"block": 24, "wait": 1}, "decisions": [...], "dry_run": true, "inspected": 25, "project_flow": "github-flow", "schema_version": "pr-review-merge-scheduler/v1"} +{"base_branch": "main", "counts": {"block": 24, "wait": 1}, "decisions": [...], "dry_run": true, "inspected": 25, "project_flow": "github-flow", "schema_version": "pr-review-merge-scheduler/v2"} $ python3 scripts/ci/pr_review_merge_scheduler.py --repo ContextualWisdomLab/bandscope --base-branch develop --project-flow git-flow --dry-run --max-prs 40 --no-trigger-reviews --no-enable-auto-merge PR #378: wait: OpenCode review is already in progress PR #381: wait: OpenCode review is already in progress ... -{"base_branch": "develop", "counts": {"block": 38, "wait": 2}, "decisions": [...], "dry_run": true, "inspected": 40, "project_flow": "git-flow", "schema_version": "pr-review-merge-scheduler/v1"} +{"base_branch": "develop", "counts": {"block": 38, "wait": 2}, "decisions": [...], "dry_run": true, "inspected": 40, "project_flow": "git-flow", "schema_version": "pr-review-merge-scheduler/v2"} ``` ## Rollout List -1. Keep `naruon`, `.github`, `VibeSec`, `bandscope`, `newsdom-api`, `pg-erd-cloud`, and `scopeweave` on `PR Review Merge Scheduler`. +1. Keep `naruon`, `.github`, `bandscope`, `newsdom-api`, `pg-erd-cloud`, `scopeweave`, and `appguardrail` on `PR Review Merge Scheduler`. 2. Keep `codec-carver` on the central `PR Review Merge Scheduler`; PR #98 completed the replacement of the legacy `Scheduled PR Review Merge` workflow. 3. `clearfolio` PR #13 is complete; keep the repo on direct guarded merge until auto-merge is deliberately enabled. 4. Decide whether `contextual-orchestrator` should join the central PR governance surface; no matching workflows or rulesets were returned. @@ -168,10 +171,13 @@ PR #381: wait: OpenCode review is already in progress ## Remaining Proof Gaps -- 2026-06-25 15:12 KST continuation snapshot: `.github` PR #58 is merged by `app/github-actions` at merge commit `52554bc408cd7aa82f28afb9d9dd376d8c20dfce`; `bandscope` PR #450 is still `BEHIND` with `REVIEW_REQUIRED`, failed `opencode-review`, and queued macOS jobs; `newsdom-api` PR #207 is merged and the remaining open PRs lack current-head OpenCode approval; `scopeweave` PR #127 is clean, approved, and green at head `c5c55307962a74f882e75a06ed2d8e3d4a9dc8c0`; `naruon` PR #760 is clean, approved, and green at head `57a2f8e4fcdfd0c23380c4a5c80a12901e4fe606`. Current dry-runs show no safe live `update_branch` candidate in the representative set. -- `newsdom-api` PR #207 is no longer an update-branch proof candidate because it has merged by `seonghobae`. The GitHub Actions bot update-branch proof must still use a head commit authored by `github-actions[bot]` or an Actions run log showing the `update-branch` API call. +- 2026-06-26 KST continuation snapshot: `.github` PR #68 is merged at merge commit `590b4ecb2ac9eac700019a183081309e28d8f25b`; `bandscope` PR #459 is merged at merge commit `a7173e45304d8681f02fdf43e4de5a6b6540bb44`. The live repo inventory contains 12 public non-fork repositories and confirms `appguardrail` is present while `VibeSec` is not in that set. +- `bandscope` scheduler run `28192186833` is the current live fixture. PR #450 produced conflict guidance with `gh pr checkout 450`, `git fetch origin develop`, merge-or-rebase, `git status --short`, same-branch push, and `--force-with-lease` only for rebase. PR #451 and PR #446 were updated through the workflow `GITHUB_TOKEN`; the resulting head commits were authored by `github-actions[bot]`. +- The same `bandscope` run exposed a non-source blocker after the `github-actions[bot]` branch updates: the new-head workflows for PR #451/#446 completed as `ACTION_REQUIRED` with no jobs, and the fork-run approval endpoint returned `This run is not from a fork pull request (HTTP 403)`. The scheduler must therefore report `workflow_action_required` and wait for approval or policy unblock instead of saying `failed check(s)` or posting a code finding when that state appears. +- The 2026-06-26 KST `bandscope` follow-up also exposed a stale-evidence attachment hazard: PR #387, #446, and #451 had OpenCode reviews whose GraphQL `review.commit.oid` matched the current head, while the review body `Gate evidence` named an older `Head SHA`. After the central scheduler added review-body Head SHA validation, the same dry-run classified all 79 inspected `bandscope` PRs as blocked; #387/#446/#451 now report `current head has no OpenCode approval` instead of auto-merge wait. +- `newsdom-api` PR #207 is no longer an update-branch proof candidate because it has merged by `seonghobae`. Future GitHub Actions bot update-branch proof should use a head commit authored by `github-actions[bot]` plus the scheduler run log showing the `update-branch` API call. - A live current-head review -> same-head manual Strix status bridge -> OpenCode approval -> guarded merge trace has been completed on `.github` PR #28. -- No live outdated -> update-branch -> new-head review -> merge/auto-merge trace has been completed yet. `bandscope` PR #450 is the corrective rollout after live evidence showed the stale scheduler was waiting instead of updating, but the current head is not proof-ready because OpenCode is failed and required jobs are queued. The update-branch leg now has a live partial proof: `bandscope` scheduler run `28139266598` selected PR #378 for `update_branch`, and the resulting PR head commit `68d5153ac9d5667c13b8e5e6a231c9fbb2a68f9f` was authored by `github-actions[bot]`. +- No live outdated -> update-branch -> new-head review -> merge/auto-merge trace has been completed yet. `bandscope` PR #459 is the merged downstream corrective rollout for REST mergeability; PR #450 is now a conflict-guidance fixture, not a merge/update proof candidate. The update-branch leg has multiple partial proofs: older scheduler run `28139266598` updated PR #378 with a `github-actions[bot]` head, and newer run `28192186833` updated PR #451/#446 with `github-actions[bot]` heads. - `bandscope` PR #378 still needs the new-head review/check/merge leg before the full outdated -> update-branch -> new-head review -> merge/auto-merge trace can be closed. At the 15:12 KST refresh, PR #378 is `BEHIND`, has an auto-merge request enabled by `app/github-actions`, and is waiting on in-progress OpenCode plus queued required checks. - `scopeweave` PR #127 now proves the current-head approval/check -> scheduler decision -> action-error leg: dry-run `28147098767` selected `auto_merge`, and live run `28147157319` reported `action_error` for `mergePullRequest` instead of posting a false code finding or aborting earlier PR decisions. Commit `6601953` showed why blindly adding `contents: write` is not an acceptable universal fix: Scorecard raised an unresolved Token-Permissions thread on the new head. Commit `c5c5530` restores the safer pattern: lower-privilege update-branch by GitHub Actions, with merge through Actions only where the repo deliberately accepts the contents-write exception. The current PR #127 head is merge-ready by review/check state but remains intentionally unmerged by the low-privilege scheduler policy. - `bandscope` also proved the large-queue scan risk: `max_prs=120` initially failed with `Resource limits for this query exceeded` while reading 80 open PRs. After reducing the GraphQL page size to 25, the same dry-run scanned all 80 open PRs and returned `{"block": 67, "update_branch": 1, "wait": 12}`, including PR #378 as `update_branch` and PR #404 as a conflict block with repair guidance. @@ -182,7 +188,7 @@ PR #381: wait: OpenCode review is already in progress - `naruon` PR #756 completed the repo-local rollout for the scheduler contract. Its initial head failed backend governance and Scorecard because `actions: write`/`contents: write` were broader than the repo policy allows; the amended and merged head restores minimal `GITHUB_TOKEN` permissions, keeps `trigger_reviews` and `enable_auto_merge` defaulted off, keeps `update_branches` defaulted on, and still dry-runs PR #694/#721 as `update_branch`. - `update-branch` `422/403` now has a safe fixture: unit tests simulate both permission-denied and stale `expected_head_sha` failures, assert they become `action_error`, and assert later PRs are still inspected. A real live `422/403` case is still useful as operational evidence, but it is no longer missing from the decision contract test surface. - `bandscope` PR #378 exposed a self-referential failed-check loop after manual retry run `28155083916`: the retry run succeeded and approved step execution, but the check rollup still contained the cancelled older `OpenCode Review/opencode-review` run `28152862698`, so OpenCode posted current-head `CHANGES_REQUESTED` review `4569063977` with the banned generic `No deterministic missing-string markers...` text. The collector now excludes OpenCode's own check by check name and by both actual (`OpenCode Review`) and legacy (`OpenCode PR Review`) workflow names before failed-check fallback evidence is built. -- Public repo drift is real, not hypothetical: only `.github` matched the central scheduler/workflow byte-for-byte in the 2026-06-25 scan. Some drift is policy-specific and should not be overwritten blindly, but `bandscope` had behaviorally unsafe drift and now has PR #450. +- Public repo drift is real, not hypothetical: only `.github` matched the central scheduler/workflow byte-for-byte in the 2026-06-26 scan. Some drift is policy-specific and should not be overwritten blindly, but `bandscope` had behaviorally unsafe drift and now has PR #459 merged downstream. - Required-check interpretation should stay delegated to GitHub native auto-merge until a repo needs immediate merge. - PR #28 proves the self-modifying trusted workflow bootstrap path after newer same-head evidence exists, but it does not prove update-branch behavior, stale approval dismissal after a head change, or cross-repository rollout. - PR #37 adds a bounded OpenCode approval publication timeout after manual current-head OpenCode run `28011338113` reached the approval step and was observed waiting on peer checks instead of finishing promptly. diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 7f86ef328..0d7a3b8bd 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -6,6 +6,7 @@ import argparse import json import os +import re import shlex import subprocess import sys @@ -85,6 +86,9 @@ OPEN_PRS_PAGE_SIZE = 25 DEFAULT_STALE_OPENCODE_MINUTES = 45 RUNNING_CHECK_STATES = {"PENDING", "EXPECTED", "QUEUED", "IN_PROGRESS", "WAITING", "REQUESTED"} +FAILED_CHECK_CONCLUSIONS = {"FAILURE", "ERROR", "CANCELLED", "TIMED_OUT", "STARTUP_FAILURE"} +ACTION_REQUIRED_CONCLUSIONS = {"ACTION_REQUIRED"} +REVIEW_BODY_HEAD_SHA_RE = re.compile(r"Head SHA:\s*`([0-9a-fA-F]{40})`") REST_MERGEABLE_STATE_MAP = { "behind": "BEHIND", "blocked": "BLOCKED", @@ -190,6 +194,21 @@ def decision_guidance(decision: Decision) -> dict[str, Any] | None: "# rebase path only: git push --force-with-lease", ], } + action_required = parse_workflow_action_required_reason(decision.reason) + if action_required: + return { + "type": "workflow_action_required", + "checks": action_required, + "summary": "A GitHub Actions run is waiting for workflow approval or a repository policy unblock; this is not a source-code failure by itself.", + "automation_limit": "The scheduler cannot safely reinterpret an ACTION_REQUIRED run as passed or failed, and should not publish a code-review finding from it.", + "next_required_evidence": [ + "GitHub Actions run approval or repository policy unblock", + "current-head check rerun after the unblock", + "OpenCode approval on the exact current head", + "same-head Strix evidence", + "zero active unresolved review threads", + ], + } if decision.action == "update_branch": return { "type": "github_actions_update_branch", @@ -363,7 +382,17 @@ def review_matches_current_head(review: dict[str, Any], pr: dict[str, Any]) -> b """Return whether a review is valid evidence for the current head commit.""" head = pr.get("headRefOid") commit = (review.get("commit") or {}).get("oid") - return bool(head and commit == head) + if not head or commit != head: + return False + body_head = review_body_head_sha(review) + return body_head is None or body_head.lower() == head.lower() + + +def review_body_head_sha(review: dict[str, Any]) -> str | None: + """Return the last explicit Head SHA from an OpenCode review body.""" + body = review.get("body") or "" + matches = REVIEW_BODY_HEAD_SHA_RE.findall(body) + return matches[-1] if matches else None def running_check_state(node: dict[str, Any]) -> str: @@ -471,7 +500,7 @@ def failed_status_checks(pr: dict[str, Any]) -> list[str]: for node in context_nodes(pr): if node.get("__typename") == "CheckRun": conclusion = (node.get("conclusion") or "").upper() - if conclusion in {"FAILURE", "ERROR", "CANCELLED", "TIMED_OUT", "ACTION_REQUIRED", "STARTUP_FAILURE"}: + if conclusion in FAILED_CHECK_CONCLUSIONS: if is_strix_context(node) and "strix" in successful_status_contexts: continue failed.append(node.get("name") or "check-run") @@ -482,6 +511,28 @@ def failed_status_checks(pr: dict[str, Any]) -> list[str]: return failed +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] = [] + for node in context_nodes(pr): + if node.get("__typename") != "CheckRun": + continue + conclusion = (node.get("conclusion") or "").upper() + if conclusion in ACTION_REQUIRED_CONCLUSIONS: + required.append(node.get("name") or "check-run") + return required + + +def workflow_action_required_reason(checks: list[str]) -> str: + """Return a scheduler reason for ACTION_REQUIRED check runs.""" + visible = checks[:5] + suffix = f", +{len(checks) - len(visible)} more" if len(checks) > len(visible) else "" + return ( + f"workflow action required: {', '.join(visible)}{suffix}; " + "approve or unblock the GitHub Actions run before treating checks as failed or passed" + ) + + def enable_auto_merge(repo: str, pr: dict[str, Any], *, dry_run: bool) -> None: """Enable merge-commit auto-merge for a PR at its current head.""" number = str(pr["number"]) @@ -677,6 +728,18 @@ def inspect_pr( ) return Decision(number, "block", f"failed check(s): {', '.join(failed_checks[:5])}") + workflow_action_required = action_required_checks(pr) + if workflow_action_required: + reason = workflow_action_required_reason(workflow_action_required) + if pr.get("autoMergeRequest"): + return disable_auto_merge_decision( + repo, + pr, + dry_run=dry_run, + reason=f"{reason}; wait for current-head checks to rerun before re-enabling auto-merge", + ) + return Decision(number, "wait", reason) + if merge_state == "BEHIND" and current_head_approved: if not update_branches: return Decision(number, "wait", "current-head OpenCode review approved; branch update disabled") @@ -816,6 +879,7 @@ def write_actions_summary( ) lines.extend(conflict_repair_summary(decisions)) lines.extend(update_branch_summary(decisions)) + lines.extend(workflow_action_required_summary(decisions)) lines.extend(action_error_summary(decisions)) with open(summary_path, "a", encoding="utf-8") as handle: @@ -923,6 +987,38 @@ def action_error_summary(decisions: list[Decision]) -> list[str]: return lines +def parse_workflow_action_required_reason(reason: str) -> str | None: + """Extract ACTION_REQUIRED check names from a scheduler reason.""" + marker = "workflow action required:" + marker_start = reason.find(marker) + if marker_start < 0: + return None + tail = reason[marker_start + len(marker) :].strip() + checks = tail.split(";", 1)[0].strip() + return checks or None + + +def workflow_action_required_summary(decisions: list[Decision]) -> list[str]: + """Return a GitHub Actions Summary section for ACTION_REQUIRED waits.""" + waits = [ + decision + for decision in decisions + if parse_workflow_action_required_reason(decision.reason) + ] + if not waits: + return [] + lines = [ + "", + "### Workflow action required", + "", + "`ACTION_REQUIRED` means GitHub Actions is waiting for approval or a repository policy unblock. It is not a source-code failure and should not be converted into an OpenCode finding.", + "Unblock or approve the run, then rerun the scheduler so it can read the new current-head check state.", + ] + for decision in waits: + lines.append(f"- PR #{decision.pr}: {decision.reason}") + return lines + + def bounded_error_summary(text: str, *, limit: int = 500) -> str: """Cap an action-error message without dropping the actionable prefix.""" return text if len(text) <= limit else text[: limit - 1].rstrip() + "..." diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index ed0250245..ae6bd406c 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -321,6 +321,33 @@ def test_review_state_and_failed_checks(): assert sched.current_head_review_state(pr, "APPROVED") assert sched.has_current_head_approval(pr) assert not sched.has_current_head_changes_requested(pr) + exact_head = "a" * 40 + stale_body_head = "b" * 40 + body_sha_mismatch = make_pr( + headRefOid=exact_head, + reviews={ + "nodes": [ + { + **opencode_review("APPROVED", exact_head), + "body": f"## Gate evidence\n\n- Head SHA: `{stale_body_head}`", + } + ] + }, + ) + assert sched.review_body_head_sha(body_sha_mismatch["reviews"]["nodes"][0]) == stale_body_head + assert not sched.has_current_head_approval(body_sha_mismatch) + body_sha_match = make_pr( + headRefOid=exact_head, + reviews={ + "nodes": [ + { + **opencode_review("APPROVED", exact_head), + "body": f"## Gate evidence\n\n- Head SHA: `{exact_head.upper()}`", + } + ] + }, + ) + assert sched.has_current_head_approval(body_sha_match) stale_review = make_pr( reviews={ "nodes": [ @@ -384,6 +411,21 @@ def test_review_state_and_failed_checks(): } ) assert sched.failed_status_checks(failed) == ["strix", "lint"] + action_required = make_pr( + statusCheckRollup={ + "contexts": { + "nodes": [ + {"__typename": "CheckRun", "name": "opencode-review", "conclusion": "ACTION_REQUIRED"}, + {"context": "lint", "state": "SUCCESS"}, + ] + } + } + ) + assert sched.failed_status_checks(action_required) == [] + assert sched.action_required_checks(action_required) == ["opencode-review"] + assert sched.workflow_action_required_reason(["a", "b", "c", "d", "e", "f"]).startswith( + "workflow action required: a, b, c, d, e, +1 more" + ) manual_strix_supersedes_pr_target_failure = make_pr( statusCheckRollup={ "contexts": { @@ -441,6 +483,11 @@ def test_print_summary_writes_github_step_summary(monkeypatch, tmp_path, capsys) "disable_auto_merge", "auto-merge disabled; OpenCode review does not postdate the current head commit; wait for a fresh same-head OpenCode review", ), + sched.Decision( + 10, + "wait", + "workflow action required: opencode-review; approve or unblock the GitHub Actions run before treating checks as failed or passed", + ), ] sched.print_summary(decisions, dry_run=True, base_branch="main", project_flow="github-flow") @@ -450,13 +497,14 @@ def test_print_summary_writes_github_step_summary(monkeypatch, tmp_path, capsys) payload = json.loads(output.splitlines()[-1]) assert payload["schema_version"] == "pr-review-merge-scheduler/v2" assert payload["base_branch"] == "main" - assert payload["counts"] == {"block": 1, "disable_auto_merge": 1, "update_branch": 1} + assert payload["counts"] == {"block": 1, "disable_auto_merge": 1, "update_branch": 1, "wait": 1} assert payload["dry_run"] is True - assert payload["inspected"] == 3 + assert payload["inspected"] == 4 assert payload["project_flow"] == "github-flow" assert payload["decisions"][0]["contract_decision"] == "WAIT" assert payload["decisions"][1]["contract_decision"] == "UPDATE_BRANCH" assert payload["decisions"][2]["contract_decision"] == "WAIT" + assert payload["decisions"][3]["contract_decision"] == "WAIT" assert payload["decisions"][0]["guidance"]["type"] == "merge_conflict_repair" assert payload["decisions"][0]["guidance"]["merge_state"] == "DIRTY" assert payload["decisions"][0]["guidance"]["base_ref"] == "main" @@ -470,6 +518,8 @@ def test_print_summary_writes_github_step_summary(monkeypatch, tmp_path, capsys) assert payload["decisions"][1]["guidance"]["required_permission"] == "pull-requests: write" assert payload["decisions"][1]["guidance"]["head_guard"] == "expected_head_sha" assert payload["decisions"][2]["guidance"]["type"] == "unsafe_auto_merge_disabled" + assert payload["decisions"][3]["guidance"]["type"] == "workflow_action_required" + assert payload["decisions"][3]["guidance"]["checks"] == "opencode-review" summary = summary_path.read_text(encoding="utf-8") assert "## PR review merge scheduler" in summary assert "| #7 | block | merge conflict: DIRTY; base=main, head=feature\\|x; run" in summary @@ -493,6 +543,9 @@ def test_print_summary_writes_github_step_summary(monkeypatch, tmp_path, capsys) assert "needs `pull-requests: write`" in summary assert "does not require the scheduler job to widen repository `contents` to write" in summary assert "github-actions[bot]" in summary + assert "### Workflow action required" in summary + assert "`ACTION_REQUIRED` means GitHub Actions is waiting for approval" in summary + assert "- PR #10: workflow action required: opencode-review" in summary def test_write_actions_summary_is_noop_without_summary_path(monkeypatch): @@ -511,8 +564,22 @@ def test_summary_section_helpers_handle_empty_and_action_error_cases(): wait_decisions = [sched.Decision(1, "wait", "nothing to do")] assert sched.conflict_repair_summary(wait_decisions) == [] assert sched.update_branch_summary(wait_decisions) == [] + assert sched.workflow_action_required_summary(wait_decisions) == [] assert sched.action_error_summary(wait_decisions) == [] + action_required_lines = sched.workflow_action_required_summary( + [ + sched.Decision( + 3, + "wait", + "workflow action required: opencode-review; approve or unblock the GitHub Actions run before treating checks as failed or passed", + ) + ] + ) + assert "### Workflow action required" in action_required_lines + assert "not a source-code failure" in "\n".join(action_required_lines) + assert "- PR #3: workflow action required: opencode-review" in "\n".join(action_required_lines) + lines = sched.action_error_summary([sched.Decision(2, "action_error", "permission failed")]) assert "### Action errors" in lines assert "not source-code review findings" in "\n".join(lines) @@ -581,6 +648,32 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): assert inspect(make_pr(reviews={"nodes": [opencode_review("CHANGES_REQUESTED", "head")]})).reason == ( "current-head OpenCode review requested changes" ) + action_required_pr = make_pr( + statusCheckRollup={ + "contexts": { + "nodes": [{"__typename": "CheckRun", "name": "opencode-review", "conclusion": "ACTION_REQUIRED"}] + } + } + ) + action_required_decision = inspect(action_required_pr) + assert action_required_decision.action == "wait" + assert action_required_decision.reason == ( + "workflow action required: opencode-review; approve or unblock the GitHub Actions run before treating checks as failed or passed" + ) + action_required_auto = inspect( + make_pr( + autoMergeRequest={"enabledAt": "now"}, + statusCheckRollup={ + "contexts": { + "nodes": [ + {"__typename": "CheckRun", "name": "opencode-review", "conclusion": "ACTION_REQUIRED"} + ] + } + }, + ) + ) + assert action_required_auto.action == "disable_auto_merge" + assert "workflow action required: opencode-review" in action_required_auto.reason same_head_auto = make_pr( autoMergeRequest={"enabledAt": "now"}, reviews={"nodes": [opencode_review("APPROVED", "head", submitted_at="2026-06-25T06:59:59Z")]}, @@ -618,6 +711,34 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): assert failed_decision.action == "block" assert failed_decision.reason == "failed check(s): strix" assert called == [] + mixed_failure_and_action_required = make_pr( + reviews={"nodes": [opencode_review("APPROVED", "head")]}, + statusCheckRollup={ + "contexts": { + "nodes": [ + {"__typename": "CheckRun", "name": "strix", "conclusion": "FAILURE"}, + {"__typename": "CheckRun", "name": "opencode-review", "conclusion": "ACTION_REQUIRED"}, + ] + } + }, + ) + mixed_decision = inspect(mixed_failure_and_action_required) + assert mixed_decision.action == "block" + assert mixed_decision.reason == "failed check(s): strix" + called.clear() + behind_action_required = make_pr( + mergeStateStatus="BEHIND", + reviews={"nodes": [opencode_review("APPROVED", "head")]}, + statusCheckRollup={ + "contexts": { + "nodes": [{"__typename": "CheckRun", "name": "opencode-review", "conclusion": "ACTION_REQUIRED"}] + } + }, + ) + action_required_decision = inspect(behind_action_required) + assert action_required_decision.action == "wait" + assert "workflow action required: opencode-review" in action_required_decision.reason + assert called == [] behind_auto_merge_enabled = make_pr( mergeStateStatus="BEHIND", reviews={"nodes": [opencode_review("APPROVED", "head")]}, From 39e75795b808481e19654d1b05d7614965edbfc2 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 26 Jun 2026 04:53:46 +0900 Subject: [PATCH 2/4] Tighten PR branch update governance --- PR_GOVERNANCE_AUDIT.md | 4 ++-- scripts/ci/pr_review_merge_scheduler.py | 18 +++++++++++++++++- tests/test_pr_review_merge_scheduler.py | 21 +++++++++++++++++++++ 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/PR_GOVERNANCE_AUDIT.md b/PR_GOVERNANCE_AUDIT.md index f7c807aa2..aff2ffea6 100644 --- a/PR_GOVERNANCE_AUDIT.md +++ b/PR_GOVERNANCE_AUDIT.md @@ -119,13 +119,13 @@ both separately; a change can improve one while harming the other. The checked-in scheduler already does the minimal central path: - skips draft, wrong-base, and fork/external-head PRs; -- blocks `DIRTY` or `CONFLICTING` with repair guidance that names the base branch, head branch, merge/rebase direction, conflict-marker cleanup, focused checks, same-branch push, and a compact `gh pr checkout` / `git fetch` / merge-or-rebase / `git status --short` command path; it explicitly does not retry `update-branch` for conflicted PRs because GitHub cannot choose the correct conflict resolution; +- blocks UI `Conflicting`, API `DIRTY`, or API `CONFLICTING` with repair guidance that names the base branch, head branch, merge/rebase direction, conflict-marker cleanup, focused checks, same-branch push, and a compact `gh pr checkout` / `git fetch` / merge-or-rebase / `git status --short` command path; it explicitly does not retry `update-branch` for conflicted PRs because GitHub cannot choose the correct conflict resolution; - blocks unresolved review threads; - blocks current-head OpenCode `CHANGES_REQUESTED`; - blocks current-head failed check runs or status contexts before enabling auto-merge; - waits on `ACTION_REQUIRED` check runs as workflow approval or repository-policy states, not as source-code failures; failed checks still take precedence for current-head-approved PRs, so `ACTION_REQUIRED` cannot mask a real failed `strix`, lint, build, or required-check result; - rejects OpenCode reviews whose GitHub review commit matches the PR head but whose review-body `Gate evidence` names a different `Head SHA`; this prevents stale review evidence from becoming current-head approval by attachment alone; -- updates `BEHIND` only when OpenCode approved the exact current head and no current-head failed check is present, using `expected_head_sha` from the scheduler workflow `GITHUB_TOKEN` so the mechanical branch update is performed by `github-actions[bot]` inside GitHub Actions instead of an OpenCode or maintainer-local credential; this path needs `pull-requests: write`, not `contents: write`; +- updates `BEHIND` only when OpenCode approved the exact current head and no current-head failed check is present, using `expected_head_sha` from the scheduler workflow `GITHUB_TOKEN` so the mechanical branch update is performed by `github-actions[bot]` inside GitHub Actions instead of an OpenCode or maintainer-local credential; the script now refuses non-dry-run `update-branch` outside GitHub Actions, and this path needs `pull-requests: write`, not `contents: write`; - enables native auto-merge only for current-head OpenCode approval; - dispatches same-head Strix evidence first when the current head has no completed Strix evidence; - waits while same-head Strix evidence is still running, so OpenCode is not started just to poll a peer check; diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 0d7a3b8bd..8bb179ac4 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -568,6 +568,7 @@ def update_branch(repo: str, pr: dict[str, Any], *, dry_run: bool) -> None: head = pr["headRefOid"] if dry_run: return + require_github_actions_mutation_actor("update-branch") run( [ "gh", @@ -581,6 +582,20 @@ def update_branch(repo: str, pr: dict[str, Any], *, dry_run: bool) -> None: ) +def require_github_actions_mutation_actor(action: str) -> None: + """Refuse mutating PR branches from a maintainer-local gh credential.""" + if os.environ.get("GITHUB_ACTIONS") != "true": + raise RuntimeError( + f"{action} refused outside GitHub Actions; dispatch PR Review Merge Scheduler " + "so the workflow GITHUB_TOKEN performs the mutation as github-actions[bot]" + ) + if not os.environ.get("GH_TOKEN"): + raise RuntimeError( + f"{action} refused without GH_TOKEN; configure the scheduler job to pass " + "secrets.GITHUB_TOKEN through GH_TOKEN so the mutation is attributable to github-actions[bot]" + ) + + def dispatch_opencode_review(repo: str, workflow: str, pr: dict[str, Any], *, dry_run: bool) -> None: """Dispatch the OpenCode Review workflow for the PR head.""" if dry_run: @@ -923,7 +938,7 @@ def conflict_repair_summary(decisions: list[Decision]) -> list[str]: "", "### Conflict repair", "", - "GitHub cannot safely update `DIRTY` or `CONFLICTING` PR branches. Repair the PR branch, then push the same branch so OpenCode and required checks can run on the new head.", + "When GitHub shows `Conflicting`, or the API reports `DIRTY`/`CONFLICTING`, this is not a code-review finding and it is not an `update-branch` candidate. Repair the PR branch, then push the same branch so OpenCode and required checks can run on the new head.", "`update-branch` is not a conflict resolver: the scheduler waits here because GitHub cannot choose which side of a conflicted hunk is correct.", ] for decision, parsed in conflicted: @@ -965,6 +980,7 @@ def update_branch_summary(decisions: list[Decision]) -> list[str]: "", f"Requested `update-branch` for PR {pr_list} with the workflow `GITHUB_TOKEN`, guarded by the observed `expected_head_sha`.", "This is intentionally done inside GitHub Actions, not from a maintainer's local `gh` credential, so the mechanical update is attributable to the automation actor.", + "The scheduler refuses a non-dry-run `update-branch` outside GitHub Actions; dispatch the workflow instead of running the mutation locally.", "This branch-update API path needs `pull-requests: write`; it does not require the scheduler job to widen repository `contents` to write.", "When repository permissions allow the mutation, GitHub records the resulting branch update as `github-actions[bot]`.", "The updated head is not merge evidence by itself. Wait for the new head to receive OpenCode approval, Strix evidence, required checks, and unresolved-thread checks before merge or auto-merge.", diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index ae6bd406c..cafb4541a 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -451,6 +451,8 @@ def test_actions_call_gh_with_expected_arguments(monkeypatch): sched.dispatch_opencode_review("owner/repo", "OpenCode Review", pr, dry_run=True) assert calls == [] + monkeypatch.setenv("GITHUB_ACTIONS", "true") + monkeypatch.setenv("GH_TOKEN", "workflow-token") sched.enable_auto_merge("owner/repo", pr, dry_run=False) sched.disable_auto_merge("owner/repo", pr, dry_run=False) sched.update_branch("owner/repo", pr, dry_run=False) @@ -464,6 +466,23 @@ def test_actions_call_gh_with_expected_arguments(monkeypatch): assert calls[4][:5] == ["gh", "workflow", "run", "OpenCode Review", "--repo"] +def test_update_branch_refuses_local_credentials(monkeypatch): + calls = [] + monkeypatch.setattr(sched, "run", lambda args: calls.append(args) or "") + monkeypatch.delenv("GITHUB_ACTIONS", raising=False) + monkeypatch.setenv("GH_TOKEN", "local-token") + + with pytest.raises(RuntimeError, match="refused outside GitHub Actions"): + sched.update_branch("owner/repo", make_pr(), dry_run=False) + assert calls == [] + + monkeypatch.setenv("GITHUB_ACTIONS", "true") + monkeypatch.delenv("GH_TOKEN", raising=False) + with pytest.raises(RuntimeError, match="refused without GH_TOKEN"): + sched.update_branch("owner/repo", make_pr(), dry_run=False) + assert calls == [] + + def test_print_summary_writes_github_step_summary(monkeypatch, tmp_path, capsys): summary_path = tmp_path / "summary.md" monkeypatch.setenv("GITHUB_STEP_SUMMARY", str(summary_path)) @@ -530,6 +549,7 @@ def test_print_summary_writes_github_step_summary(monkeypatch, tmp_path, capsys) ) in summary assert "fresh same-head OpenCode review" in summary assert "### Conflict repair" in summary + assert "When GitHub shows `Conflicting`" in summary assert "`update-branch` is not a conflict resolver" in summary assert "PR #7 is `DIRTY` against `main` from `feature\\|x`:" not in summary assert "PR #7 is `DIRTY` against `main` from `feature|x`:" in summary @@ -540,6 +560,7 @@ def test_print_summary_writes_github_step_summary(monkeypatch, tmp_path, capsys) assert "### Branch update requests" in summary assert "Requested `update-branch` for PR #8 with the workflow `GITHUB_TOKEN`" in summary assert "not from a maintainer's local `gh` credential" in summary + assert "refuses a non-dry-run `update-branch` outside GitHub Actions" in summary assert "needs `pull-requests: write`" in summary assert "does not require the scheduler job to widen repository `contents` to write" in summary assert "github-actions[bot]" in summary From e2964ebf9ec57defd1540c66ba83e120c892fc22 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 26 Jun 2026 05:38:56 +0900 Subject: [PATCH 3/4] Handle Strix token-limit fallback --- scripts/ci/strix_quick_gate.sh | 17 ++++++ scripts/ci/test_strix_quick_gate.sh | 89 ++++++++++++++++++++++++++++- 2 files changed, 105 insertions(+), 1 deletion(-) diff --git a/scripts/ci/strix_quick_gate.sh b/scripts/ci/strix_quick_gate.sh index fd90c1b1b..646d93787 100755 --- a/scripts/ci/strix_quick_gate.sh +++ b/scripts/ci/strix_quick_gate.sh @@ -2594,6 +2594,15 @@ is_midstream_fallback_error() { # originated from an LLM provider rather than the target application. LLM_PROVIDER_ONLY_REGEX='(litellm|openai|anthropic|VertexAI|Vertex_ai|vertex\.ai|google\.cloud|GitHub Models|models\.github\.ai|github_models)' +is_llm_token_limit_error() { + if grep -Eiq '(tokens_limit_reached|Request body too large|Max size:[[:space:]]*[0-9]+[[:space:]]+tokens|Error code:[[:space:]]*413|(^|[^0-9])413([^0-9]|$))' "$STRIX_LOG" && + grep -Eiq "($LLM_PROVIDER_ONLY_REGEX|OpenAIException|openai\.APIStatusError)" "$STRIX_LOG"; then + return 0 + fi + + return 1 +} + # Detect whether the strix log contains evidence of infrastructure-level # errors (timeout, rate-limit, transport failures) that indicate the scan # was interrupted or incomplete. Used as a guard to prevent the @@ -2611,6 +2620,10 @@ has_detected_infrastructure_error() { return 0 fi + if is_llm_token_limit_error; then + return 0 + fi + if is_midstream_fallback_error; then return 0 fi @@ -3292,6 +3305,10 @@ is_model_retryable_error() { return 0 fi + if is_llm_token_limit_error; then + return 0 + fi + if is_timeout_error; then if provider_signal_fail_closed_enabled; then return 1 diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index bc9e1de12..4ac8e42ea 100755 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -2040,6 +2040,10 @@ run_gate_case() { local generic_fallback_models="${28-}" local fail_on_provider_signal="${29-1}" + if [ -n "${STRIX_TEST_CASE_FILTER:-}" ] && [ "$scenario" != "$STRIX_TEST_CASE_FILTER" ]; then + return + fi + local tmp_dir tmp_dir="$(mktemp -d)" # Separate bin/ (fake strix + helper files) from workspace/ (target path) @@ -2127,7 +2131,7 @@ case "${FAKE_STRIX_SCENARIO:?}" in echo "scan ok with timeout disabled" exit 0 ;; - vertex-primary-notfound-fallback-success|github-models-fallback-success|github-models-fallback-success-deepseek-v3|github-models-fallback-requires-api-base|github-models-model-prefix-with-api-base-succeeds|github-models-meta-prefix-with-api-base-succeeds|github-models-mistral-prefix-with-api-base-succeeds) + vertex-primary-notfound-fallback-success|github-models-fallback-success|github-models-fallback-success-deepseek-v3|github-models-token-limit-fallback-success|github-models-fallback-requires-api-base|github-models-model-prefix-with-api-base-succeeds|github-models-meta-prefix-with-api-base-succeeds|github-models-mistral-prefix-with-api-base-succeeds) case "${STRIX_LLM:-}" in vertex_ai/missing-primary) echo "Error: litellm.NotFoundError: Vertex_aiException - x" @@ -2139,6 +2143,10 @@ case "${FAKE_STRIX_SCENARIO:?}" in exit 0 ;; openai/gpt-5|openai/openai/gpt-5.4|openai/meta/test-github-model|openai/mistral-ai/test-github-model) + if [ "${FAKE_STRIX_SCENARIO:?}" = "github-models-token-limit-fallback-success" ]; then + echo "openai.APIStatusError: Error code: 413 - {'error': {'code': 'tokens_limit_reached', 'message': 'Request body too large for gpt-5 model. Max size: 4000 tokens.'}}" + exit 1 + fi echo "scan ok with GitHub Models fallback" exit 0 ;; @@ -4265,6 +4273,56 @@ run_gate_case_allow_provider_signal() { run_gate_case_with_provider_signal_mode "0" "$@" } +run_filtered_gate_case_if_requested() { + case "${STRIX_TEST_CASE_FILTER:-}" in + "") + return 0 + ;; + github-models-token-limit-fallback-success) + run_gate_case "github-models-token-limit-fallback-success" \ + "openai/gpt-5" \ + "" \ + "0" \ + "REGEX:Strix quick scan succeeded with fallback model 'github_models/deepseek/deepseek-v3-0324' in [0-9]+s\\." \ + "2" \ + "openai/gpt-5|openai/deepseek/deepseek-v3-0324" \ + "https://models.github.ai/inference|https://models.github.ai/inference" \ + "openai" \ + "https://models.github.ai/inference" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "github_models/deepseek/deepseek-v3-0324 github_models/deepseek/deepseek-r1-0528" + ;; + *) + record_failure "unknown STRIX_TEST_CASE_FILTER '${STRIX_TEST_CASE_FILTER:-}'" + ;; + esac + + if [ "$FAILURES" -ne 0 ]; then + echo "$FAILURES failure(s)" >&2 + exit 1 + fi + + exit 0 +} + +run_filtered_gate_case_if_requested + run_pull_request_target_head_scope_case() { local case_name="$1" local changed_file="$2" @@ -9404,6 +9462,35 @@ run_gate_case "github-models-fallback-success" \ "" \ 0 +run_gate_case "github-models-token-limit-fallback-success" \ + "openai/gpt-5" \ + "" \ + "0" \ + "REGEX:Strix quick scan succeeded with fallback model 'github_models/deepseek/deepseek-v3-0324' in [0-9]+s\\." \ + "2" \ + "openai/gpt-5|openai/deepseek/deepseek-v3-0324" \ + "https://models.github.ai/inference|https://models.github.ai/inference" \ + "openai" \ + "https://models.github.ai/inference" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "github_models/deepseek/deepseek-v3-0324 github_models/deepseek/deepseek-r1-0528" + run_gate_case "github-models-fallback-success-deepseek-v3" \ "vertex_ai/missing-primary" \ "github_models/deepseek/deepseek-r1-0528 github_models/deepseek/deepseek-v3-0324" \ From 7b4d47d0e1fdbefd92436c7358cdb445b8653d50 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 26 Jun 2026 06:07:11 +0900 Subject: [PATCH 4/4] Resolve outdated PR review threads --- PR_GOVERNANCE_AUDIT.md | 7 +- scripts/ci/pr_review_merge_scheduler.py | 215 +++++++++++++++++------- tests/test_pr_review_merge_scheduler.py | 76 ++++++++- 3 files changed, 235 insertions(+), 63 deletions(-) diff --git a/PR_GOVERNANCE_AUDIT.md b/PR_GOVERNANCE_AUDIT.md index aff2ffea6..f29937766 100644 --- a/PR_GOVERNANCE_AUDIT.md +++ b/PR_GOVERNANCE_AUDIT.md @@ -17,7 +17,7 @@ OpenCode decides; GitHub Actions mutates. 2026-06-25 check). Do not widen `contents` just to support `update-branch`. - Old approvals and old checks are not merge evidence after a head SHA changes. - OpenCode review evidence must be internally same-head as well as GitHub-attached same-head. If the review body includes `Gate evidence` with `Head SHA: `, that SHA must match the PR current `headRefOid`; otherwise the review is stale evidence even when GitHub attaches the review to the current commit. -- Merge uses one path: current-head OpenCode approval, no unresolved review threads, required checks green or native auto-merge waiting on them, mergeable head, and no policy blocker. +- Merge uses one path: current-head OpenCode approval, no active unresolved review threads, required checks green or native auto-merge waiting on them, mergeable head, and no policy blocker. GitHub `Outdated` review threads are obsolete diff conversations; the scheduler resolves them before counting active unresolved review blockers. - Prefer `gh pr merge --auto --merge --match-head-commit ` when native auto-merge is enabled. - Use direct `gh pr merge --merge --match-head-commit ` only when the repo policy already allows immediate merge. - OpenCode app-token merges are deprecated; keep app tokens for review publication, not mechanical branch mutation. @@ -120,7 +120,8 @@ The checked-in scheduler already does the minimal central path: - skips draft, wrong-base, and fork/external-head PRs; - blocks UI `Conflicting`, API `DIRTY`, or API `CONFLICTING` with repair guidance that names the base branch, head branch, merge/rebase direction, conflict-marker cleanup, focused checks, same-branch push, and a compact `gh pr checkout` / `git fetch` / merge-or-rebase / `git status --short` command path; it explicitly does not retry `update-branch` for conflicted PRs because GitHub cannot choose the correct conflict resolution; -- blocks unresolved review threads; +- resolves GitHub `Outdated` unresolved review threads through `resolveReviewThread` before active blocker checks, using the scheduler workflow `GITHUB_TOKEN` inside GitHub Actions; dry-runs report the cleanup as `notes` without mutating the PR; +- blocks active, non-outdated unresolved review threads; - blocks current-head OpenCode `CHANGES_REQUESTED`; - blocks current-head failed check runs or status contexts before enabling auto-merge; - waits on `ACTION_REQUIRED` check runs as workflow approval or repository-policy states, not as source-code failures; failed checks still take precedence for current-head-approved PRs, so `ACTION_REQUIRED` cannot mask a real failed `strix`, lint, build, or required-check result; @@ -133,7 +134,7 @@ The checked-in scheduler already does the minimal central path: - dispatches OpenCode only after same-head Strix evidence is complete, including failed Strix evidence that OpenCode must explain from logs. - records mutation failures as `action_error` for the affected PR and continues scanning later PRs, so a permission failure on one merge/update action does not hide the rest of the queue. - writes the same per-PR decisions to the GitHub Actions step summary, so conflict repair and update-branch decisions are visible without opening raw logs. -- prints a machine-readable `pr-review-merge-scheduler/v2` JSON contract with every inspected PR, the scheduler action, the bounded decision value (`UPDATE_BRANCH`, `WAIT`, `REQUEST_CHANGES`, or `NO_ACTION`), and structured `guidance` for states that need action: `merge_conflict_repair` includes the base/head branches, repair steps, and merge-or-rebase commands; `github_actions_update_branch` names `github-actions[bot]`, the workflow `GITHUB_TOKEN`, `pull-requests: write`, `expected_head_sha`, and the new-head evidence required before merge; `workflow_action_required` names the affected check runs and requires GitHub Actions approval or policy unblock before rerunning the scheduler. +- prints a machine-readable `pr-review-merge-scheduler/v2` JSON contract with every inspected PR, the scheduler action, the bounded decision value (`UPDATE_BRANCH`, `WAIT`, `REQUEST_CHANGES`, or `NO_ACTION`), optional cleanup `notes`, and structured `guidance` for states that need action: `merge_conflict_repair` includes the base/head branches, repair steps, and merge-or-rebase commands; `github_actions_update_branch` names `github-actions[bot]`, the workflow `GITHUB_TOKEN`, `pull-requests: write`, `expected_head_sha`, and the new-head evidence required before merge; `workflow_action_required` names the affected check runs and requires GitHub Actions approval or policy unblock before rerunning the scheduler. - caps each GraphQL PR page at 25 nodes, so large queues can be scanned without hitting GitHub's query resource limit. - excludes OpenCode Review's own `opencode-review` check from peer failed-check evidence, so a cancelled or stale OpenCode run cannot become a source-code `REQUEST_CHANGES` review against the same head. diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 8bb179ac4..17810afad 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -44,7 +44,7 @@ } } reviewThreads(first: 100) { - nodes { isResolved isOutdated } + nodes { id isResolved isOutdated } } reviews(last: 50) { nodes { @@ -109,6 +109,16 @@ class Decision: pr: int action: str reason: str + notes: tuple[str, ...] = () + + +RESOLVE_REVIEW_THREAD_MUTATION = """\ +mutation($threadId: ID!) { + resolveReviewThread(input: {threadId: $threadId}) { + thread { id isResolved } + } +} +""" def contract_decision(decision: Decision) -> str: @@ -155,6 +165,8 @@ def decision_contract_entry(decision: Decision) -> dict[str, Any]: guidance = decision_guidance(decision) if guidance: entry["guidance"] = guidance + if decision.notes: + entry["notes"] = list(decision.notes) return entry @@ -457,6 +469,46 @@ def unresolved_thread_count(pr: dict[str, Any]) -> int: return sum(1 for thread in threads if not thread.get("isResolved") and not thread.get("isOutdated")) +def outdated_thread_ids(pr: dict[str, Any]) -> list[str]: + """Return unresolved review-thread IDs GitHub already marks outdated.""" + threads = ((pr.get("reviewThreads") or {}).get("nodes") or []) + return [ + thread["id"] + for thread in threads + if thread.get("id") and not thread.get("isResolved") and thread.get("isOutdated") + ] + + +def resolve_review_thread(thread_id: str) -> None: + """Resolve one GitHub review thread by GraphQL node ID.""" + gh_graphql(RESOLVE_REVIEW_THREAD_MUTATION, threadId=thread_id) + + +def resolve_outdated_review_threads(pr: dict[str, Any], *, dry_run: bool) -> int: + """Resolve obsolete diff conversations before active-thread merge checks.""" + thread_ids = outdated_thread_ids(pr) + if not thread_ids: + return 0 + if dry_run: + return len(thread_ids) + require_github_actions_mutation_actor("resolve-outdated-review-thread") + for thread_id in thread_ids: + resolve_review_thread(thread_id) + return len(thread_ids) + + +def with_outdated_thread_cleanup_note(decision: Decision, count: int, *, dry_run: bool) -> Decision: + """Annotate a decision with the outdated-thread cleanup side effect.""" + if count <= 0: + return decision + verb = "Would resolve" if dry_run else "Resolved" + note = ( + f"{verb} {count} outdated review thread(s) before active unresolved-thread checks; " + "outdated diff comments are not current-head review blockers." + ) + return Decision(decision.pr, decision.action, decision.reason, (*decision.notes, note)) + + def review_author_login(review: dict[str, Any]) -> str: """Return a normalized review author login.""" return ((review.get("author") or {}).get("login") or "").lower() @@ -688,108 +740,131 @@ def inspect_pr( if head_repo != repo: return Decision(number, "skip", f"fork or external head repo: {head_repo}") + outdated_cleanup_count = resolve_outdated_review_threads(pr, dry_run=dry_run) + + def finish(decision: Decision) -> Decision: + """Attach outdated-thread cleanup evidence to the final decision.""" + return with_outdated_thread_cleanup_note( + decision, + outdated_cleanup_count, + dry_run=dry_run, + ) + + def decide(action: str, reason: str) -> Decision: + """Create a decision after applying shared cleanup notes.""" + return finish(Decision(number, action, reason)) + merge_state = effective_merge_state(pr) if merge_state == "UNKNOWN": if pr.get("autoMergeRequest"): - return disable_auto_merge_decision( - repo, - pr, - dry_run=dry_run, - reason="mergeability is still being calculated; wait for GitHub mergeability evidence before re-enabling auto-merge", + return finish( + disable_auto_merge_decision( + repo, + pr, + dry_run=dry_run, + reason="mergeability is still being calculated; wait for GitHub mergeability evidence before re-enabling auto-merge", + ) ) - return Decision(number, "wait", "mergeability is still being calculated") + return decide("wait", "mergeability is still being calculated") if merge_state in {"DIRTY", "CONFLICTING"}: if pr.get("autoMergeRequest"): - return disable_auto_merge_decision( - repo, - pr, - dry_run=dry_run, - reason=f"{merge_conflict_guidance(pr, merge_state)}; repair the conflict before re-enabling auto-merge", + return finish( + disable_auto_merge_decision( + repo, + pr, + dry_run=dry_run, + reason=f"{merge_conflict_guidance(pr, merge_state)}; repair the conflict before re-enabling auto-merge", + ) ) - return Decision(number, "block", merge_conflict_guidance(pr, merge_state)) + return decide("block", merge_conflict_guidance(pr, merge_state)) unresolved = unresolved_thread_count(pr) if unresolved: if pr.get("autoMergeRequest"): - return disable_auto_merge_decision( - repo, - pr, - dry_run=dry_run, - reason=f"{unresolved} unresolved review thread(s); resolve the active thread(s) before re-enabling auto-merge", + return finish( + disable_auto_merge_decision( + repo, + pr, + dry_run=dry_run, + reason=f"{unresolved} unresolved review thread(s); resolve the active thread(s) before re-enabling auto-merge", + ) ) - return Decision(number, "block", f"{unresolved} unresolved review thread(s)") + return decide("block", f"{unresolved} unresolved review thread(s)") if has_current_head_changes_requested(pr): if pr.get("autoMergeRequest"): - return disable_auto_merge_decision( - repo, - pr, - dry_run=dry_run, - reason="current-head OpenCode review requested changes; address the review before re-enabling auto-merge", + return finish( + disable_auto_merge_decision( + repo, + pr, + dry_run=dry_run, + reason="current-head OpenCode review requested changes; address the review before re-enabling auto-merge", + ) ) - return Decision(number, "block", "current-head OpenCode review requested changes") + return decide("block", "current-head OpenCode review requested changes") current_head_approved = has_current_head_approval(pr) if current_head_approved: failed_checks = failed_status_checks(pr) if failed_checks: if pr.get("autoMergeRequest"): - return disable_auto_merge_decision( - repo, - pr, - dry_run=dry_run, - reason=f"failed check(s): {', '.join(failed_checks[:5])}; fix or rerun checks before re-enabling auto-merge", + return finish( + disable_auto_merge_decision( + repo, + pr, + dry_run=dry_run, + reason=f"failed check(s): {', '.join(failed_checks[:5])}; fix or rerun checks before re-enabling auto-merge", + ) ) - return Decision(number, "block", f"failed check(s): {', '.join(failed_checks[:5])}") + return decide("block", f"failed check(s): {', '.join(failed_checks[:5])}") workflow_action_required = action_required_checks(pr) if workflow_action_required: reason = workflow_action_required_reason(workflow_action_required) if pr.get("autoMergeRequest"): - return disable_auto_merge_decision( - repo, - pr, - dry_run=dry_run, - reason=f"{reason}; wait for current-head checks to rerun before re-enabling auto-merge", + return finish( + disable_auto_merge_decision( + repo, + pr, + dry_run=dry_run, + reason=f"{reason}; wait for current-head checks to rerun before re-enabling auto-merge", + ) ) - return Decision(number, "wait", reason) + return decide("wait", reason) if merge_state == "BEHIND" and current_head_approved: if not update_branches: - return Decision(number, "wait", "current-head OpenCode review approved; branch update disabled") + return decide("wait", "current-head OpenCode review approved; branch update disabled") had_auto_merge = bool(pr.get("autoMergeRequest")) if had_auto_merge: disable_auto_merge(repo, pr, dry_run=dry_run) update_branch(repo, pr, dry_run=dry_run) prefix = "auto-merge disabled before branch update; " if had_auto_merge else "" - return Decision( - number, + return decide( "update_branch", f"{prefix}current-head OpenCode review approved; branch update requested with workflow GH_TOKEN (github-actions[bot] in GitHub Actions)", ) if current_head_approved: if pr.get("autoMergeRequest"): - return Decision(number, "wait", "current head is approved; auto-merge already enabled") + return decide("wait", "current head is approved; auto-merge already enabled") if not enable_auto_merge_flag: - return Decision(number, "wait", "current head is approved; auto-merge disabled by scheduler inputs") + return decide("wait", "current head is approved; auto-merge disabled by scheduler inputs") enable_auto_merge(repo, pr, dry_run=dry_run) - return Decision(number, "auto_merge", "current head is approved; auto-merge enabled") + return decide("auto_merge", "current head is approved; auto-merge enabled") opencode_state = opencode_progress_state(pr, stale_after_minutes=stale_opencode_minutes) if opencode_state == "running": - return Decision(number, "wait", "OpenCode review is already in progress") + return decide("wait", "OpenCode review is already in progress") if opencode_state == "stale" and not trigger_reviews: - return Decision( - number, + return decide( "wait", f"OpenCode review exceeded {stale_opencode_minutes} minute retry threshold; review dispatch disabled", ) if opencode_state == "stale": dispatch_opencode_review(repo, workflow, pr, dry_run=dry_run) - return Decision( - number, + return decide( "review_dispatch", f"OpenCode review exceeded {stale_opencode_minutes} minute retry threshold; same-head OpenCode re-dispatched", ) @@ -798,31 +873,31 @@ def inspect_pr( strix_state = strix_evidence_state(pr) if strix_state == "missing": dispatch_strix_evidence(repo, security_workflow, pr, dry_run=dry_run) - return Decision( - number, + return decide( "security_dispatch", "current head has no completed Strix evidence; same-head Strix dispatched", ) if strix_state == "running": - return Decision(number, "wait", "same-head Strix evidence is still running") + return decide("wait", "same-head Strix evidence is still running") # Legacy trusted-base Strix self-test sentinel while this scheduler rollout lands: # same-head Strix and OpenCode dispatched dispatch_opencode_review(repo, workflow, pr, dry_run=dry_run) - return Decision( - number, + return decide( "review_dispatch", "current head has completed Strix evidence; same-head OpenCode dispatched", ) if pr.get("autoMergeRequest"): - return disable_auto_merge_decision( - repo, - pr, - dry_run=dry_run, - reason="current head has no OpenCode approval; wait for fresh same-head approval before re-enabling auto-merge", + return finish( + disable_auto_merge_decision( + repo, + pr, + dry_run=dry_run, + reason="current head has no OpenCode approval; wait for fresh same-head approval before re-enabling auto-merge", + ) ) - return Decision(number, "block", "current head has no OpenCode approval") + return decide("block", "current head has no OpenCode approval") def print_summary( @@ -893,6 +968,7 @@ def write_actions_summary( for decision in decisions ) lines.extend(conflict_repair_summary(decisions)) + lines.extend(outdated_thread_cleanup_summary(decisions)) lines.extend(update_branch_summary(decisions)) lines.extend(workflow_action_required_summary(decisions)) lines.extend(action_error_summary(decisions)) @@ -968,6 +1044,27 @@ def conflict_repair_summary(decisions: list[Decision]) -> list[str]: return lines +def outdated_thread_cleanup_summary(decisions: list[Decision]) -> list[str]: + """Return a summary section for obsolete diff conversations resolved by the scheduler.""" + cleanup_notes = [ + (decision, note) + for decision in decisions + for note in decision.notes + if "outdated review thread" in note + ] + if not cleanup_notes: + return [] + + lines = [ + "", + "### Outdated review threads", + "", + "GitHub `Outdated` review threads belong to obsolete diff hunks. The scheduler resolves them before counting active unresolved review threads, so stale UI conversations do not block current-head decisions.", + ] + lines.extend(f"- PR #{decision.pr}: {note}" for decision, note in cleanup_notes) + return lines + + def update_branch_summary(decisions: list[Decision]) -> list[str]: """Return a GitHub Actions Summary section explaining branch update mutations.""" updates = [decision for decision in decisions if decision.action == "update_branch"] diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index cafb4541a..a97086ed3 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -307,8 +307,17 @@ def test_context_review_and_check_helpers(): == "complete" ) - threaded = make_pr(reviewThreads={"nodes": [{"isResolved": False}, {"isResolved": True}, {"isOutdated": True}]}) + threaded = make_pr( + reviewThreads={ + "nodes": [ + {"id": "active", "isResolved": False}, + {"id": "resolved", "isResolved": True}, + {"id": "outdated", "isResolved": False, "isOutdated": True}, + ] + } + ) assert sched.unresolved_thread_count(threaded) == 1 + assert sched.outdated_thread_ids(threaded) == ["outdated"] assert sched.review_author_login({}) == "" assert sched.review_author_login({"author": {"login": "OpenCode-Agent"}}) == "opencode-agent" assert sched.is_opencode_review(opencode_review()) @@ -483,6 +492,40 @@ def test_update_branch_refuses_local_credentials(monkeypatch): assert calls == [] +def test_resolve_outdated_review_threads_uses_github_actions_actor(monkeypatch): + calls = [] + pr = make_pr( + reviewThreads={ + "nodes": [ + {"id": "thread-1", "isResolved": False, "isOutdated": True}, + {"id": "thread-2", "isResolved": True, "isOutdated": True}, + {"id": "thread-3", "isResolved": False, "isOutdated": False}, + {"id": "thread-4", "isResolved": False, "isOutdated": True}, + ] + } + ) + + def fake_graphql(query, **fields): + calls.append((query, fields)) + return {"data": {"resolveReviewThread": {"thread": {"id": fields["threadId"], "isResolved": True}}}} + + monkeypatch.setattr(sched, "gh_graphql", fake_graphql) + assert sched.resolve_outdated_review_threads(pr, dry_run=True) == 2 + assert calls == [] + + monkeypatch.delenv("GITHUB_ACTIONS", raising=False) + monkeypatch.setenv("GH_TOKEN", "local-token") + with pytest.raises(RuntimeError, match="refused outside GitHub Actions"): + sched.resolve_outdated_review_threads(pr, dry_run=False) + assert calls == [] + + monkeypatch.setenv("GITHUB_ACTIONS", "true") + monkeypatch.setenv("GH_TOKEN", "workflow-token") + assert sched.resolve_outdated_review_threads(pr, dry_run=False) == 2 + assert [fields["threadId"] for _, fields in calls] == ["thread-1", "thread-4"] + assert all(query == sched.RESOLVE_REVIEW_THREAD_MUTATION for query, _ in calls) + + def test_print_summary_writes_github_step_summary(monkeypatch, tmp_path, capsys): summary_path = tmp_path / "summary.md" monkeypatch.setenv("GITHUB_STEP_SUMMARY", str(summary_path)) @@ -506,6 +549,7 @@ def test_print_summary_writes_github_step_summary(monkeypatch, tmp_path, capsys) 10, "wait", "workflow action required: opencode-review; approve or unblock the GitHub Actions run before treating checks as failed or passed", + ("Would resolve 1 outdated review thread(s) before active unresolved-thread checks; outdated diff comments are not current-head review blockers.",), ), ] @@ -524,6 +568,9 @@ def test_print_summary_writes_github_step_summary(monkeypatch, tmp_path, capsys) assert payload["decisions"][1]["contract_decision"] == "UPDATE_BRANCH" assert payload["decisions"][2]["contract_decision"] == "WAIT" assert payload["decisions"][3]["contract_decision"] == "WAIT" + assert payload["decisions"][3]["notes"] == [ + "Would resolve 1 outdated review thread(s) before active unresolved-thread checks; outdated diff comments are not current-head review blockers." + ] assert payload["decisions"][0]["guidance"]["type"] == "merge_conflict_repair" assert payload["decisions"][0]["guidance"]["merge_state"] == "DIRTY" assert payload["decisions"][0]["guidance"]["base_ref"] == "main" @@ -543,6 +590,8 @@ def test_print_summary_writes_github_step_summary(monkeypatch, tmp_path, capsys) assert "## PR review merge scheduler" in summary assert "| #7 | block | merge conflict: DIRTY; base=main, head=feature\\|x; run" in summary assert "do not retry update-branch until the conflict is repaired" in summary + assert "### Outdated review threads" in summary + assert "Would resolve 1 outdated review thread(s)" in summary assert ( "| #8 | update_branch | current-head OpenCode review approved; " "branch update requested with workflow GH_TOKEN (github-actions[bot] in GitHub Actions) |" @@ -586,8 +635,23 @@ def test_summary_section_helpers_handle_empty_and_action_error_cases(): assert sched.conflict_repair_summary(wait_decisions) == [] assert sched.update_branch_summary(wait_decisions) == [] assert sched.workflow_action_required_summary(wait_decisions) == [] + assert sched.outdated_thread_cleanup_summary(wait_decisions) == [] assert sched.action_error_summary(wait_decisions) == [] + outdated_lines = sched.outdated_thread_cleanup_summary( + [ + sched.Decision( + 4, + "wait", + "current head is approved; auto-merge already enabled", + ("Would resolve 1 outdated review thread(s) before active unresolved-thread checks; outdated diff comments are not current-head review blockers.",), + ) + ] + ) + assert "### Outdated review threads" in outdated_lines + assert "stale UI conversations do not block current-head decisions" in "\n".join(outdated_lines) + assert "- PR #4: Would resolve 1 outdated review thread(s)" in "\n".join(outdated_lines) + action_required_lines = sched.workflow_action_required_summary( [ sched.Decision( @@ -658,6 +722,16 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): ) assert rest_clean.action == "auto_merge" assert inspect(make_pr(reviewThreads={"nodes": [{"isResolved": False}]})).reason == "1 unresolved review thread(s)" + outdated_only = inspect( + make_pr( + reviewThreads={"nodes": [{"id": "outdated-thread", "isResolved": False, "isOutdated": True}]}, + reviews={"nodes": [opencode_review("APPROVED", "head")]}, + ) + ) + assert outdated_only.action == "auto_merge" + assert outdated_only.notes == ( + "Would resolve 1 outdated review thread(s) before active unresolved-thread checks; outdated diff comments are not current-head review blockers.", + ) unresolved_auto = inspect( make_pr( reviewThreads={"nodes": [{"isResolved": False}]},