From baab47fd8301654bf13dc4f6d1939c32fb4f10d4 Mon Sep 17 00:00:00 2001 From: Hai Nguyen Date: Wed, 26 Aug 2026 02:48:54 +0000 Subject: [PATCH 1/8] fix(cli): report sandbox-first grammar for a bare sandbox action A first token that names a sandbox-scoped action reported a missing sandbox and pointed the reader at `onboard`. `nemoclaw doctor` produced "Sandbox 'doctor' does not exist. Run 'nemoclaw onboard' to create one." for a command the reader had typed correctly. Report the required grammar instead, and echo the remaining arguments so the suggestion is runnable. Registry recovery still runs first, so a live sandbox named after an action stays reachable through the name-first grammar. Signed-off-by: Hai Nguyen Co-Authored-By: Claude Opus 5 (1M context) --- src/lib/cli/public-dispatch.ts | 31 +++++++++++- test/cli/dispatch-basics.test.ts | 87 ++++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 1 deletion(-) diff --git a/src/lib/cli/public-dispatch.ts b/src/lib/cli/public-dispatch.ts index 9cc7fa8449d..c9d7f114781 100644 --- a/src/lib/cli/public-dispatch.ts +++ b/src/lib/cli/public-dispatch.ts @@ -267,17 +267,46 @@ function printGlobalStatusScopeHint(sandboxName: string, args: readonly string[] process.exit(2); } +/** + * Report the sandbox-first grammar for a first token that names a sandbox action. + * + * `nemoclaw doctor` and `nemoclaw policy list` name an action, not a sandbox. + * Reporting a missing sandbox sends the reader to `onboard` for a sandbox they + * never asked for (#10212). + */ +function printSandboxScopeHint(action: string, remainingArgs: readonly string[]): never { + const argSuffix = remainingArgs.length > 0 ? ` ${remainingArgs.join(" ")}` : ""; + console.error(` '${action}' is a sandbox command. It needs a sandbox name.`); + console.error(""); + console.error(` Run: ${CLI_NAME} ${action}${argSuffix}`); + const allNames = registeredSandboxNames(); + if (allNames.length > 0) { + console.error(` Registered sandboxes: ${allNames.join(", ")}`); + console.error(` Run '${CLI_NAME} list' to see all sandboxes.`); + } else { + console.error(` Run '${CLI_NAME} onboard' to create one.`); + } + process.exit(1); +} + async function recoverRequestedSandboxIfNeeded( sandboxName: string, action: string, rawArgsAfterSandboxName: string[], ): Promise { - if (registry().getSandbox(sandboxName) || !isKnownSandboxAction(action)) return; + if (registry().getSandbox(sandboxName)) return; + const namesSandboxAction = isKnownSandboxAction(sandboxName); + if (!namesSandboxAction && !isKnownSandboxAction(action)) return; validateName(sandboxName, "sandbox name"); await registryRecovery().recoverRegistryEntries({ requestedSandboxName: sandboxName }); if (registry().getSandbox(sandboxName)) return; + // Recovery runs first so a live sandbox named after an action stays reachable + // through the name-first grammar. A token that recovery cannot resolve is a + // scope error, not a missing sandbox. + if (namesSandboxAction) printSandboxScopeHint(sandboxName, rawArgsAfterSandboxName); + if (rawArgsAfterSandboxName.length === 0) { const suggestion = suggestGlobalCommand(sandboxName); if (suggestion) { diff --git a/test/cli/dispatch-basics.test.ts b/test/cli/dispatch-basics.test.ts index e3883ebe575..2d3a6bd3484 100644 --- a/test/cli/dispatch-basics.test.ts +++ b/test/cli/dispatch-basics.test.ts @@ -647,6 +647,93 @@ describe("CLI dispatch", () => { ); }); + it("reports the sandbox-first grammar for a bare sandbox action (#10212)", async () => { + await withDirectPublicDispatch(async ({ dispatchCli, exitSpy, stderr }) => { + await expect(dispatchCli(["doctor"])).rejects.toThrow("process.exit:1"); + + const output = stderr.join("\n"); + expect(output).toContain("'doctor' is a sandbox command. It needs a sandbox name."); + expect(output).toContain("Run: nemoclaw doctor"); + expect(output).toContain("Run 'nemoclaw onboard' to create one."); + expect(output).not.toContain("Sandbox 'doctor' does not exist."); + expect(exitSpy).toHaveBeenCalledWith(1); + }); + }); + + it("carries trailing arguments into the sandbox-first grammar hint (#10212)", async () => { + await withDirectPublicDispatch(async ({ dispatchCli, stderr }) => { + await expect(dispatchCli(["doctor", "--json"])).rejects.toThrow("process.exit:1"); + + expect(stderr.join("\n")).toContain("Run: nemoclaw doctor --json"); + }); + }); + + it("reports the sandbox-first grammar for a two-token sandbox action (#10212)", async () => { + await withDirectPublicDispatch(async ({ dispatchCli, stderr }) => { + await expect(dispatchCli(["policy", "list"])).rejects.toThrow("process.exit:1"); + + const output = stderr.join("\n"); + expect(output).toContain("'policy' is a sandbox command. It needs a sandbox name."); + expect(output).toContain("Run: nemoclaw policy list"); + expect(output).not.toContain("Unknown command: policy"); + }); + }); + + it("lists registered sandboxes in the sandbox-first grammar hint (#10212)", async () => { + await withDirectPublicDispatch( + async ({ dispatchCli, stderr }) => { + await expect(dispatchCli(["doctor"])).rejects.toThrow("process.exit:1"); + + const output = stderr.join("\n"); + expect(output).toContain("Registered sandboxes: alpha, beta"); + expect(output).not.toContain("Run 'nemoclaw onboard' to create one."); + }, + { sandboxNames: ["alpha", "beta"] }, + ); + }); + + it("does not suggest an unrelated global command for a sandbox action (#10212)", async () => { + await withDirectPublicDispatch(async ({ dispatchCli, stderr }) => { + await expect(dispatchCli(["share"])).rejects.toThrow("process.exit:1"); + + const output = stderr.join("\n"); + expect(output).toContain("'share' is a sandbox command. It needs a sandbox name."); + expect(output).not.toContain("Did you mean: nemoclaw start?"); + }); + }); + + it("keeps the name-first grammar for a sandbox literally named doctor (#10212)", async () => { + await withDirectPublicDispatch( + async ({ dispatchCli, runOclifCommandById, stderr }) => { + await dispatchCli(["doctor", "status"]); + + expect(runOclifCommandById).toHaveBeenCalledWith( + "sandbox:status", + ["doctor"], + expect.anything(), + ); + expect(stderr.join("\n")).not.toContain("is a sandbox command"); + }, + { sandboxNames: ["doctor"] }, + ); + }); + + it("recovers a live sandbox named after an action before reporting scope (#10212)", async () => { + await withDirectPublicDispatch( + async ({ dispatchCli, recoverRegistryEntries, sandboxes, stderr }) => { + recoverRegistryEntries.mockImplementation(async () => { + sandboxes.set("doctor", { name: "doctor" }); + return { sandboxes: [...sandboxes.values()], defaultSandbox: null }; + }); + + await dispatchCli(["doctor", "status"]); + + expect(recoverRegistryEntries).toHaveBeenCalledTimes(1); + expect(stderr.join("\n")).not.toContain("is a sandbox command"); + }, + ); + }); + it("does not suggest a route-only reservation in the connect command-order hint (#8801)", async () => { await withDirectPublicDispatch( async ({ dispatchCli, exitSpy, stderr }) => { From fa5c948a6df77e3c936639d4b2cbc58d3670c0ed Mon Sep 17 00:00:00 2001 From: Hai Nguyen Date: Wed, 26 Aug 2026 03:24:09 +0000 Subject: [PATCH 2/8] test(cli): pin sandbox-action precedence over a near-match global suggestion `nemoclaw agent` reported "Did you mean: nemoclaw agents?" before the scope report existed. `agent` names a sandbox action, so the exact match now wins over the edit-distance guess and the suggestion no longer prints. Record that precedence so a later change cannot restore the guess silently. Signed-off-by: Hai Nguyen Co-Authored-By: Claude Opus 5 (1M context) --- test/cli/dispatch-basics.test.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/cli/dispatch-basics.test.ts b/test/cli/dispatch-basics.test.ts index 2d3a6bd3484..ceaddeacdb3 100644 --- a/test/cli/dispatch-basics.test.ts +++ b/test/cli/dispatch-basics.test.ts @@ -702,6 +702,19 @@ describe("CLI dispatch", () => { }); }); + it("prefers the sandbox-action report over a near-match global suggestion (#10212)", async () => { + await withDirectPublicDispatch(async ({ dispatchCli, stderr }) => { + await expect(dispatchCli(["agent"])).rejects.toThrow("process.exit:1"); + + // `agent` names a sandbox action and `agents` is a separate global + // command. An exact action-token match is more accurate than an + // edit-distance guess, so the scope report replaces the suggestion. + const output = stderr.join("\n"); + expect(output).toContain("'agent' is a sandbox command. It needs a sandbox name."); + expect(output).not.toContain("Did you mean: nemoclaw agents?"); + }); + }); + it("keeps the name-first grammar for a sandbox literally named doctor (#10212)", async () => { await withDirectPublicDispatch( async ({ dispatchCli, runOclifCommandById, stderr }) => { From fc052963708ad247f368d897842c60f6a51f55cb Mon Sep 17 00:00:00 2001 From: Hai Nguyen Date: Wed, 26 Aug 2026 04:10:00 +0000 Subject: [PATCH 3/8] fix(cli): quote sandbox-first grammar hint arguments The hint joined the remaining arguments with a space. An argument that contained whitespace resplit when the reader copied the printed command, and an empty argument disappeared. `nemoclaw exec -- echo "hello world"` printed `echo hello world`, which runs `echo` with two arguments. Quote each argument through the shared `shellQuote` helper when a shell would not pass it through verbatim. An ordinary flag such as `--json` stays unquoted so the common hint stays readable. Signed-off-by: Hai Nguyen Co-Authored-By: Claude Opus 5 (1M context) --- src/lib/cli/public-dispatch.ts | 13 +++++++++++- test/cli/dispatch-basics.test.ts | 36 ++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/src/lib/cli/public-dispatch.ts b/src/lib/cli/public-dispatch.ts index c9d7f114781..71ca965c2a7 100644 --- a/src/lib/cli/public-dispatch.ts +++ b/src/lib/cli/public-dispatch.ts @@ -22,6 +22,7 @@ const { sandboxActionTokensForDispatch, } = require("./command-registry"); +import { shellQuote } from "../core/shell-quote"; import { migrateLegacyPortState } from "../state/legacy-port-migration"; import { type NormalizedArgv, @@ -274,8 +275,18 @@ function printGlobalStatusScopeHint(sandboxName: string, args: readonly string[] * Reporting a missing sandbox sends the reader to `onboard` for a sandbox they * never asked for (#10212). */ +// Arguments a POSIX shell passes through verbatim. Anything else, including an +// empty argument, is quoted so the printed command reproduces what the reader +// typed instead of resplitting on whitespace. +const VERBATIM_HINT_ARGUMENT = /^[A-Za-z0-9_@%+=:,./-]+$/u; + +function quoteHintArgument(argument: string): string { + return VERBATIM_HINT_ARGUMENT.test(argument) ? argument : shellQuote(argument); +} + function printSandboxScopeHint(action: string, remainingArgs: readonly string[]): never { - const argSuffix = remainingArgs.length > 0 ? ` ${remainingArgs.join(" ")}` : ""; + const argSuffix = + remainingArgs.length > 0 ? ` ${remainingArgs.map(quoteHintArgument).join(" ")}` : ""; console.error(` '${action}' is a sandbox command. It needs a sandbox name.`); console.error(""); console.error(` Run: ${CLI_NAME} ${action}${argSuffix}`); diff --git a/test/cli/dispatch-basics.test.ts b/test/cli/dispatch-basics.test.ts index ceaddeacdb3..0e9d8c1e4a6 100644 --- a/test/cli/dispatch-basics.test.ts +++ b/test/cli/dispatch-basics.test.ts @@ -668,6 +668,42 @@ describe("CLI dispatch", () => { }); }); + it("quotes a hint argument that contains whitespace (#10212)", async () => { + await withDirectPublicDispatch(async ({ dispatchCli, stderr }) => { + await expect(dispatchCli(["exec", "--", "echo", "hello world"])).rejects.toThrow( + "process.exit:1", + ); + + // Joining raw would print `echo hello world`, which resplits into two + // arguments when the reader copies the command. + expect(stderr.join("\n")).toContain("Run: nemoclaw exec -- echo 'hello world'"); + }); + }); + + it("quotes an empty hint argument so it survives a copied command (#10212)", async () => { + await withDirectPublicDispatch(async ({ dispatchCli, stderr }) => { + await expect(dispatchCli(["exec", ""])).rejects.toThrow("process.exit:1"); + + expect(stderr.join("\n")).toContain("Run: nemoclaw exec ''"); + }); + }); + + it("escapes a single quote inside a hint argument (#10212)", async () => { + await withDirectPublicDispatch(async ({ dispatchCli, stderr }) => { + await expect(dispatchCli(["exec", "--", "echo", "it's"])).rejects.toThrow("process.exit:1"); + + expect(stderr.join("\n")).toContain(`Run: nemoclaw exec -- echo 'it'\\''s'`); + }); + }); + + it("leaves an ordinary flag argument unquoted in the hint (#10212)", async () => { + await withDirectPublicDispatch(async ({ dispatchCli, stderr }) => { + await expect(dispatchCli(["doctor", "--json"])).rejects.toThrow("process.exit:1"); + + expect(stderr.join("\n")).toContain("Run: nemoclaw doctor --json"); + }); + }); + it("reports the sandbox-first grammar for a two-token sandbox action (#10212)", async () => { await withDirectPublicDispatch(async ({ dispatchCli, stderr }) => { await expect(dispatchCli(["policy", "list"])).rejects.toThrow("process.exit:1"); From bdf664912ceb66991a49392597ab636756f2a18a Mon Sep 17 00:00:00 2001 From: Hai Nguyen Date: Wed, 26 Aug 2026 10:16:02 +0000 Subject: [PATCH 4/8] chore(ci): record shell-quote fan-in after the hint reuse `public-dispatch.ts` now imports the shared `shellQuote` helper instead of adding a sixth local copy. That raises the measured fan-in of `src/lib/core/shell-quote.ts` from 28 to 29. The source-architecture budget rejects a limit that does not match the measured value in either direction, so record 29. Signed-off-by: Hai Nguyen Co-Authored-By: Claude Opus 5 (1M context) --- ci/source-architecture-budget.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/source-architecture-budget.json b/ci/source-architecture-budget.json index 7fd7be0ebc3..f58db4f3105 100644 --- a/ci/source-architecture-budget.json +++ b/ci/source-architecture-budget.json @@ -16,7 +16,7 @@ "src/lib/cli/terminal-style.ts": 43, "src/lib/core/json-types.ts": 37, "src/lib/core/ports.ts": 89, - "src/lib/core/shell-quote.ts": 28, + "src/lib/core/shell-quote.ts": 29, "src/lib/core/url-utils.ts": 30, "src/lib/core/wait.ts": 37, "src/lib/credentials/store.ts": 46, From 0364f982247f34f2d2655b29835976d7e7f376d0 Mon Sep 17 00:00:00 2001 From: Hai Nguyen Date: Thu, 27 Aug 2026 07:33:57 +0000 Subject: [PATCH 5/8] fix(cli): render the registered route in the sandbox-first grammar hint The hint echoed every trailing action argument to stderr. `shellQuote` protects a copied command from shell interpretation, but it preserves credential text, control characters, and the full input length. A mistaken `nemoclaw exec ...` could place a provider credential in captured terminal or CI output. A newline argument could forge a diagnostic line, and an ESC byte could rewrite terminal output. Match the typed tokens against the registered sandbox routes and render the matched route. Every rendered token now comes from the command registry, so no action argument reaches the diagnostic. A multi-token route such as `policy list` stays complete. This removes the argument-quoting path, so `src/lib/core/shell-quote.ts` returns to its previous fan-in. Signed-off-by: Hai Nguyen Co-Authored-By: Claude Opus 5 (1M context) --- ci/source-architecture-budget.json | 2 +- src/lib/cli/command-registry.ts | 18 +++++++++ src/lib/cli/public-dispatch.ts | 19 +++------- test/cli/dispatch-basics.test.ts | 59 +++++++++++++++++++++--------- 4 files changed, 66 insertions(+), 32 deletions(-) diff --git a/ci/source-architecture-budget.json b/ci/source-architecture-budget.json index f58db4f3105..7fd7be0ebc3 100644 --- a/ci/source-architecture-budget.json +++ b/ci/source-architecture-budget.json @@ -16,7 +16,7 @@ "src/lib/cli/terminal-style.ts": 43, "src/lib/core/json-types.ts": 37, "src/lib/core/ports.ts": 89, - "src/lib/core/shell-quote.ts": 29, + "src/lib/core/shell-quote.ts": 28, "src/lib/core/url-utils.ts": 30, "src/lib/core/wait.ts": 37, "src/lib/credentials/store.ts": 46, diff --git a/src/lib/cli/command-registry.ts b/src/lib/cli/command-registry.ts index 8f87b5f03ee..29dbf95167a 100644 --- a/src/lib/cli/command-registry.ts +++ b/src/lib/cli/command-registry.ts @@ -184,6 +184,24 @@ export function directGlobalCommandIds(): Set { return ids; } +/** + * Longest registered sandbox route that prefixes the given public tokens. + * + * Diagnostics render the returned route instead of the tokens a reader typed, + * so untrusted argument text never reaches terminal or log output (#10212). + */ +export function matchSandboxRoute(tokens: readonly string[]): string[] | null { + let longest: string[] | null = null; + for (const commandId of Object.keys(getRegisteredOclifCommandsMetadata())) { + for (const route of sandboxRouteTokenVariants(commandId)) { + if (route.length === 0 || route.length > tokens.length) continue; + if (!route.every((token, index) => token === tokens[index])) continue; + if (!longest || route.length > longest.length) longest = route; + } + } + return longest; +} + export function sandboxActionTokens(): string[] { const seen = new Set(); const tokens: string[] = []; diff --git a/src/lib/cli/public-dispatch.ts b/src/lib/cli/public-dispatch.ts index 71ca965c2a7..e56eabd600f 100644 --- a/src/lib/cli/public-dispatch.ts +++ b/src/lib/cli/public-dispatch.ts @@ -19,10 +19,10 @@ const { canonicalCommandFlagLines, canonicalUsageList, globalCommandTokens, + matchSandboxRoute, sandboxActionTokensForDispatch, } = require("./command-registry"); -import { shellQuote } from "../core/shell-quote"; import { migrateLegacyPortState } from "../state/legacy-port-migration"; import { type NormalizedArgv, @@ -275,21 +275,14 @@ function printGlobalStatusScopeHint(sandboxName: string, args: readonly string[] * Reporting a missing sandbox sends the reader to `onboard` for a sandbox they * never asked for (#10212). */ -// Arguments a POSIX shell passes through verbatim. Anything else, including an -// empty argument, is quoted so the printed command reproduces what the reader -// typed instead of resplitting on whitespace. -const VERBATIM_HINT_ARGUMENT = /^[A-Za-z0-9_@%+=:,./-]+$/u; - -function quoteHintArgument(argument: string): string { - return VERBATIM_HINT_ARGUMENT.test(argument) ? argument : shellQuote(argument); -} - function printSandboxScopeHint(action: string, remainingArgs: readonly string[]): never { - const argSuffix = - remainingArgs.length > 0 ? ` ${remainingArgs.map(quoteHintArgument).join(" ")}` : ""; + // Render the registered route, never the tokens the reader typed. An action + // argument can carry a credential, a newline that forges a diagnostic line, + // or an ESC byte that rewrites terminal output. + const route: string[] = matchSandboxRoute([action, ...remainingArgs]) ?? [action]; console.error(` '${action}' is a sandbox command. It needs a sandbox name.`); console.error(""); - console.error(` Run: ${CLI_NAME} ${action}${argSuffix}`); + console.error(` Run: ${CLI_NAME} ${route.join(" ")}`); const allNames = registeredSandboxNames(); if (allNames.length > 0) { console.error(` Registered sandboxes: ${allNames.join(", ")}`); diff --git a/test/cli/dispatch-basics.test.ts b/test/cli/dispatch-basics.test.ts index 0e9d8c1e4a6..a8a8405e885 100644 --- a/test/cli/dispatch-basics.test.ts +++ b/test/cli/dispatch-basics.test.ts @@ -660,47 +660,70 @@ describe("CLI dispatch", () => { }); }); - it("carries trailing arguments into the sandbox-first grammar hint (#10212)", async () => { + it("omits a flag argument from the sandbox-first grammar hint (#10212)", async () => { await withDirectPublicDispatch(async ({ dispatchCli, stderr }) => { await expect(dispatchCli(["doctor", "--json"])).rejects.toThrow("process.exit:1"); - expect(stderr.join("\n")).toContain("Run: nemoclaw doctor --json"); + const output = stderr.join("\n"); + expect(output).toContain("Run: nemoclaw doctor"); + expect(output).not.toContain("--json"); }); }); - it("quotes a hint argument that contains whitespace (#10212)", async () => { + it("omits a credential-bearing action argument from the hint (#10212)", async () => { await withDirectPublicDispatch(async ({ dispatchCli, stderr }) => { - await expect(dispatchCli(["exec", "--", "echo", "hello world"])).rejects.toThrow( - "process.exit:1", - ); + await expect( + dispatchCli(["exec", "--", "curl", "-H", "Authorization: Bearer nvapi-SECRET12345"]), + ).rejects.toThrow("process.exit:1"); - // Joining raw would print `echo hello world`, which resplits into two - // arguments when the reader copies the command. - expect(stderr.join("\n")).toContain("Run: nemoclaw exec -- echo 'hello world'"); + const output = stderr.join("\n"); + expect(output).toContain("Run: nemoclaw exec"); + expect(output).not.toContain("nvapi-SECRET12345"); + expect(output).not.toContain("Authorization"); }); }); - it("quotes an empty hint argument so it survives a copied command (#10212)", async () => { + it("omits a newline argument that would forge a diagnostic line (#10212)", async () => { await withDirectPublicDispatch(async ({ dispatchCli, stderr }) => { - await expect(dispatchCli(["exec", ""])).rejects.toThrow("process.exit:1"); + await expect( + dispatchCli(["exec", "x\n Sandbox alpha was destroyed."]), + ).rejects.toThrow("process.exit:1"); - expect(stderr.join("\n")).toContain("Run: nemoclaw exec ''"); + const output = stderr.join("\n"); + expect(output).toContain("Run: nemoclaw exec"); + expect(output).not.toContain("Sandbox alpha was destroyed."); }); }); - it("escapes a single quote inside a hint argument (#10212)", async () => { + it("omits an escape byte that would rewrite terminal output (#10212)", async () => { await withDirectPublicDispatch(async ({ dispatchCli, stderr }) => { - await expect(dispatchCli(["exec", "--", "echo", "it's"])).rejects.toThrow("process.exit:1"); + await expect(dispatchCli(["exec", "\u001b[31mRED"])).rejects.toThrow("process.exit:1"); - expect(stderr.join("\n")).toContain(`Run: nemoclaw exec -- echo 'it'\\''s'`); + const output = stderr.join("\n"); + expect(output).toContain("Run: nemoclaw exec"); + expect(output).not.toContain("\u001b"); + expect(output).not.toContain("RED"); }); }); - it("leaves an ordinary flag argument unquoted in the hint (#10212)", async () => { + it("bounds the hint length for a long action argument (#10212)", async () => { await withDirectPublicDispatch(async ({ dispatchCli, stderr }) => { - await expect(dispatchCli(["doctor", "--json"])).rejects.toThrow("process.exit:1"); + const long = "A".repeat(4000); + await expect(dispatchCli(["exec", long])).rejects.toThrow("process.exit:1"); - expect(stderr.join("\n")).toContain("Run: nemoclaw doctor --json"); + const runLine = stderr.find((line) => line.includes("Run: nemoclaw ")) ?? ""; + expect(runLine).toBe(" Run: nemoclaw exec"); + expect(runLine.length).toBeLessThan(80); + }); + }); + + it("renders the registered route for an unregistered trailing token (#10212)", async () => { + await withDirectPublicDispatch(async ({ dispatchCli, stderr }) => { + await expect(dispatchCli(["policy", "not-a-verb"])).rejects.toThrow("process.exit:1"); + + const output = stderr.join("\n"); + expect(output).toContain("Run: nemoclaw policy"); + expect(output).not.toContain("not-a-verb"); }); }); From 83c240c20d93447d1cdb2b3f9f4a2cbb53857052 Mon Sep 17 00:00:00 2001 From: Prekshi Vyas Date: Tue, 1 Sep 2026 11:46:55 -0700 Subject: [PATCH 6/8] fix(cli): address sandbox grammar review findings Signed-off-by: Prekshi Vyas --- src/lib/cli/command-registry.ts | 18 ---- src/lib/cli/public-argv-translation.ts | 18 +++- src/lib/cli/public-dispatch.ts | 25 +++-- test/cli/dispatch-basics.test.ts | 143 ++++++++++++++----------- 4 files changed, 113 insertions(+), 91 deletions(-) diff --git a/src/lib/cli/command-registry.ts b/src/lib/cli/command-registry.ts index 29dbf95167a..8f87b5f03ee 100644 --- a/src/lib/cli/command-registry.ts +++ b/src/lib/cli/command-registry.ts @@ -184,24 +184,6 @@ export function directGlobalCommandIds(): Set { return ids; } -/** - * Longest registered sandbox route that prefixes the given public tokens. - * - * Diagnostics render the returned route instead of the tokens a reader typed, - * so untrusted argument text never reaches terminal or log output (#10212). - */ -export function matchSandboxRoute(tokens: readonly string[]): string[] | null { - let longest: string[] | null = null; - for (const commandId of Object.keys(getRegisteredOclifCommandsMetadata())) { - for (const route of sandboxRouteTokenVariants(commandId)) { - if (route.length === 0 || route.length > tokens.length) continue; - if (!route.every((token, index) => token === tokens[index])) continue; - if (!longest || route.length > longest.length) longest = route; - } - } - return longest; -} - export function sandboxActionTokens(): string[] { const seen = new Set(); const tokens: string[] = []; diff --git a/src/lib/cli/public-argv-translation.ts b/src/lib/cli/public-argv-translation.ts index 822e8b860cc..418f95cc898 100644 --- a/src/lib/cli/public-argv-translation.ts +++ b/src/lib/cli/public-argv-translation.ts @@ -79,6 +79,20 @@ function startsWithTokens(tokens: readonly string[], prefix: readonly string[]): return prefix.every((token, index) => tokens[index] === token); } +function matchRegisteredSandboxRoute(tokens: readonly string[]): SandboxRoute | null { + return sandboxRoutes().find((route) => startsWithTokens(tokens, route.publicTokens)) ?? null; +} + +/** + * Return the longest registered sandbox route that prefixes the public input. + * + * Diagnostics use these registered tokens so untrusted action arguments never + * reach terminal or log output (#10212). + */ +export function matchSandboxRoute(tokens: readonly string[]): string[] | null { + return matchRegisteredSandboxRoute(tokens)?.publicTokens ?? null; +} + function nativeArgv(commandId: string, args: string[], argv?: string[]): NativeArgvTranslation { return { kind: "nativeArgv", commandId, args, argv: argv ?? [...commandId.split(":"), ...args] }; } @@ -162,8 +176,8 @@ export function translatePublicSandboxArgv( } const inputTokens = [action, ...actionArgs]; - for (const route of sandboxRoutes()) { - if (!startsWithTokens(inputTokens, route.publicTokens)) continue; + const route = matchRegisteredSandboxRoute(inputTokens); + if (route) { const remainingArgs = inputTokens.slice(route.publicTokens.length); return nativeArgv(route.commandId, [sandboxName, ...remainingArgs]); } diff --git a/src/lib/cli/public-dispatch.ts b/src/lib/cli/public-dispatch.ts index e56eabd600f..6c1f5cc57ed 100644 --- a/src/lib/cli/public-dispatch.ts +++ b/src/lib/cli/public-dispatch.ts @@ -19,7 +19,6 @@ const { canonicalCommandFlagLines, canonicalUsageList, globalCommandTokens, - matchSandboxRoute, sandboxActionTokensForDispatch, } = require("./command-registry"); @@ -34,6 +33,7 @@ import { import { resolveBareConnectArgv } from "./bare-connect-routing"; import { getRegisteredOclifCommandMetadata } from "./oclif-metadata"; import { + matchSandboxRoute, type PublicTranslationResult, translatePublicGlobalArgv, translatePublicSandboxArgv, @@ -135,11 +135,20 @@ function isMigrationRecoveryInvocation(argv: readonly string[]): boolean { ); } -function registeredSandboxNames(): string[] { +function sandboxRegistrationNames(): { published: string[]; pending: string[] } { const registryApi = registry(); - // Suggestions must use the same published sandbox inventory as `list` and global `status`. - return registryApi.listSandboxes().sandboxes.filter(registryApi.isPublishedSandboxRegistration) - .map((sandbox) => sandbox.name); + const sandboxes = registryApi.listSandboxes().sandboxes; + return { + // Suggestions must use the same published inventory as `list` and global `status`. + published: sandboxes.filter(registryApi.isPublishedSandboxRegistration).map(({ name }) => name), + pending: sandboxes + .filter(({ pendingRouteReservation }) => pendingRouteReservation === true) + .map(({ name }) => name), + }; +} + +function registeredSandboxNames(): string[] { + return sandboxRegistrationNames().published; } function findRegisteredSandboxName(tokens: string[]): string | null { @@ -283,16 +292,20 @@ function printSandboxScopeHint(action: string, remainingArgs: readonly string[]) console.error(` '${action}' is a sandbox command. It needs a sandbox name.`); console.error(""); console.error(` Run: ${CLI_NAME} ${route.join(" ")}`); - const allNames = registeredSandboxNames(); + const { published: allNames, pending: pendingNames } = sandboxRegistrationNames(); if (allNames.length > 0) { console.error(` Registered sandboxes: ${allNames.join(", ")}`); console.error(` Run '${CLI_NAME} list' to see all sandboxes.`); + } else if (pendingNames.length > 0) { + console.error(` Sandbox setup is still pending: ${pendingNames.join(", ")}`); + console.error(" Wait for onboarding to finish or remove the incomplete sandbox."); } else { console.error(` Run '${CLI_NAME} onboard' to create one.`); } process.exit(1); } +/** Recover a missing name before reporting an action-like first token as a grammar error. */ async function recoverRequestedSandboxIfNeeded( sandboxName: string, action: string, diff --git a/test/cli/dispatch-basics.test.ts b/test/cli/dispatch-basics.test.ts index ca63ae31376..bc8dcc200fb 100644 --- a/test/cli/dispatch-basics.test.ts +++ b/test/cli/dispatch-basics.test.ts @@ -659,62 +659,50 @@ describe("CLI dispatch", () => { }); }); - it("omits a flag argument from the sandbox-first grammar hint (#10212)", async () => { - await withDirectPublicDispatch(async ({ dispatchCli, stderr }) => { - await expect(dispatchCli(["doctor", "--json"])).rejects.toThrow("process.exit:1"); - - const output = stderr.join("\n"); - expect(output).toContain("Run: nemoclaw doctor"); - expect(output).not.toContain("--json"); - }); - }); - - it("omits a credential-bearing action argument from the hint (#10212)", async () => { - await withDirectPublicDispatch(async ({ dispatchCli, stderr }) => { - await expect( - dispatchCli(["exec", "--", "curl", "-H", "Authorization: Bearer nvapi-SECRET12345"]), - ).rejects.toThrow("process.exit:1"); - - const output = stderr.join("\n"); - expect(output).toContain("Run: nemoclaw exec"); - expect(output).not.toContain("nvapi-SECRET12345"); - expect(output).not.toContain("Authorization"); - }); - }); - - it("omits a newline argument that would forge a diagnostic line (#10212)", async () => { - await withDirectPublicDispatch(async ({ dispatchCli, stderr }) => { - await expect( - dispatchCli(["exec", "x\n Sandbox alpha was destroyed."]), - ).rejects.toThrow("process.exit:1"); - - const output = stderr.join("\n"); - expect(output).toContain("Run: nemoclaw exec"); - expect(output).not.toContain("Sandbox alpha was destroyed."); - }); - }); - - it("omits an escape byte that would rewrite terminal output (#10212)", async () => { - await withDirectPublicDispatch(async ({ dispatchCli, stderr }) => { - await expect(dispatchCli(["exec", "\u001b[31mRED"])).rejects.toThrow("process.exit:1"); - - const output = stderr.join("\n"); - expect(output).toContain("Run: nemoclaw exec"); - expect(output).not.toContain("\u001b"); - expect(output).not.toContain("RED"); - }); - }); - - it("bounds the hint length for a long action argument (#10212)", async () => { - await withDirectPublicDispatch(async ({ dispatchCli, stderr }) => { - const long = "A".repeat(4000); - await expect(dispatchCli(["exec", long])).rejects.toThrow("process.exit:1"); + it.each([ + { + input: "flag argument", + argv: ["doctor", "--json"], + route: "doctor", + forbidden: /--json/, + }, + { + input: "credential-bearing argument", + argv: ["exec", "--", "curl", "-H", "Authorization: Bearer nvapi-SECRET12345"], + route: "exec", + forbidden: /Authorization|nvapi-SECRET12345/, + }, + { + input: "newline argument", + argv: ["exec", "x\n Sandbox alpha was destroyed."], + route: "exec", + forbidden: /Sandbox alpha was destroyed\./, + }, + { + input: "terminal escape argument", + argv: ["exec", "\u001b[31mRED"], + route: "exec", + forbidden: /\u001b|RED/, + }, + { + input: "long argument", + argv: ["exec", "A".repeat(4000)], + route: "exec", + forbidden: /A{80}/, + }, + ])( + "omits an untrusted $input from the sandbox-first grammar hint (#10212)", + async ({ argv, route, forbidden }) => { + await withDirectPublicDispatch(async ({ dispatchCli, stderr }) => { + await expect(dispatchCli(argv)).rejects.toThrow("process.exit:1"); - const runLine = stderr.find((line) => line.includes("Run: nemoclaw ")) ?? ""; - expect(runLine).toBe(" Run: nemoclaw exec"); - expect(runLine.length).toBeLessThan(80); - }); - }); + const output = stderr.join("\n"); + const runLine = stderr.find((line) => line.includes("Run: nemoclaw ")) ?? ""; + expect(runLine).toBe(` Run: nemoclaw ${route}`); + expect(output).not.toMatch(forbidden); + }); + }, + ); it("renders the registered route for an unregistered trailing token (#10212)", async () => { await withDirectPublicDispatch(async ({ dispatchCli, stderr }) => { @@ -726,16 +714,22 @@ describe("CLI dispatch", () => { }); }); - it("reports the sandbox-first grammar for a two-token sandbox action (#10212)", async () => { - await withDirectPublicDispatch(async ({ dispatchCli, stderr }) => { - await expect(dispatchCli(["policy", "list"])).rejects.toThrow("process.exit:1"); + it.each([ + { argv: ["policy", "list"], action: "policy", route: "policy list" }, + { argv: ["policy-add"], action: "policy-add", route: "policy-add" }, + ])( + "reports the sandbox-first grammar for the registered $route route (#10212)", + async ({ argv, action, route }) => { + await withDirectPublicDispatch(async ({ dispatchCli, stderr }) => { + await expect(dispatchCli(argv)).rejects.toThrow("process.exit:1"); - const output = stderr.join("\n"); - expect(output).toContain("'policy' is a sandbox command. It needs a sandbox name."); - expect(output).toContain("Run: nemoclaw policy list"); - expect(output).not.toContain("Unknown command: policy"); - }); - }); + const output = stderr.join("\n"); + expect(output).toContain(`'${action}' is a sandbox command. It needs a sandbox name.`); + expect(output).toContain(`Run: nemoclaw ${route}`); + expect(output).not.toContain(`Unknown command: ${action}`); + }); + }, + ); it("lists registered sandboxes in the sandbox-first grammar hint (#10212)", async () => { await withDirectPublicDispatch( @@ -750,6 +744,20 @@ describe("CLI dispatch", () => { ); }); + it("reports pending setup instead of new onboarding for a bare sandbox action (#10212)", async () => { + await withDirectPublicDispatch( + async ({ dispatchCli, stderr }) => { + await expect(dispatchCli(["doctor"])).rejects.toThrow("process.exit:1"); + + const output = stderr.join("\n"); + expect(output).toContain("Sandbox setup is still pending: alpha"); + expect(output).toContain("Wait for onboarding to finish or remove the incomplete sandbox."); + expect(output).not.toContain("Run 'nemoclaw onboard' to create one."); + }, + { sandboxNames: ["alpha"], pendingSandboxNames: ["alpha"] }, + ); + }); + it("does not suggest an unrelated global command for a sandbox action (#10212)", async () => { await withDirectPublicDispatch(async ({ dispatchCli, stderr }) => { await expect(dispatchCli(["share"])).rejects.toThrow("process.exit:1"); @@ -791,7 +799,7 @@ describe("CLI dispatch", () => { it("recovers a live sandbox named after an action before reporting scope (#10212)", async () => { await withDirectPublicDispatch( - async ({ dispatchCli, recoverRegistryEntries, sandboxes, stderr }) => { + async ({ dispatchCli, recoverRegistryEntries, runOclifCommandById, sandboxes, stderr }) => { recoverRegistryEntries.mockImplementation(async () => { sandboxes.set("doctor", { name: "doctor" }); return { sandboxes: [...sandboxes.values()], defaultSandbox: null }; @@ -800,6 +808,11 @@ describe("CLI dispatch", () => { await dispatchCli(["doctor", "status"]); expect(recoverRegistryEntries).toHaveBeenCalledTimes(1); + expect(runOclifCommandById).toHaveBeenCalledWith( + "sandbox:status", + ["doctor"], + expect.anything(), + ); expect(stderr.join("\n")).not.toContain("is a sandbox command"); }, ); From d34a78f6f8a244554699265bdd0f08dd2eb778ba Mon Sep 17 00:00:00 2001 From: Prekshi Vyas Date: Tue, 1 Sep 2026 12:48:30 -0700 Subject: [PATCH 7/8] fix(cli): clarify pending onboarding recovery Signed-off-by: Prekshi Vyas --- src/lib/cli/public-dispatch.ts | 5 ++++- test/cli/dispatch-basics.test.ts | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/lib/cli/public-dispatch.ts b/src/lib/cli/public-dispatch.ts index 6c1f5cc57ed..d39055b835c 100644 --- a/src/lib/cli/public-dispatch.ts +++ b/src/lib/cli/public-dispatch.ts @@ -298,7 +298,10 @@ function printSandboxScopeHint(action: string, remainingArgs: readonly string[]) console.error(` Run '${CLI_NAME} list' to see all sandboxes.`); } else if (pendingNames.length > 0) { console.error(` Sandbox setup is still pending: ${pendingNames.join(", ")}`); - console.error(" Wait for onboarding to finish or remove the incomplete sandbox."); + console.error(" Wait for onboarding to finish."); + console.error( + ` If onboarding stopped, run '${CLI_NAME} onboard --resume' after correcting the reported condition.`, + ); } else { console.error(` Run '${CLI_NAME} onboard' to create one.`); } diff --git a/test/cli/dispatch-basics.test.ts b/test/cli/dispatch-basics.test.ts index bc8dcc200fb..f118b92347a 100644 --- a/test/cli/dispatch-basics.test.ts +++ b/test/cli/dispatch-basics.test.ts @@ -751,7 +751,10 @@ describe("CLI dispatch", () => { const output = stderr.join("\n"); expect(output).toContain("Sandbox setup is still pending: alpha"); - expect(output).toContain("Wait for onboarding to finish or remove the incomplete sandbox."); + expect(output).toContain("Wait for onboarding to finish."); + expect(output).toContain( + "If onboarding stopped, run 'nemoclaw onboard --resume' after correcting the reported condition.", + ); expect(output).not.toContain("Run 'nemoclaw onboard' to create one."); }, { sandboxNames: ["alpha"], pendingSandboxNames: ["alpha"] }, From dd955420471d555cd0771b1f5817abd59cbf3541 Mon Sep 17 00:00:00 2001 From: Prekshi Vyas Date: Tue, 1 Sep 2026 13:25:00 -0700 Subject: [PATCH 8/8] fix(cli): keep grammar errors side-effect free Signed-off-by: Prekshi Vyas --- src/lib/cli/public-dispatch.ts | 17 +++++++++++++---- test/cli/dispatch-basics.test.ts | 31 ++++++++++++++++++------------- 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/src/lib/cli/public-dispatch.ts b/src/lib/cli/public-dispatch.ts index d39055b835c..f278fa53d2e 100644 --- a/src/lib/cli/public-dispatch.ts +++ b/src/lib/cli/public-dispatch.ts @@ -299,16 +299,14 @@ function printSandboxScopeHint(action: string, remainingArgs: readonly string[]) } else if (pendingNames.length > 0) { console.error(` Sandbox setup is still pending: ${pendingNames.join(", ")}`); console.error(" Wait for onboarding to finish."); - console.error( - ` If onboarding stopped, run '${CLI_NAME} onboard --resume' after correcting the reported condition.`, - ); + console.error(` If onboarding stopped, run '${CLI_NAME} onboard --resume' to continue it.`); } else { console.error(` Run '${CLI_NAME} onboard' to create one.`); } process.exit(1); } -/** Recover a missing name before reporting an action-like first token as a grammar error. */ +/** Recover an explicit sandbox invocation before reporting an action-like name as a grammar error. */ async function recoverRequestedSandboxIfNeeded( sandboxName: string, action: string, @@ -316,6 +314,17 @@ async function recoverRequestedSandboxIfNeeded( ): Promise { if (registry().getSandbox(sandboxName)) return; const namesSandboxAction = isKnownSandboxAction(sandboxName); + const hasExplicitSandboxAction = + rawArgsAfterSandboxName.length > 0 && isKnownSandboxAction(rawArgsAfterSandboxName[0] ?? ""); + + // An action-first diagnostic must stay read-only. Seeded registry recovery + // can select or start a gateway and persist entries, which is inappropriate + // for a bare action or registered multi-token route entered without a name. + // Keep recovery for an explicit action-name sandbox invocation such as + // `doctor status`, where `doctor` can be a real sandbox with a stale entry. + if (namesSandboxAction && !hasExplicitSandboxAction) { + printSandboxScopeHint(sandboxName, rawArgsAfterSandboxName); + } if (!namesSandboxAction && !isKnownSandboxAction(action)) return; validateName(sandboxName, "sandbox name"); diff --git a/test/cli/dispatch-basics.test.ts b/test/cli/dispatch-basics.test.ts index f118b92347a..808f972f635 100644 --- a/test/cli/dispatch-basics.test.ts +++ b/test/cli/dispatch-basics.test.ts @@ -646,17 +646,20 @@ describe("CLI dispatch", () => { ); }); - it("reports the sandbox-first grammar for a bare sandbox action (#10212)", async () => { - await withDirectPublicDispatch(async ({ dispatchCli, exitSpy, stderr }) => { - await expect(dispatchCli(["doctor"])).rejects.toThrow("process.exit:1"); + it("reports the sandbox-first grammar without recovering a bare action (#10212)", async () => { + await withDirectPublicDispatch( + async ({ dispatchCli, exitSpy, recoverRegistryEntries, stderr }) => { + await expect(dispatchCli(["doctor"])).rejects.toThrow("process.exit:1"); - const output = stderr.join("\n"); - expect(output).toContain("'doctor' is a sandbox command. It needs a sandbox name."); - expect(output).toContain("Run: nemoclaw doctor"); - expect(output).toContain("Run 'nemoclaw onboard' to create one."); - expect(output).not.toContain("Sandbox 'doctor' does not exist."); - expect(exitSpy).toHaveBeenCalledWith(1); - }); + const output = stderr.join("\n"); + expect(recoverRegistryEntries).not.toHaveBeenCalled(); + expect(output).toContain("'doctor' is a sandbox command. It needs a sandbox name."); + expect(output).toContain("Run: nemoclaw doctor"); + expect(output).toContain("Run 'nemoclaw onboard' to create one."); + expect(output).not.toContain("Sandbox 'doctor' does not exist."); + expect(exitSpy).toHaveBeenCalledWith(1); + }, + ); }); it.each([ @@ -693,11 +696,12 @@ describe("CLI dispatch", () => { ])( "omits an untrusted $input from the sandbox-first grammar hint (#10212)", async ({ argv, route, forbidden }) => { - await withDirectPublicDispatch(async ({ dispatchCli, stderr }) => { + await withDirectPublicDispatch(async ({ dispatchCli, recoverRegistryEntries, stderr }) => { await expect(dispatchCli(argv)).rejects.toThrow("process.exit:1"); const output = stderr.join("\n"); const runLine = stderr.find((line) => line.includes("Run: nemoclaw ")) ?? ""; + expect(recoverRegistryEntries).not.toHaveBeenCalled(); expect(runLine).toBe(` Run: nemoclaw ${route}`); expect(output).not.toMatch(forbidden); }); @@ -720,10 +724,11 @@ describe("CLI dispatch", () => { ])( "reports the sandbox-first grammar for the registered $route route (#10212)", async ({ argv, action, route }) => { - await withDirectPublicDispatch(async ({ dispatchCli, stderr }) => { + await withDirectPublicDispatch(async ({ dispatchCli, recoverRegistryEntries, stderr }) => { await expect(dispatchCli(argv)).rejects.toThrow("process.exit:1"); const output = stderr.join("\n"); + expect(recoverRegistryEntries).not.toHaveBeenCalled(); expect(output).toContain(`'${action}' is a sandbox command. It needs a sandbox name.`); expect(output).toContain(`Run: nemoclaw ${route}`); expect(output).not.toContain(`Unknown command: ${action}`); @@ -753,7 +758,7 @@ describe("CLI dispatch", () => { expect(output).toContain("Sandbox setup is still pending: alpha"); expect(output).toContain("Wait for onboarding to finish."); expect(output).toContain( - "If onboarding stopped, run 'nemoclaw onboard --resume' after correcting the reported condition.", + "If onboarding stopped, run 'nemoclaw onboard --resume' to continue it.", ); expect(output).not.toContain("Run 'nemoclaw onboard' to create one."); },