Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 28 additions & 6 deletions .github/workflows/pr-review-merge-scheduler.yml
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,30 @@ jobs:
MAX_PRS: ${{ github.event.client_payload.max_prs || inputs.max_prs || '100' }}
PROJECT_FLOW_INPUT: ${{ github.event.client_payload.project_flow || inputs.project_flow || vars.PROJECT_FLOW || '' }}
PULL_REQUEST_NUMBER: ${{ github.event.pull_request.number || github.event.workflow_run.pull_requests[0].number || github.event.client_payload.pr_number || inputs.pr_number || '' }}
TRIGGER_REVIEWS: ${{ github.event_name == 'schedule' || github.event_name == 'workflow_run' || github.event_name == 'push' || github.event_name == 'pull_request_target' || (github.event_name == 'repository_dispatch' && github.event.client_payload.trigger_reviews != false) || inputs.trigger_reviews == true }}
# toJSON(...) != 'false' && toJSON(...) != '"false"' below (not a
# direct comparison against the boolean literal): GitHub Actions
# coerces both `false` and an absent/`null` client_payload property to
# 0 for a mixed-type inequality check, so that direct-comparison form
# is false -- not true, as the "default enabled unless explicitly
# disabled" intent requires -- whenever a repository_dispatch payload
# omits the field entirely. Stringifying first compares 'null' against
# 'false', which is exact and unaffected by that coercion. Verified
# empirically: a targeted merge-scheduler dispatch with no
# trigger_reviews field produced TRIGGER_REVIEWS=false on the old
# direct-comparison expression before this fix. The second clause
# additionally rejects the JSON *string* "false" (toJSON produces
# '"false"', quotes included) -- a real possibility since `gh api -f
# field=false` sends a string, not a boolean (CodeRabbit review
# finding on #1238) -- so a naive `-f trigger_reviews=false` caller is
# also treated as disabled, not silently left enabled.
TRIGGER_REVIEWS: ${{ github.event_name == 'schedule' || github.event_name == 'workflow_run' || github.event_name == 'push' || github.event_name == 'pull_request_target' || (github.event_name == 'repository_dispatch' && toJSON(github.event.client_payload.trigger_reviews) != 'false' && toJSON(github.event.client_payload.trigger_reviews) != '"false"') || inputs.trigger_reviews == true }}
REVIEW_DISPATCH_LIMIT_INPUT: ${{ github.event.client_payload.review_dispatch_limit || inputs.review_dispatch_limit || vars.REVIEW_DISPATCH_LIMIT || '1' }}
BRANCH_UPDATE_LIMIT_INPUT: ${{ github.event.client_payload.branch_update_limit || inputs.branch_update_limit || vars.BRANCH_UPDATE_LIMIT || '1' }}
ENABLE_AUTO_MERGE: ${{ github.event_name == 'schedule' || github.event_name == 'push' || github.event_name == 'pull_request_target' || github.event_name == 'workflow_run' || (github.event_name == 'repository_dispatch' && github.event.client_payload.enable_auto_merge != false) || inputs.enable_auto_merge == true }}
# Same toJSON(...) coercion fix as TRIGGER_REVIEWS above.
ENABLE_AUTO_MERGE: ${{ github.event_name == 'schedule' || github.event_name == 'push' || github.event_name == 'pull_request_target' || github.event_name == 'workflow_run' || (github.event_name == 'repository_dispatch' && toJSON(github.event.client_payload.enable_auto_merge) != 'false' && toJSON(github.event.client_payload.enable_auto_merge) != '"false"') || inputs.enable_auto_merge == true }}
MERGE_MODE: ${{ github.event.client_payload.merge_mode || inputs.merge_mode || vars.PR_MERGE_MODE || 'direct_or_auto' }}
UPDATE_BRANCHES: ${{ github.event_name == 'schedule' || github.event_name == 'push' || github.event_name == 'pull_request_target' || github.event_name == 'workflow_run' || (github.event_name == 'repository_dispatch' && github.event.client_payload.update_branches != false) || inputs.update_branches == true }}
# Same toJSON(...) coercion fix as TRIGGER_REVIEWS above.
UPDATE_BRANCHES: ${{ github.event_name == 'schedule' || github.event_name == 'push' || github.event_name == 'pull_request_target' || github.event_name == 'workflow_run' || (github.event_name == 'repository_dispatch' && toJSON(github.event.client_payload.update_branches) != 'false' && toJSON(github.event.client_payload.update_branches) != '"false"') || inputs.update_branches == true }}
Comment thread
seonghobae marked this conversation as resolved.
STALE_OPENCODE_MINUTES: ${{ github.event.client_payload.stale_opencode_minutes || inputs.stale_opencode_minutes || vars.STALE_OPENCODE_MINUTES || '90' }}
steps:
- name: Exchange OpenCode app token for scheduler mutations
Expand Down Expand Up @@ -606,10 +624,14 @@ jobs:
ORG_SWEEP_MAX_PRS: ${{ github.event.client_payload.max_prs || inputs.max_prs || vars.ORG_SWEEP_MAX_PRS || '1000' }}
ORG_SWEEP_REVIEW_DISPATCH_LIMIT: ${{ github.event.client_payload.review_dispatch_limit || inputs.review_dispatch_limit || vars.ORG_SWEEP_REVIEW_DISPATCH_LIMIT || '1' }}
ORG_SWEEP_BRANCH_UPDATE_LIMIT: ${{ github.event.client_payload.branch_update_limit || inputs.branch_update_limit || vars.ORG_SWEEP_BRANCH_UPDATE_LIMIT || '1' }}
ORG_SWEEP_TRIGGER_REVIEWS: ${{ github.event_name == 'schedule' || github.event_name == 'repository_dispatch' && github.event.client_payload.trigger_reviews != false || inputs.trigger_reviews == true }}
ORG_SWEEP_ENABLE_AUTO_MERGE: ${{ github.event_name == 'schedule' || github.event_name == 'repository_dispatch' && github.event.client_payload.enable_auto_merge != false || inputs.enable_auto_merge == true }}
# toJSON(...) coercion fix -- see the TRIGGER_REVIEWS comment earlier
# in this env block for why the natural-looking `!= false` form
# silently defaults to disabled on repository_dispatch, and why the
# JSON string "false" needs its own clause alongside the boolean.
ORG_SWEEP_TRIGGER_REVIEWS: ${{ github.event_name == 'schedule' || github.event_name == 'repository_dispatch' && toJSON(github.event.client_payload.trigger_reviews) != 'false' && toJSON(github.event.client_payload.trigger_reviews) != '"false"' || inputs.trigger_reviews == true }}
ORG_SWEEP_ENABLE_AUTO_MERGE: ${{ github.event_name == 'schedule' || github.event_name == 'repository_dispatch' && toJSON(github.event.client_payload.enable_auto_merge) != 'false' && toJSON(github.event.client_payload.enable_auto_merge) != '"false"' || inputs.enable_auto_merge == true }}
ORG_SWEEP_MERGE_MODE: ${{ github.event.client_payload.merge_mode || inputs.merge_mode || 'direct_or_auto' }}
ORG_SWEEP_UPDATE_BRANCHES: ${{ github.event_name == 'schedule' || github.event_name == 'repository_dispatch' && github.event.client_payload.update_branches != false || inputs.update_branches == true }}
ORG_SWEEP_UPDATE_BRANCHES: ${{ github.event_name == 'schedule' || github.event_name == 'repository_dispatch' && toJSON(github.event.client_payload.update_branches) != 'false' && toJSON(github.event.client_payload.update_branches) != '"false"' || inputs.update_branches == true }}
ORG_SWEEP_STALE_QUEUE_HOURS: ${{ vars.ORG_SWEEP_STALE_QUEUE_HOURS || '24' }}
# The review-dispatch and branch-update budgets above are organization-wide
# per sweep tick (sized to bound LLM review-provider cost/rate exposure, not
Expand Down
15 changes: 15 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,21 @@ Semantic Versioning where the repository publishes a release.

