소스 검색

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.
Yichen Jiang 3 주 전
부모
커밋
4cb2d7abb5

+ 49 - 0
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.<namespace>.<method>`,
+ * 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('<name>')` 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<string> {
+  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([])
+  })
+})

+ 2 - 2
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

+ 2 - 2
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
 

+ 2 - 2
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`。
 
 ### 用户层里的行
 

+ 5 - 5
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<PluginInstallResult> {
-    return this.exclusive('install', spec, () => this.installNow(spec, options))
+  @Remote('add')
+  async add(spec: string, options?: { enable?: boolean }): Promise<PluginInstallResult> {
+    return this.exclusive('add', spec, () => this.addNow(spec, options))
   }
 
-  private async installNow(spec: string, options?: { enable?: boolean }): Promise<PluginInstallResult> {
+  private async addNow(spec: string, options?: { enable?: boolean }): Promise<PluginInstallResult> {
     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)

+ 18 - 18
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) } })
     })
   })