diff --git a/src/commands/agents.commands.delete.ts b/src/commands/agents.commands.delete.ts index 3b147e2666ca..a95fc8125033 100644 --- a/src/commands/agents.commands.delete.ts +++ b/src/commands/agents.commands.delete.ts @@ -42,6 +42,7 @@ import { isGatewayCredentialsRequiredError, isGatewayTransportError, } from "../gateway/call.js"; +import { withAgentExecApprovalsRemoved } from "../infra/exec-approvals.js"; import { normalizeAgentId, normalizeAgentIdStrict } from "../routing/session-key.js"; import { defaultRuntime, type RuntimeEnv, writeRuntimeJson } from "../runtime.js"; import { readAgentDeletionJournal } from "../state/agent-deletion-journal.js"; @@ -276,19 +277,21 @@ export async function agentsDeleteCommand( existingJournal ?? { agentId, agentDir, workspaceDir, sessionsDir, deleteFiles }, ); try { - if (configured) { - await replaceConfigFile({ - nextConfig: result.config, - ...(baseHash !== undefined ? { baseHash } : {}), - writeOptions: { - allowedAgentRosterRemovals: [agentId], - ...(opts.json ? { skipOutputLogs: true } : {}), - }, - }); - if (!opts.json) { - logConfigUpdated(runtime); + await withAgentExecApprovalsRemoved(agentId, async () => { + if (configured) { + await replaceConfigFile({ + nextConfig: result.config, + ...(baseHash !== undefined ? { baseHash } : {}), + writeOptions: { + allowedAgentRosterRemovals: [agentId], + ...(opts.json ? { skipOutputLogs: true } : {}), + }, + }); + if (!opts.json) { + logConfigUpdated(runtime); + } } - } + }); deletion.commit(); } catch (error) { if (!existingJournal) { diff --git a/src/commands/agents.config.ts b/src/commands/agents.config.ts index 43b4e10b812c..d91ece91c410 100644 --- a/src/commands/agents.config.ts +++ b/src/commands/agents.config.ts @@ -19,7 +19,7 @@ import { pinSurvivorWorkspaceForRosterCollapse } from "../config/agent-workspace import { listRouteBindings } from "../config/bindings.js"; import type { IdentityConfig } from "../config/types.base.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; -import { normalizeAgentId } from "../routing/session-key.js"; +import { normalizeAgentId, normalizeAgentIdStrict } from "../routing/session-key.js"; export type AgentSummary = { id: string; @@ -207,6 +207,10 @@ export function pruneAgentConfig( } { const id = normalizeAgentId(agentId); const clearedOwnerRefs: string[] = []; + const targetsDeletedAgent = (candidate: string) => { + const normalized = normalizeAgentIdStrict(candidate); + return normalized.ok && normalized.value === id; + }; const clearOwnerRef = (value: T | undefined, path: string) => { const owner = normalizeOptionalString(value?.agentId); if (!value || !owner || normalizeAgentId(owner) !== id) { @@ -220,7 +224,7 @@ export function pruneAgentConfig( const pruneAllowAgents = (allowAgents: string[] | undefined) => allowAgents?.filter((entry) => { const trimmed = entry.trim(); - return !trimmed || trimmed === "*" || normalizeAgentId(trimmed) !== id; + return !trimmed || !targetsDeletedAgent(trimmed); }); const nextAgentsList = []; for (const entry of agents) { @@ -277,6 +281,23 @@ export function pruneAgentConfig( } : undefined; const nextTalk = clearOwnerRef(cfg.talk, "talk.agentId"); + const nextBroadcast = cfg.broadcast + ? Object.fromEntries( + Object.entries(cfg.broadcast).map(([peerId, value]) => [ + peerId, + Array.isArray(value) ? value.filter((entry) => !targetsDeletedAgent(entry)) : value, + ]), + ) + : undefined; + const nextHooks = cfg.hooks + ? { + ...cfg.hooks, + allowedAgentIds: cfg.hooks.allowedAgentIds?.filter((entry) => !targetsDeletedAgent(entry)), + mappings: cfg.hooks.mappings?.filter( + (mapping) => !mapping.agentId || !targetsDeletedAgent(mapping.agentId), + ), + } + : undefined; const { list: _legacyList, ownership: _ownership, ...agentsConfig } = cfg.agents ?? {}; const nextAgentsConfig = cfg.agents ? { @@ -305,6 +326,8 @@ export function pruneAgentConfig( ...cfg, agents: nextAgentsConfig, bindings: filteredBindings.length > 0 ? filteredBindings : undefined, + broadcast: nextBroadcast, + hooks: nextHooks, talk: nextTalk, tools: nextTools, }; diff --git a/src/commands/agents.delete.test.ts b/src/commands/agents.delete.test.ts index 834a4898af62..79adb4c12460 100644 --- a/src/commands/agents.delete.test.ts +++ b/src/commands/agents.delete.test.ts @@ -19,6 +19,7 @@ import { } from "../config/sessions/session-accessor.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import { GatewayTransportError } from "../gateway/transport-error.js"; +import { readExecApprovalsSnapshot, saveExecApprovals } from "../infra/exec-approvals.js"; import { parseAgentSessionKey } from "../routing/session-key.js"; import { readAgentDeletionJournal } from "../state/agent-deletion-journal.js"; import { readAgentProvenance, recordAgentProvenance } from "../state/agent-provenance.js"; @@ -319,6 +320,14 @@ describe("agents delete command", () => { "agent:main:main": { sessionId: "sess-main", updatedAt: Date.now() }, }, }); + saveExecApprovals({ + version: 1, + agents: { + "*": { security: "deny" }, + main: { security: "allowlist", allowlist: [{ pattern: "/usr/bin/old" }] }, + ops: { security: "allowlist", allowlist: [{ pattern: "/usr/bin/keep" }] }, + }, + }); await agentsDeleteCommand({ id: "main", force: true, json: true }, runtime); @@ -326,6 +335,13 @@ describe("agents delete command", () => { expect(runtime.exit).not.toHaveBeenCalledWith(1); expect(configMocks.replaceConfigFile).toHaveBeenCalledOnce(); expectSessionStore(cfg, {}, "main"); + expect(readExecApprovalsSnapshot().file.agents).toEqual({ + "*": { security: "deny" }, + ops: { + security: "allowlist", + allowlist: [expect.objectContaining({ pattern: "/usr/bin/keep" })], + }, + }); }); }); @@ -570,6 +586,7 @@ describe("agents delete command", () => { "agent:main:main": { sessionId: "sess-main", updatedAt: now + 3 }, }, }); + expect(readExecApprovalsSnapshot().exists).toBe(false); await agentsDeleteCommand({ id: "ops", force: true, json: true }, runtime); @@ -591,6 +608,7 @@ describe("agents delete command", () => { expectSessionStore(cfg, { "agent:main:main": { sessionId: "sess-main", updatedAt: now + 3 }, }); + expect(readExecApprovalsSnapshot().exists).toBe(false); }); }); diff --git a/src/commands/agents.test.ts b/src/commands/agents.test.ts index 698ebf9c1f40..2c6d5059528d 100644 --- a/src/commands/agents.test.ts +++ b/src/commands/agents.test.ts @@ -451,6 +451,19 @@ describe("agents helpers", () => { { agentId: "work", match: { channel: "whatsapp" } }, { agentId: "home", match: { channel: "telegram" } }, ], + broadcast: { + strategy: "parallel", + "peer-1": ["work", "home"], + "peer-2": ["WORK"], + }, + hooks: { + allowedAgentIds: ["*", "work", "home"], + mappings: [ + { id: "work-hook", agentId: "WORK", action: "agent" }, + { id: "home-hook", agentId: "home", action: "agent" }, + { id: "default-hook", action: "agent" }, + ], + }, tools: { agentToAgent: { enabled: true, allow: ["work", "home"] }, }, @@ -463,6 +476,16 @@ describe("agents helpers", () => { expect(result.config.bindings).toStrictEqual([ { agentId: "home", match: { channel: "telegram" } }, ]); + expect(result.config.broadcast).toEqual({ + strategy: "parallel", + "peer-1": ["home"], + "peer-2": [], + }); + expect(result.config.hooks?.allowedAgentIds).toEqual(["*", "home"]); + expect(result.config.hooks?.mappings).toEqual([ + { id: "home-hook", agentId: "home", action: "agent" }, + { id: "default-hook", action: "agent" }, + ]); expect(result.config.tools?.agentToAgent?.allow).toEqual(["home"]); expect(result.config.agents?.defaults?.subagents?.allowAgents).toEqual(["home"]); expect(result.config.agents?.defaults?.heartbeat).toEqual({ every: "5m" }); diff --git a/src/infra/exec-approvals-store.test.ts b/src/infra/exec-approvals-store.test.ts index 3f10601567dd..38041f77bd49 100644 --- a/src/infra/exec-approvals-store.test.ts +++ b/src/infra/exec-approvals-store.test.ts @@ -261,18 +261,20 @@ describe("exec approvals SQLite store", () => { expect(commit).not.toHaveBeenCalled(); }); - it("removes one agent and preserves unrelated policy", async () => { + it("removes one agent and preserves wildcard and unrelated policy", async () => { saveExecApprovals({ version: 1, agents: { - removed: { security: "allowlist", allowlist: [{ pattern: "/usr/bin/old" }] }, + "*": { security: "deny" }, + main: { security: "allowlist", allowlist: [{ pattern: "/usr/bin/old" }] }, kept: { security: "allowlist", allowlist: [{ pattern: "/usr/bin/keep" }] }, }, }); - seedAgentDeletionJournal("removed"); + seedAgentDeletionJournal("main"); - await expect(withAgentExecApprovalsRemoved("removed", async () => "ok")).resolves.toBe("ok"); + await expect(withAgentExecApprovalsRemoved("main", async () => "ok")).resolves.toBe("ok"); expect(loadExecApprovals().agents).toEqual({ + "*": { security: "deny" }, kept: expect.objectContaining({ allowlist: [expect.objectContaining({ pattern: "/usr/bin/keep" })], }), diff --git a/src/infra/exec-approvals-store.ts b/src/infra/exec-approvals-store.ts index 5618bd44408c..bd7cca8fc9c3 100644 --- a/src/infra/exec-approvals-store.ts +++ b/src/infra/exec-approvals-store.ts @@ -4,7 +4,7 @@ import { AgentDeletionCommitUncertainError, } from "../agents/agent-lifecycle-registry.js"; import { createSubsystemLogger } from "../logging/subsystem.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 { withExistingOpenClawStateDatabaseReadOnly } from "../state/openclaw-state-db-readonly.js"; import { @@ -224,9 +224,10 @@ export async function withAgentExecApprovalsRemoved( if (!operationId) { throw new ExecApprovalsMutationFencedError(); } - const removedPolicyEntries = Object.entries(snapshot.file.agents ?? {}).filter( - ([policyKey]) => normalizeAgentId(policyKey) === key, - ); + const removedPolicyEntries = Object.entries(snapshot.file.agents ?? {}).filter(([policyKey]) => { + const normalizedPolicyKey = normalizeAgentIdStrict(policyKey); + return normalizedPolicyKey.ok && normalizedPolicyKey.value === key; + }); if (removedPolicyEntries.length > 0) { const updated = updateExecApprovalsInTransaction({ baseHash: snapshot.hash,