From 907f5bacaa79abadd3e8edbb8a53e071fd951cdc Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 16 Aug 2026 17:41:00 -0700 Subject: [PATCH] fix(agents): refuse add when prompts cannot run (#124940) --- src/cli/program/register.agent.test.ts | 6 +-- src/cli/program/register.agent.ts | 9 +++- src/cli/resume-cli.runtime.ts | 3 +- src/commands/agents.add.test.ts | 53 +++++++++++++++++++--- src/commands/agents.commands.add.ts | 16 +++++-- src/commands/agents.commands.delete.ts | 3 +- src/commands/agents.delete.test.ts | 41 +++++++++++++++++ src/commands/configure.commands.ts | 4 +- src/commands/onboard-interactive-runner.ts | 3 +- 9 files changed, 120 insertions(+), 18 deletions(-) diff --git a/src/cli/program/register.agent.test.ts b/src/cli/program/register.agent.test.ts index 791427ca4be9..7f49c62cb230 100644 --- a/src/cli/program/register.agent.test.ts +++ b/src/cli/program/register.agent.test.ts @@ -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 () => { diff --git a/src/cli/program/register.agent.ts b/src/cli/program/register.agent.ts index 0b1dd6700336..5755c4715a7c 100644 --- a/src/cli/program/register.agent.ts +++ b/src/cli/program/register.agent.ts @@ -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 }, ); }); }); diff --git a/src/cli/resume-cli.runtime.ts b/src/cli/resume-cli.runtime.ts index b8f58a7037b5..375a1c981b73 100644 --- a/src/cli/resume-cli.runtime.ts +++ b/src/cli/resume-cli.runtime.ts @@ -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); } } diff --git a/src/commands/agents.add.test.ts b/src/commands/agents.add.test.ts index eea34fb799f1..9722d8ead71b 100644 --- a/src/commands/agents.add.test.ts +++ b/src/commands/agents.add.test.ts @@ -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()), + 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 --non-interactive --workspace ` 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", diff --git a/src/commands/agents.commands.add.ts b/src/commands/agents.commands.add.ts index 58ff24e90cd0..988ad58c118d 100644 --- a/src/commands/agents.commands.add.ts +++ b/src/commands/agents.commands.add.ts @@ -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 --non-interactive --workspace ")}\` 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) { diff --git a/src/commands/agents.commands.delete.ts b/src/commands/agents.commands.delete.ts index 58274fd7b015..a6db7fa49ea1 100644 --- a/src/commands/agents.commands.delete.ts +++ b/src/commands/agents.commands.delete.ts @@ -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; } diff --git a/src/commands/agents.delete.test.ts b/src/commands/agents.delete.test.ts index 735c665c2c69..d4a42e57d549 100644 --- a/src/commands/agents.delete.test.ts +++ b/src/commands/agents.delete.test.ts @@ -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("../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()), + 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 () => { diff --git a/src/commands/configure.commands.ts b/src/commands/configure.commands.ts index 46418d225262..c8869bdbbff9 100644 --- a/src/commands/configure.commands.ts +++ b/src/commands/configure.commands.ts @@ -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; } diff --git a/src/commands/onboard-interactive-runner.ts b/src/commands/onboard-interactive-runner.ts index 122eb0a43701..b170bf367186 100644 --- a/src/commands/onboard-interactive-runner.ts +++ b/src/commands/onboard-interactive-runner.ts @@ -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(