Skip to content

fix(dispatch): parse the trusted-dispatcher allowlist identically in all three consumers (#1929) - #1932

Merged
seonghobae merged 2 commits into
mainfrom
fix/dispatch-actor-allowlist
Sep 5, 2026
Merged

fix(dispatch): parse the trusted-dispatcher allowlist identically in all three consumers (#1929)#1932
seonghobae merged 2 commits into
mainfrom
fix/dispatch-actor-allowlist

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Closes nothing by itself — the variable still needs an owner decision. See #1929.

Why

Two trusted workflows send the opencode-review repository_dispatch:

dispatcher token sender
opencode-review.yml OpenCode GitHub App (since #1497, 4a5dfd82) opencode-agent[bot]
pr-review-merge-scheduler.yml its own token chain (:211, :487) github-actions[bot]

The gate in opencode-review-dispatch.yml compared both actor and sender against the single-valued vars.OPENCODE_REPOSITORY_DISPATCH_ACTOR, which still names only github-actions[bot] (last set 2026-07-16). Every app-token dispatch has therefore failed at the first job:

opencode-review-dispatch.yml lifetime:  success 466 / failure 9611   (95.4%)
failure sample n=40:  27 opencode-agent[bot]  · 11 github-actions[bot]  · 2 manual
open non-draft PRs with a successful opencode-review run on their current head:  0 / 99

Replacing the value would flip the problem onto the scheduler path. The gate has to be able to hold more than one identity.

What changes

vars.OPENCODE_REPOSITORY_DISPATCH_ACTOR is read by three workflows, and all three now parse it the same way — as a comma-separated list, exactly like the adjacent ALLOWED_DISPATCH_TARGETS block:

consumer gate before
opencode-review-dispatch.yml :127 exact match
codeql-scan-dispatch.yml :155 exact match (byte-identical block)
pr-review-fix-scheduler.yml :156 exact match (same conditions, its own error line, kept)

Widening only the first would have left codeql-scan-dispatch rejecting the App identity once #1925's toJSON fix lets it reach the gate, and the scheduler too. Three consumers of one variable with two parsers is the next drift. Everything else is preserved:

  • actor and sender must both equal the same listed identity — a dispatch whose actor and sender are two different listed identities is still rejected
  • an empty allowlist admits nothing
  • a single-valued variable keeps working unchanged

The variable is not modified. Which identities belong on the list is an authorization decision for the repository owner; this PR only makes the gate capable of expressing it.

Verification

Not claimed

Cause A is dominant but not sole. Failures from github-actions[bot] — an identity that matches the current allowlist — reach later steps and are attributed to #1883's pre-merge coverage gate (B, likely resolved) and to base-SHA drift during queue wait (C, #1931, structural). Fixing A is also the diagnostic that reveals how much of C remains.

This PR does not touch noema-review. That workflow never goes through the dispatch gate — it calls the contextual-orchestrator sidecar gateway directly — and its recent failures are gateway 502/429 after 3–44 minutes with attempts=1 (7 of 8 sampled), not authorization rejections. Restoring opencode-review verdicts is what this PR does; noema-review needs its own fix at the gateway layer.

🤖 Generated with Claude Code

Two trusted workflows send the opencode-review repository_dispatch:
opencode-review.yml through the OpenCode GitHub App (sender
opencode-agent[bot], introduced by #1497) and pr-review-merge-scheduler.yml
through its own token chain (sender github-actions[bot]). The authorization
gate in opencode-review-dispatch.yml compared both actor and sender against a
single-valued variable that still names only github-actions[bot], so every
app-token dispatch has failed at the first job -- 9611 failures to 466
successes over the workflow's lifetime, and no open PR holds a successful
review on its current head (#1929).

Parse ALLOWED_DISPATCH_ACTOR as a comma-separated list, exactly as the
adjacent ALLOWED_DISPATCH_TARGETS block already does. Semantics are preserved
otherwise: actor and sender must both equal the SAME listed identity (a
dispatch whose actor and sender are two different listed identities is still
rejected), and an empty allowlist admits nothing.

This change does not alter the variable. Which identities belong on the list
is an authorization decision for the repository owner; this only makes the
gate able to express more than one. A single-valued variable keeps working
unchanged.

Contract test extended in tests/test_opencode_agent_contract.py: both
identities pass with a listed allowlist (whitespace around commas tolerated),
an unlisted identity is rejected, and mismatched actor/sender is rejected.
Negative control: the extended test fails against the unmodified gate on
origin/main. REVIEW_DISPATCH_BLOB_SHA recomputed via git hash-object.

Verified: 2890 passed, coverage 100%, interrogate 100%.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 52 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: 1deef1fc-c68e-43ba-af8c-ec3984dfd2b8

📥 Commits

Reviewing files that changed from the base of the PR and between f250638 and d44aa07.

📒 Files selected for processing (7)
  • .github/workflows/codeql-scan-dispatch.yml
  • .github/workflows/opencode-review-dispatch.yml
  • .github/workflows/pr-review-fix-scheduler.yml
  • tests/test_codeql_scan_dispatch_workflow_contract.py
  • tests/test_opencode_agent_contract.py
  • tests/test_pr_review_autofix_nvidia_nim_contract.py
  • tests/test_pr_review_fix_hourly_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.

…sumers

vars.OPENCODE_REPOSITORY_DISPATCH_ACTOR is read by three workflows, and the
first commit widened only one of them:

  opencode-review-dispatch.yml:127   covered by the previous commit
  codeql-scan-dispatch.yml:155       byte-identical gate, was still exact-match
  pr-review-fix-scheduler.yml:156    same three conditions, different error line

Left as-is, codeql-scan-dispatch would keep rejecting the App identity once
#1925's toJSON fix lets it reach line 155, and the scheduler would too. Three
consumers of one variable with two parsers is the next drift, so all three now
run the same comma-separated parse with the same semantics: actor and sender
must both equal the SAME listed identity, empty list admits nothing, single
value unchanged. The scheduler keeps its own error line.

Tests extended in place for both: a listed identity passes (whitespace around
commas tolerated), an unlisted one is rejected, and actor/sender that are two
different listed identities are rejected. Negative control: both extended
tests fail against the unmodified gates on origin/main. The codeql helper
creates tmp_path/bin, so each invocation gets its own subdirectory.

No blob-SHA pin references either newly edited workflow. No open PR touches
either gate: #1926 covers codeql-scan-dispatch 146-152 and #1741 covers
pr-review-fix-scheduler 210-217.

Verified: 2891 passed, coverage 100%, interrogate 100%.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@seonghobae seonghobae changed the title fix(dispatch): accept a list of trusted dispatcher identities (#1929) fix(dispatch): parse the trusted-dispatcher allowlist identically in all three consumers (#1929) Sep 5, 2026
@seonghobae
seonghobae merged commit 6f8c51d into main Sep 5, 2026
4 of 16 checks passed
@seonghobae
seonghobae deleted the fix/dispatch-actor-allowlist branch September 5, 2026 13:35
seonghobae added a commit that referenced this pull request Sep 5, 2026
#1932 changed `.github/workflows/opencode-review-dispatch.yml`, which
REVIEW_DISPATCH_BLOB_SHA pins by whole-file `git hash-object`. The workflow
itself auto-merged cleanly; only the pin line conflicted, and again neither
side's value is right for the merge:

  this branch  ccc0d34…  (its own tree)
  main         26e8555…  (after #1932)
  merged       449cee2…  (recomputed)

449cee2 matches the value predicted by simulating this merge before #1932
landed, so the pin behaves exactly as expected under a base change.

Verified: 2896 passed, 1 skipped, coverage 100%, interrogate 100%; zero
conflict markers; `ruff check --select F821` clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
seonghobae pushed a commit that referenced this pull request Sep 5, 2026
#1932 landed on main and rewrote .github/workflows/opencode-review-dispatch.yml,
so REVIEW_DISPATCH_BLOB_SHA in tests/test_pr_review_autofix_nvidia_nim_contract.py
conflicted again: this branch carried 2fb3306 (the hash of the file as
merged with main before #1932) and main carries 26e8555 (#1932's file).

Neither is correct here. This branch also modifies the dispatch workflow,
and git auto-merged it without a marker, so the merged file hashes to a
third value: 0a39def. Recomputed from the
merged workflow, as before.

Same rule as the first resolution on this branch, hit a second time because
a concurrent change to the pinned file landed in between. The pin will
need recomputing again if anything else touches that workflow before this
merges; that is the cost of a hardcoded content pin, and the live-computed
sibling in test_opencode_rust_coverage_toolchain_contract.py does not pay it.

tests/test_pr_review_autofix_nvidia_nim_contract.py: 24 passed.
Full suite under GITHUB_ACTIONS=true: 2907 passed, 1 skipped, 21 subtests.
Coverage 100%, interrogate 100%, git diff --check clean.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4
seonghobae added a commit that referenced this pull request Sep 5, 2026
#1932 changed `.github/workflows/opencode-review-dispatch.yml`, which
REVIEW_DISPATCH_BLOB_SHA pins by whole-file `git hash-object`. The workflow
auto-merged cleanly; only the pin line conflicted, and neither side's value is
right for the merge:

  this branch  bcf7446…  (its own tree)
  main         26e8555…  (after #1932)
  merged       f1a9eb0…  (recomputed)

Verified: 2893 passed, 1 skipped, coverage 100%, interrogate 100%; zero
conflict markers; `ruff check --select F821` clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
seonghobae added a commit that referenced this pull request Sep 5, 2026
#1932 changed `.github/workflows/opencode-review-dispatch.yml`, which
REVIEW_DISPATCH_BLOB_SHA pins by whole-file `git hash-object`. The workflow
auto-merged cleanly; only the pin line conflicted, and neither side's value is
right for the merge:

  this branch  c341291…  (its own tree)
  main         26e8555…  (after #1932)
  merged       cbd0c0c…  (recomputed)

Verified: 2926 passed, 1 skipped, coverage 100%, interrogate 100%; zero
conflict markers; `ruff check --select F821` clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.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