fix(scheduler): require independent exact-head approval - #828
fix(scheduler): require independent exact-head approval#828seonghobae wants to merge 17 commits into
Conversation
|
Warning Review limit reached
Next review available in: 49 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough현재 HEAD에 연결된 독립 승인과 GitHub Changes승인 거버넌스
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@opencode-agent address Repair only the six stale positive scheduler fixtures on exact current Draft head Production approval policy is already the intended security fix and must not be weakened. Strix Changed Path Quality CI run
Update only the semantically authorized positive fixtures used by those paths so they model the real post-repair authorization state: Do not change the default After the test-only repair, rerun the focused independent-approval gate, complete central suite, Strix quick gate, compilation, exact 100% production statement/branch coverage and public-docstring gate, plus all exact-head security/supply-chain checks. Keep Draft; no self-approval, synthetic approval, branch-protection bypass, or temporary/write-capable repair workflow. |
|
@opencode-agent Fix only the current exact-head Strix regression on PR #828 at head The six observed failures to repair are:
Use an explicit small positive-fixture helper only if it makes the intent clearer; it must not affect default/negative fixtures. Before pushing, re-read the live PR head and target blob and abort/re-plan if either moved. Validate the focused independent-approval regression, all affected legacy scheduler tests, full suite, exact 100% production statement/branch coverage and public-docstring gate, Strix quick gate, compilation/syntax, and clean diff. Keep the PR Draft and do not merge or synthesize approval. |
|
@coderabbitai review Review-only on exact current head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
expected_head='c10607aec1f1f23f2125422310101e03de9bd86d'
repo='ContextualWisdomLab/.github'
pr='828'
echo '== Live PR metadata =='
gh pr view "$pr" --repo "$repo" --json number,isDraft,headRefOid,baseRefOid,url \
--jq '{number, isDraft, headRefOid, baseRefOid, url}'
echo '== Checked-out commit =='
git rev-parse HEAD
git status --short
echo '== Exact-head commit verification =='
test "$(git rev-parse HEAD)" = "$expected_head"
test "$(gh pr view "$pr" --repo "$repo" --json headRefOid --jq .headRefOid)" = "$expected_head"
echo 'exact-head verified'
echo '== Changed files at exact head relative to merge base =='
base="$(git merge-base HEAD origin/main)"
git diff --name-status "$base"...HEAD
echo '== Relevant source/test file locations =='
fd -t f -a '(^|/)(pr_review_merge_scheduler|_pr_review_merge_scheduler_core|test_scheduler_independent_approval_gate|test_pr_review_merge_scheduler)\.py$' .
echo '== Python structure maps =='
ast-grep outline scripts/ci/pr_review_merge_scheduler.py --items all || true
ast-grep outline scripts/ci/_pr_review_merge_scheduler_core.py --items all || true
ast-grep outline tests/test_scheduler_independent_approval_gate.py --items all || true
ast-grep outline tests/test_pr_review_merge_scheduler.py --items all || true
echo '== Policy identifiers with context =='
rg -n -C 5 --type py \
'reviewDecision|headRefOid|author|OpenCode|APPROVED|independent|formal.*review|reviews' \
scripts/ci/pr_review_merge_scheduler.py \
scripts/ci/_pr_review_merge_scheduler_core.py \
tests/test_scheduler_independent_approval_gate.py \
tests/test_pr_review_merge_scheduler.py
echo '== Exact-head diff for relevant files =='
git diff --unified=80 "$base"...HEAD -- \
scripts/ci/pr_review_merge_scheduler.py \
scripts/ci/_pr_review_merge_scheduler_core.py \
tests/test_scheduler_independent_approval_gate.py \
tests/test_pr_review_merge_scheduler.pyLength of output: 101369 🏁 Script executed: #!/bin/bash
set -euo pipefail
expected_head='c10607aec1f1f23f2125422310101e03de9bd86d'
test "$(git rev-parse HEAD)" = "$expected_head"
facade='scripts/ci/pr_review_merge_scheduler.py'
core='scripts/ci/_pr_review_merge_scheduler_core.py'
legacy='tests/test_pr_review_merge_scheduler.py'
regression='tests/test_scheduler_independent_approval_gate.py'
echo '== Facade full map and bounded source =='
nl -ba "$facade" | sed -n '1,360p'
echo '== Core approval-related definitions and entrypoint call sites =='
rg -n -C 12 --type py \
'^(def (pull_request_author_login|has_independent_current_head_approval|merge_approval_block_reason|inspect_pr|enable_auto_merge|merge_pr|fetch_pr|fetch_open_prs)|PULL_REQUEST_FIELDS_FRAGMENT)|\bmerge_approval_block_reason\(|\bhas_independent_current_head_approval\(|Decision\(.*"(merge|auto_merge)"' \
"$core"
echo '== Core exact source regions around each relevant definition/call =='
for line in $(rg -n --type py \
'^(def (pull_request_author_login|has_independent_current_head_approval|merge_approval_block_reason|inspect_pr|enable_auto_merge|merge_pr|fetch_pr|fetch_open_prs)|PULL_REQUEST_FIELDS_FRAGMENT)|\bmerge_approval_block_reason\(' \
"$core" | cut -d: -f1 | sort -nu); do
start=$(( line > 25 ? line - 25 : 1 ))
end=$(( line + 100 ))
echo "--- $core:$start-$end ---"
nl -ba "$core" | sed -n "${start},${end}p"
done
echo '== Merge and auto-merge return sites, with enclosing context =='
rg -n -C 20 --type py 'Decision\([^)]*"(merge|auto_merge)"|action="(merge|auto_merge)"' "$core"
echo '== Approval regression test, complete =='
nl -ba "$regression" | sed -n '1,280p'
echo '== Legacy fixture definitions and all modified authorized/negative-policy regions =='
rg -n -C 16 --type py \
'^(def (make_pr|merge_approved_pr|test_body_head_sha_approval_prevents_same_run_opencode_rerun|test_current_head_approval_cleans_previous_head_change_gate_before_merge|test_inspect_pr_blocks_and_waits_for_policy_states|test_inspect_pr_handles_approved_reviews_and_dispatch|test_direct_or_auto_falls_back_to_auto_merge_when_branch_policy_blocks_direct_merge|test_direct_or_auto_attempts_direct_merge_when_mergeability_is_blocked))|reviewDecision|independent-reviewer|pull-request-author|COMMENTED|anonymous|stale|previous-head' \
"$legacy"
echo '== Test-only diff, minimal context =='
base="$(git merge-base HEAD origin/main)"
git diff --unified=12 "$base"...HEAD -- "$legacy" "$regression"Length of output: 2832 🏁 Script executed: #!/bin/bash
set -euo pipefail
expected_head='c10607aec1f1f23f2125422310101e03de9bd86d'
test "$(git rev-parse HEAD)" = "$expected_head"
numbered() { awk '{printf "%6d\t%s\n", NR, $0}' "$1"; }
facade='scripts/ci/pr_review_merge_scheduler.py'
core='scripts/ci/_pr_review_merge_scheduler_core.py'
legacy='tests/test_pr_review_merge_scheduler.py'
regression='tests/test_scheduler_independent_approval_gate.py'
echo '== Facade, complete (expected small policy facade) =='
numbered "$facade"
echo '== Core approval definitions and calls =='
rg -n -C 10 --type py \
'^(def (pull_request_author_login|has_independent_current_head_approval|merge_approval_block_reason|inspect_pr|enable_auto_merge|merge_pr|fetch_pr|fetch_open_prs)|PULL_REQUEST_FIELDS_FRAGMENT)|\bmerge_approval_block_reason\(|\bhas_independent_current_head_approval\(' \
"$core"
echo '== Core exact regions =='
for line in $(rg -n --type py \
'^(def (pull_request_author_login|has_independent_current_head_approval|merge_approval_block_reason|inspect_pr|enable_auto_merge|merge_pr|fetch_pr|fetch_open_prs)|PULL_REQUEST_FIELDS_FRAGMENT)' \
"$core" | cut -d: -f1 | sort -nu); do
start=$(( line > 20 ? line - 20 : 1 ))
end=$(( line + 120 ))
echo "--- $core:$start-$end ---"
sed -n "${start},${end}p" "$core" | awk -v n="$start" '{printf "%6d\t%s\n", n++, $0}'
done
echo '== All core merge/auto-merge decision sites =='
rg -n -C 24 --type py 'Decision\([^)]*"(merge|auto_merge)"|action="(merge|auto_merge)"' "$core"
echo '== Independent-gate regression test =='
numbered "$regression"
echo '== Legacy fixture helpers =='
sed -n '1,115p' "$legacy" | awk '{printf "%6d\t%s\n", NR, $0}'
echo '== Legacy tests named in the request, with sufficient bounded context =='
for name in \
test_body_head_sha_approval_prevents_same_run_opencode_rerun \
test_current_head_approval_cleans_previous_head_change_gate_before_merge \
test_inspect_pr_blocks_and_waits_for_policy_states \
test_inspect_pr_handles_approved_reviews_and_dispatch \
test_direct_or_auto_falls_back_to_auto_merge_when_branch_policy_blocks_direct_merge \
test_direct_or_auto_attempts_direct_merge_when_mergeability_is_blocked; do
line="$(rg -n "^def ${name}\b" "$legacy" | head -1 | cut -d: -f1)"
echo "--- $legacy:$line ($name) ---"
sed -n "${line},$((line + 300))p" "$legacy" | awk -v n="$line" '{printf "%6d\t%s\n", n++, $0}' \
| sed -n '1,/^ *[0-9][0-9]*[[:space:]]def /{ /^ *[0-9][0-9]*[[:space:]]def /!p; }'
done
echo '== Direct negative-policy test cases in legacy suite =='
rg -n -C 8 --type py \
'REVIEW_REQUIRED|CHANGES_REQUESTED|COMMENTED|author.*login|author.*\{\}|author.*None|commit.*old|previous-head|anonymous|independent' \
"$legacy"
echo '== Test-only exact-head diff =='
base="$(git merge-base HEAD origin/main)"
git diff --unified=16 "$base"...HEAD -- "$legacy" "$regression"Length of output: 178950 🏁 Script executed: #!/bin/bash
set -euo pipefail
expected_head='c10607aec1f1f23f2125422310101e03de9bd86d'
repo='ContextualWisdomLab/.github'
test "$(git rev-parse HEAD)" = "$expected_head"
test "$(gh pr view 828 --repo "$repo" --json headRefOid --jq .headRefOid)" = "$expected_head"
core='scripts/ci/_pr_review_merge_scheduler_core.py'
facade='scripts/ci/pr_review_merge_scheduler.py'
echo '== Exact live head remains verified =='
gh pr view 828 --repo "$repo" --json headRefOid,isDraft --jq '{headRefOid,isDraft}'
echo '== Complete core decision segment =='
sed -n '2340,2650p' "$core" | awk -v n=2340 '{printf "%6d\t%s\n", n++, $0}'
echo '== Production direct-core imports, file execution, and scheduler entrypoints =='
rg -n -C 4 \
'_pr_review_merge_scheduler_core|pr_review_merge_scheduler(\.py)?|--self-test|inspect_pr\(' \
--glob '*.py' --glob '*.yml' --glob '*.yaml' --glob '!tests/**' .
echo '== Workflow references to scheduler files =='
rg -n -C 5 \
'scripts/ci/(pr_review_merge_scheduler|_pr_review_merge_scheduler_core)\.py' \
.github
echo '== Core module export or runtime alias changes =='
rg -n -C 4 \
'__all__|sys\.modules|importlib|exec_module|from scripts\.ci\._pr_review_merge_scheduler_core|import scripts\.ci\._pr_review_merge_scheduler_core' \
scripts tests .github
echo '== Final changed file list and production diff summary =='
base="$(git merge-base HEAD origin/main)"
git diff --stat "$base"...HEAD
git diff --name-status "$base"...HEAD
git diff --check "$base"...HEAD
git diff --unified=8 "$base"...HEAD -- "$facade" "$core"Length of output: 108137
I verified live PR The facade delegates scheduler behavior to the authoritative core. It adds the approval boundary without duplicating privileged scheduler implementation. The merge gate fails closed for:
The positive fixtures use a narrow authorized fixture helper. The default fixture remains No branch mutation, approval, merge, release, or repair automation was performed.
|
|
@opencode-agent address Reconcile this Draft with the exact current protected Before writing, refetch and require those head/main/blob identities to remain exact. Merge the protected-main tip into the existing branch with a normal non-forced merge. Resolve After reconciliation, ensure the effective feature diff remains the scheduler approval-policy facade/core, its permanent quality workflow/tests, doctoring, and changelog only. Rerun Scheduler Independent Approval Quality CI, the complete central suite, Strix Changed Path Quality CI, Python compilation, exact 100% production statement/branch and public-docstring gates, plus security/supply-chain checks. Keep Draft until fresh exact-head automated review, zero valid unresolved findings, qualifying independent non-author approval, and branch protection all pass. |
|
@opencode-agent address Reconcile this scheduler separation-of-duties repair onto current protected Use a trusted checkout and normally merge exact protected main into this existing branch. Preserve all newer main behavior and the narrow authorization invariant already implemented on this branch:
The current branch diff already updates positive scheduler fixtures toward Do not force-push, use |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/scheduler-independent-approval-quality-ci.yml:
- Around line 60-65: Update the workflow validation steps after the
scheduler-independent approval gate test to run the complete suite with `python
-m pytest -q` instead of only
`tests/test_scheduler_independent_approval_gate.py`. Replace the `git diff
--exit-code` check with validation that `git status --porcelain=v1
--untracked-files=all` produces no output, while preserving the existing
compileall checks.
In `@CHANGELOG.md`:
- Line 24: Update the changelog entry describing the central merge scheduler
security change to explicitly state that the approving reviewer must also differ
from the pull request’s most recent pusher, in addition to being non-author and
non-OpenCode.
In `@docs/doctoring/scheduler-independent-current-head-approval.md`:
- Around line 13-20: Update the production gate and regression tests to obtain
the authoritative last-pusher identity and require the independent exact-head
approver to be non-empty, different from both the pull-request author and last
pusher, and not the OpenCode reviewer. In
docs/doctoring/scheduler-independent-current-head-approval.md:13-20, add the
last-pusher exclusion to the mandatory fail-closed conditions while preserving
all existing gates. In CHANGELOG.md:24, explicitly record that last pusher
identities are excluded from independent approval.
In `@tests/test_pr_review_merge_scheduler.py`:
- Around line 84-98: Extend the independent-approval gate tests so a
current-head APPROVED review by the PR author, when that same identity is the
last pusher, does not pass as an independent approval and does not merge. Update
the synthetic PR setup around merge_approved_pr in
tests/test_pr_review_merge_scheduler.py:84-98 as needed to model the last-pusher
identity, and add the corresponding negative test in
tests/test_scheduler_independent_approval_gate.py:154-216.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 041ab136-9699-4b81-bedc-3a8f613b8af8
📒 Files selected for processing (8)
.github/workflows/scheduler-independent-approval-quality-ci.ymlCHANGELOG.mddocs/doctoring/scheduler-independent-current-head-approval.mdscripts/ci/_pr_review_merge_scheduler_core.pyscripts/ci/pr_review_merge_scheduler.pytests/test_pr_review_merge_scheduler.pytests/test_required_workflow_queue_contract.pytests/test_scheduler_independent_approval_gate.py
|
@coderabbitai review Review-only on exact current head |
Rate Limit Exceeded
|
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current headea7e80a851f647eae087969232e2e87916b319cd. -
Head SHA:
ea7e80a851f647eae087969232e2e87916b319cd -
Workflow run: 31322615000
-
Workflow attempt: 1
Coverage evidence
Coverage Decision
- Result: FAIL
- Test evidence: not proven passing
- Docstring evidence: not proven passing when configured
- Failure count: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow: scheduler-independent-approval-quality-ci.yml"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow: scheduler-independent-approval-quality-ci.yml"]
R1 --> V1["actionlint plus required checks"]
Evidence --> S2["Changed file: CHANGELOG.md"]
S2 --> I2["repository behavior"]
I2 --> R2["Review risk: Changed file: CHANGELOG.md"]
R2 --> V2["required checks"]
Evidence --> S3["Docs: scheduler-independent-current-head-approval.md"]
S3 --> I3["operator or user guidance"]
I3 --> R3["Review risk: Docs: scheduler-independent-current-head-approval.md"]
R3 --> V3["docs review"]
Evidence --> S4["CI script (2 files)"]
S4 --> I4["review and security gate shell path"]
I4 --> R4["Review risk: CI script (2 files)"]
R4 --> V4["bash -n plus Strix self-test"]
Evidence --> S5["Test (3 files)"]
S5 --> I5["regression suite"]
I5 --> R5["Review risk: Test (3 files)"]
R5 --> V5["targeted test run"]
OpenCode Review Overview
Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow: scheduler-independent-approval-quality-ci.yml"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow: scheduler-independent-approval-quality-ci.yml"]
R1 --> V1["actionlint plus required checks"]
Evidence --> S2["Changed file: CHANGELOG.md"]
S2 --> I2["repository behavior"]
I2 --> R2["Review risk: Changed file: CHANGELOG.md"]
R2 --> V2["required checks"]
Evidence --> S3["Docs: scheduler-independent-current-head-approval.md"]
S3 --> I3["operator or user guidance"]
I3 --> R3["Review risk: Docs: scheduler-independent-current-head-approval.md"]
R3 --> V3["docs review"]
Evidence --> S4["CI script (2 files)"]
S4 --> I4["review and security gate shell path"]
I4 --> R4["Review risk: CI script (2 files)"]
R4 --> V4["bash -n plus Strix self-test"]
Evidence --> S5["Test (3 files)"]
S5 --> I5["regression suite"]
I5 --> R5["Review risk: Test (3 files)"]
R5 --> V5["targeted test run"]
|
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current headea7e80a851f647eae087969232e2e87916b319cd. -
Head SHA:
ea7e80a851f647eae087969232e2e87916b319cd -
Workflow run: 31323315093
-
Workflow attempt: 1
Coverage evidence
Coverage Decision
- Result: FAIL
- Test evidence: not proven passing
- Docstring evidence: not proven passing when configured
- Failure count: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow: scheduler-independent-approval-quality-ci.yml"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow: scheduler-independent-approval-quality-ci.yml"]
R1 --> V1["actionlint plus required checks"]
Evidence --> S2["Changed file: CHANGELOG.md"]
S2 --> I2["repository behavior"]
I2 --> R2["Review risk: Changed file: CHANGELOG.md"]
R2 --> V2["required checks"]
Evidence --> S3["Docs: scheduler-independent-current-head-approval.md"]
S3 --> I3["operator or user guidance"]
I3 --> R3["Review risk: Docs: scheduler-independent-current-head-approval.md"]
R3 --> V3["docs review"]
Evidence --> S4["CI script (2 files)"]
S4 --> I4["review and security gate shell path"]
I4 --> R4["Review risk: CI script (2 files)"]
R4 --> V4["bash -n plus Strix self-test"]
Evidence --> S5["Test (3 files)"]
S5 --> I5["regression suite"]
I5 --> R5["Review risk: Test (3 files)"]
R5 --> V5["targeted test run"]
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_scheduler_independent_approval_gate.py`:
- Around line 126-129: Strengthen the workflow assertions in the relevant test
so the run block contains the exact path-free command `python -m pytest -q`,
rather than accepting it as a substring of a path-specific command. Also verify
that the `git status --porcelain=v1 --untracked-files=all` check exits non-zero
when staged or untracked changes exist, and run the complete test suite before
merging.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 17236b07-0151-4cf4-acaa-a587f04c73b9
📒 Files selected for processing (2)
.github/workflows/scheduler-independent-approval-quality-ci.ymltests/test_scheduler_independent_approval_gate.py
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/scheduler-independent-approval-quality-ci.yml
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head bounded evidence and found no blocking issues.
Findings
No blocking findings.
Summary
Approval sufficiency: PR intent (the central merge scheduler must fail closed unless GitHub reports APPROVED and a formal non-author, non-OpenCode approval is bound to the exact current head, per issue #771) is implemented end-to-end and inspected in changed files scripts/ci/pr_review_merge_scheduler.py (facade, line 222 area), scripts/ci/_pr_review_merge_scheduler_core.py (3803-line extracted engine), tests/test_scheduler_independent_approval_gate.py, .github/workflows/scheduler-independent-approval-quality-ci.yml, docs/doctoring/scheduler-independent-current-head-approval.md, CHANGELOG.md, tests/test_pr_review_merge_scheduler.py, and tests/test_required_workflow_queue_contract.py. Verification posture: Coverage execution evidence Result: PASS (supported repository test suites passed) and docstring gates passed/advisory at head 33b84e2; Failed GitHub Check evidence reports no completed failed checks; Other unresolved review thread evidence reports no unresolved non-outdated threads. Linter/static: the new workflow runs python -m compileall -q on the facade, core, and gate test, and no linter findings are recorded on the current head. TDD/regression: the permanent suite covers GitHub REVIEW_REQUIRED blocking, APPROVED without exact-head independent review blocking, predecessor-head approval blocking, author self-approval blocking, missing author identity blocking, missing reviewer identity blocking, non-APPROVED independent review blocking, the preserved normal merge path with exact-head OpenCode + non-author independent approval, auto-merge disarm on approval loss, and workflow tracking of the core module. Coverage: Coverage execution evidence reports supported repository test suites passed (Result: PASS) per the bounded evidence, satisfying the repository coverage command contract. Docstring coverage: configured repository docstring gates passed or were advisory per the same Coverage execution evidence. DAG: CodeGraph blast-radius trace of the head flow shows PR GraphQL/REST envelope extended with author.login -> facade independent-approval gate in scripts/ci/pr_review_merge_scheduler.py -> core engine scripts/ci/_pr_review_merge_scheduler_core.py with merge_pr/auto-merge entrypoints mechanically disabled on gate failure or the normal expected-head merge path on success; the quality workflow tracks and compiles both modules. PoC/execution: no dedicated execution receipts beyond the precomputed Coverage/check evidence; the workflow asserts test "$(git rev-parse HEAD)" equals github.event.pull_request.head.sha and a clean worktree, failing closed on divergence. DDD/domain: the approval-policy facade / scheduler-core split preserves the domain boundary and delegates the established check, cleanup, branch-update, review-dispatch, conflict, and expected-head machinery without duplicating privileged implementation. CDD/context: the docs/doctoring decision record matches the implementation and the #771 threat model, including ruleset bypass actors and GitHub review semantics. Similar issues: the two historical OpenCode REQUEST_CHANGES reviews (older head ea7e80a, coverage evidence) and CodeRabbit comments are stale historical context superseded by the current-head Coverage PASS and no-unresolved-threads evidence. Claim/concept check: fail-closed claims (stale/dismissed/comment-only/predecessor-head/author/OpenCode-only approvals never satisfy the gate; missing identity yields WAIT) are backed by the documented test-first list and fixtures. Standards search: GitHub protected-branch/ruleset approval semantics cited in the decision record are consistent with the repository automation posture; no external standard contradiction found in evidence. Compatibility/convention: all new identifiers (workflow file, job exact-head-independent-approval, core module, gate test file) are multi-word and idiomatic (kebab/snake_case); no sequential-id, reserved-word, or client-visible enumeration surface is introduced. Breaking-change/backcompat: scheduler behavior is strictly tightened (fail-closed), the REST fallback records author identity from the PR user field, and the new quality workflow adds exact-head coverage without weakening existing gates. Performance: concurrency cancel-in-progress and a 10-minute timeout bound the new job; hash-pinned dependencies avoid supply-chain drift. Developer experience: the path-filtered quality workflow gives authors immediate exact-head feedback on scheduler changes with a clean-worktree guarantee. User experience: non-web governance surface; the interaction surface reviewed is workflow/CLI output, review-comment evidence, and the decision-record documentation. Visual/DOM: not a web change; the non-web interaction surface (workflow output, review comments, docs) was reviewed instead. Accessibility/i18n: no UI changes; English-language docs and changelog are consistent with the PR language. Supply-chain/license: actions/checkout and actions/setup-python are pinned to immutable commit SHAs, persist-credentials is false, and pip install uses --require-hashes and --only-binary=:all: with pinned hashes for all five test-runner packages. Packaging: no new package manifests; the pyproject.toml Python contract (>=3.10) is unchanged. Security/privacy: separation-of-duties gate requires GitHub APPROVED plus a non-author, non-OpenCode formal exact-head approval; missing identities fail to WAIT, stale auto-merge state is disarmed, workflow permissions are contents: read only, and no secrets are consumed by the new job.
Approval sufficiency: bounded evidence supplied affirmative approval evidence for changed files, coverage/docstring posture, risk surfaces, and current-head verification; approval is not based merely on the absence of known blockers.
Verification posture: CodeGraph evidence was initialized and bounded current-head evidence reviewed for changed-file evidence including .github/workflows/scheduler-independent-approval-quality-ci.yml, CHANGELOG.md, docs/doctoring/scheduler-independent-current-head-approval.md, scripts/ci/_pr_review_merge_scheduler_core.py, scripts/ci/pr_review_merge_scheduler.py, and 3 more.
Linter/static: workflow/static review evidence is bounded by the current-head GitHub Checks gate and changed-file evidence.
TDD/regression: coverage execution evidence and focused changed hunks were reviewed from bounded-review-evidence.md.
Coverage: coverage execution evidence reports supported repository test suites passed.
Docstring coverage: coverage execution evidence reports configured repository docstring gates passed or docstring coverage was advisory.
DAG: CodeGraph/source-backed behavior map connects .github/workflows/scheduler-independent-approval-quality-ci.yml to the affected review, runtime, or workflow path and required checks.
PoC/execution: coverage-evidence job executed on the current head and reported PASS.
DDD/domain: workflow and repository-governance invariants were reviewed against changed files in bounded evidence.
CDD/context: CodeGraph evidence, changed-file history, and focused hunks were reviewed from bounded-review-evidence.md.
Similar issues: changed-file history evidence was reviewed for comparable local precedents.
Claim/concept check: bounded evidence, repository source, current-head workflow evidence, and, where numeric, scientific, statistical, or literature-backed claims are affected, original-paper/formula evidence and parameter-recovery expectations were used for claims.
Standards search: standards and external-source claims require trusted bounded source evidence prepared outside the isolated model process; no evidence-backed standards blocker is present in bounded evidence.
Compatibility/convention: changed workflow/script conventions, object naming, and reserved-word safety for schema/API/config/code surfaces were checked in bounded evidence.
Breaking-change/backcompat: deployment evidence and changed-file history were checked for backward-compatibility risk.
Performance: changed surfaces were checked for performance risk in bounded evidence.
Developer experience: changed automation, review, test, setup, and maintenance surfaces were checked for helpful or obstructive DX impact in bounded evidence.
User experience: connected user, operator, API, CLI, documentation, review-comment, status-check, rendering, and workflow-reader behavior was checked for contradictions against code, docs, and tests in bounded evidence.
Visual/DOM: deterministic repair does not infer browser runtime execution; source-backed DOM/UI evidence and trusted workflow receipts were reviewed when present, and non-web surfaces used API/CLI/log/docs/workflow evidence instead.
Accessibility/i18n: accessibility, localization, and human-readable text surfaces were checked where UI, CLI, API message, docs, logs, or review text changed.
Supply-chain/license: dependency, package, model, container, and external-tool changes were checked in bounded evidence.
Packaging: package, build, test, lint, and security contracts were checked in bounded evidence.
Security/privacy: workflow-token, review-gate, and repository-automation security/privacy boundaries were checked in bounded evidence.
Adversarial validation
{"status":"passed","probes":[{"path":"tests/test_scheduler_independent_approval_gate.py","line":242,"hypothesis":"An exact-head APPROVED review issued by the pull-request author or by the OpenCode automation itself could satisfy the new independent-approval gate, allowing a merge with no independent human reviewer.","attack_or_counterexample":"PR fixture with reviewDecision=APPROVED where the only APPROVED review node is author self-approval or an opencode-agent approval, both bound to HEAD_SHA (fixtures review()/make_pr() in the CodeGraph source trace of tests/test_scheduler_independent_approval_gate.py).","evidence":"Coverage execution evidence reports Result: PASS with supported repository test suites passed on head 33b84e20c97fc30b367bbdbe92c888c51e5aa0ff; the test-first list in docs/doctoring/scheduler-independent-current-head-approval.md:1 enumerates author self-approval, OpenCode-only review, and missing reviewer identity as blocking cases and the gate suite passed, so the self/OpenCode-only approvals were rejected at tests/test_scheduler_independent_approval_gate.py:242. source-line-sha256=86bead13cf27e8f48de1d52970a3ea51d33c78a61d16b6d56cbc2ce2737b864d","outcome":"falsified"},{"path":".github/workflows/scheduler-independent-approval-quality-ci.yml","line":65,"hypothesis":"The new quality workflow could silently measure GitHub's generated pull-request merge revision instead of the literal PR head, making the exact-head quality and compilation claims invalid.","attack_or_counterexample":"Event where github.event.pull_request.head.sha differs from the merge ref; an unpinned checkout ref would bind the job to the merge revision and the head-equality assertion would diverge.","evidence":"Focused hunk .github/workflows/scheduler-independent-approval-quality-ci.yml:65 pins checkout to ref: ${{ github.event.pull_request.head.sha || github.sha }} with persist-credentials: false, and the prove step asserts test \"$(git rev-parse HEAD)\" equals that SHA plus test -z \"$(git status --porcelain=v1 --untracked-files=all)\", failing the job on any divergence; Failed GitHub Check evidence records no completed failed checks for head 33b84e2 and Coverage execution evidence reports the supported suites passed, so the exact-head assertion held. source-line-sha256=a6cdd8d7d598970ff5bf3cd226825e6a537581c2f72974c12c0b4f2a6668cf86","outcome":"falsified"},{"path":"scripts/ci/pr_review_merge_scheduler.py","line":222,"hypothesis":"A predecessor-head or stale APPROVED review could satisfy the fail-closed gate and authorize merging the current head, undoing the separation-of-duties repair for issue #771.","attack_or_counterexample":"PR with reviewDecision=APPROVED whose only independent APPROVED review node is bound to BASE_SHA while headRefOid is the current HEAD_SHA; the review/head-evidence rules must reject that binding.","evidence":"Coverage execution evidence reports Result: PASS with supported repository test suites passed on head 33b84e2; the decision record docs/doctoring/scheduler-independent-current-head-approval.md:74 and the test-first list state stale, dismissed, and predecessor-head approvals never satisfy the gate, and the suite's predecessor-head blocking fixtures passed, so the stale-approval merge hypothesis was rejected at scripts/ci/pr_review_merge_scheduler.py:222. source-line-sha256=aad65802b279f6a1d469e8b9296f90c402a69ef57f5dc820f7c69ddd166e3089","outcome":"falsified"}],"residual_risk":"External enforcement boundaries remain outside the scheduler: GitHub ruleset configuration, hosted token scope, and branch-protection bypass actors are complemented, not replaced, by this application-level gate. The extracted 3803-line core engine is validated by the passing suite, compileall, and the new exact-head workflow rather than by exhaustive per-line manual audit. The new workflow runs the full repository suite inside a 10-minute budget with only the five hash-pinned test-runner packages, which bounds iteration speed if the suite grows or needs additional third-party test dependencies."}- Result: APPROVE
- Reason: The fail-closed independent exact-head approval gate is fully implemented across the scheduler facade (scripts/ci/pr_review_merge_scheduler.py) and core (scripts/ci/_pr_review_merge_scheduler_core.py), covered by the new regression suite (tests/test_scheduler_independent_approval_gate.py), locked in by the new hash-pinned exact-head quality workflow (.github/workflows/scheduler-independent-approval-quality-ci.yml), and documented consistently (docs/doctoring/scheduler-independent-current-head-approval.md, CHANGELOG.md). Coverage execution evidence reports PASS with supported repository test suites passed and docstring gates passed/advisory; no failed GitHub Checks and no unresolved review threads exist at head 33b84e2. All three adversarial probes (author/OpenCode-only approval bypass, wrong-revision quality measurement, predecessor-head stale approval) were falsified by the passing gate suite and the workflow's exact-head assertions.
- Head SHA:
33b84e20c97fc30b367bbdbe92c888c51e5aa0ff - Workflow run: 31455074212
- Workflow attempt: 1
Safety gap
The organization merge scheduler could previously enter direct-merge/auto-merge paths from a same-head OpenCode
APPROVEDreview even when GitHub still reportedreviewDecision: REVIEW_REQUIRED. That lets automation policy run ahead of the repository's independent-review gate and is unsafe if a routine scheduler credential ever acquires a ruleset bypass path.Test-first repair
Exact RED head
c7011f23356eee92c3569709a893790a6b0893c4added onlytests/test_scheduler_independent_approval_gate.pyfrom protectedmain1131b1bbafb24e455fc8619cdf316813e8721861.The production repair now requires, before merge/auto-merge entrypoints:
reviewDecision == APPROVED;APPROVEDreview from a non-author, non-OpenCode identity; andStale, predecessor-head, dismissed, comment-only, anonymous, author-self, OpenCode-only, status-only, or synthetic-merge evidence remains non-authorizing. No approval is synthesized and no branch/ruleset protection is weakened.
The mature scheduler engine remains in
scripts/ci/_pr_review_merge_scheduler_core.py;scripts/ci/pr_review_merge_scheduler.pyis the auditable approval-policy facade. This is permanent reviewed source structure, not a temporary repair workflow.Exact-current-head state
Authoritative head:
8abddc8df9ebddcb14e40e818593e71e1c4ca40a.Protected base:
mainat1131b1bbafb24e455fc8619cdf316813e8721861.The current head already repaired the stale source-layout contract so
test_privileged_review_retries_use_default_branch_repository_dispatchfollows the authoritative scheduler core rather than requiring duplicated production literals in the facade.Current exact-head workflow evidence:
31229459086: failed.That Strix quality run checked out exact head
8abddc8d...and completed the central suite with 978 passed, 16 subtests passed, 6 failed. All six failures are stale positive merge-path fixtures intests/test_pr_review_merge_scheduler.py: they still modelreviewDecision: REVIEW_REQUIREDand/or OpenCode-only approval while expecting merge/auto-merge behavior.The remaining test repair must update only those positive fixtures to model the real post-repair authorization state:
reviewDecision: APPROVED, authoritative pull-request author identity, exact-head OpenCode approval, and an exact-head formal approval from a different non-OpenCode reviewer. Negative review-policy fixtures must remain fail-closed. Do not weaken production logic or globally inject approvals into the legacy helper merely to make the suite green.The authoritative legacy test blob observed at this head is
3e421e9039654aa669bf147db3be63fd1386d461. Any head/base/blob movement invalidates this repair plan and requires a fresh read before mutation.Required next validation
After the six fixtures are corrected, rerun the focused independent-approval regression, complete central suite, Strix quick gate, Python compilation, exact 100% production statement/branch coverage and public-docstring coverage, then regenerate every exact-head security/supply-chain and automated-review surface. Keep Draft until zero valid unresolved findings, qualifying independent non-author approval, repository policy, and branch protection all pass without bypass.
Documentation and governance boundary
docs/doctoring/scheduler-independent-current-head-approval.mdandCHANGELOG.mdrecord the incident boundary, exact-head approval semantics, rollback, non-claims, and current GitHub protected-branch/ruleset references. This PR closes #771 only after protected merge; it does not itself solve the organization-wide counted-human-review availability gap tracked separately in #772.Summary by CodeRabbit
새 기능
문서
품질 개선