fix(scheduler): require approved aggregate review state - #937
fix(scheduler): require approved aggregate review state#937seonghobae wants to merge 22 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough체크런에 현재 head SHA를 연결하고, 현재 head 체크만 병합 증거로 사용합니다. Changes병합 검증과 Strix 게이트
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant GitHub
participant StrixWorkflow
participant EvidenceBinding
participant inspect_pr
participant MergeScheduler
GitHub->>StrixWorkflow: 현재 PR head에서 Strix 실행
StrixWorkflow->>EvidenceBinding: 완료된 run.json과 보고서 검증
EvidenceBinding->>inspect_pr: head SHA와 evidence provenance 제공
inspect_pr->>GitHub: aggregate reviewDecision 및 CheckRun 조회
inspect_pr->>MergeScheduler: 병합 가능 여부 결정
MergeScheduler->>GitHub: 병합 또는 auto-merge 비활성화
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
|
Audit evidence for this scheduler fix: fast-mlsirm PR #742 merged as |
|
Trusted-review evidence update: repository_dispatch run 31514989573 targeted exact head dd8f59d and failed closed. The run recorded COVERAGE_EVIDENCE_RESULT=failure, an empty model-pool outcome, and OPENCODE_REVIEW_IDENTITY_UNAVAILABLE; no formal review was published. I will not substitute a GitHub Actions/PAT identity or treat the opencode-review check as approval. The linked contextual ADR records this as a required follow-up; merge remains held until authorized App identity, coverage/model evidence, structured same-head review, aggregate APPROVED, and all required checks are present. |
|
Follow-up pushed at exact head 12e3d1f: added regression coverage for the real fast-mlsirm incident shape |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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_pr_review_merge_scheduler.py`:
- Around line 4145-4171: Extend
test_inspect_pr_requires_approved_aggregate_review to cover a PR whose
reviewDecision key is absent, distinct from the existing None case. Create the
PR with make_pr, remove the reviewDecision key using pop, and assert that
inspect returns block without auto-merge and disable_auto_merge with auto-merge
enabled, using the expected MISSING reason in both cases.
🪄 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: 03fb4e34-6c75-47a2-957e-3037e2ddf933
📒 Files selected for processing (2)
scripts/ci/pr_review_merge_scheduler.pytests/test_pr_review_merge_scheduler.py
|
Status follow-up for exact head 12e3d1f: some checks have completed, but required child jobs remain queued and Strix is still in progress; no formal authorized approval is present. Treat queued, in-progress, missing, or unreported evidence as non-passing. Do not merge or enable auto-merge until the exact head has complete required checks, structured same-head Strix evidence, an authorized independent review with aggregate approval, and a final re-fetch immediately before mutation. The OpenCode identity/coverage/model-pool failure recorded earlier remains unresolved. |
|
Exact-head update: 2756cd3 adds the pre-merge guard that blocks an approved head while any status context is queued/in progress and blocks missing same-head Strix evidence, with regression tests. Local scheduler suite: 115 passed, self-test passed; full suite: 964 passed and 18 environment/provider-dependent failures already tracked in the linked ADR. This push is not merge evidence: obtain an authorized independent review, complete exact-head required CI and structured Strix, then re-fetch immediately before any merge mutation. GitHub also reported 5 default-branch vulnerabilities on push; do not dismiss them until Dependabot graph reconciliation is complete. |
|
Exact-head update: 5856151 removes the Strix workflow's backend-outage neutral-pass branch. A provider outage or missing/no-report result now exits the required check nonzero and publishes failure/inconclusive status; scheduler tests and workflow contracts enforce this fail-closed boundary. Earlier CI/review/Strix evidence is stale again. Do not merge until this full SHA has authorized independent review, complete required checks, structured same-head Strix evidence, aggregate approval, and a final re-fetch. |
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 `@scripts/ci/pr_review_merge_scheduler.py`:
- Around line 1117-1121: Update strix_evidence_state handling in
scripts/ci/pr_review_merge_scheduler.py at lines 1117-1121 so a current-head
Strix CheckRun is treated as completed evidence only when status is COMPLETED
and conclusion is SUCCESS; classify all other conclusions, including None,
NEUTRAL, and SKIPPED, as blocking. Add regression coverage in
tests/test_pr_review_merge_scheduler.py at lines 4220-4252 verifying those
conclusions return block and disable auto-merge.
🪄 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: 09fb5a04-f4af-4c5c-be43-d51cf01be291
📒 Files selected for processing (7)
.github/workflows/strix.ymlscripts/ci/pr_review_merge_scheduler.pyscripts/ci/strix_required_workflow_smoke.shscripts/ci/test_strix_quick_gate.shtests/test_pr_review_merge_scheduler.pytests/test_required_workflow_queue_contract.pytests/test_strix_nvidia_nim_not_found_fallback.py
💤 Files with no reviewable changes (1)
- scripts/ci/strix_required_workflow_smoke.sh
|
Exact-head status update: current head is |
|
Exact-head status update: current head is |
|
Exact-head update: current head is |
|
Exact-head external-status note: current head remains |
|
Exact-head remediation update for 7750253:
|
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 `@scripts/ci/portable_timeout.py`:
- Around line 17-40: Add concise docstrings to _seconds, _signal_process_group,
_terminate, and main documenting their inputs, return or exit status, and signal
behavior. Keep the existing implementations unchanged, and ensure the new
scripts/ci helper code satisfies the required docstring coverage.
In `@scripts/ci/run_opencode_review_model_pool.sh`:
- Line 490: Update the background execution around run_with_timeout so $!
captures the actual timeout launcher PID rather than the shell subshell created
for the function. Separate the launcher path for timeout, gtimeout, or
portable_timeout.py, and update the cleanup at the opencode_pid termination
point to terminate the launcher's entire process group, ensuring child opencode
processes also stop.
In `@tests/test_pr_review_merge_scheduler.py`:
- Around line 1675-1705: Update the equal_timestamp_duplicates fixture so both
CheckRun nodes have the same startedAt value, while keeping the CANCELLED
CheckRun as the second node. Preserve the existing assertion verifying that
sched.failed_status_checks selects “scan-pr-queue”.
In `@tests/test_strix_workflow_dependency_hashes.py`:
- Around line 44-52: Update the docstring of
test_strix_requirement_locks_keep_dependabot_patch_floors to describe the actual
exact-pin contract: each Strix lock must pin every patched dependency exactly
once to its specified version, rather than merely meeting a minimum
patched-version floor.
🪄 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: 74c7863f-cca4-4608-8460-d718be7b3655
📒 Files selected for processing (11)
.github/workflows/strix.ymldocs/doctoring/strix-nvidia-nim-not-found-fallback.mddocs/doctoring/strix-quality-timeout-fixtures.mdscripts/ci/portable_timeout.pyscripts/ci/run_opencode_review_model_pool.shscripts/ci/test_strix_quick_gate.shtests/test_materialize_base_python_requirements.pytests/test_opencode_agent_contract.pytests/test_portable_timeout.pytests/test_pr_review_merge_scheduler.pytests/test_strix_workflow_dependency_hashes.py
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/strix.yml
- scripts/ci/test_strix_quick_gate.sh
|
Exact-head protection follow-up (2026-08-12 Asia/Seoul):
No merge is authorized until every required check concludes successfully on this exact SHA, aggregate reviewDecision is APPROVED, an independent current-head approval exists, structured same-head Strix evidence is present, and the final protection/check/review re-fetch passes. No self-approval, bypass, or force push. |
|
Exact-head status for central .github PR #937: current head 249ba9864eaa2309f40f947ba61fd1d126d31af. The six CodeRabbit findings from the previous central head were fixed; replies were posted and all review threads are now resolved. Local evidence: 991 tests and 16 subtests passed, 100 percent statement and branch coverage, Strix required-workflow smoke passed, Python compile and Bash syntax passed. Branch protection remains strict with one required approval, enforce_admins true, force pushes/deletions false. Remote exact-head scan-pr-queue and strix are still queued, other required checks remain pending, aggregate reviewDecision is REVIEW_REQUIRED, and no independent authorized approval exists. No merge or auto-merge will be attempted; re-fetch exact head immediately before any future mutation. |
|
@opencode-agent review Perform a fresh read-only semantic review of exact current head Focus on aggregate |
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 @.github/workflows/strix-workflow-contract.yml:
- Around line 33-39: The Strix workflow contract currently trusts marker strings
and must instead validate the complete workflow’s immutable digest or a
structured policy that proves the security gate is reachable and fail-closed.
Update `.github/workflows/strix-workflow-contract.yml` lines 33-39 accordingly,
and add an adversarial regression test in `scripts/ci/test_strix_quick_gate.sh`
lines 180-186 that rejects required markers appearing only in comments or
unreachable blocks.
🪄 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: 4e962388-269c-4d2f-a3ae-514cda9ff330
📒 Files selected for processing (7)
.github/workflows/strix-workflow-contract.yml.github/workflows/strix.ymldocs/doctoring/strix-workflow-version-evidence.mddocs/org-required-workflow-rollout.mdscripts/ci/run_opencode_review_model_pool.shscripts/ci/test_strix_quick_gate.shtests/test_opencode_model_pool_runner.py
|
@opencode-agent review the exact current head 8726df1. Inspect the separate data-only Strix workflow contract, base-workflow versus PR-head evidence boundary, provenance validation, fail-closed provider handling, and changed docs/tests. Report actionable findings only; do not treat a check, comment, or status-only result as approval. |
|
@coderabbitai review |
|
|
@cwl-noema-review |
OpenCode Review Overview
Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow (2 files)"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow (2 files)"]
R1 --> V1["actionlint plus required checks"]
Evidence --> S2["Docs (4 files)"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs (4 files)"]
R2 --> V2["docs review"]
Evidence --> S3["CI script (6 files)"]
S3 --> I3["review and security gate shell path"]
I3 --> R3["Review risk: CI script (6 files)"]
R3 --> V3["bash -n plus Strix self-test"]
Evidence --> S4["Test (8 files)"]
S4 --> I4["regression suite"]
I4 --> R4["Review risk: Test (8 files)"]
R4 --> V4["targeted test run"]
|
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
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: APPROVE; positive evidence at head 67d834f supports PR intent: .github/workflows/strix.yml fails closed (neutral-skip branch removed; ::error title=Strix evidence incomplete + exit $strix_rc for every non-zero gate exit) and writes PR_HEAD_SHA to $RUNNER_TEMP/strix_scan_head_sha; the new .github/workflows/strix-workflow-contract.yml enforces that policy per PR; pr_review_merge_scheduler.py aggregate reviewDecision APPROVED gating is backed by tests/test_pr_review_merge_scheduler.py (+370 lines). Changed-file evidence inspected: .github/workflows/strix-workflow-contract.yml (full new-file diff), .github/workflows/strix.yml (gate hunk), tests/test_portable_timeout.py (verbatim CodeGraph source). Verification posture: Failed GitHub Check evidence lists no completed failed checks at head; Other unresolved review thread evidence is empty; Coverage execution evidence PASS. Linter/static: lint_commands cover only hadolint on an untouched Dockerfile; bash -n/actionlint appear only in historical comments, not current-head receipts. TDD/regression: tests assert exact exit codes 2/124/127/143, completion passthrough (status 0, stdout ok), duration validation, signal forwarding, and the fail-closed workflow policy; scripts/ci/test_strix_quick_gate.sh extended (+164). Coverage: Coverage execution evidence Result PASS - supported repository test suites passed (coverage_commands require --fail-under=100). Docstring coverage: Coverage execution evidence - configured repository docstring gates passed or advisory (interrogate --fail-under=100). DAG: Mermaid flowchart (head flow) in the body maps strix.yml gate step -> strix_quick_gate.sh exit -> fail-closed ::error exit -> report collect/validate/upload with head binding -> pr_review_merge_scheduler.py aggregate APPROVED gate -> merge decision, with contract-workflow and test-verification edges. PoC/execution: no OPENCODE_EXECUTION_RECEIPT lines in bounded evidence; non-web repo, no browser E2E applies; behavioral evidence is Coverage PASS plus verbatim source/diff traces. DDD/domain: scheduler merge-gate domain consistent with intent - aggregate reviewDecision APPROVED bound to current-head evidence; queued auto-merge disabled on REVIEW_REQUIRED/CHANGES_REQUESTED; no domain-model drift observed. CDD/context: workflow contract codifies the fail-closed Strix policy at the PR/head boundary. Similar issues: no similar-issue or PR-precedent evidence supplied. Claim/concept check: PR body claims (block merge without aggregate APPROVED; fail closed on missing security evidence; preserve review/security dispatch pre-merge-readiness) match diff, tests, and docs updated in-tree (docs/doctoring/strix-workflow-version-evidence.md, docs/org-required-workflow-rollout.md). Standards search: no external standards material in bounded evidence; workflow runtimes follow workflow_versions evidence (strix.yml python 3.13); no Node 24/Python 3.14 rollback issue raised. Compatibility/convention: new identifiers are multi-word idiomatic names (portable_timeout.py, strix-workflow-contract.yml, workflow-contract job, strix_scan_head_sha, _signal_process_group); no single-word/reserved identifiers and no new DB/API objects. Breaking-change/backcompat: fail-closed required-check semantics is the intended contract change documented in docs/org-required-workflow-rollout.md; portable_timeout preserves GNU-timeout CLI semantics (duration parsing, exit 124). Implementation completeness: no placeholder bodies; gate/collect/validate/upload steps and portable_timeout main/_terminate/_signal_process_group are concrete and tested. Performance: contract workflow adds one read-only gh api contents call per PR event; negligible. Developer experience: CI authoring surface - self-validating workflow contract, portable timeout for macOS/Linux parity, and extended smoke suite reduce cross-platform drift and silent-evidence regressions. User experience: non-web interaction surface reviewed - required-check annotations now fail closed (::error title=Strix evidence incomplete) and the scheduler surfaces aggregate reviewDecision state; no web UI. Visual/DOM: no web UI changes; reviewed CLI/workflow/check-annotation output surface instead (web_app_review_requirements empty). Accessibility/i18n: not applicable to CI scripts; docs remain English. Supply-chain/license: no new dependencies (portable_timeout.py stdlib-only; Ruby psych preinstalled on ubuntu-latest); Strix wheel-hash and dependency-floor contracts extended in tests/test_strix_workflow_dependency_hashes.py. Packaging: pyproject.toml pytest/coverage/interrogate contracts present; no unpackaged_source_surfaces flagged. Security/privacy: fail-closed gate removes the missing-evidence neutral-pass downgrade; contract regex-validates HEAD_REPOSITORY and HEAD_SHA before gh api interpolation, uses only contents: read + github.token, and never executes decoded content; Nvidia_nimException marker retained for the trusted smoke contract; no new secrets or sequential-id surfaces.
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/strix-workflow-contract.yml, .github/workflows/strix.yml, docs/doctoring/strix-nvidia-nim-not-found-fallback.md, docs/doctoring/strix-quality-timeout-fixtures.md, docs/doctoring/strix-workflow-version-evidence.md, and 15 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/strix-workflow-contract.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":".github/workflows/strix.yml","line":837,"hypothesis":"The head Strix gate still passes missing security evidence through a backend-unavailability neutral path, so a provider outage yields a green required check.","attack_or_counterexample":"Gate exits 1 with backend-unavailable signals (RateLimitError, RESOURCE_EXHAUSTED, Nvidia_nimException 404) and no vulnerability report - the exact scenario the old code converted to a neutral exit 0.","evidence":"Trusted diff trace at .github/workflows/strix.yml:837: the head hunk deletes the backend_unavailable_signal/reported_vulnerability_signal classification and the neutral exit 0 skip; every non-zero gate exit now reaches echo ::error title=Strix evidence incomplete and exit \"$strix_rc\", so rc=1 backend outages and rc=2 config errors both fail the required check, and PR_HEAD_SHA is bound to the scan via $RUNNER_TEMP/strix_scan_head_sha. Outcome: the neutral-pass path is absent at head; hypothesis falsified. source-line-sha256=f74d540a16a223f5d8744e674937386b56b2f68cfe4717782ca16b13a3c58959","outcome":"falsified"},{"path":"tests/test_portable_timeout.py","line":163,"hypothesis":"portable_timeout.py returns a fabricated or inverted child status, so a timed-out step could pass or a completed step could fail the CI step.","attack_or_counterexample":"Child that finishes instantly (status 0) versus child that exceeds the deadline (kill -> 124); SIGTERM-forward handler; malformed duration/argument input.","evidence":"Verbatim current-head test source at tests/test_portable_timeout.py:163 asserts module.main([\"0.1\", \"0.1\", \"--\", \"echo\"]) == 124 after the deadline kill; the completing-child branch (lines 151-157) asserts return 0 and the subprocess case (lines 40-59) asserts returncode 0 with stdout ok; SIGTERM forwarding asserts SystemExit 143 (lines 144-146); argument/start failures return 2/127 (lines 111-123). Coverage execution evidence (current-head authority) reports PASS - supported repository test suites passed. Outcome: completion, timeout-kill, signal, and argument branches are each asserted to exact exit codes; fabricated-status hypothesis falsified. source-line-sha256=d673315356b60699a70fcc370297c4289fcba9fa83a49e1c5d9c6ca9991b18d2","outcome":"falsified"},{"path":".github/workflows/strix-workflow-contract.yml","line":119,"hypothesis":"The new contract workflow is a decoy: it does not actually reject a strix.yml that restores evidence neutralization or drops the provenance steps.","attack_or_counterexample":"Re-add the neutral marker or remove the Validate Strix report provenance step in a future PR head and observe whether the Ruby validation program passes it.","evidence":"Source trace of the full new-file diff: the Ruby validation program ending at .github/workflows/strix-workflow-contract.yml:119 rejects the neutral marker anywhere in executable workflow source, requires the fail-closed gate fragments (if [ \"$strix_rc\" -eq 0 ]; then, ::error title=Strix evidence incomplete::, exit \"$strix_rc\") in a statically reachable step, requires named Collect/Validate/Upload steps in strict order, and requires nine provenance fragments (scan_stage_head_sha, candidate metadata conflict rejection, completed-successful-run jq selection, non-empty report, evidence-binding.json, report_sha256 digest, completed binding flag, provider fail-closed guard, candidate report path); any absence calls reject! which warns ::error and exits 1, and the head strix.yml hunk confirms all fragments exist. Outcome: restored neutralization or dropped provenance steps would be rejected with exit 1; decoy hypothesis contradicted. source-line-sha256=96b2813274364da4defe0a77f746120c470938b4565fcbcffce9aa71f3167fd2","outcome":"falsified"}],"residual_risk":"Contract enforcement uses exact string fragments and static reachability checks that obfuscated future edits could evade; scheduler gating depends on GitHub aggregate reviewDecision and check-run head-binding semantics; fail-closed Strix means provider outages can temporarily block merges (intended availability tradeoff)."}- Result: APPROVE
- Reason: Fail-closed Strix gate, per-PR workflow contract, scheduler aggregate-review approval gating, and portable-timeout regression tests all match PR intent; no failed checks, no unresolved threads, Coverage execution evidence PASS at head 67d834f.
- Head SHA:
67d834f510fe044dd9d53cd4f4b9783353e303bd - Workflow run: 31564613829
- Workflow attempt: 1
|
@coderabbitai review Review exact current head |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="ContextualWisdomLab/.github"
head="65d4b0812792a3e1fca4d534cfe2e5b7e4075137"
printf '%s\n' '== Checked-out commit =='
git rev-parse HEAD
git status --short
git show -s --format='%H%n%P%n%s' HEAD
printf '%s\n' '== Changed files against PR base =='
git diff --name-status "$(git merge-base HEAD origin/main)" HEAD
printf '%s\n' '== Relevant symbol outlines =='
ast-grep outline scripts/ci/pr_review_merge_scheduler.py --items all --match 'inspect_pr|current_head_check_runs|running_status_checks|rest_check_node|check_run_head_sha' --view expanded || true
ast-grep outline scripts/ci/portable_timeout.py --items all --view expanded || true
ast-grep outline scripts/ci/run_opencode_review_model_pool.sh --items all --view expanded || true
printf '%s\n' '== PR metadata and exact-head review/check evidence =='
gh pr view 937 --repo "$repo" --json number,headRefOid,baseRefOid,reviewDecision,mergeStateStatus,isDraft,reviews,statusCheckRollup,url
gh api "repos/$repo/pulls/937/reviews?per_page=100"
gh api "repos/$repo/commits/$head/check-runs?per_page=100"
printf '%s\n' '== Main protection =='
gh api "repos/$repo/branches/main/protection" || trueLength of output: 201685 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="ContextualWisdomLab/.github"
head="65d4b0812792a3e1fca4d534cfe2e5b7e4075137"
base="$(git merge-base HEAD origin/main)"
printf '%s\n' '== Commit and changed files =='
git rev-parse HEAD
git diff --name-only "$base" HEAD
printf '%s\n' '== Scheduler approval and mutation paths =='
rg -n -C 12 \
'def (inspect_pr|current_head_check_runs|running_status_checks|check_run_head_sha|rest_check_node)|reviewDecision|review_decision|disable.*auto|auto.?merge|enablePullRequestAutoMerge|mergePullRequest|final.*fetch|refetch|OpenCode|opencode' \
scripts/ci/pr_review_merge_scheduler.py
printf '%s\n' '== Scheduler regression tests =='
rg -n -C 8 \
'reviewDecision|review_decision|MISSING|REVIEW_REQUIRED|CHANGES_REQUESTED|auto.?merge|OpenCode|opencode|current.?head|in.?progress|Strix|strix' \
tests/test_pr_review_merge_scheduler.py
printf '%s\n' '== Strix provenance and contract =='
rg -n -C 10 \
'PR_HEAD_SHA|scan_stage_head_sha|evidence_head_sha|candidate|run\.json|penetration_test_report|evidence-binding|report_sha256|Strix evidence incomplete|exit.*strix_rc' \
.github/workflows/strix.yml
rg -n -C 10 \
'reject!|reachable|fail.closed|neutral|provenance|Collect|Validate|Upload|workflow.*digest|Psych|YAML' \
.github/workflows/strix-workflow-contract.yml
rg -n -C 8 \
'adversarial|comment|unreachable|workflow-contract|provenance|candidate' \
scripts/ci/test_strix_quick_gate.sh
printf '%s\n' '== Process-group cleanup and regression tests =='
rg -n -C 12 \
'process_group|capture_process|signal_captured|signal_process|SIGTERM|SIGKILL|run_with_timeout|portable_timeout' \
scripts/ci/run_opencode_review_model_pool.sh
rg -n -C 10 \
'TERM|KILL|process.group|child|overlap|fatal|timeout' \
tests/test_opencode_model_pool_runner.py tests/test_portable_timeout.py
printf '%s\n' '== Exact-head aggregate decision and reviews =='
gh api graphql -F owner="ContextualWisdomLab" -F name=".github" -F number=937 -f query='
query($owner: String!, $name: String!, $number: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $number) {
headRefOid
reviewDecision
mergeStateStatus
isDraft
reviews(last: 100) {
nodes {
state
submittedAt
commit { oid }
author { login __typename }
}
}
}
}
}' --jq '.data.repository.pullRequest'Length of output: 202861 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== Workflow contract policy =='
nl -ba .github/workflows/strix-workflow-contract.yml | sed -n '33,125p'
printf '%s\n' '== Workflow contract adversarial fixtures =='
nl -ba scripts/ci/test_strix_quick_gate.sh | sed -n '115,230p'
printf '%s\n' '== Scheduler implementation: evidence and mutation =='
rg -n '^(def (inspect_pr|current_head_check_runs|running_status_checks|review_decision|has_current_head|disable_auto_merge|merge_pr)|[[:space:]]*(if |return |disable_auto_merge|merge_pr|enable_auto_merge))' \
scripts/ci/pr_review_merge_scheduler.py | head -n 300
nl -ba scripts/ci/pr_review_merge_scheduler.py | sed -n '1040,1420p'
printf '%s\n' '== Scheduler tests for aggregate review and final refetch =='
rg -n -C 5 \
'aggregate|reviewDecision|MISSING|REVIEW_REQUIRED|CHANGES_REQUESTED|final.*fetch|refetch|disable.*auto|current.?head' \
tests/test_pr_review_merge_scheduler.py | head -n 500Length of output: 1325 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== Workflow contract policy =='
cat -n .github/workflows/strix-workflow-contract.yml | sed -n '33,125p'
printf '%s\n' '== Workflow contract adversarial fixtures =='
cat -n scripts/ci/test_strix_quick_gate.sh | sed -n '115,230p'
printf '%s\n' '== Scheduler implementation symbols =='
rg -n -C 4 \
'^def (inspect_pr|current_head_check_runs|running_status_checks|review_decision|has_current_head|disable_auto_merge|merge_pr)\b|reviewDecision|review_decision|disable_auto_merge|enable_auto_merge|merge_pr\(' \
scripts/ci/pr_review_merge_scheduler.py
printf '%s\n' '== Scheduler implementation mutation region =='
cat -n scripts/ci/pr_review_merge_scheduler.py | sed -n '1040,1420p'
printf '%s\n' '== Scheduler tests for aggregate review and final re-fetch =='
rg -n -C 5 \
'aggregate|reviewDecision|MISSING|REVIEW_REQUIRED|CHANGES_REQUESTED|final.*fetch|refetch|disable.*auto|current.?head' \
tests/test_pr_review_merge_scheduler.py | head -n 500Length of output: 78686
P1 — The Strix workflow contract accepts a fail-open gate. The accepted fixture in
A PR-head The scheduler review confirms that Branch-protection settings were not verified because the GitHub integration received HTTP 403 from the branch-protection endpoint. No mutation, approval, auto-merge operation, or merge was performed. 🧠 Learnings usedYou are interacting with an AI system. |
|
@opencode-agent review Review exact current head |
Rate Limit Exceeded
|
|
Exact-current-head semantic review request for This head is broader than the original aggregate-review slice. Review the complete 31-file current diff, including: aggregate Keep this exact head unchanged. Do not transfer predecessor approval, synthesize author approval, update the branch, merge, release, neutralize missing security evidence, or weaken protection. @coderabbitai review |
Rate Limit Exceeded
|
|
Closing as a stale, over-broad integration branch superseded by independently reviewable current-main slices. Fresh comparison of this exact head The stated aggregate-review repair is already implemented more narrowly by #828: GitHub Keeping #937 open would duplicate those branches, create conflicting implementations, and permit one slice's checks to obscure another slice's review and operational requirements. No check, review, or approval from this branch transfers to any successor. Preserve this PR as historical RCA only; merge the bounded successors in their dependency order under their own exact-head gates. |
Pull request was closed
1 participant
Summary\n- block direct merge and auto-merge unless GitHub aggregate reviewDecision is APPROVED for the same current-head OpenCode approval\n- disable an already queued auto-merge when aggregate review evidence is REVIEW_REQUIRED or CHANGES_REQUESTED\n- preserve review/security dispatch for PRs that have not reached merge readiness\n- add scheduler self-test and pytest coverage for both unsafe aggregate states\n\n## Validation\n- python3 scripts/ci/pr_review_merge_scheduler.py --self-test\n- uvx --from pytest pytest -q tests/test_pr_review_merge_scheduler.py (112 passed)\n- python3 -m py_compile scripts/ci/pr_review_merge_scheduler.py tests/test_pr_review_merge_scheduler.py\n\n## Related\n- https://github.com/ContextualWisdomLab/contextual-orchestrator/pull/109\n- https://github.com/ContextualWisdomLab/contextual-orchestrator/blob/d5122b2/docs/planning/adrs/0004-pr-review-merge-loop.md
Summary by CodeRabbit
버그 수정
개선 사항
테스트