From 5952c201d3c7bbbdf498eb93722140cce6236b0b Mon Sep 17 00:00:00 2001 From: Lambda Date: Fri, 26 Jun 2026 15:02:07 +0800 Subject: [PATCH] fix(desktop): make worktree dev port/suffix collision-safe (MUL-3724) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses code review on #4598: - Renderer port base 5173 → 5174 so a worktree whose offset is 0 (e.g. cksum("/tmp/multica-3494") % 1000 === 0) no longer collides with the primary checkout's default 5173. - DESKTOP_APP_SUFFIX is now "-" instead of just the folder name, so worktrees that share a basename at different paths (or names that slug to the same fallback) get distinct single-instance locks. Without it the second Electron was still blocked by the shared lock. - Tests: offset-0 port guard, and same-basename-different-path disambiguation. Co-authored-by: multica-agent --- CONTRIBUTING.md | 10 +++--- apps/desktop/scripts/worktree-dev-env.mjs | 28 ++++++++++------ .../desktop/scripts/worktree-dev-env.test.mjs | 33 +++++++++++++++---- 3 files changed, 51 insertions(+), 20 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 46b8a486a..16f9f76a9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -496,10 +496,12 @@ own desktop dev instance at once — no extra setup. From a linked worktree it derives, from the worktree path (same `cksum % 1000` offset as the backend / frontend ports in `.env.worktree`): -- `DESKTOP_RENDERER_PORT` = `5173 + offset` — its own Vite dev server -- `DESKTOP_APP_SUFFIX` = the worktree folder name — its own single-instance - lock / `userData`, and an app named `Multica Canary ` so it is - distinguishable in Cmd+Tab +- `DESKTOP_RENDERER_PORT` = `5174 + offset` — its own Vite dev server (`5174` + base leaves `5173` for the primary checkout, even when `offset` is `0`) +- `DESKTOP_APP_SUFFIX` = `-` — its own single-instance lock / + `userData`, and an app named `Multica Canary -` so it is + distinguishable in Cmd+Tab. The offset keeps it unique across worktrees that + share a folder name at different paths. The primary checkout is left untouched (`5173`, `Multica Canary`). Set either env var explicitly to override the derived value. Which backend each instance diff --git a/apps/desktop/scripts/worktree-dev-env.mjs b/apps/desktop/scripts/worktree-dev-env.mjs index cc16406f2..c405f0454 100644 --- a/apps/desktop/scripts/worktree-dev-env.mjs +++ b/apps/desktop/scripts/worktree-dev-env.mjs @@ -16,12 +16,15 @@ import { statSync } from "node:fs"; import { basename, join } from "node:path"; -const RENDERER_PORT_BASE = 5173; +// Worktree renderer ports start at 5174 so they never reuse 5173 — the primary +// checkout's default — even when a worktree's offset is 0 (e.g. POSIX cksum of +// "/tmp/multica-3494" is 1189739000, and 1189739000 % 1000 === 0). Range 5174–6173. +const RENDERER_PORT_BASE = 5174; const OFFSET_MODULO = 1000; // POSIX cksum (CRC-32), kept byte-compatible with `cksum(1)` so the offset // matches scripts/init-worktree-env.sh — a worktree's backend (18080+offset), -// frontend (13000+offset) and desktop renderer (5173+offset) ports all share +// frontend (13000+offset) and desktop renderer (5174+offset) ports all share // one offset. Verified against coreutils: cksum of "/tmp/foo" → 427878967. function cksumTable() { const table = new Uint32Array(256); @@ -60,15 +63,20 @@ export function rendererPortForPath(path) { return RENDERER_PORT_BASE + offsetForPath(path); } -// Worktree folder name → a readable, filesystem-safe suffix. The dev app then -// shows e.g. "Multica Canary mul-3724" in Cmd+Tab and gets its own userData / -// single-instance lock under that name. +// Worktree → a readable, unique, filesystem-safe suffix "-". +// The dev app then shows e.g. "Multica Canary mul-3724-194" in Cmd+Tab and gets +// its own userData / single-instance lock under that name. The offset is what +// makes the lock unique: the folder name alone collides for worktrees that share +// a basename at different paths (e.g. /a/multica vs /b/multica) or whose names +// slug to the same fallback — those would share one lock and the second Electron +// would still be blocked. export function appSuffixForPath(path) { - const slug = basename(path) - .toLowerCase() - .replace(/[^a-z0-9]+/g, "-") - .replace(/^-+|-+$/g, ""); - return slug || "worktree"; + const slug = + basename(path) + .toLowerCase() + .replace(/[^a-z0-9]+/g, "-") + .replace(/^-+|-+$/g, "") || "worktree"; + return `${slug}-${offsetForPath(path)}`; } // A linked git worktree has a `.git` FILE (a "gitdir:" pointer); the primary diff --git a/apps/desktop/scripts/worktree-dev-env.test.mjs b/apps/desktop/scripts/worktree-dev-env.test.mjs index e5d52c1cb..b8d82b54e 100644 --- a/apps/desktop/scripts/worktree-dev-env.test.mjs +++ b/apps/desktop/scripts/worktree-dev-env.test.mjs @@ -36,14 +36,35 @@ describe("worktree-dev-env", () => { expect(offsetForPath("/tmp/foo")).toBe(427878967 % 1000); }); - it("renderer port is 5173 + offset", () => { - expect(rendererPortForPath("/tmp/foo")).toBe(5173 + (427878967 % 1000)); + it("renderer port is 5174 + offset (5173 reserved for the primary checkout)", () => { + expect(rendererPortForPath("/tmp/foo")).toBe(5174 + (427878967 % 1000)); }); - it("slugifies the worktree folder name", () => { - expect(appSuffixForPath("/work/MUL-3724_Desktop")).toBe("mul-3724-desktop"); - expect(appSuffixForPath("/work/feat/some thing")).toBe("some-thing"); - expect(appSuffixForPath("/work/___")).toBe("worktree"); + it("never reuses 5173 even when the offset is 0", () => { + // POSIX cksum("/tmp/multica-3494") === 1189739000, % 1000 === 0 + expect(offsetForPath("/tmp/multica-3494")).toBe(0); + expect(rendererPortForPath("/tmp/multica-3494")).toBe(5174); + expect(rendererPortForPath("/tmp/multica-3494")).not.toBe(5173); + }); + + it("suffix is '-' so it stays recognizable and unique", () => { + expect(appSuffixForPath("/work/MUL-3724_Desktop")).toBe( + `mul-3724-desktop-${offsetForPath("/work/MUL-3724_Desktop")}`, + ); + expect(appSuffixForPath("/work/feat/some thing")).toBe( + `some-thing-${offsetForPath("/work/feat/some thing")}`, + ); + // empty/non-ascii slug falls back to "worktree", still disambiguated by offset + expect(appSuffixForPath("/work/___")).toBe(`worktree-${offsetForPath("/work/___")}`); + }); + + it("disambiguates worktrees that share a folder name at different paths", () => { + // Same basename "multica", different parent dirs → different offsets/suffixes, + // so each gets its own single-instance lock. + expect(offsetForPath("/tmp/a/multica")).not.toBe(offsetForPath("/tmp/b/multica")); + expect(appSuffixForPath("/tmp/a/multica")).not.toBe( + appSuffixForPath("/tmp/b/multica"), + ); }); it("auto-isolates a linked worktree (.git is a file)", () => {