feat(autofix): bridge fork-PR reviews into the credentialed review lane - #8676
Conversation
A workflow run tied to a pull request whose head lives in a fork is
created with `Secret source: None`, so `secrets.CI_DEV_BOT_PAT` is empty
and the autofix review lane cannot authenticate. Those reviews therefore
wait for the scheduled scan, which GitHub throttles hard — the `*/10`
cron lands every 40-70 minutes on this repo.
Reading the PR with `github.token` instead would not help: review-address
still needs the PAT to push and comment, and it is empty for the whole
run. The credential has to come from a different run.
This is the standard `workflow_run` bridge for that.
- `qwen-autofix-fork-signal.yml` runs on `pull_request_review`, where the
fork PR's event can reach it, and records only which PR was reviewed.
It is deliberately powerless: `permissions: {}`, no checkout, no
repository code, and the hosted pool rather than the persistent
self-hosted one.
- `qwen-autofix-fork-bridge.yml` runs on `workflow_run`, which GitHub
creates on the default branch with the repository's own token, and
dispatches `qwen-autofix.yml` for that PR using `actions: write` — the
same lever Fleet Shepherd already pulls. It holds no PAT.
The dispatched run re-derives admission from live API state, so the PR
number says when to look, never who may be touched. It is bound to
`workflow_run.head_sha` regardless: the signal run carries the reviewed
PR's head SHA, and only that PR has that head.
Measured, not assumed:
- `workflow_run.pull_requests` is empty for fork PRs, and
`/commits/{sha}/pulls` does not resolve a fork head either (it does
resolve an in-repo one), so the artifact is the only way to learn the
number.
- `pull_request_review` runs the workflow file from the BASE branch: run
31152873061's PR branch predates a main-only change to
`qwen-autofix.yml` and it still executed the new file. A fork cannot
edit what the signal writes.
- That run's `head_sha` equals PR #8436's `headRefOid` exactly, which is
what makes the binding possible.
The bridge's validate-and-dispatch block is replayed under bash against a
stub API: forgery, an unreadable head, both heads empty, non-numeric and
path-shaped artifacts, a closed or retargeted PR, download and read
failures, and dispatch retry then exhaustion.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
@wenshao The content here is unusually thorough — the problem, the design, and the verification are all well documented — but the body skips the PR template entirely, so this is a formatting gate, not a code concern.
All five required headings from pull_request_template.md are missing: ## What this PR does, ## Why it's needed, ## Reviewer Test Plan (with ### How to verify, ### Evidence (Before & After), ### Tested on), ## Risk & Scope, and ## Linked Issues. Your recent merged PRs — including the CI-only ones — all use this template, so this one stands out.
Could you restructure the body under those headings? The existing sections map across cleanly: Summary → What this PR does; the Secret source: None problem and the 40-70 minute cron latency → Why it's needed; Verification → Reviewer Test Plan (Before/After is N/A since workflow_run cannot fire before merge — the stub-API replay and the 12/12 mutation results are the evidence); the safety analysis plus "Deliberately not done" → Risk & Scope. Please put the dependency on #8671 and the intended merge ordering into Linked Issues — that is the piece a merging maintainer most needs to see in the standard place.
Once the body follows the template, re-run me and I'll take it through the full review.
中文说明
@wenshao 这个 PR 的内容非常扎实——问题、设计和验证都写得很清楚——但正文完全没有使用 PR 模板，所以这是一次格式上的拦截，而不是对代码的质疑。
pull_request_template.md 要求的五个标题全部缺失：## What this PR does、## Why it's needed、## Reviewer Test Plan（含 ### How to verify、### Evidence (Before & After)、### Tested on）、## Risk & Scope、## Linked Issues。你最近合入的 PR——包括纯 CI 类的——都使用了这个模板，这一个因此显得例外。
能否把正文重新组织到这些标题下？现有章节可以直接对应：Summary → What this PR does；Secret source: None 问题和 40-70 分钟的 cron 延迟 → Why it's needed；Verification → Reviewer Test Plan（Before/After 写 N/A，因为 workflow_run 在合入前无法触发——stub API 回放和 12/12 变异测试结果就是证据）；安全性分析加上「刻意没有做的事」→ Risk & Scope。请把对 #8671 的依赖以及预期的合入顺序写进 Linked Issues——这是合入时 maintainer 最需要在标准位置看到的信息。
正文符合模板后重新触发我，我会完整走一遍审查流程。
— Qwen Code · qwen3.8-max
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
Code review — fork-review bridge (high effort)The three new fork-bridge workflows add a signal → bridge → Correctness1.
2.
3.
4. Concurrency is claimed at run creation, before the job 5.
6. route's concurrency group ( Cleanups7.
8. Lines 6-9 say the review lane "now skips itself for those PRs (see route's 9. The bridge accepts the artifact's number only after confirming it maps to 10.
Four further candidates were refuted during verification and are not listed: a claimed runner-pool switch under |
|
@qwen-code /review |
| _Qwen Code review request accepted. Review is queued in [workflow run](https://github.com/QwenLM/qwen-code/actions/runs/31160501077)._ |
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
…dged dispatches Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： AutoFix review round — summaryOne commit: Code review findings (@wenshao) — all fixed
All fixes carry regression tests: the stub-API replay now exercises the head-SHA resolution (happy path, benign push, each withdrawn consent, skip-wins, shared head, enumeration failure, dispatch retry/exhaustion), the cap-refusal and shepherd correlation guards are extracted VERBATIM and replayed under bash, and the concurrency/input wiring is pinned cross-file. The two pre-existing workflow suites were updated where they pinned the exact lines this round changed. PR body template (@qwen-code-ci-bot, CHANGES_REQUESTED) — needs a maintainer editThis one is not a code defect and this flow cannot fix it: address-review mode has no PR-body edit hook and holds no GitHub credentials, so Proposed PR body (paste-ready)What this PR doesFork PRs cannot receive repository secrets on their own review events ( Why it's neededA fork PR's review event carries no secrets, so the review lane cannot authenticate and reddens out after three failed API calls; the only other pickup is the scheduled scan, whose Reviewer Test PlanHow to verify
Evidence (Before & After)N/A — non-user-visible workflow change; Tested on
Environment (optional)Node 22, vitest via Risk & Scope
Linked Issues
中文说明这个 PR 做了什么Fork PR 自己的 review 事件拿不到仓库 secrets（ 为什么需要Fork PR 的 review 事件不带 secrets，review 通道无法认证，三次 API 调用失败后变红；唯一的兜底是定时扫描，而本仓库的 Reviewer 测试计划如何验证
证据（前后对比）N/A —— 非用户可见的 workflow 变更； 测试环境
环境（可选）Node 22，vitest 通过 风险与范围
关联 Issue
VerificationCommands actually run this round (all on the final tree, commit
中文说明AutoFix review 轮次 —— 总结一个 commit： 代码 review 发现（@wenshao）—— 全部修复
所有修复都带回归测试：stub API 回放现在演练 head SHA 解析（正常路径、良性 push、各种撤回同意、skip 优先、共享 head、枚举失败、dispatch 重试/耗尽），cap 拒绝与 shepherd 关联守卫被原样提取并在 bash 下回放，并发/输入接线被跨文件钉住。两个既有 workflow 套件中钉住本轮所改行的断言也已同步更新。 PR 正文模板（@qwen-code-ci-bot，CHANGES_REQUESTED）—— 需要 maintainer 编辑这一项不是代码缺陷，且本流程无法修复：address-review 模式没有编辑 PR 正文的钩子，也不持有任何 GitHub 凭证，因此 （可粘贴的 PR 正文草案见上方英文部分的折叠块；草案自带按仓库惯例的中文翻译折叠块。） 验证本轮实际运行的命令（均在最终代码树、commit
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
.github/workflows/qwen-autofix-fork-bridge.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix-fork-signal.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-fleet-shepherd.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
.github/workflows/qwen-autofix-fork-bridge.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix-fork-signal.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-fleet-shepherd.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— gpt-5.6-sol via Qwen Code /review (v0.21.7)
| if [[ "${COUNT}" == "1" ]]; then | ||
| LIVENESS_RUN_OUT="$(jq -r '.[0].databaseId // "" | tostring' <<< "${CANDIDATES}")" | ||
| break |
There was a problem hiding this comment.
[Critical] R1-1: The first singleton visible after DISPATCH_T0 is not necessarily the shepherd run. GitHub may expose a concurrent fork-bridge dispatch before the shepherd's own run; this branch records the foreign ID and stops polling. On the next tick, a long-running foreign scan is treated as liveness work and can suppress the watchdog even if the real liveness scan has completed or failed. Correlate the dispatch with an explicit unique value, or at minimum wait through a stabilization window instead of accepting the first singleton.
中文说明
问题： DISPATCH_T0 之后最先可见的单个候选不一定是 shepherd 自己的 run。GitHub 可能先暴露并发的 fork-bridge dispatch、稍后才暴露 shepherd 自己的 run；这里会记录外部 run ID 并立即停止轮询。下一 tick 会把长时间运行的外部 scan 当作 liveness 工作，从而在真正的 liveness scan 已完成或失败时仍抑制 watchdog。应使用显式唯一值关联 dispatch，或至少等待稳定窗口，而不是接受首次 singleton。
— gpt-5.6-sol via Qwen Code /review (v0.21.7)
| # Two open fork PRs sharing one head SHA is already ambiguous — the | ||
| # review could have targeted either — so every match is dispatched; | ||
| # admission is re-derived per PR downstream. | ||
| for PR in $(jq -r '.[]' <<< "${MATCHES}"); do |
There was a problem hiding this comment.
[Critical] R1-2: A head SHA authenticates the revision, not which PR generated the review. If managed fork PR A and PR B share a commit, a review submitted only on A dispatches both. B's downstream scan then reads B's own pending reviews/comments and may post or push even though this event never targeted B; live admission proves B is manageable, not that the review belonged to B. Preserve a GitHub-owned PR binding, or fail closed when multiple PRs match.
中文说明
问题： head SHA 只能证明代码版本，不能证明是哪一个 PR 产生了 review。如果两个 managed fork PR 共享同一提交，只对 PR A 提交 review 也会同时 dispatch A 和 B。B 的下游 scan 随后会读取 B 自己待处理的 review/comment，并可能发评论或推送；实时准入只能证明 B 可被管理，不能证明本次事件指向 B。应保留 GitHub 所有的 PR 身份绑定；若多个 PR 匹配则失败关闭。
— gpt-5.6-sol via Qwen Code /review (v0.21.7)
| source: | ||
| description: 'Dispatch-origin marker (fork-bridge = the fork-review bridge); routing metadata only, never changes what may be touched' | ||
| required: false | ||
| type: 'string' |
There was a problem hiding this comment.
[Critical] R1-3: source=fork-bridge changes trusted concurrency and cap-refusal semantics, but it is an unrestricted public workflow_dispatch input. A maintainer can manually supply this value, causing explicit human dispatches to cancel/coalesce per PR and suppress the refusal response even though the bridge did not originate them. Use a structurally distinct trusted entry point such as workflow_call, or verify bridge provenance independently before honoring this marker.
中文说明
问题： source=fork-bridge 会改变并发与 cap-refusal 语义，但它只是一个不受限制的公开 workflow_dispatch 输入。maintainer 手动填入该值后，明确的人工作业会按 PR 相互取消/合并，并抑制拒绝回复，即使它并非由 bridge 发起。应使用结构上独立的可信入口（如 workflow_call），或在采用该标记前独立验证 bridge 来源。
— gpt-5.6-sol via Qwen Code /review (v0.21.7)
| && (contains(fromJSON('["OWNER", "MEMBER", "COLLABORATOR"]'), github.event.review.author_association) | ||
| || github.event.review.user.login == 'qwen-code-ci-bot') }} |
There was a problem hiding this comment.
[Critical] R1-4: MEMBER and COLLABORATOR association do not guarantee live repository write permission. A read/triage collaborator can pass this signal gate and launch the secret-bearing lane, while the direct pull_request_review route explicitly rejects the same reviewer unless the live permission is write|maintain|admin. Preserve the reviewer identity and perform the same live permission check in the privileged bridge, retaining the review-bot exception.
中文说明
问题： MEMBER 和 COLLABORATOR association 并不保证当前拥有仓库 write 权限。只有 read/triage 权限的 collaborator 可以通过 signal 门并启动持有 secrets 的通道，而直接 pull_request_review route 会明确要求实时权限为 write|maintain|admin。应保留 reviewer 身份，并在有权限的 bridge 中执行相同的实时权限检查，同时保留 review bot 例外。
— gpt-5.6-sol via Qwen Code /review (v0.21.7)
| const refusedGuard = autofixText.match( | ||
| /(if \[\[ -n "\$\{FORCED_PR\}" && "\$\{FORCED_PR\}" == "\$\{PR\}" && "\$\{EVENT_NAME\}" == 'workflow_dispatch' && "\$\{DISPATCH_SOURCE\}" != 'fork-bridge' \]\]; then)/, | ||
| )?.[1]; |
There was a problem hiding this comment.
[Suggestion] R1-5: This suite extracts and behaviorally replays the complete cap-refusal guard that qwen-autofix-workflow.test.js already owns. A legitimate guard edit now requires updating two regex extractors and equivalent subprocess assertions, so one canonical test change can create a redundant failure here without adding independent coverage. Keep the behavioral replay in the autofix workflow suite and retain only bridge-specific cross-file source wiring assertions here.
中文说明
建议： 该套件完整提取并行为回放了 qwen-autofix-workflow.test.js 已经负责的 cap-refusal guard。一次合理的 guard 修改现在要同步维护两份正则提取器和等价 subprocess 断言；只更新 canonical 测试会在这里产生没有独立覆盖价值的重复失败。建议把行为回放留在 autofix workflow 套件，本文件仅保留 bridge 特有的跨文件 source 接线断言。
— gpt-5.6-sol via Qwen Code /review (v0.21.7)
| && github.event.pull_request.maintainer_can_modify == true | ||
| && (github.event.pull_request.user.login == (vars.AUTOFIX_BOT_LOGIN || 'qwen-code-dev-bot') | ||
| || contains(toJSON(github.event.pull_request.labels.*.name), '"autofix/takeover"')) |
There was a problem hiding this comment.
[Suggestion] R1-6: The signal omits the autofix/skip veto that the bridge applies later. A trusted review on an otherwise managed fork PR carrying the skip label still creates both workflow runs and performs the open-PR enumeration before being discarded, repeating known-useless work on every review. Add the skip-label exclusion to this gate and pin the copied label against qwen-autofix.yml in the cross-workflow test.
中文说明
建议： signal 没有包含 bridge 稍后才执行的 autofix/skip 否决。带 skip 标签的 managed fork PR 收到可信 review 后，仍会创建 signal 和 bridge 两个 workflow run，并执行 open-PR 枚举，最后才被丢弃；每次 review 都会重复这段已知无用工作。建议在这里加入 skip-label 排除，并在跨 workflow 测试中把该标签钉到 qwen-autofix.yml。
— gpt-5.6-sol via Qwen Code /review (v0.21.7)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-autofix-fork-bridge.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix-fork-signal.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-fleet-shepherd.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
[Critical] Still-standing blocker from the existing inline thread at .github/workflows/qwen-fleet-shepherd.yml:232-235 (comment 3735552469, prior review round): the singleton-candidate attribution can credit a foreign fork-bridge dispatch when the shepherd's own run is list-lagged — the mechanism is present in the code at HEAD and the new comment overclaims it. Re-checked this round against the code: not fixed. (This review's independent trace: the impact is bounded and self-heals when the foreign run completes — the LIVENESS_OUT watermark advances independently of run attribution, and a completed run drops out of SCAN_INFLIGHT — and the hole predates this diff in shape; this review rates the same mechanism a low-confidence Suggestion, but the defect as filed is present in the code, so the blocker stands.) 仍存在的阻塞：shepherd 关联循环在自身 run 延迟出现时可能把外部 fork-bridge dispatch 记为己有，该机制在 HEAD 代码中存在且新注释表述过度；本轮复核未修复。（本 review 的独立追踪：影响有界且在外部 run 结束时自愈——LIVENESS_OUT 水位与归因无关、已完成 run 不再计入 SCAN_INFLIGHT——且该漏洞形态早于本 diff；本 review 将同一机制评为低置信 Suggestion，但所报缺陷确在代码中，阻塞成立。）
中文说明
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-autofix-fork-bridge.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix-fork-signal.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-fleet-shepherd.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
[Critical] Still-standing blocker from the existing inline thread at .github/workflows/qwen-fleet-shepherd.yml:232-235 (comment 3735552469, prior review round): the singleton-candidate attribution can credit a foreign fork-bridge dispatch when the shepherd's own run is list-lagged — the mechanism is present in the code at HEAD and the new comment overclaims it. Re-checked this round against the code: not fixed. (This review's independent trace: the impact is bounded and self-heals when the foreign run completes — the LIVENESS_OUT watermark advances independently of run attribution, and a completed run drops out of SCAN_INFLIGHT — and the hole predates this diff in shape; this review rates the same mechanism a low-confidence Suggestion, but the defect as filed is present in the code, so the blocker stands.) 仍存在的阻塞：shepherd 关联循环在自身 run 延迟出现时可能把外部 fork-bridge dispatch 记为己有，该机制在 HEAD 代码中存在且新注释表述过度；本轮复核未修复。（本 review 的独立追踪：影响有界且在外部 run 结束时自愈——LIVENESS_OUT 水位与归因无关、已完成 run 不再计入 SCAN_INFLIGHT——且该漏洞形态早于本 diff；本 review 将同一机制评为低置信 Suggestion，但所报缺陷确在代码中，阻塞成立。）
— qwen3.8-max via Qwen Code /review (v0.21.7)
| # `qwen-autofix.yml` cannot authenticate. Merging this bridge REQUIRES #8671 | ||
| # first: that PR adds route's `secretless` output so the direct | ||
| # `pull_request_review` lane skips fork PRs instead of failing three API calls |
There was a problem hiding this comment.
[Suggestion] The header documents #8671 as adding route's secretless output — a mechanism #8671 never shipped. #8671 is already merged (63a99c2): it declines fork pull_request_review events inline in route's decide script and adds an empty-PAT fail-fast guard at the top of review-scan, and its commit message records the route-output design as the rejected alternative (grep -c secretless on origin/main's qwen-autofix.yml → 0). Compounding it, the new test pins the wrong word — expect(signalText).toContain('secretless') — so anyone correcting this comment fails the test.
Failure scenario: a future oncall verifying the bridge's premise greps for secretless, finds nothing, and either doubts a premise that is actually sound or resurrects a "missing" output that was deliberately rejected; the real mechanism stays undocumented and the test actively enforces the drift. Behaviorally the premise holds (the direct lane no longer reds), so this is documentation drift, not a functional defect.
Suggested fix: rewrite these lines to state what #8671 actually did (route declines fork reviews inline; review-scan fails fast on an empty PAT), drop the now-stale "Until #8671 lands" merge-ordering prose since it has landed, and replace the secretless assertion with one pinning the real mechanism.
中文说明
[Suggestion] 头部注释把 #8671 描述为给 route 增加 secretless 输出——而 #8671 从未交付这个机制。#8671 已经合入（63a99c2c11）：它在 route 的 decide 脚本中内联拒绝 fork 的 pull_request_review 事件，并在 review-scan 顶部增加空 PAT 快速失败守卫，其 commit message 明确记录了 route 输出方案是被否决的替代方案（对 origin/main 的 qwen-autofix.yml 执行 grep -c secretless → 0）。更糟的是，新测试把错误的词钉住了——expect(signalText).toContain('secretless')——任何人修正这段注释都会导致测试失败。
失败场景：未来某个 oncall 验证 bridge 前提时 grep secretless，一无所获，要么怀疑一个其实成立的前提，要么复活一个被刻意否决的「缺失」输出；真实机制始终没有文档，而测试在主动固化这种漂移。行为上前提成立（直接通道不再变红），所以这是文档漂移，不是功能缺陷。
建议修复：重写这几行，写明 #8671 实际做了什么（route 内联拒绝 fork review；review-scan 对空 PAT 快速失败），删掉已经过时的「在 #8671 落地前」合入顺序措辞（它已落地），并把 secretless 断言换成钉住真实机制的断言。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| expect(routeJob).toContain( | ||
| "cancel-in-progress: |-\n ${{ github.event_name != 'workflow_dispatch' }}", | ||
| "format('qwen-autofix-forkbridge-pr-{0}', inputs.pr_number)", | ||
| ); |
There was a problem hiding this comment.
[Suggestion] Route's concurrency group is pinned only by order-blind toContain substrings, so branch order in the || chain is unpinned. One of 4 instances of the same pattern in this review: workflow-expression connectives/branch order asserted only as substrings, with connective/order mutants proven to keep the whole suite green.
Failure scenario: proven mutation — moving the (github.event_name == 'workflow_dispatch' && inputs.source == 'fork-bridge' && ...) disjunct after the always-truthy format('qwen-autofix-route-{0}', github.run_id) fallback keeps both test files fully green. In that state the forkbridge branch never evaluates, every fork-bridge dispatch gets a run-unique group that cancels nothing, and a burst of reviews on one fork PR serializes N full autofix scans behind the per-PR lock — the exact regression this diff's own comment says it prevents.
Suggested fix: pin the order — e.g. assert the forkbridge format index precedes the run-id fallback's, or pin the whole group string with toBe.
中文说明
[Suggestion] route 的并发组只用顺序无关的 toContain 子串钉住，因此 || 链中的分支顺序未被钉住。这是本 review 中同一模式的 4 处之一：workflow 表达式的连接词/分支顺序只以子串断言，连接词/顺序类变异已被证实能让整个套件保持绿色。
失败场景：已证实的变异——把 (github.event_name == 'workflow_dispatch' && inputs.source == 'fork-bridge' && ...) 分支移到永真的 format('qwen-autofix-route-{0}', github.run_id) 兜底之后——两个测试文件仍然全绿。此时 forkbridge 分支永远不会被求值，每个 fork-bridge dispatch 落入一个不取消任何东西的 run-unique 组，一个 fork PR 上的 review 突发会在 per-PR 锁后串行化 N 次完整 autofix 扫描——正是本 diff 注释声称要防止的回归。
建议修复：钉住顺序——例如断言 forkbridge format 的下标先于 run-id 兜底，或用 toBe 钉住整个组字符串。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| expect(gate).toContain(`vars.AUTOFIX_BOT_LOGIN || '${BOT_FALLBACK}'`); | ||
| expect(gate).toContain(`'"${TAKEOVER_LABEL}"'`); |
There was a problem hiding this comment.
[Suggestion] The signal gate's Boolean composition is pinned only as condition substrings; the || between the bot-authored and takeover-labeled disjuncts is untested. One of 4 connective/order pinning gaps in this review, each proven by mutation.
Failure scenario: proven mutation — flipping that || to && in qwen-autofix-fork-signal.yml keeps all tests green. Under that mutant a human-authored fork PR carrying autofix/takeover is never signalled, so real-time pickup for takeover PRs — which the bridge-side behavioral replay (takeover scenario) explicitly proves the downstream half would serve — silently disappears, and only the scheduled-scan backstop remains. The two halves of the bridge disagree about takeover PRs and no test notices.
Suggested fix: pin the complete gate with expect(signalJob.if).toBe('<full expected gate>') — it is small and stable; one exact-string pin kills this whole mutation class (and covers the trust-clause connective tracked in the sibling thread).
中文说明
[Suggestion] signal 门的布尔组合只以条件子串钉住；bot 创建与 takeover 标签两个分支之间的 || 未被测试。这是本 review 中 4 处连接词/顺序钉住缺口之一，均经变异证实。
失败场景：已证实的变异——把 qwen-autofix-fork-signal.yml 中该 || 翻转为 &&——所有测试保持绿色。在该变异下，一个带 autofix/takeover 标签、人类创建的 fork PR 永远不会被 signal，于是 takeover PR 的实时接管——bridge 侧行为回放（takeover 场景）明确证明下游一半本可以服务它——悄悄消失，只剩定时扫描兜底。桥接的两半对 takeover PR 的判断不一致，而没有任何测试察觉。
建议修复：用 expect(signalJob.if).toBe('<完整期望的门表达式>') 钉住整个门——它小而稳定；一个精确字符串钉住即可消灭整类变异（同时覆盖姊妹线程中信任子句的连接词）。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| expect(gate).toContain('github.event.review.author_association'); | ||
| expect(gate).toContain(`github.event.review.user.login == '${REVIEW_BOT}'`); |
There was a problem hiding this comment.
[Suggestion] The trust clause's || (trusted-association OR review-bot login) in the job gate is unpinned: the tests constrain only the literals on each side (the identity-pinning test only counts/values them). One of 4 connective/order pinning gaps in this review, each proven by mutation.
Failure scenario: proven mutations — flipping this || to && makes trust require BOTH an in-list association AND the review-bot login — false for every human maintainer and for a bot whose association is not OWNER/MEMBER/COLLABORATOR; every trusted review then fails the gate and fork-PR reviews silently degrade to the throttled cron backstop while all tests stay green. Flipping the && joining the managed clause to the trust clause to || also escapes, letting any trusted review on any PR pass the gate.
Suggested fix: pin the clause shape verbatim (the full two-line trust clause) — the same verbatim standard this PR applies to the bash guards — or fold it into an exact-string pin of the whole gate.
中文说明
[Suggestion] job 门中信任子句的 ||（可信成员身份或 review-bot 登录名）未被钉住：测试只约束两侧的字面量（身份钉住测试也只统计其数量/取值）。这是本 review 中 4 处连接词/顺序钉住缺口之一，均经变异证实。
失败场景：已证实的变异——把该 || 翻转为 && 后，信任条件要求同时满足列表内成员身份与 review-bot 登录名——对每一位人类 maintainer、以及成员身份不在 OWNER/MEMBER/COLLABORATOR 列表内的 bot 都为假；所有可信 review 都无法过门，fork PR 的 review 悄悄退化为被限流的 cron 兜底，而所有测试保持绿色。把连接 managed 子句与信任子句的 && 翻转为 || 同样存活，会使任何 PR 上的任何可信 review 都通过门。
建议修复：逐字钉住该子句的完整两行形态——与本 PR 对 bash 守卫采用的逐字标准一致——或并入对整个门的精确字符串钉住。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| expect(signalGroup).toContain( | ||
| "format('qwen-autofix-fork-signal-pr-{0}', github.event.pull_request.number)", | ||
| ); |
There was a problem hiding this comment.
[Suggestion] The conditional glue separating trusted (per-PR) from untrusted (run-unique) signal concurrency groups is unpinned — this test asserts only that both format(...) literals are present, not the &&/|| structure routing reviews between them. One of 4 connective/order pinning gaps in this review, each proven by mutation.
Failure scenario: proven mutation — changing the && before the per-PR format to || keeps the entire scripts suite green. Under it a trusted review's expression evaluates to boolean true, so every trusted review on every PR shares one concurrency group literally named true with cancel-in-progress: true: a maintainer's review on PR B cancels the queued or in-flight trusted signal for PR A, which never completes success, the bridge never resolves it, and that review is silently dropped to the cron backstop — the cross-PR version of the failure the comment above the group says the trust split exists to prevent. The test named 'isolates gated-out runs so they cannot cancel the real ones' should catch this and does not.
Suggested fix: pin the resolved group structure — assert signalGroup equals the full expression string (as the bridge group is pinned with toBe), or assert structurally that the per-PR format is the &&-branch and the run-unique format the ||-fallback.
中文说明
[Suggestion] 区分可信（按 PR）与不可信（run-unique）signal 并发组的条件粘合未被钉住——本测试只断言两个 format(...) 字面量都存在，没有断言在两者之间路由 review 的 &&/|| 结构。这是本 review 中 4 处连接词/顺序钉住缺口之一，均经变异证实。
失败场景：已证实的变异——把按 PR 的 format 之前的 && 改为 ||——整个 scripts 套件保持绿色。在该变异下，可信 review 的表达式求值为布尔 true，于是每个 PR 上的每个可信 review 共享一个字面名为 true、带 cancel-in-progress: true 的并发组：maintainer 在 PR B 上的 review 会取消 PR A 排队中或运行中的可信 signal，后者永远无法以 success 完成，bridge 永远不会解析它，该 review 被悄悄丢给 cron 兜底——正是组上方注释说信任分流要防止的失败的跨 PR 版本。名为 'isolates gated-out runs so they cannot cancel the real ones' 的测试本应捕获它却没有。
建议修复：钉住解析后的组结构——断言 signalGroup 等于完整表达式字符串（如同 bridge 组用 toBe 钉住那样），或结构性断言按 PR 的 format 位于 && 分支、run-unique 的 format 位于 || 兜底。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| pull_request_review: | ||
| types: | ||
| - 'submitted' |
There was a problem hiding this comment.
[Suggestion] The bridge side of the trigger contract is pinned (bridge.on.workflow_run.workflows equals the signal's name:, types equals ['completed']), but the signal side — that the trigger is pull_request_review with type submitted — is asserted nowhere. One of 4 unpinned wiring-contract instances in this review, each proven by mutation.
Failure scenario: proven mutation — changing types: ['submitted'] to types: ['dismissed'] keeps all 142 tests green. In production, submitted reviews never fire the signal, so the bridge never runs and fork-PR review pickup silently dies (the rare dismissal-triggered signals resolve stale heads and no-op green); renaming or dropping the event decouples it from the bridge's workflows: match the same way. Both files stay individually valid YAML — the exact silent-decoupling class the first test in this suite enumerates.
Suggested fix: expect(signal.on.pull_request_review.types).toEqual(['submitted']) (optionally also expect(Object.keys(signal.on)).toEqual(['pull_request_review'])).
中文说明
[Suggestion] 触发契约的 bridge 一侧已被钉住（bridge.on.workflow_run.workflows 等于 signal 的 name:、types 等于 ['completed']），但 signal 一侧——触发器是 pull_request_review 且类型为 submitted——没有任何断言。这是本 review 中 4 处未钉住的接线契约之一，均经变异证实。
失败场景：已证实的变异——把 types: ['submitted'] 改为 types: ['dismissed']——全部 142 个测试保持绿色。生产中 submitted 的 review 永远不会触发 signal，bridge 永远不运行，fork PR 的 review 接管悄悄死亡（罕见的 dismissal 触发 signal 只会解析到过期 head 并绿色无操作）；重命名或删除该事件会以同样方式断开它与 bridge workflows: 的匹配。两个文件各自仍是合法 YAML——正是本套件第一个测试列举的那种静默失联。
建议修复：expect(signal.on.pull_request_review.types).toEqual(['submitted'])（可选地再加 expect(Object.keys(signal.on)).toEqual(['pull_request_review'])）。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| // Nothing at all → the polling loop exhausts and records nothing. | ||
| expect(runCorrelation([])).toBe(''); |
There was a problem hiding this comment.
[Suggestion] All four correlation-replay scenarios use a static gh stub, so the retry-until-appears semantics of the 5×2s polling loop are untested. One of 2 instances of the same pattern in this review: the replay stubs are state/flag-insensitive, so loop/parameter mutants escape (both proven by mutation).
Failure scenario: proven mutation — changing the loop guard in qwen-fleet-shepherd.yml from -gt 1 to -ne 1 (COUNT==0 breaks after the first empty poll — one poll instead of five) keeps every test green, because with a static stub no scenario can tell single-poll from five-poll. In production a freshly dispatched run can take more than 2s to become visible via gh run list; with the mutant the shepherd records run=none on every such tick, turning the documented rare fallback (duplicate scan) into the normal case and permanently degrading liveness tracking.
Suggested fix: add a stateful stub scenario — first gh run list call returns [], second returns one candidate: the original attributes the run id, the mutant returns ''.
中文说明
[Suggestion] 全部四个关联回放场景都使用静态 gh stub，因此 5×2s 轮询循环「重试直到出现」的语义未被测试。这是本 review 中同一模式的 2 处之一：回放 stub 对状态/标志不敏感，循环/参数类变异因此存活（均已变异证实）。
失败场景：已证实的变异——把 qwen-fleet-shepherd.yml 中循环守卫从 -gt 1 改为 -ne 1（COUNT==0 时在第一次空轮询后即 break——一次轮询取代五次）——所有测试保持绿色，因为静态 stub 下没有任何场景能区分单次轮询与五次轮询。生产中新 dispatch 的运行可能要超过 2s 才在 gh run list 中可见；在该变异下 shepherd 每个这样的 tick 都记录 run=none，把文档中的罕见兜底（重复扫描）变成常态，永久退化存活跟踪。
建议修复：增加一个有状态的 stub 场景——第一次 gh run list 返回 []，第二次返回一个候选：原实现会归因该 run id，变异实现返回 ''。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| if ! PR_LIST="$(gh pr list --repo "${REPO}" --state open --base main \ | ||
| --limit 1000 --json number,author,labels,headRefOid,isCrossRepository,maintainerCanModify 2> /dev/null)"; then |
There was a problem hiding this comment.
[Suggestion] The --limit 1000 enumeration parameter — load-bearing for this workflow's core "resolve the PR from the head SHA" function — is invisible to the entire test suite: the replay's gh stub ignores flags and no assertion mentions the limit. One of 2 replay-stub-insensitivity instances in this review (both proven by mutation). Distinct from the separate low-confidence note about 1000 itself being a cap: this is about the parameter being unpinned.
Failure scenario: proven mutation — --limit 1000 → --limit 1 keeps all 991 tests in the scripts suite green. Under it the bridge sees only the single newest open PR, so every signal whose reviewed PR is not literally the newest open PR against main resolves to the "no eligible fork PR carries head" green no-op — indistinguishable in logs and status from a healthy race — silently degrading every such fork-PR review to the cron backstop. A future cost-cutting --limit 100, or a refactor dropping the flag to gh's default 30, would ship equally green.
Suggested fix: honor --limit in the replay stub (truncate the stub's pr list output to the requested limit; add a case where the target PR sits just outside a small window), or pin the flag textually.
中文说明
[Suggestion] --limit 1000 枚举参数——本 workflow 核心功能「从 head SHA 解析 PR」的承重参数——对整个测试套件不可见：回放的 gh stub 忽略所有标志，也没有任何断言提到该 limit。这是本 review 中回放 stub 不敏感模式的 2 处之一（均已变异证实）。与另一条低置信度意见（1000 本身是上限）不同：此处的问题是参数未被钉住。
失败场景：已证实的变异——--limit 1000 → --limit 1——scripts 套件全部 991 个测试保持绿色。在该变异下 bridge 只能看到最新的一个 open PR，凡被 review 的 PR 不是恰好最新者，signal 都落入「no eligible fork PR carries head」的绿色无操作——日志与状态上与健康竞态无法区分——悄悄把这类 fork PR 的 review 全部退化为 cron 兜底。未来省成本的 --limit 100、或重构丢掉该标志回落到 gh 默认 30，都会同样绿灯地合入。
建议修复：让回放 stub 尊重 --limit（按请求的 limit 截断 stub 的 pr list 输出；补一个目标 PR 恰好在小窗口之外的用例），或在文本上钉住该标志。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| permissions: | ||
| actions: 'write' | ||
| contents: 'read' | ||
| pull-requests: 'read' |
There was a problem hiding this comment.
[Suggestion] The bridge declares contents: 'read' although no step reads repository contents — the single step runs only gh pr list (needs pull-requests: 'read'), gh workflow run (needs actions: 'write') and a local GITHUB_STEP_SUMMARY write; there is no checkout, no git operation and no contents-API call anywhere in the file. Additionally the test pins the signal's permission set exhaustively (expect(signal.permissions).toEqual({})) while pinning the bridge's only partially (bridge.permissions.actions), so this unused scope and any future scope creep land without tripping the test that exists to police this invariant.
Failure scenario: the privileged half of a fork-triggered chain ships a token scope it never uses — against the PR's own least-privilege narrative — and a future contents: 'write' or pull-requests: 'write' addition passes the very test created to police the invariant, widening the blast radius of the run a pwn-request would target. (An unused read scope is hygiene, not a live hole — the bridge executes no untrusted code — the concrete cost is the unpinned minimum.)
| permissions: | |
| actions: 'write' | |
| contents: 'read' | |
| pull-requests: 'read' | |
| permissions: | |
| actions: 'write' | |
| pull-requests: 'read' |
and pin the exact set in the test: expect(bridge.permissions).toEqual({ actions: 'write', 'pull-requests': 'read' }).
中文说明
[Suggestion] bridge 声明了 contents: 'read'，但没有任何步骤读取仓库内容——唯一步骤只运行 gh pr list（需要 pull-requests: 'read'）、gh workflow run（需要 actions: 'write'）和本地写 GITHUB_STEP_SUMMARY；整个文件没有 checkout、没有 git 操作、没有任何 contents API 调用。此外测试对 signal 的权限集做了穷尽钉住（expect(signal.permissions).toEqual({})），对 bridge 却只钉了一部分（bridge.permissions.actions），因此这个未使用的权限以及未来任何权限扩张，都不会触发那个为守护此不变量而存在的测试。
失败场景：fork 触发链条中持权的一半携带了一个它从不使用的 token 权限——与本 PR 自己的最小权限叙事相悖——未来若加上 contents: 'write' 或 pull-requests: 'write' 也能通过那个专为守护该不变量而建的测试，扩大 pwn-request 目标运行的爆炸半径。（未使用的 read 权限是卫生问题而非现实漏洞——bridge 不执行任何不可信代码——具体成本是「最小权限」未被钉住。）
并在测试中钉住精确集合：expect(bridge.permissions).toEqual({ actions: 'write', 'pull-requests': 'read' })。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| // Does the file actually READ a secret, as opposed to naming one in prose? | ||
| // Both workflows explain themselves by referring to `secrets.CI_DEV_BOT_PAT`, | ||
| // so only an expression counts. | ||
| const readsASecret = (text) => /\$\{\{[^}]*\bsecrets\./.test(text); |
There was a problem hiding this comment.
[Suggestion] The readsASecret detector that enforces the bridge's central invariant — no secret read alongside actions: write, the combination that makes a pwn-request worth attempting — is defeated by any expression containing a } before secrets.: [^}]* cannot cross the brace.
Failure scenario: proven by probe against this commit — inserting LEAKED: "${{ format('{0}', secrets.CI_DEV_BOT_PAT) }}" (or ${{ fromJSON('{"a":1}').a == secrets.CI_DEV_BOT_PAT }} — both valid, evaluated expressions) into the bridge leaves readsASecret(bridgeText) false and the invariant test green, while a plain ${{ secrets.X }} is still caught. A future edit giving the base-context, actions: write bridge a real secret read hidden behind any brace-bearing expression then ships silently — the exact regression this test exists to catch.
Suggested fix: scan each ${{ … }} expression with a scanner that skips single-quoted string literals (with '' escapes) before locating the closing }}, then test the body for \bsecrets\. — verified to detect both payloads while keeping the prose comments and both current files clean. A parsed-YAML walk needs the same expression-aware check: the bridge's own run: prose matches a bare /secrets\./ scan.
中文说明
[Suggestion] 守护 bridge 核心不变量的 readsASecret 检测器——actions: write 旁不得读取 secret，该组合正是让 pwn-request 值得尝试的条件——会被任何在 secrets. 之前含有 } 的表达式击败：[^}]* 无法跨越花括号。
失败场景：已在本 commit 上探测证实——向 bridge 插入 LEAKED: "${{ format('{0}', secrets.CI_DEV_BOT_PAT) }}"（或 ${{ fromJSON('{"a":1}').a == secrets.CI_DEV_BOT_PAT }}——两者都是合法且会被求值的表达式），readsASecret(bridgeText) 仍返回 false、不变量测试保持绿色，而裸 ${{ secrets.X }} 仍会被捕获。未来若有人给这个 base-context、持 actions: write 的 bridge 加上藏在任何带花括号表达式背后的真实 secret 读取，它将静默合入——正是本测试要捕获的那种回归。
建议修复：逐个扫描 ${{ … }} 表达式，先用能跳过单引号字符串字面量（含 '' 转义）的扫描器定位收尾的 }}，再对表达式主体测试 \bsecrets\.——已验证可检出上述两种载荷，同时保持散文注释与当前两个文件不误报。若走解析 YAML 的路线也需要同样的表达式感知检查：bridge 自己的 run: 散文会让裸 /secrets\./ 扫描误报。
— qwen3.8-max via Qwen Code /review (v0.21.7)
|
🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31176708378 🧠 Handled by Qwen Code · model/模型 |
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： AutoFix review round — PR #8676Addressed all four Critical findings and all fifteen Suggestions from @wenshao and @qwen-code-ci-bot in one commit ( Critical findingsR1-1 / rc:3735552469 — shepherd can attribute a foreign dispatch (also the ci-bot's still-standing blocker on the same thread). Fixed with the reviewer's accepted alternative — a stabilization window instead of first-singleton acceptance. The attribution loop now records R1-2 / rc:3735552476 — head SHA authenticates the revision, not the PR. The signal now encodes the reviewed PR number AND the reviewer login in its R1-3 / rc:3735552482 — R1-4 / rc:3735552489 — MEMBER/COLLABORATOR does not guarantee live write permission. The bridge now re-checks the reviewer LIVE before dispatching: Suggestions
VerificationCommands actually run on the final tree (commit
中文说明AutoFix 评审轮次 — PR #8676在一次提交（ Critical 发现R1-1 / rc:3735552469 — shepherd 可能把外部 dispatch 记为己有（也是 ci-bot 在同一线程上的仍存在阻塞）。 采用评审人接受的替代方案修复——用稳定窗口取代"接受首个 singleton"。关联循环现在在任一次轮询看到两个及以上候选时记录 R1-2 / rc:3735552476 — head SHA 只能证明代码版本，不能证明是哪一个 PR。 signal 现在把被评审的 PR 编号与评审人登录名编码进自己的 R1-3 / rc:3735552482 — R1-4 / rc:3735552489 — MEMBER/COLLABORATOR 并不保证当前拥有 write 权限。 bridge 现在在 dispatch 前对评审人做实时复查： Suggestion
验证在最终树（commit
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 217 passed · 0 failed · 217 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：217 通过 · 0 失败 · 217 总计 Verification reportPR #8676 verification — feat(autofix): bridge fork-PR reviews into the credentialed review laneVerdict: 中文摘要
ScopeCentral claim: the Secondary claims: (1) Central claim: A/B and mutation matrixBase cell (structural): at
Independent replay matrix at HEAD ( Secondary claim 1 ( Secondary claim 2 ( Targeted gates (witness CorrectionsThe PR body's Reviewer Test Plan and "measured" bullets describe the artifact-channel design that the final commit removed; the code is the intended design (the suite pins the artifact channel's absence: "a resurrected one would reintroduce a binding that read a benign concurrent push as a forgery"). Specifically, at the verified head:
These are description-accuracy items, not code defects — labeled as corrections to the body, not requests to change the code. Findings (non-blocking)
Not covered
MethodologyEnvironment: the CI verify container ( Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline. Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-autofix-fork-bridge.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix-fork-signal.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-fleet-shepherd.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查——无阻断问题。 建议见行内评论。 未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-autofix-fork-bridge.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix-fork-signal.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-fleet-shepherd.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| # conjuncts MIRROR route's fork admission (qwen-autofix.yml's | ||
| # pull_request_review path): maintainer edits on, and the PR bot-authored |
There was a problem hiding this comment.
[Suggestion] Stale comment pointer: these conjuncts are said to "MIRROR route's fork admission (qwen-autofix.yml's pull_request_review path)", but since #8671 that path no longer admits fork PRs — it declines them inline ("fork review noted … the next scheduled scan engages", qwen-autofix.yml ~line 404). The criteria actually mirrored live in review-scan's fork admission (scheduled-scan candidate filter ~2228-2250 and forced_admission_reason ~2010). — Failure scenario: a maintainer verifying the mirror claim (or "fixing" the gate to match the cited code) opens route's pull_request_review branch, finds only a fork refusal, and may reconcile the gate with it — silently killing real-time fork-review pickup; no test pins the comment's pointer.
| # conjuncts MIRROR route's fork admission (qwen-autofix.yml's | |
| # pull_request_review path): maintainer edits on, and the PR bot-authored | |
| # conjuncts MIRROR review-scan's fork admission (qwen-autofix.yml's | |
| # candidate filter + forced_admission_reason): maintainer edits on, and the PR bot-authored |
中文说明
问题： 注释指针过期：这里说这些条件「MIRROR route's fork admission (qwen-autofix.yml's pull_request_review path)」，但自 #8671 之后该路径已不再接纳 fork PR——它在行内直接拒绝（"fork review noted … the next scheduled scan engages"，qwen-autofix.yml 约第 404 行）。实际被镜像的标准位于 review-scan 的 fork 准入逻辑（定时扫描候选过滤 ~2228-2250 与 forced_admission_reason ~2010）。
失败场景： maintainer 核对这一镜像声明（或按注释所指的代码去「修正」门控）时，打开 route 的 pull_request_review 分支只会看到对 fork 的拒绝，可能据此把门控改成与拒绝一致——从而悄无声息地杀死实时接管；没有任何测试钉住这个注释指针。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| if [[ "${SIGNAL_REVIEWER}" != "${REVIEW_BOT}" ]]; then | ||
| REVIEWER_PERM="$(gh api "repos/${REPO}/collaborators/${SIGNAL_REVIEWER}/permission" --jq '.permission // ""' 2> /dev/null || echo '')" |
There was a problem hiding this comment.
[Suggestion] A failed reviewer-permission API lookup is indistinguishable from, and logged as, a genuine "below write" decision — asserting a verdict the code never obtained. Both siblings making this same call treat failure differently: route logs ::warning::Permission API call failed (qwen-autofix.yml ~420), and review-scan's read_live_permission (~1982) retries and distinguishes definitive none/404 from transient failures. — Failure scenario: a transient 5xx / secondary rate limit on the collaborators endpoint during a GitHub API brownout → every fork review silently loses real-time pickup (the whole point of this PR) while the log positively asserts a reviewer-permission cause ("permission='none' is below write"), sending oncall to inspect ACLs instead of API health; the run stays green, so nothing pages.
Suggested fix (failing closed is right — only the log and the distinction need fixing):
if [[ "${SIGNAL_REVIEWER}" != "${REVIEW_BOT}" ]]; then
if ! REVIEWER_PERM="$(gh api "repos/${REPO}/collaborators/${SIGNAL_REVIEWER}/permission" --jq '.permission // ""' 2> /dev/null)"; then
echo "::warning::reviewer permission lookup failed for '${SIGNAL_REVIEWER}' — failing closed, the scheduled scan remains the backstop"
exit 0
fi
case "${REVIEWER_PERM}" in
...中文说明
问题： reviewer 权限 API 查询失败时，与「确实低于 write」无法区分，且日志按后者输出——断言了一个代码从未获得的结论。两处做同样调用的兄弟代码处理方式不同：route 会记录 ::warning::Permission API call failed（qwen-autofix.yml ~420），review-scan 的 read_live_permission（~1982）会重试并区分「确定的 none/404」与瞬时故障。
失败场景： GitHub API 抖动期间 collaborators 端点出现瞬时 5xx / 二级限流 → 每个 fork review 都会静默失去实时接管（这正是本 PR 要恢复的能力），而日志却斩钉截铁地归因于 reviewer 权限（"permission='none' is below write"），把 oncall 引去排查 ACL 而非 API 健康度；run 仍是绿色，不会有任何告警。
建议修复： 失败关闭的方向是对的——只需修日志与区分：单独捕获查询退出码，失败时输出独立的 ::warning:: 后失败关闭（route 的模式），或重试 2-3 次并以诚实的「permission lookup failed」收尾。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| if [[ "${SIGNAL_REVIEWER}" != "${REVIEW_BOT}" ]]; then | ||
| REVIEWER_PERM="$(gh api "repos/${REPO}/collaborators/${SIGNAL_REVIEWER}/permission" --jq '.permission // ""' 2> /dev/null || echo '')" | ||
| case "${REVIEWER_PERM}" in | ||
| admin|maintain|write) : ;; |
There was a problem hiding this comment.
[Suggestion] The admin and maintain accept branches of this case are never exercised — the replay harness only supplies write (happy path) and triage/read/empty/lookup-failure (decline paths). — Failure scenario: verified empirically — mutating the pattern to write) : ;; leaves all 9 fork-bridge tests green. In production, an org owner or maintainer reviewing a fork PR (live permission admin/maintain) would then be silently declined with the factually wrong "below write" message, losing real-time pickup and degrading to the cron backstop with no test red. The harness already supports the discriminating inputs.
Suggested fix (in scripts/tests/qwen-autofix-fork-bridge-workflow.test.js): add { reviewerPermission: 'admin' } and { reviewerPermission: 'maintain' } replay cases asserting status 0 + pr_number=8436 in the recorded calls.
中文说明
问题： 该 case 的 admin 与 maintain 接受分支从未被测试执行——回放 harness 只提供 write（正常路径）和 triage/read/空值/查询失败（拒绝路径）。
失败场景： 已实测验证——把模式变异为 write) : ;;，全部 9 个 fork-bridge 测试依旧通过。生产中，org owner 或 maintainer 对 fork PR 提交 review（实时权限为 admin/maintain）时会被静默拒绝，并附带与事实不符的「below write」消息，失去实时接管、退化到 cron 兜底，而没有任何测试变红。harness 本就支持这两个区分性输入。
建议修复：（在 scripts/tests/qwen-autofix-fork-bridge-workflow.test.js 中）新增 { reviewerPermission: 'admin' } 与 { reviewerPermission: 'maintain' } 回放用例，断言 status 0 且录制调用中含 pr_number=8436。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| FORK_BRIDGE_VERIFIED=false | ||
| if [[ "${DISPATCH_SOURCE}" == 'fork-bridge' ]]; then |
There was a problem hiding this comment.
[Suggestion] The only code path that ever sets FORK_BRIDGE_VERIFIED=true — this condition, the provenance success branch, and the '360 minutes ago' window — is never executed by any test: the guard replay injects FORK_BRIDGE_VERIFIED directly from the test env, and the jq replay passes cutoff in, so the glue is pinned by substrings only. — Failure scenario: three one-line mutants empirically survive with all 130 tests in the two affected suites green — == → != on the line below; FORK_BRIDGE_VERIFIED=true → =false; and shrinking '360 minutes ago'. Each makes genuine fork-bridge dispatches on round-capped fork PRs post the loud refusal comment — resurrecting the exact #7836 refusal-spam regression this PR exists to eliminate. This is the same connective-flip mutant class the suite's own comments say it exists to catch.
Suggested fix (in scripts/tests/qwen-autofix-workflow.test.js): extract this if [[ "${DISPATCH_SOURCE}" == 'fork-bridge' ]]; then ... fi block verbatim (the suite's existing pattern) and replay it under bash with a PATH-stubbed gh run list and a stubbed/fixed date; assert FORK_BRIDGE_VERIFIED ends true for a matching bridge run, false for DISPATCH_SOURCE='', and false + warning for an unmatched/failed lookup; pin the '360 minutes ago' literal.
中文说明
问题： 唯一会把 FORK_BRIDGE_VERIFIED=true 的代码路径——此条件、溯源成功分支以及 '360 minutes ago' 窗口——从未被任何测试执行：守卫回放直接从测试环境注入 FORK_BRIDGE_VERIFIED，jq 回放则由测试传入 cutoff，所以这段胶水逻辑只被字符串级钉住。
失败场景： 三个单行变异均实测存活，且两个相关套件的 130 个测试全部保持绿色——下一行的 == → !=；FORK_BRIDGE_VERIFIED=true → =false；缩短 '360 minutes ago'。每一个都会让真实 fork-bridge dispatch 在已达上限的 fork PR 上发出响亮拒绝评论——复活本 PR 要消除的 #7836 拒绝刷屏回归。这正是套件自身注释声称要捕捉的连接词翻转类变异。
建议修复：（在 scripts/tests/qwen-autofix-workflow.test.js 中）按套件既有模式逐字抽出该 if [[ "${DISPATCH_SOURCE}" == 'fork-bridge' ]]; then ... fi 块，在 bash 下用 PATH stub 的 gh run list 与固定/stub 的 date 回放；断言匹配 bridge run 时 FORK_BRIDGE_VERIFIED 为 true，DISPATCH_SOURCE='' 时为 false，未匹配/查询失败时为 false 且带 warning；同时钉住 '360 minutes ago' 字面量。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| FORK_BRIDGE_VERIFIED=false | ||
| if [[ "${DISPATCH_SOURCE}" == 'fork-bridge' ]]; then | ||
| BRIDGE_CUTOFF="$(date -u -d '360 minutes ago' +%Y-%m-%dT%H:%M:%SZ)" | ||
| if BRIDGE_RUNS="$(gh run list --repo "${REPO}" --workflow qwen-autofix-fork-bridge.yml --limit 20 --json conclusion,createdAt,displayTitle 2> /dev/null)" \ |
There was a problem hiding this comment.
[Suggestion] The provenance horizon is the page, not the window: gh run list --limit 20 has no status filter, so skipped/queued/failed bridge runs consume slots in the newest-20, and the 360-minute cutoff filters only after the page is fetched. — Failure scenario: any GitHub account can submit ~20 COMMENTED reviews on throwaway fork PRs; each fires a signal that gates itself out as skipped, and every skipped signal still fires workflow_run: completed (documented in the bridge's own concurrency comment), creating skipped bridge runs that fill the page. A maintainer then reviews a round-capped managed fork PR, the bridge legitimately dispatches, but the scan finds no successful bridge run in the page → FORK_BRIDGE_VERIFIED=false → one loud "Dispatch refused" comment per review on the capped fork PR — the exact #7836 spam pattern this gate exists to prevent, triggerable with zero repository permissions. Organic variant: route backlog queues the dispatch for hours while 20+ bridge runs churn the page.
| if BRIDGE_RUNS="$(gh run list --repo "${REPO}" --workflow qwen-autofix-fork-bridge.yml --limit 20 --json conclusion,createdAt,displayTitle 2> /dev/null)" \ | |
| if BRIDGE_RUNS="$(gh run list --repo "${REPO}" --workflow qwen-autofix-fork-bridge.yml --status success --limit 100 --json conclusion,createdAt,displayTitle 2> /dev/null)" \ |
(raise the page so the window binds; --status success also stops non-success runs from consuming slots — they never pass the jq filter anyway)
中文说明
问题： 溯源检查的视野是这一页 20 条，而不是 360 分钟窗口：gh run list --limit 20 没有状态过滤，skipped/queued/failed 的 bridge run 也会占据最新 20 条的名额，而 360 分钟截止线要在取回这一页之后才参与过滤。
失败场景： 任意 GitHub 账号都可以在一次性 fork PR 上提交约 20 条 COMMENTED review；每条都会触发一个自我门控为 skipped 的 signal，而 skipped 的 signal 同样会触发 workflow_run: completed（bridge 自己的并发注释已说明），由此产生的 skipped bridge run 会占满页面。随后 maintainer 对一个已达轮次上限的 managed fork PR 提交 review，bridge 正常 dispatch，但扫描在页面里找不到成功的 bridge run → FORK_BRIDGE_VERIFIED=false → 该 PR 上每次 review 都收到一条响亮的「Dispatch refused」评论——正是本门控要防止的 #7836 刷屏模式，且触发者无需任何仓库权限。非对抗变体：route 积压把 dispatch 排队数小时，期间 20+ 个 bridge run 刷新了页面。
建议修复： 提高页大小让窗口成为约束（如 --limit 100），并加 --status success 使非成功 run 不再占用名额——它们本来就过不了 jq 过滤。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| FORK_BRIDGE_VERIFIED=false | ||
| if [[ "${DISPATCH_SOURCE}" == 'fork-bridge' ]]; then | ||
| BRIDGE_CUTOFF="$(date -u -d '360 minutes ago' +%Y-%m-%dT%H:%M:%SZ)" | ||
| if BRIDGE_RUNS="$(gh run list --repo "${REPO}" --workflow qwen-autofix-fork-bridge.yml --limit 20 --json conclusion,createdAt,displayTitle 2> /dev/null)" \ |
There was a problem hiding this comment.
[Suggestion] The --workflow qwen-autofix-fork-bridge.yml filename reference here is pinned by no test to the bridge file's actual path. If the bridge workflow is renamed, the fork-bridge suite fails only until its bridgePath constant is updated; afterwards all its assertions derive from the file at the new path and pass, and the literal pin in qwen-autofix-workflow.test.js matches stale-to-stale — the suite is green while this YAML reference stays wrong. — Failure scenario: in production gh run list --workflow qwen-autofix-fork-bridge.yml then 404s (verified live — the identical call shape returns HTTP 404 today, since the bridge is not merged yet: exactly the post-rename state) → the substitution dies in 2> /dev/null → FORK_BRIDGE_VERIFIED is forever false → the refusal-spam regression on capped fork PRs returns, discoverable only via the per-run ::warning::. Distinct from the run-title-prefix pin: this is the workflow-identity reference, with a file rename as the trigger.
Suggested fix (in scripts/tests/qwen-autofix-fork-bridge-workflow.test.js, which already loads autofixText):
expect(autofixText).toContain('--workflow ' + bridgePath.split('/').pop());中文说明
问题： 此处的 --workflow qwen-autofix-fork-bridge.yml 文件名引用没有任何测试把它钉到 bridge 文件的真实路径。若 bridge workflow 被重命名，fork-bridge 套件只会先在其 bridgePath 常量处失败；常量更新后，所有断言都从新路径的文件推导并通过，而 qwen-autofix-workflow.test.js 里的字面量钉住项也只是陈旧对陈旧——套件全绿，YAML 里的引用却已错误。
失败场景： 生产中 gh run list --workflow qwen-autofix-fork-bridge.yml 将返回 404（已实测——同样的调用形态今天就返回 HTTP 404，因为 bridge 尚未合入：恰与重命名后的状态一致）→ 命令替换在 2> /dev/null 中消亡 → FORK_BRIDGE_VERIFIED 永远为 false → 上限 fork PR 上的拒绝刷屏回归重现，唯一线索是每次运行的 ::warning::。与 run-title 前缀的钉住项不同：这是 workflow 身份引用，触发条件是文件重命名。
建议修复：（在已加载 autofixText 的 scripts/tests/qwen-autofix-fork-bridge-workflow.test.js 中）由真实路径推导该引用并断言。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| && jq -e --arg pr "${PR}" --arg cutoff "${BRIDGE_CUTOFF}" ' | ||
| [ .[] | ||
| | select(.conclusion == "success" and ((.createdAt // "") >= $cutoff)) | ||
| | select((.displayTitle // "") | startswith("fork-bridge: fork-signal: PR \($pr) reviewed by ")) ] |
There was a problem hiding this comment.
[Suggestion] This scan-side copy of the composed run-title prefix is the only one no test ties back to the two run-name: fields it derives from: qwen-autofix-workflow.test.js reads only qwen-autofix.yml/ci.yml/release.yml, and the fork-bridge test — which does hold autofixText — contains no assertion on the scan prefix. — Failure scenario: verified empirically — a coordinated reviewed by → reviewed-by edit across the signal run-name:, the bridge TITLE_RE and all six fork-bridge-test pins leaves 130/130 tests green, while the stale scan prefix never matches a real bridge title again: FORK_BRIDGE_VERIFIED stays false for every fork-bridge dispatch and the cap gate answers each one loudly — reintroducing the #7836 refusal spam, silently, with only a provenance unverified warning. The bridge-side copy (TITLE_RE) fails loud (red bridge runs) under the same edit; the scan leg is the silent one.
Suggested fix (in scripts/tests/qwen-autofix-fork-bridge-workflow.test.js): derive the expected prefix from the pinned SIGNAL_RUN_NAME/BRIDGE_RUN_NAME static parts and assert it appears in the review-scan script — replacing one hardcoded copy with a cross-file check.
中文说明
问题： 这个由两个 run-name: 字段组合而成的 run-title 前缀，扫描端的这一份是唯一没有被任何测试回溯到其来源字段的拷贝：qwen-autofix-workflow.test.js 只读取 qwen-autofix.yml/ci.yml/release.yml，而持有 autofixText 的 fork-bridge 测试里没有任何针对扫描端前缀的断言。
失败场景： 已实测验证——把 signal 的 run-name:、bridge 的 TITLE_RE 以及 fork-bridge 测试的六处钉住项协同地从 reviewed by 改为 reviewed-by，130/130 个测试全绿，而陈旧的扫描端前缀从此再也匹配不到真实的 bridge 标题：所有 fork-bridge dispatch 的 FORK_BRIDGE_VERIFIED 恒为 false，上限门控对每一次都响亮回应——重新引入 #7836 拒绝刷屏，且是静默的，只有一条 provenance unverified warning。同样编辑下 bridge 端的拷贝（TITLE_RE）会响亮失败（bridge run 变红）；扫描端这条腿才是沉默的那条。
建议修复：（在 scripts/tests/qwen-autofix-fork-bridge-workflow.test.js 中）由钉住的 SIGNAL_RUN_NAME/BRIDGE_RUN_NAME 静态部分推导期望前缀，并断言其出现在 review-scan 脚本中——把一份硬编码拷贝替换为跨文件检查。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| if [[ -n "${SEEN_ID}" && "${SEEN_ID}" == "${PREV_SEEN:-}" ]]; then | ||
| ATTRIBUTED="${SEEN_ID}" |
There was a problem hiding this comment.
[Suggestion] The loop's keep-polling semantics — an ambiguity arriving AFTER a two-poll stabilization still clears the attribution — are pinned by no test; the stateful stub sequences are only [empty, singleton] and [foreign-singleton, pair]. — Failure scenario: verified empirically — appending ; break after ATTRIBUTED="${SEEN_ID}" (a plausible "optimization") survives the whole suite. With polls [foreign-singleton, foreign-singleton, pair] (the bridge's dispatch lands just after stabilization), the mutant attributes the foreign run — the guess-attribution this diff's own comment forbids ("never attribute a guess") — while the original records run=none; no existing test distinguishes them.
Suggested fix (in scripts/tests/qwen-fleet-shepherd-workflow.test.js): add one stateful sequence [singleton(900002), singleton(900002), pair(900002, 900001)] expecting ''.
中文说明
问题： 该循环「持续轮询」的语义——两次轮询稳定之后才出现的歧义仍会清空归因——没有任何测试钉住；stateful stub 序列只有 [empty, singleton] 与 [foreign-singleton, pair]。
失败场景： 已实测验证——在 ATTRIBUTED="${SEEN_ID}" 后追加 ; break（一个看似合理的「优化」）即可通过整个套件。当轮询序列为 [foreign-singleton, foreign-singleton, pair]（bridge 的 dispatch 恰在稳定后出现）时，变异体会把外部 run 归为己有——正是本 diff 注释所禁止的猜测式归因（"never attribute a guess"）——而原始实现记录 run=none；现有测试无法区分两者。
建议修复：（在 scripts/tests/qwen-fleet-shepherd-workflow.test.js 中）新增一条 stateful 序列 [singleton(900002), singleton(900002), pair(900002, 900001)]，期望结果为 ''。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| expect(bridgeScript).toContain('gh workflow run qwen-autofix.yml'); | ||
| expect(bridgeScript).toContain('-f pr_number='); | ||
| expect(bridgeScript).toContain('-f source=fork-bridge'); |
There was a problem hiding this comment.
[Suggestion] Every element of the bridge's dispatch call is pinned cross-file here — except the target ref. --ref main (.github/workflows/qwen-autofix-fork-bridge.yml:161) selects which copy of qwen-autofix.yml executes; the replay's stub gh accepts any ref. — Failure scenario: verified empirically — mutating --ref main → --ref release leaves all 9 tests green. If the ref drifts, the dispatch either fails after three retries (bridge permanently red) or executes the scan lane from the wrong branch's workflow file — silent behavioral drift of the secret-bearing lane with no CI signal.
| expect(bridgeScript).toContain('gh workflow run qwen-autofix.yml'); | |
| expect(bridgeScript).toContain('-f pr_number='); | |
| expect(bridgeScript).toContain('-f source=fork-bridge'); | |
| expect(bridgeScript).toContain('gh workflow run qwen-autofix.yml'); | |
| expect(bridgeScript).toContain('-f pr_number='); | |
| expect(bridgeScript).toContain('-f source=fork-bridge'); | |
| expect(bridgeScript).toContain('--ref main'); |
中文说明
问题： bridge 的 dispatch 调用在此被跨文件钉住了每一个要素——唯独缺了目标 ref。--ref main（.github/workflows/qwen-autofix-fork-bridge.yml:161）决定执行哪一份 qwen-autofix.yml；回放用的 stub gh 接受任意 ref。
失败场景： 已实测验证——把 --ref main 变异为 --ref release，全部 9 个测试依旧通过。若 ref 发生漂移，dispatch 要么在三次重试后失败（bridge 永久变红），要么用错误分支的 workflow 文件执行扫描车道——持有 secrets 的车道发生静默行为漂移，而 CI 毫无信号。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| const replay = (ghScript) => { | ||
| const dir = mkdtempSync(join(tmpdir(), 'shepherd-corr-')); |
There was a problem hiding this comment.
[Suggestion] This PR adds two more copies of the PATH-stubbed-gh replay harness that already exists in this very file — three copies of the same skeleton now live in two test files: runBusyWalk (this file, :405, pre-existing), replay (:290, added here) and runBridge (qwen-autofix-fork-bridge-workflow.test.js:394, added here). All repeat mkdtemp → write gh stub → chmodSync 0o755 → bash with PATH prefix → rmSync in finally, and they drifted on day one: runBusyWalk omits the set -eo pipefail + sleep() stub prologue the new copies include (the production shape per GHA's bash -eo pipefail default), and runBridge uses spawnSync while the other two use execFileSync. — Concrete cost: any future fix to stub/cleanup semantics — or the next replay test, which will copy a fourth time — must be made in 3+ places independently, and a harness fix applied to one copy leaves the others simulating a different shell. scripts/tests/workflow-helpers.js already exists and is imported by sibling suites.
Suggested fix: extract the shared skeleton there, e.g. replayWithStubbedGh({ ghScript, script, env, prologue }) returning stdout/status, and have all three call sites use it.
中文说明
问题： 本 PR 又新增了两份 PATH stub gh 回放 harness 的拷贝——同一骨架现在共有三份、分布在两个测试文件里：runBusyWalk（本文件 :405，原有）、replay（:290，本次新增）、runBridge（qwen-autofix-fork-bridge-workflow.test.js:394，本次新增）。三者都重复 mkdtemp → 写 gh stub → chmodSync 0o755 → 带 PATH 前缀运行 bash → finally 里 rmSync，且第一天就已漂移：runBusyWalk 缺少新拷贝都带的 set -eo pipefail + sleep() stub 前导（按 GHA 默认 bash -eo pipefail 这才是生产形态），runBridge 用 spawnSync 而另两个用 execFileSync。
具体代价： 未来任何对 stub/清理语义的修复——或下一个回放测试（会复制出第四份）——都必须在 3+ 处独立完成；只修一份，其余拷贝就在模拟另一种 shell。scripts/tests/workflow-helpers.js 已经存在，且已被兄弟套件引用。
建议修复： 把共享骨架抽到那里，例如 replayWithStubbedGh({ ghScript, script, env, prologue }) 返回 stdout/status，三处调用点统一使用。
— qwen3.8-max via Qwen Code /review (v0.21.7)
|
@qwen-code /triage |
|
Re-run after the template gate — the body now carries every required heading ✓ Problem: an observed capability loss, not theoretical hardening. #8671 (merged) made the direct lane decline fork-PR review events because those runs arrive with Direction: aligned. This restores real-time pickup for exactly the PRs a maintainer is actively waiting on (bot-authored or takeover-labelled forks), without reopening the red-CI failure mode #8671 closed. Size: no core-package paths — ~369 production lines of workflow YAML across 4 files, ~795 lines of tests across 3 files. Below every advisory threshold. Approach: the canonical Risk: no high-risk-path matches. It is credential plumbing, but the design keeps the secretless half genuinely secretless (pinned by a brace-aware test) and every hop re-derives admission from live API state. Moving on to code review. 🔍 中文说明模板拦截后的 re-run——正文现在包含了所有必需的标题 ✓ 问题：已观测到的能力缺失，不是理论性加固。#8671（已合入）让直连车道拒绝 fork PR 的评审事件，因为这类 run 拿不到仓库 secrets（ 方向：对齐。它恢复的正是维护者正在主动等待的那类 PR（bot 作者或带 takeover 标签的 fork PR）的实时拾取，同时没有重新打开 #8671 关闭的红 CI 失败模式。 规模：不涉及 core 包路径——4 个文件约 369 行生产 workflow YAML，3 个测试文件约 795 行。低于所有提示阈值。 方案：标准的 风险：未命中高风险路径。这确实是凭据管道，但设计让无 secret 的半边真正无 secret（由一个能识别花括号的测试钉死），且每一跳都从实时 API 状态重新推导准入。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewIndependent baseline first: for "fork-PR review events carry no secrets, so the credentialed lane cannot authenticate", the canonical answer is exactly a The security trace holds up:
Two nits, neither blocking:
sequenceDiagram
participant P1 as Trusted reviewer
participant P2 as Fork Signal
participant P3 as Fork Bridge
participant P4 as Autofix scan
P1->>P2: submits a review on a fork PR
Note over P2: base-branch file, permissions empty, run-name carries the PR binding
P2->>P3: workflow_run completed
Note over P3: re-checks reviewer write permission and live PR admission
P3->>P4: workflow_dispatch with pr_number and source marker
Note over P4: re-derives admission again from live API state
Files changed (7 of 7)
Testing evidence — the PR's own CI
All 114 check-runs on the reviewed commit completed: 0 failures, 0 pending. The macOS/Windows and integration skips are the repo's standing design ( What no CI can settle pre-merge, stated plainly: 中文说明代码审查先给出独立基线：对于「fork PR 的评审事件没有 secrets，持凭据的车道无法认证」这个问题，标准解法正是 安全推演成立：fork 无法编辑 gate 或伪造绑定（fork 触发的评审 run 执行 base 分支的 workflow 文件——PR 引用的 run 31152873061 证据经核实成立， 两个不阻断的小问题：正文落后于代码（设计已从 artifact 通道转为 run-name 绑定，正文仍有 artifact 表述与「4/4 pass」，套件现为 9 个测试），建议同步；正文的变异表与回放用例清单本身是很好的方法论证据。 测试证据被审 commit 上 114 个 check-run 全部完成：0 失败、0 待定。macOS/Windows 与集成测试的跳过是仓库既有设计（ — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean across every stage; the one real nit is that the PR body still narrates the earlier artifact-channel revision instead of the run-name binding that shipped. Stepping back: my independent proposal for this problem was the same The questions I sat with: does the residual shepherd race bother me? In theory a foreign dispatch stable across two polls while the shepherd's own run stays list-invisible gets misattributed — but the failure mode is a duplicate scan that self-heals, never starvation or a security boundary, and it is replayed behaviorally. Does the pre-merge unobservability bother me? It is inherent to Approving, pinned to the reviewed commit. The stale 中文说明置信度：4/5 —— 各阶段都干净；唯一实质性的瑕疵是 PR 正文仍在描述较早的 artifact 通道设计，而不是实际落地的 run-name 绑定。 退一步看：我对这个问题的独立方案同样是 我反复掂量的问题：shepherd 的残余竞态是否构成顾虑？理论上存在「外部 dispatch 连续两次轮询稳定、而 shepherd 自己的 run 始终不可见」被错误归属的可能——但失败模式是可自愈的重复扫描，不是饥饿或安全边界，且已有行为化回放。合入前不可观测是否构成顾虑？这是 批准，锚定在被审 commit。本 PR 上残留的 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
LGTM, no P0/P1/P2 — an unusually well-hardened privilege-boundary bridge; findings are P3 defense-in-depth only. Security verified with evidence: (1) no escalation — the bridge's only write is dispatching qwen-autofix.yml with one re-validated pr_number, and review-scan's forced-fork admission requires the fork AUTHOR to hold live admin/maintain/write, so a malicious fork author can never get their code touched and anyone who can trigger this already has equivalent power (manual dispatch exists); (2) signal authenticity rides the signal run's run-name, which GitHub evaluates from the base-branch workflow file (a fork can't edit/forge it), parsed with an anchored regex + live permission re-check + live PR re-read, with no artifact channel (tests pin upload/download out); (3) fork code running with secrets is the pre-existing review-address lane's documented limit (docker sandbox, live write+ authors only), not broadened here, and the two new workflows are clean (minimal permissions, zero checkout, zero secrets expressions, brace-aware scanner pin); (4) concurrency handled (trust-split signal groups, conclusion+head keying, route refuses public source buying shared-group/cancel rights, per-PR head-write lock). P3s: REVIEW_BOT exemption from the live write re-check (document the dependency); assert workflow_run.event=='pull_request_review' in the bridge if:; include display_title in the concurrency group for per-PR isolation; raise --limit 20 provenance window for bursts; distinguish lookup-failure from low-permission in the log.
|
Sandboxed verification: Skipped because the PR is not open for verification (state=MERGED, draft=false). 中文 — 判定：
|
|
Released in v0.21.8. |
…wenLM#9469) * 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 (QwenLM#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>
4 participants
What this PR does
Restores real-time autofix pickup for pull requests opened from a fork, using the standard
workflow_runbridge.Two workflows.
qwen-autofix-fork-signal.ymlruns onpull_request_review, where a fork PR's event can reach it, and records only which PR was reviewed —permissions: {}, no checkout, no repository code, hosted runner rather than the persistent self-hosted pool.qwen-autofix-fork-bridge.ymlruns onworkflow_run, which GitHub creates on the default branch with the repository's own token, validates the number, and dispatchesqwen-autofix.ymlfor that PR usingactions: write— the same lever Fleet Shepherd already pulls. Neither half holds a PAT.The dispatched run re-derives admission from live API state (open, targeting main, bot-authored or takeover-labelled, and for a fork a live write+ author plus maintainer edits allowed), so the PR number says when to look, never who may be touched.
Why it's needed
A workflow run tied to a pull request whose head lives in a fork is created with
Secret source: None, sosecrets.CI_DEV_BOT_PATarrives empty and the autofix review lane cannot authenticate. #8671 stopped that from reddening CI by declining those events inroute, but it removed the capability rather than fixing it.The cost is latency. A fork PR's review now waits for the scheduled scan, and
qwen-autofix.ymlrecords the measurement itself: the*/10cron actually lands every 40-70 minutes on this repo. Every takeover-labelled fork PR — the ones a maintainer is actively waiting on — pays that.Reading the PR with
github.tokeninstead would not close the gap:review-addressstill needs the PAT to push and comment, and it is empty for the whole run. The credential has to come from a different run, which is whatworkflow_runprovides.Reviewer Test Plan
How to verify
Three properties make it safe to act on a number that arrives from a fork-triggered run. Each was measured rather than assumed, and a reviewer can re-run each command.
A fork cannot edit the signal —
pull_request_reviewruns the workflow file from the base branch. Run31152873061is a review on PR #8436, whose branch does not contain #8410:The run executed a script its own PR branch does not contain, so the file came from base.
The number is bound to the run that produced it — that run's
head_shaequals the reviewed PR's head exactly:The bridge fetches the named PR and refuses unless its head matches, failing closed on an empty or unreadable head.
The artifact is the only channel —
workflow_run.pull_requestsis empty for fork PRs, and/commits/{sha}/pullsdoes not resolve a fork head either:Then run the suite:
Expected: 4/4 pass. It pins the cross-file contracts (the bridge's
workflows:entry equals the signal'sname:, the download name equals the upload name, the read path equals the written path, andqwen-autofix.ymlreally accepts apr_numberdispatch input) and replays the bridge's validate-and-dispatch block underbashagainst a stub API.Evidence (Before & After)
N/A — no user-visible or TUI surface. This cannot be exercised end to end before merge either:
workflow_runonly fires for workflows already present on the default branch. The evidence is the stub-API replay and the mutation results below.The replayed cases: happy path; forgery (a real open PR whose head differs — every other check passes, only the binding refuses it); an unreadable head; both heads empty (the case a bare
!=gets wrong); non-numeric, empty and path-shaped artifacts; a closed, retargeted or in-repo PR; download and PR-read failures; and dispatch retry-then-exhaustion.Mutation-tested — 12 of 12 caught:
conclusion == 'success'filterpr_numberinputThe empty-head-guard mutation initially survived; that is what added the both-heads-empty case.
Tested on
Environment (optional)
Workflow YAML and a vitest suite only; no local runtime involved.
Risk & Scope
workflow_run.head_sha, so a signal cannot name a PR other than the one whose review produced it. Beyond that the dispatched run re-derives admission from live API state, so the number only chooses timing.workflow_runonly fires for workflows already on the default branch. The first real fork-PR review after merge is the live check. The takeover-label lane (pull_requestlabelled on a fork) is deliberately not bridged — it needs an ack comment, which apr_numberdispatch cannot produce, androutealready declines it.review-addresschecks out and builds fork code in the same run that would then hold the PAT.Linked Issues
Depends on #8671 (merged). That PR removed the fork-PR review lane because it could never authenticate; this one restores the capability with credentials that actually exist. Merging this alone would have added a working dispatch beside a still-red direct lane, so it is intended to land after #8671 — which it now does.
中文说明
What this PR does
用标准的
workflow_run桥接，恢复 fork PR 的 autofix 实时接管。两个 workflow。
qwen-autofix-fork-signal.yml在pull_request_review上运行（fork PR 的事件能触达这里），只记录被评审的是哪个 PR ——permissions: {}、不 checkout、不执行仓库代码、用托管 runner 而非持久化的自托管池。qwen-autofix-fork-bridge.yml在workflow_run上运行，GitHub 会把该 run 创建在默认分支上并带有仓库自身的 token；它校验号码后用actions: writedispatchqwen-autofix.yml——Fleet Shepherd 已经在用同一个手段。两半都不持有 PAT。被 dispatch 的 run 会从实时 API 状态重新推导准入条件（open、面向 main、bot 创建或带接管标签，fork 还需作者实时 write+ 且允许维护者编辑），所以 PR 号只说明何时去看，绝不决定谁可以被改动。
Why it's needed
绑定在 fork PR head 上的 workflow run 以
Secret source: None创建，secrets.CI_DEV_BOT_PAT为空，autofix 评审车道无法认证。#8671 通过在route中拒绝这些事件止住了 CI 报红，但那是移除能力，不是修复它。代价是延迟。fork PR 的评审现在要等定时扫描，而
qwen-autofix.yml自己记录了实测值：*/10的 cron 在本仓每 40-70 分钟才真正落一次。每一个带接管标签的 fork PR——正是维护者在等的那些——都要付这个代价。改用
github.token读 PR 并不能补上缺口：review-address仍然需要 PAT 来 push 和发评论，而它在整个 run 里都是空的。凭据必须来自另一个 run，这正是workflow_run提供的。Reviewer Test Plan
How to verify
有三个性质使得「依据一个来自 fork 触发 run 的号码去行动」是安全的。每一条都是实测而非假设，评审者可以逐条复跑。
fork 无法篡改 signal ——
pull_request_review使用 base 分支的 workflow 文件。run31152873061是对 PR #8436 的评审，而 #8436 的分支不含 #8410：该 run 执行了一份它自己 PR 分支里并不存在的脚本，说明文件来自 base。
号码与产生它的 run 绑定 —— 该 run 的
head_sha与被评审 PR 的 head 完全相等：桥接侧取回被指名的 PR，head 不匹配就拒绝；空值或读取失败一律失败关闭。
artifact 是唯一通道 —— fork PR 的
workflow_run.pull_requests为空，/commits/{sha}/pulls也解析不出 fork 的 head：然后跑测试套件：
预期 4/4 通过。它钉住了跨文件契约（桥接的
workflows:条目等于 signal 的name:、下载名等于上传名、读取路径等于写入路径，且qwen-autofix.yml确实接受pr_number这个 dispatch 输入），并用bash对着 stub API 真实回放桥接的「校验并 dispatch」代码块。Evidence (Before & After)
N/A —— 没有用户可见界面或 TUI 变化。合入前也无法端到端跑通：
workflow_run只对已经位于默认分支上的 workflow 生效。证据是下面的 stub API 回放与变异测试结果。回放覆盖的场景：正常路径；伪造（一个真实存在且 open 的 PR，但 head 不同——其他检查全部通过，只有绑定把它拦下）；head 不可读；两侧 head 都为空（裸
!=会判错的那个场景）；非数字、空、路径形状的 artifact；PR 已关闭、已改 base、或非 fork；下载与 PR 读取失败；以及 dispatch 重试直至耗尽。变异测试 —— 12 个全部被捕获：
conclusion == 'success'过滤pr_number输入空 head 守卫那个变异最初存活了，正是它促成补上「两侧 head 都为空」这个用例。
Tested on
Environment (optional)
只涉及 workflow YAML 与一个 vitest 套件，不需要本地运行时。
Risk & Scope
workflow_run.head_sha绑定，signal 无法指向「产生它的那次评审」之外的 PR。在此之上，被 dispatch 的 run 还会从实时 API 状态重新推导准入条件，所以号码只决定时机。workflow_run只对已在默认分支上的 workflow 生效；合入后第一个真实的 fork PR 评审就是实地检验。接管标签车道（fork 上的pull_requestlabeled）刻意不做桥接——它需要发出 ack 评论，而pr_numberdispatch 产生不了，且route已经明确拒绝了该路径。review-address会在同一个 run 里 checkout 并构建 fork 的代码，而那个 run 届时持有 PAT。Linked Issues
依赖 #8671（已合入）。那个 PR 移除了 fork PR 的评审车道（它从来无法认证），本 PR 用真实存在的凭据恢复该能力。单独合入本 PR，会在一条仍然报红的直连车道旁边多出一条能用的 dispatch，因此它设计为在 #8671 之后落地——目前已满足。