Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
3 changes: 1 addition & 2 deletions ci/test-file-size-budget.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
"test/install-preflight.test.ts": 3921,
"test/nemoclaw-start.test.ts": 4819,
"test/onboard-messaging.test.ts": 2049,
"test/onboard-selection.test.ts": 4769,
"test/policies.test.ts": 1530
"test/onboard-selection.test.ts": 4769
}
}
33 changes: 33 additions & 0 deletions src/lib/actions/sandbox/policy-channel-policy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,28 @@ describe("addSandboxPolicy", () => {
expect(applyPresetMock).not.toHaveBeenCalled();
});

it("exits non-zero when the add picker reaches stdin EOF (#7418)", async () => {
selectFromListMock.mockRejectedValueOnce(
Object.assign(new Error("Prompt closed before input"), { code: "EOF" }),
);

await expect(captureExit(() => addSandboxPolicy("test-sandbox"))).resolves.toBe(1);

// Names the real condition rather than reporting non-interactive mode,
// which is not what happened here.
expect(printedText()).toContain("No input available on stdin");
expect(applyPresetMock).not.toHaveBeenCalled();
});

it("propagates a non-EOF picker failure instead of exiting (#7418)", async () => {
const failure = Object.assign(new Error("stdin read failed"), { code: "EIO" });
selectFromListMock.mockRejectedValueOnce(failure);

await expect(addSandboxPolicy("test-sandbox")).rejects.toBe(failure);

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

it("filters Hermes-only presets from the OpenClaw picker", async () => {
arrangeSandbox("openclaw");

Expand Down Expand Up @@ -363,4 +385,15 @@ describe("removeSandboxPolicy", () => {
expect(printedText()).toContain("Non-interactive mode requires a preset name.");
expect(removePresetMock).not.toHaveBeenCalled();
});

it("exits non-zero when the remove picker reaches stdin EOF (#7418)", async () => {
selectForRemovalMock.mockRejectedValueOnce(
Object.assign(new Error("Prompt closed before input"), { code: "EOF" }),
);

await expect(captureExit(() => removeSandboxPolicy("test-sandbox"))).resolves.toBe(1);

expect(printedText()).toContain("No input available on stdin");
expect(removePresetMock).not.toHaveBeenCalled();
});
});
63 changes: 55 additions & 8 deletions src/lib/actions/sandbox/policy-channel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,52 @@ import { executeSandboxCommand, executeSandboxExecCommand } from "./process-reco

const isNonInteractive = isNonInteractiveEnv;

/**
* Report that `NEMOCLAW_NON_INTERACTIVE=1` leaves no interactive picker, and
* exit non-zero.
*/
function exitPresetNameRequired(usage: string): never {
console.error(" Non-interactive mode requires a preset name.");
console.error(` Usage: ${usage}`);
process.exit(1);
}

/**
* Report that the picker prompt reached stdin EOF, and exit non-zero.
*
* Separate from `exitPresetNameRequired` because the conditions differ. That
* one means the operator set `NEMOCLAW_NON_INTERACTIVE=1`. This one means
* stdin closed while that variable was unset, so naming the variable would
* misdirect whoever reads the boot-unit log.
*/
function exitPromptStdinClosed(usage: string): never {
console.error(" No input available on stdin, so the preset picker cannot prompt.");
console.error(` Usage: ${usage}`);
process.exit(1);
}

/**
* Await an interactive preset picker and convert a prompt EOF into exit 1.
*
* A boot unit that pipes or closes stdin without setting
* `NEMOCLAW_NON_INTERACTIVE=1` reaches the picker, and the prompt then hits
* EOF. Before #7418 the picker promise never settled, so the command exited 0
* having changed nothing. Automation could not distinguish an applied preset
* from a no-op. Any other failure propagates unchanged.
*/
async function pickPresetOrExit(
pick: () => Promise<string | null>,
usage: string,
): Promise<string | null> {
try {
return await pick();
} catch (error) {
const code = (error as NodeJS.ErrnoException | null)?.code;
if (code !== "EOF") throw error;
exitPromptStdinClosed(usage);
}
}

type ChannelMutationOptions = {
channel?: string;
dryRun?: boolean;
Expand Down Expand Up @@ -217,12 +263,11 @@ async function addSandboxPolicyUnlocked(
}
answer = preset.name;
} else {
const usage = `${CLI_NAME} <sandbox> policy-add <preset> [--yes] [--dry-run]`;
if (isNonInteractive()) {
console.error(" Non-interactive mode requires a preset name.");
console.error(` Usage: ${CLI_NAME} <sandbox> policy-add <preset> [--yes] [--dry-run]`);
process.exit(1);
exitPresetNameRequired(usage);
}
answer = await policies.selectFromList(allPresets, { applied });
answer = await pickPresetOrExit(() => policies.selectFromList(allPresets, { applied }), usage);
}
if (!answer) return;

Expand Down Expand Up @@ -1632,12 +1677,14 @@ async function removeSandboxPolicyUnlocked(
}
answer = preset.name;
} else {
const usage = `${CLI_NAME} <sandbox> policy-remove <preset> [--yes] [--dry-run]`;
if (isNonInteractive()) {
console.error(" Non-interactive mode requires a preset name.");
console.error(` Usage: ${CLI_NAME} <sandbox> policy-remove <preset> [--yes] [--dry-run]`);
process.exit(1);
exitPresetNameRequired(usage);
}
answer = await policies.selectForRemoval(allPresets, { applied });
answer = await pickPresetOrExit(
() => policies.selectForRemoval(allPresets, { applied }),
usage,
);
}
if (!answer) return;

Expand Down
197 changes: 104 additions & 93 deletions src/lib/policy/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -838,58 +838,88 @@ function removePreset(
}

/**
* Interactive preset picker for the `policy-remove` command. Prompts on
* stderr and resolves to the chosen preset name, or `null` if the user
* cancels or enters an invalid selection.
* Ask one preset-picker question on stderr and resolve to the raw answer.
*
* Rejects with `code: "EOF"` when readline closes before the question is
* answered. A boot unit runs `nemoclaw <sandbox> policy-add < /dev/null`, and
* the `question` callback then never fires. Without this handler the picker
* promise never settles and the command exits 0 having applied nothing
* (#7418).
*
* `finish` marks the prompt done before closing readline, because
* `rl.close()` itself emits `close`. Only a close that arrives before an
* answer rejects.
*
* Rejects with `code: "SIGINT"` when the operator presses Ctrl-C, and
* re-raises the signal so the process dies by SIGINT. Readline emits `close`
* for an interrupt as well as for EOF, so without a SIGINT listener an
* interrupt would be reported as a closed stdin.
*
* This matches the `prompt()` contract in `credentials/store.ts` (#5976).
*/
function selectForRemoval(
items: PresetInfo[],
{ applied = [] }: SelectionOptions = {},
): Promise<string | null> {
return new Promise<string | null>((resolve) => {
const appliedItems = items.filter((item) => applied.includes(item.name));
if (appliedItems.length === 0) {
process.stderr.write("\n No presets are currently applied.\n\n");
resolve(null);
return;
}
process.stderr.write("\n Applied presets:\n");
appliedItems.forEach((item, i) => {
const description = item.description ? ` — ${item.description}` : "";
process.stderr.write(` ${i + 1}) ${item.name}${description}\n`);
});
process.stderr.write("\n");
const question = " Choose preset to remove: ";
function askPreset(question: string): Promise<string> {
return new Promise<string>((resolve, reject) => {
// Re-attach stdin to the event loop — unref() on exit is sticky and
// would otherwise leave a follow-up prompt waiting on a detached handle.
if (typeof process.stdin.ref === "function") process.stdin.ref();
const rl = readline.createInterface({ input: process.stdin, output: process.stderr });
rl.question(question, (answer: string) => {
let finished = false;
const finish = (settle: () => void) => {
if (finished) return;
finished = true;
rl.close();
// pause+unref so the process exits naturally after the last prompt.
// The matching ref() above keeps subsequent prompts working.
if (typeof process.stdin.pause === "function") process.stdin.pause();
if (typeof process.stdin.unref === "function") process.stdin.unref();
const trimmed = answer.trim();
if (!trimmed) {
resolve(null);
return;
}
if (!/^\d+$/.test(trimmed)) {
process.stderr.write("\n Invalid preset number.\n");
resolve(null);
return;
}
const num = Number(trimmed);
const item = appliedItems[num - 1];
if (!item) {
process.stderr.write("\n Invalid preset number.\n");
resolve(null);
return;
}
resolve(item.name);
settle();
};
// Runs before the `close` listener below, so an interrupt settles as
// SIGINT and the close that follows is ignored.
rl.on("SIGINT", () => {
finish(() => reject(Object.assign(new Error("Prompt interrupted"), { code: "SIGINT" })));
process.kill(process.pid, "SIGINT");
});
rl.on("close", () =>
finish(() => reject(Object.assign(new Error("Prompt closed before input"), { code: "EOF" }))),
);
rl.question(question, (answer: string) => finish(() => resolve(answer)));
});
}

/**
* Interactive preset picker for the `policy-remove` command. Prompts on
* stderr and resolves to the chosen preset name, or `null` if the user
* cancels or enters an invalid selection. Rejects with `code: "EOF"` when
* stdin closes before an answer (see `askPreset`).
*/
async function selectForRemoval(
items: PresetInfo[],
{ applied = [] }: SelectionOptions = {},
): Promise<string | null> {
const appliedItems = items.filter((item) => applied.includes(item.name));
if (appliedItems.length === 0) {
process.stderr.write("\n No presets are currently applied.\n\n");
return null;
}
process.stderr.write("\n Applied presets:\n");
appliedItems.forEach((item, i) => {
const description = item.description ? ` — ${item.description}` : "";
process.stderr.write(` ${i + 1}) ${item.name}${description}\n`);
});
process.stderr.write("\n");
const trimmed = (await askPreset(" Choose preset to remove: ")).trim();
if (!trimmed) return null;
if (!/^\d+$/.test(trimmed)) {
process.stderr.write("\n Invalid preset number.\n");
return null;
}
const item = appliedItems[Number(trimmed) - 1];
if (!item) {
process.stderr.write("\n Invalid preset number.\n");
return null;
}
return item.name;
}

/**
Expand Down Expand Up @@ -1496,64 +1526,45 @@ function presetContentMatchesGateway(sandboxName: string, presetContent: string)
/**
* Interactive preset picker for the `policy-add` command. Prints the
* presets on stderr (● applied, ○ not applied), prompts for a number, and
* resolves to the chosen preset name or `null` on cancel.
* resolves to the chosen preset name or `null` on cancel. Rejects with
* `code: "EOF"` when stdin closes before an answer (see `askPreset`).
*/
function selectFromList(
async function selectFromList(
items: PresetInfo[],
{ applied = [] }: SelectionOptions = {},
): Promise<string | null> {
return new Promise<string | null>((resolve) => {
process.stderr.write("\n Available presets:\n");
items.forEach((item, i) => {
const marker = applied.includes(item.name) ? "●" : "○";
const description = item.description ? ` — ${item.description}` : "";
process.stderr.write(` ${i + 1}) ${marker} ${item.name}${description}\n`);
});
process.stderr.write("\n ● applied, ○ not applied\n\n");
const defaultIdx = items.findIndex((item) => !applied.includes(item.name));
const defaultNum = defaultIdx >= 0 ? defaultIdx + 1 : null;
const question = defaultNum ? ` Choose preset [${defaultNum}]: ` : " Choose preset: ";
// Re-attach stdin to the event loop — unref() on exit is sticky and
// would otherwise leave a follow-up prompt waiting on a detached handle.
if (typeof process.stdin.ref === "function") process.stdin.ref();
const rl = readline.createInterface({ input: process.stdin, output: process.stderr });
rl.question(question, (answer: string) => {
rl.close();
// pause+unref so the process exits naturally after the last prompt.
// The matching ref() above keeps subsequent prompts working.
if (typeof process.stdin.pause === "function") process.stdin.pause();
if (typeof process.stdin.unref === "function") process.stdin.unref();
const trimmed = answer.trim();
const effectiveInput = trimmed || (defaultNum ? String(defaultNum) : "");
if (!effectiveInput) {
resolve(null);
return;
}
if (!/^\d+$/.test(effectiveInput)) {
process.stderr.write("\n Invalid preset number.\n");
resolve(null);
return;
}
const num = Number(effectiveInput);
const item = items[num - 1];
if (!item) {
process.stderr.write("\n Invalid preset number.\n");
resolve(null);
return;
}
if (applied.includes(item.name)) {
// The picker has no live-policy context to classify drift; the named
// path (policy-add <preset>) re-applies edited presets (#7323).
process.stderr.write(`\n Preset '${item.name}' is already applied.\n`);
process.stderr.write(
` If its preset file changed, run '${CLI_NAME} <sandbox> policy-add ${item.name}' to re-apply it.\n`,
);
resolve(null);
return;
}
resolve(item.name);
});
process.stderr.write("\n Available presets:\n");
items.forEach((item, i) => {
const marker = applied.includes(item.name) ? "●" : "○";
const description = item.description ? ` — ${item.description}` : "";
process.stderr.write(` ${i + 1}) ${marker} ${item.name}${description}\n`);
});
process.stderr.write("\n ● applied, ○ not applied\n\n");
const defaultIdx = items.findIndex((item) => !applied.includes(item.name));
const defaultNum = defaultIdx >= 0 ? defaultIdx + 1 : null;
const question = defaultNum ? ` Choose preset [${defaultNum}]: ` : " Choose preset: ";
const trimmed = (await askPreset(question)).trim();
const effectiveInput = trimmed || (defaultNum ? String(defaultNum) : "");
if (!effectiveInput) return null;
if (!/^\d+$/.test(effectiveInput)) {
process.stderr.write("\n Invalid preset number.\n");
return null;
}
const item = items[Number(effectiveInput) - 1];
if (!item) {
process.stderr.write("\n Invalid preset number.\n");
return null;
}
if (applied.includes(item.name)) {
// The picker has no live-policy context to classify drift; the named
// path (policy-add <preset>) re-applies edited presets (#7323).
process.stderr.write(`\n Preset '${item.name}' is already applied.\n`);
process.stderr.write(
` If its preset file changed, run '${CLI_NAME} <sandbox> policy-add ${item.name}' to re-apply it.\n`,
);
return null;
}
return item.name;
}

const PERMISSIVE_POLICY_PATH = path.join(
Expand Down
Loading
Loading