fix(triage): resolve stage comment ids by marker at patch time, harden model injection#7703
Conversation
…n model injection Two hardenings from shepherding QwenLM#7693, plus a wording fix: - Re-run comment updates now resolve the target comment id by its stage marker (bot-author-filtered, startswith match) immediately before each PATCH, instead of trusting remembered ids or list positions. Observed on a real re-run: the agent PATCHed the stage=3 comment with stage=1 content mid-run before self-correcting — with four bot comments in the thread, remembered-id bookkeeping is fragile. - The model-name injection step previously no-opped silently if the 'qwen3.7-max' literal ever left the skill (shipping the wrong signature in every comment), and corrupted the skill text on model names carrying sed metacharacters (/ & \). It now fails the job loudly when the target literal is missing and escapes the replacement. Covered by a behavioral test that runs the extracted step script against fixture files. - The finalize status text said 'stage comments above', but the status comment is created first, so the stage comments are below it — now 'in this thread'.
|
Thanks for the PR! Template looks good ✓ Problem: observed bug with concrete evidence — the stage=3 comment on #7693 was PATCHed with stage=1 content during a re-run (comment 5073149611, edit history visible). The injection hardening also addresses real Direction: aligned — internal triage-pipeline robustness, squarely within scope. No public-contract or auth/sandbox concerns. Size: not applicable (no core module paths touched — CI workflow, skill doc, and test only). Approach: the three changes (marker-based id resolution, injection escaping + existence check, wording fix) are focused and each earns its place. The two follow-up commits since the initial review are test-only — one pins the Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的 bug，有具体证据——#7693 的一次重跑中 stage=3 评论被 PATCH 成 stage=1 内容（评论 5073149611，编辑历史可见）。注入加固也针对真实的 方向：对齐——属于内部 triage 流水线加固，完全在范围内。无公共契约或认证/沙箱方面的顾虑。 规模：不适用（未触及核心模块路径——仅 CI workflow、skill 文档与测试）。 方案：三项改动（基于标记的 id 解析、注入转义 + 存在性检查、文案修正）各自有其必要性。初始审查后的两个后续提交均为测试——一个固定 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code ReviewIndependent proposal: given the observed wrong-id PATCH on #7693, I would (1) add a Findings — no critical blockers. The injection escaping ( The The pinning test for the recipe slices the skill's Re-runs section and asserts the load-bearing constraints are present ( The latest commit ( TestingThis is a CI/infra change with no user-visible behavior — real-scenario tmux testing is N/A. CI evidence from the PR's own check runs on
The ubuntu test suite is still running on the new head. macOS/Windows tests and integration tests are skipped (expected for a workflow/skill/test-only change). Deferring approval until CI lands green. Not verified: no live re-run was driven against a production PR (author notes this is out of scope; the behavioral test pins the injection logic and the pinning test guards the recipe constraints). 中文说明代码审查独立方案： 针对观测到的错误 id PATCH，我会 (1) 在 skill 指令中添加一个 发现——无关键阻塞项。 注入转义（
固定测试切取 skill 的 Re-runs 段落，断言关键约束存在（ 最新提交（ 测试这是 CI/基础设施变更，无用户可见行为——真实场景 tmux 测试不适用。 CI 证据来自 PR 自身在 未验证：未针对生产 PR 驱动真实重跑（作者注明这不在范围内；行为测试固定了注入逻辑，固定测试守护了方案约束）。 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 4/5 — clean, well-evidenced fix; the new head adds a correct BSD sed shim on top of the already-reviewed hardening and pinning test. This PR solves a real problem verifiable from the #7693 edit history — a wrong-id PATCH that silently destroyed another stage's comment. The marker-resolution recipe is the right fix: it turns a fragile memory into a deterministic lookup, and the author filter + The two test-only commits since the initial review are both sensible: one pins the skill recipe's load-bearing constraints so they can't silently regress, the other makes the injection test pass on macOS by shimming BSD sed's If I had to maintain this in six months, I'd thank the author: the recipe is self-documenting, the tests extract the real artifacts (not copies), the failure modes are loud rather than silent, and the cross-platform shim is clearly commented. Approval deferred until CI lands green on 中文说明置信度：4/5 —— 干净、有充分证据的修复；新 head 在已审查的加固和固定测试之上添加了一个正确的 BSD sed 兼容层。 这个 PR 解决了一个可以从 #7693 编辑历史中验证的真实问题——一次错误 id 的 PATCH 静默摧毁了另一个阶段的评论。标记解析方案是正确的修复：将脆弱的记忆变为确定性查找，作者过滤 + 初始审查后的两个纯测试提交都很合理：一个固定 skill 方案的关键约束使其不会静默退化，另一个通过为 BSD sed 的 如果六个月后我要维护这段代码，我会感谢作者：方案是自文档化的，测试提取的是真实产物（而非副本），失败模式是响亮的而非静默的，跨平台兼容层有清晰的注释。 批准推迟到 CI 在 — Qwen Code · qwen3.8-max-preview Reviewed at |
The previous commit let prettier rewrite untouched lines (runs-on quoting, comment spacing) while formatting the edited step. Restore those lines to main's form; the diff now carries only the injection hardening and the status wording fix.
|
Fair nit — that was prettier reformatting untouched lines (the 中文合理的 nit——那是 prettier 在格式化被编辑步骤时顺手改写了未触碰的行（`runs-on:` 引号风格与 `uses:` 注释空格）。已在后续提交中还原：workflow 的 diff 现在只含注入加固与状态文案修正（17+/4-）。落地后会为新 head 重新触发 `@qwen-code /triage`。 |
|
@qwen-code /triage |
|
Triage re-run completed without a new review. The stage comments above were updated with the latest result. View workflow run. |
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
中文说明
已审查。 建议见行内评论。
— qwen3.7-max via Qwen Code /review
Guards the startswith match and the bot-author filter in the skill's re-run comment-id recipe against silent regression — a contains match or a dropped author filter re-introduces the wrong-comment-overwrite bug this PR fixes.
|
@qwen-code /triage |
|
Triage re-run completed without a new review. The stage comments above were updated with the latest result. View workflow run. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
中文说明
— qwen3.7-max via Qwen Code /review
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Not reviewed: Agent 0: Issue fidelity & root-cause ownership, Agent 1a: Line-by-line correctness, Agent 2: Security, Agent 3: Code quality, Agent 4: Performance & efficiency, Agent 5: Test coverage, Agent 6a: Undirected audit — attacker mindset, Agent 6b: Undirected audit — 3 AM oncall mindset, Agent 6c: Undirected audit — six-months-later maintainer, Agent 1b: Removed-behavior audit, Agent 1c: Cross-file tracer, Agent 7: Build & test verification — its prompt was built, but no agent on record was launched with it.
— qwen3.7-max via Qwen Code /review
The extracted step script uses GNU 'sed -i' (the step only runs on ubuntu runners), but this suite also runs in the macOS merge-queue job where BSD sed needs an extension argument after -i. Rewrite to sed -i '' on darwin only — on GNU sed a separated '' parses as the sed script, so the unconditional rewrite would break the Linux runs that mirror production.
|
@qwen-code /triage |
|
Triage re-run completed without a new review. The stage comments above were updated with the latest result. View workflow run. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
中文说明
未发现问题。LGTM！✅
— qwen3.7-max via Qwen Code /review
|
Note for the record: this PR served as the first live verification of the #7693 finalize loop, and it caught a real gap. The deferred approval for 中文备忘：本 PR 是 #7693 finalize 闭环的首次实链路验证，并抓到一个真实缺口。`3800f257b` 的延迟审批被静默丢弃——finalize 准时触发，但 `commits/{sha}/pulls` 对本 fork PR 的现任 head 返回空列表，于是记录 `No open PR; nothing to finalize` 后退出（本 PR 的审批由 review-pr 周期补位）。修复：#7706 改为按 `head.sha` 过滤开放 PR 列表解析（故障当时实测对同一 SHA 恰好返回本 PR），关联端点降为次要来源。闭环其余部分在本 PR 上验证干净：各阶段 marker 正确发出、表格区域原地重写、CI 未完时正确不审批、过期触发安全 no-op。 |
|
@doudouOUC The sed portability Critical is fixed in 3800f25 (darwin-only shim — an unconditional |
|
🤝 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 冲突，直到移除标签或达到轮次上限。移除 |
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here. Not reviewed: chunk 1 — no agent reported covering these; nobody read them. Not reviewed: security — Agent 2 failed to launch twice. Not reviewed: six-months-later maintainer — Agent 6c failed to launch twice. Not reviewed: build-and-test — Agent 7 failed to launch twice. Not reviewed: PR #7703 hardens the triage pipeline's comment-id resolut... — pointed at diff lines it never opened: it made tool calls, but none of them read the diff. Not reviewed: every dimension — none of the 12 required agents is on record as launched with a prompt this skill built, so this diff was reviewed, if at all, from prompts the run wrote for itself: no record shows the severity bar, the finding format or this project's own rules reaching an agent.
— qwen3.7-max via Qwen Code /review
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： No changes made this round — all actionable feedback is already addressed in the current HEAD.
The remaining review entries carry no new inline findings: two COMMENTED reviews with empty bodies, one CHANGES_REQUESTED that is a meta-note about the review-agent launch process (no code feedback), and informational issue-level comments about the #7693 finalize-loop verification and #7706. 中文说明本轮未做任何改动——所有可操作的反馈均已在当前 HEAD 中处理完毕。
其余审查条目不含新的行内发现：两条正文为空的 COMMENTED 审查、一条 CHANGES_REQUESTED（内容是关于审查 agent 启动流程的元说明，无代码反馈），以及关于 #7693 finalize 闭环验证与 #7706 的信息性 issue 级评论。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Approve ✅
Three solid hardening fixes, each addressing an observed production failure:
-
Stage comment resolution at PATCH time — the old "remember the id" pattern caused stage=3 to clobber stage=1 on re-runs. The new
startswith+ bot-author filter + re-resolve-before-each-patch is the right defensive pattern. Good call onstartswithovercontains. -
Model name injection — two failure modes closed: silent no-op when the signature literal drifts (now fails loud), and sed metacharacter corruption (
&,/,\now escaped). The cross-platform sed shim in tests (GNU vs BSD) is a nice touch. -
Wording fix — "above" → "in this thread" is more accurate for GitHub's threaded comment UI.
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review triage — no action takenI triaged the review feedback on PR #7703 and found nothing actionable to How the feedback was classified
ConclusionWith the sole suggestion dropped and the remaining content being 中文说明Autofix 评审分诊 —— 未采取任何操作我对 PR #7703 的评审反馈进行了分诊，没有发现任何需要处理的可操作项，因此未做任何代码改动。 反馈分类结果
结论由于唯一的建议已被丢弃，其余内容均为一次失败评审运行产生的基础设施噪声，因此没有需要修复的缺陷、没有需要实施或记录延期的建议、也没有需要上报的待决问题。本分支保持原样，未做改动。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
5 participants
What this PR does
Hardens two fragile spots in the triage pipeline that surfaced while shepherding #7693, and fixes one wording error. First, re-run comment updates in the triage skill now resolve the target comment id by its stage marker at PATCH time — bot-author-filtered,
startswithmatch, re-resolved immediately before each PATCH — instead of trusting ids the agent remembered from earlier in the run. Second, the model-name injection step inqwen-triage.ymlnow fails the job loudly when theqwen3.7-maxsignature literal is missing from the skill (previously a silent no-op that would ship the wrong model name in every comment) and escapes sed replacement metacharacters in the model name (previously/,&, or\in a configured model corrupted the skill text). Third, the terminal status comment said "see the stage comments above", but the status comment is created before the stage comments, so they are below it — now "in this thread".Why it's needed
Observed on a real
/triagere-run of #7693: with four bot comments in the thread (status + stages 1–3), the agent PATCHed the stage=3 comment with stage=1 content mid-run before later self-correcting. Remembered-id bookkeeping is fragile exactly when re-runs matter most — the thread's list order is not the stage order, and one wrong id silently destroys another stage's record. The marker-resolution recipe makes the correct id a lookup, not a memory. The injection hardening closes the two standing failure modes of a rawsed -i "s/…/${OPENAI_MODEL}/g": a renamed literal degrades silently, and an unusual model name writes garbage into the prompt the agent is about to execute.Reviewer Test Plan
How to verify
npx vitest run --config ./scripts/tests/vitest.config.ts qwen-triage-workflow qwen-triage-finalize-workflow— 32 tests pass (1 new). The new test extracts the actual injection step script and runs it against fixture files: a model name carrying/ & \lands verbatim in the skill text, a missing signature literal fails the step with theSignature literalerror (file untouched), and an empty model leaves the file untouched with success..qwen/skills/triage/references/pr-workflow.md: thestage_comment_idrecipe filters on the bot's own login (markers are public text; the PAT may be able to edit others' comments) and usesstartswithso a comment merely quoting a marker can't be targeted.Evidence (Before & After)
Before: on #7693's second
/triagere-run, comment 5073149611 (stage=3) was PATCHed to stage=1 content at 21:06:48 UTC before the agent self-corrected at 21:08:37 — visible in that comment's edit history. Injection step:sed -i "s/qwen3\.7-max/${OPENAI_MODEL}/g"with no existence check and no escaping.After: id resolution is a marker lookup at PATCH time (skill instruction), and the injection behavior is pinned by the new behavioral test. N/A for UI.
Tested on
Environment (optional)
vitest via
scripts/tests/vitest.config.ts; workflow YAML re-parsed with the repo'syamlpackage.Risk & Scope
Linked Issues
Follow-up to #7693 (observations from its shepherding).
中文说明
本 PR 做了什么
加固在护航 #7693 时暴露的两个脆弱点，并修正一处文案。其一，triage skill 的 re-run 评论更新改为在 PATCH 时按 stage 标记现场解析评论 id——过滤 bot 本人作者、
startswith匹配、每次 PATCH 前重新解析——不再信任 agent 在运行早期记住的 id。其二，qwen-triage.yml的模型名注入步骤在 skill 中找不到qwen3.7-max签名字面量时会大声失败（此前是静默 no-op，会让每条评论都带错误模型名），并对模型名中的 sed 替换元字符转义（此前配置的模型名含/、&或\会写坏 skill 文本）。其三，终态 status 评论写"见上方各阶段评论"，但 status 评论创建在最前、各阶段评论在其下方——改为"本线程中"。为什么需要
在 #7693 的一次真实
/triage重跑中观察到：线程里有四条 bot 评论（status + stage 1–3）时，agent 中途把 stage=3 评论 PATCH 成了 stage=1 内容，稍后才自行纠正。记忆式 id 簿记恰恰在最需要 re-run 的场景下最脆弱——线程列表顺序不等于 stage 顺序，一个错误 id 就会静默摧毁另一个 stage 的记录。标记解析法把"正确的 id"变成一次查找而非一段记忆。注入加固则堵住裸sed -i的两个既有失败模式：字面量改名后静默退化；特殊模型名向 agent 即将执行的提示词里写入垃圾。审查者验证方案
npx vitest run --config ./scripts/tests/vitest.config.ts qwen-triage-workflow qwen-triage-finalize-workflow—— 32 个测试通过（新增 1 个）。新测试抽取真实注入步骤脚本并对 fixture 文件执行：含/ & \的模型名原样落入 skill 文本；签名字面量缺失时步骤以Signature literal错误失败（文件不动）；模型名为空时文件不动、步骤成功。.qwen/skills/triage/references/pr-workflow.md的 Re-runs 一节：stage_comment_id过滤 bot 本人登录名（标记是任何人都能粘贴的公开文本，且 PAT 可能有权编辑他人评论），并用startswith防止仅引用标记的评论被误定位。证据（Before & After）
Before：#7693 第二次
/triage重跑中，评论 5073149611（stage=3）于 21:06:48 UTC 被 PATCH 成 stage=1 内容，21:08:37 才被 agent 自行纠正——该评论的编辑历史可见。注入步骤为无存在性检查、无转义的裸sed -i。After：id 解析改为 PATCH 时的标记查找（skill 指令），注入行为由新的行为测试固定。无 UI 变化。
风险与范围
关联 Issue
#7693 的后续（护航过程中的观察）。