From 555f026c247f1101270fe098bbeb96f02b4766b8 Mon Sep 17 00:00:00 2001 From: dotta Date: Thu, 26 Mar 2026 10:56:13 -0500 Subject: [PATCH] Avoid sibling worktree port collisions Co-Authored-By: Paperclip --- cli/src/__tests__/worktree.test.ts | 81 ++++++++ cli/src/commands/worktree.ts | 64 ++++++- server/src/__tests__/worktree-config.test.ts | 189 +++++++++++++++++++ server/src/worktree-config.ts | 116 +++++++++++- 4 files changed, 445 insertions(+), 5 deletions(-) diff --git a/cli/src/__tests__/worktree.test.ts b/cli/src/__tests__/worktree.test.ts index ca48b001..3c2079d2 100644 --- a/cli/src/__tests__/worktree.test.ts +++ b/cli/src/__tests__/worktree.test.ts @@ -344,6 +344,87 @@ describe("worktree helpers", () => { } }); + it("avoids ports already claimed by sibling worktree instance configs", async () => { + const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), "paperclip-worktree-claimed-ports-")); + const repoRoot = path.join(tempRoot, "repo"); + const homeDir = path.join(tempRoot, ".paperclip-worktrees"); + const siblingInstanceRoot = path.join(homeDir, "instances", "existing-worktree"); + const originalCwd = process.cwd(); + + try { + fs.mkdirSync(repoRoot, { recursive: true }); + fs.mkdirSync(siblingInstanceRoot, { recursive: true }); + fs.writeFileSync( + path.join(siblingInstanceRoot, "config.json"), + JSON.stringify( + { + ...buildSourceConfig(), + database: { + mode: "embedded-postgres", + embeddedPostgresDataDir: path.join(siblingInstanceRoot, "db"), + embeddedPostgresPort: 54330, + backup: { + enabled: true, + intervalMinutes: 60, + retentionDays: 30, + dir: path.join(siblingInstanceRoot, "backups"), + }, + }, + logging: { + mode: "file", + logDir: path.join(siblingInstanceRoot, "logs"), + }, + server: { + deploymentMode: "authenticated", + exposure: "private", + host: "127.0.0.1", + port: 3101, + allowedHostnames: ["localhost"], + serveUi: true, + }, + storage: { + provider: "local_disk", + localDisk: { + baseDir: path.join(siblingInstanceRoot, "storage"), + }, + s3: { + bucket: "paperclip", + region: "us-east-1", + prefix: "", + forcePathStyle: false, + }, + }, + secrets: { + provider: "local_encrypted", + strictMode: false, + localEncrypted: { + keyFilePath: path.join(siblingInstanceRoot, "secrets", "master.key"), + }, + }, + }, + null, + 2, + ) + "\n", + ); + + process.chdir(repoRoot); + await worktreeInitCommand({ + seed: false, + fromConfig: path.join(tempRoot, "missing", "config.json"), + home: homeDir, + }); + + const config = JSON.parse(fs.readFileSync(path.join(repoRoot, ".paperclip", "config.json"), "utf8")); + expect(config.server.port).toBe(3102); + expect(config.database.embeddedPostgresPort).not.toBe(54330); + expect(config.database.embeddedPostgresPort).not.toBe(config.server.port); + expect(config.database.embeddedPostgresPort).toBeGreaterThan(54330); + } finally { + process.chdir(originalCwd); + fs.rmSync(tempRoot, { recursive: true, force: true }); + } + }); + it("defaults the seed source config to the current repo-local Paperclip config", () => { const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), "paperclip-worktree-source-config-")); const repoRoot = path.join(tempRoot, "repo"); diff --git a/cli/src/commands/worktree.ts b/cli/src/commands/worktree.ts index 7a2bd127..a528bf5b 100644 --- a/cli/src/commands/worktree.ts +++ b/cli/src/commands/worktree.ts @@ -465,6 +465,62 @@ async function findAvailablePort(preferredPort: number, reserved = new Set; + databasePorts: Set; +} { + const serverPorts = new Set(); + const databasePorts = new Set(); + const configPaths = new Set(); + const instancesDir = path.resolve(homeDir, "instances"); + if (existsSync(instancesDir)) { + for (const entry of readdirSync(instancesDir, { withFileTypes: true })) { + if (!entry.isDirectory() || entry.name === currentInstanceId) continue; + + const configPath = path.resolve(instancesDir, entry.name, "config.json"); + if (existsSync(configPath)) { + configPaths.add(configPath); + } + } + } + + const repoManagedWorktreesRoot = resolveRepoManagedWorktreesRoot(cwd); + if (repoManagedWorktreesRoot && existsSync(repoManagedWorktreesRoot)) { + for (const entry of readdirSync(repoManagedWorktreesRoot, { withFileTypes: true })) { + if (!entry.isDirectory()) continue; + const configPath = path.resolve(repoManagedWorktreesRoot, entry.name, ".paperclip", "config.json"); + if (existsSync(configPath)) { + configPaths.add(configPath); + } + } + } + + for (const configPath of configPaths) { + try { + const config = readConfig(configPath); + if (config?.server.port) { + serverPorts.add(config.server.port); + } + if (config?.database.mode === "embedded-postgres") { + databasePorts.add(config.database.embeddedPostgresPort); + } + } catch { + // Ignore malformed sibling configs. + } + } + + return { serverPorts, databasePorts }; +} + function detectGitBranchName(cwd: string): string | null { try { const value = execFileSync("git", ["branch", "--show-current"], { @@ -886,10 +942,14 @@ async function runWorktreeInit(opts: WorktreeInitOptions): Promise { rmSync(paths.instanceRoot, { recursive: true, force: true }); } + const claimedPorts = collectClaimedWorktreePorts(paths.homeDir, paths.instanceId, paths.cwd); const preferredServerPort = opts.serverPort ?? ((sourceConfig?.server.port ?? 3100) + 1); - const serverPort = await findAvailablePort(preferredServerPort); + const serverPort = await findAvailablePort(preferredServerPort, claimedPorts.serverPorts); const preferredDbPort = opts.dbPort ?? ((sourceConfig?.database.embeddedPostgresPort ?? 54329) + 1); - const databasePort = await findAvailablePort(preferredDbPort, new Set([serverPort])); + const databasePort = await findAvailablePort( + preferredDbPort, + new Set([...claimedPorts.databasePorts, serverPort]), + ); const targetConfig = buildWorktreeConfig({ sourceConfig, paths, diff --git a/server/src/__tests__/worktree-config.test.ts b/server/src/__tests__/worktree-config.test.ts index 2ec475af..3317a254 100644 --- a/server/src/__tests__/worktree-config.test.ts +++ b/server/src/__tests__/worktree-config.test.ts @@ -139,6 +139,195 @@ describe("worktree config repair", () => { expect(process.env.PAPERCLIP_INSTANCE_ID).toBe("pap-884-ai-commits-component"); }); + it("avoids sibling worktree ports when repairing legacy configs", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-worktree-repair-ports-")); + const worktreeRoot = path.join(tempRoot, "PAP-880-thumbs-capture-for-evals-feature"); + const paperclipDir = path.join(worktreeRoot, ".paperclip"); + const configPath = path.join(paperclipDir, "config.json"); + const envPath = path.join(paperclipDir, ".env"); + const sharedRoot = path.join(tempRoot, ".paperclip", "instances", "default"); + const isolatedHome = path.join(tempRoot, ".paperclip-worktrees"); + const siblingInstanceRoot = path.join(isolatedHome, "instances", "pap-878-create-a-mine-tab-in-inbox"); + + await fs.mkdir(paperclipDir, { recursive: true }); + await fs.mkdir(siblingInstanceRoot, { recursive: true }); + await fs.writeFile(configPath, JSON.stringify(buildLegacyConfig(sharedRoot), null, 2) + "\n", "utf8"); + await fs.writeFile( + envPath, + [ + "# Paperclip environment variables", + "PAPERCLIP_IN_WORKTREE=true", + "PAPERCLIP_WORKTREE_NAME=PAP-880-thumbs-capture-for-evals-feature", + "", + ].join("\n"), + "utf8", + ); + await fs.writeFile( + path.join(siblingInstanceRoot, "config.json"), + JSON.stringify( + { + ...buildLegacyConfig(siblingInstanceRoot), + database: { + mode: "embedded-postgres", + embeddedPostgresDataDir: path.join(siblingInstanceRoot, "db"), + embeddedPostgresPort: 54330, + backup: { + enabled: true, + intervalMinutes: 60, + retentionDays: 30, + dir: path.join(siblingInstanceRoot, "data", "backups"), + }, + }, + server: { + deploymentMode: "local_trusted", + exposure: "private", + host: "127.0.0.1", + port: 3101, + allowedHostnames: [], + serveUi: true, + }, + }, + null, + 2, + ) + "\n", + "utf8", + ); + + process.chdir(worktreeRoot); + process.env.PAPERCLIP_IN_WORKTREE = "true"; + process.env.PAPERCLIP_WORKTREE_NAME = "PAP-880-thumbs-capture-for-evals-feature"; + process.env.PAPERCLIP_WORKTREES_DIR = isolatedHome; + + const result = maybeRepairLegacyWorktreeConfigAndEnvFiles(); + const repairedConfig = JSON.parse(await fs.readFile(configPath, "utf8")); + + expect(result.repairedConfig).toBe(true); + expect(repairedConfig.server.port).toBe(3102); + expect(repairedConfig.database.embeddedPostgresPort).toBe(54331); + }); + + it("rebalances duplicate ports for already isolated worktree configs", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-worktree-rebalance-")); + const isolatedHome = path.join(tempRoot, ".paperclip-worktrees"); + const repoWorktreesRoot = path.join(tempRoot, "repo", ".paperclip", "worktrees"); + const siblingWorktreeRoot = path.join(repoWorktreesRoot, "PAP-878-create-a-mine-tab-in-inbox"); + const siblingInstanceRoot = path.join(isolatedHome, "instances", "pap-878-create-a-mine-tab-in-inbox"); + const currentWorktreeRoot = path.join(repoWorktreesRoot, "PAP-884-ai-commits-component"); + const paperclipDir = path.join(currentWorktreeRoot, ".paperclip"); + const configPath = path.join(paperclipDir, "config.json"); + const envPath = path.join(paperclipDir, ".env"); + const currentInstanceRoot = path.join(isolatedHome, "instances", "pap-884-ai-commits-component"); + const siblingConfigPath = path.join(siblingWorktreeRoot, ".paperclip", "config.json"); + + await fs.mkdir(paperclipDir, { recursive: true }); + await fs.mkdir(path.dirname(siblingConfigPath), { recursive: true }); + await fs.writeFile( + configPath, + JSON.stringify( + { + ...buildLegacyConfig(currentInstanceRoot), + database: { + mode: "embedded-postgres", + embeddedPostgresDataDir: path.join(currentInstanceRoot, "db"), + embeddedPostgresPort: 54330, + backup: { + enabled: true, + intervalMinutes: 60, + retentionDays: 30, + dir: path.join(currentInstanceRoot, "data", "backups"), + }, + }, + logging: { + mode: "file", + logDir: path.join(currentInstanceRoot, "logs"), + }, + server: { + deploymentMode: "local_trusted", + exposure: "private", + host: "127.0.0.1", + port: 3101, + allowedHostnames: [], + serveUi: true, + }, + storage: { + provider: "local_disk", + localDisk: { + baseDir: path.join(currentInstanceRoot, "data", "storage"), + }, + s3: { + bucket: "paperclip", + region: "us-east-1", + prefix: "", + forcePathStyle: false, + }, + }, + secrets: { + provider: "local_encrypted", + strictMode: false, + localEncrypted: { + keyFilePath: path.join(currentInstanceRoot, "secrets", "master.key"), + }, + }, + }, + null, + 2, + ) + "\n", + "utf8", + ); + await fs.writeFile( + envPath, + [ + "# Paperclip environment variables", + "PAPERCLIP_IN_WORKTREE=true", + "PAPERCLIP_WORKTREE_NAME=PAP-884-ai-commits-component", + "", + ].join("\n"), + "utf8", + ); + await fs.writeFile( + siblingConfigPath, + JSON.stringify( + { + ...buildLegacyConfig(siblingInstanceRoot), + database: { + mode: "embedded-postgres", + embeddedPostgresDataDir: path.join(siblingInstanceRoot, "db"), + embeddedPostgresPort: 54330, + backup: { + enabled: true, + intervalMinutes: 60, + retentionDays: 30, + dir: path.join(siblingInstanceRoot, "data", "backups"), + }, + }, + server: { + deploymentMode: "local_trusted", + exposure: "private", + host: "127.0.0.1", + port: 3101, + allowedHostnames: [], + serveUi: true, + }, + }, + null, + 2, + ) + "\n", + "utf8", + ); + + process.chdir(currentWorktreeRoot); + process.env.PAPERCLIP_IN_WORKTREE = "true"; + process.env.PAPERCLIP_WORKTREE_NAME = "PAP-884-ai-commits-component"; + process.env.PAPERCLIP_WORKTREES_DIR = isolatedHome; + + const result = maybeRepairLegacyWorktreeConfigAndEnvFiles(); + const repairedConfig = JSON.parse(await fs.readFile(configPath, "utf8")); + + expect(result.repairedConfig).toBe(true); + expect(repairedConfig.server.port).toBe(3102); + expect(repairedConfig.database.embeddedPostgresPort).toBe(54331); + }); + it("persists runtime-selected worktree ports back into config", async () => { const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-worktree-ports-")); const worktreeRoot = path.join(tempRoot, "PAP-878-create-a-mine-tab-in-inbox"); diff --git a/server/src/worktree-config.ts b/server/src/worktree-config.ts index 51397b84..3656b7dc 100644 --- a/server/src/worktree-config.ts +++ b/server/src/worktree-config.ts @@ -149,10 +149,89 @@ function writeConfigFile(configPath: string, config: PaperclipConfig): void { fs.writeFileSync(configPath, JSON.stringify(config, null, 2) + "\n", { mode: 0o600 }); } +function resolveRepoManagedWorktreesRoot(worktreeRoot: string): string | null { + const normalized = path.resolve(worktreeRoot); + const marker = `${path.sep}.paperclip${path.sep}worktrees${path.sep}`; + const index = normalized.indexOf(marker); + if (index === -1) return null; + const repoRoot = normalized.slice(0, index); + return path.resolve(repoRoot, ".paperclip", "worktrees"); +} + +function collectSiblingWorktreePorts(context: WorktreeRuntimeContext): { + serverPorts: Set; + databasePorts: Set; +} { + const serverPorts = new Set(); + const databasePorts = new Set(); + const siblingConfigPaths = new Set(); + const instancesDir = path.resolve(context.homeDir, "instances"); + if (fs.existsSync(instancesDir)) { + for (const entry of fs.readdirSync(instancesDir, { withFileTypes: true })) { + if (!entry.isDirectory() || entry.name === context.instanceId) continue; + + const siblingConfigPath = path.resolve(instancesDir, entry.name, "config.json"); + if (fs.existsSync(siblingConfigPath)) { + siblingConfigPaths.add(siblingConfigPath); + } + } + } + + const repoManagedWorktreesRoot = resolveRepoManagedWorktreesRoot(path.dirname(context.configPath)); + if (repoManagedWorktreesRoot && fs.existsSync(repoManagedWorktreesRoot)) { + for (const entry of fs.readdirSync(repoManagedWorktreesRoot, { withFileTypes: true })) { + if (!entry.isDirectory()) continue; + + const siblingConfigPath = path.resolve(repoManagedWorktreesRoot, entry.name, ".paperclip", "config.json"); + if (path.resolve(siblingConfigPath) === path.resolve(context.configPath)) continue; + if (fs.existsSync(siblingConfigPath)) { + siblingConfigPaths.add(siblingConfigPath); + } + } + } + + for (const siblingConfigPath of siblingConfigPaths) { + try { + const siblingConfig = JSON.parse(fs.readFileSync(siblingConfigPath, "utf8")) as PaperclipConfig; + if (Number.isInteger(siblingConfig.server.port) && siblingConfig.server.port > 0) { + serverPorts.add(siblingConfig.server.port); + } + if ( + siblingConfig.database.mode === "embedded-postgres" && + Number.isInteger(siblingConfig.database.embeddedPostgresPort) && + siblingConfig.database.embeddedPostgresPort > 0 + ) { + databasePorts.add(siblingConfig.database.embeddedPostgresPort); + } + } catch { + // Ignore sibling configs that are missing or malformed. + } + } + + return { serverPorts, databasePorts }; +} + +function findNextUnclaimedPort(preferredPort: number, claimedPorts: Set): number { + let port = Math.max(1, Math.trunc(preferredPort)); + while (claimedPorts.has(port)) { + port += 1; + } + return port; +} + function buildIsolatedWorktreeConfig( config: PaperclipConfig, context: WorktreeRuntimeContext, + portOverrides?: { + serverPort?: number; + databasePort?: number; + }, ): PaperclipConfig { + const serverPort = portOverrides?.serverPort ?? config.server.port; + const databasePort = + config.database.mode === "embedded-postgres" + ? portOverrides?.databasePort ?? config.database.embeddedPostgresPort + : undefined; const nextConfig: PaperclipConfig = { ...config, database: { @@ -160,6 +239,7 @@ function buildIsolatedWorktreeConfig( ...(config.database.mode === "embedded-postgres" ? { embeddedPostgresDataDir: context.embeddedPostgresDataDir, + embeddedPostgresPort: databasePort ?? config.database.embeddedPostgresPort, backup: { ...config.database.backup, dir: context.backupDir, @@ -167,6 +247,10 @@ function buildIsolatedWorktreeConfig( } : {}), }, + server: { + ...config.server, + port: serverPort, + }, logging: { ...config.logging, logDir: context.logDir, @@ -190,7 +274,7 @@ function buildIsolatedWorktreeConfig( if (config.auth.baseUrlMode === "explicit" && config.auth.publicBaseUrl) { nextConfig.auth = { ...config.auth, - publicBaseUrl: rewriteLocalUrlPort(config.auth.publicBaseUrl, config.server.port), + publicBaseUrl: rewriteLocalUrlPort(config.auth.publicBaseUrl, serverPort), }; } @@ -298,8 +382,34 @@ export function maybeRepairLegacyWorktreeConfigAndEnvFiles(): { if (fs.existsSync(context.configPath)) { try { const parsed = JSON.parse(fs.readFileSync(context.configPath, "utf8")) as PaperclipConfig; - if (needsWorktreeConfigRepair(parsed, context)) { - writeConfigFile(context.configPath, buildIsolatedWorktreeConfig(parsed, context)); + const siblingPorts = collectSiblingWorktreePorts(context); + const hasSiblingPortCollision = + siblingPorts.serverPorts.has(parsed.server.port) || + (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; + + writeConfigFile( + context.configPath, + buildIsolatedWorktreeConfig(parsed, context, { + serverPort: selectedServerPort, + databasePort: selectedDatabasePort, + }), + ); repairedConfig = true; } } catch {