diff --git a/docs/reference/storage-paths.md b/docs/reference/storage-paths.md index 713231075..323fcacaa 100644 --- a/docs/reference/storage-paths.md +++ b/docs/reference/storage-paths.md @@ -44,7 +44,11 @@ When project-scoped behavior is enabled: - `~/.codex/multi-auth/projects//openai-codex-accounts.json` -`` is derived from normalized project path + short hash. +`` is derived from project identity + short hash. + +- Standard repositories: identity is the project root path. +- Linked Git worktrees: identity is the shared repository root, so all worktrees for the same repo share one account pool. +- Non-Git directories: identity falls back to the detected project path. --- diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 74bf6b633..b8d2ce895 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -27,6 +27,7 @@ codex auth login | Browser opens during login | Expected OAuth behavior | Complete auth and return to terminal | | `codex auth` unrecognized | Wrapper command path conflict | Run `where codex`, then `codex multi auth status` | | Switch says success but wrong account in Codex | Stale Codex auth state sync | Run `codex auth switch `, restart `codex` session | +| Opening a PR worktree asks for login again | Worktree was using a different legacy path key | Run `codex auth list` once in the worktree to trigger migration into repo-shared storage | | `missing field id_token` | Stale auth state payload | Re-login account with `codex auth login` | | `refresh_token_reused` | Token pair already rotated | Re-login that account | | `token_expired` | Token no longer valid | Re-login that account | diff --git a/docs/upgrade.md b/docs/upgrade.md index 2db990283..14627cba3 100644 --- a/docs/upgrade.md +++ b/docs/upgrade.md @@ -56,6 +56,14 @@ They are not canonical and should not be used for new setup. See [reference/storage-paths.md](reference/storage-paths.md). +### Worktree Storage Migration + +If you used `perProjectAccounts=true` before worktree identity sharing was added, older worktree-keyed account files are migrated automatically on first load: + +- Legacy worktree storage is merged into the canonical repo-shared project file. +- Legacy files are removed only after a successful canonical write. +- If canonical persistence fails, legacy files are retained to avoid data loss. + --- ## Common Upgrade Problems diff --git a/lib/storage.ts b/lib/storage.ts index e53034c10..2c990135d 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -5,7 +5,14 @@ import { ACCOUNT_LIMITS } from "./constants.js"; import { createLogger } from "./logger.js"; import { MODEL_FAMILIES, type ModelFamily } from "./prompts/codex.js"; import { AnyAccountStorageSchema, getValidationErrors } from "./schemas.js"; -import { getConfigDir, getProjectConfigDir, getProjectGlobalConfigDir, findProjectRoot, resolvePath } from "./storage/paths.js"; +import { + getConfigDir, + getProjectConfigDir, + getProjectGlobalConfigDir, + findProjectRoot, + resolvePath, + resolveProjectStorageIdentityRoot, +} from "./storage/paths.js"; import { migrateV1ToV3, type CooldownReason, @@ -138,6 +145,7 @@ async function ensureGitignore(storagePath: string): Promise { let currentStoragePath: string | null = null; let currentLegacyProjectStoragePath: string | null = null; +let currentLegacyWorktreeStoragePath: string | null = null; let currentProjectRoot: string | null = null; export function setStorageBackupEnabled(enabled: boolean): void { @@ -172,6 +180,7 @@ export function setStoragePath(projectPath: string | null): void { if (!projectPath) { currentStoragePath = null; currentLegacyProjectStoragePath = null; + currentLegacyWorktreeStoragePath = null; currentProjectRoot = null; return; } @@ -179,11 +188,19 @@ export function setStoragePath(projectPath: string | null): void { const projectRoot = findProjectRoot(projectPath); if (projectRoot) { currentProjectRoot = projectRoot; - currentStoragePath = join(getProjectGlobalConfigDir(projectRoot), ACCOUNTS_FILE_NAME); + const identityRoot = resolveProjectStorageIdentityRoot(projectRoot); + currentStoragePath = join(getProjectGlobalConfigDir(identityRoot), ACCOUNTS_FILE_NAME); currentLegacyProjectStoragePath = join(getProjectConfigDir(projectRoot), ACCOUNTS_FILE_NAME); + const previousWorktreeScopedPath = join( + getProjectGlobalConfigDir(projectRoot), + ACCOUNTS_FILE_NAME, + ); + currentLegacyWorktreeStoragePath = + previousWorktreeScopedPath !== currentStoragePath ? previousWorktreeScopedPath : null; } else { currentStoragePath = null; currentLegacyProjectStoragePath = null; + currentLegacyWorktreeStoragePath = null; currentProjectRoot = null; } } @@ -191,6 +208,7 @@ export function setStoragePath(projectPath: string | null): void { export function setStoragePathDirect(path: string | null): void { currentStoragePath = path; currentLegacyProjectStoragePath = null; + currentLegacyWorktreeStoragePath = null; currentProjectRoot = null; } @@ -216,52 +234,127 @@ function getLegacyFlaggedAccountsPath(): string { async function migrateLegacyProjectStorageIfNeeded( persist: (storage: AccountStorageV3) => Promise = saveAccounts, ): Promise { - if ( - !currentStoragePath || - !currentLegacyProjectStoragePath || - currentLegacyProjectStoragePath === currentStoragePath || - !existsSync(currentLegacyProjectStoragePath) - ) { + if (!currentStoragePath) { return null; } - try { - const legacyContent = await fs.readFile(currentLegacyProjectStoragePath, "utf-8"); - const legacyData = JSON.parse(legacyContent) as unknown; - const normalized = normalizeAccountStorage(legacyData); - if (!normalized) return null; + const candidatePaths = [currentLegacyWorktreeStoragePath, currentLegacyProjectStoragePath] + .filter( + (path): path is string => typeof path === "string" && path.length > 0 && path !== currentStoragePath, + ) + .filter((path, index, all) => all.indexOf(path) === index); + + if (candidatePaths.length === 0) { + return null; + } + + const existingCandidatePaths = candidatePaths.filter((legacyPath) => existsSync(legacyPath)); + if (existingCandidatePaths.length === 0) { + return null; + } + + let targetStorage = await loadNormalizedStorageFromPath(currentStoragePath, "current account storage"); + let migrated = false; + + for (const legacyPath of existingCandidatePaths) { + const legacyStorage = await loadNormalizedStorageFromPath(legacyPath, "legacy account storage"); + if (!legacyStorage) { + continue; + } + + const mergedStorage = mergeStorageForMigration(targetStorage, legacyStorage); + const fallbackStorage = targetStorage ?? legacyStorage; + + try { + await persist(mergedStorage); + targetStorage = mergedStorage; + migrated = true; + } catch (error) { + targetStorage = fallbackStorage; + log.warn("Failed to persist migrated account storage", { + from: legacyPath, + to: currentStoragePath, + error: String(error), + }); + continue; + } - await persist(normalized); try { - await fs.unlink(currentLegacyProjectStoragePath); - log.info("Removed legacy project account storage file after migration", { - path: currentLegacyProjectStoragePath, + await fs.unlink(legacyPath); + log.info("Removed legacy account storage file after migration", { + path: legacyPath, }); } catch (unlinkError) { const code = (unlinkError as NodeJS.ErrnoException).code; if (code !== "ENOENT") { - log.warn("Failed to remove legacy project account storage file after migration", { - path: currentLegacyProjectStoragePath, + log.warn("Failed to remove legacy account storage file after migration", { + path: legacyPath, error: String(unlinkError), }); } } + log.info("Migrated legacy project account storage", { - from: currentLegacyProjectStoragePath, + from: legacyPath, to: currentStoragePath, - accounts: normalized.accounts.length, + accounts: mergedStorage.accounts.length, }); + } + + if (migrated) { + return targetStorage; + } + if (targetStorage && !existsSync(currentStoragePath)) { + return targetStorage; + } + return null; +} + +async function loadNormalizedStorageFromPath( + path: string, + label: string, +): Promise { + try { + const { normalized, schemaErrors } = await loadAccountsFromPath(path); + if (schemaErrors.length > 0) { + log.warn(`${label} schema validation warnings`, { + path, + errors: schemaErrors.slice(0, 5), + }); + } return normalized; } catch (error) { - log.warn("Failed to migrate legacy project account storage", { - from: currentLegacyProjectStoragePath, - to: currentStoragePath, - error: String(error), - }); + const code = (error as NodeJS.ErrnoException).code; + if (code !== "ENOENT") { + log.warn(`Failed to load ${label}`, { + path, + error: String(error), + }); + } return null; } } +function mergeStorageForMigration( + current: AccountStorageV3 | null, + incoming: AccountStorageV3, +): AccountStorageV3 { + if (!current) { + return incoming; + } + + const merged = normalizeAccountStorage({ + version: 3, + activeIndex: current.activeIndex, + activeIndexByFamily: current.activeIndexByFamily, + accounts: [...current.accounts, ...incoming.accounts], + }); + if (!merged) { + return current; + } + return merged; +} + function selectNewestAccount( current: T | undefined, candidate: T, @@ -573,8 +666,12 @@ async function loadAccountsFromJournal(path: string): Promise Promise) | null, ): Promise { + const path = getStoragePath(); + const migratedLegacyStorage = persistMigration + ? await migrateLegacyProjectStorageIfNeeded(persistMigration) + : null; + try { - const path = getStoragePath(); const { normalized, storedVersion, schemaErrors } = await loadAccountsFromPath(path); if (schemaErrors.length > 0) { log.warn("Account storage schema validation warnings", { errors: schemaErrors.slice(0, 5) }); @@ -593,10 +690,8 @@ async function loadAccountsInternal( return normalized; } catch (error) { const code = (error as NodeJS.ErrnoException).code; - const path = getStoragePath(); - if (code === "ENOENT" && persistMigration) { - const migrated = await migrateLegacyProjectStorageIfNeeded(persistMigration); - if (migrated) return migrated; + if (code === "ENOENT" && migratedLegacyStorage) { + return migratedLegacyStorage; } const recoveredFromWal = await loadAccountsFromJournal(path); diff --git a/lib/storage/paths.ts b/lib/storage/paths.ts index 2fab17880..a85b3ce8c 100644 --- a/lib/storage/paths.ts +++ b/lib/storage/paths.ts @@ -3,9 +3,9 @@ * Extracted from storage.ts to reduce module size. */ -import { existsSync } from "node:fs"; +import { existsSync, readFileSync, statSync } from "node:fs"; import { createHash } from "node:crypto"; -import { basename, dirname, isAbsolute, join, relative, resolve } from "node:path"; +import { basename, dirname, isAbsolute, join, relative, resolve, win32 } from "node:path"; import { homedir, tmpdir } from "node:os"; import { getCodexMultiAuthDir } from "../runtime-paths.js"; @@ -13,6 +13,120 @@ const PROJECT_MARKERS = [".git", "package.json", "Cargo.toml", "go.mod", "pyproj const PROJECTS_DIR = "projects"; const PROJECT_KEY_HASH_LENGTH = 12; +function parseGitDirPointer(pointerContent: string): string | null { + const firstLine = pointerContent.split(/\r?\n/, 1)[0]?.trim(); + if (!firstLine) return null; + const match = /^gitdir:\s*(.+)$/i.exec(firstLine); + if (!match?.[1]) return null; + const value = match[1].trim(); + return value.length > 0 ? value : null; +} + +function normalizePathDelimiters(pathValue: string): string { + return pathValue.replace(/\\/g, "/"); +} + +function isWindowsRootedPath(pathValue: string): boolean { + return /^[A-Za-z]:[\\/]/.test(pathValue) || /^\\\\[^\\]/.test(pathValue) || /^\/\/[^/]/.test(pathValue); +} + +function resolveGitPath(basePath: string, pointerValue: string): string { + const trimmedPointer = pointerValue.trim(); + if (!trimmedPointer) { + return basePath; + } + + if (isWindowsRootedPath(basePath) || isWindowsRootedPath(trimmedPointer)) { + const windowsBase = win32.normalize(basePath.replace(/\//g, "\\")); + const windowsPointer = win32.normalize(trimmedPointer.replace(/\//g, "\\")); + const windowsResolved = win32.isAbsolute(windowsPointer) + ? windowsPointer + : win32.resolve(windowsBase, windowsPointer); + return process.platform === "win32" + ? windowsResolved + : normalizePathDelimiters(windowsResolved); + } + + const normalizedPointer = normalizePathDelimiters(trimmedPointer); + return isAbsolute(normalizedPointer) + ? normalizedPointer + : resolve(basePath, normalizedPointer); +} + +function readGitCommonDir(gitDirPath: string): string { + const commonDirFile = join(gitDirPath, "commondir"); + if (!existsSync(commonDirFile)) { + return gitDirPath; + } + + try { + const raw = readFileSync(commonDirFile, "utf-8").trim(); + if (!raw) return gitDirPath; + return resolveGitPath(gitDirPath, raw); + } catch { + return gitDirPath; + } +} + +function isWorktreeGitDirPath(gitDirPath: string): boolean { + const normalized = normalizePathDelimiters(gitDirPath).toLowerCase(); + return normalized.includes("/.git/worktrees/"); +} + +function normalizePathForIdentityCheck(pathValue: string): string { + const normalizedDelimiters = normalizePathDelimiters(pathValue.trim()); + if (!normalizedDelimiters) { + return normalizedDelimiters; + } + + if (isWindowsRootedPath(normalizedDelimiters)) { + return win32.normalize(normalizedDelimiters.replace(/\//g, "\\")).toLowerCase(); + } + + const resolvedPath = resolve(normalizedDelimiters); + const normalizedResolved = normalizePathDelimiters(resolvedPath); + return process.platform === "win32" ? normalizedResolved.toLowerCase() : normalizedResolved; +} + +function worktreeGitDirBelongsToProject(projectRoot: string, gitDirPath: string): boolean { + const gitdirBackRefPath = join(gitDirPath, "gitdir"); + if (!existsSync(gitdirBackRefPath)) { + return false; + } + + try { + const gitdirBackRefRaw = readFileSync(gitdirBackRefPath, "utf-8").trim(); + if (!gitdirBackRefRaw) { + return false; + } + + const resolvedBackRef = resolveGitPath(gitDirPath, gitdirBackRefRaw); + const expectedBackRef = join(projectRoot, ".git"); + return ( + normalizePathForIdentityCheck(resolvedBackRef) === + normalizePathForIdentityCheck(expectedBackRef) + ); + } catch { + return false; + } +} + +function isGitDirUnderCommonWorktrees(gitDirPath: string, commonGitDir: string): boolean { + const normalizedGitDir = normalizePathDelimiters( + normalizePathForIdentityCheck(gitDirPath), + ).replace(/\/+$/, ""); + const normalizedCommonGitDir = normalizePathDelimiters( + normalizePathForIdentityCheck(commonGitDir), + ).replace(/\/+$/, ""); + + if (!normalizedGitDir || !normalizedCommonGitDir) { + return false; + } + + const worktreesRoot = `${normalizedCommonGitDir}/worktrees/`; + return normalizedGitDir.startsWith(worktreesRoot); +} + /** * Gets the path to the global Codex multi-auth configuration directory. * @@ -112,6 +226,60 @@ export function getProjectGlobalConfigDir(projectPath: string): string { return join(getConfigDir(), PROJECTS_DIR, getProjectStorageKey(projectPath)); } +/** + * Resolve a stable project identity root for account storage keying. + * + * For standard repositories, this returns `projectRoot` unchanged. + * For linked Git worktrees, this resolves to the shared repository root so + * multiple worktrees use the same per-project account key. + * + * @param projectRoot - Detected project root path (typically from findProjectRoot) + * @returns Identity root used for per-project storage key generation + */ +export function resolveProjectStorageIdentityRoot(projectRoot: string): string { + const gitEntryPath = join(projectRoot, ".git"); + if (!existsSync(gitEntryPath)) { + return projectRoot; + } + + try { + const gitEntryStat = statSync(gitEntryPath); + if (gitEntryStat.isDirectory()) { + return projectRoot; + } + if (!gitEntryStat.isFile()) { + return projectRoot; + } + + const gitPointer = readFileSync(gitEntryPath, "utf-8"); + const gitDirValue = parseGitDirPointer(gitPointer); + if (!gitDirValue) { + return projectRoot; + } + + const gitDirPath = resolveGitPath(projectRoot, gitDirValue); + if (!isWorktreeGitDirPath(gitDirPath)) { + return projectRoot; + } + if (!worktreeGitDirBelongsToProject(projectRoot, gitDirPath)) { + return projectRoot; + } + + const commonGitDir = readGitCommonDir(gitDirPath); + if (!isGitDirUnderCommonWorktrees(gitDirPath, commonGitDir)) { + return projectRoot; + } + const candidateRepoRoot = dirname(commonGitDir); + if (!existsSync(join(candidateRepoRoot, ".git"))) { + return projectRoot; + } + + return candidateRepoRoot; + } catch { + return projectRoot; + } +} + export function isProjectDirectory(dir: string): boolean { return PROJECT_MARKERS.some((marker) => existsSync(join(dir, marker))); } diff --git a/test/paths.test.ts b/test/paths.test.ts index 67d29ed25..8b2599e6d 100644 --- a/test/paths.test.ts +++ b/test/paths.test.ts @@ -4,20 +4,38 @@ import path from "node:path"; vi.mock("node:fs", () => ({ existsSync: vi.fn(), + readFileSync: vi.fn(), + statSync: vi.fn(), })); -import { existsSync } from "node:fs"; +import { existsSync, readFileSync, statSync } from "node:fs"; import { getConfigDir, getProjectConfigDir, getProjectGlobalConfigDir, getProjectStorageKey, + resolveProjectStorageIdentityRoot, isProjectDirectory, findProjectRoot, resolvePath, } from "../lib/storage/paths.js"; const mockedExistsSync = vi.mocked(existsSync); +const mockedReadFileSync = vi.mocked(readFileSync); +const mockedStatSync = vi.mocked(statSync); + +function buildMockStat({ + isDirectory, + isFile, +}: { + isDirectory: boolean; + isFile: boolean; +}): ReturnType { + return { + isDirectory: () => isDirectory, + isFile: () => isFile, + } as unknown as ReturnType; +} describe("Storage Paths Module", () => { const _origCODEX_HOME = process.env.CODEX_HOME; @@ -146,14 +164,309 @@ describe("Storage Paths Module", () => { }); }); - describe("getProjectGlobalConfigDir", () => { - it("returns ~/.codex/multi-auth/projects/", () => { - const projectPath = "/home/user/myproject"; - const result = getProjectGlobalConfigDir(projectPath); - expect(result).toContain(path.join(homedir(), ".codex", "multi-auth", "projects")); - expect(result).toContain("myproject-"); + describe("getProjectGlobalConfigDir", () => { + it("returns ~/.codex/multi-auth/projects/", () => { + const projectPath = "/home/user/myproject"; + const result = getProjectGlobalConfigDir(projectPath); + expect(result).toContain(path.join(homedir(), ".codex", "multi-auth", "projects")); + expect(result).toContain("myproject-"); + }); + }); + + describe("resolveProjectStorageIdentityRoot", () => { + it("returns project root for standard .git directory repos", () => { + const projectRoot = "/repo/main"; + mockedExistsSync.mockImplementation((candidate) => { + return candidate === path.join(projectRoot, ".git"); + }); + mockedStatSync.mockImplementation((candidate) => { + expect(candidate).toBe(path.join(projectRoot, ".git")); + return buildMockStat({ isDirectory: true, isFile: false }); + }); + + const resolved = resolveProjectStorageIdentityRoot(projectRoot); + + expect(resolved).toBe(projectRoot); + expect(mockedReadFileSync).not.toHaveBeenCalled(); + }); + + it("returns shared repository root for linked git worktrees", () => { + const projectRoot = path.resolve("repo", "worktrees", "pr-8"); + const gitEntry = path.join(projectRoot, ".git"); + const worktreeGitDir = path.resolve("repo", ".git", "worktrees", "pr-8"); + const commondirFile = path.join(worktreeGitDir, "commondir"); + const gitdirBackRefFile = path.join(worktreeGitDir, "gitdir"); + const sharedRepoRoot = path.resolve("repo"); + const sharedGitDir = path.join(sharedRepoRoot, ".git"); + const normalize = (value: string) => + process.platform === "win32" ? value.toLowerCase() : value; + + mockedExistsSync.mockImplementation((candidate) => { + if (typeof candidate !== "string") return false; + const normalizedCandidate = normalize(candidate); + if (normalizedCandidate === normalize(gitEntry)) return true; + if (normalizedCandidate === normalize(commondirFile)) return true; + if (normalizedCandidate === normalize(gitdirBackRefFile)) return true; + if (normalizedCandidate === normalize(sharedGitDir)) return true; + return false; + }); + mockedStatSync.mockImplementation((candidate) => { + expect(normalize(String(candidate))).toBe(normalize(gitEntry)); + return buildMockStat({ isDirectory: false, isFile: true }); + }); + mockedReadFileSync.mockImplementation((candidate) => { + if (typeof candidate !== "string") { + throw new Error(`Unexpected read path: ${String(candidate)}`); + } + const normalizedCandidate = normalize(candidate); + if (normalizedCandidate === normalize(gitEntry)) { + return `gitdir: ${worktreeGitDir}\n`; + } + if (normalizedCandidate === normalize(commondirFile)) { + return "../..\n"; + } + if (normalizedCandidate === normalize(gitdirBackRefFile)) { + return `${path.join(projectRoot, ".git")}\n`; + } + throw new Error(`Unexpected read path: ${String(candidate)}`); + }); + + const resolved = resolveProjectStorageIdentityRoot(projectRoot); + + expect(resolved).toBe(sharedRepoRoot); + }); + + it("resolves relative gitdir pointers to the shared repository root", () => { + const projectRoot = path.resolve("repo", "worktrees", "pr-relative"); + const gitEntry = path.join(projectRoot, ".git"); + const worktreeGitDir = path.resolve("repo", ".git", "worktrees", "pr-relative"); + const commondirFile = path.join(worktreeGitDir, "commondir"); + const gitdirBackRefFile = path.join(worktreeGitDir, "gitdir"); + const sharedRepoRoot = path.resolve("repo"); + const sharedGitDir = path.join(sharedRepoRoot, ".git"); + const normalize = (value: string) => + process.platform === "win32" ? value.toLowerCase() : value; + + mockedExistsSync.mockImplementation((candidate) => { + if (typeof candidate !== "string") return false; + const normalizedCandidate = normalize(candidate); + if (normalizedCandidate === normalize(gitEntry)) return true; + if (normalizedCandidate === normalize(commondirFile)) return true; + if (normalizedCandidate === normalize(gitdirBackRefFile)) return true; + if (normalizedCandidate === normalize(sharedGitDir)) return true; + return false; + }); + mockedStatSync.mockImplementation((candidate) => { + expect(normalize(String(candidate))).toBe(normalize(gitEntry)); + return buildMockStat({ isDirectory: false, isFile: true }); + }); + mockedReadFileSync.mockImplementation((candidate) => { + if (typeof candidate !== "string") { + throw new Error(`Unexpected read path: ${String(candidate)}`); + } + const normalizedCandidate = normalize(candidate); + if (normalizedCandidate === normalize(gitEntry)) { + return "gitdir: ../../.git/worktrees/pr-relative\n"; + } + if (normalizedCandidate === normalize(commondirFile)) { + return "../..\n"; + } + if (normalizedCandidate === normalize(gitdirBackRefFile)) { + return `${path.join(projectRoot, ".git")}\n`; + } + throw new Error(`Unexpected read path: ${String(candidate)}`); + }); + + const resolved = resolveProjectStorageIdentityRoot(projectRoot); + + expect(resolved).toBe(sharedRepoRoot); + }); + + it("supports Windows-style backslash gitdir pointers", () => { + const projectRoot = path.win32.join("C:\\repo", "worktrees", "pr-8"); + const gitEntry = path.win32.join(projectRoot, ".git"); + const worktreeGitDir = path.win32.join("C:\\repo", ".git", "worktrees", "pr-8"); + const commondirFile = path.win32.join(worktreeGitDir, "commondir"); + const gitdirBackRefFile = path.win32.join(worktreeGitDir, "gitdir"); + const sharedRepoRoot = "C:\\repo"; + const sharedGitDir = path.win32.join(sharedRepoRoot, ".git"); + const normalize = (value: string) => path.win32.normalize(value).toLowerCase(); + + mockedExistsSync.mockImplementation((candidate) => { + if (typeof candidate !== "string") return false; + const normalizedCandidate = normalize(candidate); + if (normalizedCandidate === normalize(gitEntry)) return true; + if (normalizedCandidate === normalize(commondirFile)) return true; + if (normalizedCandidate === normalize(gitdirBackRefFile)) return true; + if (normalizedCandidate === normalize(sharedGitDir)) return true; + return false; + }); + mockedStatSync.mockImplementation((candidate) => { + expect(normalize(String(candidate))).toBe(normalize(gitEntry)); + return buildMockStat({ isDirectory: false, isFile: true }); + }); + mockedReadFileSync.mockImplementation((candidate) => { + if (typeof candidate !== "string") { + throw new Error(`Unexpected read path: ${String(candidate)}`); + } + const normalizedCandidate = normalize(candidate); + if (normalizedCandidate === normalize(gitEntry)) { + return `gitdir: ${worktreeGitDir}\n`; + } + if (normalizedCandidate === normalize(commondirFile)) { + return "..\\..\\\n"; + } + if (normalizedCandidate === normalize(gitdirBackRefFile)) { + return `${path.win32.join(projectRoot, ".git")}\n`; + } + throw new Error(`Unexpected read path: ${String(candidate)}`); + }); + + const resolved = resolveProjectStorageIdentityRoot(projectRoot); + + expect(normalize(resolved)).toBe(normalize(sharedRepoRoot)); + }); + + it("supports windows UNC gitdir pointers", () => { + const sharedRepoRoot = "\\\\server\\share\\repo"; + const projectRoot = path.win32.join(sharedRepoRoot, "worktrees", "pr-unc"); + const gitEntry = path.win32.join(projectRoot, ".git"); + const worktreeGitDir = path.win32.join(sharedRepoRoot, ".git", "worktrees", "pr-unc"); + const commondirFile = path.win32.join(worktreeGitDir, "commondir"); + const gitdirBackRefFile = path.win32.join(worktreeGitDir, "gitdir"); + const sharedGitDir = path.win32.join(sharedRepoRoot, ".git"); + const normalize = (value: string) => path.win32.normalize(value).toLowerCase(); + + mockedExistsSync.mockImplementation((candidate) => { + if (typeof candidate !== "string") return false; + const normalizedCandidate = normalize(candidate); + return ( + normalizedCandidate === normalize(gitEntry) || + normalizedCandidate === normalize(commondirFile) || + normalizedCandidate === normalize(gitdirBackRefFile) || + normalizedCandidate === normalize(sharedGitDir) + ); + }); + mockedStatSync.mockImplementation((candidate) => { + expect(normalize(String(candidate))).toBe(normalize(gitEntry)); + return buildMockStat({ isDirectory: false, isFile: true }); + }); + mockedReadFileSync.mockImplementation((candidate) => { + if (typeof candidate !== "string") { + throw new Error(`Unexpected read path: ${String(candidate)}`); + } + const normalizedCandidate = normalize(candidate); + if (normalizedCandidate === normalize(gitEntry)) { + return `gitdir: ${worktreeGitDir}\n`; + } + if (normalizedCandidate === normalize(commondirFile)) { + return "..\\..\\\n"; + } + if (normalizedCandidate === normalize(gitdirBackRefFile)) { + return `${path.win32.join(projectRoot, ".git")}\n`; + } + throw new Error(`Unexpected read path: ${String(candidate)}`); + }); + + const resolved = resolveProjectStorageIdentityRoot(projectRoot); + + expect(normalize(resolved)).toBe(normalize(sharedRepoRoot)); + }); + + it("falls back to project root for forged worktree pointers", () => { + const projectRoot = "/repo/attacker"; + const gitEntry = path.join(projectRoot, ".git"); + const foreignWorktreeGitDir = "/repo/victim/.git/worktrees/pr-8"; + const foreignCommondir = path.join(foreignWorktreeGitDir, "commondir"); + const foreignGitdirBackRef = path.join(foreignWorktreeGitDir, "gitdir"); + const victimGitDir = path.join("/repo/victim", ".git"); + + mockedExistsSync.mockImplementation((candidate) => { + return ( + candidate === gitEntry || + candidate === foreignCommondir || + candidate === foreignGitdirBackRef || + candidate === victimGitDir + ); + }); + mockedStatSync.mockImplementation((candidate) => { + expect(candidate).toBe(gitEntry); + return buildMockStat({ isDirectory: false, isFile: true }); + }); + mockedReadFileSync.mockImplementation((candidate) => { + if (candidate === gitEntry) { + return `gitdir: ${foreignWorktreeGitDir}\n`; + } + if (candidate === foreignCommondir) { + return "../..\n"; + } + if (candidate === foreignGitdirBackRef) { + return "/repo/victim/worktrees/pr-8/.git\n"; + } + throw new Error(`Unexpected read path: ${String(candidate)}`); + }); + + const resolved = resolveProjectStorageIdentityRoot(projectRoot); + + expect(resolved).toBe(projectRoot); + }); + + it("falls back to project root when commondir points to a foreign repository", () => { + const projectRoot = "/repo/attacker-worktree"; + const gitEntry = path.join(projectRoot, ".git"); + const worktreeGitDir = "/repo/attacker/.git/worktrees/pr-hostile"; + const forgedCommondir = path.join(worktreeGitDir, "commondir"); + const gitdirBackRefFile = path.join(worktreeGitDir, "gitdir"); + const foreignGitDir = "/repo/victim/.git"; + + mockedExistsSync.mockImplementation((candidate) => { + return ( + candidate === gitEntry || + candidate === forgedCommondir || + candidate === gitdirBackRefFile || + candidate === foreignGitDir + ); + }); + mockedStatSync.mockImplementation((candidate) => { + expect(candidate).toBe(gitEntry); + return buildMockStat({ isDirectory: false, isFile: true }); + }); + mockedReadFileSync.mockImplementation((candidate) => { + if (candidate === gitEntry) { + return `gitdir: ${worktreeGitDir}\n`; + } + if (candidate === forgedCommondir) { + return `${foreignGitDir}\n`; + } + if (candidate === gitdirBackRefFile) { + return `${path.join(projectRoot, ".git")}\n`; + } + throw new Error(`Unexpected read path: ${String(candidate)}`); + }); + + const resolved = resolveProjectStorageIdentityRoot(projectRoot); + + expect(resolved).toBe(projectRoot); + }); + + it("keeps project root when .git file does not point to worktrees", () => { + const projectRoot = "/repo/submodule"; + const gitEntry = path.join(projectRoot, ".git"); + mockedExistsSync.mockImplementation((candidate) => candidate === gitEntry); + mockedStatSync.mockImplementation((candidate) => { + expect(candidate).toBe(gitEntry); + return buildMockStat({ isDirectory: false, isFile: true }); + }); + mockedReadFileSync.mockImplementation((candidate) => { + expect(candidate).toBe(gitEntry); + return "gitdir: /repo/.git/modules/submodule\n"; + }); + + const resolved = resolveProjectStorageIdentityRoot(projectRoot); + + expect(resolved).toBe(projectRoot); + }); }); - }); describe("isProjectDirectory", () => { const markers = [".git", "package.json", "Cargo.toml", "go.mod", "pyproject.toml", ".codex"]; diff --git a/test/storage.test.ts b/test/storage.test.ts index 6735ab721..6c477fd97 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -1,7 +1,8 @@ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import { promises as fs, existsSync } from "node:fs"; -import { join } from "node:path"; +import { dirname, join } from "node:path"; import { tmpdir } from "node:os"; +import { getConfigDir, getProjectStorageKey } from "../lib/storage/paths.js"; import { deduplicateAccounts, deduplicateAccountsByEmail, @@ -767,6 +768,40 @@ describe("storage", () => { expect(path).toContain(".codex"); expect(path).toContain("projects"); }); + + it("uses the same storage path for main repo and linked worktree", async () => { + const testWorkDir = join(tmpdir(), "codex-worktree-key-" + Math.random().toString(36).slice(2)); + const fakeHome = join(testWorkDir, "home"); + const mainRepo = join(testWorkDir, "repo-main"); + const mainGitDir = join(mainRepo, ".git"); + const worktreeRepo = join(testWorkDir, "repo-pr-8"); + const worktreeGitDir = join(mainGitDir, "worktrees", "repo-pr-8"); + const originalHome = process.env.HOME; + const originalUserProfile = process.env.USERPROFILE; + try { + process.env.HOME = fakeHome; + process.env.USERPROFILE = fakeHome; + await fs.mkdir(mainGitDir, { recursive: true }); + await fs.mkdir(worktreeGitDir, { recursive: true }); + await fs.mkdir(worktreeRepo, { recursive: true }); + await fs.writeFile(join(worktreeRepo, ".git"), `gitdir: ${worktreeGitDir}\n`, "utf-8"); + await fs.writeFile(join(worktreeGitDir, "commondir"), "../..\n", "utf-8"); + await fs.writeFile(join(worktreeGitDir, "gitdir"), `${join(worktreeRepo, ".git")}\n`, "utf-8"); + + setStoragePath(mainRepo); + const mainPath = getStoragePath(); + setStoragePath(worktreeRepo); + const worktreePath = getStoragePath(); + expect(worktreePath).toBe(mainPath); + } finally { + setStoragePathDirect(null); + if (originalHome === undefined) delete process.env.HOME; + else process.env.HOME = originalHome; + if (originalUserProfile === undefined) delete process.env.USERPROFILE; + else process.env.USERPROFILE = originalUserProfile; + await fs.rm(testWorkDir, { recursive: true, force: true }); + } + }); }); describe("getStoragePath", () => { @@ -888,8 +923,10 @@ describe("storage", () => { afterEach(async () => { setStoragePathDirect(null); - process.env.HOME = originalHome; - process.env.USERPROFILE = originalUserProfile; + if (originalHome === undefined) delete process.env.HOME; + else process.env.HOME = originalHome; + if (originalUserProfile === undefined) delete process.env.USERPROFILE; + else process.env.USERPROFILE = originalUserProfile; await fs.rm(testWorkDir, { recursive: true, force: true }); }); @@ -977,8 +1014,10 @@ describe("storage", () => { afterEach(async () => { setStoragePathDirect(null); - process.env.HOME = originalHome; - process.env.USERPROFILE = originalUserProfile; + if (originalHome === undefined) delete process.env.HOME; + else process.env.HOME = originalHome; + if (originalUserProfile === undefined) delete process.env.USERPROFILE; + else process.env.USERPROFILE = originalUserProfile; await fs.rm(testWorkDir, { recursive: true, force: true }); }); @@ -1012,6 +1051,348 @@ describe("storage", () => { }); }); + describe("worktree-scoped storage migration", () => { + const testWorkDir = join(tmpdir(), "codex-worktree-migration-" + Math.random().toString(36).slice(2)); + const originalHome = process.env.HOME; + const originalUserProfile = process.env.USERPROFILE; + const originalMultiAuthDir = process.env.CODEX_MULTI_AUTH_DIR; + + type StoredAccountFixture = { + refreshToken: string; + accountId: string; + addedAt: number; + lastUsed: number; + }; + + const now = Date.now(); + const accountFromLegacy: StoredAccountFixture = { + refreshToken: "legacy-refresh", + accountId: "legacy-account", + addedAt: now, + lastUsed: now, + }; + const accountFromCanonical: StoredAccountFixture = { + refreshToken: "canonical-refresh", + accountId: "canonical-account", + addedAt: now + 1, + lastUsed: now + 1, + }; + + async function prepareWorktreeFixture(options?: { + pointerStyle?: "default" | "windows"; + worktreeName?: string; + }): Promise<{ + fakeHome: string; + mainRepo: string; + worktreeRepo: string; + }> { + const fakeHome = join(testWorkDir, "home"); + const mainRepo = join(testWorkDir, "repo-main"); + const worktreeName = options?.worktreeName ?? "repo-pr-8"; + const worktreeRepo = join(testWorkDir, worktreeName); + const mainGitDir = join(mainRepo, ".git"); + const worktreeGitDir = join(mainGitDir, "worktrees", worktreeName); + + process.env.HOME = fakeHome; + process.env.USERPROFILE = fakeHome; + process.env.CODEX_MULTI_AUTH_DIR = join(fakeHome, ".codex", "multi-auth"); + + await fs.mkdir(mainGitDir, { recursive: true }); + await fs.mkdir(worktreeGitDir, { recursive: true }); + await fs.mkdir(worktreeRepo, { recursive: true }); + if (options?.pointerStyle === "windows") { + const winGitDirPointer = worktreeGitDir.replace(/\//g, "\\"); + await fs.writeFile(join(worktreeRepo, ".git"), `gitdir: ${winGitDirPointer}\n`, "utf-8"); + await fs.writeFile(join(worktreeGitDir, "commondir"), "..\\..\\\n", "utf-8"); + await fs.writeFile( + join(worktreeGitDir, "gitdir"), + `${join(worktreeRepo, ".git").replace(/\//g, "\\")}\n`, + "utf-8", + ); + } else { + await fs.writeFile(join(worktreeRepo, ".git"), `gitdir: ${worktreeGitDir}\n`, "utf-8"); + await fs.writeFile(join(worktreeGitDir, "commondir"), "../..\n", "utf-8"); + await fs.writeFile(join(worktreeGitDir, "gitdir"), `${join(worktreeRepo, ".git")}\n`, "utf-8"); + } + + return { fakeHome, mainRepo, worktreeRepo }; + } + + function buildStorage(accounts: StoredAccountFixture[]) { + return { + version: 3 as const, + activeIndex: 0, + activeIndexByFamily: {}, + accounts, + }; + } + + beforeEach(async () => { + await fs.mkdir(testWorkDir, { recursive: true }); + }); + + afterEach(async () => { + setStoragePathDirect(null); + if (originalHome === undefined) delete process.env.HOME; + else process.env.HOME = originalHome; + if (originalUserProfile === undefined) delete process.env.USERPROFILE; + else process.env.USERPROFILE = originalUserProfile; + if (originalMultiAuthDir === undefined) delete process.env.CODEX_MULTI_AUTH_DIR; + else process.env.CODEX_MULTI_AUTH_DIR = originalMultiAuthDir; + await fs.rm(testWorkDir, { recursive: true, force: true }); + vi.restoreAllMocks(); + }); + + it("migrates worktree-keyed storage to repo-shared canonical path", async () => { + const { worktreeRepo } = await prepareWorktreeFixture(); + + setStoragePath(worktreeRepo); + const canonicalPath = getStoragePath(); + const legacyWorktreePath = join( + getConfigDir(), + "projects", + getProjectStorageKey(worktreeRepo), + "openai-codex-accounts.json", + ); + expect(legacyWorktreePath).not.toBe(canonicalPath); + + await fs.mkdir(join(getConfigDir(), "projects", getProjectStorageKey(worktreeRepo)), { + recursive: true, + }); + await fs.writeFile( + legacyWorktreePath, + JSON.stringify(buildStorage([accountFromLegacy]), null, 2), + "utf-8", + ); + + const loaded = await loadAccounts(); + + expect(loaded).not.toBeNull(); + expect(loaded?.accounts).toHaveLength(1); + expect(loaded?.accounts[0]?.accountId).toBe("legacy-account"); + expect(existsSync(canonicalPath)).toBe(true); + expect(existsSync(legacyWorktreePath)).toBe(false); + }); + + it("merges canonical and legacy worktree storage when both exist", async () => { + const { worktreeRepo } = await prepareWorktreeFixture(); + + setStoragePath(worktreeRepo); + const canonicalPath = getStoragePath(); + const legacyWorktreePath = join( + getConfigDir(), + "projects", + getProjectStorageKey(worktreeRepo), + "openai-codex-accounts.json", + ); + await fs.mkdir(join(getConfigDir(), "projects", getProjectStorageKey(worktreeRepo)), { + recursive: true, + }); + await fs.mkdir(join(getConfigDir(), "projects", getProjectStorageKey(join(testWorkDir, "repo-main"))), { + recursive: true, + }); + + await fs.writeFile( + canonicalPath, + JSON.stringify(buildStorage([accountFromCanonical]), null, 2), + "utf-8", + ); + await fs.writeFile( + legacyWorktreePath, + JSON.stringify(buildStorage([accountFromLegacy]), null, 2), + "utf-8", + ); + + const loaded = await loadAccounts(); + + expect(loaded).not.toBeNull(); + expect(loaded?.accounts).toHaveLength(2); + const accountIds = loaded?.accounts.map((account) => account.accountId) ?? []; + expect(accountIds).toContain("canonical-account"); + expect(accountIds).toContain("legacy-account"); + expect(existsSync(legacyWorktreePath)).toBe(false); + }); + + it("keeps legacy worktree file when migration persist fails", async () => { + const { worktreeRepo } = await prepareWorktreeFixture(); + + setStoragePath(worktreeRepo); + const canonicalPath = getStoragePath(); + const canonicalWalPath = `${canonicalPath}.wal`; + const legacyWorktreePath = join( + getConfigDir(), + "projects", + getProjectStorageKey(worktreeRepo), + "openai-codex-accounts.json", + ); + await fs.mkdir(join(getConfigDir(), "projects", getProjectStorageKey(worktreeRepo)), { + recursive: true, + }); + await fs.writeFile( + legacyWorktreePath, + JSON.stringify(buildStorage([accountFromLegacy]), null, 2), + "utf-8", + ); + + const originalWriteFile = fs.writeFile.bind(fs); + const writeSpy = vi + .spyOn(fs, "writeFile") + .mockImplementation(async (...args: Parameters) => { + const [targetPath] = args; + if (typeof targetPath === "string" && targetPath === canonicalWalPath) { + const error = new Error("forced write failure") as NodeJS.ErrnoException; + error.code = "EACCES"; + throw error; + } + return originalWriteFile(...args); + }); + + const loaded = await loadAccounts(); + + writeSpy.mockRestore(); + expect(loaded).not.toBeNull(); + expect(loaded?.accounts).toHaveLength(1); + expect(loaded?.accounts[0]?.accountId).toBe("legacy-account"); + expect(existsSync(legacyWorktreePath)).toBe(true); + }); + + it("handles concurrent loadAccounts migration without duplicate race artifacts", async () => { + const { worktreeRepo } = await prepareWorktreeFixture({ worktreeName: "repo-pr-race" }); + + setStoragePath(worktreeRepo); + const canonicalPath = getStoragePath(); + const legacyWorktreePath = join( + getConfigDir(), + "projects", + getProjectStorageKey(worktreeRepo), + "openai-codex-accounts.json", + ); + await fs.mkdir(join(getConfigDir(), "projects", getProjectStorageKey(worktreeRepo)), { + recursive: true, + }); + await fs.mkdir(dirname(canonicalPath), { recursive: true }); + await fs.writeFile( + canonicalPath, + JSON.stringify(buildStorage([accountFromCanonical]), null, 2), + "utf-8", + ); + await fs.writeFile( + legacyWorktreePath, + JSON.stringify(buildStorage([accountFromLegacy]), null, 2), + "utf-8", + ); + + const results = await Promise.all([ + loadAccounts(), + loadAccounts(), + loadAccounts(), + loadAccounts(), + ]); + + for (const result of results) { + expect(result).not.toBeNull(); + expect(result?.accounts).toHaveLength(2); + } + + const persistedRaw = await fs.readFile(canonicalPath, "utf-8"); + const persistedNormalized = normalizeAccountStorage(JSON.parse(persistedRaw) as unknown); + expect(persistedNormalized).not.toBeNull(); + expect(persistedNormalized?.accounts).toHaveLength(2); + expect(existsSync(legacyWorktreePath)).toBe(false); + }); + + it("migrates worktree storage with Windows-style gitdir pointer fixtures", async () => { + const { worktreeRepo } = await prepareWorktreeFixture({ + pointerStyle: "windows", + worktreeName: "repo-pr-win-ptr", + }); + + setStoragePath(worktreeRepo); + const canonicalPath = getStoragePath(); + const legacyWorktreePath = join( + getConfigDir(), + "projects", + getProjectStorageKey(worktreeRepo), + "openai-codex-accounts.json", + ); + expect(legacyWorktreePath).not.toBe(canonicalPath); + + await fs.mkdir(join(getConfigDir(), "projects", getProjectStorageKey(worktreeRepo)), { + recursive: true, + }); + await fs.writeFile( + legacyWorktreePath, + JSON.stringify(buildStorage([accountFromLegacy]), null, 2), + "utf-8", + ); + + const loaded = await loadAccounts(); + + expect(loaded).not.toBeNull(); + expect(loaded?.accounts).toHaveLength(1); + expect(loaded?.accounts[0]?.accountId).toBe("legacy-account"); + expect(existsSync(canonicalPath)).toBe(true); + expect(existsSync(legacyWorktreePath)).toBe(false); + }); + + it("rejects forged commondir aliasing and keeps storage scoped to the current worktree", async () => { + const worktreeName = "repo-pr-hostile"; + const { mainRepo, worktreeRepo } = await prepareWorktreeFixture({ worktreeName }); + const worktreeGitDir = join(mainRepo, ".git", "worktrees", worktreeName); + const foreignRepo = join(testWorkDir, "repo-foreign"); + const foreignGitDir = join(foreignRepo, ".git"); + const foreignAccount: StoredAccountFixture = { + refreshToken: "foreign-refresh", + accountId: "foreign-account", + addedAt: now + 2, + lastUsed: now + 2, + }; + + await fs.mkdir(foreignGitDir, { recursive: true }); + await fs.writeFile(join(worktreeGitDir, "commondir"), `${foreignGitDir}\n`, "utf-8"); + + setStoragePath(worktreeRepo); + const canonicalPath = getStoragePath(); + const safeCanonicalPath = join( + getConfigDir(), + "projects", + getProjectStorageKey(worktreeRepo), + "openai-codex-accounts.json", + ); + const foreignCanonicalPath = join( + getConfigDir(), + "projects", + getProjectStorageKey(foreignRepo), + "openai-codex-accounts.json", + ); + await fs.mkdir(dirname(safeCanonicalPath), { recursive: true }); + await fs.mkdir(dirname(foreignCanonicalPath), { recursive: true }); + await fs.writeFile( + safeCanonicalPath, + JSON.stringify(buildStorage([accountFromLegacy]), null, 2), + "utf-8", + ); + await fs.writeFile( + foreignCanonicalPath, + JSON.stringify(buildStorage([foreignAccount]), null, 2), + "utf-8", + ); + + const loaded = await loadAccounts(); + + expect(canonicalPath).toBe(safeCanonicalPath); + expect(canonicalPath).not.toBe(foreignCanonicalPath); + expect(loaded).not.toBeNull(); + expect(loaded?.accounts).toHaveLength(1); + expect(loaded?.accounts[0]?.accountId).toBe("legacy-account"); + expect(existsSync(canonicalPath)).toBe(true); + + const foreignRaw = await fs.readFile(foreignCanonicalPath, "utf-8"); + const foreignStorage = normalizeAccountStorage(JSON.parse(foreignRaw) as unknown); + expect(foreignStorage?.accounts[0]?.accountId).toBe("foreign-account"); + }); + }); + describe("saveAccounts EPERM/EBUSY retry logic", () => { const testWorkDir = join(tmpdir(), "codex-retry-" + Math.random().toString(36).slice(2)); let testStoragePath: string;