mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-08-27 01:23:52 +00:00
2a57f86198
* fix(ci): gate the fork signal on fields the review payload delivers `qwen-autofix-fork-signal.yml` gated on `github.event.pull_request.maintainer_can_modify == true`. That field does not exist in a `pull_request_review` payload: the event carries the SIMPLE pull-request object, and `maintainer_can_modify` — like `mergeable`, `additions`, and `changed_files` — ships only on the full object the `pull_request` event sends. The expression evaluated to null on every delivery, `null == true` is false, and the job's `if` could never hold. Measured on the repository: across the 300 runs between the bridge shipping (#8676, 2026-08-07) and this change, 290 skipped, 7 cancelled, 1 action_required, and 0 success. Not one signal ever reached its step, so the bridge behind it has never fired either — every fork-PR review has been served by the scheduled scan alone, which is exactly the throttled backstop this bridge exists to get ahead of. The consent check is not lost, and does not move: the bridge already re-reads it live (`gh pr view --json maintainerCanModify`, then `select(… .maintainerCanModify == true)`), and that read was always the authoritative one — consent can be withdrawn between the review and the dispatch, so a payload copy could only ever have been a stale early-out. The signal job cannot make that call itself: it holds `permissions: {}`, no secrets and no checkout, deliberately, because it runs on a fork-triggered event. What the removal does cost is one signal + bridge run and one PR read for a takeover-labeled fork PR whose author has turned maintainer edits off, where the gate previously intended to spend nothing. Ordinary contributor fork PRs are unaffected — the bot-authored-or-takeover-labeled conjunct still excludes them. Also adds a regression test asserting the gate references no full-object-only field. A gate like this fails silently: the job's entire body is one echo, so "never opens" and "no fork review happened to qualify" look identical from outside, which is why this went twelve days unnoticed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(ci): match the full-object-only guard on a word boundary (R1-1) The guard asserted the signal gate references no full-object-only field via bare substring containment, but four deny-list entries are strict prefixes of fields the simple pull-request object DOES deliver: `merged` in `merged_at`, `commits` in `commits_url`, `comments` in `comments_url`, `review_comments` in `review_comments_url`. A future edit adding a legal conjunct such as `github.event.pull_request.merged_at == null` would turn the suite red with a message blaming a full-object-only field — pushing the author to drop the conjunct or weaken the guard itself. Anchor each check on a word boundary, and add a test that pins the matcher's discrimination on all four prefix pairs so the substring form cannot come back unnoticed. * test(ci): see full-object fields through the index operator (R2-1) The full-object-only guard matched `pull_request.<field>` literally, so it only saw the `.` property de-reference. GitHub Actions reaches the same property through the documented `[ ]` index operator, on any segment of the path, and `github.event.pull_request['maintainer_can_modify'] == true` evaluates exactly as the dot form did: the field is absent from the simple `pull_request_review` payload, `null == true` is false, and the gate never opens. The guard stayed green through it — a silent replay of the incident this PR fixes, invisible to the test written to catch it. Rewrite the index form to the dot form before matching rather than enumerating spellings, so one matcher covers every combination of the two at any depth. The word-boundary anchor from R1-1 is unchanged, so the four delivered fields the deny-list names prefix (`merged_at`, `commits_url`, `comments_url`, `review_comments_url`) still pass in every spelling. A `fromJSON(toJSON(github.event.pull_request))` round-trip still evades this; no textual guard catches that one, and the comment says so. Mutation-verified, each mutant reddening the tests that pin it: | mutant | result | |---|---| | normalization removed (identity) | 2 failed — bracket and mixed spellings go unseen | | word boundary dropped | 1 failed — `merged_at` rejected as `merged` | `npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-autofix-fork-bridge-workflow.test.js` -> 12 passed (12). eslint and prettier clean. * test(ci): pin the fork-signal guard's bracket-whitespace tolerance R3-1: `asDotAccess` rewrites `pull_request['field']` to the dot form before matching, and its regex deliberately tolerates whitespace inside the index (`\[\s*…\s*\]`) because GitHub Actions accepts `github.event.pull_request[ 'maintainer_can_modify' ]` as a legal expression. No spelling in `referenceSpellings` carried that whitespace, so the tolerance was unpinned: deleting both `\s*` left all 12 tests green, and a later gate edit written in the spaced form would have reached the same absent field and restored the always-false gate this PR fixes. Add the spaced-bracket spelling, which both index-operator tests consume. Mutation-verified: with the two `\s*` deleted from `asDotAccess`, this file now fails 2 tests ("rejects a full-object field without rejecting the fields it prefixes" and "sees a full-object field through the index operator"); before this commit the same mutation left 12 passed. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|---|---|---|
| .. | ||
| actions | ||
| assets | ||
| ISSUE_TEMPLATE | ||
| scripts | ||
| workflows | ||
| actionlint.yaml | ||
| CODEOWNERS | ||
| dependabot.yml | ||
| issue-owners.json | ||
| pull_request_template.md | ||
| release.yml | ||
| spam-blocklist.txt | ||