Skip to content

Revert "fix(opencode): proceed on head-only advance in review dispatch validation (#1533)" - #1540

Merged
seonghobae merged 1 commit into
mainfrom
revert/1533-head-sha-incomplete-20260901
Sep 1, 2026
Merged

Revert "fix(opencode): proceed on head-only advance in review dispatch validation (#1533)"#1540
seonghobae merged 1 commit into
mainfrom
revert/1533-head-sha-incomplete-20260901

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Reverts #1533 — Codex P1×3/P2 + Devin found it incomplete (contract test red at tests/test_opencode_agent_contract.py:2663; draft-review auth bypass; run-name/scheduler-ownership mismatch self-cancels the recovered review; empty/malformed supplied head now passes). The head-advance TOCTOU fix belongs with the scheduler run-identity / queued-run lifecycle work already in flight (#1531, #1532, #1539). See #1531.


Devin Review

@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 22 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: c907cf0f-bcba-47ab-8af4-d3d0e391e39e

📥 Commits

Reviewing files that changed from the base of the PR and between 1186a9f and 439d945.

📒 Files selected for processing (1)
  • .github/workflows/opencode-review-dispatch.yml

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@seonghobae
seonghobae merged commit 7b1a028 into main Sep 1, 2026
14 of 30 checks passed
@seonghobae
seonghobae deleted the revert/1533-head-sha-incomplete-20260901 branch September 1, 2026 01:48

@devin-ai-integration devin-ai-integration 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.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

seonghobae pushed a commit that referenced this pull request Sep 1, 2026
origin/main reverted a3f9f9b (commit 7b1a028, #1540, no rationale
given) right after this branch merged it and adapted
test_pr_review_autofix_nvidia_nim_contract.py's REVIEW_DISPATCH_BLOB_SHA
and test_opencode_agent_contract.py's metadata-step assertion to match
its new warn-and-proceed behavior. Restore both to their original
exact-match values now that opencode-review-dispatch.yml itself is back
to its pre-a3f9f9b6 content (blob 2aa245e).
seonghobae pushed a commit that referenced this pull request Sep 1, 2026
main has continued to churn on opencode-review-dispatch.yml since this
branch was last rebased (most recently #1540 reverting #1533's head_sha
warn-and-proceed relaxation back to a uniform hard-fail on any
base_ref/base_sha/head_ref/head_sha mismatch). This branch never touched
that workflow itself, but inherited a stale REVIEW_DISPATCH_BLOB_SHA pin
and a metadata_step assertion still pinning the retired #1533 contract.

- Bumped REVIEW_DISPATCH_BLOB_SHA to the workflow's current blob hash
  (verified: file is byte-identical to origin/main's copy, and the
  "pr-review-autofix" isolation invariant still holds).
