Skip to content

Commit 9512294

Browse files
jalehmanjalehman
authored
fix: bridge ACP metadata to session accessors (#96195)
* fix: bridge ACP metadata to session accessors * fix: simplify ACP accessor key ownership * fix: bind ACP metadata after session canonicalization
1 parent 8a7b3c7 commit 9512294

8 files changed

Lines changed: 398 additions & 113 deletions

File tree

‎scripts/check-session-accessor-boundary.mjs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,8 @@ export const allowedSessionStoreRuntimeFileBackedCompatExports = new Set([
7474

7575
export const migratedSessionAccessorFiles = new Set([
7676
"packages/memory-host-sdk/src/host/session-files.ts",
77+
"src/acp/runtime/session-meta.ts",
78+
"src/agents/acp-spawn.ts",
7779
"src/agents/embedded-agent-runner/compaction-successor-transcript.ts",
7880
"src/agents/embedded-agent-runner/run/attempt.ts",
7981
"src/agents/embedded-agent-runner/tool-result-truncation.ts",
@@ -124,6 +126,7 @@ export const migratedBundledPluginSessionAccessorFiles = new Set([
124126
]);
125127

126128
export const migratedSessionAccessorWriteFiles = new Set([
129+
"src/acp/runtime/session-meta.ts",
127130
"src/agents/command/attempt-execution.shared.ts",
128131
"src/agents/command/session-store.ts",
129132
"src/agents/embedded-agent-runner/run.ts",
@@ -535,6 +538,7 @@ export async function main() {
535538
"extensions/discord/src/monitor",
536539
"extensions/memory-core/src",
537540
"extensions/telegram/src",
541+
"src/acp",
538542
"src/agents",
539543
"src/auto-reply",
540544
"src/commands",
@@ -546,6 +550,7 @@ export async function main() {
546550
"src/tui",
547551
]);
548552
const writeSourceRoots = resolveSourceRoots(repoRoot, [
553+
"src/acp",
549554
"src/agents",
550555
"src/auto-reply",
551556
"src/commands",

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

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,24 @@ describe("ACP session metadata SQLite store", () => {
152152
})?.acp?.runtimeSessionName,
153153
).toBe("codex-normalized");
154154
expect(loadSessionStore(storePath)[storeSessionKey]?.acp).toBeUndefined();
155+
const legacyEmbeddedEntry = loadSessionStore(storePath)[storeSessionKey];
156+
expect(legacyEmbeddedEntry).toBeDefined();
157+
if (!legacyEmbeddedEntry) {
158+
throw new Error("expected normalized ACP session entry");
159+
}
160+
await writeSessionStoreForTestAsync(storePath, {
161+
[storeSessionKey]: {
162+
...legacyEmbeddedEntry,
163+
acp: {
164+
backend: "acpx",
165+
agent: "codex",
166+
runtimeSessionName: "legacy-embedded",
167+
mode: "persistent",
168+
state: "idle",
169+
lastActivityAt: 120,
170+
},
171+
},
172+
});
155173

156174
await upsertAcpSessionMeta({
157175
cfg,
@@ -170,6 +188,105 @@ describe("ACP session metadata SQLite store", () => {
170188
sessionKey: storeSessionKey,
171189
})?.acp,
172190
).toBeUndefined();
191+
expect(loadSessionStore(storePath)[storeSessionKey]?.acp).toBeUndefined();
192+
});
193+
});
194+
195+
it("keeps SQLite ACP metadata visible when legacy store keys are canonicalized", async () => {
196+
await withTempDir({ prefix: "openclaw-acp-meta-" }, async (dir) => {
197+
const storePath = path.join(dir, "sessions.json");
198+
const databasePath = path.join(dir, "state", "openclaw.sqlite");
199+
const cfg = { session: { store: storePath } } as OpenClawConfig;
200+
const legacyStoreSessionKey = "agent:CODEX:acp:legacy-runtime";
201+
const canonicalSessionKey = "agent:codex:acp:legacy-runtime";
202+
await fs.writeFile(
203+
storePath,
204+
JSON.stringify({
205+
[legacyStoreSessionKey]: {
206+
sessionId: "sess-acp",
207+
updatedAt: 100,
208+
},
209+
}),
210+
"utf8",
211+
);
212+
213+
await upsertAcpSessionMeta({
214+
cfg,
215+
databasePath,
216+
sessionKey: canonicalSessionKey,
217+
now: () => 200,
218+
mutate: () => ({
219+
backend: "acpx",
220+
agent: "codex",
221+
runtimeSessionName: "codex-canonicalized",
222+
mode: "persistent",
223+
state: "idle",
224+
lastActivityAt: 123,
225+
}),
226+
});
227+
228+
const store = loadSessionStore(storePath);
229+
expect(store[legacyStoreSessionKey]).toBeUndefined();
230+
expect(store[canonicalSessionKey]?.sessionId).toBe("sess-acp");
231+
expect(
232+
readAcpSessionEntry({
233+
cfg,
234+
databasePath,
235+
sessionKey: canonicalSessionKey,
236+
})?.acp?.runtimeSessionName,
237+
).toBe("codex-canonicalized");
238+
expect(await listAcpSessionEntries({ cfg, databasePath })).toHaveLength(1);
239+
});
240+
});
241+
242+
it("binds ACP metadata to the final accessor-selected entry for alias writes", async () => {
243+
await withTempDir({ prefix: "openclaw-acp-meta-" }, async (dir) => {
244+
const storePath = path.join(dir, "sessions.json");
245+
const databasePath = path.join(dir, "state", "openclaw.sqlite");
246+
const cfg = { session: { store: storePath } } as OpenClawConfig;
247+
const canonicalSessionKey = "agent:codex:acp:alias-runtime";
248+
const legacyStoreSessionKey = "agent:CODEX:acp:alias-runtime";
249+
await fs.writeFile(
250+
storePath,
251+
JSON.stringify({
252+
[canonicalSessionKey]: {
253+
sessionId: "sess-canonical",
254+
updatedAt: 100,
255+
},
256+
[legacyStoreSessionKey]: {
257+
sessionId: "sess-legacy",
258+
updatedAt: 150,
259+
},
260+
}),
261+
"utf8",
262+
);
263+
264+
await upsertAcpSessionMeta({
265+
cfg,
266+
databasePath,
267+
sessionKey: legacyStoreSessionKey,
268+
now: () => 200,
269+
mutate: () => ({
270+
backend: "acpx",
271+
agent: "codex",
272+
runtimeSessionName: "codex-alias",
273+
mode: "persistent",
274+
state: "idle",
275+
lastActivityAt: 123,
276+
}),
277+
});
278+
279+
const store = loadSessionStore(storePath);
280+
expect(store[legacyStoreSessionKey]).toBeUndefined();
281+
expect(store[canonicalSessionKey]?.sessionId).toBe("sess-legacy");
282+
expect(
283+
readAcpSessionEntry({
284+
cfg,
285+
databasePath,
286+
sessionKey: canonicalSessionKey,
287+
})?.acp?.runtimeSessionName,
288+
).toBe("codex-alias");
289+
expect(await listAcpSessionEntries({ cfg, databasePath })).toHaveLength(1);
173290
});
174291
});
175292

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

Lines changed: 97 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,11 @@ import { normalizeLowercaseStringOrEmpty } from "@openclaw/normalization-core/st
44
import type { Insertable, Selectable } from "kysely";
55
import { getRuntimeConfig } from "../../config/config.js";
66
import { resolveStorePath } from "../../config/sessions/paths.js";
7-
import { loadSessionStore } from "../../config/sessions/store-load.js";
7+
import {
8+
listSessionEntries,
9+
patchSessionEntryWithKey,
10+
type SessionEntrySummary,
11+
} from "../../config/sessions/session-accessor.js";
812
import {
913
mergeSessionEntry,
1014
type AcpSessionRuntimeOptions,
@@ -43,30 +47,24 @@ type AcpSessionsTable = OpenClawStateKyselyDatabase["acp_sessions"];
4347
type AcpSessionMetaDatabase = Pick<OpenClawStateKyselyDatabase, "acp_sessions">;
4448
type AcpSessionRow = Selectable<AcpSessionsTable>;
4549

46-
let sessionStoreRuntimePromise:
47-
| Promise<typeof import("../../config/sessions/store.runtime.js")>
48-
| undefined;
49-
50-
function loadSessionStoreRuntime() {
51-
sessionStoreRuntimePromise ??= import("../../config/sessions/store.runtime.js");
52-
return sessionStoreRuntimePromise;
53-
}
54-
55-
function resolveStoreSessionKey(store: Record<string, SessionEntry>, sessionKey: string): string {
50+
function resolveStoreSessionKey(
51+
entries: readonly SessionEntrySummary[],
52+
sessionKey: string,
53+
): string {
5654
const normalized = sessionKey.trim();
5755
if (!normalized) {
5856
return "";
5957
}
60-
if (store[normalized]) {
58+
if (entries.some((entry) => entry.sessionKey === normalized)) {
6159
return normalized;
6260
}
6361
const lower = normalizeLowercaseStringOrEmpty(normalized);
64-
if (store[lower]) {
62+
if (entries.some((entry) => entry.sessionKey === lower)) {
6563
return lower;
6664
}
67-
for (const key of Object.keys(store)) {
68-
if (normalizeLowercaseStringOrEmpty(key) === lower) {
69-
return key;
65+
for (const entry of entries) {
66+
if (normalizeLowercaseStringOrEmpty(entry.sessionKey) === lower) {
67+
return entry.sessionKey;
7068
}
7169
}
7270
return lower;
@@ -371,12 +369,13 @@ function readSessionEntryFromStore(params: {
371369
env: params.env,
372370
});
373371
try {
374-
const store = loadSessionStore(
372+
const entries = listSessionEntries({
375373
storePath,
376-
params.clone === false ? { clone: false } : undefined,
377-
);
378-
const storeSessionKey = resolveStoreSessionKey(store, params.sessionKey);
379-
return { cfg, storePath, storeSessionKey, entry: store[storeSessionKey] };
374+
...(params.clone === false ? { clone: false } : {}),
375+
});
376+
const storeSessionKey = resolveStoreSessionKey(entries, params.sessionKey);
377+
const entry = entries.find((candidate) => candidate.sessionKey === storeSessionKey)?.entry;
378+
return { cfg, storePath, storeSessionKey, entry };
380379
} catch {
381380
return {
382381
cfg,
@@ -437,14 +436,19 @@ export async function listAcpSessionEntries(params: {
437436
cfg,
438437
env: params.env,
439438
});
440-
let store: Record<string, SessionEntry>;
439+
let sessionEntries: SessionEntrySummary[];
441440
try {
442-
store = loadSessionStore(storePath, params.clone === false ? { clone: false } : undefined);
441+
sessionEntries = listSessionEntries({
442+
storePath,
443+
...(params.clone === false ? { clone: false } : {}),
444+
});
443445
} catch {
444446
continue;
445447
}
446-
const storeSessionKey = resolveStoreSessionKey(store, sessionKey);
447-
const entry = store[storeSessionKey];
448+
const storeSessionKey = resolveStoreSessionKey(sessionEntries, sessionKey);
449+
const entry = sessionEntries.find(
450+
(candidate) => candidate.sessionKey === storeSessionKey,
451+
)?.entry;
448452
if (!entry || !acpSessionRowMatchesEntry(row, entry)) {
449453
continue;
450454
}
@@ -518,63 +522,86 @@ export async function upsertAcpSessionMeta(params: {
518522
current,
519523
current ? mergeAcpForReturn(preparedEntry, current) : entry,
520524
);
521-
if (nextMeta === null) {
522-
executeSqliteQuerySync(
523-
database.db,
524-
getAcpSessionKysely(database.db)
525-
.deleteFrom("acp_sessions")
526-
.where("session_key", "=", storageSessionKey),
527-
);
528-
return;
529-
}
530-
if (nextMeta !== undefined) {
531-
upsertAcpSessionMetaRow(
532-
database.db,
533-
bindAcpSessionMeta({
534-
sessionKey: storageSessionKey,
535-
sessionId: preparedEntry.sessionId,
536-
meta: nextMeta,
537-
updatedAt,
538-
}),
539-
);
540-
}
541525
},
542526
{ env: params.env, path: params.databasePath },
543527
);
544-
if (nextMeta === undefined) {
528+
const metaToPersist = nextMeta;
529+
if (metaToPersist === undefined) {
545530
return current ? mergeAcpForReturn(entry, current) : (entry ?? null);
546531
}
547-
if (nextMeta === null) {
548-
if (!entry) {
549-
return null;
550-
}
551-
const { updateSessionStore } = await loadSessionStoreRuntime();
552-
return await updateSessionStore(
553-
storeEntry.storePath,
554-
(store) => {
555-
const storeSessionKey = resolveStoreSessionKey(store, storageSessionKey);
556-
const next = { ...(store[storeSessionKey] ?? entry) };
557-
delete next.acp;
558-
store[storeSessionKey] = next;
559-
return next;
532+
if (metaToPersist === null) {
533+
const patched = entry
534+
? await patchSessionEntryWithKey(
535+
{ storePath: storeEntry.storePath, sessionKey: storageSessionKey },
536+
(currentEntry) => {
537+
const next = { ...currentEntry };
538+
delete next.acp;
539+
return next;
540+
},
541+
{
542+
...sessionStoreUpdateOptions({ ...params, sessionKey: storageSessionKey }),
543+
replaceEntry: true,
544+
},
545+
)
546+
: null;
547+
runOpenClawStateWriteTransaction(
548+
(database) => {
549+
const sessionKeysToDelete = new Set([storageSessionKey]);
550+
if (patched?.sessionKey) {
551+
sessionKeysToDelete.add(patched.sessionKey);
552+
}
553+
for (const key of sessionKeysToDelete) {
554+
executeSqliteQuerySync(
555+
database.db,
556+
getAcpSessionKysely(database.db)
557+
.deleteFrom("acp_sessions")
558+
.where("session_key", "=", key),
559+
);
560+
}
560561
},
561-
sessionStoreUpdateOptions({ ...params, sessionKey: storageSessionKey }),
562+
{ env: params.env, path: params.databasePath },
562563
);
564+
return patched?.entry ?? null;
563565
}
564-
const { updateSessionStore } = await loadSessionStoreRuntime();
565-
const persisted = await updateSessionStore(
566-
storeEntry.storePath,
567-
(store) => {
568-
const storeSessionKey = resolveStoreSessionKey(store, storageSessionKey);
569-
const next = mergeSessionEntry(store[storeSessionKey], {
570-
sessionId: preparedEntry?.sessionId,
566+
const persisted = await patchSessionEntryWithKey(
567+
{ storePath: storeEntry.storePath, sessionKey: storageSessionKey },
568+
(currentEntry) => {
569+
const next = mergeSessionEntry(currentEntry, {
571570
updatedAt,
572571
});
573572
delete next.acp;
574-
store[storeSessionKey] = next;
575573
return next;
576574
},
577-
sessionStoreUpdateOptions({ ...params, sessionKey: storageSessionKey }),
575+
{
576+
...sessionStoreUpdateOptions({ ...params, sessionKey: storageSessionKey }),
577+
fallbackEntry: preparedEntry,
578+
replaceEntry: true,
579+
},
580+
);
581+
if (!persisted) {
582+
return null;
583+
}
584+
runOpenClawStateWriteTransaction(
585+
(database) => {
586+
upsertAcpSessionMetaRow(
587+
database.db,
588+
bindAcpSessionMeta({
589+
sessionKey: persisted.sessionKey,
590+
sessionId: persisted.entry.sessionId,
591+
meta: metaToPersist,
592+
updatedAt,
593+
}),
594+
);
595+
if (persisted.sessionKey !== storageSessionKey) {
596+
executeSqliteQuerySync(
597+
database.db,
598+
getAcpSessionKysely(database.db)
599+
.deleteFrom("acp_sessions")
600+
.where("session_key", "=", storageSessionKey),
601+
);
602+
}
603+
},
604+
{ env: params.env, path: params.databasePath },
578605
);
579-
return mergeAcpForReturn(persisted, nextMeta);
606+
return mergeAcpForReturn(persisted.entry, metaToPersist);
580607
}

0 commit comments

Comments
 (0)