Skip to content

Commit cca78fa

Browse files
committed
fix(scripts): stop unconfigured siblings from falsely claiming the sync port base
Independent review caught a real logic defect: readSyncPort defaulted to SYNC_PORT_BASE whenever a sibling worktree had no sync.port set at all, so every worktree that hasn't run sync yet (the common case — sync: stays absent until mongo.uri is present) looked like it was actively occupying port 3010 to every other worktree's conflict check. That's the exact opposite of the comment above findNextSyncPort, which assumes the base is usually free since most worktrees have never had one assigned. readSyncPort now returns null when sync.port isn't actually configured, and readSiblingSyncPorts filters those out, so only a worktree that has genuinely claimed a sync port counts as a claim.
1 parent 8c3dc9c commit cca78fa

2 files changed

Lines changed: 17 additions & 10 deletions

File tree

packages/scripts/src/commands/ensure-sync-config.test.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,14 @@ describe("readSyncPort", () => {
3535
expect(readSyncPort("sync:\n port: 3011\n")).toBe(3011);
3636
});
3737

38-
it("falls back to the base when unset", () => {
39-
expect(readSyncPort("web:\n port: 9080\n")).toBe(3010);
38+
it("returns null when no sync.port is configured, not a default guess", () => {
39+
// A worktree with no sync: block yet must not read as claiming
40+
// SYNC_PORT_BASE to every other worktree's conflict check.
41+
expect(readSyncPort("web:\n port: 9080\n")).toBeNull();
4042
});
4143

42-
it("falls back to the base for malformed yaml", () => {
43-
expect(readSyncPort("{{ not yaml")).toBe(3010);
44+
it("returns null for malformed yaml", () => {
45+
expect(readSyncPort("{{ not yaml")).toBeNull();
4446
});
4547
});
4648

packages/scripts/src/commands/ensure-sync-config.ts

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,17 @@ import { readFileSync, writeFileSync } from "node:fs";
3131
export const SYNC_PORT_BASE = 3010;
3232
const SYNC_DATABASE_NAME = "compass_sync";
3333

34-
export function readSyncPort(yamlText: string): number {
34+
// null means no sync.port is actually configured — distinct from "configured
35+
// to SYNC_PORT_BASE", so an unconfigured sibling (the common case; sync: is
36+
// nullish until mongo.uri is present) doesn't falsely read as claiming the
37+
// default port to every other worktree's conflict check.
38+
export function readSyncPort(yamlText: string): number | null {
3539
try {
3640
const config = parse(yamlText) as { sync?: { port?: string | number } };
37-
return Number(config?.sync?.port) || SYNC_PORT_BASE;
41+
const port = Number(config?.sync?.port);
42+
return port > 0 ? port : null;
3843
} catch {
39-
return SYNC_PORT_BASE;
44+
return null;
4045
}
4146
}
4247

@@ -93,9 +98,9 @@ export function ensureSyncConfig(
9398
}
9499

95100
export function readSiblingSyncPorts(root: string): number[] {
96-
return siblingConfigPaths(root).map((file) =>
97-
readSyncPort(readFileSync(file, "utf8")),
98-
);
101+
return siblingConfigPaths(root)
102+
.map((file) => readSyncPort(readFileSync(file, "utf8")))
103+
.filter((port): port is number => port !== null);
99104
}
100105

101106
// Unlike dev-ports.ts's findNextPorts, this searches from the base itself

0 commit comments

Comments
 (0)