From 080e22ee036bb4d89d942f1532c236ea73a8853d Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Tue, 8 Sep 2026 12:52:45 +0800 Subject: [PATCH] feat(plugin-manager): keep pnpm's colours and name the command on install-log chunks The installer now decides pnpm's colouring per consumer: the Web host keeps colours on for the install dialog's terminal, the CLI turns them on only when stdout is a terminal, and a plain log strips what a colour=always pnpm still prints. Every chunk carries the argv it came from, so a renderer can label each pnpm run. --- ...manager-over-the-profile-runtime.i18n.yaml | 4 +-- ...plugin-manager-over-the-profile-runtime.md | 2 +- ...gin-manager-over-the-profile-runtime.zh.md | 2 +- apps/cli/src/plugin.ts | 2 ++ apps/cli/tests/plugin.spec.ts | 24 ++++++++++++++- packages/boot/plugin-manager/README.i18n.yaml | 4 +-- packages/boot/plugin-manager/README.md | 4 +-- packages/boot/plugin-manager/README.zh.md | 4 +-- packages/boot/plugin-manager/src/installer.ts | 29 ++++++++++++++----- packages/boot/plugin-manager/src/manager.ts | 2 ++ packages/boot/plugin-manager/src/types.ts | 5 +++- .../tests/plugin-manager.spec.ts | 25 ++++++++++++++-- 12 files changed, 85 insertions(+), 22 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 80516a9a32..eebfad4ff7 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: a486806da2a62c4f7265e6e2b2204fff1b274843 -2026-09-04-plugin-manager-over-the-profile-runtime.zh.md: 5ca6b436d5e446743b28d610e3957c1618e93818 +2026-09-04-plugin-manager-over-the-profile-runtime.md: 95153138b7d10ea6d13b96e815bd0a52c15d663e +2026-09-04-plugin-manager-over-the-profile-runtime.zh.md: 8f6794568f7aced809b44454b5192415c2ae097c 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 a486806da2..95153138b7 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 @@ -14,7 +14,7 @@ Installing a plugin was a terminal-only act: `dsh plugin --profile web add loadProfile(NAME, profile, INSTALL_ANCHOR, undefined, { userLayer: false }), config: TOOLING, installLog: (chunk) => { (chunk.stream === 'stdout' ? process.stdout : process.stderr).write(chunk.text) }, + // pnpm's colours reach the terminal the user is looking at, never a redirected file. + color: process.stdout.isTTY, ...internals, }) for (const argument of specs) { diff --git a/apps/cli/tests/plugin.spec.ts b/apps/cli/tests/plugin.spec.ts index 4e60db940d..add2fbe314 100644 --- a/apps/cli/tests/plugin.spec.ts +++ b/apps/cli/tests/plugin.spec.ts @@ -19,6 +19,8 @@ let home: string let previousHome: string | undefined let stderr: string let stdout: string +/** The FORCE_COLOR each fake pnpm run was spawned with. */ +let spawnEnvs: (string | undefined)[] = [] beforeEach(() => { home = mkdtempSync(join(tmpdir(), 'dsh-plugin-command-')) @@ -26,6 +28,7 @@ beforeEach(() => { process.env.DSH_HOME = home stderr = '' stdout = '' + spawnEnvs = [] vi.spyOn(process.stderr, 'write').mockImplementation((chunk) => { stderr += String(chunk); return true }) vi.spyOn(process.stdout, 'write').mockImplementation((chunk) => { stdout += String(chunk); return true }) }) @@ -64,6 +67,7 @@ function uninstall(profileDir: string, name: string): void { function fakePnpm(calls: string[][], failWith?: { code: number } | { error: NodeJS.ErrnoException }): SpawnLike { return (_command, args, options) => { calls.push([...args]) + spawnEnvs.push(options.env?.FORCE_COLOR) const child = new EventEmitter() as EventEmitter & { stdout: PassThrough; stderr: PassThrough } child.stdout = new PassThrough() child.stderr = new PassThrough() @@ -81,7 +85,8 @@ function fakePnpm(calls: string[][], failWith?: { code: number } | { error: Node const profileDir = options.cwd as string if (verb === 'add' && target !== undefined) install(profileDir, target, target === 'ext-bundle') if (verb === 'remove' && target !== undefined) uninstall(profileDir, target) - child.stdout.write(`${verb === 'add' ? '+' : '-'} ${String(target)}\n`) + // A coloured line, as a pnpm told to colour anyway would print one. + child.stdout.write(`\u001b[32m${verb === 'add' ? '+' : '-'}\u001b[39m ${String(target)}\n`) child.emit('close', 0) }, 5) return child as unknown as ChildProcess @@ -104,6 +109,20 @@ const fakeProbe: typeof probePackage = (options) => { } describe('dsh plugin', () => { + it('lets pnpm colour its output when stdout is a terminal', async () => { + const calls: string[][] = [] + const descriptor = Object.getOwnPropertyDescriptor(process.stdout, 'isTTY') + Object.defineProperty(process.stdout, 'isTTY', { value: true, configurable: true }) + try { + expect(await runPlugin('web', ['add', 'ext-bundle'], { spawn: fakePnpm(calls), probe: fakeProbe })).toBe(0) + } finally { + if (descriptor === undefined) delete (process.stdout as { isTTY?: boolean }).isTTY + else Object.defineProperty(process.stdout, 'isTTY', descriptor) + } + expect(spawnEnvs).toEqual(['1']) + expect(stdout).toContain('\u001b[32m+\u001b[39m ext-bundle') + }) + it('add initializes the profile, installs through the installer, enables the new bundle, and removes a plain library again', async () => { const calls: string[][] = [] @@ -117,7 +136,10 @@ describe('dsh plugin', () => { expect(Object.keys(manifest.dependencies ?? {})).toContain('ext-bundle') expect(Object.keys(manifest.dependencies ?? {})).not.toContain('ext-lib') expect(manifest.dsh?.profile?.bundles).toContain('ext-bundle') + // The log reaches its readers plain: colours are off for the child and stripped from what it still prints. + expect(spawnEnvs).toEqual(['0', '0', '0']) expect(stdout).toContain('+ ext-bundle') + expect(stdout).not.toContain('\u001b[') expect(stderr).toContain('dsh: removed ext-lib again: declares neither a dsh bundle nor a plugin module') expect(existsSync(join(profileDir, '.dsh-plugins', 'ext-bundle.json'))).toBe(true) }) diff --git a/packages/boot/plugin-manager/README.i18n.yaml b/packages/boot/plugin-manager/README.i18n.yaml index 3b467f3696..6276a3dcab 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: c488373c22b696d3dfe605335cc11d8b942cc75a -README.zh.md: 4cdfa4c90b6f5fc91c1a29255b09bdda566f588d +README.md: 0666043f69619fab042228d5f298d63eeeada87b +README.zh.md: 2c659f51e41b366a793f36ab8865bea2a68e0b65 diff --git a/packages/boot/plugin-manager/README.md b/packages/boot/plugin-manager/README.md index c488373c22..0666043f69 100644 --- a/packages/boot/plugin-manager/README.md +++ b/packages/boot/plugin-manager/README.md @@ -27,7 +27,7 @@ English | [中文](README.zh.md) ### Installing without booting -Build a `PluginInstaller` from the profile directory, the install anchor (the dsh app's `package.json`), a `loadProfile` that answers the profile's composed layers, the tooling bounds, and a sink for pnpm's output; then `add(spec)` or `remove(name)`: +Build a `PluginInstaller` from the profile directory, the install anchor (the dsh app's `package.json`), a `loadProfile` that answers the profile's composed layers, the tooling bounds, a sink for pnpm's output, and whether pnpm colours it; then `add(spec)` or `remove(name)`: ```ts import { loadProfile } from '@deepseek-ai/dsh-app-boot' @@ -77,7 +77,7 @@ console.log(await manager.list()) `addRow` inserts a row naming one of the package's modules — its main export for a `plugin` package, or a `dsh.plugins` entry — into the profile's global `cordis.patch.yml` (`target: { kind: 'global' }`) or an agent preset's user layer (`{ kind: 'preset', preset }`, through the roster's `overlayPathFor`). The row id derives from the package name and subpath unless given; a taken id fails with `plugins/row-conflict`. `removeRow` removes an inserted row and `setRowDisabled` writes or removes a `disabled: true` for any row — deny-only, so a bundle's own `!!js` gate is restored rather than overridden. The global layer is recomposed live on the spot; a preset's layer reaches its next standing generation. `dependents` says what disabling or removing a package would strand: services its rows provide that rows outside it inject, and user-layer rows naming its modules. -The manager runs one mutation at a time — a second call while one runs fails with `plugins/busy` naming the operation in flight — and `add` and `uninstall` refuse to change `node_modules` while a session is running, with `plugins/agents-running`. Every change is followed by a `plugins/changed` event on the context, and an install run emits pnpm's output as `plugins/install-log` chunks. +The manager runs one mutation at a time — a second call while one runs fails with `plugins/busy` naming the operation in flight — and `add` and `uninstall` refuse to change `node_modules` while a session is running, with `plugins/agents-running`. Every change is followed by a `plugins/changed` event on the context, and an install run emits pnpm's output as `plugins/install-log` chunks, each naming the command line it ran and, with colour on, carrying pnpm's SGR escapes. ### Failures diff --git a/packages/boot/plugin-manager/README.zh.md b/packages/boot/plugin-manager/README.zh.md index 4cdfa4c90b..2c659f51e4 100644 --- a/packages/boot/plugin-manager/README.zh.md +++ b/packages/boot/plugin-manager/README.zh.md @@ -27,7 +27,7 @@ kind: "package-reference" ### 不启动也能安装 -用 profile 目录、安装锚点（dsh 应用的 `package.json`）、一个回答 profile 已组合层列表的 `loadProfile`、工具边界，以及 pnpm 输出的去处构造 `PluginInstaller`，然后 `add(spec)` 或 `remove(name)`： +用 profile 目录、安装锚点（dsh 应用的 `package.json`）、一个回答 profile 已组合层列表的 `loadProfile`、工具边界、pnpm 输出的去处，以及是否让 pnpm 上色，构造 `PluginInstaller`，然后 `add(spec)` 或 `remove(name)`： ```ts import { loadProfile } from '@deepseek-ai/dsh-app-boot' @@ -77,7 +77,7 @@ console.log(await manager.list()) `addRow` 把一条点名该包某个模块的行——`plugin` 包的主导出，或某个 `dsh.plugins` 条目——插入 profile 的全局 `cordis.patch.yml`（`target: { kind: 'global' }`）或某个 agent preset 的用户层（`{ kind: 'preset', preset }`，经 roster 的 `overlayPathFor`）。行 id 未给出时由包名与子路径派生；已被占用的 id 以 `plugins/row-conflict` 失败。`removeRow` 移除一条插入的行，`setRowDisabled` 为任意行写入或移除 `disabled: true`——只写拒绝，因此组合包自己的 `!!js` 门被恢复而不是被覆盖。全局层当场在线重新组合；preset 的层在其下一个常驻代际生效。`dependents` 说明停用或移除一个包会搁浅什么：其行提供而包外的行注入的服务，以及点名其模块的用户层行。 -管理器一次只跑一个变更——上一个还在跑时再调用会以 `plugins/busy` 失败并点名正在进行的操作——`add` 与 `uninstall` 在有会话运行时拒绝改动 `node_modules`，报 `plugins/agents-running`。每次变更之后在上下文上发出 `plugins/changed` 事件，安装运行把 pnpm 的输出以 `plugins/install-log` 分块发出。 +管理器一次只跑一个变更——上一个还在跑时再调用会以 `plugins/busy` 失败并点名正在进行的操作——`add` 与 `uninstall` 在有会话运行时拒绝改动 `node_modules`，报 `plugins/agents-running`。每次变更之后在上下文上发出 `plugins/changed` 事件，安装运行把 pnpm 的输出以 `plugins/install-log` 分块发出，每块都写明所跑的命令行，开了颜色时还带着 pnpm 的 SGR 转义。 ### 失败 diff --git a/packages/boot/plugin-manager/src/installer.ts b/packages/boot/plugin-manager/src/installer.ts index 4df379a3d0..f2205f2ab5 100644 --- a/packages/boot/plugin-manager/src/installer.ts +++ b/packages/boot/plugin-manager/src/installer.ts @@ -32,6 +32,9 @@ import type { PluginInstallLogChunk, PluginInstallRejection, PluginInstallResult /** What one install run changed before any newly installed bundle was enabled. */ export type PluginInstallOutcome = Omit +/** A terminal escape sequence (CSI): colours, cursor moves, and the rest of what a coloured pnpm prints. */ +const ANSI_SEQUENCE = /\u001b\[[0-9;?]*[ -/]*[@-~]/g + /** What {@link PluginInstaller} needs: the profile on disk, the tooling bounds, and where pnpm's output goes. */ export interface PluginInstallerOptions { /** The profile directory pnpm runs in and whose manifest records installs. */ @@ -45,6 +48,13 @@ export interface PluginInstallerOptions { readonly config: PluginToolingConfig /** Receives every chunk of a pnpm run's output, in order; the last chunk carries the exit code. */ readonly installLog: (chunk: PluginInstallLogChunk) => void + /** + * Whether pnpm colours its output. On, the chunks carry SGR escapes for a + * consumer that draws them (the Web install dialog's terminal); off, they + * are plain text, with the escapes a pnpm told to colour anyway prints + * (a `color=always` config) dropped. + */ + readonly color: boolean /** Test seam: the child spawner; defaults to `node:child_process`. */ readonly spawn?: SpawnLike /** Test seam: the package probe; defaults to app-boot's. */ @@ -240,27 +250,32 @@ export class PluginInstaller { * @throws {PluginOperationError} `plugins/install-failed` on a non-zero exit, a signal, or the timeout. */ private async runPnpm(args: readonly string[], spec: string): Promise { - const { config, profileDir } = this.options + const { color, config, profileDir } = this.options const jobId = randomUUID() + const argv = [config.pnpmCommand, ...args] const tail: string[] = [] let tailBytes = 0 - const record = (stream: 'stdout' | 'stderr', text: string): void => { + const record = (stream: 'stdout' | 'stderr', chunk: string): void => { + const text = color ? chunk : chunk.replace(ANSI_SEQUENCE, '') tail.push(text) tailBytes += Buffer.byteLength(text) while (tailBytes > config.installLogTailBytes && tail.length > 1) { tailBytes -= Buffer.byteLength(tail.shift() as string) } - this.options.installLog({ jobId, spec, stream, text }) + this.options.installLog({ jobId, argv, spec, stream, text }) } // Windows resolves pnpm through its .cmd shim, which spawn() refuses // without a shell since the CVE-2024-27980 hardening. The parent // environment is passed whole, as the `dsh plugin` command does: pnpm - // needs the user's registry, proxy, and auth settings. + // needs the user's registry, proxy, and auth settings. pnpm writes to a + // pipe and would decide against colour on its own, so `FORCE_COLOR` + // decides for it either way: a parent forcing colours for its own + // terminal cannot leak escapes into a plain log. const child = this.spawn(config.pnpmCommand, args, { cwd: profileDir, stdio: ['ignore', 'pipe', 'pipe'], shell: process.platform === 'win32', - env: process.env, + env: { ...process.env, FORCE_COLOR: color ? '1' : '0' }, }) child.stdout?.setEncoding('utf8') child.stderr?.setEncoding('utf8') @@ -272,10 +287,10 @@ export class PluginInstaller { ).catch((error: unknown) => { const message = messageOf(error) record('stderr', `${message}\n`) - this.options.installLog({ jobId, spec, stream: 'stderr', text: '', exitCode: null }) + this.options.installLog({ jobId, argv, spec, stream: 'stderr', text: '', exitCode: null }) throw new PluginOperationError('plugins/install-failed', `${NAME}: ${message}`, { spec, exitCode: null, log: tail.join('') }, { cause: error }) }) - this.options.installLog({ jobId, spec, stream: 'stdout', text: '', exitCode }) + this.options.installLog({ jobId, argv, spec, stream: 'stdout', text: '', exitCode }) if (exitCode !== 0) { throw new PluginOperationError( 'plugins/install-failed', diff --git a/packages/boot/plugin-manager/src/manager.ts b/packages/boot/plugin-manager/src/manager.ts index da62038812..967ad168d1 100644 --- a/packages/boot/plugin-manager/src/manager.ts +++ b/packages/boot/plugin-manager/src/manager.ts @@ -154,6 +154,8 @@ export class PluginManager { loadProfile: () => runtime.current, config: this.options.config, installLog: (chunk) => { this.ctx.emit('plugins/install-log', chunk) }, + // The chunks feed the Web install dialog's terminal, which draws SGR colour. + color: true, ...this.options.spawn === undefined ? {} : { spawn: this.options.spawn }, ...this.options.probe === undefined ? {} : { probe: this.options.probe }, }) diff --git a/packages/boot/plugin-manager/src/types.ts b/packages/boot/plugin-manager/src/types.ts index b5a5987317..a9521892f5 100644 --- a/packages/boot/plugin-manager/src/types.ts +++ b/packages/boot/plugin-manager/src/types.ts @@ -188,11 +188,14 @@ export type PluginChangeReason = 'install' | 'uninstall' | 'enable' | 'disable' export interface PluginInstallLogChunk { /** The run the chunk belongs to. */ readonly jobId: string + /** The command line the run executes: pnpm's command name, then its arguments. */ + readonly argv: readonly string[] /** The package spec the run installs or removes. */ readonly spec: string readonly stream: 'stdout' | 'stderr' + /** The output as pnpm wrote it; with the installer's colours on, its SGR escapes included. */ readonly text: string - /** Present on the run's last chunk, with pnpm's exit code (null for a signal). */ + /** Present on the run's last chunk, with pnpm's exit code (null when it ended by a signal or never started). */ readonly exitCode?: number | null } diff --git a/packages/boot/plugin-manager/tests/plugin-manager.spec.ts b/packages/boot/plugin-manager/tests/plugin-manager.spec.ts index 427a673bbd..dfef45eb75 100644 --- a/packages/boot/plugin-manager/tests/plugin-manager.spec.ts +++ b/packages/boot/plugin-manager/tests/plugin-manager.spec.ts @@ -575,13 +575,32 @@ describe('PluginManager', () => { // 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'], 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], + expect(log.map(chunk => [chunk.argv, chunk.stream, chunk.text, chunk.exitCode])).toEqual([ + [['pnpm', 'add', 'github:acme/ext-new'], 'stdout', '+ github:acme/ext-new 1.0.0\n', undefined], + [['pnpm', 'add', 'github:acme/ext-new'], 'stdout', '', 0], ]) expect(changes).toEqual([{ reason: 'install' }]) }) + it('has pnpm colour its output and streams the escapes as they come', async () => { + const staged = await stageHome() + stagePackage(staged.profileDir, 'ext-new', { patch: BUNDLE_ONE_ROW }) + const colours: (string | undefined)[] = [] + const pnpm = fakePnpm(staged.profileDir, (args) => { + addDependency(staged.profileDir, args[1] as string) + return { code: 0, stdout: '\u001b[32m+\u001b[39m ext-new \u001b[90m1.0.0\u001b[39m\n' } + }) + const { manager, log } = await bootProfile(staged, { + spawn: (command, args, options) => { + colours.push(options.env?.FORCE_COLOR) + return pnpm(command, args, options) + }, + }) + await manager.add('ext-new') + expect(colours).toEqual(['1']) + expect(log[0]?.text).toBe('\u001b[32m+\u001b[39m ext-new \u001b[90m1.0.0\u001b[39m\n') + }) + it('reconciles by the installed name, and enables the new bundle when asked', async () => { const staged = await stageHome() stagePackage(staged.profileDir, 'ext-new', { patch: BUNDLE_ONE_ROW })