Skip to content

feat(core): add a bundled goal-draft skill that writes verifier-judgeable Goals - #10002

Merged
qqqys merged 6 commits into
QwenLM:mainfrom
qqqys:feat/goal-draft-skill
Aug 26, 2026
Merged

feat(core): add a bundled goal-draft skill that writes verifier-judgeable Goals#10002
qqqys merged 6 commits into
QwenLM:mainfrom
qqqys:feat/goal-draft-skill

Conversation

@qqqys

@qqqys qqqys commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Adds a bundled, read-only skill /goal-draft <intent> that turns a fuzzy intention into a /goal objective the Goal verifier can actually judge. The skill first decides whether the request is a Goal at all (one-shot tasks and judgement calls are not), reads the active Goal with get_goal, grounds the draft in the workspace (real test/lint/build commands from package.json, Makefile, CI config; never invented paths), asks at most one round of 1–3 multiple-choice questions and only when the answer changes the check, scope, or budget, then drafts the objective in a fixed contract — Outcome: / Done when: (numbered, binary, at least one tool-observable check with "paste that line") / Must not: / Budget: / On block: / Context: — runs a nine-item self-check, and prints the objective plus a one-line /goal set … the user can run as-is. It never starts the work and never sets the Goal itself (built-in commands are not model-invocable by design).

It also adds a user doc docs/users/features/goals.md (commands, how a Goal is judged, how to write an objective, /goal-draft), a /goal-draft row in the built-in skills table and a Goals link on the /goal row in commands.md, a pointer from headless.md, a design note under docs/design/, and replaces the web-shell Goals dialog placeholder (all tests pass and the lint is clean) with an example that carries a check, a guardrail, and a budget in both locales.

Why it's needed

/goal accepts any non-empty string, but the objective is judged by an independent verifier that only sees transcript evidence (goalJudge.ts, goal-verifier.ts): printed text cannot prove that tests passed or files changed, a claim about a user action needs a real user message, and an objective nobody can evidence keeps the loop running until a limit stops it. Nothing in the product told users this; the only guidance was the web-shell placeholder, which itself has no check attached. The skill encodes the verifier's rules as a writing procedure, following the shape the closest existing tools converge on (OpenAI Codex's curated define-goal skill, the community agent-goal-skill, Claude Code's /goal guidance of "one measurable end state, a stated check, constraints that matter" plus a turn/time clause) while adding the qwen-code-specific parts: evidence must land in the transcript as tool results, "user confirms" is a blocker not a completion condition, and the objective is handed over on one line because parseGoalCommand re-joins whitespace anyway.

Reviewer Test Plan

How to verify

  1. cd packages/core && npx vitest run src/skills/bundled/goal-draft/SKILL.test.ts src/skills/bundled-skills.integration.test.ts — 2 files, 23 tests pass, re-measured on head d15ba05 (the new SKILL.md parses with the real loader; the skill test pins the allowed tools, step order, contract labels, question rules, single-line hand-off, and the explicit "do not run /goal, do not begin the task" stop).
  2. npm run build, then in any small project with a test script run qwen -p "/goal-draft make the auth tests pass and raise coverage". Expected: a fenced objective with the six labels citing the project's real scripts, [ASSUMPTION] notes for what headless could not ask, a /goal set … line, and no file changes in the project (git status clean apart from .qwen/).
  3. Interactive: type /goal-d/goal-draft appears next to /goal in completion; /goal-draft with no argument asks what you want done instead of drafting.
  4. Web shell: open the Goals dialog — the condition textarea placeholder now shows an objective with a check, a guardrail, and a budget (English and Chinese).

Evidence (Before & After)

Before: /goal-draft is an unknown command; the Goals dialog placeholder reads e.g. all tests pass and the lint is clean.

After — built CLI, headless, in a throwaway project whose package.json has test: node --test test/ and coverage: c8 --reporter=text node --test test/:

The run below happened in a throwaway project at /tmp/goal-draft-demo, not in this repository — every path inside it (login.js, login.test.js, the src/auth and test/ directories) belongs to that fixture. Indented rather than fenced for the same reason: they are not claims about this tree.

$ qwen -p "/goal-draft make the auth tests pass and raise coverage"
Outcome: auth tests pass and src/auth has full test coverage.
Done when: 1) `npm test` exits 0 and its TAP summary shows `# fail 0` (paste that line); 2) `npm run coverage` exits 0 and its text table row for src/auth/login.js shows 100 for Stmts, Branch, Funcs, Lines (paste that row).
Must not: weaken or delete existing assertions in test/login.test.js; remove branches or error checks in src/auth/login.js to game coverage; push, publish, delete, or install any package other than the coverage tooling the existing npm script needs; edit files outside src/auth/, test/, package.json, and the lockfile.
Budget: stop as blocked after 20 turns or 15 minutes without measurable progress.
On block: propose blocked with the exact blocker and the decision a human must make; never claim completion without evidence for every Done-when item.
Context: branch master; verified scripts: test=`node --test test/`, coverage=`c8 --reporter=text node --test test/`; node_modules absent and c8 not declared, so adding c8 as devDependency is in scope [ASSUMPTION]; 100% target chosen because the module is a single line [ASSUMPTION].

`/goal set Outcome: auth tests pass and src/auth has full test coverage. Done when: 1) ... Context: ... [ASSUMPTION].`

