From d41c9860d79d816034bfa79a21c7f64b99f29c05 Mon Sep 17 00:00:00 2001 From: BitToby <218712309+bittoby@users.noreply.github.com> Date: Mon, 20 Apr 2026 00:47:52 +0200 Subject: [PATCH] fix: invalidate orphaned sessions on agent deletion (#65986) Merged via squash. Prepared head SHA: bc7c167dd946fd9b2e76fc3022c60e655e01f1cd Co-authored-by: bittoby <218712309+bittoby@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras --- CHANGELOG.md | 1 + src/commands/agents.command-shared.ts | 36 ++++ src/commands/agents.commands.delete.ts | 9 +- src/commands/agents.delete.test.ts | 158 ++++++++++++++++++ src/gateway/server-methods/agents.ts | 4 + .../chat.send-deleted-agent.test.ts | 52 ++++++ src/gateway/server-methods/chat.ts | 13 ++ .../sessions.send-deleted-agent.test.ts | 59 +++++++ .../sessions.send-followup-status.test.ts | 1 + src/gateway/server-methods/sessions.ts | 16 +- src/gateway/session-store-key.ts | 86 +++++++++- src/gateway/session-utils.test.ts | 28 ++++ src/gateway/session-utils.ts | 35 +++- src/gateway/sessions-resolve-store.test.ts | 112 +++++++++++++ src/gateway/sessions-resolve.test.ts | 120 +++++++++++++ src/gateway/sessions-resolve.ts | 35 ++++ 16 files changed, 753 insertions(+), 12 deletions(-) create mode 100644 src/commands/agents.delete.test.ts create mode 100644 src/gateway/server-methods/chat.send-deleted-agent.test.ts create mode 100644 src/gateway/server-methods/sessions.send-deleted-agent.test.ts create mode 100644 src/gateway/sessions-resolve-store.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index fd60b682b8f..82bc36856fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ Docs: https://docs.openclaw.ai - BlueBubbles: always set `method` explicitly on outbound text sends (`"private-api"` when available, `"apple-script"` otherwise), and prefer Private API on macOS 26 even for plain text. Fixes silent delivery failure on macOS setups without Private API where an omitted `method` let BB Server fall back to version-dependent default behavior that silently drops the message (#64480), and the AppleScript `-1700` error on macOS 26 Tahoe plain text sends (#53159). (#69070) Thanks @xqing3. - Matrix/commands: recognize slash commands that are prefixed with the bot's Matrix mention, so room messages like `@bot:server /new` trigger the command path without requiring custom mention regexes. (#68570) Thanks @nightq and @johnlanni. - Agents/subagents: include requested role and runtime timing on subagent failure payloads so parent agents can correlate failed or timed-out child work. (#68726) Thanks @BKF-Gitty. +- Gateway/sessions: reject stale agent-scoped sessions after an agent is removed from config while preserving legacy default-agent main-session aliases. (#65986) Thanks @bittoby. ## 2026.4.19-beta.2 diff --git a/src/commands/agents.command-shared.ts b/src/commands/agents.command-shared.ts index 3508047f3e1..4597bd9d86a 100644 --- a/src/commands/agents.command-shared.ts +++ b/src/commands/agents.command-shared.ts @@ -1,4 +1,9 @@ +import { resolveDefaultAgentId } from "../agents/agent-scope.js"; +import { resolveStorePath, updateSessionStore } from "../config/sessions.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; +import { resolveStoredSessionOwnerAgentId } from "../gateway/session-store-key.js"; +import { getLogger } from "../logging/logger.js"; +import { normalizeAgentId } from "../routing/session-key.js"; import type { RuntimeEnv } from "../runtime.js"; import { requireValidConfigFileSnapshot as requireValidConfigFileSnapshotBase, @@ -16,3 +21,34 @@ export async function requireValidConfigFileSnapshot(runtime: RuntimeEnv) { export async function requireValidConfig(runtime: RuntimeEnv): Promise { return await requireValidConfigSnapshot(runtime); } + +/** Purge session store entries for a deleted agent (#65524). Best-effort. */ +export async function purgeAgentSessionStoreEntries( + cfg: OpenClawConfig, + agentId: string, +): Promise { + try { + const normalizedAgentId = normalizeAgentId(agentId); + const storeConfig = cfg.session?.store; + const storeAgentId = + typeof storeConfig === "string" && storeConfig.includes("{agentId}") + ? normalizedAgentId + : normalizeAgentId(resolveDefaultAgentId(cfg)); + const storePath = resolveStorePath(cfg.session?.store, { agentId: normalizedAgentId }); + await updateSessionStore(storePath, (store) => { + for (const key of Object.keys(store)) { + if ( + resolveStoredSessionOwnerAgentId({ + cfg, + agentId: storeAgentId, + sessionKey: key, + }) === normalizedAgentId + ) { + delete store[key]; + } + } + }); + } catch (err) { + getLogger().debug("session store purge skipped during agent delete", err); + } +} diff --git a/src/commands/agents.commands.delete.ts b/src/commands/agents.commands.delete.ts index 84421ed60c5..b7a28af57fb 100644 --- a/src/commands/agents.commands.delete.ts +++ b/src/commands/agents.commands.delete.ts @@ -6,7 +6,11 @@ import { DEFAULT_AGENT_ID, normalizeAgentId } from "../routing/session-key.js"; import { type RuntimeEnv, writeRuntimeJson } from "../runtime.js"; import { defaultRuntime } from "../runtime.js"; import { createClackPrompter } from "../wizard/clack-prompter.js"; -import { createQuietRuntime, requireValidConfigFileSnapshot } from "./agents.command-shared.js"; +import { + createQuietRuntime, + purgeAgentSessionStoreEntries, + requireValidConfigFileSnapshot, +} from "./agents.command-shared.js"; import { findAgentEntryIndex, listAgentEntries, pruneAgentConfig } from "./agents.config.js"; import { moveToTrash } from "./onboard-helpers.js"; @@ -80,6 +84,9 @@ export async function agentsDeleteCommand( logConfigUpdated(runtime); } + // Purge session store entries for this agent so orphaned sessions cannot be targeted (#65524). + await purgeAgentSessionStoreEntries(cfg, agentId); + const quietRuntime = opts.json ? createQuietRuntime(runtime) : runtime; await moveToTrash(workspaceDir, quietRuntime); await moveToTrash(agentDir, quietRuntime); diff --git a/src/commands/agents.delete.test.ts b/src/commands/agents.delete.test.ts new file mode 100644 index 00000000000..9492adfd1ea --- /dev/null +++ b/src/commands/agents.delete.test.ts @@ -0,0 +1,158 @@ +import fs from "node:fs/promises"; +import path from "node:path"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { loadSessionStore, resolveStorePath, saveSessionStore } from "../config/sessions.js"; +import type { OpenClawConfig } from "../config/types.openclaw.js"; +import { withStateDirEnv } from "../test-helpers/state-dir-env.js"; +import { baseConfigSnapshot, createTestRuntime } from "./test-runtime-config-helpers.js"; + +const configMocks = vi.hoisted(() => ({ + readConfigFileSnapshot: vi.fn(), + replaceConfigFile: vi.fn(async () => {}), +})); + +const processMocks = vi.hoisted(() => ({ + runCommandWithTimeout: vi.fn(async () => ({ stdout: "", stderr: "", code: 0 })), +})); + +vi.mock("../config/config.js", async () => ({ + ...(await vi.importActual("../config/config.js")), + readConfigFileSnapshot: configMocks.readConfigFileSnapshot, + replaceConfigFile: configMocks.replaceConfigFile, +})); + +vi.mock("../process/exec.js", () => ({ + runCommandWithTimeout: processMocks.runCommandWithTimeout, +})); + +import { agentsDeleteCommand } from "./agents.js"; + +const runtime = createTestRuntime(); + +describe("agents delete command", () => { + beforeEach(() => { + configMocks.readConfigFileSnapshot.mockReset(); + configMocks.replaceConfigFile.mockReset(); + processMocks.runCommandWithTimeout.mockClear(); + runtime.log.mockClear(); + runtime.error.mockClear(); + runtime.exit.mockClear(); + }); + + it("purges deleted agent entries from the session store", async () => { + await withStateDirEnv("openclaw-agents-delete-", async ({ stateDir }) => { + const cfg: OpenClawConfig = { + agents: { + list: [ + { id: "main", workspace: path.join(stateDir, "workspace-main") }, + { id: "ops", workspace: path.join(stateDir, "workspace-ops") }, + ], + }, + } satisfies OpenClawConfig; + const storePath = resolveStorePath(cfg.session?.store, { agentId: "ops" }); + await saveSessionStore(storePath, { + "agent:ops:main": { sessionId: "sess-ops-main", updatedAt: 1 }, + "agent:ops:discord:direct:u1": { sessionId: "sess-ops-direct", updatedAt: 2 }, + "agent:main:main": { sessionId: "sess-main", updatedAt: 3 }, + }); + await fs.mkdir(path.join(stateDir, "workspace-ops"), { recursive: true }); + await fs.mkdir(path.join(stateDir, "agents", "ops", "agent"), { recursive: true }); + + configMocks.readConfigFileSnapshot.mockResolvedValue({ + ...baseConfigSnapshot, + config: cfg, + runtimeConfig: cfg, + sourceConfig: cfg, + resolved: cfg, + }); + + await agentsDeleteCommand({ id: "ops", force: true, json: true }, runtime); + + expect(runtime.exit).not.toHaveBeenCalled(); + expect(configMocks.replaceConfigFile).toHaveBeenCalledWith( + expect.objectContaining({ + nextConfig: { + agents: { list: [{ id: "main", workspace: path.join(stateDir, "workspace-main") }] }, + }, + }), + ); + expect(loadSessionStore(storePath, { skipCache: true })).toEqual({ + "agent:main:main": { sessionId: "sess-main", updatedAt: 3 }, + }); + }); + }); + + it("purges legacy main-alias entries owned by the deleted default agent", async () => { + await withStateDirEnv("openclaw-agents-delete-main-alias-", async ({ stateDir }) => { + const cfg: OpenClawConfig = { + agents: { + list: [{ id: "ops", default: true, workspace: path.join(stateDir, "workspace-ops") }], + }, + }; + const storePath = resolveStorePath(cfg.session?.store, { agentId: "ops" }); + await saveSessionStore(storePath, { + "agent:main:main": { sessionId: "sess-default-alias", updatedAt: 1 }, + "agent:ops:discord:direct:u1": { sessionId: "sess-ops-direct", updatedAt: 2 }, + "agent:main:discord:direct:u2": { sessionId: "sess-stale-main", updatedAt: 3 }, + global: { sessionId: "sess-global", updatedAt: 4 }, + }); + await fs.mkdir(path.join(stateDir, "workspace-ops"), { recursive: true }); + await fs.mkdir(path.join(stateDir, "agents", "ops", "agent"), { recursive: true }); + + configMocks.readConfigFileSnapshot.mockResolvedValue({ + ...baseConfigSnapshot, + config: cfg, + runtimeConfig: cfg, + sourceConfig: cfg, + resolved: cfg, + }); + + await agentsDeleteCommand({ id: "ops", force: true, json: true }, runtime); + + expect(runtime.exit).not.toHaveBeenCalled(); + expect(loadSessionStore(storePath, { skipCache: true })).toEqual({ + "agent:main:discord:direct:u2": { sessionId: "sess-stale-main", updatedAt: 3 }, + global: { sessionId: "sess-global", updatedAt: 4 }, + }); + }); + }); + + it("preserves shared-store legacy default keys when deleting another agent", async () => { + await withStateDirEnv("openclaw-agents-delete-shared-store-", async ({ stateDir }) => { + const cfg: OpenClawConfig = { + session: { store: path.join(stateDir, "sessions.json") }, + agents: { + list: [ + { id: "main", default: true, workspace: path.join(stateDir, "workspace-main") }, + { id: "ops", workspace: path.join(stateDir, "workspace-ops") }, + ], + }, + }; + const storePath = resolveStorePath(cfg.session?.store, { agentId: "ops" }); + await saveSessionStore(storePath, { + main: { sessionId: "sess-main", updatedAt: 1 }, + "discord:direct:u1": { sessionId: "sess-main-direct", updatedAt: 2 }, + "agent:ops:main": { sessionId: "sess-ops-main", updatedAt: 3 }, + "agent:ops:discord:direct:u2": { sessionId: "sess-ops-direct", updatedAt: 4 }, + }); + await fs.mkdir(path.join(stateDir, "workspace-ops"), { recursive: true }); + await fs.mkdir(path.join(stateDir, "agents", "ops", "agent"), { recursive: true }); + + configMocks.readConfigFileSnapshot.mockResolvedValue({ + ...baseConfigSnapshot, + config: cfg, + runtimeConfig: cfg, + sourceConfig: cfg, + resolved: cfg, + }); + + await agentsDeleteCommand({ id: "ops", force: true, json: true }, runtime); + + expect(runtime.exit).not.toHaveBeenCalled(); + expect(loadSessionStore(storePath, { skipCache: true })).toEqual({ + main: { sessionId: "sess-main", updatedAt: 1 }, + "discord:direct:u1": { sessionId: "sess-main-direct", updatedAt: 2 }, + }); + }); + }); +}); diff --git a/src/gateway/server-methods/agents.ts b/src/gateway/server-methods/agents.ts index d905876538e..b3df9a30471 100644 --- a/src/gateway/server-methods/agents.ts +++ b/src/gateway/server-methods/agents.ts @@ -20,6 +20,7 @@ import { ensureAgentWorkspace, isWorkspaceSetupCompleted, } from "../../agents/workspace.js"; +import { purgeAgentSessionStoreEntries } from "../../commands/agents.command-shared.js"; import { applyAgentConfig, findAgentEntryIndex, @@ -650,6 +651,9 @@ export const agentsHandlers: GatewayRequestHandlers = { const result = pruneAgentConfig(cfg, agentId); await writeConfigFile(result.config); + // Purge session store entries so orphaned sessions cannot be targeted (#65524). + await purgeAgentSessionStoreEntries(cfg, agentId); + if (deleteFiles) { await Promise.all([ moveToTrashBestEffort(workspaceDir), diff --git a/src/gateway/server-methods/chat.send-deleted-agent.test.ts b/src/gateway/server-methods/chat.send-deleted-agent.test.ts new file mode 100644 index 00000000000..21e51466f27 --- /dev/null +++ b/src/gateway/server-methods/chat.send-deleted-agent.test.ts @@ -0,0 +1,52 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { ErrorCodes } from "../protocol/index.js"; +import type { RespondFn } from "./types.js"; + +const loadSessionEntryMock = vi.fn(); +const resolveDeletedAgentIdFromSessionKeyMock = vi.fn(); + +vi.mock("../session-utils.js", async () => { + const actual = await vi.importActual("../session-utils.js"); + return { + ...actual, + loadSessionEntry: (...args: unknown[]) => loadSessionEntryMock(...args), + resolveDeletedAgentIdFromSessionKey: (...args: unknown[]) => + resolveDeletedAgentIdFromSessionKeyMock(...args), + }; +}); + +import { chatHandlers } from "./chat.js"; + +describe("chat.send deleted-agent guard", () => { + beforeEach(() => { + loadSessionEntryMock.mockReset(); + resolveDeletedAgentIdFromSessionKeyMock.mockReset(); + }); + + it("rejects keys belonging to a deleted agent", async () => { + const orphanKey = "agent:deleted-agent:main"; + loadSessionEntryMock.mockReturnValue({ + cfg: {}, + canonicalKey: orphanKey, + storePath: "/tmp/sessions.json", + entry: { sessionId: "sess-orphan" }, + }); + resolveDeletedAgentIdFromSessionKeyMock.mockReturnValue("deleted-agent"); + + const respond = vi.fn() as unknown as RespondFn; + + await chatHandlers["chat.send"]({ + req: { id: "req-1" } as never, + params: { sessionKey: orphanKey, message: "hi", idempotencyKey: "run-1" }, + respond, + context: {} as never, + client: null, + isWebchatConnect: () => false, + }); + + expect(respond).toHaveBeenCalledWith(false, undefined, { + code: ErrorCodes.INVALID_REQUEST, + message: 'Agent "deleted-agent" no longer exists in configuration', + }); + }); +}); diff --git a/src/gateway/server-methods/chat.ts b/src/gateway/server-methods/chat.ts index 3c351398a2a..dc4e6a2319d 100644 --- a/src/gateway/server-methods/chat.ts +++ b/src/gateway/server-methods/chat.ts @@ -74,6 +74,7 @@ import { capArrayByJsonBytes, loadSessionEntry, resolveGatewayModelSupportsImages, + resolveDeletedAgentIdFromSessionKey, readSessionMessages, resolveSessionModelRef, } from "../session-utils.js"; @@ -1846,6 +1847,18 @@ export const chatHandlers: GatewayRequestHandlers = { } const rawSessionKey = p.sessionKey; const { cfg, entry, canonicalKey: sessionKey } = loadSessionEntry(rawSessionKey); + const deletedAgentId = resolveDeletedAgentIdFromSessionKey(cfg, sessionKey); + if (deletedAgentId !== null) { + respond( + false, + undefined, + errorShape( + ErrorCodes.INVALID_REQUEST, + `Agent "${deletedAgentId}" no longer exists in configuration`, + ), + ); + return; + } const agentId = resolveSessionAgentId({ sessionKey, config: cfg, diff --git a/src/gateway/server-methods/sessions.send-deleted-agent.test.ts b/src/gateway/server-methods/sessions.send-deleted-agent.test.ts new file mode 100644 index 00000000000..447cc639467 --- /dev/null +++ b/src/gateway/server-methods/sessions.send-deleted-agent.test.ts @@ -0,0 +1,59 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { ErrorCodes } from "../protocol/index.js"; +import type { GatewayRequestContext, RespondFn } from "./types.js"; + +const loadSessionEntryMock = vi.fn(); +const resolveDeletedAgentIdFromSessionKeyMock = vi.fn(); + +vi.mock("../session-utils.js", async () => { + const actual = await vi.importActual("../session-utils.js"); + return { + ...actual, + loadSessionEntry: (...args: unknown[]) => loadSessionEntryMock(...args), + resolveDeletedAgentIdFromSessionKey: (...args: unknown[]) => + resolveDeletedAgentIdFromSessionKeyMock(...args), + }; +}); + +import { sessionsHandlers } from "./sessions.js"; + +describe("sessions.send / sessions.steer deleted-agent guard", () => { + beforeEach(() => { + loadSessionEntryMock.mockReset(); + resolveDeletedAgentIdFromSessionKeyMock.mockReset(); + }); + + for (const method of ["sessions.send", "sessions.steer"] as const) { + it(`${method} rejects keys belonging to a deleted agent`, async () => { + const orphanKey = "agent:deleted-agent:main"; + loadSessionEntryMock.mockReturnValue({ + cfg: {}, + canonicalKey: orphanKey, + storePath: "/tmp/sessions.json", + entry: { sessionId: "sess-orphan" }, + }); + resolveDeletedAgentIdFromSessionKeyMock.mockReturnValue("deleted-agent"); + + const respond = vi.fn() as unknown as RespondFn; + const context = { + chatAbortControllers: new Map(), + broadcastToConnIds: vi.fn(), + getSessionEventSubscriberConnIds: () => new Set(), + } as unknown as GatewayRequestContext; + + await sessionsHandlers[method]({ + req: { id: "req-1" } as never, + params: { key: orphanKey, message: "hi" }, + respond, + context, + client: null, + isWebchatConnect: () => false, + }); + + expect(respond).toHaveBeenCalledWith(false, undefined, { + code: ErrorCodes.INVALID_REQUEST, + message: 'Agent "deleted-agent" no longer exists in configuration', + }); + }); + } +}); diff --git a/src/gateway/server-methods/sessions.send-followup-status.test.ts b/src/gateway/server-methods/sessions.send-followup-status.test.ts index b1bbffb8f7c..b9a94d6f814 100644 --- a/src/gateway/server-methods/sessions.send-followup-status.test.ts +++ b/src/gateway/server-methods/sessions.send-followup-status.test.ts @@ -69,6 +69,7 @@ describe("sessions.send completed subagent follow-up status", () => { }; loadSessionEntryMock.mockReturnValue({ + cfg: {}, canonicalKey: childSessionKey, storePath: "/tmp/sessions.json", entry: { sessionId: "sess-followup" }, diff --git a/src/gateway/server-methods/sessions.ts b/src/gateway/server-methods/sessions.ts index bb9ab29fc35..a83b71570f0 100644 --- a/src/gateway/server-methods/sessions.ts +++ b/src/gateway/server-methods/sessions.ts @@ -72,6 +72,7 @@ import { loadSessionEntry, migrateAndPruneGatewaySessionStoreKey, readSessionPreviewItemsFromTranscript, + resolveDeletedAgentIdFromSessionKey, resolveFreshestSessionEntryFromStoreKeys, resolveGatewaySessionStoreTarget, resolveSessionModelRef, @@ -461,7 +462,20 @@ async function handleSessionSend(params: { if (!key) { return; } - const { entry, canonicalKey, storePath } = loadSessionEntry(key); + const { cfg, entry, canonicalKey, storePath } = loadSessionEntry(key); + // Reject sends/steers targeting sessions whose owning agent was deleted (#65524). + const deletedAgentId = resolveDeletedAgentIdFromSessionKey(cfg, canonicalKey); + if (deletedAgentId !== null) { + params.respond( + false, + undefined, + errorShape( + ErrorCodes.INVALID_REQUEST, + `Agent "${deletedAgentId}" no longer exists in configuration`, + ), + ); + return; + } if (!entry?.sessionId) { params.respond( false, diff --git a/src/gateway/session-store-key.ts b/src/gateway/session-store-key.ts index 206a9a9566b..8c165c69eb8 100644 --- a/src/gateway/session-store-key.ts +++ b/src/gateway/session-store-key.ts @@ -1,13 +1,15 @@ -import { resolveDefaultAgentId } from "../agents/agent-scope.js"; +import { listAgentIds, resolveDefaultAgentId } from "../agents/agent-scope.js"; import { canonicalizeMainSessionAlias, resolveMainSessionKey, } from "../config/sessions/main-session.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import { + DEFAULT_AGENT_ID, normalizeAgentId, normalizeMainKey, parseAgentSessionKey, + type ParsedAgentSessionKey, } from "../routing/session-key.js"; import { normalizeLowercaseStringOrEmpty, @@ -29,9 +31,45 @@ function resolveDefaultStoreAgentId(cfg: OpenClawConfig): string { return normalizeAgentId(resolveDefaultAgentId(cfg)); } +function shouldRemapLegacyDefaultMainAlias( + cfg: OpenClawConfig, + parsed: ParsedAgentSessionKey, + options?: { storeAgentId?: string }, +): boolean { + const agentId = normalizeAgentId(parsed.agentId); + if (agentId !== DEFAULT_AGENT_ID || listAgentIds(cfg).includes(DEFAULT_AGENT_ID)) { + return false; + } + const defaultAgentId = resolveDefaultStoreAgentId(cfg); + if (options?.storeAgentId && normalizeAgentId(options.storeAgentId) !== defaultAgentId) { + return false; + } + const rest = normalizeLowercaseStringOrEmpty(parsed.rest); + const mainKey = normalizeMainKey(cfg.session?.mainKey); + return rest === "main" || rest === mainKey; +} + +function resolveParsedSessionStoreKey( + cfg: OpenClawConfig, + raw: string, + parsed: ParsedAgentSessionKey, + options?: { storeAgentId?: string }, +): { agentId: string; sessionKey: string } { + if (!shouldRemapLegacyDefaultMainAlias(cfg, parsed, options)) { + return { + agentId: normalizeAgentId(parsed.agentId), + sessionKey: normalizeLowercaseStringOrEmpty(raw), + }; + } + const agentId = resolveDefaultStoreAgentId(cfg); + const rest = normalizeLowercaseStringOrEmpty(parsed.rest); + return { agentId, sessionKey: `agent:${agentId}:${rest}` }; +} + export function resolveSessionStoreKey(params: { cfg: OpenClawConfig; sessionKey: string; + storeAgentId?: string; }): string { const raw = normalizeOptionalString(params.sessionKey) ?? ""; if (!raw) { @@ -44,17 +82,18 @@ export function resolveSessionStoreKey(params: { const parsed = parseAgentSessionKey(raw); if (parsed) { - const agentId = normalizeAgentId(parsed.agentId); - const lowered = normalizeLowercaseStringOrEmpty(raw); + const resolved = resolveParsedSessionStoreKey(params.cfg, raw, parsed, { + storeAgentId: params.storeAgentId, + }); const canonical = canonicalizeMainSessionAlias({ cfg: params.cfg, - agentId, - sessionKey: lowered, + agentId: resolved.agentId, + sessionKey: resolved.sessionKey, }); - if (canonical !== lowered) { + if (canonical !== resolved.sessionKey) { return canonical; } - return lowered; + return resolved.sessionKey; } const lowered = normalizeLowercaseStringOrEmpty(raw); @@ -77,6 +116,39 @@ export function resolveSessionStoreAgentId(cfg: OpenClawConfig, canonicalKey: st return resolveDefaultStoreAgentId(cfg); } +export function resolveStoredSessionKeyForAgentStore(params: { + cfg: OpenClawConfig; + agentId: string; + sessionKey: string; +}): string { + const raw = normalizeOptionalString(params.sessionKey) ?? ""; + if (!raw) { + return raw; + } + const lowered = normalizeLowercaseStringOrEmpty(raw); + if (lowered === "global" || lowered === "unknown") { + return lowered; + } + const key = parseAgentSessionKey(raw) ? raw : canonicalizeSessionKeyForAgent(params.agentId, raw); + return resolveSessionStoreKey({ + cfg: params.cfg, + sessionKey: key, + storeAgentId: params.agentId, + }); +} + +export function resolveStoredSessionOwnerAgentId(params: { + cfg: OpenClawConfig; + agentId: string; + sessionKey: string; +}): string | null { + const canonicalKey = resolveStoredSessionKeyForAgentStore(params); + if (canonicalKey === "global" || canonicalKey === "unknown") { + return null; + } + return resolveSessionStoreAgentId(params.cfg, canonicalKey); +} + export function canonicalizeSpawnedByForAgent( cfg: OpenClawConfig, agentId: string, diff --git a/src/gateway/session-utils.test.ts b/src/gateway/session-utils.test.ts index 8a68652cbfd..300758aaaeb 100644 --- a/src/gateway/session-utils.test.ts +++ b/src/gateway/session-utils.test.ts @@ -16,6 +16,7 @@ import { migrateAndPruneGatewaySessionStoreKey, parseGroupKey, pruneLegacyStoreKeys, + resolveDeletedAgentIdFromSessionKey, resolveGatewayModelSupportsImages, resolveGatewaySessionStoreTarget, resolveSessionModelIdentityRef, @@ -105,9 +106,36 @@ describe("gateway session utils", () => { expect(resolveSessionStoreKey({ cfg, sessionKey: "work" })).toBe("agent:ops:work"); expect(resolveSessionStoreKey({ cfg, sessionKey: "agent:ops:main" })).toBe("agent:ops:work"); expect(resolveSessionStoreKey({ cfg, sessionKey: "agent:ops:MAIN" })).toBe("agent:ops:work"); + expect(resolveSessionStoreKey({ cfg, sessionKey: "agent:main:main" })).toBe("agent:ops:work"); + expect(resolveSessionStoreKey({ cfg, sessionKey: "agent:main:work" })).toBe("agent:ops:work"); expect(resolveSessionStoreKey({ cfg, sessionKey: "MAIN" })).toBe("agent:ops:work"); }); + test("resolveSessionStoreKey preserves non-alias agent:main keys for deleted-agent checks", () => { + const cfg = { + session: { mainKey: "work" }, + agents: { list: [{ id: "ops", default: true }] }, + } as OpenClawConfig; + expect(resolveSessionStoreKey({ cfg, sessionKey: "agent:main:discord:direct:u1" })).toBe( + "agent:main:discord:direct:u1", + ); + }); + + test("resolveDeletedAgentIdFromSessionKey rejects non-alias main keys when main is absent", () => { + const cfg = { + session: { mainKey: "work" }, + agents: { list: [{ id: "ops", default: true }] }, + } as OpenClawConfig; + const legacyMainAlias = resolveSessionStoreKey({ cfg, sessionKey: "agent:main:main" }); + + expect(legacyMainAlias).toBe("agent:ops:work"); + expect(resolveDeletedAgentIdFromSessionKey(cfg, legacyMainAlias)).toBeNull(); + expect(resolveDeletedAgentIdFromSessionKey(cfg, "global")).toBeNull(); + expect(resolveDeletedAgentIdFromSessionKey(cfg, "unknown")).toBeNull(); + expect(resolveDeletedAgentIdFromSessionKey(cfg, "main")).toBeNull(); + expect(resolveDeletedAgentIdFromSessionKey(cfg, "agent:main:discord:direct:u1")).toBe("main"); + }); + test("resolveSessionStoreKey canonicalizes bare keys to default agent", () => { const cfg = { session: { mainKey: "main" }, diff --git a/src/gateway/session-utils.ts b/src/gateway/session-utils.ts index 282b93f79b6..9b145fa80e0 100644 --- a/src/gateway/session-utils.ts +++ b/src/gateway/session-utils.ts @@ -1,6 +1,7 @@ import fs from "node:fs"; import path from "node:path"; import { + listAgentIds, resolveAgentEffectiveModelPrimary, resolveAgentModelFallbacksOverride, resolveAgentWorkspaceDir, @@ -62,10 +63,10 @@ import { import { normalizeSessionDeliveryFields } from "../utils/delivery-context.shared.js"; import { estimateUsageCost, resolveModelCostConfig } from "../utils/usage-format.js"; import { - canonicalizeSessionKeyForAgent, canonicalizeSpawnedByForAgent, resolveSessionStoreAgentId, resolveSessionStoreKey, + resolveStoredSessionKeyForAgentStore, } from "./session-store-key.js"; import { readLatestSessionUsageFromTranscript, @@ -391,6 +392,26 @@ function resolveTranscriptUsageFallback(params: { }; } +/** + * Returns the owning agent id if the session key belongs to an agent that is no + * longer present in config (deleted). Returns null for non-agent legacy/global + * keys, or when the owning agent still exists (#65524). + */ +export function resolveDeletedAgentIdFromSessionKey( + cfg: OpenClawConfig, + sessionKey: string, +): string | null { + const parsed = parseAgentSessionKey(sessionKey); + if (!parsed) { + return null; + } + const agentId = normalizeAgentId(parsed.agentId); + if (listAgentIds(cfg).includes(agentId)) { + return null; + } + return agentId; +} + export function loadSessionEntry(sessionKey: string) { const cfg = loadConfig(); const canonicalKey = resolveSessionStoreKey({ cfg, sessionKey }); @@ -906,7 +927,11 @@ export function loadCombinedSessionStoreForGateway(cfg: OpenClawConfig): { const store = loadSessionStore(storePath); const combined: Record = {}; for (const [key, entry] of Object.entries(store)) { - const canonicalKey = canonicalizeSessionKeyForAgent(defaultAgentId, key); + const canonicalKey = resolveStoredSessionKeyForAgentStore({ + cfg, + agentId: defaultAgentId, + sessionKey: key, + }); mergeSessionEntryIntoCombined({ cfg, combined, @@ -925,7 +950,11 @@ export function loadCombinedSessionStoreForGateway(cfg: OpenClawConfig): { const storePath = target.storePath; const store = loadSessionStore(storePath); for (const [key, entry] of Object.entries(store)) { - const canonicalKey = canonicalizeSessionKeyForAgent(agentId, key); + const canonicalKey = resolveStoredSessionKeyForAgentStore({ + cfg, + agentId, + sessionKey: key, + }); mergeSessionEntryIntoCombined({ cfg, combined, diff --git a/src/gateway/sessions-resolve-store.test.ts b/src/gateway/sessions-resolve-store.test.ts new file mode 100644 index 00000000000..3b849f4f920 --- /dev/null +++ b/src/gateway/sessions-resolve-store.test.ts @@ -0,0 +1,112 @@ +import path from "node:path"; +import { describe, expect, it } from "vitest"; +import { resolveStorePath, saveSessionStore } from "../config/sessions.js"; +import type { OpenClawConfig } from "../config/types.openclaw.js"; +import { withStateDirEnv } from "../test-helpers/state-dir-env.js"; +import { ErrorCodes } from "./protocol/index.js"; +import { resolveSessionKeyFromResolveParams } from "./sessions-resolve.js"; + +describe("resolveSessionKeyFromResolveParams store canonicalization", () => { + it("resolves legacy main-alias matches by sessionId and label for the configured default agent", async () => { + await withStateDirEnv("openclaw-sessions-resolve-alias-", async ({ stateDir }) => { + const storePath = path.join(stateDir, "sessions.json"); + const cfg = { + session: { store: storePath, mainKey: "main" }, + agents: { list: [{ id: "ops", default: true }] }, + } satisfies OpenClawConfig; + await saveSessionStore(storePath, { + "agent:main:main": { + sessionId: "sess-default-alias", + label: "default-alias", + updatedAt: 1, + }, + }); + + await expect( + resolveSessionKeyFromResolveParams({ + cfg, + p: { sessionId: "sess-default-alias" }, + }), + ).resolves.toEqual({ ok: true, key: "agent:ops:main" }); + + await expect( + resolveSessionKeyFromResolveParams({ + cfg, + p: { label: "default-alias" }, + }), + ).resolves.toEqual({ ok: true, key: "agent:ops:main" }); + }); + }); + + it("still rejects non-alias agent:main matches when main is no longer configured", async () => { + await withStateDirEnv("openclaw-sessions-resolve-stale-main-", async ({ stateDir }) => { + const storePath = path.join(stateDir, "sessions.json"); + const cfg = { + session: { store: storePath, mainKey: "main" }, + agents: { list: [{ id: "ops", default: true }] }, + } satisfies OpenClawConfig; + await saveSessionStore(storePath, { + "agent:main:discord:direct:u1": { + sessionId: "sess-stale-main", + label: "stale-main", + updatedAt: 1, + }, + }); + + await expect( + resolveSessionKeyFromResolveParams({ + cfg, + p: { sessionId: "sess-stale-main" }, + }), + ).resolves.toEqual({ + ok: false, + error: { + code: ErrorCodes.INVALID_REQUEST, + message: 'Agent "main" no longer exists in configuration', + }, + }); + }); + }); + + it("does not adopt legacy main aliases from discovered deleted-agent stores", async () => { + await withStateDirEnv("openclaw-sessions-resolve-discovered-main-", async () => { + const cfg: OpenClawConfig = { + agents: { list: [{ id: "ops", default: true }] }, + }; + const staleMainStorePath = resolveStorePath(cfg.session?.store, { agentId: "main" }); + await saveSessionStore(staleMainStorePath, { + "agent:main:main": { + sessionId: "sess-discovered-main", + label: "discovered-main", + updatedAt: 1, + }, + }); + + await expect( + resolveSessionKeyFromResolveParams({ + cfg, + p: { sessionId: "sess-discovered-main" }, + }), + ).resolves.toEqual({ + ok: false, + error: { + code: ErrorCodes.INVALID_REQUEST, + message: 'Agent "main" no longer exists in configuration', + }, + }); + + await expect( + resolveSessionKeyFromResolveParams({ + cfg, + p: { label: "discovered-main" }, + }), + ).resolves.toEqual({ + ok: false, + error: { + code: ErrorCodes.INVALID_REQUEST, + message: 'Agent "main" no longer exists in configuration', + }, + }); + }); + }); +}); diff --git a/src/gateway/sessions-resolve.test.ts b/src/gateway/sessions-resolve.test.ts index 84992a1863b..a40fde36e3a 100644 --- a/src/gateway/sessions-resolve.test.ts +++ b/src/gateway/sessions-resolve.test.ts @@ -8,8 +8,20 @@ const hoisted = vi.hoisted(() => ({ listSessionsFromStoreMock: vi.fn(), migrateAndPruneGatewaySessionStoreKeyMock: vi.fn(), resolveGatewaySessionStoreTargetMock: vi.fn(), + loadCombinedSessionStoreForGatewayMock: vi.fn(), + listAgentIdsMock: vi.fn(), })); +vi.mock("../agents/agent-scope.js", async () => { + const actual = await vi.importActual( + "../agents/agent-scope.js", + ); + return { + ...actual, + listAgentIds: hoisted.listAgentIdsMock, + }; +}); + vi.mock("../config/sessions.js", async () => { const actual = await vi.importActual("../config/sessions.js"); @@ -27,6 +39,7 @@ vi.mock("./session-utils.js", async () => { listSessionsFromStore: hoisted.listSessionsFromStoreMock, migrateAndPruneGatewaySessionStoreKey: hoisted.migrateAndPruneGatewaySessionStoreKeyMock, resolveGatewaySessionStoreTarget: hoisted.resolveGatewaySessionStoreTargetMock, + loadCombinedSessionStoreForGateway: hoisted.loadCombinedSessionStoreForGatewayMock, }; }); @@ -43,6 +56,10 @@ describe("resolveSessionKeyFromResolveParams", () => { hoisted.listSessionsFromStoreMock.mockReset(); hoisted.migrateAndPruneGatewaySessionStoreKeyMock.mockReset(); hoisted.resolveGatewaySessionStoreTargetMock.mockReset(); + hoisted.loadCombinedSessionStoreForGatewayMock.mockReset(); + hoisted.listAgentIdsMock.mockReset(); + // Default: all agents are known (main is always present). + hoisted.listAgentIdsMock.mockReturnValue(["main"]); hoisted.resolveGatewaySessionStoreTargetMock.mockReturnValue({ canonicalKey, storeKeys: [canonicalKey, legacyKey], @@ -113,4 +130,107 @@ describe("resolveSessionKeyFromResolveParams", () => { }, }); }); + + it("rejects sessions belonging to a deleted agent (key-based lookup)", async () => { + const deletedAgentKey = "agent:deleted-agent:main"; + hoisted.resolveGatewaySessionStoreTargetMock.mockReturnValue({ + canonicalKey: deletedAgentKey, + storeKeys: [deletedAgentKey], + storePath, + }); + hoisted.loadSessionStoreMock.mockReturnValue({ + [deletedAgentKey]: { sessionId: "sess-orphan", updatedAt: 1 }, + }); + // "deleted-agent" is not in the known agents list. + hoisted.listAgentIdsMock.mockReturnValue(["main"]); + + const result = await resolveSessionKeyFromResolveParams({ + cfg: {}, + p: { key: deletedAgentKey }, + }); + + expect(result).toEqual({ + ok: false, + error: { + code: ErrorCodes.INVALID_REQUEST, + message: 'Agent "deleted-agent" no longer exists in configuration', + }, + }); + }); + + it("rejects non-alias agent:main sessions when main is no longer configured", async () => { + const staleMainKey = "agent:main:discord:direct:u1"; + hoisted.resolveGatewaySessionStoreTargetMock.mockReturnValue({ + canonicalKey: staleMainKey, + storeKeys: [staleMainKey], + storePath, + }); + hoisted.loadSessionStoreMock.mockReturnValue({ + [staleMainKey]: { sessionId: "sess-stale-main", updatedAt: 1 }, + }); + hoisted.listAgentIdsMock.mockReturnValue(["ops"]); + + const result = await resolveSessionKeyFromResolveParams({ + cfg: { agents: { list: [{ id: "ops", default: true }] } }, + p: { key: staleMainKey }, + }); + + expect(result).toEqual({ + ok: false, + error: { + code: ErrorCodes.INVALID_REQUEST, + message: 'Agent "main" no longer exists in configuration', + }, + }); + }); + + it("rejects sessions belonging to a deleted agent (sessionId-based lookup)", async () => { + const deletedAgentKey = "agent:deleted-agent:main"; + hoisted.loadCombinedSessionStoreForGatewayMock.mockReturnValue({ + storePath, + store: { [deletedAgentKey]: { sessionId: "sess-orphan", updatedAt: 1 } }, + }); + hoisted.listSessionsFromStoreMock.mockReturnValue({ + sessions: [{ key: deletedAgentKey, sessionId: "sess-orphan" }], + }); + hoisted.listAgentIdsMock.mockReturnValue(["main"]); + + const result = await resolveSessionKeyFromResolveParams({ + cfg: {}, + p: { sessionId: "sess-orphan" }, + }); + + expect(result).toEqual({ + ok: false, + error: { + code: ErrorCodes.INVALID_REQUEST, + message: 'Agent "deleted-agent" no longer exists in configuration', + }, + }); + }); + + it("rejects sessions belonging to a deleted agent (label-based lookup)", async () => { + const deletedAgentKey = "agent:deleted-agent:main"; + hoisted.loadCombinedSessionStoreForGatewayMock.mockReturnValue({ + storePath, + store: { [deletedAgentKey]: { sessionId: "sess-orphan", updatedAt: 1, label: "my-label" } }, + }); + hoisted.listSessionsFromStoreMock.mockReturnValue({ + sessions: [{ key: deletedAgentKey, sessionId: "sess-orphan", label: "my-label" }], + }); + hoisted.listAgentIdsMock.mockReturnValue(["main"]); + + const result = await resolveSessionKeyFromResolveParams({ + cfg: {}, + p: { label: "my-label" }, + }); + + expect(result).toEqual({ + ok: false, + error: { + code: ErrorCodes.INVALID_REQUEST, + message: 'Agent "deleted-agent" no longer exists in configuration', + }, + }); + }); }); diff --git a/src/gateway/sessions-resolve.ts b/src/gateway/sessions-resolve.ts index 33431f57dff..40e39e20f03 100644 --- a/src/gateway/sessions-resolve.ts +++ b/src/gateway/sessions-resolve.ts @@ -12,6 +12,7 @@ import { listSessionsFromStore, loadCombinedSessionStoreForGateway, migrateAndPruneGatewaySessionStoreKey, + resolveDeletedAgentIdFromSessionKey, resolveGatewaySessionStoreTarget, } from "./session-utils.js"; @@ -33,6 +34,24 @@ function noSessionFoundResult(key: string): SessionsResolveResult { }; } +/** Rejects sessions whose owning agent no longer exists in config (#65524). */ +function validateSessionAgentExists( + cfg: OpenClawConfig, + key: string, +): SessionsResolveResult | null { + const deletedAgentId = resolveDeletedAgentIdFromSessionKey(cfg, key); + if (deletedAgentId === null) { + return null; + } + return { + ok: false, + error: errorShape( + ErrorCodes.INVALID_REQUEST, + `Agent "${deletedAgentId}" no longer exists in configuration`, + ), + }; +} + function isResolvedSessionKeyVisible(params: { cfg: OpenClawConfig; p: SessionsResolveParams; @@ -94,6 +113,10 @@ export async function resolveSessionKeyFromResolveParams(params: { ) { return noSessionFoundResult(key); } + const agentCheck = validateSessionAgentExists(cfg, target.canonicalKey); + if (agentCheck) { + return agentCheck; + } return { ok: true, key: target.canonicalKey }; } const legacyKey = target.storeKeys.find((candidate) => store[candidate]); @@ -117,6 +140,10 @@ export async function resolveSessionKeyFromResolveParams(params: { ) { return noSessionFoundResult(key); } + const agentCheckLegacy = validateSessionAgentExists(cfg, target.canonicalKey); + if (agentCheckLegacy) { + return agentCheckLegacy; + } return { ok: true, key: target.canonicalKey }; } @@ -152,6 +179,10 @@ export async function resolveSessionKeyFromResolveParams(params: { ), }; } + const agentCheckSessionId = validateSessionAgentExists(cfg, matches[0].key); + if (agentCheckSessionId) { + return agentCheckSessionId; + } return { ok: true, key: matches[0].key }; } @@ -197,5 +228,9 @@ export async function resolveSessionKeyFromResolveParams(params: { }; } + const agentCheckLabel = validateSessionAgentExists(cfg, list.sessions[0].key); + if (agentCheckLabel) { + return agentCheckLabel; + } return { ok: true, key: list.sessions[0].key }; }