feat(triage): assign one accountable owner when a PR opens, and when deferring - #9813
feat(triage): assign one accountable owner when a PR opens, and when deferring#9813yiliang114 wants to merge 27 commits into
Conversation
yiliang114
changed the title
|
Re-run at Template: the body uses the headings it was opened with ( Problem: real and observed, unchanged from earlier passes. Core-touching PRs fan review requests out to all five CODEOWNERS and nobody ends up specifically accountable; the defer fallback only @mentions, so nothing lands in anyone's Direction: aligned. Complements CODEOWNERS instead of replacing it, routes off the same reviewed owner map and load/rotation logic as issue assignment, and is a continuation of already-merged assignment work by a maintainer. Size: no core-package paths — everything sits in Approach: scope is right and unchanged; the only new code since the last gate is the two blocker fixes, both squarely inside the stated goal — no drive-bys. The standing non-blocking question from earlier rounds stays on the record: Risk: no elevated risk signals — none of the changed files match the revert-correlated high-risk paths. Moving on to code review. 🔍 中文说明在 模板：正文使用的是开 PR 时的旧标题（ 问题：真实且已观测，与早前结论一致。触碰 core 的 PR 会向全部 5 位 CODEOWNERS 扇出请求而没有明确责任人；defer 兜底只做 @mention，不会出现在任何人的 Assigned 过滤器里。本版本同时解决两半：打开时分配、defer 时分配（而非仅 mention）。 方向：对齐。补充而非替换 CODEOWNERS，复用同一份受评审的 owner 地图与负载/轮转逻辑，是 maintainer 已合入分配工作的延续。 规模：未触及核心包路径——全部在 方案：范围合理且未变；上次门禁后新增的代码只有两个阻断项修复，都在既定目标之内，无夹带。早前各轮留下的非阻断问题继续记录在案：两个分配脚本间仍重复 风险：无升级风险信号——变更文件均未命中与高 revert 率相关的路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code review — at the post-fix headThe shape is unchanged since the last full pass except for the merge of main and one fix commit, Both human-review blockers are fixed as prescribed, and pinned by tests.
Re-verified by reading at this head, beyond the findings ledger:
Changed files (9 of 9)
Testing evidence — this PR's own CI at the reviewed commit (not re-run here)Per gate rules I did not build or run PR code. Every check the head's CI runs finished — zero failures, zero pending. The previously red
The skips are profile decisions this PR did not author: main's own #10059 moved the macOS/Windows lanes off pull requests, and the platform-sensitivity classifier skips platform lanes and build-cli for a Sandboxed verification would settle the one gap CI cannot — and it is already in flight: the Real-scenario testing: N/A — unattended run, and this PR has no CLI/TUI surface (GitHub workflow automation only). 中文说明代码审查 —— 修复后 head自上次完整审查以来形态未变，仅合入 main 并新增一个修复提交，回应 @chiga0 在上个 head 提出的两个阻断项。我的独立方案仍与实现一致。 两个人工评审阻断项均按处方修复并有测试钉死：改名文件的源路径现在与目标路径一并逐条 base64 输出（换行伪造防御保持有效），新测试把文件移出 另行复读核实：信任模型成立（base.sha 检出、SHA 钉死的 checkout、凭据不落地、稀疏清单含两个脚本、token 步骤级、脚本从不读 PR 文本）；skill 文档 defer heredoc 导入的四个符号在 issue 脚本中均已导出，文档不会与模块静默漂移；基线字节数与合并后实测精确一致（2125 / 79063）；review job 第 10 轮在此 SHA 零发现，其披露的缺口（反审未收敛、actionlint 限制）与六条延后探针均为测试强度/工具备注，逐条看过，均非现行代码的正确性缺陷；早前各轮的遗留条目维持"已记录、不阻断"。 测试证据 —— 本 PR 自身 CI（未在此重跑）按门禁规则未构建或运行 PR 代码。本 head 的 CI 全部完成：零失败、零挂起。此前红掉的 沙箱验证可补齐 CI 补不上的唯一缺口——且已在途：本次触发的 真实场景测试：N/A——无人值守运行，且本 PR 无 CLI/TUI 界面。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — the two blockers from the human review are fixed exactly as prescribed and pinned by tests, the review job posted zero findings at this SHA, CI is fully green at the exact head, and the conflict that blocked the last run is gone; what keeps this from a 5 is the residual non-transactional write window and the fact that @chiga0 hasn't re-confirmed the fixes yet. Stepping back: my independent proposal for this problem is what landed — path-prefix routing over the reviewed owner map, least-loaded pick with PR-number rotation, one owner per PR, permission and coverage re-checked at write time, defer path assigning instead of mentioning. Every file in the diff serves that one goal. Ten rounds plus a human review shook out the hard parts (null-author shapes, filename forging, the snapshot-to-write window, bootstrap on a base without the script, renamed-file sources, mid-run head changes), and each is defended with a test now. The two things that made the last run defer — merge conflict and a red CI — are both resolved: the branch is MERGEABLE, and the head's CI finished with zero failures, including the ubuntu suite that was red before. The only new code since then is the blocker fix, which I checked line by line against the reviewer's own prescription. Approving, pinned to the reviewed commit. Two notes for the humans:
One caveat, plainly: the sandboxed 中文说明置信度：4/5 —— 人工评审的两个阻断项已按处方修复并有测试钉死，review job 在此 SHA 零发现，CI 在该 head 全绿，上次阻塞的合并冲突已消除；没给 5 是因为残余的非事务写入窗口，以及 @chiga0 尚未复核修复。 退一步看：我对该问题的独立设想与最终实现一致——基于受评审地图的路径前缀路由、负载最低+编号轮转、每 PR 一位 owner、写前复核权限与覆盖、defer 改为分配。diff 中每个文件都服务于同一目标。十轮评审加一次人工评审逼出了所有难点（null 作者形态、文件名伪造、快照到写入窗口、缺脚本 base 的引导、改名源路径、运行中 head 变化），且如今每项都有测试防御。上次移交的两个原因——合并冲突与红色 CI——均已解决：分支 MERGEABLE，本 head CI 零失败（含此前红掉的 ubuntu 套件）。此后唯一的新代码是阻断项修复，已逐行对照评审者处方核实。 批准，钉在所审提交上。给人类的两点说明：
一点坦白的保留：随本次触发的沙箱 — 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.
Three findings to address before this can land — the coverage check makes the routing a no-op on every core PR (CODEOWNERS auto-requests already occupy the pool), CI is red on the missing workflow size-baseline entry, and the new guard suite isn't wired into HELPER_TESTS. Details in my notes above. 🙏
b58d991 to
bafef3e
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push，因为这会使已有的评审评论失效。另外，供日后参考：作为集成流程的一部分，机器人始终会自动将所有改动压缩（squash）为单个提交。 |
yiliang114
changed the title
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/assign-pr-reviewer.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/assign-pr-reviewer.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
Closeout — cap-4 round on the four Criticals of the 18:36Z review, pushed R1-1 — coverage no-op FIXED ( R1-2 — missing token scope FIXED ( R1-3 — 100-file cap FIXED ( R1-6 — size ratchet FIXED ( Verification: The 7 Suggestions from this review round are deliberately deferred to a follow-up round (cap budget). |
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. |
|
Closeout — round-1 cap (4 of the 7 standing Suggestions fixed, one non-force push
Verification: Deferred to a future cap round (unchanged, still open): R1-9 (prefix-shape validation), R1-10 2/2 (case-insensitive idempotency test), R1-14 ( |
|
Closeout — remaining 3 round-1 Suggestions fixed (one non-force push
Verification: both guard suites |
|
@qwen-code /triage |
|
Sandboxed verification: ❌ not passed — findings reported (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: 72 passed · 0 failed · 72 total Flakiness gate: ✅ 1 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：72 通过 · 0 失败 · 72 总计 抖动门：✅ 1 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #9813 Deep Verification — feat(ci): request an area reviewer on PRs by changed-file pathsVerdict: 中文摘要结论：
Central claim
A/B table (wire oracle: real script driven through a logging
|
| # | Cell | Scenario | Oracle | Result |
|---|---|---|---|---|
| 02 | head | fresh core PR; all 5 CODEOWNERS owners already auto-requested (the live shape on PR open) | exactly one pr edit --add-reviewer <owner>; owner ∈ mapped ∖ CODEOWNERS |
✅ requested @… |
| 03 | control A (CODEOWNERS-exclusion reverted, 8cd770a) | identical | no pr edit; skip reason |
✅ skips already reviewing — without the hunk the routing never fires on the only configured path |
| 04 | head | 120 changed files, the only core file at index 115 | area matched, one request | ✅ requested @… via pulls/77/files --paginate |
| 05 | control B (file list via capped pr view --json files, 24f822b) |
identical | no pr edit; skip reason |
✅ skips no area path matched — silently unroutes |
Witness: evidence/01-ab-codeowners-pagination.png (all 33 cells as printed, 33/33 PASS).
Full cell list (skip policy, fork-403 graceful vs other-errors loud, TOCTOU re-checks, permission tolerance, rotation determinism 77→pomelo-nwu / 78→DennisYu07, env validation, no-PR-text census): 01-ab-harness.mjs, log logs/01-ab-harness.txt.
Call census for one write run (scripted, from the stub log): 19 gh invocations — 2× pr view (fields exactly state,isDraft,author,reviewRequests,latestReviews), 2× paginated files, 7× collaborator permission (candidates only, all before the write), 7× open-issue load, 1× pr edit. No call touches title, body, comments, or reactions (asserted over the full log).
Corrections
- Test count. The description says "56 tests"; the two guard files actually contain 70 tests (39 in
assign-pr-reviewer.test.mjs, 31 inassign-issue-owner.test.mjs), all green:# tests 70 / # pass 70 / # fail 0. - "qwen-triage-workflow.test.mjs fails 42 tests on clean origin/main" does not reproduce in this container: the full
HELPER_TESTSgate is green on both arms (head 422/422, base 383/383). Whatever environment produced the author's 42 failures, it is not thenode:22-bookwormCI lane. - Design doc trigger list.
docs/design/2026-08-23-pr-reviewer-auto-assignment.mdlists threepull_requesttypes; the shipped workflow has four (reopenedwas added in commit 339de6f and is pinned by the guard suite). Doc-only drift.
Findings
F1 (nit) — the committed size baseline is already stale: 2444 recorded vs 2886 actual
The ratchet entry added by commit c716758 says 2444 assign-pr-reviewer.yml ("record the current 2444-byte size"), but later commits in this same PR — the reopened trigger (339de6f) and the base-sha checkout pin with its CODEOWNERS sparse line (04fd5e4) — grew the file, and the entry was never bumped. At the verified head the file is 2886 bytes.
wc -c .github/workflows/assign-pr-reviewer.yml # 2886
grep assign-pr-reviewer .github/workflows/.size-baseline # 2444 assign-pr-reviewer.yml
git show HEAD^2:.github/workflows/assign-pr-reviewer.yml | wc -c # 2886 — stale at final commitImpact: the gate still passes (2886 ≤ 2444 + 4096 allowance), so this is not blocking — but the ratchet exists to make growth "a reviewed line", and the line being reviewed is 442 bytes behind reality, silently spending allowance the PR never asked for. The baseline file's own header says: "if the growth is real, bump the number and say why". Suggested fix: 2886 assign-pr-reviewer.yml.
F2 (nit) — paths validation accepts prefixes that can never match (../, //)
loadPolicy rejects empty entries, leading / (documented as "silently and permanently unroute the area"), and missing trailing / — but the sibling shapes with the same failure mode pass:
node -e 'import("./.github/scripts/assign-issue-owner.mjs").then(async m => {
const raw = require("fs").readFileSync(".github/issue-owners.json","utf8");
for (const p of ["packages/core/../", "packages//core/"]) {
const j = JSON.parse(raw); j.areas[0].paths = [p];
try { m.loadPolicy(JSON.stringify(j)); console.log("accepted:", p); }
catch (e) { console.log("rejected:", p); }
} })'
# accepted: packages/core/../
# accepted: packages//core/API file paths are normalized repo-relative, so neither can ever match — the area silently unroutes, exactly the hazard the leading-slash rule exists to prevent. No misrouting is possible (failure is absence, not wrong target), and the map is a reviewed file, hence nit severity. Measured candidate fix (logs/05-candidate-fix.txt, patch in mutants/candidate-fix/patch.diff): add path.includes('//') || path.split('/').includes('..') to the predicate — guard suite stays 70/0 (it pins nothing along this axis), both hostile fixtures go red, checked-in map parses and routes identically. If adopted, ship with the fixture.
Not covered
- Live GitHub behaviour. No credentials/network in this sandbox: no real
workflow_dispatchdry-run, no real reviewer request, no real fork read-only token. The fork 403 cell replays the wire shape (HTTP 403onpr edit), not the token semantics that produce it. The replay is uncalibrated — there is no real emitted artifact from a production run of this workflow to reproduce byte-for-byte (it has never run; first round, noprevious-report.md). Calibration will become possible after the first real dispatch writes a step summary. - Per-commit attribution. Checkout is depth 2:
git rev-list --count HEAD^1..HEAD^2returns the shallow artifact1while the metadata snapshot lists 12 commits, so individual commits were not exercisable. Verified the aggregateHEAD^1..HEADdiff instead; commit messages were used only as hypotheses. - yamllint: container has no
pip, so the pinned yamllint could not be installed (scripts/lint.js --setupfails at that step). actionlint covers the workflow YAML and was proven live; yamllint style rules on the new file remain unverified here. - Repo-wide
npm run lint:ciand the full vitest profile were not run (CI covers them); I ran scoped eslint + prettier on the changed files and thetest:scriptsgate instead. - Pre-existing failures observed on both arms (attributed, not caused by the PR):
workflow-size.test.jsfails 1 test on head and base, byte-identical message —cd-cua-driver.ymlgrew to 42519 vs baseline 29715+4096;install-script.test.jsfails to collect on head and base because the container ships nozip/unzip(A/A control). - Day-one arrival-rate math for the new trigger: depth-2 history prevents local PR-arrival counting. The job is bounded (
timeout-minutes: 5, one node process, 19 API calls per write run, zero writes after the first request per PR thanks to idempotency). To confirm the event volume, a maintainer can run:gh api 'repos/QwenLM/qwen-code/pulls?state=all&per_page=100' --paginate --jq '[.[] | .created_at] | length'over the last 30 days.
Targeted gates executed
| Gate | Head | Base | Result |
|---|---|---|---|
Guard suite (node --test on both files, the plan's exact command) |
70/70, exit 0 | n/a (files new) | ✅ witness evidence/03-guard-suite-green.png |
HELPER_TESTS full list (exact CI invocation) |
422/422, exit 0 | 383/383, exit 0 | ✅ +39 tests, zero regressions |
npm run test:scripts (incl. size ratchet) |
1592/1593 | base measured for the two files that fail on head: workflow-size.test.js 180/181 (same sole failure), install-script.test.js same collection error |
sole failure cd-cua-driver.yml identical on both arms (pre-existing); the PR's 3 new ratchet tests are green (head 184 vs base 181 tests) |
| actionlint (repo-pinned binary, exact repo flags) | clean | — | ✅ gate proven live: planted types: ["not-a-real-type"] is caught |
| shellcheck | exit 0 (pre-existing warnings only in untouched files) | — | ✅ |
prettier --check on the 7 prettier-parseable changed files |
clean | — | ✅ proven live with a planted violation (--ignore-path /dev/null; prettier ignores files outside the project root by default) |
eslint (scoped to the 3 changed .mjs) |
clean | — | ✅ |
| Mutation matrix on the guard suite | 12/12 mutants as predicted; control 70/70 | — | ✅ witness evidence/02-mutation-matrix.png |
| Issue-assignment backward compat (head map vs base map, end-to-end) | 4/4; gh logs byte-identical | — | ✅ |
Mutation matrix detail
| Mutant | Reverted guard | Suite result | Pinned by |
|---|---|---|---|
| M00 control | none | 70/70 green | — |
| M01 | coverage-pool exclusion (8cd770a, hunk 1) | 68/70 red | does not treat CODEOWNERS auto-requests as coverage, requests on top of CODEOWNERS auto-requests |
| M02 | pick exclusion (8cd770a, hunk 2) | 68/70 red | requests on top of CODEOWNERS auto-requests, never requests the PR author… |
| M03 | both hunks together (combination row) | 67/70 red — strictly more than either single revert | union of the above |
| M04 | --paginate dropped (24f822b) |
69/70 red | sources the changed-file list from the paginated files endpoint |
| M05 | paths shape validation (01d1d67) | 67/70 red | the three rejects… map tests |
| M06 | dispatch dry_run default true→false (46eb927) |
69/70 red | defaults manual dispatches to report-only |
| M07 | author exclusion (5222e73) | 69/70 red | never requests the PR author to review their own work |
| M08 | case-sensitive coverage match (a4ad137) | 69/70 red | matches requested reviewers against the map case-insensitively |
| M09 | job issues: read scope (250376b) |
69/70 red | grants pull-requests:write to the job… |
| M10 | reopened trigger (339de6f) |
69/70 red | fires on PR updates… |
| M11 | repository if: guard |
69/70 red | runs only on the canonical repository |
| M12 | positive control in the other file (pickOwner rotation) |
68/70 red | assign-issue-owner: owner selection — proves the runner collects both files |
All reds are expected-vs-actual assertion failures (e.g. M02: The input did not match the regular expression /pr edit 77 .*--add-reviewer pomelo-nwu/ against the recorded stub log), not import or compile errors. Both CODEOWNERS-exclusion hunks are pinned independently — neither is redundant defence.
Methodology
Environment: the designated CI sandbox (container, no credentials), working tree at refs/pull/9813/merge (depth 2); npm ci + npm run build pre-done; Node v22.23.2. The PR changes no package.json/lockfile, so reusing the root node_modules is a clean control; the only non-builtin runtime dependency of the scripts is yaml, realpath /__w/qwen-code/qwen-code/node_modules/yaml (a real directory, shared, untouched by the PR) — no workspace symlinks cross the A/B boundary. All harnesses live in this artifact dir and drive the real .mjs scripts from dist-free source: 01-ab-harness.mjs (wire oracle; fake gh peer logs every invocation; control builds revert one hunk each in scratch copies), 02-mutation-matrix.mjs (scratch trees per mutant under mutants/, running the PR's own test files via node --test), 03-issue-compat.mjs, 04-extra-probes.mjs, 05-candidate-fix.mjs. Raw per-cell/per-mutant output: logs/ (incl. gate-helper-head.txt, gate-helper-base.txt, gate-guard-suite-head.txt, mutant-*.txt). Image evidence in evidence/ captured via scripts/verify-capture.mjs. Base arm used git worktree tmp/base-tree at HEAD^1, removed after the run.
Flakiness gate log
rounds=5 files=1 skipped=0
file .github/scripts/assign-pr-reviewer.test.mjs: (cd .) node --test ./.github/scripts/assign-pr-reviewer.test.mjs
per-file results (P=pass F=fail I=infra-exit, one letter per run):
.github/scripts/assign-pr-reviewer.test.mjs: PPPPP
verdict: pass
summary: 1 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · .github/scripts/assign-pr-reviewer.test.mjs: P (exit 0)
round 2 · .github/scripts/assign-pr-reviewer.test.mjs: P (exit 0)
round 3 · .github/scripts/assign-pr-reviewer.test.mjs: P (exit 0)
round 4 · .github/scripts/assign-pr-reviewer.test.mjs: P (exit 0)
round 5 · .github/scripts/assign-pr-reviewer.test.mjs: P (exit 0)
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.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/assign-pr-reviewer.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/assign-pr-reviewer.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.22.0)
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. ✅
|
Closeout update:
|
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/assign-pr-reviewer.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
.github/scripts/assign-pr-reviewer.mjs:150 — [review] four infrastructure helpers pasted verbatim from assign-issue-owner.mjs instead of imported
Convergence: round 3 posted 9 inline comment(s), 4 of them reported for the first time; the previous round posted 10 (10 new). Findings keep coming back to the same files: .github/scripts/assign-pr-reviewer.mjs (findings in round 2; 2 more now); .github/workflows/assign-pr-reviewer.yml (findings in round 2; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/assign-pr-reviewer.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 3 轮发布了 9 条行内评论，其中 4 条是首次提出；上一轮发布了 10 条（其中 10 条首次提出）。发现反复回到同一批文件：.github/scripts/assign-pr-reviewer.mjs（第 2 轮已出过发现，本轮又有 2 条）；.github/workflows/assign-pr-reviewer.yml（第 2 轮已出过发现，本轮又有 1 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.0)
75d93aa to
5f3a251
Compare
The defer path resolves the maintainer deterministically from the existing owner map (same area/labels/load/rotation logic as issue assignment) and assigns the PR to them, instead of an ambiguous $QWEN_MAINTAINER_HANDLE / most-recent-reviewer chain. Replaces the reviewer-request routing prototype entirely.
5f3a251 to
d2c4cd6
Compare
`gh pr view --json author` exports `"author": null` for deleted accounts; guard both author dereferences so every later trigger skips gracefully (exit 0) instead of throwing and running the assignment check red. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
The routing tests only pinned core-skills and core-goals; a typo in any of the six other module prefixes would silently reroute those PRs to the generic fallback with the suite green. Probe every mapped prefix — fixed literals, so a typo'd prefix fails instead of shifting the probe with it. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
The assign-on-defer heredoc dereferenced pr.author.login unguarded; with "author": null the eligibility filter threw and 2>/dev/null silently bypassed deterministic owner resolution. Null-safe exclusion keeps the resolver running, matching the jq fallback below it. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Closeout — all 4 unresolved findings verified real at
Mutation checks: reverting either guard → 2 tests fail; corrupting one prefix in issue-owners.json → 1 test fails; all restored green. All 4 threads replied with evidence + SHA and resolved. Push auto-triggers re-review; no |
|
CI attribution for the failing
Leaving the re-run to maintainers / next push; no code change warranted. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Deferred under the convergence posture (round 8, not a blocker) — recorded, not requested in this round:
.github/scripts/assign-pr-owner.mjs:100 — [review] five helper blocks byte-copied from assign-issue-owner.mjs instead of imported.github/scripts/assign-pr-owner.mjs:162 — [review] changedFiles() runs before the skip checks.github/scripts/assign-pr-owner.mjs:95 — [probe] the null-review-author guard in alreadyCovered is pinned by no test (mutation survives).qwen/skills/triage/references/pr-workflow.md:787 — [review] defer-assign stacks a second owner without a coverage check.qwen/skills/triage/references/pr-workflow.md:723 — [probe] resolver's node invocation contradicts the skill's no-interpreter rule and slips past the node deny entry.github/scripts/assign-pr-owner.mjs:91 — [probe] COMMENTED (comment-only) reviews count as coverage, permanently blocking assignment
中文说明
收敛姿态下延后（第 8 轮，非阻断）——已记录，本轮不要求修改：共 6 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
@qwen-code /triage |
|
Sandboxed verification: Skipped because the PR has merge conflicts, so refs/pull/9813/merge is unavailable — resolve conflicts and re-run. 中文 — 判定：
|
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
|
Correction to my earlier CI attribution: the exact failed log does not show an ENOSPC error. The red result is three unrelated ESLint integration tests timing out at 30s, followed by Vitest worker RPC |
|
Stage 3–4 closeout: the review queue is clean (53/53 threads resolved; 0 unresolved), but the PR still needs a base refresh. The current conflict is limited to two workflow files:
That resolution is mechanically clear, but I did not merge or push it from this worker because the required post-merge workflow/helper tests cannot be validated reliably here: this worktree's The existing Ubuntu failure is unrelated runner/load flakiness, not a PR regression: three untouched ESLint integration tests timed out at 30s, followed by Vitest worker RPC |
Resolve the two mechanical conflicts from the main sync: - ci.yml HELPER_TESTS: union of both sides' additions — main's ci/classify-platform-sensitivity.test.mjs in its inserted position plus this PR's trailing assign-pr-owner.test.mjs. - .size-baseline: ratchet ci.yml to the merged file's actual byte size (79063), per the same-PR-update rule, instead of keeping either side's stale record. Co-authored-by: Qwen-Coder <qwen-coder @alibabacloud.com>
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/assign-pr-owner.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 9, not a blocker) — recorded, not requested in this round:
.qwen/skills/triage/references/pr-workflow.md:722 — [review] the new assign-on-defer resolver heredoc has no test.github/scripts/assign-issue-owner.mjs:90 — [probe] paths validation never pins tree/config agreement (stale directory silently unroutes the module).qwen/skills/triage/references/pr-workflow.md:741 — [probe] defer resolver routes labels-only and can never reach the module areas this PR adds
中文说明
仅完成部分审查，审查缺口已披露。
未审查：reverse audit — did not converge within the reverse-audit round cap of 5。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/assign-pr-owner.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 9 轮，非阻断）——已记录，本轮不要求修改：共 3 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Two routing-correctness blockers remain at the current head.
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Two routing-correctness blockers at the current head (confirmed independently of prior reviews).
CI: Ubuntu Node 22.x passes (25m 16s). build-cli, Integration Tests, and macOS/Windows Node tests are SKIPPED. The change is entirely .github/ scripts; no platform-sensitive code paths, so the cross-platform skips are non-material. build-cli skip is disclosed per coverage policy.
Checked: changedFiles() filename-decoding (base64 path, newline-injection defence); alreadyCovered() pool scope; matchedAreasByPath() longest-prefix sort; pre-write re-fetch fields; skipPrReason() null-author guard; workflow pull_request_target checkout pinning (base.sha, persist-credentials: false, sparse-checkout membership); isPathPrefix validation rules in loadPolicy.
Not checked: pr-workflow.md resolver heredoc end-to-end execution; integration of assign-pr-owner.mjs against the live GitHub API; Windows/macOS behaviour (no host).
Blocker 1 — renamed-file source paths are invisible to the router
changedFiles() uses .[].filename | @base64, which returns only the destination path. GitHub's pulls/:pr/files endpoint also exposes previous_filename for renamed/moved files, but this query discards it.
Trigger: any PR that moves a file out of a mapped area (e.g. packages/core/src/foo.ts → packages/cli/src/bar.ts). The router sees only the destination, so the source area is never matched. The PR receives no assignment even though it removes code from a mapped area.
Fix: include both fields, e.g. .[].filename, (.[].previous_filename // empty) | @base64, so the returned list always covers the full set of affected paths.
Blocker 2 — pre-write re-fetch does not compare head SHA
The final re-fetch before gh pr edit --add-assignee checks state,isDraft,author,assignees,latestReviews but not headRefOid. The matched area was derived from the opening snapshot's file list; up to ~30 sequential API calls (one permission check + one load check per candidate) sit between that snapshot and this write.
Trigger: two pushes in rapid succession. With cancel-in-progress: false, runs serialise — Run A (from push 1) completes and assigns based on push 1's files; Run B (from push 2) starts, sees the assignee from Run A, and alreadyCovered returns true, permanently skipping re-evaluation even if push 2 changed which area applies.
Fix: include headRefOid in the re-fetch and bail out (skip, do not assign) when it has changed since the opening snapshot, so the queued run for the newer push handles the correct file list.
Reviewed with AI assistance.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/assign-pr-owner.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 10, not a blocker) — recorded, not requested in this round:
.github/scripts/assign-pr-owner.mjs:61 — [probe] equal-length prefix tie-break is pinned by no test (mutation flips the owner, suite stays green).qwen/skills/triage/references/pr-workflow.md:726 — [probe] assign-on-defer resolver heredoc is untested; 2>/dev/null hides module-load failures.github/scripts/assign-pr-owner.mjs:93 — [probe] alreadyCovered's null-review-author guard survives mutation — no test supplies a null-author review.github/scripts/assign-issue-owner.mjs:31 — [probe] isPathPrefix '.'-segment branch unpinned — mutant survives the whole suite.github/scripts/assign-pr-owner.test.mjs:603 — [probe] workflow-invariant tests use partial patterns — head.ref/merge-ref spellings, widened guard, and mutable checkout tag all land green.github/scripts/assign-pr-owner.test.mjs:171 — [probe] alreadyCovered's cross-area pool survives mutation — no test presents a mapped owner absent from areas[0]
中文说明
仅完成部分审查，审查缺口已披露。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/assign-pr-owner.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 10 轮，非阻断）——已记录，本轮不要求修改：共 6 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
@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: 153 passed · 0 failed · 153 total Flakiness gate: ✅ 2 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：153 通过 · 0 失败 · 153 总计 抖动门：✅ 2 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #9813 Deep Verification (round 2) — feat(triage): assign one accountable owner when a PR opens, and when deferringVerdict: 中文摘要结论：
Previous-finding status (follow-up round)The previous round verified head
The previous round's three Corrections (test count, author's 42-failure claim, design-doc trigger list) referred to the deleted prototype and no longer apply to this head. Central claim
A/B table (wire oracle: real script driven through a logging
|
| # | Cell | Scenario | Oracle | Result |
|---|---|---|---|---|
| A1 | head | fresh core PR, one candidate at load 0 | exactly one pr edit --add-assignee, least-loaded owner |
✅ assigned @DennisYu07 (0 open) |
| A2 | base | feature absence | script & workflow exist at HEAD^1? |
✅ both missing — the PR is the sole cause of any assignment |
| C-base64 | control | pre-fix newline split; forged x<LF>packages/core/poc as the only file |
area routed via phantom entry | ✅ attack succeeds (one pr edit) — head skips it (A27) |
| C-toctou | control | pre-write re-fetch removed; owner lands mid-run | second assignee stacked | ✅ one pr edit on a covered PR — head skips (A21) |
| C-paginate | control | --paginate removed; core file is #120 of 120 |
area matched | ✅ silently unroutes — head routes (A26) |
| C-headref | control | headRefOid equality check removed; head changes mid-run | write aborted | ✅ assigns against stale head — head skips (A23) |
Witnesses: 01-ab-head-cells.png (43 cells as printed, 43/43 PASS) and 02-ab-controls.png (4/4). Cell inventory (all scripted in 01-ab-harness.mjs / 01b-controls.mjs, logs in logs/01-*.txt): idempotency vs mapped assignee and reviewer (case-insensitive) · unmapped assignee still routes · skip policy (merged/draft/bot/deleted-author/no-match/dry-run) · module routing (skills→wenshao, mixed files→longest prefix, runtime→yiliang114) · fallback on module-owner-as-author and on denied module owner · terminal skip when nobody passes the access check (both read and lookup-error routes) · author never self-assigned · rotation determinism (equal loads: PR77→LaZzyMan, PR78→doudouOUC, repeat-run identical) · rename counts previous_filename · 403 tolerated (exit 0) vs other errors re-thrown (exit 1).
Call census for one write run (scripted, from the stub log): 34 gh invocations — 2× pr view (fields exactly state,isDraft,author,assignees,latestReviews,headRefOid), 1× paginated files, 15× collaborator permission (all candidates, all pre-write), 15× open-issue load (all pre-write), 1× pr edit. No call mentions title, body, or comments anywhere in the log.
Corrections
- Body §1 routing rule. The description's first half says
pathsmatching is "first match wins"; the implementation (and body §3) is longest matching prefix wins — "first match wins" is the label rule for issue areas, which module areas deliberately sit after. Doc-only clarification; no code change requested.
Findings
S1 (Suggestion, non-blocking) — --paginate is load-bearing but unpinned by any test
Mutation row M05 (drop the single '--paginate', line from changedFiles) leaves the full guard suite green (60/60) — no test distinguishes head from that mutant. The behaviour itself is load-bearing: control build C-paginate proves a 120-file PR whose only core file sits at #120 is silently unrouted without it (no area path matched), which is the exact failure class this PR's paths validation exists to prevent. Classification: coverage gap (behaviour right, nothing asserts it). One fixture would pin it — the suite's stub already branches on the files call, so serving 120 entries only when --paginate is present (as this round's harness does) and routing a core file at index 115+ turns the mutant red.
Informational — the two coverage checks are layered defence, both correct
Reverting only the initial alreadyCovered check (M03) leaves the suite green because the pre-write re-check subsumes it; reverting only the pre-write check (M02) turns three tests red; reverting both (M04) turns four red. The initial check is therefore redundant behaviourally — it exists to skip ~30 API calls on already-covered PRs — and is correct exactly as it stands. Not a finding.
Not covered
- Live GitHub behaviour. No credentials/network in this sandbox: no real
pull_request_targetdispatch, no real assignment write, no real fork token. The 403 cell replays the wire shape (HTTP 403onpr edit), not the token semantics that produce it. The replay is uncalibrated — the workflow has never run in production, so there is no real emitted artifact to reproduce byte-for-byte; calibration becomes possible after the first real run writes a step summary. - Per-commit attribution. Checkout is depth 2 (
git rev-parse --is-shallow-repository= true):git rev-list HEAD^1..HEAD^2returns the shallow artifact1while the metadata snapshot lists 25 commits, so individual commits were not exercisable. Verified the aggregateHEAD^1..HEADdiff; commit messages were used only as hypotheses. - yamllint:
scripts/lint.js --setupfails at that step (pip3: Permission denied— same container blocker as the previous round). actionlint covers the workflow YAML and was proven live; yamllint style rules on the new file remain unverified here. - Drift against live main. No network/token to fetch current
main. The merge ref was built cleanly by GitHub against base9b3ccee5; the PR's own last commit is a conflict-resolution merge oforigin/mainwhose two conflict files (ci.ymlHELPER_TESTS union,.size-baselineratchet to 79063) were both re-verified here. - PR-body verification claims ("verified against real PRs refactor(core): remove unused LruCache utility #9926/fix(review): reply carried findings into their thread, resolve fixed ones #9940/fix(cli): Rank skills last in root slash completion #9943") cannot be checked without API access.
- Day-one arrival-rate math: depth-2 history prevents local PR-arrival counting. The job is bounded (
timeout-minutes: 5, one node process, 34 API calls per write run, zero writes after the first assignment per PR thanks to idempotency, serialized per-PR concurrency). To confirm event volume, a maintainer can run:gh api 'repos/QwenLM/qwen-code/pulls?state=all&per_page=100' --paginate --jq '[.[] | select(.created_at > "2026-07-26")] | length'. - Working-tree divergence (environment note, not a PR finding). The container's checkout carried an uncommitted revert of
.qwen/skills/triage/references/pr-workflow.md(the defer resolver deleted, i.e. the base text). All harnesses used the committed HEAD content — the defer block was extracted viagit show HEAD:...— and the divergence was left untouched. - Repo-wide
npm run lint:ciand the full vitest profile were not run (CI covers them); scoped prettier/eslint, the exactHELPER_TESTSinvocation, the size-ratchet script + its vitest mirror, and the ownership-workflow vitest ran instead.
Targeted gates executed
| Gate | Head | Base | Result |
|---|---|---|---|
HELPER_TESTS (exact CI invocation, YAML-extracted list) |
431/431, exit 0 | 402/402, exit 0 | ✅ +29 = the new tests, zero regressions (witness 06-gates.png) |
Size ratchet check-workflow-size.sh |
exit 0 | exit 0 | ✅ baseline entries byte-exact at head (2125, 79063) |
Size ratchet vitest mirror + ownership-workflow tests (test:scripts scope, 2 files) |
202/202, exit 0 | — | ✅ the previous round's pre-existing cd-cua-driver.yml size failure no longer reproduces at this head (main bumped its baseline to 49610); log gate-vitest-head.txt |
| Guard suite (mutation control, both files together) | 60/60 | n/a | ✅ |
| Flakiness: 2 changed test files × 5 rounds | 10/10 green | — | ✅ no divergence (logs/flakiness.txt) |
| actionlint (repo-pinned 1.7.12, full tree) | clean | — | ✅ proven live: planted types: ["not-a-real-type"] caught |
| shellcheck (repo-pinned 0.11.0, full tree) | exit 0; warnings only in untouched scripts/test-rewind-e2e.sh |
— | ✅ proven live (planted SC2034); the extracted defer block itself passes clean |
prettier --check on the 7 changed files |
clean | — | ✅ proven live via stdin reformat |
eslint on the 4 changed .mjs |
clean | — | ✅ |
| Issue-assignment backward compat (base/head × maps, end-to-end) | gh logs byte-identical across all cells | — | ✅ (9/9) |
F2 sibling sweep through changedFiles decode |
8/8 shapes one-entry-intact | — | ✅ (16/16) |
Mutation matrix detail (witness 05-mutation-matrix.png; 25/25 scripted, control 60/60)
| Mutant | Reverted guard | Suite result | Pinned by |
|---|---|---|---|
| M00 control | none | 60/60 green | — |
| M01 | base64 decode → newline split | 2 red | untrusted filename decoding |
| M02 | pre-write re-fetch removed | 3 red | apply boundary (the three TOCTOU tests) |
| M03 | initial alreadyCovered only removed |
survives (green) — redundant defence (pre-write check subsumes it) | — |
| M04 | both coverage checks removed (combination row) | 4 red | proves the set is load-bearing |
| M05 | --paginate dropped |
survives (green) — coverage gap, see Finding S1 | — |
| M06 | canWrite always true |
2 red | denied/failing push-access tests |
| M07 | non-permission edit errors swallowed | 1 red | re-throw test |
| M08 | author exclusion removed | 2 red | self-assignment + fallback tests |
| M09 | case-sensitive coverage pool | 1 red | case-insensitive coverage test |
| M10 | paths validation disabled |
1 red | assign-issue-owner: owner map |
| M11 | trigger pull_request_target→pull_request |
1 red | workflow invariants |
| M12 | sparse-checkout sibling entry dropped | 1 red | workflow invariants |
| M13 | pickOwner rotation broken (other file) |
1 red | positive control: runner collects both files |
All reds are expected-vs-actual assertion failures, not import/syntax errors (scripted per row).
Methodology
Environment: the designated CI sandbox (container, no credentials), working tree at refs/pull/9813/merge (depth 2); npm ci + npm run build pre-done; Node v22.23.2, jq 1.6. The PR changes no package.json/lockfile (verified via diff stat), so reusing the root node_modules is a clean control; the scripts' only non-builtin dependency yaml realpath-resolves into the shared root tree, and no workspace symlinks cross the A/B boundary. Harnesses (all in this artifact dir, all driving real committed code): 01-ab-harness.mjs (43-cell wire oracle; logging gh stub in gh-stub/), 01b-controls.mjs (four hunk-revert scratch builds under mutants/c-*, each syntax-checked before running), 02-paths-validation.mjs (F2 re-measure vs base module), 03-compat.mjs + gh-issue-stub/ (issue-assignment compat), 04-defer-replay.mjs + gh-defer-stub/ (defer block extracted verbatim from git show HEAD:.qwen/skills/triage/references/pr-workflow.md — 68 lines, bash -n clean — driven under bash --noprofile --norc with session shell vars per the skill's convention), 05-mutation-matrix.mjs (scratch trees per mutant under mutants/suite-*, running the PR's own test files via node --test), 07-gates.mjs, 08-sibling-probes.mjs. Raw per-cell output in logs/ (incl. gate-helper-head.txt, gate-helper-base.txt, gate-vitest-head.txt, flakiness.txt). Assertion total: 43 (A/B) + 4 (controls) + 16 (F2) + 9 (compat) + 20 (defer) + 25 (matrix) + 18 (gates) + 16 (siblings) + 2 (vitest gate runs) = 153. Image evidence in evidence/ captured via scripts/verify-capture.mjs. The base arm used git worktree tmp/base-tree at HEAD^1, removed after the A/B cells were captured.
Flakiness gate log
rounds=5 files=2 skipped=0
file .github/scripts/assign-issue-owner.test.mjs: (cd .) node --test ./.github/scripts/assign-issue-owner.test.mjs
file .github/scripts/assign-pr-owner.test.mjs: (cd .) node --test ./.github/scripts/assign-pr-owner.test.mjs
per-file results (P=pass F=fail I=infra-exit, one letter per run):
.github/scripts/assign-issue-owner.test.mjs: PPPPP
.github/scripts/assign-pr-owner.test.mjs: PPPPP
verdict: pass
summary: 2 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · .github/scripts/assign-issue-owner.test.mjs: P (exit 0)
round 1 · .github/scripts/assign-pr-owner.test.mjs: P (exit 0)
round 2 · .github/scripts/assign-issue-owner.test.mjs: P (exit 0)
round 2 · .github/scripts/assign-pr-owner.test.mjs: P (exit 0)
round 3 · .github/scripts/assign-issue-owner.test.mjs: P (exit 0)
round 3 · .github/scripts/assign-pr-owner.test.mjs: P (exit 0)
round 4 · .github/scripts/assign-issue-owner.test.mjs: P (exit 0)
round 4 · .github/scripts/assign-pr-owner.test.mjs: P (exit 0)
round 5 · .github/scripts/assign-issue-owner.test.mjs: P (exit 0)
round 5 · .github/scripts/assign-pr-owner.test.mjs: P (exit 0)
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.
LGTM — both human-review blockers fixed as prescribed and pinned by tests, review job converged to zero findings at this SHA, and CI is fully green on this exact head. ✅
Maintainer verification: real Actions runs + live-queue replayI verified this on a real stack rather than by reading the diff: six real GitHub Actions runs of Headline: the mechanism works — the one thing I could not settle locally (does Harness — what was real and what was substituted
What holds up1. Real
The PR really does end up assigned:
2. 3. Idempotent on repeated pushes. The next
4. Every skip branch reproduces on live production data (
5. Dry-run writes nothing. The wire-tee ledger recorded 141 real 6. 7. No regression to issue assignment. Replaying 8. The new tests are load-bearing. 8/8 mutations went red:
FindingsF1 — A failed assign shows up as a red ✗ on the contributor's PR (please fix before merge)Two real runs failed, both surfacing as a failed check on the PR:
Neither message matches the graceful-skip filter, which only recognises Two asymmetries worth closing:
Severity note: F2 — "Longest matching prefix wins" becomes "longest directory name wins" between siblingsLongest-prefix is a specificity rule only when one prefix nests inside the other ( The extreme case: #9812 touches 2656 files across every core module and routes to F3 — The load metric counts issues only, so PR assignment has no feedback loopProven against the live API: Consequence on today's queue: Compounding it: 7 of the 9 areas have exactly one owner, so load and rotation do nothing there at all. Replaying the whole open queue:
F4 — The module → owner map does not match the history it was drafted fromThe PR says the map came from six months of per-directory commits and asks for correction in review, so here is that data (
Two things stand out:
Commit count is of course a proxy — if these owners were chosen for review load rather than authorship, that is a fine answer; it just is not what the PR description says the map was derived from. F5 — Part 2 (the defer path) cannot fire today
Running the
So all four branches are correct — but in production the deterministic owner-map branch is unreachable until PRs start carrying area labels. The new (Also verified: the relative F6 — A review counts as coverage, so the existing backlog stays ownerless
New PRs are unaffected — at F7 — Minor
VerdictRecommend merge once F1 is addressed. It is the only finding that puts a red ✗ on a contributor's PR, and both halves of the fix are small and match patterns already in the file ( F2–F4 are tuning rather than correctness, but F4 is worth correcting before this starts routing — the PR explicitly asks reviewers to fix the map, and Everything the description claims about safety checks out at the wire: trusted-base checkout, no PR-authored text read, sparse credential-free checkout, push access re-verified live, pre-write re-fetch, base64 filename decoding. 中文版报告（点击展开）维护者验证：真实 Actions 运行 + 全量队列回放我没有停留在读 diff，而是搭了真实环境：在 fork 上真跑了 6 次 GitHub Actions（真实 结论先说：机制是work的。本地唯一无法回答的问题—— 验证手段说明
站得住的部分
问题清单F1 — assign 失败会在贡献者 PR 上留下红叉（建议合并前修） 两次真实运行失败，且都表现为 PR 上的失败检查：
两条错误都不匹配 有两处不对称值得补齐：
严重性说明： F2 —「最长前缀优先」在兄弟模块之间退化成「目录名更长者胜」 只有当一个前缀嵌套在另一个里面时（ 极端案例：#9812 改动 2656 个文件、横跨 core 全部模块，最终路由到 F3 — 负载口径只数 issue，PR 分配没有反馈闭环 线上实证： 今天队列上的后果： 叠加问题：9 个 area 里有 7 个只有一位 owner，负载与轮转在那里完全不起作用。全量回放分布：
F4 — 模块 owner 映射与它自称的依据（提交历史）对不上 PR 说这份映射来自最近半年的分目录提交记录并请求 review 校正，那就把这份数据摆出来（
两点最突出：
当然提交数只是代理指标——如果这些 owner 是按 review 负担而非编码量选的，那完全合理；只是 PR 描述里说的依据不是这个。 F5 — 第二部分（defer 路径）今天根本触发不了
逐字执行
四条分支逻辑都正确——但在生产环境里，"确定性 owner 地图"这条分支在 PR 开始带 area label 之前是不可达的。新增的 （另外已验证： F6 — review 被当作"已覆盖"，导致存量积压永远拿不到责任人
新 PR 不受影响—— F7 — 次要
结论建议在处理 F1 后合并。 它是唯一会在贡献者 PR 上留下红叉的问题，而两半修复都很小，且与文件里已有的写法一致（ F2–F4 属于调优而非正确性，但 F4 值得在它开始真正路由之前修正——PR 自己就请求 review 校正这份映射，而 描述中关于安全性的每一条都在协议层面得到确认：可信 base 检出、不读取任何 PR 正文、稀疏且无凭据的 checkout、写入前复核 push 权限、写入前重新拉取、文件名 base64 解码。 Actions runs used as evidence / 作为证据的 Actions 运行
|
# Conflicts: # .github/workflows/.size-baseline # .github/workflows/ci.yml
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
5 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- changedFiles() fetch-before-skip ordering (D11-1) — already reported as a deferred finding in the round 6 and round 8 review bodies
- assign-on-defer resolver heredoc untested (D11-2) — already reported as a deferred finding in the round 9 and round 10 review bodies
- alreadyCovered null-review-author guard unpinned (D11-5) — already reported as a deferred finding in the round 8 and round 10 review bodies
- $comment points at a contradicting design doc (D11-6) — already reported as a deferred finding in the round 6 and round 7 review bodies
- bootstrap-guard invariant regex lacks a start anchor (D11-8) — already reported as a deferred finding in the round 6 review body
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/assign-pr-owner.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 11, not a blocker) — recorded, not requested in this round:
.github/scripts/assign-pr-owner.mjs:159 — [probe] 'never reads PR text' invariant unpinned.github/scripts/assign-issue-owner.mjs:31 — [probe] paths validation accepts glob/whitespace prefixes.github/scripts/assign-pr-owner.test.mjs:457 — [probe] previous_filename newline defense unpinned
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 5 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：reverse audit — did not converge within the reverse-audit round cap of 5。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/assign-pr-owner.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 11 轮，非阻断）——已记录，本轮不要求修改：共 3 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.2)
The triage lane's permissions.deny blocks any run_shell_command whose text contains a backtick, so the template literal in canWrite() and the backticked "author": null comment made the whole resolver command EXECUTION_DENIED before approval. Use string concatenation, de-backtick the comment, and pin the invariant with a witness test that turns red if a backtick is reintroduced into the resolver block. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
CI failure classification for run 33157589820 at head a78d5d5: not caused by this PR.
Failed-jobs rerun triggered. |
4 participants
Motivation / 动机
Today, when a PR needs a human maintainer (e.g. a fork PR hitting the approval guardrail), the triage bot only @mentions someone — easily lost in notification noise, and nobody's
Assignedfilter ever shows the PR. Worse, assignment only happens in the "AI is unsure" fallback path, so most PRs that touch core areas never get an accountable owner at all.目前 triage bot 在需要人工介入时只会 @mention，容易淹没在通知里，且没有任何人的 Assigned 过滤器会显示该 PR。更关键的是，assign 只发生在"AI 无法判断"的兜底路径，绝大多数触碰核心领域的 PR 从头到尾都没有一位责任人。
Changes / 改动
Two halves, one goal: every PR touching a mapped area ends up with exactly one accountable owner.
两部分改动，同一个目标：每个触碰已映射领域的 PR 都恰好有一位责任人。
1. Assign on open (the main path) — a new
Assign PR ownerworkflow runs onpull_request_target(fork PRs covered; checkout stays on the trusted base ref). Routing is a pure function of the PR's changed file paths and.github/issue-owners.json(new optionalpathsfield per area, first match wins) — no model runs and the script never reads PR title/body/comments, so untrusted PR text cannot steer who gets assigned. The pick reuses the exact load metric and rotation of issue assignment (shared with the existing issue-assignment script), re-verifies push access at write time, and no-ops once a mapped owner is already assigned or has reviewed, so repeated pushes never stack assignments.1. PR 打开时即分配（主路径） — 新增
Assign PR ownerworkflow，触发于pull_request_target（覆盖 fork PR；checkout 固定在可信 base ref）。路由是"PR 改动文件路径 +.github/issue-owners.json（新增可选paths字段，首匹配生效）"的纯函数——不跑模型、脚本从不读取 PR 标题/正文/评论，不可信的 PR 内容无法操纵分配结果。选人复用 issue 分配完全相同的负载口径与编号轮转（与现有 issue 分配脚本共用同一套选人逻辑），写入前复核 push 权限，且一旦某位映射 owner 已是 assignee 或已 review 即 no-op，反复 push 不会叠加分配。2. Assign on defer (the fallback) — when the triage bot defers a PR to a human, it now resolves one maintainer deterministically (env override → owner map by labels → most recent human reviewer) and assigns the PR, instead of only mentioning.
2. defer 时分配（兜底路径） — triage bot 将 PR 移交给人类时，现在会确定性地解析一位 maintainer（环境变量 → 按 label 查 owner 地图 → 最近人类 reviewer）并 assign，而不是仅仅 mention。
Currently only the
corearea declarespaths(packages/core/); other areas can opt in by adding apathslist — issue assignment ignores it.目前只有
core领域声明了paths（packages/core/）；其他领域加一个paths列表即可启用，issue 分配不受该字段影响。3. Module-level routing — the longest matching path prefix wins, so a PR goes to the owner of the specific module it touches, not the whole-area pool. It only falls back to the area pool when no module claims the change, or the module owner authored the PR / lacks push access. The initial core module map was drafted from the last six months of per-directory commit history and is meant to be corrected in review.
3. 模块级路由 — 最长路径前缀优先：PR 分配给它实际触碰的那个模块的负责人，而不是整个领域的大池子。只有在没有任何模块认领该改动、或模块负责人就是 PR 作者/没有 push 权限时，才回落到领域池。初始的 core 模块映射由最近半年的分目录提交记录起草，请在 review 中校正。
How to verify / 验证方式
packages/core/: theAssign PR ownerrun assigns exactly one core owner; subsequent pushes do not re-assign.GITHUB_REPOSITORY=QwenLM/qwen-code PR_NUMBER=<n> DRY_RUN=true node .github/scripts/assign-pr-owner.mjsprints the chosen owner without writing (verified against real PRs refactor(core): remove unused LruCache utility #9926/fix(review): reply carried findings into their thread, resolve fixed ones #9940/fix(cli): Rank skills last in root slash completion #9943: assign → idempotent skip, path match, and no-match paths).packages/core/下文件的 PR：workflow 会恰好 assign 一位 core owner；再次 push 不会重复分配。文件路径未命中任何领域的 PR 不受影响。本地 dry-run 已用真实 PR 验证三条路径。