From a2daa87779ab0573f4f597e78781fcd878fc053a Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Wed, 7 Oct 2026 12:24:03 -0400 Subject: [PATCH] fix: preserve native provider state during legacy sync --- docs/capability-registry.md | 8 + src/manage.test.ts | 316 ++++++++++++++++++++++++++++++++++-- src/manage.ts | 113 +++++++++---- 3 files changed, 385 insertions(+), 52 deletions(-) diff --git a/docs/capability-registry.md b/docs/capability-registry.md index d3f499e4..d773810c 100644 --- a/docs/capability-registry.md +++ b/docs/capability-registry.md @@ -115,3 +115,11 @@ A missing target never implies intentional disable; use `enabled: false` to expr `plan` and `inventory` do not write state. `apply` rebuilds and compares the plan before writing. Stale plans fail. A per-target lock serializes cooperating applies; unexpected locks are not automatically removed. Updates stage and verify a copy in the runtime state directory, retain the previous target under `recovery-*/previous` there, publish, verify, and write the receipt. Runtime state and the target must share a filesystem; otherwise apply refuses before changing the target. Recovery content stays outside provider discovery. The apply result returns `recoveryPath`; save it with deployment evidence. The old content is never automatically deleted. If publication fails after moving the old target and the destination remains absent, the command restores it. A process kill may leave a transaction directory, lock, or missing target: inspect them, preserve recovery content, and plan recovery before clearing the stale lock. These checks protect against ordinary drift and cooperating deployments. Stop other installers while applying; the command does not provide an OS-level transaction against unrelated processes racing filesystem changes. A successful apply proves file content and provenance match. It does not prove provider discovery, plugin installation, MCP authentication, or tool execution. + +## Sharing capabilities without replacing provider settings + +A capability target is a whole file or directory, not a selection of configuration keys. Keep provider-managed configuration, installed plugins, and MCP connections native-owned when sharing only skills or instructions. An explicit fclt-owned configuration artifact is still subject to source pins, target hashes, and a reviewed plan; it does not merge independent provider edits. + +Legacy `sync` and autosync require the existing legacy-mutation opt-in. They skip edited rendered targets and targets without ownership evidence, preserve provider-added agent and rule files, and enroll only absent surfaces while repairing legacy state. Existing native surfaces stay unowned and remain intact after unmanage. A replaced symlink or hardlink is a conflict even when its bytes match the last render. Missing ownership hashes permit only the exact empty generated MCP object, with no additional native keys. Dry-run does not repair state or adopt live content. + +Legacy sync is not a transaction across provider files and does not coordinate with concurrent provider writers. Prefer the narrow capability plan/apply workflow for selected sharing; do not treat broad sync as a background configuration merger. Initial `manage`, explicit adoption, and builtin overwrite remain separate actions with broader effects. diff --git a/src/manage.test.ts b/src/manage.test.ts index 098afaf4..6f6115f7 100644 --- a/src/manage.test.ts +++ b/src/manage.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "bun:test"; import { chmod, + link, lstat, mkdir, mkdtemp, @@ -12,6 +13,7 @@ import { } from "node:fs/promises"; import { tmpdir } from "node:os"; import { dirname, isAbsolute, join } from "node:path"; +import { runAutosyncService } from "./autosync"; import { facultBuiltinPackRoot } from "./builtin"; import { LEGACY_MANAGED_MUTATION_FLAG } from "./legacy-mutation-policy"; import { @@ -763,6 +765,33 @@ describe("managed state", () => { "utf8" ); expect(livePluginManifest).toContain('"name": "autoresearch"'); + const livePath = join( + home, + "plugins", + "autoresearch", + ".codex-plugin", + "plugin.json" + ); + const nativeUpdate = { + name: "autoresearch", + version: "9.0.0", + nativeSetting: true, + }; + await writeJson(livePath, nativeUpdate); + await writeJson( + join( + rootDir, + "tools", + "codex", + "plugins", + "autoresearch", + ".codex-plugin", + "plugin.json" + ), + { name: "autoresearch", version: "2.0.0" } + ); + await syncManagedTools({ homeDir: home, rootDir, tool: "codex" }); + expect(JSON.parse(await readFile(livePath, "utf8"))).toEqual(nativeUpdate); }); it("syncs builtin operating-model skills, agents, and global docs by default", async () => { @@ -2786,7 +2815,7 @@ describe("syncManagedTools", () => { ); }); - it("repairs legacy managed codex state and adopts new global surfaces on sync", async () => { + it("repairs legacy managed codex state without replacing existing global surfaces", async () => { const home = await createTempDir(); const rootDir = join(home, ".ai"); @@ -2896,15 +2925,9 @@ describe("syncManagedTools", () => { await syncManagedTools({ homeDir: home, rootDir, tool: "codex" }); const repairedState = await loadManagedState(home); - expect(repairedState.tools.codex?.globalAgentsPath).toBe( - join(home, ".codex", "AGENTS.md") - ); - expect(repairedState.tools.codex?.toolConfig).toBe( - join(home, ".codex", "config.toml") - ); - expect(repairedState.tools.codex?.rulesDir).toBe( - join(home, ".codex", "rules") - ); + expect(repairedState.tools.codex?.globalAgentsPath).toBeUndefined(); + expect(repairedState.tools.codex?.toolConfig).toBeUndefined(); + expect(repairedState.tools.codex?.rulesDir).toBeUndefined(); expect(repairedState.tools.codex?.pluginsDir).toBe(join(home, "plugins")); expect(repairedState.tools.codex?.pluginMarketplacePath).toBe( join(home, ".agents", "plugins", "marketplace.json") @@ -2917,20 +2940,18 @@ describe("syncManagedTools", () => { join(home, ".codex", "AGENTS.md"), "utf8" ); - expect(globalAgents).toContain(join(rootDir, "instructions", "REVIEW.md")); + expect(globalAgents).toBe("legacy global\n"); const toolConfig = await readFile( join(home, ".codex", "config.toml"), "utf8" ); expect(toolConfig).toContain('approval_policy = "never"'); - expect(toolConfig).toContain("TEAM_GUIDE.md"); + expect(toolConfig).not.toContain("TEAM_GUIDE.md"); - const rulesFile = await readFile( - join(home, ".codex", "rules", "default.rules"), - "utf8" - ); - expect(rulesFile).toContain('pattern = ["gh"]'); + expect( + await Bun.file(join(home, ".codex", "rules", "default.rules")).exists() + ).toBe(false); const pluginMarketplace = await readFile( join(home, ".agents", "plugins", "marketplace.json"), @@ -3619,3 +3640,264 @@ exit 7 ); }); }); + +describe("provider configuration ownership during sync", () => { + it("preserves native config and plugin files while repairing legacy receipts", async () => { + const home = await createTempDir(); + const rootDir = join(home, ".ai"); + const configPath = join(home, ".codex", "config.toml"); + const nativePlugin = join(home, "plugins", "native", "plugin.json"); + await mkdir(join(rootDir, "tools", "codex", "plugins", "shared"), { + recursive: true, + }); + await Bun.write( + join(rootDir, "tools", "codex", "config.toml"), + 'model = "shared-default"\n' + ); + await writeJson( + join( + rootDir, + "tools", + "codex", + "plugins", + "shared", + ".codex-plugin", + "plugin.json" + ), + { name: "shared", version: "1.0.0" } + ); + await mkdir(dirname(configPath), { recursive: true }); + const live = + '# Provider-owned settings\nmodel = "local-choice"\nnew_native_key = true\n\n[plugins.native]\nenabled = false\n'; + await Bun.write(configPath, live); + await writeJson(nativePlugin, { version: "new-native-version" }); + await saveManagedState( + { + version: 1, + tools: { + codex: { tool: "codex", managedAt: "2026-01-01T00:00:00.000Z" }, + }, + }, + home, + rootDir + ); + const before = await snapshotTree(home); + await syncManagedTools({ + homeDir: home, + rootDir, + tool: "codex", + dryRun: true, + }); + expect(await snapshotTree(home)).toEqual(before); + await syncManagedTools({ homeDir: home, rootDir, tool: "codex" }); + expect(await readFile(configPath, "utf8")).toBe(live); + expect(JSON.parse(await readFile(nativePlugin, "utf8"))).toEqual({ + version: "new-native-version", + }); + const state = await loadManagedState(home, rootDir); + expect(state.tools.codex?.toolConfigBackup).toBeUndefined(); + expect(state.tools.codex?.pluginsBackup).toBeUndefined(); + expect(state.tools.codex?.renderedTargets?.[configPath]).toBeUndefined(); + expect(state.tools.codex?.toolConfig).toBeUndefined(); + expect(state.tools.codex?.pluginsDir).toBeUndefined(); + await unmanageTool("codex", { homeDir: home, rootDir }); + expect(await readFile(configPath, "utf8")).toBe(live); + expect(JSON.parse(await readFile(nativePlugin, "utf8"))).toEqual({ + version: "new-native-version", + }); + }); + + it("preserves native keys beside an empty MCP map with no ownership hash", async () => { + const home = await createTempDir(); + const rootDir = join(home, ".ai"); + const targetPath = join(home, ".codex", "mcp.json"); + await writeJson(join(rootDir, "mcp", "servers.json"), { + servers: { shared: { command: "fixture-command" } }, + }); + const live = { mcpServers: {}, nativeSetting: { enabled: false } }; + await writeJson(targetPath, live); + await saveManagedState( + { + version: 1, + tools: { + codex: { + tool: "codex", + managedAt: "2026-01-01T00:00:00.000Z", + mcpConfig: targetPath, + }, + }, + }, + home, + rootDir + ); + await syncManagedTools({ homeDir: home, rootDir, tool: "codex" }); + expect(JSON.parse(await readFile(targetPath, "utf8"))).toEqual(live); + }); + it("preserves edited native config with current, missing, and stale ownership receipts", async () => { + for (const receipt of ["current", "missing", "stale"] as const) { + const home = await createTempDir(); + const rootDir = join(home, ".ai"); + const sourcePath = join(rootDir, "tools", "codex", "config.toml"); + const targetPath = join(home, ".codex", "config.toml"); + await mkdir(dirname(sourcePath), { recursive: true }); + await Bun.write(sourcePath, 'model = "shared-v1"\n'); + await manageTool("codex", { homeDir: home, rootDir }); + const state = await loadManagedState(home, rootDir); + if (receipt === "missing") { + delete state.tools.codex!.renderedTargets![targetPath]; + } else if (receipt === "stale") { + state.tools.codex!.renderedTargets![targetPath]!.hash = "0".repeat(64); + } + await saveManagedState(state, home, rootDir); + const live = + '# Native edit\nmodel = "local-choice"\nnew_key = { enabled = true }\n\n[mcp_servers.native]\ncommand = "native-command"\n\n[plugins.native]\nenabled = false\n'; + await Bun.write(targetPath, live); + await Bun.write(sourcePath, 'model = "shared-v2"\n'); + const before = await snapshotTree(home); + await syncManagedTools({ + homeDir: home, + rootDir, + tool: "codex", + dryRun: true, + }); + expect(await snapshotTree(home)).toEqual(before); + await syncManagedTools({ + homeDir: home, + rootDir, + tool: "codex", + adoptLive: true, + builtinConflictMode: "overwrite", + }); + expect(await readFile(targetPath, "utf8")).toBe(live); + expect(await readFile(sourcePath, "utf8")).toBe('model = "shared-v2"\n'); + } + }); + + it("can populate the exact empty generated MCP shape without an old hash", async () => { + const home = await createTempDir(); + const rootDir = join(home, ".ai"); + const targetPath = join(home, ".codex", "mcp.json"); + const servers = { shared: { command: "fixture-command" } }; + await writeJson(join(rootDir, "mcp", "servers.json"), { servers }); + await writeJson(targetPath, { mcpServers: {} }); + await saveManagedState( + { + version: 1, + tools: { + codex: { + tool: "codex", + managedAt: "2026-01-01T00:00:00.000Z", + mcpConfig: targetPath, + }, + }, + }, + home, + rootDir + ); + await syncManagedTools({ homeDir: home, rootDir, tool: "codex" }); + expect(JSON.parse(await readFile(targetPath, "utf8"))).toEqual({ + mcpServers: servers, + }); + }); + it("does not write through a provider config replaced by a native symlink or hardlink", async () => { + const home = await createTempDir(); + const rootDir = join(home, ".ai"); + const sourcePath = join(rootDir, "tools", "codex", "config.toml"); + const targetPath = join(home, ".codex", "config.toml"); + const nativePath = join(home, "native-config.toml"); + await mkdir(dirname(sourcePath), { recursive: true }); + await Bun.write(sourcePath, 'model = "shared-v1"\n'); + await manageTool("codex", { homeDir: home, rootDir }); + const original = await readFile(targetPath, "utf8"); + await Bun.write(nativePath, original); + await rm(targetPath); + await symlink(nativePath, targetPath); + await Bun.write(sourcePath, 'model = "shared-v2"\n'); + await syncManagedTools({ homeDir: home, rootDir, tool: "codex" }); + expect(await readFile(nativePath, "utf8")).toBe(original); + expect(await readlink(targetPath)).toBe(nativePath); + await rm(targetPath); + await link(nativePath, targetPath); + await syncManagedTools({ homeDir: home, rootDir, tool: "codex" }); + expect(await readFile(nativePath, "utf8")).toBe(original); + expect((await lstat(targetPath)).nlink).toBe(2); + }); + + it("autosync preserves native settings when legacy repair discovers canonical config", async () => { + const home = await createTempDir(); + const rootDir = join(home, ".ai"); + const sourcePath = join(rootDir, "tools", "codex", "config.toml"); + const targetPath = join(home, ".codex", "config.toml"); + await mkdir(dirname(sourcePath), { recursive: true }); + await mkdir(dirname(targetPath), { recursive: true }); + await Bun.write(sourcePath, 'model = "shared-default"\n'); + const live = 'model = "native-choice"\nnew_setting = true\n'; + await Bun.write(targetPath, live); + await saveManagedState( + { + version: 1, + tools: { + codex: { tool: "codex", managedAt: "2026-01-01T00:00:00.000Z" }, + }, + }, + home, + rootDir + ); + await runAutosyncService( + { + version: 1, + name: "fixture", + rootDir, + tool: "codex", + debounceMs: 1, + git: { + enabled: false, + remote: "origin", + branch: "main", + intervalMinutes: 10, + autoCommit: false, + commitPrefix: "fixture", + source: "fixture", + }, + }, + { + homeDir: home, + expectedRootDir: rootDir, + once: true, + allowLegacyManagedMutation: true, + } + ); + expect(await readFile(targetPath, "utf8")).toBe(live); + }); + it("preserves provider-added agent and rule files absent from ownership receipts", async () => { + const home = await createTempDir(); + const rootDir = join(home, ".ai"); + const agentsDir = join(home, ".codex", "agents"); + const rulesDir = join(home, ".codex", "rules"); + await mkdir(rootDir, { recursive: true }); + await mkdir(agentsDir, { recursive: true }); + await mkdir(rulesDir, { recursive: true }); + const agentPath = join(agentsDir, "native.toml"); + const rulePath = join(rulesDir, "native.rules"); + await Bun.write(agentPath, 'name = "native"\n'); + await Bun.write(rulePath, "# Native rule\n"); + await saveManagedState( + { + version: 1, + tools: { + codex: { + tool: "codex", + managedAt: "2026-01-01T00:00:00.000Z", + agentsDir, + rulesDir, + }, + }, + }, + home, + rootDir + ); + await syncManagedTools({ homeDir: home, rootDir, tool: "codex" }); + expect(await readFile(agentPath, "utf8")).toBe('name = "native"\n'); + expect(await readFile(rulePath, "utf8")).toBe("# Native rule\n"); + }); +}); diff --git a/src/manage.ts b/src/manage.ts index 92eb2946..168b823a 100644 --- a/src/manage.ts +++ b/src/manage.ts @@ -3392,8 +3392,12 @@ async function isEmptyGeneratedMcpConfig(pathValue: string): Promise { } try { const parsed = JSON.parse(text) as unknown; - const servers = extractServersObject(parsed); - return servers != null && Object.keys(servers).length === 0; + return ( + isPlainObject(parsed) && + Object.keys(parsed).length === 1 && + isPlainObject(parsed.mcpServers) && + Object.keys(parsed.mcpServers).length === 0 + ); } catch { return false; } @@ -4228,6 +4232,18 @@ async function canonicalAgentsExist(rootDir: string): Promise { } } +async function managedTargetIsAbsent(path: string): Promise { + try { + await lstat(path); + return false; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") { + return true; + } + throw error; + } +} + async function repairManagedToolEntry(args: { homeDir: string; rootDir: string; @@ -4240,6 +4256,8 @@ async function repairManagedToolEntry(args: { return { entry: args.entry, changed: false }; } + // Repair missing surfaces only. Existing provider files must not become owned + // implicitly: later unmanage must not remove a native surface without a backup. const next: ManagedToolState = { ...args.entry }; let changed = false; @@ -4247,11 +4265,9 @@ async function repairManagedToolEntry(args: { tool === "codex" && toolPaths.skillsDir && (await canonicalSkillsExist(rootDir)) && - next.skillsDir !== toolPaths.skillsDir + next.skillsDir !== toolPaths.skillsDir && + (await managedTargetIsAbsent(toolPaths.skillsDir)) ) { - if (!next.skillsBackup) { - next.skillsBackup = await backupPath(toolPaths.skillsDir); - } next.skillsDir = toolPaths.skillsDir; changed = true; } @@ -4259,9 +4275,9 @@ async function repairManagedToolEntry(args: { if ( !next.agentsDir && toolPaths.agentsDir && - (await canonicalAgentsExist(rootDir)) + (await canonicalAgentsExist(rootDir)) && + (await managedTargetIsAbsent(toolPaths.agentsDir)) ) { - next.agentsBackup = await backupPath(toolPaths.agentsDir); next.agentsDir = toolPaths.agentsDir; changed = true; } @@ -4282,15 +4298,17 @@ async function repairManagedToolEntry(args: { toolPaths.pluginMarketplacePath && (await canonicalCodexPluginSourceExists(rootDir)) ) { - if (!next.pluginsDir) { - next.pluginsBackup = await backupPath(toolPaths.pluginsDir); + if ( + !next.pluginsDir && + (await managedTargetIsAbsent(toolPaths.pluginsDir)) + ) { next.pluginsDir = toolPaths.pluginsDir; changed = true; } - if (!next.pluginMarketplacePath) { - next.pluginMarketplaceBackup = await backupPath( - toolPaths.pluginMarketplacePath - ); + if ( + !next.pluginMarketplacePath && + (await managedTargetIsAbsent(toolPaths.pluginMarketplacePath)) + ) { next.pluginMarketplacePath = toolPaths.pluginMarketplacePath; changed = true; } @@ -4327,25 +4345,29 @@ async function repairManagedToolEntry(args: { const overridePath = targets.override; if ( preview.managedTargets.includes(agentsPath) && - !next.globalAgentsPath + !next.globalAgentsPath && + (await managedTargetIsAbsent(agentsPath)) ) { - next.globalAgentsBackup = await backupPath(agentsPath); next.globalAgentsPath = agentsPath; changed = true; } if ( overridePath && preview.managedTargets.includes(overridePath) && - !next.globalAgentsOverridePath + !next.globalAgentsOverridePath && + (await managedTargetIsAbsent(overridePath)) ) { - next.globalAgentsOverrideBackup = await backupPath(overridePath); next.globalAgentsOverridePath = overridePath; changed = true; } } } - if (!next.rulesDir && toolPaths.rulesDir) { + if ( + !next.rulesDir && + toolPaths.rulesDir && + (await managedTargetIsAbsent(toolPaths.rulesDir)) + ) { const preview = await syncToolRules({ homeDir, rootDir, @@ -4354,13 +4376,16 @@ async function repairManagedToolEntry(args: { dryRun: true, }); if (preview.managedRulesDir) { - next.rulesBackup = await backupPath(toolPaths.rulesDir); next.rulesDir = toolPaths.rulesDir; changed = true; } } - if (!next.toolConfig && toolPaths.toolConfig) { + if ( + !next.toolConfig && + toolPaths.toolConfig && + (await managedTargetIsAbsent(toolPaths.toolConfig)) + ) { const preview = await syncToolConfig({ homeDir, rootDir, @@ -4369,7 +4394,6 @@ async function repairManagedToolEntry(args: { dryRun: true, }); if (preview.managedConfig) { - next.toolConfigBackup = await backupPath(toolPaths.toolConfig); next.toolConfig = toolPaths.toolConfig; changed = true; } @@ -4380,9 +4404,9 @@ async function repairManagedToolEntry(args: { interface RenderedConflict { targetPath: string; - sourcePath: string; + sourcePath?: string; sourceKind: ManagedRenderedTargetState["sourceKind"]; - reason: "modified" | "unknown_state" | "missing_source"; + reason: "modified" | "unknown_state" | "missing_source" | "unsafe_target"; } interface RenderedApplyPlan { @@ -4412,14 +4436,31 @@ async function planRenderedTargetConflicts(args: { const sourcePath = args.desiredSources.get(targetPath) ?? previous[targetPath]?.sourcePath; if (!sourcePath) { - if (args.desiredWrites.includes(targetPath)) { - write.push(targetPath); - } else { - remove.push(targetPath); - } + conflicts.push({ + targetPath, + sourceKind: previous[targetPath]?.sourceKind ?? "canonical", + reason: "unknown_state", + }); continue; } const sourceKind = renderedSourceKindForPath(sourcePath); + const targetStat = await lstat(targetPath).catch( + (error: NodeJS.ErrnoException) => { + if (error.code === "ENOENT") { + return null; + } + throw error; + } + ); + if (targetStat && (!targetStat.isFile() || targetStat.nlink !== 1)) { + conflicts.push({ + targetPath, + sourcePath, + sourceKind, + reason: "unsafe_target", + }); + continue; + } if (args.conflictMode === "overwrite" && sourceKind === "builtin") { if (args.desiredWrites.includes(targetPath)) { write.push(targetPath); @@ -4521,17 +4562,19 @@ function logRenderedConflicts( for (const conflict of conflicts) { const verb = dryRun ? "would skip" : "skipped"; const state = - conflict.reason === "missing_source" - ? `canonical source is missing at ${conflict.sourcePath}` - : conflict.reason === "unknown_state" - ? "no prior managed hash is recorded" - : "local edits were detected"; + conflict.reason === "unsafe_target" + ? "the target is not an independent regular file" + : conflict.reason === "missing_source" + ? `canonical source is missing at ${conflict.sourcePath}` + : conflict.reason === "unknown_state" + ? "no prior managed hash is recorded" + : "local edits were detected"; const surface = conflict.sourceKind === "builtin" ? "builtin-backed target" : "managed target"; console.warn( - conflict.sourceKind === "builtin" + conflict.sourceKind === "builtin" && conflict.reason !== "unsafe_target" ? `${tool}: ${verb} ${surface} ${conflict.targetPath} because ${state}. Rerun with "--builtin-conflicts overwrite" to replace it with the latest packaged default.` : `${tool}: ${verb} ${surface} ${conflict.targetPath} because ${state}.` );