fix(server): prevent recurring worktree port conflicts (#9642)
## Thinking Path > - Paperclip is the control plane operators use to run AI-agent companies and their isolated development workspaces. > - Worktree startup assigns each workspace a server port and an embedded PostgreSQL port. > - Existing collision detection depended on discovering sibling configs from the current repository layout, so worktrees in different repository roots could select the same ports. > - Concurrent startup also had no shared critical section, allowing two worktrees to observe the same available ports before either persisted its selection. > - Repeated collisions prevented otherwise isolated workspaces from starting reliably and could recur after a port was repaired once. > - This pull request adds a shared, locked registry of active worktree config paths and uses it during port selection and repair. > - The benefit is stable, persisted, cross-repository port isolation for both the Paperclip server and embedded PostgreSQL. ## Linked Issues or Issue Description ### What happened? When multiple Paperclip worktrees shared the same worktree home but lived under different repository roots, startup could assign duplicate server and embedded PostgreSQL ports. The prior sibling scan did not reliably discover configs outside the current repository, and simultaneous repairs were not serialized. ### Expected behavior Each active worktree should reserve unique server and database ports across repository roots, persist any repaired selection, and reuse the persisted ports on subsequent starts. ### Steps to reproduce 1. Create two Paperclip worktrees in different repository roots that share `PAPERCLIP_WORKTREES_DIR`. 2. Give both worktree configs the same server and embedded PostgreSQL ports. 3. Start or repair both worktrees. 4. Observe that both can retain the same ports because neither reliably discovers the other configuration. ### Environment - Version: reproducible on `master` before this change - Deployment: local development worktrees built from source - Adapter: not adapter-specific - Database: embedded PostgreSQL Related prior reliability work: #1829. Related documentation for recovering port conflicts: #9407. ## What Changed - Add a shared `worktree-port-reservations.json` registry under the worktree home, containing live worktree config paths. - Serialize registry reads, collision detection, config repair, and registry updates with a stale-safe filesystem lock. - Include registered configs and isolated instance configs when collecting reserved server and embedded PostgreSQL ports. - Atomically prune stale registry entries and persist repaired ports plus the matching public base URL. - Add regression coverage for cross-repository collisions, persisted repairs, and repeat startup behavior. ## Verification - `pnpm exec vitest run server/src/__tests__/worktree-config.test.ts` — 14 tests passed, including stale-lock recovery. - `pnpm --filter @paperclipai/server typecheck` — passed. - Rebased onto current `public-gh/master` before verification. ## Risks - Low-to-moderate risk: worktree startup now briefly acquires a filesystem lock in the shared worktree home. - The lock has a 10-second acquisition timeout and removes lock directories older than 5 seconds so interrupted owners are recoverable within the wait window. - Registry writes are atomic and stale config paths are pruned, limiting persistent state to existing worktree configs. - The change is scoped to worktree runtime configuration and does not affect normal main-instance configuration. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI Codex using GPT-5.3 Codex and GPT-5.4 with repository access, terminal execution, and code-review tooling. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Dotta
GitHub
parent
f1508a7929
commit
5588ddf681
|
|
@ -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");
|
||||
|
|
|
|||
|
|
@ -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<T>(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<string> {
|
||||
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<WorktreePortRegistry>;
|
||||
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<string>): 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<string> = [],
|
||||
): {
|
||||
serverPorts: Set<number>;
|
||||
databasePorts: Set<number>;
|
||||
configPaths: Set<string>;
|
||||
} {
|
||||
const serverPorts = new Set<number>();
|
||||
const databasePorts = new Set<number>();
|
||||
const siblingConfigPaths = new Set<string>();
|
||||
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>): 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)
|
||||
|
|
|
|||
Loading…
Reference in New Issue