fix(sessions): unify target errors and reject blank agent selectors (#136664)

This commit is contained in:
Peter Steinberger 2026-09-02 15:29:54 -07:00 • committed by GitHub
parent b22ab48280
commit 2ff8497755
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 165 additions and 208 deletions

View file

@ -50,6 +50,10 @@ SQLite target, verifies the target exists and is usable, and reports the physica
path it actually read. Combine it with `--agent <id>` when you must select the
configured agent that owns the store.
`--agent` and `--store` require non-blank values. Selection errors exit non-zero
and use the standard [CLI JSON failure envelope](/cli#json-failures) when `--json`
is set.
`openclaw sessions` and the Gateway `sessions.list` RPC are bounded by default
so large long-lived stores cannot monopolize the CLI process or Gateway event
loop. The CLI returns the newest 100 sessions by default; pass `--limit <n>`
@ -191,6 +195,10 @@ print before follow mode; default `80`, and `0` starts at the current end.
fixed-width terminal columns, with long keys truncated at whole grapheme boundaries
so CJK characters, combining accents, and joined emoji keep progress lines aligned.
A fully qualified `--session-key` selects its agent only when `--agent`, `--store`,
and `--all-agents` are absent. An explicitly empty or whitespace-only `--agent`
is rejected instead of selecting an inferred agent.
The progress view is intentionally conservative: prompt text, tool arguments,
and tool result bodies are not printed. Tool calls show the tool name with
`{...redacted...}`; tool results show status such as `ok`, `error`, or `done`;

View file

@ -4,8 +4,8 @@ import path from "node:path";
import { DatabaseSync } from "node:sqlite";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { useAutoCleanupTempDirTracker } from "../../test/helpers/temp-dir.js";
import type { RuntimeEnv } from "../runtime.js";
import { resolveSessionStoreTargetsOrExit } from "./session-store-targets.js";
import { ExpectedCliError } from "../cli/failure-output.js";
import { resolveCommandSessionStoreTargets } from "./session-store-targets.js";
const resolveSessionStoreTargetsMock = vi.hoisted(() => vi.fn());
@ -13,14 +13,6 @@ vi.mock("../config/sessions.js", () => ({
resolveSessionStoreTargets: resolveSessionStoreTargetsMock,
}));
function createRuntime(): RuntimeEnv {
return {
log: vi.fn(),
error: vi.fn(),
exit: vi.fn(),
};
}
function createRepairableSessionDatabase(pathname: string): void {
const database = new DatabaseSync(pathname);
database.exec(`
@ -36,7 +28,7 @@ function createRepairableSessionDatabase(pathname: string): void {
database.close();
}
describe("resolveSessionStoreTargetsOrExit", () => {
describe("resolveCommandSessionStoreTargets", () => {
const tempDirs = useAutoCleanupTempDirTracker(afterEach);
beforeEach(() => {
@ -47,96 +39,57 @@ describe("resolveSessionStoreTargetsOrExit", () => {
resolveSessionStoreTargetsMock.mockReturnValue([
{ agentId: "main", storePath: "/tmp/main-sessions.json" },
]);
const runtime = createRuntime();
const targets = resolveSessionStoreTargetsOrExit({
const targets = resolveCommandSessionStoreTargets({
cfg: {},
opts: {},
runtime,
});
expect(targets).toEqual([{ agentId: "main", storePath: "/tmp/main-sessions.json" }]);
expect(resolveSessionStoreTargetsMock).toHaveBeenCalledWith({}, {});
expect(runtime.exit).not.toHaveBeenCalled();
});
it("reports resolution errors and exits the command", () => {
it("hands resolution errors to the CLI failure owner", () => {
resolveSessionStoreTargetsMock.mockImplementation(() => {
throw new Error("Unknown agent id: ghost");
});
const runtime = createRuntime();
const targets = resolveSessionStoreTargetsOrExit({
cfg: {},
opts: { agent: "ghost" },
runtime,
});
expect(targets).toBeNull();
expect(runtime.error).toHaveBeenCalledWith("Unknown agent id: ghost");
expect(runtime.exit).toHaveBeenCalledWith(1);
expect(() => resolveCommandSessionStoreTargets({ cfg: {}, opts: { agent: "ghost" } })).toThrow(
ExpectedCliError,
);
});
it.each([
["missing", "human"],
["missing", "json"],
["suffixless", "human"],
["suffixless", "json"],
["non-database", "human"],
["non-database", "json"],
["foreign-database", "human"],
["foreign-database", "json"],
] as const)("rejects a %s explicit store in %s mode", (storeKind, mode) => {
const dir = tempDirs.make("openclaw-explicit-session-store-");
const storePath =
storeKind === "missing"
? path.join(dir, "missing.sqlite")
: storeKind === "suffixless"
? path.join(dir, "requested-store")
: storeKind === "foreign-database"
? path.join(dir, "foreign.sqlite")
: path.join(dir, "not-a-database.sqlite");
const resolvedPath = storeKind === "suffixless" ? `${storePath}.sqlite` : storePath;
if (storeKind === "suffixless") {
fs.mkdirSync(storePath);
} else if (storeKind === "non-database") {
fs.writeFileSync(storePath, "not a db");
} else if (storeKind === "foreign-database") {
const database = new DatabaseSync(storePath);
database.exec("CREATE TABLE unrelated (id INTEGER PRIMARY KEY)");
database.close();
}
resolveSessionStoreTargetsMock.mockReturnValue([{ agentId: "main", storePath }]);
const runtime = createRuntime();
const targets = resolveSessionStoreTargetsOrExit({
cfg: {},
opts: { store: storePath },
runtime,
json: mode === "json",
});
expect(targets).toBeNull();
expect(runtime.exit).toHaveBeenCalledWith(1);
const output = [...vi.mocked(runtime.log).mock.calls, ...vi.mocked(runtime.error).mock.calls]
.flat()
.join("\n");
expect(output).toContain(resolvedPath);
expect(output).toMatch(/session store/iu);
expect(output).toMatch(/resolved SQLite target exists|not a session store/iu);
if (storeKind === "suffixless") {
expect(output).toContain(storePath);
}
if (mode === "json") {
expect(JSON.parse(String(vi.mocked(runtime.log).mock.calls[0]?.[0]))).toEqual({
error: expect.stringContaining(resolvedPath),
});
expect(runtime.error).not.toHaveBeenCalled();
} else {
expect(runtime.error).toHaveBeenCalledOnce();
expect(runtime.log).not.toHaveBeenCalled();
}
});
it.each(["missing", "suffixless", "directory", "non-database", "foreign-database"] as const)(
"rejects a %s explicit store through the CLI failure owner",
(storeKind) => {
const dir = tempDirs.make("openclaw-explicit-session-store-");
const storePath = path.join(
dir,
storeKind === "suffixless" ? "requested-store" : `${storeKind}.sqlite`,
);
const resolvedPath = storeKind === "suffixless" ? `${storePath}.sqlite` : storePath;
if (storeKind === "suffixless" || storeKind === "directory") {
fs.mkdirSync(storePath);
} else if (storeKind === "non-database") {
fs.writeFileSync(storePath, "not a db");
} else if (storeKind === "foreign-database") {
const database = new DatabaseSync(storePath);
database.exec("CREATE TABLE unrelated (id INTEGER PRIMARY KEY)");
database.close();
}
resolveSessionStoreTargetsMock.mockReturnValue([{ agentId: "main", storePath }]);
expect(() =>
resolveCommandSessionStoreTargets({ cfg: {}, opts: { store: storePath } }),
).toThrow(
expect.objectContaining({
name: "ExpectedCliError",
message: expect.stringMatching(
/resolved SQLite target exists|not a session store|not a regular file/iu,
),
humanOutput: expect.stringContaining(resolvedPath),
machineOutput: expect.stringContaining(resolvedPath),
}),
);
},
);
it.each([
["legacy JSON locator", "sessions.json", "openclaw-agent.sqlite"],
@ -146,16 +99,11 @@ describe("resolveSessionStoreTargetsOrExit", () => {
const storePath = path.join(dir, locator);
createRepairableSessionDatabase(path.join(dir, target));
resolveSessionStoreTargetsMock.mockReturnValue([{ agentId: "main", storePath }]);
const runtime = createRuntime();
const targets = resolveSessionStoreTargetsOrExit({
const targets = resolveCommandSessionStoreTargets({
cfg: {},
opts: { store: storePath },
runtime,
});
expect(targets).toEqual([{ agentId: "main", storePath }]);
expect(runtime.exit).not.toHaveBeenCalled();
expect(runtime.error).not.toHaveBeenCalled();
});
});

View file

@ -1,12 +1,7 @@
/**
* Session store target resolution wrapper for CLI commands.
*
* The config helper throws on invalid agent/store combinations; this module
* converts those errors into command output and exit codes.
*/
import fs from "node:fs";
import path from "node:path";
import { AgentSelectionRequiredError } from "../agents/agent-scope-config.js";
import { ExpectedCliError } from "../cli/failure-output.js";
import {
resolveSessionStoreTargets,
type SessionStoreSelectionOptions,
@ -16,7 +11,6 @@ import { resolveSqliteTargetFromSessionStorePath } from "../config/sessions/sess
import type { OpenClawConfig } from "../config/types.openclaw.js";
import { formatErrorMessage } from "../infra/errors.js";
import { openNodeSqliteDatabase } from "../infra/node-sqlite.js";
import { type RuntimeEnv, writeRuntimeJson } from "../runtime.js";
const SESSION_STORE_SELECTION_CONTEXT = {
surface: "session-store selection",
@ -99,72 +93,35 @@ export function resolveExplicitSessionStorePath(params: {
return storePath;
}
/** Resolves and validates an operator-supplied legacy selector without changing its semantics. */
function resolveExplicitSessionStorePathOrExit(params: {
storePath: string;
inputStorePath?: string;
agentId: string;
runtime: RuntimeEnv;
json?: boolean;
}): string | null {
try {
return resolveExplicitSessionStorePath({
agentId: params.agentId,
inputStorePath: params.inputStorePath ?? params.storePath,
storePath: params.storePath,
});
} catch (error) {
return exitSessionStoreError(params, error);
}
}
function exitSessionStoreError(
params: { runtime: RuntimeEnv; json?: boolean },
error: unknown,
): null {
const message = formatErrorMessage(error);
if (params.json) {
writeRuntimeJson(params.runtime, { error: message });
} else {
params.runtime.error(message);
}
params.runtime.exit(1);
return null;
}
/** Resolves session store targets or exits the current command on validation errors. */
export function resolveSessionStoreTargetsOrExit(params: {
/** Selection failures reach the root CLI handler for shared JSON output and cleanup. */
export function resolveCommandSessionStoreTargets(params: {
cfg: OpenClawConfig;
opts: SessionStoreSelectionOptions;
runtime: RuntimeEnv;
json?: boolean;
}): SessionStoreTarget[] | null {
let targets: SessionStoreTarget[];
}): SessionStoreTarget[] {
try {
targets = resolveSessionStoreTargets(params.cfg, params.opts);
const targets = resolveSessionStoreTargets(params.cfg, params.opts);
if (!params.opts.store) {
return targets;
}
const target = targets[0];
if (!target) {
throw new Error("Explicit session store selection did not resolve a target.");
}
return [
{
...target,
storePath: resolveExplicitSessionStorePath({
...target,
inputStorePath: params.opts.store,
}),
},
];
} catch (error) {
const displayError =
const message = formatErrorMessage(
error instanceof AgentSelectionRequiredError
? new AgentSelectionRequiredError(error.agentIds, SESSION_STORE_SELECTION_CONTEXT)
: error;
return exitSessionStoreError(params, displayError);
}
if (!params.opts.store) {
return targets;
}
const target = targets[0];
if (!target) {
return exitSessionStoreError(
params,
new Error("Explicit session store selection did not resolve a target."),
: error,
);
throw new ExpectedCliError({ message, humanOutput: message, machineOutput: message });
}
const storePath = resolveExplicitSessionStorePathOrExit({
storePath: target.storePath,
inputStorePath: params.opts.store,
agentId: target.agentId,
runtime: params.runtime,
json: params.json,
});
return storePath ? [{ ...target, storePath }] : null;
}

View file

@ -19,7 +19,7 @@ mock.module(new URL("../config/config.ts", import.meta.url), {
namedExports: { getRuntimeConfig: () => ({}) },
});
mock.module(new URL("./session-store-targets.ts", import.meta.url), {
namedExports: { resolveSessionStoreTargetsOrExit: () => [{ agentId: "main", storePath }] },
namedExports: { resolveCommandSessionStoreTargets: () => [{ agentId: "main", storePath }] },
});
mock.module(new URL("../config/sessions.ts", import.meta.url), {
namedExports: {

View file

@ -8,7 +8,7 @@ import type { RuntimeEnv } from "../runtime.js";
const mocks = vi.hoisted(() => ({
loadConfig: vi.fn(),
resolveSessionStoreTargetsOrExit: vi.fn(),
resolveCommandSessionStoreTargets: vi.fn(),
resolveSessionCleanupAction: vi.fn(),
runSessionsCleanup: vi.fn(),
runLocalSessionsCleanup: vi.fn(),
@ -24,7 +24,7 @@ vi.mock("./sessions-cleanup.runtime.js", () => ({
}));
vi.mock("./session-store-targets.js", () => ({
resolveSessionStoreTargetsOrExit: mocks.resolveSessionStoreTargetsOrExit,
resolveCommandSessionStoreTargets: mocks.resolveCommandSessionStoreTargets,
}));
vi.mock("../config/sessions.js", async (importOriginal) => ({
@ -96,7 +96,7 @@ describe("sessionsCleanupCommand", () => {
process.exitCode = undefined;
mocks.runLocalSessionsCleanup.mockImplementation((params) => mocks.runSessionsCleanup(params));
mocks.loadConfig.mockReturnValue({ session: { store: "/cfg/sessions.json" } });
mocks.resolveSessionStoreTargetsOrExit.mockReturnValue([
mocks.resolveCommandSessionStoreTargets.mockReturnValue([
{ agentId: "main", storePath: "/resolved/sessions.json" },
]);
mocks.callGateway.mockResolvedValue(null);
@ -139,7 +139,7 @@ describe("sessionsCleanupCommand", () => {
await sessionsCleanupCommand({ store: "", enforce: true }, runtime);
expect(mocks.callGateway).not.toHaveBeenCalled();
expect(mocks.resolveSessionStoreTargetsOrExit).toHaveBeenCalledWith(
expect(mocks.resolveCommandSessionStoreTargets).toHaveBeenCalledWith(
expect.objectContaining({ opts: expect.objectContaining({ store: "" }) }),
);
});
@ -761,7 +761,7 @@ describe("sessionsCleanupCommand", () => {
});
it("returns grouped JSON for --all-agents dry-runs", async () => {
mocks.resolveSessionStoreTargetsOrExit.mockReturnValue([
mocks.resolveCommandSessionStoreTargets.mockReturnValue([
{ agentId: "main", storePath: "/resolved/main-sessions.json" },
{ agentId: "work", storePath: "/resolved/work-sessions.json" },
]);

View file

@ -24,7 +24,7 @@ import type { OpenClawConfig } from "../config/types.openclaw.js";
import { callGateway, isGatewayTransportError } from "../gateway/call.js";
import { type RuntimeEnv, writeRuntimeJson } from "../runtime.js";
import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "../utils/message-channel.js";
import { resolveSessionStoreTargetsOrExit } from "./session-store-targets.js";
import { resolveCommandSessionStoreTargets } from "./session-store-targets.js";
import { resolveSessionDisplayModel } from "./sessions-display-model.js";
import {
formatSessionAgeCell,
@ -312,19 +312,7 @@ export async function sessionsCleanupCommand(opts: SessionsCleanupOptions, runti
}
const cfg = getRuntimeConfig();
const targets = resolveSessionStoreTargetsOrExit({
cfg,
opts: {
store: opts.store,
agent: opts.agent,
allAgents: opts.allAgents,
},
runtime,
json: opts.json,
});
if (!targets) {
return;
}
const targets = resolveCommandSessionStoreTargets({ cfg, opts });
const cleanupParams = { cfg, opts, targets };
let cleanupResult;
if (opts.dryRun) {

View file

@ -4,6 +4,7 @@ import os from "node:os";
import path from "node:path";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { visibleWidth } from "../../packages/terminal-core/src/ansi.js";
import { ExpectedCliError } from "../cli/failure-output.js";
import { upsertSessionEntryCore } from "../config/sessions/session-accessor.js";
import type { SessionEntry } from "../config/sessions/types.js";
import type { RuntimeEnv } from "../runtime.js";
@ -432,6 +433,23 @@ describe("sessionsTailCommand", () => {
expect(runtime.exit).toHaveBeenCalledWith(1);
});
it.each([
{ agent: "" },
{ agent: " " },
{ agent: "", sessionKey },
{ agent: " ", sessionKey },
])("rejects an explicit blank agent without inferring a store: %j", async (opts) => {
mocks.getRuntimeConfig.mockReturnValue({});
const runtime = makeRuntime();
const result = sessionsTailCommand(opts, runtime);
await expect(result).rejects.toBeInstanceOf(ExpectedCliError);
await expect(result).rejects.toMatchObject({ message: "--agent must not be blank" });
expect(runtime.log).not.toHaveBeenCalled();
expect(runtime.error).not.toHaveBeenCalled();
expect(runtime.exit).not.toHaveBeenCalled();
});
it("resolves the target store from a fully qualified non-default agent session key", async () => {
const runtime = makeRuntime();
const opsSessionKey = "agent:ops:telegram:direct:owner";

View file

@ -15,7 +15,7 @@ import { resolveAgentIdFromSessionKey } from "../routing/session-key.js";
import type { RuntimeEnv } from "../runtime.js";
import { loadSqliteTrajectoryRuntimeEventRowsSync } from "../trajectory/runtime-store.sqlite.js";
import type { TrajectoryEvent } from "../trajectory/types.js";
import { resolveSessionStoreTargetsOrExit } from "./session-store-targets.js";
import { resolveCommandSessionStoreTargets } from "./session-store-targets.js";
import { formatTextCell } from "./text-format.js";
type SessionsTailOptions = {
@ -258,7 +258,8 @@ async function followSelections(
}
function resolveTailTargetAgent(opts: SessionsTailOptions): string | undefined {
if (opts.agent?.trim() || opts.store?.trim() || opts.allAgents === true) {
// Keep explicit blanks for the selector to reject instead of inferring a different owner.
if (opts.agent !== undefined || opts.store !== undefined || opts.allAgents === true) {
return opts.agent;
}
return opts.sessionKey?.trim() ? resolveAgentIdFromSessionKey(opts.sessionKey) : undefined;
@ -277,18 +278,14 @@ export async function sessionsTailCommand(
}
const cfg = getRuntimeConfig();
const targets = resolveSessionStoreTargetsOrExit({
const targets = resolveCommandSessionStoreTargets({
cfg,
opts: {
store: opts.store,
agent: resolveTailTargetAgent(opts),
allAgents: opts.allAgents,
},
runtime,
});
if (!targets) {
return;
}
const selections: TailSelection[] = [];
for (const target of targets) {

View file

@ -1,5 +1,6 @@
// Sessions default-agent store tests cover default session-store selection and runtime config loading.
import { beforeEach, describe, expect, it, vi } from "vitest";
import { ExpectedCliError } from "../cli/failure-output.js";
import type { RuntimeEnv } from "../runtime.js";
const loadConfigMock = vi.hoisted(() => vi.fn());
@ -179,12 +180,14 @@ describe("sessionsCommand default store agent selection", () => {
});
const { runtime } = createRuntime();
await sessionsCommand({}, runtime);
expect(runtime.error).toHaveBeenCalledWith(
"Multiple agents are configured, but session-store selection has no explicit owner. Pass --agent <id> to select one agent, or --all-agents to include every configured agent.",
);
expect(runtime.exit).toHaveBeenCalledWith(1);
const result = sessionsCommand({}, runtime);
await expect(result).rejects.toBeInstanceOf(ExpectedCliError);
await expect(result).rejects.toMatchObject({
message:
"Multiple agents are configured, but session-store selection has no explicit owner. Pass --agent <id> to select one agent, or --all-agents to include every configured agent.",
});
expect(runtime.error).not.toHaveBeenCalled();
expect(runtime.exit).not.toHaveBeenCalled();
});
it("uses all configured agent stores with --all-agents", async () => {

View file

@ -42,7 +42,7 @@ import {
deliveryContextFromSession,
sessionDeliveryOrigin,
} from "../utils/delivery-context.shared.js";
import { resolveSessionStoreTargetsOrExit } from "./session-store-targets.js";
import { resolveCommandSessionStoreTargets } from "./session-store-targets.js";
import {
resolveSessionDisplayModelRef,
resolveSessionDisplayDefaults,
@ -312,19 +312,7 @@ export async function sessionsCommand(
await contextLookupRuntimeLoader.load();
const configContextTokens =
lookupContextTokens(displayDefaults.model, { allowAsyncLoad: false }) ?? DEFAULT_CONTEXT_TOKENS;
const targets = resolveSessionStoreTargetsOrExit({
cfg,
opts: {
store: opts.store,
agent: opts.agent,
allAgents: opts.allAgents,
},
runtime,
json: opts.json,
});
if (!targets) {
return;
}
const targets = resolveCommandSessionStoreTargets({ cfg, opts });
let activeMinutes: number | undefined;
if (opts.active !== undefined) {

View file

@ -99,6 +99,56 @@ describe("cli json stdout contract", () => {
"`sessions cleanup` does not support the parent `sessions` option --active; session-list filters cannot scope session maintenance.",
human: true,
},
{
name: "bare list unknown agent",
args: ["sessions", "--agent", "unknown-agent", "--json"],
message:
'Unknown agent id "unknown-agent". Run openclaw agents list to see configured agents.',
},
{
name: "Commander list unknown agent through dual-TTY finalization",
args: ["sessions", "--json", "--agent", "unknown-agent"],
message:
'Unknown agent id "unknown-agent". Run openclaw agents list to see configured agents.',
commander: true,
tty: true,
},
{
name: "list alias blank store with inherited parent JSON",
args: ["sessions", "--json", "list", "--store", ""],
message: "--store must not be blank",
},
{
name: "bare list missing explicit store",
args: ["sessions", "--store", "$MISSING_STORE", "--json"],
message:
"Session store target does not exist: $MISSING_STORE. Pass a selector whose resolved SQLite target exists.",
},
{
name: "cleanup missing explicit store",
args: ["sessions", "cleanup", "--dry-run", "--store", "$MISSING_STORE", "--json"],
message:
"Session store target does not exist: $MISSING_STORE. Pass a selector whose resolved SQLite target exists.",
},
{
name: "list unknown agent in human mode",
args: ["sessions", "--agent", "unknown-agent"],
message:
'Unknown agent id "unknown-agent". Run openclaw agents list to see configured agents.',
human: true,
},
{
name: "tail blank explicit agent",
args: ["sessions", "tail", "--agent", ""],
message: "--agent must not be blank",
human: true,
},
{
name: "tail whitespace parent agent with a session key",
args: ["sessions", "--agent", " ", "tail", "--session-key", "agent:main:test"],
message: "--agent must not be blank",
human: true,
},
{
name: "cleanup inherited filter with leaf JSON",
args: ["sessions", "--active", "5", "cleanup", "--json"],