fix(agents): refuse add when prompts cannot run (#124940)

This commit is contained in:
Peter Steinberger
2026-08-16 17:41:00 -07:00
committed by GitHub
parent c299c0da31
commit 907f5bacaa
9 changed files with 120 additions and 18 deletions
+3 -3
View File
@@ -269,7 +269,7 @@ describe("agent command registration", () => {
expect((alphaOptions as { workspace?: string }).workspace).toBeUndefined();
expect((alphaOptions as { bind?: string[] }).bind).toEqual([]);
expect(alphaRuntime).toBe(runtime);
expect(alphaFlags).toEqual({ hasFlags: false });
expect(alphaFlags).toEqual({ hasFlags: false, hasAutomationFlags: false });
await runCli([
"agents",
@@ -291,7 +291,7 @@ describe("agent command registration", () => {
expect((betaOptions as { nonInteractive?: boolean }).nonInteractive).toBe(true);
expect((betaOptions as { json?: boolean }).json).toBe(true);
expect(betaRuntime).toBe(runtime);
expect(betaFlags).toEqual({ hasFlags: true });
expect(betaFlags).toEqual({ hasFlags: true, hasAutomationFlags: true });
});
it("keeps JSON-only agent creation non-interactive", async () => {
@@ -302,7 +302,7 @@ describe("agent command registration", () => {
expect.objectContaining({ name: "alpha", json: true, nonInteractive: false }),
);
expect(callRuntime).toBe(runtime);
expect(flags).toEqual({ hasFlags: true });
expect(flags).toEqual({ hasFlags: true, hasAutomationFlags: false });
});
it("runs agents list when root agents command is invoked", async () => {
+8 -1
View File
@@ -184,6 +184,13 @@ export function registerAgentsCommands(program: Command): void {
"nonInteractive",
"json",
]);
const hasAutomationFlags = hasExplicitOptions(command, [
"workspace",
"model",
"agentDir",
"bind",
"nonInteractive",
]);
const agentsAddCommand = await loadAgentsAddCommand();
await agentsAddCommand(
{
@@ -196,7 +203,7 @@ export function registerAgentsCommands(program: Command): void {
json: Boolean(opts.json),
},
runtime,
{ hasFlags },
{ hasFlags, hasAutomationFlags },
);
});
});
+2 -1
View File
@@ -16,6 +16,7 @@ import {
type SessionPickerChoice,
} from "../tui/tui-session-picker.js";
import type { ResumeCliOptions } from "./resume-cli.js";
import { isTerminalInteractive } from "./terminal-interactivity.js";
const RESUME_INTERACTIVE_TERMINAL_GUIDANCE =
"Attaching to a session requires an interactive terminal. Re-run `openclaw resume [query]` from an interactive terminal.";
@@ -115,7 +116,7 @@ function parseHandoffSessionResolveResult(value: unknown): ParsedHandoffSessionR
}
function requireInteractiveResumeTerminal() {
if (!process.stdin.isTTY || !process.stdout.isTTY) {
if (!isTerminalInteractive()) {
throw new Error(RESUME_INTERACTIVE_TERMINAL_GUIDANCE);
}
}
+47 -6
View File
@@ -73,6 +73,9 @@ const transformConfigWithPendingPluginInstallsMock = vi.hoisted(() =>
const wizardMocks = vi.hoisted(() => ({
createClackPrompter: vi.fn(),
}));
const terminalMocks = vi.hoisted(() => ({
isTerminalInteractive: vi.fn(() => true),
}));
const authChoiceMocks = vi.hoisted(() => ({
applyAuthChoice: vi.fn(),
warnIfModelConfigLooksOff: vi.fn(async () => {}),
@@ -111,6 +114,11 @@ vi.mock("../wizard/clack-prompter.js", () => ({
createClackPrompter: wizardMocks.createClackPrompter,
}));
vi.mock("../cli/terminal-interactivity.js", async (importOriginal) => ({
...(await importOriginal<typeof import("../cli/terminal-interactivity.js")>()),
isTerminalInteractive: terminalMocks.isTerminalInteractive,
}));
vi.mock("./auth-choice.js", () => ({
applyAuthChoice: authChoiceMocks.applyAuthChoice,
warnIfModelConfigLooksOff: authChoiceMocks.warnIfModelConfigLooksOff,
@@ -192,6 +200,7 @@ describe("agents add command", () => {
},
);
wizardMocks.createClackPrompter.mockClear();
terminalMocks.isTerminalInteractive.mockReset().mockReturnValue(true);
authChoiceMocks.applyAuthChoice.mockClear();
authChoiceMocks.warnIfModelConfigLooksOff.mockClear();
onboardChannelsMocks.setupChannels.mockClear();
@@ -316,22 +325,51 @@ describe("agents add command", () => {
expect(writeConfigFileMock).not.toHaveBeenCalled();
});
it.each([{ json: false }, { json: true }])(
"refuses the interactive wizard without a usable terminal (json=$json)",
async ({ json }) => {
readConfigFileSnapshotMock.mockResolvedValue({ ...baseConfigSnapshot });
terminalMocks.isTerminalInteractive.mockReturnValue(false);
wizardMocks.createClackPrompter.mockReturnValue({
intro: vi.fn(),
text: vi.fn().mockRejectedValue(new WizardCancelledError()),
confirm: vi.fn(),
note: vi.fn(),
outro: vi.fn(),
});
await agentsAddCommand({ json }, runtime);
expect(runtime.error).toHaveBeenCalledWith(
"Agent creation needs an interactive TTY. Use `openclaw agents add <id> --non-interactive --workspace <dir>` for automation.",
);
expect(runtime.exit).toHaveBeenCalledWith(1);
expect(runtime.log).not.toHaveBeenCalled();
expect(wizardMocks.createClackPrompter).not.toHaveBeenCalled();
expect(createAgentMock).not.toHaveBeenCalled();
expect(writeConfigFileMock).not.toHaveBeenCalled();
},
);
it("uses the explicit agent target and skips catalog validation", async () => {
readConfigFileSnapshotMock.mockResolvedValue({
...baseConfigSnapshot,
config: { agents: { list: [{ id: "main", default: true }] } },
sourceConfig: { agents: { list: [{ id: "main", default: true }] } },
});
wizardMocks.createClackPrompter.mockReturnValue({
const prompter = {
intro: vi.fn(),
text: vi.fn().mockResolvedValueOnce("Jon").mockResolvedValueOnce("/tmp/openclaw-jon"),
confirm: vi.fn().mockResolvedValue(false),
note: vi.fn(),
outro: vi.fn(),
});
};
wizardMocks.createClackPrompter.mockReturnValue(prompter);
await agentsAddCommand({}, runtime);
expect(terminalMocks.isTerminalInteractive).toHaveBeenCalledOnce();
expect(prompter.intro).toHaveBeenCalledWith("Add OpenClaw agent");
expect(authChoiceMocks.warnIfModelConfigLooksOff).toHaveBeenCalledOnce();
expect(authChoiceMocks.warnIfModelConfigLooksOff).toHaveBeenCalledWith(
expect.objectContaining({ agents: expect.any(Object) }),
@@ -474,16 +512,19 @@ describe("agents add command", () => {
});
describe("non-interactive config mutation", () => {
it("delegates creation to the canonical service", async () => {
it("creates with explicit non-interactive inputs without a usable terminal", async () => {
readConfigFileSnapshotMock.mockResolvedValue({
...baseConfigSnapshot,
config: { agents: { list: [{ id: "main", default: true }] } },
sourceConfig: { agents: { list: [{ id: "main", default: true }] } },
});
terminalMocks.isTerminalInteractive.mockReturnValue(false);
await agentsAddCommand({ name: "Work", workspace: "/tmp/work" }, runtime, {
hasFlags: true,
});
await agentsAddCommand(
{ name: "Work", workspace: "/tmp/work", nonInteractive: true },
runtime,
{ hasFlags: false },
);
expect(createAgentMock).toHaveBeenCalledWith({
name: "Work",
+13 -3
View File
@@ -32,6 +32,7 @@ import {
saveAuthProfileStore,
} from "../agents/auth-profiles/store.js";
import { formatCliCommand } from "../cli/command-format.js";
import { isTerminalInteractive } from "../cli/terminal-interactivity.js";
import { logConfigUpdated } from "../config/logging.js";
import {
commitConfigWithPendingPluginInstalls,
@@ -96,8 +97,19 @@ function formatSkippedOAuthProfilesMessage(
export async function agentsAddCommand(
opts: AgentsAddOptions,
runtime: RuntimeEnv = defaultRuntime,
params?: { hasFlags?: boolean },
params?: { hasFlags?: boolean; hasAutomationFlags?: boolean },
) {
const hasFlags = params?.hasFlags === true;
const hasAutomationFlags = params?.hasAutomationFlags ?? hasFlags;
const nonInteractive = opts.nonInteractive === true || hasFlags;
if (!opts.nonInteractive && !hasAutomationFlags && !isTerminalInteractive()) {
runtime.error(
`Agent creation needs an interactive TTY. Use \`${formatCliCommand("openclaw agents add <id> --non-interactive --workspace <dir>")}\` for automation.`,
);
runtime.exit(1);
return;
}
const configSnapshot = await requireValidConfigFileSnapshot(runtime);
if (!configSnapshot) {
return;
@@ -107,8 +119,6 @@ export async function agentsAddCommand(
const workspaceFlag = opts.workspace?.trim();
const nameInput = opts.name?.trim();
const hasFlags = params?.hasFlags === true;
const nonInteractive = opts.nonInteractive === true || hasFlags;
if (nonInteractive) {
if (!workspaceFlag) {
+2 -1
View File
@@ -29,6 +29,7 @@ import {
} from "../agents/workspace-state-store.js";
import { formatCliCommand } from "../cli/command-format.js";
import { formatCliJsonFailure } from "../cli/failure-output.js";
import { isTerminalInteractive } from "../cli/terminal-interactivity.js";
import { replaceConfigFile } from "../config/config.js";
import { logConfigUpdated } from "../config/logging.js";
import {
@@ -202,7 +203,7 @@ export async function agentsDeleteCommand(
}
if (!opts.force) {
if (!process.stdin.isTTY) {
if (!isTerminalInteractive()) {
failAgentsDelete(opts, runtime, "Non-interactive session. Re-run with --force.");
return;
}
+41
View File
@@ -53,6 +53,13 @@ const workspaceStateMocks = vi.hoisted(() => ({
prepareWorkspaceStateDeletion: vi.fn((workspaceDir: string) => ({ workspaceDir })),
}));
const terminalMocks = vi.hoisted(() => ({
isTerminalInteractive: vi.fn(() => true),
}));
const wizardMocks = vi.hoisted(() => ({
createClackPrompter: vi.fn(),
}));
vi.mock("../config/config.js", async () => ({
...(await vi.importActual<typeof import("../config/config.js")>("../config/config.js")),
readConfigFileSnapshot: configMocks.readConfigFileSnapshot,
@@ -81,6 +88,15 @@ vi.mock("../agents/workspace-state-store.js", async () => ({
prepareWorkspaceStateDeletion: workspaceStateMocks.prepareWorkspaceStateDeletion,
}));
vi.mock("../cli/terminal-interactivity.js", async (importOriginal) => ({
...(await importOriginal<typeof import("../cli/terminal-interactivity.js")>()),
isTerminalInteractive: terminalMocks.isTerminalInteractive,
}));
vi.mock("../wizard/clack-prompter.js", () => ({
createClackPrompter: wizardMocks.createClackPrompter,
}));
import { agentsDeleteCommand } from "./agents.commands.delete.js";
const runtime = createTestRuntime();
@@ -211,6 +227,31 @@ describe("agents delete command", () => {
runtime.log.mockClear();
runtime.error.mockClear();
runtime.exit.mockClear();
terminalMocks.isTerminalInteractive.mockReset().mockReturnValue(true);
wizardMocks.createClackPrompter.mockReset();
});
it("requires --force when confirmation cannot use an interactive terminal", async () => {
await withStateDirEnv("openclaw-agents-delete-non-tty-", async ({ stateDir }) => {
const cfg: OpenClawConfig = {
agents: {
list: [
{ id: "main", default: true, workspace: path.join(stateDir, "workspace-main") },
{ id: "ops", workspace: path.join(stateDir, "workspace-ops") },
],
},
};
await arrangeAgentsDeleteTest({ stateDir, cfg, deletedAgentId: "ops", sessions: {} });
terminalMocks.isTerminalInteractive.mockReturnValue(false);
await agentsDeleteCommand({ id: "ops" }, runtime);
expect(runtime.error).toHaveBeenCalledWith("Non-interactive session. Re-run with --force.");
expect(runtime.exit).toHaveBeenCalledWith(1);
expect(wizardMocks.createClackPrompter).not.toHaveBeenCalled();
expect(configMocks.replaceConfigFile).not.toHaveBeenCalled();
expect(fsSafeMocks.movePathToTrash).not.toHaveBeenCalled();
});
});
it("refuses deleting main even when another agent is default", async () => {
+2 -2
View File
@@ -1,6 +1,6 @@
// Entry points for the full configure wizard and section-limited runs.
import process from "node:process";
import { formatCliCommand } from "../cli/command-format.js";
import { isTerminalInteractive } from "../cli/terminal-interactivity.js";
import type { RuntimeEnv } from "../runtime.js";
import { defaultRuntime } from "../runtime.js";
import type { WizardSection } from "./configure.shared.js";
@@ -34,7 +34,7 @@ const CONFIGURE_NON_TTY_HINT = [
* Returns true when the wizard may proceed.
*/
function assertInteractiveConfigureTerminal(runtime: RuntimeEnv, interactive?: boolean): boolean {
const interactiveTerminal = interactive ?? (process.stdin.isTTY && process.stdout.isTTY);
const interactiveTerminal = interactive ?? isTerminalInteractive();
if (interactiveTerminal) {
return true;
}
+2 -1
View File
@@ -1,10 +1,11 @@
// Shared lifecycle handling for interactive onboarding entrypoints.
import { restoreTerminalState } from "../../packages/terminal-core/src/restore.js";
import { isTerminalInteractive } from "../cli/terminal-interactivity.js";
import type { RuntimeEnv } from "../runtime.js";
import { WizardCancelledError } from "../wizard/prompts.js";
export function hasInteractiveOnboardingTty(): boolean {
return process.stdin.isTTY && process.stdout.isTTY;
return isTerminalInteractive();
}
export async function runInteractiveOnboarding(