- Replaced the stale head_sha assertions in
  test_opencode_agent_contract.py with the current uniform hard-fail
  contract (all four fields hard-fail via mismatches+=(...); no separate
  warn-and-proceed branch exists post-#1540).

Full suite: 2127 passed, 1 skipped, 21 subtests passed. 100% coverage,
100% docstrings.
seonghobae pushed a commit that referenced this pull request Sep 1, 2026
…e draft-gate fix

main's opencode-review.yml has been substantially redesigned since this
branch last synced (#1507/#1532): the old 325-minute synchronous poll
loop is gone, replaced by a fast check-once-dispatch-and-fail-closed
"Resolve current-head formal OpenCode verdict" step plus a separate
formal-receipt "wake" callback that reruns the failed job once a verdict
actually lands, instead of blocking a runner for hours. #1533's
head_sha warn-and-proceed design (which an earlier revision of this
fix's blob-pin port matched) was also reverted upstream (#1540, real
bugs found by Codex/Devin) -- restored to the original hard-fail
assertions and current blob pin.

The draft-gate exemption itself is unaffected by any of that and is
re-applied cleanly against the new three-step structure:
- "Resolve current-head formal OpenCode verdict" now exits early with
  verdict=DRAFT for a draft PR, mirroring its existing closed exit.
- "Request current-head OpenCode review execution"'s own if: also skips
  drafts, so a transient OIDC/dispatch failure can't turn a draft PR's
  check red before the exemption runs.
- The now-trivial "Fail closed without a current-head OpenCode verdict"
  step (no gh calls or loop left in it at all) treats VERDICT=DRAFT the
  same as VERDICT=CLOSED.
- converted_to_draft added to the trigger types, so a ready PR converted
  back to draft with no new commit still gets a fresh run.

tests/test_opencode_required_verdict_regression.py's old _run_step
helper and its six tests assumed the removed monolithic polling step;
replaced with _run_verdict_step/_run_fail_closed_step matching the new
split, keeping the same draft/closed/ready-for-review coverage.

Full suite: 2216 passed, 1 skipped, 21 subtests. Ruff, interrogate,
YAML, and shell-syntax checks clean.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4
seonghobae added a commit that referenced this pull request Sep 3, 2026
…#1479)

* fix(ci): remove dormant nvidia-nim provider block from opencode.jsonc

Two investigation passes traced every remaining candidate direct-NVIDIA-NIM
communication path in the repo. Only one turned out to be worth fixing: the
`nvidia-nim` provider block in opencode.jsonc has been fully dead for every
automated/CI review path since PR #1364 (enabled_providers excludes it, the
dispatch/autofix workflows generate their own from-scratch config that never
copies it in, and OPENCODE_MODEL_CANDIDATES never references it) -- yet two
assertions in scripts/ci/test_strix_quick_gate.sh still pinned its presence
as if it were required, a stale artifact left over from before #1364 flipped
the config to gateway-only.

- Remove the nvidia-nim provider block (opencode.jsonc:289-378).
- Fix the two orphaned assertions to assert_file_not_contains, matching the
  sibling assertions that already forbid the old NVIDIA NIM model-id
  defaults.
- Delete docs/nvidia-nim-opencode-hotfix.md per its own Rollback section
  (all three rollback steps are now complete).
- Add a doctoring record and a dated gap-baseline follow-up entry.

The other candidate path (strix_quick_gate.sh's is_known_foreign_provider_api_base
allowlist, and zdr_policy.py's PROVIDER_BASE_URLS) was audited and confirmed
to be either a leak-blocking guard or the vendored gateway's own internal
routing table -- not a bypass -- so left untouched.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Gs7KmNvH75nxz1sL8mKjw

* fix(ci): bound required-workflow-bootstrap awk extraction to its own job

Hand-ports the fix from #1506 (still open, not yet on main) into this
branch so the exact-head-path-policy check stops failing on an
unrelated swept-in line from PR #1497.

The awk range pattern /^  required-workflow-bootstrap:$/,/^[^ ]/ never
matched its end condition because every job key in opencode-review.yml
is indented 2 spaces, never column 0 -- so the "block" it captured ran
to EOF and incorrectly swept in an unrelated if: line from a different
job. Replaced with an explicit state-flag awk program that stops at
the next 2-space-indented job key.

Also drops -q from both grep calls piped from an awk producer under
set -o pipefail (the required-workflow-bootstrap check and the
opencode review PR-level REQUEST_CHANGES fenced-diff check), since a
short-circuiting grep -q can SIGPIPE the still-writing awk producer
and surface awk's 141 instead of grep's real exit code on large
inputs.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Gs7KmNvH75nxz1sL8mKjw

* fix(ci): repair review-dispatch blob pin drift after merging main

Same fix already applied on .github#1536: main's #1533 changed
opencode-review-dispatch.yml's head_sha handling from a hard exact-match
to a warn-and-proceed (fixing the org-wide merge-freeze bug), which
changed the workflow's blob hash without updating either the pinned
REVIEW_DISPATCH_BLOB_SHA constant or the security-boundary test asserting
the old hard-fail behavior. Ported the identical fix here after merging
main brought that same break onto this branch: bumped the pin to the
current blob hash (verified safe: #1533 was Devin-reviewed and merged,
and the "pr-review-autofix" isolation invariant this pin protects still
holds), and updated the stale assertion to pin the new, correct
invariant instead of reverting #1533.

---
Generated by Claude Code

* fix(ci): repair stale review-dispatch blob pin and head_sha contract

main has continued to churn on opencode-review-dispatch.yml since this
branch was last rebased (most recently #1540 reverting #1533's head_sha
warn-and-proceed relaxation back to a uniform hard-fail on any
base_ref/base_sha/head_ref/head_sha mismatch). This branch never touched
that workflow itself, but inherited a stale REVIEW_DISPATCH_BLOB_SHA pin
and a metadata_step assertion still pinning the retired #1533 contract.

- Bumped REVIEW_DISPATCH_BLOB_SHA to the workflow's current blob hash
  (verified: file is byte-identical to origin/main's copy, and the
  "pr-review-autofix" isolation invariant still holds).
- Replaced the stale head_sha assertions in
  test_opencode_agent_contract.py with the current uniform hard-fail
  contract (all four fields hard-fail via mismatches+=(...); no separate
  warn-and-proceed branch exists post-#1540).

Full suite: 2127 passed, 1 skipped, 21 subtests passed. 100% coverage,
100% docstrings.

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: opencode-agent[bot] <219766164+opencode-agent[bot]@users.noreply.github.com>
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.

1 participant