fix(cli): render sessions list JSON failures (#128316)

This commit is contained in:
Peter Steinberger
2026-08-23 14:11:26 -07:00
committed by GitHub
parent c48e973c66
commit a3e69b2ba0
3 changed files with 109 additions and 66 deletions
+40 -59
View File
@@ -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();
});
});
+5 -6
View File
@@ -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);
+64 -1
View File
@@ -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(