fix: add orphaned session key migration (#57217)

* fix: add orphaned session key migration

* fix: address session migration review comments
This commit is contained in:
Ayaan Zaidi
2026-03-30 07:36:46 +05:30
committed by GitHub
parent 9d005e6fbb
commit b3c69b941e
5 changed files with 572 additions and 17 deletions

View File

@@ -0,0 +1,75 @@
import { describe, expect, it, vi } from "vitest";
import { runStartupSessionMigration } from "./server-startup-session-migration.js";
function makeLog() {
return {
info: vi.fn(),
warn: vi.fn(),
};
}
function makeCfg() {
return { agents: { defaults: {} }, session: {} } as Parameters<
typeof runStartupSessionMigration
>[0]["cfg"];
}
describe("runStartupSessionMigration", () => {
it("logs changes when orphaned keys are canonicalized", async () => {
const log = makeLog();
const migrate = vi.fn().mockResolvedValue({
changes: ["Canonicalized 2 orphaned session key(s) in /tmp/store.json"],
warnings: [],
});
await runStartupSessionMigration({
cfg: makeCfg(),
log,
deps: { migrateOrphanedSessionKeys: migrate },
});
expect(migrate).toHaveBeenCalledOnce();
expect(log.info).toHaveBeenCalledOnce();
expect(log.info.mock.calls[0][0]).toContain("canonicalized orphaned session keys");
expect(log.warn).not.toHaveBeenCalled();
});
it("logs warnings from migration", async () => {
const log = makeLog();
const migrate = vi.fn().mockResolvedValue({
changes: [],
warnings: ["Could not read /bad/path: ENOENT"],
});
await runStartupSessionMigration({
cfg: makeCfg(),
log,
deps: { migrateOrphanedSessionKeys: migrate },
});
expect(log.info).not.toHaveBeenCalled();
expect(log.warn).toHaveBeenCalledOnce();
expect(log.warn.mock.calls[0][0]).toContain("session key migration warnings");
});
it("silently continues when no changes needed", async () => {
const log = makeLog();
const migrate = vi.fn().mockResolvedValue({ changes: [], warnings: [] });
await runStartupSessionMigration({
cfg: makeCfg(),
log,
deps: { migrateOrphanedSessionKeys: migrate },
});
expect(log.info).not.toHaveBeenCalled();
expect(log.warn).not.toHaveBeenCalled();
});
it("catches and logs migration errors without throwing", async () => {
const log = makeLog();
const migrate = vi.fn().mockRejectedValue(new Error("disk full"));
await runStartupSessionMigration({
cfg: makeCfg(),
log,
deps: { migrateOrphanedSessionKeys: migrate },
});
expect(log.warn).toHaveBeenCalledOnce();
expect(log.warn.mock.calls[0][0]).toContain("migration failed during startup");
expect(log.warn.mock.calls[0][0]).toContain("disk full");
});
});

View File

@@ -0,0 +1,46 @@
import type { OpenClawConfig } from "../config/config.js";
import { migrateOrphanedSessionKeys } from "../infra/state-migrations.js";
type SessionMigrationLogger = {
info: (message: string) => void;
warn: (message: string) => void;
};
/**
* Run orphan-key session migration at gateway startup.
*
* Idempotent and best-effort: if the migration fails, gateway startup
* continues normally. This ensures accumulated orphaned session keys
* (from the write-path bug #29683) are cleaned up automatically on
* upgrade rather than requiring a manual `openclaw doctor` run.
*/
export async function runStartupSessionMigration(params: {
cfg: OpenClawConfig;
env?: NodeJS.ProcessEnv;
log: SessionMigrationLogger;
deps?: {
migrateOrphanedSessionKeys?: typeof migrateOrphanedSessionKeys;
};
}): Promise<void> {
const migrate = params.deps?.migrateOrphanedSessionKeys ?? migrateOrphanedSessionKeys;
try {
const result = await migrate({
cfg: params.cfg,
env: params.env ?? process.env,
});
if (result.changes.length > 0) {
params.log.info(
`gateway: canonicalized orphaned session keys:\n${result.changes.map((c) => `- ${c}`).join("\n")}`,
);
}
if (result.warnings.length > 0) {
params.log.warn(
`gateway: session key migration warnings:\n${result.warnings.map((w) => `- ${w}`).join("\n")}`,
);
}
} catch (err) {
params.log.warn(
`gateway: orphaned session key migration failed during startup; continuing: ${String(err)}`,
);
}
}

View File

@@ -122,6 +122,7 @@ import { createGatewayRuntimeState } from "./server-runtime-state.js";
import { resolveSessionKeyForRun } from "./server-session-key.js";
import { logGatewayStartup } from "./server-startup-log.js";
import { runStartupMatrixMigration } from "./server-startup-matrix-migration.js";
import { runStartupSessionMigration } from "./server-startup-session-migration.js";
import { startGatewaySidecars } from "./server-startup.js";
import { startGatewayTailscaleExposure } from "./server-tailscale.js";
import { createWizardSessionTracker } from "./server-wizard-sessions.js";
@@ -545,6 +546,11 @@ export async function startGatewayServer(
env: process.env,
log,
});
await runStartupSessionMigration({
cfg: cfgAtStart,
env: process.env,
log,
});
const matrixInstallPathIssue = await detectPluginInstallPathIssue({
pluginId: "matrix",
install: cfgAtStart.plugins?.installs?.matrix,

View File

@@ -0,0 +1,206 @@
import fs from "node:fs";
import os from "node:os";
import path from "node:path";
import { afterEach, beforeEach, describe, expect, it } from "vitest";
import type { OpenClawConfig } from "../config/config.js";
import { migrateOrphanedSessionKeys } from "./state-migrations.js";
function makeTmpDir(): string {
return fs.mkdtempSync(path.join(os.tmpdir(), "orphan-keys-test-"));
}
function writeStore(storePath: string, store: Record<string, unknown>): void {
fs.mkdirSync(path.dirname(storePath), { recursive: true });
fs.writeFileSync(storePath, JSON.stringify(store));
}
function readStore(storePath: string): Record<string, unknown> {
return JSON.parse(fs.readFileSync(storePath, "utf-8"));
}
describe("migrateOrphanedSessionKeys", () => {
let tmpDir: string;
let stateDir: string;
beforeEach(() => {
tmpDir = makeTmpDir();
stateDir = path.join(tmpDir, ".openclaw");
fs.mkdirSync(stateDir, { recursive: true });
});
afterEach(() => {
fs.rmSync(tmpDir, { recursive: true, force: true });
});
it("renames orphaned raw key to canonical form", async () => {
const storePath = path.join(stateDir, "agents", "ops", "sessions", "sessions.json");
writeStore(storePath, {
"agent:main:main": { sessionId: "abc-123", updatedAt: 1000 },
});
const cfg = {
session: { mainKey: "work" },
agents: { list: [{ id: "ops", default: true }] },
} as OpenClawConfig;
const result = await migrateOrphanedSessionKeys({
cfg,
env: { OPENCLAW_STATE_DIR: stateDir },
});
expect(result.changes.length).toBeGreaterThan(0);
const store = readStore(storePath);
expect(store["agent:ops:work"]).toBeDefined();
expect((store["agent:ops:work"] as { sessionId: string }).sessionId).toBe("abc-123");
expect(store["agent:main:main"]).toBeUndefined();
});
it("keeps most recently updated entry when both orphan and canonical exist", async () => {
const storePath = path.join(stateDir, "agents", "ops", "sessions", "sessions.json");
writeStore(storePath, {
"agent:main:main": { sessionId: "old-orphan", updatedAt: 500 },
"agent:ops:work": { sessionId: "current", updatedAt: 2000 },
});
const cfg = {
session: { mainKey: "work" },
agents: { list: [{ id: "ops", default: true }] },
} as OpenClawConfig;
await migrateOrphanedSessionKeys({
cfg,
env: { OPENCLAW_STATE_DIR: stateDir },
});
const store = readStore(storePath);
expect((store["agent:ops:work"] as { sessionId: string }).sessionId).toBe("current");
expect(store["agent:main:main"]).toBeUndefined();
});
it("skips stores that are already fully canonical", async () => {
const storePath = path.join(stateDir, "agents", "ops", "sessions", "sessions.json");
writeStore(storePath, {
"agent:ops:work": { sessionId: "abc-123", updatedAt: 1000 },
});
const cfg = {
session: { mainKey: "work" },
agents: { list: [{ id: "ops", default: true }] },
} as OpenClawConfig;
const result = await migrateOrphanedSessionKeys({
cfg,
env: { OPENCLAW_STATE_DIR: stateDir },
});
expect(result.changes).toHaveLength(0);
expect(result.warnings).toHaveLength(0);
});
it("handles missing store files gracefully", async () => {
const cfg = {
session: { mainKey: "work" },
agents: { list: [{ id: "ops", default: true }] },
} as OpenClawConfig;
const result = await migrateOrphanedSessionKeys({
cfg,
env: { OPENCLAW_STATE_DIR: stateDir },
});
expect(result.changes).toHaveLength(0);
expect(result.warnings).toHaveLength(0);
});
it("is idempotent — running twice produces same result", async () => {
const storePath = path.join(stateDir, "agents", "ops", "sessions", "sessions.json");
writeStore(storePath, {
"agent:main:main": { sessionId: "abc-123", updatedAt: 1000 },
});
const cfg = {
session: { mainKey: "work" },
agents: { list: [{ id: "ops", default: true }] },
} as OpenClawConfig;
const env = { OPENCLAW_STATE_DIR: stateDir };
await migrateOrphanedSessionKeys({ cfg, env });
const result2 = await migrateOrphanedSessionKeys({ cfg, env });
expect(result2.changes).toHaveLength(0);
const store = readStore(storePath);
expect((store["agent:ops:work"] as { sessionId: string }).sessionId).toBe("abc-123");
});
it("preserves legitimate agent:main:* keys in shared stores with both main and non-main agents", async () => {
// When session.store lacks {agentId}, all agents resolve to the same file.
// The "main" agent's keys must not be remapped into the "ops" namespace.
const sharedStorePath = path.join(tmpDir, "shared-sessions.json");
writeStore(sharedStorePath, {
"agent:main:main": { sessionId: "main-session", updatedAt: 2000 },
"agent:ops:work": { sessionId: "ops-session", updatedAt: 1000 },
});
const cfg = {
session: { mainKey: "work", store: sharedStorePath },
agents: { list: [{ id: "main" }, { id: "ops", default: true }] },
} as OpenClawConfig;
await migrateOrphanedSessionKeys({
cfg,
env: { OPENCLAW_STATE_DIR: stateDir },
});
const store = readStore(sharedStorePath);
// main agent's session is canonicalised to use configured mainKey ("work"),
// but stays in the "main" agent namespace — NOT remapped into "ops".
expect(store["agent:main:work"]).toBeDefined();
expect((store["agent:main:work"] as { sessionId: string }).sessionId).toBe("main-session");
expect(store["agent:ops:work"]).toBeDefined();
expect((store["agent:ops:work"] as { sessionId: string }).sessionId).toBe("ops-session");
// The key must NOT have been merged into ops namespace
expect(Object.keys(store).filter((k) => k.startsWith("agent:ops:")).length).toBe(1);
});
it("lets the main agent claim bare main aliases in shared stores", async () => {
const sharedStorePath = path.join(tmpDir, "shared-sessions.json");
writeStore(sharedStorePath, {
main: { sessionId: "main-session", updatedAt: 2000 },
"agent:ops:work": { sessionId: "ops-session", updatedAt: 1000 },
});
const cfg = {
session: { mainKey: "work", store: sharedStorePath },
agents: { list: [{ id: "main" }, { id: "ops", default: true }] },
} as OpenClawConfig;
await migrateOrphanedSessionKeys({
cfg,
env: { OPENCLAW_STATE_DIR: stateDir },
});
const store = readStore(sharedStorePath);
expect(store["agent:main:work"]).toBeDefined();
expect((store["agent:main:work"] as { sessionId: string }).sessionId).toBe("main-session");
expect(store.main).toBeUndefined();
expect(store["agent:ops:work"]).toBeDefined();
});
it("no-ops when default agentId is main and mainKey is main", async () => {
const storePath = path.join(stateDir, "agents", "main", "sessions", "sessions.json");
writeStore(storePath, {
"agent:main:main": { sessionId: "abc-123", updatedAt: 1000 },
});
const cfg = {} as OpenClawConfig;
const result = await migrateOrphanedSessionKeys({
cfg,
env: { OPENCLAW_STATE_DIR: stateDir },
});
expect(result.changes).toHaveLength(0);
const store = readStore(storePath);
expect(store["agent:main:main"]).toBeDefined();
});
});

View File

@@ -19,9 +19,13 @@ import { resolveChannelAllowFromPath } from "../pairing/pairing-store.js";
import {
buildAgentMainSessionKey,
DEFAULT_ACCOUNT_ID,
DEFAULT_AGENT_ID,
DEFAULT_MAIN_KEY,
normalizeAgentId,
normalizeMainKey,
parseAgentSessionKey,
} from "../routing/session-key.js";
import { expandHomePrefix } from "./home-dir.js";
import { isWithinDir } from "./path-safety.js";
import {
ensureDir,
@@ -133,6 +137,7 @@ function canonicalizeSessionKeyForAgent(params: {
agentId: string;
mainKey: string;
scope?: SessionScope;
skipCrossAgentRemap?: boolean;
}): string {
const agentId = normalizeAgentId(params.agentId);
const raw = params.key.trim();
@@ -143,6 +148,25 @@ function canonicalizeSessionKeyForAgent(params: {
return raw.toLowerCase();
}
// When shared-store guard is active, do not remap keys that belong to a
// different agent — they are legitimate records for that agent, not orphans.
// Without this check, canonicalizeMainSessionAlias (which now recognises
// legacy agent:main:* aliases) would rewrite them before the
// skipCrossAgentRemap guard below has a chance to block it.
if (params.skipCrossAgentRemap) {
const parsed = parseAgentSessionKey(raw);
if (parsed && normalizeAgentId(parsed.agentId) !== agentId) {
return raw.toLowerCase();
}
const rawLower = raw.toLowerCase();
if (
agentId !== DEFAULT_AGENT_ID &&
(rawLower === DEFAULT_MAIN_KEY || rawLower === params.mainKey)
) {
return rawLower;
}
}
const canonicalMain = canonicalizeMainSessionAlias({
cfg: { session: { scope: params.scope, mainKey: params.mainKey } },
agentId,
@@ -152,6 +176,31 @@ function canonicalizeSessionKeyForAgent(params: {
return canonicalMain.toLowerCase();
}
// Handle cross-agent orphaned main-session keys: "agent:main:main" or
// "agent:main:<mainKey>" in a store belonging to a different agent (e.g.
// "ops"). Only remap provable orphan aliases — other agent:main:* keys
// (hooks, subagents, cron, per-sender) may be intentional cross-agent
// references and must not be touched (#29683).
const defaultPrefix = `agent:${DEFAULT_AGENT_ID}:`;
const rawLower = raw.toLowerCase();
if (
rawLower.startsWith(defaultPrefix) &&
agentId !== DEFAULT_AGENT_ID &&
!params.skipCrossAgentRemap
) {
const rest = rawLower.slice(defaultPrefix.length);
const isOrphanAlias = rest === DEFAULT_MAIN_KEY || rest === params.mainKey;
if (isOrphanAlias) {
const remapped = `agent:${agentId}:${rest}`;
const canonicalized = canonicalizeMainSessionAlias({
cfg: { session: { scope: params.scope, mainKey: params.mainKey } },
agentId,
sessionKey: remapped,
});
return canonicalized.toLowerCase();
}
}
if (raw.toLowerCase().startsWith("agent:")) {
return raw.toLowerCase();
}
@@ -265,6 +314,7 @@ function canonicalizeSessionStore(params: {
agentId: string;
mainKey: string;
scope?: SessionScope;
skipCrossAgentRemap?: boolean;
}): { store: Record<string, SessionEntryLike>; legacyKeys: string[] } {
const canonical: Record<string, SessionEntryLike> = {};
const meta = new Map<string, { isCanonical: boolean; updatedAt: number }>();
@@ -279,6 +329,7 @@ function canonicalizeSessionStore(params: {
agentId: params.agentId,
mainKey: params.mainKey,
scope: params.scope,
skipCrossAgentRemap: params.skipCrossAgentRemap,
});
const isCanonical = canonicalKey === key;
if (!isCanonical) {
@@ -981,6 +1032,146 @@ export async function autoMigrateLegacyAgentDir(params: {
return await autoMigrateLegacyState(params);
}
/**
* Canonicalize orphaned raw session keys in all known agent session stores.
*
* Keys written by resolveSessionKey() used DEFAULT_AGENT_ID="main" regardless
* of the configured default agent; reads always use resolveSessionStoreKey()
* which canonicalizes via canonicalizeMainSessionAlias. This migration renames
* any orphaned raw keys to their canonical form in-place, merging with any
* existing canonical entry by preferring the most recently updated.
*
* Safe to run multiple times (idempotent). See #29683.
*/
export async function migrateOrphanedSessionKeys(params: {
cfg: OpenClawConfig;
env?: NodeJS.ProcessEnv;
}): Promise<{ changes: string[]; warnings: string[] }> {
const changes: string[] = [];
const warnings: string[] = [];
const env = params.env ?? process.env;
const stateDir = resolveStateDir(env);
const agentId = normalizeAgentId(resolveDefaultAgentId(params.cfg));
const mainKey = normalizeMainKey(params.cfg.session?.mainKey);
const scope = params.cfg.session?.scope as SessionScope | undefined;
const storeConfig = params.cfg.session?.store;
// Collect all known agent store paths with their owning agentIds.
// A single path may be shared by multiple agents when session.store
// does not contain {agentId}.
const storeMap = new Map<string, Set<string>>();
const addToStoreMap = (p: string, id: string) => {
const existing = storeMap.get(p);
if (existing) {
existing.add(id);
} else {
storeMap.set(p, new Set([id]));
}
};
// Default agent store.
const defaultStorePath = storeConfig
? resolveStorePathFromTemplate(storeConfig, agentId, env)
: path.join(stateDir, "agents", agentId, "sessions", "sessions.json");
addToStoreMap(defaultStorePath, agentId);
// Configured agents.
for (const entry of params.cfg.agents?.list ?? []) {
if (entry?.id) {
const id = normalizeAgentId(entry.id);
const p = storeConfig
? resolveStorePathFromTemplate(storeConfig, id, env)
: path.join(stateDir, "agents", id, "sessions", "sessions.json");
addToStoreMap(p, id);
}
}
// Agent directories present on disk.
// This only covers the standard state-dir layout so we can still pick up
// orphaned stores left behind by older configs. Active custom-template paths
// are already covered by the configured-agents loop above.
const agentsDir = path.join(stateDir, "agents");
if (existsDir(agentsDir)) {
for (const dirEntry of safeReadDir(agentsDir)) {
if (dirEntry.isDirectory()) {
const diskAgentId = normalizeAgentId(dirEntry.name);
if (diskAgentId) {
const diskPath = path.join(agentsDir, diskAgentId, "sessions", "sessions.json");
addToStoreMap(diskPath, diskAgentId);
}
}
}
}
for (const [storePath, storeAgentIds] of storeMap) {
if (!fileExists(storePath)) {
continue;
}
let parsed: ReturnType<typeof readSessionStoreJson5>;
try {
parsed = readSessionStoreJson5(storePath);
} catch (err) {
warnings.push(`Could not read ${storePath}: ${String(err)}`);
continue;
}
if (!parsed.ok) {
continue;
}
// When multiple agents share a single store file (session.store without
// {agentId}), run canonicalization once per agent so each agent's keys are
// handled correctly. Skip cross-agent "agent:main:*" remapping when "main"
// is a legitimate configured agent to avoid merging its data into another
// agent's namespace.
let working = parsed.store;
let totalLegacy = 0;
for (const storeAgentId of storeAgentIds) {
const { store: canonicalized, legacyKeys } = canonicalizeSessionStore({
store: working,
agentId: storeAgentId,
mainKey,
scope,
// When multiple agents share the store and "main" is one of them,
// agent:main:* keys are legitimate — don't cross-agent remap them.
skipCrossAgentRemap: storeAgentIds.size > 1 && storeAgentIds.has(DEFAULT_AGENT_ID),
});
working = canonicalized;
// Each pass only counts keys it changed from the current working store, so
// once a key is canonicalized it is not counted again by later agent passes.
totalLegacy += legacyKeys.length;
}
if (totalLegacy === 0) {
continue;
}
const normalized: Record<string, SessionEntry> = {};
for (const [key, entry] of Object.entries(working)) {
const ne = normalizeSessionEntry(entry);
if (ne) {
normalized[key] = ne;
}
}
try {
await saveSessionStore(storePath, normalized, { skipMaintenance: true });
changes.push(`Canonicalized ${totalLegacy} orphaned session key(s) in ${storePath}`);
} catch (err) {
warnings.push(`Failed to write canonicalized store ${storePath}: ${String(err)}`);
}
}
return { changes, warnings };
}
function resolveStorePathFromTemplate(
template: string,
agentId: string,
env?: NodeJS.ProcessEnv,
): string {
const expand = (s: string) =>
s.startsWith("~") ? expandHomePrefix(s, { env: env ?? process.env, homedir: os.homedir }) : s;
if (template.includes("{agentId}")) {
return path.resolve(expand(template.replaceAll("{agentId}", agentId)));
}
return path.resolve(expand(template));
}
export async function autoMigrateLegacyState(params: {
cfg: OpenClawConfig;
env?: NodeJS.ProcessEnv;
@@ -1004,12 +1195,38 @@ export async function autoMigrateLegacyState(params: {
homedir: params.homedir,
log: params.log,
});
// Canonicalize orphaned session keys regardless of whether legacy migration
// is needed — the orphan-key bug (#29683) affects all installs with
// non-default agent IDs or mainKey configuration.
const orphanKeys = await migrateOrphanedSessionKeys({
cfg: params.cfg,
env,
});
const logMigrationResults = (changes: string[], warnings: string[]) => {
const logger = params.log ?? createSubsystemLogger("state-migrations");
if (changes.length > 0) {
logger.info(
`Auto-migrated legacy state:\n${changes.map((entry) => `- ${entry}`).join("\n")}`,
);
}
if (warnings.length > 0) {
logger.warn(
`Legacy state migration warnings:\n${warnings.map((entry) => `- ${entry}`).join("\n")}`,
);
}
};
if (env.OPENCLAW_AGENT_DIR?.trim() || env.PI_CODING_AGENT_DIR?.trim()) {
const changes = [...stateDirResult.changes, ...orphanKeys.changes];
const warnings = [...stateDirResult.warnings, ...orphanKeys.warnings];
logMigrationResults(changes, warnings);
return {
migrated: stateDirResult.migrated,
migrated: stateDirResult.migrated || orphanKeys.changes.length > 0,
skipped: true,
changes: stateDirResult.changes,
warnings: stateDirResult.warnings,
changes,
warnings,
};
}
@@ -1019,29 +1236,34 @@ export async function autoMigrateLegacyState(params: {
homedir: params.homedir,
});
if (!detected.sessions.hasLegacy && !detected.agentDir.hasLegacy) {
const changes = [...stateDirResult.changes, ...orphanKeys.changes];
const warnings = [...stateDirResult.warnings, ...orphanKeys.warnings];
logMigrationResults(changes, warnings);
return {
migrated: stateDirResult.migrated,
migrated: stateDirResult.migrated || orphanKeys.changes.length > 0,
skipped: false,
changes: stateDirResult.changes,
warnings: stateDirResult.warnings,
changes,
warnings,
};
}
const now = params.now ?? (() => Date.now());
const sessions = await migrateLegacySessions(detected, now);
const agentDir = await migrateLegacyAgentDir(detected, now);
const changes = [...stateDirResult.changes, ...sessions.changes, ...agentDir.changes];
const warnings = [...stateDirResult.warnings, ...sessions.warnings, ...agentDir.warnings];
const changes = [
...stateDirResult.changes,
...orphanKeys.changes,
...sessions.changes,
...agentDir.changes,
];
const warnings = [
...stateDirResult.warnings,
...orphanKeys.warnings,
...sessions.warnings,
...agentDir.warnings,
];
const logger = params.log ?? createSubsystemLogger("state-migrations");
if (changes.length > 0) {
logger.info(`Auto-migrated legacy state:\n${changes.map((entry) => `- ${entry}`).join("\n")}`);
}
if (warnings.length > 0) {
logger.warn(
`Legacy state migration warnings:\n${warnings.map((entry) => `- ${entry}`).join("\n")}`,
);
}
logMigrationResults(changes, warnings);
return {
migrated: changes.length > 0,