Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/opencode-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,7 @@ jobs:
| .[]
| if .__typename == "CheckRun" then
select((.name // "") != "opencode-review")
| select((.checkSuite.workflowRun.workflow.name // "") != "OpenCode Review")
| select((.checkSuite.workflowRun.workflow.name // "") != "OpenCode PR Review")
| select((.status // "") != "COMPLETED")
elif .__typename == "StatusContext" then
Expand Down
6 changes: 6 additions & 0 deletions .github/workflows/pr-review-merge-scheduler.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ on:
required: false
default: true
type: boolean
stale_opencode_minutes:
description: Redispatch OpenCode Review when an in-progress OpenCode check is older than this many minutes
required: false
default: "45"

concurrency:
group: pr-review-merge-scheduler
Expand All @@ -52,6 +56,7 @@ jobs:
TRIGGER_REVIEWS: ${{ github.event_name != 'workflow_dispatch' || inputs.trigger_reviews == true }}
ENABLE_AUTO_MERGE: ${{ github.event_name != 'workflow_dispatch' || inputs.enable_auto_merge == true }}
UPDATE_BRANCHES: ${{ github.event_name != 'workflow_dispatch' || inputs.update_branches == true }}
STALE_OPENCODE_MINUTES: ${{ inputs.stale_opencode_minutes || vars.STALE_OPENCODE_MINUTES || '45' }}
steps:
- name: Checkout trusted scheduler
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
Expand All @@ -70,6 +75,7 @@ jobs:
--max-prs "$MAX_PRS"
--project-flow "$PROJECT_FLOW"
--review-workflow "OpenCode Review"
--stale-opencode-minutes "$STALE_OPENCODE_MINUTES"
)
if [ "$DRY_RUN" = "true" ]; then
args+=(--dry-run)
Expand Down
6 changes: 4 additions & 2 deletions PR_GOVERNANCE_AUDIT.md
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,11 @@ 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;
- 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 unresolved review threads;
- blocks current-head OpenCode `CHANGES_REQUESTED`;
- blocks current-head failed check runs or status contexts before enabling auto-merge;
- 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]` instead of an OpenCode or personal 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; 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;
Expand All @@ -132,6 +132,7 @@ The checked-in scheduler already does the minimal central path:
- 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.
- 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.

Small proof run:

Expand Down Expand Up @@ -180,6 +181,7 @@ PR #381: wait: OpenCode review is already in progress
- `naruon` workflow run `28073490721` failed at `gh pr merge 694 --auto --merge --match-head-commit 76416321742af4c8dcd0f96927f64b7548d66fd8` with `GraphQL: Resource not accessible by integration (enablePullRequestAutoMerge)`. This is a DX/governance action failure, not a source-code finding, and the scheduler now records it per PR instead of aborting the scan.
- `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.
- 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.
Expand Down
3 changes: 3 additions & 0 deletions scripts/ci/collect_failed_check_evidence.sh
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,9 @@ gh api graphql \
select((.status // "") == "COMPLETED")
| select((.conclusion // "" | ascii_upcase) as $c | ["FAILURE","TIMED_OUT","ACTION_REQUIRED","CANCELLED","STARTUP_FAILURE"] | index($c))
| select(((.conclusion // "" | ascii_downcase) == "cancelled" and (.name // "") == "metadata-only gate evaluation" and (.checkSuite.workflowRun.workflow.name // "") == "PR Governance") | not)
| select((.name // "") != "opencode-review")
| select((.checkSuite.workflowRun.workflow.name // "") != "OpenCode Review")
| select((.checkSuite.workflowRun.workflow.name // "") != "OpenCode PR Review")
| [
"check_run",
(((.checkSuite.workflowRun.workflow.name // "") + "/" + (.name // "check")) | gsub("^/"; "")),
Expand Down
91 changes: 82 additions & 9 deletions scripts/ci/pr_review_merge_scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import sys
from collections.abc import Sequence
from dataclasses import dataclass
from datetime import datetime, timezone
from typing import Any


Expand Down Expand Up @@ -52,6 +53,7 @@
name
status
conclusion
startedAt
checkSuite {
workflowRun {
workflow { name }
Expand All @@ -72,6 +74,8 @@
"""

OPEN_PRS_PAGE_SIZE = 25
DEFAULT_STALE_OPENCODE_MINUTES = 45
RUNNING_CHECK_STATES = {"PENDING", "EXPECTED", "QUEUED", "IN_PROGRESS", "WAITING", "REQUESTED"}


@dataclass
Expand Down Expand Up @@ -144,6 +148,7 @@ def decision_guidance(decision: Decision) -> dict[str, Any] | None:
"base_ref": base_ref,
"head_ref": head_ref,
"summary": "Repair the PR branch against the latest base branch, then push the same branch so review and required checks rerun on the new head.",
"automation_limit": "GitHub update-branch cannot choose merge-conflict resolutions; the scheduler must wait until the PR branch is repaired.",
"steps": [
"Check out the PR branch.",
"Fetch the latest base branch.",
Expand Down Expand Up @@ -274,15 +279,58 @@ def is_strix_context(node: dict[str, Any]) -> bool:
return (node.get("context") or "") in {"strix", "Strix Security Scan"}


def opencode_in_progress(pr: dict[str, Any]) -> bool:
"""Return whether any OpenCode review status for the PR is still running."""
def parse_github_datetime(value: str | None) -> datetime | None:
"""Parse a GitHub API timestamp into an aware UTC datetime."""
if not value:
return None
try:
parsed = datetime.fromisoformat(value.replace("Z", "+00:00"))
except ValueError:
return None
if parsed.tzinfo is None:
return parsed.replace(tzinfo=timezone.utc)
return parsed.astimezone(timezone.utc)


def running_check_state(node: dict[str, Any]) -> str:
"""Return running, complete, or absent for a check/status context."""
status = (node.get("status") or node.get("state") or "").upper()
if not status:
return "absent"
return "running" if status in RUNNING_CHECK_STATES else "complete"


def opencode_progress_state(
pr: dict[str, Any],
*,
stale_after_minutes: int,
now: datetime | None = None,
) -> str:
"""Return absent, running, stale, or complete for current OpenCode review status."""
now = now or datetime.now(timezone.utc)
saw_complete = False
for node in context_nodes(pr):
if not is_opencode_context(node):
continue
status = (node.get("status") or node.get("state") or "").upper()
if status and status not in {"COMPLETED", "SUCCESS", "FAILURE", "ERROR"}:
return True
return False
state = running_check_state(node)
if state == "absent":
continue
if state != "running":
saw_complete = True
continue
Comment thread
Copilot marked this conversation as resolved.
started_at = parse_github_datetime(node.get("startedAt"))
if started_at and stale_after_minutes >= 0:
age_seconds = (now - started_at).total_seconds()
if age_seconds >= stale_after_minutes * 60:
return "stale"
return "running"
return "complete" if saw_complete else "absent"


def opencode_in_progress(pr: dict[str, Any], *, stale_after_minutes: int | None = None) -> bool:
"""Return whether any OpenCode review status for the PR is still actively running."""
stale_after = DEFAULT_STALE_OPENCODE_MINUTES if stale_after_minutes is None else stale_after_minutes
return opencode_progress_state(pr, stale_after_minutes=stale_after) == "running"


def strix_evidence_state(pr: dict[str, Any]) -> str:
Expand All @@ -293,7 +341,7 @@ def strix_evidence_state(pr: dict[str, Any]) -> str:
continue
found = True
status = (node.get("status") or node.get("state") or "").upper()
if status in {"PENDING", "EXPECTED", "QUEUED", "IN_PROGRESS", "WAITING", "REQUESTED"}:
if status in RUNNING_CHECK_STATES:
return "running"
if node.get("__typename") == "CheckRun" and status != "COMPLETED":
return "running"
Expand Down Expand Up @@ -452,7 +500,8 @@ def merge_conflict_guidance(pr: dict[str, Any], merge_state: str) -> str:
f"`git merge --no-ff origin/{base_ref}` or `git rebase origin/{base_ref}`; "
"use `git status --short` to find conflicted files, resolve conflict markers in the PR branch, "
f"rerun focused checks, and push the same {head_ref} branch "
"(use `git push --force-with-lease` only if rebased)"
"(use `git push --force-with-lease` only if rebased); "
"do not retry update-branch until the conflict is repaired"
)


Expand All @@ -467,6 +516,7 @@ def inspect_pr(
workflow: str,
security_workflow: str,
base_branch: str,
stale_opencode_minutes: int = DEFAULT_STALE_OPENCODE_MINUTES,
) -> Decision:
"""Decide and optionally act on one pull request's merge-readiness state."""
number = pr["number"]
Expand Down Expand Up @@ -515,8 +565,22 @@ def inspect_pr(
enable_auto_merge(repo, pr, dry_run=dry_run)
return Decision(number, "auto_merge", "current head is approved; auto-merge enabled")

if opencode_in_progress(pr):
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")
if opencode_state == "stale" and not trigger_reviews:
return Decision(
number,
"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,
"review_dispatch",
f"OpenCode review exceeded {stale_opencode_minutes} minute retry threshold; same-head OpenCode re-dispatched",
)

if trigger_reviews:
strix_state = strix_evidence_state(pr)
Expand Down Expand Up @@ -652,6 +716,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.",
"`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:
assert parsed is not None
Expand Down Expand Up @@ -691,6 +756,7 @@ def update_branch_summary(decisions: list[Decision]) -> list[str]:
"### Branch update requests",
"",
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.",
"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.",
Expand Down Expand Up @@ -922,6 +988,7 @@ def self_test() -> None:
assert conflict_guidance
assert conflict_guidance["type"] == "merge_conflict_repair"
assert conflict_guidance["merge_state"] == "DIRTY"
assert "update-branch cannot choose" in conflict_guidance["automation_limit"]
assert "git status --short" in conflict_guidance["commands"]
assert contract_decision(Decision(1, "update_branch", "ok")) == "UPDATE_BRANCH"
assert contract_decision(Decision(1, "wait", "ok")) == "WAIT"
Expand Down Expand Up @@ -964,6 +1031,11 @@ def parse_args(argv: list[str]) -> argparse.Namespace:
parser.add_argument("--update-branches", action=argparse.BooleanOptionalAction, default=True)
parser.add_argument("--review-workflow", default="OpenCode Review")
parser.add_argument("--security-workflow", default="Strix Security Scan")
parser.add_argument(
"--stale-opencode-minutes",
type=int,
default=int(os.environ.get("STALE_OPENCODE_MINUTES", str(DEFAULT_STALE_OPENCODE_MINUTES))),
)
parser.add_argument("--self-test", action="store_true")
return parser.parse_args(argv)

Expand Down Expand Up @@ -994,6 +1066,7 @@ def main(argv: list[str]) -> int:
workflow=args.review_workflow,
security_workflow=args.security_workflow,
base_branch=args.base_branch,
stale_opencode_minutes=args.stale_opencode_minutes,
)
except RuntimeError as exc:
decision = Decision(
Expand Down
4 changes: 4 additions & 0 deletions scripts/ci/test_strix_quick_gate.sh
Original file line number Diff line number Diff line change
Expand Up @@ -634,8 +634,12 @@ assert_opencode_review_uses_codegraph_and_gpt5_fallback() {
assert_file_contains "$workflow_file" "collect_failed_check_evidence_or_note()" "opencode approval handles repos without the failed-check helper before publishing fallback reviews"
assert_file_contains "$workflow_file" "current_peer_checks_still_running" "opencode review workflow distinguishes pending peer checks from completed check state"
assert_file_contains "$workflow_file" 'select((.name // "") != "opencode-review")' "opencode review evidence wait excludes its own check run"
assert_file_contains "$workflow_file" 'select((.checkSuite.workflowRun.workflow.name // "") != "OpenCode Review")' "opencode review evidence wait excludes its own actual workflow name"
assert_file_contains "$workflow_file" 'select((.checkSuite.workflowRun.workflow.name // "") != "OpenCode PR Review")' "opencode review evidence wait excludes its own workflow"
assert_file_contains "$workflow_file" "No completed failed GitHub Checks were present" "opencode review evidence wait retries while no failed checks are available yet"
assert_file_contains "$REPO_ROOT/scripts/ci/collect_failed_check_evidence.sh" 'select((.name // "") != "opencode-review")' "failed-check evidence excludes OpenCode's own required check"
assert_file_contains "$REPO_ROOT/scripts/ci/collect_failed_check_evidence.sh" 'select((.checkSuite.workflowRun.workflow.name // "") != "OpenCode Review")' "failed-check evidence excludes OpenCode's own workflow by actual name"
assert_file_contains "$REPO_ROOT/scripts/ci/collect_failed_check_evidence.sh" 'select((.checkSuite.workflowRun.workflow.name // "") != "OpenCode PR Review")' "failed-check evidence excludes OpenCode's own workflow by legacy name"
assert_file_contains "$REPO_ROOT/scripts/ci/collect_failed_check_evidence.sh" 'gh run view "$run_id"' "failed-check evidence collector reads failed GitHub Actions job logs"
assert_file_contains "$REPO_ROOT/scripts/ci/collect_failed_check_evidence.sh" 'check-runs/${check_run_id}/annotations' "failed-check evidence collector reads GitHub Check annotations"
assert_file_contains "$REPO_ROOT/scripts/ci/collect_failed_check_evidence.sh" "Line-specific repair contract" "failed-check evidence requires line-specific repairs"
Expand Down
Loading