From a572fcb9be4cceee35110387529856c470741e1f Mon Sep 17 00:00:00 2001 From: Brent <67750428+eggpeat@users.noreply.github.com> Date: Fri, 31 Jul 2026 22:01:19 +0000 Subject: [PATCH] fix(cli): preserve help metadata contracts --- packages/coding-agent/CHANGELOG.md | 5 ++- packages/coding-agent/src/cli.ts | 24 ++++++------- .../coding-agent/src/cli/thinking-levels.ts | 13 ++----- .../test/cli-command-metadata.test.ts | 30 ++++++++++++++++ .../test/eval/process-entry-import.test.ts | 11 ++---- packages/utils/CHANGELOG.md | 8 ++--- packages/utils/src/cli.ts | 34 ++++++++++++------- packages/utils/test/cli-help.test.ts | 23 +++++++++++++ 8 files changed, 98 insertions(+), 50 deletions(-) create mode 100644 packages/coding-agent/test/cli-command-metadata.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 451438f96..8f7bde306 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Changed + +- Reduced `omp --help` cold-start latency and memory use by rendering lightweight command metadata without loading every runtime command and provider graph. + ## [17.2.2] - 2026-07-31 ### Added @@ -91,7 +95,6 @@ - Optimized tool guidance for bash, grep, and glob to be more concise while clarifying shell boundaries and search timeouts. - Optimized models configuration resource probing to run in a single child process, reducing startup contention. - Startup release notes now default to a compact change-count summary. Use `startup.changelogMode` (`summary` | `expanded` | `hidden`) to control them; legacy `collapseChangelog` choices migrate automatically ([#6771](https://github.com/can1357/oh-my-pi/issues/6771)). -- Reduced `omp --help` cold-start latency and memory use by rendering lightweight command metadata without loading runtime command, provider, or native-addon graphs. ### Fixed diff --git a/packages/coding-agent/src/cli.ts b/packages/coding-agent/src/cli.ts index 492ac7797..20c7ca0b7 100755 --- a/packages/coding-agent/src/cli.ts +++ b/packages/coding-agent/src/cli.ts @@ -15,7 +15,7 @@ try { * lightweight CLI runner from pi-utils. */ import { parentPort } from "node:worker_threads"; -import type { CliConfig } from "@oh-my-pi/pi-utils/cli"; +import type { CliConfig, CommandMetadata } from "@oh-my-pi/pi-utils/cli"; import { APP_NAME, getActiveProfile, @@ -29,10 +29,13 @@ import { setProcessName } from "@oh-my-pi/pi-utils/process-name"; import { declareWorkerHostEntry, installWorkerInbox, isWorkerHostSelector } from "@oh-my-pi/pi-utils/worker-host"; import { installProfileAlias, resolveProfileAliasCommandFromProcess } from "./cli/profile-alias"; import { extractProfileFlags } from "./cli/profile-bootstrap"; +import { startJsEvalProcess } from "./eval/js/process-entry"; import type { WorkerInbound as JsWorkerInbound, WorkerOutbound as JsWorkerOutbound } from "./eval/js/worker-protocol"; import { DAEMON_BROKER_WORKER_ARG } from "./launch/protocol"; import { TERMINAL_OUTPUT_WORKER_ARG } from "./launch/terminal-output-worker-protocol"; import { COMPUTER_WORKER_ARG } from "./tools/computer/protocol"; +import { smokeTestComputerWorker } from "./tools/computer/supervisor"; +import { startComputerWorker } from "./tools/computer/worker-entry"; if (Bun.semver.order(Bun.version, MIN_BUN_VERSION) < 0) { process.stderr.write( @@ -57,7 +60,7 @@ const isProcessEntry = import.meta.main || process.env.PI_COMPILED === "true"; // `@oh-my-pi/pi-utils/env` eagerly loads `.env` from the agent directory at // import time, so it must not be imported before `setProfile` runs. -async function showHelp(config: CliConfig): Promise { +async function showHelp(config: CliConfig): Promise { // Root help historically loads the selected profile's environment. Keep that // contract after profile bootstrap without pulling in command/provider graphs. await import("@oh-my-pi/pi-utils/env"); @@ -88,7 +91,6 @@ async function runSmokeTest(): Promise { const { smokeTestSttWorker } = await import("./stt/asr-client"); const { smokeTestTtsWorker } = await import("./tts/tts-client"); const { smokeTestMnemopiEmbedWorker } = await import("./mnemopi/embed-client"); - const { smokeTestComputerWorker } = await import("./tools/computer/supervisor"); const { smokeTestJsEvalWorker } = await import("./eval/js/context-manager"); // Other smoke dependencies stay lazy so normal CLI startup does not load their worker clients. const { smokeTestDaemonBroker } = await import("./launch/client"); @@ -166,8 +168,6 @@ async function runWorkerEntrypoint(arg: string | undefined): Promise { } if (arg === COMPUTER_WORKER_ARG) { if (parentPort) installWorkerInbox(parentPort); - // This selector is the module-loading boundary; desktop capture dependencies are worker-only. - const { startComputerWorker } = await import("./tools/computer/worker-entry"); startComputerWorker(); return true; } @@ -177,13 +177,11 @@ async function runWorkerEntrypoint(arg: string | undefined): Promise { return true; } if (arg === JS_EVAL_PROCESS_ARG) { - // This selector is the module-loading boundary; the evaluator process entry is worker-only. - // The bootstrap-safe interceptor stays linked statically so profile-scoped - // environment state cannot load after dispatch has begun. The JS evaluator - // forwards user-controlled payloads (tool-call args, display outputs); a - // non-serializable one must fail that cell, not SIGKILL the kernel and erase - // the eval session's state. - const { startJsEvalProcess } = await import("./eval/js/process-entry"); + // The bootstrap-safe interceptor seam is linked statically so this selector + // cannot load profile-scoped environment state after dispatch has begun. + // The JS evaluator forwards user-controlled payloads (tool-call args, + // display outputs); a non-serializable one must fail that cell, not + // SIGKILL the kernel and erase the eval session's state. await runIpcSubprocessWorker( transport => startJsEvalProcess(transport, interceptUnhandledRejections), { rethrowConnectedSendErrors: true }, @@ -404,7 +402,7 @@ export async function runCli(argv: string[]): Promise { process.exitCode = 1; return; } - return run({ bin: APP_NAME, version: VERSION, argv: resolved.argv, commands, help: showHelp }); + return run({ bin: APP_NAME, version: VERSION, argv: resolved.argv, commands, metadataHelp: showHelp }); } // Floating call instead of top-level await: TLA forces `--bytecode` (CJS diff --git a/packages/coding-agent/src/cli/thinking-levels.ts b/packages/coding-agent/src/cli/thinking-levels.ts index 0898f5581..e954f2f30 100644 --- a/packages/coding-agent/src/cli/thinking-levels.ts +++ b/packages/coding-agent/src/cli/thinking-levels.ts @@ -1,14 +1,7 @@ +import { THINKING_EFFORTS } from "@oh-my-pi/pi-catalog/effort"; + /** * Thinking selectors accepted by the `--thinking` CLI flag, in display order. * Shared by help metadata, shell completions, and validation warnings. */ -export const CLI_THINKING_LEVELS: readonly string[] = [ - "off", - "minimal", - "low", - "medium", - "high", - "xhigh", - "max", - "auto", -]; +export const CLI_THINKING_LEVELS: readonly string[] = ["off", ...THINKING_EFFORTS, "auto"]; diff --git a/packages/coding-agent/test/cli-command-metadata.test.ts b/packages/coding-agent/test/cli-command-metadata.test.ts new file mode 100644 index 000000000..221af0518 --- /dev/null +++ b/packages/coding-agent/test/cli-command-metadata.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from "bun:test"; +import type { CommandMetadata } from "@oh-my-pi/pi-utils/cli"; +import { commands } from "../src/cli-commands"; + +const METADATA_KEYS = [ + "description", + "hidden", + "flags", + "args", + "examples", +] as const satisfies readonly (keyof CommandMetadata)[]; + +describe("CLI command help metadata", () => { + it("is complete and matches every loaded command", async () => { + for (const entry of commands) { + const help = entry.help; + expect(help, `${entry.name} must provide static help metadata`).toBeDefined(); + if (!help) continue; + + const Command = await entry.load(); + for (const key of METADATA_KEYS) { + if (help[key] !== undefined) { + const expected: unknown = help[key]; + const actual: unknown = Command[key]; + expect(expected, `${entry.name}.${key} drifted from its command class`).toEqual(actual); + } + } + } + }); +}); diff --git a/packages/coding-agent/test/eval/process-entry-import.test.ts b/packages/coding-agent/test/eval/process-entry-import.test.ts index 0c12d8d43..d6bb57961 100644 --- a/packages/coding-agent/test/eval/process-entry-import.test.ts +++ b/packages/coding-agent/test/eval/process-entry-import.test.ts @@ -47,25 +47,18 @@ async function pingComputerWorker( } } -it("starts lightweight CLI paths without loading the native addon", async () => { +it("starts ordinary CLI paths without loading the native computer addon", async () => { const cliPath = path.resolve(import.meta.dir, "../../src/cli.ts"); for (const args of [ ["--no-addons", cliPath, "--version"], [cliPath, "--help"], ]) { const proc = Bun.spawn([process.execPath, ...args], { - env: { ...process.env, PI_DEBUG_STARTUP: "1" }, stdout: "pipe", stderr: "pipe", }); - const [exitCode, stdout, stderr] = await Promise.all([ - proc.exited, - new Response(proc.stdout).text(), - new Response(proc.stderr).text(), - ]); + const [exitCode, stderr] = await Promise.all([proc.exited, new Response(proc.stderr).text()]); expect(exitCode, `${args.at(-1)}: ${stderr}`).toBe(0); - expect(stdout).toContain(args.at(-1) === "--help" ? "USAGE" : "omp/"); - expect(stderr).not.toContain("native:loadNative"); } // Two cold CLI spawns (`--version`, `--help`) per run; the assertion is the exit // code, not the wall time. diff --git a/packages/utils/CHANGELOG.md b/packages/utils/CHANGELOG.md index a5fee29f9..3443fc6d5 100644 --- a/packages/utils/CHANGELOG.md +++ b/packages/utils/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Changed + +- Added static command metadata support to the lightweight CLI runner so root help can render without importing command implementations. + ## [17.2.1] - 2026-07-30 ### Added @@ -9,10 +13,6 @@ - Added a `postmortem.quit` configuration option to safely handle shutdown paths when the terminal output has already disconnected. - Added project-keyed OMP security-state directory helpers under the user state root. -### Changed - -- Added static command metadata support to the lightweight CLI runner so root help can render without importing command implementations. - ## [17.1.8] - 2026-07-28 ### Added diff --git a/packages/utils/src/cli.ts b/packages/utils/src/cli.ts index ca15b854c..65a7a4d8a 100644 --- a/packages/utils/src/cli.ts +++ b/packages/utils/src/cli.ts @@ -148,11 +148,11 @@ export interface CommandCtor extends CommandMetadata { } /** Configuration passed to every command instance and help renderers. */ -export interface CliConfig { +export interface CliConfig { bin: string; version: string; /** All registered commands keyed by their canonical name. */ - commands: Map; + commands: Map; } /** Minimal Command base matching the oclif surface we use. */ @@ -293,7 +293,7 @@ export abstract class Command { // --------------------------------------------------------------------------- /** Render full root help: header, default command details, subcommand list. */ -export function renderRootHelp(config: CliConfig): void { +export function renderRootHelp(config: CliConfig): void { const { bin, version, commands } = config; const lines: string[] = []; lines.push(`${bin} v${version}\n`); @@ -418,8 +418,10 @@ export interface RunOptions { version: string; argv: string[]; commands: CommandEntry[]; - /** Custom help renderer. Receives fully-populated config. */ + /** Custom help renderer with the fully loaded command constructors. */ help?: (config: CliConfig) => Promise | void; + /** Lightweight help renderer backed by static command metadata. */ + metadataHelp?: (config: CliConfig) => Promise | void; } /** Find a command entry by exact name or alias. */ @@ -441,11 +443,15 @@ export async function run(opts: RunOptions): Promise { // Top-level help if (commandId === "--help" || commandId === "-h" || commandId === "help" || commandId === "") { - const config = await loadAllCommands(opts); if (opts.help) { - await opts.help(config); + await opts.help(await loadAllCommands(opts)); } else { - renderRootHelp(config); + const config = await loadAllCommandMetadata(opts); + if (opts.metadataHelp) { + await opts.metadataHelp(config); + } else { + renderRootHelp(config); + } } return; } @@ -508,14 +514,16 @@ async function loadEntry(entry: CommandEntry): Promise { return Cmd; } -/** Resolve all command loaders for help/alias display. */ +/** Load every command constructor for backward-compatible custom help callbacks. */ async function loadAllCommands(opts: RunOptions): Promise { - const commands = new Map(); + const loaded = await Promise.all(opts.commands.map(async entry => [entry.name, await loadEntry(entry)] as const)); + return { bin: opts.bin, version: opts.version, commands: new Map(loaded) }; +} + +/** Resolve static command metadata for lightweight root help. */ +async function loadAllCommandMetadata(opts: RunOptions): Promise> { const loaded = await Promise.all( opts.commands.map(async entry => [entry.name, entry.help ?? (await loadEntry(entry))] as const), ); - for (const [name, command] of loaded) { - commands.set(name, command); - } - return { bin: opts.bin, version: opts.version, commands }; + return { bin: opts.bin, version: opts.version, commands: new Map(loaded) }; } diff --git a/packages/utils/test/cli-help.test.ts b/packages/utils/test/cli-help.test.ts index cdeee02e9..80f9e5450 100644 --- a/packages/utils/test/cli-help.test.ts +++ b/packages/utils/test/cli-help.test.ts @@ -96,6 +96,29 @@ describe("run() root help", () => { expect(output).toContain("--model="); expect(output).toContain("good prints good things"); }); + + it("preserves constructable commands for existing custom help callbacks", async () => { + const commands: CommandEntry[] = [ + { name: "good", load: async () => GoodCommand, help: { description: "static summary" } }, + ]; + let receivedConstructor = false; + + await run({ + bin: "omp", + version: "0.0.0", + argv: ["--help"], + commands, + help: config => { + const Command = config.commands.get("good"); + expect(Command).toBe(GoodCommand); + if (Command) { + receivedConstructor = new Command([], config) instanceof GoodCommand; + } + }, + }); + + expect(receivedConstructor).toBe(true); + }); }); describe("run() usage errors", () => {