Skip to content

Commit 09854d9

Browse files
shakkernerdshakkernerd
committed
fix: make ACP metadata key repair idempotent
1 parent d31d26e commit 09854d9

4 files changed

Lines changed: 86 additions & 22 deletions

File tree

‎src/acp/runtime/session-meta.test.ts‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@ import { closeOpenClawStateDatabaseForTest } from "../../state/openclaw-state-db
88
import { withTempDir } from "../../test-helpers/temp-dir.js";
99
import {
1010
listAcpSessionEntries,
11-
moveAcpSessionMetaForMigration,
1211
readAcpSessionEntry,
1312
readAcpSessionMetaForEntry,
13+
repairAcpSessionMetaKeyForMigration,
1414
upsertAcpSessionMeta,
1515
writeAcpSessionMetaForMigration,
1616
} from "./session-meta.js";
@@ -215,7 +215,7 @@ describe("ACP session metadata SQLite store", () => {
215215
});
216216
});
217217

218-
it("moves ACP metadata rows when session-store keys are canonicalized", async () => {
218+
it("repairs ACP metadata rows when session-store keys are canonicalized", async () => {
219219
await withTempDir({ prefix: "openclaw-acp-meta-" }, async (dir) => {
220220
const storePath = path.join(dir, "sessions.json");
221221
const databasePath = path.join(dir, "state", "openclaw.sqlite");
@@ -243,10 +243,9 @@ describe("ACP session metadata SQLite store", () => {
243243
});
244244

245245
expect(
246-
moveAcpSessionMetaForMigration({
246+
repairAcpSessionMetaKeyForMigration({
247247
databasePath,
248-
fromSessionKey: legacyKey,
249-
toSessionKey: canonicalKey,
248+
sessionKey: canonicalKey,
250249
entry: { sessionId: "sess-acp" },
251250
now: () => 200,
252251
}),

‎src/acp/runtime/session-meta.ts‎

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -251,43 +251,59 @@ export function writeAcpSessionMetaForMigration(params: {
251251
);
252252
}
253253

254-
export function moveAcpSessionMetaForMigration(params: {
255-
fromSessionKey: string;
256-
toSessionKey: string;
254+
export function repairAcpSessionMetaKeyForMigration(params: {
255+
sessionKey: string;
257256
entry?: Pick<SessionEntry, "sessionId">;
258257
env?: NodeJS.ProcessEnv;
259258
databasePath?: string;
260259
now?: () => number;
261260
}): boolean {
262-
const fromSessionKey = params.fromSessionKey.trim();
263-
const toSessionKey = params.toSessionKey.trim();
264-
if (!fromSessionKey || !toSessionKey || fromSessionKey === toSessionKey) {
261+
const sessionKey = params.sessionKey.trim();
262+
if (!sessionKey) {
265263
return false;
266264
}
267265

268-
let moved = false;
266+
let repaired = false;
269267
runOpenClawStateWriteTransaction(
270268
(database) => {
271-
const row = selectAcpSessionRow(database.db, fromSessionKey);
272-
if (!row || !acpSessionRowMatchesEntry(row, params.entry)) {
269+
const currentRow = selectAcpSessionRow(database.db, sessionKey);
270+
if (currentRow && acpSessionRowMatchesEntry(currentRow, params.entry)) {
271+
return;
272+
}
273+
274+
const normalizedSessionKey = normalizeLowercaseStringOrEmpty(sessionKey);
275+
const row = executeSqliteQuerySync(
276+
database.db,
277+
getAcpSessionKysely(database.db)
278+
.selectFrom("acp_sessions")
279+
.selectAll()
280+
.orderBy("last_activity_at", "desc")
281+
.orderBy("session_key", "asc"),
282+
).rows.find(
283+
(candidate) =>
284+
candidate.session_key !== sessionKey &&
285+
normalizeLowercaseStringOrEmpty(candidate.session_key) === normalizedSessionKey &&
286+
acpSessionRowMatchesEntry(candidate, params.entry),
287+
);
288+
if (!row) {
273289
return;
274290
}
275291
upsertAcpSessionMetaRow(database.db, {
276292
...row,
277-
session_key: toSessionKey,
293+
session_key: sessionKey,
278294
updated_at: params.now?.() ?? Date.now(),
279295
});
280296
executeSqliteQuerySync(
281297
database.db,
282298
getAcpSessionKysely(database.db)
283299
.deleteFrom("acp_sessions")
284-
.where("session_key", "=", fromSessionKey),
300+
.where("session_key", "=", row.session_key),
285301
);
286-
moved = true;
302+
repaired = true;
287303
},
288304
{ env: params.env, path: params.databasePath },
289305
);
290-
return moved;
306+
return repaired;
291307
}
292308

293309
function upsertAcpSessionMetaRow(db: DatabaseSync, row: Insertable<AcpSessionsTable>): void {

‎src/gateway/sessions-resolve-store.test.ts‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,50 @@ describe("resolveSessionKeyFromResolveParams store canonicalization", () => {
305305
});
306306
});
307307

308+
it("repairs ACP metadata when the session store key was already canonicalized", async () => {
309+
await withStateDirEnv("openclaw-sessions-resolve-acp-harness-partial-", async () => {
310+
const cfg: OpenClawConfig = {
311+
agents: { list: [{ id: "main", default: true }] },
312+
};
313+
const acpKey = "agent:claude:acp:44444444-4444-4444-8444-444444444444";
314+
const legacyAcpKey = "agent:CLAUDE:acp:44444444-4444-4444-8444-444444444444";
315+
const claudeStorePath = resolveStorePath(cfg.session?.store, { agentId: "claude" });
316+
await saveSessionStore(claudeStorePath, {
317+
[acpKey]: {
318+
sessionId: "sess-acp-harness-partial",
319+
label: "claude-delegate-partial",
320+
updatedAt: freshUpdatedAt(),
321+
},
322+
});
323+
writeAcpSessionMetaForMigration({
324+
sessionKey: legacyAcpKey,
325+
sessionId: "sess-acp-harness-partial",
326+
meta: {
327+
backend: "acpx",
328+
agent: "claude",
329+
runtimeSessionName: legacyAcpKey,
330+
mode: "oneshot",
331+
state: "idle",
332+
lastActivityAt: freshUpdatedAt(),
333+
},
334+
});
335+
336+
await expect(
337+
resolveSessionKeyFromResolveParams({
338+
cfg,
339+
p: { key: acpKey },
340+
}),
341+
).resolves.toEqual({ ok: true, key: acpKey });
342+
343+
await expect(
344+
resolveSessionKeyFromResolveParams({
345+
cfg,
346+
p: { key: acpKey },
347+
}),
348+
).resolves.toEqual({ ok: true, key: acpKey });
349+
});
350+
});
351+
308352
it("rejects ACP-shaped bridge sessions without ACP runtime metadata under deleted agents", async () => {
309353
await withStateDirEnv("openclaw-sessions-resolve-acp-bridge-deleted-", async () => {
310354
const cfg: OpenClawConfig = {

‎src/gateway/sessions-resolve.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import {
77
errorShape,
88
type SessionsResolveParams,
99
} from "../../packages/gateway-protocol/src/index.js";
10-
import { moveAcpSessionMetaForMigration } from "../acp/runtime/session-meta.js";
10+
import { repairAcpSessionMetaKeyForMigration } from "../acp/runtime/session-meta.js";
1111
import { loadSessionStore, updateSessionStore, type SessionEntry } from "../config/sessions.js";
1212
import type { OpenClawConfig } from "../config/types.openclaw.js";
1313
import { resolveSessionIdMatchSelection } from "../sessions/session-id-resolution.js";
@@ -130,6 +130,12 @@ export async function resolveSessionKeyFromResolveParams(params: {
130130
const target = resolveGatewaySessionStoreTarget({ cfg, key });
131131
const store = loadSessionStore(target.storePath);
132132
if (store[target.canonicalKey]) {
133+
if (isAcpSessionKey(target.canonicalKey)) {
134+
repairAcpSessionMetaKeyForMigration({
135+
sessionKey: target.canonicalKey,
136+
entry: store[target.canonicalKey],
137+
});
138+
}
133139
if (
134140
!isResolvedSessionKeyVisible({
135141
cfg,
@@ -164,9 +170,8 @@ export async function resolveSessionKeyFromResolveParams(params: {
164170
});
165171
const migratedStore = loadSessionStore(target.storePath);
166172
if (isAcpSessionKey(target.canonicalKey) || isAcpSessionKey(legacyKey)) {
167-
moveAcpSessionMetaForMigration({
168-
fromSessionKey: legacyKey,
169-
toSessionKey: target.canonicalKey,
173+
repairAcpSessionMetaKeyForMigration({
174+
sessionKey: target.canonicalKey,
170175
entry: migratedStore[target.canonicalKey],
171176
});
172177
}

0 commit comments

Comments
 (0)