fix(config): allow prompt hooks in settings schema - #8779
Conversation
Verification reportEnvironmentmacOS arm64, Node.js v22.23.2, npm workspaces; PR head Reproduction and resultThe generated Tests executed
EvidenceNon-UI schema change; screenshots are not applicable. The generated JSON Schema was regenerated from the canonical TypeScript schema and directly validated with Ajv. 中文验证报告验证报告环境macOS arm64、Node.js v22.23.2、npm workspaces;PR head 为 复现与结果生成的 已执行测试
证据这是非 UI 的 schema 修改,不适用截图。JSON Schema 已从规范 TypeScript schema 重新生成,并使用 Ajv 直接验证。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): PR #8779 adds the runtime-supported prompt hook type (w...: I could not run scripts/generate-settings-schema.ts to byte-diff the regenerated artifacts (the worktree has no node_modules/tsx; npm install is too costly wi…; PR #8779 adds the runtime-supported prompt hook type (w...: could not execute scripts/generate-settings-schema.ts to byte-diff the regenerated artifact against the committed one (no node_modules / tsx in this worktre…; PR #8779 adds the runtime-supported prompt hook type (w...: could not run npm run generate:settings-schema to byte-compare the regenerated JSON (no node_modules in this review worktree — tsx missing); substituted w…; PR #8779 adds the runtime-supported prompt hook type (w...: could not execute npm run generate:settings-schema and byte-diff its output against the committed settings.schema.json (worktree has no node_modules and cor….
中文说明
未探索到全部深度(达到工具调用预算):PR #8779 adds the runtime-supported prompt hook type (w...:I could not run scripts/generate-settings-schema.ts to byte-diff the regenerated artifacts (the worktree has no node_modules/tsx; npm install is too costly wi…;PR #8779 adds the runtime-supported prompt hook type (w...:could not execute scripts/generate-settings-schema.ts to byte-diff the regenerated artifact against the committed one (no node_modules / tsx in this worktre…;PR #8779 adds the runtime-supported prompt hook type (w...:could not run npm run generate:settings-schema to byte-compare the regenerated JSON (no node_modules in this review worktree — tsx missing); substituted w…;PR #8779 adds the runtime-supported prompt hook type (w...:could not execute npm run generate:settings-schema and byte-diff its output against the committed settings.schema.json (worktree has no node_modules and cor…。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| expect(hook.properties?.type.enum).toContain('prompt'); | ||
| expect(hook.properties?.prompt).toMatchObject({ type: 'string' }); | ||
| expect(hook.properties?.model).toMatchObject({ type: 'string' }); |
There was a problem hiding this comment.
[Critical] The test added by this PR does not compile under the repo's tsconfig. npm run build --workspace=packages/cli (tsc --build) fails with 7 errors: 4× TS4111 — index-signature members must be accessed with bracket notation (SettingItemDefinition.properties is a Record<string, …> and noPropertyAccessFromIndexSignature is enabled) — and 3× TS18048 — hook is SettingItemDefinition | undefined but is dereferenced as hook.properties. — Failure scenario: any npm run build or CI run → packages/cli fails to compile and the monorepo build aborts before the remaining workspaces and before any tests run. Already realized on this PR's own CI (Test job, ubuntu-latest Node 22.x, fails with exactly these errors). The test passes under vitest (transpile-only), which is why it only surfaces at the build/typecheck stage.
Suggested fix (covers the const hook chain at lines 30-31 as well as the anchored assertions):
const hook =
getSettingsSchema().hooks.properties.PreToolUse.items.properties?.[
'hooks'
]?.items;
expect(hook).toBeDefined();
expect(hook?.properties?.['type'].enum).toContain('prompt');
expect(hook?.properties?.['prompt']).toMatchObject({ type: 'string' });
expect(hook?.properties?.['model']).toMatchObject({ type: 'string' });中文说明
本 PR 新增的测试无法通过当前仓库 tsconfig 的编译。npm run build --workspace=packages/cli(tsc --build)报 7 个错误:4 个 TS4111——索引签名成员必须使用方括号访问(SettingItemDefinition.properties 是 Record<string, …>,且开启了 noPropertyAccessFromIndexSignature);3 个 TS18048——hook 的类型是 SettingItemDefinition | undefined,却被直接以 hook.properties 解引用。——失败场景:任何 npm run build 或 CI 运行都会导致 packages/cli 编译失败,整个 monorepo 构建在其余 workspace 和所有测试运行之前中止。该 PR 自己的 CI 已经复现(Test job,ubuntu-latest Node 22.x,报的正是这些错误)。该测试在 vitest 下能通过(vitest 只做转译、不做类型检查),因此问题只在 build/typecheck 阶段暴露。修复建议见上方代码块(同时覆盖第 30-31 行的 const hook 链)。
— qwen3.8-max via Qwen Code /review (v0.21.8)
There was a problem hiding this comment.
Fixed in d125a98935d1e5833639d17a1f7a01a03a611901.
The assertion now uses bracket notation for the Record<string, ...> entries and optional chaining through every nullable schema node. This removes all four TS4111 diagnostics and all three TS18048 diagnostics without changing runtime or generated-schema behavior.
Verification on the exact commit:
cd packages/cli && npx tsc --noEmit --project tsconfig.json --pretty false— passed with no output.cd packages/cli && npx vitest run src/config/settingsSchema.test.ts— 42/42 tests passed.npm run build,npm run bundle, andnpm run typecheck— all passed.- Independent post-fix verification:
VERIFIED_FIXED.
中文说明
已在 d125a98935d1e5833639d17a1f7a01a03a611901 中修复。
断言现在对 Record<string, ...> 条目使用方括号访问,并在所有可能为空的 schema 节点上使用可选链。4 个 TS4111 和 3 个 TS18048 已全部消失,且没有改变运行时或生成 schema 的行为。
精确提交上的验证结果:CLI TypeScript 检查无输出通过;聚焦测试 42/42 通过;全仓 build、bundle、typecheck 全部通过;独立修复后核验结论为 VERIFIED_FIXED。
| prompt: { | ||
| type: 'string', | ||
| description: | ||
| 'The prompt to send to the model. Required for "prompt" type.', | ||
| }, |
There was a problem hiding this comment.
[Suggestion] The restored field descriptions are paraphrases of the #3388 originals rather than verbatim restorations, dropping two documented user-facing details: the $ARGUMENTS placeholder note on prompt and the defaults-to-current-model note on model. The agreed fix direction on issue #8752 was to reinstate the definitions "exactly as #3388 had them"; the #3388 text (verified in git history 57282ebb7d) was 'The prompt template to send to the LLM. Required for "prompt" type. Use $ARGUMENTS as placeholder for hook input JSON.' and 'Optional model override for "prompt" type hooks. Defaults to your current model.' — Concrete cost: these description strings are what VS Code IntelliSense renders for every **/.qwen/settings.json; without the $ARGUMENTS note users lose the only in-editor guidance on how hook input reaches the prompt (promptHookRunner.ts substitutes $ARGUMENTS with the hook input JSON — without the placeholder, no input is injected), and docs/users/features/hooks.md documents both details. Validation behavior is unaffected.
| prompt: { | |
| type: 'string', | |
| description: | |
| 'The prompt to send to the model. Required for "prompt" type.', | |
| }, | |
| prompt: { | |
| type: 'string', | |
| description: | |
| 'The prompt template to send to the LLM. Required for "prompt" type. Use $ARGUMENTS as placeholder for hook input JSON.', | |
| }, |
Also restore the adjacent model description (lines 222-224) to 'Optional model override for "prompt" type hooks. Defaults to your current model.', then re-run npm run generate:settings-schema.
中文说明
恢复的字段描述是对 #3388 原始文本的改写而非逐字恢复,丢失了两处已文档化的用户可见信息:prompt 字段关于 $ARGUMENTS 占位符的说明,以及 model 字段关于默认使用当前模型的说明。issue #8752 中商定的修复方向是"完全按 #3388 原样"恢复这些定义;#3388 的原文(已通过 git 历史 57282ebb7d 核实)为 'The prompt template to send to the LLM. Required for "prompt" type. Use $ARGUMENTS as placeholder for hook input JSON.' 和 'Optional model override for "prompt" type hooks. Defaults to your current model.'——具体代价:这些描述字符串是 VS Code IntelliSense 在每个 **/.qwen/settings.json 上渲染的提示内容;缺少 $ARGUMENTS 说明后,用户将失去编辑器内关于 hook 输入如何注入 prompt 的唯一指引(promptHookRunner.ts 会把 $ARGUMENTS 替换为 hook 输入 JSON——没有该占位符则不会注入任何输入),且 docs/users/features/hooks.md 对这两点均有记载。不影响校验行为。同时请将相邻的 model 描述(第 222-224 行)恢复为 'Optional model override for "prompt" type hooks. Defaults to your current model.',然后重新运行 npm run generate:settings-schema。
— qwen3.8-max via Qwen Code /review (v0.21.8)
There was a problem hiding this comment.
Thanks — this is a valid documentation improvement, but it is not changed by d125a98935d1e5833639d17a1f7a01a03a611901.
This maintenance commit is intentionally limited to the Critical build blocker. The maintainer's exact-head verification found the schema restoration correct and complete and identified the test compilation failure as the only merge blocker. Changing the source descriptions and regenerating the schema here would widen the already verified diff.
I am therefore leaving this Suggestion explicitly unresolved and deferred for a separate maintainer decision or follow-up; I am not claiming it as fixed.
中文说明
感谢指出,这是一项有效的文档改进建议,但 d125a98935d1e5833639d17a1f7a01a03a611901 没有修改它。
本次维护提交刻意只处理阻塞构建的 Critical。维护者已在精确 head 上确认 schema 恢复正确且完整,并认定测试编译失败是唯一合并阻塞项。此时再修改源码描述并重新生成 schema,会扩大已经完成验证的差异范围。
因此这条 Suggestion 会明确保持未解决并 defer,留给维护者单独决定或后续处理;这里不会把它声称为已修复。
| description: | ||
| 'The type of hook. Note: "function" type is only available via SDK registration, not settings.json.', | ||
| enum: ['command', 'http'], | ||
| enum: ['command', 'http', 'prompt'], |
There was a problem hiding this comment.
[Suggestion] The hook-type allowlist widened here is duplicated in the ACP settings-read mirror — QwenHookConfig in packages/cli/src/acp-integration/acpAgent.ts declares type: 'command' | 'http' and normalizeHookConfig throws 'Hook type must be command or http' — and in the desktop DTO/UI (packages/desktop/packages/shared/src/protocol/dto.ts, QwenSettingsPage.tsx). This change makes those lists diverge on a config the schema now blesses. — Concrete cost: with {"hooks": {"PreToolUse": [{"hooks": [{"type": "prompt", "prompt": "..."}]}]}} in settings.json (schema-valid after this PR), the ACP settings-read path (readHooks → normalizeHookDefinition → normalizeHookConfig) throws for that entry; readHooks catches it and skips it with only a debugLogger.warn ("Skipping malformed hook entry"). Result: a valid prompt hook is silently invisible in the IDE-side user/workspace/merged hook listings served by qwen/settings/getCore, and the desktop DTO/UI cannot represent it — the same "valid config rejected" class this PR fixes for the schema, surviving in the mirrors. Before this PR the editor flagged such configs too, so the divergence is newly introduced by this diff. Either extend the mirrors in this PR as well (QwenHookConfig type + normalizeHookConfig acceptance with a prompt/model branch, plus the desktop DTO/UI), or file a follow-up issue and reference it here so the divergence is a decision, not an accident.
中文说明
此处放宽的 hook 类型白名单在 ACP 设置读取侧存在镜像副本——packages/cli/src/acp-integration/acpAgent.ts 中的 QwenHookConfig 声明 type: 'command' | 'http',且 normalizeHookConfig 会抛出 'Hook type must be command or http'——desktop 的 DTO/UI(packages/desktop/packages/shared/src/protocol/dto.ts、QwenSettingsPage.tsx)中同样如此。本次改动使这些列表对 schema 现已接受的配置产生分歧。——具体代价:settings.json 中配置 {"hooks": {"PreToolUse": [{"hooks": [{"type": "prompt", "prompt": "..."}]}]}}(本 PR 之后 schema 校验通过)时,ACP 设置读取路径(readHooks → normalizeHookDefinition → normalizeHookConfig)会抛错;readHooks 捕获后仅以 debugLogger.warn("Skipping malformed hook entry")跳过该条目。结果:有效的 prompt hook 在 qwen/settings/getCore 提供的 IDE 侧 user/workspace/merged hook 列表中静默不可见,desktop 的 DTO/UI 也无法表示它——这正是本 PR 为 schema 修复的"有效配置被拒绝"同类问题,在镜像副本中依然存在。本 PR 之前编辑器同样会标记此类配置,因此该分歧是本次 diff 新引入的。建议在本 PR 中一并扩展镜像副本(QwenHookConfig 类型 + normalizeHookConfig 的接受逻辑及 prompt/model 分支,加上 desktop 的 DTO/UI),或者创建一个后续 issue 并在本 PR 中注明,使该分歧成为有意决策而非意外遗漏。
— qwen3.8-max via Qwen Code /review (v0.21.8)
There was a problem hiding this comment.
Thanks — the cross-consumer divergence is valid, but d125a98935d1e5833639d17a1f7a01a03a611901 does not address it.
Supporting prompt hooks in the ACP normalizer and desktop DTO/UI is a separate cross-package behavioral expansion, with downstream consumers and regression tests beyond the schema-test compilation blocker maintained in this pass. Adding it here would materially widen the PR.
I am explicitly deferring this Suggestion and leaving it unresolved rather than claiming it is fixed. No follow-up issue was created in this maintenance pass.
中文说明
感谢指出,跨消费端的不一致确实存在,但 d125a98935d1e5833639d17a1f7a01a03a611901 没有处理它。
让 ACP normalizer 与 desktop DTO/UI 支持 prompt hook 是一项独立的跨 package 行为扩展,涉及本次 schema 测试编译阻塞之外的下游消费者与回归测试。把它加入当前修复会实质扩大 PR 范围。
因此这里明确 defer 这条 Suggestion,并保持未解决,不会声称已修复。本次维护没有创建后续 issue。
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): PR #8779 (QwenLM/qwen-code) adds the runtime-supported p...: did not re-run scripts/generate-settings-schema.ts byte-for-byte** (worktree has no node_modules ; npm install was out of budget). Structural verification …; PR #8779 (QwenLM/qwen-code) adds the runtime-supported p...: could not execute npm run generate:settings-schema for a byte-for-byte drift diff — node_modules is not installed in this review worktree (tsx missing); verif…; PR #8779 (QwenLM/qwen-code) adds the runtime-supported p...: did not run the new vitest spec or npm run typecheck locally (same missing-install reason); round-2 CI after the main merge is reported green per the review c…; PR #8779 (QwenLM/qwen-code) adds the runtime-supported p...: could not run tsc --build or vitest locally (no node_modules in worktree or parent checkout) — substituted CI-log evidence plus static analysis, which agree…; PR #8779 (QwenLM/qwen-code) adds the runtime-supported p...: could not execute scripts/generate-settings-schema.ts for a byte-level diff of the regenerated artifact — substituted a structural equivalence check against t…`.
[Critical] R1-1 [build]: The test added by this PR does not compile under the repo's tsconfig. npm run build --workspace=packages/cli (tsc --build) fails with 7 errors, all inside the 10 lines this PR adds: 4x TS4111 (index-signature members must be accessed with bracket notation — line 30 .hooks, lines 33-35 .type/.prompt/.model; SettingItemDefinition.properties is Record<string, …> and noPropertyAccessFromIndexSignature is enabled) and 3x TS18048 (lines 33-35: hook is SettingItemDefinition | undefined but dereferenced as hook.properties). Failure scenario: every build of packages/cli fails — locally, in npm run preflight, and in CI — aborting the monorepo build before downstream workspaces (webui, vscode-ide-companion) build and before any test phase runs. The test passes under vitest (transpile-only), so the failure surfaces only at build/typecheck. Still standing from round 1: the only change since was a merge of main; the test file is byte-identical, and the error is realized on this PR's own CI at the current HEAD (Test job). Already reported inline at settingsSchema.test.ts:33-35 in the previous round and not re-posted to avoid duplication, but the blocker stands. Suggested fix: bracket access + optional-chain guard, e.g. const hook = getSettingsSchema().hooks.properties.PreToolUse.items.properties?.['hooks']?.items; then expect(hook?.properties?.['type']?.enum).toContain('prompt'); and expect(hook?.properties?.['prompt']) / expect(hook?.properties?.['model']) with toMatchObject({ type: 'string' }).
中文说明
未探索到全部深度(达到工具调用预算):PR #8779 (QwenLM/qwen-code) adds the runtime-supported p...:did not re-run scripts/generate-settings-schema.ts byte-for-byte** (worktree has no node_modules ; npm install was out of budget). Structural verification …;PR #8779 (QwenLM/qwen-code) adds the runtime-supported p...:could not execute npm run generate:settings-schema for a byte-for-byte drift diff — node_modules is not installed in this review worktree (tsx missing); verif…;PR #8779 (QwenLM/qwen-code) adds the runtime-supported p...:did not run the new vitest spec or npm run typecheck locally (same missing-install reason); round-2 CI after the main merge is reported green per the review c…;PR #8779 (QwenLM/qwen-code) adds the runtime-supported p...:could not run tsc --build or vitest locally (no node_modules in worktree or parent checkout) — substituted CI-log evidence plus static analysis, which agree…;PR #8779 (QwenLM/qwen-code) adds the runtime-supported p...:could not execute scripts/generate-settings-schema.ts for a byte-level diff of the regenerated artifact — substituted a structural equivalence check against t…`。
[Critical] R1-1 [build]: The test added by this PR does not compile under the repo's tsconfig. npm run build --workspace=packages/cli (tsc --build) fails with 7 errors, all inside the 10 lines this PR adds: 4x TS4111 (index-signature members must be accessed with bracket notation — line 30 .hooks, lines 33-35 .type/.prompt/.model; SettingItemDefinition.properties is Record<string, …> and noPropertyAccessFromIndexSignature is enabled) and 3x TS18048 (lines 33-35: hook is SettingItemDefinition | undefined but dereferenced as hook.properties). Failure scenario: every build of packages/cli fails — locally, in npm run preflight, and in CI — aborting the monorepo build before downstream workspaces (webui, vscode-ide-companion) build and before any test phase runs. The test passes under vitest (transpile-only), so the failure surfaces only at build/typecheck. Still standing from round 1: the only change since was a merge of main; the test file is byte-identical, and the error is realized on this PR's own CI at the current HEAD (Test job). Already reported inline at settingsSchema.test.ts:33-35 in the previous round and not re-posted to avoid duplication, but the blocker stands. Suggested fix: bracket access + optional-chain guard, e.g. const hook = getSettingsSchema().hooks.properties.PreToolUse.items.properties?.['hooks']?.items; then expect(hook?.properties?.['type']?.enum).toContain('prompt'); and expect(hook?.properties?.['prompt']) / expect(hook?.properties?.['model']) with toMatchObject({ type: 'string' }).
— qwen3.8-max via Qwen Code /review (v0.21.9)
Maintainer verification — built and ran this locallyI merged this PR into current Verdict: the schema change itself is correct, complete and well-scoped — but the PR cannot be merged as-is, because the new test does not typecheck and breaks Environment: macOS arm64 (darwin 25.6.0), Node v24.18.1, npm workspaces, 1. Blocker — the new test breaks the build (CI is already red for this reason)
Two points worth calling out against the PR description's "Not validated / out of scope" note:
Suggested fix (verified locally: - const hook =
- getSettingsSchema().hooks.properties.PreToolUse.items.properties?.hooks
- .items;
-
- expect(hook.properties?.type.enum).toContain('prompt');
- expect(hook.properties?.prompt).toMatchObject({ type: 'string' });
- expect(hook.properties?.model).toMatchObject({ type: 'string' });
+ const hookProps =
+ getSettingsSchema().hooks.properties.PreToolUse.items.properties?.[
+ 'hooks'
+ ]?.items?.properties;
+
+ expect(hookProps?.['type']?.enum).toContain('prompt');
+ expect(hookProps?.['prompt']).toMatchObject({ type: 'string' });
+ expect(hookProps?.['model']).toMatchObject({ type: 'string' });I also mutation-checked the test with that fix applied — it fails as expected for each of the three things it guards:
2. The schema fix does what it claims — and only thatAjv (draft-07, the dialect the generated schema declares) against real fixtures, before =
So the enum is widened by exactly one value and the two new fields are typed, not free-form. Per-event coverage is complete: 17/17 hook event groups in the generated schema gained
3. What a user actually sees in VS CodeRun through
4. The runtime really does support what the schema now allowsI ran the actual bundled CLI built from this branch, with an isolated
Also confirmed: Summary
Please push the test fix above (or an equivalent); after that I'm happy to merge. 中文版本(Chinese version)维护者本地验证报告我把该 PR 合入当前 结论:schema 的修改本身是正确、完整且范围克制的,但当前状态不能直接合并——新增的测试无法通过类型检查,会让 环境:macOS arm64(darwin 25.6.0)、Node v24.18.1、npm workspaces、 1. 阻塞项:新测试导致构建失败(CI 已因此变红)
针对 PR 描述里"未验证/超出范围"的说法,有两点需要澄清:
建议的修复见英文部分的 diff(本地已验证: 我还对修复后的测试做了变异测试,三项守护都能被触发:去掉 enum 里的 2. schema 修改只做了它声称的事用 Ajv(draft-07,即生成的 schema 声明的方言)跑真实 fixture,before =
也就是说 enum 只放开了一个值,新增的两个字段是有类型约束的。覆盖面完整:生成的 schema 里 17/17 个 hook 事件组都拿到了 在合并后的树上执行 3. 用户在 VS Code 里实际看到的效果用
4. 运行时确实支持 schema 现在放行的配置我用本分支构建出的真实 CLI bundle 跑了一次:隔离的
另外确认: 小结
麻烦推一下上面那段测试修复(或等价写法),之后我这边就可以合并了。 |
|
Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with |
|
Maintenance verification for The new commit changes only the prompt-hook schema assertion in the existing CLI test. It uses type-safe bracket access and optional chaining, removing the build-blocking 4 × TS4111 and 3 × TS18048 diagnostics while preserving the assertion's behavior. Verification:
No product E2E scenario or screenshot is applicable to this incremental commit because it changes only the typed traversal in an existing test; runtime and generated-schema behavior are unchanged. The exact failure boundary is compilation and is covered by the checks above. Feedback disposition: the Critical compilation finding is fixed. The two non-blocking Suggestions about description wording and ACP/desktop mirrors are explicitly not fixed by this commit and remain deferred rather than being silently marked complete. 中文说明
新提交只修改现有 CLI 测试中的 prompt-hook schema 断言,通过类型安全的方括号访问和可选链,消除了阻塞构建的 4 个 TS4111 与 3 个 TS18048,同时保持断言行为不变。 验证结果:旧 head 上由独立测试工程师确认 本次增量提交不适用产品 E2E 场景或截图,因为它只改变现有测试中的类型化访问链,运行时与生成 schema 行为均未改变。实际失败边界就是编译,已由上述检查覆盖。 反馈处理:Critical 编译问题已修复;关于描述文案和 ACP/desktop 镜像的两条非阻塞 Suggestion 没有被本提交修复,已明确 defer,不会静默标记为完成。 |
|
Latest-main synchronization for The first CI run on I merged Post-merge verification:
No additional product behavior was introduced by the sync. New CI should now exercise the corrected test on the current base instead of failing on the missing repository script. 中文说明
我已无冲突合入 合并后验证:原失败的 workflow-size 步骤本地通过,脚本存在且可执行;锁定依赖安装、全量 build 与 bundle 通过且没有生成文件漂移;全仓 typecheck 通过;CLI TypeScript 检查无输出通过;聚焦 schema 测试 42/42 通过;Prettier 与 ESLint 通过;独立测试工程师结论为 同步没有引入额外产品行为。新 CI 现在应能在当前 base 上验证已修正的测试,而不会先因仓库脚本缺失失败。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent 6b": could not execute scripts/generate-settings-schema.ts to byte-diff its output against the committed JSON (review worktree has no node_modules); JSON/generator…; "agent 6b": could not run npx vitest run src/config/settingsSchema.test.ts or npm run typecheck in this dependency-less worktree; test/typecheck soundness verified by r….
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/cli/src/config/settingsSchema.ts:218 — [probe] missing required 'prompt' field validates in IDE but is silently discarded at runtime
中文说明
已审查。 建议见行内评论。
未探索到全部深度(达到工具调用预算):"agent 6b":could not execute scripts/generate-settings-schema.ts to byte-diff its output against the committed JSON (review worktree has no node_modules); JSON/generator…;"agent 6b":could not run npx vitest run src/config/settingsSchema.test.ts or npm run typecheck in this dependency-less worktree; test/typecheck soundness verified by r…。
收敛姿态下延后(第 3 轮,非阻断)——已记录,本轮不要求修改:共 1 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| 'hooks' | ||
| ]?.items?.properties; | ||
|
|
||
| expect(hookProperties?.['type']?.enum).toContain('prompt'); |
There was a problem hiding this comment.
[Suggestion] This assertion pins only membership of the new prompt value; nothing in the suite pins the pre-existing command/http entries (no other test references the hook enum — verified by grep). A one-line mutation of the schema to enum: ['prompt'] survives this test and every other test — measured: with the mutation applied this file still reports Tests 42 passed (42), while toEqual(['command', 'http', 'prompt']) on this assertion fails it (42 tests | 1 failed) and stays green on unmutated code. What that lets through: a future edit that replaces the enum list instead of extending it (a bad merge, a partial regeneration, a type-list cleanup) ships green — the CI schema-freshness gate regenerates settings.schema.json from the same mutated source, so no diff appears — and VS Code then flags every existing type: "command" and type: "http" hook in users' settings as invalid, a replay of issue 8752 against the established types.
| expect(hookProperties?.['type']?.enum).toContain('prompt'); | |
| expect(hookProperties?.['type']?.enum).toEqual([ | |
| 'command', | |
| 'http', | |
| 'prompt', | |
| ]); |
中文说明
该断言只固定了新增 prompt 值的成员关系;整个测试套件中没有任何测试固定既有的 command/http 条目(已用 grep 确认无其他测试引用 hook 枚举)。将 schema 变异为 enum: ['prompt'] 的一行改动即可绕过本测试及所有其他测试——实测:变异后该文件仍报告 Tests 42 passed (42);而将该断言改为 toEqual(['command', 'http', 'prompt']) 后变异会失败(42 tests | 1 failed),且在未变异代码上保持绿色。这会放行的故障:未来某次替换(而非扩展)枚举列表的修改(坏合并、部分重新生成、类型列表清理)将全绿通过——CI schema 新鲜度门禁从同一变异源重新生成 settings.schema.json,不会出现 diff——随后 VS Code 会把用户 settings 中所有既有的 type: "command" 和 type: "http" hook 标为无效,等于 issue 8752 在既有类型上重演。
— qwen3.8-max via Qwen Code /review (v0.21.15)
There was a problem hiding this comment.
Accepted and fixed in 09ac86ce24acb97f2b8c92c0daed33d1a400b48e.
The focused schema test now asserts the exact hook type enum — command, http, and prompt — instead of checking only membership of prompt. This preserves coverage for the established types and makes a future accidental replacement such as ['prompt'] fail.
Verification on the exact commit: the focused CLI test passed 42/42, CLI TypeScript checking passed, the full workspace build and bundle passed, full workspace typecheck passed, targeted Prettier and ESLint passed, and an independent test engineer returned VERIFIED_FIXED. No production source or generated schema changed.
中文说明
已在 09ac86ce24acb97f2b8c92c0daed33d1a400b48e 中接受并修复。
聚焦 schema 测试现在精确断言完整的 hook 类型枚举:command、http 和 prompt,不再只检查是否包含 prompt。这样既覆盖新增类型,也保护既有类型;未来若误把枚举替换为 ['prompt'],测试会直接失败。
精确提交上的验证结果:CLI 聚焦测试 42/42 通过、CLI TypeScript 检查通过、全仓 build 与 bundle 通过、全仓 typecheck 通过、目标 Prettier 与 ESLint 通过,独立 test-engineer 结论为 VERIFIED_FIXED。本次没有修改生产源码或生成 schema。
|
Round 3 verification update: PASS Exact commit: The new review suggestion exposed a regression-coverage gap: the test required only that Verification:
No runtime E2E or screenshot applies to this test-only assertion hardening. The relevant regression is directly enforced by the focused test, while the full build, bundle, and typecheck cover repository integration. 中文验证报告第 3 轮验证更新:通过 精确提交: 新评审建议指出了一个回归覆盖缺口:原测试只要求枚举中包含 验证结果:
本次是纯测试断言加固,不涉及运行时行为,因此不适用运行时 E2E 或截图。相关回归由聚焦测试直接约束,全仓 build、bundle 和 typecheck 则覆盖仓库集成。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
中文说明
未发现问题。LGTM!✅
— qwen3.8-max via Qwen Code /review (v0.21.15)
|
Released in v0.22.2. |




What this PR does
Adds the runtime-supported
prompthook type and itspromptand optionalmodelfields to the canonical settings schema, then regenerates the VS Code companion schema.Why it's needed
Prompt hooks are accepted and executed by the core runtime and documented for
settings.json, but both schema layers currently reject them. This makes valid configurations appear invalid in VS Code and prevents schema-based validation from matching runtime behavior.Reviewer Test Plan
How to verify
Use a settings file containing
hooks.PreToolUse[].hooks[]withtype: "prompt", a stringprompt, and an optionalmodel. The generated VS Code schema should accept the configuration, while the existing core prompt-hook tests continue to pass.Evidence (Before & After)
N/A — non-UI schema and validation change. Ajv validation of the documented prompt-hook configuration passes after the change.
Tested on
Environment (optional)
macOS arm64, Node.js v22.23.2, npm workspaces.
Risk & Scope
Linked Issues
Fixes #8752
中文说明
此 PR 的内容
将运行时已经支持的
prompthook 类型以及prompt和可选model字段加入规范设置 schema,并重新生成 VS Code companion schema。修复原因
核心运行时已经接受并执行
prompthook,settings.json文档也已经说明了该配置,但当前两个 schema 层都拒绝它。这导致 VS Code 将有效配置标记为无效,也使基于 schema 的校验与实际运行时行为不一致。评审验证计划
验证方法
使用包含
hooks.PreToolUse[].hooks[]的设置文件,其中 hook 使用type: "prompt"、字符串类型的prompt和可选的model。生成的 VS Code schema 应接受该配置,同时现有核心 prompt-hook 测试应继续通过。证据(修复前与修复后)
不适用——这是非 UI 的 schema 与校验修复。修复后,使用 Ajv 对文档中的 prompt-hook 配置进行校验已通过。
测试平台
环境(可选)
macOS arm64、Node.js v22.23.2、npm workspaces。
风险与范围
关联 Issue
Fixes #8752