diff --git a/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift b/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift index 6da2aa254ff..0fc21f30a4b 100644 --- a/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift +++ b/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift @@ -7544,6 +7544,7 @@ public struct SystemAgentChatResult: Codable, Sendable { public let sensitive: Bool? public let action: AnyCodable public let agentdraft: String? + public let agentid: String? public let needsapproval: Bool? public let proposalid: String? public let question: [String: AnyCodable]? @@ -7554,6 +7555,7 @@ public struct SystemAgentChatResult: Codable, Sendable { sensitive: Bool? = nil, action: AnyCodable, agentdraft: String? = nil, + agentid: String? = nil, needsapproval: Bool? = nil, proposalid: String? = nil, question: [String: AnyCodable]? = nil) @@ -7563,6 +7565,7 @@ public struct SystemAgentChatResult: Codable, Sendable { self.sensitive = sensitive self.action = action self.agentdraft = agentdraft + self.agentid = agentid self.needsapproval = needsapproval self.proposalid = proposalid self.question = question @@ -7574,6 +7577,7 @@ public struct SystemAgentChatResult: Codable, Sendable { case sensitive case action case agentdraft = "agentDraft" + case agentid = "agentId" case needsapproval = "needsApproval" case proposalid = "proposalId" case question @@ -9142,14 +9146,14 @@ public struct AgentSummary: Codable, Sendable { public struct AgentsCreateParams: Codable, Sendable { public let name: String - public let workspace: String + public let workspace: String? public let model: String? public let emoji: String? public let avatar: String? public init( name: String, - workspace: String, + workspace: String? = nil, model: String? = nil, emoji: String? = nil, avatar: String? = nil) diff --git a/packages/gateway-protocol/src/schema/agents-models-skills.ts b/packages/gateway-protocol/src/schema/agents-models-skills.ts index 89b310f3d0e..eac8e2de249 100644 --- a/packages/gateway-protocol/src/schema/agents-models-skills.ts +++ b/packages/gateway-protocol/src/schema/agents-models-skills.ts @@ -97,10 +97,10 @@ export const AgentsListResultSchema = closedObject({ agents: Type.Array(AgentSummarySchema), }); -/** Creates a configured agent with workspace, identity, and optional model. */ +/** Creates a configured agent; the server supplies an omitted workspace. */ export const AgentsCreateParamsSchema = closedObject({ name: NonEmptyString, - workspace: NonEmptyString, + workspace: Type.Optional(NonEmptyString), model: Type.Optional(NonEmptyString), emoji: Type.Optional(Type.String()), avatar: Type.Optional(Type.String()), diff --git a/packages/gateway-protocol/src/schema/openclaw.ts b/packages/gateway-protocol/src/schema/openclaw.ts index 4adfc0e43eb..1d568fd5f44 100644 --- a/packages/gateway-protocol/src/schema/openclaw.ts +++ b/packages/gateway-protocol/src/schema/openclaw.ts @@ -14,8 +14,10 @@ import { WizardStartResultSchema } from "./wizard.js"; export const SystemAgentChatParamsSchema = closedObject({ sessionId: NonEmptyString, message: Type.Optional(Type.String()), - /** "onboarding" seeds the first-run setup proposal in the greeting. */ - welcomeVariant: Type.Optional(Type.Union([Type.Literal("onboarding")])), + /** Seeds a purpose-specific first greeting for a fresh conversation. */ + welcomeVariant: Type.Optional( + Type.Union([Type.Literal("onboarding"), Type.Literal("new-agent")]), + ), /** Drop any in-flight approval/wizard state and start the session over. */ reset: Type.Optional(Type.Boolean()), /** Host-only regular-agent delegation context. Never model-authored. */ @@ -69,6 +71,8 @@ export const SystemAgentChatResultSchema = closedObject({ ]), /** Optional localized-draft intent for an `open-agent` handoff. */ agentDraft: Type.Optional(Type.Literal("hatch")), + /** Destination agent for a specific `open-agent` handoff. */ + agentId: Type.Optional(NonEmptyString), needsApproval: Type.Optional(Type.Boolean()), proposalId: Type.Optional(NonEmptyString), question: Type.Optional(SystemAgentChatQuestionSchema), diff --git a/packages/sdk/src/types.ts b/packages/sdk/src/types.ts index 290f4a51a28..101c36d15b2 100644 --- a/packages/sdk/src/types.ts +++ b/packages/sdk/src/types.ts @@ -373,7 +373,7 @@ export type RunCreateParams = AgentRunParams; export type AgentsCreateParams = { name: string; - workspace: string; + workspace?: string; model?: string; emoji?: string; avatar?: string; diff --git a/src/agents/agent-create.test.ts b/src/agents/agent-create.test.ts new file mode 100644 index 00000000000..ae367a4c825 --- /dev/null +++ b/src/agents/agent-create.test.ts @@ -0,0 +1,224 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { FsSafeError } from "../infra/fs-safe.js"; + +const mocks = vi.hoisted(() => ({ + config: {} as Record, + persisted: {} as Record, + transformConfigFileWithRetry: vi.fn(), + withConfigMutationExclusive: vi.fn(), + parseBindingSpecs: vi.fn(), + ensureAgentWorkspace: vi.fn(), + resolveAgentWorkspaceDir: vi.fn(), + resolveAgentDir: vi.fn(), + rootRead: vi.fn(), + rootWrite: vi.fn(), + mkdir: vi.fn(), +})); + +vi.mock("node:fs/promises", () => ({ default: { mkdir: mocks.mkdir } })); + +vi.mock("../config/config.js", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + transformConfigFileWithRetry: mocks.transformConfigFileWithRetry, + withConfigMutationExclusive: mocks.withConfigMutationExclusive, + }; +}); + +vi.mock("../commands/agents.bindings.js", async (importOriginal) => ({ + ...(await importOriginal()), + parseBindingSpecs: mocks.parseBindingSpecs, +})); + +vi.mock("./agent-scope.js", async (importOriginal) => ({ + ...(await importOriginal()), + resolveAgentWorkspaceDir: mocks.resolveAgentWorkspaceDir, + resolveAgentDir: mocks.resolveAgentDir, +})); + +vi.mock("./workspace.js", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, ensureAgentWorkspace: mocks.ensureAgentWorkspace }; +}); + +vi.mock("../config/sessions/paths.js", () => ({ + resolveSessionTranscriptsDirForAgent: (agentId: string) => `/tmp/transcripts-${agentId}`, +})); + +vi.mock("../infra/fs-safe.js", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + root: vi.fn(async () => ({ + read: mocks.rootRead, + write: mocks.rootWrite, + })), + }; +}); + +import { createAgent } from "./agent-create.js"; + +describe("createAgent", () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.config = {}; + mocks.persisted = {}; + mocks.resolveAgentWorkspaceDir.mockReturnValue("/tmp/default-researcher"); + mocks.resolveAgentDir.mockReturnValue("/tmp/agent-researcher"); + mocks.ensureAgentWorkspace.mockImplementation(async ({ dir }: { dir: string }) => ({ + dir, + bootstrapPending: true, + })); + mocks.rootRead.mockResolvedValue({ buffer: Buffer.from("") }); + mocks.rootWrite.mockResolvedValue(undefined); + mocks.mkdir.mockResolvedValue(undefined); + mocks.parseBindingSpecs.mockReturnValue({ bindings: [], errors: [] }); + mocks.withConfigMutationExclusive.mockImplementation(async (fn: () => Promise) => { + return await fn(); + }); + mocks.transformConfigFileWithRetry.mockImplementation( + async ({ + transform, + }: { + transform: (config: Record) => Promise; + }) => { + const transformed = (await transform(structuredClone(mocks.config))) as { + nextConfig: Record; + result: unknown; + }; + mocks.persisted = transformed.nextConfig; + mocks.config = transformed.nextConfig; + return { result: transformed.result, nextConfig: transformed.nextConfig }; + }, + ); + }); + + it("returns validation errors before mutation", async () => { + await expect(createAgent({ name: " " })).resolves.toMatchObject({ + status: "error", + reason: "invalid-name", + }); + for (const name of ["main", "OpenClaw", "crestodian"]) { + await expect(createAgent({ name })).resolves.toMatchObject({ + status: "error", + reason: "reserved-id", + }); + } + expect(mocks.transformConfigFileWithRetry).not.toHaveBeenCalled(); + }); + + it("defaults the workspace through the agent-scoped resolver", async () => { + const result = await createAgent({ name: "Researcher" }); + + expect(mocks.resolveAgentWorkspaceDir).toHaveBeenCalledWith(expect.any(Object), "researcher"); + expect(result).toMatchObject({ + status: "created", + agentId: "researcher", + workspace: "/tmp/default-researcher", + bootstrapPending: true, + }); + }); + + it("respects skipBootstrap from the current config", async () => { + mocks.config = { agents: { defaults: { skipBootstrap: true } } }; + + await createAgent({ name: "researcher", workspace: "/tmp/work" }); + + expect(mocks.ensureAgentWorkspace).toHaveBeenCalledWith( + expect.objectContaining({ ensureBootstrapFiles: false }), + ); + }); + + it("persists the authoritative workspace returned by setup", async () => { + mocks.ensureAgentWorkspace.mockResolvedValue({ + dir: "/normalized/work", + bootstrapPending: true, + }); + + const result = await createAgent({ name: "researcher", workspace: "/tmp/work" }); + + const agents = mocks.persisted.agents as { list?: Array<{ workspace?: string }> } | undefined; + expect(agents?.list?.at(-1)?.workspace).toBe("/normalized/work"); + expect(result).toMatchObject({ status: "created", workspace: "/normalized/work" }); + }); + + it("persists the canonical agent entry through retrying mutation", async () => { + const result = await createAgent({ + name: "Researcher", + workspace: "/tmp/work", + model: "openai/gpt-5.5", + emoji: "🔎", + }); + + expect(mocks.transformConfigFileWithRetry).toHaveBeenCalledOnce(); + expect(mocks.persisted).toMatchObject({ + agents: { + list: expect.arrayContaining([ + { + id: "researcher", + name: "Researcher", + workspace: "/tmp/work", + agentDir: "/tmp/agent-researcher", + model: "openai/gpt-5.5", + identity: { name: "Researcher", emoji: "🔎" }, + }, + ]), + }, + }); + expect(result).toMatchObject({ status: "created", agentId: "researcher" }); + }); + + it("finishes workspace setup before publishing config", async () => { + mocks.ensureAgentWorkspace.mockImplementation(async ({ dir }: { dir: string }) => { + expect(mocks.persisted).not.toHaveProperty("agents"); + return { dir, bootstrapPending: true }; + }); + + await createAgent({ name: "researcher" }); + + expect(mocks.ensureAgentWorkspace).toHaveBeenCalledOnce(); + }); + + it("does not publish config when identity setup is unsafe", async () => { + mocks.rootRead.mockRejectedValue(new FsSafeError("invalid-path", "unsafe identity path")); + + await expect(createAgent({ name: "researcher" })).resolves.toMatchObject({ + status: "error", + reason: "unsafe-identity-file", + }); + expect(mocks.transformConfigFileWithRetry).toHaveBeenCalledOnce(); + expect(mocks.persisted).not.toHaveProperty("agents"); + }); + + it("rejects a concurrent duplicate from the mutation snapshot", async () => { + mocks.config = { agents: { list: [{ id: "researcher" }] } }; + + await expect(createAgent({ name: "researcher" })).resolves.toMatchObject({ + status: "error", + reason: "already-exists", + }); + expect(mocks.ensureAgentWorkspace).not.toHaveBeenCalled(); + }); + + it("parses binding specs from the locked winning snapshot", async () => { + mocks.parseBindingSpecs.mockReturnValue({ + bindings: [], + errors: ['Unknown channel "removed".'], + }); + const transformConfig = vi.fn(async ({ maxAttempts, transform }) => { + expect(maxAttempts).toBe(1); + return await transform({ agents: { list: [] } }); + }); + + await expect( + createAgent({ + name: "researcher", + bindingSpecs: ["removed"], + transformConfig: transformConfig as never, + }), + ).resolves.toMatchObject({ status: "error", reason: "invalid-bindings" }); + expect(mocks.parseBindingSpecs).toHaveBeenCalledOnce(); + expect(mocks.ensureAgentWorkspace).not.toHaveBeenCalled(); + }); +}); diff --git a/src/agents/agent-create.ts b/src/agents/agent-create.ts new file mode 100644 index 00000000000..a2467949a7e --- /dev/null +++ b/src/agents/agent-create.ts @@ -0,0 +1,199 @@ +import fs from "node:fs/promises"; +import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce"; +import { applyAgentBindings, parseBindingSpecs } from "../commands/agents.bindings.js"; +import { + applyAgentConfig, + findAgentEntryIndex, + listAgentEntries, +} from "../commands/agents.config.js"; +import { transformConfigFileWithRetry, withConfigMutationExclusive } from "../config/config.js"; +import { resolveSessionTranscriptsDirForAgent } from "../config/sessions/paths.js"; +import { FsSafeError, root } from "../infra/fs-safe.js"; +import { DEFAULT_AGENT_ID, normalizeAgentId } from "../routing/session-key.js"; +import { isReservedSystemAgentId } from "../system-agent/agent-id.js"; +import { resolveUserPath } from "../utils.js"; +import { resolveAgentDir, resolveAgentWorkspaceDir } from "./agent-scope.js"; +import { + createAgentIdentityConfig, + mergeIdentityMarkdownContent, + sanitizeAgentIdentityLine, +} from "./identity-file.js"; +import { DEFAULT_IDENTITY_FILENAME, ensureAgentWorkspace } from "./workspace.js"; + +type CreateAgentResult = + | { + status: "created"; + agentId: string; + name: string; + workspace: string; + agentDir: string; + model?: string; + bootstrapPending: boolean; + bindingResult?: ReturnType; + } + | { + status: "error"; + reason: + | "invalid-name" + | "reserved-id" + | "already-exists" + | "invalid-bindings" + | "unsafe-identity-file"; + agentId?: string; + message: string; + }; + +type CreateError = Extract; + +type CreateAgentParams = { + name: string; + workspace?: string; + model?: string; + emoji?: unknown; + avatar?: unknown; + agentDir?: string; + bindingSpecs?: string[]; + transformConfig?: typeof transformConfigFileWithRetry; +}; + +class DuplicateAgentError extends Error {} +class InvalidAgentBindingsError extends Error {} + +function createError( + reason: CreateError["reason"], + message: string, + agentId?: string, +): CreateError { + return { status: "error", reason, message, ...(agentId ? { agentId } : {}) }; +} + +async function writeIdentityFile(params: { + workspaceDir: string; + identity: NonNullable>; +}): Promise { + const workspaceRoot = await root(params.workspaceDir); + let existing: string | undefined; + try { + const result = await workspaceRoot.read(DEFAULT_IDENTITY_FILENAME, { + hardlinks: "reject", + nonBlockingRead: true, + }); + existing = result.buffer.toString("utf-8"); + } catch (error) { + if (!(error instanceof FsSafeError && error.code === "not-found")) { + throw error; + } + } + const content = mergeIdentityMarkdownContent(existing, params.identity); + await workspaceRoot.write(DEFAULT_IDENTITY_FILENAME, content, { encoding: "utf8" }); +} + +export async function createAgent(params: CreateAgentParams): Promise { + const rawName = params.name.trim(); + if (!rawName) { + return createError("invalid-name", "agent name is required"); + } + const agentId = normalizeAgentId(rawName); + if (agentId === DEFAULT_AGENT_ID || isReservedSystemAgentId(agentId)) { + return createError("reserved-id", `"${agentId}" is reserved`, agentId); + } + + const safeName = sanitizeAgentIdentityLine(rawName); + const model = normalizeOptionalString(params.model); + const identity = createAgentIdentityConfig({ + name: safeName, + emoji: params.emoji, + avatar: params.avatar, + }) ?? { name: safeName }; + const explicitWorkspace = params.workspace?.trim() + ? resolveUserPath(params.workspace.trim()) + : undefined; + const explicitAgentDir = params.agentDir?.trim() + ? resolveUserPath(params.agentDir.trim()) + : undefined; + const transformConfig = params.transformConfig ?? transformConfigFileWithRetry; + + try { + return await withConfigMutationExclusive(async () => { + const committed = await transformConfig({ + afterWrite: { mode: "auto" }, + maxAttempts: 1, + transform: async (currentConfig) => { + if (findAgentEntryIndex(listAgentEntries(currentConfig), agentId) >= 0) { + throw new DuplicateAgentError(); + } + + const workspaceDir = + explicitWorkspace ?? resolveAgentWorkspaceDir(currentConfig, agentId); + const agentDir = explicitAgentDir ?? resolveAgentDir(currentConfig, agentId); + let nextConfig = applyAgentConfig(currentConfig, { + agentId, + name: safeName, + workspace: workspaceDir, + agentDir, + model, + identity, + }); + const bindingParse = parseBindingSpecs({ + agentId, + specs: params.bindingSpecs, + config: nextConfig, + }); + if (bindingParse.errors.length > 0) { + throw new InvalidAgentBindingsError(bindingParse.errors.join("\n")); + } + const bindingResult = bindingParse.bindings.length + ? applyAgentBindings(nextConfig, bindingParse.bindings) + : undefined; + nextConfig = bindingResult?.config ?? nextConfig; + + // The outer lock makes this result-bearing transform single-attempt: setup + // finishes before the final entry becomes visible to readers or delete flows. + const workspace = await ensureAgentWorkspace({ + dir: workspaceDir, + ensureBootstrapFiles: !nextConfig.agents?.defaults?.skipBootstrap, + skipOptionalBootstrapFiles: nextConfig.agents?.defaults?.skipOptionalBootstrapFiles, + }); + if (workspace.dir !== workspaceDir) { + nextConfig = applyAgentConfig(nextConfig, { + agentId, + workspace: workspace.dir, + }); + } + await fs.mkdir(resolveSessionTranscriptsDirForAgent(agentId), { recursive: true }); + await writeIdentityFile({ workspaceDir: workspace.dir, identity }); + + return { + nextConfig, + result: { + status: "created", + agentId, + name: safeName, + workspace: workspace.dir, + agentDir, + ...(model ? { model } : {}), + bootstrapPending: workspace.bootstrapPending === true, + ...(bindingResult ? { bindingResult } : {}), + }, + }; + }, + }); + return committed.result!; + }); + } catch (error) { + if (error instanceof DuplicateAgentError) { + return createError("already-exists", `agent "${agentId}" already exists`, agentId); + } + if (error instanceof InvalidAgentBindingsError) { + return createError("invalid-bindings", error.message, agentId); + } + if (error instanceof FsSafeError) { + return createError( + "unsafe-identity-file", + `unsafe workspace file "${DEFAULT_IDENTITY_FILENAME}"`, + agentId, + ); + } + throw error; + } +} diff --git a/src/agents/identity-file.ts b/src/agents/identity-file.ts index 49f205f06dd..21ba27ee71b 100644 --- a/src/agents/identity-file.ts +++ b/src/agents/identity-file.ts @@ -5,7 +5,11 @@ */ import fs from "node:fs"; import path from "node:path"; -import { normalizeLowercaseStringOrEmpty } from "@openclaw/normalization-core/string-coerce"; +import { + normalizeLowercaseStringOrEmpty, + normalizeOptionalString, +} from "@openclaw/normalization-core/string-coerce"; +import type { IdentityConfig } from "../config/types.base.js"; import { readRegularFile, readRegularFileSync } from "../infra/regular-file.js"; import { DEFAULT_IDENTITY_FILENAME } from "./workspace.js"; @@ -41,6 +45,41 @@ const IDENTITY_PLACEHOLDER_VALUES = new Set([ "workspace-relative path, http(s) url, or data uri", ]); +export function sanitizeAgentIdentityLine(value: string): string { + return value.replace(/\s+/g, " ").trim(); +} + +const IDENTITY_CONFIG_FIELDS = ["name", "theme", "emoji", "avatar"] as const; + +function compactIdentityConfig(identity: IdentityConfig): IdentityConfig | undefined { + const resolved: IdentityConfig = {}; + for (const field of IDENTITY_CONFIG_FIELDS) { + const value = identity[field]?.trim(); + if (value) { + resolved[field] = value; + } + } + return Object.keys(resolved).length ? resolved : undefined; +} + +export function createAgentIdentityConfig(params: { + name?: string; + emoji?: unknown; + avatar?: unknown; +}): IdentityConfig | undefined { + return compactIdentityConfig({ + ...(params.name ? { name: sanitizeAgentIdentityLine(params.name) } : {}), + emoji: sanitizeAgentIdentityLine(normalizeOptionalString(params.emoji) ?? ""), + avatar: sanitizeAgentIdentityLine(normalizeOptionalString(params.avatar) ?? ""), + }); +} + +export function normalizeIdentityForFile( + identity: IdentityConfig | undefined, +): IdentityConfig | undefined { + return identity ? compactIdentityConfig(identity) : undefined; +} + function normalizeIdentityValue(value: string): string { // Normalize markdown decoration and punctuation so generated template // placeholders do not accidentally become real identity values. diff --git a/src/commands/agents.add.test.ts b/src/commands/agents.add.test.ts index cef3f56ed47..766ff48dba1 100644 --- a/src/commands/agents.add.test.ts +++ b/src/commands/agents.add.test.ts @@ -18,6 +18,7 @@ const writeConfigFileMock = vi.hoisted(() => vi.fn().mockResolvedValue(undefined const replaceConfigFileMock = vi.hoisted(() => vi.fn(async (params: { nextConfig: unknown }) => await writeConfigFileMock(params.nextConfig)), ); +const createAgentMock = vi.hoisted(() => vi.fn()); const commitConfigWithPendingPluginInstallsMock = vi.hoisted(() => vi.fn(async (params: { nextConfig: Record }) => { await writeConfigFileMock(params.nextConfig); @@ -86,6 +87,8 @@ vi.mock("../config/config.js", async () => ({ replaceConfigFile: replaceConfigFileMock, })); +vi.mock("../agents/agent-create.js", () => ({ createAgent: createAgentMock })); + vi.mock("../plugins/install-record-commit.js", async () => ({ ...(await vi.importActual( "../plugins/install-record-commit.js", @@ -136,6 +139,41 @@ describe("agents add command", () => { replaceConfigFileMock.mockClear(); commitConfigWithPendingPluginInstallsMock.mockClear(); transformConfigWithPendingPluginInstallsMock.mockClear(); + createAgentMock.mockReset(); + createAgentMock.mockImplementation( + async (params: { name: string; workspace: string; bindingSpecs?: string[] }) => { + const agentId = params.name.toLowerCase(); + if (agentId === "openclaw" || agentId === "crestodian") { + return { status: "error", reason: "reserved-id", agentId }; + } + const binding = params.bindingSpecs?.[0] + ? { + type: "route", + agentId, + match: { channel: params.bindingSpecs[0].split(":")[0] }, + } + : undefined; + return { + status: "created" as const, + agentId, + name: params.name, + workspace: params.workspace, + agentDir: `/tmp/agent-${agentId}`, + bootstrapPending: true, + ...(binding + ? { + bindingResult: { + config: {}, + added: [], + updated: [], + skipped: [], + conflicts: [{ binding, existingAgentId: "other-agent" }], + }, + } + : {}), + }; + }, + ); wizardMocks.createClackPrompter.mockClear(); authChoiceMocks.applyAuthChoice.mockClear(); authChoiceMocks.warnIfModelConfigLooksOff.mockClear(); @@ -424,54 +462,39 @@ describe("agents add command", () => { }); describe("non-interactive config mutation", () => { - it("rebases agent creation on the latest config snapshot", async () => { - readConfigFileSnapshotMock - .mockResolvedValueOnce({ - ...baseConfigSnapshot, - hash: "hash-1", - config: { agents: { list: [] } }, - sourceConfig: { agents: { list: [] } }, - }) - .mockResolvedValueOnce({ - ...baseConfigSnapshot, - hash: "hash-2", - config: { agents: { list: [{ id: "other-agent" }] } }, - sourceConfig: { agents: { list: [{ id: "other-agent" }] } }, - }); + it("delegates creation to the canonical service", async () => { + readConfigFileSnapshotMock.mockResolvedValue({ + ...baseConfigSnapshot, + config: { agents: { list: [] } }, + sourceConfig: { agents: { list: [] } }, + }); await agentsAddCommand({ name: "Work", workspace: "/tmp/work" }, runtime, { hasFlags: true, }); - expect(transformConfigWithPendingPluginInstallsMock).toHaveBeenCalledOnce(); - expect(writeConfigFileMock).toHaveBeenCalledWith( - expect.objectContaining({ - agents: { - list: [ - { id: "other-agent" }, - expect.objectContaining({ id: "work", workspace: "/tmp/work" }), - ], - }, - }), - ); + expect(createAgentMock).toHaveBeenCalledWith({ + name: "Work", + workspace: "/tmp/work", + transformConfig: transformConfigWithPendingPluginInstallsMock, + }); + expect(transformConfigWithPendingPluginInstallsMock).not.toHaveBeenCalled(); expect(runtime.exit).not.toHaveBeenCalled(); expect(runtime.error).not.toHaveBeenCalled(); }); - it("fails instead of overwriting when the same agent appears before commit", async () => { - readConfigFileSnapshotMock - .mockResolvedValueOnce({ - ...baseConfigSnapshot, - hash: "hash-1", - config: { agents: { list: [] } }, - sourceConfig: { agents: { list: [] } }, - }) - .mockResolvedValueOnce({ - ...baseConfigSnapshot, - hash: "hash-2", - config: { agents: { list: [{ id: "work", workspace: "/tmp/other" }] } }, - sourceConfig: { agents: { list: [{ id: "work", workspace: "/tmp/other" }] } }, - }); + it("reports a duplicate rejected by the canonical service", async () => { + readConfigFileSnapshotMock.mockResolvedValue({ + ...baseConfigSnapshot, + config: { agents: { list: [] } }, + sourceConfig: { agents: { list: [] } }, + }); + createAgentMock.mockResolvedValueOnce({ + status: "error", + reason: "already-exists", + agentId: "work", + message: 'agent "work" already exists', + }); await agentsAddCommand({ name: "Work", workspace: "/tmp/work" }, runtime, { hasFlags: true, diff --git a/src/commands/agents.commands.add.ts b/src/commands/agents.commands.add.ts index 8ca6781fb7f..f86566da5a2 100644 --- a/src/commands/agents.commands.add.ts +++ b/src/commands/agents.commands.add.ts @@ -5,6 +5,7 @@ import { normalizeLowercaseStringOrEmpty, normalizeOptionalString, } from "@openclaw/normalization-core/string-coerce"; +import { createAgent } from "../agents/agent-create.js"; import { resolveAgentDir, resolveAgentWorkspaceDir, @@ -30,14 +31,9 @@ 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"; -import { - applyAgentBindings, - buildChannelBindings, - describeBinding, - parseBindingSpecs, -} from "./agents.bindings.js"; -import { createQuietRuntime, requireValidConfigFileSnapshot } from "./agents.command-shared.js"; -import { applyAgentConfig, findAgentEntryIndex, listAgentEntries } from "./agents.config.js"; +import { applyAgentBindings, buildChannelBindings, describeBinding } from "./agents.bindings.js"; +import { requireValidConfigFileSnapshot } from "./agents.command-shared.js"; +import { applyAgentConfig, listAgentEntries } from "./agents.config.js"; import { promptAuthChoiceGrouped } from "./auth-choice-prompt.js"; import { applyAuthChoice, warnIfModelConfigLooksOff } from "./auth-choice.js"; import { setupChannels } from "./onboard-channels.js"; @@ -56,18 +52,6 @@ type AgentsAddOptions = { type AgentBindingResult = ReturnType; -type AgentsAddMutationResult = { - agentDir: string; - bindingResult: AgentBindingResult; -}; - -class AgentsAddMutationError extends Error { - constructor(message: string) { - super(message); - this.name = "AgentsAddMutationError"; - } -} - function emptyBindingResult(config: Parameters[0]): AgentBindingResult { return { config, added: [], updated: [], skipped: [], conflicts: [] }; } @@ -138,92 +122,41 @@ export async function agentsAddCommand( return; } const agentId = normalizeAgentId(nameInput); - if (agentId === DEFAULT_AGENT_ID || isReservedSystemAgentId(agentId)) { - runtime.error( - `"${agentId}" is reserved. Choose another name, or run ${formatCliCommand("openclaw agents list")} to inspect configured agents.`, - ); - runtime.exit(1); - return; - } if (agentId !== nameInput) { runtime.log(`Normalized agent id to "${agentId}".`); } - if (findAgentEntryIndex(listAgentEntries(cfg), agentId) >= 0) { + + const created = await createAgent({ + name: nameInput, + workspace: workspaceFlag, + ...(opts.agentDir ? { agentDir: opts.agentDir } : {}), + ...(opts.model ? { model: opts.model } : {}), + ...(opts.bind?.length ? { bindingSpecs: opts.bind } : {}), + transformConfig: transformConfigWithPendingPluginInstalls, + }); + if (created.status === "error") { runtime.error( - `Agent "${agentId}" already exists. Run ${formatCliCommand("openclaw agents list")} to inspect configured agents.`, + created.reason === "reserved-id" + ? `"${created.agentId}" is reserved. Choose another name, or run ${formatCliCommand("openclaw agents list")} to inspect configured agents.` + : created.reason === "already-exists" + ? `Agent "${created.agentId}" already exists.` + : created.message, ); runtime.exit(1); return; } - const workspaceDir = resolveUserPath(workspaceFlag); - const explicitAgentDir = opts.agentDir?.trim() - ? resolveUserPath(opts.agentDir.trim()) - : undefined; - const model = opts.model?.trim(); - - let committed; - try { - committed = await transformConfigWithPendingPluginInstalls({ - transform: (latestConfig) => { - if (findAgentEntryIndex(listAgentEntries(latestConfig), agentId) >= 0) { - throw new AgentsAddMutationError(`Agent "${agentId}" already exists.`); - } - const agentDir = explicitAgentDir ?? resolveAgentDir(latestConfig, agentId); - const nextConfig = applyAgentConfig(latestConfig, { - agentId, - name: nameInput, - workspace: workspaceDir, - agentDir, - ...(model ? { model } : {}), - }); - const bindingParse = parseBindingSpecs({ - agentId, - specs: opts.bind, - config: nextConfig, - }); - if (bindingParse.errors.length > 0) { - throw new AgentsAddMutationError(bindingParse.errors.join("\n")); - } - const bindingResult = - bindingParse.bindings.length > 0 - ? applyAgentBindings(nextConfig, bindingParse.bindings) - : emptyBindingResult(nextConfig); - return { - nextConfig: bindingResult.config, - result: { agentDir, bindingResult }, - }; - }, - }); - } catch (err) { - if (err instanceof AgentsAddMutationError) { - runtime.error(err.message); - runtime.exit(1); - return; - } - throw err; - } - const mutationResult = committed.result; - if (!mutationResult) { - throw new Error("Agent config mutation did not return a result."); - } - const { agentDir, bindingResult } = mutationResult; + const bindingResult = created.bindingResult ?? emptyBindingResult(cfg); if (!opts.json) { logConfigUpdated(runtime); } - const quietRuntime = opts.json ? createQuietRuntime(runtime) : runtime; - await ensureWorkspaceAndSessions(workspaceDir, quietRuntime, { - skipBootstrap: Boolean(committed.nextConfig.agents?.defaults?.skipBootstrap), - skipOptionalBootstrapFiles: committed.nextConfig.agents?.defaults?.skipOptionalBootstrapFiles, - agentId, - }); const payload = { - agentId, - name: nameInput, - workspace: workspaceDir, - agentDir, - model, + agentId: created.agentId, + name: created.name, + workspace: created.workspace, + agentDir: created.agentDir, + model: created.model, bindings: { added: bindingResult.added.map(describeBinding), updated: bindingResult.updated.map(describeBinding), @@ -237,10 +170,10 @@ export async function agentsAddCommand( writeRuntimeJson(runtime, payload); } else { runtime.log(`Agent: ${agentId}`); - runtime.log(`Workspace: ${shortenHomePath(workspaceDir)}`); - runtime.log(`Agent dir: ${shortenHomePath(agentDir)}`); - if (model) { - runtime.log(`Model: ${model}`); + runtime.log(`Workspace: ${shortenHomePath(created.workspace)}`); + runtime.log(`Agent dir: ${shortenHomePath(created.agentDir)}`); + if (created.model) { + runtime.log(`Model: ${created.model}`); } if (bindingResult.conflicts.length > 0) { runtime.error( diff --git a/src/config/config.ts b/src/config/config.ts index 0774dc87db0..d7a0cbe30d7 100644 --- a/src/config/config.ts +++ b/src/config/config.ts @@ -58,6 +58,7 @@ export { replaceConfigFile, transformConfigFile, transformConfigFileWithRetry, + withConfigMutationExclusive, } from "./mutate.js"; export type { ConfigMutationCommit, diff --git a/src/config/mutate.test.ts b/src/config/mutate.test.ts index 63c9a5cba34..9fe85f2c834 100644 --- a/src/config/mutate.test.ts +++ b/src/config/mutate.test.ts @@ -12,6 +12,7 @@ import { mutateConfigFile, replaceConfigFile, transformConfigFileWithRetry, + withConfigMutationExclusive, } from "./mutate.js"; import { resolveConfigPath } from "./paths.js"; import { @@ -428,6 +429,34 @@ describe("config mutate helpers", () => { ); }); + it("allows nested mutation helpers while holding the exclusive config lock", async () => { + const configPath = resolveConfigPath(); + const snapshot = createSnapshot({ + hash: "hash-1", + path: configPath, + sourceConfig: { agents: { list: [] } }, + }); + ioMocks.readConfigFileSnapshotForWrite.mockResolvedValue({ + snapshot, + writeOptions: { expectedConfigPath: configPath }, + }); + ioMocks.writeConfigFile.mockResolvedValue(undefined); + + const result = await withConfigMutationExclusive(async () => { + return await transformConfigFileWithRetry({ + maxAttempts: 1, + transform: (config) => ({ + nextConfig: { ...config, agents: { list: [{ id: "work" }] } }, + result: "created", + }), + }); + }); + + expect(result.result).toBe("created"); + expect(ioMocks.readConfigFileSnapshotForWrite).toHaveBeenCalledTimes(2); + expect(ioMocks.writeConfigFile).toHaveBeenCalledOnce(); + }); + it("rejects stale replace attempts when the base hash changed", async () => { ioMocks.readConfigFileSnapshotForWrite.mockResolvedValue({ snapshot: createSnapshot({ diff --git a/src/config/mutate.ts b/src/config/mutate.ts index 785630323e1..4332d508ec5 100644 --- a/src/config/mutate.ts +++ b/src/config/mutate.ts @@ -314,6 +314,14 @@ async function withConfigMutationSnapshotLock( }); } +/** + * Run a multi-phase operation under the canonical cross-process write lock. + * Nested mutation helpers are reentrant through activeConfigMutationLocks. + */ +export async function withConfigMutationExclusive(fn: () => Promise): Promise { + return await withConfigMutationSnapshotLock({}, async () => await fn()); +} + function getChangedTopLevelKeys(base: unknown, next: unknown): string[] { if (!isRecord(base) || !isRecord(next)) { return isDeepStrictEqual(base, next) ? [] : [""]; diff --git a/src/gateway/server-methods/agents-config-mutations.ts b/src/gateway/server-methods/agents-config-mutations.ts index 6b9069a0731..87d00d29efb 100644 --- a/src/gateway/server-methods/agents-config-mutations.ts +++ b/src/gateway/server-methods/agents-config-mutations.ts @@ -20,53 +20,13 @@ type AgentDeleteMutationResult = { }; /** Typed precondition failure surfaced by agent mutation handlers as gateway errors. */ -export class AgentConfigPreconditionError extends Error { - constructor( - readonly kind: "already-exists" | "not-found", - readonly agentId: string, - ) { - super( - kind === "already-exists" - ? `agent "${agentId}" already exists` - : `agent "${agentId}" not found`, - ); - this.name = "AgentConfigPreconditionError"; - } -} +export class AgentConfigPreconditionError extends Error {} /** Checks the current config snapshot for a concrete agent entry. */ export function isConfiguredAgent(cfg: OpenClawConfig, agentId: string): boolean { return findAgentEntryIndex(listAgentEntries(cfg), agentId) >= 0; } -/** Adds a new agent entry through the retrying config mutation path. */ -export async function createAgentConfigEntry(params: { - agentId: string; - name: string; - workspace: string; - model?: string; - identity?: IdentityConfig; - agentDir: string; -}): Promise { - await mutateConfigFileWithRetry({ - afterWrite: { mode: "auto" }, - mutate: (draft) => { - if (isConfiguredAgent(draft, params.agentId)) { - throw new AgentConfigPreconditionError("already-exists", params.agentId); - } - const latestNextConfig = applyAgentConfig(draft, { - agentId: params.agentId, - name: params.name, - workspace: params.workspace, - model: params.model, - identity: params.identity, - agentDir: params.agentDir, - }); - Object.assign(draft, latestNextConfig); - }, - }); -} - /** Updates an existing agent entry while preserving omitted fields. */ export async function updateAgentConfigEntry(params: { agentId: string; @@ -79,7 +39,7 @@ export async function updateAgentConfigEntry(params: { afterWrite: { mode: "auto" }, mutate: (draft) => { if (!isConfiguredAgent(draft, params.agentId)) { - throw new AgentConfigPreconditionError("not-found", params.agentId); + throw new AgentConfigPreconditionError(`agent "${params.agentId}" not found`); } const latestNextConfig = applyAgentConfig(draft, { agentId: params.agentId, @@ -102,7 +62,7 @@ export async function deleteAgentConfigEntry(params: { agentId: string }): Promi afterWrite: { mode: "auto" }, mutate: (draft) => { if (!isConfiguredAgent(draft, params.agentId)) { - throw new AgentConfigPreconditionError("not-found", params.agentId); + throw new AgentConfigPreconditionError(`agent "${params.agentId}" not found`); } const workspaceDir = resolveAgentWorkspaceDir(draft, params.agentId); const agentDir = resolveAgentDir(draft, params.agentId); diff --git a/src/gateway/server-methods/agents-mutate.test.ts b/src/gateway/server-methods/agents-mutate.test.ts index cbfe4f924b3..cd4a8cb466a 100644 --- a/src/gateway/server-methods/agents-mutate.test.ts +++ b/src/gateway/server-methods/agents-mutate.test.ts @@ -20,7 +20,9 @@ const mocks = vi.hoisted(() => ({ ensureAgentWorkspace: vi.fn( async (params?: { dir?: string }): Promise<{ dir: string; identityPathCreated: boolean }> => ({ dir: params?.dir - ? `/resolved${params.dir.startsWith("/") ? "" : "/"}${params.dir}` + ? params.dir.startsWith("/resolved/") + ? params.dir + : `/resolved${params.dir.startsWith("/") ? "" : "/"}${params.dir}` : "/resolved/workspace", identityPathCreated: false, }), @@ -101,6 +103,32 @@ vi.mock("../../config/config.js", async () => { followUp: { action: "none" }, }; }, + transformConfigFileWithRetry: async (params: { + transform: ( + config: Record, + context: unknown, + ) => Promise<{ nextConfig: Record; result?: unknown }>; + }) => { + const transformed = await params.transform(structuredClone(mocks.loadConfigReturn), { + snapshot: { path: "/tmp/openclaw/config.json" }, + previousHash: "test-hash", + attempt: 0, + }); + await mocks.writeConfigFile(transformed.nextConfig); + mocks.loadConfigReturn = transformed.nextConfig; + return { + path: "/tmp/openclaw/config.json", + previousHash: "test-hash", + persistedHash: "persisted-hash", + snapshot: { path: "/tmp/openclaw/config.json" }, + nextConfig: transformed.nextConfig, + result: transformed.result, + attempts: 1, + afterWrite: { mode: "auto" }, + followUp: { action: "none" }, + }; + }, + withConfigMutationExclusive: async (fn: () => Promise) => await fn(), }; }); @@ -528,7 +556,6 @@ describe("agents.create", () => { beforeEach(() => { vi.clearAllMocks(); mocks.loadConfigReturn = {}; - mocks.findAgentEntryIndex.mockReturnValue(-1); }); it("creates a new agent successfully", async () => { @@ -547,7 +574,20 @@ describe("agents.create", () => { expect(mocks.writeConfigFile).toHaveBeenCalled(); }); - it("ensures workspace is set up before writing config", async () => { + it("defaults an omitted workspace", async () => { + const { respond, promise } = makeCall("agents.create", { name: "Test Agent" }); + + await promise; + + expect(mocks.resolveAgentWorkspaceDir).toHaveBeenCalledWith(expect.any(Object), "test-agent"); + expectRespondOk(respond, { + ok: true, + agentId: "test-agent", + workspace: "/resolved/workspace/test-agent", + }); + }); + + it("sets up the workspace before publishing agent config", async () => { const callOrder: string[] = []; mocks.ensureAgentWorkspace.mockImplementation(async () => { callOrder.push("ensureAgentWorkspace"); @@ -605,23 +645,6 @@ describe("agents.create", () => { expect(mocks.writeConfigFile).not.toHaveBeenCalled(); }); - it("returns an invalid request when a concurrent create wins the config race", async () => { - let findCallCount = 0; - mocks.findAgentEntryIndex.mockImplementation(() => { - findCallCount += 1; - return findCallCount >= 2 ? 0 : -1; - }); - - const { respond, promise } = makeCall("agents.create", { - name: "Race Agent", - workspace: "/tmp/ws", - }); - await promise; - - expectRespondErrorContaining(respond, "already exists"); - expect(mocks.writeConfigFile).not.toHaveBeenCalled(); - }); - it("rejects invalid params (missing name)", async () => { const { respond, promise } = makeCall("agents.create", { workspace: "/tmp/ws", @@ -678,7 +701,7 @@ describe("agents.create", () => { ); }); - it("does not persist config when IDENTITY.md write fails with FsSafeError", async () => { + it("does not publish config when IDENTITY.md write fails with FsSafeError", async () => { mocks.rootWrite.mockRejectedValueOnce( new FsSafeError("path-mismatch", "path escapes workspace root"), ); @@ -691,67 +714,7 @@ describe("agents.create", () => { expectRespondErrorContaining(respond, "unsafe workspace file"); expect(mocks.writeConfigFile).not.toHaveBeenCalled(); - }); - - it("does not persist config when IDENTITY.md read fails", async () => { - agentsTesting.setDepsForTests({ - root: makeRootForTest({ - read: async () => { - throw createErrnoError("EACCES"); - }, - }), - }); - mocks.ensureAgentWorkspace.mockResolvedValueOnce({ - dir: "/resolved/tmp/ws", - identityPathCreated: false, - }); - - const { promise } = makeCall("agents.create", { - name: "Unreadable Identity", - workspace: "/tmp/ws", - }); - - await expect(promise).rejects.toHaveProperty("code", "EACCES"); - expect(mocks.writeConfigFile).not.toHaveBeenCalled(); - expect(mocks.rootWrite).not.toHaveBeenCalled(); - }); - - it("treats unsafe IDENTITY.md reads as invalid create requests", async () => { - agentsTesting.setDepsForTests({ - root: makeRootForTest({ - read: async () => { - throw new FsSafeError("invalid-path", "path is not a regular file under root"); - }, - }), - }); - - const { respond, promise } = makeCall("agents.create", { - name: "Unsafe Identity Read", - workspace: "/tmp/ws", - }); - await promise; - - expectRespondErrorContaining(respond, 'unsafe workspace file "IDENTITY.md"'); - expect(mocks.writeConfigFile).not.toHaveBeenCalled(); - expect(mocks.rootWrite).not.toHaveBeenCalled(); - }); - - it("uses non-blocking reads for IDENTITY.md during agents.create", async () => { - const rootRead = vi.fn(async () => { - throw new FsSafeError("not-found", "file not found"); - }); - agentsTesting.setDepsForTests({ root: makeRootForTest({ read: rootRead }) }); - - const { promise } = makeCall("agents.create", { - name: "NB Agent", - workspace: "/tmp/ws", - }); - await promise; - - expectRecordFields(mockCallArg(rootRead), { - relativePath: "IDENTITY.md", - nonBlockingRead: true, - }); + expect(getAgentList(mocks.loadConfigReturn)).toEqual([]); }); it("passes model to applyAgentConfig when provided", async () => { diff --git a/src/gateway/server-methods/agents.ts b/src/gateway/server-methods/agents.ts index e5c68b4aaa1..4452b19dbc3 100644 --- a/src/gateway/server-methods/agents.ts +++ b/src/gateway/server-methods/agents.ts @@ -15,13 +15,15 @@ import { validateAgentsListParams, validateAgentsUpdateParams, } from "../../../packages/gateway-protocol/src/index.js"; +import { createAgent } from "../../agents/agent-create.js"; import { findOverlappingWorkspaceAgentIds } from "../../agents/agent-delete-safety.js"; +import { listAgentIds, resolveAgentWorkspaceDir } from "../../agents/agent-scope.js"; import { - listAgentIds, - resolveAgentDir, - resolveAgentWorkspaceDir, -} from "../../agents/agent-scope.js"; -import { mergeIdentityMarkdownContent } from "../../agents/identity-file.js"; + createAgentIdentityConfig, + mergeIdentityMarkdownContent, + normalizeIdentityForFile, + sanitizeAgentIdentityLine, +} from "../../agents/identity-file.js"; import { resolveAgentIdentity } from "../../agents/identity.js"; import { prepareLegacyWorkspaceStateReset, @@ -44,21 +46,16 @@ import { isWorkspaceSetupCompleted, } from "../../agents/workspace.js"; import { applyAgentConfig } from "../../commands/agents.config.js"; -import { - purgeAgentSessionStoreEntries, - resolveSessionTranscriptsDirForAgent, -} from "../../config/sessions.js"; +import { purgeAgentSessionStoreEntries } from "../../config/sessions.js"; import type { IdentityConfig } from "../../config/types.base.js"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; import { root, FsSafeError, type ReadResult } from "../../infra/fs-safe.js"; import { movePathToTrash } from "../../plugin-sdk/browser-maintenance.js"; import { DEFAULT_AGENT_ID, normalizeAgentId } from "../../routing/session-key.js"; -import { isReservedSystemAgentId } from "../../system-agent/agent-id.js"; import { resolveUserPath } from "../../utils.js"; import { listAgentsForGateway } from "../session-utils.js"; import { AgentConfigPreconditionError, - createAgentConfigEntry, deleteAgentConfigEntry, isConfiguredAgent, updateAgentConfigEntry, @@ -281,10 +278,6 @@ function resolveAgentIdOrError(agentIdRaw: string, cfg: OpenClawConfig) { return agentId; } -function sanitizeIdentityLine(value: string): string { - return value.replace(/\s+/g, " ").trim(); -} - function respondInvalidMethodParams( respond: RespondFn, method: string, @@ -304,21 +297,6 @@ function respondAgentNotFound(respond: RespondFn, agentId: string): void { respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, `agent "${agentId}" not found`)); } -function respondAgentConfigPreconditionError( - respond: RespondFn, - error: AgentConfigPreconditionError, -): void { - if (error.kind === "not-found") { - respondAgentNotFound(respond, error.agentId); - return; - } - respond( - false, - undefined, - errorShape(ErrorCodes.INVALID_REQUEST, `agent "${error.agentId}" already exists`), - ); -} - type AgentDeleteRemovedPath = { path: string; method: "trash" | "missing"; @@ -400,55 +378,6 @@ async function writeWorkspaceFileOrRespond(params: { return true; } -function normalizeIdentityForFile( - identity: IdentityConfig | undefined, -): IdentityConfig | undefined { - if (!identity) { - return undefined; - } - const resolved = { - name: identity.name?.trim() || undefined, - theme: identity.theme?.trim() || undefined, - emoji: identity.emoji?.trim() || undefined, - avatar: identity.avatar?.trim() || undefined, - } satisfies IdentityConfig; - if (!resolved.name && !resolved.theme && !resolved.emoji && !resolved.avatar) { - return undefined; - } - return resolved; -} - -function createAgentIdentityConfig(params: { - safeName?: string; - emoji?: unknown; - avatar?: unknown; -}): IdentityConfig | undefined { - const emoji = resolveOptionalStringParam(params.emoji); - const avatar = resolveOptionalStringParam(params.avatar); - const identity = { - ...(params.safeName ? { name: params.safeName } : {}), - ...(emoji ? { emoji: sanitizeIdentityLine(emoji) } : {}), - ...(avatar ? { avatar: sanitizeIdentityLine(avatar) } : {}), - } satisfies IdentityConfig; - return identity.name || identity.emoji || identity.avatar ? identity : undefined; -} - -function buildAgentConfigUpdate(params: { - agentId: string; - safeName?: string; - workspaceDir?: string; - model?: string | null; - identity?: IdentityConfig; -}): Parameters[0] { - return { - agentId: params.agentId, - ...(params.safeName ? { name: params.safeName } : {}), - ...(params.workspaceDir ? { workspace: params.workspaceDir } : {}), - ...(params.model !== undefined ? { model: params.model } : {}), - ...(params.identity ? { identity: params.identity } : {}), - }; -} - async function readWorkspaceFileContent( workspaceDir: string, name: string, @@ -529,100 +458,34 @@ export const agentsHandlers: GatewayRequestHandlers = { const result = listAgentsForGateway(cfg, modelCatalog); respond(true, result, undefined); }, - "agents.create": async ({ params, respond, context }) => { + "agents.create": async ({ params, respond }) => { if (!validateAgentsCreateParams(params)) { respondInvalidMethodParams(respond, "agents.create", validateAgentsCreateParams.errors); return; } - const cfg = context.getRuntimeConfig(); - const rawName = params.name.trim(); - const agentId = normalizeAgentId(rawName); - if (agentId === DEFAULT_AGENT_ID || isReservedSystemAgentId(agentId)) { - respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, `"${agentId}" is reserved`)); - return; - } - - if (isConfiguredAgent(cfg, agentId)) { - respond( - false, - undefined, - errorShape(ErrorCodes.INVALID_REQUEST, `agent "${agentId}" already exists`), - ); - return; - } - - const workspaceDir = resolveUserPath(params.workspace.trim()); - - const safeName = sanitizeIdentityLine(rawName); - const model = resolveOptionalStringParam(params.model); - const identity = createAgentIdentityConfig({ - safeName, + const result = await createAgent({ + name: params.name, + workspace: params.workspace, + model: params.model, emoji: params.emoji, avatar: params.avatar, - }) ?? { name: safeName }; - - // Resolve agentDir against the config we're about to persist (vs the pre-write config), - // so subsequent resolutions can't disagree about the agent's directory. - let nextConfig = applyAgentConfig(cfg, { - agentId, - name: safeName, - workspace: workspaceDir, - model, - identity, }); - const agentDir = resolveAgentDir(nextConfig, agentId); - nextConfig = applyAgentConfig(nextConfig, { agentId, agentDir }); - - // Ensure workspace & transcripts exist BEFORE writing config so a failure - // here does not leave a broken config entry behind. - const skipBootstrap = Boolean(nextConfig.agents?.defaults?.skipBootstrap); - await ensureAgentWorkspace({ - dir: workspaceDir, - ensureBootstrapFiles: !skipBootstrap, - skipOptionalBootstrapFiles: nextConfig.agents?.defaults?.skipOptionalBootstrapFiles, - }); - await fs.mkdir(resolveSessionTranscriptsDirForAgent(agentId), { recursive: true }); - - const persistedIdentity = normalizeIdentityForFile(resolveAgentIdentity(nextConfig, agentId)); - if (persistedIdentity) { - const identityContent = await buildIdentityMarkdownOrRespondUnsafe({ - respond, - workspaceDir, - identity: persistedIdentity, - }); - if (identityContent === null) { - return; - } - if ( - !(await writeWorkspaceFileOrRespond({ - respond, - workspaceDir, - name: DEFAULT_IDENTITY_FILENAME, - content: identityContent, - })) - ) { - return; - } + if (result.status === "error") { + respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, result.message)); + return; } - try { - await createAgentConfigEntry({ - agentId, - name: safeName, - workspace: workspaceDir, - model, - identity, - agentDir, - }); - } catch (error) { - if (error instanceof AgentConfigPreconditionError) { - respondAgentConfigPreconditionError(respond, error); - return; - } - throw error; - } - - respond(true, { ok: true, agentId, name: safeName, workspace: workspaceDir, model }, undefined); + respond( + true, + { + ok: true, + agentId: result.agentId, + name: result.name, + workspace: result.workspace, + ...(result.model ? { model: result.model } : {}), + }, + undefined, + ); }, "agents.update": async ({ params, respond, context }) => { if (!validateAgentsUpdateParams(params)) { @@ -646,23 +509,23 @@ export const agentsHandlers: GatewayRequestHandlers = { const safeName = typeof params.name === "string" && params.name.trim() - ? sanitizeIdentityLine(params.name.trim()) + ? sanitizeAgentIdentityLine(params.name.trim()) : undefined; const identity = createAgentIdentityConfig({ - safeName, + name: safeName, emoji: params.emoji, avatar: params.avatar, }); const hasIdentityFields = Boolean(identity); - const agentConfigUpdate = buildAgentConfigUpdate({ + const agentConfigUpdate: Parameters[0] = { agentId, - safeName, - workspaceDir, - model, - identity, - }); + ...(safeName ? { name: safeName } : {}), + ...(workspaceDir ? { workspace: workspaceDir } : {}), + ...(model !== undefined ? { model } : {}), + ...(identity ? { identity } : {}), + }; const nextConfig = applyAgentConfig(cfg, agentConfigUpdate); let ensuredWorkspace: Awaited> | undefined; @@ -710,7 +573,7 @@ export const agentsHandlers: GatewayRequestHandlers = { await updateAgentConfigEntry(agentConfigUpdate); } catch (error) { if (error instanceof AgentConfigPreconditionError) { - respondAgentConfigPreconditionError(respond, error); + respondAgentNotFound(respond, agentId); return; } throw error; @@ -745,7 +608,7 @@ export const agentsHandlers: GatewayRequestHandlers = { committed = await deleteAgentConfigEntry({ agentId }); } catch (error) { if (error instanceof AgentConfigPreconditionError) { - respondAgentConfigPreconditionError(respond, error); + respondAgentNotFound(respond, agentId); return; } throw error; diff --git a/src/gateway/server-methods/system-agent.test.ts b/src/gateway/server-methods/system-agent.test.ts index 7651aaea940..a308533b8f0 100644 --- a/src/gateway/server-methods/system-agent.test.ts +++ b/src/gateway/server-methods/system-agent.test.ts @@ -870,7 +870,7 @@ describe("openclaw.chat", () => { text: "Your agent is hatching.", action: "open-tui", agentDraft: "hatch", - handoff: { kind: "open-tui" }, + handoff: { kind: "open-tui", agentId: "researcher" }, }); const sessions = new Map([["s1", seededSession({ engine })]]); @@ -879,7 +879,11 @@ describe("openclaw.chat", () => { message: "yes", }); - expect(call.payload).toMatchObject({ action: "open-agent", agentDraft: "hatch" }); + expect(call.payload).toMatchObject({ + action: "open-agent", + agentDraft: "hatch", + agentId: "researcher", + }); }); it("resets a session on request", async () => { diff --git a/src/gateway/server-methods/system-agent.ts b/src/gateway/server-methods/system-agent.ts index 5112ab71bdf..cedb03983fb 100644 --- a/src/gateway/server-methods/system-agent.ts +++ b/src/gateway/server-methods/system-agent.ts @@ -22,6 +22,7 @@ import { defaultRuntime } from "../../runtime.js"; import { SystemAgentChatEngine } from "../../system-agent/chat-engine.js"; import { resolveSystemAgentDelegationKey } from "../../system-agent/delegation-session.js"; import { isSystemAgentInferenceUnavailableError } from "../../system-agent/inference-error.js"; +import { buildNewAgentWelcome } from "../../system-agent/new-agent-welcome.js"; import { buildOnboardingWelcome } from "../../system-agent/onboarding-welcome.js"; import { describeSystemAgentPersistentOperation } from "../../system-agent/operations.js"; import { formatSystemAgentStartupMessage } from "../../system-agent/overview.js"; @@ -488,6 +489,8 @@ export const systemAgentHandlers: GatewayRequestHandlers = { const onboardingWelcome = await buildOnboardingWelcome({ engine }); welcome = onboardingWelcome.text; welcomeQuestion = onboardingWelcome.question; + } else if (params.welcomeVariant === "new-agent") { + welcome = buildNewAgentWelcome({ engine }); } else { welcome = formatSystemAgentStartupMessage(await engine.loadOverview()); engine.noteAssistantMessage(welcome); @@ -594,6 +597,11 @@ export const systemAgentHandlers: GatewayRequestHandlers = { ...(action === "open-agent" && reply.agentDraft ? { agentDraft: reply.agentDraft } : {}), + ...(action === "open-agent" && + reply.handoff?.kind === "open-tui" && + reply.handoff.agentId + ? { agentId: reply.handoff.agentId } + : {}), ...(reply.sensitive === true ? { sensitive: true } : {}), ...(reply.question ? { question: reply.question } : {}), ...(proposalId ? { needsApproval: true, proposalId } : {}), diff --git a/src/system-agent/chat-engine.test.ts b/src/system-agent/chat-engine.test.ts index f65593f18f7..ec35500d389 100644 --- a/src/system-agent/chat-engine.test.ts +++ b/src/system-agent/chat-engine.test.ts @@ -425,6 +425,35 @@ describe("SystemAgentChatEngine", () => { expect(reply.text).toContain("Settings → Ask OpenClaw"); }); + it("hatches into a newly created agent and carries its id", async () => { + useTempStateDir(); + const createAgent = vi.fn(async () => ({ + status: "created" as const, + agentId: "researcher", + name: "researcher", + workspace: "/tmp/researcher", + agentDir: "/tmp/agent-researcher", + bootstrapPending: true, + })); + const engine = new SystemAgentChatEngine({ + runAgentTurn: async () => null, + planWithAssistant: async () => null, + classifyApproval: async ({ message }) => (message === "yes" ? "approve" : "other"), + deps: { createAgent, loadOverview: fakeOverviewLoader() }, + }); + engine.propose({ kind: "create-agent", agentId: "researcher" }); + + const reply = await engine.handle("yes"); + + expect(createAgent).toHaveBeenCalledWith({ name: "researcher" }); + expect(reply.action).toBe("open-tui"); + expect(reply.handoff).toMatchObject({ + kind: "open-tui", + agentId: "researcher", + agentDraft: "hatch", + }); + }); + it("stays in setup when an established workspace has no bootstrap pending", async () => { useTempStateDir(); const applySetup = vi.fn(async () => ({ diff --git a/src/system-agent/chat-engine.ts b/src/system-agent/chat-engine.ts index 82163256c4c..07a98eb965c 100644 --- a/src/system-agent/chat-engine.ts +++ b/src/system-agent/chat-engine.ts @@ -628,14 +628,14 @@ export class SystemAgentChatEngine { const baseText = [capture.read() || "Applied. Audit entry written.", verify, followUp] .filter(Boolean) .join("\n\n"); - // The hatch is a ceremony: a fresh-install setup just created the agent, + // The hatch is a ceremony: setup or an explicit creation just seeded the agent, // so hand the user straight into it instead of parking them here. The // seeded BOOTSTRAP runs the birth sequence on the agent's first turn. // Only on clean post-write verification: a non-null verify means the // written config is suspect, and handing off would bury the warning in an // agent session that may not answer — stay in setup to repair it. if ( - operation.kind === "setup" && + (operation.kind === "setup" || operation.kind === "create-agent") && result?.applied && result.bootstrapPending === true && verify === null @@ -651,6 +651,7 @@ export class SystemAgentChatEngine { kind: "open-tui", agentDraft: "hatch", ...(operation.workspace ? { workspace: operation.workspace } : {}), + ...(result.agentId ? { agentId: result.agentId } : {}), }, }; } diff --git a/src/system-agent/new-agent-welcome.test.ts b/src/system-agent/new-agent-welcome.test.ts new file mode 100644 index 00000000000..3c815ff99ed --- /dev/null +++ b/src/system-agent/new-agent-welcome.test.ts @@ -0,0 +1,16 @@ +import { describe, expect, it, vi } from "vitest"; +import { buildNewAgentWelcome } from "./new-agent-welcome.js"; + +describe("buildNewAgentWelcome", () => { + it("starts a purpose-and-name creation conversation", () => { + const noteAssistantMessage = vi.fn(); + + const welcome = buildNewAgentWelcome({ engine: { noteAssistantMessage } as never }); + + expect(welcome).toContain("What should it be called"); + expect(welcome).toContain("what kind of work is it for"); + expect(welcome).toContain("learn its role during hatch"); + expect(welcome).toContain("approval"); + expect(noteAssistantMessage).toHaveBeenCalledWith(welcome); + }); +}); diff --git a/src/system-agent/new-agent-welcome.ts b/src/system-agent/new-agent-welcome.ts new file mode 100644 index 00000000000..9a3a951467b --- /dev/null +++ b/src/system-agent/new-agent-welcome.ts @@ -0,0 +1,8 @@ +import type { SystemAgentChatEngine } from "./chat-engine.js"; + +export function buildNewAgentWelcome(params: { engine: SystemAgentChatEngine }): string { + const welcome = + "Let's hatch a new agent. What should it be called, and what kind of work is it for? I'll use that context to settle its name, then propose creation for your approval. The new agent will learn its role during hatch."; + params.engine.noteAssistantMessage(welcome); + return welcome; +} diff --git a/src/system-agent/operations-execute.ts b/src/system-agent/operations-execute.ts index 37b979c490d..9d149210484 100644 --- a/src/system-agent/operations-execute.ts +++ b/src/system-agent/operations-execute.ts @@ -1,5 +1,6 @@ // Public operation dispatcher. Parsing and mutation helpers live in focused modules. import { truncateUtf16Safe } from "@openclaw/normalization-core/utf16-slice"; +import { createAgent } from "../agents/agent-create.js"; import { buildAgentMainSessionKey, normalizeAgentId } from "../routing/session-key.js"; import type { RuntimeEnv } from "../runtime.js"; import { resolveUserPath, shortenHomePath } from "../utils.js"; @@ -360,32 +361,28 @@ export async function executeSystemAgentOperation( "OpenClaw cannot save an explicit per-agent model until that new route can be live-tested. Retry without `model`; the new agent inherits the verified default, then use `set_default_model` with agentId to live-test and save its own model.", ); } - const workspace = resolveUserPath(operation.workspace ?? process.cwd()); return await applyPersistentOperation({ auditOperation: "agents.create", operation, runtime, opts, run: async (ctx) => { - const runAgentsAdd = - ctx.deps?.runAgentsAdd ?? - (await import("../commands/agents.commands.add.js")).agentsAddCommand; - await ctx.commit(async () => { - await runAgentsAdd( - { - name: operation.agentId, - workspace, - nonInteractive: true, - }, - ctx.runtime, - { hasFlags: true }, - ); + const result = await ctx.commit(async () => { + return await (ctx.deps?.createAgent ?? createAgent)({ + name: operation.agentId, + ...(operation.workspace ? { workspace: operation.workspace } : {}), + }); }); + if (result.status === "error") { + throw new Error(result.message); + } return { - summary: `Created agent ${operation.agentId}`, + summary: `Created agent ${result.agentId}`, + bootstrapPending: result.bootstrapPending, + agentId: result.agentId, details: { - agentId: operation.agentId, - workspace, + agentId: result.agentId, + workspace: result.workspace, }, }; }, diff --git a/src/system-agent/operations-execution-helpers.ts b/src/system-agent/operations-execution-helpers.ts index 63fde48307f..55022a1aa8e 100644 --- a/src/system-agent/operations-execution-helpers.ts +++ b/src/system-agent/operations-execution-helpers.ts @@ -237,6 +237,7 @@ type PersistentApplyContext = { type PersistentApplyOutcome = { summary: string; bootstrapPending?: boolean; + agentId?: string; details?: Record; /** Overrides the after-snapshot config path in the audit record. */ configPath?: string; @@ -286,6 +287,7 @@ export async function applyPersistentOperation(params: { ...(outcome.bootstrapPending === undefined ? {} : { bootstrapPending: outcome.bootstrapPending }), + ...(outcome.agentId ? { agentId: outcome.agentId } : {}), }; } diff --git a/src/system-agent/operations-parse.ts b/src/system-agent/operations-parse.ts index 18b8ae9bd96..12321676e8b 100644 --- a/src/system-agent/operations-parse.ts +++ b/src/system-agent/operations-parse.ts @@ -19,8 +19,10 @@ export type { SystemAgentOperation }; /** Result returned by the operation executor. */ export type SystemAgentOperationResult = { applied: boolean; - /** Setup created or preserved BOOTSTRAP.md for the agent's first turn. */ + /** Creation created or preserved BOOTSTRAP.md for the agent's first turn. */ bootstrapPending?: boolean; + /** Agent created by this operation, when applicable. */ + agentId?: string; exitsInteractive?: boolean; message?: string; nextInput?: string; @@ -37,17 +39,7 @@ export type SystemAgentCommandDeps = { resolveApiKeyForProvider?: typeof import("../agents/model-auth.js").resolveApiKeyForProvider; formatOverview?: SystemAgentOverviewFormatter; loadOverview?: SystemAgentOverviewLoader; - runAgentsAdd?: ( - opts: { - name?: string; - workspace?: string; - model?: string; - nonInteractive?: boolean; - json?: boolean; - }, - runtime: RuntimeEnv, - params?: { hasFlags?: boolean }, - ) => Promise; + createAgent?: typeof import("../agents/agent-create.js").createAgent; runConfigSet?: (opts: { path?: string; value?: string; diff --git a/src/system-agent/operations.test.ts b/src/system-agent/operations.test.ts index ba5fce87bd0..4f52e00d4ee 100644 --- a/src/system-agent/operations.test.ts +++ b/src/system-agent/operations.test.ts @@ -493,7 +493,7 @@ describe("parseSystemAgentOperation", () => { const tempDir = opTempDirs.make("openclaw-agent-model-rejected-"); setTestEnvValue("OPENCLAW_STATE_DIR", tempDir); const { runtime, lines } = createSystemAgentTestRuntime(); - const runAgentsAdd = vi.fn(async () => {}); + const createAgent = vi.fn(); expect( isPersistentSystemAgentOperation({ kind: "create-agent", @@ -512,11 +512,11 @@ describe("parseSystemAgentOperation", () => { model: "openai/gpt-5.5", }, runtime, - { approved: true, deps: { runAgentsAdd } }, + { approved: true, deps: { createAgent } }, ), ).rejects.toThrow("Retry without `model`; the new agent inherits"); - expect(runAgentsAdd).not.toHaveBeenCalled(); + expect(createAgent).not.toHaveBeenCalled(); expect(lines.join("\n")).not.toContain("[openclaw] running: agents.create"); await expect(fs.access(path.join(tempDir, "audit", "system-agent.jsonl"))).rejects.toThrow(); }); @@ -525,7 +525,7 @@ describe("parseSystemAgentOperation", () => { const tempDir = opTempDirs.make("openclaw-agent-id-reserved-"); setTestEnvValue("OPENCLAW_STATE_DIR", tempDir); const { runtime, lines } = createSystemAgentTestRuntime(); - const runAgentsAdd = vi.fn(async () => {}); + const createAgent = vi.fn(); const operation = { kind: "create-agent" as const, agentId: "OpenClaw", @@ -536,18 +536,18 @@ describe("parseSystemAgentOperation", () => { await expect( executeSystemAgentOperation(operation, runtime, { approved: true, - deps: { runAgentsAdd }, + deps: { createAgent }, }), ).rejects.toThrow('Agent id "openclaw" is reserved'); - expect(runAgentsAdd).not.toHaveBeenCalled(); + expect(createAgent).not.toHaveBeenCalled(); expect(lines.join("\n")).not.toContain("[openclaw] running: agents.create"); await expect(fs.access(path.join(tempDir, "audit", "system-agent.jsonl"))).rejects.toThrow(); }); it("keeps the retired agent identity reserved", async () => { const { runtime } = createSystemAgentTestRuntime(); - const runAgentsAdd = vi.fn(async () => {}); + const createAgent = vi.fn(); const operation = { kind: "create-agent" as const, agentId: "crestodian", // reserved retired id @@ -558,10 +558,10 @@ describe("parseSystemAgentOperation", () => { await expect( executeSystemAgentOperation(operation, runtime, { approved: true, - deps: { runAgentsAdd }, + deps: { createAgent }, }), ).rejects.toThrow('Agent id "crestodian" is reserved'); // reserved retired id - expect(runAgentsAdd).not.toHaveBeenCalled(); + expect(createAgent).not.toHaveBeenCalled(); }); it("requires approval before restarting gateway", async () => { diff --git a/src/system-agent/rescue-message.test.ts b/src/system-agent/rescue-message.test.ts index acb09d8ceba..c17438a06d3 100644 --- a/src/system-agent/rescue-message.test.ts +++ b/src/system-agent/rescue-message.test.ts @@ -663,7 +663,16 @@ describe("OpenClaw rescue message", () => { it("queues and applies agent creation through conversational approval", async () => { await withRescueStateDir("agent-", async () => { const cfg: OpenClawConfig = { systemAgent: { rescue: { enabled: true } } }; - const deps = { runAgentsAdd: vi.fn(async () => {}) }; + const deps = { + createAgent: vi.fn(async () => ({ + status: "created" as const, + agentId: "work", + name: "work", + workspace: "/tmp/work", + agentDir: "/tmp/agent-work", + bootstrapPending: true, + })), + }; await expect( runRescue("/openclaw create agent work workspace /tmp/work", cfg, commandContext(), deps), @@ -674,22 +683,14 @@ describe("OpenClaw rescue message", () => { "[openclaw] done: agents.create", ); - expect(deps.runAgentsAdd).toHaveBeenCalledTimes(1); - const [agentParams, agentRuntime, agentOptions] = requireFirstMockCall( - deps.runAgentsAdd, - "agents add", - ) as unknown as [ - { name: string; workspace: string; nonInteractive: boolean }, - object, - { hasFlags: boolean }, + expect(deps.createAgent).toHaveBeenCalledTimes(1); + const [agentParams] = requireFirstMockCall(deps.createAgent, "agents add") as unknown as [ + { name: string; workspace: string }, ]; expect(agentParams).toEqual({ name: "work", workspace: "/tmp/work", - nonInteractive: true, }); - expect(agentRuntime).toBeTypeOf("object"); - expect(agentOptions).toEqual({ hasFlags: true }); const audit = readLastAuditEntry() as { operation?: string; details?: { diff --git a/ui/src/components/agent-select.test.ts b/ui/src/components/agent-select.test.ts index c831dfdc905..854a74713e5 100644 --- a/ui/src/components/agent-select.test.ts +++ b/ui/src/components/agent-select.test.ts @@ -18,6 +18,7 @@ type AgentSelectElement = HTMLElement & { authToken: string | null; disabled: boolean; onSelect: (agentId: string) => void; + onCreateAgent: () => void; updateComplete: Promise; }; @@ -351,7 +352,7 @@ it("renders the agent picker as a Web Awesome dropdown", async () => { const dropdown = element.querySelector("wa-dropdown"); const options = Array.from( element.querySelectorAll( - "wa-dropdown-item", + "wa-dropdown-item[data-agent-id]", ), ); expect(dropdown).not.toBeNull(); @@ -419,12 +420,28 @@ it("selects a different agent and ignores the already-selected agent", async () } }); -it("renders a disabled trigger with the empty-state label", async () => { +it("opens the new-agent flow from the footer item", async () => { + const onCreateAgent = vi.fn(); + const element = await createAgentSelect({ onCreateAgent }); + + try { + const item = element.querySelector("[data-create-agent]"); + element + .querySelector("wa-dropdown") + ?.dispatchEvent(new CustomEvent("wa-select", { detail: { item }, bubbles: true })); + + expect(onCreateAgent).toHaveBeenCalledOnce(); + } finally { + element.remove(); + } +}); + +it("keeps the new-agent footer reachable with an empty roster", async () => { const element = await createAgentSelect({ agents: [], selectedId: null }); try { const trigger = element.querySelector(".agent-select__trigger"); - expect(trigger?.disabled).toBe(true); + expect(trigger?.disabled).toBe(false); expect(element.querySelector(".agent-select__label")?.textContent?.trim()).toBe("No agents"); } finally { element.remove(); diff --git a/ui/src/components/agent-select.ts b/ui/src/components/agent-select.ts index 20e5f7146ee..98ec4ea09b7 100644 --- a/ui/src/components/agent-select.ts +++ b/ui/src/components/agent-select.ts @@ -19,7 +19,6 @@ type AvatarFetch = { authToken: string; controller: AbortController }; /** Bound local avatar fetches so a stalled Control UI media route cannot pin pending state forever. */ const AGENT_SELECT_AVATAR_FETCH_TIMEOUT_MS = 30_000; - export class AgentSelect extends OpenClawLightDomElement { @property({ attribute: false }) agents: GatewayAgentRow[] = []; @property({ attribute: false }) selectedId: string | null = null; @@ -28,6 +27,7 @@ export class AgentSelect extends OpenClawLightDomElement { @property({ attribute: false }) authToken: string | null = null; @property({ attribute: false }) disabled = false; @property({ attribute: false }) onSelect: (agentId: string) => void = () => {}; + @property({ attribute: false }) onCreateAgent: () => void = () => {}; private readonly avatarBlobUrlByRoute = new Map(); private readonly avatarFetchByRoute = new Map(); @@ -144,6 +144,10 @@ export class AgentSelect extends OpenClawLightDomElement { private readonly handleSelect = (event: WebAwesomeSelectEvent) => { const item = event.detail.item as HTMLElement & { checked?: boolean; value?: string }; + if (item.hasAttribute("data-create-agent")) { + this.onCreateAgent(); + return; + } const agentId = item.value ?? item.getAttribute("value"); if (!agentId) { return; @@ -165,7 +169,7 @@ export class AgentSelect extends OpenClawLightDomElement { this.agents.find((agent) => agent.id === this.defaultId) ?? this.agents[0]; const selectedBadge = selectedAgent ? agentBadgeText(selectedAgent.id, this.defaultId) : null; - const unavailable = this.disabled || this.agents.length === 0; + const unavailable = this.disabled; return html` @@ -200,6 +204,11 @@ export class AgentSelect extends OpenClawLightDomElement { `; })} + + + ${icons.users} + ${t("custodian.newAgent")} + `; } diff --git a/ui/src/components/app-sidebar-agent-menu.ts b/ui/src/components/app-sidebar-agent-menu.ts index 06d0d423eb3..918610d00f8 100644 --- a/ui/src/components/app-sidebar-agent-menu.ts +++ b/ui/src/components/app-sidebar-agent-menu.ts @@ -224,6 +224,9 @@ export function renderSidebarAgentMenu(params: SidebarAgentMenuParams) { case `${COMMAND_VALUE_PREFIX}agent-settings`: params.onNavigate("agents", { search: `?agent=${encodeURIComponent(activeId)}` }); break; + case `${COMMAND_VALUE_PREFIX}new-agent`: + params.onNavigate("custodian", { search: "?intent=new-agent" }); + break; case `${COMMAND_VALUE_PREFIX}settings`: params.onNavigate("config"); break; @@ -306,6 +309,10 @@ export function renderSidebarAgentMenu(params: SidebarAgentMenuParams) { ` : nothing} + + + ${t("custodian.newAgent")} + togglePinnedAgent(this.context.navigation, agentId), onRefresh: () => this.refreshAgents(), onSelectAgent: (agentId) => this.selectAgent(agentId), + onCreateAgent: () => this.context.navigate("custodian", { search: "?intent=new-agent" }), onSelectPanel: (panel) => this.selectPanel(panel), onLoadFiles: (agentId) => void this.loadAgentFiles(agentId, true), onSelectFile: (name) => { diff --git a/ui/src/pages/agents/view.test.ts b/ui/src/pages/agents/view.test.ts index 446669c3c09..423137cc612 100644 --- a/ui/src/pages/agents/view.test.ts +++ b/ui/src/pages/agents/view.test.ts @@ -125,6 +125,7 @@ function createProps(overrides: Partial = {}): AgentsProps { pinnedAgentIds: [], onRefresh: () => undefined, onSelectAgent: () => undefined, + onCreateAgent: () => undefined, onSelectPanel: () => undefined, onLoadFiles: () => undefined, onSelectFile: () => undefined, diff --git a/ui/src/pages/agents/view.ts b/ui/src/pages/agents/view.ts index 78793cf1690..457e57fdc41 100644 --- a/ui/src/pages/agents/view.ts +++ b/ui/src/pages/agents/view.ts @@ -110,6 +110,7 @@ type AgentsProps = { onTogglePinnedAgent: (agentId: string) => void; onRefresh: () => void; onSelectAgent: (agentId: string) => void; + onCreateAgent: () => void; onSelectPanel: (panel: AgentsPanel) => void; onLoadFiles: (agentId: string) => void; onSelectFile: (name: string) => void; @@ -175,6 +176,7 @@ export function renderAgents(props: AgentsProps) { .authToken=${props.authToken} .disabled=${props.loading} .onSelect=${props.onSelectAgent} + .onCreateAgent=${props.onCreateAgent} >
diff --git a/ui/src/pages/custodian/custodian-page-agent-create.test.ts b/ui/src/pages/custodian/custodian-page-agent-create.test.ts new file mode 100644 index 00000000000..61875cdb34c --- /dev/null +++ b/ui/src/pages/custodian/custodian-page-agent-create.test.ts @@ -0,0 +1,117 @@ +/* @vitest-environment jsdom */ + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { GatewayBrowserClient } from "../../api/gateway.ts"; +import type { + ApplicationContext, + ApplicationGateway, + ApplicationGatewaySnapshot, +} from "../../app/context.ts"; +import { createApplicationContextProvider } from "../../test-helpers/application-context.ts"; +import { waitForFast } from "../../test-helpers/wait-for.ts"; +import "./custodian-page.ts"; + +type TestCustodianPage = HTMLElement & { + onboarding: boolean; + newAgentIntent: boolean; + updateComplete: Promise; +}; + +function createContext(request: ReturnType) { + const client = { request } as unknown as GatewayBrowserClient; + const snapshot: ApplicationGatewaySnapshot = { + client, + connected: true, + reconnecting: false, + hello: { + type: "hello-ok", + protocol: 1, + auth: { role: "operator", scopes: ["operator.admin"] }, + features: { methods: ["openclaw.chat"] }, + }, + assistantAgentId: "main", + sessionKey: "main", + lastError: null, + lastErrorCode: null, + }; + const setSessionKey = vi.fn(); + const gateway = { + snapshot, + connection: { + gatewayUrl: "ws://gateway.test/control", + token: "", + bootstrapToken: "", + password: "", + }, + setSessionKey, + subscribe: () => () => undefined, + subscribeEvents: () => () => undefined, + } as unknown as ApplicationGateway; + const refreshList = vi.fn().mockResolvedValue({ + defaultId: "main", + mainKey: "main", + scope: "global", + agents: [{ id: "main" }, { id: "researcher" }], + }); + const context = { + gateway, + agents: { refreshList }, + basePath: "", + navigate: vi.fn(), + } as unknown as ApplicationContext; + return { context, refreshList, setSessionKey }; +} + +async function mountPage(context: ApplicationContext): Promise { + const provider = createApplicationContextProvider(context); + const page = document.createElement("openclaw-custodian-page") as TestCustodianPage; + page.onboarding = false; + page.newAgentIntent = true; + provider.append(page); + document.body.append(provider); + await page.updateComplete; + return page; +} + +describe("custodian new-agent flow", () => { + beforeEach(() => { + vi.spyOn(crypto, "randomUUID").mockReturnValue("00000000-0000-4000-8000-000000000001"); + }); + + afterEach(() => { + document.body.replaceChildren(); + vi.restoreAllMocks(); + }); + + it("requests the new-agent welcome variant", async () => { + const request = vi.fn().mockResolvedValue({ + sessionId: "control-ui-onboarding-00000000-0000-4000-8000-000000000001", + reply: "What should your new agent do?", + action: "none", + }); + const { context } = createContext(request); + await mountPage(context); + + await waitForFast(() => expect(request).toHaveBeenCalledOnce()); + expect(request.mock.calls[0]?.[1]).toMatchObject({ welcomeVariant: "new-agent" }); + }); + + it("refreshes the roster and opens the created agent hatch session", async () => { + const request = vi.fn().mockResolvedValue({ + sessionId: "control-ui-onboarding-00000000-0000-4000-8000-000000000001", + reply: "Your agent is hatching.", + action: "open-agent", + agentDraft: "hatch", + agentId: "researcher", + }); + const { context, refreshList, setSessionKey } = createContext(request); + await mountPage(context); + + await waitForFast(() => expect(context.navigate).toHaveBeenCalledOnce()); + expect(refreshList).toHaveBeenCalledOnce(); + expect(setSessionKey).toHaveBeenCalledWith("agent:researcher:main"); + expect(context.navigate).toHaveBeenCalledWith("chat", { + search: "?session=agent%3Aresearcher%3Amain&draft=Wake%20up%2C%20my%20friend!", + }); + }); +}); diff --git a/ui/src/pages/custodian/custodian-page.ts b/ui/src/pages/custodian/custodian-page.ts index df45407de3d..16d85152fec 100644 --- a/ui/src/pages/custodian/custodian-page.ts +++ b/ui/src/pages/custodian/custodian-page.ts @@ -10,6 +10,7 @@ import { t } from "../../i18n/index.ts"; import type { MessageGroup } from "../../lib/chat/chat-types.ts"; import { isGatewayMethodAdvertised } from "../../lib/gateway-methods.ts"; import { searchForSession } from "../../lib/sessions/navigation.ts"; +import { buildAgentMainSessionKey } from "../../lib/sessions/session-key.ts"; import { OpenClawLightDomElement } from "../../lit/openclaw-element.ts"; import { SubscriptionsController } from "../../lit/subscriptions-controller.ts"; import "../../styles/chat/grouped.css"; @@ -66,6 +67,9 @@ export class CustodianPage extends OpenClawLightDomElement { /** Onboarding mode shows the Exit setup control; the route view sets this. */ @property({ attribute: false }) onboarding = false; + /** New-agent mode starts a creation proposal conversation. */ + @property({ attribute: false }) newAgentIntent = false; + @state() private messages: CustodianMessage[] = []; @state() private input = ""; @state() private sending = false; @@ -93,7 +97,7 @@ export class CustodianPage extends OpenClawLightDomElement { () => this.context?.gateway, (gateway) => gateway.subscribeEvents((event) => { - if (this.onboarding || this.eventNudgeClosed) { + if (this.onboarding || this.newAgentIntent || this.eventNudgeClosed) { return; } const next = classifyCustodianEventNudge(event); @@ -144,7 +148,17 @@ export class CustodianPage extends OpenClawLightDomElement { private currentSessionScopeKey(): string { // Mode selects the welcome contract, so changing it starts a new session // instead of carrying the previous route's transcript across modes. - return JSON.stringify([this.onboarding, this.connectionScopeKey()]); + return JSON.stringify([this.onboarding, this.newAgentIntent, this.connectionScopeKey()]); + } + + private welcomeVariant(): Pick { + if (this.onboarding) { + return { welcomeVariant: "onboarding" }; + } + if (this.newAgentIntent) { + return { welcomeVariant: "new-agent" }; + } + return {}; } private synchronizeClient(): void { @@ -186,11 +200,11 @@ export class CustodianPage extends OpenClawLightDomElement { this.sessionScopeKey = scopeKey; this.sessionStarted = true; this.clearConversation(); - // The onboarding variant seeds the first-run setup proposal; the permanent + // Route variants seed their dedicated proposal conversation; the permanent // presence surface gets the normal caretaker greeting instead. void this.requestReply(client, { sessionId: this.sessionId, - ...(this.onboarding ? { welcomeVariant: "onboarding" as const } : {}), + ...this.welcomeVariant(), }); } @@ -237,7 +251,18 @@ export class CustodianPage extends OpenClawLightDomElement { this.retryParams = null; this.appendAssistant(result.reply, parseCustodianQuestion(result.question)); if (result.action === "open-agent") { - const sessionKey = this.context.gateway.snapshot.sessionKey?.trim(); + let sessionKey = this.context.gateway.snapshot.sessionKey?.trim(); + if (result.agentId) { + const roster = await this.context.agents.refreshList(); + if (epoch !== this.requestEpoch || client !== this.activeClient) { + return; + } + sessionKey = buildAgentMainSessionKey({ + agentId: result.agentId, + mainKey: roster?.mainKey, + }); + this.context.gateway.setSessionKey(sessionKey); + } if (result.agentDraft === "hatch" && sessionKey) { // Preserve the destination session while preloading the localized // birth-sequence opener; draft-only chat routes are intentionally invalid. @@ -289,7 +314,7 @@ export class CustodianPage extends OpenClawLightDomElement { this.input = ""; void this.requestReply(client, { sessionId: this.sessionId, - ...(this.onboarding ? { welcomeVariant: "onboarding" as const } : {}), + ...this.welcomeVariant(), message, }); } diff --git a/ui/src/pages/custodian/route-view.ts b/ui/src/pages/custodian/route-view.ts index 78c4b5e8c9a..857ed2a3165 100644 --- a/ui/src/pages/custodian/route-view.ts +++ b/ui/src/pages/custodian/route-view.ts @@ -3,6 +3,9 @@ import type { CustodianRouteData } from "./route.ts"; export function renderCustodianRoute(data: CustodianRouteData | undefined) { return html` - + `; } diff --git a/ui/src/pages/custodian/route.test.ts b/ui/src/pages/custodian/route.test.ts index 99a2840576f..e2472c59999 100644 --- a/ui/src/pages/custodian/route.test.ts +++ b/ui/src/pages/custodian/route.test.ts @@ -64,22 +64,23 @@ describe("custodian route", () => { it("keys and resolves route data from onboarding search", () => { const context = {} as ApplicationContext; expect(page.loaderDeps?.(context, location(""))).toBe(""); - expect(loadRoute("")).toEqual({ onboarding: false }); - expect(loadRoute("?onboarding=1")).toEqual({ onboarding: true }); + expect(loadRoute("")).toEqual({ onboarding: false, intent: null }); + expect(loadRoute("?onboarding=1")).toEqual({ onboarding: true, intent: null }); + expect(loadRoute("?intent=new-agent")).toEqual({ onboarding: false, intent: "new-agent" }); }); it("renders Exit setup only in onboarding mode", async () => { const provider = createApplicationContextProvider(createContext()); document.body.append(provider); - render(renderCustodianRoute({ onboarding: false }), provider); + render(renderCustodianRoute({ onboarding: false, intent: null }), provider); const normalPage = provider.querySelector }>( "openclaw-custodian-page", ); await normalPage?.updateComplete; expect(normalPage?.querySelector(".custodian__header > .btn")).toBeNull(); - render(renderCustodianRoute({ onboarding: true }), provider); + render(renderCustodianRoute({ onboarding: true, intent: null }), provider); const onboardingPage = provider.querySelector< HTMLElement & { updateComplete: Promise } >("openclaw-custodian-page"); diff --git a/ui/src/pages/custodian/route.ts b/ui/src/pages/custodian/route.ts index 6c73bf8d49f..22a8f80fa36 100644 --- a/ui/src/pages/custodian/route.ts +++ b/ui/src/pages/custodian/route.ts @@ -5,14 +5,20 @@ import { resolveOnboardingMode } from "../../app/onboarding-mode.ts"; export type CustodianRouteData = { onboarding: boolean; + intent: "new-agent" | null; }; +function resolveCustodianIntent(search: string): CustodianRouteData["intent"] { + return new URLSearchParams(search).get("intent") === "new-agent" ? "new-agent" : null; +} + export const page = definePage({ id: "custodian", path: "/custodian", loaderDeps: (_context: ApplicationContext, location: RouteLocation) => location.search, loader: (_context: ApplicationContext, { location }): CustodianRouteData => ({ onboarding: resolveOnboardingMode(location.search), + intent: resolveCustodianIntent(location.search), }), component: () => import("./custodian-page.ts").then(() => diff --git a/ui/src/styles/components.css b/ui/src/styles/components.css index 4c08d549843..13d405eaaf6 100644 --- a/ui/src/styles/components.css +++ b/ui/src/styles/components.css @@ -3174,6 +3174,12 @@ td.data-table-key-col { font-size: 13px; } +.agent-select__separator { + height: 1px; + margin: 6px 4px; + background: var(--border); +} + .agent-select__option-label { flex: 1; min-width: 0;