Skip to content

feat: extension file reload — watch for plugin changes and hot-reload runtime#6347

Open
ZijianZhang989 wants to merge 8 commits into
QwenLM:mainfrom
ZijianZhang989:feat/extension-file-reload
Open

feat: extension file reload — watch for plugin changes and hot-reload runtime#6347
ZijianZhang989 wants to merge 8 commits into
QwenLM:mainfrom
ZijianZhang989:feat/extension-file-reload

Conversation

@ZijianZhang989

@ZijianZhang989 ZijianZhang989 commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Adds a file watcher on extension directories so that editing extension files on disk is automatically detected. Content changes to commands, skills, and agent files are applied inline without user action. Configuration and hook changes prompt the user once to run /reload-plugins.

The runtime refresh logic that was previously inline in ExtensionManager (restart MCP servers, reload skill/subagent caches, refresh hierarchical memory) is extracted into a single refreshExtensionRuntime function used by all enable/disable/install/uninstall paths as well as the new /reload-plugins command.

The slash command reports a summary of what was loaded: extension count, commands, skills, agents, hooks, MCP servers, and LSP servers, in the user's locale.

Hook configuration is now reloaded on extension changes. Agent-scoped hooks are preserved across the reload, and a failure during hook rebuild restores the previous state.

Extension manager mutations (enable, disable, install, uninstall, update, scope changes, favorites, MCP server toggles) are now wrapped with a lifecycle listener so the file watcher can suppress itself during programmatic changes and restart watching after the mutation settles.

Why it's needed

Today there is no way to detect extension file changes on disk. Every edit to a command, skill, hook, or extension config requires the user to manually run /reload-plugins. During extension development, where you iterate on files rapidly, the manual step adds significant friction.

Extension runtime refresh was split across two private methods on ExtensionManager (refreshMemory / refreshTools) with slightly different call chains depending on the caller, and hooks were never reloaded mid-session. This consolidates the refresh into one function used by every path.

Reviewer Test Plan

How to verify

  1. Install an extension that has commands, skills, or agents. Run the CLI normally.
  2. Edit a file in the extension's commands/, skills/, or agents/ directory and save.
  3. Wait ~250ms — the change should take effect without any manual action.
  4. Edit the extension's qwen-extension.json, a hook file, or a context file.
  5. A message should appear: "Extensions changed on disk. Run /reload-plugins to apply updates."
  6. Run /reload-plugins — a summary should appear showing counts like "1 extension · 2 commands · 3 skills · 4 agents · 5 hooks · 1 extension MCP server".
  7. Link an extension from a local path (/extensions link), edit a file in the linked directory, and confirm the watcher detects the change.
  8. Enable and disable an extension — the file watcher should suppress itself during the operation and restart watching afterward.
  9. Run with --bare — no watcher should be started.

Evidence (Before & After)

Before: Editing a skill file in an extension has no effect. The user must type /reload-plugins manually.

After: The skill is auto-refreshed within ~250ms of saving. Config and hook changes prompt the user once with a specific message.

Tested on

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

Environment (optional)

npm run dev on macOS. Unit tests and i18n strict-parity tests pass.

Risk & Scope

  • Main risk or tradeoff: The chokidar watcher consumes file descriptors. On macOS the default limit is 256 FSEvents handles; with many linked extensions this could approach the limit. The watcher logs errors on failure but has no user-visible notification for ENOSPC.
  • Not validated / out of scope: MCP server startup failures during /reload-plugins are not surfaced to the user (only logged). Hook disable state set via /hooks is lost across extension reloads. No backend daemon — the watcher stops with the process.
  • Breaking changes / migration notes: ExtensionManager.refreshMemory() was removed. It had no external callers — it was only called internally by refreshTools().

Linked Issues

Refs #3696

中文说明

这个 PR 做了什么

为扩展目录增加了文件监控,编辑磁盘上的扩展文件可被自动检测。对 commands、skills、agent 文件的内容变更会内联自动应用,无需用户操作。配置和钩子变更会一次性提示用户运行 /reload-plugins

之前分散在 ExtensionManager 中的运行时刷新逻辑(重启 MCP 服务器、重新加载 skill/subagent 缓存、刷新层次化记忆)被提取为一个 refreshExtensionRuntime 函数,所有 enable/disable/install/uninstall 路径以及新的 /reload-plugins 命令都使用它。

斜杠命令会以用户的语言报告加载内容汇总:扩展数量、命令、技能、代理、钩子、MCP 服务器和 LSP 服务器。

钩子配置现在会在扩展变更时重新加载。代理作用域的钩子会在重载过程中保留,钩子重建失败时会恢复到之前的状态。

扩展管理器变更操作(启用、禁用、安装、卸载、更新、作用域变更、收藏、MCP 服务器开关)现在通过生命周期监听器包裹,使文件监控器能够在程序化变更期间抑制自身,并在变更结束后重新开始监控。

为什么需要

当前无法检测磁盘上的扩展文件变更。每次编辑命令、技能、钩子或扩展配置文件,都需要用户手动运行 /reload-plugins。在扩展开发中频繁迭代文件时,手动操作增加了显著的摩擦。

扩展运行时刷新分散在 ExtensionManager 的两个私有方法中(refreshMemory / refreshTools),调用链因调用方不同而略有差异,且钩子从未在会话中间被重新加载。此次将其整合为一个所有路径都使用的函数。

Reviewer 测试计划

如何验证

  1. 安装一个包含 commands、skills 或 agents 的扩展,正常启动 CLI。
  2. 编辑扩展 commands/skills/agents/ 目录下的文件并保存。
  3. 等待约 250ms——变更应无需手动操作即生效。
  4. 编辑扩展的 qwen-extension.json、钩子文件或上下文文件。
  5. 应出现提示消息:"磁盘上的扩展已变更。运行 /reload-plugins 来应用更新。"
  6. 运行 /reload-plugins——应显示类似 "1 个扩展 · 2 个命令 · 3 个技能 · 4 个代理 · 5 个钩子 · 1 个扩展 MCP 服务器" 的汇总。
  7. 从本地路径 link 一个扩展,在链接目录中编辑文件,确认 watcher 能检测到变更。
  8. 启用和禁用一个扩展——文件 watcher 应在操作期间抑制自身,操作结束后重新开始监控。
  9. 使用 --bare 运行——不应启动 watcher。

Before / After

Before:编辑扩展中的技能文件无任何效果。用户必须手动输入 /reload-plugins

After:技能在保存后约 250ms 内自动刷新。配置和钩子变更会一次性提示用户,并给出具体消息。

测试平台

操作系统 状态
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

环境(可选)

npm run dev 在 macOS 上。单元测试和 i18n 严格校验测试均通过。

风险与范围

  • 主要风险或权衡:chokidar watcher 消耗文件描述符。macOS 默认限制为 256 个 FSEvents handle;有大量 linked extension 时可能接近此限制。watcher 在失败时会记录错误,但 ENOSPC 没有用户可见通知。
  • 未验证 / 超出范围:/reload-plugins 期间 MCP 服务器启动失败不会提示用户(仅记录日志)。通过 /hooks 设置的钩子禁用状态在扩展重载后会丢失。没有后台守护进程——watcher 随进程退出而停止。
  • 破坏性变更 / 迁移说明:ExtensionManager.refreshMemory() 已移除。该方法无外部调用方——仅由 refreshTools() 内部调用。

关联 Issue

Refs #3696

… runtime

- Extract refreshExtensionRuntime to centralize MCP, skills, subagents, hooks, and memory refresh
- Add ExtensionFileWatcher (chokidar) for auto-detecting extension file changes
- Add ExtensionRefreshState with per-session scoped instance and mutation suppression
- Replace monkey-patching with ExtensionManager native mutation listeners
- Add /reload-plugins slash command with i18n-aware summary across all 9 locales
- Add auto-refresh of extension content (commands/skills/agents) on file change
- Add HookRegistry.reloadConfiguredHooks() with correct error recovery
- Fix async mutation pairing via id-based Map instead of LIFO stack
- Fix bootstrap watcher close() UB with queueMicrotask deferral
- Fix concurrent refresh with runningRef/pendingRef guard
- Fix error propagation from refreshExtensionContentRuntime to UI
- Fix isIgnored cross-platform path splitting (path.sep → regex)
- Fix wrong ExtensionMutationEvent type via import from core
- Fix addItem on unmounted component with mountedRef guard
- Set followSymlinks: false on chokidar watchers

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

This PR touches core infrastructure (packages/core/src/extension/*, packages/core/src/hooks/*) at scale — 781 changed lines in core paths (437 source + 344 tests), exceeding the 500-line threshold for non-maintainer PRs.

Core refactors of this scope must be maintainer-initiated. The changes introduce a mutation listener pattern across ~15 ExtensionManager methods, a new extension-runtime-refresh module, and hook registry reload capabilities — these are architectural decisions that need maintainer design review first.

@ZijianZhang989 — the direction looks interesting, but the scope of core changes requires a maintainer to sign off on the design. Could you open an issue to discuss the approach before proceeding? Specifically:

  1. The mutation listener pattern wrapping every ExtensionManager public method
  2. The refreshExtensionRuntime extraction and its contract with callers
  3. The reloadConfiguredHooks rollback semantics in HookRegistry

Once there's alignment on the design, a maintainer can champion the implementation.

中文说明

此 PR 涉及核心基础设施(packages/core/src/extension/*packages/core/src/hooks/*)的大规模变更——核心路径共 781 行变更(437 行源码 + 344 行测试),超出非维护者 PR 的 500 行阈值。

此规模的核心重构必须由维护者发起。变更包括在约 15 个 ExtensionManager 方法中引入变更监听器模式、新增 extension-runtime-refresh 模块、以及钩子注册表的重载能力——这些都是需要维护者先进行设计评审的架构决策。

@ZijianZhang989 —— 方向看起来很有意思,但核心变更的规模需要维护者先确认设计方案。能否先开一个 issue 讨论方案?具体包括:

  1. 包裹每个 ExtensionManager 公开方法的变更监听器模式
  2. refreshExtensionRuntime 提取及其与调用方的契约
  3. HookRegistryreloadConfiguredHooks 的回滚语义

设计方案达成一致后,维护者可以推进实现。

Qwen Code · qwen3.7-max

@wenshao wenshao 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.

Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.

@wenshao

wenshao commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator

Suggestions — commit 2a15d1ad9c06657dce646ab188d9009816509fb6

File Issue Suggested fix
packages/core/src/extension/extension-runtime-refresh.ts:37 MCP server restart silently demoted from Tier 1 (fatal) to Tier 2 (swallowed). Callers of enable/disable/install/uninstall get no signal when MCP tools are stale after a failed reconcile. Keep MCP as Tier 1 (propagate error), or return a status/errors object from refreshExtensionRuntime so callers can surface partial failures.
packages/cli/src/config/extension-file-watcher.ts:64 No depth limit on chokidar watcher for linked extension sources. Recursive watch of arbitrary directories risks inotify/FSEvents resource exhaustion. Add depth: 2-3 for linked sources, or watch only relevant subdirectories (commands/, skills/, agents/, hooks/).
packages/core/src/extension/extensionManager.ts:564-607 Pattern (4 locations): toggleFavorite, setExtensionScope, setMcpServerDisabled, markSourceUpdated are wrapped in beginMutation/endMutation despite only modifying in-memory preferences. Each triggers a full watcher teardown + recreation. Remove beginMutation wrapping from preference-only methods, or add a diskMutating flag to ExtensionMutationEvent.
packages/core/src/extension/extensionManager.ts:1098,1468 Missing mutation event tests for installExtension/uninstallExtension. Unpaired mutation events would permanently freeze auto-refresh. Add tests asserting [start, end] mutation events for install/uninstall paths.
packages/cli/src/ui/hooks/slashCommandProcessor.ts:246-312 ~168 lines of new auto-refresh logic (debounce, re-entrancy guard, retry loop, useEffect hooks) added with zero test coverage. Add tests for debounced refresh, re-entrant pending coalescing, ExtensionRefreshNeeded bypass, and cleanup.
packages/cli/src/ui/hooks/slashCommandProcessor.ts:289 do...while auto-refresh retry loop has no upper bound. Rapid file changes could cause extended loop. Add MAX_RETRIES = 5 guard with a warning if exceeded.
packages/cli/src/i18n/locales/ja.js + 5 others 20 new reload-plugins locale keys are untranslated English key: key pairs in ja.js, de.js, fr.js, ru.js, ca.js, pt.js. Only zh.js and zh-TW.js received proper translations. Provide actual translations for all 6 locale files.

— qwen3.7-max via Qwen Code /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.

Found 3 Critical issues around silent failure modes. The watcher architecture and mutation-wrapping design look solid overall, but three error paths silently swallow failures that previously propagated or that the user needs to know about. See inline comments.

Comment thread packages/cli/src/ui/commands/reload-plugins-command.ts
Comment thread packages/core/src/extension/extension-runtime-refresh.ts Outdated
Comment thread packages/core/src/extension/extension-runtime-refresh.ts Outdated
Comment thread packages/cli/src/ui/hooks/slashCommandProcessor.ts
Comment thread packages/cli/src/config/extension-refresh-state.ts
Comment thread packages/cli/src/config/extension-file-watcher.ts
Comment thread packages/cli/src/config/extension-runtime-reload.ts
Comment thread packages/cli/src/config/extension-file-watcher.ts
Comment thread packages/cli/src/config/extension-file-watcher.ts
Comment thread packages/cli/src/config/extension-file-watcher.ts
Comment thread packages/cli/src/config/extension-file-watcher.ts
Comment thread packages/core/src/extension/extensionManager.ts
Comment thread packages/core/src/hooks/hookRegistry.ts
Comment thread packages/core/src/extension/extension-runtime-refresh.ts Outdated
Comment thread packages/cli/src/config/extension-refresh-state.ts
Comment thread packages/cli/src/config/extension-runtime-reload.ts
Comment thread packages/core/src/hooks/hookRegistry.ts
Comment thread packages/cli/src/config/extension-refresh-state.ts
Comment thread packages/cli/src/config/extension-file-watcher.ts
Comment thread packages/cli/src/config/extension-file-watcher.ts
Comment thread packages/core/src/extension/extension-runtime-refresh.ts
Comment thread packages/core/src/extension/extension-runtime-refresh.ts
Comment thread packages/cli/src/config/extension-file-watcher.ts
Comment thread packages/cli/src/config/extension-file-watcher.ts
Comment thread packages/cli/src/ui/commands/reload-plugins-command.ts
Comment thread packages/core/src/extension/extensionManager.ts Outdated
Comment thread packages/cli/src/config/extension-file-watcher.test.ts
Comment thread packages/cli/src/config/extension-file-watcher.ts
Comment thread packages/cli/src/ui/hooks/slashCommandProcessor.ts
Comment thread packages/core/src/extension/extensionManager.ts
'Failed to refresh extension content: {{message}}. Run /reload-plugins to apply updates.':
'Failed to refresh extension content: {{message}}. Run /reload-plugins to apply updates.',
'Failed to refresh extension content. Run /reload-plugins to apply updates.':
'Failed to refresh extension content. Run /reload-plugins to apply updates.',

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 i18n key 'Extension reload did not complete. Run /reload-plugins to try again.' is used with t() in slashCommandProcessor.ts:623 but is missing from en.js and all other 8 locale files. All other new keys from this PR are correctly registered. Non-English users will see the raw English fallback string when a reload fails.

Suggested change
'Failed to refresh extension content. Run /reload-plugins to apply updates.',
'Failed to refresh extension content. Run /reload-plugins to apply updates.':
'Failed to refresh extension content. Run /reload-plugins to apply updates.',
'Extension reload did not complete. Run /reload-plugins to try again.':
'Extension reload did not complete. Run /reload-plugins to try again.',

— qwen3.7-max via Qwen Code /review

reloadPluginsRuntime,
} from './extension-runtime-reload.js';

describe('reloadPluginsRuntime', () => {

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] reloadPluginsRuntime has no test for the refreshTools() failure path. The success test covers refreshCache + refreshTools + reloadCommands, but no test exercises the scenario where refreshTools rejects after refreshCache succeeds — the exact partial-failure scenario that was a focus of earlier review discussion. A regression in error propagation or sequencing would go undetected.

Suggested test: mock refreshCache to resolve and refreshTools to reject. Assert that reloadCommands was NOT called, the error propagates, and getActiveExtensions was not queried for the summary.

— qwen3.7-max via Qwen Code /review

expect(reloadCommands).toHaveBeenCalledOnce();
});

it('continues refreshing content before reporting refresh failures', async () => {

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] refreshExtensionContentRuntime has no test for reloadCommands rejection inside Promise.allSettled. The existing failure test only fails refreshCache and refreshSkillCache. If someone replaces allSettled with Promise.all, the regression would go undetected — reloadCommands rejection would prevent other legs from completing.

Suggested test: mock reloadCommands to reject while other mocks succeed. Assert that refreshCache, refreshSkillCache, and refreshSubagentCache were all called, and the combined error message includes the reloadCommands failure.

— qwen3.7-max via Qwen Code /review

).toBe(true);
});

it('bootstraps on the parent when the extensions directory does not exist', () => {

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 runtime transition from main watcher to bootstrap watcher when the extensions directory is deleted mid-session (unlinkDir on extensionsDir itself) is not tested. The bootstrap test here only covers the initial setup when the directory doesn't exist at startup. If the unlinkDir handler fails to call watchExtensionsParent() or mark stale, the watcher would stop detecting new extension installations after the directory is removed and recreated.

Suggested test: start watching with the extensions directory present, fire an unlinkDir event on the extensions directory path, assert that markExtensionsChanged was called and a second watchFs call was made on the parent directory (bootstrap mode).

— qwen3.7-max via Qwen Code /review

@wenshao

wenshao commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

✅ Hands-on verification (real binary) — merge reference

I built the real qwen bundle from this PR's head (60750480, base 47f62a46) and drove the feature end-to-end. Core was resolved from the PR's own source (an esbuild alias over the workspace symlink), and I confirmed the PR code is actually in the bundle (addMutationListener, Extensions changed on disk, reload-plugins all present in dist/chunks/). This complements the existing "no blockers" review — it answers does it actually behave as described?

Verdict: yes. All three user-facing paths work in the live TUI, the author's 341 tests pass, and the suite provably bears weight under mutation. No functional regression observed.

1) Real TUI — the actual qwen binary, isolated HOME, a demo extension, edits driven on disk

real qwen TUI scenarios

# Trigger (on disk, live session) Observed in the real TUI
A add commands/farewell.md /farewell becomes resolvable inline, with no /reload-plugins (silent auto content-refresh)
B edit qwen-extension.json (config) ● Extensions changed on disk. Run /reload-plugins to apply updates. (one-shot prompt)
C run /reload-plugins ● Reloaded extensions: 1 extension · 2 commands · 1 skill · 1 agent · 0 hooks · 0 extension MCP servers · 0 extension LSP servers — the file hot-added in step A is counted

2) Tests — author's suites + a real-fs E2E + adversarial mutation

test evidence

  • 341 / 341 of the PR's own tests pass (core 195 + cli 146, 11 files).
  • Real chokidar E2E (added for this review): the shipped suite mocks chokidar + node:fs and fires synthetic events, so I wrote a harness that drives the real watcher against a real temp dir with real edits. 9/9, deterministic across 4 runscommands|skills|agents → content, hooks|qwen-extension.json|QWEN.md → stale, linked-extension source → content, .DS_Store|.swp|README.md → ignored.
  • Mutation testing — 5/5 caught: dropping skills from AUTO_REFRESH_DIRS (RED in both unit and the real E2E), isSuppressed()false, summary separator, throw hookReloadError, and dropping agent-scoped-hook preservation each turn the right suite RED and revert cleanly to green.
  • --bare → no watcher is gated in source (gemini.tsx:621, isBareMode(argv.bare) ? undefined : new ExtensionFileWatcher(...)) and covered by the bare-mode test.

Notes (non-blocking, for the record)

  • The one error path that is surfaced rather than swallowed — hook-reload failure (refreshExtensionRuntime re-throws hookReloadError) — is real and tested (mutation RED when the throw is removed). The remaining silent paths (ENOSPC, MCP startup, /hooks disable state across reload) are the author's explicitly-acknowledged out-of-scope items.
  • On macOS, FSEvents occasionally emits an incidental sibling re-stat under rapid multi-file writes. It is an OS-level watcher trait (reproduced with bare chokidar, not specific to this PR) and is absorbed by awaitWriteFinish (200 ms) + the "prompt once" idempotency in ExtensionRefreshState. Worst case is one extra /reload-plugins prompt, never a wrong classification.
  • By design, SUPPRESS_AFTER_MS = 1000 means a user file edit made within ~1 s after a programmatic enable/disable settles is intentionally ignored (anti-self-trigger). Sensible tradeoff; just noting it.

Bottom line: behavior matches the description, error surfacing on the hook path works, and the tests hold. LGTM to merge from a verification standpoint. 🚢

🇨🇳 中文完整版

✅ 真实二进制动手验证 —— 合并参考

我用本 PR head(60750480,base 47f62a46)构建了真实的 qwen bundle,端到端跑通了该特性。core 从 PR 自身源码解析(用一条 esbuild alias 覆盖工作区符号链接),并确认 PR 代码确实进了 bundle(addMutationListenerExtensions changed on diskreload-plugins 均在 dist/chunks/ 中命中)。这是对已有"no blockers" review 的补充——回答它是否真的按描述工作

结论:是。 三条用户可见路径在真实 TUI 中全部工作,作者的 341 个测试全绿,且变异测试证明测试套件真正承重。未发现功能回归。

1)真实 TUI —— 真 qwen 二进制、隔离 HOME、demo 扩展、在磁盘上真实编辑

(见上方第一张图)

# 触发(磁盘上,会话进行中) 真实 TUI 中的观察
A 新增 commands/farewell.md /farewell 无需 /reload-plugins 即内联可用(静默自动内容刷新)
B 编辑 qwen-extension.json(配置) ● Extensions changed on disk. Run /reload-plugins to apply updates.(一次性提示)
C 运行 /reload-plugins ● Reloaded extensions: 1 extension · 2 commands · 1 skill · 1 agent · 0 hooks · 0 extension MCP servers · 0 extension LSP servers——步骤 A 中热加的文件被计入

2)测试 —— 作者套件 + 真实文件系统 E2E + 对抗式变异

(见上方第二张图)

  • PR 自带测试 341 / 341 全绿(core 195 + cli 146,共 11 个文件)。
  • 真实 chokidar E2E(为本次评审新增): 现有套件把 chokidar + node:fs 都 mock 了、fire 合成事件,所以我写了一个 harness,用 watcher 对临时目录做编辑。9/9,连续 4 次确定性通过——commands|skills|agents → contenthooks|qwen-extension.json|QWEN.md → stale、软链扩展源 → content.DS_Store|.swp|README.md → 忽略
  • 变异测试——5/5 全部被捕获:AUTO_REFRESH_DIRS 删掉 skills(单测真实 E2E 同时变红)、isSuppressed()false、汇总分隔符、throw hookReloadError、以及删掉 agent 作用域 hook 的保留——每一个都让对应套件变红,还原后干净转绿。
  • --bare → 不启动 watcher 在源码中有门控(gemini.tsx:621isBareMode(argv.bare) ? undefined : new ExtensionFileWatcher(...)),并有 bare-mode 测试覆盖。

备注(非阻塞,留档)

  • 唯一被上抛而非吞掉的错误路径——hook 重载失败(refreshExtensionRuntime 重新 throw hookReloadError)——是真实且被测试的(删掉 throw 后变异变红)。其余静默路径(ENOSPC、MCP 启动、/hooks 禁用状态跨重载丢失)是作者已明确声明的超范围项。
  • 在 macOS 上,FSEvents 在快速多文件写入时偶发对 sibling 文件的连带 re-stat。这是操作系统层面的 watcher 特性(用裸 chokidar 也能复现,非本 PR 特有),被 awaitWriteFinish(200ms)+ ExtensionRefreshState 的"只提示一次"幂等吸收。最坏情况只是多一次 /reload-plugins 提示,绝不会误分类。
  • 按设计,SUPPRESS_AFTER_MS = 1000 意味着在程序化 enable/disable 结束后约 1 秒内的用户文件编辑会被有意忽略(防自触发)。合理的权衡,仅作说明。

总结: 行为与描述一致,hook 路径的错误上抛有效,测试站得住。从验证角度 LGTM,可以合并。🚢

Verified locally on macOS with a from-source qwen bundle; screenshots are faithful renders of the real captured terminal output.

@wenshao

wenshao commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓ — all required headings present, bilingual details included.

Problem: This is an observed, well-defined problem. PR #6347 links to P1 issue #3696 — extensions require a full CLI restart to take effect after install/update/config changes. Users lose context, session state, and MCP connections when they restart just to pick up a new command or skill. Clear user pain.

Direction: Aligned ✓. Extension hot-reload is a quality-of-life feature that any serious CLI tool needs. The approach — file watcher + event-driven refresh + mutation lifecycle — is the right architecture for this problem.

Size: ~1,636 production logic lines across 20 non-test files, plus ~1,760 lines of tests. This exceeds the 1,000-line advisory threshold. The scope is large but the feature is cohesive (watcher → state → reload → UI integration). Consider splitting i18n or the /reload-plugins slash command into separate PRs if future iterations happen.

⚠️ Maintainer awareness flag: This is a cross-repo fork PR touching core infrastructure (packages/core/src/extension/extensionManager.ts, packages/core/src/extension/extension-runtime-refresh.ts, packages/core/src/hooks/). 450 core production lines (under 500 hard-block threshold). Proceeds under Tier 2 evaluation.

Approach: The modular decomposition is solid — watcher, state primitive, runtime reload, slash command, UI integration. Each piece has a clear responsibility. The mutation listener pattern with paired begin/end IDs is a clean abstraction. No obvious scope bloat — everything serves the hot-reload goal.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓ — 所有必需标题齐全,包含双语详情。

问题:这是一个已观测到的、定义明确的问题。PR #6347 关联了 P1 issue #3696 — 扩展在安装/更新/配置变更后需要完全重启 CLI 才能生效。用户为了加载新命令或技能而重启时会丢失上下文、会话状态和 MCP 连接。用户痛点清晰。

方向:对齐 ✓。扩展热重载是任何成熟 CLI 工具都需要的质量提升功能。方案——文件监听 + 事件驱动刷新 + 变更生命周期——是解决此问题的正确架构。

规模:20 个非测试文件约 1,636 行生产逻辑代码,另有约 1,760 行测试代码。超过 1,000 行建议阈值。范围较大但功能内聚(监听器 → 状态 → 重载 → UI 集成)。后续迭代可考虑将 i18n 或 /reload-plugins 斜杠命令拆分为单独 PR。

⚠️ 维护者关注标记: 这是一个跨仓库 fork PR,触及核心基础设施(packages/core/src/extension/extensionManager.tspackages/core/src/extension/extension-runtime-refresh.tspackages/core/src/hooks/)。核心生产代码 450 行(低于 500 行硬限制阈值)。按 Tier 2 评估流程继续。

方案:模块化分解合理——监听器、状态原语、运行时重载、斜杠命令、UI 集成。各部分职责清晰。带配对 begin/end ID 的变更监听器模式是一个简洁的抽象。没有明显的范围膨胀——所有改动都服务于热重载目标。

进入代码审查 🔍

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

Code Review

Independent proposal: I'd use a file watcher (chokidar or native fs.watch) on extension directories, classify events by file type (config changes need full reload, content changes need lightweight refresh), debounce to avoid thrashing, and integrate with the existing extension manager's cache refresh. The UI would show a notification when stale state is detected.

The PR's approach matches this closely and goes further in good ways:

  • Mutation lifecycle (beginMutation/emitMutation with paired IDs) cleanly suppresses watcher events during self-initiated operations (install, enable, etc.). This avoids the classic "watcher fires on my own changes" trap.
  • Atomic cache replacement in refreshCache — builds nextCache separately, then assigns — prevents the partial-state window that existed in the old code where extensionCache was cleared before repopulating.
  • Generation-guarded watcher events prevent stale async callbacks from acting on outdated state.
  • Hook reload preserving agent-scoped hooks in hookRegistry.reloadConfiguredHooks() is careful and correct.
  • ExtensionRefreshState as an EventEmitter with deduplication, deferral during reload-in-progress, and post-mutation suppression windows is well-designed.

No critical blockers found. The code follows project conventions — ESM imports, strict TypeScript, proper error handling at boundaries. The reuse-before-new check passes: chokidar is already a project dependency, the EventEmitter pattern is standard.

One concern: extensionManager.test.ts has 41 failures when QWEN_HOME is set in the environment. The tests mock os.homedir() but Storage.getGlobalQwenDir() checks process.env.QWEN_HOME first. This is a pre-existing test isolation gap (not introduced by this PR), but the PR's new tests are equally affected. The test beforeEach should either delete process.env.QWEN_HOME and restore it in afterEach, or mock Storage.getGlobalQwenDir(). All 60 tests pass with QWEN_HOME= unset.

Unit Test Results

All PR-related test suites pass (run with QWEN_HOME= to avoid the pre-existing isolation issue):

Core package:

✓ extension-runtime-refresh.test.ts   6/6
✓ hookRegistry.test.ts               39/39
✓ hookSystem.test.ts                 90/90
✓ extensionManager.test.ts           60/60

CLI package:

✓ extension-file-watcher.test.ts     13/13
✓ extension-refresh-state.test.ts     9/9
✓ extension-runtime-reload.test.ts    4/4
✓ reload-plugins-command.test.ts      3/3
✓ slashCommandProcessor.test.ts      72/72
✓ BuiltinCommandLoader.test.ts       12/12
✓ gemini.test.tsx                    33/33

Total: 341 tests passed, 0 failed across all PR-related suites.

Real-Scenario Testing (tmux)

Skipped with rationale: This feature (file watcher + hot-reload) requires live filesystem mutations to extension directories while the CLI is interactive. In headless CI, reliably simulating "user installs an extension, then edits a command file, and observes the reload notification" requires a multi-step orchestration (writing extension configs, waiting for chokidar debounce, capturing the TUI notification) that is fragile and non-deterministic in tmux capture-pane. The unit tests cover the event flows, debounce logic, and UI integration comprehensively. A real-world smoke test would be more valuable on a developer's machine than in CI.

中文说明

代码审查

独立方案:使用文件监听器(chokidar 或原生 fs.watch)监听扩展目录,按文件类型分类事件(配置变更需完整重载,内容变更需轻量刷新),防抖避免抖动,并与现有扩展管理器的缓存刷新集成。UI 在检测到过期状态时显示通知。

PR 的方案与此高度一致,并在以下方面做得更好:

  • 变更生命周期beginMutation/emitMutation 配对 ID)在自身操作(安装、启用等)期间干净地抑制监听器事件,避免了经典的"监听器对自己变更触发"陷阱。
  • 原子缓存替换 refreshCache — 单独构建 nextCache,然后赋值 — 消除了旧代码中先清空再填充 extensionCache 的部分状态窗口。
  • 代数守卫的监听器事件防止过期的异步回调操作过期状态。
  • 钩子重载保留 agent 作用域钩子 hookRegistry.reloadConfiguredHooks() 实现仔细且正确。
  • ExtensionRefreshState 作为 EventEmitter 具备去重、重载进行中延迟、变更后抑制窗口等功能,设计良好。

未发现关键阻塞问题。代码遵循项目规范——ESM 导入、严格 TypeScript、边界处正确的错误处理。

一个关注点:QWEN_HOME 环境变量被设置时,extensionManager.test.ts 有 41 个测试失败。测试 mock 了 os.homedir()Storage.getGlobalQwenDir() 优先检查 process.env.QWEN_HOME。这是一个预先存在的测试隔离缺陷(非本 PR 引入),但 PR 的新测试同样受影响。所有 60 个测试在 QWEN_HOME= 未设置时通过。

单元测试结果

所有 PR 相关测试套件通过(使用 QWEN_HOME= 运行以避免预先存在的隔离问题):

核心包:extension-runtime-refresh (6/6)、hookRegistry (39/39)、hookSystem (90/90)、extensionManager (60/60)

CLI 包:extension-file-watcher (13/13)、extension-refresh-state (9/9)、extension-runtime-reload (4/4)、reload-plugins-command (3/3)、slashCommandProcessor (72/72)、BuiltinCommandLoader (12/12)、gemini (33/33)

总计:341 个测试通过,0 个失败

真实场景测试(tmux)

跳过并说明原因: 此功能(文件监听 + 热重载)需要在 CLI 交互运行时对扩展目录进行实时文件系统变更。在无头 CI 中,可靠地模拟"用户安装扩展,然后编辑命令文件,并观察重载通知"需要多步骤编排(写入扩展配置、等待 chokidar 防抖、捕获 TUI 通知),在 tmux capture-pane 中脆弱且不确定。单元测试已全面覆盖事件流、防抖逻辑和 UI 集成。

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

Reflection

This is a well-executed feature PR. The hot-reload problem is real — users lose session context when restarting the CLI for extension changes — and the solution is architecturally sound. The mutation lifecycle, atomic cache replacement, and generation-guarded watcher events show careful thought about race conditions and edge cases.

The test suite is thorough: 341 tests across 11 files covering the watcher, state machine, runtime reload, slash command, UI integration, hook reload, and extension manager mutations. All pass.

What I like:

  • The paired begin/end mutation IDs are a clean pattern that other parts of the codebase could adopt.
  • Atomic cache replacement eliminates the partial-state window — this is a genuine improvement over the old approach.
  • The ExtensionRefreshState deduplication and suppression windows show the author thought about rapid-fire filesystem events.
  • Hook reload preserving agent-scoped hooks is the right call.

What gives me pause:

  • Test isolation with QWEN_HOME. 41 test failures when the environment variable is set. This is pre-existing, not introduced by this PR, but it means CI could break depending on runner configuration. A quick fix: delete process.env.QWEN_HOME in beforeEach, restore in afterEach. Worth addressing before merge.
  • 1,636 production lines is a lot for a single PR from a fork. The code is cohesive, but the blast radius of a regression is large. A maintainer who knows the extension system intimately should review this.
  • No real-scenario tmux test. The unit tests are comprehensive, but I couldn't verify the actual TUI behavior (notification appearing, /reload-plugins working in a live session). The feature works in theory; I'd feel better seeing it work in practice.

Verdict: Escalating to maintainer. The PR is technically strong and the feature is needed, but the combination of cross-repo fork, core infrastructure changes, and 1,600+ production lines means this needs a human maintainer's architectural sign-off. The existing CHANGES_REQUESTED reviews already gate this PR. I'm updating the triage comments with fresh analysis — the maintainer can dismiss the prior reviews if they agree with this assessment.

中文说明

反思

这是一个执行良好的功能 PR。热重载问题是真实的——用户为扩展变更重启 CLI 时会丢失会话上下文——解决方案在架构上是合理的。变更生命周期、原子缓存替换和代数守卫的监听器事件体现了对竞态条件和边缘情况的仔细思考。

测试套件全面:11 个文件中 341 个测试覆盖了监听器、状态机、运行时重载、斜杠命令、UI 集成、钩子重载和扩展管理器变更。全部通过。

优点:

  • 配对的 begin/end 变更 ID 是一个简洁的模式,代码库其他部分可以借鉴。
  • 原子缓存替换消除了部分状态窗口——这是对旧方案的真正改进。
  • ExtensionRefreshState 的去重和抑制窗口表明作者考虑了快速连续的文件系统事件。
  • 钩子重载保留 agent 作用域钩子是正确的选择。

顾虑:

  • QWEN_HOME 环境变量下的测试隔离问题。 设置该变量时 41 个测试失败。这是预先存在的问题,非本 PR 引入,但意味着 CI 可能因 runner 配置不同而中断。快速修复:在 beforeEachdelete process.env.QWEN_HOME,在 afterEach 中恢复。建议在合并前修复。
  • 1,636 行生产代码 对于来自 fork 的单个 PR 来说数量较大。代码是内聚的,但回归的影响范围较大。熟悉扩展系统的维护者应该审查此 PR。
  • 缺少真实场景 tmux 测试。 单元测试很全面,但我无法验证实际的 TUI 行为(通知出现、/reload-plugins 在活跃会话中工作)。该功能在理论上是可行的;看到它在实践中工作会让我更放心。

判定: 转交维护者。PR 技术上很强且功能是需要的,但跨仓库 fork、核心基础设施变更和 1,600+ 行生产代码的组合意味着需要人类维护者的架构签核。现有的 CHANGES_REQUESTED 审查已经限制了此 PR。我正在用新分析更新 triage 评论——如果维护者同意此评估,可以撤销之前的审查。

Qwen Code · qwen3.7-max

@wenshao

wenshao commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants