Merge pull request #2310 from deepseek-harness/refactor/cmdline-plan-trim

refactor(cmdline): run the program's own commander action instead of a plan callback
2026-08-12 13:20:30 +08:00
parent 80255be10b 54dd75a969
commit 15c84cdc4b
17 changed files with 229 additions and 116 deletions
--- a/packages/boot/cmdline/README.i18n.yaml
+++ b/packages/boot/cmdline/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/cmdline/README.md
-README.md: 2e8e58b23785fa78bd2663a459817669309a81be
-README.zh.md: c04d76905edb4afa6b18b36b8284b14990be6bdd
+README.md: 33125014539e801dbd2952a3b4513cafc80bdcee
+README.zh.md: 7ef49a1027d3c17817c9171e1166ed6feecd8559
--- a/packages/boot/cmdline/README.md
+++ b/packages/boot/cmdline/README.md
@@ -15,15 +15,16 @@ An embedding host with no command line provides an empty list; that is the hones

 ## Ordinary providers and injected config

-Any app plugin may inject `cmdlineArgs`, parse it, and publish an ordinary app-owned service. `parseCmdline(ctx, program, plan)` is only a commander adapter; the caller owns the returned value and service:
+Any app plugin may inject `cmdlineArgs`, parse it, and publish an ordinary app-owned service. `parseCmdline(ctx, program)` is only a commander adapter; the program's own action owns validation and the published service:

 ```ts ignore
 export const name = 'web-startup'
 export const inject = ['cmdlineArgs']

 export function apply(ctx: Context): void {
-  const values = parseCmdline(ctx, webCommand(), planWebStartup)
-  if (values !== undefined) ctx.provide('webStartup', values)
+  const program = webCommand()
+  program.action(() => ctx.provide('webStartup', webValuesFrom(program)))
+  parseCmdline(ctx, program)
 }
 ```

@@ -45,7 +46,7 @@ Every row configured from those values uses ordinary service injection and direc
    port: !!js ctx.webStartup.port ?? 3080
 ```

-`parseCmdline` parses the immutable arguments and asks `plan` for the app-owned value. On `--help`, `--version`, a parse error, or a `program.error(...)` from the plan, it writes commander's text, requests exit, and returns `undefined`; the provider publishes nothing, so dependent rows never activate.
+`parseCmdline` refuses at load a program in which no command declares an action, routes every command's exit and output through the launcher (commander copies those settings into subcommands only at registration), and parses the immutable arguments; commander runs the invoked command's synchronous action on success. An action rejects an invalid invocation with `program.error(...)` — before publishing, since statements ahead of the rejection have already run. On `--help`, `--version`, a parse error, or that rejection, the helper writes commander's text and requests exit; the provider publishes nothing, so dependent rows never activate.

 ### How injection orders config

--- a/packages/boot/cmdline/README.zh.md
+++ b/packages/boot/cmdline/README.zh.md
@@ -15,15 +15,16 @@ dsh 启动器交给它所引导应用的那条命令行。启动器只解析属

 ## 普通提供方与注入配置

-任何应用插件都可以注入 `cmdlineArgs`、解析它，再发布一个普通的应用自有服务。`parseCmdline(ctx, program, plan)` 只适配 commander；返回值与服务都归调用方持有：
+任何应用插件都可以注入 `cmdlineArgs`、解析它，再发布一个普通的应用自有服务。`parseCmdline(ctx, program)` 只适配 commander；校验与发布的服务都归 program 自己的 action 持有：

 ```ts ignore
 export const name = 'web-startup'
 export const inject = ['cmdlineArgs']

 export function apply(ctx: Context): void {
-  const values = parseCmdline(ctx, webCommand(), planWebStartup)
-  if (values !== undefined) ctx.provide('webStartup', values)
+  const program = webCommand()
+  program.action(() => ctx.provide('webStartup', webValuesFrom(program)))
+  parseCmdline(ctx, program)
 }
 ```

@@ -45,7 +46,7 @@ export function apply(ctx: Context): void {
    port: !!js ctx.webStartup.port ?? 3080
 ```

-`parseCmdline` 解析不可变参数，再向 `plan` 索取应用自有取值。遇到 `--help`、`--version`、解析错误，或 `plan` 发出的 `program.error(...)` 时，它输出 commander 文本、请求退出并返回 `undefined`；提供方什么也不发布，因此依赖行不会激活。
+`parseCmdline` 在加载时拒绝整棵命令树中没有任何命令声明 action 的 program，把每个命令的退出与输出都接到启动器上（commander 只在注册时把这些设置复制进子命令），再解析不可变参数；解析成功时 commander 运行被调用命令的同步 action。action 用 `program.error(...)` 拒绝无效调用——必须先拒绝后发布，因为写在拒绝之前的语句已经执行。遇到 `--help`、`--version`、解析错误或这种拒绝时，该适配器输出 commander 文本并请求退出；提供方什么也不发布，因此依赖行不会激活。

 ### 注入如何排列配置求值

--- a/packages/boot/cmdline/src/index.ts
+++ b/packages/boot/cmdline/src/index.ts
@@ -8,7 +8,8 @@
 * text, and its parse errors instead of the launcher knowing them.
 *
 * Any app plugin can inject `cmdlineArgs` and call {@link parseCmdline}. A
- * provider may publish the parsed values as its own service, and ordinary rows
+ * provider may publish the parsed values as its own service from its program's
+ * commander action, and ordinary rows
 * can inject that service and read it from lazily resolved config —
 * `port: !!js ctx.webStartup.port ?? 3080` — so a flag beats the value written
 * beside it. No row has launcher-level command-line status.
@@ -76,35 +77,25 @@ export const internals: { stdout: { write(chunk: string): unknown }; stderr: { w
  stderr: process.stderr,
 }

-/**
- * Resolve parsed arguments into an app-owned value. Call
- * `program.error(...)` to reject the invocation with a usage message instead
- * of throwing.
- * @param program - the parsed commander program.
- * @param ctx - the plugin context that received the command line.
- * @returns the value an ordinary provider plugin may publish.
- */
-export type CmdlinePlan<T = unknown> = (program: Command, ctx: Context) => T
-
 /**
 * Parse the launcher's immutable argument snapshot with an app's commander
- * program. The caller decides whether and how to publish the returned value;
- * this helper has no Loader-row or service ownership semantics.
+ * program. Commander runs the program's own synchronous action handler on a
+ * successful parse; app code there publishes its service and rejects an
+ * invalid invocation with `program.error(...)`. This helper has no Loader-row
+ * or service ownership semantics.
 *
- * Help, version, and rejected arguments are terminal for the process: commander
- * writes the text, the helper requests `ctx.appExit`, and it returns
- * `undefined` so the caller publishes nothing.
+ * Help, version, and rejected arguments — from the grammar or from an action
+ * — are terminal for the process: commander writes the text and the helper
+ * requests `ctx.appExit`. The action never runs on help, version, or a
+ * grammar rejection; an action must reject before it publishes, because
+ * statements before its `program.error(...)` have already run.
 * @param ctx - plugin context carrying `cmdlineArgs` and `appExit`.
- * @param program - the app's commander program, with its flags and description already declared.
- * @param plan - this invocation's resolved value; omitted returns an empty object.
- * @returns the resolved value, or `undefined` when the app asked to exit.
- * @throws when the launcher did not provide the command line and exit request.
+ * @param program - the app's commander program, with its flags, description,
+ * actions, and any subcommands already declared.
+ * @throws when the launcher did not provide the command line and exit request,
+ * or when no command in the program declares an action.
 */
-export function parseCmdline<T>(
-  ctx: Context,
-  program: Command,
-  plan: CmdlinePlan<T> = (() => ({}) as T),
-): T | undefined {
+export function parseCmdline(ctx: Context, program: Command): void {
  // Read through the global service store, not the property proxy: appExit is
  // an optional host value and the plugin only needs to inject cmdlineArgs.
  const args = ctx.get('cmdlineArgs')
@@ -112,23 +103,54 @@ export function parseCmdline<T>(
  if (args === undefined || exit === undefined) {
    throw new Error(`${program.name()}: the launcher must provide ctx.cmdlineArgs and ctx.appExit before the tree mounts`)
  }
-  program
+  if (!hasAction(program)) {
+    throw new Error(`${program.name()}: no command in the program declares an action; parseCmdline runs the invoked command's action on a successful parse, and app code there publishes its service`)
+  }
+  configureExitAndOutput(program)
+  try {
+    program.parse(args.get(), { from: 'user' })
+  } catch (error) {
+    // exitOverride turns help, version, a parse error, and the action's own
+    // program.error() into a CommanderError; commander has already written the
+    // text through the output configured above.
+    if (!isCommanderError(error)) throw error
+    exit(error.exitCode)
+  }
+}
+
+/**
+ * Whether any command in the tree declares an action handler.
+ *
+ * The `Command` type cannot express the action precondition, so the handler is
+ * read structurally (as {@link isCommanderError} reads commander's control-flow
+ * errors): without this guard, a program that forgot its action would parse
+ * successfully, publish nothing, and surface only as dependent rows pending on
+ * the absent service.
+ * @param command - the command whose tree is inspected.
+ * @returns true when the command or any registered subcommand has an action.
+ */
+function hasAction(command: Command): boolean {
+  if (typeof (command as unknown as { _actionHandler?: unknown })._actionHandler === 'function') return true
+  return command.commands.some(hasAction)
+}
+
+/**
+ * Route every command's exit and output through the launcher adapter.
+ *
+ * Commander copies `exitOverride` and output configuration into a subcommand
+ * only at registration, so a root-only override would let an
+ * already-registered subcommand's rejection write to the process streams and
+ * call `process.exit` directly, bypassing `ctx.appExit`.
+ * @param command - the root of the command tree to configure.
+ */
+function configureExitAndOutput(command: Command): void {
+  command
    .exitOverride()
    .configureOutput({
      writeOut: text => void internals.stdout.write(text),
      writeErr: text => void internals.stderr.write(text),
    })
-  try {
-    program.parse(args.get(), { from: 'user' })
-    return plan(program, ctx)
-  } catch (error) {
-    // exitOverride turns help, version, a parse error, and a plan's own
-    // program.error() into a CommanderError; commander has already written the
-    // text through the output configured above.
-    if (!isCommanderError(error)) throw error
-    exit(error.exitCode)
-    return undefined
-  }
+  for (const child of command.commands) configureExitAndOutput(child)
 }

 /**
--- a/packages/boot/cmdline/tests/cmdline.spec.ts
+++ b/packages/boot/cmdline/tests/cmdline.spec.ts
@@ -14,7 +14,7 @@ import Loader from '@deepseek-ai/cordis-plugin-loader'
 import Include from '@deepseek-ai/cordis-plugin-include'
 import type { PatchOptions } from '@deepseek-ai/cordis-plugin-include'
 import { afterEach, describe, expect, it } from 'vitest'
-import { internals, parseCmdline, provideCmdline, type CmdlinePlan } from '../src/index.ts'
+import { internals, parseCmdline, provideCmdline } from '../src/index.ts'

 /** Every value one boot of the fixture tree observed. */
 interface Observed {
@@ -43,8 +43,8 @@ function demoCommand(): Command {
  return new Command().name('demo').exitOverride().option('--port <port>', 'listen port')
 }

-/** The fixture app's plan: the resolved values its rows read. */
-const demoPlan: CmdlinePlan<{ port?: number }> = (program) => {
+/** The fixture app's action body: the resolved values its rows read. */
+const resolveDemo = (program: Command): { port?: number } => {
  const port = program.opts<{ port?: string }>().port
  if (port === undefined) return {}
  if (!/^\d+$/.test(port)) program.error(`error: --port must be a number, got ${JSON.stringify(port)}`)
@@ -58,12 +58,12 @@ const expression = (source: string): unknown => ({ __jsExpr: source })
 * Mount a two-row composition the way a profile boot does: both rows at once,
 * with Loader ordering config resolution from their injections.
 * @param args - the invocation's inner arguments.
- * @param plan - the app's plan; defaults to the fixture's own.
+ * @param resolve - the app's action body; defaults to the fixture's own.
 * @returns the booted fixture.
 */
 async function bootFixture(
  args: string[],
-  plan: CmdlinePlan = demoPlan,
+  resolve: (program: Command) => unknown = resolveDemo,
  options: { objectInject?: boolean; withoutProvider?: boolean } = {},
 ): Promise<Fixture> {
  const dir = mkdtempSync(join(tmpdir(), 'dsh-cmdline-'))
@@ -88,8 +88,9 @@ export function apply(ctx) { return globalThis.__provideDemoArgs(ctx) }
  const globals = globalThis as unknown as { __observed: Observed; __provideDemoArgs: (ctx: Context) => void }
  globals.__observed = observed
  globals.__provideDemoArgs = (ctx: Context) => {
-    const values = parseCmdline(ctx, demoCommand(), plan)
-    if (values !== undefined) ctx.provide('demoStartup', values)
+    const program = demoCommand()
+    program.action(() => { ctx.provide('demoStartup', resolve(program)) })
+    parseCmdline(ctx, program)
  }

  // The composition, exactly as a profile delivers one: include patches whose
@@ -133,7 +134,7 @@ describe('parseCmdline', () => {
  })

  it('recognizes the Loader object form of a provider-service injection', async () => {
-    const { observed } = await bootFixture(['--port', '8080'], demoPlan, { objectInject: true })
+    const { observed } = await bootFixture(['--port', '8080'], resolveDemo, { objectInject: true })
    expect(observed.started).toEqual({ port: 8080 })
  })

@@ -144,31 +145,35 @@ describe('parseCmdline', () => {
    expect(observed.exits).toEqual([0])
  })

-  it('rejects the invocation from the plan without starting the app', async () => {
+  it('rejects the invocation from the action without starting the app', async () => {
    const { observed } = await bootFixture(['--port', 'abc'])
    expect(observed.out).toContain('--port must be a number')
    expect(observed.started).toBeUndefined()
    expect(observed.exits).toEqual([1])
  })

-  it('rethrows a plan failure that is not commander asking to exit', async () => {
-    const { ctx } = await bootFixture([], demoPlan, { withoutProvider: true })
-    const plan: CmdlinePlan = () => { throw new Error('plan exploded') }
-    expect(() => { parseCmdline(ctx, demoCommand(), plan) }).toThrow('plan exploded')
+  it('rethrows an action failure that is not commander asking to exit', async () => {
+    const { ctx } = await bootFixture([], resolveDemo, { withoutProvider: true })
+    const program = demoCommand().action(() => { throw new Error('action exploded') })
+    expect(() => { parseCmdline(ctx, program) }).toThrow('action exploded')
  })

  it('rethrows a thrown value that is not an object at all', async () => {
-    const { ctx } = await bootFixture([], demoPlan, { withoutProvider: true })
-    const plan: CmdlinePlan = () => {
-      const thrown: unknown = 'plan threw a string'
+    const { ctx } = await bootFixture([], resolveDemo, { withoutProvider: true })
+    const program = demoCommand().action(() => {
+      const thrown: unknown = 'action threw a string'
      throw thrown
-    }
-    expect(() => { parseCmdline(ctx, demoCommand(), plan) }).toThrow('plan threw a string')
+    })
+    expect(() => { parseCmdline(ctx, program) }).toThrow('action threw a string')
  })

-  it('returns values without inspecting Loader rows or owning a service', async () => {
-    const { ctx } = await bootFixture([], demoPlan, { withoutProvider: true })
-    expect(parseCmdline(ctx, demoCommand())).toEqual({})
+  it('runs the action without inspecting Loader rows or owning a service', async () => {
+    const { ctx } = await bootFixture([], resolveDemo, { withoutProvider: true })
+    let values: unknown
+    const program = demoCommand()
+    program.action(() => { values = resolveDemo(program) })
+    parseCmdline(ctx, program)
+    expect(values).toEqual({})
    expect(ctx.get('demoStartup')).toBeUndefined()
  })
 })
@@ -182,6 +187,28 @@ describe('provideCmdline', () => {
    expect(ctx.cmdlineArgs?.get()).toEqual(['--resume', 'abc'])
  })

+  it('refuses at load a program in which no command declares an action', async () => {
+    const { ctx } = await bootFixture([], resolveDemo, { withoutProvider: true })
+    expect(() => { parseCmdline(ctx, demoCommand()) })
+      .toThrow('no command in the program declares an action')
+  })
+
+  it('routes a pre-registered subcommand rejection through the launcher exit request', () => {
+    const ctx = new Context()
+    const exits: number[] = []
+    let err = ''
+    internals.stderr = { write: (chunk: string) => { err += chunk; return true } }
+    provideCmdline(ctx, { args: ['serve'], exit: code => void exits.push(code) })
+    // The root declares no action of its own: the tree-wide guard accepts the
+    // subcommand's, and the subcommand inherits the exit and output routing.
+    const program = new Command().name('demo')
+    const child = program.command('serve')
+    child.action(() => { child.error('error: serve rejected') })
+    parseCmdline(ctx, program)
+    expect(err).toContain('serve rejected')
+    expect(exits).toEqual([1])
+  })
+
  it('fails loud when a parser runs without the launcher values', () => {
    const ctx = new Context()
    expect(() => { parseCmdline(ctx, demoCommand()) })
@@ -191,8 +218,15 @@ describe('provideCmdline', () => {
  it('lets multiple parsers read the same immutable snapshot', () => {
    const ctx = new Context()
    provideCmdline(ctx, { args: ['--port', '8080'], exit: () => {} })
-    expect(parseCmdline(ctx, demoCommand(), demoPlan)).toEqual({ port: 8080 })
-    expect(parseCmdline(ctx, demoCommand(), demoPlan)).toEqual({ port: 8080 })
+    const parseOnce = (): unknown => {
+      let values: unknown
+      const program = demoCommand()
+      program.action(() => { values = resolveDemo(program) })
+      parseCmdline(ctx, program)
+      return values
+    }
+    expect(parseOnce()).toEqual({ port: 8080 })
+    expect(parseOnce()).toEqual({ port: 8080 })
    expect(Object.isFrozen(ctx.cmdlineArgs?.get())).toBe(true)
  })
 })