Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ci/source-architecture-budget.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"src/lib/adapters/openshell/client.ts": 20,
"src/lib/adapters/openshell/resolve.ts": 27,
"src/lib/adapters/openshell/runtime.ts": 55,
"src/lib/adapters/openshell/timeouts.ts": 39,
"src/lib/adapters/openshell/timeouts.ts": 38,
"src/lib/agent/defs.ts": 33,
"src/lib/cli/branding.ts": 86,
"src/lib/cli/nemoclaw-oclif-command.ts": 107,
Expand Down
48 changes: 23 additions & 25 deletions src/commands/debug.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,32 +9,33 @@ import { runDebug } from "../lib/diagnostics/debug";
import type { DebugOptions } from "../lib/diagnostics/debug";
import type { RunDebugCommandDeps } from "../lib/diagnostics/debug-command";
import { runDebugCommandWithOptions } from "../lib/diagnostics/debug-command";
import type { CaptureOpenshellResult } from "../lib/adapters/openshell/client";
import { captureOpenshellCommand } from "../lib/adapters/openshell/client";
import { OPENSHELL_PROBE_TIMEOUT_MS } from "../lib/adapters/openshell/timeouts";
import * as registry from "../lib/state/registry";
import { resolveOpenshell } from "../lib/adapters/openshell/resolve";
import { parseLiveSandboxNames } from "../lib/runtime-recovery";
import { createCliOpenShellSandboxObserver } from "../lib/adapters/openshell/sandbox-observer-cli";
import { selectedOpenShellGateway } from "../lib/adapters/openshell/sandbox-observer";
import * as registry from "../lib/state/registry";

const useColor = !process.env.NO_COLOR && !!process.stderr.isTTY;
const B = useColor ? "\x1b[1m" : "";
const R = useColor ? "\x1b[0m" : "";
const RD = useColor ? "\x1b[1;31m" : "";

