diff --git a/apps/cli/src/plugin.ts b/apps/cli/src/plugin.ts index 391634814c..1ffcdbceae 100644 --- a/apps/cli/src/plugin.ts +++ b/apps/cli/src/plugin.ts @@ -137,7 +137,7 @@ async function runManaged( let interrupted = 0 const interrupt = (): void => { interrupted = 130; controller.abort() } const terminate = (): void => { interrupted = 143; controller.abort() } - const control = { requestId: randomUUID() as PluginInstallRequestId, signal: controller.signal, prepared() {} } + const control = { requestId: randomUUID() as PluginInstallRequestId, signal: controller.signal } process.on('SIGINT', interrupt) process.on('SIGTERM', terminate) try { diff --git a/docs/subsystems/core.i18n.yaml b/docs/subsystems/core.i18n.yaml index 8a014cd966..3639fe1700 100644 --- a/docs/subsystems/core.i18n.yaml +++ b/docs/subsystems/core.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write docs/subsystems/core.md -core.md: 33085c8035abc96dc9117d5758dfa93018ed0563 -core.zh.md: 7ee2fac7b079e746c6f9c519acf94fb49273fe9b +core.md: 6f3bd0ec50b7dbc7704503a0371428efcd33a8e7 +core.zh.md: 2d95002999a58180cc830744d566e1b9866fe06f diff --git a/docs/subsystems/core.md b/docs/subsystems/core.md index 33085c8035..6f3bd0ec50 100644 --- a/docs/subsystems/core.md +++ b/docs/subsystems/core.md @@ -919,7 +919,7 @@ The row injects only the Loader; the profile runtime and the agent registry are /** * Install a package with pnpm, read its declarations, and leave it disabled unless asked otherwise. * @param spec - what to install, in pnpm's own vocabulary. - * @param options - `enable` puts every newly installed bundle into the layer list at once. + * @param options - `enable` selects new bundles; `requestId` identifies the install for progress and cancellation. * @returns what the run installed and enabled. */ @Remote('add') async add(spec: string, options?: PluginInstallOptions): Promise diff --git a/docs/subsystems/core.zh.md b/docs/subsystems/core.zh.md index 7ee2fac7b0..2d95002999 100644 --- a/docs/subsystems/core.zh.md +++ b/docs/subsystems/core.zh.md @@ -929,7 +929,7 @@ The row injects only the Loader; the profile runtime and the agent registry are /** * Install a package with pnpm, read its declarations, and leave it disabled unless asked otherwise. * @param spec - what to install, in pnpm's own vocabulary. - * @param options - `enable` puts every newly installed bundle into the layer list at once. + * @param options - `enable` selects new bundles; `requestId` identifies the install for progress and cancellation. * @returns what the run installed and enabled. */ @Remote('add') async add(spec: string, options?: PluginInstallOptions): Promise diff --git a/packages/boot/plugin-manager/README.i18n.yaml b/packages/boot/plugin-manager/README.i18n.yaml index 1250ea37b0..5834fc2ecb 100644 --- a/packages/boot/plugin-manager/README.i18n.yaml +++ b/packages/boot/plugin-manager/README.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write packages/boot/plugin-manager/README.md -README.md: e044fe9e9f042aadf3255f819b327e6d39428c4d -README.zh.md: 8a13c15fa5545f5475ffecccf128dcfb72be9b76 +README.md: 208c315b28bca2a987c5f50f5a6e7b40e70cd4a1 +README.zh.md: 8d4734513ec6a6c1090441bd13688a072cdda6b4 diff --git a/packages/boot/plugin-manager/README.md b/packages/boot/plugin-manager/README.md index e044fe9e9f..208c315b28 100644 --- a/packages/boot/plugin-manager/README.md +++ b/packages/boot/plugin-manager/README.md @@ -84,7 +84,7 @@ Every refusal or failure is a `PluginOperationError` with a stable `code` and `d ----- -Installation callers may supply a UUID `requestId` in `add` options and call `cancelInstall(requestId)`. `plugins/install-state` announces installation, cancellation, and the non-cancellable application phase. Cancellation waits for pnpm's process range to exit, restores the manifest and lockfile, and releases the mutation lock before returning `cancelled`; the original add reports `plugins/install-cancelled`. A request for another operation returns `not-running`, and application returns `too-late`. Plugin disposal stops an installation still preparing packages without waiting recursively on runtime application. +Installation callers may supply a UUID `requestId` in `add` options and call `cancelInstall(requestId)`. `plugins/install-state` announces installation, cancellation, and the non-cancellable application phase. Cancellation waits for pnpm's process range to exit, restores the manifest and lockfile, and releases the mutation lock before returning `cancelled`; the original add reports `plugins/install-cancelled`. A request for another operation returns `not-running`, and application returns `too-late`. Manager disposal waits for package preparation or removal to stop, without waiting recursively on runtime application. ## Understand the implementation diff --git a/packages/boot/plugin-manager/README.zh.md b/packages/boot/plugin-manager/README.zh.md index 8a13c15fa5..8d4734513e 100644 --- a/packages/boot/plugin-manager/README.zh.md +++ b/packages/boot/plugin-manager/README.zh.md @@ -84,7 +84,7 @@ console.log(await manager.list()) ----- -安装调用方可在 `add` 选项中提供 UUID `requestId`,并调用 `cancelInstall(requestId)`。`plugins/install-state` 通知安装、取消及不可取消的配置应用阶段。取消会等待 pnpm 进程组退出、恢复清单和 `pnpm-lock.yaml`、释放修改锁,然后返回 `cancelled`;原 add 调用报告 `plugins/install-cancelled`。请求不匹配当前操作时返回 `not-running`,已进入应用阶段则返回 `too-late`。插件卸载会停止仍在准备包的安装,不反向等待 runtime 应用过程。下载或解包文件可能留在 `node_modules` 或 pnpm 缓存中。 +安装调用方可在 `add` 选项中提供 UUID `requestId`,并调用 `cancelInstall(requestId)`。`plugins/install-state` 通知安装、取消及不可取消的配置应用阶段。取消会等待 pnpm 进程组退出、恢复清单和 `pnpm-lock.yaml`、释放修改锁,然后返回 `cancelled`;原 add 调用报告 `plugins/install-cancelled`。请求不匹配当前操作时返回 `not-running`,已进入应用阶段则返回 `too-late`。管理器随 Host 卸载时,会等待包准备或移除进程停止,不反向等待 runtime 应用过程。下载或解包文件可能留在 `node_modules` 或 pnpm 缓存中。 ## 理解实现 diff --git a/packages/boot/plugin-manager/src/installer.ts b/packages/boot/plugin-manager/src/installer.ts index d09aa97325..ef9ec5d84c 100644 --- a/packages/boot/plugin-manager/src/installer.ts +++ b/packages/boot/plugin-manager/src/installer.ts @@ -32,7 +32,7 @@ import type { PluginInstallLogChunk, PluginInstallRejection, PluginInstallResult export interface PluginInstallControl { readonly requestId: PluginInstallRequestId readonly signal: AbortSignal - readonly prepared: () => void + readonly prepared?: () => void } /** Check cancellation between subprocesses and before committing the installation. */ @@ -167,7 +167,7 @@ export class PluginInstaller { * Undeclared packages remain installed; new bundles are left disabled. * @param spec - what to install, in pnpm's own vocabulary: a registry * name, a `github:` or git URL, a tarball, or an absolute path. - * @param control - cancellation and the handoff to runtime application; omitted by standalone CLI operations. + * @param control - optional cancellation and the handoff to runtime application. * @returns what the run installed and what it removed again. * @throws {PluginOperationError} `plugins/bad-request` for an empty spec, * `plugins/install-failed` when pnpm exits non-zero, cannot be spawned, or times out; @@ -203,7 +203,7 @@ export class PluginInstaller { removed.push({ name, reason }) } checkCancelled(control) - control?.prepared() + control?.prepared?.() const kept = new Set(installed) return { installed, @@ -225,7 +225,7 @@ export class PluginInstaller { /** * Run `pnpm remove` and reconcile the layer list. * @param packageName - the dependency to remove. - * @param control - the enclosing installation when removing a rejected new package. + * @param control - optional cancellation for this package operation. * @throws {PluginOperationError} `plugins/install-failed` when pnpm fails. */ async remove(packageName: string, control?: PluginInstallControl): Promise { diff --git a/packages/boot/plugin-manager/src/manager.ts b/packages/boot/plugin-manager/src/manager.ts index 209a42368d..0a556f2d2f 100644 --- a/packages/boot/plugin-manager/src/manager.ts +++ b/packages/boot/plugin-manager/src/manager.ts @@ -54,6 +54,7 @@ interface ActiveMutation { readonly operation: string readonly subject: string install?: ActiveInstall + stopFiles?: () => Promise } /** What {@link PluginManager} needs beyond the Cordis context it reads the tree through. */ @@ -95,6 +96,7 @@ export class PluginManager { const install = this.active?.install // Runtime application may itself unload this plugin; never join that recompose from its disposer. if (install !== undefined && install.phase !== 'applying') await this.cancelInstall(install.requestId) + else await this.active?.stopFiles?.() }) } @@ -187,7 +189,7 @@ export class PluginManager { * `plugins/install-log` chunks carrying the returned `jobId`. * @param spec - what to install, in pnpm's own vocabulary: a registry * name, a `github:` or git URL, a tarball, or an absolute path. - * @param options - `enable` puts every newly installed bundle into the layer list at once. + * @param options - `enable` selects new bundles; `requestId` identifies the install for progress and cancellation. * @returns what the run installed and enabled. * @throws {PluginOperationError} `plugins/agents-running` while a session * runs, `plugins/install-failed` when pnpm exits non-zero or the run times @@ -271,10 +273,10 @@ export class PluginManager { * or `plugins/install-failed` when pnpm exits non-zero. */ async uninstall(packageName: string): Promise { - return this.exclusive('uninstall', packageName, () => this.uninstallNow(packageName)) + return this.exclusive('uninstall', packageName, active => this.uninstallNow(packageName, active)) } - private async uninstallNow(packageName: string): Promise { + private async uninstallNow(packageName: string, active: ActiveMutation): Promise { const runtime = this.runtime() const installer = this.installer(runtime) installer.assertInstalled(packageName) @@ -286,7 +288,20 @@ export class PluginManager { for (const reference of references) { await this.editLayer(runtime, (document) => { document.removeInsert(reference.rowId) }) } - await installer.remove(packageName) + if (this.disposed) { + throw new PluginOperationError('plugins/unavailable', `${NAME}: manager was disposed before package removal`, { reason: 'manager disposed' }) + } + const controller = new AbortController() + const settled = Promise.withResolvers() + active.stopFiles = async () => { controller.abort(); await settled.promise } + try { + await installer.remove(packageName, { + requestId: randomUUID() as PluginInstallRequestId, signal: controller.signal, + }) + } finally { + delete active.stopFiles + settled.resolve(undefined) + } if (references.length > 0 && runtime.patchReload === 'live') { await runtime.recompose() } diff --git a/packages/boot/plugin-manager/src/types.ts b/packages/boot/plugin-manager/src/types.ts index 9ef2a09fee..d1b5cd35b9 100644 --- a/packages/boot/plugin-manager/src/types.ts +++ b/packages/boot/plugin-manager/src/types.ts @@ -172,7 +172,7 @@ export type PluginChangeReason = 'install' | 'uninstall' | 'enable' | 'disable' /** One chunk of an install run's output. */ export interface PluginInstallLogChunk { - /** The enclosing installation; absent for standalone package removal. */ + /** The owning operation identifier when a cancellation controller is present. */ readonly requestId?: PluginInstallRequestId /** The run the chunk belongs to. */ readonly jobId: string diff --git a/packages/boot/plugin-manager/tests/plugin-manager.spec.ts b/packages/boot/plugin-manager/tests/plugin-manager.spec.ts index 66e35b6686..d55a9b82c0 100644 --- a/packages/boot/plugin-manager/tests/plugin-manager.spec.ts +++ b/packages/boot/plugin-manager/tests/plugin-manager.spec.ts @@ -13,7 +13,7 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { PassThrough } from 'node:stream' import { execFileSync } from 'node:child_process' -import { afterEach, describe, expect, it } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import { Context, type Plugin } from '@deepseek-ai/cordis' import Loader from '@deepseek-ai/cordis-plugin-loader' import { @@ -21,7 +21,7 @@ import { type ComposedStack, type Profile, readPackageMetadata, } from '@deepseek-ai/dsh-app-boot' import { - PluginManager, type PluginInstallLogChunk, + PluginManager, PluginInstaller, type PluginInstallLogChunk, type PluginInstallRequestId, type PluginInstallProgress, type PluginToolingConfig, type SpawnLike, } from '@deepseek-ai/dsh-plugin-manager' import type {} from '@deepseek-ai/dsh-agent' @@ -756,6 +756,55 @@ describe('PluginManager', () => { expect(await answer).toMatchObject({ code: 'plugins/install-cancelled' }) }) + it('uses the standalone installer without a runtime or cancellation controller', async () => { + const staged = await stageHome() + stagePackage(staged.profileDir, 'ext-plain', {}) + const log: PluginInstallLogChunk[] = [] + const installer = new PluginInstaller({ + profileDir: staged.profileDir, profileName: 'web', installAnchor: staged.anchor, + loadProfile: () => loadProfile(NAME, 'web', staged.anchor, staged.home), + config: managerConfig(), color: false, spawn: recordingPnpm(staged.profileDir), installLog: (chunk) => { log.push(chunk) }, + }) + expect(await installer.add('ext-plain')).toMatchObject({ installed: ['ext-plain'] }) + expect(manifestOf(staged.profileDir).dependencies).toHaveProperty('ext-plain') + expect(log.every(chunk => chunk.requestId === undefined)).toBe(true) + await installer.remove('ext-plain') + expect(manifestOf(staged.profileDir).dependencies).not.toHaveProperty('ext-plain') + }) + + it('does not start package removal after removing its user row disposes the manager', async () => { + const staged = await stageHome() + stagePackage(staged.profileDir, 'ext-old', { main: 'export function apply() {}' }) + writeFileSync(join(staged.profileDir, 'cordis.patch.yml'), '- insert:\n - id: old-row\n name: ext-old\n') + addDependency(staged.profileDir, 'ext-old') + const calls: string[][] = [] + const { ctx, manager, runtime } = await bootProfile(staged, { spawn: recordingPnpm(staged.profileDir, calls) }) + const recompose = runtime.recompose.bind(runtime) + vi.spyOn(runtime, 'recompose').mockImplementationOnce(async (options) => { + const result = await recompose(options) + await ctx.fiber.dispose() + return result + }) + await expect(manager.uninstall('ext-old')).rejects.toMatchObject({ code: 'plugins/unavailable', details: { reason: 'manager disposed' } }) + expect(calls).toEqual([]) + }) + + it('joins a package removal process during host disposal without exposing installation cancellation for it', async () => { + const staged = await stageHome() + stagePackage(staged.profileDir, 'ext-old', {}) + addDependency(staged.profileDir, 'ext-old') + const started = Promise.withResolvers() + const { ctx, manager, log } = await bootProfile(staged, { spawn: fakePnpm(staged.profileDir, () => { + started.resolve(undefined) + return { code: null, hang: true, stdout: 'removing' } + }) }) + const result = manager.uninstall('ext-old').catch((error: unknown) => error) + await started.promise + expect(await manager.cancelInstall(log[0]?.requestId as PluginInstallRequestId)).toEqual({ status: 'not-running' }) + await ctx.fiber.dispose() + expect(await result).toMatchObject({ code: 'plugins/install-cancelled' }) + }) + it('refuses a second mutation while one is still running', async () => { const staged = await stageHome() stagePackage(staged.profileDir, 'ext-slow', { patch: BUNDLE_ONE_ROW }) diff --git a/packages/extensions/tool-cordis/src/api-catalog.ts b/packages/extensions/tool-cordis/src/api-catalog.ts index 1d73c07226..3b81d195aa 100644 --- a/packages/extensions/tool-cordis/src/api-catalog.ts +++ b/packages/extensions/tool-cordis/src/api-catalog.ts @@ -1411,7 +1411,7 @@ export const SERVICE_API: readonly ServiceApiEntry[] = [ { signature: '@Remote(\'add\') async add(spec: string, options?: PluginInstallOptions): Promise', description: 'Install a package with pnpm, read its declarations, and leave it disabled unless asked otherwise.', - parameters: [{ name: 'spec', description: 'what to install, in pnpm\'s own vocabulary.' }, { name: 'options', description: '`enable` puts every newly installed bundle into the layer list at once.' }], + parameters: [{ name: 'spec', description: 'what to install, in pnpm\'s own vocabulary.' }, { name: 'options', description: '`enable` selects new bundles; `requestId` identifies the install for progress and cancellation.' }], returns: 'what the run installed and enabled.', }, { diff --git a/packages/host/plugin-manager/src/index.ts b/packages/host/plugin-manager/src/index.ts index 2d46935840..1f2979b487 100644 --- a/packages/host/plugin-manager/src/index.ts +++ b/packages/host/plugin-manager/src/index.ts @@ -138,7 +138,7 @@ export class PluginManagerRemote extends TypertRemoteService { /** * Install a package with pnpm, read its declarations, and leave it disabled unless asked otherwise. * @param spec - what to install, in pnpm's own vocabulary. - * @param options - `enable` puts every newly installed bundle into the layer list at once. + * @param options - `enable` selects new bundles; `requestId` identifies the install for progress and cancellation. * @returns what the run installed and enabled. */ @Remote('add')