diff --git a/extensions/codex/doctor-contract-api.test.ts b/extensions/codex/doctor-contract-api.test.ts index 4bfea48cf5a1..92c4314ea1ca 100644 --- a/extensions/codex/doctor-contract-api.test.ts +++ b/extensions/codex/doctor-contract-api.test.ts @@ -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([]); diff --git a/extensions/codex/src/migration/session-binding-sidecars.ts b/extensions/codex/src/migration/session-binding-sidecars.ts index 5b3354527ea6..50af616a0c96 100644 --- a/extensions/codex/src/migration/session-binding-sidecars.ts +++ b/extensions/codex/src/migration/session-binding-sidecars.ts @@ -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 }; }, }, ]; diff --git a/src/commands/doctor-state-migrations.test.ts b/src/commands/doctor-state-migrations.test.ts index 9bc5a9276ef0..36b9d39d0ac8 100644 --- a/src/commands/doctor-state-migrations.test.ts +++ b/src/commands/doctor-state-migrations.test.ts @@ -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); diff --git a/src/infra/state-migrations.state-dir.test.ts b/src/infra/state-migrations.state-dir.test.ts index 71638c0b47b7..4fb1c52f3376 100644 --- a/src/infra/state-migrations.state-dir.test.ts +++ b/src/infra/state-migrations.state-dir.test.ts @@ -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"); diff --git a/src/infra/state-migrations.ts b/src/infra/state-migrations.ts index 4327ccb1bee0..858019f230d7 100644 --- a/src/infra/state-migrations.ts +++ b/src/infra/state-migrations.ts @@ -2839,7 +2839,7 @@ async function migrateLegacyPluginStateSidecar(params: { async function migrateLegacyInstalledPluginIndex(params: { stateDir: string; -}): Promise<{ changes: string[]; warnings: string[] }> { +}): Promise { 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);