diff --git a/src/commands/sessions.test.ts b/src/commands/sessions.test.ts index d5ed5ad0a86d..d68211bbb644 100644 --- a/src/commands/sessions.test.ts +++ b/src/commands/sessions.test.ts @@ -1,5 +1,6 @@ // Sessions command tests cover listing, details, filtering, and transcript display behavior. import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { ExpectedCliError } from "../cli/failure-output.js"; import { assignSessionOwner, recordSessionParticipant, @@ -16,9 +17,6 @@ import { writeStore, } from "./sessions.test-helpers.js"; -// Disable colors for deterministic snapshots. -process.env.FORCE_COLOR = "0"; - mockSessionsConfig(); import { sessionsCommand } from "./sessions.js"; @@ -31,6 +29,7 @@ describe("sessionsCommand", () => { afterEach(() => { resetMockSessionsConfig(); + vi.restoreAllMocks(); vi.useRealTimers(); }); @@ -727,63 +726,45 @@ describe("sessionsCommand", () => { ]); }); - it("rejects invalid --active values", async () => { - const store = await writeStore( - { - "agent:main:demo": { - sessionId: "demo", - updatedAt: Date.now() - 5 * 60_000, - }, - }, - "sessions-active-invalid", + it.each([ + { + name: "invalid active minutes", + options: { active: "0" }, + message: "--active must be a positive number of minutes, for example --active 30.", + }, + { + name: "partially numeric active minutes", + options: { active: "10m" }, + message: "--active must be a positive number of minutes, for example --active 30.", + }, + { + name: "an invalid limit", + options: { limit: "0" }, + message: '--limit must be a positive integer or "all", for example --limit 25.', + }, + { + name: "active minutes before an invalid limit", + options: { active: "0", limit: "0" }, + message: "--active must be a positive number of minutes, for example --active 30.", + }, + ])("rejects $name before reading session stores", async ({ options, message }) => { + const listSessionEntries = vi.spyOn( + await import("../config/sessions/session-accessor.js"), + "listSessionEntriesReadOnly", ); - const { runtime, errors } = makeRuntime(); + const { runtime, logs, errors } = makeRuntime(); + const runtimeExit = vi.spyOn(runtime, "exit"); + const execution = sessionsCommand(options, runtime); - await expect(sessionsCommand({ store, active: "0" }, runtime)).rejects.toThrow("exit 1"); - expect(errors).toStrictEqual([ - "--active must be a positive number of minutes, for example --active 30.", - ]); - - cleanupStore(store); - }); - - it("rejects partial --active values", async () => { - const store = await writeStore( - { - "agent:main:demo": { - sessionId: "demo", - updatedAt: Date.now() - 5 * 60_000, - }, - }, - "sessions-active-partial", - ); - const { runtime, errors } = makeRuntime(); - - await expect(sessionsCommand({ store, active: "10m" }, runtime)).rejects.toThrow("exit 1"); - expect(errors).toStrictEqual([ - "--active must be a positive number of minutes, for example --active 30.", - ]); - - cleanupStore(store); - }); - - it("rejects invalid --limit values", async () => { - const store = await writeStore( - { - "agent:main:demo": { - sessionId: "demo", - updatedAt: Date.now() - 5 * 60_000, - }, - }, - "sessions-limit-invalid", - ); - const { runtime, errors } = makeRuntime(); - - await expect(sessionsCommand({ store, limit: "0" }, runtime)).rejects.toThrow("exit 1"); - expect(errors).toStrictEqual([ - '--limit must be a positive integer or "all", for example --limit 25.', - ]); - - cleanupStore(store); + await expect(execution).rejects.toBeInstanceOf(ExpectedCliError); + await expect(execution).rejects.toMatchObject({ + message, + humanOutput: message, + machineOutput: message, + }); + expect(logs).toEqual([]); + expect(errors).toEqual([]); + expect(runtimeExit).not.toHaveBeenCalled(); + expect(listSessionEntries).not.toHaveBeenCalled(); }); }); diff --git a/src/commands/sessions.ts b/src/commands/sessions.ts index e572cc54040f..66e55f13a518 100644 --- a/src/commands/sessions.ts +++ b/src/commands/sessions.ts @@ -20,6 +20,7 @@ import { } from "../agents/model-selection.js"; import { resolveRuntimePolicySessionKey } from "../auto-reply/reply/runtime-policy-session-key.js"; import { normalizeChatType } from "../channels/chat-type.js"; +import { ExpectedCliError } from "../cli/failure-output.js"; import { getRuntimeConfig } from "../config/config.js"; import { resolveFreshSessionTotalTokens, resolveSessionTotalTokens } from "../config/sessions.js"; import { resolveProjectedSessionContextTokens } from "../config/sessions/context-token-provenance.js"; @@ -341,18 +342,16 @@ export async function sessionsCommand( if (opts.active !== undefined) { const parsed = parseStrictPositiveInteger(opts.active); if (parsed === undefined) { - runtime.error("--active must be a positive number of minutes, for example --active 30."); - runtime.exit(1); - return; + const message = "--active must be a positive number of minutes, for example --active 30."; + throw new ExpectedCliError({ message, humanOutput: message, machineOutput: message }); } activeMinutes = parsed; } const limit = parseSessionsLimit(opts.limit); if (limit === null) { - runtime.error('--limit must be a positive integer or "all", for example --limit 25.'); - runtime.exit(1); - return; + const message = '--limit must be a positive integer or "all", for example --limit 25.'; + throw new ExpectedCliError({ message, humanOutput: message, machineOutput: message }); } const classifyCliProvider = prepareCliProviderClassifier(cfg); diff --git a/test/cli-json-stdout.e2e.test.ts b/test/cli-json-stdout.e2e.test.ts index bdad276a44c4..cab2d87f634a 100644 --- a/test/cli-json-stdout.e2e.test.ts +++ b/test/cli-json-stdout.e2e.test.ts @@ -332,6 +332,69 @@ describe("cli json stdout contract", () => { }); it.each([ + { + name: "bare list active filter in human mode", + args: ["sessions", "--active", "0"], + message: "--active must be a positive number of minutes, for example --active 30.", + human: true, + }, + { + name: "bare list limit in human mode through forced Commander", + args: ["sessions", "--limit", "0"], + message: '--limit must be a positive integer or "all", for example --limit 25.', + human: true, + commander: true, + }, + { + name: "routed bare list active filter with JSON before its option", + args: ["sessions", "--json", "--active", "0"], + message: "--active must be a positive number of minutes, for example --active 30.", + }, + { + name: "routed bare list limit with JSON after its option", + args: ["sessions", "--limit", "0", "--json"], + message: '--limit must be a positive integer or "all", for example --limit 25.', + }, + { + name: "Commander bare list active filter with JSON after its option", + args: ["sessions", "--active", "0", "--json"], + message: "--active must be a positive number of minutes, for example --active 30.", + commander: true, + }, + { + name: "Commander bare list limit with JSON before its option", + args: ["sessions", "--json", "--limit", "0"], + message: '--limit must be a positive integer or "all", for example --limit 25.', + commander: true, + }, + { + name: "list alias active filter with inherited parent JSON", + args: ["sessions", "--json", "list", "--active", "0"], + message: "--active must be a positive number of minutes, for example --active 30.", + }, + { + name: "list alias limit with leaf JSON", + args: ["sessions", "list", "--limit", "0", "--json"], + message: '--limit must be a positive integer or "all", for example --limit 25.', + }, + { + name: "bare list active filter before an invalid limit", + args: ["sessions", "--json", "--limit", "0", "--active", "0"], + message: "--active must be a positive number of minutes, for example --active 30.", + }, + { + name: "routed bare list active filter through dual-TTY finalization", + args: ["sessions", "--json", "--active", "0"], + message: "--active must be a positive number of minutes, for example --active 30.", + tty: true, + }, + { + name: "Commander bare list limit through dual-TTY finalization", + args: ["sessions", "--limit", "0", "--json"], + message: '--limit must be a positive integer or "all", for example --limit 25.', + commander: true, + tty: true, + }, { name: "cleanup with an inherited filter in human mode", args: ["sessions", "--active", "5", "cleanup"], @@ -421,7 +484,7 @@ describe("cli json stdout contract", () => { message: "--max-lines must be a positive integer.", tty: true, }, - ])("renders sessions registration validation failures for $name", async (testCase) => { + ])("renders sessions list and registration validation failures for $name", async (testCase) => { await withTempHome( async (tempHome) => { const preload = Buffer.from(