diff --git a/src/sync/apply.test.ts b/src/sync/apply.test.ts index 1d881c1..f2bfc4b 100644 --- a/src/sync/apply.test.ts +++ b/src/sync/apply.test.ts @@ -713,6 +713,10 @@ describe('portable extra path manifests', () => { 'linux' ); + expect(machineBPlan.extraConfigs.entries[0]?.repoPath).toBe( + machineAPlan.extraConfigs.entries[0]?.repoPath + ); + await syncLocalToRepo(machineAPlan, null); const manifest = JSON.parse( await fs.readFile(machineAPlan.extraConfigs.manifestPath, 'utf8') @@ -768,6 +772,49 @@ describe('portable extra path manifests', () => { }); }); + it('restores a case-folded manifest entry to the local path spelling on POSIX', async () => { + await withTempDir(async (root) => { + const homeDir = path.join(root, 'home'); + const repoRoot = path.join(root, 'repo'); + const locations = resolveSyncLocations({ HOME: homeDir }, 'linux'); + const localPath = path.join(locations.configRoot, 'MCPNotes.md'); + const repoPath = path.join(repoRoot, 'config', 'extra', 'payload'); + await fs.mkdir(path.dirname(repoPath), { recursive: true }); + await fs.writeFile(repoPath, 'windows-pushed-extra', 'utf8'); + + const plan = buildSyncPlan( + normalizeSyncConfig({ + repo: { owner: 'acme', name: 'config' }, + includeSecrets: false, + extraConfigPaths: [localPath], + }), + locations, + repoRoot, + 'linux' + ); + await fs.mkdir(path.dirname(plan.extraConfigs.manifestPath), { recursive: true }); + await fs.writeFile( + plan.extraConfigs.manifestPath, + JSON.stringify({ + entries: [ + { + // Windows pushes fold the source path to lowercase. + sourcePath: 'mcpnotes.md', + repoPath: 'config/extra/payload', + type: 'file', + }, + ], + }), + 'utf8' + ); + + await syncRepoToLocal(plan, null); + + await expect(fs.readFile(localPath, 'utf8')).resolves.toBe('windows-pushed-extra'); + await expect(fs.access(path.join(locations.configRoot, 'mcpnotes.md'))).rejects.toThrow(); + }); + }); + it('ignores manifest repository paths outside the sync repo', async () => { await withTempDir(async (root) => { const homeDir = path.join(root, 'home'); diff --git a/src/sync/apply.ts b/src/sync/apply.ts index 0e0e9f7..96d58c7 100644 --- a/src/sync/apply.ts +++ b/src/sync/apply.ts @@ -23,7 +23,7 @@ import { stripOverrideKeys, } from './mcp-secrets.js'; import type { ExtraPathPlan, SyncItem, SyncPlan } from './paths.js'; -import { fromPortablePath, normalizePath, toPortablePath } from './paths.js'; +import { foldPortablePath, fromPortablePath, normalizePath, toPortablePath } from './paths.js'; type ExtraPathType = 'file' | 'dir'; @@ -597,8 +597,13 @@ async function applyExtraPaths(plan: SyncPlan, extra: ExtraPathPlan): Promise foldPortablePath(allowed) === foldedSource); + if (!targetPath) continue; const repoPath = resolveManifestRepoPath(plan.repoRoot, entry.repoPath); if (!repoPath) continue; @@ -606,8 +611,8 @@ async function applyExtraPaths(plan: SyncPlan, extra: ExtraPathPlan): Promise { expect(path.relative('/repo/config/extra', entry?.repoPath ?? '').startsWith('..')).toBe(false); }); + it('names extra path repositories independently of the local home directory', () => { + const configFor = (home: string) => { + const locations = resolveSyncLocations({ HOME: home } as NodeJS.ProcessEnv, 'linux'); + return buildSyncPlan( + normalizeSyncConfig({ + repo: { owner: 'acme', name: 'config' }, + includeSecrets: false, + extraConfigPaths: ['custom.json'], + }), + locations, + '/repo', + 'linux' + ); + }; + + const alicePlan = configFor('/home/alice'); + const bobPlan = configFor('/home/bob'); + + expect(alicePlan.extraConfigs.entries[0]?.sourcePath).not.toBe( + bobPlan.extraConfigs.entries[0]?.sourcePath + ); + expect(alicePlan.extraConfigs.entries[0]?.repoPath).toBe( + bobPlan.extraConfigs.entries[0]?.repoPath + ); + }); + + it('case-folds Windows and POSIX names for the same config-relative extra path', () => { + const winLocations = resolveSyncLocations( + { USERPROFILE: 'C:\\Users\\Test' } as NodeJS.ProcessEnv, + 'win32' + ); + const linuxLocations = resolveSyncLocations( + { HOME: '/home/test' } as NodeJS.ProcessEnv, + 'linux' + ); + const config = normalizeSyncConfig({ + repo: { owner: 'acme', name: 'config' }, + includeSecrets: false, + extraConfigPaths: ['~/.config/opencode/Shared.json'], + }); + + const winPlan = buildSyncPlan(config, winLocations, '/repo', 'win32'); + const linuxPlan = buildSyncPlan(config, linuxLocations, '/repo', 'linux'); + + // Win32 lowercases absolute paths; POSIX preserves case. Repo names must + // case-fold so the same portable path yields the same repository entry + // name. Compare entry names so the assertion does not depend on how each + // platform spells the repository root. + expect(path.win32.basename(winPlan.extraConfigs.entries[0]?.repoPath ?? '')).toBe( + path.posix.basename(linuxPlan.extraConfigs.entries[0]?.repoPath ?? '') + ); + }); + + it('collapses duplicate extra path declarations to a single entry', () => { + const env = { HOME: '/home/test' } as NodeJS.ProcessEnv; + const locations = resolveSyncLocations(env, 'linux'); + const config: SyncConfig = { + repo: { owner: 'acme', name: 'config' }, + includeSecrets: false, + extraConfigPaths: ['custom.json', 'custom.json'], + }; + + const plan = buildSyncPlan(normalizeSyncConfig(config), locations, '/repo', 'linux'); + + expect(plan.extraConfigs.allowlist).toEqual(['/home/test/.config/opencode/custom.json']); + expect(plan.extraConfigs.entries).toHaveLength(1); + }); + + it('rejects case-colliding extra paths that resolve to the same repository path', () => { + const env = { HOME: '/home/test' } as NodeJS.ProcessEnv; + const locations = resolveSyncLocations(env, 'linux'); + const config: SyncConfig = { + repo: { owner: 'acme', name: 'config' }, + includeSecrets: false, + extraConfigPaths: ['Foo.md', 'foo.md'], + }; + + expect(() => buildSyncPlan(normalizeSyncConfig(config), locations, '/repo', 'linux')).toThrow( + /Duplicate extra path resolves to the same repository path.*Foo\.md and .*foo\.md/ + ); + }); + + it('collapses case-sibling extra path declarations on windows without failing', () => { + const winLocations = resolveSyncLocations( + { USERPROFILE: 'C:\\Users\\Test' } as NodeJS.ProcessEnv, + 'win32' + ); + const config: SyncConfig = { + repo: { owner: 'acme', name: 'config' }, + includeSecrets: false, + extraConfigPaths: ['Foo.md', 'foo.md'], + }; + + const plan = buildSyncPlan(normalizeSyncConfig(config), winLocations, '/repo', 'win32'); + + expect(plan.extraConfigs.entries).toHaveLength(1); + }); + + it('rejects extra paths that differ only by unicode normalization', () => { + const env = { HOME: '/home/test' } as NodeJS.ProcessEnv; + const locations = resolveSyncLocations(env, 'linux'); + const config: SyncConfig = { + repo: { owner: 'acme', name: 'config' }, + includeSecrets: false, + extraConfigPaths: ['É.md', 'E\u0301.md'], + }; + + expect(() => buildSyncPlan(normalizeSyncConfig(config), locations, '/repo', 'linux')).toThrow( + /Duplicate extra path resolves to the same repository path/ + ); + }); + it('includes skills directory in default sync items', () => { const env = { HOME: '/home/test' } as NodeJS.ProcessEnv; const locations = resolveSyncLocations(env, 'linux'); diff --git a/src/sync/paths.ts b/src/sync/paths.ts index a35f487..55a99af 100644 --- a/src/sync/paths.ts +++ b/src/sync/paths.ts @@ -165,13 +165,19 @@ export function normalizePath( return resolved; } +function resolveExtraRoot( + locations: SyncLocations, + platform: NodeJS.Platform = process.platform +): string { + return pathApiFor(platform).join(locations.xdg.configDir, 'opencode'); +} + export function resolveExtraPath( inputPath: string, locations: SyncLocations, platform: NodeJS.Platform = process.platform ): string { - const pathApi = platform === 'win32' ? path.win32 : path.posix; - const configRoot = pathApi.join(locations.xdg.configDir, 'opencode'); + const configRoot = resolveExtraRoot(locations, platform); return normalizePath(inputPath, locations.xdg.homeDir, platform, configRoot); } @@ -353,11 +359,18 @@ export function buildSyncPlan( ) : extraSecretPaths; + // Extra-path repo names are derived from the XDG opencode dir (ignoring the + // opencode_config_dir override), unlike plan.configRoot used for manifest + // sourcePath. Naming and sourcePath roots are intentionally different. + const extraNamingRoot = resolveExtraRoot(locations, platform); + const extraSecrets = buildExtraPathPlan( filteredExtraSecrets, repoExtraDir, manifestPath, - platform + platform, + extraNamingRoot, + locations.xdg.homeDir ); const extraConfigPaths = (config.extraConfigPaths ?? []) @@ -371,7 +384,9 @@ export function buildSyncPlan( extraConfigPaths, repoConfigExtraDir, configManifestPath, - platform + platform, + extraNamingRoot, + locations.xdg.homeDir ); return { @@ -389,20 +404,53 @@ function buildExtraPathPlan( sourcePaths: string[], repoExtraDir: string, manifestPath: string, - platform: NodeJS.Platform + platform: NodeJS.Platform, + extraRoot: string, + homeDir: string ): ExtraPathPlan { - const entries = sourcePaths.map((sourcePath) => ({ - sourcePath, - repoPath: pathApiFor(platform).join(repoExtraDir, encodeExtraPath(sourcePath)), - })); + const pathApi = pathApiFor(platform); + const seenSourcePaths = new Set(); + const repoPathBySource = new Map(); + const entries: ExtraPathPlan['entries'] = []; + + for (const sourcePath of sourcePaths) { + const normalizedSourcePath = normalizePath(sourcePath, homeDir, platform); + if (seenSourcePaths.has(normalizedSourcePath)) continue; + seenSourcePaths.add(normalizedSourcePath); + + const portable = toPortablePath(normalizedSourcePath, extraRoot, homeDir, pathApi); + const repoPath = pathApi.join(repoExtraDir, encodeExtraPath(foldPortablePath(portable))); + + const existingSource = repoPathBySource.get(repoPath); + if (existingSource !== undefined) { + throw new Error( + `Duplicate extra path resolves to the same repository path: ${repoPath} (${existingSource} and ${normalizedSourcePath}). ` + + 'Names fold case to stay stable across platforms; rename or remove one of these paths.' + ); + } + repoPathBySource.set(repoPath, normalizedSourcePath); + + entries.push({ sourcePath, repoPath }); + } return { - allowlist: sourcePaths, + allowlist: entries.map((entry) => entry.sourcePath), manifestPath, entries, }; } +/** + * Fold a path to the canonical cross-platform form: NFC-normalized (macOS + * stores decomposed names) and lowercased (Windows resolves paths + * case-insensitively). Accepts portable and native paths; folding ignores + * separators. Used for repository names and manifest matching so the same + * logical file maps to the same repository path on every machine. + */ +export function foldPortablePath(inputPath: string): string { + return inputPath.normalize('NFC').toLowerCase(); +} + /** * Convert an absolute sourcePath to a portable form for the manifest. * If the path is configRoot, store it as ".". Paths inside it are relative.