mirror of
https://github.com/openclaw/openclaw.git
synced 2026-06-06 05:51:15 +08:00
Keep machine-readable CLI startup output parseable (#88689)
Constraint: CLI startup progress can render before Commander resolves a command's JSON output contract. Rejected: Leaving Clack on its default stdout | contaminates JSON stdout when startup progress appears. Confidence: high Scope-risk: narrow Directive: Keep progress output off stdout before full command parsing for machine-readable invocations. Tested: git diff --check origin/main; OPENCLAW_HEAVY_CHECK_LOCK_SCOPE=worktree OPENCLAW_VITEST_MAX_WORKERS=1 node scripts/run-vitest.mjs src/cli/progress.test.ts src/cli/run-main.exit.test.ts; source CLI sessions --json parse proof. Not-tested: broad pnpm check.
This commit is contained in:
@@ -1,7 +1,23 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { MAX_TIMER_TIMEOUT_MS } from "../shared/number-coercion.js";
|
||||
import { createCliProgress, shouldUseInteractiveProgressSpinner } from "./progress.js";
|
||||
|
||||
const clackMocks = vi.hoisted(() => {
|
||||
const spinnerInstance = {
|
||||
start: vi.fn(),
|
||||
message: vi.fn(),
|
||||
stop: vi.fn(),
|
||||
};
|
||||
return {
|
||||
spinner: vi.fn(() => spinnerInstance),
|
||||
spinnerInstance,
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock("@clack/prompts", () => ({
|
||||
spinner: clackMocks.spinner,
|
||||
}));
|
||||
|
||||
function withStdinIsRaw<T>(isRaw: boolean, run: () => T): T {
|
||||
const original = Object.getOwnPropertyDescriptor(process.stdin, "isRaw");
|
||||
Object.defineProperty(process.stdin, "isRaw", {
|
||||
@@ -20,6 +36,13 @@ function withStdinIsRaw<T>(isRaw: boolean, run: () => T): T {
|
||||
}
|
||||
|
||||
describe("cli progress", () => {
|
||||
beforeEach(() => {
|
||||
clackMocks.spinner.mockClear();
|
||||
clackMocks.spinnerInstance.start.mockClear();
|
||||
clackMocks.spinnerInstance.message.mockClear();
|
||||
clackMocks.spinnerInstance.stop.mockClear();
|
||||
});
|
||||
|
||||
it("logs progress when non-tty and fallback=log", () => {
|
||||
const writes: string[] = [];
|
||||
const stream = {
|
||||
@@ -69,6 +92,15 @@ describe("cli progress", () => {
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("uses the progress stream instead of stdout to decide spinner interactivity", () => {
|
||||
expect(
|
||||
shouldUseInteractiveProgressSpinner({
|
||||
streamIsTty: true,
|
||||
stdinIsRaw: false,
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("keeps the normal interactive spinner for regular tty commands", () => {
|
||||
expect(
|
||||
shouldUseInteractiveProgressSpinner({
|
||||
@@ -78,6 +110,25 @@ describe("cli progress", () => {
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("routes clack spinner output through the progress stream", () => {
|
||||
const stream = {
|
||||
isTTY: true,
|
||||
write: vi.fn(),
|
||||
} as unknown as NodeJS.WriteStream;
|
||||
|
||||
const progress = createCliProgress({
|
||||
label: "Loading",
|
||||
stream,
|
||||
});
|
||||
progress.done();
|
||||
|
||||
expect(clackMocks.spinner).toHaveBeenCalledWith({ output: stream });
|
||||
expect(clackMocks.spinnerInstance.start).toHaveBeenCalledWith(
|
||||
expect.stringContaining("Loading"),
|
||||
);
|
||||
expect(clackMocks.spinnerInstance.stop).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("does not write terminal controls when raw TUI input suppresses the default spinner", () => {
|
||||
const writes: string[] = [];
|
||||
const stream = {
|
||||
|
||||
@@ -102,7 +102,7 @@ export function createCliProgress(options: ProgressOptions): ProgressReporter {
|
||||
})
|
||||
: null;
|
||||
|
||||
const spin = allowSpinner ? spinner() : null;
|
||||
const spin = allowSpinner ? spinner({ output: stream }) : null;
|
||||
const renderLine = allowLine
|
||||
? () => {
|
||||
if (!started) {
|
||||
|
||||
@@ -383,6 +383,33 @@ describe("runCli exit behavior", () => {
|
||||
expect(progressDoneMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("suppresses startup progress for json output commands before full CLI parsing", async () => {
|
||||
tryRouteCliMock.mockResolvedValueOnce(false);
|
||||
const parseAsync = vi.fn().mockResolvedValueOnce(undefined);
|
||||
buildProgramMock.mockReturnValueOnce({
|
||||
commands: [{ name: () => "sessions", aliases: () => [] }],
|
||||
parseAsync,
|
||||
});
|
||||
|
||||
await runCli(["node", "openclaw", "sessions", "--json", "--limit", "all"]);
|
||||
|
||||
expect(createCliProgressMock).toHaveBeenCalledWith({
|
||||
label: "Loading OpenClaw CLI…",
|
||||
indeterminate: true,
|
||||
delayMs: 0,
|
||||
enabled: false,
|
||||
});
|
||||
expect(parseAsync).toHaveBeenCalledWith([
|
||||
"node",
|
||||
"openclaw",
|
||||
"sessions",
|
||||
"--json",
|
||||
"--limit",
|
||||
"all",
|
||||
]);
|
||||
expect(progressDoneMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("pauses non-tty stdin after full CLI command completion", async () => {
|
||||
tryRouteCliMock.mockResolvedValueOnce(false);
|
||||
const parseAsync = vi.fn().mockResolvedValueOnce(undefined);
|
||||
|
||||
@@ -751,11 +751,14 @@ export async function runCli(argv: string[] = process.argv) {
|
||||
return;
|
||||
}
|
||||
|
||||
const parseArgv = normalizeGeneratedHelpCommandArgv(rewriteUpdateFlagArgv(normalizedArgv));
|
||||
const suppressStartupProgress = hasJsonOutputFlag(parseArgv);
|
||||
const { createCliProgress } = await loadProgressModule();
|
||||
const startupProgress = createCliProgress({
|
||||
label: "Loading OpenClaw CLI…",
|
||||
indeterminate: true,
|
||||
delayMs: 0,
|
||||
...(suppressStartupProgress ? { enabled: false } : {}),
|
||||
});
|
||||
let startupProgressStopped = false;
|
||||
const stopStartupProgress = () => {
|
||||
@@ -823,7 +826,6 @@ export async function runCli(argv: string[] = process.argv) {
|
||||
process.exit(1);
|
||||
});
|
||||
|
||||
const parseArgv = normalizeGeneratedHelpCommandArgv(rewriteUpdateFlagArgv(normalizedArgv));
|
||||
const invocation = resolveCliArgvInvocation(parseArgv);
|
||||
// Register the primary command (builtin or subcli) so help and command parsing
|
||||
// are correct even with lazy command registration.
|
||||
|
||||
Reference in New Issue
Block a user