Skip to content

Commit 474d660

Browse files
obviyusobviyus
committed
fix(infra): converge legacy state migrations on archive collisions
1 parent 25e18fc commit 474d660

12 files changed

Lines changed: 558 additions & 105 deletions

‎extensions/codex/doctor-contract-api.test.ts‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,68 @@ describe("codex doctor contract", () => {
231231
await fs.rm(stateDir, { recursive: true, force: true });
232232
});
233233

234+
it("reports unresolved-owner binding sidecars as notices after importing conversation binding", async () => {
235+
const stateDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-codex-doctor-"));
236+
const env = { ...process.env, OPENCLAW_STATE_DIR: stateDir };
237+
const sessionsDir = path.join(stateDir, "agents", "main", "sessions");
238+
const transcriptPath = path.join(sessionsDir, "orphan.jsonl");
239+
const sidecarPath = `${transcriptPath}.codex-app-server.json`;
240+
await fs.mkdir(sessionsDir, { recursive: true });
241+
await fs.writeFile(
242+
sidecarPath,
243+
JSON.stringify({
244+
schemaVersion: 2,
245+
threadId: "thread-orphan",
246+
sessionFile: transcriptPath,
247+
updatedAt: "2026-01-01T00:00:00.000Z",
248+
pluginAppPolicyContext: {
249+
fingerprint: "policy-1",
250+
apps: {},
251+
pluginAppIds: {},
252+
},
253+
}),
254+
"utf8",
255+
);
256+
const params = {
257+
config: {},
258+
env,
259+
stateDir,
260+
oauthDir: path.join(stateDir, "oauth"),
261+
context: createDoctorContext(env),
262+
};
263+
const migration = stateMigrations[0];
264+
if (!migration) {
265+
throw new Error("missing Codex binding migration");
266+
}
267+
268+
const result = await migration.migrateLegacyState(params);
269+
const canonicalSidecarPath = await fs.realpath(sidecarPath);
270+
271+
expect(result.warnings).toStrictEqual([]);
272+
expect(result.notices).toStrictEqual([
273+
`Left Codex binding sidecar in place after importing its conversation binding because its session owner could not be resolved: ${canonicalSidecarPath}`,
274+
]);
275+
expect(result.changes).toContain(
276+
"Migrated 1 safe Codex app-server binding row(s) to plugin state; retained legacy sidecars needing review",
277+
);
278+
await expect(fs.access(sidecarPath)).resolves.toBeUndefined();
279+
const store = createDoctorContext(env).openPluginStateKeyedStore<StoredCodexAppServerBinding>({
280+
namespace: CODEX_APP_SERVER_BINDING_NAMESPACE,
281+
maxEntries: CODEX_APP_SERVER_BINDING_MAX_ENTRIES,
282+
overflowPolicy: "reject-new",
283+
});
284+
await expect(
285+
store.lookup(
286+
bindingStoreKey({
287+
kind: "conversation",
288+
bindingId: legacyCodexConversationBindingId(transcriptPath),
289+
}),
290+
),
291+
).resolves.toMatchObject({ state: "active", binding: { threadId: "thread-orphan" } });
292+
293+
await fs.rm(stateDir, { recursive: true, force: true });
294+
});
295+
234296
it("does not scan above stateDir when a session store sits at its parent", async () => {
235297
const outerDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-codex-doctor-outer-"));
236298
const stateDir = path.join(outerDir, "state");

‎extensions/codex/src/migration/session-binding-sidecars.ts‎

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ type SourceMigrationResult = {
5757
archived: boolean;
5858
importedKeys: number;
5959
warning?: string;
60+
notice?: string;
6061
};
6162

6263
// Keep the doctor contract graph independent from the full Codex runtime.
@@ -430,7 +431,7 @@ async function migrateSource(
430431
return {
431432
archived: false,
432433
importedKeys,
433-
warning: `Left Codex binding sidecar in place after importing its conversation binding because its session owner could not be resolved: ${source.sidecarPath}`,
434+
notice: `Left Codex binding sidecar in place after importing its conversation binding because its session owner could not be resolved: ${source.sidecarPath}`,
434435
};
435436
}
436437
const ownershipWarning = await recordSessionOwner(owner);
@@ -442,11 +443,7 @@ async function migrateSource(
442443
return retain(`canonical plugin state changed at ${entry.key}`);
443444
}
444445
}
445-
const archivePath = `${source.sidecarPath}.migrated`;
446-
if (await pathExists(archivePath)) {
447-
return retain(`its archive already exists at ${archivePath}`);
448-
}
449-
await fs.rename(source.sidecarPath, archivePath);
446+
await archiveBindingSidecar(source.sidecarPath);
450447
return { archived: true, importedKeys };
451448
});
452449
} catch (error) {
@@ -527,6 +524,32 @@ async function pathExists(filePath: string): Promise<boolean> {
527524
}
528525
}
529526