function captureOpenshell(rootDir: string, args: string[]): CaptureOpenshellResult {
const openshell = resolveOpenshell();
if (!openshell) {
return { status: 1, output: "" };
}
return captureOpenshellCommand(openshell, args, {
cwd: rootDir,
ignoreError: true,
timeout: OPENSHELL_PROBE_TIMEOUT_MS,
function buildDebugCommandDeps(rootDir: string): RunDebugCommandDeps {
const sandboxObserver = createCliOpenShellSandboxObserver({
capture: (args, options) => {
const openshell = resolveOpenshell();
if (!openshell) return { status: 1, output: "" };
return captureOpenshellCommand(openshell, args, { cwd: rootDir, ...options });
},
});
}

function buildDebugCommandDeps(rootDir: string): RunDebugCommandDeps {
const getDefaultSandbox = (): string | undefined => {
const liveSandboxNames = async (): Promise<ReadonlySet<string> | undefined> => {
const result = await sandboxObserver.listSandboxes({ target: selectedOpenShellGateway() });
if (!result.ok) return undefined;
return new Set(result.value.sandboxes.map((sandbox) => sandbox.name));
};
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

const getDefaultSandbox = async (): Promise<string | undefined> => {
const { defaultSandbox, sandboxes } = registry.listSandboxes();
if (!defaultSandbox) return undefined;
if (!sandboxes.find((sandbox) => sandbox.name === defaultSandbox)) {
Expand All @@ -46,8 +47,8 @@ function buildDebugCommandDeps(rootDir: string): RunDebugCommandDeps {
);
return undefined;
}
const liveList = captureOpenshell(rootDir, ["sandbox", "list"]);
if (liveList.status === 0 && !parseLiveSandboxNames(liveList.output).has(defaultSandbox)) {
const liveNames = await liveSandboxNames();
if (liveNames && !liveNames.has(defaultSandbox)) {
console.error(
`${RD}Warning:${R} default sandbox '${defaultSandbox}' exists in the local registry but not in OpenShell.`,
);
Expand All @@ -59,14 +60,11 @@ function buildDebugCommandDeps(rootDir: string): RunDebugCommandDeps {
return defaultSandbox;
};

const isSandboxKnown = (name: string): boolean => {
const isSandboxKnown = async (name: string): Promise<boolean> => {
const { sandboxes } = registry.listSandboxes();
if (!sandboxes.find((sandbox) => sandbox.name === name)) return false;
const liveList = captureOpenshell(rootDir, ["sandbox", "list"]);
if (liveList.status === 0 && !parseLiveSandboxNames(liveList.output).has(name)) {
return false;
}
return true;
const liveNames = await liveSandboxNames();
return !liveNames || liveNames.has(name);
};

return {
Expand Down Expand Up @@ -99,6 +97,6 @@ export default class DebugCliCommand extends NemoClawCommand {
if (flags.quick) options.quick = true;
if (flags.output) options.output = flags.output;
if (flags.sandbox) options.sandboxName = flags.sandbox;
runDebugCommandWithOptions(options, buildDebugCommandDeps(this.config.root));
await runDebugCommandWithOptions(options, buildDebugCommandDeps(this.config.root));
}
}
37 changes: 35 additions & 2 deletions src/commands/simple-global-oclif-adapters.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,14 +177,47 @@ describe("simple global oclif adapters", testTimeoutOptions(30_000), () => {
await DebugCliCommand.run(["--quick"], rootDir);

const deps = mocks.runDebugCommandWithOptions.mock.calls[0][1];
expect(deps.getDefaultSandbox()).toBe("alpha");
await expect(deps.getDefaultSandbox()).resolves.toBe("alpha");
expect(mocks.captureOpenshellCommand).toHaveBeenCalledWith(
"/usr/bin/openshell",
["sandbox", "list"],
expect.objectContaining({ cwd: rootDir, ignoreError: true }),
expect.objectContaining({
cwd: rootDir,
ignoreError: true,
includeStderr: true,
includeStreams: true,
}),
);
});

it("rejects a registered debug sandbox missing from a successful OpenShell observation", async () => {
mocks.listSandboxes.mockReturnValue({
defaultSandbox: "alpha",
sandboxes: [{ name: "alpha" }],
} as never);
mocks.captureOpenshellCommand.mockReturnValue({ status: 0, output: "beta\n" });

await DebugCliCommand.run(["--quick"], rootDir);

const deps = mocks.runDebugCommandWithOptions.mock.calls[0][1];
await expect(deps.getDefaultSandbox()).resolves.toBeUndefined();
await expect(deps.isSandboxKnown("alpha")).resolves.toBe(false);
});

it("keeps registered debug sandboxes when OpenShell observation fails", async () => {
mocks.listSandboxes.mockReturnValue({
defaultSandbox: "alpha",
sandboxes: [{ name: "alpha" }],
} as never);
mocks.captureOpenshellCommand.mockReturnValue({ status: 1, output: "unavailable" });

await DebugCliCommand.run(["--quick"], rootDir);

const deps = mocks.runDebugCommandWithOptions.mock.calls[0][1];
await expect(deps.getDefaultSandbox()).resolves.toBe("alpha");
await expect(deps.isSandboxKnown("alpha")).resolves.toBe(true);
});

it("maps gateway-token flags to the gateway token action", async () => {
const getSandboxAgent = vi.fn(() => "openclaw");
const fetchToken = mocks.fetchGatewayAuthTokenFromSandbox;
Expand Down
52 changes: 26 additions & 26 deletions src/lib/diagnostics/debug-command.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ import { describe, expect, it, vi } from "vitest";
import { runDebugCommandWithOptions } from "./debug-command";

describe("debug command", () => {
it("runs parsed debug options and falls back to the default sandbox", () => {
it("runs parsed debug options and falls back to the default sandbox", async () => {
const runDebug = vi.fn();
runDebugCommandWithOptions(
await runDebugCommandWithOptions(
{ quick: true, output: "/tmp/out.tgz" },
{
getDefaultSandbox: () => "alpha",
isSandboxKnown: () => true,
getDefaultSandbox: async () => "alpha",
isSandboxKnown: async () => true,
runDebug,
},
);
Expand All @@ -23,13 +23,13 @@ describe("debug command", () => {
});
});

it("accepts an explicit --sandbox name that is registered", () => {
it("accepts an explicit --sandbox name that is registered", async () => {
const runDebug = vi.fn();
const isSandboxKnown = vi.fn().mockReturnValue(true);
runDebugCommandWithOptions(
await runDebugCommandWithOptions(
{ sandboxName: "alpha" },
{
getDefaultSandbox: () => undefined,
getDefaultSandbox: async () => undefined,
isSandboxKnown,
runDebug,
},
Expand All @@ -38,68 +38,68 @@ describe("debug command", () => {
expect(runDebug).toHaveBeenCalledWith({ sandboxName: "alpha" });
});

it("rejects an explicit --sandbox name that is not registered, exits non-zero, skips runDebug", () => {
it("rejects an explicit --sandbox name that is not registered, exits non-zero, skips runDebug", async () => {
const runDebug = vi.fn();
const errorLines: string[] = [];
const exit = vi.fn(() => {
throw new Error("exit");
}) as unknown as (code: number) => never;
expect(() =>
await expect(
runDebugCommandWithOptions(
{ sandboxName: "does-not-exist", output: "/tmp/out.tgz" },
{
getDefaultSandbox: () => "alpha",
isSandboxKnown: () => false,
getDefaultSandbox: async () => "alpha",
isSandboxKnown: async () => false,
runDebug,
errorLine: (msg) => errorLines.push(msg),
exit,
},
),
).toThrow("exit");
).rejects.toThrow("exit");
expect(exit).toHaveBeenCalledWith(1);
expect(runDebug).not.toHaveBeenCalled();
expect(errorLines[0]).toContain("does-not-exist");
expect(errorLines[0]).toContain("not registered");
expect(errorLines.join("\n")).toContain("nemoclaw list");
});

it("validates an env-sourced sandbox name and reports the env source on failure", () => {
it("validates an env-sourced sandbox name and reports the env source on failure", async () => {
const runDebug = vi.fn();
const errorLines: string[] = [];
const exit = vi.fn(() => {
throw new Error("exit");
}) as unknown as (code: number) => never;
expect(() =>
await expect(
runDebugCommandWithOptions(
{},
{
env: { NEMOCLAW_SANDBOX_NAME: "ghost" } as NodeJS.ProcessEnv,
getDefaultSandbox: () => "alpha",
isSandboxKnown: () => false,
getDefaultSandbox: async () => "alpha",
isSandboxKnown: async () => false,
runDebug,
errorLine: (msg) => errorLines.push(msg),
exit,
},
),
).toThrow("exit");
).rejects.toThrow("exit");
expect(exit).toHaveBeenCalledWith(1);
expect(runDebug).not.toHaveBeenCalled();
expect(errorLines[0]).toContain("ghost");
expect(errorLines[0]).toContain("NEMOCLAW_SANDBOX_NAME");
});

it("prefers NEMOCLAW_SANDBOX_NAME over NEMOCLAW_SANDBOX and SANDBOX_NAME", () => {
it("prefers NEMOCLAW_SANDBOX_NAME over NEMOCLAW_SANDBOX and SANDBOX_NAME", async () => {
const runDebug = vi.fn();
const isSandboxKnown = vi.fn().mockReturnValue(true);
runDebugCommandWithOptions(
await runDebugCommandWithOptions(
{},
{
env: {
NEMOCLAW_SANDBOX_NAME: "primary",
NEMOCLAW_SANDBOX: "secondary",
SANDBOX_NAME: "tertiary",
} as NodeJS.ProcessEnv,
getDefaultSandbox: () => undefined,
getDefaultSandbox: async () => undefined,
isSandboxKnown,
runDebug,
},
Expand All @@ -108,14 +108,14 @@ describe("debug command", () => {
expect(runDebug).toHaveBeenCalledWith({ sandboxName: "primary" });
});

it("flag overrides env vars when both are present", () => {
it("flag overrides env vars when both are present", async () => {
const runDebug = vi.fn();
const isSandboxKnown = vi.fn().mockReturnValue(true);
runDebugCommandWithOptions(
await runDebugCommandWithOptions(
{ sandboxName: "alpha" },
{
env: { NEMOCLAW_SANDBOX: "beta" } as NodeJS.ProcessEnv,
getDefaultSandbox: () => undefined,
getDefaultSandbox: async () => undefined,
isSandboxKnown,
runDebug,
},
Expand All @@ -125,14 +125,14 @@ describe("debug command", () => {
expect(runDebug).toHaveBeenCalledWith({ sandboxName: "alpha" });
});

it("falls back to getDefaultSandbox when neither flag nor env is set", () => {
it("falls back to getDefaultSandbox when neither flag nor env is set", async () => {
const runDebug = vi.fn();
const isSandboxKnown = vi.fn();
runDebugCommandWithOptions(
await runDebugCommandWithOptions(
{},
{
env: {} as NodeJS.ProcessEnv,
getDefaultSandbox: () => "alpha",
getDefaultSandbox: async () => "alpha",
isSandboxKnown,
runDebug,
},
Expand Down
13 changes: 8 additions & 5 deletions src/lib/diagnostics/debug-command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
import type { DebugOptions } from "./debug";

export interface RunDebugCommandDeps {
getDefaultSandbox: () => string | undefined;
isSandboxKnown: (name: string) => boolean;
getDefaultSandbox: () => Promise<string | undefined>;
isSandboxKnown: (name: string) => Promise<boolean>;
runDebug: (options: DebugOptions) => void;
env?: NodeJS.ProcessEnv;
errorLine?: (message: string) => void;
Expand All @@ -31,7 +31,10 @@ function resolveExplicitName(
return null;
}

export function runDebugCommandWithOptions(options: DebugOptions, deps: RunDebugCommandDeps): void {
export async function runDebugCommandWithOptions(
options: DebugOptions,
deps: RunDebugCommandDeps,
): Promise<void> {
const opts = { ...options };
const env = deps.env ?? process.env;
const errorLine = deps.errorLine ?? ((msg: string) => console.error(msg));
Expand All @@ -43,7 +46,7 @@ export function runDebugCommandWithOptions(options: DebugOptions, deps: RunDebug

const explicit = resolveExplicitName(opts, env);
if (explicit) {
if (!deps.isSandboxKnown(explicit.name)) {
if (!(await deps.isSandboxKnown(explicit.name))) {
const sourceLabel =
explicit.source === "env" && explicit.envVar ? ` (from ${explicit.envVar})` : "";
errorLine(`Error: Sandbox '${explicit.name}'${sourceLabel} is not registered.`);
Expand All @@ -53,7 +56,7 @@ export function runDebugCommandWithOptions(options: DebugOptions, deps: RunDebug
}
opts.sandboxName = explicit.name;
} else {
opts.sandboxName = deps.getDefaultSandbox();
opts.sandboxName = await deps.getDefaultSandbox();
}

deps.runDebug(opts);
Expand Down
Loading