Assumed (headless, couldn't ask): coverage target = 100% on all four c8 metrics for `src/auth`, and installing `c8` as a devDependency is in scope since the `coverage` script already references it but no dependencies are declared.

The project tree was untouched afterwards. A second run, /goal-draft clean up the auth module, did not accept "clean" as a condition: it rewrote it into an explicit, assumption-tagged observable definition (npm test exits 0, every src/auth export is referenced from test/, git diff --stat confined to src/auth), noted that no lint config exists so lint was dropped, and ended by inviting the user to redefine the target instead of starting work.

Tested on

OS Status
🍏 macOS ⚠️
🪟 Windows ⚠️
🐧 Linux

Environment (optional)

Linux, Node 22, npm ci + npm run build from this branch, headless run against DashScope qwen3.8-max.

Risk & Scope

  • Main risk or tradeoff: the skill is model-invocable, so a user who says "keep going until the tests pass" may get a drafting detour before the work starts; the description is written narrowly ("wants to set or define a goal", "asks whether a goal is good enough") to limit that, and disable-model-invocation: true is the one-line fallback if it fires too eagerly. In headless mode the skill cannot ask, so it picks the recommended default and tags it [ASSUMPTION] rather than refusing.
  • Not validated / out of scope: the P2/P3 follow-ups in the design note — a propose_goal core tool with an approval dialog (mirroring Claude Code's ProposeGoal / modelProposedGoals), keeping newlines in parseGoalCommand, a deterministic lint on /goal set, and a "refine" entry in the web-shell Goals dialog. No runtime code changes here.
  • Breaking changes / migration notes: none. A project or personal skill named goal-draft shadows the bundled one, and skills.disabled can hide it, like every bundled skill.

Linked Issues

None.

中文说明

这个 PR 做了什么

新增一个内置、只读的 skill /goal-draft <意图>,把模糊的意图改写成 Goal verifier 真正能判定的 /goal objective。skill 先判断这个请求该不该设 Goal(一次性任务和需要设计判断的事不该设),用 get_goal 读当前 Goal,在工作区里做 grounding(从 package.jsonMakefile、CI 配置里找真实的 test/lint/build 命令,绝不编造路径),最多问一轮 1–3 个选择题、且只在答案会改变检查方式/范围/预算时才问,然后按固定契约起草——Outcome: / Done when:(编号、二值、至少一条工具可观测且要求"贴出那一行")/ Must not: / Budget: / On block: / Context:——过一遍九条自检,最后打印 objective 和一行可直接回车的 /goal set …。它不会开始干活,也不会自己设 Goal(内置命令按设计不可被模型调用)。

同时新增用户文档 docs/users/features/goals.md(命令、Goal 如何被判定、如何写 objective、/goal-draft),在 commands.md 的内置 skill 表加 /goal-draft 一行并在 /goal 行链到 Goals 文档,headless.md 加一句指引,docs/design/ 下加设计说明,并把 web-shell Goals 对话框的 placeholder(all tests pass and the lint is clean)换成带检查、护栏和预算的中英文示例。

为什么需要

/goal 接受任意非空字符串,但 objective 是由一个只看 transcript 证据的独立 verifier 判定的(goalJudge.tsgoal-verifier.ts):打印出来的文字不能证明测试通过或文件变了,关于用户动作的主张需要真实的用户消息,而一个没人能给出证据的 objective 会让循环一直跑到撞上限。产品里没有任何地方告诉用户这一点;唯一的指引是 web-shell 的 placeholder,而它本身就没有检查。这个 skill 把 verifier 的规则编码成写作流程,沿用现有工具收敛出来的形态(OpenAI Codex 官方的 define-goal skill、社区的 agent-goal-skill、Claude Code /goal 文档的"一个可测量终态、一个明确的检查、必须不变的约束"加轮数/时间条款),并补上 qwen-code 特有的部分:证据必须以 tool result 的形式落在 transcript 里,"用户确认"是阻塞而不是完成条件,objective 单行交接是因为 parseGoalCommand 本来就会把空白重新拼接。

Reviewer 测试计划

如何验证

  1. cd packages/core && npx vitest run src/skills/bundled/goal-draft/SKILL.test.ts src/skills/bundled-skills.integration.test.ts——2 个文件、23 个测试通过,已在 head d15ba05 上重新实测(新 SKILL.md 能被真实 loader 解析;skill 测试锁定了允许的工具、步骤顺序、契约标签、提问规则、单行交接以及明确的"不要运行 /goal、不要开始任务"停止语句)。
  2. npm run build,然后在任意带 test 脚本的小项目里运行 qwen -p "/goal-draft make the auth tests pass and raise coverage"。预期:一个带六个标签、引用项目真实脚本的 objective 代码块,对 headless 无法提问的部分标注 [ASSUMPTION],一行 /goal set …,并且项目里没有文件改动(除 .qwen/git status 干净)。
  3. 交互模式:输入 /goal-d——/goal-draft 出现在 /goal 旁边的补全里;不带参数的 /goal-draft 会先问你想做什么而不是直接起草。
  4. Web shell:打开 Goals 对话框——条件输入框的 placeholder 现在显示带检查、护栏和预算的 objective(中英文)。

证据(Before & After)

Before:/goal-draft 是未知命令;Goals 对话框 placeholder 是 e.g. all tests pass and the lint is clean

After:见上方英文部分的 headless 运行输出——在一个 package.json 里有 test: node --test test/coverage: c8 --reporter=text node --test test/ 的一次性项目里,skill 产出了引用真实脚本的六段式 objective、两处 [ASSUMPTION]/goal set … 行,项目文件未被改动。第二次运行 /goal-draft clean up the auth module 没有把"clean"当成条件接受:它把它改写成带假设标注的可观测定义(npm test 退出码 0、src/auth 的每个导出都被 test/ 引用、git diff --stat 只在 src/auth 内),指出仓库没有 lint 配置因此放弃 lint 检查,并以邀请用户重新定义目标而不是开始干活结束。

测试平台

Linux ✅;macOS、Windows ⚠️ 未测试。

环境(可选)

Linux,Node 22,本分支 npm ci + npm run build,headless 运行使用 DashScope qwen3.8-max

风险与范围

  • 主要风险/取舍:skill 可被模型调用,所以用户说"一直跑到测试通过"时可能先绕一趟起草再开工;description 写得很窄("想设/定义 goal"、"问这个 goal 行不行")来限制这种情况,若触发过于频繁,disable-model-invocation: true 是一行就能改的兜底。headless 模式下 skill 无法提问,会采用推荐默认并标注 [ASSUMPTION],而不是拒绝。
  • 未验证/范围外:设计说明里的 P2/P3 后续——带审批对话框的 propose_goal 核心工具(对齐 Claude Code 的 ProposeGoal / modelProposedGoals)、让 parseGoalCommand 保留换行、/goal set 上的确定性 lint、web-shell Goals 对话框的"改写"入口。本 PR 不改任何运行时代码。
  • 破坏性变更/迁移说明:无。和其他内置 skill 一样,项目或个人的同名 goal-draft skill 会覆盖内置的,skills.disabled 可以隐藏它。

关联 Issue

无。

https://claude.ai/code/session_01FV7i3w7egJ2kMw4AhQC38Z

…able Goals

`/goal` accepts any string, but the Goal verifier only accepts transcript
evidence, so objectives like "all tests pass and the lint is clean" or
"clean up the auth module" never accumulate proof and loop until a limit.
`/goal-draft <intent>` is a read-only bundled skill that checks whether a
Goal is warranted, reads the workspace for the real check commands, asks at
most one round of multiple-choice questions, drafts the objective in a fixed
Outcome / Done when / Must not / Budget / On block / Context contract, runs
a self-check against the verifier rules, and prints a `/goal set …` line
the user can run as-is. It never starts the work and never sets the Goal.

Also adds docs/users/features/goals.md (how a Goal is judged, how to write
one), rows in commands.md, a pointer from headless.md, a design note under
docs/design/, and a web-shell Goals dialog placeholder that shows a check,
a guardrail, and a budget.

Claude-Session: https://claude.ai/code/session_01FV7i3w7egJ2kMw4AhQC38Z
@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finishedview run. See the stage comments in this thread for the result.

Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

@qqqys

qqqys commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /takeover

@qwen-code-dev-bot qwen-code-dev-bot added the autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+) label Aug 25, 2026
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤝 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. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the autofix/takeover label (or comment @qwen-code /takeover stop) to release.

中文说明

🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。本 PR 来自 fork,首轮处理将由下一次定时扫描执行(通常几分钟内)。移除 autofix/takeover 标签(或评论 @qwen-code /takeover stop)即可释放。

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR — this one is unusually well grounded.

Template looks good ✓

Problem: this is a real, verifiable gap, not a theoretical one. I checked the tree: /goal accepts any non-empty string (parseGoalCommand in goalCommand.ts), the completion judge only reads transcript evidence (goal-evidence.ts classifies delivered_output / external_fact / user_input), and the only in-product guidance today is the web-shell placeholder e.g. all tests pass and the lint is clean — which itself has no check attached. So an objective like that placeholder would loop until a budget stops it. The PR's framing matches the code.

Direction: aligned. Goals are an established surface here (goal loop, verifier, web-shell dialog, headless docs), and Claude Code's CHANGELOG shows sustained /goal investment in the space (check-in backoff, evaluator fixes, resume handling), so helping users write judgeable objectives is on-mission. It touches no escalation surface (auth, sandbox, telemetry, public contract).

Size: core path touched (packages/core/src/skills/bundled/goal-draft/) — 108 production lines (the SKILL.md content itself) + 139 test lines; docs ~107 lines; web-shell i18n 6 lines. Well under any maintainer-awareness threshold.

Approach: the scope feels right for what it claims. Everything is additive content — one SKILL.md, one colocated test, docs, and a placeholder swap — with no runtime code changes, and the heavier follow-ups (propose_goal tool, deterministic lint on /goal set) are explicitly deferred to the design doc's later phases. One honest question: could docs + the improved placeholder alone have covered most of this? Probably for users who read docs — the skill earns its keep by engaging at the moment of intent, and it's content-only, so I'm not blocking on the question. No unrelated edits or drive-by changes spotted.

Risk: no high-risk paths matched (no streaming/parser/shell/MCP/sandbox/relaunch surfaces). Note for later stages: this is a fork PR touching packages/core/src/**, so the 100%-confidence bar applies before approval.

Moving on to code review. 🔍

中文说明

感谢贡献——这个 PR 的依据非常扎实。

模板完整 ✓

问题:这是一个真实、可验证的产品缺口,不是理论性问题。我在代码树里核实过:/goal 接受任意非空字符串(goalCommand.tsparseGoalCommand),完成度判定器只读 transcript 证据(goal-evidence.ts 区分 delivered_output / external_fact / user_input),而目前产品内唯一的指引是 web-shell 的占位文案 e.g. all tests pass and the lint is clean——它自己就没有附带检查条件。也就是说,像占位文案这样的 objective 会一直循环到撞上限为止。PR 的描述与代码一致。

方向:对齐。Goals 在这里是成熟的功能面(goal 循环、verifier、web-shell 对话框、headless 文档),Claude Code 的 CHANGELOG 也显示该领域在持续投入(check-in 退避、evaluator 修复、恢复处理),帮助用户写出可判定的 objective 符合产品方向。不涉及需要升级的敏感面(auth、sandbox、telemetry、公共契约)。

规模:触及核心路径(packages/core/src/skills/bundled/goal-draft/)——108 行生产行(即 SKILL.md 内容本身)+ 139 行测试;文档约 107 行;web-shell i18n 6 行。远低于任何需要维护者关注的阈值。

方案:范围与其声明的目标匹配。全部是增量内容——一个 SKILL.md、一个同目录测试、文档、一处占位文案替换——没有运行时代码改动,更重的后续项(propose_goal 工具、/goal set 的确定性 lint)已明确推迟到设计文档的后续阶段。一个诚实的问题:只做文档 + 改进占位文案是否就能覆盖大部分场景?对会读文档的用户来说大概可以——skill 的价值在于在用户表达意图的当下介入,而且它是纯内容,所以这个问题不构成阻塞。未发现无关改动或顺手重构。

风险:未命中任何高风险路径(不涉及 streaming/parser/shell/MCP/sandbox/relaunch 等)。供后续阶段注意:这是触及 packages/core/src/** 的 fork PR,批准前需满足 100% 置信度标准。

进入代码审查 🔍

Qwen Code · qwen3.8-max

Reviewed at afbbc09475332d9fb6e79680077ce1db4a484bec · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Code review

I proposed my baseline before reading the diff: for "users write Goals the verifier can't judge", the minimal fix is (a) document the verifier's evidence rules, (b) a content-only drafting aid, and defer any deterministic lint on /goal set. That is exactly what this PR does, so the approach matches my independent proposal — nothing simpler came to mind that still engages at the moment of intent.

Then I checked every load-bearing claim against the tree, and they hold up:

  • parseGoalCommand splits on /\s+/ and re-joins with single spaces (goalCommand.ts), so the "hand over on one line" rule is real, not cargo cult.
  • The verifier's evidence taxonomy (delivered_output / external_fact / user_input) exists in goal-evidence.ts / goal-protocol.ts, and SKILL.md's "transcript-only evidence" section describes it accurately.
  • All five allowedTools are real tool names (get_goal in tool-names.ts, ask_user_question in askUserQuestion.ts, etc.), and the read-only set genuinely excludes edit/shell/update_goal.
  • Bundled skills are auto-discovered by directory scan — no registry to forget; the existing bundled-skills.integration.test.ts picks the new SKILL.md up automatically and parses it with the real loader.
  • Docs cross-references resolve: skills.md exists, headless.md#run-a-persistent-goal matches the real heading, and the trust claim in goals.md matches goalCommand.ts (only set/edit/resume require a trusted folder).
  • Conventions hold: colocated SKILL.test.ts follows the review skill's precedent, the date-prefixed design doc matches ~120 existing ones, and the 22-test count in the description is internally consistent (7 new + 15 from the integration suite with the new skill added).
  • The test file pins the right things — allowed tools, step order, contract labels, the one-line hand-off, and the "do not run /goal, do not begin the task" stop — and asserts the absence of mutating tools, which is the part that matters for a skill that must stay read-only.

No critical issues. One cosmetic nit, non-blocking: the new web-shell placeholder carries Markdown backticks (`npm test`), but it renders as a plain <textarea placeholder> in GoalsDialog.tsx, so the backticks show up as literal characters. Harmless, and arguably still readable as code-marking; not worth a re-roll.

I also read SKILL.md as prompt content, since a fork-authored prompt is untrusted input: it contains no tool-widening instructions, no exfiltration patterns, no injection-style directives — it spends its effort teaching the model to refuse starting work, which is safety-positive if anything.

Testing

Unattended CI run — I did not build or execute any PR code; the evidence below is the PR's own CI read through the API, fetched once (no polling).

Final CI results for afbbc09 (auto-updated by the triage finalize job after CI completed):

Check Conclusion
Capture web-shell visuals (ubuntu-latest, Node 22.x) ✅ success
Classify PR ✅ success
Dependency CVE audit ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
route ✅ success
Secret scan (TruffleHog) ✅ success
Test (ubuntu-latest, Node 22.x) ✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success

One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。

The macOS/Windows/integration skips are the repo's Classify PR job trimming the matrix, not failures. The Desktop Shell jobs that did run cover the only compiled-code change (the web-shell i18n strings) on both platforms. The Linux unit suite — which includes the new SKILL.test.ts and the integration parse of the new SKILL.md — was still running at fetch time; the table above is updated in place once CI settles.

Everything user-visible that the author reports (the headless /goal-draft runs, the completion-list entry) is the author's claim on Linux only, not independently re-run here — the new skill's parseability and contract are pinned by CI, but the quality of what a model actually drafts from the prompt is not. Sandboxed verification would settle that: @qwen-code /verify as a sponsored run (the author lacks write access, so /tmux is gated out) — a maintainer-triggered run would confirm the skill loads and is invocable end-to-end on the reviewed head; note it carries the pre-execution risk screen and a workspace wipe, and its report should still be read with the same skepticism as the fork's own CI logs, since the code under verification is adversarial input.

中文说明

代码审查

我在看 diff 之前先给出自己的基线方案:针对"用户写出 verifier 无法判定的 Goal",最小修法是 (a) 把 verifier 的证据规则写成文档,(b) 提供一个纯内容的起草辅助,把 /goal set 的确定性 lint 推迟。这个 PR 做的正是这些,方案与我的独立提案一致——我想不到更简单、又能在用户表达意图当下介入的做法。

随后我逐条核实了 PR 的关键论断,全部成立:

  • parseGoalCommand/\s+/ 切分再用单空格拼接(goalCommand.ts),所以"单行交接"规则是真实的,不是凭空设计。
  • verifier 的证据分类(delivered_output / external_fact / user_input)存在于 goal-evidence.ts / goal-protocol.ts,SKILL.md 对"只看 transcript 证据"的描述与代码一致。
  • allowedTools 里五个工具都是真实工具名(get_goaltool-names.tsask_user_questionaskUserQuestion.ts 等),且只读集合确实排除了编辑类工具、shell 和 update_goal
  • 内置 skill 按目录扫描自动发现——没有需要手动注册的清单;现有的 bundled-skills.integration.test.ts 会自动用真实 loader 解析新 SKILL.md。
  • 文档交叉引用都能解析:skills.md 存在,headless.md#run-a-persistent-goal 对应真实标题,goals.md 里的信任工作区说法与 goalCommand.ts 一致(只有 set/edit/resume 要求 trusted folder)。
  • 约定符合:同目录 SKILL.test.ts 沿用了 review skill 的先例,带日期前缀的设计文档与现有约 120 篇一致,描述里 22 个测试的说法自洽(新增 7 个 + 集成测试加入新 skill 后的 15 个)。
  • 测试锁定的内容正确——允许的工具、步骤顺序、契约标签、单行交接、以及"不要运行 /goal、不要开始任务"的停止语句——并且断言了可变工具的缺席,这对一个必须保持只读的 skill 才是关键。

无阻塞问题。一个非阻塞的小瑕疵:web-shell 新占位文案里带了 Markdown 反引号(`npm test`),但它渲染在 GoalsDialog.tsx 的普通 <textarea placeholder> 里,反引号会以原始字符显示。无害,甚至仍可读作代码标记;不值得返工。

我也把 SKILL.md 当作 prompt 内容审了一遍——fork 提供的 prompt 是不可信输入:其中没有扩权指令、没有外泄模式、没有注入式指令;它花大量篇幅教模型拒绝开工,反而是安全正向的。

测试

无人值守 CI 运行——我没有构建或执行任何 PR 代码;以下证据是通过 API 读取的 PR 自身 CI,一次性抓取(不轮询)。

macOS/Windows/集成测试的跳过是仓库 Classify PR 任务裁剪矩阵的结果,不是失败。实际运行的 Desktop Shell 任务在两个平台上覆盖了唯一的编译代码改动(web-shell i18n 字符串)。抓取时 Linux 单元测试套件(包含新的 SKILL.test.ts 和新 SKILL.md 的集成解析)仍在运行;上表会在 CI 结束后原地更新。

作者报告的所有用户可见效果(headless /goal-draft 运行、补全列表条目)都是作者单方面的说法且仅在 Linux 上测试,这里没有独立复跑——新 skill 的可解析性和契约由 CI 锁定,但模型实际起草出的内容质量无法由此证明。沙盒验证可以补齐这一点:@qwen-code /verify赞助运行方式(作者无写权限,/tmux 被门槛挡住)——由维护者触发即可在受审提交上确认 skill 能端到端加载和调用;该运行带有执行前风险筛查和工作区清理,但报告仍应像对待 fork 的 CI 日志一样保持怀疑,因为被验证的代码本身是对抗性输入。

Qwen Code · qwen3.8-max

Reviewed at afbbc09475332d9fb6e79680077ce1db4a484bec · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — clean across every stage; the only open item is CI still in flight, plus one cosmetic nit.

Stepping back: this PR does one thing — teach users to write Goals the verifier can actually judge — and does it with zero runtime code. My independent proposal before reading the diff was exactly this shape (docs + a content-only drafting aid, lint deferred), and the PR matches it; I found no simpler path that still engages at the moment of intent. The grounding is unusually honest: every load-bearing claim about parseGoalCommand, the evidence taxonomy, and the trust boundary checked out against the tree, the design doc cites its analogues and defers the heavier propose_goal follow-up instead of sneaking it in, and the test asserts what matters for a read-only skill — including the absence of mutating tools.

Reservations, named plainly: the new web-shell placeholder carries literal backticks (cosmetic), and the "the model drafts a good objective" part is prompt-dependent and rests on the author's Linux headless runs until someone exercises it — the /verify sponsored-run line in my review comment names that gap. Neither blocks.

One context note for the maintainer: this author has a batch of goal-related PRs open right now. I evaluated this one on its own merits — it stands on them — but it may be worth reviewing it alongside its siblings (e.g. the goal-summary and stall-detection ones) so the Goals surface evolves as one decision rather than several.

CI is still running on this commit (Linux unit suite + web-shell visuals at fetch time), so approval is deferred until CI lands green on afbbc09475332d9fb6e79680077ce1db4a484bec; the finalize workflow will post it.

中文说明

置信度:4/5 —— 各阶段都很干净;唯一未定的是 CI 仍在运行,外加一个小瑕疵。

退一步看:这个 PR 只做一件事——教用户写出 verifier 真正能判定的 Goal——而且零运行时代码。我在读 diff 之前的独立提案正是这个形态(文档 + 纯内容起草辅助,lint 推迟),PR 与之吻合;我想不到更简单、又能在意图表达当下介入的方案。依据异常扎实:关于 parseGoalCommand、证据分类、信任边界的每条关键论断都在代码树里核实过,设计文档引用了同类方案并把更重的 propose_goal 后续项明确推迟而非夹带进来,测试也锁定了对只读 skill 真正重要的东西——包括可变工具的缺席。

坦率地说出两点保留:web-shell 新占位文案带字面反引号(纯外观问题);"模型能起草出好的 objective"这一点依赖 prompt,在有人实际运行之前只以作者的 Linux headless 运行结果为依据——审查评论里的 /verify 赞助运行一行指明了这个缺口。两者都不构成阻塞。

给维护者的一个背景提示:该作者目前有一批 goal 相关的 PR 在开。我按各自的价值独立评估了这个——它站得住——但值得和它的兄弟 PR(比如 goal 摘要和停滞检测)放在一起审,让 Goals 这个功能面作为一个整体决策演进,而不是若干个分散的决定。

CI 在该提交上仍在运行(抓取时是 Linux 单元测试套件 + web-shell visuals),因此批准推迟到 CI 在 afbbc09475332d9fb6e79680077ce1db4a484bec 上全绿后,由 finalize 工作流发布。

Qwen Code · qwen3.8-max

Reviewed at afbbc09475332d9fb6e79680077ce1db4a484bec · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

🖼️ web-shell visual preview

Rendered against a mock daemon (no real backend): the PR base vs this PR head 95e0642. Only screenshots that changed are shown (flows below, if any, are head-only) — refreshes on every push.

Screenshots · before / after

⚠️ No preview: one or more scenarios failed to render on this head — see the workflow run. This is not "no visual change" — a scenario that times out or throws produces no image. Fix the failing scenario (or a genuine regression it caught) and the preview returns on the next push.

Full-resolution recordings (.webm) are attached to the workflow run.

Qwen Code · web-shell visuals

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship — CI landed green after the review. ✅

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially reviewed — gaps disclosed. Suggestions are inline.

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

Test Plan (not a blocker): src/auth/login.jsno such file or directory; test/login.test.jsno such file or directory.

中文说明

仅完成部分审查,审查缺口已披露。 建议见行内评论。

未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。

Test Plan(非阻断):src/auth/login.jsno such file or directory; test/login.test.jsno such file or directory

— qwen3.8-max via Qwen Code /review (v0.22.0)

Comment thread docs/users/features/headless.md Outdated
Comment thread docs/users/features/goals.md Outdated
Comment thread packages/web-shell/client/i18n.tsx
Comment thread packages/core/src/skills/bundled/goal-draft/SKILL.test.ts
@qwen-code-dev-bot

qwen-code-dev-bot commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

AutoFix round 3 finishedview run. See this round's report below.

中文说明

AutoFix 第 3 轮已完成 —— 查看运行。本轮报告见下方。

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下:

Autofix review round for PR #10002

Round commit: fix: resolve goal-draft review findings on docs accuracy and web-shell i18n (#10002). No base merge (--conflict false); the branch had no conflict with origin/main.

Feedback points and dispositions

[rc:3852018989] docs/users/features/headless.md:84 — backslash-escaped quotes in the documented command — RESOLVED.
Reproduced at the reviewed commit: bash -c 'echo -p \"/goal-draft <intent>\"' aborts with intent: No such file or directory (exit 1, qwen never invoked), while the plain-quote form passes one argument and exits 0. Markdown code spans render backslashes literally, so the published docs showed a command that is unusable when copied. Fixed by using plain quotes inside the code span: `qwen -p "/goal-draft <intent>"`, matching every other qwen -p "..." example in the file.

[rc:3852019015] docs/users/features/goals.md:43 — unconditional newline-collapse claim — RESOLVED.
Reproduced with a probe against the real parseGoalCommand at this commit: the bare /goal <objective> form returns the raw input (\n survives), while /goal set and /goal edit split on whitespace and re-join with single spaces (\n collapsed); normalizeObjective in goal-reducer.ts only trims. The page's own command table documents the bare form, so the unconditional claim misdescribed a documented path. Scoped the sentence to the collapsing forms: "/goal set and /goal edit collapse newlines to spaces, so number the items rather than relying on line breaks." SKILL.md's matching sentence was left as-is deliberately: it is scoped to the hand-off ("when you hand it over"), which always uses the one-line /goal set form where collapsing holds.

[rc:3852019024] packages/web-shell/client/i18n.tsx:2095goal-draft missing from SKILL_DESCRIPTION_KEYS — RESOLVED.
Verified: a grep over packages/web-shell at the reviewed commit found zero references to goal-draft, while all nine other bundled skills are listed in SKILL_DESCRIPTION_KEYS with skilldesc.* keys in both EN and ZH tables. Added the 'goal-draft': 'skilldesc.goalDraft' map entry (alphabetical in the bundled block), the EN key (Turn a fuzzy intention into a verifiable /goal objective) and the ZH key (将模糊意图改写为可验证的 /goal 目标), and a test witness in localCommands.test.ts asserting the map entry resolves to the localized EN and ZH strings. Mutation probe: removing the map entry makes the new assertion fail (1 failed / 9 passed); restoring it returns the file to 10/10 green.

[rc:3852019034] packages/core/src/skills/bundled/goal-draft/SKILL.test.ts:40 — test name and docs present allowedTools as the read-only enforcement boundary — RESOLVED.
Verified against skills/types.ts ("This is an additive grant only: it never hides or restricts the tools the model can see") and applySkillAllowedTools in skill-utils.ts (only adds session allow rules). Changes:

  • Renamed the test to what it actually pins: "auto-approves only the non-mutating tools: Goal read, workspace reads, and questions", and rewrote the internal comment to state that allowedTools is an additive auto-approval grant, not a sandbox, and that read-only behavior is enforced by the SKILL.md prose.
  • Softened docs/users/features/goals.md to "The skill is instructed to be read-only, and only its non-mutating tools are auto-approved …".
  • Applied the same honest phrasing to the design doc sentence cited by the finding ("It is read-only: …" → instructed to be read-only; allowedTools is an additive grant, enforcement lives in the skill's prose).
    Runtime enforcement (coordinate's subagent executionAllowedTools path) is left as future work, as the finding itself scoped it.

[rv:5017787224] review body — no code change needed.
The review is a partial-review disclosure; its actionable content is the four inline suggestions above, all addressed. The "Test Plan" note (src/auth/login.js, test/login.test.js — no such file or directory) is explicitly marked "not a blocker" and references files that do not exist in this repository; nothing to fix.

[ic:5408367318] web-shell visual preview: one or more scenarios failed to render on this head — investigated, no code-level link to this PR.
Evidence gathered: (1) the only web-shell change in this PR is the value of the goals.conditionPlaceholder string in the EN and ZH i18n tables, consumed solely by GoalsDialog.tsx; (2) no visual scenario references Goals anywhere (grep over packages/web-shell/client/e2e/visuals finds zero matches), so no scenario renders the changed surface; (3) all 112 CI checks on this head are green (21 SUCCESS / 91 SKIPPED / 0 failures), and the visuals job is non-gating by design (continue-on-error). A local render to reproduce the scenario failure was attempted but is environment-blocked on this runner: the container lacks chromium's system libraries (libglib-2.0.so.0: cannot open shared object file) and there is no root access to install them (EACCES: permission denied, mkdir '/home/github-runner/.cache'; apt-get needs root). The preview refreshes on every push; with no scenario exercising the changed surface, no code change is warranted from this round.

Files changed

  • docs/users/features/headless.md — plain quotes in the /goal-draft example command
  • docs/users/features/goals.md — newline-collapse claim scoped to /goal set//goal edit; read-only phrasing softened
  • docs/design/2026-08-25-goal-draft-skill.md — read-only phrasing softened
  • packages/core/src/skills/bundled/goal-draft/SKILL.test.ts — test renamed to what it pins; comment on where enforcement lives
  • packages/web-shell/client/constants/localCommands.ts'goal-draft': 'skilldesc.goalDraft' added
  • packages/web-shell/client/i18n.tsxskilldesc.goalDraft added to EN and ZH tables
  • packages/web-shell/client/constants/localCommands.test.ts — witness assertions for the new entry

Verification

  • npm run build — passed (exit 0)
  • npm run typecheck — passed (exit 0)
  • npm run lint — passed (exit 0)
  • cd packages/core && npx vitest run src/skills/bundled/goal-draft/SKILL.test.ts — 7 passed
  • cd packages/web-shell && npx vitest run client/constants/localCommands.test.ts — 10 passed
  • cd packages/web-shell && npx vitest run --config vitest.config.ts (full web-shell suite) — 199 files / 4226 tests passed
  • Mutation probe for the new i18n map entry: guard removed → 1 test failed; guard restored → 10/10 green
  • npx prettier --experimental-cli --check on all seven changed files — clean
  • Reproduction probes: bash quote-parsing probe (exit 1 with backslashes, exit 0 with plain quotes) and a tsx probe of parseGoalCommand (BARE keeps \n, SET/EDIT collapse)
  • Settings schema regeneration not needed (no settings source changed); integration tests not applicable (changes are docs, a test rename, and web-shell menu-description data, all covered by the unit runs above)
  • Unavailable environment-specific check: local web-shell Playwright visuals render (missing chromium system libraries, no root) — see ic:5408367318 notes above; the workflow's CI remains the final gate
中文说明

PR #10002 的 Autofix 审查轮次

轮次提交:fix: resolve goal-draft review findings on docs accuracy and web-shell i18n (#10002)。未合并 base(--conflict false);分支与 origin/main 无冲突。

反馈点与处理结果

[rc:3852018989] docs/users/features/headless.md:84 — 文档命令中用反斜杠转义引号 — 已解决。
已在被审查的提交上复现:bash -c 'echo -p \"/goal-draft <intent>\"'intent: No such file or directory 中止(退出码 1,qwen 根本未被调用),而普通引号形式以单个参数执行、退出码 0。Markdown 行内代码块会原样渲染反斜杠,因此发布后的文档展示的命令复制后无法使用。修复方式是在代码块内改用普通引号:`qwen -p "/goal-draft <intent>"`,与本文件中其他所有 qwen -p "..." 示例保持一致。

[rc:3852019015] docs/users/features/goals.md:43 — 无条件的"换行折叠"说法 — 已解决。
通过对本提交上真实的 parseGoalCommand 运行探针复现:裸形式 /goal <objective> 返回原始输入(\n 保留),而 /goal set/goal edit 按空白切分后以单空格重新拼接(\n 被折叠);goal-reducer.ts 中的 normalizeObjective 只做 trim。本页的命令表本身就收录了裸形式,因此这一无条件说法错误描述了其记录的一条路径。已把该句限定为折叠的形式:"/goal set and /goal edit collapse newlines to spaces, so number the items rather than relying on line breaks."。SKILL.md 中的对应句子有意保留原样:它限定在交接场景("when you hand it over"),该场景始终使用单行 /goal set 形式,折叠行为在该路径上成立。

[rc:3852019024] packages/web-shell/client/i18n.tsx:2095goal-draft 未加入 SKILL_DESCRIPTION_KEYS — 已解决。
已核实:在被审查的提交上对整个 packages/web-shell grep 不到任何 goal-draft 引用,而其余九个内置 skill 都列于 SKILL_DESCRIPTION_KEYS,且 EN 与 ZH 表中都有对应的 skilldesc.* 键。已添加 'goal-draft': 'skilldesc.goalDraft' 映射项(按字母序置于内置块中)、EN 键(Turn a fuzzy intention into a verifiable /goal objective)与 ZH 键(将模糊意图改写为可验证的 /goal 目标),并在 localCommands.test.ts 中新增见证断言,验证该映射项可解析为本地化的 EN 与 ZH 文本。变异探针:移除映射项后新断言失败(1 失败 / 9 通过);恢复后该文件回到 10/10 全绿。

[rc:3852019034] packages/core/src/skills/bundled/goal-draft/SKILL.test.ts:40 — 测试名与文档把 allowedTools 呈现为只读强制边界 — 已解决。
已对照 skills/types.ts("This is an additive grant only: it never hides or restricts the tools the model can see")与 skill-utils.ts 中的 applySkillAllowedTools(只添加会话 allow 规则)核实。改动如下:

  • 将测试重命名为它实际锁定的内容:"auto-approves only the non-mutating tools: Goal read, workspace reads, and questions",并重写内部注释,说明 allowedTools 是附加式的自动批准授权而非沙箱,只读行为由 SKILL.md 的文字强制。
  • docs/users/features/goals.md 的措辞软化为 "The skill is instructed to be read-only, and only its non-mutating tools are auto-approved …"。
  • 对本发现引用的设计文档句子应用了同样诚实的表述("It is read-only: …" → 被指示为只读;allowedTools 是附加授权,强制力在 skill 的文字中)。
    运行时强制(coordinate 的 subagent executionAllowedTools 路径)按该发现自身的界定留作后续工作。

[rv:5017787224] 审查正文 — 无需代码改动。
该审查是部分审查的缺口披露;其可操作内容即上述四条行内建议,均已处理。"Test Plan" 备注(src/auth/login.jstest/login.test.js — 文件不存在)明确标注 "not a blocker",且引用的是本仓库中不存在的文件;无可修复项。

[ic:5408367318] web-shell 视觉预览:本 head 上有一个或多个场景渲染失败 — 已调查,未发现与本 PR 的代码级关联。
收集的证据:(1)本 PR 对 web-shell 的唯一改动是 EN 与 ZH i18n 表中 goals.conditionPlaceholder 字符串的值,仅被 GoalsDialog.tsx 消费;(2)没有任何视觉场景引用 Goals(对 packages/web-shell/client/e2e/visuals grep 零匹配),因此没有场景会渲染到改动面;(3)本 head 上全部 112 项 CI 检查均为绿色(21 SUCCESS / 91 SKIPPED / 0 失败),且视觉预览任务按设计不作为门禁(continue-on-error)。曾尝试本地渲染以复现场景失败,但受本运行器环境限制:容器缺少 chromium 的系统库(libglib-2.0.so.0: cannot open shared object file),且无 root 权限安装(EACCES: permission denied, mkdir '/home/github-runner/.cache'apt-get 需要 root)。预览会在每次 push 后刷新;在没有任何场景执行改动面的情况下,本轮不需要代码改动。

变更文件

  • docs/users/features/headless.md/goal-draft 示例命令改用普通引号
  • docs/users/features/goals.md — 换行折叠说法限定到 /goal set//goal edit;只读措辞软化
  • docs/design/2026-08-25-goal-draft-skill.md — 只读措辞软化
  • packages/core/src/skills/bundled/goal-draft/SKILL.test.ts — 测试重命名为其实际锁定的内容;注释说明强制力所在
  • packages/web-shell/client/constants/localCommands.ts — 新增 'goal-draft': 'skilldesc.goalDraft'
  • packages/web-shell/client/i18n.tsx — EN 与 ZH 表新增 skilldesc.goalDraft
  • packages/web-shell/client/constants/localCommands.test.ts — 新映射项的见证断言

验证

  • npm run build — 通过(退出码 0)
  • npm run typecheck — 通过(退出码 0)
  • npm run lint — 通过(退出码 0)
  • cd packages/core && npx vitest run src/skills/bundled/goal-draft/SKILL.test.ts — 7 通过
  • cd packages/web-shell && npx vitest run client/constants/localCommands.test.ts — 10 通过
  • cd packages/web-shell && npx vitest run --config vitest.config.ts(web-shell 全量套件)— 199 个文件 / 4226 个测试通过
  • 新 i18n 映射项的变异探针:移除守卫 → 1 个测试失败;恢复守卫 → 10/10 全绿
  • 对全部七个变更文件执行 npx prettier --experimental-cli --check — 干净
  • 复现探针:bash 引号解析探针(带反斜杠退出码 1,普通引号退出码 0)以及对 parseGoalCommandtsx 探针(BARE 保留 \n,SET/EDIT 折叠)
  • 无需重新生成 settings schema(未改动 settings 源);集成测试不适用(改动为文档、测试重命名与 web-shell 菜单描述数据,均由上述单元测试覆盖)
  • 不可用的环境相关检查:本地 web-shell Playwright 视觉渲染(缺少 chromium 系统库且无 root 权限)— 见上文 ic:5408367318 说明;工作流的 CI 仍是最终验证门禁

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/模型 qwen3.8-max

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not explored to full depth (tool budget reached): "agent 1a": actually executing SKILL.test.ts and localCommands.test.ts via vitest — this worktree has no node_modules (vitest fails to resolve), and a full monorepo ….

Test Plan (not a blocker): src/auth/login.jsno such file or directory; test/login.test.jsno such file or directory; npm run buildexit 1.

Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:

  • packages/core/src/skills/bundled/goal-draft/SKILL.test.ts:140 — [probe] ordering assertion is the only pin on the up-front 'not doing the work' sentence; deleting it passes silently (indexOf returns -1)
  • packages/core/src/skills/bundled/goal-draft/SKILL.test.ts:106 — [probe] contract-label assertions use body-wide toContain; 5 of 6 labels also occur in SKILL.md's example table, so a template-label rename survives
  • packages/core/src/skills/bundled/goal-draft/SKILL.test.ts:128 — [probe] test 'ends with the self-check list and an explicit stop' has no positional pin; relocating the stop line mid-file passes silently
中文说明

未探索到全部深度(达到工具调用预算):"agent 1a"actually executing SKILL.test.ts and localCommands.test.ts via vitest — this worktree has no node_modules (vitest fails to resolve), and a full monorepo …

Test Plan(非阻断):src/auth/login.jsno such file or directory; test/login.test.jsno such file or directory; npm run buildexit 1

收敛姿态下延后(第 2 轮,非阻断)——已记录,本轮不要求修改:共 3 条(原文未翻译,列表见上方英文部分)。

— qwen3.8-max via Qwen Code /review (v0.22.0)

Comment thread packages/core/src/skills/bundled/goal-draft/SKILL.test.ts
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 2/100 轮)。改动内容与我反驳保留之处如下:

Autofix review round — PR #10002

Commit: e512166ae2fix(core): drop ask_user_question from goal-draft allowedTools grant (#10002)

Feedback points and dispositions

[rc:3853636272] [Critical] ask_user_question in the goal-draft allowedTools grant defeats the question dialog — Fixed

The claim was verified in code before touching anything: AskUserQuestionToolInvocation returns 'ask' as its interactive L3 default and does not override requiresUserInteraction() (base default false), while applySkillAllowedTools feeds each allowedTools entry to PermissionManager.addSessionAllowRule. A bare-name session allow rule therefore overrides 'ask' with 'allow' at L4, needsConfirmation returns false, the scheduler executes the tool with no dialog, and execute() returns User declined to answer the questions. as a successful result — a fabricated refusal that poisons verifier evidence. The grant is session-wide, so every later ask_user_question in the session would lose its dialog too.

Changes (the fix the finding requested):

  • packages/core/src/skills/bundled/goal-draft/SKILL.md — removed ask_user_question from allowedTools; the tool's default 'ask' permission already surfaces the dialog, no grant is needed. Headless behavior is unchanged (the tool cannot prompt there regardless).
  • packages/core/src/skills/bundled/goal-draft/SKILL.test.ts — updated the pinned list, added an explicit exclusion pin, and added a behavioral witness test that applies the frontmatter grant to a real PermissionManager and asserts an ask_user_question permission check still resolves to 'ask'.
  • docs/users/features/goals.md and docs/design/2026-08-25-goal-draft-skill.md — corrected the grant descriptions and recorded why ask_user_question is deliberately ungranted (design doc test count updated 7 → 8 to match).

The residual the finding itself flags as a maintainer call — batch and extension-creator carry the same entry, and an AskUserQuestionToolInvocation.requiresUserInteraction() override would be the core root fix — is verified real (entries confirmed at batch/SKILL.md:13 and extension-creator/SKILL.md:12) but lies outside this PR's footprint; it is recorded in deferred-findings.json for the follow-up queue.

[rv:5019683037] CHANGES_REQUESTED review body — Declined with evidence

The review body states it did not explore to full depth (tool budget reached). Its "Test Plan" cites files that do not exist in this repository (src/auth/login.js, test/login.test.js) and an npm run build failure in a worktree that, by the review's own quote, "has no node_modules". There is no checkable defect claim to reproduce, so there is nothing to act on.

Its three deferred probe findings on SKILL.test.ts pin weaknesses were nonetheless each verified as real against the code and fixed as optional hardening (they sit in the file this round edits anyway, and closing them now prevents recurrence):

  • Ordering assertion now guards both indexOf values with >= 0 before comparing — probe: deleting the up-front "NOT doing the work" sentence previously passed silently, now fails.
  • Contract-label assertions are scoped to the ```text template block (where the drafter copies from) and assert the six labels appear in order — probe: renaming Outcome: in the template previously survived because five of six labels also occur in the Weak→strong table, now fails.
  • The stop sentence is pinned as the very last line of the skill body — probe: appending any trailing line previously passed, now fails.

Failed checks — no action

All listed checks are CANCELLED (not failed on code); CI state is owned by the workflow. The local verification commands below were run instead.

Mutation probes (each restored before committing)

  • Probe A (re-add ask_user_question to the grant): both witness tests FAIL — the behavioral test reports finalPermission: 'allow' (the exact defect reproduced live), the list pin fails. Restored → green.
  • Probe B (delete the up-front "NOT doing the work" sentence): the stop/ordering test FAILS. Restored → green.
  • Probe C (rename Outcome: in the template block only): the contract-label test FAILS. Restored → green.
  • Probe D (append a trailing line after the stop sentence): the stop test FAILS. Restored → green.

Verification

  • npx vitest run src/skills/bundled/goal-draft/SKILL.test.ts src/skills/bundled-skills.integration.test.ts (in packages/core) — 23 passed (8 goal-draft tests incl. the new grant witness, 15 bundled-skills tests), re-run green on the committed tree
  • Mutation probes A–D above — each made the intended test fail, then restored to green (recorded in the summary)
  • npx vitest run src/skills src/tools/skill-utils.test.ts (in packages/core) — 410 passed, 4 failed; the 4 failures are all in skill-manager.test.ts (untouched by this PR) and reproduce identically with the HEAD versions of this round's changed files in place, so they are pre-existing in this environment, not caused by this round (evidence kept in the round log)
  • npm run buildpassed
  • npm run typecheckpassed
  • npm run lintpassed
  • npx prettier --check on the four changed files — passed
  • npm run bundle + integration tests — not run: this round's change is skill frontmatter content, its test, and docs; the behavior is exercised by the packages/core unit/integration tests above (bundled-skills.integration.test.ts parses every bundled SKILL.md), not only through the bundled CLI
  • npm run generate:settings-schema — not applicable (no settings source changed)
中文说明

Autofix 评审轮次 — PR #10002

提交:e512166ae2fix(core): drop ask_user_question from goal-draft allowedTools grant (#10002)

反馈点与处置

[rc:3853636272] [Critical] goal-draft 的 allowedTools 授权中包含 ask_user_question,导致提问对话框失效 — 已修复

在动手修改前,该结论已在代码中得到验证:AskUserQuestionToolInvocation 在交互模式下的 L3 默认权限为 'ask',且没有覆写 requiresUserInteraction()(基类默认返回 false);而 applySkillAllowedTools 会把 allowedTools 的每个条目交给 PermissionManager.addSessionAllowRule。因此,裸工具名的会话级 allow 规则会在 L4 层把 'ask' 覆盖为 'allow'needsConfirmation 返回 false,调度器不弹任何对话框直接执行该工具,execute() 随后返回 User declined to answer the questions. 这一"成功"结果——一个伪造的用户拒绝,会污染 verifier 的证据。该授权是会话级的,因此该会话中之后所有的 ask_user_question 都会失去对话框。

修改内容(即该发现所请求的修复):

  • packages/core/src/skills/bundled/goal-draft/SKILL.md — 从 allowedTools 中移除 ask_user_question;该工具默认的 'ask' 权限本来就会弹出对话框,无需授权。headless 行为不受影响(该模式下工具本来就无法提问)。
  • packages/core/src/skills/bundled/goal-draft/SKILL.test.ts — 更新锁定列表,新增显式的排除断言,并新增一个行为见证测试:将 frontmatter 授权应用到真实的 PermissionManager 上,断言 ask_user_question 的权限检查仍解析为 'ask'
  • docs/users/features/goals.mddocs/design/2026-08-25-goal-draft-skill.md — 更正对授权的描述,并记录为何刻意不授权 ask_user_question(设计文档中的测试数由 7 更新为 8 以保持一致)。

该发现自身指出的、属于维护者决策的遗留项——batchextension-creator 带有相同条目,且让 AskUserQuestionToolInvocation 覆写 requiresUserInteraction() 才是核心层根因修复——已核实属实(条目确认位于 batch/SKILL.md:13extension-creator/SKILL.md:12),但超出本 PR 的范围;已记录到 deferred-findings.json 进入后续处理队列。

[rv:5019683037] CHANGES_REQUESTED 评审正文 — 有证据地拒绝

该评审正文自述未探索到全部深度(达到工具预算)。其"Test Plan"引用了本仓库中不存在的文件(src/auth/login.jstest/login.test.js),以及在一个按评审自己引述"没有 node_modules"的 worktree 中 npm run build 失败。不存在可核查的缺陷主张,因此没有可执行的修复。

其中三条被延后的、关于 SKILL.test.ts 锁定弱点的探针发现,仍逐条对照代码核实为真,并作为可选加固一并修复(它们就在本轮编辑的文件中,现在修复可避免以后再次出现):

  • 顺序断言现在先对两个 indexOf 值做 >= 0 守卫再比较 — 探针:删除开头的 "NOT doing the work" 句子原本会静默通过,现在会失败。
  • 契约标签断言被限定在 ```text 模板块内(草稿实际复制的位置),并断言六个标签按序出现 — 探针:仅在模板中重命名 Outcome: 原本能存活(因为六个标签中的五个也出现在 Weak→strong 表格中),现在会失败。
  • 停止句被锁定为 skill 正文的最后一行 — 探针:在停止句后追加任何一行原本会通过,现在会失败。

失败的检查 — 无需处理

所有列出的检查均为 CANCELLED(并非因代码失败);CI 状态由工作流负责。改以下方列出的本地验证命令为准。

变异探针(提交前均已还原)

  • 探针 A(把 ask_user_question 重新加回授权):两个见证测试均失败 — 行为测试报告 finalPermission: 'allow'(缺陷被实时复现),列表锁定断言失败。还原后转绿。
  • 探针 B(删除开头的 "NOT doing the work" 句子):停止/顺序测试失败。还原后转绿。
  • 探针 C(仅在模板块中重命名 Outcome:):契约标签测试失败。还原后转绿。
  • 探针 D(在停止句后追加一行):停止测试失败。还原后转绿。

验证

  • npx vitest run src/skills/bundled/goal-draft/SKILL.test.ts src/skills/bundled-skills.integration.test.ts(在 packages/core 中)— 23 通过(8 个 goal-draft 测试,含新的授权见证测试;15 个 bundled-skills 测试),并已在提交后的代码树上复跑为绿
  • 上述变异探针 A–D — 每个都使目标测试失败,还原后转绿(已记录在本摘要中)
  • npx vitest run src/skills src/tools/skill-utils.test.ts(在 packages/core 中)— 410 通过,4 失败;4 个失败全部位于 skill-manager.test.ts(本 PR 未触碰),且在本轮改动文件换回 HEAD 版本后完全相同地复现,故为本环境既有问题,并非本轮引入(证据保留在轮次日志中)
  • npm run build通过
  • npm run typecheck通过
  • npm run lint通过
  • 对四个改动文件执行 npx prettier --check通过
  • npm run bundle + 集成测试 — 未运行:本轮改动仅为 skill frontmatter 内容、其测试与文档;相关行为由上述 packages/core 单元/集成测试覆盖(bundled-skills.integration.test.ts 会解析每个 bundled SKILL.md),并非只能通过打包后的 CLI 验证
  • npm run generate:settings-schema — 不适用(未改动任何 settings 源)

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/模型 qwen3.8-max

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Downgraded from Approve to Comment: CI failing: Test (windows-latest, Node 22.x). Partially reviewed — gaps disclosed.

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

Test Plan (not a blocker): src/auth/login.jsno such file or directory; test/login.test.jsno such file or directory; npm testexit 1; 22 tests pass — this review observed 21490, 1702, 23961, 1659, 601, 4226, 627 passed.

中文说明

⚠️ 已从批准降级为评论:CI failing: Test (windows-latest, Node 22.x)。 仅完成部分审查,审查缺口已披露。

未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。

Test Plan(非阻断):src/auth/login.jsno such file or directory; test/login.test.jsno such file or directory; npm testexit 1; 22 tests pass — this review observed 21490, 1702, 23961, 1659, 601, 4226, 627 passed

— qwen3.8-max via Qwen Code /review (v0.22.0)

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下:

🤖 Addressed the latest review feedback (round 3/100). No code changes this round — the only feedback item was traced outside this PR's footprint and deferred to the follow-up queue. · 已处理最新评审反馈(第 3/100 轮)。本轮无代码改动 —— 唯一的反馈项被追溯到本 PR 范围之外,已移交后续处理队列。

Autofix review round — PR #10002

Commit: none (no-action round). No base merge (--conflict false).

Feedback points and dispositions

[rv:5021258031] "Downgraded from Approve to Comment: CI failing: Test (windows-latest, Node 22.x)" — root-caused outside this PR; failing tests deferred to the follow-up queue

The review carries zero findings (its ledger shows findings: []); its sole actionable content is the red required check Test (windows-latest, Node 22.x) from this PR's merge-queue run (run 32861758802, check run 97849021613, 14:53–15:49 UTC). This agent has no GitHub credentials, so the failure was identified through the check run's public annotations (unauthenticated read-only API) instead of guessing. Exactly four tests failed, all in packages this PR never touches:

  1. packages/vscode-ide-companion/src/webview/handlers/SessionMessageHandler.test.ts:946 and :1243 — the tests pin literal drive-less POSIX export links (file:///workspace/export.html), but on Windows the fixture path resolves drive-relative and the handler renders file:///D:/workspace/export.html (the runner's current drive). Introduced by fix(vscode): preserve Windows file links in session exports #8953 / feat(vscode-ide-companion): adopt WebShell transcript as the default timeline #9719 on 2026-08-24.
  2. packages/channels/dws/src/dws-event-stream.test.ts:78 — "clears stale process errors after a healthy event": win32 child-process exit semantics deliver a structured DwsEventProcessError with retryable: false instead of the expected synthetic stop marker DWS event consumer stopped (0). with retryable: undefined. The DWS channel package (feat(channels): add DingTalk Workspace channel #9394) landed 2026-08-25T06:40Z and had never run on Windows.
  3. packages/node-repl/src/node-repl.semantics.test.ts:374 — "resolves bare packages from a symlinked cwd node_modules": the finally-block fs.rmSync hits EBUSY because the REPL kernel child still holds the qwen-node-repl-cwd-* temp dir (mandatory Windows file locks) — the same failure class fix: repair the Windows and macOS test lane failures #9728 fixed for the drive bound-address test. Introduced by refactor(node-repl)!: deliver the persistent Node REPL as a standalone MCP server #9499 on 2026-08-23.

Why this is a lane problem, not a PR problem:

  • The same merged tree passed ubuntu in the very same CI run (Test (ubuntu-latest, Node 22.x) SUCCESS), so the red/green delta is purely platform-specific.
  • This PR's diff is content-only (the goal-draft SKILL.md and its colocated test, docs pages, and web-shell i18n keys) and has zero file overlap with the three failing packages — verified by diffing origin/main...HEAD against the failing paths. Line endings cannot explain it either: .gitattributes pins * text=auto eol=lf and the lane additionally disables core.autocrlf before checkout.
  • The Windows lane ran today for the first time since it went dark on 2026-07-02: fix(ci): give the macOS and Windows lanes a trigger again #9370 revived the trigger and fix: repair the Windows and macOS test lane failures #9728 repaired the 72 dark-window failures it found. All three failing suites above were introduced (or last changed) during that dark window, after or beside fix: repair the Windows and macOS test lane failures #9728's census, so the revived lane is now surfacing them.

All three failures are verified real (exact assertion diffs from the lane) but their fixes live in vscode-ide-companion, channels/dws, and node-repl — outside this PR's footprint — so they are recorded in deferred-findings.json for the follow-up queue rather than declined. Note for the maintainer: re-queueing this PR will fail the Windows lane again until those suites are repaired in their own packages.

Verification

  • npx vitest run src/skills/bundled/goal-draft/SKILL.test.ts (packages/core) — 8 passed
  • npx vitest run client/constants/localCommands.test.ts (packages/web-shell) — 10 passed
  • npm run typecheck — passed
  • npm run lint — passed
  • Evidence gathering (read-only): check-run annotations via the unauthenticated public API for job 97849021613; git log dating of the three failing test files; git diff --name-only origin/main...HEAD footprint comparison (no overlap); .gitattributes + ci.yml line-ending audit.
中文说明

🤖 已处理最新评审反馈(第 3/100 轮)。本轮无代码改动 —— 唯一的反馈项被追溯到本 PR 范围之外,已移交后续处理队列。

Autofix 评审轮次 — PR #10002

提交:无(无改动轮次)。未合并 base(--conflict false)。

反馈项与处置

[rv:5021258031] "已从批准降级为评论:CI failing: Test (windows-latest, Node 22.x)" —— 根因定位在本 PR 之外;失败测试已移交后续处理队列

该评审本身没有任何 finding(其台账显示 findings: []),唯一可操作的内容是本 PR 合并队列运行(run 32861758802,check run 97849021613,UTC 14:53–15:49)中变红的必需检查 Test (windows-latest, Node 22.x)。本代理没有 GitHub 凭据,因此通过该 check run 的公开 annotations(未认证的只读 API)定位失败,而不是凭空猜测。恰好有 4 个测试失败,且全部位于本 PR 从未触及的包中:

  1. packages/vscode-ide-companion/src/webview/handlers/SessionMessageHandler.test.ts:946:1243 —— 测试将无盘符的 POSIX 导出链接字面量(file:///workspace/export.html)写死为断言,但在 Windows 上该 fixture 路径按当前盘符相对解析,处理器实际渲染出 file:///D:/workspace/export.html(运行器的当前盘符)。由 fix(vscode): preserve Windows file links in session exports #8953 / feat(vscode-ide-companion): adopt WebShell transcript as the default timeline #9719 于 2026-08-24 引入。
  2. packages/channels/dws/src/dws-event-stream.test.ts:78 —— "clears stale process errors after a healthy event":win32 的子进程退出语义送达的是结构化错误 DwsEventProcessErrorretryable: false),而非预期的合成停止标记 DWS event consumer stopped (0).retryable: undefined)。DWS channel 包(feat(channels): add DingTalk Workspace channel #9394)于 2026-08-25T06:40Z 合入,此前从未在 Windows 上运行过。
  3. packages/node-repl/src/node-repl.semantics.test.ts:374 —— "resolves bare packages from a symlinked cwd node_modules":finally 块中的 fs.rmSyncEBUSY,因为 REPL kernel 子进程仍占用 qwen-node-repl-cwd-* 临时目录(Windows 强制文件锁)—— 与 fix: repair the Windows and macOS test lane failures #9728 为 drive 绑定地址测试修复的失败类别相同。由 refactor(node-repl)!: deliver the persistent Node REPL as a standalone MCP server #9499 于 2026-08-23 引入。

为什么这是平台通道(lane)的问题,而不是本 PR 的问题:

  • 同一份合并后的代码树在同一次 CI 运行中通过了 ubuntu(Test (ubuntu-latest, Node 22.x) SUCCESS),因此红/绿差异完全是平台相关的。
  • 本 PR 的 diff 是纯内容改动(goal-draft 的 SKILL.md 及其同目录测试、文档页面、web-shell i18n 键),与三个失败包零文件重叠 —— 已通过将 origin/main...HEAD 的 diff 与失败路径比对核实。换行符也无法解释该失败:.gitattributes 固定 * text=auto eol=lf,且该通道在 checkout 前额外关闭了 core.autocrlf
  • Windows 通道自 2026-07-02 变暗(dark)以来今天是首次运行:fix(ci): give the macOS and Windows lanes a trigger again #9370 恢复了触发器,fix: repair the Windows and macOS test lane failures #9728 修复了由此暴露的 72 个暗窗期失败。上述三个失败套件均在该暗窗期内引入(或最后修改),在 fix: repair the Windows and macOS test lane failures #9728 的清点之外或与其并行,因此恢复后的通道现在正把它们暴露出来。

三个失败均已核实为真实存在(来自该通道的精确断言 diff),但修复分别位于 vscode-ide-companionchannels/dwsnode-repl —— 在本 PR 的范围之外 —— 因此记入 deferred-findings.json 移交后续处理队列,而非拒绝(decline)。给维护者的提示:在这些套件于各自包内修复之前,重新将本 PR 排入合并队列仍会在 Windows 通道上失败。

验证

  • npx vitest run src/skills/bundled/goal-draft/SKILL.test.ts(packages/core)—— 8 通过
  • npx vitest run client/constants/localCommands.test.ts(packages/web-shell)—— 10 通过
  • npm run typecheck —— 通过
  • npm run lint —— 通过
  • 证据收集(只读):通过未认证公开 API 获取 job 97849021613 的 check-run annotations;用 git log 对三个失败测试文件断代;用 git diff --name-only origin/main...HEAD 做足迹比对(无重叠);审计 .gitattributesci.yml 的换行符处理。

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially reviewed — gaps disclosed.

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

Test Plan (not a blocker): src/auth/login.jsno such file or directory; test/login.test.jsno such file or directory.

中文说明

仅完成部分审查,审查缺口已披露。

未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。

Test Plan(非阻断):src/auth/login.jsno such file or directory; test/login.test.jsno such file or directory

— qwen3.8-max via Qwen Code /review (v0.22.0)

@qqqys

qqqys commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

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: 61 passed · 0 failed · 61 total

Flakiness gate: ✅ 2 changed test file(s) x 5 identical rounds, no divergence

中文 — 判定:✅ 通过 · 可合入(agent 判定)

沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查

脚本断言:61 通过 · 0 失败 · 61 总计

抖动门:✅ 2 changed test file(s) x 5 identical rounds, no divergence

Verification report

PR #10002 verification — feat(core): add a bundled goal-draft skill that writes verifier-judgeable Goals

Verdict: merge-ready — 61/61 scripted assertions passed (0 unexpected failures), verified head d15ba05b35a7da8e31bb035e794ea8a3fd5fda5c (merge-ref HEAD^2), base 0756be0ce72e54935fb2e8a00a66a25dd5564694 (HEAD^1). One non-blocking Suggestion (an unpinned i18n string, pre-existing gap class) and two description corrections below.

中文摘要
  • 结论merge-ready。61/61 脚本断言通过,无意外失败。
  • A/B 结论(base 0756be0 vs head d15ba05,真实 SkillManager/SkillTool 编译产物 + web-shell 真实源码):base 上 /goal-draft 在 bundled 发现、模型可见的 <available_skills> 块、SkillTool.validateToolParams 三处均不存在(报 "not found");head 上完整接入——12 个 bundled skill、allowedTools 恰为 4 个只读工具、模型可调用、web-shell 菜单 EN/ZH 本地化与 Goals 对话框 placeholder(检查+护栏+预算)全部就位。30/30,见证图 01-ab-skill-discovery-and-webshell.png
  • 测试钉定:12 行变异矩阵全部符合预期——5 处 SKILL.md 变异被 SKILL.test.ts 捕获、frontmatter 破坏被集成测试捕获、2 处 web-shell hunk 变异被 localCommands.test.ts 捕获;2 处"存活"均为预期内的覆盖边界(表格散文、placeholder 无测试钉定,后者为既有缺口,见 Findings)。
  • Findings:1 条 Suggestion(goals.conditionPlaceholder 两个 locale 均无测试钉定,非阻塞);2 条描述纠正(测试数 22→23;"无运行时代码改动"与 web-shell 客户端改动不符)。
  • 未覆盖:真实模型 headless 运行(沙箱无凭据)、TUI 按键补全、浏览器渲染 Goals 对话框、逐 commit 归因(shallow checkout)、skills 域之外的 core 全量测试。

Central claim and A/B load-bearing proof

Central claim: goal-draft is correctly bundled — discovered by the real runtime loader, invocable by user (/goal-draft) and model (<available_skills> + SkillTool), with the permission grant limited to the four non-mutating tools. Secondary: the web-shell surfaces it (localized slash-menu description, Goals-dialog placeholder with check/guardrail/budget in both locales); tertiary: the new tests pin the contract they advertise.

The A/B drove the compiled SkillManager/SkillTool of each tree (safe mode → bundled level only) and the real web-shell client sources via tsx; each arm's harness asserts the module realpath lives inside its own tree (no workspace-symlink bleed). Witness: 01-ab-skill-discovery-and-webshell.png (30/30).

# Cell / oracle base 0756be0 (control) head d15ba05
1 Bundled discovery (SkillManager.refreshCache + listSkills) 11 skills, no goal-draft, 0 parse errors 12 skills incl. goal-draft; allowedTools = [get_goal, read_file, glob, grep_search]; model-invocable; user-invocable; hint parsed
2 Model-visible <available_skills> block (collectAvailableSkillEntries + renderAvailableSkillsBlock) no goal-draft entry entry present with XML-escaped description, (bundled)
3 SkillTool.validateToolParams({skill:'goal-draft'}) Skill "goal-draft" not found. Available skills: batch, … null (accepted)
4 loadSkill/loadSkillForRuntime null config returned; body's last line is the stop instruction
5 web-shell skillDescriptionKey + getTranslator EN/ZH undefined (menu falls back to authored English in zh UI) skilldesc.goalDraft, EN + ZH descriptions resolve
6 goals.conditionPlaceholder EN/ZH (GoalsDialog textarea) e.g. all tests pass and the lint is clean / 旧中文 new strings with check + guardrail + budget, both locales
Positive controls (both arms identical) bundled set = head minus goal-draft; /review localization identical; /goal builtin present same

The base arm is the load-bearing proof: the identical harness fails to find the skill at every layer on base and succeeds at every layer on head, with no other delta in the bundled set.

Vacuity / mutation matrix (12/12 as expected)

Witnesses: 02-core-skill-mutation-matrix.png, 03-webshell-hunk-mutation-matrix.png. Controls green on both suites before mutating.

Mutation Suite Outcome
M0 unmutated control SKILL.test.ts 8/8 green
M1 grant ask_user_question SKILL.test.ts caught — both grant tests fail (list assertion + permission stays ask)
M2 disable-model-invocation: true SKILL.test.ts caught — invocability test
M3 swap Step 2/3 headings SKILL.test.ts caught — step-order test
M4 delete final stop line SKILL.test.ts caught — expected/received mismatch on the last line
M5 delete Budget: from the ```text template SKILL.test.ts caught — contract-label test
M6 reword unpinned table prose SKILL.test.ts survives (expected — prose boundary, not a defect)
M7 delete description: frontmatter bundled-skills.integration.test.ts caught — goal-draft/SKILL.md parses with required fields fails
W0 unmutated control localCommands.test.ts 10/10 green
W1 drop 'goal-draft' menu key localCommands.test.ts caught — expected undefined to be 'skilldesc.goalDraft'
W2 drop EN skilldesc.goalDraft localCommands.test.ts caught — key falls through to itself
W3 revert EN placeholder localCommands.test.ts survives — no test pins the placeholder (see Findings)

All reverted files verified clean afterwards (git status --porcelain empty). Note the PR's own tests parse via the extension parser (parseSkillContent), while my A/B exercised the runtime bundled parser (SkillManager.parseSkillContent) — both accept the shipped frontmatter identically.

Targeted gates (9/9) and pre-existing failures

Witness: 04-targeted-gates.png.

Gate Result
Reviewer Test Plan step 1 (the two named core files) 23/23 pass
core src/skills/ domain 401 passed, 4 failed — pre-existing: the same 4 test names fail byte-identically on the base arm (A/A); they hardcode TEST_HOME=/home/user while this container's HOME differs; PR touches neither file
web-shell full suite 198 files / 4235 tests, all pass
core typecheck / web-shell typecheck exit 0 / exit 0
eslint on the four changed code files clean; liveness probe (planted unused var) reported as expected
dist/bundled/goal-draft/SKILL.md byte-identical to src; SKILL.test.ts correctly not shipped; built web-shell SPA assets carry the new EN description + placeholder

Reviewer Test Plan walkthrough

  1. Step 1 (two core test files): performed, 23/23.
  2. Step 2 (headless qwen -p "/goal-draft …" with a real model): not performable here — the sandbox has no model credentials and no network by contract; the oracle would be model-generated prose. The integration mechanics it exercises (discovery → validation → body injection) are covered by A/B cells 1–4 instead.
  3. Step 3 (interactive /goal-d completion): data source verified — the CLI's BundledSkillLoader builds slash commands dynamically from skillManager.listSkills({level:'bundled'}) (no static list to forget), and cell 1 proves that list gains goal-draft. TUI keystrokes themselves not driven.
  4. Step 4 (web-shell Goals dialog placeholder): verified at data level (cells 5–6), GoalsDialog.tsx consumes the key on the condition textarea, and the built SPA assets contain both new strings. Browser not rendered.

Corrections to the PR description

  • "22 tests pass" — the two named files run 23 tests at this head (15 integration + 8): the integration it.each gained a 12th bundled directory. Cosmetic; the count in the body predates the final skill set.
  • "No runtime code changes here" — the web-shell client runtime is changed (localCommands.ts, i18n.tsx); what is untouched is the Goal/verifier runtime. The intent reads correctly, but the sentence as written is inaccurate.

Findings

S1 (Suggestion, non-blocking) — the Goals-dialog placeholder is unpinned. W3 survives: no test asserts goals.conditionPlaceholder in either locale, so the PR's most user-visible web-shell string can silently regress (the old placeholder was equally unpinned, so this is an inherited gap, not a regression). Suggested follow-up: extend localCommands.test.ts (or a small i18n test) to assert the placeholder carries a check, a guardrail, and a budget in both locales. Not a merge condition — the string itself is correct in source and in the built SPA.

Completeness notes (explicitly not merge conditions): M6 documents that the Weak→strong table prose is intentionally unpinned; computer-use/coordinate are absent from SKILL_DESCRIPTION_KEYS — pre-existing and by design (unlisted skills fall back to their authored description).

Not covered

  • Live model runs of /goal-draft (headless or interactive) — no credentials/network in this sandbox; the [ASSUMPTION] headless behavior and prose quality are model behavior, unverifiable offline.
  • TUI completion keystrokes and browser rendering of the Goals dialog (data-level + built-asset checks instead).
  • Per-commit attribution — checkout is depth 2 (merge + 2 parents reachable; metadata lists 5 commits), so only the aggregate HEAD^1..HEAD diff was verified.
  • Core suites outside src/skills/; repo-wide lint/format; macOS/Windows.
  • The 4 skill-manager.test.ts failures are environmental on this container (identical on base); they would need a normal runner to confirm green there.

Methodology

Environment: CI merge-ref checkout (HEAD = merge, HEAD^1 = base tip, HEAD^2 = PR head), npm ci + npm run build pre-run at head. Base arm: scratch worktree at HEAD^1 with packages/core rebuilt in-place (root node_modules reused; the PR leaves package.json/lockfile untouched, and two symlinks supplied the workspace's unhoisted nested deps — packages/core/node_modules and the worktree root — after realpath controls confirmed every loaded module lived in its own tree). Harnesses (0110 in this directory) drove the compiled SkillManager/SkillTool and the real web-shell client sources with no mocks; raw logs (ab-run-1.log, mutation-*.log, webmut-*.log, web-shell-full-suite.log, base-core-build.log, docs-check.log, tally.log, matrix-assert.log) sit beside them. Counts: 30 (A/B) + 12 (matrix) + 10 (docs) + 9 (gates) = 61.

Flakiness gate log

rounds=5 files=2 skipped=0
file packages/core/src/skills/bundled/goal-draft/SKILL.test.ts: (cd packages/core) npx --no-install vitest run ./src/skills/bundled/goal-draft/SKILL.test.ts
file packages/web-shell/client/constants/localCommands.test.ts: (cd packages/web-shell) npx --no-install vitest run ./client/constants/localCommands.test.ts


per-file results (P=pass F=fail I=infra-exit, one letter per run):
  packages/core/src/skills/bundled/goal-draft/SKILL.test.ts: PPPPP
  packages/web-shell/client/constants/localCommands.test.ts: 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 · packages/core/src/skills/bundled/goal-draft/SKILL.test.ts: P (exit 0)
round 1 · packages/web-shell/client/constants/localCommands.test.ts: P (exit 0)
round 2 · packages/core/src/skills/bundled/goal-draft/SKILL.test.ts: P (exit 0)
round 2 · packages/web-shell/client/constants/localCommands.test.ts: P (exit 0)
round 3 · packages/core/src/skills/bundled/goal-draft/SKILL.test.ts: P (exit 0)
round 3 · packages/web-shell/client/constants/localCommands.test.ts: P (exit 0)
round 4 · packages/core/src/skills/bundled/goal-draft/SKILL.test.ts: P (exit 0)
round 4 · packages/web-shell/client/constants/localCommands.test.ts: P (exit 0)
round 5 · packages/core/src/skills/bundled/goal-draft/SKILL.test.ts: P (exit 0)
round 5 · packages/web-shell/client/constants/localCommands.test.ts: P (exit 0)

Evidence images

01-ab-skill-discovery-and-webshell

02-core-skill-mutation-matrix

03-webshell-hunk-mutation-matrix

04-targeted-gates

Harness scripts and raw logs are in the workflow run artifacts (7-day retention).

Qwen Code · sandboxed verification

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Triage re-run completed without a new review.

⚠️ The bot's only review on d15ba05b35a7da8e31bb035e794ea8a3fd5fda5c is a COMMENTED one, which carries no vote — so it has no verdict of its own on this commit, and main needs two approving reviews: an approval left by another account is a separate vote and does not count as the bot's own. Two different things look like this, and the stage-3 comment above says which: the triage skill deferring on purpose at 3/5 — a fork refactor hitting the approval guardrail, or a core change escalated for maintainer awareness, both normal outcomes — or an earlier approval that a push dismissed, leaving only the comment behind, which needs a fresh review.

⚠️ 机器人在 d15ba05b35a7da8e31bb035e794ea8a3fd5fda5c 上唯一的评审是 COMMENTED不带票 —— 因此它在该 commit 上没有自己的裁决,而 main 需要两个批准(其他账号的批准是另一张票)。有两种情况长这样,上方的 stage-3 评论会说明是哪一种:triage skill 在 3/5 时有意 defer(fork refactor 命中审批护栏,或核心改动被升级交由维护者把关,两者都是正常结果);或者更早的批准被一次推送作废、只剩下这条评论,此时需要重新评审。

The stage comments above were updated with the latest result. View workflow run.

上方各阶段评论已更新为最新结果。查看工作流运行

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下:

Autofix review round — PR #10002

No code changes this round — the new feedback carries zero findings, and both disclosed review gaps were closed with evidence instead: the integration suite the automated reviewer could not see was run locally (192 tests passed; the single failure proven to be a runner filesystem condition, not a code defect), and the test-plan paths were shown to belong to a demo project quoted in the PR body, not to this repository.

Commit: none (no-action round). No base merge (--conflict false). Working tree clean at d15ba05b35.

Feedback points and dispositions

[rv:5027700986] COMMENTED, zero findings — both disclosed gaps closed with evidence

The round-4 review of d15ba05b35 posted no findings (ledger: findings: [], posted: 0) and carries no vote. Its actionable content is the two gap disclosures:

Gap 1 — "build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally."

  • Why it skips in CI: the lane is merge-queue-only by design — integration_cli in ci.yml is conditioned on github.event_name == 'merge_group', so it never runs on PR pushes; it runs before the PR lands on main, where the base-repo model secrets are available. The skip is structural, not a regression introduced by this PR.
  • Gap closed locally: this runner has the model credentials, so the exact suite the lane runs was executed here — npm run build && npm run bundle, then test:integration:cli:sandbox:none (QWEN_SANDBOX=false): 32 of 38 files passed; 192 tests passed, 18 self-skipped by their own opt-in environment gates (load test, daemon benchmarks, gen-ai telemetry), 1 failed.
  • The single failure is environmental, proven by probe rather than asserted: cli/qwen-config-dir.test.ts 1d deliberately unsets QWEN_HOME, forcing the CLI to create the default config dir under $HOME/.qwen. On this self-hosted runner /home/github-runner is owned root:root with mode drwxr-xr-x, so the test process (user node) gets EACCES: permission denied, mkdir '/home/github-runner/.qwen' — reproduced by re-running that one file as-is (1 failed / 6 passed, same EACCES). Re-running the same file with a writable HOME passes 7/7: the code path is sound, only the runner's home ownership blocks it. The merge-queue job contains an explicit "Restore workspace ownership" step for exactly this class of leftover state on reused self-hosted runners. This PR's diff is content-only (SKILL.md and its test, docs, web-shell i18n) and does not touch the failing bootstrap path (initializeLlmOutputLanguagewriteOutputLanguageFile), which is byte-identical to origin/main at this merged head — the same result would occur on the base branch in this environment.

Gap 2 — "Test Plan (not a blocker): src/auth/login.js, test/login.test.js — no such file or directory."

  • Declined with evidence: these paths are not and never have been part of this repository (git log --all --follow on both is empty; this is a TypeScript monorepo where all code lives under packages/*). They appear in the PR body only inside the Evidence section's quoted /goal-draft sample output — the throwaway demo project the sample objective is about (whose package.json defines the test and coverage scripts the objective references). The reviewer's probe misattributes demo-project paths to this repo, and the review itself marks the item not a blocker. There is nothing to fix in this tree, and no change in this tree could fix it.

[ic:5422095679] "Sandboxed verification is running" — informational only, no action

A status line posted by the triage workflow's verify stage in response to the maintainer's /triage re-run; it carries no finding and asks for nothing. The re-run has since completed without a new review ("Triage re-run completed without a new review"). No thread to answer, nothing to address.

Verification

  • npx vitest run src/skills/bundled/goal-draft/SKILL.test.ts src/skills/bundled-skills.integration.test.ts (packages/core) — 23 passed (2 files)
  • npx vitest run client/constants/localCommands.test.ts (packages/web-shell) — 10 passed
  • npm run build — passed
  • npm run bundle — passed (dist/bundled/goal-draft/SKILL.md present in the bundle; node dist/cli.js --version → 0.22.0)
  • npm run test:integration:cli:sandbox:none (the exact merge-queue lane suite) — 32 of 38 files passed; 192 passed / 18 opt-in-gated skips / 1 environmental failure (diagnosed above), 480s
  • cli/qwen-config-dir.test.ts re-run as-is — failure reproduced: 1 failed / 6 passed (EACCES on root-owned /home/github-runner)
  • cli/qwen-config-dir.test.ts re-run with writable HOME=/tmp/qwen-int-home-probe — 7 passed
  • npm run typecheck — passed
  • npm run lint — passed
  • Evidence gathering (read-only): ci.yml integration_cli gate (merge_group only); git log --all --follow on the two demo paths (empty); ls -ld /home/github-runner (root:root drwxr-xr-x); PR body fetch locating the demo-path source; git status --short clean and HEAD unchanged at d15ba05b35.
中文说明

Autofix 评审轮次 — PR #10002

本轮无代码改动 —— 新反馈没有任何 finding,两处已披露的审查缺口改为用证据闭环:自动评审看不到的集成测试套件已在本地实际运行(192 个测试通过;唯一失败已证明是运行器文件系统状态问题,而非代码缺陷);测试计划中的路径已查明属于 PR 正文引用的演示项目,而非本仓库。

提交:无(无改动轮次)。未合并 base(--conflict false)。工作树在 d15ba05b35 上保持干净。

反馈项与处置

[rv:5027700986] COMMENTED,零 finding —— 两处已披露缺口均以证据闭环

针对 d15ba05b35 的第 4 轮审查没有发布任何 finding(台账:findings: [], posted: 0),也不带投票。其可操作内容是两处缺口披露:

缺口 1 —— "build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。"

  • 为什么 CI 里被跳过:该通道按设计只在合并队列运行 —— ci.yml 中的 integration_cligithub.event_name == 'merge_group' 为条件,因此在 PR push 上从不运行;它在 PR 落入 main 之前运行,那里才有 base 仓库的模型 secrets。这一跳过是结构性的,不是本 PR 引入的回归。
  • 缺口已在本地闭环:本运行器具备模型凭据,因此这里实际执行了该通道会运行的同一套命令 —— npm run build && npm run bundle,然后 test:integration:cli:sandbox:noneQWEN_SANDBOX=false):38 个文件中 32 个通过;192 个测试通过,18 个按自身选择性环境开关自跳过(压测、daemon 基准、gen-ai telemetry),1 个失败
  • 唯一失败是环境问题,由探针证明而非口头断言:cli/qwen-config-dir.test.ts 1d 故意取消 QWEN_HOME,迫使 CLI 在 $HOME/.qwen 下创建默认配置目录。在本自托管运行器上,/home/github-runner 属主为 root:root、权限 drwxr-xr-x,测试进程(用户 node)得到 EACCES: permission denied, mkdir '/home/github-runner/.qwen' —— 单独重跑该文件原样复现(1 失败 / 6 通过,同样是 EACCES)。用可写的 HOME 重跑同一文件则 7/7 通过:代码路径本身没有问题,只是运行器的 home 目录属主不允许写入。合并队列的 job 里恰好有一个 "Restore workspace ownership" 步骤,专门处理复用型自托管运行器上的这类遗留状态。本 PR 的 diff 是纯内容改动(SKILL.md 及其测试、文档、web-shell i18n),不触及失败的启动路径(initializeLlmOutputLanguagewriteOutputLanguageFile);在该合并头上这段代码与 origin/main 逐字节一致 —— 在此环境下跑 base 分支也会得到同样结果。

缺口 2 —— "Test Plan(非阻断):src/auth/login.jstest/login.test.js — no such file or directory。"

  • 以证据拒绝(Decline):这两个路径现在不是、也从来不是本仓库的一部分(对两者的 git log --all --follow 均为空;本仓库是 TypeScript monorepo,所有代码都在 packages/* 之下)。它们只出现在 PR 正文的 Evidence 部分所引用的 /goal-draft 示例输出里 —— 即示例 objective 所针对的那个一次性演示项目(其 package.json 定义了该 objective 引用的 testcoverage 脚本)。评审的探针把演示项目的路径误当成了本仓库的路径,且评审本身已标注该项非阻断。本代码树中没有任何可修之处,本代码树内的任何改动也无法修复它。

[ic:5422095679] "Sandboxed verification is running" —— 纯状态信息,无需处理

这是 triage 工作流的验证阶段针对维护者 /triage 重跑而发布的状态行,不含任何 finding,也没有任何要求。该重跑随后已完成且未产生新审查("Triage re-run completed without a new review")。没有线程需要回复,也没有任何要处理的事项。

验证

  • npx vitest run src/skills/bundled/goal-draft/SKILL.test.ts src/skills/bundled-skills.integration.test.ts(packages/core)—— 23 通过(2 个文件)
  • npx vitest run client/constants/localCommands.test.ts(packages/web-shell)—— 10 通过
  • npm run build —— 通过
  • npm run bundle —— 通过(bundle 中含 dist/bundled/goal-draft/SKILL.mdnode dist/cli.js --version → 0.22.0)
  • npm run test:integration:cli:sandbox:none(与合并队列通道完全相同的套件)—— 38 个文件中 32 个通过;192 通过 / 18 个选择性开关自跳过 / 1 个环境性失败(诊断见上),耗时 480 秒
  • 原样重跑 cli/qwen-config-dir.test.ts —— 复现失败:1 失败 / 6 通过(root 属主的 /home/github-runner 上 EACCES)
  • 用可写 HOME=/tmp/qwen-int-home-probe 重跑 cli/qwen-config-dir.test.ts —— 7 通过
  • npm run typecheck —— 通过
  • npm run lint —— 通过
  • 证据收集(只读):ci.ymlintegration_cli 的门控(仅 merge_group);对两个演示路径执行 git log --all --follow(为空);ls -ld /home/github-runnerroot:root drwxr-xr-x);拉取 PR 正文定位演示路径来源;git status --short 干净,HEAD 仍为 d15ba05b35 未变。

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max

@qqqys

qqqys commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /review

@github-actions

Copy link
Copy Markdown
Contributor

Qwen Code review request accepted. Review is queued in workflow run.

@wenshao

wenshao commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Local verification — real build, real model, real TUI + Web Shell

I built this branch on my own machine and drove it end to end against a live model. All four steps of the Reviewer Test Plan reproduce, including the numbers. Three non-blocking findings are below; none of them should block merge.

Environment

PR head d15ba05 (merge of main)
Base for the A/B 0756be0 (merge-base with main)
Host macOS 26.6.2 (Darwin 25.6.0), arm64
Node 24.18.1 for npm install + npm run build + npm run bundle + vitest; 22.23.2 only for the integration-tests/terminal-capture harness
Model real gpt-5.6-sol behind a local recording reverse proxy, so every /v1/chat/completions request — the tool list and the tool calls — is on the record
Fixture a throwaway git repo outside the qwen-code tree, test: node --test test/, coverage: c8 --reporter=text node --test test/, lint: eslint src, isolated QWEN_HOME

What reproduces

1 — Tests and static gates. cd packages/core && npx vitest run src/skills/bundled/goal-draft/SKILL.test.ts src/skills/bundled-skills.integration.test.ts2 files, 23 tests passed, exactly the count claimed. packages/web-shell client/constants/localCommands.test.ts → 10 passed. npx eslint over every touched source → clean. Full npm run build (tsc --build across all workspaces) → exit 0.

2 — It actually ships in the bundle. After npm run build && npm run bundle, dist/bundled/goal-draft/SKILL.md is byte-identical to the source (sha256 8421f792057fb50c…), and the wire capture shows the live session receiving Base directory for this skill: …/dist/bundled/goal-draft followed by the SKILL body — the skill is delivered from the bundle, not from the source tree.

3 — Discovery A/B. Typing /goal in the TUI: base offers only goal; this branch offers goal and goal-draft [Skill] with its argument hint. Same binary, same fixture, only dist/bundled/goal-draft differs. (Evidence 1 / 2)

4 — Headless run, grounded and read-only. qwen -p "/goal-draft make the auth tests pass and raise coverage" -y in the fixture. The provider ledger for the whole run is three round-trips:

get_goal   {"view":"summary"}  ->  {"active":false}
read_file  package.json
read_file  src/auth/login.js
read_file  test/login.test.js

No run_shell_command, no edit, no write_file. The output is a six-label objective quoting the project's real npm test / npm run coverage / npm run lint, an [ASSUMPTION] note for what headless could not ask, and a one-line /goal set …. The tracked tree is sha256-identical before and after; only .qwen/ appears — the "git status clean apart from .qwen/" claim holds.

5 — The active-Goal branch of Step 1 works. With a Goal already on the session, get_goal returns it ({"active":true,…"objective":"Outcome: npm test exits 0…"}) and the hand-off switches from /goal set … to /goal edit … without being told to.

6 — Step 0 gate fires. /goal-draft clean up the auth module returned: "This is not suitable as a Goal because 'clean up' has no objective, verifiable completion condition. Use a one-shot code review/refactor task instead, or define a measurable target…" — it refused rather than drafting an unjudgeable objective.

7 — ask_user_question really does stay behind its dialog. This is the point of e512166 and it holds in a live TUI session in Ask permissions mode: get_goal and the three read_file calls run with no prompt (the allowedTools grant), while the clarifying question is rendered as a real dialog with a recommended default. Answering it produces the objective and the /goal set line. (Evidence 3 / 7)

8 — Web Shell placeholder. Base vs. this branch, both locales. (Evidence 4 / 5 / 6)

Findings — non-blocking

A. Headless /goal-draft without -y fails (reproduced 2/2). Step 2 of the test plan says to run qwen -p "/goal-draft …", with no approval flag. In a full-tool session that run ends with:

Warning: Tool "skill" requires user approval but cannot execute in non-interactive mode.
Unable to draft the goal because permission to invoke the required `goal-draft` skill was declined
in non-interactive mode. No files or goal state were changed.

The slash path has already injected the SKILL body (confirmed on the wire), and the model then calls skill{"skill":"goal-draft"} a second time; that call needs approval, is declined, and the model gives up instead of continuing with the body it already has. /qc-helper how do I configure MCP? in the same mode answers normally, so this is not generic to every bundled skill. Cheapest fixes: put -y in the docs' headless example, or add one line to SKILL.md telling the model it is already inside the skill and must not re-invoke it.

B. The hand-off line renders with backslashes. The model wraps the /goal set … line in inline code and escapes the backticks inside it, so the terminal shows `npm test` as \npm test\ — a user copying the line "as-is" pastes backslashes into the objective. The fenced objective right above it renders correctly. Step 5 could ask for the hand-off as a plain, unquoted line (or ask that the objective avoid backticks, since it is consumed as a raw string anyway). Visible at the bottom of Evidence 7.

C. The model-invocation risk, measured. The PR flags it; here is the shape of it, first tool call only, default approval:

Prompt (no slash command) First action
keep going until the auth tests pass skill{goal-draft} (2/2 runs)
keep working until npm test exits 0 skill{goal-draft}
make the auth tests pass read_file — skill did not fire

So the narrow description does what it says: it does not hijack ordinary work requests, only the "keep going until X" phrasing it explicitly claims. Whether that phrasing should be intercepted into a drafting detour is a product call — the documented disable-model-invocation: true fallback stays available if it proves annoying in practice.

Documentation spot-checks

docs/users/features/goals.md matches the implementation I read: the /goal sub-command table matches parseGoalCommand, the trust sentence matches the set/edit/resume gate in goalCommand.ts, and the "/goal set and /goal edit collapse newlines to spaces" wording is now precise (bare /goal <objective> keeps the raw string, and the doc no longer claims otherwise). ./headless.md#run-a-persistent-goal resolves to a real heading, and goals is registered in _meta.ts.

Evidence

Images are hosted on pr-assets/10002-verify @ b72a951.

1. Base (main): /goal offers only goal

2. This branch: /goal offers goal and goal-draft [Skill]

3. One round of questions, shown as a real dialog (Ask-permissions mode)

4. Web Shell Goals dialog — base

5. Web Shell Goals dialog — this branch (en)

6. Web Shell Goals dialog — this branch (zh-CN)

7. End-to-end: get_goal → reads → one question → objective + /goal set (finding B visible at the bottom)

中文说明

本地验证 —— 真实构建、真实模型、真实 TUI 与 Web Shell

我在本机把这个分支完整构建并对着真实模型端到端跑通。Reviewer 测试计划的四步全部复现,包括其中的数字。 下面三条是非阻塞发现,都不影响合并。

环境

PR head d15ba05(已合 main
A/B 对照 base 0756be0(与 main 的 merge-base)
主机 macOS 26.6.2(Darwin 25.6.0),arm64
Node npm install + npm run build + npm run bundle + vitest 用 24.18.1;仅 integration-tests/terminal-capture 截图管线用 22.23.2
模型 真实 gpt-5.6-sol,经本地记录型反向代理,因此每次 /v1/chat/completions 的工具清单和工具调用都有台账
夹具 qwen-code 目录之外的一次性 git 仓库,test: node --test test/coverage: c8 --reporter=text node --test test/lint: eslint srcQWEN_HOME 隔离

复现结果

1 —— 测试与静态门。 cd packages/core && npx vitest run src/skills/bundled/goal-draft/SKILL.test.ts src/skills/bundled-skills.integration.test.ts2 个文件、23 个测试通过,与 PR 声称的数字完全一致。packages/web-shellclient/constants/localCommands.test.ts → 10 个通过。对所有改动源文件跑 npx eslint → 干净。完整 npm run build(全 workspace 的 tsc --build)→ 退出码 0。

2 —— 确实进了产物。 npm run build && npm run bundle 之后,dist/bundled/goal-draft/SKILL.md 与源文件逐字节相同(sha256 8421f792057fb50c…),且线上抓包显示会话收到的正是 Base directory for this skill: …/dist/bundled/goal-draft 加 SKILL 正文——skill 来自 bundle 而非源码树。

3 —— 发现路径 A/B。 TUI 里输入 /goal:base 只给出 goal;本分支给出 goal goal-draft [Skill] 及其参数提示。同一个二进制、同一个夹具,唯一差别是 dist/bundled/goal-draft。(证据 1 / 2)

4 —— headless 运行:有 grounding、且只读。 在夹具里跑 qwen -p "/goal-draft make the auth tests pass and raise coverage" -y,整轮的 provider 台账只有三次往返:

get_goal   {"view":"summary"}  ->  {"active":false}
read_file  package.json
read_file  src/auth/login.js
read_file  test/login.test.js

没有 run_shell_command、没有 edit、没有 write_file。产出是引用项目真实 npm test / npm run coverage / npm run lint 的六段式 objective、一条 headless 无法提问而标注的 [ASSUMPTION],以及单行 /goal set …。被跟踪文件运行前后 sha256 完全一致,只多出 .qwen/——"除 .qwen/git status 干净"的说法成立。

5 —— Step 1 的"已有 Goal"分支有效。 会话里已存在 Goal 时,get_goal 会把它读回来({"active":true,…"objective":"Outcome: npm test exits 0…"}),交接行自动从 /goal set … 变成 /goal edit …,无需额外提示。

6 —— Step 0 的闸门会触发。 /goal-draft clean up the auth module 的回答是:"这不适合作为 Goal,因为'clean up'没有客观可验证的完成条件。请改用一次性的代码审查/重构任务,或者定义一个可度量的目标……"——它选择拒绝,而不是硬写一个无法判定的 objective。

7 —— ask_user_question 确实仍在弹窗后面。 这正是 e512166 的用意,在真实 TUI 的 Ask permissions 模式下成立:get_goal 和三次 read_file 无需确认直接执行(allowedTools 授权生效),而澄清问题以带推荐默认项的真实弹窗呈现。回答之后才产出 objective 和 /goal set 行。(证据 3 / 7)

8 —— Web Shell placeholder。 base 与本分支对照,中英双语。(证据 4 / 5 / 6)

发现 —— 均非阻塞

A. headless 不加 -y/goal-draft 会失败(2/2 复现)。 测试计划第 2 步给的命令是 qwen -p "/goal-draft …",不带审批参数。在工具齐全的会话里,这条命令的结局是:

Warning: Tool "skill" requires user approval but cannot execute in non-interactive mode.
Unable to draft the goal because permission to invoke the required `goal-draft` skill was declined
in non-interactive mode. No files or goal state were changed.

slash 路径已经注入了 SKILL 正文(抓包可证),模型却又调了一次 skill{"skill":"goal-draft"};这次调用需要审批、被拒绝,模型于是放弃,而没有拿着手里已有的正文继续。同样模式下 /qc-helper how do I configure MCP? 能正常作答,所以这不是所有内置 skill 的通病。最省事的两条修法:文档的 headless 示例里加 -y,或者在 SKILL.md 里加一句"你已经在这个 skill 里了,不要再次调用它"。

B. 交接行渲染出反斜杠。 模型把 /goal set … 整行包进 inline code,并把行内的反引号转义,于是终端把 `npm test` 显示成 \npm test\——用户"照抄"这一行会把反斜杠粘进 objective。正上方代码块里的 objective 渲染是正常的。Step 5 可以要求交接行以纯文本(不加引号/不加代码标记)输出,或者要求 objective 内不要用反引号(反正它是按原始字符串消费的)。证据 7 的底部可见。

C. 模型可调用带来的风险,实测。 PR 自己标了这条风险,这是它的实际形状——只看第一个动作,默认审批模式:

提示词(不带 slash 命令) 第一个动作
keep going until the auth tests pass skill{goal-draft}(2/2 次)
keep working until npm test exits 0 skill{goal-draft}
make the auth tests pass read_file——skill 没有触发

也就是说,收窄后的 description 确实做到了它承诺的事:不会劫持普通的干活请求,只会命中它明写的"keep going until X"句式。这种句式该不该被拦成一次起草绕行,是产品判断;真嫌烦的话,文档里那条 disable-model-invocation: true 的一行兜底随时可用。

文档抽查

docs/users/features/goals.md 与我读到的实现一致:/goal 子命令表与 parseGoalCommand 对得上,信任那句话与 goalCommand.tsset/edit/resume 的门对得上,"/goal set/goal edit 会把换行折叠成空格"的措辞现在是准确的(裸 /goal <objective> 保留原始字符串,文档也不再声称相反)。./headless.md#run-a-persistent-goal 指向真实存在的标题,goals 也已登记进 _meta.ts

证据

图片托管在 pr-assets/10002-verifyb72a951,与上方英文部分的编号一一对应:1 base 的 /goal 补全、2 本分支的 /goal 补全、3 提问弹窗、4 Web Shell base placeholder、5 本分支英文 placeholder、6 本分支中文 placeholder、7 端到端产出。

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially reviewed — gaps disclosed.

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

Test Plan (not a blocker): npm testexit 1; npm run coverageno package defines this script; 23 tests pass — this review observed 21616, 4235, 1702, 24738, 1659, 601, 630 passed.

中文说明

仅完成部分审查,审查缺口已披露。

未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。

Test Plan(非阻断):npm testexit 1; npm run coverageno package defines this script; 23 tests pass — this review observed 21616, 4235, 1702, 24738, 1659, 601, 630 passed

— qwen3.8-max via Qwen Code /review (v0.22.0)

…wenLM#10002)

The `/goal-draft` slash path injects the skill body, but the model could
then call the `skill` tool a second time; in headless runs that call
needs an approval nobody can give, so the session stopped without
drafting anything. The body now states it is already loaded and must not
be re-invoked.

The hand-off line could also be wrapped in inline code with escaped
backticks, so copying it verbatim pasted backslashes into the objective.
Step 5 now requires the line as plain text.

Both wording changes are pinned by SKILL.test.ts assertions.
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 3/100 轮)。改动内容与我反驳保留之处如下:

Autofix review round — PR #10002

Feedback triage and dispositions

One feedback item this round: [ic:5423813920], a maintainer end-to-end local verification report carrying three non-blocking findings. No failed checks, no inline comments, no review bodies.

Finding A — headless /goal-draft without -y fails → addressed. Reproduced 2/2 by the maintainer with a wire capture: the /goal-draft slash path injects the skill body, but the model then called skill{"skill":"goal-draft"} a second time; that call needs approval, is declined in non-interactive mode, and the session stopped without drafting. The exact warning was traced to handleToolError in packages/cli/src/utils/errors.ts. Of the two fixes the reviewer proposed, the root-cause one was applied: the skill body now opens with "You are already inside the loaded goal-draft skill — do not call the skill tool to invoke it again; start with Step 0." This holds for both entry paths (the slash-command injection and the model's own Skill call both deliver the body through buildSkillLlmContent) and makes the documented headless example (qwen -p "/goal-draft <intent>" in headless.md) work without -y. Adding -y to the docs example was deliberately not done: -y auto-approves every tool and would silently drop the skill's read-only posture, which relies on only the non-mutating tools being auto-approved.

Finding B — the hand-off line renders with backslashes → addressed. The reviewer's Evidence 7 shows the /goal set … line wrapped in inline code with escaped backticks, so copying it "as-is" pastes backslashes into the objective. Step 5 now requires: "Print it as plain text with no code markers, so it can be copied verbatim." This fix was chosen over the reviewer's alternative (banning backticks in the objective) because it removes the copy-paste hazard directly without changing the objective format or the Weak→strong examples.

Finding C — model-invocation risk, measured → declined (with evidence). The reviewer's own measurement shows the narrow description behaves exactly as designed and documented: make the auth tests pass did not fire the skill (first action read_file), and only the "keep going until X" phrasing the description explicitly claims fired it (2/2). That interception is the PR's deliberate product call, with recorded fallbacks (skills.disabled; the disable-model-invocation: true frontmatter). The finding confirms the current design rather than reporting a defect, and no change was requested — none was made.

Changes

  • packages/core/src/skills/bundled/goal-draft/SKILL.md (+2 lines, 1 extended):
    • new opening line forbidding a second skill tool invocation (finding A);
    • Step 5 hand-off item 2 now mandates plain text with no code markers (finding B).
  • packages/core/src/skills/bundled/goal-draft/SKILL.test.ts (+11/-1): two existing tests now pin both new wordings, with comments recording the failure modes. The test count is unchanged (8 tests in this file; 23 across the two skill test files).

Both new guards were mutation-probed before committing: removing either SKILL.md sentence makes exactly its witness test fail (1 of 8, the rest green); restoring returns the suite to green.

No conflicts (--conflict false); origin/main was not merged.

Verification

  • npm run build — passed (exit 0); the built copy packages/core/dist/src/skills/bundled/goal-draft/SKILL.md was checked to contain the new wording.
  • npm run typecheck — passed (exit 0).
  • npm run lint — passed (exit 0, full repo).
  • npx prettier --check on both changed files — passed (after --write collapsed one assertion to one line).
  • Focused Vitest (packages/core): src/skills/bundled/goal-draft/SKILL.test.ts + src/skills/bundled-skills.integration.test.ts — 2 files, 23 tests passed (the same count the reviewer verified independently).
  • Focused Vitest (packages/web-shell): client/constants/localCommands.test.ts — 10 tests passed (references the skill's description key; unchanged area re-run for safety).
  • Mutation probe 1: removed the no-re-invocation sentence → stays model-invocable and user-invocable … failed (1/8); restored → green.
  • Mutation probe 2: removed the plain-text clause → fixes the objective contract labels … failed (1/8); restored → final re-run 23/23 green.
  • Not run: bundled-CLI integration suites — the changed behavior is model-side compliance with skill wording, which no deterministic harness can exercise without a live model; the wording itself is pinned by the unit tests above, and the bundle-copy mechanism (byte-identical in the maintainer's run) is unchanged. No settings source changed, so npm run generate:settings-schema was not needed.
中文说明

Autofix 审查轮次 — PR #10002

反馈分类与处置

本轮只有一条反馈:[ic:5423813920],一位维护者端到端本地验证报告,内含三条非阻塞发现。没有失败检查,没有行内评论,没有 review body。

发现 A —— headless 下不加 -y/goal-draft 失败 → 已处理。 维护者以 2/2 复现并附线上抓包:/goal-draft 的 slash 路径已经注入了 skill 正文,但模型随后又调了一次 skill{"skill":"goal-draft"};该调用需要审批,在非交互模式下被拒绝,会话因此停止、什么都没起草。这条警告的确切来源已追溯到 packages/cli/src/utils/errors.tshandleToolError。在审阅者提出的两条修法中,我们采用了根因修法:skill 正文开头新增一句 "You are already inside the loaded goal-draft skill — do not call the skill tool to invoke it again; start with Step 0."(你已在加载好的 goal-draft skill 内——不要再调用 skill 工具重新调用它;直接从 Step 0 开始)。这句话对两条入口路径都成立(slash 命令注入与模型自己的 Skill 调用都经由 buildSkillLlmContent 送达正文),并使文档中的 headless 示例(headless.md 里的 qwen -p "/goal-draft <intent>")在不加 -y 时也能工作。我们刻意没有给文档示例加 -y-y 会自动批准所有工具,会悄悄破坏该 skill 的只读姿态——只读依赖的正是"只有非变更类工具被自动批准"。

发现 B —— 交接行渲染出反斜杠 → 已处理。 审阅者的证据 7 显示 /goal set … 行被包进 inline code、反引号被转义,用户"照抄"这一行会把反斜杠粘进 objective。Step 5 现在要求:"Print it as plain text with no code markers, so it can be copied verbatim."(以纯文本输出,不加任何代码标记,以便逐字复制)。相比审阅者的另一个备选方案(禁止 objective 里出现反引号),选择这一修法是因为它直接消除了复制粘贴风险,且不需要改动 objective 格式或 Weak→strong 示例表。

发现 C —— 模型可调用的风险,实测 → 拒绝修改(附证据)。 审阅者自己的测量表明,收窄后的 description 行为与设计和文档完全一致:make the auth tests pass 没有触发该 skill(第一个动作是 read_file),只有 description 明写的 "keep going until X" 句式会触发(2/2)。这种拦截是 PR 明确做出的产品决策,且已记录兜底手段(skills.disableddisable-model-invocation: true frontmatter)。该发现确认的是当前设计而非缺陷,也没有要求任何改动——因此未做改动。

变更内容

  • packages/core/src/skills/bundled/goal-draft/SKILL.md(+2 行、1 行扩写):
    • 新增开头一句,禁止第二次调用 skill 工具(发现 A);
    • Step 5 交接项 2 现在要求纯文本、不加代码标记(发现 B)。
  • packages/core/src/skills/bundled/goal-draft/SKILL.test.ts(+11/-1):两个既有测试现在分别钉住上述两处新措辞,注释记录了各自的失败模式。测试数量不变(该文件仍为 8 个测试;两个 skill 测试文件合计 23 个)。

两个新守卫在提交前都做了变异探针验证:分别删掉 SKILL.md 中的任一句,恰好使其对应的见证测试失败(8 个中失败 1 个,其余为绿);恢复后整套测试恢复全绿。

无冲突(--conflict false);未合并 origin/main

验证

  • npm run build —— 通过(退出码 0);并确认构建产物 packages/core/dist/src/skills/bundled/goal-draft/SKILL.md 包含新措辞。
  • npm run typecheck —— 通过(退出码 0)。
  • npm run lint —— 通过(退出码 0,全仓库)。
  • 对两个改动文件执行 npx prettier --check —— 通过(--write 将一条断言折成单行之后)。
  • 聚焦 Vitest(packages/core):src/skills/bundled/goal-draft/SKILL.test.ts + src/skills/bundled-skills.integration.test.ts —— 2 个文件、23 个测试通过(与审阅者独立验证的数字一致)。
  • 聚焦 Vitest(packages/web-shell):client/constants/localCommands.test.ts —— 10 个测试通过(该测试引用了 skill 的 description key;属未改动区域,为保险复跑)。
  • 变异探针 1:删除"禁止再次调用"句 → stays model-invocable and user-invocable … 失败(1/8);恢复 → 全绿。
  • 变异探针 2:删除"纯文本"从句 → fixes the objective contract labels … 失败(1/8);恢复 → 最终复跑 23/23 全绿。
  • 未执行:bundle 后的 CLI 集成测试 —— 本次改动的行为是模型对 skill 措辞的遵从,没有真实模型时任何确定性测试框架都无法演练;措辞本身已由上述单元测试钉住,而 bundle 拷贝机制(维护者运行时逐字节一致)未被改动。未改动任何 settings 源,因此无需执行 npm run generate:settings-schema

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/模型 qwen3.8-max

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found. LGTM! ✅

Not explored to full depth (tool budget reached): "agent 3b": executing SKILL.test.ts (worktree has no node_modules; npm ci + workspace build exceeds this review's budget — interaction verified by reading the production ….

Test Plan (not a blocker): npm testexit 1; npm run coverageno package defines this script; 23 tests pass — this review observed 21616, 1702, 24738, 1659, 601, 4235, 630 passed.

中文说明

未发现问题。LGTM!✅

未探索到全部深度(达到工具调用预算):"agent 3b"executing SKILL.test.ts (worktree has no node_modules; npm ci + workspace build exceeds this review's budget — interaction verified by reading the production …

Test Plan(非阻断):npm testexit 1; npm run coverageno package defines this script; 23 tests pass — this review observed 21616, 1702, 24738, 1659, 601, 4235, 630 passed

— qwen3.8-max via Qwen Code /review (v0.22.0)

@qqqys
qqqys added this pull request to the merge queue Aug 26, 2026
Merged via the queue into QwenLM:main with commit 4c51f8c Aug 26, 2026
58 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants