From 4cb2d7abb50ee1b78be61ec9c4bea7c29b07a8ea Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Sat, 5 Sep 2026 17:37:28 +0800 Subject: [PATCH] refactor(plugins): name the install verb add, as the CLI does The client namespace service keeps install and remove for its own members and refuses a Remote method of that name at mount, so the browser could never reach plugins/install. A spec now reads the reserved names off the gateway and checks every @Remote name against them. --- .../tests/remote-method-names.host.spec.ts | 49 +++++++++++++++++++ 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/src/index.ts | 10 ++-- .../tests/plugin-manager.spec.ts | 36 +++++++------- 6 files changed, 78 insertions(+), 29 deletions(-) create mode 100644 packages/api/remotes/tests/remote-method-names.host.spec.ts diff --git a/packages/api/remotes/tests/remote-method-names.host.spec.ts b/packages/api/remotes/tests/remote-method-names.host.spec.ts new file mode 100644 index 0000000000..e7df5181f6 --- /dev/null +++ b/packages/api/remotes/tests/remote-method-names.host.spec.ts @@ -0,0 +1,49 @@ +/** + * A Remote method is reached on the browser as `ctx.remote..`, + * a member of the namespace service the client gateway mounts. The mount + * refuses a method named after one of that service's own members — its + * fields and its private `install`/`remove` helpers — and it refuses at page + * load, after every unit suite passed. This spec reads the reserved names + * off the gateway's own source and checks every `@Remote('')` in the + * workspace against them, so the collision fails here instead. + */ + +import { readFileSync } from 'node:fs' +import { globSync } from 'node:fs' +import { join } from 'node:path' +import { fileURLToPath } from 'node:url' +import { describe, expect, it } from 'vitest' + +const ROOT = fileURLToPath(new URL('../../../..', import.meta.url)) + +/** The names the client namespace service keeps for itself. */ +function reservedMethodNames(): Set { + const source = readFileSync(join(ROOT, 'packages/api/gateway/src/client/index.ts'), 'utf8') + const fields = /const REMOTE_NAMESPACE_FIELDS = new Set\(\[([^\]]*)\]\)/.exec(source)?.[1] + const classBody = /class RemoteNamespaceService extends Service \{([\s\S]*?)\n\}/.exec(source)?.[1] + if (fields === undefined || classBody === undefined) { + throw new Error('the client gateway no longer spells its namespace service the way this spec reads it') + } + const reserved = new Set([...fields.matchAll(/'([^']+)'/g)].map(match => match[1] as string)) + for (const match of classBody.matchAll(/^ {2}(?:private |static |readonly |get |async )*([A-Za-z_$][\w$]*)\s*[(:=]/gm)) { + reserved.add(match[1] as string) + } + return reserved +} + +describe('Remote method names', () => { + it('never name a member of the client namespace service', () => { + const reserved = reservedMethodNames() + expect(reserved.has('install')).toBe(true) + expect(reserved.has('remove')).toBe(true) + const offenders: string[] = [] + for (const file of globSync('packages/*/*/src/**/*.ts', { cwd: ROOT })) { + const source = readFileSync(join(ROOT, file), 'utf8') + for (const match of source.matchAll(/@Remote\(\s*'([^']+)'/g)) { + const method = match[1] as string + if (reserved.has(method)) offenders.push(`${file}: @Remote('${method}')`) + } + } + expect(offenders).toEqual([]) + }) +}) diff --git a/packages/host/plugin-manager/README.i18n.yaml b/packages/host/plugin-manager/README.i18n.yaml index a660b9a963..7136a2fbb1 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: d155ce2a77bf303ea0f05b41c8116c794db88172 -README.zh.md: 63df4863c52aebd0935046b27a3ce116f74bf10c +README.md: fd2f5ac7da5004f5dbd202e1cde19718f8fd021d +README.zh.md: 41d5505be0ce6ada8811ba5bfdd4622fd5dd78f2 diff --git a/packages/host/plugin-manager/README.md b/packages/host/plugin-manager/README.md index d155ce2a77..fd2f5ac7da 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, 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/add` 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. Like `install`, it waits for running sessions: `plugins/agents-running` while any agent is running. +`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 `add`, 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 63df4863c5..41d5505be0 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` 与日志尾部让调用失败，并把 profile manifest 恢复到运行前的样子。`pnpm add` 成功还不等于装好了插件：新包既不声明组合包也不声明插件模块，或者是某个行 id 已被已组合层占有的组合包，会再以 `pnpm remove` 移除并连同原因列在 `removed` 里；探针拒绝的包保留在原处，由视图说明它为何不能启用。管理器一次只跑一个变更——上一个还在跑时再调用会以 `plugins/busy` 失败并点名正在进行的操作——并且在有会话运行时拒绝改动 `node_modules`，报 `plugins/agents-running`。 +`plugins/add` 接受一个 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`，并忘掉探针记录。与 `install` 一样，它等待运行中的会话：只要有 agent 在运行就报 `plugins/agents-running`。 +`plugins/uninstall` 在组合包已启用时先停用它，删除每一条点名该包模块的用户层行，运行 `pnpm remove`，并忘掉探针记录。与 `add` 一样，它等待运行中的会话：只要有 agent 在运行就报 `plugins/agents-running`。 ### 用户层里的行 diff --git a/packages/host/plugin-manager/src/index.ts b/packages/host/plugin-manager/src/index.ts index cdd4e59240..2e407b9014 100644 --- a/packages/host/plugin-manager/src/index.ts +++ b/packages/host/plugin-manager/src/index.ts @@ -403,17 +403,17 @@ export class PluginManager extends TypertRemoteService { * or the run times out, `plugins/enable-failed` when enabling was asked * for and the tree rejected the bundle. */ - @Remote('install') - async install(spec: string, options?: { enable?: boolean }): Promise { - return this.exclusive('install', spec, () => this.installNow(spec, options)) + @Remote('add') + async add(spec: string, options?: { enable?: boolean }): Promise { + return this.exclusive('add', spec, () => this.addNow(spec, options)) } - private async installNow(spec: string, options?: { enable?: boolean }): Promise { + private async addNow(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') + this.assertNoRunningAgents('add') const manifestPath = join(runtime.dir, 'package.json') const snapshot = readFileSync(manifestPath, 'utf8') const before = readProfileManifest(NAME, runtime.dir) diff --git a/packages/host/plugin-manager/tests/plugin-manager.spec.ts b/packages/host/plugin-manager/tests/plugin-manager.spec.ts index b9ad419033..ca8f77f993 100644 --- a/packages/host/plugin-manager/tests/plugin-manager.spec.ts +++ b/packages/host/plugin-manager/tests/plugin-manager.spec.ts @@ -238,7 +238,7 @@ describe('PluginManager', () => { const { manager } = await bootProfile(staged) expect(manager.typertRemote).toMatchObject({ serviceKey: 'pluginManager', namespace: 'plugins' }) expect(remoteMethods(manager).map(marker => marker.method)).toEqual([ - 'list', 'install', 'uninstall', 'enable', 'disable', 'retry', 'addRow', 'removeRow', 'setRowDisabled', 'dependents', + 'list', 'add', 'uninstall', 'enable', 'disable', 'retry', 'addRow', 'removeRow', 'setRowDisabled', 'dependents', ]) }) @@ -442,7 +442,7 @@ describe('PluginManager', () => { const calls: string[][] = [] const { manager } = await bootProfile(staged, { spawn: recordingPnpm(staged.profileDir, calls) }) - const result = await manager.install('ext-lib') + const result = await manager.add('ext-lib') expect(result).toMatchObject({ installed: [], plain: [], installedOnly: [], @@ -465,7 +465,7 @@ describe('PluginManager', () => { const calls: string[][] = [] const { manager } = await bootProfile(staged, { spawn: recordingPnpm(staged.profileDir, calls) }) - const result = await manager.install('ext-two', { enable: true }) + const result = await manager.add('ext-two', { enable: true }) expect(result).toMatchObject({ installed: [], enabled: [], installedOnly: [], @@ -484,7 +484,7 @@ describe('PluginManager', () => { const calls: string[][] = [] const { manager } = await bootProfile(staged, { spawn: recordingPnpm(staged.profileDir, calls) }) - const result = await manager.install('ext-odd') + const result = await manager.add('ext-odd') expect(result).toMatchObject({ installed: [], installedOnly: [], @@ -499,7 +499,7 @@ describe('PluginManager', () => { 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') + const result = await manager.add('ext-broken') expect(result).toMatchObject({ installed: ['ext-broken'], removed: [] }) expect((await manager.list()).find(view => view.name === 'ext-broken')).toMatchObject({ status: 'not-enableable' }) @@ -514,7 +514,7 @@ describe('PluginManager', () => { return { code: 1, stderr: 'ERR_PNPM_FETCH_404\n' } }) }) - await expect(manager.install('ext-ghost')).rejects.toMatchObject({ code: 'plugins/install-failed' }) + await expect(manager.add('ext-ghost')).rejects.toMatchObject({ code: 'plugins/install-failed' }) expect(readFileSync(manifestPath, 'utf8')).toBe(before) }) @@ -537,9 +537,9 @@ describe('PluginManager', () => { } const { manager } = await bootProfile(staged, { spawn }) - const first = manager.install('ext-slow') + const first = manager.add('ext-slow') await expect(manager.enable('ext-slow')).rejects.toMatchObject({ - code: 'plugins/busy', details: { operation: 'enable', active: { operation: 'install', subject: 'ext-slow' } }, + code: 'plugins/busy', details: { operation: 'enable', active: { operation: 'add', subject: 'ext-slow' } }, }) release() await expect(first).resolves.toMatchObject({ installed: ['ext-slow'] }) @@ -555,7 +555,7 @@ describe('PluginManager', () => { 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.add('ext-new')).rejects.toMatchObject({ code: 'plugins/agents-running', details: { operation: 'add', 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. @@ -568,7 +568,7 @@ describe('PluginManager', () => { const calls: string[][] = [] const { manager, changes, log } = await bootProfile(staged, { spawn: recordingPnpm(staged.profileDir, calls) }) - const result = await manager.install('github:acme/ext-new') + const result = await manager.add('github:acme/ext-new') expect(calls).toEqual([['pnpm', 'add', 'github:acme/ext-new']]) // The fake pnpm records the spec itself as the dependency name, which @@ -586,7 +586,7 @@ describe('PluginManager', () => { stagePackage(staged.profileDir, 'ext-new', { patch: BUNDLE_ONE_ROW }) const { ctx, manager, changes } = await bootProfile(staged, { spawn: recordingPnpm(staged.profileDir) }) - const result = await manager.install('ext-new', { enable: true }) + const result = await manager.add('ext-new', { enable: true }) expect(result).toMatchObject({ installed: ['ext-new'], enabled: ['ext-new'], installedOnly: [], plain: [] }) expect(manifestOf(staged.profileDir).dsh.profile.bundles).toEqual(['ext-new']) @@ -601,7 +601,7 @@ describe('PluginManager', () => { 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: [], removed: [] }) + expect(await manager.add('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) @@ -618,26 +618,26 @@ describe('PluginManager', () => { return { code: 0 } }) }) - expect(await manager.install('ext-new')).toMatchObject({ installed: ['ext-new'], installedOnly: ['ext-new'] }) + expect(await manager.add('ext-new')).toMatchObject({ installed: ['ext-new'], installedOnly: ['ext-new'] }) }) it('fails loud on a non-zero exit, a spawn error, a timeout, and an empty spec', async () => { const staged = await stageHome() const exits = await bootProfile(staged, { spawn: fakePnpm(staged.profileDir, () => ({ code: 1, stderr: 'ERR_PNPM_NO_MATCHING_VERSION\n' })) }) - await expect(exits.manager.install('nope')).rejects.toMatchObject({ + await expect(exits.manager.add('nope')).rejects.toMatchObject({ code: 'plugins/install-failed', details: { spec: 'nope', exitCode: 1, log: 'ERR_PNPM_NO_MATCHING_VERSION\n' }, }) expect(exits.log.at(-1)).toMatchObject({ exitCode: 1 }) - await expect(exits.manager.install(' ')).rejects.toMatchObject({ code: 'gateway/bad-request' }) + await expect(exits.manager.add(' ')).rejects.toMatchObject({ code: 'gateway/bad-request' }) const erroringHome = await stageHome() const erroring = await bootProfile(erroringHome, { spawn: fakePnpm(erroringHome.profileDir, () => ({ code: null, error: 'spawn pnpm ENOENT' })) }) - await expect(erroring.manager.install('x')).rejects.toMatchObject({ code: 'plugins/install-failed', details: { exitCode: null } }) + await expect(erroring.manager.add('x')).rejects.toMatchObject({ code: 'plugins/install-failed', details: { exitCode: null } }) expect(erroring.log.some(chunk => chunk.text.includes('ENOENT'))).toBe(true) const hangingHome = await stageHome() const hanging = await bootProfile(hangingHome, { spawn: fakePnpm(hangingHome.profileDir, () => ({ code: null, hang: true })) }) - await expect(hanging.manager.install('x')).rejects.toMatchObject({ code: 'plugins/install-failed' }) + await expect(hanging.manager.add('x')).rejects.toMatchObject({ code: 'plugins/install-failed' }) expect(hanging.log.some(chunk => chunk.text.includes('timed out'))).toBe(true) }) @@ -646,7 +646,7 @@ describe('PluginManager', () => { const { manager } = await bootProfile(staged, { spawn: fakePnpm(staged.profileDir, () => ({ code: 2, stdout: 'a'.repeat(300), stderr: 'b'.repeat(300) })), }, { installLogTailBytes: 256 }) - await expect(manager.install('x')).rejects.toMatchObject({ details: { log: 'b'.repeat(300) } }) + await expect(manager.add('x')).rejects.toMatchObject({ details: { log: 'b'.repeat(300) } }) }) })