Skip to content

fix(ci): repair main's stale review-dispatch blob pin and security-boundary test - #1536

Closed
seonghobae wants to merge 1 commit into
mainfrom
fix/review-dispatch-blob-sha-pin-stale
Closed

fix(ci): repair main's stale review-dispatch blob pin and security-boundary test#1536
seonghobae wants to merge 1 commit into
mainfrom
fix/review-dispatch-blob-sha-pin-stale

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Problem

main is currently red on two tests, both caused by legitimate, already-merged, already-reviewed changes to .github/workflows/opencode-review-dispatch.yml that didn't update the tests pinning its exact content:

  1. test_independent_review_agent_workflow_matches_reviewed_blob (tests/test_pr_review_autofix_nvidia_nim_contract.py) and test_review_dispatch_blob_sha_stays_paired_with_trusted_workflow (tests/test_opencode_rust_coverage_toolchain_contract.py, which reads the same REVIEW_DISPATCH_BLOB_SHA constant via regex) both fail: the pinned blob hash (2aa245e7…) no longer matches the workflow's actual current blob (3762183e…).
  2. test_opencode_privileged_review_security_boundaries_are_fail_closed (tests/test_opencode_agent_contract.py) still pins the pre-#1533 behavior — a hard exit 1 on any head_sha mismatch between dispatch-supplied and live PR metadata.

Root cause

#1533 ("fix(opencode): proceed on head-only advance in review dispatch validation", Devin-reviewed, merged) deliberately dropped head_sha from the hard-fail exact-match set in validate-pr-metadata, replacing it with a ::warning:: + proceed-on-live-head path — because every downstream job already re-validates the live head independently via STALE_HEAD guards, and the hard fail was breaking the required opencode-review check under the org's current Actions queue backlog (PR head advancing between dispatch and job start). That change edited the workflow's blob content without updating either the blob-pin constant or the test asserting the old hard-fail-on-head_sha-mismatch behavior.

Verified both failures pre-exist this change (reproduced against unmodified origin/main before touching anything) — this PR does not introduce either bug, it repairs main's existing broken state.

Fix

  • Bumped REVIEW_DISPATCH_BLOB_SHA to the workflow's current blob hash. Before bumping, verified the file's current content is safe to pin: #1533 already went through Devin Review before merging, and the isolation invariant this pin exists to protect ("pr-review-autofix" not in the read-only reviewer workflow) still holds.
  • Replaced the stale test_opencode_privileged_review_security_boundaries_are_fail_closed assertion with ones pinning the new, correct invariant: base_ref/base_sha/head_ref remain hard-fail exact matches (mismatches+=(...)), head_sha is explicitly excluded from that set, and the warn-and-proceed path (::warning::repository_dispatch head advanced since dispatch) is present. This updates the pin to match #1533's intentional design rather than reverting it.

Test plan

  • Reproduced both failures against unmodified origin/main first, confirming they pre-exist this change.
  • coverage run -m pytest tests -q — 2127 passed, 1 skipped, 21 subtests passed.
  • coverage report --show-missing — 100%.
  • interrogate — 100%, PASSED.

Generated by Claude Code


Devin Review

…undary test

Two tests broke on main after two independently-legitimate, already-merged
changes to .github/workflows/opencode-review-dispatch.yml, neither of
which updated the assertions that pin its exact content:

- REVIEW_DISPATCH_BLOB_SHA in test_pr_review_autofix_nvidia_nim_contract.py
  still pinned an old blob hash. Both
  test_independent_review_agent_workflow_matches_reviewed_blob (same file)
  and test_review_dispatch_blob_sha_stays_paired_with_trusted_workflow
  (test_opencode_rust_coverage_toolchain_contract.py, which reads this same
  constant) failed as a result. Verified the file's current content is
  legitimate before bumping the pin: it was last touched by #1533
  (Devin-reviewed, merged), and the isolation invariant this pin exists to
  protect ("pr-review-autofix" must never appear in the read-only reviewer
  workflow) still holds. Updated the constant to the current blob hash.

