mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-27 12:56:01 -06:00
fix(sessions): scope legacy-main owner notice to real legacy rows (#126551)
* fix(sessions): scope legacy-main owner notice to real legacy rows resolveArmingDecision returns owner-unresolved purely from config, before any session store is read, so the unresolved-owner notice and the agents-create gate both fired without checking whether any legacy `main` rows exist. A new explicit-ownership fleet with zero session data printed "legacy main rows have no unambiguous configured owner" on every startup, and `agents create main` dead-ended: it told operators to run `openclaw doctor --fix`, which cannot resolve anything when there is nothing to migrate. Probe the candidate stores in the unresolved-owner branch and report no-legacy-rows when every store proves clean. Unreadable stores, legacy JSON stores, and genuine read failures still fail open and keep the notice, so the precaution survives exactly where absence cannot be proven. The main-creation gate now blocks an unarmed run only when the scan did not prove the fleet clean; armed runs still require a completed ledger. Store scanning moves to legacy-main-session-key-scan.ts: the probe and the armed claim reader share one normalization-aware key predicate, so a store holding `agent:Main:...` or another key that normalizes to `main` cannot be misread as clean, and the migration module stays under the max-lines limit. * test(gateway): stop compaction read-error mocks depending on factory order src/gateway/server.sessions.compaction-read-errors.test.ts captured the real loadTranscriptEvents as a side effect of its vi.mock factory, then required it in beforeEach. The factory runs on first import of the mocked module, so once a shard shares a worker (--isolate=false) the field could still be unset and all five tests failed with "transcript reader mock was not initialized". Resolve the real reader with vi.importActual in beforeAll instead, so the default implementation no longer depends on whether the factory has run. Pre-existing on main:0a8226c3fails the same checks-node-compact-small-4 shard with the identical five tests. Not reproducible locally, including with the exact 25-file shard composition; CI on Linux runners is the verification. * test(gateway): load the mocked transcript reader deterministically src/gateway/server.sessions.compaction-read-errors.test.ts captures the real loadTranscriptEvents as a side effect of its vi.mock factory and requires it in beforeEach. The factory only runs when the mocked module is first imported, and nothing in this file imported it directly, so once a shard shares a worker (--isolate=false) the capture could still be unset and all five tests failed with "transcript reader mock was not initialized". Import the mocked module for its side effect so the factory always runs. Resolving the reader with vi.importActual instead was wrong: that returns a separately instantiated, unmocked copy that does not share the module-level SQLite state the harness sets up, so it read no events, compaction no-opped successfully and the three rejection tests failed on response.ok. Capturing through importOriginal keeps the harness's own module instance. Pre-existing on main:0a8226c3fails the same checks-node-compact-small-4 shard with the identical five tests.
This commit is contained in:
committed by
GitHub
parent
f612675284
commit
a947730c66
@@ -256,6 +256,27 @@ describe("createAgent", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("creates main when an unarmed scan proves every legacy store clean", async () => {
|
||||
mocks.config = { agents: { entries: { robby: { id: "robby" } } } };
|
||||
mocks.migrateLegacyMainSessionKeys.mockResolvedValueOnce({
|
||||
armed: false,
|
||||
changes: [],
|
||||
complete: true,
|
||||
ledgerComplete: false,
|
||||
legacyAgentId: "main",
|
||||
mainKey: "main",
|
||||
outcomes: [{ kind: "no-legacy-rows", detail: "no configured owner" }],
|
||||
warnings: [],
|
||||
});
|
||||
|
||||
await expect(createAgent({ name: "main" })).resolves.toMatchObject({
|
||||
status: "created",
|
||||
agentId: "main",
|
||||
});
|
||||
expect(mocks.resolveSharedAuthStoreOwnership).toHaveBeenCalledOnce();
|
||||
expect(mocks.transformConfigFileWithRetry).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it("defaults the workspace through the agent-scoped resolver", async () => {
|
||||
const result = await createAgent({ name: "Researcher" });
|
||||
|
||||
|
||||
@@ -176,7 +176,10 @@ async function evaluateMainCreationGate(
|
||||
legacyAgentId: BOOTSTRAP_AGENT_ID,
|
||||
mode: "detect",
|
||||
});
|
||||
if (!migration.armed || !migration.ledgerComplete) {
|
||||
// An unarmed scan can proceed only when every candidate store proved collision-free.
|
||||
const provenClean = migration.outcomes.every((outcome) => outcome.kind === "no-legacy-rows");
|
||||
const blocked = migration.armed ? !migration.ledgerComplete : !provenClean;
|
||||
if (blocked) {
|
||||
const details = migration.outcomes.map(describeLegacySessionOutcome).join("; ");
|
||||
return createError(
|
||||
"legacy-session-migration-required",
|
||||
|
||||
@@ -0,0 +1,77 @@
|
||||
import { executeSqliteQuerySync } from "../../infra/kysely-sync.js";
|
||||
import { normalizeAgentId, parseAgentSessionKey } from "../../routing/session-key.js";
|
||||
import { withOpenClawAgentDatabaseReadOnly } from "../../state/openclaw-agent-db-readonly.js";
|
||||
import { readClaim } from "./legacy-main-session-migration-operations.js";
|
||||
import type { PhysicalStore, SessionClaim } from "./legacy-main-session-migration.contract.js";
|
||||
import { getSessionKysely } from "./session-accessor.sqlite-scope.js";
|
||||
|
||||
/** Returns the stored `agent:<id>:` prefix when the key is owned by the legacy agent. */
|
||||
function legacyAgentKeyPrefix(key: string, legacyAgentId: string): string | null {
|
||||
const parsed = parseAgentSessionKey(key);
|
||||
if (!parsed || normalizeAgentId(parsed.agentId) !== legacyAgentId) {
|
||||
return null;
|
||||
}
|
||||
const prefix = `agent:${parsed.agentId}:`;
|
||||
return key.startsWith(prefix) ? prefix : null;
|
||||
}
|
||||
|
||||
function canonicalKeyFor(key: string, legacyAgentId: string, ownerAgentId: string): string | null {
|
||||
const prefix = legacyAgentKeyPrefix(key, legacyAgentId);
|
||||
return prefix ? `agent:${ownerAgentId}:${key.slice(prefix.length)}` : null;
|
||||
}
|
||||
|
||||
export function storeHasLegacyAgentSessionKey(params: {
|
||||
legacyAgentId: string;
|
||||
store: PhysicalStore;
|
||||
env: NodeJS.ProcessEnv;
|
||||
}): boolean {
|
||||
const result = withOpenClawAgentDatabaseReadOnly(
|
||||
(database) =>
|
||||
executeSqliteQuerySync(
|
||||
database.db,
|
||||
getSessionKysely(database.db).selectFrom("session_nodes").select("session_key"),
|
||||
).rows.some((row) => legacyAgentKeyPrefix(row.session_key, params.legacyAgentId) !== null),
|
||||
{ agentId: params.store.databaseAgentId, env: params.env, path: params.store.path },
|
||||
);
|
||||
// A missing database, schema, or table proves absence exactly as the armed claim
|
||||
// reader does below; only genuine read failures throw and let the caller fail open.
|
||||
return result.found ? result.value : false;
|
||||
}
|
||||
|
||||
export function readClaimsFromStore(params: {
|
||||
legacyAgentId: string;
|
||||
ownerAgentId: string;
|
||||
store: PhysicalStore;
|
||||
env: NodeJS.ProcessEnv;
|
||||
}): { canonical: SessionClaim[]; legacy: SessionClaim[] } {
|
||||
const result = withOpenClawAgentDatabaseReadOnly(
|
||||
(database) => {
|
||||
const keys = executeSqliteQuerySync(
|
||||
database.db,
|
||||
getSessionKysely(database.db).selectFrom("session_nodes").select("session_key"),
|
||||
).rows.map((row) => row.session_key);
|
||||
const legacy: SessionClaim[] = [];
|
||||
const canonical: SessionClaim[] = [];
|
||||
for (const key of keys) {
|
||||
const canonicalKey = canonicalKeyFor(key, params.legacyAgentId, params.ownerAgentId);
|
||||
if (canonicalKey) {
|
||||
const claim = readClaim(database, params.store, key, canonicalKey);
|
||||
if (claim) {
|
||||
legacy.push(claim);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
const parsed = parseAgentSessionKey(key);
|
||||
if (parsed && normalizeAgentId(parsed.agentId) === params.ownerAgentId) {
|
||||
const claim = readClaim(database, params.store, key, key);
|
||||
if (claim) {
|
||||
canonical.push(claim);
|
||||
}
|
||||
}
|
||||
}
|
||||
return { canonical, legacy };
|
||||
},
|
||||
{ agentId: params.store.databaseAgentId, env: params.env, path: params.store.path },
|
||||
);
|
||||
return result.found ? result.value : { canonical: [], legacy: [] };
|
||||
}
|
||||
@@ -559,6 +559,11 @@ describe("legacy main session migration", () => {
|
||||
const unresolved = createFixture({
|
||||
agents: { ownership: "explicit", entries: { ops: {}, research: {} } },
|
||||
});
|
||||
seedClaim({
|
||||
databaseAgentId: "main",
|
||||
databasePath: databasePath(unresolved.stateDir, "main"),
|
||||
key: "agent:main:chat",
|
||||
});
|
||||
const perAgentPinned = createFixture({
|
||||
agents: {
|
||||
ownership: "explicit",
|
||||
@@ -592,6 +597,49 @@ describe("legacy main session migration", () => {
|
||||
expect(notArmed.warnings[0]).toContain("agents.defaults.sessionStore.agentId");
|
||||
});
|
||||
|
||||
it("proves an unresolved-owner store clean when it has no legacy rows", async () => {
|
||||
const fixture = createFixture({
|
||||
agents: { ownership: "explicit", entries: { ops: {}, research: {} } },
|
||||
});
|
||||
|
||||
const result = await migrateLegacyMainSessionKeys({
|
||||
cfg: fixture.cfg,
|
||||
env: fixture.env,
|
||||
mode: "detect",
|
||||
});
|
||||
|
||||
expect(result).toMatchObject({
|
||||
armed: false,
|
||||
complete: true,
|
||||
ledgerComplete: false,
|
||||
outcomes: [{ kind: "no-legacy-rows", detail: "no configured owner" }],
|
||||
warnings: [],
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps unresolved ownership advisory when a candidate store is unreadable", async () => {
|
||||
const unreadablePath = path.join(tempDirs.make("unresolved-unreadable-"), "sessions.sqlite");
|
||||
fs.symlinkSync(`${unreadablePath}.missing`, unreadablePath);
|
||||
const fixture = createFixture({
|
||||
agents: { ownership: "explicit", entries: { ops: {}, research: {} } },
|
||||
session: { store: unreadablePath },
|
||||
});
|
||||
|
||||
const result = await migrateLegacyMainSessionKeys({
|
||||
cfg: fixture.cfg,
|
||||
env: fixture.env,
|
||||
mode: "automatic",
|
||||
});
|
||||
|
||||
expect(result).toMatchObject({
|
||||
armed: false,
|
||||
complete: false,
|
||||
ledgerComplete: false,
|
||||
outcomes: [{ kind: "not-armed", detail: "owner-unresolved" }],
|
||||
});
|
||||
expect(result.warnings[0]).toContain("agents.defaults.sessionStore.agentId");
|
||||
});
|
||||
|
||||
it("uses an explicit migration owner for retired main rows in per-agent stores", async () => {
|
||||
const fixture = createFixture({
|
||||
agents: {
|
||||
|
||||
@@ -8,22 +8,20 @@ import {
|
||||
executeSqliteQueryTakeFirstSync,
|
||||
getNodeSqliteKysely,
|
||||
} from "../../infra/kysely-sync.js";
|
||||
import {
|
||||
normalizeAgentId,
|
||||
normalizeMainKey,
|
||||
parseAgentSessionKey,
|
||||
} from "../../routing/session-key.js";
|
||||
import { withOpenClawAgentDatabaseReadOnly } from "../../state/openclaw-agent-db-readonly.js";
|
||||
import { normalizeAgentId, normalizeMainKey } from "../../routing/session-key.js";
|
||||
import { isSameOpenClawAgentDatabasePath } from "../../state/openclaw-agent-db-registry.js";
|
||||
import { withExistingOpenClawStateDatabaseReadOnly } from "../../state/openclaw-state-db-readonly.js";
|
||||
import type { DB as OpenClawStateKyselyDatabase } from "../../state/openclaw-state-db.generated.js";
|
||||
import { runOpenClawStateWriteTransaction } from "../../state/openclaw-state-db.js";
|
||||
import { resolveStateDir } from "../paths.js";
|
||||
import type { OpenClawConfig } from "../types.openclaw.js";
|
||||
import {
|
||||
readClaimsFromStore,
|
||||
storeHasLegacyAgentSessionKey,
|
||||
} from "./legacy-main-session-key-scan.js";
|
||||
import {
|
||||
claimsMatch,
|
||||
processIdenticalClaims,
|
||||
readClaim,
|
||||
repairDivergentClaims,
|
||||
samePhysicalStore,
|
||||
warningForDivergence,
|
||||
@@ -36,11 +34,11 @@ import type {
|
||||
SessionClaim,
|
||||
} from "./legacy-main-session-migration.contract.js";
|
||||
import { resolveSessionStorePathCore } from "./paths.js";
|
||||
import { getSessionKysely } from "./session-accessor.sqlite-scope.js";
|
||||
import { resolveSqliteTargetFromSessionStorePath } from "./session-sqlite-target.js";
|
||||
import {
|
||||
resolveAllAgentSessionStoreCandidateTargetsSync,
|
||||
resolveAgentSessionStoreTargetsSync,
|
||||
resolveSessionStoreCompatibilityAgentId,
|
||||
} from "./targets.js";
|
||||
|
||||
const SOURCE_KEY = "legacy-main-session-keys";
|
||||
@@ -82,15 +80,6 @@ function resolveArmingDecision(cfg: OpenClawConfig, legacyAgentId: string): Armi
|
||||
return { armed: false, reason: "owner-unresolved" };
|
||||
}
|
||||
|
||||
function canonicalKeyFor(key: string, legacyAgentId: string, ownerAgentId: string): string | null {
|
||||
const parsed = parseAgentSessionKey(key);
|
||||
if (!parsed || normalizeAgentId(parsed.agentId) !== legacyAgentId) {
|
||||
return null;
|
||||
}
|
||||
const prefix = `agent:${parsed.agentId}:`;
|
||||
return key.startsWith(prefix) ? `agent:${ownerAgentId}:${key.slice(prefix.length)}` : null;
|
||||
}
|
||||
|
||||
function addPhysicalStore(stores: PhysicalStore[], candidate: PhysicalStore): void {
|
||||
if (!stores.some((store) => samePhysicalStore(store, candidate))) {
|
||||
stores.push(candidate);
|
||||
@@ -160,7 +149,7 @@ function resolvePhysicalStores(params: {
|
||||
env: NodeJS.ProcessEnv;
|
||||
legacyAgentId: string;
|
||||
mode: LegacyMainSessionMigrationMode;
|
||||
ownerAgentId: string;
|
||||
ownerAgentId?: string;
|
||||
}): ResolvedPhysicalStores {
|
||||
const logicalTargets = [
|
||||
...resolveAllAgentSessionStoreCandidateTargetsSync(params.cfg, { env: params.env }),
|
||||
@@ -172,14 +161,17 @@ function resolvePhysicalStores(params: {
|
||||
env: params.env,
|
||||
}),
|
||||
},
|
||||
{
|
||||
];
|
||||
if (params.ownerAgentId) {
|
||||
logicalTargets.push({
|
||||
agentId: params.ownerAgentId,
|
||||
storePath: resolveSessionStorePathCore(params.cfg.session?.store, {
|
||||
agentId: params.ownerAgentId,
|
||||
env: params.env,
|
||||
}),
|
||||
},
|
||||
];
|
||||
});
|
||||
}
|
||||
const defaultAgentId = params.ownerAgentId ?? resolveSessionStoreCompatibilityAgentId(params.cfg);
|
||||
const stores: PhysicalStore[] = [];
|
||||
const jsonPaths = new Set<string>();
|
||||
const unreadable: LegacyMainSessionMigrationOutcome[] = [];
|
||||
@@ -190,7 +182,7 @@ function resolvePhysicalStores(params: {
|
||||
}
|
||||
const resolved = resolveSqliteTargetFromSessionStorePath(target.storePath, {
|
||||
agentId: target.agentId,
|
||||
defaultAgentId: params.ownerAgentId,
|
||||
defaultAgentId,
|
||||
env: params.env,
|
||||
});
|
||||
const physical: PhysicalStore = {
|
||||
@@ -229,47 +221,6 @@ function resolveSourceLayout(resolved: ResolvedPhysicalStores): string[] {
|
||||
].toSorted();
|
||||
}
|
||||
|
||||
function readClaimsFromStore(params: {
|
||||
legacyAgentId: string;
|
||||
ownerAgentId: string;
|
||||
store: PhysicalStore;
|
||||
env: NodeJS.ProcessEnv;
|
||||
}): { canonical: SessionClaim[]; legacy: SessionClaim[] } {
|
||||
if (inspectPath(params.store.path) === "missing") {
|
||||
return { canonical: [], legacy: [] };
|
||||
}
|
||||
const result = withOpenClawAgentDatabaseReadOnly(
|
||||
(database) => {
|
||||
const keys = executeSqliteQuerySync(
|
||||
database.db,
|
||||
getSessionKysely(database.db).selectFrom("session_nodes").select("session_key"),
|
||||
).rows.map((row) => row.session_key);
|
||||
const legacy: SessionClaim[] = [];
|
||||
const canonical: SessionClaim[] = [];
|
||||
for (const key of keys) {
|
||||
const canonicalKey = canonicalKeyFor(key, params.legacyAgentId, params.ownerAgentId);
|
||||
if (canonicalKey) {
|
||||
const claim = readClaim(database, params.store, key, canonicalKey);
|
||||
if (claim) {
|
||||
legacy.push(claim);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
const parsed = parseAgentSessionKey(key);
|
||||
if (parsed && normalizeAgentId(parsed.agentId) === params.ownerAgentId) {
|
||||
const claim = readClaim(database, params.store, key, key);
|
||||
if (claim) {
|
||||
canonical.push(claim);
|
||||
}
|
||||
}
|
||||
}
|
||||
return { canonical, legacy };
|
||||
},
|
||||
{ agentId: params.store.databaseAgentId, env: params.env, path: params.store.path },
|
||||
);
|
||||
return result.found ? result.value : { canonical: [], legacy: [] };
|
||||
}
|
||||
|
||||
function readLedger(env: NodeJS.ProcessEnv): { report: LedgerReport; status: string } | undefined {
|
||||
return (
|
||||
withExistingOpenClawStateDatabaseReadOnly(
|
||||
@@ -420,6 +371,38 @@ async function migrateLegacyMainSessionKeysInternal(params: {
|
||||
warnings: [] as string[],
|
||||
};
|
||||
if (!arming.armed) {
|
||||
if (arming.reason === "owner-unresolved") {
|
||||
// Owner guidance is useful only when legacy rows may exist; unreadable or JSON
|
||||
// candidates fail open because they cannot prove the fleet is clean.
|
||||
let rowsMayExist: boolean;
|
||||
try {
|
||||
const resolved = resolvePhysicalStores({
|
||||
cfg: params.cfg,
|
||||
env,
|
||||
legacyAgentId,
|
||||
mode: params.mode,
|
||||
});
|
||||
rowsMayExist =
|
||||
resolved.unreadable.length > 0 ||
|
||||
resolved.jsonPaths.length > 0 ||
|
||||
resolved.stores.some(
|
||||
(store) =>
|
||||
inspectPath(store.path) === "present" &&
|
||||
storeHasLegacyAgentSessionKey({ env, legacyAgentId, store }),
|
||||
);
|
||||
} catch {
|
||||
rowsMayExist = true;
|
||||
}
|
||||
if (!rowsMayExist) {
|
||||
return {
|
||||
...base,
|
||||
armed: false,
|
||||
complete: true,
|
||||
ledgerComplete: false,
|
||||
outcomes: [{ kind: "no-legacy-rows", detail: "no configured owner" }],
|
||||
};
|
||||
}
|
||||
}
|
||||
const unresolved = arming.reason === "owner-unresolved";
|
||||
return {
|
||||
...base,
|
||||
@@ -497,6 +480,9 @@ async function migrateLegacyMainSessionKeysInternal(params: {
|
||||
const allCanonical: SessionClaim[] = [];
|
||||
for (const store of resolved.stores) {
|
||||
try {
|
||||
if (inspectPath(store.path) === "missing") {
|
||||
continue;
|
||||
}
|
||||
const claims = readClaimsFromStore({ env, legacyAgentId, ownerAgentId, store });
|
||||
allLegacy.push(...claims.legacy);
|
||||
allCanonical.push(...claims.canonical);
|
||||
|
||||
@@ -4,6 +4,11 @@ import {
|
||||
appendTranscriptMessage,
|
||||
upsertSessionEntryCore,
|
||||
} from "../config/sessions/session-accessor.js";
|
||||
// Force the mocked module to load here: the factory below captures the real reader as a
|
||||
// side effect, and beforeEach needs that capture. Without this import the factory only ran
|
||||
// if some other module in the graph pulled it in first, which is not guaranteed once a
|
||||
// shard shares a worker (--isolate=false).
|
||||
import "../config/sessions/session-accessor.sqlite-read.js";
|
||||
import { rpcReq } from "./test-helpers.js";
|
||||
import {
|
||||
sessionStoreEntry,
|
||||
|
||||
@@ -976,6 +976,16 @@ describe("state migrations", () => {
|
||||
const cfg: OpenClawConfig = {
|
||||
agents: { ownership: "explicit", entries: { alpha: {}, beta: {} } },
|
||||
};
|
||||
runOpenClawAgentWriteTransaction(
|
||||
(database) =>
|
||||
writeSessionEntry(
|
||||
database,
|
||||
"agent:main:chat",
|
||||
{ sessionId: "legacy-main-session", updatedAt: 100 },
|
||||
{ allowStoredAliases: true, previousEntry: null },
|
||||
),
|
||||
{ agentId: "main", env },
|
||||
);
|
||||
|
||||
const result = await autoMigrateLegacyState({ cfg, env, homedir: () => root });
|
||||
|
||||
@@ -985,6 +995,22 @@ describe("state migrations", () => {
|
||||
]);
|
||||
});
|
||||
|
||||
it("starts a new explicit-ownership fleet without a legacy-main owner notice", async () => {
|
||||
const root = await createTempDir();
|
||||
const stateDir = path.join(root, ".openclaw");
|
||||
const env = createEnv(stateDir);
|
||||
const cfg: OpenClawConfig = {
|
||||
agents: { ownership: "explicit", entries: { alpha: {}, beta: {} } },
|
||||
};
|
||||
|
||||
const result = await autoMigrateLegacyState({ cfg, env, homedir: () => root });
|
||||
|
||||
expect(result.warnings).toEqual([]);
|
||||
expect(result.notices ?? []).not.toContainEqual(
|
||||
expect.stringContaining("legacy main rows have no unambiguous configured owner"),
|
||||
);
|
||||
});
|
||||
|
||||
it("ignores a schema-only legacy agent database without selecting an owner", async () => {
|
||||
const root = await createTempDir();
|
||||
const stateDir = path.join(root, ".openclaw");
|
||||
|
||||
Reference in New Issue
Block a user