diff --git a/.github/workflows/opencode-review.yml b/.github/workflows/opencode-review.yml index 90c867b44..ced41f6d4 100644 --- a/.github/workflows/opencode-review.yml +++ b/.github/workflows/opencode-review.yml @@ -2135,6 +2135,7 @@ jobs: local review_payload_file gh_error_file="$(mktemp)" review_payload_file="$(mktemp)" + emit_review_body_to_action_log "$event" "$body" jq -n \ --arg event "$event" \ --arg body "$body" \ @@ -2150,6 +2151,61 @@ jobs: update_review_overview "$event" "$body" } + emit_review_body_to_action_log() { + local event="$1" body="$2" review_payload_file="${3:-}" + local stop_token + + case "$event" in + REQUEST_CHANGES | INLINE_COMMENT_PUBLISH_FAILED) ;; + *) return 0 ;; + esac + + stop_token="opencode-review-body-${RUN_ID}-${RUN_ATTEMPT}-${RANDOM}" + printf '::group::OpenCode %s review body\n' "$event" + printf '::stop-commands::%s\n' "$stop_token" + printf 'OpenCode is publishing this review content to PR #%s.\n\n' "$PR_NUMBER" + printf -- '- Event: %s\n' "$event" + printf -- '- Head SHA: %s\n' "$HEAD_SHA" + printf -- '- Workflow run: %s\n' "$RUN_ID" + printf -- '- Workflow attempt: %s\n\n' "$RUN_ATTEMPT" + printf '%s\n' "$body" + if [ -s "$review_payload_file" ]; then + printf '\n## Inline review comments\n\n' + jq -r ' + (.comments // []) + | to_entries[] + | "### Inline comment " + ((.key + 1) | tostring) + + " on `" + (.value.path // "unknown") + ":" + ((.value.line // 0) | tostring) + "`\n\n" + + (.value.body // "") + + "\n" + ' "$review_payload_file" || true + fi + printf '::%s::\n' "$stop_token" + printf '::endgroup::\n' + + if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then + { + printf '## OpenCode %s review body\n\n' "$event" + printf -- '- Head SHA: `%s`\n' "$HEAD_SHA" + printf -- '- Workflow run: %s\n' "$RUN_ID" + printf -- '- Workflow attempt: %s\n\n' "$RUN_ATTEMPT" + printf '%s\n' "$body" + if [ -s "$review_payload_file" ]; then + printf '\n## Inline review comments\n\n' + jq -r ' + (.comments // []) + | to_entries[] + | "### Inline comment " + ((.key + 1) | tostring) + + " on `" + (.value.path // "unknown") + ":" + ((.value.line // 0) | tostring) + "`\n\n" + + (.value.body // "") + + "\n" + ' "$review_payload_file" || true + fi + printf '\n' + } >>"$GITHUB_STEP_SUMMARY" + fi + } + stop_approval_without_review() { local result="$1" local body="$2" @@ -2348,6 +2404,7 @@ jobs: local event="$1" body="$2" review_payload_file="$3" fallback_body_file="$4" local gh_error_file gh_error_file="$(mktemp)" + emit_review_body_to_action_log "$event" "$body" "$review_payload_file" if ! gh api -X POST "repos/${GH_REPOSITORY}/pulls/${PR_NUMBER}/reviews" --input "$review_payload_file" >/dev/null 2>"$gh_error_file"; then warn_gh_publication_failure "pull review inline comments" "$gh_error_file" rm -f "$gh_error_file" diff --git a/.github/workflows/pr-review-merge-scheduler.yml b/.github/workflows/pr-review-merge-scheduler.yml index 320e3a10f..dfdc1136d 100644 --- a/.github/workflows/pr-review-merge-scheduler.yml +++ b/.github/workflows/pr-review-merge-scheduler.yml @@ -1,6 +1,53 @@ name: PR Review Merge Scheduler on: + workflow_call: + inputs: + dry_run: + description: Print planned actions without mutating PRs + required: false + default: false + type: boolean + max_prs: + description: Maximum open PRs to inspect + required: false + default: "100" + type: string + trigger_reviews: + description: Dispatch OpenCode Review for PR heads without current approval + required: false + default: true + type: boolean + enable_auto_merge: + description: Enable auto-merge for current-head approved PRs + required: false + default: true + type: boolean + update_branches: + description: Update outdated PR branches after OpenCode approval + 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" + type: string + project_flow: + description: Project flow, usually github-flow or git-flow + required: false + default: "" + type: string + base_branch: + description: Base branch to scan; defaults to the caller repository default branch + required: false + default: "" + type: string + canonical_ref: + description: Ref of ContextualWisdomLab/.github to use for scheduler code + required: false + default: "main" + type: string schedule: - cron: "17 */2 * * *" workflow_dispatch: @@ -35,7 +82,7 @@ on: default: "45" concurrency: - group: pr-review-merge-scheduler + group: pr-review-merge-scheduler-${{ github.repository }} cancel-in-progress: false jobs: @@ -48,19 +95,22 @@ jobs: pull-requests: write env: FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} - DRY_RUN: ${{ github.event_name == 'workflow_dispatch' && inputs.dry_run == true }} + GH_TOKEN: ${{ github.token }} + DEFAULT_BRANCH: ${{ inputs.base_branch || github.event.repository.default_branch }} + DRY_RUN: ${{ inputs.dry_run == true }} MAX_PRS: ${{ inputs.max_prs || '100' }} - PROJECT_FLOW: ${{ vars.PROJECT_FLOW || 'git-flow' }} - 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 }} + PROJECT_FLOW: ${{ inputs.project_flow || vars.PROJECT_FLOW || 'git-flow' }} + TRIGGER_REVIEWS: ${{ github.event_name == 'schedule' || inputs.trigger_reviews == true }} + ENABLE_AUTO_MERGE: ${{ github.event_name == 'schedule' || inputs.enable_auto_merge == true }} + UPDATE_BRANCHES: ${{ github.event_name == 'schedule' || inputs.update_branches == true }} STALE_OPENCODE_MINUTES: ${{ inputs.stale_opencode_minutes || vars.STALE_OPENCODE_MINUTES || '45' }} + CANONICAL_REF: ${{ inputs.canonical_ref || 'main' }} steps: - name: Checkout trusted scheduler uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: + repository: ContextualWisdomLab/.github + ref: ${{ env.CANONICAL_REF }} fetch-depth: 1 - name: Self-test scheduler diff --git a/PR_GOVERNANCE_AUDIT.md b/PR_GOVERNANCE_AUDIT.md index f29937766..6c43efbe4 100644 --- a/PR_GOVERNANCE_AUDIT.md +++ b/PR_GOVERNANCE_AUDIT.md @@ -6,9 +6,34 @@ Live check: 2026-06-26 KST, GitHub API via `gh` as `seonghobae`. OpenCode decides; GitHub Actions mutates. +- The canonical implementation belongs in `ContextualWisdomLab/.github`. + Repository-local copies of the scheduler, OpenCode review workflow, Strix + gate, or helper scripts are drift sources, not repo-specific contracts. +- Target repositories should contain at most thin workflow callers, or no caller + at all when an organization required-workflow/ruleset mechanism can provide + the trigger. Thick downstream sync PRs are an anti-pattern unless they are a + temporary rollback bridge. +- `fork` versus `non-fork` is not the rollout boundary. Central governance + applies to every target repository that opts into the organization contract. + Runtime decisions classify the PR head capability instead: observable, + reviewable, updateable, auto-mergeable, and mergeable. External heads may be + fully reviewable while remaining non-mutable by the scheduler credential. +- GitHub workflow templates can help create thin callers, but templates are + scaffolding, not centralized execution. Reusable workflows (`workflow_call`) + centralize implementation while a caller or required-workflow trigger supplies + the target repository event and token context. +- Live organization state at the 2026-06-26 KST check: Actions are enabled for + all repositories, all actions and reusable workflows are allowed, and the + organization rulesets API returned no organization-level rulesets. That means + central rollout should start with reusable workflows plus thin callers, then + replace callers with organization required workflows only after the feature is + verified live for this plan/account. - OpenCode may return only a decision: `UPDATE_BRANCH`, `WAIT`, `REQUEST_CHANGES`, or `NO_ACTION`. -- GitHub Actions updates same-repository PR heads with `expected_head_sha` - only after current-head failed checks have been ruled out. +- GitHub Actions updates only mutable PR heads with `expected_head_sha` after + current-head failed checks have been ruled out. Same-repository heads are + normally mutable; external heads are attempted only when GitHub exposes a + maintainer-writable head path, and otherwise receive explicit update + guidance instead of being skipped. - The GitHub REST permission surfaces are split: `update-branch` uses Pull requests write permission, while merge uses Contents write permission (GitHub REST pull request endpoint docs: @@ -24,6 +49,10 @@ OpenCode decides; GitHub Actions mutates. - OpenCode approval publication must be bounded. Peer GitHub Checks can be awaited, but the approval step itself must time out instead of running for hours; the current central limit is a 45 minute approval step with 81 peer-check probes at 30 seconds. - Tool failures are not source findings. Model failure, API transient, update-branch `422/403`, fork/write-permission failure, conflict, failed checks, and stale review state must be reported as distinct scheduler outcomes. A failed current-head check blocks `UPDATE_BRANCH`; the scheduler must not use a branch update as a way to hide or bypass failed evidence. - Developer experience and user experience are separate review surfaces. Reviews must adopt helpful sibling-repo automation, review, setup, documentation, and product-flow patterns when they reduce friction, and flag noisy automation, false failures, misleading status, repeated waiting, or URL-only diagnostics as experience defects instead of treating them as neutral implementation detail. +- When OpenCode publishes `REQUEST_CHANGES`, the same review body and attempted + inline-comment payload must also be emitted to the GitHub Actions log and job + summary. Humans and later agents should not have to infer the review content + from a URL-only failure or a missing PR-side publication. ## Non-Actionable Findings Ban @@ -48,7 +77,7 @@ them to a local defect. ## Live Repository Inventory -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. +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 organization target inventory contains 12 public repositories from the prior scan; `VibeSec` was not in that target set, and `appguardrail` was. | Repo | Flow | Default | Auto | Rulesets | Required checks | Stale dismissal | Open PRs | Workflows | Recent merged actor | |---|---:|---:|---:|---|---|---:|---:|---|---| @@ -70,27 +99,27 @@ Live generated: 2026-06-26 KST via GitHub REST/GraphQL APIs. PR #28 post-merge r | Repo | Gap | |---|---| | `.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. | +| `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. Follow-up PR #460 was closed because it copied the central scheduler into `bandscope` and would preserve exactly the repo-local drift this rollout should remove. | | `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. | -| `hyosung-itx-slogan-brief` | Public non-fork repo discovered in the 2026-06-25 13:46 KST refresh. It has OpenCode Review and PR Review Merge Scheduler but auto-merge is off and the only ruleset prevents branch deletion, so it should either stay as a lightweight GitHub Flow repo or explicitly opt into the default-branch lock contract. | +| `hyosung-itx-slogan-brief` | Public organization repo discovered in the 2026-06-25 13:46 KST refresh. It has OpenCode Review and PR Review Merge Scheduler but auto-merge is off and the only ruleset prevents branch deletion, so it should either stay as a lightweight GitHub Flow repo or explicitly opt into the default-branch lock contract. | | `naruon` | Canonical strict check source. PR #756 synced the central scheduler into `naruon`; its first head proved that widening `GITHUB_TOKEN` permissions to solve DX creates Scorecard and governance failures, so the merged rollout keeps minimal token permissions and defaults risky review-dispatch/auto-merge paths off. PR #721 remains the useful historical fixture for `BEHIND` handling: central dry-run selected `update_branch`, while the older repo-local workflow treated it as `wait`. Current PR #760 is clean, approved, and green on head `57a2f8e4`, so it is a merge-readiness sample; current dry-run with auto-merge disabled reports `wait`, as expected for the low-privilege scheduler profile. | | `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. | -| `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. | +| `appguardrail` | Public organization 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 #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. | +| `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 mutable 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. | -| `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. | +| `appguardrail` | Current public organization 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 @@ -102,10 +131,10 @@ both separately; a change can improve one while harming the other. | Repo | Borrow because it helps DX/UX | Improve because it creates friction | Central action | |---|---|---|---| | `.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. | +| `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. | Move the implementation contract back to `.github` reusable workflows, keep only a thin caller or required-workflow trigger in `naruon`, 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. | | `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. | +| `bandscope` | Broad required checks encode repo-specific release, build, SBOM, and security expectations. | Copying the central scheduler into this repo turns one canonical contract into another repo-local drift surface. | Let GitHub native auto-merge and rulesets interpret required checks, and replace thick local governance files with a thin caller or organization required workflow. | | `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`. | | `clearfolio` | Direct guarded merge works while auto-merge is intentionally off. | Treating it like an auto-merge repo would create confusing expectations. | Use immediate guarded merge only after same-head evidence, unresolved-thread check, and head guard pass. | @@ -118,7 +147,10 @@ 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; +- skips only draft PRs and PRs whose base branch is outside the configured + scheduler target branch; +- keeps external-head PRs in the same observation and review pipeline, then + gates only the write actions by PR head mutation capability; - 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; - 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; @@ -126,7 +158,8 @@ The checked-in scheduler already does the minimal central path: - 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; the script now refuses non-dry-run `update-branch` outside GitHub Actions, and this path needs `pull-requests: write`, not `contents: write`; +- updates `BEHIND` only when OpenCode approved the exact current head, no current-head failed check is present, and the PR head is actually mutable by the scheduler credential, 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`; +- waits with `external_head_update_required` guidance when a current-head-approved external PR head is behind but is not writable by the scheduler credential, instead of treating fork/non-fork as an onboarding exception; - 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; @@ -164,15 +197,26 @@ PR #381: wait: OpenCode review is already in progress ## Rollout List -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. -5. Keep `pg-erd-cloud` autofix workflows repo-local; do not make autofix part of the central merge contract. +1. Stop thick per-repository scheduler/OpenCode/Strix copies. `bandscope` PR + #460 is closed as the negative example of the wrong rollout shape. +2. Add or preserve canonical workflows in `ContextualWisdomLab/.github` and make + them callable through `workflow_call` or an organization required-workflow + mechanism after live feature verification. +3. For repositories that need PR-event triggers, keep only thin callers that + pass PR number, base ref/SHA, head ref/SHA, target flow, and inherited + secrets/permissions into `.github`. +4. Treat fork and non-fork repositories uniformly for onboarding. At runtime, + classify only the PR head mutation capability: observable/reviewable, + updateable, auto-mergeable, or mergeable. +5. Keep repo-specific product/build/autofix workflows repo-local only when they + are not part of the governance contract. `pg-erd-cloud` autofix stays + repo-local; PR review/merge governance should not. +6. Decide whether `contextual-orchestrator` should join the central governance + surface; no matching workflows or rulesets were returned. ## Remaining Proof Gaps -- 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. +- 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 organization target inventory contains 12 public 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. @@ -190,6 +234,12 @@ PR #381: wait: OpenCode review is already in progress - `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-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. +- The previous drift response still over-indexed on copying. `bandscope` PR + #460 proved the correction: even when the copied scheduler produced the right + dry-run result, the PR itself was the wrong operating model because it + preserved per-repository implementation ownership. The rollout proof must now + show a target repository invoking `.github` canonical logic without carrying a + thick local copy. - 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 e16101259..c3af855ae 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -32,6 +32,8 @@ baseRefOid headRefName headRefOid + isCrossRepository + maintainerCanModify headRepository { nameWithOwner } autoMergeRequest { enabledAt } commits(last: 1) { @@ -231,6 +233,22 @@ def decision_guidance(decision: Decision) -> dict[str, Any] | None: "zero active unresolved review threads", ], } + external_update = parse_external_head_update_reason(decision.reason) + if external_update: + return { + "type": "external_head_update_required", + "head_repository": external_update, + "summary": "The PR can be reviewed centrally, but this head branch is not writable by the scheduler credential.", + "automation_limit": "The scheduler should not skip the PR; it waits for the author to update the branch or for maintainers to enable a writable head path.", + "next_required_evidence": [ + "PR author updates the head branch against the base branch, or maintainer edit permission is enabled", + "new head SHA after the branch update", + "OpenCode approval on that exact new head", + "same-head Strix evidence", + "required GitHub Checks success", + "zero active unresolved review threads", + ], + } if decision.action == "update_branch": return { "type": "github_actions_update_branch", @@ -646,6 +664,26 @@ def update_branch(repo: str, pr: dict[str, Any], *, dry_run: bool) -> None: ) +def can_update_pr_head(repo: str, pr: dict[str, Any]) -> bool: + """Return whether the scheduler may try to mutate the PR head branch.""" + head_repo = (pr.get("headRepository") or {}).get("nameWithOwner") + if head_repo == repo: + return True + return bool(pr.get("maintainerCanModify")) + + +def non_mutable_head_reason(repo: str, pr: dict[str, Any]) -> str: + """Explain why a PR can be reviewed but not mechanically updated.""" + head_repo = (pr.get("headRepository") or {}).get("nameWithOwner") or "" + if head_repo == repo: + return "current-head OpenCode review approved, but same-repository head update permission is unavailable" + return ( + f"current-head OpenCode review approved, but head repo {head_repo} is external and not writable by " + "the scheduler credential; ask the PR author to update the branch against the base branch, or enable " + "a maintainer-writable head path before rerunning" + ) + + 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": @@ -742,15 +780,12 @@ def inspect_pr( ) -> Decision: """Decide and optionally act on one pull request's merge-readiness state.""" number = pr["number"] - head_repo = (pr.get("headRepository") or {}).get("nameWithOwner") base_ref = pr.get("baseRefName") if pr.get("isDraft"): return Decision(number, "skip", "draft PR") if base_ref != base_branch: return Decision(number, "skip", f"base branch is {base_ref}; expected {base_branch}") - 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) @@ -848,6 +883,8 @@ def decide(action: str, reason: str) -> Decision: if merge_state == "BEHIND" and current_head_approved: if not update_branches: return decide("wait", "current-head OpenCode review approved; branch update disabled") + if not can_update_pr_head(repo, pr): + return decide("wait", non_mutable_head_reason(repo, pr)) had_auto_merge = bool(pr.get("autoMergeRequest")) if had_auto_merge: disable_auto_merge(repo, pr, dry_run=dry_run) @@ -982,6 +1019,7 @@ def write_actions_summary( lines.extend(conflict_repair_summary(decisions)) lines.extend(outdated_thread_cleanup_summary(decisions)) lines.extend(update_branch_summary(decisions)) + lines.extend(external_head_update_summary(decisions)) lines.extend(workflow_action_required_summary(decisions)) lines.extend(action_error_summary(decisions)) @@ -1096,6 +1134,40 @@ def update_branch_summary(decisions: list[Decision]) -> list[str]: ] +def parse_external_head_update_reason(reason: str) -> str | None: + """Extract the external head repository from non-mutable update guidance.""" + match = re.search(r"head repo ([^\s]+) is external and not writable", reason) + if not match: + return None + return match.group(1) + + +def external_head_update_summary(decisions: list[Decision]) -> list[str]: + """Return a GitHub Actions Summary section for non-mutable external PR heads.""" + external_waits = [ + (decision, parse_external_head_update_reason(decision.reason)) + for decision in decisions + if parse_external_head_update_reason(decision.reason) + ] + if not external_waits: + return [] + + lines = [ + "", + "### External head update required", + "", + "These PRs remain in the central review pipeline, but their head branches are not writable by the scheduler credential. This is a mutation-capability limit, not a fork/non-fork onboarding exception.", + ] + for decision, head_repo in external_waits: + lines.extend( + [ + "", + f"- PR #{decision.pr}: ask the author of `{head_repo}` to update the branch against the base branch, or enable maintainer edit permission and rerun the scheduler.", + ] + ) + return lines + + def action_error_summary(decisions: list[Decision]) -> list[str]: """Return a GitHub Actions Summary section for mutation failures.""" errors = [decision for decision in decisions if decision.action == "action_error"] @@ -1190,6 +1262,8 @@ def self_test() -> None: "mergeStateStatus": "CLEAN", "restMergeableState": "CLEAN", "isDraft": False, + "isCrossRepository": False, + "maintainerCanModify": False, "headRepository": {"nameWithOwner": "owner/repo"}, "reviewDecision": "REVIEW_REQUIRED", "commits": { @@ -1408,6 +1482,39 @@ def self_test() -> None: base_branch="main", ) assert decision.action == "update_branch" + sample["headRepository"] = {"nameWithOwner": "external/repo"} + sample["isCrossRepository"] = True + sample["maintainerCanModify"] = False + decision = inspect_pr( + "owner/repo", + sample, + dry_run=True, + trigger_reviews=True, + enable_auto_merge_flag=True, + update_branches=True, + workflow="OpenCode Review", + security_workflow="Strix Security Scan", + base_branch="main", + ) + assert decision.action == "wait" + assert "external/repo" in decision.reason + assert decision_guidance(decision)["type"] == "external_head_update_required" + sample["maintainerCanModify"] = True + decision = inspect_pr( + "owner/repo", + sample, + dry_run=True, + trigger_reviews=True, + enable_auto_merge_flag=True, + update_branches=True, + workflow="OpenCode Review", + security_workflow="Strix Security Scan", + base_branch="main", + ) + assert decision.action == "update_branch" + sample["headRepository"] = {"nameWithOwner": "owner/repo"} + sample["isCrossRepository"] = False + sample["maintainerCanModify"] = False sample["autoMergeRequest"] = {"enabledAt": "2026-01-01T00:02:00Z"} decision = inspect_pr( "owner/repo", diff --git a/scripts/ci/test_opencode_fact_gate_contract.sh b/scripts/ci/test_opencode_fact_gate_contract.sh index 6b369c6e8..27c548b7f 100755 --- a/scripts/ci/test_opencode_fact_gate_contract.sh +++ b/scripts/ci/test_opencode_fact_gate_contract.sh @@ -25,5 +25,9 @@ check_contains 'Latest unresolved human review thread evidence' check_contains 'OpenCode reviewed the current-head evidence but found unresolved human review threads before approval.' check_contains 'bounded-review-evidence-excerpt.md' check_contains 'Current-head bounded evidence excerpt, inlined to prevent false no-change or no-coverage approvals when tool/file reads are skipped:' +check_contains 'emit_review_body_to_action_log()' +check_contains '::stop-commands::%s' +check_contains 'OpenCode is publishing this review content to PR #%s.' +check_contains '## Inline review comments' printf 'OpenCode fact-gate contract OK\n' diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index 4ac8e42ea..37c57c8db 100755 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -812,7 +812,9 @@ assert_pr_review_merge_scheduler_uses_github_actions_bot_token() { local scheduler_file="$REPO_ROOT/scripts/ci/pr_review_merge_scheduler.py" local readme_file="$REPO_ROOT/README.md" - assert_file_contains "$workflow_file" 'GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}' "scheduler branch updates and merges use the GitHub Actions bot token" + assert_file_contains "$workflow_file" 'workflow_call:' "scheduler can run as the central reusable workflow contract" + assert_file_contains "$workflow_file" 'GH_TOKEN: ${{ github.token }}' "scheduler uses the caller workflow token so mutations are attributed to GitHub Actions in the target repository" + assert_file_contains "$workflow_file" 'repository: ContextualWisdomLab/.github' "scheduler checks out the canonical implementation instead of relying on repo-local copies" assert_file_contains "$workflow_file" "contents: write" "scheduler has write permission for GitHub Actions bot branch updates" assert_file_contains "$workflow_file" "pull-requests: write" "scheduler has pull-request write permission for update-branch and auto-merge" assert_file_contains "$scheduler_file" "update-branch" "scheduler calls the GitHub update-branch API for outdated approved PRs" diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 38a601ea3..457f9fd28 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -20,6 +20,8 @@ def make_pr(**overrides): "baseRefOid": "base", "headRefName": "feature", "headRefOid": "head", + "isCrossRepository": False, + "maintainerCanModify": False, "headRepository": {"nameWithOwner": "owner/repo"}, "autoMergeRequest": None, "commits": { @@ -551,6 +553,11 @@ def test_print_summary_writes_github_step_summary(monkeypatch, tmp_path, capsys) "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.",), ), + sched.Decision( + 11, + "wait", + "current-head OpenCode review approved, but head repo fork/repo is external and not writable by the scheduler credential; ask the PR author to update the branch against the base branch, or enable a maintainer-writable head path before rerunning", + ), ] sched.print_summary(decisions, dry_run=True, base_branch="main", project_flow="github-flow") @@ -560,14 +567,15 @@ 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, "wait": 1} + assert payload["counts"] == {"block": 1, "disable_auto_merge": 1, "update_branch": 1, "wait": 2} assert payload["dry_run"] is True - assert payload["inspected"] == 4 + assert payload["inspected"] == 5 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"][4]["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." ] @@ -586,6 +594,8 @@ def test_print_summary_writes_github_step_summary(monkeypatch, tmp_path, capsys) 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" + assert payload["decisions"][4]["guidance"]["type"] == "external_head_update_required" + assert payload["decisions"][4]["guidance"]["head_repository"] == "fork/repo" 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 @@ -616,6 +626,9 @@ def test_print_summary_writes_github_step_summary(monkeypatch, tmp_path, capsys) 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 + assert "### External head update required" in summary + assert "mutation-capability limit" in summary + assert "- PR #11: ask the author of `fork/repo` to update the branch" in summary def test_write_actions_summary_is_noop_without_summary_path(monkeypatch): @@ -634,6 +647,7 @@ 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.external_head_update_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) == [] @@ -674,7 +688,8 @@ def test_summary_section_helpers_handle_empty_and_action_error_cases(): def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): assert inspect(make_pr(isDraft=True)).action == "skip" assert inspect(make_pr(baseRefName="develop")).reason == "base branch is develop; expected main" - assert inspect(make_pr(headRepository={"nameWithOwner": "fork/repo"})).action == "skip" + external_head = inspect(make_pr(headRepository={"nameWithOwner": "fork/repo"}, isCrossRepository=True)) + assert external_head.action == "security_dispatch" conflict = inspect(make_pr(mergeStateStatus="DIRTY")) assert conflict.action == "block" assert "merge conflict: DIRTY" in conflict.reason @@ -797,6 +812,32 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): assert "github-actions[bot]" in decision.reason assert called == [("owner/repo", 1, True)] called.clear() + external_behind = make_pr( + mergeStateStatus="BEHIND", + isCrossRepository=True, + maintainerCanModify=False, + headRepository={"nameWithOwner": "fork/repo"}, + reviews={"nodes": [opencode_review("APPROVED", "head")]}, + ) + external_decision = inspect(external_behind) + assert external_decision.action == "wait" + assert "fork/repo is external and not writable" in external_decision.reason + assert sched.decision_guidance(external_decision)["type"] == "external_head_update_required" + assert called == [] + external_mutable = make_pr( + mergeStateStatus="BEHIND", + isCrossRepository=True, + maintainerCanModify=True, + headRepository={"nameWithOwner": "fork/repo"}, + reviews={"nodes": [opencode_review("APPROVED", "head")]}, + ) + assert inspect(external_mutable).action == "update_branch" + assert called == [("owner/repo", 1, True)] + called.clear() + assert sched.can_update_pr_head("owner/repo", behind) + assert not sched.can_update_pr_head("owner/repo", external_behind) + assert sched.can_update_pr_head("owner/repo", external_mutable) + assert "same-repository head update permission" in sched.non_mutable_head_reason("owner/repo", behind) behind_failed = make_pr( mergeStateStatus="BEHIND", reviews={"nodes": [opencode_review("APPROVED", "head")]},