diff --git a/config/assertion-safety-baseline.txt b/config/assertion-safety-baseline.txt index 44620008d14b..de4833df9dc6 100644 --- a/config/assertion-safety-baseline.txt +++ b/config/assertion-safety-baseline.txt @@ -3858,7 +3858,6 @@ src/snapshot/manifest.ts 1 src/state/agent-deletion-journal.ts 15 src/state/backup-run-records.ts 1 src/state/config-machine-state.ts 2 -src/state/control-ui-device-auth-migration.ts 1 src/state/local-onboarding-state.ts 1 src/state/openclaw-agent-board-schema.ts 1 src/state/openclaw-agent-db-migration-required.ts 1 @@ -4150,7 +4149,7 @@ ui/src/lib/cron/index.ts 16 ui/src/lib/gateway-diagnostics.ts 3 ui/src/lib/gateway-errors.ts 1 ui/src/lib/keyboard-shortcuts.ts 1 -ui/src/lib/nodes/index.ts 4 +ui/src/lib/nodes/index.ts 3 ui/src/lib/nodes/inventory.ts 1 ui/src/lib/plugin-activation.ts 3 ui/src/lib/session-pull-requests.ts 2 diff --git a/docs/plugins/sdk-subpaths.md b/docs/plugins/sdk-subpaths.md index 39bacbf8307e..1200b7160e77 100644 --- a/docs/plugins/sdk-subpaths.md +++ b/docs/plugins/sdk-subpaths.md @@ -253,7 +253,7 @@ Use `isLoopbackHost(host)` when a plugin must accept only the local machine. It | `plugin-sdk/cron-store-runtime` | Private-local after July 2026; Cron store path/load/save helpers | | `plugin-sdk/state-paths` | State/OAuth dir path helpers | | `plugin-sdk/plugin-state-runtime` | Private-local after July 2026; Plugin-scoped keyed-state and BLOB contracts plus connection pragma, verified WAL maintenance, and atomic STRICT-schema migration helpers. Plugin-state leases were removed; use SQLite transactions and keyed stores instead | - | `plugin-sdk/routing` | Route/session-key/account binding helpers such as `resolveAgentRoute`, `buildAgentSessionKey`, and `resolveDefaultAgentBoundAccountId` | + | `plugin-sdk/routing` | Route/session-key/account binding helpers such as `resolveAgentRoute`, `buildAgentSessionKey`, and `resolveDefaultAgentBoundAccountId`. Use `normalizeAgentId` when omitted input should resolve to `main`; use the Result-returning `normalizeAgentIdStrict` for an explicitly supplied ID that must not fall back to the default agent. | | `plugin-sdk/status-helpers` | Shared channel/account status summary helpers, runtime-state defaults, and issue metadata helpers | | `plugin-sdk/target-resolver-runtime` | Private-local after July 2026; Shared target resolver helpers | | `plugin-sdk/string-normalization-runtime` | Private-local after July 2026; Slug/string normalization helpers | diff --git a/packages/normalization-core/src/agent-id.test.ts b/packages/normalization-core/src/agent-id.test.ts index e94af18454ac..6e977961863f 100644 --- a/packages/normalization-core/src/agent-id.test.ts +++ b/packages/normalization-core/src/agent-id.test.ts @@ -1,4 +1,8 @@ -import { isValidAgentId, normalizeAgentId } from "@openclaw/normalization-core/agent-id"; +import { + isValidAgentId, + normalizeAgentId, + normalizeAgentIdStrict, +} from "@openclaw/normalization-core/agent-id"; import { describe, expect, it } from "vitest"; describe("normalization-core/agent-id", () => { @@ -21,4 +25,16 @@ describe("normalization-core/agent-id", () => { ])("validates %j", (input, expected) => { expect(isValidAgentId(input)).toBe(expected); }); + + it.each([ + ["", { ok: false, error: "unrepresentable" }], + [" ", { ok: false, error: "unrepresentable" }], + ["агент✨", { ok: false, error: "unrepresentable" }], + ["---", { ok: false, error: "unrepresentable" }], + ["valid-id", { ok: true, value: "valid-id" }], + ["../../etc/evil", { ok: true, value: "etc-evil" }], + ["a".repeat(65), { ok: true, value: "a".repeat(64) }], + ])("strictly normalizes %j", (input, expected) => { + expect(normalizeAgentIdStrict(input)).toEqual(expected); + }); }); diff --git a/packages/normalization-core/src/agent-id.ts b/packages/normalization-core/src/agent-id.ts index b32b3db6c66e..a6cb5f58c81b 100644 --- a/packages/normalization-core/src/agent-id.ts +++ b/packages/normalization-core/src/agent-id.ts @@ -1,3 +1,4 @@ +import { err, ok, type Result } from "./result.js"; import { normalizeLowercaseStringOrEmpty } from "./string-coerce.js"; const DEFAULT_AGENT_ID = "main"; @@ -8,21 +9,25 @@ const TRAILING_DASH_RE = /-+$/; /** Normalizes an OpenClaw agent id to its filesystem-safe canonical form. */ export function normalizeAgentId(value: string | undefined | null): string { + const result = normalizeAgentIdStrict(value); + return result.ok ? result.value : DEFAULT_AGENT_ID; +} + +/** Normalizes an explicitly supplied agent id without falling back to the default agent. */ +export function normalizeAgentIdStrict( + value: string | undefined | null, +): Result { const trimmed = (value ?? "").trim(); - if (!trimmed) { - return DEFAULT_AGENT_ID; - } const normalized = normalizeLowercaseStringOrEmpty(trimmed); if (VALID_ID_RE.test(trimmed)) { - return normalized; + return ok(normalized); } - return ( - normalized - .replace(INVALID_CHARS_RE, "-") - .replace(LEADING_DASH_RE, "") - .replace(TRAILING_DASH_RE, "") - .slice(0, 64) || DEFAULT_AGENT_ID - ); + const agentId = normalized + .replace(INVALID_CHARS_RE, "-") + .replace(LEADING_DASH_RE, "") + .replace(TRAILING_DASH_RE, "") + .slice(0, 64); + return agentId ? ok(agentId) : err("unrepresentable"); } /** Returns whether a value is already a canonical agent-id input. */ diff --git a/scripts/check-coercion-helper-declarations.mts b/scripts/check-coercion-helper-declarations.mts index 76c7dfdee3ab..f407fb80553c 100644 --- a/scripts/check-coercion-helper-declarations.mts +++ b/scripts/check-coercion-helper-declarations.mts @@ -33,7 +33,7 @@ export const CANONICAL_COERCION_HELPER_OWNERS = [ { file: "packages/normalization-core/src/agent-id.ts", kind: "function", - names: ["isValidAgentId", "normalizeAgentId"], + names: ["isValidAgentId", "normalizeAgentId", "normalizeAgentIdStrict"], }, { file: "packages/normalization-core/src/string-coerce.ts", diff --git a/scripts/plugin-sdk-surface-report.mts b/scripts/plugin-sdk-surface-report.mts index ad246e490b25..8334e6a0de36 100644 --- a/scripts/plugin-sdk-surface-report.mts +++ b/scripts/plugin-sdk-surface-report.mts @@ -287,7 +287,8 @@ export function readPluginSdkSurfaceBudgets(env: NodeJS.ProcessEnv = process.env // -17: retire the messaging-targets subpath, embedded Pi aliases, and shipped // channel setup compatibility helpers. // +1: concrete plugin side-effect owner resolution for agent harness runtimes. - 4317, + // +1: strict explicit agent-id normalization without default-agent fallback. + 4318, env, ), publicFunctionExports: readPluginSdkSurfaceBudgetEnv( @@ -362,7 +363,8 @@ export function readPluginSdkSurfaceBudgets(env: NodeJS.ProcessEnv = process.env // -1: remove the orphan diagnostic traceparent propagation export. // -12: retire the callable messaging-targets, embedded Pi, and channel setup helpers. // +1: concrete plugin side-effect owner resolution for agent harness runtimes. - 2565, + // +1: strict explicit agent-id normalization without default-agent fallback. + 2566, env, ), publicDeprecatedExports: readPluginSdkSurfaceBudgetEnv( diff --git a/src/agents/agent-create.ts b/src/agents/agent-create.ts index e409e574c37d..40e12525d6aa 100644 --- a/src/agents/agent-create.ts +++ b/src/agents/agent-create.ts @@ -18,7 +18,7 @@ import { resolveSessionTranscriptsDirForAgent } from "../config/sessions/paths.j import type { OptionalBootstrapFileName } from "../config/types.agent-defaults.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import { FsSafeError, root } from "../infra/fs-safe.js"; -import { normalizeAgentId } from "../routing/session-key.js"; +import { normalizeAgentId, normalizeAgentIdStrict } from "../routing/session-key.js"; import { readAgentDeletionJournal } from "../state/agent-deletion-journal.js"; import { isReservedSystemAgentId } from "../system-agent/agent-id.js"; import { resolveUserPath } from "../utils.js"; @@ -99,11 +99,6 @@ function createError( return { status: "error", reason, message, ...(agentId ? { agentId } : {}) }; } -/** True when raw user input contains a character that can survive agent-id normalization. */ -function hasValidRawAgentIdCharacters(value: string): boolean { - return /[a-z0-9]/iu.test(value); -} - export function validateAgentIdInput( rawId: string, options: { displayName?: string } = {}, @@ -111,14 +106,15 @@ export function validateAgentIdInput( | { ok: true; agentId: string } | { ok: false; reason: "invalid-name" | "reserved-id"; message: string; agentId?: string } { const displayName = options.displayName ?? rawId; - if (!hasValidRawAgentIdCharacters(rawId)) { + const normalized = normalizeAgentIdStrict(rawId); + if (!normalized.ok) { return { ok: false, reason: "invalid-name", - message: `agent name "${displayName}" has no valid id characters`, + message: `Agent name "${displayName}" has no valid id characters. Use at least one letter a-z or digit.`, }; } - const agentId = normalizeAgentId(rawId); + const agentId = normalized.value; if (isReservedSystemAgentId(agentId)) { return { ok: false, reason: "reserved-id", message: `"${agentId}" is reserved`, agentId }; } diff --git a/src/agents/subagents/spawn/acp-spawn-target.test.ts b/src/agents/subagents/spawn/acp-spawn-target.test.ts new file mode 100644 index 000000000000..d2783379af97 --- /dev/null +++ b/src/agents/subagents/spawn/acp-spawn-target.test.ts @@ -0,0 +1,24 @@ +import { describe, expect, it } from "vitest"; +import { resolveTargetAcpAgentId } from "./acp-spawn-target.js"; + +describe("resolveTargetAcpAgentId", () => { + it.each(["", " ", "агент✨", "---"])( + "rejects explicit unrepresentable ACP agent id %j", + (agentId) => { + expect( + resolveTargetAcpAgentId({ + requestedAgentId: agentId, + cfg: { acp: { defaultAgent: "codex" } }, + }), + ).toEqual({ ok: false, error: `agentId "${agentId}" was not found` }); + }, + ); + + it("keeps omitted ACP agent ids on the configured default path", () => { + expect( + resolveTargetAcpAgentId({ + cfg: { acp: { defaultAgent: "codex" } }, + }), + ).toEqual({ ok: true, agentId: "codex" }); + }); +}); diff --git a/src/agents/subagents/spawn/acp-spawn-target.ts b/src/agents/subagents/spawn/acp-spawn-target.ts index 6106d8b93c81..ce3acdcd5920 100644 --- a/src/agents/subagents/spawn/acp-spawn-target.ts +++ b/src/agents/subagents/spawn/acp-spawn-target.ts @@ -1,5 +1,5 @@ import type { OpenClawConfig } from "../../../config/types.openclaw.js"; -import { normalizeOptionalAgentId } from "../../../routing/session-key.js"; +import { normalizeAgentIdStrict, normalizeOptionalAgentId } from "../../../routing/session-key.js"; import { listAgentEntries } from "../../agent-scope-config.js"; import { listAgentIds } from "../../agent-scope.js"; @@ -7,7 +7,12 @@ export function resolveTargetAcpAgentId(params: { requestedAgentId?: string; cfg: OpenClawConfig; }): { ok: true; agentId: string; configAgentId?: string } | { ok: false; error: string } { - const requested = normalizeOptionalAgentId(params.requestedAgentId); + const normalizedRequest = + params.requestedAgentId === undefined ? null : normalizeAgentIdStrict(params.requestedAgentId); + if (normalizedRequest && !normalizedRequest.ok) { + return { ok: false, error: `agentId "${params.requestedAgentId}" was not found` }; + } + const requested = normalizedRequest?.value; if (requested) { const configuredAgent = listAgentEntries(params.cfg).find( (agent) => normalizeOptionalAgentId(agent.id) === requested, diff --git a/src/agents/tools/sessions-send-tool.ts b/src/agents/tools/sessions-send-tool.ts index 7352eccadeee..0f399177c4d1 100644 --- a/src/agents/tools/sessions-send-tool.ts +++ b/src/agents/tools/sessions-send-tool.ts @@ -34,6 +34,7 @@ import { isSubagentSessionKey, normalizeAccountId, normalizeAgentId, + normalizeAgentIdStrict, toAgentStoreSessionKey, } from "../../routing/session-key.js"; import { annotateInterSessionPromptText } from "../../sessions/input-provenance.js"; @@ -500,30 +501,38 @@ export function createSessionsSendTool(opts?: { const sessionKeyParam = readToolStringParam(params, "sessionKey"); const labelParam = normalizeOptionalString(readToolStringParam(params, "label")); - const labelAgentIdParam = normalizeOptionalString(readToolStringParam(params, "agentId")); + const labelAgentIdInput = readToolStringParam(params, "agentId"); + const normalizedLabelAgentId = + labelAgentIdInput === undefined ? null : normalizeAgentIdStrict(labelAgentIdInput); + if (normalizedLabelAgentId && !normalizedLabelAgentId.ok) { + return jsonResult({ + runId: crypto.randomUUID(), + status: "error", + error: `Agent "${labelAgentIdInput}" not found. Run openclaw agents list to see configured agents.`, + }); + } + const explicitTargetAgentId = normalizedLabelAgentId?.value; let sessionKey = sessionKeyParam; let resolvedTargetAgentId: string | undefined; let resolvedLabelKey: string | undefined; - if (!sessionKey && !labelParam && labelAgentIdParam) { + if (!sessionKey && !labelParam && explicitTargetAgentId) { const agentMainKey = resolveConfiguredAgentMainSessionKey({ cfg, - agentId: labelAgentIdParam, + agentId: explicitTargetAgentId, mainKey, }); if (!agentMainKey) { return jsonResult({ runId: crypto.randomUUID(), status: "error", - error: `agent not found: ${labelAgentIdParam}`, + error: `Agent "${labelAgentIdInput}" not found. Run openclaw agents list to see configured agents.`, }); } sessionKey = agentMainKey; } if (!sessionKey && labelParam) { - const requestedAgentId = labelAgentIdParam - ? normalizeAgentId(labelAgentIdParam) - : undefined; + const requestedAgentId = explicitTargetAgentId; if (restrictToSpawned && requestedAgentId && requestedAgentId !== requesterAgentId) { return jsonResult({ @@ -700,7 +709,7 @@ export function createSessionsSendTool(opts?: { const resolvedTargetOwner = visibleSession.agentId ?? resolvedTargetAgentId ?? - (labelParam && labelAgentIdParam ? normalizeAgentId(labelAgentIdParam) : undefined); + (labelParam ? explicitTargetAgentId : undefined); if ( persistedTargetOwner.kind === "configured" && resolvedTargetOwner && diff --git a/src/agents/tools/sessions.test.ts b/src/agents/tools/sessions.test.ts index 57edaf40040e..b042e27b1f7d 100644 --- a/src/agents/tools/sessions.test.ts +++ b/src/agents/tools/sessions.test.ts @@ -1036,6 +1036,22 @@ describe("sessions_send gating", () => { expect(requireGatewayRequest().method).toBe("sessions.resolve"); }); + it("rejects an unrepresentable agent id before resolving a main session", async () => { + const tool = createMainSessionsSendTool(); + + const result = await tool.execute("call-invalid-agent", { + agentId: "агент✨", + message: "hello", + timeoutSeconds: 5, + }); + + expect(requireDetails(result)).toMatchObject({ + status: "error", + error: 'Agent "агент✨" not found. Run openclaw agents list to see configured agents.', + }); + expect(callGatewayMock).not.toHaveBeenCalled(); + }); + it("conceals missing explicit keys denied by session visibility", async () => { callGatewayMock.mockRejectedValueOnce(new Error("No session found: agent:main:missing")); const tool = createSessionsSendTool({ diff --git a/src/commands/agents.add.test.ts b/src/commands/agents.add.test.ts index 1241f9f2a8b1..eea34fb799f1 100644 --- a/src/commands/agents.add.test.ts +++ b/src/commands/agents.add.test.ts @@ -91,7 +91,10 @@ vi.mock("../config/config.js", async () => ({ replaceConfigFile: replaceConfigFileMock, })); -vi.mock("../agents/agent-create.js", () => ({ +vi.mock("../agents/agent-create.js", async () => ({ + ...(await vi.importActual( + "../agents/agent-create.js", + )), checkAgentCreationGate: checkAgentCreationGateMock, createAgent: createAgentMock, })); @@ -249,6 +252,33 @@ describe("agents add command", () => { }, ); + it("rejects an unrepresentable positional name before targeting an existing agent", async () => { + readConfigFileSnapshotMock.mockResolvedValue({ + ...baseConfigSnapshot, + config: { agents: { entries: { main: {} } } }, + sourceConfig: { agents: { entries: { main: {} } } }, + }); + const prompter = { + intro: vi.fn(), + text: vi.fn(), + confirm: vi.fn(), + note: vi.fn(), + outro: vi.fn(), + }; + wizardMocks.createClackPrompter.mockReturnValue(prompter); + + await agentsAddCommand({ name: "агент✨" }, runtime); + + expect(prompter.outro).toHaveBeenCalledWith( + 'Agent name "агент✨" has no valid id characters. Use at least one letter a-z or digit.', + ); + expect(prompter.confirm).not.toHaveBeenCalled(); + expect(prompter.note).not.toHaveBeenCalled(); + expect(checkAgentCreationGateMock).not.toHaveBeenCalled(); + expect(createAgentMock).not.toHaveBeenCalled(); + expect(writeConfigFileMock).not.toHaveBeenCalled(); + }); + it.each(RESERVED_SYSTEM_AGENT_IDS_FOR_TEST)( "rejects reserved system-agent id %s from an interactive positional argument", async (name) => { diff --git a/src/commands/agents.bind.commands.test.ts b/src/commands/agents.bind.commands.test.ts index 1cb35f9582aa..49ad506cba26 100644 --- a/src/commands/agents.bind.commands.test.ts +++ b/src/commands/agents.bind.commands.test.ts @@ -203,6 +203,24 @@ describe("agents bind/unbind commands", () => { expect(runtime.exit).not.toHaveBeenCalled(); }); + it.each(["агент✨", " "])( + "rejects an explicit unrepresentable agent %j instead of binding the default", + async (agent) => { + readConfigFileSnapshotMock.mockResolvedValue({ + ...baseConfigSnapshot, + config: {}, + }); + + await agentsBindCommand({ agent, bind: ["telegram"] }, runtime); + + expect(runtime.error).toHaveBeenCalledWith( + `Agent "${agent}" not found. Run openclaw agents list to see configured agents.`, + ); + expect(runtime.exit).toHaveBeenCalledWith(1); + expect(writeConfigFileMock).not.toHaveBeenCalled(); + }, + ); + it("uses a wildcard account binding for multi-account channels", async () => { readConfigFileSnapshotMock.mockResolvedValue({ ...baseConfigSnapshot, diff --git a/src/commands/agents.commands.add.ts b/src/commands/agents.commands.add.ts index 013000080c27..2b42d2def68b 100644 --- a/src/commands/agents.commands.add.ts +++ b/src/commands/agents.commands.add.ts @@ -5,7 +5,11 @@ import { normalizeLowercaseStringOrEmpty, normalizeOptionalString, } from "@openclaw/normalization-core/string-coerce"; -import { checkAgentCreationGate, createAgent } from "../agents/agent-create.js"; +import { + checkAgentCreationGate, + createAgent, + validateAgentIdInput, +} from "../agents/agent-create.js"; import { resolveAgentDir, resolveAgentWorkspaceDir, @@ -37,7 +41,6 @@ import { withPluginLifecycleLease } from "../plugins/plugin-lifecycle-lease.js"; import { normalizeAgentId } from "../routing/session-key.js"; import { type RuntimeEnv, writeRuntimeJson } from "../runtime.js"; import { defaultRuntime } from "../runtime.js"; -import { isReservedSystemAgentId } from "../system-agent/agent-id.js"; import { resolveUserPath, shortenHomePath } from "../utils.js"; import { createClackPrompter } from "../wizard/clack-prompter.js"; import { WizardCancelledError } from "../wizard/prompts.js"; @@ -123,7 +126,17 @@ export async function agentsAddCommand( runtime.exit(1); return; } - const agentId = normalizeAgentId(nameInput); + const validation = validateAgentIdInput(nameInput); + if (!validation.ok) { + runtime.error( + validation.reason === "reserved-id" + ? `"${validation.agentId}" is reserved. Choose another name, or run ${formatCliCommand("openclaw agents list")} to inspect configured agents.` + : validation.message, + ); + runtime.exit(1); + return; + } + const agentId = validation.agentId; if (agentId !== nameInput) { runtime.log(`Normalized agent id to "${agentId}".`); } @@ -205,20 +218,27 @@ export async function agentsAddCommand( if (!value?.trim()) { return "Required"; } - const normalized = normalizeAgentId(value); - if (isReservedSystemAgentId(normalized)) { - return `"${normalized}" is reserved. Choose another name.`; + const validation = validateAgentIdInput(value); + if (!validation.ok) { + return validation.reason === "reserved-id" + ? `"${validation.agentId}" is reserved. Choose another name.` + : validation.message; } return undefined; }, })); const agentName = normalizeOptionalString(name) ?? ""; - const agentId = normalizeAgentId(agentName); - if (isReservedSystemAgentId(agentId)) { - await prompter.outro(`"${agentId}" is reserved. Choose another name.`); + const validation = validateAgentIdInput(agentName); + if (!validation.ok) { + if (validation.reason === "reserved-id") { + await prompter.outro(`"${validation.agentId}" is reserved. Choose another name.`); + return; + } + await prompter.outro(validation.message); return; } + const agentId = validation.agentId; if (agentName !== agentId) { await prompter.note(`Normalized id to "${agentId}".`, "Agent id"); } diff --git a/src/commands/agents.commands.bind.ts b/src/commands/agents.commands.bind.ts index a7fee3587dd9..9790fb8acc4f 100644 --- a/src/commands/agents.commands.bind.ts +++ b/src/commands/agents.commands.bind.ts @@ -6,7 +6,7 @@ import { isRouteBinding, listRouteBindings } from "../config/bindings.js"; import { replaceConfigFile } from "../config/config.js"; import { logConfigUpdated } from "../config/logging.js"; import type { AgentRouteBinding } from "../config/types.js"; -import { normalizeAgentId } from "../routing/session-key.js"; +import { normalizeAgentId, normalizeAgentIdStrict } from "../routing/session-key.js"; import { type RuntimeEnv, writeRuntimeJson } from "../runtime.js"; import { defaultRuntime } from "../runtime.js"; import { createLazyImportLoader } from "../shared/lazy-promise.js"; @@ -41,23 +41,6 @@ function loadAgentBindingsModule(): Promise { return agentBindingsModuleLoader.load(); } -function resolveAgentId( - cfg: Awaited>, - agentInput: string | undefined, - params?: { fallbackToDefault?: boolean }, -): string | null { - if (!cfg) { - return null; - } - if (agentInput?.trim()) { - return normalizeAgentId(agentInput); - } - if (params?.fallbackToDefault) { - return resolveDefaultAgentId(cfg); - } - return null; -} - function hasAgent(cfg: Awaited>, agentId: string): boolean { if (!cfg) { return false; @@ -75,20 +58,20 @@ function formatBindingOwnerLine(binding: AgentRouteBinding): string { } function resolveTargetAgentIdOrExit(params: { - cfg: Awaited>; + cfg: NonNullable>>; runtime: RuntimeEnv; agentInput: string | undefined; }): string | null { - const agentId = resolveAgentId(params.cfg, params.agentInput?.trim(), { - fallbackToDefault: true, - }); - if (!agentId) { + const normalized = + params.agentInput === undefined ? null : normalizeAgentIdStrict(params.agentInput); + if (normalized && !normalized.ok) { params.runtime.error( - `Unable to resolve agent id. Run ${formatCliCommand("openclaw agents list")} to choose one.`, + `Agent "${params.agentInput}" not found. Run ${formatCliCommand("openclaw agents list")} to see configured agents.`, ); params.runtime.exit(1); return null; } + const agentId = normalized?.value ?? resolveDefaultAgentId(params.cfg); if (!hasAgent(params.cfg, agentId)) { params.runtime.error( `Agent "${agentId}" not found. Run ${formatCliCommand("openclaw agents list")} to see configured agents.`, @@ -184,14 +167,15 @@ export async function agentsBindingsCommand( return; } - const filterAgentId = resolveAgentId(cfg, opts.agent?.trim()); - if (opts.agent && !filterAgentId) { + const normalizedFilter = opts.agent === undefined ? null : normalizeAgentIdStrict(opts.agent); + if (normalizedFilter && !normalizedFilter.ok) { runtime.error( - `Agent id is required. Run ${formatCliCommand("openclaw agents list")} to choose one.`, + `Agent "${opts.agent}" not found. Run ${formatCliCommand("openclaw agents list")} to see configured agents.`, ); runtime.exit(1); return; } + const filterAgentId = normalizedFilter?.value; if (filterAgentId && !hasAgent(cfg, filterAgentId)) { runtime.error( `Agent "${filterAgentId}" not found. Run ${formatCliCommand("openclaw agents list")} to see configured agents.`, diff --git a/src/commands/agents.commands.delete.ts b/src/commands/agents.commands.delete.ts index 80c961ba332e..93fae9f185d3 100644 --- a/src/commands/agents.commands.delete.ts +++ b/src/commands/agents.commands.delete.ts @@ -39,7 +39,7 @@ import { isGatewayCredentialsRequiredError, isGatewayTransportError, } from "../gateway/call.js"; -import { normalizeAgentId } from "../routing/session-key.js"; +import { normalizeAgentId, normalizeAgentIdStrict } from "../routing/session-key.js"; import { type RuntimeEnv, writeRuntimeJson } from "../runtime.js"; import { defaultRuntime } from "../runtime.js"; import { readAgentDeletionJournal } from "../state/agent-deletion-journal.js"; @@ -65,6 +65,16 @@ type AgentsDeleteGatewayResult = { failed?: Array<{ path: string; reason: string }>; }; +function failAgentsDelete(opts: AgentsDeleteOptions, runtime: RuntimeEnv, message: string): void { + if (opts.json) { + writeRuntimeJson(runtime, { error: message }); + runtime.exit(1, { resetStream: process.stderr }); + } else { + runtime.error(message); + runtime.exit(1); + } +} + function logClearedOwnerRefs(runtime: RuntimeEnv, clearedOwnerRefs: readonly string[]): void { if (clearedOwnerRefs.length > 0) { runtime.log(`Cleared owner references: ${clearedOwnerRefs.join(", ")}`); @@ -108,24 +118,35 @@ export async function agentsDeleteCommand( const input = opts.id?.trim(); if (!input) { - runtime.error( + failAgentsDelete( + opts, + runtime, `Agent id is required. Run ${formatCliCommand("openclaw agents list")} to choose one.`, ); - runtime.exit(1); return; } - const agentId = normalizeAgentId(input); - if (agentId !== input) { + const normalized = normalizeAgentIdStrict(input); + if (!normalized.ok) { + failAgentsDelete( + opts, + runtime, + `Agent "${input}" not found. Run ${formatCliCommand("openclaw agents list")} to see configured agents.`, + ); + return; + } + const agentId = normalized.value; + if (!opts.json && agentId !== input) { runtime.log(`Normalized agent id to "${agentId}".`); } const configured = findAgentEntryIndex(listAgentEntries(cfg), agentId) >= 0; let existingJournal = configured ? undefined : readAgentDeletionJournal(agentId); if (!configured && (!existingJournal || existingJournal.cleanupCompleted)) { - runtime.error( + failAgentsDelete( + opts, + runtime, `Agent "${agentId}" not found. Run ${formatCliCommand("openclaw agents list")} to see configured agents.`, ); - runtime.exit(1); return; } const configuredAgentDir = configured ? resolveAgentDir(cfg, agentId) : undefined; @@ -141,14 +162,16 @@ export async function agentsDeleteCommand( sharedAuthDbPath: resolveSharedAuthStorePath(), }) ) { - runtime.error(formatSharedAuthStoreOwnerDeleteError(agentId)); - runtime.exit(1); + failAgentsDelete(opts, runtime, formatSharedAuthStoreOwnerDeleteError(agentId)); return; } if (configured && agentId === tryResolveSoleAgentId(cfg)) { - runtime.error(`Agent "${agentId}" is the only configured agent and cannot be deleted.`); - runtime.exit(1); + failAgentsDelete( + opts, + runtime, + `Agent "${agentId}" is the only configured agent and cannot be deleted.`, + ); return; } const explicitInheritedAuthAgentId = cfg.agents?.defaults?.authInheritance?.agentId?.trim(); @@ -156,10 +179,11 @@ export async function agentsDeleteCommand( explicitInheritedAuthAgentId || (sharedAuthOwnership.location === "legacy-main" ? resolveLegacyInheritedAuthAgentId(cfg) : ""); if (inheritedAuthAgentId && agentId === normalizeAgentId(inheritedAuthAgentId)) { - runtime.error( + failAgentsDelete( + opts, + runtime, `Agent "${agentId}" owns inherited credentials through agents.defaults.authInheritance.agentId and cannot be deleted. Relocate those credentials, then re-point or remove that binding before retrying.`, ); - runtime.exit(1); return; } diff --git a/src/commands/agents.commands.identity.ts b/src/commands/agents.commands.identity.ts index 24a9b851cbf1..c5cbd7068fd1 100644 --- a/src/commands/agents.commands.identity.ts +++ b/src/commands/agents.commands.identity.ts @@ -16,7 +16,7 @@ import { logConfigUpdated } from "../config/logging.js"; import type { AgentConfig, IdentityConfig } from "../config/types.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import { formatErrorMessage } from "../infra/errors.js"; -import { normalizeAgentId } from "../routing/session-key.js"; +import { normalizeAgentId, normalizeAgentIdStrict } from "../routing/session-key.js"; import { type RuntimeEnv, writeRuntimeJson } from "../runtime.js"; import { defaultRuntime } from "../runtime.js"; import { resolveUserPath, shortenHomePath } from "../utils.js"; @@ -71,7 +71,6 @@ export async function agentsSetIdentityCommand( ).config as OpenClawConfig; const baseHash = configSnapshot.hash; - const agentRaw = normalizeOptionalString(opts.agent); const nameRaw = normalizeOptionalString(opts.name); const emojiRaw = normalizeOptionalString(opts.emoji); const themeRaw = normalizeOptionalString(opts.theme); @@ -81,7 +80,13 @@ export async function agentsSetIdentityCommand( const identityFileRaw = normalizeOptionalString(opts.identityFile); const workspaceRaw = normalizeOptionalString(opts.workspace); const wantsIdentityFile = Boolean(opts.fromIdentity || identityFileRaw || !hasExplicitIdentity); - let agentId = agentRaw ? normalizeAgentId(agentRaw) : undefined; + const normalizedAgent = opts.agent === undefined ? null : normalizeAgentIdStrict(opts.agent); + if (normalizedAgent && !normalizedAgent.ok) { + runtime.error(`Agent "${opts.agent}" not found. Create it with \`openclaw agents add\`.`); + runtime.exit(1); + return; + } + let agentId = normalizedAgent?.value; let identityFilePath: string | undefined; let workspaceDir: string | undefined; diff --git a/src/commands/agents.delete.test.ts b/src/commands/agents.delete.test.ts index 8072da56ffe2..7b0865bbf366 100644 --- a/src/commands/agents.delete.test.ts +++ b/src/commands/agents.delete.test.ts @@ -237,10 +237,14 @@ describe("agents delete command", () => { expect(gatewayMocks.callGateway).not.toHaveBeenCalled(); expect(configMocks.replaceConfigFile).not.toHaveBeenCalled(); - expect(runtime.error).toHaveBeenCalledWith( - 'Agent "main" owns the legacy shared auth store and cannot be deleted. Run openclaw doctor --fix to migrate shared auth, then retry.', - ); - expect(runtime.exit).toHaveBeenCalledWith(1); + expect(runtime.error).not.toHaveBeenCalled(); + expect(readJsonLogs()).toEqual([ + { + error: + 'Agent "main" owns the legacy shared auth store and cannot be deleted. Run openclaw doctor --fix to migrate shared auth, then retry.', + }, + ]); + expect(runtime.exit).toHaveBeenCalledWith(1, { resetStream: process.stderr }); expectSessionStore(cfg, sessions, "main"); }); }); @@ -274,6 +278,36 @@ describe("agents delete command", () => { }); }); + it("rejects an unrepresentable id before targeting or deleting an agent", async () => { + await withStateDirEnv("openclaw-agents-delete-invalid-id-", async ({ stateDir }) => { + const cfg: OpenClawConfig = { + agents: { + list: [ + { id: "main", workspace: path.join(stateDir, "workspace-main") }, + { id: "second", default: true, workspace: path.join(stateDir, "workspace-second") }, + ], + }, + }; + const sessions = { + "agent:main:main": { sessionId: "sess-main", updatedAt: Date.now() }, + }; + writeConfigMachineState("auth.sharedStore", { location: "state-db" }); + await arrangeAgentsDeleteTest({ stateDir, cfg, deletedAgentId: "main", sessions }); + + await agentsDeleteCommand({ id: "агент✨", force: true }, runtime); + + expect(runtime.error).toHaveBeenCalledWith( + 'Agent "агент✨" not found. Run openclaw agents list to see configured agents.', + ); + expect(runtime.exit).toHaveBeenCalledWith(1); + expect(gatewayMocks.callGateway).not.toHaveBeenCalled(); + expect(configMocks.replaceConfigFile).not.toHaveBeenCalled(); + expect(fsSafeMocks.movePathToTrash).not.toHaveBeenCalled(); + expect(workspaceStateMocks.deleteWorkspaceState).not.toHaveBeenCalled(); + expectSessionStore(cfg, sessions, "main"); + }); + }); + it("refuses deleting the auth-inheritance owner until credentials are relocated", async () => { await withStateDirEnv("openclaw-agents-delete-auth-owner-", async ({ stateDir }) => { const cfg: OpenClawConfig = { @@ -609,10 +643,11 @@ describe("agents delete command", () => { await agentsDeleteCommand({ id: "ops", force: true, json: true }, runtime); - expect(runtime.error).toHaveBeenCalledWith( - 'Agent "ops" is the only configured agent and cannot be deleted.', - ); - expect(runtime.exit).toHaveBeenCalledWith(1); + expect(runtime.error).not.toHaveBeenCalled(); + expect(readJsonLogs()).toEqual([ + { error: 'Agent "ops" is the only configured agent and cannot be deleted.' }, + ]); + expect(runtime.exit).toHaveBeenCalledWith(1, { resetStream: process.stderr }); expectSessionStore(cfg, { "agent:main:main": { sessionId: "sess-default-alias", updatedAt: now + 1 }, "agent:ops:quietchat:direct:u1": { sessionId: "sess-ops-direct", updatedAt: now + 2 }, diff --git a/src/commands/agents.identity.test.ts b/src/commands/agents.identity.test.ts index 3338542b72ec..74d50fc39832 100644 --- a/src/commands/agents.identity.test.ts +++ b/src/commands/agents.identity.test.ts @@ -261,20 +261,23 @@ describe("agents set-identity command", () => { }); }); - it("errors without changing config when --agent names an unknown agent", async () => { - configMocks.readConfigFileSnapshot.mockResolvedValue({ - ...baseConfigSnapshot, - config: { agents: { entries: { main: {} } } }, - }); + it.each(["ghostzzz", "агент✨", " "])( + "errors without changing config when --agent names %j", + async (agent) => { + configMocks.readConfigFileSnapshot.mockResolvedValue({ + ...baseConfigSnapshot, + config: { agents: { entries: { main: {} } } }, + }); - await agentsSetIdentityCommand({ agent: "ghostzzz", name: "Ghost" }, runtime); + await agentsSetIdentityCommand({ agent, name: "Ghost" }, runtime); - expect(runtime.error).toHaveBeenCalledWith( - 'Agent "ghostzzz" not found. Create it with `openclaw agents add`.', - ); - expect(runtime.exit).toHaveBeenCalledWith(1); - expect(configMocks.writeConfigFile).not.toHaveBeenCalled(); - }); + expect(runtime.error).toHaveBeenCalledWith( + `Agent "${agent}" not found. Create it with \`openclaw agents add\`.`, + ); + expect(runtime.exit).toHaveBeenCalledWith(1); + expect(configMocks.writeConfigFile).not.toHaveBeenCalled(); + }, + ); it.each(["main", "openclaw", "crestodian"])( "does not create absent reserved agent %s", diff --git a/src/gateway/server-methods/agents-mutate.test.ts b/src/gateway/server-methods/agents-mutate.test.ts index 7dbe3249b1ed..62a8452eef91 100644 --- a/src/gateway/server-methods/agents-mutate.test.ts +++ b/src/gateway/server-methods/agents-mutate.test.ts @@ -3023,6 +3023,23 @@ describe("agents.delete", () => { expect(mocks.writeConfigFile).not.toHaveBeenCalled(); }); + it("rejects an unrepresentable id before targeting the main agent", async () => { + mocks.sharedAuthStoreOwnership = { location: "state-db" }; + mocks.loadConfigReturn = { + agents: { list: [{ id: "main" }, { id: "ops", default: true }] }, + }; + + const { respond, promise } = makeCall("agents.delete", { + agentId: "агент✨", + }); + await promise; + + expectRespondErrorContaining(respond, 'agent "агент✨" not found'); + expect(mocks.beginAgentDeletionCommit).not.toHaveBeenCalled(); + expect(mocks.writeConfigFile).not.toHaveBeenCalled(); + expect(mocks.movePathToTrash).not.toHaveBeenCalled(); + }); + it("deletes main through the normal journal path after shared auth relocation", async () => { mocks.sharedAuthStoreOwnership = { location: "state-db" }; mocks.loadConfigReturn = { diff --git a/src/gateway/server-methods/agents-workspace.test.ts b/src/gateway/server-methods/agents-workspace.test.ts index fd9a8b14b531..8191a0a39570 100644 --- a/src/gateway/server-methods/agents-workspace.test.ts +++ b/src/gateway/server-methods/agents-workspace.test.ts @@ -165,7 +165,7 @@ describe("agents.workspace RPC handlers", () => { const error = expectError( await invokeWorkspaceHandler("agents.workspace.list", { agentId: "ghost" }), ); - expect(error.message).toContain("unknown agent id"); + expect(error.message).toContain('agent "ghost" not found'); }); it("rejects traversal outside the workspace for list and get", async () => { diff --git a/src/gateway/server-methods/agents-workspace.ts b/src/gateway/server-methods/agents-workspace.ts index 27e52a235bb6..6f73505eea02 100644 --- a/src/gateway/server-methods/agents-workspace.ts +++ b/src/gateway/server-methods/agents-workspace.ts @@ -12,7 +12,7 @@ import { } from "../../../packages/gateway-protocol/src/index.js"; import { listAgentIds, resolveAgentWorkspaceDir } from "../../agents/agent-scope.js"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; -import { normalizeAgentId } from "../../routing/session-key.js"; +import { normalizeAgentIdStrict } from "../../routing/session-key.js"; import type { GatewayRequestHandlers, RespondFn } from "./types.js"; import { assertValidParams } from "./validation.js"; import { @@ -70,11 +70,16 @@ function resolveWorkspaceScopeOrRespond( cfg: OpenClawConfig, respond: RespondFn, ): { agentId: string; workspaceDir: string; browserPath: string } | null { - const agentId = normalizeAgentId(params.agentId); - if (!new Set(listAgentIds(cfg)).has(agentId)) { - respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "unknown agent id")); + const normalized = normalizeAgentIdStrict(params.agentId); + if (!normalized.ok || !new Set(listAgentIds(cfg)).has(normalized.value)) { + respond( + false, + undefined, + errorShape(ErrorCodes.INVALID_REQUEST, `agent "${params.agentId}" not found`), + ); return null; } + const agentId = normalized.value; const workspaceDir = resolveAgentWorkspaceDir(cfg, agentId); const rawPath = params.path ?? ""; const portablePath = rawPath.replaceAll("\\", "/"); diff --git a/src/gateway/server-methods/agents.ts b/src/gateway/server-methods/agents.ts index daa5bfe6ec88..b0785a8f7eae 100644 --- a/src/gateway/server-methods/agents.ts +++ b/src/gateway/server-methods/agents.ts @@ -88,7 +88,7 @@ import { root, FsSafeError, type ReadResult } from "../../infra/fs-safe.js"; import { isPathInside } from "../../infra/path-guards.js"; import { resolveSqliteDatabaseFilePaths } from "../../infra/sqlite-files.js"; import { movePathToTrash } from "../../plugin-sdk/browser-maintenance.js"; -import { normalizeAgentId } from "../../routing/session-key.js"; +import { normalizeAgentId, normalizeAgentIdStrict } from "../../routing/session-key.js"; import { readAgentDeletionJournal, type AgentDeletionJournalCleanupPath, @@ -170,7 +170,7 @@ function resolveAgentWorkspaceFileOrRespondError( cfg, ); if (!agentId) { - respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "unknown agent id")); + respondAgentNotFound(respond, String(rawAgentId)); return null; } const rawName = params.name; @@ -299,7 +299,11 @@ async function listAgentFiles(workspaceDir: string, options?: { hideBootstrap?: } function resolveAgentIdOrError(agentIdRaw: string, cfg: OpenClawConfig) { - const agentId = normalizeAgentId(agentIdRaw); + const normalized = normalizeAgentIdStrict(agentIdRaw); + if (!normalized.ok) { + return null; + } + const agentId = normalized.value; const allowed = new Set(listAgentIds(cfg)); if (!allowed.has(agentId)) { return null; @@ -935,7 +939,12 @@ export const agentsHandlers: GatewayRequestHandlers = { } const cfg = context.getRuntimeConfig(); - const agentId = normalizeAgentId(params.agentId); + const normalized = normalizeAgentIdStrict(params.agentId); + if (!normalized.ok) { + respondAgentNotFound(respond, params.agentId); + return; + } + const agentId = normalized.value; if (!isConfiguredAgent(cfg, agentId)) { respondAgentNotFound(respond, agentId); return; @@ -1029,7 +1038,12 @@ export const agentsHandlers: GatewayRequestHandlers = { } const cfg = context.getRuntimeConfig(); - const agentId = normalizeAgentId(params.agentId); + const normalized = normalizeAgentIdStrict(params.agentId); + if (!normalized.ok) { + respondAgentNotFound(respond, params.agentId); + return; + } + const agentId = normalized.value; if (agentOwnsSharedAuthStore(cfg, agentId)) { respond( false, @@ -1522,7 +1536,7 @@ export const agentsHandlers: GatewayRequestHandlers = { const cfg = context.getRuntimeConfig(); const agentId = resolveAgentIdOrError(params.agentId, cfg); if (!agentId) { - respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "unknown agent id")); + respondAgentNotFound(respond, params.agentId); return; } const workspaceDir = resolveAgentWorkspaceDir(cfg, agentId); diff --git a/src/gateway/server-methods/model-auth-agent-scope.ts b/src/gateway/server-methods/model-auth-agent-scope.ts index 23ff529741cf..834c6937084e 100644 --- a/src/gateway/server-methods/model-auth-agent-scope.ts +++ b/src/gateway/server-methods/model-auth-agent-scope.ts @@ -7,7 +7,7 @@ import { import { AgentSelectionRequiredError } from "../../agents/agent-scope-config.js"; import { listAgentIds, resolveAgentDir, resolveDefaultAgentId } from "../../agents/agent-scope.js"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; -import { normalizeAgentId } from "../../routing/session-key.js"; +import { normalizeAgentIdStrict } from "../../routing/session-key.js"; type ModelAuthAgentScopeResult = | { ok: true; agentId: string; agentDir: string } @@ -53,13 +53,11 @@ export function resolveModelAuthAgentScope( if (!rawAgentId) { return { ok: false, agentId: requestedAgentId }; } - const agentId = normalizeAgentId(rawAgentId); - // normalizeAgentId falls back to "main" when sanitization erases the entire - // input; explicit garbage must not inherit the default agent's credentials. - const collapsedToFallback = !/[A-Za-z0-9_]/u.test(rawAgentId); - if (collapsedToFallback || !listAgentIds(cfg).includes(agentId)) { + const normalized = normalizeAgentIdStrict(rawAgentId); + if (!normalized.ok || !listAgentIds(cfg).includes(normalized.value)) { return { ok: false, agentId: rawAgentId }; } + const agentId = normalized.value; return { ok: true, agentId, agentDir: resolveAgentDir(cfg, agentId) }; } diff --git a/src/gateway/server-methods/sessions-search-scope.ts b/src/gateway/server-methods/sessions-search-scope.ts index 18023661d0a2..dc878b6b93b7 100644 --- a/src/gateway/server-methods/sessions-search-scope.ts +++ b/src/gateway/server-methods/sessions-search-scope.ts @@ -6,7 +6,7 @@ import { import { isConfiguredSessionStoreAgentId } from "../../config/sessions.js"; import { resolvePersistedSessionStoreOwnerForKey } from "../../config/sessions/session-store-owner.js"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; -import { normalizeAgentId } from "../../routing/session-key.js"; +import { normalizeAgentIdStrict } from "../../routing/session-key.js"; import { resolveRequestedSessionAgentId } from "../session-request-agent.js"; import { resolveSessionStoreAgentId, @@ -15,7 +15,15 @@ import { } from "../session-store-key.js"; export function resolveSessionSearchScope(cfg: OpenClawConfig, params: SessionsSearchParams) { - const requestedAgentId = params.agentId ? normalizeAgentId(params.agentId) : undefined; + const normalizedRequest = + params.agentId === undefined ? null : normalizeAgentIdStrict(params.agentId); + if (normalizedRequest && !normalizedRequest.ok) { + return { + ok: false as const, + error: errorShape(ErrorCodes.INVALID_REQUEST, `Unknown agent id "${params.agentId}"`), + }; + } + const requestedAgentId = normalizedRequest?.value; const resolvedSessionKeys: | Array<{ sessionKey: string; agentId: string | undefined }> | undefined = params.sessionKeys ? [] : undefined; diff --git a/src/gateway/session-request-agent.test.ts b/src/gateway/session-request-agent.test.ts index 5d67496e9f2d..32061ce5d402 100644 --- a/src/gateway/session-request-agent.test.ts +++ b/src/gateway/session-request-agent.test.ts @@ -75,6 +75,23 @@ describe("requested session agent ownership", () => { }); }); + it.each(["", " ", "агент✨", "---"])( + "rejects explicit unrepresentable agent id %j instead of selecting main", + (agentId) => { + const cfg: OpenClawConfig = { + agents: { ownership: "explicit", entries: { main: {}, ops: {} } }, + }; + + expect(resolveRequestedSessionAgentId(cfg, "global", agentId)).toMatchObject({ + ok: false, + error: { + code: "INVALID_REQUEST", + message: `Unknown agent id "${agentId}"`, + }, + }); + }, + ); + it("keeps retired agent-qualified history readable outside global scope", () => { const cfg: OpenClawConfig = { agents: { ownership: "explicit", entries: { ops: {}, research: {} } }, diff --git a/src/gateway/session-request-agent.ts b/src/gateway/session-request-agent.ts index f36baa4f4d89..a0b3b1a44a76 100644 --- a/src/gateway/session-request-agent.ts +++ b/src/gateway/session-request-agent.ts @@ -1,4 +1,3 @@ -import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce"; import { ErrorCodes, type ErrorShape, @@ -10,6 +9,7 @@ import { resolvePersistedSessionStoreOwnerForKey } from "../config/sessions/sess import type { OpenClawConfig } from "../config/types.openclaw.js"; import { normalizeAgentId, + normalizeAgentIdStrict, normalizeMainKey, parseAgentSessionKey, } from "../routing/session-key.js"; @@ -39,11 +39,16 @@ export function resolveRequestedSessionAgentId( options?: { allowUnconfiguredExplicitAgent?: boolean }, ): RequestedSessionAgentIdResolution { const parsed = parseAgentSessionKey(key.trim()); - const requestedAgentId = normalizeOptionalString(explicitAgentId); const configuredAgentIds = listAgentIds(cfg); - const normalizedRequestedAgentId = requestedAgentId - ? normalizeAgentId(requestedAgentId) - : undefined; + const normalizedRequest = + explicitAgentId === undefined ? null : normalizeAgentIdStrict(explicitAgentId); + if (normalizedRequest && !normalizedRequest.ok) { + return { + ok: false, + error: errorShape(ErrorCodes.INVALID_REQUEST, `Unknown agent id "${explicitAgentId}"`), + }; + } + const normalizedRequestedAgentId = normalizedRequest?.value; if ( normalizedRequestedAgentId && !options?.allowUnconfiguredExplicitAgent && diff --git a/src/plugin-sdk/routing.ts b/src/plugin-sdk/routing.ts index 22ca2125a810..a7e4d765e7c0 100644 --- a/src/plugin-sdk/routing.ts +++ b/src/plugin-sdk/routing.ts @@ -21,6 +21,7 @@ export { isSubagentSessionKey, normalizeAccountId, normalizeAgentId, + normalizeAgentIdStrict, normalizeMainKey, normalizeOptionalAccountId, parseAgentSessionKey, diff --git a/src/routing/session-key.ts b/src/routing/session-key.ts index 28525acc4de0..792291629774 100644 --- a/src/routing/session-key.ts +++ b/src/routing/session-key.ts @@ -1,4 +1,8 @@ -import { isValidAgentId, normalizeAgentId } from "@openclaw/normalization-core/agent-id"; +import { + isValidAgentId, + normalizeAgentId, + normalizeAgentIdStrict, +} from "@openclaw/normalization-core/agent-id"; // Routing session key helpers build stable session keys from route targets. import { normalizeLowercaseStringOrEmpty, @@ -29,7 +33,7 @@ export { normalizeAccountId, normalizeOptionalAccountId, } from "./account-id.js"; -export { isValidAgentId, normalizeAgentId }; +export { isValidAgentId, normalizeAgentId, normalizeAgentIdStrict }; /** Legacy on-disk identity used only by doctor/migration and their fixtures. */ export const LEGACY_IMPLICIT_AGENT_ID = "main"; diff --git a/src/system-agent/operations-execution-helpers.ts b/src/system-agent/operations-execution-helpers.ts index 4728931e2717..0e35ad1a4f07 100644 --- a/src/system-agent/operations-execution-helpers.ts +++ b/src/system-agent/operations-execution-helpers.ts @@ -542,7 +542,7 @@ export async function executeSetDefaultModel( run: async (ctx) => { const { mutateConfigFile, readConfigFileSnapshot } = await loadConfigModule(); const { applySystemAgentModelSelection, createSystemAgentModelSelectionUpdater } = - await import("./setup-apply.js"); + await import("./setup-model-selection.js"); const targetAgentId = operation.agentId; const snapshot = await readConfigFileSnapshot(); // Route projection and the live probes below all take the same optional diff --git a/src/system-agent/operations-parse.test.ts b/src/system-agent/operations-parse.test.ts index bb0530d68732..c1ac1624acdc 100644 --- a/src/system-agent/operations-parse.test.ts +++ b/src/system-agent/operations-parse.test.ts @@ -33,6 +33,13 @@ describe("parseSystemAgentOperation", () => { kind: "set-default-model", model: "openai/gpt-5.2", }); + expect(parseSystemAgentOperation("set default model openai/gpt-5.2 for agent агент✨")).toEqual( + { + kind: "set-default-model", + model: "openai/gpt-5.2", + agentId: "агент✨", + }, + ); }); it("parses interactive model provider setup", () => { diff --git a/src/system-agent/operations-parse.ts b/src/system-agent/operations-parse.ts index 345d25387560..bd9d293251b4 100644 --- a/src/system-agent/operations-parse.ts +++ b/src/system-agent/operations-parse.ts @@ -3,7 +3,7 @@ import { parseConfigSetPath } from "../cli/config-cli-path.js"; import type { ConfigSetOptions } from "../cli/config-set-input.js"; import type { DoctorOptions } from "../commands/doctor.types.js"; import { DEFAULT_SECRET_PROVIDER_ALIAS } from "../config/types.secrets.js"; -import { normalizeAgentId } from "../routing/session-key.js"; +import { normalizeAgentIdStrict } from "../routing/session-key.js"; import type { RuntimeEnv } from "../runtime.js"; import { isValidSecretRef } from "../secrets/ref-contract.js"; import type { TuiResult } from "../tui/tui-types.js"; @@ -139,6 +139,12 @@ const OPEN_GATEWAY_SETUP_RE = /^open\s+gateway\s+wizard$/i; const NO_MATCH_MESSAGE = "I can run doctor/status/health, check or restart Gateway, configure gateway settings, list agents/models, configure skills or web search, import memory, set default model, connect channels (`connect telegram`), show `channel info `, open the setup wizard, show audit, or switch to your agent TUI."; +function normalizeExplicitSystemAgentId(agentId: string): string { + const normalized = normalizeAgentIdStrict(agentId); + // Preserve an unrepresentable input so the execution owner rejects it instead of targeting main. + return normalized.ok ? normalized.value : agentId; +} + function parseConfigSetCommand( input: string, ): { path: string; value: string; valid: true } | { valid: false } | undefined { @@ -443,7 +449,7 @@ export function parseSystemAgentOperation(input: string): SystemAgentOperation { const model = createMatch.groups.model; return { kind: "create-agent", - agentId: normalizeAgentId(createMatch.groups.agent), + agentId: normalizeExplicitSystemAgentId(createMatch.groups.agent), ...(workspace ? { workspace } : {}), ...(model ? { model } : {}), }; @@ -463,7 +469,7 @@ export function parseSystemAgentOperation(input: string): SystemAgentOperation { return { kind: "set-default-model", model: setModelMatch.groups.model, - ...(agent ? { agentId: normalizeAgentId(agent) } : {}), + ...(agent ? { agentId: normalizeExplicitSystemAgentId(agent) } : {}), }; } return { kind: "none", message: NO_MATCH_MESSAGE }; diff --git a/src/system-agent/setup-apply.model-selection.test.ts b/src/system-agent/setup-apply.model-selection.test.ts index e5528af8670f..e9d04160a20c 100644 --- a/src/system-agent/setup-apply.model-selection.test.ts +++ b/src/system-agent/setup-apply.model-selection.test.ts @@ -1,8 +1,25 @@ import { describe, expect, it } from "vitest"; import type { OpenClawConfig } from "../config/types.openclaw.js"; -import { applySystemAgentModelSelection } from "./setup-apply.js"; +import { applySystemAgentModelSelection } from "./setup-model-selection.js"; describe("applySystemAgentModelSelection", () => { + it("rejects an unrepresentable explicit agent instead of updating main", async () => { + const config = { + agents: { + entries: { main: { default: true }, ops: {} }, + }, + } satisfies OpenClawConfig; + + await expect( + applySystemAgentModelSelection({ + config, + model: "openai/gpt-5.5", + targetAgentId: "агент✨", + }), + ).rejects.toThrow('Could not resolve configured agent "агент✨".'); + expect(config.agents.entries.main).toEqual({ default: true }); + }); + it("clears stale harness pins in both model scopes for a native route", async () => { const config = { agents: { diff --git a/src/system-agent/setup-apply.ts b/src/system-agent/setup-apply.ts index 006075b50903..76c1bdef62c7 100644 --- a/src/system-agent/setup-apply.ts +++ b/src/system-agent/setup-apply.ts @@ -12,7 +12,6 @@ import { validateConfigObjectWithPlugins, } from "../config/config.js"; import { applyMergePatch } from "../config/merge-patch.js"; -import type { AgentModelEntryConfig } from "../config/types.agent-defaults.js"; import type { ConfigFileSnapshot, OpenClawConfig } from "../config/types.openclaw.js"; import { formatErrorMessage } from "../infra/errors.js"; import { formatExternalSupervisorActionRequired } from "../infra/gateway-supervision.js"; @@ -32,6 +31,7 @@ import { sameSetupConfiguredRoute, sameSetupInferenceRoute, } from "./setup-inference-route-guard.js"; +import { applySystemAgentModelSelection } from "./setup-model-selection.js"; /** * The whole first-run setup as one approved operation: the user says "yes" in @@ -139,139 +139,6 @@ function applySecurityAcknowledgement(config: OpenClawConfig): OpenClawConfig { }; } -type SystemAgentModelSelectionParams = { - config: OpenClawConfig; - model: string; - /** Write the model onto this configured agent instead of the default route. */ - targetAgentId?: string; - agentRuntimeId?: string; - /** Pin the selected model to the exact credential that passed inference. */ - authProfileId?: string; -}; - -type SystemAgentModelSelectionModules = { - agentScope: typeof import("../agents/agent-scope.js"); - modelConfig: typeof import("../commands/models/shared.js"); - runtimePolicy: typeof import("../agents/model-runtime-policy.js"); -}; - -function applySystemAgentModelSelectionWithModules( - params: SystemAgentModelSelectionParams, - modules: SystemAgentModelSelectionModules, -): OpenClawConfig { - const { agentScope, modelConfig, runtimePolicy } = modules; - const nextConfig = structuredClone(params.config); - const targetAgentId = params.targetAgentId ? normalizeAgentId(params.targetAgentId) : undefined; - const agentId = targetAgentId ?? agentScope.resolveDefaultAgentId(nextConfig); - const roster = agentScope.listAgentEntries(nextConfig); - if (targetAgentId && !roster.some((entry) => normalizeAgentId(entry.id) === targetAgentId)) { - throw new Error(`Could not resolve configured agent "${targetAgentId}".`); - } - // A targeted selection always lands on the agent entry; the default-route - // selection only writes the agent when it already carries an explicit model. - const writesAgent = Boolean( - targetAgentId || agentScope.resolveAgentExplicitModelPrimary(nextConfig, agentId), - ); - nextConfig.agents ??= {}; - nextConfig.agents.defaults ??= {}; - const agentDefaults = nextConfig.agents.defaults; - const target = modelConfig.resolveModelTarget({ raw: params.model, cfg: nextConfig }); - const key = modelConfig.upsertCanonicalModelConfigEntry({}, target); - - const configuredVisibleModels = agentDefaults.models; - if (configuredVisibleModels && Object.keys(configuredVisibleModels).length > 0) { - // An authored global visibility map is restrictive. Extend it for the - // approved selection; never create one merely to carry runtime metadata. - const defaultModels = { ...configuredVisibleModels }; - modelConfig.upsertCanonicalModelConfigEntry(defaultModels, target); - agentDefaults.models = defaultModels; - } - - const agentEntries = toAgentEntriesRecord(roster); - if (writesAgent || params.agentRuntimeId) { - const { list: _legacyList, ...agentConfig } = nextConfig.agents; - nextConfig.agents = { ...agentConfig, entries: agentEntries }; - } - const agentEntryKey = - roster.find((entry) => normalizeAgentId(entry.id) === agentId)?.id ?? agentId; - let agent = agentEntries[agentEntryKey]; - if (writesAgent) { - if (!agent) { - throw new Error(`Could not resolve configured default agent "${agentId}".`); - } - const agentModels = { ...agent.models }; - agent.models = agentModels; - modelConfig.upsertCanonicalModelConfigEntry(agentModels, target); - } - - if (params.agentRuntimeId) { - if (!agent) { - agent = { default: true }; - agentEntries[agentEntryKey] = agent; - } - const agentModels = { ...agent.models }; - const agentKey = modelConfig.upsertCanonicalModelConfigEntry(agentModels, target); - agentModels[agentKey] = { - ...agentModels[agentKey], - agentRuntime: { id: params.agentRuntimeId }, - }; - agent.models = agentModels; - } else { - const clearRuntimePin = ( - models: Record, - ): Record => { - const nextModels = { ...models }; - const modelKey = modelConfig.upsertCanonicalModelConfigEntry(nextModels, target); - const entry = { ...nextModels[modelKey] }; - delete entry.agentRuntime; - nextModels[modelKey] = entry; - return nextModels; - }; - const defaultModels = agentDefaults.models; - if (defaultModels && Object.keys(defaultModels).length > 0) { - agentDefaults.models = clearRuntimePin(defaultModels); - } - if (agent?.models && Object.keys(agent.models).length > 0) { - agent.models = clearRuntimePin(agent.models); - } - } - const selectedModel = params.authProfileId ? `${key}@${params.authProfileId}` : key; - agentScope.setAgentEffectiveModelPrimary(nextConfig, agentId, selectedModel, { - forceAgent: Boolean(targetAgentId), - }); - if (params.agentRuntimeId) { - const effectiveRuntime = runtimePolicy.resolveModelRuntimePolicy({ - config: nextConfig, - provider: target.provider, - modelId: target.model, - agentId, - }).policy?.id; - if (effectiveRuntime !== params.agentRuntimeId) { - throw new Error(`Could not pin ${key} to the ${params.agentRuntimeId} runtime.`); - } - } - return nextConfig; -} - -export async function createSystemAgentModelSelectionUpdater( - params: Omit, -): Promise<(config: OpenClawConfig) => OpenClawConfig> { - const [agentScope, modelConfig, runtimePolicy] = await Promise.all([ - import("../agents/agent-scope.js"), - import("../commands/models/shared.js"), - import("../agents/model-runtime-policy.js"), - ]); - const modules = { agentScope, modelConfig, runtimePolicy }; - return (config) => applySystemAgentModelSelectionWithModules({ ...params, config }, modules); -} - -export async function applySystemAgentModelSelection( - params: SystemAgentModelSelectionParams, -): Promise { - const update = await createSystemAgentModelSelectionUpdater(params); - return update(params.config); -} - export async function applySystemAgentSetup( params: SystemAgentSetupApplyParams, hooks?: SystemAgentSetupApplyHooks, diff --git a/src/system-agent/setup-inference-activate-persist.ts b/src/system-agent/setup-inference-activate-persist.ts index afca105a34ef..5f3aadbc3ee8 100644 --- a/src/system-agent/setup-inference-activate-persist.ts +++ b/src/system-agent/setup-inference-activate-persist.ts @@ -15,7 +15,6 @@ import type { SystemAgentConfiguredRoute, SystemAgentConfiguredRouteDeps, } from "./inference-route.js"; -import { createSystemAgentModelSelectionUpdater } from "./setup-apply.js"; import { SetupInferenceActivationIndeterminateError, SetupInferenceActivationUnavailableError, @@ -40,6 +39,7 @@ import { resolveSetupAgentRuntimeId, type SetupInferenceTestPlan, } from "./setup-inference-plan-helpers.js"; +import { createSystemAgentModelSelectionUpdater } from "./setup-model-selection.js"; import type { SystemAgentOwnerPluginArtifactSnapshot } from "./verified-inference.js"; type ProjectedInferenceRoute = Awaited>; diff --git a/src/system-agent/setup-inference-activate.ts b/src/system-agent/setup-inference-activate.ts index ec4545378801..3d03549b5245 100644 --- a/src/system-agent/setup-inference-activate.ts +++ b/src/system-agent/setup-inference-activate.ts @@ -25,7 +25,7 @@ import { resolveSystemAgentConfiguredRouteFromConfig, sameDefaultInferenceRoute, } from "./inference-route.js"; -import { applySystemAgentModelSelection, createQuickstartNotePrompter } from "./setup-apply.js"; +import { createQuickstartNotePrompter } from "./setup-apply.js"; import { persistActivatedSetupInference, type SetupInferenceActivationPersistenceState, @@ -56,6 +56,7 @@ import { resolveSetupAgentRuntimeId, } from "./setup-inference-plan-helpers.js"; import { buildTestPlan } from "./setup-inference-plan.js"; +import { applySystemAgentModelSelection } from "./setup-model-selection.js"; import { captureSystemAgentOwnerPluginArtifacts, type SystemAgentOwnerPluginArtifactSnapshot, diff --git a/src/system-agent/setup-inference.test.ts b/src/system-agent/setup-inference.test.ts index cf8a4092bcef..5fd2c8753a5a 100644 --- a/src/system-agent/setup-inference.test.ts +++ b/src/system-agent/setup-inference.test.ts @@ -44,7 +44,6 @@ import { createSuiteTempRootTracker } from "../test-helpers/temp-dir.js"; import { cleanupSystemAgentSession, createSystemAgentSession } from "./agent-turn.js"; import { runSystemAgentTurnWithDeps } from "./agent-turn.test-support.js"; import { resolveSystemAgentConfiguredRouteFromConfig } from "./inference-route.js"; -import { applySystemAgentModelSelection } from "./setup-apply.js"; import { setupInferenceLog } from "./setup-inference-core.js"; import { runSetupInferenceTest } from "./setup-inference-persist.js"; import { resolveSetupInferenceProbeStreamParams } from "./setup-inference-probe.js"; @@ -61,6 +60,7 @@ import { verifySetupInference as verifySetupInferenceImpl, verifySetupInferenceConfig as verifySetupInferenceConfigImpl, } from "./setup-inference.js"; +import { applySystemAgentModelSelection } from "./setup-model-selection.js"; import { installSystemAgentPluginMetadataTestSnapshot, type SystemAgentPluginMetadataTestSnapshot, diff --git a/src/system-agent/setup-model-selection.ts b/src/system-agent/setup-model-selection.ts new file mode 100644 index 000000000000..3ce466a35688 --- /dev/null +++ b/src/system-agent/setup-model-selection.ts @@ -0,0 +1,142 @@ +import { toAgentEntriesRecord } from "../agents/agent-scope-config.js"; +import type { AgentModelEntryConfig } from "../config/types.agent-defaults.js"; +import type { OpenClawConfig } from "../config/types.openclaw.js"; +import { normalizeAgentId, normalizeAgentIdStrict } from "../routing/session-key.js"; + +type SystemAgentModelSelectionParams = { + config: OpenClawConfig; + model: string; + /** Write the model onto this configured agent instead of the default route. */ + targetAgentId?: string; + agentRuntimeId?: string; + /** Pin the selected model to the exact credential that passed inference. */ + authProfileId?: string; +}; + +type SystemAgentModelSelectionModules = { + agentScope: typeof import("../agents/agent-scope.js"); + modelConfig: typeof import("../commands/models/shared.js"); + runtimePolicy: typeof import("../agents/model-runtime-policy.js"); +}; + +function applySystemAgentModelSelectionWithModules( + params: SystemAgentModelSelectionParams, + modules: SystemAgentModelSelectionModules, +): OpenClawConfig { + const { agentScope, modelConfig, runtimePolicy } = modules; + const nextConfig = structuredClone(params.config); + const normalizedTarget = + params.targetAgentId === undefined ? null : normalizeAgentIdStrict(params.targetAgentId); + if (normalizedTarget && !normalizedTarget.ok) { + throw new Error(`Could not resolve configured agent "${params.targetAgentId}".`); + } + const targetAgentId = normalizedTarget?.value; + const agentId = targetAgentId ?? agentScope.resolveDefaultAgentId(nextConfig); + const roster = agentScope.listAgentEntries(nextConfig); + if (targetAgentId && !roster.some((entry) => normalizeAgentId(entry.id) === targetAgentId)) { + throw new Error(`Could not resolve configured agent "${targetAgentId}".`); + } + // A targeted selection always lands on the agent entry; the default-route + // selection only writes the agent when it already carries an explicit model. + const writesAgent = Boolean( + targetAgentId || agentScope.resolveAgentExplicitModelPrimary(nextConfig, agentId), + ); + nextConfig.agents ??= {}; + nextConfig.agents.defaults ??= {}; + const agentDefaults = nextConfig.agents.defaults; + const target = modelConfig.resolveModelTarget({ raw: params.model, cfg: nextConfig }); + const key = modelConfig.upsertCanonicalModelConfigEntry({}, target); + + const configuredVisibleModels = agentDefaults.models; + if (configuredVisibleModels && Object.keys(configuredVisibleModels).length > 0) { + // An authored global visibility map is restrictive. Extend it for the + // approved selection; never create one merely to carry runtime metadata. + const defaultModels = { ...configuredVisibleModels }; + modelConfig.upsertCanonicalModelConfigEntry(defaultModels, target); + agentDefaults.models = defaultModels; + } + + const agentEntries = toAgentEntriesRecord(roster); + if (writesAgent || params.agentRuntimeId) { + const { list: _legacyList, ...agentConfig } = nextConfig.agents; + nextConfig.agents = { ...agentConfig, entries: agentEntries }; + } + const agentEntryKey = + roster.find((entry) => normalizeAgentId(entry.id) === agentId)?.id ?? agentId; + let agent = agentEntries[agentEntryKey]; + if (writesAgent) { + if (!agent) { + throw new Error(`Could not resolve configured default agent "${agentId}".`); + } + const agentModels = { ...agent.models }; + agent.models = agentModels; + modelConfig.upsertCanonicalModelConfigEntry(agentModels, target); + } + + if (params.agentRuntimeId) { + if (!agent) { + agent = { default: true }; + agentEntries[agentEntryKey] = agent; + } + const agentModels = { ...agent.models }; + const agentKey = modelConfig.upsertCanonicalModelConfigEntry(agentModels, target); + agentModels[agentKey] = { + ...agentModels[agentKey], + agentRuntime: { id: params.agentRuntimeId }, + }; + agent.models = agentModels; + } else { + const clearRuntimePin = ( + models: Record, + ): Record => { + const nextModels = { ...models }; + const modelKey = modelConfig.upsertCanonicalModelConfigEntry(nextModels, target); + const entry = { ...nextModels[modelKey] }; + delete entry.agentRuntime; + nextModels[modelKey] = entry; + return nextModels; + }; + const defaultModels = agentDefaults.models; + if (defaultModels && Object.keys(defaultModels).length > 0) { + agentDefaults.models = clearRuntimePin(defaultModels); + } + if (agent?.models && Object.keys(agent.models).length > 0) { + agent.models = clearRuntimePin(agent.models); + } + } + const selectedModel = params.authProfileId ? `${key}@${params.authProfileId}` : key; + agentScope.setAgentEffectiveModelPrimary(nextConfig, agentId, selectedModel, { + forceAgent: Boolean(targetAgentId), + }); + if (params.agentRuntimeId) { + const effectiveRuntime = runtimePolicy.resolveModelRuntimePolicy({ + config: nextConfig, + provider: target.provider, + modelId: target.model, + agentId, + }).policy?.id; + if (effectiveRuntime !== params.agentRuntimeId) { + throw new Error(`Could not pin ${key} to the ${params.agentRuntimeId} runtime.`); + } + } + return nextConfig; +} + +export async function createSystemAgentModelSelectionUpdater( + params: Omit, +): Promise<(config: OpenClawConfig) => OpenClawConfig> { + const [agentScope, modelConfig, runtimePolicy] = await Promise.all([ + import("../agents/agent-scope.js"), + import("../commands/models/shared.js"), + import("../agents/model-runtime-policy.js"), + ]); + const modules = { agentScope, modelConfig, runtimePolicy }; + return (config) => applySystemAgentModelSelectionWithModules({ ...params, config }, modules); +} + +export async function applySystemAgentModelSelection( + params: SystemAgentModelSelectionParams, +): Promise { + const update = await createSystemAgentModelSelectionUpdater(params); + return update(params.config); +}