- test_opencode_privileged_review_security_boundaries_are_fail_closed
  (test_opencode_agent_contract.py) pinned the pre-#1533 hard-fail-on-any-
  head_sha-mismatch behavior. #1533 deliberately relaxed that to a warning
  (downstream jobs already re-validate the live head independently via
  STALE_HEAD guards), which is why the required opencode-review check was
  failing closed on ordinary PR activity under the org's Actions queue
  backlog. Updated the assertion to pin the new, correct invariant instead
  of reverting #1533's fix: base_ref/base_sha/head_ref remain hard-fail
  exact matches, head_sha is explicitly excluded from that set, and the
  warn-and-proceed path is present.

Verified against unmodified origin/main that both failures pre-existed
this change (not introduced by it) before fixing.

---
Generated by Claude Code

@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 found 2 potential issues.

Devin Review

Comment thread tests/test_pr_review_autofix_nvidia_nim_contract.py
Comment thread tests/test_opencode_agent_contract.py
@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 44 minutes.

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: 51cf7af5-56dc-45df-a712-6e0c656e0c35

📥 Commits

Reviewing files that changed from the base of the PR and between 1186a9f and 5cba0cc.

📒 Files selected for processing (2)
  • tests/test_opencode_agent_contract.py
  • tests/test_pr_review_autofix_nvidia_nim_contract.py

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.

Copy link
Copy Markdown
Contributor Author

@opencode-agent review

Re-review the exact current head 5cba0ccf57b043ba39f46912bf532cd945ea916d. Focus on whether the updated reviewed-blob pin exactly matches the trusted dispatch workflow and whether the #1533 head-advance contract still fails closed before any stale head can receive privileged model/review execution. Do not reuse predecessor-head evidence.

seonghobae pushed a commit that referenced this pull request Sep 1, 2026
CI on this PR's own head surfaced 3 failures caused by main itself
being red, not by this PR: #1533 ("proceed on head-only advance in
review dispatch validation") changed opencode-review-dispatch.yml's
head_sha handling (warn-and-proceed instead of hard-fail on a
dispatch/live head mismatch, since downstream jobs already
re-validate the live head independently) without updating the two
tests pinning that workflow's exact blob hash or the test asserting
the old hard-fail behavior.

#1536 already fixes this upstream (root-caused, reproduced against
unmodified main first, Devin-reviewed, full suite green) but hasn't
merged yet. Porting the same two-file diff here per the drive-to-green
protocol -- it no-ops once main carries #1536. Full suite: 2128
passed, 1 skipped, 0 failures.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4
seonghobae pushed a commit that referenced this pull request Sep 1, 2026
Resolves a real conflict in tests/test_opencode_required_verdict_regression.py
(both branches added independent, non-overlapping tests to the same file:
this branch's draft-gate regression tests, main's #1532 poll-budget-ceiling
guard) by keeping both.

Also fixes two issues the merge exposed:
- The merged-in poll loop bound increase (180->660 attempts via #1532) turned
  test_non_draft_pr_without_a_verdict_still_fails_closed's real `sleep 30`
  calls into a ~5.5-hour test; stub `sleep` as a no-op on PATH alongside the
  existing fake `gh`.
- main is currently red on the review-dispatch blob pin and the
  security-boundary test (stale since #1533's already-merged head_sha
  warn-and-proceed change); re-pin both to match, same fix already applied
  in #1482, pending #1536.

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

Copy link
Copy Markdown
Contributor Author

Closing as superseded by protected-main revert #1540 (main@7b1a028e704a98ae8a807bb827f44aeaee0399af).

At exact head 5cba0ccf57b043ba39f46912bf532cd945ea916d, this PR now differs from protected main only in two test files. One change re-authorizes #1533's head-mismatch warning/proceed behavior, which #1540 deliberately removed after the cross-head security-authority regression was verified. The former blob-pin/test-red rationale is therefore historical rather than a current repair target.

Current admission evidence is also non-passing: approvals 0; checks 3 success, 14 queued, 2 cancelled, 14 skipped. No source commit is being moved or rewritten.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants