Skip to content

fix(config): allow prompt hooks in settings schema #8779

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wenshao merged 6 commits into from
Aug 23, 2026
Merged

fix(config): allow prompt hooks in settings schema #8779

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
069e54e
fix(config): allow prompt hooks in settings schema (#8752)
Aug 9, 2026
5737e55
Merge branch 'main' into fix/issue-8752-prompt-hook-schema
wenshao Aug 11, 2026
4c0ff74
Merge branch 'main' into fix/issue-8752-prompt-hook-schema
wenshao Aug 17, 2026
d125a98
fix(cli): make prompt hook schema test type-safe
zjunothing Aug 21, 2026
cfa2f9d
Merge remote-tracking branch 'origin/main' into fix/issue-8752-prompt…
zjunothing Aug 21, 2026
09ac86c
test(cli): preserve hook type schema coverage
zjunothing Aug 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions packages/cli/src/config/settingsSchema.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,21 @@ import {

describe('SettingsSchema', () => {
describe('getSettingsSchema', () => {
it('should describe prompt hooks supported by the runtime', () => {
const hookProperties =
getSettingsSchema().hooks.properties.PreToolUse.items.properties?.[
'hooks'
]?.items?.properties;

expect(hookProperties?.['type']?.enum).toEqual([
'command',
'http',
'prompt',
]);
expect(hookProperties?.['prompt']).toMatchObject({ type: 'string' });
expect(hookProperties?.['model']).toMatchObject({ type: 'string' });
});

it('should contain all expected top-level settings', () => {
const expectedSettings: Array<keyof Settings> = [
'mcpServers',
Expand Down
11 changes: 10 additions & 1 deletion packages/cli/src/config/settingsSchema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ const HOOK_DEFINITION_ITEMS: SettingItemDefinition = {
type: 'string',
description:
'The type of hook. Note: "function" type is only available via SDK registration, not settings.json.',
enum: ['command', 'http'],
enum: ['command', 'http', 'prompt'],

@qwen-code-ci-bot qwen-code-ci-bot Aug 9, 2026

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.

[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)

@zjunothing zjunothing Aug 21, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

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。

required: true,
},
command: {
Expand All @@ -215,6 +215,15 @@ const HOOK_DEFINITION_ITEMS: SettingItemDefinition = {
description:
'The URL to send the POST request to. Required for "http" type.',
},
prompt: {
type: 'string',
description:
'The prompt to send to the model. Required for "prompt" type.',
},
Comment on lines +218 to +222

@qwen-code-ci-bot qwen-code-ci-bot Aug 9, 2026

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.

[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.

Suggested change
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)

@zjunothing zjunothing Aug 21, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

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，留给维护者单独决定或后续处理；这里不会把它声称为已修复。

model: {
type: 'string',
description: 'The optional model to use for a "prompt" hook.',
},
headers: {
type: 'object',
description:
Expand Down
Loading
Loading