From 3bc646f6ffa6fbc9cac95b59d05d0fb26cb92a91 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 23 Aug 2026 00:16:53 +0900 Subject: [PATCH 1/4] fix(scheduler): stop repository_dispatch defaulting review/merge/branch flags off TRIGGER_REVIEWS, ENABLE_AUTO_MERGE, and UPDATE_BRANCHES (and their ORG_SWEEP_ variants) used `client_payload. != false`. GitHub Actions coerces both an explicit `false` and an absent/null client_payload property to 0 for a mixed-type `!=` comparison, so the expression is false -- not true, as the "default enabled unless explicitly disabled" naming implies -- whenever a repository_dispatch payload simply omits the field. Every targeted self-service dispatch that didn't spell out all three flags as `true` silently no-op'd on review/merge/branch-update. This is the scheduler-side root cause behind item 4 in docs/doctoring/agent-mention-concurrency-isolation.md's incident writeup, which previously worked around it only for the OpenCode mention wrapper by having that one caller always send trigger_reviews=true explicitly. Fixed at the source with toJSON(client_payload.) != 'false', an exact string comparison unaffected by the null/false coercion. Verified empirically: an identical dispatch payload produced TRIGGER_REVIEWS=false before this fix and true after, with no other change. Co-Authored-By: Claude Sonnet 5 --- .../workflows/pr-review-merge-scheduler.yml | 28 +++++++++++++++---- CHANGELOG.md | 14 ++++++++++ .../agent-mention-concurrency-isolation.md | 18 ++++++++++++ .../test_required_workflow_queue_contract.py | 16 +++++++++-- 4 files changed, 67 insertions(+), 9 deletions(-) diff --git a/.github/workflows/pr-review-merge-scheduler.yml b/.github/workflows/pr-review-merge-scheduler.yml index 697038d1c..02f69491e 100644 --- a/.github/workflows/pr-review-merge-scheduler.yml +++ b/.github/workflows/pr-review-merge-scheduler.yml @@ -157,12 +157,25 @@ 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' 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. + 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') || 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(...) != 'false' 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') || 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(...) != 'false' 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') || inputs.update_branches == true }} 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 @@ -606,10 +619,13 @@ 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(...) != 'false' 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. + ORG_SWEEP_TRIGGER_REVIEWS: ${{ github.event_name == 'schedule' || github.event_name == 'repository_dispatch' && 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' || 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' || 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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bc40394c..a731d709f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,20 @@ 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 — meaning every + review-dispatch/auto-merge/branch-update self-service call that did not + spell out all three flags as `true` quietly no-op'd. 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. - Retried the Strix scan up to `STRIX_TRANSIENT_RETRY_PER_MODEL` times, same model, when the log shows the upstream strix-agent Caido sandbox bootstrap timing race (`loginAsGuest failed after N attempts` / `Failed to connect to 127.0.0.1 port `; tracked upstream as usestrix/strix#1036, #1037, #1056). A slow CI runner can exceed strix-agent's fixed 10-attempt sandbox-login budget before its local intercepting proxy is reachable, even though the penetration test itself never started and no vulnerability evidence was produced or lost; the Docker image is already cached from the failed attempt, so a same-model retry is cheap and typically clears the one-off boot race. Not wired into cross-model fallback, since switching LLM models cannot change local sandbox container boot timing. - Replaced nonexistent `job.workflow_repository` / `job.workflow_sha` / `job.workflow_ref` / `job.workflow_file_path` context references (actionlint: "property ... is not defined in object type") in `pr-review-fix-scheduler.yml`'s called-workflow source verification and `exact-artifact-sbom-attestation.yml`'s trusted-verifier checkout. Both always failed closed on the missing properties (ContextualWisdomLab/.github#1212) or, for the SBOM attestation checkout, silently resolved an empty repository/ref instead of the pinned trusted source (downstream `gh attestation verify --signer-repo`/`--signer-workflow`, using the separately hardcoded `SIGNER_REPOSITORY` constant rather than any workflow_ref, still failed closed on the resulting empty signer identity). `github.workflow_ref`/`github.workflow_sha` are real, documented properties, but for a `workflow_call` target they reflect the top-level *calling* workflow, not the reusable workflow's own file — a prefix match against the reusable workflow's own path can never succeed. `exact-artifact-sbom-attestation.yml`'s checkout now uses `github.workflow_sha` (correct today: it has no callers yet); `pr-review-fix-scheduler.yml`'s identity check instead validates `github.repository`, since every current caller uses a local, same-repo `uses: ./...` where caller and callee share one commit and `github.workflow_sha` is still the right pin. Tracked follow-up for the SBOM attestation checkout once a real (potentially cross-repo) caller exists: ContextualWisdomLab/.github#1228. - Used the receiving repository's workflow token for same-repository scheduler diff --git a/docs/doctoring/agent-mention-concurrency-isolation.md b/docs/doctoring/agent-mention-concurrency-isolation.md index 1beacd4f8..4da547e32 100644 --- a/docs/doctoring/agent-mention-concurrency-isolation.md +++ b/docs/doctoring/agent-mention-concurrency-isolation.md @@ -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. != 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.) +!= '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. diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index b440bc5b9..79a4ee792 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -882,20 +882,30 @@ 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' || 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' || 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' || 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. + 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 '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 From 7724e15485231db215662a3b9398d7b96ad2812d Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 23 Aug 2026 01:24:06 +0900 Subject: [PATCH 2/4] fix(ci): update strix quick-gate literal-match assertions for the toJSON fix scripts/ci/test_strix_quick_gate.sh asserts the scheduler workflow's raw text contains specific literal expressions as an invariant, independent of the pytest suite. Its trigger_reviews/enable_auto_merge/update_branches assertions still matched the old, buggy `client_payload. != false` substring removed in 3bc646f6, so the gate failed after that fix even though the underlying behavior is now correct (OpenCode Review flagged this as a same-head Checks failure on PR #1238). Updated the three assertions to match the new toJSON(client_payload.) != 'false' expressions. Verified: `bash scripts/ci/test_strix_quick_gate.sh` -> test_strix_quick_gate: PASS. Co-Authored-By: Claude Sonnet 5 --- scripts/ci/test_strix_quick_gate.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index 5a37ffc0c..49e710864 100755 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -1514,11 +1514,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') || 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" From cb64eb329b9729a6362775dc8ab87bf545618214 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 23 Aug 2026 01:51:17 +0900 Subject: [PATCH 3/4] docs(changelog): correct per-flag independence claim (CodeRabbit finding on #1238) The Fixed entry implied a call that didn't set all three flags to true was entirely a no-op. Each flag is independent: omitting one disables only that one operation, not the whole dispatch (e.g. explicit trigger_reviews=true with enable_auto_merge omitted still ran review dispatch, just not merge). Co-Authored-By: Claude Sonnet 5 --- CHANGELOG.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a731d709f..d343b0496 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,9 +52,10 @@ Semantic Versioning where the repository publishes a release. 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 — meaning every - review-dispatch/auto-merge/branch-update self-service call that did not - spell out all three flags as `true` quietly no-op'd. Replaced the direct + 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 From 23f022c9863c66dadc60fdf21fb3da91965d350c Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 23 Aug 2026 07:05:38 +0900 Subject: [PATCH 4/4] fix(scheduler): also reject the JSON string "false" for the three dispatch flags toJSON(client_payload.) != 'false' correctly rejects the boolean false and the absent/null case, but a repository_dispatch payload sent via `gh api -f field=false` (as opposed to `-F`) carries the JSON *string* "false", not the boolean. toJSON of that string is '"false"' (quotes included), which is != 'false', so the flag stayed enabled -- a naive `-f` caller's explicit disable was silently ignored (CodeRabbit review finding on #1238). Added a second toJSON(...) != '"false"' clause to all six TRIGGER_REVIEWS/ENABLE_AUTO_MERGE/UPDATE_BRANCHES expressions (top-level and ORG_SWEEP_ variants) so both encodings of "false" are treated as disabled. Updated the matching exact-string test assertions and the separate scripts/ci/test_strix_quick_gate.sh literal-match gate. Co-Authored-By: Claude Sonnet 5 --- .../workflows/pr-review-merge-scheduler.yml | 50 +++++++++++-------- scripts/ci/test_strix_quick_gate.sh | 2 +- .../test_required_workflow_queue_contract.py | 16 ++++-- 3 files changed, 41 insertions(+), 27 deletions(-) diff --git a/.github/workflows/pr-review-merge-scheduler.yml b/.github/workflows/pr-review-merge-scheduler.yml index e2cb5b932..d3695478d 100644 --- a/.github/workflows/pr-review-merge-scheduler.yml +++ b/.github/workflows/pr-review-merge-scheduler.yml @@ -157,25 +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 || '' }} - # 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. - 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') || 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' }} - # Same toJSON(...) != 'false' 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') || 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' }} - # Same toJSON(...) != 'false' 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') || 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 }} 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 @@ -619,13 +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' }} - # toJSON(...) != 'false' 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. - ORG_SWEEP_TRIGGER_REVIEWS: ${{ github.event_name == 'schedule' || github.event_name == 'repository_dispatch' && 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' || 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' && toJSON(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 diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index 49e710864..505ae72b0 100755 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -1518,7 +1518,7 @@ assert_pr_review_merge_scheduler_uses_github_actions_bot_token() { 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" "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') || inputs.update_branches == true" "scheduler enables branch updates after review completion or an explicit default-branch dispatch" + 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" diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index 01570d8b7..8b7d44b74 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -1120,18 +1120,18 @@ 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' && toJSON(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' && toJSON(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' && toJSON(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 @@ -1140,10 +1140,18 @@ def test_org_queue_sweep_manual_cadence_inputs_reach_the_sweep_job() -> None: # 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. + # 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