### Fixed

- Made `pr-review-merge-scheduler.yml`'s `TRIGGER_REVIEWS`, `ENABLE_AUTO_MERGE`,
and `UPDATE_BRANCHES` (and their `ORG_SWEEP_` counterparts) actually default
to enabled on a `repository_dispatch`, as their `!= false` comparisons were
written to intend. GitHub Actions coerces both boolean `false` and an
absent/`null` `client_payload` property to `0` for a mixed-type `!=`
comparison, so a targeted self-service dispatch that simply omitted one of
these fields (rather than explicitly setting it to `false`) silently
produced the same `false` result as an explicit opt-out for that field
specifically — each of the three flags is independent, so a call that
set, say, `trigger_reviews: true` but omitted `enable_auto_merge` still
disabled only the merge step, not the whole dispatch. Replaced the direct
comparison with `toJSON(...) != 'false'`, an exact string comparison
unaffected by that coercion. Verified empirically: an identical dispatch
payload produced `TRIGGER_REVIEWS=false` before this fix and
`TRIGGER_REVIEWS=true` after, with no other change.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
- Publish only the sanitized cumulative Strix report tree, avoiding a later
copy of relative scanner output that could reintroduce known internal warning
text into uploaded security evidence.
Expand Down
18 changes: 18 additions & 0 deletions docs/doctoring/agent-mention-concurrency-isolation.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,24 @@ Trusted `@opencode-agent` comments could remain unacknowledged and fail to start

