Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
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
14 changes: 8 additions & 6 deletions scripts/install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1617,13 +1617,15 @@ main() {
# that they have to manually `rm -rf` before retry, while their license
# has not actually been accepted.
#
# Skipped (and the install proceeds) when any of:
# Skipped (and the install proceeds) only when either:
# - NON_INTERACTIVE=1 (also implied by ACCEPT_THIRD_PARTY_SOFTWARE=1 above)
# - stdin is a TTY — license helper prompts the user directly
# - /dev/tty is openable — show_usage_notice falls back to /dev/tty input
if [ "${NON_INTERACTIVE:-}" != "1" ] \
&& [ ! -t 0 ] \
&& ! (: </dev/tty) 2>/dev/null; then
# - stdin is a TTY — license helper prompts the user directly before install
#
# Do not treat an openable /dev/tty as sufficient here. In curl|bash mode,
# stdin is a pipe even though /dev/tty may still be available; falling back to
# /dev/tty later would run phases 1/2 before the license prompt and could leave
# a partial install behind if the user declines or no terminal is attached.
if [ "${NON_INTERACTIVE:-}" != "1" ] && [ ! -t 0 ]; then
error "Interactive third-party software acceptance requires a TTY. Re-run in a terminal or pass --yes-i-accept-third-party-software (or set NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE=1)."
fi
Comment on lines +1628 to 1630

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Piped install now hard-fails against the documented default flow

At Line 1628, this condition rejects all curl ... | bash runs unless NON_INTERACTIVE=1 (or equivalent) is pre-set. That conflicts with the usage text at Line 483-484 and effectively makes the existing /dev/tty fallback paths unreachable for interactive piped installs.

Suggested minimal fix
-  if [ "${NON_INTERACTIVE:-}" != "1" ] && [ ! -t 0 ]; then
-    error "Interactive third-party software acceptance requires a TTY. Re-run in a terminal or pass --yes-i-accept-third-party-software (or set NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE=1)."
-  fi
+  if [ "${NON_INTERACTIVE:-}" != "1" ] && [ ! -t 0 ]; then
+    if { exec 3</dev/tty; } 2>/dev/null; then
+      exec 3<&-
+    else
+      error "Interactive third-party software acceptance requires a TTY. Re-run in a terminal or pass --yes-i-accept-third-party-software (or set NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE=1)."
+    fi
+  fi
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if [ "${NON_INTERACTIVE:-}" != "1" ] && [ ! -t 0 ]; then
error "Interactive third-party software acceptance requires a TTY. Re-run in a terminal or pass --yes-i-accept-third-party-software (or set NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE=1)."
fi
if [ "${NON_INTERACTIVE:-}" != "1" ] && [ ! -t 0 ]; then
if { exec 3</dev/tty; } 2>/dev/null; then
exec 3<&-
else
error "Interactive third-party software acceptance requires a TTY. Re-run in a terminal or pass --yes-i-accept-third-party-software (or set NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE=1)."
fi
fi
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/install.sh` around lines 1628 - 1630, The current guard incorrectly
rejects piped interactive installs by requiring stdin to be a TTY; update the
conditional around NON_INTERACTIVE to allow cases where /dev/tty exists (the
documented fallback) so interactive piped installs can prompt. Concretely,
change the if that checks [ "${NON_INTERACTIVE:-}" != "1" ] && [ ! -t 0 ] to
also allow when /dev/tty is present (e.g. [ ! -t 0 ] && [ ! -e /dev/tty ]),
leaving the existing error message and references to
--yes-i-accept-third-party-software / NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE
untouched; this ensures code paths that open /dev/tty for prompts remain
reachable during curl ... | bash flows.


Expand Down
3 changes: 3 additions & 0 deletions src/lib/legacy-oclif-dispatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,9 @@ export function resolveSandboxOclifDispatch(
case "snapshot": {
const snapshotSub = actionArgs[0];
const snapshotArgs = actionArgs.slice(1);
if (!snapshotSub || snapshotSub === "--help" || snapshotSub === "-h") {
return { kind: "oclif", commandId: "sandbox:snapshot", args: [sandboxName] };
}
if (snapshotSub === "list") {
if (hasHelpFlag(snapshotArgs)) return { kind: "help", usage: "snapshot list" };
return { kind: "oclif", commandId: "sandbox:snapshot:list", args: [sandboxName, ...snapshotArgs] };
Expand Down
19 changes: 19 additions & 0 deletions src/lib/snapshot-cli-commands.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { describe, expect, it, vi } from "vitest";

import {
setSnapshotRuntimeBridgeFactoryForTest,
SnapshotCommand,
SnapshotCreateCommand,
SnapshotListCommand,
SnapshotRestoreCommand,
Expand All @@ -13,6 +14,24 @@ import {
const rootDir = process.cwd();

describe("snapshot oclif commands", () => {
it("shows parent snapshot usage through the action", async () => {
const sandboxSnapshot = vi.fn().mockResolvedValue(undefined);
setSnapshotRuntimeBridgeFactoryForTest(() => ({ sandboxSnapshot }));

await SnapshotCommand.run(["alpha"], rootDir);

expect(sandboxSnapshot).toHaveBeenCalledWith("alpha", []);
});

it("rejects unknown parent snapshot args before dispatch", async () => {
const sandboxSnapshot = vi.fn().mockResolvedValue(undefined);
setSnapshotRuntimeBridgeFactoryForTest(() => ({ sandboxSnapshot }));

await expect(SnapshotCommand.run(["alpha", "bogus"], rootDir)).rejects.toThrow(/bogus/);

expect(sandboxSnapshot).not.toHaveBeenCalled();
});

it("runs snapshot list through the legacy snapshot implementation", async () => {
const sandboxSnapshot = vi.fn().mockResolvedValue(undefined);
setSnapshotRuntimeBridgeFactoryForTest(() => ({ sandboxSnapshot }));
Expand Down
29 changes: 22 additions & 7 deletions src/lib/snapshot-cli-commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,22 @@ const sandboxNameArg = Args.string({

export class SnapshotCommand extends Command {
static id = "sandbox:snapshot";
static strict = false;
static strict = true;
static summary = "Show snapshot usage";
static description = "Show snapshot usage or report unknown snapshot subcommands.";
static description = "Show snapshot usage for create, list, and restore subcommands.";
static usage = ["<name> snapshot <create|list|restore>"];
static examples = [
"<%= config.bin %> alpha snapshot create",
"<%= config.bin %> alpha snapshot list",
"<%= config.bin %> alpha snapshot restore",
];
static args = {
sandboxName: sandboxNameArg,
};

public async run(): Promise<void> {
const [sandboxName, ...actionArgs] = this.argv;
if (!sandboxName || sandboxName.trim() === "") {
this.error("Missing required sandboxName for snapshot.", { exit: 2 });
}
await getRuntimeBridge().sandboxSnapshot(sandboxName, actionArgs);
const { args } = await this.parse(SnapshotCommand);
await getRuntimeBridge().sandboxSnapshot(args.sandboxName, []);
}
}

Expand All @@ -47,6 +52,7 @@ export class SnapshotListCommand extends Command {
static summary = "List available snapshots";
static description = "List available snapshots for a sandbox.";
static usage = ["<name> snapshot list"];
static examples = ["<%= config.bin %> alpha snapshot list"];
static args = {
sandboxName: sandboxNameArg,
};
Expand All @@ -66,6 +72,11 @@ export class SnapshotRestoreCommand extends Command {
static summary = "Restore state from a snapshot";
static description = "Restore sandbox workspace state from a snapshot.";
static usage = ["<name> snapshot restore [selector] [--to <dst>]"];
static examples = [
"<%= config.bin %> alpha snapshot restore",
"<%= config.bin %> alpha snapshot restore v2",
"<%= config.bin %> alpha snapshot restore before-upgrade --to beta",
];
static args = {
sandboxName: sandboxNameArg,
selector: Args.string({
Expand Down Expand Up @@ -94,6 +105,10 @@ export class SnapshotCreateCommand extends Command {
static summary = "Create a snapshot of sandbox state";
static description = "Create an auto-versioned snapshot of sandbox workspace state.";
static usage = ["<name> snapshot create [--name <label>]"];
static examples = [
"<%= config.bin %> alpha snapshot create",
"<%= config.bin %> alpha snapshot create --name before-upgrade",
];
static args = {
sandboxName: sandboxNameArg,
};
Expand Down
15 changes: 15 additions & 0 deletions test/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1490,6 +1490,12 @@ describe("CLI dispatch", () => {
const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-cli-snapshot-help-"));
writeSandboxRegistry(home);

const parent = runWithEnv("alpha snapshot --help", { HOME: home });
expect(parent.code).toBe(0);
expect(parent.out).toContain("nemoclaw alpha snapshot create");
expect(parent.out).toContain("nemoclaw alpha snapshot list");
expect(parent.out).not.toContain("sandbox:snapshot");

const list = runWithEnv("alpha snapshot list --help", { HOME: home });
expect(list.code).toBe(0);
expect(list.out).toContain("<name> snapshot list");
Expand All @@ -1515,6 +1521,15 @@ describe("CLI dispatch", () => {
expect(r.out).toContain("No snapshots found for 'alpha'.");
});

it("unknown snapshot subcommands fail before action dispatch", () => {
const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-cli-snapshot-unknown-"));
writeSandboxRegistry(home);

const r = runWithEnv("alpha snapshot bogus 2>&1", { HOME: home });
expect(r.code).not.toBe(0);
expect(r.out).toContain("Unexpected argument: bogus");
});

it("routes logs to OpenClaw and OpenShell log sources", () => {
const setup = createLogsTestSetup("nemoclaw-cli-logs-routing-");
const r = setup.runLogs();
Expand Down
Loading