mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-23 10:55:31 -06:00
fix(memory): reject unknown --agent ids and keep the hint runnable (#126570)
The memory CLI resolved --agent by returning the caller's string verbatim, so an id that is not configured produced a confident empty result: `memory status` rendered a panel for it, `memory index` fabricated a workspace-<id> path, and `memory search` reported No matches. A typo read as an empty memory rather than a nonexistent agent, while hooks, status --usage, capability, migrate, and session targets already rejected unknown ids. Consolidate that duplicated check into resolveConfiguredAgentId beside the agent roster owner, reuse it at the matching core sites, and route memory to it through the existing memory-core host-runtime facade so no new plugin SDK surface is added. The canonical hint uses formatCliCommand rather than a literal: under a profile or container the bare command is wrong, so consolidating on a literal would have regressed the hooks and migrate hints and left the status, capability, and session-target hints unrunnable.
This commit is contained in:
committed by
GitHub
parent
a19d924797
commit
75fcb1fbb9
@@ -1,8 +1,11 @@
|
||||
import { listAgentIds } from "openclaw/plugin-sdk/agent-runtime";
|
||||
import {
|
||||
normalizeExtraMemoryPathEntries,
|
||||
type MemoryExtraPath,
|
||||
} from "openclaw/plugin-sdk/memory-core-host-engine-storage";
|
||||
import {
|
||||
listAgentIds,
|
||||
resolveConfiguredAgentId,
|
||||
} from "openclaw/plugin-sdk/memory-core-host-runtime-core";
|
||||
import { buildAgentSessionKey } from "openclaw/plugin-sdk/routing";
|
||||
import { asNullableRecord } from "openclaw/plugin-sdk/string-coerce-runtime";
|
||||
import {
|
||||
@@ -127,10 +130,10 @@ export function formatAuditCounts(audit: ShortTermAuditSummary): string {
|
||||
}
|
||||
function resolveAgent(cfg: OpenClawConfig, agent?: string) {
|
||||
const trimmed = agent?.trim();
|
||||
if (trimmed) {
|
||||
return trimmed;
|
||||
if (agent !== undefined && !trimmed) {
|
||||
throw new Error("--agent must not be blank");
|
||||
}
|
||||
return resolveDefaultAgentId(cfg);
|
||||
return trimmed ? resolveConfiguredAgentId(cfg, trimmed) : resolveDefaultAgentId(cfg);
|
||||
}
|
||||
export function buildCliMemorySearchSessionKey(agentId: string): string {
|
||||
return buildAgentSessionKey({
|
||||
@@ -142,10 +145,10 @@ export function buildCliMemorySearchSessionKey(agentId: string): string {
|
||||
}
|
||||
function resolveAgentIds(cfg: OpenClawConfig, agent?: string): string[] {
|
||||
const trimmed = agent?.trim();
|
||||
if (trimmed) {
|
||||
return [trimmed];
|
||||
if (agent !== undefined && !trimmed) {
|
||||
throw new Error("--agent must not be blank");
|
||||
}
|
||||
return listAgentIds(cfg);
|
||||
return trimmed ? [resolveConfiguredAgentId(cfg, trimmed)] : listAgentIds(cfg);
|
||||
}
|
||||
export function formatExtraPaths(workspaceDir: string, extraPaths: MemoryExtraPath[]): string[] {
|
||||
return normalizeExtraMemoryPathEntries(workspaceDir, extraPaths).map((entry) => {
|
||||
|
||||
@@ -301,6 +301,78 @@ describe("memory cli", () => {
|
||||
await program.parseAsync(["memory", ...args], { from: "user" });
|
||||
}
|
||||
|
||||
const configuredAgents = {
|
||||
agents: { ownership: "explicit" as const, entries: { main: {}, ops: {} } },
|
||||
};
|
||||
|
||||
function mockCommandManagerForConfiguredAgents() {
|
||||
getMemorySearchManager.mockImplementation(async () => ({
|
||||
manager: {
|
||||
status: () => makeMemoryStatus({ workspaceDir: undefined }),
|
||||
sync: vi.fn(async () => {}),
|
||||
search: vi.fn(async () => []),
|
||||
close: vi.fn(async () => {}),
|
||||
},
|
||||
}));
|
||||
}
|
||||
|
||||
it.each([
|
||||
["status", ["status", "--agent", "nope-zzz"]],
|
||||
["index", ["index", "--agent", "nope-zzz"]],
|
||||
["search", ["search", "foo", "--agent", "nope-zzz"]],
|
||||
])("rejects an unknown explicit agent before %s acquires a manager", async (_name, args) => {
|
||||
getRuntimeConfig.mockReturnValue(configuredAgents);
|
||||
mockCommandManagerForConfiguredAgents();
|
||||
|
||||
await expect(runMemoryCli(args)).rejects.toThrow(
|
||||
'Unknown agent id "nope-zzz". Run openclaw agents list to see configured agents.',
|
||||
);
|
||||
expect(getMemorySearchManager).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each([
|
||||
["status", ["status", "--agent", ""]],
|
||||
["search", ["search", "foo", "--agent", ""]],
|
||||
])("rejects an explicitly blank agent before %s acquires a manager", async (_name, args) => {
|
||||
getRuntimeConfig.mockReturnValue(configuredAgents);
|
||||
mockCommandManagerForConfiguredAgents();
|
||||
|
||||
await expect(runMemoryCli(args)).rejects.toThrow("--agent must not be blank");
|
||||
expect(getMemorySearchManager).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each([
|
||||
["status", ["status", "--agent", "ops"]],
|
||||
["index", ["index", "--agent", "ops"]],
|
||||
["search", ["search", "foo", "--agent", "ops"]],
|
||||
])("keeps a valid explicit agent working for %s", async (_name, args) => {
|
||||
getRuntimeConfig.mockReturnValue(configuredAgents);
|
||||
mockCommandManagerForConfiguredAgents();
|
||||
|
||||
await runMemoryCli(args);
|
||||
|
||||
expect(getMemorySearchManager).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ agentId: "ops" }),
|
||||
);
|
||||
});
|
||||
|
||||
it.each([
|
||||
["status", ["status"]],
|
||||
["index", ["index"]],
|
||||
["search", ["search", "foo"]],
|
||||
])("keeps a configured single-agent install working for %s", async (_name, args) => {
|
||||
getRuntimeConfig.mockReturnValue({ agents: { entries: { solo: {} } } });
|
||||
resolveDefaultAgentId.mockReturnValue("solo");
|
||||
mockCommandManagerForConfiguredAgents();
|
||||
|
||||
await runMemoryCli(args);
|
||||
|
||||
expect(getMemorySearchManager).toHaveBeenCalledTimes(1);
|
||||
expect(getMemorySearchManager).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ agentId: "solo" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("drains session backfill in one apply command before preview", async () => {
|
||||
const workspaceDir = path.join(workspaceFixtureRoot, `session-backfill-${workspaceCaseId++}`);
|
||||
vi.stubEnv("OPENCLAW_STATE_DIR", path.join(workspaceDir, "state"));
|
||||
@@ -728,6 +800,28 @@ describe("memory cli", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("fans index out to every keyed agent entry", async () => {
|
||||
const agentIds = ["main", "ops"];
|
||||
getRuntimeConfig.mockReturnValue(configuredAgents);
|
||||
const syncedAgentIds: string[] = [];
|
||||
getMemorySearchManager.mockImplementation(async ({ agentId }: { agentId: string }) => ({
|
||||
manager: {
|
||||
sync: vi.fn(async () => {
|
||||
syncedAgentIds.push(agentId);
|
||||
}),
|
||||
status: () => makeMemoryStatus({ workspaceDir: undefined }),
|
||||
close: vi.fn(async () => {}),
|
||||
},
|
||||
}));
|
||||
|
||||
await runMemoryCli(["index"]);
|
||||
|
||||
expect(
|
||||
getMemorySearchManager.mock.calls.map(([params]) => (params as { agentId: string }).agentId),
|
||||
).toEqual(agentIds);
|
||||
expect(syncedAgentIds).toEqual(agentIds);
|
||||
});
|
||||
|
||||
it("resolves configured memory SecretRefs through gateway snapshot", async () => {
|
||||
const config = {
|
||||
memory: {
|
||||
|
||||
@@ -6,6 +6,7 @@ import {
|
||||
AgentSelectionRequiredError,
|
||||
listAgentEntriesWithSource,
|
||||
listAgentIds,
|
||||
resolveConfiguredAgentId,
|
||||
resolveAgentConfig,
|
||||
resolveAgentOperationAgentId,
|
||||
resolveAgentWorkspaceDir,
|
||||
@@ -21,6 +22,33 @@ import {
|
||||
vi.unmock("./agent-scope-config.js");
|
||||
|
||||
describe("agent roster resolution", () => {
|
||||
it("rejects unknown configured-agent selections with canonical CLI guidance", () => {
|
||||
const cfg = { agents: { entries: { main: {}, ops: {} } } };
|
||||
|
||||
expect(resolveConfiguredAgentId(cfg, "ops")).toBe("ops");
|
||||
expect(() => resolveConfiguredAgentId(cfg, "nope-zzz")).toThrow(
|
||||
'Unknown agent id "nope-zzz". Run openclaw agents list to see configured agents.',
|
||||
);
|
||||
});
|
||||
|
||||
it("keeps the guidance runnable under a profile", () => {
|
||||
const cfg = { agents: { entries: { main: {}, ops: {} } } };
|
||||
const previous = process.env.OPENCLAW_PROFILE;
|
||||
process.env.OPENCLAW_PROFILE = "testprof";
|
||||
try {
|
||||
// A hint the operator cannot paste back is worse than none, so the profile must survive.
|
||||
expect(() => resolveConfiguredAgentId(cfg, "nope-zzz")).toThrow(
|
||||
"Run openclaw --profile testprof agents list to see configured agents.",
|
||||
);
|
||||
} finally {
|
||||
if (previous === undefined) {
|
||||
delete process.env.OPENCLAW_PROFILE;
|
||||
} else {
|
||||
process.env.OPENCLAW_PROFILE = previous;
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it("preserves the Plugin SDK fallback only when the roster property is absent", () => {
|
||||
expect(listAgentIds({})).toEqual(["main"]);
|
||||
expect(listAgentIds({ agents: { entries: {} } })).toEqual([]);
|
||||
|
||||
@@ -4,6 +4,7 @@ import {
|
||||
normalizeOptionalString,
|
||||
readStringValue,
|
||||
} from "@openclaw/normalization-core/string-coerce";
|
||||
import { formatCliCommand } from "../cli/command-format.js";
|
||||
import { getRetainedLegacyDefaultAgentId } from "../config/legacy.default-agent-owner-state.js";
|
||||
import { hasExplicitModelPolicyAllow } from "../config/model-policy-allowlist-migration.js";
|
||||
import { resolveStateDir } from "../config/paths.js";
|
||||
@@ -180,6 +181,18 @@ export function listAgentIds(cfg: OpenClawConfig): string[] {
|
||||
return ids;
|
||||
}
|
||||
|
||||
/** Returns a configured agent id or throws the canonical CLI selection error. */
|
||||
export function resolveConfiguredAgentId(cfg: OpenClawConfig, agentId: string): string {
|
||||
if (!listAgentIds(cfg).includes(agentId)) {
|
||||
// formatCliCommand, not a literal: under a profile or container the bare command is wrong,
|
||||
// so a hint that cannot be pasted back is worse than none.
|
||||
throw new Error(
|
||||
`Unknown agent id "${agentId}". Run ${formatCliCommand("openclaw agents list")} to see configured agents.`,
|
||||
);
|
||||
}
|
||||
return agentId;
|
||||
}
|
||||
|
||||
export function tryResolveSoleAgentId(cfg: OpenClawConfig): string | undefined {
|
||||
const agents = listAgentEntries(cfg);
|
||||
if (agents.length === 0) {
|
||||
|
||||
@@ -39,6 +39,7 @@ export {
|
||||
listAgentEntries,
|
||||
listAgentEntriesWithSource,
|
||||
listAgentIds,
|
||||
resolveConfiguredAgentId,
|
||||
resolveMutableAgentEntry,
|
||||
toAgentEntriesRecord,
|
||||
resolveAgentConfig,
|
||||
|
||||
@@ -3,7 +3,10 @@ import {
|
||||
parseStrictPositiveInteger,
|
||||
} from "@openclaw/normalization-core/number-coercion";
|
||||
import type { Command } from "commander";
|
||||
import { listAgentIds, resolveAgentOperationAgentId } from "../../agents/agent-scope-config.js";
|
||||
import {
|
||||
resolveAgentOperationAgentId,
|
||||
resolveConfiguredAgentId,
|
||||
} from "../../agents/agent-scope-config.js";
|
||||
import { resolveAgentDir } from "../../agents/agent-scope.js";
|
||||
import {
|
||||
listProfilesForProvider,
|
||||
@@ -117,12 +120,7 @@ export function resolveCapabilityProviderAgentId(
|
||||
surface,
|
||||
hint: "Pass --agent <id> or set agents.defaults.systemAgent.agentId.",
|
||||
});
|
||||
if (!listAgentIds(cfg).includes(agentId)) {
|
||||
throw new Error(
|
||||
`Unknown agent id "${agentId}". Run \`openclaw agents list\` to see configured agents.`,
|
||||
);
|
||||
}
|
||||
return agentId;
|
||||
return resolveConfiguredAgentId(cfg, agentId);
|
||||
}
|
||||
|
||||
export function resolveCapabilityAgentOption(
|
||||
|
||||
@@ -16,6 +16,7 @@ const mocks = vi.hoisted(() => ({
|
||||
requestExitAfterOneShotOutput: vi.fn(),
|
||||
listAgentIds: vi.fn(),
|
||||
resolveAgentWorkspaceDir: vi.fn(),
|
||||
resolveConfiguredAgentId: vi.fn(),
|
||||
resolveDefaultAgentId: vi.fn(),
|
||||
tryResolveLegacyCompatibilityAgentId: vi.fn(),
|
||||
}));
|
||||
@@ -30,6 +31,7 @@ vi.mock("../state/config-machine-state.js", () => ({
|
||||
vi.mock("../agents/agent-scope.js", () => ({
|
||||
listAgentIds: mocks.listAgentIds,
|
||||
resolveAgentWorkspaceDir: mocks.resolveAgentWorkspaceDir,
|
||||
resolveConfiguredAgentId: mocks.resolveConfiguredAgentId,
|
||||
resolveDefaultAgentId: mocks.resolveDefaultAgentId,
|
||||
tryResolveLegacyCompatibilityAgentId: mocks.tryResolveLegacyCompatibilityAgentId,
|
||||
}));
|
||||
@@ -163,6 +165,14 @@ describe("hooks CLI metadata config keys", () => {
|
||||
mocks.buildWorkspaceHookStatus.mockReturnValue(report);
|
||||
mocks.getRuntimeConfig.mockReturnValue(sourceConfig);
|
||||
mocks.listAgentIds.mockReturnValue(["main"]);
|
||||
mocks.resolveConfiguredAgentId.mockImplementation(
|
||||
(_config: OpenClawConfig, agentId: string) => {
|
||||
if (!mocks.listAgentIds().includes(agentId)) {
|
||||
throw new Error(`Unknown agent id "${agentId}"`);
|
||||
}
|
||||
return agentId;
|
||||
},
|
||||
);
|
||||
mocks.resolveAgentWorkspaceDir.mockReturnValue("/tmp/openclaw-hook-workspace");
|
||||
mocks.resolveDefaultAgentId.mockReturnValue("main");
|
||||
mocks.tryResolveLegacyCompatibilityAgentId.mockReturnValue("main");
|
||||
|
||||
@@ -11,8 +11,8 @@ import { formatDocsLink } from "../../packages/terminal-core/src/links.js";
|
||||
import { getTerminalTableWidth, renderTable } from "../../packages/terminal-core/src/table.js";
|
||||
import { theme } from "../../packages/terminal-core/src/theme.js";
|
||||
import {
|
||||
listAgentIds,
|
||||
resolveAgentWorkspaceDir,
|
||||
resolveConfiguredAgentId,
|
||||
resolveDefaultAgentId,
|
||||
tryResolveLegacyCompatibilityAgentId,
|
||||
} from "../agents/agent-scope.js";
|
||||
@@ -79,10 +79,8 @@ type HooksReportTarget = {
|
||||
function resolveHooksReportTarget(config: OpenClawConfig, rawAgentId?: string): HooksReportTarget {
|
||||
const requested = rawAgentId?.trim();
|
||||
const requestedAgentId = requested ? normalizeAgentId(requested) : undefined;
|
||||
if (requestedAgentId && !listAgentIds(config).includes(requestedAgentId)) {
|
||||
throw new Error(
|
||||
`Unknown agent id "${requested}". Run ${formatCliCommand("openclaw agents list")} to see configured agents.`,
|
||||
);
|
||||
if (requestedAgentId) {
|
||||
resolveConfiguredAgentId(config, requestedAgentId);
|
||||
}
|
||||
const agentId =
|
||||
requestedAgentId ??
|
||||
|
||||
@@ -2,8 +2,7 @@
|
||||
import path from "node:path";
|
||||
import { isValidAgentId, normalizeAgentId } from "@openclaw/normalization-core/agent-id";
|
||||
import { timestampMsToIsoFileStamp } from "@openclaw/normalization-core/number-coercion";
|
||||
import { listAgentIds } from "../../agents/agent-scope.js";
|
||||
import { formatCliCommand } from "../../cli/command-format.js";
|
||||
import { resolveConfiguredAgentId } from "../../agents/agent-scope-config.js";
|
||||
import { getRuntimeConfig } from "../../config/config.js";
|
||||
import { resolveStateDir } from "../../config/paths.js";
|
||||
import type { OpenClawConfig } from "../../config/types.openclaw.js";
|
||||
@@ -48,13 +47,7 @@ export function resolveMigrationTargetAgentId(
|
||||
throw new Error(`Invalid agent id "${raw}".`);
|
||||
}
|
||||
const agentId = normalizeAgentId(raw);
|
||||
const knownAgentIds = new Set(listAgentIds(config).map(normalizeAgentId));
|
||||
if (!knownAgentIds.has(agentId)) {
|
||||
throw new Error(
|
||||
`Unknown agent id "${raw}". Use "${formatCliCommand("openclaw agents list")}" to see configured agents.`,
|
||||
);
|
||||
}
|
||||
return agentId;
|
||||
return resolveConfiguredAgentId(config, agentId);
|
||||
}
|
||||
|
||||
/** Builds the provider-facing migration context from CLI options and runtime state. */
|
||||
|
||||
@@ -2,7 +2,10 @@
|
||||
// Heavy modules stay lazily loaded so fast status output avoids security/provider/gateway costs.
|
||||
|
||||
import type { Result } from "@openclaw/normalization-core/result";
|
||||
import { listAgentIds, resolveAmbientOwnerAgentId } from "../agents/agent-scope-config.js";
|
||||
import {
|
||||
resolveAmbientOwnerAgentId,
|
||||
resolveConfiguredAgentId,
|
||||
} from "../agents/agent-scope-config.js";
|
||||
import { resolveAgentDir } from "../agents/agent-scope.js";
|
||||
import { resolveAgentHarnessPolicy } from "../agents/harness/policy.js";
|
||||
import { resolveModelAuthLabel } from "../agents/model-auth-label.js";
|
||||
@@ -126,10 +129,8 @@ export async function resolveStatusUsageSummary(params: StatusUsageSummaryOption
|
||||
throw new Error("--agent must not be blank");
|
||||
}
|
||||
const agentId = rawAgentId ? normalizeAgentId(rawAgentId) : undefined;
|
||||
if (agentId && !listAgentIds(params.config).includes(agentId)) {
|
||||
throw new Error(
|
||||
`Unknown agent id "${agentId}". Run \`openclaw agents list\` to see configured agents.`,
|
||||
);
|
||||
if (agentId) {
|
||||
resolveConfiguredAgentId(params.config, agentId);
|
||||
}
|
||||
let resolvedAgentId = agentId;
|
||||
let agentDir = params.agentDir;
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
// Session store target discovery maps configured and on-disk agent stores to canonical targets.
|
||||
import fsSync from "node:fs";
|
||||
import path from "node:path";
|
||||
import { resolveConfiguredAgentId } from "../../agents/agent-scope-config.js";
|
||||
import { listAgentEntries, listAgentIds, resolveDefaultAgentId } from "../../agents/agent-scope.js";
|
||||
import { resolveAgentSessionDirsFromAgentsDirSync } from "../../agents/session-dirs.js";
|
||||
import { normalizeAgentId, parseAgentSessionKey } from "../../routing/session-key.js";
|
||||
@@ -655,11 +656,8 @@ export function resolveSessionStoreTargets(
|
||||
// offers --agent/--all-agents instead of the ambient owner chain.
|
||||
tryResolveLegacyCompatibilityAgentId(cfg) ??
|
||||
resolveDefaultAgentId(cfg);
|
||||
const knownAgentIds = new Set(listAgentIds(cfg).map(normalizeAgentId));
|
||||
if (hasAgent && !knownAgentIds.has(defaultAgentId)) {
|
||||
throw new Error(
|
||||
`Unknown agent id "${opts.agent}". Use "openclaw agents list" to see configured agents.`,
|
||||
);
|
||||
if (hasAgent) {
|
||||
resolveConfiguredAgentId(cfg, defaultAgentId);
|
||||
}
|
||||
const target = resolveExplicitSessionStoreTarget({ defaultAgentId, env, store: opts.store });
|
||||
if (
|
||||
@@ -689,13 +687,8 @@ export function resolveSessionStoreTargets(
|
||||
}
|
||||
|
||||
if (hasAgent) {
|
||||
const knownAgents = listAgentIds(cfg);
|
||||
const requested = normalizeAgentId(opts.agent ?? "");
|
||||
if (!knownAgents.includes(requested)) {
|
||||
throw new Error(
|
||||
`Unknown agent id "${opts.agent}". Use "openclaw agents list" to see configured agents.`,
|
||||
);
|
||||
}
|
||||
resolveConfiguredAgentId(cfg, requested);
|
||||
return [
|
||||
{
|
||||
agentId: requested,
|
||||
|
||||
@@ -13,6 +13,7 @@ export type { AnyAgentTool } from "../agents/tools/common.js";
|
||||
export { resolveCronStyleNow } from "../agents/current-time.js";
|
||||
export {
|
||||
listAgentIds,
|
||||
resolveConfiguredAgentId,
|
||||
resolveDefaultAgentId,
|
||||
resolveSessionAgentIds,
|
||||
} from "../agents/agent-scope.js";
|
||||
|
||||
Reference in New Issue
Block a user