test: cover guide staleness, deskAlive, and stable-port fallback

Three invariants the spec promises but nothing tested: guideStale()'s
baseDiffHash comparison, deskAlive()'s dead-vs-live desk-lock probe, and
startServer()'s EADDRINUSE fallback to a different port. Adds regression
tests for all three, plus the minimal testability tweaks each needed
(guideStale()'s hash logic extracted into a store-free guide-derive.ts
since importing the Alpine store crashes under node:test; deskAlive
exported and cli.ts gated behind an entry-point guard so importing it
for the test doesn't launch a real desk).

The entry-point guard initially compared import.meta.url to
process.argv[1] directly, which breaks under npm's bin symlink
(.bin/galley -> dist/cli.js): Node resolves import.meta.url through the
symlink to the real path but leaves argv[1] as the symlink path, so the
guard never matched and the CLI silently no-op'd. Fixed by realpath'ing
argv[1] before comparing, with a regression test that runs cli.ts
through an actual symlink.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: ymansurozer

src/cli.test.ts

@@ -0,0 +1,61 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { spawnSync } from "node:child_process";
+import { mkdtemp, rm, symlink } from "node:fs/promises";
+import http from "node:http";
+import { tmpdir } from "node:os";
+import path from "node:path";
+import { fileURLToPath } from "node:url";
+import { deskAlive } from "./cli.js";
+
+// A desk lock can outlive its process (crash, SIGKILL) — deskAlive is the only thing standing
+// between "trust the lock" and "trust it only if the server answers" (see state.ts's stablePort
+// invariant notes). Cover both sides: a dead URL and a URL that actually answers.
+
+test("deskAlive resolves false for a URL nothing is listening on, within the abort budget", async () => {
+  // Port 1 is a privileged, essentially-never-bound port — connection refused (or a hang the
+  // ~1500ms internal abort catches) either way lands on false. We only assert the outcome and a
+  // generous wall-clock ceiling, not the exact abort timing.
+  const start = Date.now();
+  const alive = await deskAlive("http://127.0.0.1:1/");
+  assert.equal(alive, false);
+  assert.ok(Date.now() - start < 5000, "resolves well within the ~1500ms abort budget plus slack");
+});
+
+test("deskAlive resolves true for a live server that answers", async () => {
+  const server = http.createServer((_req, res) => {
+    res.writeHead(200, { "content-type": "application/json" });
+    res.end("{}");
+  });
+  await new Promise<void>((resolve) => server.listen(0, "127.0.0.1", resolve));
+  try {
+    const address = server.address();
+    const port = typeof address === "object" && address ? address.port : 0;
+    const alive = await deskAlive(`http://127.0.0.1:${port}/`);
+    assert.equal(alive, true);
+  } finally {
+    server.close();
+  }
+});
+
+// Regression: npm installs the published bin as a SYMLINK (.bin/galley -> dist/cli.js). Node
+// resolves import.meta.url through the symlink to cli.ts's real path, but leaves
+// process.argv[1] as the symlink path — an entry-point guard that compares the two directly
+// (no realpath) never fires under a symlinked invocation, and the CLI silently no-ops (exits 0,
+// no output). Reproduce that exact shape here: symlink to the real src/cli.ts and run it through
+// `node --import tsx` the same way the built bin runs through node directly.
+test("running cli.ts through a symlink (npm bin shape) still runs main()", async () => {
+  const dir = await mkdtemp(path.join(tmpdir(), "galley-cli-symlink-"));
+  const link = path.join(dir, "galley");
+  const cliPath = fileURLToPath(new URL("./cli.ts", import.meta.url));
+  try {
+    await symlink(cliPath, link);
+    const result = spawnSync(process.execPath, ["--import", "tsx", link, "--help"], {
+      encoding: "utf8",
+    });
+    assert.equal(result.status, 0);
+    assert.match(result.stdout, /galley — an integrated review environment/);
+  } finally {
+    await rm(dir, { recursive: true, force: true });
+  }
+});

src/cli.ts

@@ -1,7 +1,8 @@
 #!/usr/bin/env node
-import { readFileSync, unlinkSync, writeFileSync } from "node:fs";
+import { readFileSync, realpathSync, unlinkSync, writeFileSync } from "node:fs";
 import http from "node:http";
 import path from "node:path";
+import { pathToFileURL } from "node:url";
 
 import { getBranch, getGitRoot, gh, git } from "./git.js";
 import { validateGuide } from "./guide.js";
@@ -103,8 +104,8 @@ function httpGetJson(urlStr: string): Promise<{ status: number; body: any }> {
 }
 
 // A desk lock can outlive its process (crash, SIGKILL) — trust it only if the
-// server actually answers.
-async function deskAlive(url: string): Promise<boolean> {
+// server actually answers. Exported for cli.test.ts; behavior is unchanged.
+export async function deskAlive(url: string): Promise<boolean> {
   const ctrl = new AbortController();
   const timer = setTimeout(() => ctrl.abort(), 1500);
   try {
@@ -662,7 +663,22 @@ async function main() {
   return runDesk("repo", undefined, args);
 }
 
-main().catch((error) => {
-  console.error(error instanceof Error ? error.stack || error.message : String(error));
-  process.exitCode = 1;
-});
+// Run only when executed as the bin (`galley` / `node dist/cli.js`), not when cli.test.ts
+// imports this module to reach deskAlive() — otherwise import alone would launch a desk. npm
+// installs the bin as a SYMLINK (.bin/galley -> dist/cli.js); Node resolves import.meta.url
+// through the symlink to the real path, but leaves process.argv[1] as the symlink path — so
+// argv[1] must be realpath'd before comparing, or the guard never fires under the published
+// bin and the CLI silently no-ops. try/catch guards a dangling/unusual argv[1].
+let isMain = false;
+try {
+  isMain =
+    !!process.argv[1] && import.meta.url === pathToFileURL(realpathSync(process.argv[1])).href;
+} catch {
+  /* argv[1] doesn't resolve — treat as not the entry point */
+}
+if (isMain) {
+  main().catch((error) => {
+    console.error(error instanceof Error ? error.stack || error.message : String(error));
+    process.exitCode = 1;
+  });
+}

src/server.test.ts

@@ -719,3 +719,36 @@ test("settings API round-trips editorCommand", async () => {
     assert.equal(loaded.settings?.editorCommand, "cursor -g {file}:{line}");
   });
 });
+
+test("stable-port EADDRINUSE falls back to a different port instead of throwing", async () => {
+  // A restarted desk's stablePort (deterministic per repo+session) can still be held by another
+  // process — startServer must rebind elsewhere rather than crash the launch.
+  const root = await mkdtemp(path.join(tmpdir(), "galley-portfallback-"));
+  const oldHome = process.env.HOME;
+  process.env.HOME = root;
+  const occupied = http.createServer();
+  await new Promise<void>((resolve) => occupied.listen(0, "127.0.0.1", resolve));
+  const address = occupied.address();
+  const takenPort = typeof address === "object" && address ? address.port : 0;
+  try {
+    const handle = await startServer({
+      state: state(root),
+      open: false,
+      idleTimeoutMs: 0,
+      port: takenPort,
+    });
+    try {
+      const boundPort = Number(new URL(handle.url).port);
+      assert.notEqual(boundPort, takenPort, "rebound to a different, non-zero port");
+      assert.ok(boundPort > 0);
+      const res = await fetch(`${handle.url}api/state`);
+      assert.equal(res.status, 200, "the fallback server actually answers");
+    } finally {
+      handle.server.close();
+    }
+  } finally {
+    occupied.close();
+    process.env.HOME = oldHome;
+    await rm(root, { recursive: true, force: true });
+  }
+});

src/ui/guide-derive.test.ts

@@ -0,0 +1,15 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { isGuideBaseStale } from "./guide-derive.js";
+
+test("matching baseDiffHash is not stale", () => {
+  assert.equal(isGuideBaseStale("h1", "h1"), false);
+});
+
+test("a differing baseDiffHash is stale", () => {
+  assert.equal(isGuideBaseStale("h2", "h1"), true);
+});
+
+test("a guide with no baseDiffHash is never stale", () => {
+  assert.equal(isGuideBaseStale("h1", undefined), false);
+});

src/ui/guide-derive.ts

@@ -0,0 +1,11 @@
+// Pure guide-staleness check (store-free, so it's unit-testable — see skim-derive.ts for the
+// pattern). A guide is stale when it carries a baseDiffHash that no longer matches the diff now
+// loaded (the agent rewrote code and the desk reloaded onto a newer diff). A guide with no
+// baseDiffHash predates the field and is never flagged stale. guide.ts's guideStale() is the
+// thin store-reading wrapper.
+export function isGuideBaseStale(
+  baseDiffHash: string,
+  guideBaseDiffHash: string | undefined,
+): boolean {
+  return !!guideBaseDiffHash && guideBaseDiffHash !== baseDiffHash;
+}

src/ui/guide.ts

@@ -1,6 +1,7 @@
 import { S, $, esc } from "./store";
 import { flowIndex } from "./changes";
 import type { FlowIndex } from "./flow-index";
+import { isGuideBaseStale } from "./guide-derive";
 import { navFileOrder, nextUnreviewed, wrapNextTarget, wrapPrevTarget } from "./seek";
 import { isSkimGroupExpanded } from "./skim";
 import { renderMarkdown } from "./markdown";
@@ -215,11 +216,7 @@ export function showGuideBar(): boolean {
 // The guide was generated against an older diff than the one now loaded (e.g. the agent
 // edited code and the desk reloaded). Advisory only — the guide still renders.
 export function guideStale(): boolean {
-  return (
-    hasGuide() &&
-    !!S.state.guide!.baseDiffHash &&
-    S.state.guide!.baseDiffHash !== S.state.baseDiffHash
-  );
+  return hasGuide() && isGuideBaseStale(S.state.baseDiffHash, S.state.guide!.baseDiffHash);
 }
 
 // Render the Overview page into #diff: overview → optional PR description → Start. No file