None of these defects is evidence that the requesting maintainer, model, repository allowlist, or final review result is invalid.

**Follow-up — scheduler-side root cause of item 4 closed.** The wrapper-side
workaround above (always send `trigger_reviews=true`, `enable_auto_merge`,
and `update_branches` explicitly) remains correct and in place, but it
addressed only one caller. `pr-review-merge-scheduler.yml`'s
`TRIGGER_REVIEWS`/`ENABLE_AUTO_MERGE`/`UPDATE_BRANCHES` env-block expressions
(and their `ORG_SWEEP_` counterparts) used `client_payload.<field> != false`,
which GitHub Actions evaluates identically for an explicit `false` and for a
field the payload never mentions at all (both coerce to `0` on a mixed-type
`!=` comparison) — so *any* `repository_dispatch` caller that omitted one of
these three fields silently got the disabled behavior, not only this
wrapper. Fixed at the scheduler by comparing `toJSON(client_payload.<field>)
!= 'false'` instead, an exact string comparison the `null`/`false` coercion
cannot collapse. Verified empirically against a live dispatch: an identical
payload produced `TRIGGER_REVIEWS=false` before the fix and `true` after.
Every existing explicit-payload caller (including this wrapper) is
unaffected; a caller that now omits the field gets the originally-intended
default instead of a silent no-op.

## Test-first repair

