fix(release): keep reviewed migration residue nonfatal (#106101)

Co-authored-by: Eva <eva@100yen.org>
This commit is contained in:
Eva
2026-07-15 07:21:31 +07:00
committed by GitHub
parent 09229c7aaa
commit 311d9bcc5e
5 changed files with 125 additions and 18 deletions
+2 -1
View File
@@ -894,7 +894,8 @@ describe("codex doctor contract", () => {
const result = await fixture.migration.migrateLegacyState(fixture.params);
expect(result.changes).toEqual([]);
expect(result.warnings).toEqual([expect.stringContaining("owned by agent harness pi")]);
expect(result.warnings).toEqual([]);
expect(result.notices).toEqual([expect.stringContaining("owned by agent harness pi")]);
await expect(fs.access(fixture.sidecarPath)).resolves.toBeUndefined();
await expect(openBindingStore(fixture.env).entries()).resolves.toEqual([]);
@@ -70,6 +70,7 @@ type SourceMigrationResult = {
archived: boolean;
importedKeys: number;
warning?: string;
notice?: string;
};
// Keep the doctor contract graph independent from the full Codex runtime.
@@ -415,6 +416,11 @@ async function migrateSource(
importedKeys,
warning: `Left Codex binding sidecar in place because ${reason}: ${source.sidecarPath}`,
});
const retainNotice = (reason: string): SourceMigrationResult => ({
archived: false,
importedKeys,
notice: `Left Codex binding sidecar in place because ${reason}: ${source.sidecarPath}`,
});
const owner = candidates.length === 1 ? candidates[0] : undefined;
try {
return await withFileLock(source.sidecarPath, LEGACY_BINDING_LOCK_OPTIONS, async () => {
@@ -453,7 +459,7 @@ async function migrateSource(
return retain(`${candidates.length} matching session owners make ownership ambiguous`);
}
if (owner?.agentHarnessId && owner.agentHarnessId !== CODEX_AGENT_HARNESS_ID) {
return retain(`its session is owned by agent harness ${owner.agentHarnessId}`);
return retainNotice(`its session is owned by agent harness ${owner.agentHarnessId}`);
}
const sourceSessionFile =
typeof raw.sessionFile === "string" && raw.sessionFile.trim()
@@ -780,6 +786,7 @@ export const stateMigrations: PluginDoctorStateMigration[] = [
async migrateLegacyState(params) {
const changes: string[] = [];
const warnings: string[] = [];
const notices: string[] = [];
const { sources, surfaces } = await collectLegacyBindingSources(params);
if (sources.length === 0) {
return { changes, warnings };
@@ -805,6 +812,9 @@ export const stateMigrations: PluginDoctorStateMigration[] = [
if (result.warning) {
warnings.push(result.warning);
}
if (result.notice) {
notices.push(result.notice);
}
if (result.archived) {
migrated++;
} else {
@@ -821,7 +831,7 @@ export const stateMigrations: PluginDoctorStateMigration[] = [
`Migrated ${partialImports} safe Codex app-server binding row(s) to plugin state; retained legacy sidecars needing review`,
);
}
return { changes, warnings };
return notices.length > 0 ? { changes, warnings, notices } : { changes, warnings };
},
},
];
+4 -2
View File
@@ -2241,7 +2241,8 @@ describe("doctor legacy state migrations", () => {
const result = await runLegacyStateMigrationsForRoot(root);
expect(result.warnings).toStrictEqual([
expect(result.warnings).toStrictEqual([]);
expect(result.notices).toStrictEqual([
"Left plugin install index in place because shared SQLite state has conflicting plugin install metadata for: demo",
]);
expect(fs.existsSync(sourcePath)).toBe(true);
@@ -2358,7 +2359,8 @@ describe("doctor legacy state migrations", () => {
const result = await runLegacyStateMigrationsForRoot(root);
expect(result.warnings).toStrictEqual([
expect(result.warnings).toStrictEqual([]);
expect(result.notices).toStrictEqual([
"Left plugin install index in place because shared SQLite state has conflicting plugin install metadata for: demo",
]);
expect(fs.existsSync(sourcePath)).toBe(true);
+49 -1
View File
@@ -2,7 +2,10 @@
import fs from "node:fs";
import path from "node:path";
import { describe, expect, it } from "vitest";
import { readPersistedInstalledPluginIndex } from "../plugins/installed-plugin-index-store.js";
import {
readPersistedInstalledPluginIndex,
writePersistedInstalledPluginIndex,
} from "../plugins/installed-plugin-index-store.js";
import { withTempDir } from "../test-helpers/temp-dir.js";
import {
autoMigrateLegacyStateDir,
@@ -105,6 +108,51 @@ describe("legacy state dir auto-migration", () => {
});
});
it("reports conflicting plugin install metadata as a notice from the early state-dir pass", async () => {
await withStateDirFixture(async (root) => {
const stateDir = path.join(root, "custom-state");
const sourcePath = path.join(stateDir, "plugins", "installs.json");
await writePersistedInstalledPluginIndex(
{
version: 1,
hostContractVersion: "test",
compatRegistryVersion: "test",
migrationVersion: 1,
policyHash: "test",
generatedAtMs: 1,
installRecords: {
demo: { source: "npm", spec: "demo@latest", version: "1.0.0" },
},
plugins: [],
diagnostics: [],
},
{ stateDir },
);
fs.mkdirSync(path.dirname(sourcePath), { recursive: true });
fs.writeFileSync(
sourcePath,
JSON.stringify({
records: {
demo: { source: "npm", spec: "demo@1.0.0", version: "1.0.0" },
},
}),
"utf8",
);
const result = await autoMigrateLegacyStateDir({
env: { OPENCLAW_STATE_DIR: stateDir } as NodeJS.ProcessEnv,
homedir: () => root,
});
expect(result.warnings).toStrictEqual([]);
expect(result.notices).toStrictEqual([
"Left plugin install index in place because shared SQLite state has conflicting plugin install metadata for: demo",
]);
expect(result.skipped).toBe(false);
expect(fs.existsSync(sourcePath)).toBe(true);
});
});
it("removes legacy plugin install index source when the existing archive has identical bytes", async () => {
await withStateDirFixture(async (root) => {
const stateDir = path.join(root, "custom-state");
+58 -12
View File
@@ -2839,7 +2839,7 @@ async function migrateLegacyPluginStateSidecar(params: {
async function migrateLegacyInstalledPluginIndex(params: {
stateDir: string;
}): Promise<{ changes: string[]; warnings: string[] }> {
}): Promise<MigrationMessages> {
const sourcePath = resolveLegacyInstalledPluginIndexStorePath({ stateDir: params.stateDir });
if (!fileExists(sourcePath)) {
return { changes: [], warnings: [] };
@@ -2875,7 +2875,8 @@ async function migrateLegacyInstalledPluginIndex(params: {
if (merged.conflicts.length > 0) {
return {
changes,
warnings: [
warnings: [],
notices: [
`Left plugin install index in place because shared SQLite state has conflicting plugin install metadata for: ${merged.conflicts.join(", ")}`,
],
};
@@ -3893,20 +3894,23 @@ export async function autoMigrateLegacyStateDir(params: {
const env = params.env ?? process.env;
const warnings: string[] = [];
const changes: string[] = [];
const notices: string[] = [];
const hasCustomStateDir = Boolean(env.OPENCLAW_STATE_DIR?.trim());
const targetDir = hasCustomStateDir ? resolveStateDir(env, homedir) : resolveNewStateDir(homedir);
const migratePluginInstallIndex = async () => {
const result = await migrateLegacyInstalledPluginIndex({ stateDir: targetDir });
changes.push(...result.changes);
warnings.push(...result.warnings);
notices.push(...(result.notices ?? []));
};
if (hasCustomStateDir) {
await migratePluginInstallIndex();
return {
migrated: changes.length > 0,
skipped: changes.length === 0 && warnings.length === 0,
skipped: changes.length === 0 && warnings.length === 0 && notices.length === 0,
changes,
warnings,
...(notices.length > 0 ? { notices } : {}),
};
}
@@ -3927,7 +3931,13 @@ export async function autoMigrateLegacyStateDir(params: {
}
if (!legacyStat) {
await migratePluginInstallIndex();
return { migrated: changes.length > 0, skipped: false, changes, warnings };
return {
migrated: changes.length > 0,
skipped: false,
changes,
warnings,
...(notices.length > 0 ? { notices } : {}),
};
}
if (!legacyStat.isDirectory() && !legacyStat.isSymbolicLink()) {
warnings.push(`Legacy state path is not a directory: ${legacyDir}`);
@@ -3945,7 +3955,13 @@ export async function autoMigrateLegacyStateDir(params: {
}
if (path.resolve(legacyTarget) === path.resolve(targetDir)) {
await migratePluginInstallIndex();
return { migrated: changes.length > 0, skipped: false, changes, warnings };
return {
migrated: changes.length > 0,
skipped: false,
changes,
warnings,
...(notices.length > 0 ? { notices } : {}),
};
}
if (legacyDirs.some((dir) => path.resolve(dir) === path.resolve(legacyTarget))) {
legacyDir = legacyTarget;
@@ -3978,13 +3994,25 @@ export async function autoMigrateLegacyStateDir(params: {
if (isDirPath(targetDir)) {
if (legacyDir && isLegacyDirSymlinkMirror(legacyDir, targetDir)) {
await migratePluginInstallIndex();
return { migrated: changes.length > 0, skipped: false, changes, warnings };
return {
migrated: changes.length > 0,
skipped: false,
changes,
warnings,
...(notices.length > 0 ? { notices } : {}),
};
}
await migratePluginInstallIndex();
warnings.push(
`State dir migration skipped: target already exists (${targetDir}). Remove or merge manually.`,
);
return { migrated: changes.length > 0, skipped: false, changes, warnings };
return {
migrated: changes.length > 0,
skipped: false,
changes,
warnings,
...(notices.length > 0 ? { notices } : {}),
};
}
try {
@@ -4039,7 +4067,13 @@ export async function autoMigrateLegacyStateDir(params: {
}
await migratePluginInstallIndex();
return { migrated: changes.length > 0, skipped: false, changes, warnings };
return {
migrated: changes.length > 0,
skipped: false,
changes,
warnings,
...(notices.length > 0 ? { notices } : {}),
};
}
export async function autoMigrateLegacyTaskStateSidecars(params: {
@@ -5214,8 +5248,8 @@ export async function runLegacyStateMigrations(params: {
...agentDir.warnings,
...channelPlans.warnings,
],
...(pluginPlans.notices && pluginPlans.notices.length > 0
? { notices: [...pluginPlans.notices] }
...((pluginInstallIndex.notices?.length ?? 0) > 0 || (pluginPlans.notices?.length ?? 0) > 0
? { notices: [...(pluginInstallIndex.notices ?? []), ...(pluginPlans.notices ?? [])] }
: {}),
};
}
@@ -6001,7 +6035,13 @@ export async function autoMigrateLegacyState(params: {
...preSessionChannelPlans.warnings,
...pluginPlans.warnings,
];
const notices = [...(stateDirResult.notices ?? []), ...(pluginPlans.notices ?? [])];
const notices = [
...new Set([
...(stateDirResult.notices ?? []),
...(pluginInstallIndex.notices ?? []),
...(pluginPlans.notices ?? []),
]),
];
logMigrationResults(changes, warnings, notices);
return {
migrated:
@@ -6180,7 +6220,13 @@ export async function autoMigrateLegacyState(params: {
...agentDir.warnings,
...channelPlans.warnings,
];
const notices = [...(stateDirResult.notices ?? []), ...(pluginPlans.notices ?? [])];
const notices = [
...new Set([
...(stateDirResult.notices ?? []),
...(pluginInstallIndex.notices ?? []),
...(pluginPlans.notices ?? []),
]),
];
logMigrationResults(changes, warnings, notices);