From 6c3bb32ccd3339b9deb557656ca0b3e7fa140e20 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Tue, 11 Aug 2026 04:03:58 +0200 Subject: [PATCH 1/2] refactor(cli): move CLI head dispatch into src/cli/root.ts Phase 1 of the CLI deepening: version/help early exits, the ocx ready pre-parse (exit 64 before any preflight), and the bounded Codex-shim auto-restore preflight move from src/cli/index.ts into a new pure module src/cli/root.ts. index.ts now awaits runCli() and dispatches via its existing switch; behavior is preserved byte-for-byte. - parseCliHead is pure and unit-tested (tests/cli-head.test.ts) - P1 source-guards in tests/cli-ready.test.ts relocate to root.ts - cli-head/cli-ready/cli-help: 74 pass; typecheck green --- src/cli/index.ts | 61 ++++++------------- src/cli/root.ts | 86 ++++++++++++++++++++++++++ tests/cli-head.test.ts | 130 ++++++++++++++++++++++++++++++++++++++++ tests/cli-ready.test.ts | 37 ++++++++---- 4 files changed, 259 insertions(+), 55 deletions(-) create mode 100644 src/cli/root.ts create mode 100644 tests/cli-head.test.ts diff --git a/src/cli/index.ts b/src/cli/index.ts index 35da313d7..840989d7b 100755 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -35,11 +35,12 @@ import { } from "./tray-proxy"; import { requestBoundSystemRestart } from "./system-restart-client"; import { installCrashGuards } from "../lib/crash-guard"; -import { hasHelpFlag, printSubcommandUsage, printUsage, printVersion } from "./help"; +import { hasHelpFlag, printSubcommandUsage, printUsage } from "./help"; import { findAvailablePort, isAddrInUse, PortUnavailableError, shouldPersistSelectedPort, waitForPortAvailable } from "../server/ports"; import { findLiveProxy, probeHostname, type LiveProxy } from "../server/proxy-liveness"; import { createReadinessGate } from "../server/readiness"; -import { parseReadyArgs, runReady, type ReadyArgs } from "./ready"; +import { runReady, type ReadyArgs } from "./ready"; +import { runCli } from "./root"; import { ProxyOwnershipRefusedError, stopProxy } from "../lib/process-control"; import { loadServiceTokenFromFile } from "../lib/service-secrets"; import { diagnoseService, isServiceOwnershipError, serviceCommand, serviceEnvironmentOwnedHere, serviceStartableFromTray, serviceStatusSummary, stopServiceIfInstalled, uninstallServiceIfInstalled } from "../service"; @@ -50,7 +51,6 @@ import { buildDesktop3pRegistry } from "../claude/desktop-3p"; import { installShellHook, uninstallShellHook } from "../server/system-env"; import { startTokenGuardian } from "../oauth/token-guardian"; import { startHistoryMigrationGuardian } from "../codex/history-migration-guardian"; -import { maybeAutoRestoreCodexShim } from "./codex-shim-autorestore"; import { maybeShowStarPrompt } from "./star-prompt"; import { scheduleCatalogPrewarm } from "./catalog-prewarm"; import { maybeShowUpdatePrompt } from "../update/notify"; @@ -65,43 +65,15 @@ import { createLocalAttestationSecret } from "../lib/local-management-attestatio import { MEMORY_DRAIN_RESTART_MS, REPLACEMENT_READY_TIMEOUT_MS } from "../lib/system-restart-contract"; initializeNodeLauncherContext(); -const args = process.argv.slice(2); -const command = args[0]; -if (command === "--version" || command === "-v" || command === "version") { - printVersion(); - process.exit(0); -} - -if (command === undefined || command === "help" || command === "--help" || command === "-h") { - if (command === "help" && args[1]) printSubcommandUsage(args[1]); - else printUsage(); - process.exit(0); -} - -if (command !== undefined && command !== "help" && hasHelpFlag(args.slice(1))) { - printSubcommandUsage(command); - process.exit(0); -} - -// P1: pre-parse `ocx ready` and reject invalid arguments with exit 64 BEFORE -// maybeAutoRestoreCodexShim (or any discovery/probe/filesystem-capable global -// preflight) runs. `ready --help` / `help ready` already exited above, so this -// only sees ready args without a help flag. Valid args are stashed so the -// switch dispatch can call runReady without a second parse. -let readyArgs: ReadyArgs | undefined; -if (command === "ready") { - const parsed = parseReadyArgs(args.slice(1)); - if (!parsed.ok) { - console.error("Usage: ocx ready [--json] [--wait [--timeout ]]"); - console.error(" --timeout requires --wait; must be a positive integer (1..300)."); - console.error(" Default wait timeout is 45 seconds."); - process.exit(parsed.code); - } - readyArgs = parsed.args; -} - -maybeAutoRestoreCodexShim(command, args); +// Head: version/help early exits, `ready` pre-parse (exit 64 before any +// preflight), and the bounded Codex-shim auto-restore preflight live in +// src/cli/root.ts (Phase 1 of the CLI deepening). runCli exits for +// version/help and returns the dispatchable head otherwise; the switch below +// owns command dispatch. +const head = await runCli(process.argv.slice(2)); +const args = head.args; +const command = head.command; function parsePortOption(): number | undefined { if (args.length === 1) return undefined; @@ -1258,14 +1230,17 @@ switch (command) { } process.exit(live ? 0 : 1); } - case "ready": + case "ready": { // Fail-closed impossible-state guard: readyArgs is populated by the - // preparse block before maybeAutoRestoreCodexShim, so reaching here - // without it means dispatch diverged. Refuse with code 64 and perform - // NO I/O (no discovery/probe). process.exit is `never`, narrowing below. + // preparse block in src/cli/root.ts before maybeAutoRestoreCodexShim, so + // reaching here without it means dispatch diverged. Refuse with code 64 + // and perform NO I/O (no discovery/probe). process.exit is `never`, + // narrowing below. + const readyArgs = head.readyArgs; if (!readyArgs) process.exit(64); await handleReady(readyArgs); break; + } case "provider": { const { handleProviderCommand } = await import("./provider"); await handleProviderCommand(args.slice(1)); diff --git a/src/cli/root.ts b/src/cli/root.ts new file mode 100644 index 000000000..65eaf21cd --- /dev/null +++ b/src/cli/root.ts @@ -0,0 +1,86 @@ +/** + * CLI head: version/help early exits, `ocx ready` pre-parse, and the bounded + * Codex-shim auto-restore preflight, in that order (Phase 1 of the CLI + * deepening — moved out of src/cli/index.ts). + * + * `parseCliHead` is pure (no I/O, no process access) so the ordering and the + * single-parse contract are unit-testable without a subprocess. `runCli` owns + * the exit paths and the shim preflight, then returns the dispatchable head + * for the command switch in src/cli/index.ts. + */ +import { hasHelpFlag, printSubcommandUsage, printUsage, printVersion } from "./help"; +import { parseReadyArgs, type ReadyArgs } from "./ready"; +import { maybeAutoRestoreCodexShim } from "./codex-shim-autorestore"; + +export interface CliHead { + kind: "version" | "help" | "ready" | "command"; + command: string | undefined; + args: string[]; + /** For kind "help": the subcommand whose usage should print, if any. */ + helpTarget?: string; + /** Present only for `ready`; undefined when the ready args failed to parse. */ + readyArgs?: ReadyArgs; +} + +export function parseCliHead(argv: string[]): CliHead { + const command = argv[0]; + const args = argv; + if (command === "--version" || command === "-v" || command === "version") { + return { kind: "version", command, args }; + } + if (command === undefined || command === "help" || command === "--help" || command === "-h") { + // `ocx help ` carries the subcommand; bare/flag help prints the full usage. + return { + kind: "help", + command, + args, + ...(command === "help" && args[1] ? { helpTarget: args[1] } : {}), + }; + } + if (command !== "help" && hasHelpFlag(args.slice(1))) { + // `ocx --help|-h|help` prints that command's usage, not the full list. + return { kind: "help", command, args, helpTarget: command }; + } + // P1: pre-parse `ocx ready` and reject invalid arguments with exit 64 BEFORE + // maybeAutoRestoreCodexShim (or any discovery/probe/filesystem-capable global + // preflight) runs. `ready --help` / `help ready` already exited above, so this + // only sees ready args without a help flag. Valid args are stashed so the + // switch dispatch can call runReady without a second parse. + if (command === "ready") { + const parsed = parseReadyArgs(args.slice(1)); + if (!parsed.ok) return { kind: "ready", command, args, readyArgs: undefined }; + return { kind: "ready", command, args, readyArgs: parsed.args }; + } + return { kind: "command", command, args }; +} + +export async function runCli(argv: string[]): Promise { + const head = parseCliHead(argv); + switch (head.kind) { + case "version": + printVersion(); + process.exit(0); + case "help": { + if (head.helpTarget) printSubcommandUsage(head.helpTarget); + else printUsage(); + process.exit(0); + } + case "ready": { + // Fail-closed impossible-state guard: parseCliHead already ran the + // pre-parse before any shim/preflight side effect, so reaching here + // without readyArgs means dispatch diverged. Refuse with code 64 and + // perform NO I/O (no discovery/probe). + if (!head.readyArgs) { + console.error("Usage: ocx ready [--json] [--wait [--timeout ]]"); + console.error(" --timeout requires --wait; must be a positive integer (1..300)."); + console.error(" Default wait timeout is 45 seconds."); + process.exit(64); + } + maybeAutoRestoreCodexShim(head.command, head.args); + return head; + } + case "command": + maybeAutoRestoreCodexShim(head.command, head.args); + return head; + } +} diff --git a/tests/cli-head.test.ts b/tests/cli-head.test.ts new file mode 100644 index 000000000..380d8bb97 --- /dev/null +++ b/tests/cli-head.test.ts @@ -0,0 +1,130 @@ +import { describe, expect, test } from "bun:test"; +import { parseCliHead } from "../src/cli/root"; +import { DEFAULT_READY_WAIT_TIMEOUT_SECONDS } from "../src/cli/ready"; + +describe("parseCliHead (pure CLI head, Phase 1)", () => { + test("version flags exit as version", () => { + expect(parseCliHead(["--version"])).toEqual({ kind: "version", command: "--version", args: ["--version"] }); + expect(parseCliHead(["-v"])).toEqual({ kind: "version", command: "-v", args: ["-v"] }); + expect(parseCliHead(["version"])).toEqual({ kind: "version", command: "version", args: ["version"] }); + }); + + test("bare help forms exit as help", () => { + expect(parseCliHead([])).toEqual({ kind: "help", command: undefined, args: [] }); + expect(parseCliHead(["help"])).toEqual({ kind: "help", command: "help", args: ["help"] }); + expect(parseCliHead(["--help"])).toEqual({ kind: "help", command: "--help", args: ["--help"] }); + expect(parseCliHead(["-h"])).toEqual({ kind: "help", command: "-h", args: ["-h"] }); + }); + + test("help with a subcommand carries the subcommand", () => { + expect(parseCliHead(["help", "service"])).toEqual({ + kind: "help", + command: "help", + args: ["help", "service"], + helpTarget: "service", + }); + }); + + test("help with an unknown subcommand still carries the helpTarget", () => { + expect(parseCliHead(["help", "nosuch"])).toEqual({ + kind: "help", + command: "help", + args: ["help", "nosuch"], + helpTarget: "nosuch", + }); + }); + + test("help flag after position 0 is a help exit for that command", () => { + expect(parseCliHead(["sync", "--help"])).toEqual({ + kind: "help", + command: "sync", + args: ["sync", "--help"], + helpTarget: "sync", + }); + expect(parseCliHead(["provider", "-h"])).toEqual({ + kind: "help", + command: "provider", + args: ["provider", "-h"], + helpTarget: "provider", + }); + }); + + test("helpTarget routes the subcommand or command whose usage should print", () => { + expect(parseCliHead(["help", "service"])).toEqual({ + kind: "help", + command: "help", + args: ["help", "service"], + helpTarget: "service", + }); + expect(parseCliHead(["sync", "--help"])).toEqual({ + kind: "help", + command: "sync", + args: ["sync", "--help"], + helpTarget: "sync", + }); + expect(parseCliHead(["provider", "-h"])).toEqual({ + kind: "help", + command: "provider", + args: ["provider", "-h"], + helpTarget: "provider", + }); + expect(parseCliHead(["ready", "--help"])).toEqual({ + kind: "help", + command: "ready", + args: ["ready", "--help"], + helpTarget: "ready", + }); + }); + + test("valid ready args are pre-parsed and stashed", () => { + expect(parseCliHead(["ready"])).toEqual({ + kind: "ready", + command: "ready", + args: ["ready"], + readyArgs: { json: false, wait: false, timeoutSeconds: DEFAULT_READY_WAIT_TIMEOUT_SECONDS }, + }); + expect(parseCliHead(["ready", "--json", "--wait", "--timeout", "120"])).toEqual({ + kind: "ready", + command: "ready", + args: ["ready", "--json", "--wait", "--timeout", "120"], + readyArgs: { json: true, wait: true, timeoutSeconds: 120 }, + }); + }); + + test("invalid ready args fail closed with no readyArgs", () => { + expect(parseCliHead(["ready", "--timeout", "5"])).toEqual({ + kind: "ready", + command: "ready", + args: ["ready", "--timeout", "5"], + readyArgs: undefined, + }); + expect(parseCliHead(["ready", "--nope"])).toEqual({ + kind: "ready", + command: "ready", + args: ["ready", "--nope"], + readyArgs: undefined, + }); + expect(parseCliHead(["ready", "--wait", "--timeout", "abc"])).toEqual({ + kind: "ready", + command: "ready", + args: ["ready", "--wait", "--timeout", "abc"], + readyArgs: undefined, + }); + }); + + test("ordinary commands dispatch as command", () => { + expect(parseCliHead(["status"])).toEqual({ kind: "command", command: "status", args: ["status"] }); + expect(parseCliHead(["start", "--port", "8080"])).toEqual({ + kind: "command", + command: "start", + args: ["start", "--port", "8080"], + }); + expect(parseCliHead(["sync"])).toEqual({ kind: "command", command: "sync", args: ["sync"] }); + expect(parseCliHead(["provider", "list"])).toEqual({ + kind: "command", + command: "provider", + args: ["provider", "list"], + }); + expect(parseCliHead([""])).toEqual({ kind: "command", command: "", args: [""] }); + }); +}); diff --git a/tests/cli-ready.test.ts b/tests/cli-ready.test.ts index 329ae9211..0adbd8403 100644 --- a/tests/cli-ready.test.ts +++ b/tests/cli-ready.test.ts @@ -681,29 +681,36 @@ describe("handleStart readinessGate wiring (source-level)", () => { // maybeAutoRestoreCodexShim preflight (or any discovery/probe/filesystem-capable // step) runs. These source-level guards pin that ordering and the single-parse // contract so a future edit cannot silently move parsing back into handleReady -// or after auto-restore. No subprocess/network/HOME is used. +// or after auto-restore. The head block lives in src/cli/root.ts (Phase 1 of the +// CLI deepening); the dispatch switch stays in src/cli/index.ts. No +// subprocess/network/HOME is used. describe("ready pre-parse before maybeAutoRestoreCodexShim (source-level, P1)", () => { + const rootSource = readFileSync(join(import.meta.dir, "../src/cli/root.ts"), "utf8"); const cliSource = readFileSync(join(import.meta.dir, "../src/cli/index.ts"), "utf8"); test("ready pre-parse call runs BEFORE maybeAutoRestoreCodexShim", () => { - const preparseIdx = cliSource.indexOf("parseReadyArgs(args.slice(1))"); + const preparseIdx = rootSource.indexOf("parseReadyArgs(args.slice(1))"); expect(preparseIdx, "pre-parse must call parseReadyArgs(args.slice(1))").toBeGreaterThanOrEqual(0); - const autoIdx = cliSource.indexOf("maybeAutoRestoreCodexShim(command, args)"); - expect(autoIdx, "maybeAutoRestoreCodexShim(command, args) call must be present").toBeGreaterThanOrEqual(0); + const autoIdx = rootSource.indexOf("maybeAutoRestoreCodexShim(head.command, head.args)"); + expect(autoIdx, "maybeAutoRestoreCodexShim must be called in runCli").toBeGreaterThanOrEqual(0); expect(preparseIdx, "ready pre-parse must precede maybeAutoRestoreCodexShim").toBeLessThan(autoIdx); }); test("invalid ready exits 64 inside the pre-parse block, before auto-restore", () => { - const autoIdx = cliSource.indexOf("maybeAutoRestoreCodexShim(command, args)"); - const beforeAuto = cliSource.slice(0, autoIdx); + // parseCliHead (pure) returns readyArgs: undefined for invalid args; the + // fail-closed runCli guard then exits 64 before any shim/discovery side + // effect can run. + const autoIdx = rootSource.indexOf("maybeAutoRestoreCodexShim(head.command, head.args)"); + const beforeAuto = rootSource.slice(0, autoIdx); expect(beforeAuto).toContain('command === "ready"'); expect(beforeAuto).toContain("parseReadyArgs(args.slice(1))"); - expect(beforeAuto).toContain("process.exit(parsed.code)"); + expect(beforeAuto).toContain("process.exit(64)"); }); - test("exactly one runtime parseReadyArgs(args.slice(1)) call site in cli/index.ts", () => { - const matches = cliSource.match(/parseReadyArgs\(args\.slice\(1\)\)/g); - expect(matches, "parseReadyArgs(args.slice(1)) must appear exactly once (no re-parse)").toHaveLength(1); + test("exactly one runtime parseReadyArgs(args.slice(1)) call site across the CLI head", () => { + const rootMatches = rootSource.match(/parseReadyArgs\(args\.slice\(1\)\)/g); + expect(rootMatches, "parseReadyArgs(args.slice(1)) must appear exactly once in root.ts (no re-parse)").toHaveLength(1); + expect(cliSource).not.toContain("parseReadyArgs("); }); test("handleReady accepts pre-parsed ReadyArgs and never re-parses", () => { @@ -720,10 +727,16 @@ describe("ready pre-parse before maybeAutoRestoreCodexShim (source-level, P1)", }); test("valid ready dispatch reaches handleReady AFTER maybeAutoRestoreCodexShim, with fail-closed guard", () => { - const autoIdx = cliSource.indexOf("maybeAutoRestoreCodexShim(command, args)"); + // Ordering: index.ts awaits runCli (which runs parseCliHead and the shim + // preflight inside root.ts) BEFORE the switch dispatches the ready case. + const runCliIdx = cliSource.indexOf("await runCli(process.argv.slice(2))"); + expect(runCliIdx, "index.ts must await runCli before dispatch").toBeGreaterThanOrEqual(0); + const switchIdx = cliSource.indexOf("switch (command)"); + expect(switchIdx, "the command switch must exist").toBeGreaterThanOrEqual(0); + expect(runCliIdx).toBeLessThan(switchIdx); + expect(rootSource).toContain("maybeAutoRestoreCodexShim(head.command, head.args)"); const readyCaseIdx = cliSource.indexOf('case "ready":'); expect(readyCaseIdx, 'a "ready" switch case must exist').toBeGreaterThanOrEqual(0); - expect(autoIdx).toBeLessThan(readyCaseIdx); // Slice the whole ready case body (up to the next case), not a fixed width. const nextCaseIdx = cliSource.indexOf("case ", readyCaseIdx + 1); const caseBody = cliSource.slice(readyCaseIdx, nextCaseIdx === -1 ? undefined : nextCaseIdx); From c2da0f609175889cf2a7838a49616e3e34777113 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Tue, 11 Aug 2026 04:38:24 +0200 Subject: [PATCH 2/2] test(cli): cover the trailing help alias in parseCliHead CodeRabbit finding on #1444: the help-flag test covered --help and -h after position 0 but not the bare 'help' token, which hasHelpFlag also matches. --- tests/cli-head.test.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/cli-head.test.ts b/tests/cli-head.test.ts index 380d8bb97..9b35a9359 100644 --- a/tests/cli-head.test.ts +++ b/tests/cli-head.test.ts @@ -41,6 +41,12 @@ describe("parseCliHead (pure CLI head, Phase 1)", () => { args: ["sync", "--help"], helpTarget: "sync", }); + expect(parseCliHead(["sync", "help"])).toEqual({ + kind: "help", + command: "sync", + args: ["sync", "help"], + helpTarget: "sync", + }); expect(parseCliHead(["provider", "-h"])).toEqual({ kind: "help", command: "provider",