refactor: tighten unit-file port regex and warn on explicit-port collision

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: hugocasa

bin/src/install-ports.test.ts

@@ -87,6 +87,63 @@ describe("readPortFromUnit", () => {
   it("returns null for a missing file", () => {
     expect(readPortFromUnit("/no/such/path.service")).toBeNull();
   });
+
+  // Round-trips against the exact strings `service.ts` emits, so a future
+  // re-indent or wrapping change in `generateLaunchdPlist` / `generateSystemdUnit`
+  // surfaces as a failing test rather than a silent regression to "auto-pick".
+  it("round-trips against the systemd unit format service.ts writes", async () => {
+    const dir = await makeTempDir();
+    const filePath = join(dir, "webmux-roundtrip.service");
+    const content = [
+      "[Unit]",
+      "Description=webmux dashboard — roundtrip",
+      "",
+      "[Service]",
+      "Type=simple",
+      "ExecStart=/usr/local/bin/webmux serve --port 5117",
+      "WorkingDirectory=/home/x/proj",
+      "Restart=on-failure",
+      "RestartSec=5",
+      "Environment=PORT=5117",
+      "Environment=WEBMUX_PROJECT_DIR=/home/x/proj",
+      "Environment=PATH=/usr/local/bin",
+      "",
+      "[Install]",
+      "WantedBy=default.target",
+      "",
+    ].join("\n");
+    await writeFile(filePath, content);
+
+    expect(readPortFromUnit(filePath)).toBe(5117);
+  });
+
+  it("round-trips against the launchd plist format service.ts writes", async () => {
+    const dir = await makeTempDir();
+    const filePath = join(dir, "com.webmux.roundtrip.plist");
+    const content = [
+      "<?xml version=\"1.0\" encoding=\"UTF-8\"?>",
+      "<!DOCTYPE plist PUBLIC \"-//Apple//DTD PLIST 1.0//EN\" \"http://www.apple.com/DTDs/PropertyList-1.0.dtd\">",
+      "<plist version=\"1.0\">",
+      "<dict>",
+      "  <key>Label</key>",
+      "  <string>com.webmux.roundtrip</string>",
+      "  <key>ProgramArguments</key>",
+      "  <array>",
+      "    <string>/usr/local/bin/webmux</string>",
+      "    <string>serve</string>",
+      "    <string>--port</string>",
+      "    <string>5222</string>",
+      "  </array>",
+      "  <key>WorkingDirectory</key>",
+      "  <string>/Users/x/proj</string>",
+      "</dict>",
+      "</plist>",
+      "",
+    ].join("\n");
+    await writeFile(filePath, content);
+
+    expect(readPortFromUnit(filePath)).toBe(5222);
+  });
 });
 
 describe("readInstalledServicePorts", () => {

bin/src/install-ports.ts

@@ -6,7 +6,8 @@ import { createInstanceRegistry } from "../../backend/src/adapters/instance-regi
 export const DEFAULT_SYSTEMD_DIR = join(homedir(), ".config", "systemd", "user");
 export const DEFAULT_LAUNCHD_DIR = join(homedir(), "Library", "LaunchAgents");
 
-const UNIT_PORT_RE = /--port[\s\S]{0,40}?(\d{2,5})/;
+const SYSTEMD_PORT_RE = /--port\s+(\d+)/;
+const LAUNCHD_PORT_RE = /<string>--port<\/string>\s*<string>(\d+)<\/string>/;
 
 /** Lowest port `>= start` not in `taken`. */
 export function pickFreePort(start: number, taken: Iterable<number>): number {
@@ -51,21 +52,22 @@ export function readInstalledServicePorts(opts: {
   return ports;
 }
 
-/** Parse a `--port N` value out of a service unit file. Tolerates the gap
- *  between the flag and value being whitespace (systemd) or whitespace +
- *  XML wrapping (launchd plist). Returns null when no port is declared or
- *  the file is unreadable. */
+/** Parse a `--port N` value out of a service unit file. Dispatches on file
+ *  extension so each format gets a tight regex (`--port 5111` for systemd vs
+ *  `<string>--port</string>\s*<string>5111</string>` for launchd plists) —
+ *  no shared char-window assumption that breaks if either generator's
+ *  indentation changes. Returns null when no port is declared or the file
+ *  is unreadable. */
 export function readPortFromUnit(filePath: string): number | null {
   let text: string;
   try {
     text = readFileSync(filePath, "utf8");
   } catch {
     return null;
   }
-  const match = UNIT_PORT_RE.exec(text);
-  if (!match) return null;
-  const port = parseInt(match[1], 10);
-  return Number.isNaN(port) ? null : port;
+  const regex = filePath.endsWith(".plist") ? LAUNCHD_PORT_RE : SYSTEMD_PORT_RE;
+  const match = regex.exec(text);
+  return match ? parseInt(match[1], 10) : null;
 }
 
 /** Combine live-registry ports and installed-unit ports into a single set

bin/src/service.ts

@@ -193,6 +193,7 @@ async function install(config: ServiceConfig, portExplicit: boolean): Promise<vo
   const requestedPort = config.port;
   let chosenPort = requestedPort;
   let portNote: string | null = null;
+  let portWarning: string | null = null;
 
   if (!portExplicit) {
     const existingPort = alreadyInstalled ? readPortFromUnit(filePath) : null;
@@ -208,6 +209,14 @@ async function install(config: ServiceConfig, portExplicit: boolean): Promise<vo
         portNote = `Port ${requestedPort} is already used by another webmux instance — picked ${chosenPort} instead (pass --port to override).`;
       }
     }
+  } else {
+    // Explicit `--port` always wins, but the service will fail to bind on
+    // start if something else is already there — surface it now rather than
+    // making the user dig through `journalctl` / `launchctl` logs later.
+    const taken = discoverTakenPorts({ excludeUnitPath: filePath });
+    if (taken.has(requestedPort)) {
+      portWarning = `Port ${requestedPort} is already claimed by another webmux instance. The service will fail to bind on start; omit --port to auto-pick a free port.`;
+    }
   }
 
   config = { ...config, port: chosenPort };
@@ -227,6 +236,7 @@ async function install(config: ServiceConfig, portExplicit: boolean): Promise<vo
   );
 
   if (portNote) p.log.info(portNote);
+  if (portWarning) p.log.warn(portWarning);
 
   const ok = await p.confirm({ message: "Proceed?" });
   if (p.isCancel(ok) || !ok) {