527+
async function firstFreeArchivePath(sourcePath: string): Promise<string> {
528+
for (let index = 2; ; index++) {
529+
const candidate = `${sourcePath}.migrated.${index}`;
530+
if (!(await pathExists(candidate))) {
531+
return candidate;
532+
}
533+
}
534+
}
535+
536+
async function archiveBindingSidecar(sourcePath: string): Promise<void> {
537+
const archivePath = `${sourcePath}.migrated`;
538+
if (await pathExists(archivePath)) {
539+
const [sourceBytes, archiveBytes] = await Promise.all([
540+
fs.readFile(sourcePath),
541+
fs.readFile(archivePath),
542+
]);
543+
if (sourceBytes.equals(archiveBytes)) {
544+
await fs.rm(sourcePath, { force: true });
545+
return;
546+
}
547+
await fs.rename(sourcePath, await firstFreeArchivePath(sourcePath));
548+
return;
549+
}
550+
await fs.rename(sourcePath, archivePath);
551+
}
552+
530553
export const stateMigrations: PluginDoctorStateMigration[] = [
531554
{
532555
id: "codex-app-server-sidecars-to-plugin-state",
@@ -544,6 +567,7 @@ export const stateMigrations: PluginDoctorStateMigration[] = [
544567
async migrateLegacyState(params) {
545568
const changes: string[] = [];
546569
const warnings: string[] = [];
570+
const notices: string[] = [];
547571
const { sources, surfaces } = await collectLegacyBindingSources(params);
548572
if (sources.length === 0) {
549573
return { changes, warnings };
@@ -563,6 +587,9 @@ export const stateMigrations: PluginDoctorStateMigration[] = [
563587
if (result.warning) {
564588
warnings.push(result.warning);
565589
}
590+
if (result.notice) {
591+
notices.push(result.notice);
592+
}
566593
if (result.archived) {
567594
migrated++;
568595
} else {
@@ -579,7 +606,7 @@ export const stateMigrations: PluginDoctorStateMigration[] = [
579606
`Migrated ${partialImports} safe Codex app-server binding row(s) to plugin state; retained legacy sidecars needing review`,
580607
);
581608
}
582-
return { changes, warnings };
609+
return notices.length > 0 ? { changes, warnings, notices } : { changes, warnings };
583610
},
584611
},
585612
];

‎src/commands/doctor-config-preflight.state-migration.test.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ type StateMigrationResult = {
66
skipped: boolean;
77
changes: string[];
88
warnings: string[];
9+
notices?: string[];
910
};
1011

1112
type StartupConvergenceWarning = {
@@ -294,6 +295,30 @@ describe("runDoctorConfigPreflight state migration", () => {
294295
expect(startupMigrationLeaseRelease).toHaveBeenCalledOnce();
295296
});
296297

298+
it("records the startup migration checkpoint when state migrations only leave notices", async () => {
299+
needsStartupMigrationCheckpoint.mockReturnValue(true);
300+
autoMigrateLegacyStateDir.mockResolvedValueOnce({
301+
migrated: true,
302+
skipped: false,
303+
changes: [],
304+
warnings: [],
305+
notices: ["Left reviewed residue in place."],
306+
});
307+
308+
await runDoctorConfigPreflight({
309+
migrateLegacyConfig: false,
310+
invalidConfigNote: false,
311+
requireStartupMigrationCheckpoint: true,
312+
});
313+
314+
expect(recordSuccessfulStartupMigrations).toHaveBeenCalledWith({
315+
env: process.env,
316+
lease: startupMigrationLease,
317+
});
318+
expect(note).toHaveBeenCalledWith("- Left reviewed residue in place.", "Doctor notices");
319+
expect(startupMigrationLeaseRelease).toHaveBeenCalledOnce();
320+
});
321+
297322
it("does not acquire the startup migration lease when the checkpoint is current", async () => {
298323
await runDoctorConfigPreflight({
299324
migrateLegacyConfig: false,

‎src/commands/doctor-config-preflight.ts‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,10 +100,18 @@ export function shouldSkipPluginValidationForDoctorConfigPreflight(
100100
return isTruthyEnvValue(env.OPENCLAW_UPDATE_IN_PROGRESS);
101101
}
102102

103-
function noteStateMigrationResult(result: { changes: string[]; warnings: string[] }): void {
103+
function noteStateMigrationResult(result: {
104+
changes: string[];
105+
warnings: string[];
106+
notices?: string[];
107+
}): void {
104108
if (result.changes.length > 0) {
105109
note(result.changes.map((entry) => `- ${entry}`).join("\n"), "Doctor changes");
106110
}
111+
const notices = result.notices ?? [];
112+
if (notices.length > 0) {
113+
note(notices.map((entry) => `- ${entry}`).join("\n"), "Doctor notices");
114+
}
107115
if (result.warnings.length > 0) {
108116
note(result.warnings.map((entry) => `- ${entry}`).join("\n"), "Doctor warnings");
109117
}
@@ -178,7 +186,11 @@ export async function runDoctorConfigPreflight(
178186
let startupMigrationHeartbeat: ReturnType<typeof setInterval> | undefined;
179187
let startupMigrationHeartbeatError: unknown;
180188
const startupMigrationWarnings: string[] = [];
181-
const noteStartupStateMigrationResult = (result: { changes: string[]; warnings: string[] }) => {
189+
const noteStartupStateMigrationResult = (result: {
190+
changes: string[];
191+
warnings: string[];
192+
notices?: string[];
193+
}) => {
182194
startupMigrationWarnings.push(...result.warnings);
183195
noteStateMigrationResult(result);
184196
};

‎src/flows/doctor-health-contributions.test.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1453,6 +1453,31 @@ describe("doctor health contributions", () => {
14531453
});
14541454
});
14551455

1456+
it("prints legacy state migration notices during manual doctor runs", async () => {
1457+
const contribution = requireDoctorContribution("doctor:legacy-state");
1458+
const detected = { preview: ["legacy sessions"], warnings: [] };
1459+
mocks.detectLegacyStateMigrations.mockResolvedValue(detected);
1460+
mocks.runLegacyStateMigrations.mockResolvedValue({
1461+
changes: [],
1462+
warnings: [],
1463+
notices: ["Left reviewed legacy residue in place."],
1464+
});
1465+
const ctx = {
1466+
cfg: {},
1467+
sourceConfigValid: true,
1468+
prompter: buildDoctorPrompter(true),
1469+
runtime: { log: vi.fn(), error: vi.fn(), exit: vi.fn() },
1470+
options: { nonInteractive: true },
1471+
} as unknown as Parameters<(typeof contribution)["run"]>[0];
1472+
1473+
await contribution.run(ctx);
1474+
1475+
expect(mocks.note).toHaveBeenCalledWith(
1476+
"Left reviewed legacy residue in place.",
1477+
"Doctor notices",
1478+
);
1479+
});
1480+
14561481
it("skips Gateway health probes for exec SecretRefs unless allow-exec is set", async () => {
14571482
const contribution = requireDoctorContribution("doctor:gateway-health");
14581483
mocks.gatewaySecretInputPathCanWin.mockImplementation(

‎src/flows/doctor-health-contributions.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -544,6 +544,10 @@ async function runLegacyStateHealth(ctx: DoctorHealthFlowContext): Promise<void>
544544
if (migrated.changes.length > 0) {
545545
note(migrated.changes.join("\n"), "Doctor changes");
546546
}
547+
const notices = migrated.notices ?? [];
548+
if (notices.length > 0) {
549+
note(notices.join("\n"), "Doctor notices");
550+
}
547551
if (migrated.warnings.length > 0) {
548552
note(migrated.warnings.join("\n"), "Doctor warnings");
549553
}

‎src/infra/state-migrations.debug-proxy.ts‎

Lines changed: 47 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -320,28 +320,52 @@ function archiveLegacyDebugProxySqlite(params: {
320320
if (existingSources.length === 0) {
321321
return;
322322
}
323-
const existingArchives = existingSources
324-
.map((sourcePath) => `${sourcePath}.migrated`)
325-
.filter(fileExists);
326-
if (existingArchives.length > 0) {
327-
params.warnings.push(
328-
`Left migrated debug proxy capture sidecar in place because archive already exists: ${existingArchives[0]}`,
329-
);
330-
return;
331-
}
323+
const resolutions: Array<{ sourcePath: string; targetPath: string; removed: boolean }> = [];
332324
for (const sourcePath of existingSources) {
325+
const archivedPath = `${sourcePath}.migrated`;
333326
try {
334-
fs.renameSync(sourcePath, `${sourcePath}.migrated`);
327+
if (fileExists(archivedPath)) {
328+
if (fs.readFileSync(sourcePath).equals(fs.readFileSync(archivedPath))) {
329+
fs.rmSync(sourcePath, { force: true });
330+
resolutions.push({ sourcePath, targetPath: archivedPath, removed: true });
331+
continue;
332+
}
333+
let index = 2;
334+
while (fs.existsSync(`${sourcePath}.migrated.${index}`)) {
335+
index++;
336+
}
337+
const nextArchivePath = `${sourcePath}.migrated.${index}`;
338+
fs.renameSync(sourcePath, nextArchivePath);
339+
resolutions.push({ sourcePath, targetPath: nextArchivePath, removed: false });
340+
continue;
341+
}
342+
fs.renameSync(sourcePath, archivedPath);
343+
resolutions.push({ sourcePath, targetPath: archivedPath, removed: false });
335344
} catch (err) {
336345
params.warnings.push(
337346
`Failed archiving debug proxy capture sidecar ${sourcePath}: ${String(err)}`,
338347
);
339348
return;
340349
}
341350
}
342-
params.changes.push(
343-
`Archived debug proxy capture sidecar legacy source → ${params.sourcePath}.migrated`,
344-
);
351+
if (
352+
resolutions.every(
353+
(resolution) =>
354+
!resolution.removed && resolution.targetPath === `${resolution.sourcePath}.migrated`,
355+
)
356+
) {
357+
params.changes.push(
358+
`Archived debug proxy capture sidecar legacy source → ${params.sourcePath}.migrated`,
359+
);
360+
return;
361+
}
362+
for (const resolution of resolutions) {
363+
params.changes.push(
364+
resolution.removed
365+
? `Removed already-archived debug proxy capture sidecar legacy source ${resolution.sourcePath}`
366+
: `Archived debug proxy capture sidecar legacy source → ${resolution.targetPath}`,
367+
);
368+
}
345369
}
346370

347371
function archiveLegacyDebugProxyBlobs(params: {
@@ -353,15 +377,17 @@ function archiveLegacyDebugProxyBlobs(params: {
353377
return;
354378
}
355379
const archivePath = `${params.blobDir}.migrated`;
356-
if (dirExists(archivePath)) {
357-
params.warnings.push(
358-
`Left migrated debug proxy capture blobs in place because archive already exists: ${archivePath}`,
359-
);
360-
return;
361-
}
362380
try {
363-
fs.renameSync(params.blobDir, archivePath);
364-
params.changes.push(`Archived debug proxy capture blobs → ${archivePath}`);
381+
let targetPath = archivePath;
382+
if (dirExists(archivePath)) {
383+
let index = 2;
384+
while (fs.existsSync(`${params.blobDir}.migrated.${index}`)) {
385+
index++;
386+
}
387+
targetPath = `${params.blobDir}.migrated.${index}`;
388+
}
389+
fs.renameSync(params.blobDir, targetPath);
390+
params.changes.push(`Archived debug proxy capture blobs → ${targetPath}`);
365391
} catch (err) {
366392
params.warnings.push(
367393
`Failed archiving debug proxy capture blobs ${params.blobDir}: ${String(err)}`,

0 commit comments

Comments
 (0)