The permanent regression contracts were committed before their corresponding production changes.
Expand Down
6 changes: 3 additions & 3 deletions scripts/ci/test_strix_quick_gate.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1531,11 +1531,11 @@ assert_pr_review_merge_scheduler_uses_github_actions_bot_token() {
assert_file_contains "$workflow_file" "ORG_SWEEP_ENABLE_AUTO_MERGE: \${{ github.event_name == 'schedule' ||" "scheduled organization sweeps merge approved current heads"
assert_file_contains "$workflow_file" "ORG_SWEEP_UPDATE_BRANCHES: \${{ github.event_name == 'schedule' ||" "scheduled organization sweeps refresh eligible stale branches"
assert_file_contains "$workflow_file" 'github.event.workflow_run.pull_requests[0].number' "scheduler scopes OpenCode workflow_run events to the completed review PR"
assert_file_contains "$workflow_file" "github.event.client_payload.trigger_reviews != false" "scheduler enables review dispatch by default for default-branch dispatch events"
assert_file_contains "$workflow_file" "toJSON(github.event.client_payload.trigger_reviews) != 'false'" "scheduler enables review dispatch by default for default-branch dispatch events"
assert_file_contains "$workflow_file" "github.event_name == 'workflow_run' || github.event_name == 'push'" "scheduler can dispatch a bounded follow-up OpenCode review after review workflow completion"
assert_file_contains "$workflow_file" "github.event_name == 'push' || github.event_name == 'pull_request_target'" "scheduler treats base-branch pushes as queue-maintenance events"
assert_file_contains "$workflow_file" "github.event.client_payload.enable_auto_merge != false" "scheduler enables auto-merge by default for default-branch dispatch events"
assert_file_contains "$workflow_file" "github.event_name == 'workflow_run' || (github.event_name == 'repository_dispatch' && github.event.client_payload.update_branches != false) || inputs.update_branches == true" "scheduler enables branch updates after review completion or an explicit default-branch dispatch"
assert_file_contains "$workflow_file" "toJSON(github.event.client_payload.enable_auto_merge) != 'false'" "scheduler enables auto-merge by default for default-branch dispatch events"
assert_file_contains "$workflow_file" "github.event_name == 'workflow_run' || (github.event_name == 'repository_dispatch' && toJSON(github.event.client_payload.update_branches) != 'false' && toJSON(github.event.client_payload.update_branches) != '\"false\"') || inputs.update_branches == true" "scheduler enables branch updates after review completion or an explicit default-branch dispatch"
assert_file_contains "$workflow_file" "review_dispatch_limit:" "scheduler exposes a bounded review dispatch budget"
assert_file_contains "$workflow_file" "REVIEW_DISPATCH_LIMIT_INPUT" "scheduler forwards the review dispatch budget to the canonical script"
assert_file_contains "$workflow_file" 'review_dispatch_limit="-1"' "scheduler dispatches every eligible same-head review or Strix evidence job immediately unless an explicit budget overrides it"
Expand Down
24 changes: 21 additions & 3 deletions tests/test_required_workflow_queue_contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -1149,20 +1149,38 @@ def test_org_queue_sweep_manual_cadence_inputs_reach_the_sweep_job() -> None:
"ORG_SWEEP_MAX_PRS: ${{ github.event.client_payload.max_prs || inputs.max_prs || vars.ORG_SWEEP_MAX_PRS || '1000' }}"
) in workflow
assert (
"ORG_SWEEP_TRIGGER_REVIEWS: ${{ github.event_name == 'schedule' || github.event_name == 'repository_dispatch' && github.event.client_payload.trigger_reviews != false || inputs.trigger_reviews == true }}"
"ORG_SWEEP_TRIGGER_REVIEWS: ${{ github.event_name == 'schedule' || github.event_name == 'repository_dispatch' && toJSON(github.event.client_payload.trigger_reviews) != 'false' && toJSON(github.event.client_payload.trigger_reviews) != '\"false\"' || inputs.trigger_reviews == true }}"
in workflow
)
assert (
"ORG_SWEEP_ENABLE_AUTO_MERGE: ${{ github.event_name == 'schedule' || github.event_name == 'repository_dispatch' && github.event.client_payload.enable_auto_merge != false || inputs.enable_auto_merge == true }}"
"ORG_SWEEP_ENABLE_AUTO_MERGE: ${{ github.event_name == 'schedule' || github.event_name == 'repository_dispatch' && toJSON(github.event.client_payload.enable_auto_merge) != 'false' && toJSON(github.event.client_payload.enable_auto_merge) != '\"false\"' || inputs.enable_auto_merge == true }}"
) in workflow
assert (
"ORG_SWEEP_MERGE_MODE: ${{ github.event.client_payload.merge_mode || inputs.merge_mode || 'direct_or_auto' }}"
in workflow
)
assert (
"ORG_SWEEP_UPDATE_BRANCHES: ${{ github.event_name == 'schedule' || github.event_name == 'repository_dispatch' && github.event.client_payload.update_branches != false || inputs.update_branches == true }}"
"ORG_SWEEP_UPDATE_BRANCHES: ${{ github.event_name == 'schedule' || github.event_name == 'repository_dispatch' && toJSON(github.event.client_payload.update_branches) != 'false' && toJSON(github.event.client_payload.update_branches) != '\"false\"' || inputs.update_branches == true }}"
in workflow
)
# Neither the top-level nor the org-sweep variant of these three flags
# may use the bare `!= false` form: GitHub Actions coerces an absent
# repository_dispatch payload field and boolean `false` to the same
# value (0) for a mixed-type comparison, so `!= false` is false --
# meaning "not explicitly disabled" -- for both a payload that
# explicitly disabled the flag AND one that never mentioned it at all.
# Verified empirically against a live dispatch before this fix. Nor may
# either variant omit the JSON-string-"false" clause: `gh api -f
# field=false` sends a JSON string, not a boolean, and toJSON of that
# string is '"false"' (quotes included) -- distinct from toJSON(false)
# == 'false' -- so a bare `!= 'false'` alone would leave a naive `-f`
# caller's explicit disable silently ignored (CodeRabbit finding on #1238).
assert "client_payload.trigger_reviews != false" not in workflow
assert "client_payload.enable_auto_merge != false" not in workflow
assert "client_payload.update_branches != false" not in workflow
assert 'toJSON(github.event.client_payload.trigger_reviews) != \'"false"\'' in workflow
assert 'toJSON(github.event.client_payload.enable_auto_merge) != \'"false"\'' in workflow
assert 'toJSON(github.event.client_payload.update_branches) != \'"false"\'' in workflow
assert 'if [ "$ORG_SWEEP_TRIGGER_REVIEWS" = "true" ]; then' in workflow
assert 'if [ "$ORG_SWEEP_ENABLE_AUTO_MERGE" = "true" ]; then' in workflow
assert '--merge-mode "$ORG_SWEEP_MERGE_MODE"' in workflow
Expand Down
Loading