From e3386d110577c60afb5d0056005a3221104558f2 Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Fri, 4 Sep 2026 13:41:20 +0800 Subject: [PATCH] feat(plugins): judge an install after pnpm, restore the manifest on failure, run one mutation at a time A successful `pnpm add` is no longer the end of `plugins/install`: a new package that declares neither a bundle nor a plugin module, or a bundle whose row id a composed layer already owns, is removed again and reported under `removed` with its reason, while a package the probe refused stays for the view to explain. The profile manifest is snapshotted before the run and restored when pnpm fails. The manager refuses a second mutation while one runs (`plugins/busy`) instead of racing on the manifest and user layers, and `install`/`uninstall` refuse while a session is running (`plugins/agents-running`), because pnpm rewrites the directory those sessions import from. `resolveProfileLayer` factors one bundle's layer out of `loadProfile` so the manager can judge a candidate against the composed layers. --- ...manager-over-the-profile-runtime.i18n.yaml | 4 +- ...plugin-manager-over-the-profile-runtime.md | 6 +- ...gin-manager-over-the-profile-runtime.zh.md | 6 +- docs/config-catalog.i18n.yaml | 4 +- docs/config-catalog.md | 2 +- docs/config-catalog.zh.md | 2 +- packages/boot/app-boot/src/index.ts | 1 + packages/boot/app-boot/src/profile.ts | 57 +++--- .../extensions/tool-cordis/src/api-catalog.ts | 6 +- packages/host/plugin-manager/README.i18n.yaml | 4 +- packages/host/plugin-manager/README.md | 4 +- packages/host/plugin-manager/README.zh.md | 4 +- packages/host/plugin-manager/package.json | 2 + packages/host/plugin-manager/src/index.ts | 174 +++++++++++++++--- packages/host/plugin-manager/src/types.ts | 20 +- .../tests/plugin-manager.spec.ts | 116 +++++++++++- packages/host/plugin-manager/tsconfig.json | 3 + pnpm-lock.yaml | 41 +++-- 18 files changed, 373 insertions(+), 83 deletions(-) diff --git a/.agents/notes/implemented/architecture/2026-09-04-plugin-manager-over-the-profile-runtime.i18n.yaml b/.agents/notes/implemented/architecture/2026-09-04-plugin-manager-over-the-profile-runtime.i18n.yaml index 5014e8a675..3e44406d65 100644 --- a/.agents/notes/implemented/architecture/2026-09-04-plugin-manager-over-the-profile-runtime.i18n.yaml +++ b/.agents/notes/implemented/architecture/2026-09-04-plugin-manager-over-the-profile-runtime.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 .agents/notes/implemented/architecture/2026-09-04-plugin-manager-over-the-profile-runtime.md -2026-09-04-plugin-manager-over-the-profile-runtime.md: 8b12c1dcbee5e5f463fcba181441717de5b20c39 -2026-09-04-plugin-manager-over-the-profile-runtime.zh.md: 2f1fa6682124677c0614f24ef005f3efc212bd57 +2026-09-04-plugin-manager-over-the-profile-runtime.md: 493ddad8a151c1fa3e9d926ba0d7c4c39f301e10 +2026-09-04-plugin-manager-over-the-profile-runtime.zh.md: 4e8e4a400f84b6b65b67ebd2d6d60d2bc45e8a08 diff --git a/.agents/notes/implemented/architecture/2026-09-04-plugin-manager-over-the-profile-runtime.md b/.agents/notes/implemented/architecture/2026-09-04-plugin-manager-over-the-profile-runtime.md index 8b12c1dcbe..493ddad8a1 100644 --- a/.agents/notes/implemented/architecture/2026-09-04-plugin-manager-over-the-profile-runtime.md +++ b/.agents/notes/implemented/architecture/2026-09-04-plugin-manager-over-the-profile-runtime.md @@ -16,6 +16,8 @@ Installing a plugin was a terminal-only act: `dsh plugin --profile web add diff --git a/docs/config-catalog.zh.md b/docs/config-catalog.zh.md index cb531c531d..9c365b0e65 100644 --- a/docs/config-catalog.zh.md +++ b/docs/config-catalog.zh.md @@ -914,7 +914,7 @@ export interface Config { } ``` -来源:[`packages/host/plugin-manager/src/index.ts:96`](../packages/host/plugin-manager/src/index.ts) +来源:[`packages/host/plugin-manager/src/index.ts:98`](../packages/host/plugin-manager/src/index.ts) diff --git a/packages/boot/app-boot/src/index.ts b/packages/boot/app-boot/src/index.ts index 98dcef4c45..18c2e96d48 100644 --- a/packages/boot/app-boot/src/index.ts +++ b/packages/boot/app-boot/src/index.ts @@ -44,6 +44,7 @@ export { readProfileManifest, resolveBundleDir, resolveProfileDir, + resolveProfileLayer, writeProfileManifest, type BundleStage, type BundleTrust, diff --git a/packages/boot/app-boot/src/profile.ts b/packages/boot/app-boot/src/profile.ts index 6131beeb26..8e3b15a6bd 100644 --- a/packages/boot/app-boot/src/profile.ts +++ b/packages/boot/app-boot/src/profile.ts @@ -849,6 +849,41 @@ export function resolveBundleDir( ) } +/** + * Resolve one bundle into the layer it contributes: its directory through + * the two resolution anchors, its patch list, its trust from the profile + * manifest, and its stage from the profile's `dsh.profile.stages` override or + * its own `dsh.bundle.stage`. + * @param binName - the diagnostic prefix on thrown errors. + * @param manifest - the profile manifest, for trust and stage overrides. + * @param packageName - the bundle package. + * @param installAnchor - absolute path of the dsh app's package.json (first resolution anchor). + * @param dir - the profile directory (second resolution anchor). + * @returns the resolved layer. + * @throws when the package cannot be resolved, declares no `dsh.bundle`, or names an unknown stage. + */ +export function resolveProfileLayer( + binName: string, manifest: ProfileManifest, packageName: string, installAnchor: string, dir: string, +): ProfileLayer { + const packageDir = resolveBundleDir(binName, packageName, installAnchor, dir) + const bundleManifest = JSON.parse(readFileSync(join(packageDir, 'package.json'), 'utf8')) as ProfileManifest + const declared = bundleManifest.dsh?.bundle?.patch + if (declared === undefined) { + throw new Error(`${binName}: profile bundle ${JSON.stringify(packageName)} declares no dsh.bundle in its package.json`) + } + const patchPath = join(packageDir, declared) + const stages = manifest.dsh?.profile?.stages ?? {} + return { + packageName, + version: bundleManifest.version, + packageDir, + patchPath, + trust: layerTrust(manifest, packageName), + stage: readBundleStage(binName, packageName, stages[packageName] ?? bundleManifest.dsh?.bundle?.stage), + patches: loadOverlayPatches(binName, patchPath), + } +} + /** * Load a profile: resolve every `dsh.profile.bundles` entry to its patch * layer and parse the profile's own patch file. A listed bundle without a @@ -887,27 +922,7 @@ export function loadProfile( ) } const patchReload = rawPatchReload ?? DEFAULT_PROFILE_PATCH_RELOAD - const stages = manifest.dsh?.profile?.stages ?? {} - const layers = bundles.map((packageName): ProfileLayer => { - const packageDir = resolveBundleDir(binName, packageName, installAnchor, dir) - const bundleManifest = JSON.parse(readFileSync(join(packageDir, 'package.json'), 'utf8')) as ProfileManifest - const declared = bundleManifest.dsh?.bundle?.patch - if (declared === undefined) { - throw new Error(`${binName}: profile bundle ${JSON.stringify(packageName)} declares no dsh.bundle in its package.json`) - } - const patchPath = join(packageDir, declared) - const trust = layerTrust(manifest, packageName) - const stage = readBundleStage(binName, packageName, stages[packageName] ?? bundleManifest.dsh?.bundle?.stage) - return { - packageName, - version: bundleManifest.version, - packageDir, - patchPath, - trust, - stage, - patches: loadOverlayPatches(binName, patchPath), - } - }) + const layers = bundles.map(packageName => resolveProfileLayer(binName, manifest, packageName, installAnchor, dir)) const patchPath = join(dir, PROFILE_PATCH_FILENAME) const patches = options.userLayer !== false && existsSync(patchPath) ? loadOverlayPatches(binName, patchPath) diff --git a/packages/extensions/tool-cordis/src/api-catalog.ts b/packages/extensions/tool-cordis/src/api-catalog.ts index 1f792307e4..b819b15580 100644 --- a/packages/extensions/tool-cordis/src/api-catalog.ts +++ b/packages/extensions/tool-cordis/src/api-catalog.ts @@ -4648,9 +4648,13 @@ export const TYPE_API: readonly TypeApiEntry[] = [ name: 'PluginInstallLogChunk', declaration: 'export interface PluginInstallLogChunk {\n readonly jobId: string;\n readonly spec: string;\n readonly stream: \'stdout\' | \'stderr\';\n readonly text: string;\n readonly exitCode?: number | null;\n}', }, + { + name: 'PluginInstallRejection', + declaration: 'export interface PluginInstallRejection {\n readonly name: string;\n readonly reason: string;\n}', + }, { name: 'PluginInstallResult', - declaration: 'export interface PluginInstallResult {\n readonly installed: readonly string[];\n readonly enabled: readonly string[];\n readonly installedOnly: readonly string[];\n readonly plain: readonly string[];\n readonly jobId: string;\n}', + declaration: 'export interface PluginInstallResult {\n readonly installed: readonly string[];\n readonly removed: readonly PluginInstallRejection[];\n readonly enabled: readonly string[];\n readonly installedOnly: readonly string[];\n readonly plain: readonly string[];\n readonly jobId: string;\n}', }, { name: 'PluginPackageAddableView', diff --git a/packages/host/plugin-manager/README.i18n.yaml b/packages/host/plugin-manager/README.i18n.yaml index d638533b79..a660b9a963 100644 --- a/packages/host/plugin-manager/README.i18n.yaml +++ b/packages/host/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/host/plugin-manager/README.md -README.md: 6541e5230616643fe7c4dc2f2ff10d2ac52e2bb7 -README.zh.md: ef1bb5c2214dd18f4cdec35ba616bb84aeb96f93 +README.md: d155ce2a77bf303ea0f05b41c8116c794db88172 +README.zh.md: 63df4863c52aebd0935046b27a3ce116f74bf10c diff --git a/packages/host/plugin-manager/README.md b/packages/host/plugin-manager/README.md index 6541e52306..d155ce2a77 100644 --- a/packages/host/plugin-manager/README.md +++ b/packages/host/plugin-manager/README.md @@ -33,11 +33,11 @@ Mount the row in a host composition beside the plugin inventory; the web bundle ### Installing and enabling -`plugins/install` takes a pnpm spec — a registry name, a `github:` or git URL, a tarball, an absolute path — runs `pnpm add` in the profile directory, records what pnpm wrote to `dependencies`, probes every new package in a child process, and leaves new bundles disabled unless `enable` was asked for. pnpm's output arrives as `plugins/install-log` chunks carrying the run's `jobId`; the last chunk carries the exit code. A non-zero exit, a spawn failure, or the timeout fails the call with `plugins/install-failed` and the tail of the log. +`plugins/install` takes a pnpm spec — a registry name, a `github:` or git URL, a tarball, an absolute path — runs `pnpm add` in the profile directory, records what pnpm wrote to `dependencies`, probes every new package in a child process, and leaves new bundles disabled unless `enable` was asked for. pnpm's output arrives as `plugins/install-log` chunks carrying the run's `jobId`; the last chunk carries the exit code. A non-zero exit, a spawn failure, or the timeout fails the call with `plugins/install-failed` and the tail of the log, and the profile manifest is restored to what it was before the run. A successful `pnpm add` is not yet an installed plugin: a new package that declares neither a bundle nor a plugin module, or a bundle whose row id a composed layer already owns, is removed again with `pnpm remove` and listed under `removed` with the reason; a package the probe refused stays installed and the view reports why it cannot be enabled. The manager runs one mutation at a time — a second call while one runs fails with `plugins/busy` naming the operation in flight — and refuses to change `node_modules` while a session is running, with `plugins/agents-running`. `plugins/enable` puts an installed bundle into the layer list and, on a live profile, recomposes the tree with it through the profile runtime. The recomposition is the Loader's own transaction: a bundle the tree rejects — a `boot`-stage bundle whose row throws — rolls back, the layer list is restored, and the call fails with `plugins/enable-failed` naming the reason, while the tree that was running keeps running. A `runtime`-stage bundle whose row fails is isolated instead: the call succeeds, the view reports the row's failure, and `plugins/retry` composes the bundle again from scratch. `plugins/disable` is the reverse; a template bundle, which is not a dependency, cannot be disabled. On a profile whose `patchReload` is `startup`, both write the manifest and report `effect: 'restart'`. -`plugins/uninstall` disables the bundle when enabled, drops every user-layer row that names one of the package's modules, runs `pnpm remove`, and forgets the probe record. +`plugins/uninstall` disables the bundle when enabled, drops every user-layer row that names one of the package's modules, runs `pnpm remove`, and forgets the probe record. Like `install`, it waits for running sessions: `plugins/agents-running` while any agent is running. ### Rows in user layers diff --git a/packages/host/plugin-manager/README.zh.md b/packages/host/plugin-manager/README.zh.md index ef1bb5c221..63df4863c5 100644 --- a/packages/host/plugin-manager/README.zh.md +++ b/packages/host/plugin-manager/README.zh.md @@ -33,11 +33,11 @@ kind: "package-reference" ### 安装与启用 -`plugins/install` 接受一个 pnpm spec——registry 名字、`github:` 或 git URL、tarball、绝对路径——在 profile 目录运行 `pnpm add`,记录 pnpm 写进 `dependencies` 的内容,在子进程里探测每个新包,并让新组合包保持停用,除非调用方要求 `enable`。pnpm 的输出以带本次 `jobId` 的 `plugins/install-log` 分块到达;最后一块携带退出码。非零退出、spawn 失败或超时都以 `plugins/install-failed` 与日志尾部让调用失败。 +`plugins/install` 接受一个 pnpm spec——registry 名字、`github:` 或 git URL、tarball、绝对路径——在 profile 目录运行 `pnpm add`,记录 pnpm 写进 `dependencies` 的内容,在子进程里探测每个新包,并让新组合包保持停用,除非调用方要求 `enable`。pnpm 的输出以带本次 `jobId` 的 `plugins/install-log` 分块到达;最后一块携带退出码。非零退出、spawn 失败或超时都以 `plugins/install-failed` 与日志尾部让调用失败,并把 profile manifest 恢复到运行前的样子。`pnpm add` 成功还不等于装好了插件:新包既不声明组合包也不声明插件模块,或者是某个行 id 已被已组合层占有的组合包,会再以 `pnpm remove` 移除并连同原因列在 `removed` 里;探针拒绝的包保留在原处,由视图说明它为何不能启用。管理器一次只跑一个变更——上一个还在跑时再调用会以 `plugins/busy` 失败并点名正在进行的操作——并且在有会话运行时拒绝改动 `node_modules`,报 `plugins/agents-running`。 `plugins/enable` 把已安装的组合包放进层列表,并在 live profile 上经 profile runtime 带着它重新组合树。这次重新组合就是 Loader 自己的事务:树拒绝的组合包——`boot` 阶段而行抛错的组合包——回滚,层列表恢复,调用以点名原因的 `plugins/enable-failed` 失败,而原本运行的树继续运行。`runtime` 阶段而行失败的组合包则被隔离:调用成功,视图报告该行的失败,`plugins/retry` 从头重新组合它。`plugins/disable` 是反向操作;模板组合包不是依赖,无法停用。在 `patchReload` 为 `startup` 的 profile 上,两者只写 manifest 并报告 `effect: 'restart'`。 -`plugins/uninstall` 在组合包已启用时先停用它,删除每一条点名该包模块的用户层行,运行 `pnpm remove`,并忘掉探针记录。 +`plugins/uninstall` 在组合包已启用时先停用它,删除每一条点名该包模块的用户层行,运行 `pnpm remove`,并忘掉探针记录。与 `install` 一样,它等待运行中的会话:只要有 agent 在运行就报 `plugins/agents-running`。 ### 用户层里的行 diff --git a/packages/host/plugin-manager/package.json b/packages/host/plugin-manager/package.json index 662f48a2e1..61fb968ebd 100644 --- a/packages/host/plugin-manager/package.json +++ b/packages/host/plugin-manager/package.json @@ -50,6 +50,7 @@ "peerDependencies": { "@deepseek-ai/cordis": "workspace:^", "@deepseek-ai/cordis-plugin-loader": "workspace:^", + "@deepseek-ai/dsh-agent": "workspace:^", "@deepseek-ai/dsh-agent-presets": "workspace:^", "@deepseek-ai/dsh-app-boot": "workspace:^", "@deepseek-ai/dsh-patch-file": "workspace:^", @@ -66,6 +67,7 @@ "@deepseek-ai/cordis-plugin-group": "workspace:^", "@deepseek-ai/cordis-plugin-include": "workspace:^", "@deepseek-ai/cordis-plugin-loader": "workspace:^", + "@deepseek-ai/dsh-agent": "workspace:^", "@deepseek-ai/dsh-agent-presets": "workspace:^", "@deepseek-ai/dsh-app-boot": "workspace:^", "@deepseek-ai/dsh-patch-file": "workspace:^", diff --git a/packages/host/plugin-manager/src/index.ts b/packages/host/plugin-manager/src/index.ts index 72c457a6a2..ec17f3d9b3 100644 --- a/packages/host/plugin-manager/src/index.ts +++ b/packages/host/plugin-manager/src/index.ts @@ -17,7 +17,7 @@ import { spawn as spawnChild, type ChildProcess, type SpawnOptions } from 'node:child_process' import { randomUUID } from 'node:crypto' -import { readFileSync, rmSync } from 'node:fs' +import { readFileSync, rmSync, writeFileSync } from 'node:fs' import { join } from 'node:path' import type { Context, Fiber, FiberState } from '@deepseek-ai/cordis' import type { Entry } from '@deepseek-ai/cordis-plugin-loader' @@ -25,6 +25,7 @@ import z from '@deepseek-ai/schemastery' import type { AgentPresets } from '@deepseek-ai/dsh-agent-presets' import { bundleGroupId, + claimLayerIds, disableBundle, enableBundle, healProfilesModuleFallback, @@ -37,12 +38,14 @@ import { reconcileInstalledBundles, recordContainedStates, resolveBundleDir, + resolveProfileLayer, writeProbeCache, type BundleStage, type PluginProbe, type ProfileManifest, type ProfileRuntime, } from '@deepseek-ai/dsh-app-boot' +import type {} from '@deepseek-ai/dsh-agent' import { mutatePatchFile, readPatchListFile, type PatchRow } from '@deepseek-ai/dsh-patch-file' import type { JsonValue } from '@deepseek-ai/dsh-util-values' import { Remote, RemoteError, TypertRemoteService } from '@deepseek-ai/dsh-typert-protocol' @@ -52,7 +55,7 @@ import type { PluginChangeReason, PluginDependents, PluginEnableResult, - PluginInstallResult, + PluginInstallRejection, PluginInstallResult, PluginPackageAddableView, PluginPackageRowView, PluginPackageStatus, @@ -170,6 +173,46 @@ export class PluginManager extends TypertRemoteService { this.probeRunner = internals.probe ?? probePackage } + /** The mutation in flight, while one is; a second caller is refused rather than queued. */ + private active: { operation: string; subject: string } | undefined + + /** + * Run one mutation with the manager to itself. Every write touches the + * profile manifest, the user layers, or `node_modules`, and two at once + * would race on those files, so a second call while one runs is refused. + * @throws {RemoteError} `plugins/busy` naming the operation in flight. + */ + private async exclusive(operation: string, subject: string, run: () => Promise): Promise { + if (this.active !== undefined) { + throw new RemoteError( + 'plugins/busy', + `plugin-manager: ${operation} ${subject} refused while ${this.active.operation} ${this.active.subject} is still running`, + { operation, subject, active: this.active }, + ) + } + this.active = { operation, subject } + try { + return await run() + } finally { + this.active = undefined + } + } + + /** + * Refuse a change to `node_modules` while a session is running: pnpm + * rewrites the directory the running agents import from. + * @throws {RemoteError} `plugins/agents-running` with the count. + */ + private assertNoRunningAgents(operation: string): void { + const running = (this.ctx.get('agents')?.list() ?? []).filter(agent => agent.status === 'running').length + if (running === 0) return + throw new RemoteError( + 'plugins/agents-running', + `plugin-manager: ${operation} waits for ${String(running)} running session(s) to go idle`, + { operation, running }, + ) + } + /** The profile runtime, or the failure a caller without one receives. */ private runtime(): ProfileRuntime { const runtime = this.ctx.get('profileRuntime') @@ -361,42 +404,101 @@ export class PluginManager extends TypertRemoteService { */ @Remote('install') async install(spec: string, options?: { enable?: boolean }): Promise { + return this.exclusive('install', spec, () => this.installNow(spec, options)) + } + + private async installNow(spec: string, options?: { enable?: boolean }): Promise { const runtime = this.runtime() if (spec.trim().length === 0) { throw new RemoteError('gateway/bad-request', 'plugin-manager: the package spec must not be empty', {}) } + this.assertNoRunningAgents('install') + const manifestPath = join(runtime.dir, 'package.json') + const snapshot = readFileSync(manifestPath, 'utf8') const before = readProfileManifest(NAME, runtime.dir) - const jobId = await this.runPnpm(runtime, ['add', spec], spec) + let jobId: string + try { + jobId = await this.runPnpm(runtime, ['add', spec], spec) + } catch (error) { + // pnpm may have written the manifest before failing; the profile keeps + // the manifest it had, and what pnpm left under node_modules is not a + // dependency until a manifest names it. + if (readFileSync(manifestPath, 'utf8') !== snapshot) writeFileSync(manifestPath, snapshot) + throw error + } const outcome = reconcileInstalledBundles(NAME, runtime.dir, runtime.installAnchor, before, { autoEnable: false }) const after = readProfileManifest(NAME, runtime.dir) - const installed = Object.keys(dependenciesOf(after)).filter(name => !(name in dependenciesOf(before))) + const added = Object.keys(dependenciesOf(after)).filter(name => !(name in dependenciesOf(before))) await healProfilesModuleFallback({ installAnchor: runtime.installAnchor, profile: runtime.current }) - for (const name of installed) { - // A probe that cannot run leaves no record; the view reports the - // package as not enableable with the probe's own reason. - try { - await this.probe(runtime, name) - } catch { - // The failure is re-derived on every list and shown there. + const installed: string[] = [] + const removed: PluginInstallRejection[] = [] + for (const name of added) { + const reason = await this.rejection(runtime, name) + if (reason === undefined) { + installed.push(name) + continue } + await this.removeDependency(runtime, name) + removed.push({ name, reason }) } + const kept = new Set(installed) const enabled: string[] = [] if (options?.enable === true) { for (const name of outcome.installedOnly) { - await this.enable(name) + if (!kept.has(name)) continue + await this.enableNow(name) enabled.push(name) } } this.changed('install') return { installed, + removed, enabled, - installedOnly: outcome.installedOnly.filter(name => !enabled.includes(name)), - plain: outcome.plain, + installedOnly: outcome.installedOnly.filter(name => kept.has(name) && !enabled.includes(name)), + plain: outcome.plain.filter(name => kept.has(name)), jobId, } } + /** + * The post-install check of one package pnpm added: a package that is + * neither a bundle nor a plugin module has no place in a profile, and a + * bundle whose row id another layer already owns could only mount as a + * conflict record. A package the probe cannot run stays installed: the + * view reports it as not enableable with the probe's own reason. + * @returns why the package is removed again, or undefined to keep it. + */ + private async rejection(runtime: ProfileRuntime, name: string): Promise { + let probe: PluginProbe + try { + probe = await this.probe(runtime, name) + } catch { + return undefined // the failure is re-derived on every list and shown there + } + // A refused probe (an import that throws, another cordis copy) keeps the + // package: the view shows it as not enableable with the probe's reason. + if (!probe.ok) return undefined + if (probe.kind === 'library') return 'declares neither a dsh bundle nor a plugin module' + if (probe.kind !== 'bundle') return undefined + try { + const layer = resolveProfileLayer(NAME, readProfileManifest(NAME, runtime.dir), name, runtime.installAnchor, runtime.dir) + const lost = claimLayerIds([...runtime.current.layers, layer]).skipped.get(name) + if (lost === undefined) return undefined + return lost.map(conflict => `row ${JSON.stringify(conflict.rowId)} is already declared by ${conflict.declaredBy}`).join('; ') + } catch (error) { + return messageOf(error) + } + } + + /** Run `pnpm remove`, reconcile the layer list, and forget the probe record. */ + private async removeDependency(runtime: ProfileRuntime, name: string): Promise { + const before = readProfileManifest(NAME, runtime.dir) + await this.runPnpm(runtime, ['remove', name], name) + reconcileInstalledBundles(NAME, runtime.dir, runtime.installAnchor, before, { autoEnable: false }) + rmSync(join(runtime.dir, PLUGIN_PROBE_DIR, `${name.replaceAll('/', '__')}.json`), { force: true }) + } + /** * Remove a package from the profile: disable it when enabled, drop every * user-layer row that names it, run `pnpm remove`, and forget its probe. @@ -405,19 +507,21 @@ export class PluginManager extends TypertRemoteService { */ @Remote('uninstall') async uninstall(packageName: string): Promise { + return this.exclusive('uninstall', packageName, () => this.uninstallNow(packageName)) + } + + private async uninstallNow(packageName: string): Promise { const runtime = this.runtime() this.assertInstalled(runtime, packageName) + this.assertNoRunningAgents('uninstall') const references = await this.rowReferences(runtime, packageName) if (bundlesOf(readProfileManifest(NAME, runtime.dir)).includes(packageName)) { - await this.disable(packageName) + await this.disableNow(packageName) } for (const reference of references) { await this.editLayer(runtime, reference.target, (document) => { document.removeInsert(reference.rowId) }) } - const before = readProfileManifest(NAME, runtime.dir) - await this.runPnpm(runtime, ['remove', packageName], packageName) - reconcileInstalledBundles(NAME, runtime.dir, runtime.installAnchor, before, { autoEnable: false }) - rmSync(join(runtime.dir, PLUGIN_PROBE_DIR, `${packageName.replaceAll('/', '__')}.json`), { force: true }) + await this.removeDependency(runtime, packageName) if (references.some(reference => reference.target.kind === 'global') && runtime.patchReload === 'live') { await runtime.recompose() } @@ -436,6 +540,10 @@ export class PluginManager extends TypertRemoteService { */ @Remote('enable') async enable(packageName: string): Promise { + return this.exclusive('enable', packageName, () => this.enableNow(packageName)) + } + + private async enableNow(packageName: string): Promise { const runtime = this.runtime() this.assertInstalled(runtime, packageName) const probe = await this.probe(runtime, packageName).catch((error: unknown) => { @@ -499,6 +607,10 @@ export class PluginManager extends TypertRemoteService { */ @Remote('disable') async disable(packageName: string): Promise { + return this.exclusive('disable', packageName, () => this.disableNow(packageName)) + } + + private async disableNow(packageName: string): Promise { const runtime = this.runtime() let changed: boolean try { @@ -524,12 +636,16 @@ export class PluginManager extends TypertRemoteService { */ @Remote('retry') async retry(packageName: string): Promise { + return this.exclusive('retry', packageName, () => this.retryNow(packageName)) + } + + private async retryNow(packageName: string): Promise { const runtime = this.runtime() if (!bundlesOf(readProfileManifest(NAME, runtime.dir)).includes(packageName)) { throw new RemoteError('gateway/bad-request', `plugin-manager: ${packageName} is not enabled`, {}) } - await this.disable(packageName) - const result = await this.enable(packageName) + await this.disableNow(packageName) + const result = await this.enableNow(packageName) this.changed('retry', packageName) return result } @@ -551,6 +667,14 @@ export class PluginManager extends TypertRemoteService { packageName: string, target: PluginRowTarget, options?: { module?: string; id?: string; config?: JsonValue }, + ): Promise { + return this.exclusive('addRow', packageName, () => this.addRowNow(packageName, target, options)) + } + + private async addRowNow( + packageName: string, + target: PluginRowTarget, + options?: { module?: string; id?: string; config?: JsonValue }, ): Promise { const runtime = this.runtime() this.assertInstalled(runtime, packageName) @@ -582,6 +706,10 @@ export class PluginManager extends TypertRemoteService { */ @Remote('removeRow') async removeRow(target: PluginRowTarget, rowId: string): Promise { + return this.exclusive('removeRow', rowId, () => this.removeRowNow(target, rowId)) + } + + private async removeRowNow(target: PluginRowTarget, rowId: string): Promise { const runtime = this.runtime() // A holder rather than a `let`: the assignment happens inside the edit // callback, which control-flow narrowing does not see. @@ -603,6 +731,10 @@ export class PluginManager extends TypertRemoteService { */ @Remote('setRowDisabled') async setRowDisabled(target: PluginRowTarget, rowId: string, disabled: boolean): Promise { + return this.exclusive('setRowDisabled', rowId, () => this.setRowDisabledNow(target, rowId, disabled)) + } + + private async setRowDisabledNow(target: PluginRowTarget, rowId: string, disabled: boolean): Promise { const runtime = this.runtime() await this.editLayer(runtime, target, (document) => { if (disabled) document.setRowField(rowId, 'disabled', true) diff --git a/packages/host/plugin-manager/src/types.ts b/packages/host/plugin-manager/src/types.ts index aa745a2e7b..30c0a64366 100644 --- a/packages/host/plugin-manager/src/types.ts +++ b/packages/host/plugin-manager/src/types.ts @@ -116,10 +116,20 @@ export type PluginRowTarget = | { readonly kind: 'global' } | { readonly kind: 'preset'; readonly preset: string } +/** A package pnpm installed that the run removed again, with the check it failed. */ +export interface PluginInstallRejection { + /** The package name. */ + readonly name: string + /** Why it was removed: not a dsh package, or a row id another layer already owns. */ + readonly reason: string +} + /** What one install run changed. */ export interface PluginInstallResult { - /** Dependencies present after the run and absent before it, by name. */ + /** Dependencies present after the run and absent before it that passed the post-install checks, by name. */ readonly installed: readonly string[] + /** Dependencies pnpm added that the run removed again, each with its reason. */ + readonly removed: readonly PluginInstallRejection[] /** Bundles newly enabled, when the caller asked for it. */ readonly enabled: readonly string[] /** Newly installed bundles left disabled. */ @@ -199,6 +209,14 @@ declare module '@deepseek-ai/dsh-typert-protocol' { 'plugins/install-failed': { readonly spec: string; readonly exitCode: number | null; readonly log: string } /** The row id is already taken in the target user layer. */ 'plugins/row-conflict': { readonly rowId: string; readonly target: PluginRowTarget } + /** Another mutation is still running; the manager runs one at a time and refuses rather than queues. */ + 'plugins/busy': { + readonly operation: string + readonly subject: string + readonly active: { readonly operation: string; readonly subject: string } + } + /** `node_modules` cannot change while a session runs; `running` counts the agents in `running` status. */ + 'plugins/agents-running': { readonly operation: string; readonly running: number } } } diff --git a/packages/host/plugin-manager/tests/plugin-manager.spec.ts b/packages/host/plugin-manager/tests/plugin-manager.spec.ts index c3afc363dd..a94abcd118 100644 --- a/packages/host/plugin-manager/tests/plugin-manager.spec.ts +++ b/packages/host/plugin-manager/tests/plugin-manager.spec.ts @@ -435,6 +435,114 @@ describe('PluginManager', () => { }) describe('install', () => { + it('removes a package that is neither a bundle nor a plugin module and says why', async () => { + const staged = await stageHome() + stagePackage(staged.profileDir, 'ext-lib', { main: 'export const answer = 42\n' }) + const calls: string[][] = [] + const { manager } = await bootProfile(staged, { spawn: recordingPnpm(staged.profileDir, calls) }) + + const result = await manager.install('ext-lib') + + expect(result).toMatchObject({ + installed: [], plain: [], installedOnly: [], + removed: [{ name: 'ext-lib', reason: 'declares neither a dsh bundle nor a plugin module' }], + }) + expect(calls).toEqual([['pnpm', 'add', 'ext-lib'], ['pnpm', 'remove', 'ext-lib']]) + expect(manifestOf(staged.profileDir).dependencies).not.toHaveProperty('ext-lib') + expect(existsSync(join(staged.profileDir, '.dsh-plugins', 'ext-lib.json'))).toBe(false) + expect((await manager.list()).some(view => view.name === 'ext-lib')).toBe(false) + }) + + it('removes an installed bundle whose row id another layer already owns', async () => { + const staged = await stageHome() + stagePackage(staged.profileDir, 'ext-one', { patch: BUNDLE_ONE_ROW }) + addDependency(staged.profileDir, 'ext-one') + const manifest = manifestOf(staged.profileDir) + manifest.dsh.profile.bundles.push('ext-one') + writeFileSync(join(staged.profileDir, 'package.json'), JSON.stringify(manifest, null, 2)) + stagePackage(staged.profileDir, 'ext-two', { patch: BUNDLE_ONE_ROW }) + const calls: string[][] = [] + const { manager } = await bootProfile(staged, { spawn: recordingPnpm(staged.profileDir, calls) }) + + const result = await manager.install('ext-two', { enable: true }) + + expect(result).toMatchObject({ + installed: [], enabled: [], installedOnly: [], + removed: [{ name: 'ext-two', reason: 'row "hello" is already declared by ext-one' }], + }) + expect(calls).toEqual([['pnpm', 'add', 'ext-two'], ['pnpm', 'remove', 'ext-two']]) + expect(manifestOf(staged.profileDir)).toMatchObject({ dsh: { profile: { bundles: expect.not.arrayContaining(['ext-two']) as string[] } } }) + expect(manifestOf(staged.profileDir).dependencies).not.toHaveProperty('ext-two') + }) + + it('keeps a package whose probe refused it, for the view to explain', async () => { + const staged = await stageHome() + stagePackage(staged.profileDir, 'ext-broken', { patch: BUNDLE_ONE_ROW, main: 'throw new Error("no import for you")\n' }) + const { manager } = await bootProfile(staged, { spawn: recordingPnpm(staged.profileDir) }) + + const result = await manager.install('ext-broken') + + expect(result).toMatchObject({ installed: ['ext-broken'], removed: [] }) + expect((await manager.list()).find(view => view.name === 'ext-broken')).toMatchObject({ status: 'not-enableable' }) + }) + + it('restores the manifest when pnpm fails after writing it', async () => { + const staged = await stageHome() + const manifestPath = join(staged.profileDir, 'package.json') + const before = readFileSync(manifestPath, 'utf8') + const { manager } = await bootProfile(staged, { spawn: fakePnpm(staged.profileDir, (args) => { + addDependency(staged.profileDir, args[1] ?? 'ext-ghost') + return { code: 1, stderr: 'ERR_PNPM_FETCH_404\n' } + }) }) + + await expect(manager.install('ext-ghost')).rejects.toMatchObject({ code: 'plugins/install-failed' }) + + expect(readFileSync(manifestPath, 'utf8')).toBe(before) + }) + + it('refuses a second mutation while one is still running', async () => { + const staged = await stageHome() + stagePackage(staged.profileDir, 'ext-slow', { patch: BUNDLE_ONE_ROW }) + let release = (): void => {} + const gate = new Promise((resolve) => { release = resolve }) + const spawn: SpawnLike = (_command, args) => { + const child = new EventEmitter() as EventEmitter & { stdout: PassThrough; stderr: PassThrough; kill: () => boolean } + child.stdout = new PassThrough() + child.stderr = new PassThrough() + child.kill = () => true + void gate.then(() => { + addDependency(staged.profileDir, args[1] ?? 'ext-slow') + child.emit('close', 0) + }) + return child as unknown as ChildProcess + } + const { manager } = await bootProfile(staged, { spawn }) + + const first = manager.install('ext-slow') + await expect(manager.enable('ext-slow')).rejects.toMatchObject({ + code: 'plugins/busy', details: { operation: 'enable', active: { operation: 'install', subject: 'ext-slow' } }, + }) + release() + await expect(first).resolves.toMatchObject({ installed: ['ext-slow'] }) + // The lock is released with the run: the refused call now goes through. + await expect(manager.enable('ext-slow')).resolves.toMatchObject({ changed: true }) + }) + + it('refuses to change node_modules while a session is running', async () => { + const staged = await stageHome() + stagePackage(staged.profileDir, 'ext-bundle', { patch: BUNDLE_ONE_ROW }) + addDependency(staged.profileDir, 'ext-bundle') + const calls: string[][] = [] + const { ctx, manager } = await bootProfile(staged, { spawn: recordingPnpm(staged.profileDir, calls) }) + ctx.provide('agents', { list: () => [{ status: 'running' }, { status: 'idle' }] } as never) + + await expect(manager.install('ext-new')).rejects.toMatchObject({ code: 'plugins/agents-running', details: { operation: 'install', running: 1 } }) + await expect(manager.uninstall('ext-bundle')).rejects.toMatchObject({ code: 'plugins/agents-running', details: { operation: 'uninstall' } }) + expect(calls).toEqual([]) + // Enabling recomposes the tree without touching node_modules. + await expect(manager.enable('ext-bundle')).resolves.toMatchObject({ changed: true }) + }) + it('runs pnpm add, records the dependency, probes the package, and leaves it disabled', async () => { const staged = await stageHome() stagePackage(staged.profileDir, 'ext-new', { patch: BUNDLE_ONE_ROW }) @@ -446,7 +554,7 @@ describe('PluginManager', () => { expect(calls).toEqual([['pnpm', 'add', 'github:acme/ext-new']]) // The fake pnpm records the spec itself as the dependency name, which // resolves to nothing: a plain dependency whose probe cannot run. - expect(result).toEqual({ installed: ['github:acme/ext-new'], enabled: [], installedOnly: [], plain: ['github:acme/ext-new'], jobId: expect.any(String) as string }) + expect(result).toEqual({ installed: ['github:acme/ext-new'], removed: [], enabled: [], installedOnly: [], plain: ['github:acme/ext-new'], jobId: expect.any(String) as string }) expect(log.map(chunk => [chunk.stream, chunk.text, chunk.exitCode])).toEqual([ ['stdout', '+ github:acme/ext-new 1.0.0\n', undefined], ['stdout', '', 0], @@ -469,12 +577,12 @@ describe('PluginManager', () => { expect((await manager.list()).find(view => view.name === 'ext-new')?.status).toBe('running') }) - it('reports a plain dependency and a non-zero exit with the log tail', async () => { + it('reports a plugin module as plain and uninstalls it', async () => { const staged = await stageHome() - stagePackage(staged.profileDir, 'ext-lib', { main: 'export const x = 1\n' }) + stagePackage(staged.profileDir, 'ext-lib', { main: 'export function apply() {}\n' }) const { manager } = await bootProfile(staged, { spawn: recordingPnpm(staged.profileDir) }) - expect(await manager.install('ext-lib')).toMatchObject({ installed: ['ext-lib'], plain: ['ext-lib'], installedOnly: [] }) + expect(await manager.install('ext-lib')).toMatchObject({ installed: ['ext-lib'], plain: ['ext-lib'], installedOnly: [], removed: [] }) expect((await manager.list()).find(view => view.name === 'ext-lib')?.status).toBe('plain') await manager.uninstall('ext-lib') expect((await manager.list()).some(view => view.name === 'ext-lib')).toBe(false) diff --git a/packages/host/plugin-manager/tsconfig.json b/packages/host/plugin-manager/tsconfig.json index e956c9019b..7e17e209b9 100644 --- a/packages/host/plugin-manager/tsconfig.json +++ b/packages/host/plugin-manager/tsconfig.json @@ -34,6 +34,9 @@ }, { "path": "../../util/values" + }, + { + "path": "../../core/agent" } ] } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 462c6cb81a..aa8ad76e94 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -184,6 +184,9 @@ importers: '@deepseek-ai/dsh-fs-local': specifier: workspace:^ version: link:../../packages/fs/fs-local + '@deepseek-ai/dsh-global-tool-mask': + specifier: workspace:^ + version: link:../../packages/preset/global-tool-mask '@deepseek-ai/dsh-goal': specifier: workspace:^ version: link:../../packages/goal/goal @@ -295,9 +298,6 @@ importers: '@deepseek-ai/dsh-tool-ralph': specifier: workspace:^ version: link:../../packages/workflow/tool-ralph - '@deepseek-ai/dsh-global-tool-mask': - specifier: workspace:^ - version: link:../../packages/preset/global-tool-mask '@deepseek-ai/dsh-tool-skill': specifier: workspace:^ version: link:../../packages/skill/tool-skill @@ -5835,6 +5835,9 @@ importers: '@deepseek-ai/cordis-plugin-loader': specifier: workspace:^ version: link:../../../vendor/loader + '@deepseek-ai/dsh-agent': + specifier: workspace:^ + version: link:../../core/agent '@deepseek-ai/dsh-agent-presets': specifier: workspace:^ version: link:../../preset/agent-presets @@ -6660,22 +6663,6 @@ importers: specifier: workspace:^ version: link:../../typert/protocol - packages/preset/persona: - dependencies: - '@deepseek-ai/schemastery': - specifier: link:../../../vendor/schemastery - version: link:../../../vendor/schemastery - devDependencies: - '@deepseek-ai/cordis': - specifier: workspace:^ - version: link:../../../vendor/cordis - '@deepseek-ai/dsh-scope': - specifier: workspace:^ - version: link:../../core/scope - '@deepseek-ai/dsh-system-prompt': - specifier: workspace:^ - version: link:../../core/system-prompt - packages/preset/global-tool-mask: dependencies: '@deepseek-ai/schemastery': @@ -6695,6 +6682,22 @@ importers: specifier: workspace:^ version: link:../../core/tools + packages/preset/persona: + dependencies: + '@deepseek-ai/schemastery': + specifier: link:../../../vendor/schemastery + version: link:../../../vendor/schemastery + devDependencies: + '@deepseek-ai/cordis': + specifier: workspace:^ + version: link:../../../vendor/cordis + '@deepseek-ai/dsh-scope': + specifier: workspace:^ + version: link:../../core/scope + '@deepseek-ai/dsh-system-prompt': + specifier: workspace:^ + version: link:../../core/system-prompt + packages/runtime-diagnostics/invariants: dependencies: '@deepseek-ai/schemastery':