diff --git a/server/src/__tests__/worktree-config.test.ts b/server/src/__tests__/worktree-config.test.ts index 351122dce5..ea1a4c8764 100644 --- a/server/src/__tests__/worktree-config.test.ts +++ b/server/src/__tests__/worktree-config.test.ts @@ -1,7 +1,7 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; -import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { applyRuntimePortSelectionToConfig, maybePersistWorktreeRuntimePorts, @@ -94,6 +94,21 @@ function buildLegacyConfig(sharedRoot: string, publicBaseUrl = "http://127.0.0.1 }; } +function buildIsolatedConfig(instanceRoot: string, serverPort: number, databasePort: number) { + const config = buildLegacyConfig(instanceRoot, `http://127.0.0.1:${serverPort}`); + return { + ...config, + database: { + ...config.database, + embeddedPostgresPort: databasePort, + }, + server: { + ...config.server, + port: serverPort, + }, + }; +} + describe("worktree config repair", () => { it("repairs legacy repo-local worktree config and env files into an isolated instance", async () => { const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-worktree-repair-")); @@ -361,6 +376,88 @@ describe("worktree config repair", () => { expect(repairedConfig.database.embeddedPostgresPort).toBe(54331); }); + it("serializes and persists cross-repo worktree port reservations", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-worktree-port-registry-")); + const isolatedHome = path.join(tempRoot, ".paperclip-worktrees"); + const firstWorktreeRoot = path.join(tempRoot, "repo-one", "PAP-14013-import-bulk-skills"); + const secondWorktreeRoot = path.join(tempRoot, "repo-two", "PAP-14069-port-conflicts"); + const firstConfigPath = path.join(firstWorktreeRoot, ".paperclip", "config.json"); + const secondConfigPath = path.join(secondWorktreeRoot, ".paperclip", "config.json"); + + const writeWorktree = async (worktreeRoot: string, name: string) => { + const paperclipDir = path.join(worktreeRoot, ".paperclip"); + const instanceRoot = path.join(isolatedHome, "instances", name.toLowerCase()); + await fs.mkdir(paperclipDir, { recursive: true }); + await fs.writeFile( + path.join(paperclipDir, "config.json"), + `${JSON.stringify(buildIsolatedConfig(instanceRoot, 45439, 55439), null, 2)}\n`, + "utf8", + ); + await fs.writeFile( + path.join(paperclipDir, ".env"), + [ + "# Paperclip environment variables", + "PAPERCLIP_IN_WORKTREE=true", + `PAPERCLIP_WORKTREE_NAME=${name}`, + `PAPERCLIP_HOME=${JSON.stringify(isolatedHome)}`, + `PAPERCLIP_INSTANCE_ID=${name.toLowerCase()}`, + `PAPERCLIP_CONFIG=${JSON.stringify(path.join(paperclipDir, "config.json"))}`, + "", + ].join("\n"), + "utf8", + ); + }; + + const activateWorktree = (worktreeRoot: string, name: string) => { + process.chdir(worktreeRoot); + process.env.PAPERCLIP_IN_WORKTREE = "true"; + process.env.PAPERCLIP_WORKTREE_NAME = name; + process.env.PAPERCLIP_WORKTREES_DIR = isolatedHome; + process.env.PAPERCLIP_HOME = isolatedHome; + process.env.PAPERCLIP_INSTANCE_ID = name.toLowerCase(); + process.env.PAPERCLIP_CONFIG = path.join(worktreeRoot, ".paperclip", "config.json"); + delete process.env.PORT; + delete process.env.DATABASE_URL; + }; + + await writeWorktree(firstWorktreeRoot, "PAP-14013-import-bulk-skills"); + await writeWorktree(secondWorktreeRoot, "PAP-14069-port-conflicts"); + const staleLockPath = path.join(isolatedHome, ".worktree-port-reservations.lock"); + await fs.mkdir(staleLockPath, { recursive: true }); + const staleLockTime = new Date(Date.now() - 6_000); + await fs.utimes(staleLockPath, staleLockTime, staleLockTime); + const warning = vi.spyOn(console, "warn").mockImplementation(() => undefined); + + activateWorktree(firstWorktreeRoot, "PAP-14013-import-bulk-skills"); + expect(maybeRepairLegacyWorktreeConfigAndEnvFiles().repairedConfig).toBe(false); + await expect(fs.stat(staleLockPath)).rejects.toMatchObject({ code: "ENOENT" }); + + activateWorktree(secondWorktreeRoot, "PAP-14069-port-conflicts"); + expect(maybeRepairLegacyWorktreeConfigAndEnvFiles().repairedConfig).toBe(true); + + const firstConfig = JSON.parse(await fs.readFile(firstConfigPath, "utf8")); + const secondConfig = JSON.parse(await fs.readFile(secondConfigPath, "utf8")); + const registry = JSON.parse( + await fs.readFile(path.join(isolatedHome, "worktree-port-reservations.json"), "utf8"), + ); + + expect(firstConfig.server.port).toBe(45439); + expect(firstConfig.database.embeddedPostgresPort).toBe(55439); + expect(secondConfig.server.port).toBe(45440); + expect(secondConfig.database.embeddedPostgresPort).toBe(55440); + expect(secondConfig.auth.publicBaseUrl).toBe("http://127.0.0.1:45440/"); + expect(registry.configPaths).toEqual([firstConfigPath, secondConfigPath].sort()); + expect(warning).toHaveBeenCalledWith(expect.stringContaining("Worktree port conflict detected")); + expect(warning).toHaveBeenCalledWith(expect.stringContaining("server: 45439 -> 45440")); + + warning.mockClear(); + expect(maybeRepairLegacyWorktreeConfigAndEnvFiles().repairedConfig).toBe(false); + const persistedConfig = JSON.parse(await fs.readFile(secondConfigPath, "utf8")); + expect(persistedConfig.server.port).toBe(45440); + expect(persistedConfig.database.embeddedPostgresPort).toBe(55440); + expect(warning).not.toHaveBeenCalled(); + }); + it("ignores stale migrated env paths when the dev runner resolved the local config", async () => { const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-worktree-migrated-env-")); const worktreeRoot = path.join(tempRoot, "PAP-9940-what-can-we-learn"); diff --git a/server/src/worktree-config.ts b/server/src/worktree-config.ts index 653b4fa9d3..dc5d620528 100644 --- a/server/src/worktree-config.ts +++ b/server/src/worktree-config.ts @@ -106,6 +106,89 @@ type WorktreeRuntimeContext = { secretsKeyFilePath: string; }; +type WorktreePortRegistry = { + version: 1; + configPaths: string[]; +}; + +const WORKTREE_PORT_REGISTRY_FILE = "worktree-port-reservations.json"; +const WORKTREE_PORT_REGISTRY_LOCK_DIR = ".worktree-port-reservations.lock"; +const WORKTREE_PORT_REGISTRY_LOCK_STALE_MS = 5_000; +const WORKTREE_PORT_REGISTRY_LOCK_TIMEOUT_MS = 10_000; +const sleepSyncBuffer = new Int32Array(new SharedArrayBuffer(4)); + +function sleepSync(durationMs: number): void { + Atomics.wait(sleepSyncBuffer, 0, 0, durationMs); +} + +function withWorktreePortRegistryLock(homeDir: string, run: () => T): T { + fs.mkdirSync(homeDir, { recursive: true }); + const lockPath = path.resolve(homeDir, WORKTREE_PORT_REGISTRY_LOCK_DIR); + const deadline = Date.now() + WORKTREE_PORT_REGISTRY_LOCK_TIMEOUT_MS; + + while (true) { + try { + fs.mkdirSync(lockPath); + break; + } catch (error) { + const code = error instanceof Error && "code" in error ? error.code : null; + if (code !== "EEXIST") throw error; + + try { + const ageMs = Date.now() - fs.statSync(lockPath).mtimeMs; + if (ageMs > WORKTREE_PORT_REGISTRY_LOCK_STALE_MS) { + fs.rmSync(lockPath, { recursive: true, force: true }); + continue; + } + } catch { + continue; + } + + if (Date.now() >= deadline) { + throw new Error(`Timed out waiting for worktree port reservation lock at ${lockPath}`); + } + sleepSync(25); + } + } + + try { + return run(); + } finally { + fs.rmSync(lockPath, { recursive: true, force: true }); + } +} + +function readWorktreePortRegistry(homeDir: string): Set { + const registryPath = path.resolve(homeDir, WORKTREE_PORT_REGISTRY_FILE); + if (!fs.existsSync(registryPath)) return new Set(); + + try { + const parsed = JSON.parse(fs.readFileSync(registryPath, "utf8")) as Partial; + if (parsed.version !== 1 || !Array.isArray(parsed.configPaths)) return new Set(); + return new Set( + parsed.configPaths + .filter((configPath): configPath is string => typeof configPath === "string" && configPath.length > 0) + .map((configPath) => path.resolve(configPath)), + ); + } catch { + return new Set(); + } +} + +function writeWorktreePortRegistry(homeDir: string, configPaths: Iterable): void { + const registryPath = path.resolve(homeDir, WORKTREE_PORT_REGISTRY_FILE); + const persistedPaths = Array.from(new Set(Array.from(configPaths, (configPath) => path.resolve(configPath)))) + .filter((configPath) => fs.existsSync(configPath)) + .sort(); + const registry: WorktreePortRegistry = { + version: 1, + configPaths: persistedPaths, + }; + const temporaryPath = `${registryPath}.${process.pid}.tmp`; + fs.writeFileSync(temporaryPath, `${JSON.stringify(registry, null, 2)}\n`, { mode: 0o600 }); + fs.renameSync(temporaryPath, registryPath); +} + function resolveWorktreeRuntimeContext( env: NodeJS.ProcessEnv, overrideConfigPath?: string, @@ -178,13 +261,23 @@ function resolveRepoManagedWorktreesRoot(worktreeRoot: string): string | null { return path.resolve(repoRoot, ".paperclip", "worktrees"); } -function collectSiblingWorktreePorts(context: WorktreeRuntimeContext): { +function collectSiblingWorktreePorts( + context: WorktreeRuntimeContext, + registeredConfigPaths: Iterable = [], +): { serverPorts: Set; databasePorts: Set; + configPaths: Set; } { const serverPorts = new Set(); const databasePorts = new Set(); const siblingConfigPaths = new Set(); + for (const configPath of registeredConfigPaths) { + const resolvedConfigPath = path.resolve(configPath); + if (resolvedConfigPath !== path.resolve(context.configPath) && fs.existsSync(resolvedConfigPath)) { + siblingConfigPaths.add(resolvedConfigPath); + } + } const instancesDir = path.resolve(context.homeDir, "instances"); if (fs.existsSync(instancesDir)) { for (const entry of fs.readdirSync(instancesDir, { withFileTypes: true })) { @@ -228,7 +321,7 @@ function collectSiblingWorktreePorts(context: WorktreeRuntimeContext): { } } - return { serverPorts, databasePorts }; + return { serverPorts, databasePorts, configPaths: siblingConfigPaths }; } function findNextUnclaimedPort(preferredPort: number, claimedPorts: Set): number { @@ -401,36 +494,60 @@ export function maybeRepairLegacyWorktreeConfigAndEnvFiles(): { let repairedConfig = false; if (fs.existsSync(context.configPath)) { try { - const parsed = JSON.parse(fs.readFileSync(context.configPath, "utf8")) as PaperclipConfig; - let runtimeConfig = parsed; - const siblingPorts = collectSiblingWorktreePorts(context); - const hasSiblingPortCollision = - siblingPorts.serverPorts.has(parsed.server.port) || - (parsed.database.mode === "embedded-postgres" && - siblingPorts.databasePorts.has(parsed.database.embeddedPostgresPort)); + const runtimeConfig = withWorktreePortRegistryLock(context.homeDir, () => { + const parsed = JSON.parse(fs.readFileSync(context.configPath, "utf8")) as PaperclipConfig; + let selectedConfig = parsed; + const registeredConfigPaths = readWorktreePortRegistry(context.homeDir); + const siblingPorts = collectSiblingWorktreePorts(context, registeredConfigPaths); + const serverPortCollision = siblingPorts.serverPorts.has(parsed.server.port); + const databasePortCollision = + parsed.database.mode === "embedded-postgres" && + siblingPorts.databasePorts.has(parsed.database.embeddedPostgresPort); - if (needsWorktreeConfigRepair(parsed, context) || hasSiblingPortCollision) { - const selectedServerPort = findNextUnclaimedPort( - parsed.server.port === 3100 ? 3101 : parsed.server.port, - siblingPorts.serverPorts, - ); - const selectedDatabasePort = - parsed.database.mode === "embedded-postgres" - ? findNextUnclaimedPort( - parsed.database.embeddedPostgresPort === 54329 - ? 54330 - : parsed.database.embeddedPostgresPort, - new Set([...siblingPorts.databasePorts, selectedServerPort]), - ) - : undefined; + if (needsWorktreeConfigRepair(parsed, context) || serverPortCollision || databasePortCollision) { + const selectedServerPort = findNextUnclaimedPort( + parsed.server.port === 3100 ? 3101 : parsed.server.port, + siblingPorts.serverPorts, + ); + const selectedDatabasePort = + parsed.database.mode === "embedded-postgres" + ? findNextUnclaimedPort( + parsed.database.embeddedPostgresPort === 54329 + ? 54330 + : parsed.database.embeddedPostgresPort, + new Set([...siblingPorts.databasePorts, selectedServerPort]), + ) + : undefined; - runtimeConfig = buildIsolatedWorktreeConfig(parsed, context, { - serverPort: selectedServerPort, - databasePort: selectedDatabasePort, - }); - writeConfigFile(context.configPath, runtimeConfig); - repairedConfig = true; - } + selectedConfig = buildIsolatedWorktreeConfig(parsed, context, { + serverPort: selectedServerPort, + databasePort: selectedDatabasePort, + }); + writeConfigFile(context.configPath, selectedConfig); + repairedConfig = true; + + if (serverPortCollision || databasePortCollision) { + console.warn( + [ + `Worktree port conflict detected for ${context.worktreeName}; updated and persisted workspace ports.`, + ...(serverPortCollision + ? [`server: ${parsed.server.port} -> ${selectedServerPort}`] + : []), + ...(databasePortCollision && parsed.database.mode === "embedded-postgres" + ? [`database: ${parsed.database.embeddedPostgresPort} -> ${selectedDatabasePort}`] + : []), + ].join(" "), + ); + } + } + + writeWorktreePortRegistry(context.homeDir, [ + ...registeredConfigPaths, + ...siblingPorts.configPaths, + context.configPath, + ]); + return selectedConfig; + }); if ( !nonEmpty(process.env.PORT)