Skip to content

[codex] Sync PR governance scheduler contract - #460

Closed
seonghobae wants to merge 1 commit into
developfrom
codex/sync-pr-governance-scheduler
Closed

[codex] Sync PR governance scheduler contract#460
seonghobae wants to merge 1 commit into
developfrom
codex/sync-pr-governance-scheduler

Conversation

@seonghobae

Copy link
Copy Markdown
Collaborator

Summary

  • Sync the local PR Review Merge Scheduler with the centralized governance contract.
  • Resolve GitHub Outdated review threads before active unresolved-thread checks, using the workflow GITHUB_TOKEN only.
  • Require OpenCode approvals to match the current head commit and, when present, the explicit review-body Head SHA.
  • Treat ACTION_REQUIRED check runs as workflow/policy waits instead of source-code failures.
  • Preserve the repo-local GH_HOST guard so scheduler tokens are only used against github.com.

Live evidence

Validation

  • python3 scripts/ci/pr_review_merge_scheduler.py --self-test
  • python3 -m py_compile scripts/ci/pr_review_merge_scheduler.py
  • bash -n scripts/ci/collect_failed_check_evidence.sh scripts/ci/emit_opencode_failed_check_fallback_findings.sh scripts/ci/opencode_review_approve_gate.sh scripts/ci/validate_opencode_failed_check_review.sh scripts/ci/test_opencode_fact_gate_contract.sh
  • GH_HOST=evil.example python3 scripts/ci/pr_review_merge_scheduler.py --repo ContextualWisdomLab/bandscope --base-branch develop --project-flow git-flow --dry-run --max-prs 1 exits with unsupported GH_HOST.
  • python3 scripts/ci/pr_review_merge_scheduler.py --repo ContextualWisdomLab/bandscope --base-branch develop --project-flow git-flow --dry-run --max-prs 100 --no-trigger-reviews --no-enable-auto-merge --update-branches
  • ./scripts/harness/quickcheck.sh
  • git diff --check

Copilot AI review requested due to automatic review settings June 25, 2026 22:02

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the local PR Review Merge Scheduler to match the centralized governance contract, tightening “current head” approval validation, proactively resolving GitHub-marked outdated review threads, and reclassifying ACTION_REQUIRED check runs as workflow/policy waits rather than source-code failures.

Changes:

  • Resolve GitHub Outdated review threads (via GraphQL thread IDs) before enforcing unresolved-thread gates, and annotate decisions with cleanup notes.
  • Strengthen OpenCode approval validation by requiring the review to match the PR head commit and (when present) an explicit Head SHA embedded in the review body.
  • Split ACTION_REQUIRED check runs out of “failed checks” into a dedicated “wait for workflow unblock” decision/guidance path, and improve error logging by scrubbing sensitive data.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
services/analysis-engine/tests/test_supply_chain_policy.py Updates the supply-chain policy test assertions to track the updated scheduler contract (new helpers/constants and logic).
scripts/ci/pr_review_merge_scheduler.py Implements the governance sync: outdated-thread cleanup, head-SHA enforcement for OpenCode reviews, ACTION_REQUIRED handling, and decision contract enhancements.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scripts/ci/pr_review_merge_scheduler.py Outdated
Comment on lines +128 to +130
text = re.sub(r'(?i)(bearer\s+)[^\s"\'\\]+', r'\1***', text)
text = re.sub(r'(?i)(token\s+)[^\s"\'\\]+', r'\1***', text)
text = re.sub(r'(ghp_[A-Za-z0-9_]+|github_pat_[A-Za-z0-9_]+)', '***', text)
reason: str,
) -> Decision:
"""Disable auto-merge and return a disable_auto_merge decision with the concrete unsafe reason."""
"""Disable auto-merge and return a WAIT decision with the concrete unsafe reason."""
@seonghobae
seonghobae force-pushed the codex/sync-pr-governance-scheduler branch from 2825534 to 60e3b11 Compare June 25, 2026 22:06
@seonghobae

Copy link
Copy Markdown
Collaborator Author

Superseded by the organization-level reusable workflow rollout direction. This PR copied the central scheduler into the repository, which would preserve the per-repository drift we are trying to remove. The next change should keep target repositories as thin callers, or use a central scheduler/app path, with the canonical OpenCode/Strix/PR-governance logic owned by ContextualWisdomLab/.github.

@seonghobae seonghobae closed this Jun 25, 2026
@opencode-agent

opencode-agent Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

OpenCode Review Overview

  • Head SHA: 60e3b11c7ba98980ccbdebd1c6357d660ade1e30
  • Workflow run: 28203520862
  • Workflow attempt: 1
  • Gate result: REQUEST_CHANGES (approval step)

OpenCode reviewed the current-head evidence but found unresolved human review threads before approval.

  • Problem: OpenCode reached an APPROVE control result, but the approval step found unresolved, non-outdated human review thread evidence on the current pull request.
  • Root cause: Human review feedback can arrive after bounded model evidence is prepared, so the approval step must re-query GitHub immediately before publishing an approval.
  • Fix: Address or resolve the listed human review thread(s), then re-run OpenCode on the current head.
  • Regression test: Keep the approval gate querying reviewThreads(first: 100) after model output and before create_pull_review APPROVE.

Review thread evidence

Latest unresolved human review thread evidence

scripts/ci/pr_review_merge_scheduler.py line 633

  • Latest human comment: @copilot-pull-request-reviewer at 2026-06-25T22:04:45Z

  • Comment URL: [codex] Sync PR governance scheduler contract #460 (comment)

  • Comment excerpt: The docstring says this returns a "WAIT decision", but the returned Decision.action is "disable_auto_merge" (the contract mapping happens later via contract_decision()). This is easy to misread when working with Decision.action directly.

  • Result: REQUEST_CHANGES

  • Reason: unresolved human review thread(s) were present before approval.

  • Head SHA: 60e3b11c7ba98980ccbdebd1c6357d660ade1e30

  • Workflow run: 28203520862

  • Workflow attempt: 1

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OpenCode reviewed the current-head evidence but found unresolved human review threads before approval.

  • Problem: OpenCode reached an APPROVE control result, but the approval step found unresolved, non-outdated human review thread evidence on the current pull request.
  • Root cause: Human review feedback can arrive after bounded model evidence is prepared, so the approval step must re-query GitHub immediately before publishing an approval.
  • Fix: Address or resolve the listed human review thread(s), then re-run OpenCode on the current head.
  • Regression test: Keep the approval gate querying reviewThreads(first: 100) after model output and before create_pull_review APPROVE.

Review thread evidence

Latest unresolved human review thread evidence

scripts/ci/pr_review_merge_scheduler.py line 633

  • Latest human comment: @copilot-pull-request-reviewer at 2026-06-25T22:04:45Z

  • Comment URL: #460 (comment)

  • Comment excerpt: The docstring says this returns a "WAIT decision", but the returned Decision.action is "disable_auto_merge" (the contract mapping happens later via contract_decision()). This is easy to misread when working with Decision.action directly.

  • Result: REQUEST_CHANGES

  • Reason: unresolved human review thread(s) were present before approval.

  • Head SHA: 60e3b11c7ba98980ccbdebd1c6357d660ade1e30

  • Workflow run: 28203520862

  • Workflow attempt: 1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants