From bb28cd1faaaeb81a73dac96c076ce723de0919c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8F=B6=E5=85=AC?= Date: Mon, 7 Sep 2026 15:44:48 +0800 Subject: [PATCH 1/8] feat(daemon): enumerate installed extension skills locally --- docs/design/daemon-extension-skill-catalog.md | 28 ++ .../src/serve/workspace-skills-status.test.ts | 290 +++++++++++++++++- .../cli/src/serve/workspace-skills-status.ts | 94 +++++- 3 files changed, 382 insertions(+), 30 deletions(-) create mode 100644 docs/design/daemon-extension-skill-catalog.md diff --git a/docs/design/daemon-extension-skill-catalog.md b/docs/design/daemon-extension-skill-catalog.md new file mode 100644 index 00000000000..96def807531 --- /dev/null +++ b/docs/design/daemon-extension-skill-catalog.md @@ -0,0 +1,28 @@ +# Daemon extension Skill catalog + +This implements stage 2 of #11274. The daemon-local workspace Skill provider +currently supplies an empty active-extension list, so its first response omits +installed extension Skills when no child snapshot exists. + +Use an unbound `ExtensionManager` for the selected workspace to load installed +extensions through the existing consistent store reader. Supply active +extensions to `SkillManager`, preserving project > user > extension > bundled +precedence. Append inactive extension Skills as management entries with the +existing `inactive_extension` status, retaining their identity and metadata. +Resolve settings and extension Skill defaults/overrides with the existing +parsers. A settings opt-in does not enable an inactive parent extension. + +Keep the lightweight Config surface: do not construct a runtime Config, start a +child, initialize MCP, execute hooks, or install watchers. Honor safe mode, +disabled discovery levels and workspace trust; inert untrusted inventory must +not load workspace settings or extension runtime context. Directory failures +continue to return an uninitialized error status. + +The implementation and collocated regressions live in the daemon-local provider. +Tests cover real manifests, active/inactive state, source collisions, persisted +Skill settings, safe/untrusted contexts and explicit cache invalidation. E2E +evidence uses an isolated home and a daemon with no child session. + +The facade still prefers child snapshots in this stage. Replacing that source, +changing toggle/refresh semantics, adding configured-state fields and changing +Web Shell projections belong to later PRs. No public schema changes are needed. diff --git a/packages/cli/src/serve/workspace-skills-status.test.ts b/packages/cli/src/serve/workspace-skills-status.test.ts index ba4e2bb1e28..74dcda37970 100644 --- a/packages/cli/src/serve/workspace-skills-status.test.ts +++ b/packages/cli/src/serve/workspace-skills-status.test.ts @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { describe, it, expect, vi, afterEach } from 'vitest'; +import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest'; import * as fsp from 'node:fs/promises'; import * as os from 'node:os'; import * as path from 'node:path'; @@ -14,7 +14,11 @@ vi.mock('../utils/stdioHelpers.js', () => ({ writeStderrLine: mockWriteStderrLine, })); -import { SkillManager } from '@qwen-code/qwen-code-core'; +import { + ExtensionManager, + ExtensionStore, + SkillManager, +} from '@qwen-code/qwen-code-core'; import { ENV_CORRUPTED_PATH, ENV_WAS_RECOVERED, @@ -23,7 +27,14 @@ import { import { createWorkspaceSkillsStatusProvider } from './workspace-skills-status.js'; describe('createWorkspaceSkillsStatusProvider', () => { - afterEach(() => { + let qwenHome: string; + beforeEach(async () => { + qwenHome = await fsp.mkdtemp(path.join(os.tmpdir(), 'qwen-catalog-home-')); + vi.stubEnv('QWEN_HOME', qwenHome); + }); + afterEach(async () => { + vi.unstubAllEnvs(); + await fsp.rm(qwenHome, { recursive: true, force: true }); vi.restoreAllMocks(); mockWriteStderrLine.mockClear(); }); @@ -137,17 +148,17 @@ describe('createWorkspaceSkillsStatusProvider', () => { const status = await provider(workspace); expect(status.skills).toMatchObject([ - { - name: 'enabled', - status: 'ok', - installedPath: '/skills/enabled/SKILL.md', - }, { name: 'disabled', status: 'disabled', disabledReason: 'default', installedPath: '/skills/disabled/SKILL.md', }, + { + name: 'enabled', + status: 'ok', + installedPath: '/skills/enabled/SKILL.md', + }, ]); }); @@ -185,17 +196,17 @@ describe('createWorkspaceSkillsStatusProvider', () => { const status = await provider(workspace); expect(status.skills).toMatchObject([ - { - name: 'enabled', - status: 'ok', - installedPath: '/skills/enabled/SKILL.md', - }, { name: 'disabled', status: 'disabled', disabledReason: 'hard', installedPath: '/skills/disabled/SKILL.md', }, + { + name: 'enabled', + status: 'ok', + installedPath: '/skills/enabled/SKILL.md', + }, ]); // A workspace-scope hard disable is not locked by a higher scope. const hardDisabled = status.skills.find((s) => s.name === 'disabled'); @@ -411,4 +422,257 @@ describe('createWorkspaceSkillsStatusProvider', () => { expect(listSpy).toHaveBeenCalledTimes(2); expect(listSpy.mock.instances[0]).toBe(listSpy.mock.instances[1]); }); + + async function writeExtension( + name: string, + skillNames: string[], + skillStates: Record = {}, + ) { + const directory = path.join(qwenHome, 'extensions', name); + await fsp.mkdir(directory, { recursive: true }); + await fsp.writeFile( + path.join(directory, 'qwen-extension.json'), + JSON.stringify({ + name, + version: '1.0.0', + displayName: `${name} display`, + skillStates, + mcpServers: { sentinel: { command: 'must-not-execute' } }, + }), + ); + for (const skill of skillNames) { + const skillDir = path.join(directory, 'skills', skill); + await fsp.mkdir(skillDir, { recursive: true }); + await fsp.writeFile( + path.join(skillDir, 'SKILL.md'), + `---\nname: ${skill}\ndescription: ${skill} description\nargument-hint: \nuser-invocable: false\n---\nInstructions`, + ); + } + return directory; + } + + it('lists active and inactive extension Skills without a runtime Config', async () => { + const active = await writeExtension('active', ['active-skill']); + await writeExtension('inactive', ['inactive-skill']); + await fsp.writeFile( + path.join(qwenHome, 'extensions', 'extension-enablement.json'), + JSON.stringify({ inactive: { overrides: ['!*'] } }), + ); + const refreshRuntime = vi.spyOn(ExtensionManager.prototype, 'refreshTools'); + const status = await createWorkspaceSkillsStatusProvider()(qwenHome); + expect(status.initialized).toBe(true); + expect(status.skills).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + name: 'active-skill', + status: 'ok', + level: 'extension', + extensionName: 'active', + extensionDisplayName: 'active display', + installedPath: path.join( + active, + 'skills', + 'active-skill', + 'SKILL.md', + ), + argumentHint: '', + userInvocable: false, + }), + expect.objectContaining({ + name: 'inactive-skill', + status: 'disabled', + disabledReason: 'inactive_extension', + extensionName: 'inactive', + }), + ]), + ); + expect(refreshRuntime).not.toHaveBeenCalled(); + }); + + it('preserves project precedence and appends same-name inactive sources', async () => { + await writeExtension('active', ['shared']); + await writeExtension('inactive', ['shared']); + await fsp.writeFile( + path.join(qwenHome, 'extensions', 'extension-enablement.json'), + JSON.stringify({ inactive: { overrides: ['!*'] } }), + ); + const projectSkill = path.join(qwenHome, '.qwen', 'skills', 'shared'); + await fsp.mkdir(projectSkill, { recursive: true }); + await fsp.writeFile( + path.join(projectSkill, 'SKILL.md'), + '---\nname: shared\ndescription: Project wins\n---\nBody', + ); + const status = await createWorkspaceSkillsStatusProvider()(qwenHome); + expect(status.skills.filter((s) => s.name === 'shared')).toMatchObject([ + { level: 'project', status: 'ok' }, + { + level: 'extension', + extensionName: 'inactive', + disabledReason: 'inactive_extension', + }, + ]); + }); + + it('uses persisted workspace activation and Skill overrides from the store', async () => { + await writeExtension('suite', ['blocked', 'opt-in', 'overridden'], { + blocked: false, + 'opt-in': false, + overridden: false, + }); + const workspace = path.join(qwenHome, 'workspace'); + const other = path.join(qwenHome, 'other'); + await fsp.mkdir(path.join(workspace, '.qwen'), { recursive: true }); + await fsp.writeFile( + path.join(workspace, '.qwen', 'settings.json'), + JSON.stringify({ + skills: { enabled: ['OPT-IN'], disabled: ['blocked'] }, + }), + ); + const manager = new ExtensionManager({ + workspaceDir: workspace, + isWorkspaceTrusted: true, + }); + await manager.refreshCache(); + const extension = manager.getLoadedExtensions()[0]!; + const store = new ExtensionStore(); + await store.setWorkspaceActivation(extension, other, 'disabled'); + await store.setSkillWorkspaceOverrides( + extension, + workspace, + { overridden: true }, + 0, + ); + const provider = createWorkspaceSkillsStatusProvider(); + const status = await provider(workspace); + expect( + status.skills.filter((s) => s.extensionName === 'suite'), + ).toMatchObject([ + { name: 'blocked', disabledReason: 'hard' }, + { name: 'opt-in', status: 'ok' }, + { name: 'overridden', status: 'ok' }, + ]); + const otherStatus = await provider(other); + expect( + otherStatus.skills.filter((s) => s.extensionName === 'suite'), + ).toHaveLength(3); + expect( + otherStatus.skills + .filter((s) => s.extensionName === 'suite') + .every((s) => s.disabledReason === 'inactive_extension'), + ).toBe(true); + }); + + it('maps manifest defaults, reloads settings, and rebuilds after invalidation', async () => { + await writeExtension('suite', ['default-off'], { 'default-off': false }); + const workspace = path.join(qwenHome, 'workspace'); + await fsp.mkdir(path.join(workspace, '.qwen'), { recursive: true }); + const provider = createWorkspaceSkillsStatusProvider(); + const readSkill = async () => + (await provider(workspace)).skills.find((s) => s.name === 'default-off'); + expect(await readSkill()).toMatchObject({ + status: 'disabled', + disabledReason: 'default', + }); + await fsp.writeFile( + path.join(workspace, '.qwen', 'settings.json'), + JSON.stringify({ skills: { enabled: ['default-off'] } }), + ); + expect(await readSkill()).toMatchObject({ status: 'ok' }); + await fsp.rm(path.join(qwenHome, 'extensions', 'suite'), { + recursive: true, + }); + provider.invalidate?.(workspace); + expect(await readSkill()).toBeUndefined(); + }); + + it.each(['safe', 'untrusted', 'inert-untrusted', 'disabled-level'] as const)( + 'does not load extensions in %s mode', + async (mode) => { + await writeExtension('suite', ['hidden']); + if (mode === 'safe') vi.stubEnv('QWEN_CODE_SAFE_MODE', '1'); + if (mode === 'disabled-level') { + await fsp.mkdir(path.join(qwenHome, '.qwen'), { recursive: true }); + await fsp.writeFile( + path.join(qwenHome, '.qwen', 'settings.json'), + JSON.stringify({ skills: { disabledLevels: ['extension'] } }), + ); + } + const refresh = vi.spyOn(ExtensionManager.prototype, 'refreshCache'); + const status = await createWorkspaceSkillsStatusProvider({ + workspaceTrusted: mode !== 'untrusted' && mode !== 'inert-untrusted', + includeUntrustedSkills: mode === 'inert-untrusted', + })(qwenHome); + expect(status.initialized).toBe(true); + expect(status.skills.some((s) => s.level === 'extension')).toBe(false); + expect(refresh).not.toHaveBeenCalled(); + }, + ); + + it('returns an error for an unreadable extension directory', async () => { + await fsp.writeFile(path.join(qwenHome, 'extensions'), 'not a directory'); + const status = await createWorkspaceSkillsStatusProvider()(qwenHome); + expect(status).toMatchObject({ + initialized: false, + skills: [], + errors: [{ kind: 'skills', status: 'error' }], + }); + }); + + it('loads linked extensions and Agent Plugin manifests through the shared loader', async () => { + const source = await writeExtension('linked', ['linked-skill']); + const relocated = path.join(qwenHome, 'linked-source'); + await fsp.rename(source, relocated); + await fsp.mkdir(source); + await fsp.writeFile( + path.join(source, '.qwen-extension-install.json'), + JSON.stringify({ type: 'link', source: relocated }), + ); + const plugin = await writeExtension('portable', ['portable-skill']); + await fsp.rm(path.join(plugin, 'qwen-extension.json')); + await fsp.writeFile( + path.join(plugin, 'plugin.json'), + JSON.stringify({ + $schema: 'https://agent-plugins.org/schemas/1.0.0/plugin.schema.json', + name: 'portable', + version: '1.0.0', + }), + ); + const status = await createWorkspaceSkillsStatusProvider()(qwenHome); + expect(status.initialized).toBe(true); + expect(status.skills).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + name: 'linked-skill', + extensionName: 'linked', + installedPath: path.join( + relocated, + 'skills', + 'linked-skill', + 'SKILL.md', + ), + }), + expect.objectContaining({ + name: 'portable-skill', + extensionName: 'portable', + }), + ]), + ); + }); + + it('does not cache a failed store read as an initialized empty catalog', async () => { + await writeExtension('suite', ['visible']); + vi.spyOn(ExtensionManager.prototype, 'refreshCache').mockRejectedValueOnce( + new Error('store unavailable'), + ); + const provider = createWorkspaceSkillsStatusProvider(); + expect(await provider(qwenHome)).toMatchObject({ + initialized: false, + errors: [{ kind: 'skills', error: 'store unavailable' }], + }); + expect((await provider(qwenHome)).skills).toEqual( + expect.arrayContaining([ + expect.objectContaining({ name: 'visible', status: 'ok' }), + ]), + ); + }); }); diff --git a/packages/cli/src/serve/workspace-skills-status.ts b/packages/cli/src/serve/workspace-skills-status.ts index d677554465a..fbe1769a0a3 100644 --- a/packages/cli/src/serve/workspace-skills-status.ts +++ b/packages/cli/src/serve/workspace-skills-status.ts @@ -25,12 +25,17 @@ * shim is sufficient — no full `Config` construction (and no `initialize()` * side effects) required. The live child, when present, stays authoritative: * the facade only falls back here after a real child answer and the cached - * last answer are both unavailable, and this daemon-local view intentionally - * omits extension-provided skills (there is no active-extension context - * outside the child) — those still surface once a session exists. + * last answer are both unavailable. This daemon-local view includes installed + * extension Skills using the persistent extension store without binding a + * runtime Config to the ExtensionManager. */ -import { SkillManager, isSafeModeEnv } from '@qwen-code/qwen-code-core'; +import { + ExtensionManager, + SkillManager, + Storage, + isSafeModeEnv, +} from '@qwen-code/qwen-code-core'; import type { Config, SkillLevel } from '@qwen-code/qwen-code-core'; import type { ServeWorkspaceSkillsStatus } from '@qwen-code/acp-bridge/status'; import { STATUS_SCHEMA_VERSION } from '@qwen-code/acp-bridge/status'; @@ -76,6 +81,11 @@ type SkillManagerConfigShim = Pick< | 'getDisabledSkillLevels' >; +interface WorkspaceSkillManagers { + skillManager: SkillManager; + extensionManager?: ExtensionManager; +} + export function createWorkspaceSkillsStatusProvider( options: WorkspaceSkillsStatusProviderOptions = {}, ): WorkspaceSkillsStatusProvider { @@ -84,7 +94,7 @@ export function createWorkspaceSkillsStatusProvider( // globs for) every level on each call. This is a best-effort pre-child // fallback, so slight staleness between explicit invalidation points is // acceptable: the live child re-lists authoritatively once a session exists. - const managers = new Map(); + const managers = new Map(); const provider = ((workspaceCwd: string) => buildWorkspaceSkillsStatus( workspaceCwd, @@ -98,7 +108,7 @@ export function createWorkspaceSkillsStatusProvider( async function buildWorkspaceSkillsStatus( workspaceCwd: string, - managers: Map, + managers: Map, workspaceTrusted: boolean, includeUntrustedSkills: boolean, ): Promise { @@ -109,8 +119,8 @@ async function buildWorkspaceSkillsStatus( skipWorkspaceSettings: !workspaceTrusted, workspaceTrusted, }); - let skillManager = managers.get(workspaceCwd); - if (!skillManager) { + let cached = managers.get(workspaceCwd); + if (!cached) { // Mirror the CLI guard in loadCliConfig: safe mode nullifies // disabledSkillLevels so the child session loads all bundled skills. const rawLevels = @@ -127,6 +137,19 @@ async function buildWorkspaceSkillsStatus( ); const safeMode = (!workspaceTrusted && !includeUntrustedSkills) || isSafeModeEnv(); + let extensionManager: ExtensionManager | undefined; + if (workspaceTrusted && !safeMode && !disabledLevels.has('extension')) { + try { + await fs.readdir(Storage.getUserExtensionsDir()); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; + } + extensionManager = new ExtensionManager({ + workspaceDir: workspaceCwd, + isWorkspaceTrusted: workspaceTrusted, + }); + await extensionManager.refreshCache(); + } const shim: SkillManagerConfigShim = { // Honor the safe-mode env the same way `Config` does when no explicit // flag is passed, so an operator running in safe mode gets the same @@ -136,12 +159,12 @@ async function buildWorkspaceSkillsStatus( // bare, so it is always off here. getBareMode: () => false, getProjectRoot: () => workspaceCwd, - // Extension skills need active-extension context that only the child - // has; omit them here and let the session snapshot surface them. - getActiveExtensions: () => [], + getActiveExtensions: () => + extensionManager?.getLoadedExtensions().filter((e) => e.isActive) ?? + [], getDisabledSkillLevels: () => disabledLevels, }; - skillManager = new SkillManager(shim as Config); + const skillManager = new SkillManager(shim as Config); if (!safeMode) { for (const level of ['project', 'user'] as const) { if (disabledLevels.has(level)) continue; @@ -156,17 +179,54 @@ async function buildWorkspaceSkillsStatus( } } } - managers.set(workspaceCwd, skillManager); + cached = { skillManager, extensionManager }; + managers.set(workspaceCwd, cached); } - const disablements = resolveSkillSettings(settings).disablements; + const { disablements, enabledNames } = resolveSkillSettings(settings); + const { skillManager, extensionManager } = cached; + const extensions = extensionManager?.getLoadedExtensions() ?? []; const skills = await skillManager.listSkills(); + const statuses = skills.map((skill) => { + const extension = + skill.level === 'extension' + ? extensions.find((e) => e.name === skill.extensionName) + : undefined; + const state = + extensionManager && extension + ? extensionManager.getExtensionSkillState(extension.id, skill.name) + : undefined; + return mapSkillConfigToStatus(skill, disablements, { + enabled: + !state || + enabledNames.has(skill.name.trim().toLowerCase()) || + (state.workspaceEnabled ?? state.defaultEnabled), + }); + }); + for (const extension of extensions) { + if (extension.isActive) continue; + const seenNames = new Set(); + for (const skill of extension.skills ?? []) { + if (seenNames.has(skill.name)) continue; + seenNames.add(skill.name); + statuses.push( + mapSkillConfigToStatus( + { + ...skill, + level: 'extension', + extensionName: extension.name, + extensionDisplayName: extension.displayName, + }, + disablements, + { disabled: true }, + ), + ); + } + } return { v: STATUS_SCHEMA_VERSION, workspaceCwd, initialized: true, - skills: skills.map((skill) => - mapSkillConfigToStatus(skill, disablements), - ), + skills: statuses.sort((a, b) => a.name.localeCompare(b.name)), }; } catch (error) { const message = error instanceof Error ? error.message : String(error); From d90e39c50a030167b583e29aca39aca87178f843 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8F=B6=E5=85=AC?= Date: Mon, 7 Sep 2026 18:26:18 +0800 Subject: [PATCH 2/8] fix(daemon): resolve local extension skill display names by locale --- docs/design/daemon-extension-skill-catalog.md | 2 + packages/cli/src/i18n/index.ts | 4 +- .../src/serve/workspace-skills-status.test.ts | 47 +++++++++++++++++++ .../cli/src/serve/workspace-skills-status.ts | 6 +++ 4 files changed, 58 insertions(+), 1 deletion(-) diff --git a/docs/design/daemon-extension-skill-catalog.md b/docs/design/daemon-extension-skill-catalog.md index 96def807531..c083e86c03a 100644 --- a/docs/design/daemon-extension-skill-catalog.md +++ b/docs/design/daemon-extension-skill-catalog.md @@ -11,6 +11,8 @@ precedence. Append inactive extension Skills as management entries with the existing `inactive_extension` status, retaining their identity and metadata. Resolve settings and extension Skill defaults/overrides with the existing parsers. A settings opt-in does not enable an inactive parent extension. +Resolve localized extension names with the existing language setting and locale +helpers, without changing the daemon process language. Keep the lightweight Config surface: do not construct a runtime Config, start a child, initialize MCP, execute hooks, or install watchers. Honor safe mode, diff --git a/packages/cli/src/i18n/index.ts b/packages/cli/src/i18n/index.ts index 7bfaca79f85..1b853a6fd74 100644 --- a/packages/cli/src/i18n/index.ts +++ b/packages/cli/src/i18n/index.ts @@ -223,7 +223,9 @@ function interpolate( } // Language setting helpers -function resolveLanguage(lang: SupportedLanguage | 'auto'): SupportedLanguage { +export function resolveLanguage( + lang: SupportedLanguage | 'auto', +): SupportedLanguage { if (lang === 'auto') { return detectSystemLanguage(); } diff --git a/packages/cli/src/serve/workspace-skills-status.test.ts b/packages/cli/src/serve/workspace-skills-status.test.ts index 74dcda37970..55273689b90 100644 --- a/packages/cli/src/serve/workspace-skills-status.test.ts +++ b/packages/cli/src/serve/workspace-skills-status.test.ts @@ -675,4 +675,51 @@ describe('createWorkspaceSkillsStatusProvider', () => { ]), ); }); + + it.each([ + { language: 'zh', envLanguage: '', expected: '扩展' }, + { language: 'zh-CN', envLanguage: '', expected: '扩展' }, + { language: 'en', envLanguage: 'zh', expected: '扩展' }, + { language: 'auto', envLanguage: '', expected: '扩展' }, + { language: 'en', envLanguage: '', expected: 'Extension' }, + ])( + 'resolves extension names for $language with env $envLanguage', + async ({ language, envLanguage, expected }) => { + vi.stubEnv('QWEN_CODE_LANG', envLanguage); + vi.stubEnv('LANG', 'zh_CN.UTF-8'); + for (const name of ['active', 'inactive']) { + const directory = await writeExtension(name, [`${name}-skill`]); + const manifestPath = path.join(directory, 'qwen-extension.json'); + const manifest = JSON.parse(await fsp.readFile(manifestPath, 'utf8')); + await fsp.writeFile( + manifestPath, + JSON.stringify({ + ...manifest, + displayName: { en: 'Extension', zh: '扩展' }, + }), + ); + } + await fsp.writeFile( + path.join(qwenHome, 'extensions', 'extension-enablement.json'), + JSON.stringify({ inactive: { overrides: ['!*'] } }), + ); + await fsp.mkdir(path.join(qwenHome, '.qwen'), { recursive: true }); + await fsp.writeFile( + path.join(qwenHome, '.qwen', 'settings.json'), + JSON.stringify({ general: { language } }), + ); + const status = await createWorkspaceSkillsStatusProvider()(qwenHome); + expect(status.initialized).toBe(true); + expect( + status.skills.filter((s) => s.level === 'extension'), + ).toMatchObject([ + { name: 'active-skill', extensionDisplayName: expected, status: 'ok' }, + { + name: 'inactive-skill', + extensionDisplayName: expected, + disabledReason: 'inactive_extension', + }, + ]); + }, + ); }); diff --git a/packages/cli/src/serve/workspace-skills-status.ts b/packages/cli/src/serve/workspace-skills-status.ts index fbe1769a0a3..54aa6ac8b3d 100644 --- a/packages/cli/src/serve/workspace-skills-status.ts +++ b/packages/cli/src/serve/workspace-skills-status.ts @@ -41,6 +41,7 @@ import type { ServeWorkspaceSkillsStatus } from '@qwen-code/acp-bridge/status'; import { STATUS_SCHEMA_VERSION } from '@qwen-code/acp-bridge/status'; import * as fs from 'node:fs/promises'; import { loadSettings } from '../config/settings.js'; +import { resolveLanguage, resolveLanguageSetting } from '../i18n/index.js'; import { writeStderrLine } from '../utils/stdioHelpers.js'; import { mapSkillConfigToStatus } from '../runtime/workspace-skills-mapping.js'; import { resolveSkillSettings } from '../config/skill-settings.js'; @@ -147,6 +148,11 @@ async function buildWorkspaceSkillsStatus( extensionManager = new ExtensionManager({ workspaceDir: workspaceCwd, isWorkspaceTrusted: workspaceTrusted, + locale: resolveLanguage( + resolveLanguageSetting( + settings.merged.general?.language as string | undefined, + ), + ), }); await extensionManager.refreshCache(); } From 813893bb261fef1c2a9b3d62057373a6d19fc18e Mon Sep 17 00:00:00 2001 From: qwen-code-bot Date: Mon, 7 Sep 2026 19:42:52 +0000 Subject: [PATCH 3/8] fix(daemon): harden local skills catalog cache and failure domains (#11281) - Give the extension load its own failure domain: a fault inside the store load degrades only extension entries (logged, not cached, so the next read retries), while an unreadable extensions root keeps the documented all-or-nothing error status. - Resolve the extension locale on every call and key the cache on it, so a language change rebuilds instead of serving a frozen locale; guard a non-string general.language from throwing in resolution. - Guard cache installs with a per-workspace invalidation epoch so an invalidate() delivered mid-build cannot be undone by that build, and coalesce concurrent cold builds. - Gate skills.disabledLevels at discovery only; inactive-extension management entries still appear, matching the child producer. - Pin the readdir guard with an error-identity assertion and a searchable-but-unlistable root case; extract assertReadableDir. Co-authored-by: Qwen-Coder --- docs/design/daemon-extension-skill-catalog.md | 51 +++- .../src/serve/workspace-skills-status.test.ts | 244 ++++++++++++++++-- .../cli/src/serve/workspace-skills-status.ts | 130 ++++++++-- 3 files changed, 371 insertions(+), 54 deletions(-) diff --git a/docs/design/daemon-extension-skill-catalog.md b/docs/design/daemon-extension-skill-catalog.md index c083e86c03a..6345ab6a4ba 100644 --- a/docs/design/daemon-extension-skill-catalog.md +++ b/docs/design/daemon-extension-skill-catalog.md @@ -12,19 +12,60 @@ existing `inactive_extension` status, retaining their identity and metadata. Resolve settings and extension Skill defaults/overrides with the existing parsers. A settings opt-in does not enable an inactive parent extension. Resolve localized extension names with the existing language setting and locale -helpers, without changing the daemon process language. +helpers, without changing the daemon process language. The resolved locale is +part of the provider cache key — recomputed from freshly loaded settings on +every call — because a language change reaches no invalidation point. Keep the lightweight Config surface: do not construct a runtime Config, start a child, initialize MCP, execute hooks, or install watchers. Honor safe mode, disabled discovery levels and workspace trust; inert untrusted inventory must -not load workspace settings or extension runtime context. Directory failures -continue to return an uninitialized error status. +not load workspace settings or extension runtime context. +`skills.disabledLevels` gates extension _discovery_ only; inactive-extension +management entries are appended regardless, matching the child producer. + +Failure handling is two-tier. An unreadable extensions _root_ (or a configured +skills base directory) keeps the documented all-or-nothing behavior: an +uninitialized error status. The explicit `readdir` probe is what surfaces that +state — the store loader swallows listing errors, so without the probe an +unlistable root would silently yield an initialized catalog missing every +extension Skill. A fault _inside_ the extension load itself (corrupt store +state, a dangling extension symlink, lock contention) degrades only the +extension entries instead: the catalog is served initialized with the project, +user and bundled Skills, the failure is logged to the daemon's stderr, and the +degraded build is not cached so the next read retries extension enumeration. + +Cache installs are guarded by a per-workspace invalidation epoch, so an +invalidation delivered while a cold build is in flight cannot be undone by +that build, and concurrent cold builds of one workspace are coalesced. Startup +`--enable-extension` overrides are not propagated here (the daemon's child +spawn generally does not carry them either). The implementation and collocated regressions live in the daemon-local provider. Tests cover real manifests, active/inactive state, source collisions, persisted -Skill settings, safe/untrusted contexts and explicit cache invalidation. E2E -evidence uses an isolated home and a daemon with no child session. +Skill settings, safe/untrusted contexts, explicit and mid-build cache +invalidation, language changes, the extension-load failure domain, unreadable +directory roots and discovery-level gating. E2E evidence uses an isolated home +and a daemon with no child session. The facade still prefers child snapshots in this stage. Replacing that source, changing toggle/refresh semantics, adding configured-state fields and changing Web Shell projections belong to later PRs. No public schema changes are needed. + +Known later-stage items (recorded during review, deliberately not in this +stage): + +- The sibling `/workspace/extensions` route resolves its locale through a + settings load that admits the workspace `.env` into the daemon's + process-global environment, so a workspace `QWEN_CODE_LANG` can diverge the + two panels of one page. This provider follows the documented daemon + convention (`skipLoadEnvironment`); fixing the leak belongs to that route. +- The inactive-append and sort assembly is a second copy of the child's + (`acpAgent.ts`); extracting it into the shared + `runtime/workspace-skills-mapping.ts` module — keeping the child's + `level:extensionName:name` dedupe key — belongs to a follow-up. +- Retaining the cached `ExtensionManager` across skill-settings invalidations + behind `refreshCacheIfSourcesChanged` needs invalidation provenance + (extension-store mutation vs skill-settings mutation) that the current + call sites do not carry. +- The active-Skill `enabled` judgment mirrors `Config.isSkillEnabled`; + sharing that decision with core config belongs to a follow-up. diff --git a/packages/cli/src/serve/workspace-skills-status.test.ts b/packages/cli/src/serve/workspace-skills-status.test.ts index 55273689b90..7847b9c1f29 100644 --- a/packages/cli/src/serve/workspace-skills-status.test.ts +++ b/packages/cli/src/serve/workspace-skills-status.test.ts @@ -412,6 +412,7 @@ describe('createWorkspaceSkillsStatusProvider', () => { it('reuses one SkillManager per workspace across calls', async () => { const listSpy = vi.spyOn(SkillManager.prototype, 'listSkills'); + const refreshSpy = vi.spyOn(ExtensionManager.prototype, 'refreshCache'); const provider = createWorkspaceSkillsStatusProvider(); await provider('/ws'); @@ -421,6 +422,20 @@ describe('createWorkspaceSkillsStatusProvider', () => { // listSkills is invoked on the same object rather than a freshly-scanned one. expect(listSpy).toHaveBeenCalledTimes(2); expect(listSpy.mock.instances[0]).toBe(listSpy.mock.instances[1]); + // The cached ExtensionManager is reused as well: the extension store is + // read once, not on every request. + expect(refreshSpy).toHaveBeenCalledTimes(1); + }); + + it('coalesces concurrent cold builds of the same workspace', async () => { + await writeExtension('suite', ['visible']); + const refreshSpy = vi.spyOn(ExtensionManager.prototype, 'refreshCache'); + const provider = createWorkspaceSkillsStatusProvider(); + + const [a, b] = await Promise.all([provider(qwenHome), provider(qwenHome)]); + + expect(a).toBe(b); + expect(refreshSpy).toHaveBeenCalledTimes(1); }); async function writeExtension( @@ -458,6 +473,13 @@ describe('createWorkspaceSkillsStatusProvider', () => { path.join(qwenHome, 'extensions', 'extension-enablement.json'), JSON.stringify({ inactive: { overrides: ['!*'] } }), ); + // A settings opt-in must not enable a Skill whose parent extension is + // inactive. + await fsp.mkdir(path.join(qwenHome, '.qwen'), { recursive: true }); + await fsp.writeFile( + path.join(qwenHome, '.qwen', 'settings.json'), + JSON.stringify({ skills: { enabled: ['inactive-skill'] } }), + ); const refreshRuntime = vi.spyOn(ExtensionManager.prototype, 'refreshTools'); const status = await createWorkspaceSkillsStatusProvider()(qwenHome); expect(status.initialized).toBe(true); @@ -514,11 +536,15 @@ describe('createWorkspaceSkillsStatusProvider', () => { }); it('uses persisted workspace activation and Skill overrides from the store', async () => { - await writeExtension('suite', ['blocked', 'opt-in', 'overridden'], { - blocked: false, - 'opt-in': false, - overridden: false, - }); + await writeExtension( + 'suite', + ['blocked', 'opt-in', 'overridden', 'default-on'], + { + blocked: false, + 'opt-in': false, + overridden: false, + }, + ); const workspace = path.join(qwenHome, 'workspace'); const other = path.join(qwenHome, 'other'); await fsp.mkdir(path.join(workspace, '.qwen'), { recursive: true }); @@ -539,7 +565,7 @@ describe('createWorkspaceSkillsStatusProvider', () => { await store.setSkillWorkspaceOverrides( extension, workspace, - { overridden: true }, + { overridden: true, 'default-on': false }, 0, ); const provider = createWorkspaceSkillsStatusProvider(); @@ -548,13 +574,15 @@ describe('createWorkspaceSkillsStatusProvider', () => { status.skills.filter((s) => s.extensionName === 'suite'), ).toMatchObject([ { name: 'blocked', disabledReason: 'hard' }, + // Manifest default is ON, but the workspace override turns it off. + { name: 'default-on', status: 'disabled', disabledReason: 'default' }, { name: 'opt-in', status: 'ok' }, { name: 'overridden', status: 'ok' }, ]); const otherStatus = await provider(other); expect( otherStatus.skills.filter((s) => s.extensionName === 'suite'), - ).toHaveLength(3); + ).toHaveLength(4); expect( otherStatus.skills .filter((s) => s.extensionName === 'suite') @@ -585,18 +613,11 @@ describe('createWorkspaceSkillsStatusProvider', () => { expect(await readSkill()).toBeUndefined(); }); - it.each(['safe', 'untrusted', 'inert-untrusted', 'disabled-level'] as const)( + it.each(['safe', 'untrusted', 'inert-untrusted'] as const)( 'does not load extensions in %s mode', async (mode) => { await writeExtension('suite', ['hidden']); if (mode === 'safe') vi.stubEnv('QWEN_CODE_SAFE_MODE', '1'); - if (mode === 'disabled-level') { - await fsp.mkdir(path.join(qwenHome, '.qwen'), { recursive: true }); - await fsp.writeFile( - path.join(qwenHome, '.qwen', 'settings.json'), - JSON.stringify({ skills: { disabledLevels: ['extension'] } }), - ); - } const refresh = vi.spyOn(ExtensionManager.prototype, 'refreshCache'); const status = await createWorkspaceSkillsStatusProvider({ workspaceTrusted: mode !== 'untrusted' && mode !== 'inert-untrusted', @@ -608,6 +629,37 @@ describe('createWorkspaceSkillsStatusProvider', () => { }, ); + it('gates extension discovery but still lists inactive entries when the extension level is disabled', async () => { + await writeExtension('active', ['active-skill']); + await writeExtension('inactive', ['inactive-skill']); + await fsp.writeFile( + path.join(qwenHome, 'extensions', 'extension-enablement.json'), + JSON.stringify({ inactive: { overrides: ['!*'] } }), + ); + await fsp.mkdir(path.join(qwenHome, '.qwen'), { recursive: true }); + await fsp.writeFile( + path.join(qwenHome, '.qwen', 'settings.json'), + JSON.stringify({ skills: { disabledLevels: ['extension'] } }), + ); + const refresh = vi.spyOn(ExtensionManager.prototype, 'refreshCache'); + const status = await createWorkspaceSkillsStatusProvider()(qwenHome); + expect(status.initialized).toBe(true); + // Discovery is gated: no active extension Skill is listed as usable... + expect(status.skills.some((s) => s.name === 'active-skill')).toBe(false); + // ...but inactive management entries still appear, matching the child + // producer, which appends them unconditionally. + expect(status.skills).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + name: 'inactive-skill', + level: 'extension', + disabledReason: 'inactive_extension', + }), + ]), + ); + expect(refresh).toHaveBeenCalled(); + }); + it('returns an error for an unreadable extension directory', async () => { await fsp.writeFile(path.join(qwenHome, 'extensions'), 'not a directory'); const status = await createWorkspaceSkillsStatusProvider()(qwenHome); @@ -616,8 +668,33 @@ describe('createWorkspaceSkillsStatusProvider', () => { skills: [], errors: [{ kind: 'skills', status: 'error' }], }); + // The readdir probe is what surfaces the broken root — pin the identity + // of its error, not just the error cell's shape. + expect(status.errors?.[0]?.error).toContain('ENOTDIR'); }); + // The store loader swallows listing errors, so the readdir probe is the + // only thing that reports a searchable-but-unlistable extensions root. + it.skipIf(process.platform === 'win32' || process.getuid?.() === 0)( + 'fails closed for a searchable but unlistable extensions root', + async () => { + const extensionsRoot = path.join(qwenHome, 'extensions'); + await fsp.mkdir(extensionsRoot); + await fsp.writeFile( + path.join(extensionsRoot, 'extension-enablement.json'), + JSON.stringify({}), + ); + await fsp.chmod(extensionsRoot, 0o111); + try { + const status = await createWorkspaceSkillsStatusProvider()(qwenHome); + expect(status.initialized).toBe(false); + expect(status.errors?.[0]?.error).toContain('EACCES'); + } finally { + await fsp.chmod(extensionsRoot, 0o755); + } + }, + ); + it('loads linked extensions and Agent Plugin manifests through the shared loader', async () => { const source = await writeExtension('linked', ['linked-skill']); const relocated = path.join(qwenHome, 'linked-source'); @@ -659,23 +736,148 @@ describe('createWorkspaceSkillsStatusProvider', () => { ); }); - it('does not cache a failed store read as an initialized empty catalog', async () => { + it('serves the non-extension catalog when the extension load fails, and retries it', async () => { + const workspace = path.join(qwenHome, 'workspace'); + const projectSkillDir = path.join(workspace, '.qwen', 'skills', 'proj'); + await fsp.mkdir(projectSkillDir, { recursive: true }); + await fsp.writeFile( + path.join(projectSkillDir, 'SKILL.md'), + '---\nname: proj\ndescription: Project skill\n---\nBody', + ); await writeExtension('suite', ['visible']); vi.spyOn(ExtensionManager.prototype, 'refreshCache').mockRejectedValueOnce( new Error('store unavailable'), ); const provider = createWorkspaceSkillsStatusProvider(); - expect(await provider(qwenHome)).toMatchObject({ - initialized: false, - errors: [{ kind: 'skills', error: 'store unavailable' }], - }); - expect((await provider(qwenHome)).skills).toEqual( + // A fault inside the extension load degrades only the extension entries; + // project, user and bundled Skills do not depend on that store. + const degraded = await provider(workspace); + expect(degraded.initialized).toBe(true); + expect(degraded.errors).toBeUndefined(); + expect(degraded.skills.some((s) => s.level === 'extension')).toBe(false); + expect(degraded.skills).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + name: 'proj', + level: 'project', + status: 'ok', + }), + expect.objectContaining({ name: 'review', level: 'bundled' }), + ]), + ); + // The degraded pair is not cached, so the next call retries the + // extension enumeration instead of freezing an extension-less catalog. + expect((await provider(workspace)).skills).toEqual( expect.arrayContaining([ expect.objectContaining({ name: 'visible', status: 'ok' }), ]), ); }); + it('re-resolves the extension locale when the configured language changes', async () => { + vi.stubEnv('QWEN_CODE_LANG', ''); + for (const name of ['active', 'inactive']) { + const directory = await writeExtension(name, [`${name}-skill`]); + const manifestPath = path.join(directory, 'qwen-extension.json'); + const manifest = JSON.parse(await fsp.readFile(manifestPath, 'utf8')); + await fsp.writeFile( + manifestPath, + JSON.stringify({ + ...manifest, + displayName: { en: 'Extension', zh: '扩展' }, + }), + ); + } + await fsp.writeFile( + path.join(qwenHome, 'extensions', 'extension-enablement.json'), + JSON.stringify({ inactive: { overrides: ['!*'] } }), + ); + const workspace = path.join(qwenHome, 'workspace'); + await fsp.mkdir(path.join(workspace, '.qwen'), { recursive: true }); + await fsp.writeFile( + path.join(workspace, '.qwen', 'settings.json'), + JSON.stringify({ general: { language: 'en' } }), + ); + const provider = createWorkspaceSkillsStatusProvider(); + const readNames = async () => + (await provider(workspace)).skills + .filter((s) => s.level === 'extension') + .map((s) => `${s.name}:${s.extensionDisplayName ?? ''}`); + expect(await readNames()).toEqual([ + 'active-skill:Extension', + 'inactive-skill:Extension', + ]); + await fsp.writeFile( + path.join(workspace, '.qwen', 'settings.json'), + JSON.stringify({ general: { language: 'zh' } }), + ); + // No provider.invalidate: a language change reaches no invalidation + // point, so the locale is part of the cache key. + expect(await readNames()).toEqual([ + 'active-skill:扩展', + 'inactive-skill:扩展', + ]); + }); + + it('ignores a non-string general.language instead of failing the catalog', async () => { + const workspace = path.join(qwenHome, 'workspace'); + await fsp.mkdir(path.join(workspace, '.qwen'), { recursive: true }); + await fsp.writeFile( + path.join(workspace, '.qwen', 'settings.json'), + JSON.stringify({ general: { language: 1 } }), + ); + const status = await createWorkspaceSkillsStatusProvider()(workspace); + expect(status.initialized).toBe(true); + expect(status.skills.some((s) => s.name === 'review')).toBe(true); + }); + + it('does not install a pre-mutation snapshot when invalidate lands mid-build', async () => { + await writeExtension('first', ['first-skill']); + let release!: () => void; + const gate = new Promise((resolve) => { + release = resolve; + }); + vi.spyOn(ExtensionManager.prototype, 'refreshCache').mockImplementationOnce( + () => gate, + ); + const provider = createWorkspaceSkillsStatusProvider(); + const first = provider(qwenHome); + // Commit a store mutation and deliver the invalidation while the cold + // build is still in flight. + await writeExtension('second', ['second-skill']); + provider.invalidate?.(qwenHome); + release(); + await first; + const second = await provider(qwenHome); + expect(second.skills).toEqual( + expect.arrayContaining([ + expect.objectContaining({ name: 'second-skill', status: 'ok' }), + ]), + ); + }); + + it('lists a duplicate Skill name within one inactive extension only once', async () => { + const directory = path.join(qwenHome, 'extensions', 'inactive'); + for (const dir of ['a', 'b']) { + const skillDir = path.join(directory, 'skills', dir); + await fsp.mkdir(skillDir, { recursive: true }); + await fsp.writeFile( + path.join(skillDir, 'SKILL.md'), + '---\nname: dup\ndescription: dup description\n---\nInstructions', + ); + } + await fsp.writeFile( + path.join(directory, 'qwen-extension.json'), + JSON.stringify({ name: 'inactive', version: '1.0.0' }), + ); + await fsp.writeFile( + path.join(qwenHome, 'extensions', 'extension-enablement.json'), + JSON.stringify({ inactive: { overrides: ['!*'] } }), + ); + const status = await createWorkspaceSkillsStatusProvider()(qwenHome); + expect(status.skills.filter((s) => s.name === 'dup')).toHaveLength(1); + }); + it.each([ { language: 'zh', envLanguage: '', expected: '扩展' }, { language: 'zh-CN', envLanguage: '', expected: '扩展' }, diff --git a/packages/cli/src/serve/workspace-skills-status.ts b/packages/cli/src/serve/workspace-skills-status.ts index 54aa6ac8b3d..cdd74a71141 100644 --- a/packages/cli/src/serve/workspace-skills-status.ts +++ b/packages/cli/src/serve/workspace-skills-status.ts @@ -85,6 +85,20 @@ type SkillManagerConfigShim = Pick< interface WorkspaceSkillManagers { skillManager: SkillManager; extensionManager?: ExtensionManager; + locale: string; +} + +/** + * Fails closed on an unreadable directory while tolerating one that does not + * exist. The store loader swallows listing errors, so without this probe an + * unlistable root would silently yield a catalog missing its entries. + */ +async function assertReadableDir(directory: string): Promise { + try { + await fs.readdir(directory); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; + } } export function createWorkspaceSkillsStatusProvider( @@ -96,20 +110,51 @@ export function createWorkspaceSkillsStatusProvider( // fallback, so slight staleness between explicit invalidation points is // acceptable: the live child re-lists authoritatively once a session exists. const managers = new Map(); - const provider = ((workspaceCwd: string) => - buildWorkspaceSkillsStatus( + // Per-workspace invalidation epochs, bumped synchronously by `invalidate`. + // A cold build captures the epoch before its first await and installs only + // while it is unchanged, so an invalidation delivered mid-build cannot be + // undone by that build's own `managers.set`. + const epochs = new Map(); + const inFlight = new Map< + string, + { epoch: number; promise: Promise } + >(); + const provider = ((workspaceCwd: string) => { + // Coalesce concurrent cold builds of one workspace; a caller arriving + // after an invalidation (newer epoch) starts a fresh build instead of + // joining a pre-mutation one. + const epoch = epochs.get(workspaceCwd) ?? 0; + const pending = inFlight.get(workspaceCwd); + if (pending?.epoch === epoch) return pending.promise; + const promise = buildWorkspaceSkillsStatus( workspaceCwd, managers, + epochs, + epoch, options.workspaceTrusted ?? true, options.includeUntrustedSkills ?? false, - )) as WorkspaceSkillsStatusProvider; - provider.invalidate = (workspaceCwd) => managers.delete(workspaceCwd); + ); + inFlight.set(workspaceCwd, { epoch, promise }); + const clear = () => { + if (inFlight.get(workspaceCwd)?.promise === promise) { + inFlight.delete(workspaceCwd); + } + }; + void promise.then(clear, clear); + return promise; + }) as WorkspaceSkillsStatusProvider; + provider.invalidate = (workspaceCwd) => { + managers.delete(workspaceCwd); + epochs.set(workspaceCwd, (epochs.get(workspaceCwd) ?? 0) + 1); + }; return provider; } async function buildWorkspaceSkillsStatus( workspaceCwd: string, managers: Map, + epochs: Map, + epoch: number, workspaceTrusted: boolean, includeUntrustedSkills: boolean, ): Promise { @@ -120,7 +165,22 @@ async function buildWorkspaceSkillsStatus( skipWorkspaceSettings: !workspaceTrusted, workspaceTrusted, }); + // Resolve the extension locale from this call's settings: a language + // change reaches no invalidation point, so the locale is part of the + // cache key. Settings carry no value validation, so guard the raw value — + // a non-string `general.language` would otherwise throw inside locale + // resolution and fail the whole catalog. + const rawLanguage = settings.merged.general?.language; + const locale = resolveLanguage( + resolveLanguageSetting( + typeof rawLanguage === 'string' ? rawLanguage : undefined, + ), + ); let cached = managers.get(workspaceCwd); + if (cached && cached.locale !== locale) { + managers.delete(workspaceCwd); + cached = undefined; + } if (!cached) { // Mirror the CLI guard in loadCliConfig: safe mode nullifies // disabledSkillLevels so the child session loads all bundled skills. @@ -139,22 +199,31 @@ async function buildWorkspaceSkillsStatus( const safeMode = (!workspaceTrusted && !includeUntrustedSkills) || isSafeModeEnv(); let extensionManager: ExtensionManager | undefined; - if (workspaceTrusted && !safeMode && !disabledLevels.has('extension')) { + // A failed extension load is served without extension Skills but not + // cached, so the next call retries enumeration. + let extensionLoadFailed = false; + if (workspaceTrusted && !safeMode) { + // Keep this probe outside the inner failure domain: an unreadable + // extensions *root* still fails the whole catalog (the store loader + // would silently swallow it), while a fault inside the load itself + // degrades to a catalog without extension Skills. + await assertReadableDir(Storage.getUserExtensionsDir()); try { - await fs.readdir(Storage.getUserExtensionsDir()); + extensionManager = new ExtensionManager({ + workspaceDir: workspaceCwd, + isWorkspaceTrusted: workspaceTrusted, + locale, + }); + await extensionManager.refreshCache(); } catch (error) { - if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; + extensionLoadFailed = true; + extensionManager = undefined; + writeStderrLine( + `qwen serve: extension skill enumeration skipped for ${workspaceCwd}: ${ + error instanceof Error ? error.message : String(error) + }`, + ); } - extensionManager = new ExtensionManager({ - workspaceDir: workspaceCwd, - isWorkspaceTrusted: workspaceTrusted, - locale: resolveLanguage( - resolveLanguageSetting( - settings.merged.general?.language as string | undefined, - ), - ), - }); - await extensionManager.refreshCache(); } const shim: SkillManagerConfigShim = { // Honor the safe-mode env the same way `Config` does when no explicit @@ -165,9 +234,15 @@ async function buildWorkspaceSkillsStatus( // bare, so it is always off here. getBareMode: () => false, getProjectRoot: () => workspaceCwd, + // disabledLevels gates discovery only (SkillManager applies the same + // level gate); inactive-extension management entries are appended + // regardless, matching the child producer. getActiveExtensions: () => - extensionManager?.getLoadedExtensions().filter((e) => e.isActive) ?? - [], + disabledLevels.has('extension') + ? [] + : (extensionManager + ?.getLoadedExtensions() + .filter((e) => e.isActive) ?? []), getDisabledSkillLevels: () => disabledLevels, }; const skillManager = new SkillManager(shim as Config); @@ -175,19 +250,18 @@ async function buildWorkspaceSkillsStatus( for (const level of ['project', 'user'] as const) { if (disabledLevels.has(level)) continue; for (const directory of skillManager.getSkillsBaseDirs(level)) { - try { - await fs.readdir(directory); - } catch (error) { - if ((error as NodeJS.ErrnoException).code !== 'ENOENT') { - throw error; - } - } + await assertReadableDir(directory); } } } - cached = { skillManager, extensionManager }; - managers.set(workspaceCwd, cached); + cached = { skillManager, extensionManager, locale }; + if (!extensionLoadFailed && (epochs.get(workspaceCwd) ?? 0) === epoch) { + managers.set(workspaceCwd, cached); + } } + // Settings re-load on every call, while the extension store snapshot + // stays frozen in the cached manager until invalidation — the two + // freshness clocks are deliberate for this best-effort fallback. const { disablements, enabledNames } = resolveSkillSettings(settings); const { skillManager, extensionManager } = cached; const extensions = extensionManager?.getLoadedExtensions() ?? []; From 284fcb464aebefa503a05b64062873d444926f77 Mon Sep 17 00:00:00 2001 From: qwen-code-bot Date: Tue, 8 Sep 2026 00:49:04 +0000 Subject: [PATCH 4/8] fix(daemon): close skills-catalog probe, tier, and latch gaps from review (#11281) - Decide directory absence by lstat and readability by readdir, so a dangling symlink at the extensions root fails closed instead of reading as absent and silently dropping every extension Skill. - Degrade an unreadable extensions root for workspaces that disabled extension discovery: the catalog-fatal probe no longer empties a catalog that opted out of extension Skills. - Stop latching daemon-local skills answers in the workspace facade (the latch stays child-only), so a degraded extension enumeration retries on the next poll instead of freezing the pre-child window. - Drop the unreachable disabled-level arm from the active-extensions shim; SkillManager already applies that gate before the only call. - Pin the degrade stderr line, the non-string language guard, and the superseded-build coalescing bookkeeping with discriminating tests; record the child locale-normalization divergence and the correct --extensions flag name in the design doc. Co-authored-by: Qwen-Coder --- docs/design/daemon-extension-skill-catalog.md | 45 +++++--- .../__tests__/facade.test.ts | 77 ++++++++++++- .../cli/src/serve/workspace-service/index.ts | 14 +-- .../src/serve/workspace-skills-status.test.ts | 103 ++++++++++++++++++ .../cli/src/serve/workspace-skills-status.ts | 47 +++++--- 5 files changed, 244 insertions(+), 42 deletions(-) diff --git a/docs/design/daemon-extension-skill-catalog.md b/docs/design/daemon-extension-skill-catalog.md index 6345ab6a4ba..ab8c85b0e89 100644 --- a/docs/design/daemon-extension-skill-catalog.md +++ b/docs/design/daemon-extension-skill-catalog.md @@ -25,20 +25,27 @@ management entries are appended regardless, matching the child producer. Failure handling is two-tier. An unreadable extensions _root_ (or a configured skills base directory) keeps the documented all-or-nothing behavior: an -uninitialized error status. The explicit `readdir` probe is what surfaces that -state — the store loader swallows listing errors, so without the probe an -unlistable root would silently yield an initialized catalog missing every -extension Skill. A fault _inside_ the extension load itself (corrupt store -state, a dangling extension symlink, lock contention) degrades only the -extension entries instead: the catalog is served initialized with the project, -user and bundled Skills, the failure is logged to the daemon's stderr, and the -degraded build is not cached so the next read retries extension enumeration. +uninitialized error status. "Unreadable" covers a root that exists but cannot +be listed — a regular file, an unlistable permission mode, or a dangling +symlink; the probe stats the entry before reading it precisely so a dangling +link's `ENOENT` is not mistaken for an absent root. The explicit `readdir` +probe is what surfaces that state — the store loader swallows listing errors, +so without the probe an unlistable root would silently yield an initialized +catalog missing every extension Skill. A fault _inside_ the extension load +itself (corrupt store state, a dangling extension symlink, lock contention) +degrades only the extension entries instead: the catalog is served initialized +with the project, user and bundled Skills, the failure is logged to the +daemon's stderr, and the degraded build is not cached so the next read retries +extension enumeration. A workspace that disabled the extension discovery level +opted out of the all-or-nothing tier: an unreadable root takes the same +degrade path as any other load fault for it, so the rest of its catalog stays +served. Cache installs are guarded by a per-workspace invalidation epoch, so an invalidation delivered while a cold build is in flight cannot be undone by that build, and concurrent cold builds of one workspace are coalesced. Startup -`--enable-extension` overrides are not propagated here (the daemon's child -spawn generally does not carry them either). +`--extensions` overrides are not propagated here (the daemon's child spawn +generally does not carry them either). The implementation and collocated regressions live in the daemon-local provider. Tests cover real manifests, active/inactive state, source collisions, persisted @@ -47,9 +54,13 @@ invalidation, language changes, the extension-load failure domain, unreadable directory roots and discovery-level gating. E2E evidence uses an isolated home and a daemon with no child session. -The facade still prefers child snapshots in this stage. Replacing that source, -changing toggle/refresh semantics, adding configured-state fields and changing -Web Shell projections belong to later PRs. No public schema changes are needed. +The facade still prefers child snapshots in this stage, and latches only +child-produced answers: daemon-local answers are re-requested on each +unlatched read (the provider's own manager cache keeps repeats cheap), so a +degraded extension enumeration retries on the next poll instead of freezing +for the rest of the pre-child window. Replacing that source, changing +toggle/refresh semantics, adding configured-state fields and changing Web +Shell projections belong to later PRs. No public schema changes are needed. Known later-stage items (recorded during review, deliberately not in this stage): @@ -59,6 +70,14 @@ stage): process-global environment, so a workspace `QWEN_CODE_LANG` can diverge the two panels of one page. This provider follows the documented daemon convention (`skipLoadEnvironment`); fixing the leak belongs to that route. +- The daemon normalizes `general.language` (a POSIX form such as + `zh_CN.UTF-8`, an alias, or a native name) before resolving extension + display names, while the child passes the raw setting string to + `resolveLocalizableString`, so such a value shows a localized name + pre-child and the English fallback once a child answers. Extracting one + shared locale resolver — used by this provider, the child's + `resolveLocaleForExtensions`, and the `/workspace/extensions` controller, + preserving env-over-settings precedence — belongs to a follow-up. - The inactive-append and sort assembly is a second copy of the child's (`acpAgent.ts`); extracting it into the shared `runtime/workspace-skills-mapping.ts` module — keeping the child's diff --git a/packages/cli/src/serve/workspace-service/__tests__/facade.test.ts b/packages/cli/src/serve/workspace-service/__tests__/facade.test.ts index 1193784be52..79017450dee 100644 --- a/packages/cli/src/serve/workspace-service/__tests__/facade.test.ts +++ b/packages/cli/src/serve/workspace-service/__tests__/facade.test.ts @@ -1280,13 +1280,84 @@ describe('createDaemonWorkspaceService', () => { ); const result = await svc.getWorkspaceSkillsStatus(makeCtx()); - const cached = await svc.getWorkspaceSkillsStatus(makeCtx()); + const reread = await svc.getWorkspaceSkillsStatus(makeCtx()); expect(workspaceSkillsStatusProvider).toHaveBeenCalledWith('/ws'); - expect(workspaceSkillsStatusProvider).toHaveBeenCalledOnce(); + // Daemon-local answers are deliberately not latched (unlike child + // answers): the provider caches its own managers, and skipping the + // latch lets a degraded extension enumeration retry on the next read. + expect(workspaceSkillsStatusProvider).toHaveBeenCalledTimes(2); expect(result.initialized).toBe(true); expect(result.skills.map((s) => s.name)).toEqual(['review']); - expect(cached).toEqual(result); + expect(reread).toEqual(result); + }); + + it('getWorkspaceSkillsStatus retries a degraded daemon-local answer instead of latching it', async () => { + let now = 10_000; + const nowSpy = vi.spyOn(Date, 'now').mockImplementation(() => now); + const queryWorkspaceStatus = vi + .fn() + .mockImplementation((_m: string, idle: () => unknown) => + Promise.resolve(idle()), + ); + const degraded = { + v: 1, + workspaceCwd: '/ws', + initialized: true, + skills: [ + { + kind: 'skill', + status: 'ok', + name: 'review', + description: 'Review changed code', + level: 'bundled', + modelInvocable: true, + }, + ], + }; + const complete = { + ...degraded, + skills: [ + ...degraded.skills, + { + kind: 'skill', + status: 'ok', + name: 'ext-skill', + description: 'Extension skill', + level: 'extension', + extensionName: 'suite', + modelInvocable: true, + }, + ], + }; + const workspaceSkillsStatusProvider = vi + .fn() + .mockResolvedValueOnce(degraded) + .mockResolvedValue(complete); + const svc = createDaemonWorkspaceService( + makeDeps({ + queryWorkspaceStatus, + workspaceSkillsStatusProvider, + boundWorkspace: '/ws', + }), + ); + + try { + const first = await svc.getWorkspaceSkillsStatus(makeCtx()); + now += 6_000; // past the 5s snapshot TTL + const second = await svc.getWorkspaceSkillsStatus(makeCtx()); + + expect(first.skills.map((s) => s.name)).toEqual(['review']); + // The degraded answer is served but not latched, so the next read + // past the TTL retries enumeration and self-heals. + expect(second.skills.map((s) => s.name)).toEqual([ + 'review', + 'ext-skill', + ]); + expect(workspaceSkillsStatusProvider).toHaveBeenCalledTimes(2); + } finally { + nowSpy.mockRestore(); + } }); it('getWorkspaceSkillsStatus prefers the cached child answer over the daemon-local provider', async () => { diff --git a/packages/cli/src/serve/workspace-service/index.ts b/packages/cli/src/serve/workspace-service/index.ts index f3a6116f398..3ba118c28d7 100644 --- a/packages/cli/src/serve/workspace-service/index.ts +++ b/packages/cli/src/serve/workspace-service/index.ts @@ -357,15 +357,11 @@ export function createDaemonWorkspaceService( // matching getWorkspaceEnvStatus / getWorkspacePreflightStatus. if (workspaceSkillsStatusProvider) { try { - const localStatus = await workspaceSkillsStatusProvider(boundWorkspace); - if ( - localStatus.initialized && - generation === workspaceSkillsGeneration - ) { - lastWorkspaceSkillsStatus = localStatus; - lastWorkspaceSkillsStatusAt = Date.now(); - } - return localStatus; + // Daemon-local answers are deliberately not latched (the latch above + // is child-only): the provider is always callable and caches its own + // managers, and latching a degraded answer — an extension enumeration + // fault degrades to initialized:true — would suppress its retry. + return await workspaceSkillsStatusProvider(boundWorkspace); } catch (err) { writeStderrLine( `qwen serve: getWorkspaceSkillsStatus local provider failed: ${err instanceof Error ? err.message : String(err)}`, diff --git a/packages/cli/src/serve/workspace-skills-status.test.ts b/packages/cli/src/serve/workspace-skills-status.test.ts index 7847b9c1f29..219853ce6e1 100644 --- a/packages/cli/src/serve/workspace-skills-status.test.ts +++ b/packages/cli/src/serve/workspace-skills-status.test.ts @@ -673,6 +673,41 @@ describe('createWorkspaceSkillsStatusProvider', () => { expect(status.errors?.[0]?.error).toContain('ENOTDIR'); }); + // A dangling symlink lstat()s fine but readdir()s ENOENT: a present, + // broken root, not an absent one, so it must fail closed like the + // regular-file shape above. + it.skipIf(process.platform === 'win32')( + 'fails closed for a dangling symlink at the extensions root', + async () => { + await fsp.symlink( + path.join(qwenHome, 'missing-target'), + path.join(qwenHome, 'extensions'), + ); + const status = await createWorkspaceSkillsStatusProvider()(qwenHome); + expect(status.initialized).toBe(false); + expect(status.skills).toEqual([]); + expect(status.errors?.[0]?.error).toContain('ENOENT'); + }, + ); + + it('degrades an unreadable extensions root when the extension level is disabled', async () => { + await fsp.writeFile(path.join(qwenHome, 'extensions'), 'not a directory'); + await fsp.mkdir(path.join(qwenHome, '.qwen'), { recursive: true }); + await fsp.writeFile( + path.join(qwenHome, '.qwen', 'settings.json'), + JSON.stringify({ skills: { disabledLevels: ['extension'] } }), + ); + const status = await createWorkspaceSkillsStatusProvider()(qwenHome); + // The workspace opted out of extension discovery, so the catalog-fatal + // root probe must not take the rest of its catalog down with it. + expect(status.initialized).toBe(true); + expect(status.errors).toBeUndefined(); + expect(status.skills.some((s) => s.name === 'review')).toBe(true); + expect(status.skills.some((s) => s.level === 'extension')).toBe(false); + expect(mockWriteStderrLine).toHaveBeenCalledTimes(1); + expect(mockWriteStderrLine.mock.calls[0][0]).toContain('ENOTDIR'); + }); + // The store loader swallows listing errors, so the readdir probe is the // only thing that reports a searchable-but-unlistable extensions root. it.skipIf(process.platform === 'win32' || process.getuid?.() === 0)( @@ -765,6 +800,10 @@ describe('createWorkspaceSkillsStatusProvider', () => { expect.objectContaining({ name: 'review', level: 'bundled' }), ]), ); + // The degraded response carries no errors cell, so this stderr line is + // the only observable signal that extension enumeration failed. + expect(mockWriteStderrLine).toHaveBeenCalledTimes(1); + expect(mockWriteStderrLine.mock.calls[0][0]).toContain('store unavailable'); // The degraded pair is not cached, so the next call retries the // extension enumeration instead of freezing an extension-less catalog. expect((await provider(workspace)).skills).toEqual( @@ -820,6 +859,10 @@ describe('createWorkspaceSkillsStatusProvider', () => { }); it('ignores a non-string general.language instead of failing the catalog', async () => { + // Pin the env out of the way ('' loses to settings): an ambient + // QWEN_CODE_LANG would shadow the invalid setting and leave the + // non-string guard unexercised. + vi.stubEnv('QWEN_CODE_LANG', ''); const workspace = path.join(qwenHome, 'workspace'); await fsp.mkdir(path.join(workspace, '.qwen'), { recursive: true }); await fsp.writeFile( @@ -856,6 +899,66 @@ describe('createWorkspaceSkillsStatusProvider', () => { ); }); + it('starts a fresh build for post-invalidation callers and lets later callers join it', async () => { + await writeExtension('first', ['first-skill']); + const realRefresh = ExtensionManager.prototype.refreshCache; + const parked: Array<() => void> = []; + const releases: Array<() => void> = []; + const parkedPromises = [ + new Promise((resolve) => parked.push(resolve)), + new Promise((resolve) => parked.push(resolve)), + ]; + let refreshCalls = 0; + const refreshSpy = vi + .spyOn(ExtensionManager.prototype, 'refreshCache') + .mockImplementation(async function (this: ExtensionManager) { + const index = refreshCalls++; + await realRefresh.call(this); + // Park the first two builds after their store reads: the first must + // hold a pre-mutation snapshot, and both must be in flight while the + // invalidation, the first settle, and the joining caller land. A + // third build means the join failed, so let it run through. + if (index > 1) return; + parked[index]?.(); + await new Promise((resolve) => { + releases[index] = resolve; + }); + }); + const provider = createWorkspaceSkillsStatusProvider(); + + const first = provider(qwenHome); + // Commit the store mutation and deliver the invalidation only after the + // first build's snapshot is taken, or the mutation races that read. + await parkedPromises[0]; + await writeExtension('second', ['second-skill']); + provider.invalidate?.(qwenHome); + // The newer epoch must start a fresh build, not join the pre-mutation one. + const second = provider(qwenHome); + releases[0]!(); + await first; + // The superseded build's cleanup must not evict the fresh build's entry. + const third = provider(qwenHome); + await parkedPromises[1]; + releases[1]!(); + const [firstStatus, secondStatus, thirdStatus] = await Promise.all([ + first, + second, + third, + ]); + + expect(firstStatus.skills.some((s) => s.name === 'second-skill')).toBe( + false, + ); + expect(refreshSpy).toHaveBeenCalledTimes(2); + expect(third).toBe(second); + expect(thirdStatus).toBe(secondStatus); + expect(secondStatus.skills).toEqual( + expect.arrayContaining([ + expect.objectContaining({ name: 'second-skill', status: 'ok' }), + ]), + ); + }); + it('lists a duplicate Skill name within one inactive extension only once', async () => { const directory = path.join(qwenHome, 'extensions', 'inactive'); for (const dir of ['a', 'b']) { diff --git a/packages/cli/src/serve/workspace-skills-status.ts b/packages/cli/src/serve/workspace-skills-status.ts index cdd74a71141..b7e2ce77494 100644 --- a/packages/cli/src/serve/workspace-skills-status.ts +++ b/packages/cli/src/serve/workspace-skills-status.ts @@ -90,15 +90,22 @@ interface WorkspaceSkillManagers { /** * Fails closed on an unreadable directory while tolerating one that does not - * exist. The store loader swallows listing errors, so without this probe an - * unlistable root would silently yield a catalog missing its entries. + * exist. `lstat` decides absence and `readdir` decides readability: a + * dangling symlink lstat()s fine but readdir()s `ENOENT`, and a + * present-but-unlistable root must fail closed rather than read as absent + * (`fs.stat` cannot separate the two — it follows the link and throws the + * same `ENOENT`). The store loader swallows listing errors, so without this + * probe an unlistable root would silently yield a catalog missing its + * entries. */ async function assertReadableDir(directory: string): Promise { try { - await fs.readdir(directory); + await fs.lstat(directory); } catch (error) { - if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return; + throw error; } + await fs.readdir(directory); } export function createWorkspaceSkillsStatusProvider( @@ -203,12 +210,19 @@ async function buildWorkspaceSkillsStatus( // cached, so the next call retries enumeration. let extensionLoadFailed = false; if (workspaceTrusted && !safeMode) { - // Keep this probe outside the inner failure domain: an unreadable - // extensions *root* still fails the whole catalog (the store loader - // would silently swallow it), while a fault inside the load itself - // degrades to a catalog without extension Skills. - await assertReadableDir(Storage.getUserExtensionsDir()); + // A workspace that disabled extension discovery opted out of the + // catalog-fatal tier: for it an unreadable root degrades like any + // other load fault instead of failing the whole catalog. + const extensionLevelDisabled = disabledLevels.has('extension'); + // The store loader swallows listing errors, so without the root + // probe an unlistable root would silently yield a catalog missing + // every extension Skill. Only the probe is catalog-fatal (and only + // while discovery is enabled); a fault inside the load itself + // degrades the extension entries either way. + let rootProbed = false; try { + await assertReadableDir(Storage.getUserExtensionsDir()); + rootProbed = true; extensionManager = new ExtensionManager({ workspaceDir: workspaceCwd, isWorkspaceTrusted: workspaceTrusted, @@ -216,6 +230,7 @@ async function buildWorkspaceSkillsStatus( }); await extensionManager.refreshCache(); } catch (error) { + if (!rootProbed && !extensionLevelDisabled) throw error; extensionLoadFailed = true; extensionManager = undefined; writeStderrLine( @@ -234,15 +249,13 @@ async function buildWorkspaceSkillsStatus( // bare, so it is always off here. getBareMode: () => false, getProjectRoot: () => workspaceCwd, - // disabledLevels gates discovery only (SkillManager applies the same - // level gate); inactive-extension management entries are appended - // regardless, matching the child producer. + // SkillManager applies the disabled-level gate itself (through + // getDisabledSkillLevels) before ever calling this; inactive-extension + // management entries are appended regardless, matching the child + // producer. getActiveExtensions: () => - disabledLevels.has('extension') - ? [] - : (extensionManager - ?.getLoadedExtensions() - .filter((e) => e.isActive) ?? []), + extensionManager?.getLoadedExtensions().filter((e) => e.isActive) ?? + [], getDisabledSkillLevels: () => disabledLevels, }; const skillManager = new SkillManager(shim as Config); From eef154cdb5379aa338652c33d303e48b69945737 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8F=B6=E5=85=AC?= Date: Tue, 8 Sep 2026 10:35:50 +0800 Subject: [PATCH 5/8] fix(daemon): restore scoped skill enumeration contracts --- docs/design/daemon-extension-skill-catalog.md | 89 +---- .../__tests__/facade.test.ts | 77 +--- .../cli/src/serve/workspace-service/index.ts | 14 +- .../src/serve/workspace-skills-status.test.ts | 374 ++++++------------ .../cli/src/serve/workspace-skills-status.ts | 148 ++----- 5 files changed, 182 insertions(+), 520 deletions(-) diff --git a/docs/design/daemon-extension-skill-catalog.md b/docs/design/daemon-extension-skill-catalog.md index ab8c85b0e89..89761f1c0d8 100644 --- a/docs/design/daemon-extension-skill-catalog.md +++ b/docs/design/daemon-extension-skill-catalog.md @@ -12,79 +12,30 @@ existing `inactive_extension` status, retaining their identity and metadata. Resolve settings and extension Skill defaults/overrides with the existing parsers. A settings opt-in does not enable an inactive parent extension. Resolve localized extension names with the existing language setting and locale -helpers, without changing the daemon process language. The resolved locale is -part of the provider cache key — recomputed from freshly loaded settings on -every call — because a language change reaches no invalidation point. +helpers on every response, without changing the daemon process language or +rebuilding the directory cache when only the language changes. Keep the lightweight Config surface: do not construct a runtime Config, start a child, initialize MCP, execute hooks, or install watchers. Honor safe mode, disabled discovery levels and workspace trust; inert untrusted inventory must -not load workspace settings or extension runtime context. -`skills.disabledLevels` gates extension _discovery_ only; inactive-extension -management entries are appended regardless, matching the child producer. - -Failure handling is two-tier. An unreadable extensions _root_ (or a configured -skills base directory) keeps the documented all-or-nothing behavior: an -uninitialized error status. "Unreadable" covers a root that exists but cannot -be listed — a regular file, an unlistable permission mode, or a dangling -symlink; the probe stats the entry before reading it precisely so a dangling -link's `ENOENT` is not mistaken for an absent root. The explicit `readdir` -probe is what surfaces that state — the store loader swallows listing errors, -so without the probe an unlistable root would silently yield an initialized -catalog missing every extension Skill. A fault _inside_ the extension load -itself (corrupt store state, a dangling extension symlink, lock contention) -degrades only the extension entries instead: the catalog is served initialized -with the project, user and bundled Skills, the failure is logged to the -daemon's stderr, and the degraded build is not cached so the next read retries -extension enumeration. A workspace that disabled the extension discovery level -opted out of the all-or-nothing tier: an unreadable root takes the same -degrade path as any other load fault for it, so the rest of its catalog stays -served. - -Cache installs are guarded by a per-workspace invalidation epoch, so an -invalidation delivered while a cold build is in flight cannot be undone by -that build, and concurrent cold builds of one workspace are coalesced. Startup -`--extensions` overrides are not propagated here (the daemon's child spawn -generally does not carry them either). +not load workspace settings or extension runtime context. Directory failures +continue to return an uninitialized error status. The implementation and collocated regressions live in the daemon-local provider. Tests cover real manifests, active/inactive state, source collisions, persisted -Skill settings, safe/untrusted contexts, explicit and mid-build cache -invalidation, language changes, the extension-load failure domain, unreadable -directory roots and discovery-level gating. E2E evidence uses an isolated home -and a daemon with no child session. - -The facade still prefers child snapshots in this stage, and latches only -child-produced answers: daemon-local answers are re-requested on each -unlatched read (the provider's own manager cache keeps repeats cheap), so a -degraded extension enumeration retries on the next poll instead of freezing -for the rest of the pre-child window. Replacing that source, changing -toggle/refresh semantics, adding configured-state fields and changing Web -Shell projections belong to later PRs. No public schema changes are needed. - -Known later-stage items (recorded during review, deliberately not in this -stage): - -- The sibling `/workspace/extensions` route resolves its locale through a - settings load that admits the workspace `.env` into the daemon's - process-global environment, so a workspace `QWEN_CODE_LANG` can diverge the - two panels of one page. This provider follows the documented daemon - convention (`skipLoadEnvironment`); fixing the leak belongs to that route. -- The daemon normalizes `general.language` (a POSIX form such as - `zh_CN.UTF-8`, an alias, or a native name) before resolving extension - display names, while the child passes the raw setting string to - `resolveLocalizableString`, so such a value shows a localized name - pre-child and the English fallback once a child answers. Extracting one - shared locale resolver — used by this provider, the child's - `resolveLocaleForExtensions`, and the `/workspace/extensions` controller, - preserving env-over-settings precedence — belongs to a follow-up. -- The inactive-append and sort assembly is a second copy of the child's - (`acpAgent.ts`); extracting it into the shared - `runtime/workspace-skills-mapping.ts` module — keeping the child's - `level:extensionName:name` dedupe key — belongs to a follow-up. -- Retaining the cached `ExtensionManager` across skill-settings invalidations - behind `refreshCacheIfSourcesChanged` needs invalidation provenance - (extension-store mutation vs skill-settings mutation) that the current - call sites do not carry. -- The active-Skill `enabled` judgment mirrors `Config.isSkillEnabled`; - sharing that decision with core config belongs to a follow-up. +Skill settings, safe/untrusted contexts and explicit cache invalidation. E2E +evidence uses an isolated home and a daemon with no child session. + +The facade still prefers child snapshots in this stage. Replacing that source, +changing toggle/refresh semantics, adding configured-state fields and changing +Web Shell projections belong to later PRs. No public schema changes are needed. + +Discovery-level disabling suppresses active extension Skills through +`SkillManager`; inactive extension management entries are still appended, as in +the child producer. Safe mode and untrusted contexts never load extensions. + +Enumeration errors, including extension-store faults, return +`initialized: false` with explicit errors. This stage does not certify an +incomplete extension inventory as a successful response. Existing facade +caching, source preference and invalidation behavior remain unchanged; the +tracking issue assigns cache lifecycle and concurrency changes to stage 4. diff --git a/packages/cli/src/serve/workspace-service/__tests__/facade.test.ts b/packages/cli/src/serve/workspace-service/__tests__/facade.test.ts index 79017450dee..1193784be52 100644 --- a/packages/cli/src/serve/workspace-service/__tests__/facade.test.ts +++ b/packages/cli/src/serve/workspace-service/__tests__/facade.test.ts @@ -1280,84 +1280,13 @@ describe('createDaemonWorkspaceService', () => { ); const result = await svc.getWorkspaceSkillsStatus(makeCtx()); - const reread = await svc.getWorkspaceSkillsStatus(makeCtx()); + const cached = await svc.getWorkspaceSkillsStatus(makeCtx()); expect(workspaceSkillsStatusProvider).toHaveBeenCalledWith('/ws'); - // Daemon-local answers are deliberately not latched (unlike child - // answers): the provider caches its own managers, and skipping the - // latch lets a degraded extension enumeration retry on the next read. - expect(workspaceSkillsStatusProvider).toHaveBeenCalledTimes(2); + expect(workspaceSkillsStatusProvider).toHaveBeenCalledOnce(); expect(result.initialized).toBe(true); expect(result.skills.map((s) => s.name)).toEqual(['review']); - expect(reread).toEqual(result); - }); - - it('getWorkspaceSkillsStatus retries a degraded daemon-local answer instead of latching it', async () => { - let now = 10_000; - const nowSpy = vi.spyOn(Date, 'now').mockImplementation(() => now); - const queryWorkspaceStatus = vi - .fn() - .mockImplementation((_m: string, idle: () => unknown) => - Promise.resolve(idle()), - ); - const degraded = { - v: 1, - workspaceCwd: '/ws', - initialized: true, - skills: [ - { - kind: 'skill', - status: 'ok', - name: 'review', - description: 'Review changed code', - level: 'bundled', - modelInvocable: true, - }, - ], - }; - const complete = { - ...degraded, - skills: [ - ...degraded.skills, - { - kind: 'skill', - status: 'ok', - name: 'ext-skill', - description: 'Extension skill', - level: 'extension', - extensionName: 'suite', - modelInvocable: true, - }, - ], - }; - const workspaceSkillsStatusProvider = vi - .fn() - .mockResolvedValueOnce(degraded) - .mockResolvedValue(complete); - const svc = createDaemonWorkspaceService( - makeDeps({ - queryWorkspaceStatus, - workspaceSkillsStatusProvider, - boundWorkspace: '/ws', - }), - ); - - try { - const first = await svc.getWorkspaceSkillsStatus(makeCtx()); - now += 6_000; // past the 5s snapshot TTL - const second = await svc.getWorkspaceSkillsStatus(makeCtx()); - - expect(first.skills.map((s) => s.name)).toEqual(['review']); - // The degraded answer is served but not latched, so the next read - // past the TTL retries enumeration and self-heals. - expect(second.skills.map((s) => s.name)).toEqual([ - 'review', - 'ext-skill', - ]); - expect(workspaceSkillsStatusProvider).toHaveBeenCalledTimes(2); - } finally { - nowSpy.mockRestore(); - } + expect(cached).toEqual(result); }); it('getWorkspaceSkillsStatus prefers the cached child answer over the daemon-local provider', async () => { diff --git a/packages/cli/src/serve/workspace-service/index.ts b/packages/cli/src/serve/workspace-service/index.ts index 3ba118c28d7..f3a6116f398 100644 --- a/packages/cli/src/serve/workspace-service/index.ts +++ b/packages/cli/src/serve/workspace-service/index.ts @@ -357,11 +357,15 @@ export function createDaemonWorkspaceService( // matching getWorkspaceEnvStatus / getWorkspacePreflightStatus. if (workspaceSkillsStatusProvider) { try { - // Daemon-local answers are deliberately not latched (the latch above - // is child-only): the provider is always callable and caches its own - // managers, and latching a degraded answer — an extension enumeration - // fault degrades to initialized:true — would suppress its retry. - return await workspaceSkillsStatusProvider(boundWorkspace); + const localStatus = await workspaceSkillsStatusProvider(boundWorkspace); + if ( + localStatus.initialized && + generation === workspaceSkillsGeneration + ) { + lastWorkspaceSkillsStatus = localStatus; + lastWorkspaceSkillsStatusAt = Date.now(); + } + return localStatus; } catch (err) { writeStderrLine( `qwen serve: getWorkspaceSkillsStatus local provider failed: ${err instanceof Error ? err.message : String(err)}`, diff --git a/packages/cli/src/serve/workspace-skills-status.test.ts b/packages/cli/src/serve/workspace-skills-status.test.ts index 219853ce6e1..860bdeae14c 100644 --- a/packages/cli/src/serve/workspace-skills-status.test.ts +++ b/packages/cli/src/serve/workspace-skills-status.test.ts @@ -412,7 +412,6 @@ describe('createWorkspaceSkillsStatusProvider', () => { it('reuses one SkillManager per workspace across calls', async () => { const listSpy = vi.spyOn(SkillManager.prototype, 'listSkills'); - const refreshSpy = vi.spyOn(ExtensionManager.prototype, 'refreshCache'); const provider = createWorkspaceSkillsStatusProvider(); await provider('/ws'); @@ -422,20 +421,6 @@ describe('createWorkspaceSkillsStatusProvider', () => { // listSkills is invoked on the same object rather than a freshly-scanned one. expect(listSpy).toHaveBeenCalledTimes(2); expect(listSpy.mock.instances[0]).toBe(listSpy.mock.instances[1]); - // The cached ExtensionManager is reused as well: the extension store is - // read once, not on every request. - expect(refreshSpy).toHaveBeenCalledTimes(1); - }); - - it('coalesces concurrent cold builds of the same workspace', async () => { - await writeExtension('suite', ['visible']); - const refreshSpy = vi.spyOn(ExtensionManager.prototype, 'refreshCache'); - const provider = createWorkspaceSkillsStatusProvider(); - - const [a, b] = await Promise.all([provider(qwenHome), provider(qwenHome)]); - - expect(a).toBe(b); - expect(refreshSpy).toHaveBeenCalledTimes(1); }); async function writeExtension( @@ -629,37 +614,6 @@ describe('createWorkspaceSkillsStatusProvider', () => { }, ); - it('gates extension discovery but still lists inactive entries when the extension level is disabled', async () => { - await writeExtension('active', ['active-skill']); - await writeExtension('inactive', ['inactive-skill']); - await fsp.writeFile( - path.join(qwenHome, 'extensions', 'extension-enablement.json'), - JSON.stringify({ inactive: { overrides: ['!*'] } }), - ); - await fsp.mkdir(path.join(qwenHome, '.qwen'), { recursive: true }); - await fsp.writeFile( - path.join(qwenHome, '.qwen', 'settings.json'), - JSON.stringify({ skills: { disabledLevels: ['extension'] } }), - ); - const refresh = vi.spyOn(ExtensionManager.prototype, 'refreshCache'); - const status = await createWorkspaceSkillsStatusProvider()(qwenHome); - expect(status.initialized).toBe(true); - // Discovery is gated: no active extension Skill is listed as usable... - expect(status.skills.some((s) => s.name === 'active-skill')).toBe(false); - // ...but inactive management entries still appear, matching the child - // producer, which appends them unconditionally. - expect(status.skills).toEqual( - expect.arrayContaining([ - expect.objectContaining({ - name: 'inactive-skill', - level: 'extension', - disabledReason: 'inactive_extension', - }), - ]), - ); - expect(refresh).toHaveBeenCalled(); - }); - it('returns an error for an unreadable extension directory', async () => { await fsp.writeFile(path.join(qwenHome, 'extensions'), 'not a directory'); const status = await createWorkspaceSkillsStatusProvider()(qwenHome); @@ -668,68 +622,9 @@ describe('createWorkspaceSkillsStatusProvider', () => { skills: [], errors: [{ kind: 'skills', status: 'error' }], }); - // The readdir probe is what surfaces the broken root — pin the identity - // of its error, not just the error cell's shape. expect(status.errors?.[0]?.error).toContain('ENOTDIR'); }); - // A dangling symlink lstat()s fine but readdir()s ENOENT: a present, - // broken root, not an absent one, so it must fail closed like the - // regular-file shape above. - it.skipIf(process.platform === 'win32')( - 'fails closed for a dangling symlink at the extensions root', - async () => { - await fsp.symlink( - path.join(qwenHome, 'missing-target'), - path.join(qwenHome, 'extensions'), - ); - const status = await createWorkspaceSkillsStatusProvider()(qwenHome); - expect(status.initialized).toBe(false); - expect(status.skills).toEqual([]); - expect(status.errors?.[0]?.error).toContain('ENOENT'); - }, - ); - - it('degrades an unreadable extensions root when the extension level is disabled', async () => { - await fsp.writeFile(path.join(qwenHome, 'extensions'), 'not a directory'); - await fsp.mkdir(path.join(qwenHome, '.qwen'), { recursive: true }); - await fsp.writeFile( - path.join(qwenHome, '.qwen', 'settings.json'), - JSON.stringify({ skills: { disabledLevels: ['extension'] } }), - ); - const status = await createWorkspaceSkillsStatusProvider()(qwenHome); - // The workspace opted out of extension discovery, so the catalog-fatal - // root probe must not take the rest of its catalog down with it. - expect(status.initialized).toBe(true); - expect(status.errors).toBeUndefined(); - expect(status.skills.some((s) => s.name === 'review')).toBe(true); - expect(status.skills.some((s) => s.level === 'extension')).toBe(false); - expect(mockWriteStderrLine).toHaveBeenCalledTimes(1); - expect(mockWriteStderrLine.mock.calls[0][0]).toContain('ENOTDIR'); - }); - - // The store loader swallows listing errors, so the readdir probe is the - // only thing that reports a searchable-but-unlistable extensions root. - it.skipIf(process.platform === 'win32' || process.getuid?.() === 0)( - 'fails closed for a searchable but unlistable extensions root', - async () => { - const extensionsRoot = path.join(qwenHome, 'extensions'); - await fsp.mkdir(extensionsRoot); - await fsp.writeFile( - path.join(extensionsRoot, 'extension-enablement.json'), - JSON.stringify({}), - ); - await fsp.chmod(extensionsRoot, 0o111); - try { - const status = await createWorkspaceSkillsStatusProvider()(qwenHome); - expect(status.initialized).toBe(false); - expect(status.errors?.[0]?.error).toContain('EACCES'); - } finally { - await fsp.chmod(extensionsRoot, 0o755); - } - }, - ); - it('loads linked extensions and Agent Plugin manifests through the shared loader', async () => { const source = await writeExtension('linked', ['linked-skill']); const relocated = path.join(qwenHome, 'linked-source'); @@ -771,46 +666,99 @@ describe('createWorkspaceSkillsStatusProvider', () => { ); }); - it('serves the non-extension catalog when the extension load fails, and retries it', async () => { - const workspace = path.join(qwenHome, 'workspace'); - const projectSkillDir = path.join(workspace, '.qwen', 'skills', 'proj'); - await fsp.mkdir(projectSkillDir, { recursive: true }); - await fsp.writeFile( - path.join(projectSkillDir, 'SKILL.md'), - '---\nname: proj\ndescription: Project skill\n---\nBody', - ); + it('does not cache a failed store read as an initialized empty catalog', async () => { await writeExtension('suite', ['visible']); vi.spyOn(ExtensionManager.prototype, 'refreshCache').mockRejectedValueOnce( new Error('store unavailable'), ); const provider = createWorkspaceSkillsStatusProvider(); - // A fault inside the extension load degrades only the extension entries; - // project, user and bundled Skills do not depend on that store. - const degraded = await provider(workspace); - expect(degraded.initialized).toBe(true); - expect(degraded.errors).toBeUndefined(); - expect(degraded.skills.some((s) => s.level === 'extension')).toBe(false); - expect(degraded.skills).toEqual( + expect(await provider(qwenHome)).toMatchObject({ + initialized: false, + errors: [{ kind: 'skills', error: 'store unavailable' }], + }); + expect((await provider(qwenHome)).skills).toEqual( expect.arrayContaining([ - expect.objectContaining({ - name: 'proj', - level: 'project', - status: 'ok', - }), - expect.objectContaining({ name: 'review', level: 'bundled' }), + expect.objectContaining({ name: 'visible', status: 'ok' }), ]), ); - // The degraded response carries no errors cell, so this stderr line is - // the only observable signal that extension enumeration failed. - expect(mockWriteStderrLine).toHaveBeenCalledTimes(1); - expect(mockWriteStderrLine.mock.calls[0][0]).toContain('store unavailable'); - // The degraded pair is not cached, so the next call retries the - // extension enumeration instead of freezing an extension-less catalog. - expect((await provider(workspace)).skills).toEqual( + }); + + it.each([ + { language: 'zh', envLanguage: '', expected: '扩展' }, + { language: 'zh-CN', envLanguage: '', expected: '扩展' }, + { language: 'en', envLanguage: 'zh', expected: '扩展' }, + { language: 'auto', envLanguage: '', expected: '扩展' }, + { language: 'en', envLanguage: '', expected: 'Extension' }, + ])( + 'resolves extension names for $language with env $envLanguage', + async ({ language, envLanguage, expected }) => { + vi.stubEnv('QWEN_CODE_LANG', envLanguage); + vi.stubEnv('LANG', 'zh_CN.UTF-8'); + for (const name of ['active', 'inactive']) { + const directory = await writeExtension(name, [`${name}-skill`]); + const manifestPath = path.join(directory, 'qwen-extension.json'); + const manifest = JSON.parse(await fsp.readFile(manifestPath, 'utf8')); + await fsp.writeFile( + manifestPath, + JSON.stringify({ + ...manifest, + displayName: { en: 'Extension', zh: '扩展' }, + }), + ); + } + await fsp.writeFile( + path.join(qwenHome, 'extensions', 'extension-enablement.json'), + JSON.stringify({ inactive: { overrides: ['!*'] } }), + ); + await fsp.mkdir(path.join(qwenHome, '.qwen'), { recursive: true }); + await fsp.writeFile( + path.join(qwenHome, '.qwen', 'settings.json'), + JSON.stringify({ general: { language } }), + ); + const status = await createWorkspaceSkillsStatusProvider()(qwenHome); + expect(status.initialized).toBe(true); + expect( + status.skills.filter((s) => s.level === 'extension'), + ).toMatchObject([ + { name: 'active-skill', extensionDisplayName: expected, status: 'ok' }, + { + name: 'inactive-skill', + extensionDisplayName: expected, + disabledReason: 'inactive_extension', + }, + ]); + }, + ); + + it('gates extension discovery but still lists inactive entries when the extension level is disabled', async () => { + await writeExtension('active', ['active-skill']); + await writeExtension('inactive', ['inactive-skill']); + await fsp.writeFile( + path.join(qwenHome, 'extensions', 'extension-enablement.json'), + JSON.stringify({ inactive: { overrides: ['!*'] } }), + ); + await fsp.mkdir(path.join(qwenHome, '.qwen'), { recursive: true }); + await fsp.writeFile( + path.join(qwenHome, '.qwen', 'settings.json'), + JSON.stringify({ skills: { disabledLevels: ['extension'] } }), + ); + const refresh = vi.spyOn(ExtensionManager.prototype, 'refreshCache'); + const status = await createWorkspaceSkillsStatusProvider()(qwenHome); + expect(status.initialized).toBe(true); + // Discovery is gated: no active extension Skill is listed as usable... + expect(status.skills.some((s) => s.name === 'active-skill')).toBe(false); + // ...but inactive management entries still appear, matching the child + // producer, which appends them unconditionally. + expect(status.skills).toEqual( expect.arrayContaining([ - expect.objectContaining({ name: 'visible', status: 'ok' }), + expect.objectContaining({ + name: 'inactive-skill', + level: 'extension', + disabledReason: 'inactive_extension', + }), ]), ); + expect(refresh).toHaveBeenCalled(); }); it('re-resolves the extension locale when the configured language changes', async () => { @@ -837,6 +785,7 @@ describe('createWorkspaceSkillsStatusProvider', () => { path.join(workspace, '.qwen', 'settings.json'), JSON.stringify({ general: { language: 'en' } }), ); + const refresh = vi.spyOn(ExtensionManager.prototype, 'refreshCache'); const provider = createWorkspaceSkillsStatusProvider(); const readNames = async () => (await provider(workspace)).skills @@ -850,12 +799,11 @@ describe('createWorkspaceSkillsStatusProvider', () => { path.join(workspace, '.qwen', 'settings.json'), JSON.stringify({ general: { language: 'zh' } }), ); - // No provider.invalidate: a language change reaches no invalidation - // point, so the locale is part of the cache key. expect(await readNames()).toEqual([ 'active-skill:扩展', 'inactive-skill:扩展', ]); + expect(refresh).toHaveBeenCalledTimes(1); }); it('ignores a non-string general.language instead of failing the catalog', async () => { @@ -874,91 +822,6 @@ describe('createWorkspaceSkillsStatusProvider', () => { expect(status.skills.some((s) => s.name === 'review')).toBe(true); }); - it('does not install a pre-mutation snapshot when invalidate lands mid-build', async () => { - await writeExtension('first', ['first-skill']); - let release!: () => void; - const gate = new Promise((resolve) => { - release = resolve; - }); - vi.spyOn(ExtensionManager.prototype, 'refreshCache').mockImplementationOnce( - () => gate, - ); - const provider = createWorkspaceSkillsStatusProvider(); - const first = provider(qwenHome); - // Commit a store mutation and deliver the invalidation while the cold - // build is still in flight. - await writeExtension('second', ['second-skill']); - provider.invalidate?.(qwenHome); - release(); - await first; - const second = await provider(qwenHome); - expect(second.skills).toEqual( - expect.arrayContaining([ - expect.objectContaining({ name: 'second-skill', status: 'ok' }), - ]), - ); - }); - - it('starts a fresh build for post-invalidation callers and lets later callers join it', async () => { - await writeExtension('first', ['first-skill']); - const realRefresh = ExtensionManager.prototype.refreshCache; - const parked: Array<() => void> = []; - const releases: Array<() => void> = []; - const parkedPromises = [ - new Promise((resolve) => parked.push(resolve)), - new Promise((resolve) => parked.push(resolve)), - ]; - let refreshCalls = 0; - const refreshSpy = vi - .spyOn(ExtensionManager.prototype, 'refreshCache') - .mockImplementation(async function (this: ExtensionManager) { - const index = refreshCalls++; - await realRefresh.call(this); - // Park the first two builds after their store reads: the first must - // hold a pre-mutation snapshot, and both must be in flight while the - // invalidation, the first settle, and the joining caller land. A - // third build means the join failed, so let it run through. - if (index > 1) return; - parked[index]?.(); - await new Promise((resolve) => { - releases[index] = resolve; - }); - }); - const provider = createWorkspaceSkillsStatusProvider(); - - const first = provider(qwenHome); - // Commit the store mutation and deliver the invalidation only after the - // first build's snapshot is taken, or the mutation races that read. - await parkedPromises[0]; - await writeExtension('second', ['second-skill']); - provider.invalidate?.(qwenHome); - // The newer epoch must start a fresh build, not join the pre-mutation one. - const second = provider(qwenHome); - releases[0]!(); - await first; - // The superseded build's cleanup must not evict the fresh build's entry. - const third = provider(qwenHome); - await parkedPromises[1]; - releases[1]!(); - const [firstStatus, secondStatus, thirdStatus] = await Promise.all([ - first, - second, - third, - ]); - - expect(firstStatus.skills.some((s) => s.name === 'second-skill')).toBe( - false, - ); - expect(refreshSpy).toHaveBeenCalledTimes(2); - expect(third).toBe(second); - expect(thirdStatus).toBe(secondStatus); - expect(secondStatus.skills).toEqual( - expect.arrayContaining([ - expect.objectContaining({ name: 'second-skill', status: 'ok' }), - ]), - ); - }); - it('lists a duplicate Skill name within one inactive extension only once', async () => { const directory = path.join(qwenHome, 'extensions', 'inactive'); for (const dir of ['a', 'b']) { @@ -981,50 +844,37 @@ describe('createWorkspaceSkillsStatusProvider', () => { expect(status.skills.filter((s) => s.name === 'dup')).toHaveLength(1); }); - it.each([ - { language: 'zh', envLanguage: '', expected: '扩展' }, - { language: 'zh-CN', envLanguage: '', expected: '扩展' }, - { language: 'en', envLanguage: 'zh', expected: '扩展' }, - { language: 'auto', envLanguage: '', expected: '扩展' }, - { language: 'en', envLanguage: '', expected: 'Extension' }, - ])( - 'resolves extension names for $language with env $envLanguage', - async ({ language, envLanguage, expected }) => { - vi.stubEnv('QWEN_CODE_LANG', envLanguage); - vi.stubEnv('LANG', 'zh_CN.UTF-8'); - for (const name of ['active', 'inactive']) { - const directory = await writeExtension(name, [`${name}-skill`]); - const manifestPath = path.join(directory, 'qwen-extension.json'); - const manifest = JSON.parse(await fsp.readFile(manifestPath, 'utf8')); - await fsp.writeFile( - manifestPath, - JSON.stringify({ - ...manifest, - displayName: { en: 'Extension', zh: '扩展' }, - }), - ); - } - await fsp.writeFile( - path.join(qwenHome, 'extensions', 'extension-enablement.json'), - JSON.stringify({ inactive: { overrides: ['!*'] } }), + it.skipIf(process.platform === 'win32')( + 'fails closed for a dangling symlink at the extensions root', + async () => { + await fsp.symlink( + path.join(qwenHome, 'missing-target'), + path.join(qwenHome, 'extensions'), ); - await fsp.mkdir(path.join(qwenHome, '.qwen'), { recursive: true }); + const status = await createWorkspaceSkillsStatusProvider()(qwenHome); + expect(status.initialized).toBe(false); + expect(status.skills).toEqual([]); + expect(status.errors?.[0]?.error).toContain('ENOENT'); + }, + ); + + it.skipIf(process.platform === 'win32' || process.getuid?.() === 0)( + 'fails closed for a searchable but unlistable extensions root', + async () => { + const extensionsRoot = path.join(qwenHome, 'extensions'); + await fsp.mkdir(extensionsRoot); await fsp.writeFile( - path.join(qwenHome, '.qwen', 'settings.json'), - JSON.stringify({ general: { language } }), + path.join(extensionsRoot, 'extension-enablement.json'), + JSON.stringify({}), ); - const status = await createWorkspaceSkillsStatusProvider()(qwenHome); - expect(status.initialized).toBe(true); - expect( - status.skills.filter((s) => s.level === 'extension'), - ).toMatchObject([ - { name: 'active-skill', extensionDisplayName: expected, status: 'ok' }, - { - name: 'inactive-skill', - extensionDisplayName: expected, - disabledReason: 'inactive_extension', - }, - ]); + await fsp.chmod(extensionsRoot, 0o111); + try { + const status = await createWorkspaceSkillsStatusProvider()(qwenHome); + expect(status.initialized).toBe(false); + expect(status.errors?.[0]?.error).toContain('EACCES'); + } finally { + await fsp.chmod(extensionsRoot, 0o755); + } }, ); }); diff --git a/packages/cli/src/serve/workspace-skills-status.ts b/packages/cli/src/serve/workspace-skills-status.ts index b7e2ce77494..91bc53c0a38 100644 --- a/packages/cli/src/serve/workspace-skills-status.ts +++ b/packages/cli/src/serve/workspace-skills-status.ts @@ -35,6 +35,7 @@ import { SkillManager, Storage, isSafeModeEnv, + getExtensionDisplayName, } from '@qwen-code/qwen-code-core'; import type { Config, SkillLevel } from '@qwen-code/qwen-code-core'; import type { ServeWorkspaceSkillsStatus } from '@qwen-code/acp-bridge/status'; @@ -85,27 +86,6 @@ type SkillManagerConfigShim = Pick< interface WorkspaceSkillManagers { skillManager: SkillManager; extensionManager?: ExtensionManager; - locale: string; -} - -/** - * Fails closed on an unreadable directory while tolerating one that does not - * exist. `lstat` decides absence and `readdir` decides readability: a - * dangling symlink lstat()s fine but readdir()s `ENOENT`, and a - * present-but-unlistable root must fail closed rather than read as absent - * (`fs.stat` cannot separate the two — it follows the link and throws the - * same `ENOENT`). The store loader swallows listing errors, so without this - * probe an unlistable root would silently yield a catalog missing its - * entries. - */ -async function assertReadableDir(directory: string): Promise { - try { - await fs.lstat(directory); - } catch (error) { - if ((error as NodeJS.ErrnoException).code === 'ENOENT') return; - throw error; - } - await fs.readdir(directory); } export function createWorkspaceSkillsStatusProvider( @@ -117,51 +97,20 @@ export function createWorkspaceSkillsStatusProvider( // fallback, so slight staleness between explicit invalidation points is // acceptable: the live child re-lists authoritatively once a session exists. const managers = new Map(); - // Per-workspace invalidation epochs, bumped synchronously by `invalidate`. - // A cold build captures the epoch before its first await and installs only - // while it is unchanged, so an invalidation delivered mid-build cannot be - // undone by that build's own `managers.set`. - const epochs = new Map(); - const inFlight = new Map< - string, - { epoch: number; promise: Promise } - >(); - const provider = ((workspaceCwd: string) => { - // Coalesce concurrent cold builds of one workspace; a caller arriving - // after an invalidation (newer epoch) starts a fresh build instead of - // joining a pre-mutation one. - const epoch = epochs.get(workspaceCwd) ?? 0; - const pending = inFlight.get(workspaceCwd); - if (pending?.epoch === epoch) return pending.promise; - const promise = buildWorkspaceSkillsStatus( + const provider = ((workspaceCwd: string) => + buildWorkspaceSkillsStatus( workspaceCwd, managers, - epochs, - epoch, options.workspaceTrusted ?? true, options.includeUntrustedSkills ?? false, - ); - inFlight.set(workspaceCwd, { epoch, promise }); - const clear = () => { - if (inFlight.get(workspaceCwd)?.promise === promise) { - inFlight.delete(workspaceCwd); - } - }; - void promise.then(clear, clear); - return promise; - }) as WorkspaceSkillsStatusProvider; - provider.invalidate = (workspaceCwd) => { - managers.delete(workspaceCwd); - epochs.set(workspaceCwd, (epochs.get(workspaceCwd) ?? 0) + 1); - }; + )) as WorkspaceSkillsStatusProvider; + provider.invalidate = (workspaceCwd) => managers.delete(workspaceCwd); return provider; } async function buildWorkspaceSkillsStatus( workspaceCwd: string, managers: Map, - epochs: Map, - epoch: number, workspaceTrusted: boolean, includeUntrustedSkills: boolean, ): Promise { @@ -172,11 +121,6 @@ async function buildWorkspaceSkillsStatus( skipWorkspaceSettings: !workspaceTrusted, workspaceTrusted, }); - // Resolve the extension locale from this call's settings: a language - // change reaches no invalidation point, so the locale is part of the - // cache key. Settings carry no value validation, so guard the raw value — - // a non-string `general.language` would otherwise throw inside locale - // resolution and fail the whole catalog. const rawLanguage = settings.merged.general?.language; const locale = resolveLanguage( resolveLanguageSetting( @@ -184,10 +128,6 @@ async function buildWorkspaceSkillsStatus( ), ); let cached = managers.get(workspaceCwd); - if (cached && cached.locale !== locale) { - managers.delete(workspaceCwd); - cached = undefined; - } if (!cached) { // Mirror the CLI guard in loadCliConfig: safe mode nullifies // disabledSkillLevels so the child session loads all bundled skills. @@ -206,39 +146,21 @@ async function buildWorkspaceSkillsStatus( const safeMode = (!workspaceTrusted && !includeUntrustedSkills) || isSafeModeEnv(); let extensionManager: ExtensionManager | undefined; - // A failed extension load is served without extension Skills but not - // cached, so the next call retries enumeration. - let extensionLoadFailed = false; if (workspaceTrusted && !safeMode) { - // A workspace that disabled extension discovery opted out of the - // catalog-fatal tier: for it an unreadable root degrades like any - // other load fault instead of failing the whole catalog. - const extensionLevelDisabled = disabledLevels.has('extension'); - // The store loader swallows listing errors, so without the root - // probe an unlistable root would silently yield a catalog missing - // every extension Skill. Only the probe is catalog-fatal (and only - // while discovery is enabled); a fault inside the load itself - // degrades the extension entries either way. - let rootProbed = false; - try { - await assertReadableDir(Storage.getUserExtensionsDir()); - rootProbed = true; - extensionManager = new ExtensionManager({ - workspaceDir: workspaceCwd, - isWorkspaceTrusted: workspaceTrusted, - locale, + const directory = Storage.getUserExtensionsDir(); + const entry = await fs + .lstat(directory) + .catch((error: NodeJS.ErrnoException) => { + if (error.code === 'ENOENT') return undefined; + throw error; }); - await extensionManager.refreshCache(); - } catch (error) { - if (!rootProbed && !extensionLevelDisabled) throw error; - extensionLoadFailed = true; - extensionManager = undefined; - writeStderrLine( - `qwen serve: extension skill enumeration skipped for ${workspaceCwd}: ${ - error instanceof Error ? error.message : String(error) - }`, - ); - } + if (entry) await fs.readdir(directory); + extensionManager = new ExtensionManager({ + workspaceDir: workspaceCwd, + isWorkspaceTrusted: workspaceTrusted, + locale, + }); + await extensionManager.refreshCache(); } const shim: SkillManagerConfigShim = { // Honor the safe-mode env the same way `Config` does when no explicit @@ -249,10 +171,6 @@ async function buildWorkspaceSkillsStatus( // bare, so it is always off here. getBareMode: () => false, getProjectRoot: () => workspaceCwd, - // SkillManager applies the disabled-level gate itself (through - // getDisabledSkillLevels) before ever calling this; inactive-extension - // management entries are appended regardless, matching the child - // producer. getActiveExtensions: () => extensionManager?.getLoadedExtensions().filter((e) => e.isActive) ?? [], @@ -263,18 +181,19 @@ async function buildWorkspaceSkillsStatus( for (const level of ['project', 'user'] as const) { if (disabledLevels.has(level)) continue; for (const directory of skillManager.getSkillsBaseDirs(level)) { - await assertReadableDir(directory); + try { + await fs.readdir(directory); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') { + throw error; + } + } } } } - cached = { skillManager, extensionManager, locale }; - if (!extensionLoadFailed && (epochs.get(workspaceCwd) ?? 0) === epoch) { - managers.set(workspaceCwd, cached); - } + cached = { skillManager, extensionManager }; + managers.set(workspaceCwd, cached); } - // Settings re-load on every call, while the extension store snapshot - // stays frozen in the cached manager until invalidation — the two - // freshness clocks are deliberate for this best-effort fallback. const { disablements, enabledNames } = resolveSkillSettings(settings); const { skillManager, extensionManager } = cached; const extensions = extensionManager?.getLoadedExtensions() ?? []; @@ -288,7 +207,13 @@ async function buildWorkspaceSkillsStatus( extensionManager && extension ? extensionManager.getExtensionSkillState(extension.id, skill.name) : undefined; - return mapSkillConfigToStatus(skill, disablements, { + const localizedSkill = extension?.config._rawLocalizable?.displayName + ? { + ...skill, + extensionDisplayName: getExtensionDisplayName(extension, locale), + } + : skill; + return mapSkillConfigToStatus(localizedSkill, disablements, { enabled: !state || enabledNames.has(skill.name.trim().toLowerCase()) || @@ -307,7 +232,10 @@ async function buildWorkspaceSkillsStatus( ...skill, level: 'extension', extensionName: extension.name, - extensionDisplayName: extension.displayName, + extensionDisplayName: extension.config._rawLocalizable + ?.displayName + ? getExtensionDisplayName(extension, locale) + : extension.displayName, }, disablements, { disabled: true }, From 8d5907ed268b7f26208455ce79f44f3e5b780340 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8F=B6=E5=85=AC?= Date: Tue, 8 Sep 2026 16:11:04 +0800 Subject: [PATCH 6/8] fix(daemon): skip absent extension stores during skill reads --- docs/design/daemon-extension-skill-catalog.md | 14 ++++-- .../src/serve/workspace-skills-status.test.ts | 48 ++++++++++++++++++- .../cli/src/serve/workspace-skills-status.ts | 38 ++++++++------- 3 files changed, 77 insertions(+), 23 deletions(-) diff --git a/docs/design/daemon-extension-skill-catalog.md b/docs/design/daemon-extension-skill-catalog.md index 89761f1c0d8..56c62f2a84e 100644 --- a/docs/design/daemon-extension-skill-catalog.md +++ b/docs/design/daemon-extension-skill-catalog.md @@ -34,8 +34,12 @@ Discovery-level disabling suppresses active extension Skills through `SkillManager`; inactive extension management entries are still appended, as in the child producer. Safe mode and untrusted contexts never load extensions. -Enumeration errors, including extension-store faults, return -`initialized: false` with explicit errors. This stage does not certify an -incomplete extension inventory as a successful response. Existing facade -caching, source preference and invalidation behavior remain unchanged; the -tracking issue assigns cache lifecycle and concurrency changes to stage 4. +An absent extensions root is an empty inventory; no extension store is created. +Unreadable roots and errors propagated by the shared store/loader return +`initialized: false` with explicit errors. Individual artifact handling remains +owned by the shared loader: malformed manifests are skipped with its diagnostic, +whereas a dangling extension entry propagates an error. This stage does not add +per-artifact diagnostics to the response or change the loader's failure policy. +Existing facade caching, source preference and invalidation behavior remain +unchanged; the tracking issue assigns cache lifecycle and concurrency changes +to stage 4. diff --git a/packages/cli/src/serve/workspace-skills-status.test.ts b/packages/cli/src/serve/workspace-skills-status.test.ts index 860bdeae14c..b66fd05bd0d 100644 --- a/packages/cli/src/serve/workspace-skills-status.test.ts +++ b/packages/cli/src/serve/workspace-skills-status.test.ts @@ -490,6 +490,7 @@ describe('createWorkspaceSkillsStatusProvider', () => { status: 'disabled', disabledReason: 'inactive_extension', extensionName: 'inactive', + extensionDisplayName: 'inactive display', }), ]), ); @@ -841,7 +842,9 @@ describe('createWorkspaceSkillsStatusProvider', () => { JSON.stringify({ inactive: { overrides: ['!*'] } }), ); const status = await createWorkspaceSkillsStatusProvider()(qwenHome); - expect(status.skills.filter((s) => s.name === 'dup')).toHaveLength(1); + const duplicates = status.skills.filter((s) => s.name === 'dup'); + expect(duplicates).toHaveLength(1); + expect(duplicates[0]).not.toHaveProperty('extensionDisplayName'); }); it.skipIf(process.platform === 'win32')( @@ -877,4 +880,47 @@ describe('createWorkspaceSkillsStatusProvider', () => { } }, ); + + it('does not create an extension store when no extensions directory exists', async () => { + const refresh = vi.spyOn(ExtensionManager.prototype, 'refreshCache'); + const status = await createWorkspaceSkillsStatusProvider()(qwenHome); + expect(status.initialized).toBe(true); + expect(status.skills.some((skill) => skill.name === 'review')).toBe(true); + expect(refresh).not.toHaveBeenCalled(); + await expect( + fsp.stat(path.join(qwenHome, 'extensions')), + ).rejects.toMatchObject({ code: 'ENOENT' }); + await expect( + fsp.stat(path.join(qwenHome, 'extension-store')), + ).rejects.toMatchObject({ code: 'ENOENT' }); + }); + + it('keeps the shared loader behavior for malformed extension manifests', async () => { + await writeExtension('healthy', ['healthy-skill']); + const broken = await writeExtension('broken', ['broken-skill']); + await fsp.writeFile(path.join(broken, 'qwen-extension.json'), '{invalid'); + const status = await createWorkspaceSkillsStatusProvider()(qwenHome); + expect(status.initialized).toBe(true); + expect(status.errors).toBeUndefined(); + expect( + status.skills + .filter((skill) => skill.level === 'extension') + .map((skill) => skill.name), + ).toEqual(['healthy-skill']); + }); + + it.skipIf(process.platform === 'win32')( + 'reports propagated errors for a dangling extension entry', + async () => { + await writeExtension('healthy', ['healthy-skill']); + await fsp.symlink( + path.join(qwenHome, 'missing'), + path.join(qwenHome, 'extensions', 'dangling'), + ); + const status = await createWorkspaceSkillsStatusProvider()(qwenHome); + expect(status.initialized).toBe(false); + expect(status.skills).toEqual([]); + expect(status.errors?.[0]?.error).toContain('ENOENT'); + }, + ); }); diff --git a/packages/cli/src/serve/workspace-skills-status.ts b/packages/cli/src/serve/workspace-skills-status.ts index 91bc53c0a38..6bc3a4fa2ad 100644 --- a/packages/cli/src/serve/workspace-skills-status.ts +++ b/packages/cli/src/serve/workspace-skills-status.ts @@ -154,13 +154,15 @@ async function buildWorkspaceSkillsStatus( if (error.code === 'ENOENT') return undefined; throw error; }); - if (entry) await fs.readdir(directory); - extensionManager = new ExtensionManager({ - workspaceDir: workspaceCwd, - isWorkspaceTrusted: workspaceTrusted, - locale, - }); - await extensionManager.refreshCache(); + if (entry) { + await fs.readdir(directory); + extensionManager = new ExtensionManager({ + workspaceDir: workspaceCwd, + isWorkspaceTrusted: workspaceTrusted, + locale, + }); + await extensionManager.refreshCache(); + } } const shim: SkillManagerConfigShim = { // Honor the safe-mode env the same way `Config` does when no explicit @@ -207,12 +209,14 @@ async function buildWorkspaceSkillsStatus( extensionManager && extension ? extensionManager.getExtensionSkillState(extension.id, skill.name) : undefined; - const localizedSkill = extension?.config._rawLocalizable?.displayName - ? { - ...skill, - extensionDisplayName: getExtensionDisplayName(extension, locale), - } - : skill; + // Preserve missing display names; the helper otherwise falls back to the name. + const localizedSkill = + extension?.displayName !== undefined + ? { + ...skill, + extensionDisplayName: getExtensionDisplayName(extension, locale), + } + : skill; return mapSkillConfigToStatus(localizedSkill, disablements, { enabled: !state || @@ -232,10 +236,10 @@ async function buildWorkspaceSkillsStatus( ...skill, level: 'extension', extensionName: extension.name, - extensionDisplayName: extension.config._rawLocalizable - ?.displayName - ? getExtensionDisplayName(extension, locale) - : extension.displayName, + extensionDisplayName: + extension.displayName === undefined + ? undefined + : getExtensionDisplayName(extension, locale), }, disablements, { disabled: true }, From cf66404d03961c6c6065b493316f846286a10801 Mon Sep 17 00:00:00 2001 From: qwen-code-bot Date: Tue, 8 Sep 2026 20:20:36 +0000 Subject: [PATCH 7/8] docs(daemon): record skills-catalog read contract and later-stage items (#11281) Co-authored-by: Qwen-Coder --- docs/design/daemon-extension-skill-catalog.md | 34 ++++++++++++++++--- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/docs/design/daemon-extension-skill-catalog.md b/docs/design/daemon-extension-skill-catalog.md index 56c62f2a84e..731e7be13dc 100644 --- a/docs/design/daemon-extension-skill-catalog.md +++ b/docs/design/daemon-extension-skill-catalog.md @@ -35,11 +35,35 @@ Discovery-level disabling suppresses active extension Skills through the child producer. Safe mode and untrusted contexts never load extensions. An absent extensions root is an empty inventory; no extension store is created. -Unreadable roots and errors propagated by the shared store/loader return -`initialized: false` with explicit errors. Individual artifact handling remains -owned by the shared loader: malformed manifests are skipped with its diagnostic, -whereas a dangling extension entry propagates an error. This stage does not add -per-artifact diagnostics to the response or change the loader's failure policy. +With a present root the read reconciles through the shared store: a missing or +drifted store is initialized in place under the store's exclusive lock, +rewriting `extension-store/state.json`, creating its `state.previous.json` +rollback copy, and rewriting the legacy `extension-enablement.json` projection. +Once reconciled, later reads write nothing. Unreadable roots and errors +propagated by the shared store/loader return `initialized: false` with explicit +errors, and that failure is deliberately all-or-nothing: one bad extension +artifact (for example a dangling directory entry) fails the entire catalog, +project, user and bundled Skills included, until the artifact is repaired. +Individual artifact handling remains owned by the shared loader: malformed +manifests are skipped with its diagnostic, whereas a dangling extension entry +propagates an error. This stage does not add per-artifact diagnostics or +per-level degradation to the response, or change the loader's failure policy. Existing facade caching, source preference and invalidation behavior remain unchanged; the tracking issue assigns cache lifecycle and concurrency changes to stage 4. + +**Known later-stage items (recorded during review, deliberately not in this +stage):** + +- The sibling `/workspace/extensions` route resolves its locale through + `loadSettings` without `skipLoadEnvironment`, so a workspace `.env` + `QWEN_CODE_LANG` can diverge from this provider's language resolution. +- The inactive-entry append and sort assembly duplicates the child producer's + (`acpAgent.ts`), keeping the child's `level:extensionName:name` dedupe key. +- Extension mutations do not invalidate the config-catalog providers this stage + populates, so a committed install, update, enable/disable or uninstall can + leave stale extension Skill state on the skills config routes until an + unrelated skill mutation, a workspace removal, or a restart. The + invalidation wiring and `refreshCacheIfSourcesChanged` revalidation belong + to stage 4. +- The active-Skill `enabled` judgment mirrors `Config.isSkillEnabled` by hand. From 95859798cf9ff9677f1d2c0dd3c9beca30f1f67d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8F=B6=E5=85=AC?= Date: Wed, 9 Sep 2026 13:59:00 +0800 Subject: [PATCH 8/8] fix(daemon): resolve skill state from the loaded extension snapshot --- docs/design/daemon-extension-skill-catalog.md | 28 +++++-- .../daemon-extension-skill-catalog.zh-CN.md | 69 +++++++++++++++++ .../src/serve/workspace-skills-status.test.ts | 74 +++++++++++++++++-- .../cli/src/serve/workspace-skills-status.ts | 46 +++++++++--- 4 files changed, 193 insertions(+), 24 deletions(-) create mode 100644 docs/design/daemon-extension-skill-catalog.zh-CN.md diff --git a/docs/design/daemon-extension-skill-catalog.md b/docs/design/daemon-extension-skill-catalog.md index 731e7be13dc..f87f1d8ec38 100644 --- a/docs/design/daemon-extension-skill-catalog.md +++ b/docs/design/daemon-extension-skill-catalog.md @@ -1,8 +1,10 @@ # Daemon extension Skill catalog -This implements stage 2 of #11274. The daemon-local workspace Skill provider -currently supplies an empty active-extension list, so its first response omits -installed extension Skills when no child snapshot exists. +[English](daemon-extension-skill-catalog.md) | [简体中文](daemon-extension-skill-catalog.zh-CN.md) + +This implements stage 2 of #11274. Previously the daemon-local workspace Skill +provider supplied an empty active-extension list, so its first response omitted +installed extension Skills when no child snapshot existed. Use an unbound `ExtensionManager` for the selected workspace to load installed extensions through the existing consistent store reader. Supply active @@ -11,6 +13,12 @@ precedence. Append inactive extension Skills as management entries with the existing `inactive_extension` status, retaining their identity and metadata. Resolve settings and extension Skill defaults/overrides with the existing parsers. A settings opt-in does not enable an inactive parent extension. +Resolve each extension's Skill defaults from its own manifest, with workspace +Skill overrides from the same consistent store snapshot used to load it. Cache +those booleans by the loaded extension object and normalized Skill name. This +avoids selecting the wrong owner through a colliding ID; it does not create a +new identity namespace or change store policy. Explicit opt-ins and hard +settings disablements retain their existing precedence. Resolve localized extension names with the existing language setting and locale helpers on every response, without changing the daemon process language or rebuilding the directory cache when only the language changes. @@ -39,9 +47,11 @@ With a present root the read reconciles through the shared store: a missing or drifted store is initialized in place under the store's exclusive lock, rewriting `extension-store/state.json`, creating its `state.previous.json` rollback copy, and rewriting the legacy `extension-enablement.json` projection. -Once reconciled, later reads write nothing. Unreadable roots and errors -propagated by the shared store/loader return `initialized: false` with explicit -errors, and that failure is deliberately all-or-nothing: one bad extension +Cache hits reuse the loaded managers. A later rebuild still takes the lock and +may perform directory/permission maintenance even if policy is unchanged. +Unreadable roots and errors propagated by the shared store/loader return +`initialized: false` with explicit errors. That failure is deliberately +all-or-nothing: one bad extension artifact (for example a dangling directory entry) fails the entire catalog, project, user and bundled Skills included, until the artifact is repaired. Individual artifact handling remains owned by the shared loader: malformed @@ -59,7 +69,8 @@ stage):** `loadSettings` without `skipLoadEnvironment`, so a workspace `.env` `QWEN_CODE_LANG` can diverge from this provider's language resolution. - The inactive-entry append and sort assembly duplicates the child producer's - (`acpAgent.ts`), keeping the child's `level:extensionName:name` dedupe key. + (`acpAgent.ts`). Here a per-extension name set removes duplicate inactive + Skill names; it provides the same source separation without that string key. - Extension mutations do not invalidate the config-catalog providers this stage populates, so a committed install, update, enable/disable or uninstall can leave stale extension Skill state on the skills config routes until an @@ -67,3 +78,6 @@ stage):** invalidation wiring and `refreshCacheIfSourcesChanged` revalidation belong to stage 4. - The active-Skill `enabled` judgment mirrors `Config.isSkillEnabled` by hand. +- Shared store/API handling of duplicate extension IDs remains separate. This + provider preserves per-manifest defaults instead of querying an ambiguous + ID for its owner; it still consumes workspace overrides by the store's ID key. diff --git a/docs/design/daemon-extension-skill-catalog.zh-CN.md b/docs/design/daemon-extension-skill-catalog.zh-CN.md new file mode 100644 index 00000000000..7717c294e76 --- /dev/null +++ b/docs/design/daemon-extension-skill-catalog.zh-CN.md @@ -0,0 +1,69 @@ +# Daemon extension Skill 目录 + +[English](daemon-extension-skill-catalog.md) | [简体中文](daemon-extension-skill-catalog.zh-CN.md) + +本设计实现 #11274 的第 2 阶段。此前 daemon 本地 workspace Skill provider +提供空的 active extension 列表，因此没有 child 快照时，首次响应缺少已安装 +extension 的 Skill。 + +为选定 workspace 使用不绑定运行时 Config 的 `ExtensionManager`，通过现有 +store 一致性读取加载扩展。将 active extension 交给 `SkillManager`，保留 +project > user > extension > bundled 优先级。将 inactive extension 的 Skill +作为管理条目追加，使用既有 `inactive_extension` 状态，保留身份和元数据。 +设置、extension Skill 默认值与覆盖沿用现有解析器。settings 显式启用不会启用 +inactive 父扩展。 + +每个 extension 的 Skill 默认值来自其自身 manifest；workspace Skill 覆盖来自 +加载扩展时使用的同一份 store 一致性快照。按已加载 extension 对象和归一化 +Skill 名称缓存布尔结果，避免通过冲突 ID 再次选错 owner。这不新增身份命名空间， +也不修改 store 策略。显式启用与 settings 硬禁用保持原有优先级。 + +每次响应通过现有语言设置和 locale helper 解析本地化扩展名，不修改 daemon +进程语言，也不因单独的语言变化重建目录缓存。 + +保持轻量 Config 接口：不构造运行时 Config、不启动 child、不初始化 MCP、 +不执行 hooks、不安装 watcher。遵守 safe mode、发现层级禁用及 workspace 信任 +规则；未信任目录的静态盘点不加载 workspace 设置或 extension 运行时上下文。 +目录读取失败继续返回未初始化错误状态。 + +实现与相邻回归测试位于 daemon 本地 provider。测试覆盖真实 manifest、 +active/inactive 状态、来源冲突、持久化 Skill 设置、safe/untrusted 上下文及 +显式缓存失效。E2E 使用隔离 home 和无 child 会话的 daemon。 + +本阶段 facade 仍优先使用 child 快照。替换来源、修改开关或刷新语义、增加配置 +状态字段及修改 Web Shell 投影属于后续 PR。不修改公共响应 schema。 + +发现层级禁用通过 `SkillManager` 隐藏 active extension Skill；与 child producer +一致，仍追加 inactive extension 管理条目。safe mode 和未信任上下文不加载扩展。 + +extensions 根目录不存在代表空目录，不创建 extension store。根目录存在时， +通过共享 store 协调读取：缺失或发生漂移的 store 在独占锁内初始化，写入 +`extension-store/state.json`、生成 `state.previous.json` 回滚副本，并写入旧格式 +`extension-enablement.json` 投影。命中缓存时复用已加载 manager；后续重建仍会 +获取锁，即使策略未变也可能维护目录与权限。 + +不可读根目录和共享 store/loader 抛出的错误返回 `initialized: false` 及明确 +错误。该失败有意影响整份目录：例如一个悬空 extension 目录条目会使 project、 +user 和 bundled Skill 一同不可用，直到制品被修复。单个制品的处理仍由共享 +loader 决定：损坏 manifest 被跳过并记录其诊断，悬空条目则向外抛错。本阶段不 +增加逐制品诊断或逐层降级，不修改 loader 失败策略。 + +现有 facade 缓存、来源优先级及失效行为不变；跟踪 issue 将缓存生命周期和并发 +改造安排在第 4 阶段。 + +**评审中记录、明确不在本阶段实现的后续事项：** + +- 相邻 `/workspace/extensions` 路由调用 `loadSettings` 时未设置 + `skipLoadEnvironment`，workspace `.env` 的 `QWEN_CODE_LANG` 可能使其语言 + 解析与本 provider 不同。 +- inactive 条目追加及排序与 child producer（`acpAgent.ts`）存在重复。这里按 + extension 分别对 Skill 名去重，与字符串 `level:extensionName:name` key + 提供相同的来源区分，并未使用该字符串 key。 +- extension 变更没有失效本阶段填充的 config-catalog provider，因此安装、更新、 + 启停或卸载提交后，skills config 路由可能保留旧状态，直到无关 Skill 变更、 + workspace 移除或重启。失效接线与 `refreshCacheIfSourcesChanged` 重新验证 + 属于第 4 阶段。 +- active Skill 的 `enabled` 判断沿用 `Config.isSkillEnabled` 的规则，尚未共享 + 判定实现。 +- 共享 store/API 的重复 extension ID 处理另行跟进。本 provider 从各自 manifest + 保留默认值，不通过歧义 ID 查询 owner；workspace 覆盖仍使用 store 的 ID key。 diff --git a/packages/cli/src/serve/workspace-skills-status.test.ts b/packages/cli/src/serve/workspace-skills-status.test.ts index b66fd05bd0d..728d1b3e3d7 100644 --- a/packages/cli/src/serve/workspace-skills-status.test.ts +++ b/packages/cli/src/serve/workspace-skills-status.test.ts @@ -604,7 +604,10 @@ describe('createWorkspaceSkillsStatusProvider', () => { async (mode) => { await writeExtension('suite', ['hidden']); if (mode === 'safe') vi.stubEnv('QWEN_CODE_SAFE_MODE', '1'); - const refresh = vi.spyOn(ExtensionManager.prototype, 'refreshCache'); + const refresh = vi.spyOn( + ExtensionManager.prototype, + 'refreshCacheWithSnapshot', + ); const status = await createWorkspaceSkillsStatusProvider({ workspaceTrusted: mode !== 'untrusted' && mode !== 'inert-untrusted', includeUntrustedSkills: mode === 'inert-untrusted', @@ -669,9 +672,10 @@ describe('createWorkspaceSkillsStatusProvider', () => { it('does not cache a failed store read as an initialized empty catalog', async () => { await writeExtension('suite', ['visible']); - vi.spyOn(ExtensionManager.prototype, 'refreshCache').mockRejectedValueOnce( - new Error('store unavailable'), - ); + vi.spyOn( + ExtensionManager.prototype, + 'refreshCacheWithSnapshot', + ).mockRejectedValueOnce(new Error('store unavailable')); const provider = createWorkspaceSkillsStatusProvider(); expect(await provider(qwenHome)).toMatchObject({ initialized: false, @@ -743,7 +747,10 @@ describe('createWorkspaceSkillsStatusProvider', () => { path.join(qwenHome, '.qwen', 'settings.json'), JSON.stringify({ skills: { disabledLevels: ['extension'] } }), ); - const refresh = vi.spyOn(ExtensionManager.prototype, 'refreshCache'); + const refresh = vi.spyOn( + ExtensionManager.prototype, + 'refreshCacheWithSnapshot', + ); const status = await createWorkspaceSkillsStatusProvider()(qwenHome); expect(status.initialized).toBe(true); // Discovery is gated: no active extension Skill is listed as usable... @@ -786,7 +793,10 @@ describe('createWorkspaceSkillsStatusProvider', () => { path.join(workspace, '.qwen', 'settings.json'), JSON.stringify({ general: { language: 'en' } }), ); - const refresh = vi.spyOn(ExtensionManager.prototype, 'refreshCache'); + const refresh = vi.spyOn( + ExtensionManager.prototype, + 'refreshCacheWithSnapshot', + ); const provider = createWorkspaceSkillsStatusProvider(); const readNames = async () => (await provider(workspace)).skills @@ -882,7 +892,10 @@ describe('createWorkspaceSkillsStatusProvider', () => { ); it('does not create an extension store when no extensions directory exists', async () => { - const refresh = vi.spyOn(ExtensionManager.prototype, 'refreshCache'); + const refresh = vi.spyOn( + ExtensionManager.prototype, + 'refreshCacheWithSnapshot', + ); const status = await createWorkspaceSkillsStatusProvider()(qwenHome); expect(status.initialized).toBe(true); expect(status.skills.some((skill) => skill.name === 'review')).toBe(true); @@ -923,4 +936,51 @@ describe('createWorkspaceSkillsStatusProvider', () => { expect(status.errors?.[0]?.error).toContain('ENOENT'); }, ); + + it('preserves each manifest default and store override when extension ids collide', async () => { + for (const [name, type, source] of [ + ['first', 'git', 'https://github.com/example/suite'], + ['second', 'github-release', 'https://github.com/example/suite.git'], + ]) { + const directory = await writeExtension(name!, [`${name}-skill`], { + [`${name}-skill`]: name !== 'second', + }); + await fsp.writeFile( + path.join(directory, '.qwen-extension-install.json'), + JSON.stringify({ type, source }), + ); + } + const manager = new ExtensionManager({ + workspaceDir: qwenHome, + isWorkspaceTrusted: true, + }); + await manager.refreshCache(); + const [first, second] = manager.getLoadedExtensions(); + expect(first!.id).toBe(second!.id); + const provider = createWorkspaceSkillsStatusProvider(); + for (let i = 0; i < 2; i++) { + const status = await provider(qwenHome); + expect(status.initialized).toBe(true); + expect( + status.skills.filter((s) => s.level === 'extension'), + ).toMatchObject([ + { name: 'first-skill', status: 'ok' }, + { name: 'second-skill', status: 'disabled', disabledReason: 'default' }, + ]); + } + const store = new ExtensionStore(); + await store.setSkillWorkspaceOverrides( + second!, + qwenHome, + { 'first-skill': false, 'second-skill': true }, + 0, + ); + provider.invalidate?.(qwenHome); + const status = await provider(qwenHome); + expect(status.initialized).toBe(true); + expect(status.skills.filter((s) => s.level === 'extension')).toMatchObject([ + { name: 'first-skill', status: 'disabled', disabledReason: 'default' }, + { name: 'second-skill', status: 'ok' }, + ]); + }); }); diff --git a/packages/cli/src/serve/workspace-skills-status.ts b/packages/cli/src/serve/workspace-skills-status.ts index 6bc3a4fa2ad..5225319c0d8 100644 --- a/packages/cli/src/serve/workspace-skills-status.ts +++ b/packages/cli/src/serve/workspace-skills-status.ts @@ -32,12 +32,13 @@ import { ExtensionManager, + ExtensionStore, SkillManager, Storage, isSafeModeEnv, getExtensionDisplayName, } from '@qwen-code/qwen-code-core'; -import type { Config, SkillLevel } from '@qwen-code/qwen-code-core'; +import type { Config, Extension, SkillLevel } from '@qwen-code/qwen-code-core'; import type { ServeWorkspaceSkillsStatus } from '@qwen-code/acp-bridge/status'; import { STATUS_SCHEMA_VERSION } from '@qwen-code/acp-bridge/status'; import * as fs from 'node:fs/promises'; @@ -86,6 +87,7 @@ type SkillManagerConfigShim = Pick< interface WorkspaceSkillManagers { skillManager: SkillManager; extensionManager?: ExtensionManager; + extensionSkillStates: Map>; } export function createWorkspaceSkillsStatusProvider( @@ -146,6 +148,7 @@ async function buildWorkspaceSkillsStatus( const safeMode = (!workspaceTrusted && !includeUntrustedSkills) || isSafeModeEnv(); let extensionManager: ExtensionManager | undefined; + const extensionSkillStates = new Map>(); if (workspaceTrusted && !safeMode) { const directory = Storage.getUserExtensionsDir(); const entry = await fs @@ -156,12 +159,35 @@ async function buildWorkspaceSkillsStatus( }); if (entry) { await fs.readdir(directory); + const extensionStore = new ExtensionStore(); extensionManager = new ExtensionManager({ + extensionStore, workspaceDir: workspaceCwd, isWorkspaceTrusted: workspaceTrusted, locale, }); - await extensionManager.refreshCache(); + const snapshot = await extensionManager.refreshCacheWithSnapshot(); + for (const extension of extensionManager.getLoadedExtensions()) { + const states = new Map(); + for (const skill of extension.skills ?? []) { + const name = skill.name.trim().toLowerCase(); + const defaults = extension.config.skillStates; + const defaultEnabled = + defaults && Object.hasOwn(defaults, name) + ? defaults[name]! + : true; + states.set( + name, + extensionStore.getSkillWorkspaceOverride( + snapshot, + extension.id, + workspaceCwd, + name, + ) ?? defaultEnabled, + ); + } + extensionSkillStates.set(extension, states); + } } } const shim: SkillManagerConfigShim = { @@ -193,11 +219,11 @@ async function buildWorkspaceSkillsStatus( } } } - cached = { skillManager, extensionManager }; + cached = { skillManager, extensionManager, extensionSkillStates }; managers.set(workspaceCwd, cached); } const { disablements, enabledNames } = resolveSkillSettings(settings); - const { skillManager, extensionManager } = cached; + const { skillManager, extensionManager, extensionSkillStates } = cached; const extensions = extensionManager?.getLoadedExtensions() ?? []; const skills = await skillManager.listSkills(); const statuses = skills.map((skill) => { @@ -205,10 +231,11 @@ async function buildWorkspaceSkillsStatus( skill.level === 'extension' ? extensions.find((e) => e.name === skill.extensionName) : undefined; - const state = - extensionManager && extension - ? extensionManager.getExtensionSkillState(extension.id, skill.name) - : undefined; + const enabled = extension + ? extensionSkillStates + .get(extension) + ?.get(skill.name.trim().toLowerCase()) + : undefined; // Preserve missing display names; the helper otherwise falls back to the name. const localizedSkill = extension?.displayName !== undefined @@ -219,9 +246,8 @@ async function buildWorkspaceSkillsStatus( : skill; return mapSkillConfigToStatus(localizedSkill, disablements, { enabled: - !state || enabledNames.has(skill.name.trim().toLowerCase()) || - (state.workspaceEnabled ?? state.defaultEnabled), + enabled !== false, }); }); for (const extension of extensions) {