fix(plugins): join package removal when the manager is disposed

This commit is contained in:
Yichen Jiang committed 2026-09-14 22:53:10 +08:00
1 parent cf992b52f9
commit 70f6f30817
13 files changed
+86 -22

No files matched your search

+1 -1
View File
@@ -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 {
+2 -2
View File
@@ -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
+1 -1
View File
@@ -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<PluginInstallResult>
+1 -1
View File
@@ -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<PluginInstallResult>
@@ -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
+1 -1
View File
@@ -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.
<a id="understand-the-implementation"></a>
## Understand the implementation
+1 -1
View File
@@ -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 缓存中。
<a id="understand-the-implementation"></a>
## 理解实现
@@ -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<void> {
+19 -4
View File
@@ -54,6 +54,7 @@ interface ActiveMutation {
readonly operation: string
readonly subject: string
install?: ActiveInstall
stopFiles?: () => Promise<void>
}
/** 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<void> {
return this.exclusive('uninstall', packageName, () => this.uninstallNow(packageName))
return this.exclusive('uninstall', packageName, active => this.uninstallNow(packageName, active))
}
private async uninstallNow(packageName: string): Promise<void> {
private async uninstallNow(packageName: string, active: ActiveMutation): Promise<void> {
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<undefined>()
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()
}
+1 -1
View File
@@ -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
@@ -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<undefined>()
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 })
@@ -1411,7 +1411,7 @@ export const SERVICE_API: readonly ServiceApiEntry[] = [
{
signature: '@Remote(\'add\') async add(spec: string, options?: PluginInstallOptions): Promise<PluginInstallResult>',
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.',
},
{
+1 -1
View File
@@ -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')