fix: approve reviewed merge-update fallback - #247
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new “review fallback approval” path in the OpenCode review workflow for model-pool exhaustion cases where the current head is a 2-parent merge-update commit and the first parent already has an OpenCode approval. Updates the contract test to ensure the workflow continues to expose the expected fallback messaging.
Changes:
- Add
approve_previously_reviewed_merge_update_after_model_exhaustion()fallback approval logic for 2-parent merge-update commits in the model-failure path. - Gate fallback approval on prior OpenCode approval on the first parent plus base-parent ancestry evidence for the second parent.
- Extend
tests/test_opencode_agent_contract.pyto assert the new fallback function and messaging are present in the workflow.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/test_opencode_agent_contract.py | Adds contract assertions ensuring the workflow contains the new merge-update fallback strings. |
| .github/workflows/opencode-review.yml | Implements and wires a new fallback approval path for previously-reviewed merge-update commits after model exhaustion. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
OpenCode Review Overview
Pull request overviewOpenCode reviewed the current-head bounded evidence and found no blocking issues. FindingsNo blocking findings. SummaryApproval 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.
Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow: opencode-review.yml"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow: opencode-review.yml"]
R1 --> V1["actionlint plus required checks"]
Evidence --> S2["Test: test_opencode_agent_contract.py"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test: test_opencode_agent_contract.py"]
R2 --> V2["targeted test run"]
|
|
중앙 정책이 #246에서 retry/대체 모델/고 reasoning effort 기반으로 정리되면서, 이 PR의 merge-update deterministic approval fallback은 더 이상 채택하지 않습니다. 모델 소진이나 timeout은 승인 사유가 아니라 재시도 또는 대체 모델 실행 사유이며, 최종적으로도 이전 parent 승인만으로 current head를 APPROVE하지 않는 방향으로 유지합니다. |
![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 evidence but found unresolved human review threads before approval.
Findings
1. HIGH .github/workflows/opencode-review.yml:1 - Unresolved human review thread blocks automated 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
.github/workflows/opencode-review.yml line 4557
-
Latest human comment: @copilot-pull-request-reviewer at 2026-07-01T00:24:28Z
-
Comment URL: #247 (comment)
-
Comment excerpt: This treats a DISMISSED OpenCode review on the first parent as sufficient approval evidence. Dismissed reviews are explicitly revoked in GitHub, and counting them here can resurrect an approval that was intentionally dismissed (e.g., due to incorrect review). If the intent is only to accept branch-protection stale-review dismissal, the dismissal reason/metadata should be validated and manual dismissals should not qualify.
-
Result: REQUEST_CHANGES
-
Reason: unresolved human review thread(s) were present before approval.
-
Head SHA:
9846b936a71ff97fa61909c7b11907026ac819cc -
Workflow run: 28485501620
-
Workflow attempt: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow: opencode-review.yml"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow: opencode-review.yml"]
R1 --> V1["actionlint plus required checks"]
Evidence --> S2["Test: test_opencode_agent_contract.py"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test: test_opencode_agent_contract.py"]
R2 --> V2["targeted test run"]
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: 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/opencode-review.yml, tests/test_opencode_agent_contract.py.
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/opencode-review.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 checks are delegated to configured OpenCode web_search/Context7/DeepWiki sources when applicable; 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: Playwright visual, DOM locator, ARIA snapshot, console, and responsive evidence were checked when a web UI surface was present; for non-web surfaces, API/CLI/log/docs/workflow interaction evidence was reviewed 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.
- Result: APPROVE
- Reason: Safe fallback mechanism with strict conditions and full test coverage
- Head SHA:
9846b936a71ff97fa61909c7b11907026ac819cc - Workflow run: 28486869838
- Workflow attempt: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow: opencode-review.yml"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow: opencode-review.yml"]
R1 --> V1["actionlint plus required checks"]
Evidence --> S2["Test: test_opencode_agent_contract.py"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test: test_opencode_agent_contract.py"]
R2 --> V2["targeted test run"]
Summary
Tests
py -3 -m pytest tests/test_opencode_agent_contract.py -q