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 9cc7fa8449d..f278fa53d2e 100644 --- a/src/lib/cli/public-dispatch.ts +++ b/src/lib/cli/public-dispatch.ts @@ -33,6 +33,7 @@ import { import { resolveBareConnectArgv } from "./bare-connect-routing"; import { getRegisteredOclifCommandMetadata } from "./oclif-metadata"; import { + matchSandboxRoute, type PublicTranslationResult, translatePublicGlobalArgv, translatePublicSandboxArgv, @@ -134,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 { @@ -267,17 +277,65 @@ 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 { + // 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} ${route.join(" ")}`); + 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."); + 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 an explicit sandbox invocation before reporting an action-like name as a grammar error. */ 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); + 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"); 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 6d5ceaac59c..808f972f635 100644 --- a/test/cli/dispatch-basics.test.ts +++ b/test/cli/dispatch-basics.test.ts @@ -646,6 +646,186 @@ describe("CLI dispatch", () => { ); }); + 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(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([ + { + 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, 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); + }); + }, + ); + + 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"); + }); + }); + + 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, 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}`); + }); + }, + ); + + 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("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."); + expect(output).toContain( + "If onboarding stopped, run 'nemoclaw onboard --resume' to continue it.", + ); + 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"); + + 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("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 }) => { + 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, runOclifCommandById, 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(runOclifCommandById).toHaveBeenCalledWith( + "sandbox:status", + ["doctor"], + expect.anything(), + ); + 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 }) => {