From 75fcb1fbb9aa84017e0ce4103ee95465c8fb74a5 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Thu, 20 Aug 2026 00:32:59 -0700 Subject: [PATCH] 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- 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. --- .../memory-core/src/cli-runtime-common.ts | 17 ++-- extensions/memory-core/src/cli.test.ts | 94 +++++++++++++++++++ src/agents/agent-scope-config.test.ts | 28 ++++++ src/agents/agent-scope-config.ts | 13 +++ src/agents/agent-scope.ts | 1 + src/cli/capability-cli/shared.ts | 12 +-- src/cli/hooks-cli.toggle.test.ts | 10 ++ src/cli/hooks-cli.ts | 8 +- src/commands/migrate/context.ts | 11 +-- src/commands/status-runtime-shared.ts | 11 ++- src/config/sessions/targets.ts | 15 +-- .../memory-core-host-runtime-core.ts | 1 + 12 files changed, 177 insertions(+), 44 deletions(-) diff --git a/extensions/memory-core/src/cli-runtime-common.ts b/extensions/memory-core/src/cli-runtime-common.ts index c0313dc83892..e1c9bf323b89 100644 --- a/extensions/memory-core/src/cli-runtime-common.ts +++ b/extensions/memory-core/src/cli-runtime-common.ts @@ -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) => { diff --git a/extensions/memory-core/src/cli.test.ts b/extensions/memory-core/src/cli.test.ts index 2ec582947f89..ed18abba760a 100644 --- a/extensions/memory-core/src/cli.test.ts +++ b/extensions/memory-core/src/cli.test.ts @@ -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: { diff --git a/src/agents/agent-scope-config.test.ts b/src/agents/agent-scope-config.test.ts index 52668b13c54f..9e735d4b2eec 100644 --- a/src/agents/agent-scope-config.test.ts +++ b/src/agents/agent-scope-config.test.ts @@ -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([]); diff --git a/src/agents/agent-scope-config.ts b/src/agents/agent-scope-config.ts index 133f7a814016..547f1a69a2d0 100644 --- a/src/agents/agent-scope-config.ts +++ b/src/agents/agent-scope-config.ts @@ -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) { diff --git a/src/agents/agent-scope.ts b/src/agents/agent-scope.ts index 411f13081436..ec21b124e647 100644 --- a/src/agents/agent-scope.ts +++ b/src/agents/agent-scope.ts @@ -39,6 +39,7 @@ export { listAgentEntries, listAgentEntriesWithSource, listAgentIds, + resolveConfiguredAgentId, resolveMutableAgentEntry, toAgentEntriesRecord, resolveAgentConfig, diff --git a/src/cli/capability-cli/shared.ts b/src/cli/capability-cli/shared.ts index 3ede3a1f3fdc..2d6b5edb3d38 100644 --- a/src/cli/capability-cli/shared.ts +++ b/src/cli/capability-cli/shared.ts @@ -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 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( diff --git a/src/cli/hooks-cli.toggle.test.ts b/src/cli/hooks-cli.toggle.test.ts index ee4a107ebe41..6a506c5f2c83 100644 --- a/src/cli/hooks-cli.toggle.test.ts +++ b/src/cli/hooks-cli.toggle.test.ts @@ -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"); diff --git a/src/cli/hooks-cli.ts b/src/cli/hooks-cli.ts index 099d94a66926..6f89257571bc 100644 --- a/src/cli/hooks-cli.ts +++ b/src/cli/hooks-cli.ts @@ -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 ?? diff --git a/src/commands/migrate/context.ts b/src/commands/migrate/context.ts index ed1d19e6576e..16579ef57a14 100644 --- a/src/commands/migrate/context.ts +++ b/src/commands/migrate/context.ts @@ -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. */ diff --git a/src/commands/status-runtime-shared.ts b/src/commands/status-runtime-shared.ts index 8205c39e0be2..e6c64f13244e 100644 --- a/src/commands/status-runtime-shared.ts +++ b/src/commands/status-runtime-shared.ts @@ -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; diff --git a/src/config/sessions/targets.ts b/src/config/sessions/targets.ts index 6d78d125f561..36f144347211 100644 --- a/src/config/sessions/targets.ts +++ b/src/config/sessions/targets.ts @@ -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, diff --git a/src/plugin-sdk/memory-core-host-runtime-core.ts b/src/plugin-sdk/memory-core-host-runtime-core.ts index a8b6e9796471..b744e3c09b1a 100644 --- a/src/plugin-sdk/memory-core-host-runtime-core.ts +++ b/src/plugin-sdk/memory-core-host-runtime-core.ts @@ -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";