From f44e5ca3de1e8ca1c480271b203edd08a60b5ab0 Mon Sep 17 00:00:00 2001 From: Karn Date: Sun, 27 Sep 2026 01:54:15 +0530 Subject: [PATCH 1/3] feat: reins restart, and a stale daemon restarts itself after an upgrade After `npm i -g @karnstack/reins@latest` the background daemon kept running the old code until someone knew to `reins kill` it. Now: - ensureDaemon restarts a live daemon whose /health version is older than the CLI. Only older: a newer daemon is left alone so two installed CLI versions can't bounce it back and forth. - `reins restart` stops the daemon, waits for its port to go quiet, spawns a fresh one and waits for previously connected browsers to reconnect. - `reins kill` now waits until the daemon has actually exited, so a command straight after it can't reach the dying process. The old service-manager `restart` (launchd/systemd) was removed in 4e7cf33; this one is for the spawn-on-demand daemon, so the help-text guard against it is dropped. Co-Authored-By: Claude Opus 5.5 --- .changeset/reins-restart.md | 5 + docs/RUNNING.md | 11 +- packages/cli/src/cli-commands.test.ts | 32 +++++- packages/cli/src/cli-commands.ts | 17 ++++ packages/cli/src/cli.ts | 30 ++++-- packages/cli/src/ensure.test.ts | 117 +++++++++++++++++++++- packages/cli/src/ensure.ts | 109 +++++++++++++++++--- packages/web/src/routes/docs/commands.tsx | 1 + packages/web/src/routes/docs/faq.tsx | 6 ++ 9 files changed, 298 insertions(+), 30 deletions(-) create mode 100644 .changeset/reins-restart.md diff --git a/.changeset/reins-restart.md b/.changeset/reins-restart.md new file mode 100644 index 0000000..01ca363 --- /dev/null +++ b/.changeset/reins-restart.md @@ -0,0 +1,5 @@ +--- +"@karnstack/reins": minor +--- + +`reins restart` stops the background daemon and starts a fresh one, waiting for connected browsers to come back. After `npm i -g @karnstack/reins@latest`, the first command now notices a daemon older than the CLI and restarts it on the new version, instead of leaving the old code running. `reins kill` now waits until the daemon has actually exited. diff --git a/docs/RUNNING.md b/docs/RUNNING.md index 677b5cb..e2be937 100644 --- a/docs/RUNNING.md +++ b/docs/RUNNING.md @@ -57,10 +57,11 @@ id (e.g. `b1 (Chrome)`). To debug the daemon itself, run it in the foreground: `pnpm daemon` (Ctrl-C stops it). `pnpm reins kill` stops a background one. -After changing code, `pnpm build`, then `pnpm reins kill` for CLI/daemon -changes and `pnpm reins extension --reload` for extension changes — it -reloads the unpacked extension in place, instead of clicking ⟳ Reload in -`chrome://extensions`. (It also re-stages the sideload copy in +After changing code, `pnpm build`, then `pnpm reins restart` for CLI/daemon +changes (a dev build keeps its version number, so the automatic +restart-on-upgrade doesn't kick in) and `pnpm reins extension --reload` for +extension changes — it reloads the unpacked extension in place, instead of +clicking ⟳ Reload in `chrome://extensions`. (It also re-stages the sideload copy in `~/.reins/extension` from this checkout, so a profile that loads that copy picks up your dev build on its next reload.) @@ -74,7 +75,7 @@ a CLI on PATH, any agent with a shell can use it — no per-agent registration. - **Popover stays "Disconnected":** daemon not running (`reins status` — any tool command starts it), or the dev extension ID isn't allowlisted - (`reins allow `, then `reins kill` — it respawns on demand). + (`reins allow `, then `reins restart`). - **Port collisions:** none, normally — the daemon walks 8765–8774 and the extension + CLI discover it. `REINS_PORT=` pins an exact port for everything (no walking); the popover's Advanced section can pin the diff --git a/packages/cli/src/cli-commands.test.ts b/packages/cli/src/cli-commands.test.ts index 3046d0c..c62cbfa 100644 --- a/packages/cli/src/cli-commands.test.ts +++ b/packages/cli/src/cli-commands.test.ts @@ -9,6 +9,7 @@ import { healthSummary, helpText, logsInfo, + restartText, tabsText, } from "./cli-commands.js"; import { TOOL_COMMANDS } from "./commands.js"; @@ -32,10 +33,19 @@ describe("helpText", () => { for (const name of Object.keys(TOOL_COMMANDS)) { expect(text, name).toContain(name); } - for (const cmd of ["browsers", "status", "allow", "kill", "doctor", "logs", "daemon"]) { + for (const cmd of [ + "browsers", + "status", + "allow", + "restart", + "kill", + "doctor", + "logs", + "daemon", + ]) { expect(text).toContain(cmd); } - for (const gone of ["reins up", "install claude", "--stdio", "restart"]) { + for (const gone of ["reins up", "install claude", "--stdio"]) { expect(text, gone).not.toContain(gone); } }); @@ -49,6 +59,24 @@ describe("helpText", () => { }); }); +describe("restartText", () => { + it("shows the version change and the reconnected browsers", () => { + const s = restartText("0.0.9", 8765, HEALTH); + expect(s).toContain("daemon restarted on 127.0.0.1:8765 (v0.0.9 → v0.1.0)"); + expect(s).toContain("browser: 1 connected (Chrome)"); + }); + + it("drops the arrow when the version is unchanged", () => { + expect(restartText("0.1.0", 8765, HEALTH)).toContain("(v0.1.0)\n"); + }); + + it("says when nothing was running, and when no browser is back yet", () => { + const s = restartText(undefined, 8765, { ...HEALTH, browsers: [] }); + expect(s).toContain("no daemon was running — started v0.1.0"); + expect(s).toContain("none connected yet"); + }); +}); + describe("healthSummary", () => { it("reports a running daemon with its browsers", () => { const s = healthSummary(HEALTH, 8765); diff --git a/packages/cli/src/cli-commands.ts b/packages/cli/src/cli-commands.ts index ada7bfe..44f6543 100644 --- a/packages/cli/src/cli-commands.ts +++ b/packages/cli/src/cli-commands.ts @@ -58,6 +58,7 @@ export function helpText(version: string, tools: Record): s line("extension", "install the extension without the Chrome Web Store (load unpacked)"), line("", " --reload: re-stage, then reload an unpacked (dev/sideload) build"), line("allow ", "allow an unpacked/dev extension to connect"), + line("restart", "restart the background daemon (e.g. after an upgrade)"), line("kill", "stop the background daemon"), line("doctor", "run diagnostic checks"), line("logs", "show the daemon log location and recent lines"), @@ -69,6 +70,22 @@ export function helpText(version: string, tools: Record): s ].join("\n"); } +/** Result line(s) for `reins restart`. `previous` is the old daemon's version, if one ran. */ +export function restartText(previous: string | undefined, port: number, h: DaemonHealth): string { + const head = + previous === undefined + ? `no daemon was running — started v${h.version} on 127.0.0.1:${port}` + : previous === h.version + ? `daemon restarted on 127.0.0.1:${port} (v${h.version})` + : `daemon restarted on 127.0.0.1:${port} (v${previous} → v${h.version})`; + const names = [...new Set(h.browsers.map((b) => b.browser))].join(", "); + const browsers = + h.browsers.length === 0 + ? "browser: none connected yet — the extension reconnects within ~10s (`reins status`)" + : `browser: ${h.browsers.length} connected (${names})`; + return `${head}\n${browsers}`; +} + /** Human status lines for `reins status`. */ export function healthSummary(h: DaemonHealth | undefined, port: number): string { if (!h) { diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index 60d71a4..68d2f32 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -3,10 +3,17 @@ import { mkdirSync, writeFileSync } from "node:fs"; import { homedir } from "node:os"; import { join, resolve } from "node:path"; import { parseArgs, UsageError } from "./args.js"; -import { browsersText, doctorReport, healthSummary, helpText, logsInfo } from "./cli-commands.js"; +import { + browsersText, + doctorReport, + healthSummary, + helpText, + logsInfo, + restartText, +} from "./cli-commands.js"; import { TOOL_COMMANDS, type ToolCommand } from "./commands.js"; import { loadOrCreateConfig } from "./config.js"; -import { ensureDaemon, findDaemon, waitForBrowsers } from "./ensure.js"; +import { ensureDaemon, findDaemon, restartDaemon, stopDaemon, waitForBrowsers } from "./ensure.js"; import { logsDir } from "./log.js"; import { packageVersion } from "./version.js"; @@ -92,14 +99,23 @@ async function main(): Promise { console.log("no reins daemon running."); break; } - await fetch(`http://127.0.0.1:${found.port}/shutdown`, { - method: "POST", - signal: AbortSignal.timeout(3000), - }); + await stopDaemon(found.port); console.log(`daemon on port ${found.port} stopped.`); break; } + case "restart": { + const { previous, current } = await restartDaemon(loadOrCreateConfig()); + // Browsers that were connected come back on their own; wait for one so + // the next command doesn't race the extension's reconnect. + let health = current.health; + if (previous && previous.health.browsers.length > 0) { + health = await waitForBrowsers(current.port).catch(() => current.health); + } + console.log(restartText(previous?.health.version, current.port, health)); + break; + } + case "extension": { const a = parseArgs(rest, { booleans: ["reload"] }); const { bundledExtensionDir, extractExtension, sideloadInstructions } = await import( @@ -161,7 +177,7 @@ async function main(): Promise { if (!id) throw new UsageError("usage: reins allow "); const { allowExtension } = await import("./allowlist.js"); allowExtension(loadOrCreateConfig().dir, id); - console.log(`allowed ${id} — restart the daemon (\`reins kill\`; it respawns on demand).`); + console.log(`allowed ${id} — restart the daemon (\`reins restart\`) to pick it up.`); break; } diff --git a/packages/cli/src/ensure.test.ts b/packages/cli/src/ensure.test.ts index d6ca50f..e4f805e 100644 --- a/packages/cli/src/ensure.test.ts +++ b/packages/cli/src/ensure.test.ts @@ -1,11 +1,19 @@ import { describe, expect, it, vi } from "vitest"; import type { DaemonHealth } from "./cli-commands.js"; -import { ensureDaemon, type FoundDaemon, waitForBrowsers } from "./ensure.js"; +import { + ensureDaemon, + type FoundDaemon, + isOlderVersion, + restartDaemon, + stopDaemon, + waitForBrowsers, +} from "./ensure.js"; import { lowerPortRival } from "./serve.js"; +import { packageVersion } from "./version.js"; -const health = (browsers = 0): DaemonHealth => ({ +const health = (browsers = 0, version = packageVersion()): DaemonHealth => ({ ok: true, - version: "0.0.0", + version, paired: browsers > 0, browsers: Array.from({ length: browsers }, (_, i) => ({ id: `b${i + 1}`, @@ -42,6 +50,109 @@ describe("ensureDaemon", () => { }); }); +describe("ensureDaemon version check", () => { + it("restarts a daemon older than the CLI", async () => { + const stop = vi.fn(async () => {}); + const spawn = vi.fn(); + const notice = vi.fn(); + let calls = 0; + const find = async (): Promise => + ++calls === 1 + ? { port: 8765, health: health(1, "0.4.0") } + : { port: 8765, health: health(0, "0.5.0") }; + const result = await ensureDaemon(cfg, { + version: "0.5.0", + find, + stop, + spawn, + notice, + pollMs: 1, + }); + expect(stop).toHaveBeenCalledWith(8765); + expect(spawn).toHaveBeenCalledOnce(); + expect(result).toMatchObject({ spawned: true, health: { version: "0.5.0" } }); + expect(notice).toHaveBeenCalledWith("reins: daemon runs v0.4.0, CLI is v0.5.0 — restarting it"); + }); + + it("leaves a newer daemon alone (two installed CLIs must not bounce it)", async () => { + const stop = vi.fn(async () => {}); + const found: FoundDaemon = { port: 8765, health: health(1, "0.6.0") }; + const result = await ensureDaemon(cfg, { version: "0.5.0", find: async () => found, stop }); + expect(result.spawned).toBe(false); + expect(stop).not.toHaveBeenCalled(); + }); +}); + +describe("isOlderVersion", () => { + it("compares major.minor.patch numerically", () => { + expect(isOlderVersion("0.4.0", "0.5.0")).toBe(true); + expect(isOlderVersion("0.9.0", "0.10.0")).toBe(true); + expect(isOlderVersion("1.0.0", "0.10.0")).toBe(false); + expect(isOlderVersion("0.5.0", "0.5.0")).toBe(false); + expect(isOlderVersion("0.5.0-next.1", "0.5.0")).toBe(false); + }); +}); + +describe("stopDaemon", () => { + it("requests shutdown, then waits until the port stops answering", async () => { + const shutdown = vi.fn(async () => {}); + let calls = 0; + const probe = async (port: number) => (++calls < 3 ? { port, health: health() } : undefined); + await stopDaemon(8765, { shutdown, probe, pollMs: 1 }); + expect(shutdown).toHaveBeenCalledWith(8765); + expect(calls).toBe(3); + }); + + it("errors when the daemon never goes away", async () => { + const probe = async (port: number) => ({ port, health: health() }); + await expect( + stopDaemon(8765, { shutdown: async () => {}, probe, pollMs: 1, timeoutMs: 10 }), + ).rejects.toThrow("did not stop"); + }); +}); + +describe("restartDaemon", () => { + it("stops the live daemon, spawns a fresh one, and reports the old one", async () => { + const order: string[] = []; + let stopped = false; + const find = async (): Promise => + stopped + ? order.includes("spawn") + ? { port: 8765, health: health(0) } + : undefined + : { port: 8766, health: health(1, "0.4.0") }; + const result = await restartDaemon(cfg, { + find, + stop: async (port) => { + order.push(`stop:${port}`); + stopped = true; + }, + spawn: () => order.push("spawn"), + pollMs: 1, + }); + expect(order).toEqual(["stop:8766", "spawn"]); + expect(result.previous?.health.version).toBe("0.4.0"); + expect(result.current).toMatchObject({ port: 8765, spawned: true }); + }); + + it("just spawns when nothing was running", async () => { + const stop = vi.fn(async () => {}); + let spawned = false; + const find = async () => (spawned ? { port: 8765, health: health() } : undefined); + const result = await restartDaemon(cfg, { + find, + stop, + spawn: () => { + spawned = true; + }, + pollMs: 1, + }); + expect(stop).not.toHaveBeenCalled(); + expect(result.previous).toBeUndefined(); + expect(result.current.spawned).toBe(true); + }); +}); + describe("waitForBrowsers", () => { it("resolves as soon as a browser appears", async () => { let calls = 0; diff --git a/packages/cli/src/ensure.ts b/packages/cli/src/ensure.ts index c4ccfde..6904897 100644 --- a/packages/cli/src/ensure.ts +++ b/packages/cli/src/ensure.ts @@ -4,6 +4,7 @@ import { fileURLToPath } from "node:url"; import type { BrowserInfo } from "@reins/protocol"; import type { DaemonHealth } from "./cli-commands.js"; import { candidatePorts, type ReinsConfig } from "./config.js"; +import { packageVersion } from "./version.js"; export interface FoundDaemon { port: number; @@ -46,28 +47,67 @@ export function spawnDaemon(): void { const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); -export interface EnsuredDaemon extends FoundDaemon { - /** True when this call had to spawn the daemon (extension may still be reconnecting). */ - spawned: boolean; +/** True when semver `a` is older than `b` (major.minor.patch; pre-release tags ignored). */ +export function isOlderVersion(a: string, b: string): boolean { + const parts = (v: string) => + v + .split("-")[0] + ?.split(".") + .map((n) => Number(n) || 0) ?? []; + const [pa, pb] = [parts(a), parts(b)]; + for (let i = 0; i < 3; i++) { + const d = (pa[i] ?? 0) - (pb[i] ?? 0); + if (d !== 0) return d < 0; + } + return false; +} + +function requestShutdown(port: number): Promise { + return fetch(`http://127.0.0.1:${port}/shutdown`, { + method: "POST", + signal: AbortSignal.timeout(3000), + }); } /** - * Make sure a daemon is running: reuse the live one, else spawn it detached - * and wait for /health. + * Ask a daemon to shut down and wait until its port stops answering, so a + * follow-up spawn can't race the dying process for the port. */ -export async function ensureDaemon( - cfg: ReinsConfig, +export async function stopDaemon( + port: number, opts: { - spawn?: () => void; - find?: (cfg: ReinsConfig) => Promise; + shutdown?: (port: number) => Promise; + probe?: (port: number) => Promise; pollMs?: number; timeoutMs?: number; } = {}, -): Promise { - const find = opts.find ?? findDaemon; - const existing = await find(cfg); - if (existing) return { ...existing, spawned: false }; +): Promise { + const probe = opts.probe ?? probeHealth; + await (opts.shutdown ?? requestShutdown)(port); + const pollMs = opts.pollMs ?? 100; + const deadline = Date.now() + (opts.timeoutMs ?? 4000); + while (Date.now() < deadline) { + if (!(await probe(port))) return; + await sleep(pollMs); + } + throw new Error(`reins daemon on port ${port} did not stop — check \`reins logs\``); +} +export interface EnsuredDaemon extends FoundDaemon { + /** True when this call had to spawn the daemon (extension may still be reconnecting). */ + spawned: boolean; +} + +interface SpawnOpts { + spawn?: () => void; + find?: (cfg: ReinsConfig) => Promise; + pollMs?: number; + timeoutMs?: number; +} + +/** Spawn a detached daemon and wait for /health. */ +async function spawnAndWait(cfg: ReinsConfig, opts: SpawnOpts): Promise { + const find = opts.find ?? findDaemon; (opts.spawn ?? spawnDaemon)(); const pollMs = opts.pollMs ?? 150; const deadline = Date.now() + (opts.timeoutMs ?? 4000); @@ -79,6 +119,49 @@ export async function ensureDaemon( throw new Error("reins daemon failed to start — check `reins logs`"); } +/** + * Make sure a daemon is running: reuse the live one, else spawn it detached + * and wait for /health. A live daemon older than this CLI (left over from + * before an `npm i -g` upgrade) is restarted so it runs the installed code. + * Only older: a newer daemon is left alone, so two installed CLI versions + * can't bounce it back and forth. + */ +export async function ensureDaemon( + cfg: ReinsConfig, + opts: SpawnOpts & { + /** This CLI's version (default: package.json). */ + version?: string; + stop?: (port: number) => Promise; + notice?: (msg: string) => void; + } = {}, +): Promise { + const find = opts.find ?? findDaemon; + const existing = await find(cfg); + if (existing) { + const version = opts.version ?? packageVersion(); + if (!isOlderVersion(existing.health.version, version)) return { ...existing, spawned: false }; + (opts.notice ?? console.error)( + `reins: daemon runs v${existing.health.version}, CLI is v${version} — restarting it`, + ); + await (opts.stop ?? stopDaemon)(existing.port); + } + return spawnAndWait(cfg, opts); +} + +/** + * `reins restart`: stop the live daemon (if any) and spawn a fresh one. + * `previous` is what was running before, for the old → new version line. + */ +export async function restartDaemon( + cfg: ReinsConfig, + opts: SpawnOpts & { stop?: (port: number) => Promise } = {}, +): Promise<{ previous?: FoundDaemon; current: EnsuredDaemon }> { + const previous = await (opts.find ?? findDaemon)(cfg); + if (previous) await (opts.stop ?? stopDaemon)(previous.port); + const current = await spawnAndWait(cfg, opts); + return previous ? { previous, current } : { current }; +} + /** * Wait for at least one browser to appear on the daemon. Used after a fresh * spawn: the extension's reconnect backoff caps at 10s, so give it 15s. diff --git a/packages/web/src/routes/docs/commands.tsx b/packages/web/src/routes/docs/commands.tsx index 24559f2..490b8a9 100644 --- a/packages/web/src/routes/docs/commands.tsx +++ b/packages/web/src/routes/docs/commands.tsx @@ -161,6 +161,7 @@ const GROUPS: Array<{ id: string; title: string; intro?: string; rows: [string, ], ["reins allow ", "Allow an unpacked or dev extension to connect."], ["reins audit [--denied]", "Render the audit trail, or only what policy blocked."], + ["reins restart", "Restart the background daemon, e.g. after changing config."], ["reins kill", "Stop the background daemon."], ["reins doctor", "Run diagnostic checks."], ["reins logs", "Show the daemon log location and recent lines."], diff --git a/packages/web/src/routes/docs/faq.tsx b/packages/web/src/routes/docs/faq.tsx index 21917bd..8a247be 100644 --- a/packages/web/src/routes/docs/faq.tsx +++ b/packages/web/src/routes/docs/faq.tsx @@ -33,6 +33,12 @@ const FAQS = [ answer: "No. Any reins command starts the daemon on demand, and the extension finds it on its own through localhost port discovery. reins kill stops it; reins status shows what is connected.", }, + { + id: "update", + question: "How do I update reins?", + answer: + "Run npm i -g @karnstack/reins@latest. The next reins command notices the running daemon is older than the CLI and restarts it on the new version. The Chrome Web Store extension updates itself.", + }, { id: "mcp", question: "How is this different from an MCP browser server?", From d98bb8c918558e32fa29204ab6b051024f38c7fc Mon Sep 17 00:00:00 2001 From: Karn Date: Sun, 27 Sep 2026 02:12:23 +0530 Subject: [PATCH 2/3] fix: harden reins restart against parallel CLIs and stubborn daemons Review follow-ups for the restart / restart-on-upgrade work: - stopDaemon treats a rejected /shutdown as "already going"; the /health poll decides. - Stop and spawn run under a best-effort lockfile (~/.reins/daemon.lock, stale by dead pid or age, never blocks longer than 10s), and the decision is re-made once it's held. A parallel CLI can no longer /shutdown the daemon another one just started, and parallel spawns collapse into one. A daemon found under the lock counts as fresh, so callers wait for the extension to reconnect. - Auto-restart that can't stop the old daemon warns and uses it as-is instead of failing every command; explicit restart/kill still error. - "0.0.0" (unreadable package.json) never triggers a restart. - findDaemon prefers the lowest live port, the one lowerPortRival keeps. - reins restart reports each case truthfully and exits 1 when browsers that were connected don't come back within 15s. - reins doctor flags a daemon/CLI version mismatch; reins status hints when the daemon is older. Docs and changeset match what auto-restarts. Co-Authored-By: Claude Opus 5.5 --- .changeset/reins-restart.md | 2 +- docs/RUNNING.md | 3 +- packages/cli/src/cli-commands.test.ts | 155 +++++++++++++-- packages/cli/src/cli-commands.ts | 104 ++++++++-- packages/cli/src/cli.ts | 31 +-- packages/cli/src/ensure.test.ts | 232 ++++++++++++++++++++-- packages/cli/src/ensure.ts | 104 +++++++--- packages/cli/src/lock.test.ts | 163 +++++++++++++++ packages/cli/src/lock.ts | 124 ++++++++++++ packages/web/src/routes/docs/commands.tsx | 2 +- packages/web/src/routes/docs/faq.tsx | 2 +- 11 files changed, 826 insertions(+), 96 deletions(-) create mode 100644 packages/cli/src/lock.test.ts create mode 100644 packages/cli/src/lock.ts diff --git a/.changeset/reins-restart.md b/.changeset/reins-restart.md index 01ca363..d062ce4 100644 --- a/.changeset/reins-restart.md +++ b/.changeset/reins-restart.md @@ -2,4 +2,4 @@ "@karnstack/reins": minor --- -`reins restart` stops the background daemon and starts a fresh one, waiting for connected browsers to come back. After `npm i -g @karnstack/reins@latest`, the first command now notices a daemon older than the CLI and restarts it on the new version, instead of leaving the old code running. `reins kill` now waits until the daemon has actually exited. +`reins restart` stops the background daemon and starts a fresh one, waiting for previously connected browsers to come back (and exiting 1 if they don't). After `npm i -g @karnstack/reins@latest`, the next tool command (e.g. `reins tabs`) notices a daemon older than the CLI and restarts it on the new version, instead of leaving the old code running; `reins status` and `reins doctor` point out the mismatch. `reins kill` now waits until the daemon has actually exited. diff --git a/docs/RUNNING.md b/docs/RUNNING.md index e2be937..1a649c5 100644 --- a/docs/RUNNING.md +++ b/docs/RUNNING.md @@ -82,5 +82,6 @@ a CLI on PATH, any agent with a shell can use it — no per-agent registration. extension too. - **`no browser connected`:** the extension isn't installed/allowed in any open browser, or it's still reconnecting (up to ~10 s after a daemon - restart). `reins doctor` shows what's reachable. + restart — `reins restart` waits 15 s for it). `reins doctor` shows what's + reachable, including a daemon that's older than the CLI. - **Anything else:** `reins logs` tails the newest daemon log. diff --git a/packages/cli/src/cli-commands.test.ts b/packages/cli/src/cli-commands.test.ts index c62cbfa..cafd043 100644 --- a/packages/cli/src/cli-commands.test.ts +++ b/packages/cli/src/cli-commands.test.ts @@ -9,7 +9,8 @@ import { healthSummary, helpText, logsInfo, - restartText, + RESTART_WAIT_MS, + runRestart, tabsText, } from "./cli-commands.js"; import { TOOL_COMMANDS } from "./commands.js"; @@ -57,23 +58,99 @@ describe("helpText", () => { it("help lists the audit command", () => { expect(helpText("1.2.3", TOOL_COMMANDS)).toContain("audit"); }); + + it("describes restart as the thing to run after an upgrade or `reins allow`", () => { + expect(helpText("1.2.3", TOOL_COMMANDS)).toContain( + "restart the background daemon (after an upgrade or `reins allow`)", + ); + }); }); -describe("restartText", () => { - it("shows the version change and the reconnected browsers", () => { - const s = restartText("0.0.9", 8765, HEALTH); - expect(s).toContain("daemon restarted on 127.0.0.1:8765 (v0.0.9 → v0.1.0)"); - expect(s).toContain("browser: 1 connected (Chrome)"); +describe("runRestart", () => { + const NONE = { ...HEALTH, browsers: [] }; + const daemon = (port: number, health = HEALTH) => ({ port, health }); + + /** Deps whose wait resolves (or times out) and whose probe answers `after`. */ + function deps(opts: { + previous?: { port: number; health: typeof HEALTH }; + after: typeof HEALTH; + waitTimesOut?: boolean; + }) { + const calls = { waited: 0, probed: 0 }; + return { + calls, + restart: async () => ({ + ...(opts.previous ? { previous: opts.previous } : {}), + current: daemon(8765, NONE), + }), + waitForBrowsers: async () => { + calls.waited++; + if (opts.waitTimesOut) throw new Error("no browser connected"); + return opts.after; + }, + probe: async (port: number) => { + calls.probed++; + return daemon(port, opts.after); + }, + }; + } + + it("reports the browsers that came back, with the version change", async () => { + const d = deps({ previous: daemon(8765, { ...HEALTH, version: "0.0.9" }), after: HEALTH }); + const { text, ok } = await runRestart(d); + expect(ok).toBe(true); + expect(text).toBe( + "daemon restarted on 127.0.0.1:8765 (v0.0.9 → v0.1.0)\nbrowser: 1 connected (Chrome)", + ); + expect(d.calls.waited).toBe(1); + }); + + it("drops the arrow when the version is unchanged", async () => { + const { text } = await runRestart(deps({ previous: daemon(8765), after: HEALTH })); + expect(text).toContain("(v0.1.0)\n"); + }); + + it("fails plainly when browsers were connected before but none came back", async () => { + const d = deps({ previous: daemon(8765), after: NONE, waitTimesOut: true }); + const { text, ok } = await runRestart(d); + expect(ok).toBe(false); + expect(text).toContain( + `browser: none reconnected within ${RESTART_WAIT_MS / 1000}s — check the extension (\`reins status\`)`, + ); + expect(text).not.toContain("~10s"); + }); + + it("does not wait, and does not imply a reconnect, when nothing was connected before", async () => { + const d = deps({ previous: daemon(8765, NONE), after: NONE }); + const { text, ok } = await runRestart(d); + expect(ok).toBe(true); + expect(text).toContain("browser: none connected (none were before the restart)"); + expect(d.calls.waited).toBe(0); }); - it("drops the arrow when the version is unchanged", () => { - expect(restartText("0.1.0", 8765, HEALTH)).toContain("(v0.1.0)\n"); + it("says when no daemon was running at all", async () => { + const { text, ok } = await runRestart(deps({ after: NONE })); + expect(ok).toBe(true); + expect(text).toContain("no daemon was running — started v0.1.0 on 127.0.0.1:8765"); + expect(text).toContain("none were before the restart"); }); - it("says when nothing was running, and when no browser is back yet", () => { - const s = restartText(undefined, 8765, { ...HEALTH, browsers: [] }); - expect(s).toContain("no daemon was running — started v0.1.0"); - expect(s).toContain("none connected yet"); + it("reports fresh health rather than the spawn-time snapshot", async () => { + // Spawn snapshot has no browsers; the re-probe after the wait does. + const d = deps({ previous: daemon(8765), after: HEALTH }); + const { text } = await runRestart(d); + expect(d.calls.probed).toBe(1); + expect(text).toContain("browser: 1 connected (Chrome)"); + }); + + it("lets a restart failure through untouched", async () => { + const d = { + ...deps({ after: NONE }), + restart: async () => { + throw new Error("did not stop"); + }, + }; + await expect(runRestart(d)).rejects.toThrow("did not stop"); }); }); @@ -90,6 +167,22 @@ describe("healthSummary", () => { expect(s).toContain("not running"); expect(s).toContain("on demand"); }); + + it("hints at `reins restart` when the daemon is older than the CLI", () => { + const s = healthSummary(HEALTH, 8765, "0.2.0"); + expect(s).toContain("older than the CLI (v0.2.0) — `reins restart`"); + expect(s.split("\n").filter((l) => l.includes("older than")).length).toBe(1); + }); + + it("stays quiet when versions match, the daemon is newer, or either is unknown", () => { + expect(healthSummary(HEALTH, 8765, "0.1.0")).not.toContain("older than"); + expect(healthSummary(HEALTH, 8765, "0.0.9")).not.toContain("older than"); + expect(healthSummary(HEALTH, 8765, "0.0.0")).not.toContain("older than"); + expect(healthSummary({ ...HEALTH, version: "0.0.0" }, 8765, "0.2.0")).not.toContain( + "older than", + ); + expect(healthSummary(HEALTH, 8765)).not.toContain("older than"); + }); }); describe("browsersText / tabsText", () => { @@ -173,17 +266,45 @@ describe("groupsText", () => { }); describe("doctorReport", () => { + const check = (r: ReturnType, name: string) => + r.checks.find((c) => c.name === name); + it("passes all checks with a healthy daemon and a browser", () => { - const report = doctorReport(cfg(), HEALTH); + const report = doctorReport(cfg(), HEALTH, "0.1.0"); expect(report.ok).toBe(true); - expect(report.checks.find((c) => c.name === "daemon")?.ok).toBe(true); - expect(report.checks.find((c) => c.name === "browser")?.ok).toBe(true); + expect(check(report, "daemon")?.ok).toBe(true); + expect(check(report, "browser")?.ok).toBe(true); + expect(check(report, "version")).toEqual({ + name: "version", + ok: true, + detail: "daemon and CLI both v0.1.0", + }); }); it("fails the daemon and browser checks when nothing is running", () => { - const report = doctorReport(cfg(), undefined); + const report = doctorReport(cfg(), undefined, "0.1.0"); expect(report.ok).toBe(false); - expect(report.checks.find((c) => c.name === "daemon")?.ok).toBe(false); + expect(check(report, "daemon")?.ok).toBe(false); + // Nothing to compare against — no version line. + expect(check(report, "version")).toBeUndefined(); + }); + + it("fails the version check, with the fix, when the daemon predates the CLI", () => { + const report = doctorReport(cfg(), { ...HEALTH, version: "0.4.0" }, "0.5.0"); + expect(report.ok).toBe(false); + expect(check(report, "version")).toEqual({ + name: "version", + ok: false, + detail: "daemon v0.4.0, CLI v0.5.0 — run `reins restart`", + }); + }); + + it("skips the version check when either side is the unknown 0.0.0", () => { + expect( + check(doctorReport(cfg(), { ...HEALTH, version: "0.0.0" }, "0.5.0"), "version"), + ).toBeUndefined(); + expect(check(doctorReport(cfg(), HEALTH, "0.0.0"), "version")).toBeUndefined(); + expect(doctorReport(cfg(), HEALTH, "0.0.0").ok).toBe(true); }); }); diff --git a/packages/cli/src/cli-commands.ts b/packages/cli/src/cli-commands.ts index 44f6543..23cc2e1 100644 --- a/packages/cli/src/cli-commands.ts +++ b/packages/cli/src/cli-commands.ts @@ -3,6 +3,7 @@ import { join } from "node:path"; import type { BrowserInfo, SkippedBrowser, Tab, TabGroup } from "@reins/protocol"; import type { ToolCommand } from "./commands.js"; import type { ReinsConfig } from "./config.js"; +import { wantsRestart } from "./ensure.js"; export interface DaemonHealth { ok: boolean; @@ -58,7 +59,7 @@ export function helpText(version: string, tools: Record): s line("extension", "install the extension without the Chrome Web Store (load unpacked)"), line("", " --reload: re-stage, then reload an unpacked (dev/sideload) build"), line("allow ", "allow an unpacked/dev extension to connect"), - line("restart", "restart the background daemon (e.g. after an upgrade)"), + line("restart", "restart the background daemon (after an upgrade or `reins allow`)"), line("kill", "stop the background daemon"), line("doctor", "run diagnostic checks"), line("logs", "show the daemon log location and recent lines"), @@ -70,24 +71,70 @@ export function helpText(version: string, tools: Record): s ].join("\n"); } -/** Result line(s) for `reins restart`. `previous` is the old daemon's version, if one ran. */ -export function restartText(previous: string | undefined, port: number, h: DaemonHealth): string { +/** packageVersion() falls back to "0.0.0" when package.json is unreadable — not a real version. */ +const knownVersion = (v: string) => v !== "0.0.0"; + +/** A live daemon: its port and last /health reply. */ +export interface Daemon { + port: number; + health: DaemonHealth; +} + +/** How long `reins restart` waits for previously connected browsers to come + * back (the extension's reconnect backoff caps at 10s). */ +export const RESTART_WAIT_MS = 15_000; + +export interface RestartDeps { + /** Stop the live daemon (if any) and spawn a fresh one. */ + restart(): Promise<{ previous?: Daemon; current: Daemon }>; + /** Resolve once a browser is on `port`; reject when RESTART_WAIT_MS runs out. */ + waitForBrowsers(port: number): Promise; + /** Fresh /health, for the final report. */ + probe(port: number): Promise; +} + +/** + * `reins restart`: restart the daemon, then report what came back. Browsers + * that were connected reconnect on their own, so wait for one when there was + * one — the next command would otherwise race the extension. `ok` is false + * when they don't return in time. + */ +export async function runRestart(deps: RestartDeps): Promise<{ text: string; ok: boolean }> { + const { previous, current } = await deps.restart(); + const hadBrowsers = (previous?.health.browsers.length ?? 0) > 0; + // Only the wait's timeout is swallowed — the report below says so. + if (hadBrowsers) await deps.waitForBrowsers(current.port).catch(() => undefined); + // Re-probe rather than trust the spawn-time snapshot, which predates any reconnect. + const h = (await deps.probe(current.port))?.health ?? current.health; + + const prev = previous?.health.version; const head = - previous === undefined - ? `no daemon was running — started v${h.version} on 127.0.0.1:${port}` - : previous === h.version - ? `daemon restarted on 127.0.0.1:${port} (v${h.version})` - : `daemon restarted on 127.0.0.1:${port} (v${previous} → v${h.version})`; - const names = [...new Set(h.browsers.map((b) => b.browser))].join(", "); - const browsers = - h.browsers.length === 0 - ? "browser: none connected yet — the extension reconnects within ~10s (`reins status`)" - : `browser: ${h.browsers.length} connected (${names})`; - return `${head}\n${browsers}`; + prev === undefined + ? `no daemon was running — started v${h.version} on 127.0.0.1:${current.port}` + : prev === h.version + ? `daemon restarted on 127.0.0.1:${current.port} (v${h.version})` + : `daemon restarted on 127.0.0.1:${current.port} (v${prev} → v${h.version})`; + + if (h.browsers.length > 0) { + const names = [...new Set(h.browsers.map((b) => b.browser))].join(", "); + return { text: `${head}\nbrowser: ${h.browsers.length} connected (${names})`, ok: true }; + } + if (hadBrowsers) { + const secs = Math.round(RESTART_WAIT_MS / 1000); + return { + text: `${head}\nbrowser: none reconnected within ${secs}s — check the extension (\`reins status\`)`, + ok: false, + }; + } + return { text: `${head}\nbrowser: none connected (none were before the restart)`, ok: true }; } -/** Human status lines for `reins status`. */ -export function healthSummary(h: DaemonHealth | undefined, port: number): string { +/** Human status lines for `reins status`. `cliVersion` adds a hint when the daemon is older. */ +export function healthSummary( + h: DaemonHealth | undefined, + port: number, + cliVersion?: string, +): string { if (!h) { return [ `daemon : not running (no reins daemon answered on the candidate ports around ${port})`, @@ -95,6 +142,11 @@ export function healthSummary(h: DaemonHealth | undefined, port: number): string ].join("\n"); } const lines = [`daemon : running on 127.0.0.1:${port} (v${h.version})`]; + if (cliVersion !== undefined && wantsRestart(h.version, cliVersion)) { + lines.push( + ` older than the CLI (v${cliVersion}) — \`reins restart\`, or the next tool command restarts it`, + ); + } if (h.browsers.length === 0) { lines.push( "browser: none connected — install the reins extension (or `reins allow ` for dev builds)", @@ -152,8 +204,12 @@ export interface DoctorReport { ok: boolean; } -/** Diagnostic checks for `reins doctor`. */ -export function doctorReport(cfg: ReinsConfig, health?: DaemonHealth): DoctorReport { +/** Diagnostic checks for `reins doctor`. `cliVersion` is compared against the daemon's. */ +export function doctorReport( + cfg: ReinsConfig, + health: DaemonHealth | undefined, + cliVersion: string, +): DoctorReport { const checks = [ { name: "config-dir", ok: cfg.dir.length > 0, detail: cfg.dir }, { name: "port", ok: Number.isInteger(cfg.port) && cfg.port > 0, detail: String(cfg.port) }, @@ -165,6 +221,18 @@ export function doctorReport(cfg: ReinsConfig, health?: DaemonHealth): DoctorRep ? `running (v${health.version})` : "not running — starts on demand (`reins tabs`), or run `reins daemon`", }, + // A daemon left over from before an upgrade runs the old code until restarted. + ...(health && knownVersion(health.version) && knownVersion(cliVersion) + ? [ + health.version === cliVersion + ? { name: "version", ok: true, detail: `daemon and CLI both v${cliVersion}` } + : { + name: "version", + ok: false, + detail: `daemon v${health.version}, CLI v${cliVersion} — run \`reins restart\``, + }, + ] + : []), { name: "browser", ok: (health?.browsers.length ?? 0) > 0, diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index 68d2f32..8bb5793 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -9,11 +9,19 @@ import { healthSummary, helpText, logsInfo, - restartText, + RESTART_WAIT_MS, + runRestart, } from "./cli-commands.js"; import { TOOL_COMMANDS, type ToolCommand } from "./commands.js"; import { loadOrCreateConfig } from "./config.js"; -import { ensureDaemon, findDaemon, restartDaemon, stopDaemon, waitForBrowsers } from "./ensure.js"; +import { + ensureDaemon, + findDaemon, + probeHealth, + restartDaemon, + stopDaemon, + waitForBrowsers, +} from "./ensure.js"; import { logsDir } from "./log.js"; import { packageVersion } from "./version.js"; @@ -105,14 +113,13 @@ async function main(): Promise { } case "restart": { - const { previous, current } = await restartDaemon(loadOrCreateConfig()); - // Browsers that were connected come back on their own; wait for one so - // the next command doesn't race the extension's reconnect. - let health = current.health; - if (previous && previous.health.browsers.length > 0) { - health = await waitForBrowsers(current.port).catch(() => current.health); - } - console.log(restartText(previous?.health.version, current.port, health)); + const { text, ok } = await runRestart({ + restart: () => restartDaemon(loadOrCreateConfig()), + waitForBrowsers: (port) => waitForBrowsers(port, { timeoutMs: RESTART_WAIT_MS }), + probe: probeHealth, + }); + console.log(text); + if (!ok) process.exitCode = 1; break; } @@ -214,7 +221,7 @@ async function main(): Promise { case "status": { const cfg = loadOrCreateConfig(); const found = await findDaemon(cfg); - console.log(healthSummary(found?.health, found?.port ?? cfg.port)); + console.log(healthSummary(found?.health, found?.port ?? cfg.port, packageVersion())); console.log(`logs : ${logsDir()}`); break; } @@ -222,7 +229,7 @@ async function main(): Promise { case "doctor": { const cfg = loadOrCreateConfig(); const found = await findDaemon(cfg); - const report = doctorReport(cfg, found?.health); + const report = doctorReport(cfg, found?.health, packageVersion()); for (const c of report.checks) { console.log(`${c.ok ? "✓" : "✗"} ${c.name}: ${c.detail}`); } diff --git a/packages/cli/src/ensure.test.ts b/packages/cli/src/ensure.test.ts index e4f805e..32a8864 100644 --- a/packages/cli/src/ensure.test.ts +++ b/packages/cli/src/ensure.test.ts @@ -1,13 +1,19 @@ -import { describe, expect, it, vi } from "vitest"; +import { existsSync, mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterAll, describe, expect, it, vi } from "vitest"; import type { DaemonHealth } from "./cli-commands.js"; import { ensureDaemon, type FoundDaemon, + findDaemon, isOlderVersion, restartDaemon, stopDaemon, waitForBrowsers, + wantsRestart, } from "./ensure.js"; +import { lockPath } from "./lock.js"; import { lowerPortRival } from "./serve.js"; import { packageVersion } from "./version.js"; @@ -22,54 +28,100 @@ const health = (browsers = 0, version = packageVersion()): DaemonHealth => ({ })), }); -const cfg = { dir: "/tmp/nowhere", port: 8765, exact: false }; +// A real dir so the default (uninjected) lock has somewhere to live. +const cfg = { dir: mkdtempSync(join(tmpdir(), "reins-ensure-")), port: 8765, exact: false }; +afterAll(() => rmSync(cfg.dir, { recursive: true, force: true })); + +/** Lock stub that records whether stop/spawn ran inside it. */ +function fakeLock() { + let held = false; + const events: string[] = []; + const lock = async (fn: () => Promise): Promise => { + held = true; + events.push("lock"); + try { + return await fn(); + } finally { + held = false; + events.push("unlock"); + } + }; + return { lock, events, inside: (what: string) => events.push(held ? what : `${what}:UNLOCKED`) }; +} describe("ensureDaemon", () => { - it("reuses a live daemon without spawning", async () => { + it("reuses a live daemon without spawning — and without taking the lock", async () => { const spawn = vi.fn(); + const { lock, events } = fakeLock(); const found: FoundDaemon = { port: 8766, health: health(1) }; - const result = await ensureDaemon(cfg, { spawn, find: async () => found }); + const result = await ensureDaemon(cfg, { spawn, lock, find: async () => found }); expect(result).toEqual({ ...found, spawned: false }); expect(spawn).not.toHaveBeenCalled(); + expect(events).toEqual([]); }); - it("spawns and polls until the daemon answers", async () => { - const spawn = vi.fn(); + it("spawns under the lock and polls until the daemon answers", async () => { + const { lock, events, inside } = fakeLock(); let calls = 0; - const find = async () => (++calls >= 3 ? { port: 8765, health: health() } : undefined); - const result = await ensureDaemon(cfg, { spawn, find, pollMs: 1 }); + const find = async () => (++calls >= 4 ? { port: 8765, health: health() } : undefined); + const result = await ensureDaemon(cfg, { spawn: () => inside("spawn"), lock, find, pollMs: 1 }); expect(result.spawned).toBe(true); expect(result.port).toBe(8765); - expect(spawn).toHaveBeenCalledOnce(); + expect(events).toEqual(["lock", "spawn", "unlock"]); + }); + + it("uses the real lockfile by default and releases it", async () => { + let spawned = false; + const find = async () => (spawned ? { port: 8765, health: health() } : undefined); + const spawn = () => { + // The lock is held while we spawn. + expect(existsSync(lockPath(cfg.dir))).toBe(true); + spawned = true; + }; + const result = await ensureDaemon(cfg, { spawn, find, pollMs: 1 }); + expect(result.spawned).toBe(true); + expect(existsSync(lockPath(cfg.dir))).toBe(false); + }); + + it("does not spawn when the re-find under the lock shows a daemon (parallel CLI won)", async () => { + const spawn = vi.fn(); + let calls = 0; + // No daemon on the lock-free look, but one by the time the lock is held. + const find = async () => (++calls >= 2 ? { port: 8765, health: health() } : undefined); + const result = await ensureDaemon(cfg, { spawn, lock: (fn) => fn(), find }); + // Fresh from the other CLI: the caller should wait for the extension. + expect(result).toMatchObject({ port: 8765, spawned: true }); + expect(spawn).not.toHaveBeenCalled(); }); it("errors when the spawned daemon never becomes healthy", async () => { await expect( ensureDaemon(cfg, { spawn: () => {}, find: async () => undefined, pollMs: 1, timeoutMs: 10 }), ).rejects.toThrow("daemon failed to start — check `reins logs`"); + expect(existsSync(lockPath(cfg.dir))).toBe(false); }); }); describe("ensureDaemon version check", () => { - it("restarts a daemon older than the CLI", async () => { - const stop = vi.fn(async () => {}); - const spawn = vi.fn(); + it("restarts a daemon older than the CLI, stop and spawn under the lock", async () => { + const { lock, events, inside } = fakeLock(); const notice = vi.fn(); let calls = 0; + // Old daemon on the lock-free look and on the re-find; new one after spawn. const find = async (): Promise => - ++calls === 1 + ++calls <= 2 ? { port: 8765, health: health(1, "0.4.0") } : { port: 8765, health: health(0, "0.5.0") }; const result = await ensureDaemon(cfg, { version: "0.5.0", find, - stop, - spawn, + lock, + stop: async (port) => void inside(`stop:${port}`), + spawn: () => inside("spawn"), notice, pollMs: 1, }); - expect(stop).toHaveBeenCalledWith(8765); - expect(spawn).toHaveBeenCalledOnce(); + expect(events).toEqual(["lock", "stop:8765", "spawn", "unlock"]); expect(result).toMatchObject({ spawned: true, health: { version: "0.5.0" } }); expect(notice).toHaveBeenCalledWith("reins: daemon runs v0.4.0, CLI is v0.5.0 — restarting it"); }); @@ -81,6 +133,72 @@ describe("ensureDaemon version check", () => { expect(result.spawned).toBe(false); expect(stop).not.toHaveBeenCalled(); }); + + it("does not stop a daemon someone else already restarted (re-find under lock)", async () => { + // Two CLIs see the v0.4.0 daemon; the other one restarts it first. By the + // time we hold the lock, port 8765 is the fresh v0.5.0 — killing it would + // be the TOCTOU bug. + const stop = vi.fn(async () => {}); + const spawn = vi.fn(); + const notice = vi.fn(); + let calls = 0; + const find = async (): Promise => + ++calls === 1 + ? { port: 8765, health: health(1, "0.4.0") } + : { port: 8765, health: health(1, "0.5.0") }; + const result = await ensureDaemon(cfg, { + version: "0.5.0", + find, + lock: (fn) => fn(), + stop, + spawn, + notice, + }); + expect(result).toMatchObject({ port: 8765, spawned: true, health: { version: "0.5.0" } }); + expect(stop).not.toHaveBeenCalled(); + expect(spawn).not.toHaveBeenCalled(); + expect(notice).not.toHaveBeenCalled(); + }); + + it("keeps using the old daemon when it won't stop, with a warning", async () => { + const spawn = vi.fn(); + const notice = vi.fn(); + const found: FoundDaemon = { port: 8765, health: health(1, "0.4.0") }; + const result = await ensureDaemon(cfg, { + version: "0.5.0", + find: async () => found, + lock: (fn) => fn(), + stop: async () => { + throw new Error("reins daemon on port 8765 did not stop — check `reins logs`"); + }, + spawn, + notice, + }); + expect(result).toEqual({ ...found, spawned: false }); + expect(spawn).not.toHaveBeenCalled(); + expect(notice).toHaveBeenLastCalledWith( + "reins: could not restart the v0.4.0 daemon — using it as-is (`reins restart` / `reins logs`)", + ); + }); + + it.each([ + ["daemon", "0.0.0", "0.5.0"], + ["CLI", "0.4.0", "0.0.0"], + ])("never restarts when the %s version is unknown (0.0.0)", async (_, daemon, cli) => { + const stop = vi.fn(async () => {}); + const spawn = vi.fn(); + const found: FoundDaemon = { port: 8765, health: health(1, daemon) }; + const result = await ensureDaemon(cfg, { + version: cli, + find: async () => found, + lock: (fn) => fn(), + stop, + spawn, + }); + expect(result).toEqual({ ...found, spawned: false }); + expect(stop).not.toHaveBeenCalled(); + expect(spawn).not.toHaveBeenCalled(); + }); }); describe("isOlderVersion", () => { @@ -91,6 +209,25 @@ describe("isOlderVersion", () => { expect(isOlderVersion("0.5.0", "0.5.0")).toBe(false); expect(isOlderVersion("0.5.0-next.1", "0.5.0")).toBe(false); }); + + it("treats garbage as 0.0.0 (pinned: never throws)", () => { + expect(isOlderVersion("abc", "0.5.0")).toBe(true); + expect(isOlderVersion("0.5.0", "abc")).toBe(false); + expect(isOlderVersion("", "")).toBe(false); + expect(isOlderVersion("1.x.2", "1.0.1")).toBe(false); // "x" → 0, so 1.0.2 + }); +}); + +describe("wantsRestart", () => { + it("restarts only an older daemon whose version (and ours) is known", () => { + expect(wantsRestart("0.4.0", "0.5.0")).toBe(true); + expect(wantsRestart("0.5.0", "0.5.0")).toBe(false); + expect(wantsRestart("0.6.0", "0.5.0")).toBe(false); + expect(wantsRestart("0.0.0", "0.5.0")).toBe(false); + expect(wantsRestart("0.4.0", "0.0.0")).toBe(false); + // Garbage parses as 0.0.0 too — no restart loop from an unparsable version. + expect(wantsRestart("abc", "0.5.0")).toBe(true); + }); }); describe("stopDaemon", () => { @@ -103,6 +240,26 @@ describe("stopDaemon", () => { expect(calls).toBe(3); }); + it("tolerates a rejected shutdown request (daemon died mid-request)", async () => { + const shutdown = vi.fn(async () => { + throw Object.assign(new Error("fetch failed"), { cause: { code: "ECONNRESET" } }); + }); + let calls = 0; + const probe = async (port: number) => (++calls < 2 ? { port, health: health() } : undefined); + await expect(stopDaemon(8765, { shutdown, probe, pollMs: 1 })).resolves.toBeUndefined(); + expect(calls).toBe(2); + }); + + it("still errors when shutdown rejects but the daemon keeps answering", async () => { + const shutdown = async () => { + throw new Error("fetch failed"); + }; + const probe = async (port: number) => ({ port, health: health() }); + await expect(stopDaemon(8765, { shutdown, probe, pollMs: 1, timeoutMs: 10 })).rejects.toThrow( + "did not stop", + ); + }); + it("errors when the daemon never goes away", async () => { const probe = async (port: number) => ({ port, health: health() }); await expect( @@ -111,30 +268,61 @@ describe("stopDaemon", () => { }); }); +describe("findDaemon", () => { + it("prefers the lowest live port (the one lowerPortRival lets survive)", async () => { + // Sticky port 8767 is probed first, but a spawn race left 8765 live too — + // 8767 is about to bow out, so it must not be the answer. + const live = new Set([8767, 8765, 8770]); + const probe = async (port: number) => (live.has(port) ? { port, health: health() } : undefined); + const found = await findDaemon({ ...cfg, port: 8767 }, probe); + expect(found?.port).toBe(8765); + }); + + it("returns undefined when nothing answers", async () => { + expect(await findDaemon(cfg, async () => undefined)).toBeUndefined(); + }); +}); + describe("restartDaemon", () => { it("stops the live daemon, spawns a fresh one, and reports the old one", async () => { - const order: string[] = []; + const { lock, events, inside } = fakeLock(); let stopped = false; const find = async (): Promise => stopped - ? order.includes("spawn") + ? events.includes("spawn") ? { port: 8765, health: health(0) } : undefined : { port: 8766, health: health(1, "0.4.0") }; const result = await restartDaemon(cfg, { find, + lock, stop: async (port) => { - order.push(`stop:${port}`); + inside(`stop:${port}`); stopped = true; }, - spawn: () => order.push("spawn"), + spawn: () => inside("spawn"), pollMs: 1, }); - expect(order).toEqual(["stop:8766", "spawn"]); + expect(events).toEqual(["lock", "stop:8766", "spawn", "unlock"]); expect(result.previous?.health.version).toBe("0.4.0"); expect(result.current).toMatchObject({ port: 8765, spawned: true }); }); + it("fails loudly when the daemon won't stop (explicit restart, unlike ensureDaemon)", async () => { + const spawn = vi.fn(); + await expect( + restartDaemon(cfg, { + find: async () => ({ port: 8765, health: health(1) }), + stop: async () => { + throw new Error("reins daemon on port 8765 did not stop — check `reins logs`"); + }, + spawn, + }), + ).rejects.toThrow("did not stop"); + expect(spawn).not.toHaveBeenCalled(); + expect(existsSync(lockPath(cfg.dir))).toBe(false); + }); + it("just spawns when nothing was running", async () => { const stop = vi.fn(async () => {}); let spawned = false; diff --git a/packages/cli/src/ensure.ts b/packages/cli/src/ensure.ts index 6904897..5f1206a 100644 --- a/packages/cli/src/ensure.ts +++ b/packages/cli/src/ensure.ts @@ -4,6 +4,7 @@ import { fileURLToPath } from "node:url"; import type { BrowserInfo } from "@reins/protocol"; import type { DaemonHealth } from "./cli-commands.js"; import { candidatePorts, type ReinsConfig } from "./config.js"; +import { withDaemonLock } from "./lock.js"; import { packageVersion } from "./version.js"; export interface FoundDaemon { @@ -25,10 +26,19 @@ export async function probeHealth(port: number): Promise { - const results = await Promise.all(candidatePorts(cfg).map(probeHealth)); - return results.find((r) => r !== undefined); +/** + * Find the live daemon across the candidate ports (sticky port included). + * Two daemons can briefly coexist after a spawn race; the lowest port wins + * (see `lowerPortRival`), so prefer it over the one that's about to bow out. + */ +export async function findDaemon( + cfg: ReinsConfig, + probe: (port: number) => Promise = probeHealth, +): Promise { + const results = await Promise.all(candidatePorts(cfg).map((p) => probe(p))); + return results + .filter((r): r is FoundDaemon => r !== undefined) + .sort((a, b) => a.port - b.port)[0]; } /** Path to the bundled CLI entry (this module lands in dist/ next to cli.js). */ @@ -62,6 +72,20 @@ export function isOlderVersion(a: string, b: string): boolean { return false; } +/** What `packageVersion()` reports when package.json can't be read. */ +const UNKNOWN_VERSION = "0.0.0"; + +/** + * Should a live daemon at `daemon` be restarted by a CLI at `cli`? Only when + * it's older — a newer daemon is left alone so two installed CLI versions + * can't bounce it back and forth — and never when either side is unknown + * ("0.0.0" would restart on every command). + */ +export function wantsRestart(daemon: string, cli: string): boolean { + if (daemon === UNKNOWN_VERSION || cli === UNKNOWN_VERSION) return false; + return isOlderVersion(daemon, cli); +} + function requestShutdown(port: number): Promise { return fetch(`http://127.0.0.1:${port}/shutdown`, { method: "POST", @@ -71,7 +95,9 @@ function requestShutdown(port: number): Promise { /** * Ask a daemon to shut down and wait until its port stops answering, so a - * follow-up spawn can't race the dying process for the port. + * follow-up spawn can't race the dying process for the port. A rejected + * /shutdown (already gone, or died mid-request) is fine — the /health probe + * is the source of truth; only "still answering at the deadline" is an error. */ export async function stopDaemon( port: number, @@ -83,7 +109,7 @@ export async function stopDaemon( } = {}, ): Promise { const probe = opts.probe ?? probeHealth; - await (opts.shutdown ?? requestShutdown)(port); + await (opts.shutdown ?? requestShutdown)(port).catch(() => {}); const pollMs = opts.pollMs ?? 100; const deadline = Date.now() + (opts.timeoutMs ?? 4000); while (Date.now() < deadline) { @@ -103,6 +129,13 @@ interface SpawnOpts { find?: (cfg: ReinsConfig) => Promise; pollMs?: number; timeoutMs?: number; + /** Serialises stop+spawn across parallel CLIs (default: `withDaemonLock` in cfg.dir). */ + lock?: (fn: () => Promise) => Promise; + notice?: (msg: string) => void; +} + +function lockOf(cfg: ReinsConfig, opts: SpawnOpts): NonNullable { + return opts.lock ?? ((fn) => withDaemonLock(cfg.dir, fn, { notice: opts.notice })); } /** Spawn a detached daemon and wait for /health. */ @@ -122,9 +155,13 @@ async function spawnAndWait(cfg: ReinsConfig, opts: SpawnOpts): Promise Promise; - notice?: (msg: string) => void; } = {}, ): Promise { const find = opts.find ?? findDaemon; + const notice = opts.notice ?? console.error; + const version = opts.version ?? packageVersion(); + const usable = (d: FoundDaemon) => !wantsRestart(d.health.version, version); + const existing = await find(cfg); - if (existing) { - const version = opts.version ?? packageVersion(); - if (!isOlderVersion(existing.health.version, version)) return { ...existing, spawned: false }; - (opts.notice ?? console.error)( - `reins: daemon runs v${existing.health.version}, CLI is v${version} — restarting it`, - ); - await (opts.stop ?? stopDaemon)(existing.port); - } - return spawnAndWait(cfg, opts); + if (existing && usable(existing)) return { ...existing, spawned: false }; + + const lock = lockOf(cfg, opts); + return lock(async () => { + const current = await find(cfg); + // A parallel CLI started this one while we waited on the lock: treat it + // as fresh, so callers wait for the extension to reconnect. + if (current && usable(current)) return { ...current, spawned: true }; + if (current) { + const old = current.health.version; + notice(`reins: daemon runs v${old}, CLI is v${version} — restarting it`); + try { + await (opts.stop ?? stopDaemon)(current.port); + } catch { + // Don't brick the command over a stubborn daemon; the user can force it. + notice( + `reins: could not restart the v${old} daemon — using it as-is (\`reins restart\` / \`reins logs\`)`, + ); + return { ...current, spawned: false }; + } + } + return spawnAndWait(cfg, opts); + }); } /** * `reins restart`: stop the live daemon (if any) and spawn a fresh one. * `previous` is what was running before, for the old → new version line. + * Unlike `ensureDaemon`, a daemon that won't stop is an error here. */ export async function restartDaemon( cfg: ReinsConfig, opts: SpawnOpts & { stop?: (port: number) => Promise } = {}, ): Promise<{ previous?: FoundDaemon; current: EnsuredDaemon }> { - const previous = await (opts.find ?? findDaemon)(cfg); - if (previous) await (opts.stop ?? stopDaemon)(previous.port); - const current = await spawnAndWait(cfg, opts); - return previous ? { previous, current } : { current }; + const lock = lockOf(cfg, opts); + return lock(async () => { + const previous = await (opts.find ?? findDaemon)(cfg); + if (previous) await (opts.stop ?? stopDaemon)(previous.port); + const current = await spawnAndWait(cfg, opts); + return previous ? { previous, current } : { current }; + }); } /** diff --git a/packages/cli/src/lock.test.ts b/packages/cli/src/lock.test.ts new file mode 100644 index 0000000..53e2123 --- /dev/null +++ b/packages/cli/src/lock.test.ts @@ -0,0 +1,163 @@ +import { spawnSync } from "node:child_process"; +import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { isPidAlive, isStaleLock, lockPath, withDaemonLock } from "./lock.js"; + +const dirs: string[] = []; +function tmp(): string { + const d = mkdtempSync(join(tmpdir(), "reins-lock-")); + dirs.push(d); + return d; +} +afterEach(() => { + for (const d of dirs.splice(0)) rmSync(d, { recursive: true, force: true }); +}); + +/** A pid that certainly existed and certainly doesn't any more. */ +function deadPid(): number { + const child = spawnSync(process.execPath, ["-e", "0"]); + if (child.pid === undefined || child.pid === 0) throw new Error("no child pid"); + return child.pid; +} + +const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); + +describe("isPidAlive", () => { + it("is true for ourselves and false for an exited process", () => { + expect(isPidAlive(process.pid)).toBe(true); + expect(isPidAlive(deadPid())).toBe(false); + }); +}); + +describe("withDaemonLock", () => { + it("creates the lockfile with our pid while fn runs, and removes it after", async () => { + const dir = tmp(); + const path = lockPath(dir); + const result = await withDaemonLock(dir, async () => { + const lock = JSON.parse(readFileSync(path, "utf8")) as { pid: number; at: number }; + expect(lock.pid).toBe(process.pid); + expect(typeof lock.at).toBe("number"); + return "done"; + }); + expect(result).toBe("done"); + expect(existsSync(path)).toBe(false); + }); + + it("releases the lock when fn throws", async () => { + const dir = tmp(); + await expect( + withDaemonLock(dir, async () => { + throw new Error("boom"); + }), + ).rejects.toThrow("boom"); + expect(existsSync(lockPath(dir))).toBe(false); + }); + + it("serialises two holders: the second waits for the first to release", async () => { + const dir = tmp(); + const events: string[] = []; + const first = withDaemonLock(dir, async () => { + events.push("a:start"); + await sleep(40); + events.push("a:end"); + }); + await sleep(5); + const second = withDaemonLock( + dir, + async () => { + events.push("b:start"); + }, + { pollMs: 5 }, + ); + await Promise.all([first, second]); + expect(events).toEqual(["a:start", "a:end", "b:start"]); + }); + + it("sweeps a lock whose owner is dead", async () => { + const dir = tmp(); + writeFileSync(lockPath(dir), JSON.stringify({ pid: deadPid(), at: Date.now() })); + const notice = vi.fn(); + let ran = false; + await withDaemonLock( + dir, + async () => { + ran = true; + const lock = JSON.parse(readFileSync(lockPath(dir), "utf8")) as { pid: number }; + expect(lock.pid).toBe(process.pid); + }, + { pollMs: 5, timeoutMs: 500, notice }, + ); + expect(ran).toBe(true); + expect(notice).not.toHaveBeenCalled(); + expect(existsSync(lockPath(dir))).toBe(false); + }); + + it("sweeps a lock older than staleMs even if its owner is alive", async () => { + const dir = tmp(); + writeFileSync(lockPath(dir), JSON.stringify({ pid: process.pid, at: Date.now() - 60_000 })); + const notice = vi.fn(); + let ran = false; + await withDaemonLock( + dir, + async () => { + ran = true; + }, + { pollMs: 5, timeoutMs: 500, staleMs: 15_000, notice }, + ); + expect(ran).toBe(true); + expect(notice).not.toHaveBeenCalled(); + }); + + it("gives up after timeoutMs and runs fn anyway, leaving the other lock alone", async () => { + const dir = tmp(); + const theirs = JSON.stringify({ pid: process.pid, at: Date.now() }); + writeFileSync(lockPath(dir), theirs); + const notice = vi.fn(); + const result = await withDaemonLock(dir, async () => "ran", { + pollMs: 5, + timeoutMs: 30, + notice, + }); + expect(result).toBe("ran"); + expect(notice).toHaveBeenCalledOnce(); + expect(notice.mock.calls[0]?.[0]).toMatch(/could not take .*daemon\.lock/); + expect(readFileSync(lockPath(dir), "utf8")).toBe(theirs); + }); + + it("runs fn without a lock when the dir doesn't exist", async () => { + const notice = vi.fn(); + const result = await withDaemonLock(join(tmp(), "missing"), async () => 1, { notice }); + expect(result).toBe(1); + expect(notice).toHaveBeenCalledOnce(); + }); +}); + +describe("isStaleLock", () => { + it("is false for a fresh lock held by a live pid", () => { + const dir = tmp(); + writeFileSync(lockPath(dir), JSON.stringify({ pid: process.pid, at: Date.now() })); + expect(isStaleLock(lockPath(dir), 15_000)).toBe(false); + }); + + it("is true past staleMs, or for a dead pid", () => { + const dir = tmp(); + const path = lockPath(dir); + writeFileSync(path, JSON.stringify({ pid: process.pid, at: 1000 })); + expect(isStaleLock(path, 15_000, 20_000)).toBe(true); + writeFileSync(path, JSON.stringify({ pid: deadPid(), at: Date.now() })); + expect(isStaleLock(path, 15_000)).toBe(true); + }); + + it("leaves an unparsable (mid-write) lock to the age check", () => { + const dir = tmp(); + writeFileSync(lockPath(dir), ""); + expect(isStaleLock(lockPath(dir), 15_000)).toBe(false); + expect(isStaleLock(lockPath(dir), 15_000, Date.now() + 60_000)).toBe(true); + }); + + it("is false when the file is already gone", () => { + expect(isStaleLock(join(tmp(), "daemon.lock"), 15_000)).toBe(false); + }); +}); diff --git a/packages/cli/src/lock.ts b/packages/cli/src/lock.ts new file mode 100644 index 0000000..7d4274d --- /dev/null +++ b/packages/cli/src/lock.ts @@ -0,0 +1,124 @@ +import { closeSync, openSync, readFileSync, statSync, unlinkSync, writeSync } from "node:fs"; +import { join } from "node:path"; + +/** + * Best-effort lockfile that serialises daemon stop/spawn across parallel CLIs. + * `{pid, at}` lives in `/daemon.lock`; the file's existence is the lock + * (created with "wx", so creation is atomic). A lock is stale when its owner + * is dead or it's older than `staleMs` — then it's removed and retried. + */ + +export interface LockOpts { + /** Give up waiting after this long (default 10s). */ + timeoutMs?: number; + /** Retry interval while another process holds the lock (default 75ms). */ + pollMs?: number; + /** Locks older than this are presumed abandoned (default 15s). */ + staleMs?: number; + /** Where to report a timed-out acquire (default: stderr). */ + notice?: (msg: string) => void; +} + +export function lockPath(dir: string): string { + return join(dir, "daemon.lock"); +} + +/** True when a process with this pid exists (EPERM = exists but not ours). */ +export function isPidAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch (err) { + return (err as NodeJS.ErrnoException).code === "EPERM"; + } +} + +function readLock(path: string): { pid?: number; at?: number } | undefined { + try { + const parsed = JSON.parse(readFileSync(path, "utf8")) as unknown; + return parsed && typeof parsed === "object" ? (parsed as { pid?: number; at?: number }) : {}; + } catch { + return undefined; + } +} + +/** Stale = older than `staleMs`, or held by a pid that no longer exists. */ +export function isStaleLock(path: string, staleMs: number, now = Date.now()): boolean { + let mtime: number; + try { + mtime = statSync(path).mtimeMs; + } catch { + return false; // gone already — nothing to remove + } + const lock = readLock(path); + const at = typeof lock?.at === "number" ? lock.at : mtime; + if (now - at > staleMs) return true; + // Unparsable (e.g. caught mid-write) — leave it to the age check. + return typeof lock?.pid === "number" && !isPidAlive(lock.pid); +} + +const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); + +function errCode(err: unknown): string | undefined { + return (err as NodeJS.ErrnoException | undefined)?.code; +} + +/** Create the lockfile; false when it couldn't be had in time (or at all). */ +async function acquire(path: string, opts: Required>): Promise { + const deadline = Date.now() + opts.timeoutMs; + for (;;) { + try { + const fd = openSync(path, "wx"); + writeSync(fd, JSON.stringify({ pid: process.pid, at: Date.now() })); + closeSync(fd); + return true; + } catch (err) { + // Anything but "already exists" (missing dir, read-only fs): don't lock. + if (errCode(err) !== "EEXIST") return false; + } + if (isStaleLock(path, opts.staleMs)) { + try { + unlinkSync(path); + } catch {} + } + if (Date.now() >= deadline) return false; + await sleep(opts.pollMs); + } +} + +/** Remove the lockfile, but only if it's still ours (a stale-sweep may have replaced it). */ +function release(path: string): void { + if (readLock(path)?.pid !== process.pid) return; + try { + unlinkSync(path); + } catch {} +} + +/** + * Run `fn` holding `/daemon.lock`. If the lock can't be acquired within + * `timeoutMs`, `fn` still runs (after a notice) — a stuck CLI is worse than + * an unlocked spawn. + */ +export async function withDaemonLock( + dir: string, + fn: () => Promise, + opts: LockOpts = {}, +): Promise { + const path = lockPath(dir); + const held = await acquire(path, { + timeoutMs: opts.timeoutMs ?? 10_000, + pollMs: opts.pollMs ?? 75, + staleMs: opts.staleMs ?? 15_000, + }); + if (!held) { + (opts.notice ?? console.error)( + `reins: could not take ${path} (another reins command may be starting the daemon) — proceeding without it`, + ); + return fn(); + } + try { + return await fn(); + } finally { + release(path); + } +} diff --git a/packages/web/src/routes/docs/commands.tsx b/packages/web/src/routes/docs/commands.tsx index 490b8a9..729b9a0 100644 --- a/packages/web/src/routes/docs/commands.tsx +++ b/packages/web/src/routes/docs/commands.tsx @@ -161,7 +161,7 @@ const GROUPS: Array<{ id: string; title: string; intro?: string; rows: [string, ], ["reins allow ", "Allow an unpacked or dev extension to connect."], ["reins audit [--denied]", "Render the audit trail, or only what policy blocked."], - ["reins restart", "Restart the background daemon, e.g. after changing config."], + ["reins restart", "Restart the background daemon (after an upgrade or reins allow)."], ["reins kill", "Stop the background daemon."], ["reins doctor", "Run diagnostic checks."], ["reins logs", "Show the daemon log location and recent lines."], diff --git a/packages/web/src/routes/docs/faq.tsx b/packages/web/src/routes/docs/faq.tsx index 8a247be..87ebf89 100644 --- a/packages/web/src/routes/docs/faq.tsx +++ b/packages/web/src/routes/docs/faq.tsx @@ -37,7 +37,7 @@ const FAQS = [ id: "update", question: "How do I update reins?", answer: - "Run npm i -g @karnstack/reins@latest. The next reins command notices the running daemon is older than the CLI and restarts it on the new version. The Chrome Web Store extension updates itself.", + "Run npm i -g @karnstack/reins@latest. The next tool command (reins tabs, say) notices the running daemon is older than the CLI and restarts it on the new version; reins restart does it right away. reins status and reins doctor only report the mismatch. The Chrome Web Store extension updates itself.", }, { id: "mcp", From 1e33c750c50a52289e54cb531fc127ef30242880 Mon Sep 17 00:00:00 2001 From: Karn Date: Sun, 27 Sep 2026 02:15:09 +0530 Subject: [PATCH 3/3] fix(doctor): a newer daemon means the CLI is stale, not the daemon doctor told users to `reins restart` on any daemon/CLI version mismatch. When the daemon is newer (a second, newer install started it), a restart from this older CLI downgrades the daemon, and the newer CLI's next command bumps it straight back up. For that direction doctor now says to upgrade this CLI or look for a second install with `which -a reins`. Versions that differ only by pre-release tag pass, matching the restart logic. Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/cli-commands.test.ts | 16 ++++++++++ packages/cli/src/cli-commands.ts | 44 +++++++++++++++++++-------- 2 files changed, 48 insertions(+), 12 deletions(-) diff --git a/packages/cli/src/cli-commands.test.ts b/packages/cli/src/cli-commands.test.ts index cafd043..25cbe80 100644 --- a/packages/cli/src/cli-commands.test.ts +++ b/packages/cli/src/cli-commands.test.ts @@ -299,6 +299,22 @@ describe("doctorReport", () => { }); }); + it("points at the CLI, not a restart, when the daemon is newer", () => { + // A restart from this older CLI would downgrade the daemon. + const report = doctorReport(cfg(), { ...HEALTH, version: "0.6.0" }, "0.5.0"); + expect(report.ok).toBe(false); + const detail = check(report, "version")?.detail ?? ""; + expect(detail).toBe( + "daemon v0.6.0 is newer than this CLI (v0.5.0) — upgrade it (`npm i -g @karnstack/reins@latest`) or check `which -a reins` for a second install", + ); + expect(detail).not.toContain("reins restart"); + }); + + it("passes when only a pre-release tag differs", () => { + const report = doctorReport(cfg(), { ...HEALTH, version: "0.5.0-next.1" }, "0.5.0"); + expect(check(report, "version")?.ok).toBe(true); + }); + it("skips the version check when either side is the unknown 0.0.0", () => { expect( check(doctorReport(cfg(), { ...HEALTH, version: "0.0.0" }, "0.5.0"), "version"), diff --git a/packages/cli/src/cli-commands.ts b/packages/cli/src/cli-commands.ts index 23cc2e1..62d874a 100644 --- a/packages/cli/src/cli-commands.ts +++ b/packages/cli/src/cli-commands.ts @@ -3,7 +3,7 @@ import { join } from "node:path"; import type { BrowserInfo, SkippedBrowser, Tab, TabGroup } from "@reins/protocol"; import type { ToolCommand } from "./commands.js"; import type { ReinsConfig } from "./config.js"; -import { wantsRestart } from "./ensure.js"; +import { isOlderVersion, wantsRestart } from "./ensure.js"; export interface DaemonHealth { ok: boolean; @@ -199,11 +199,40 @@ export function groupsText(groups: TabGroup[], skipped: SkippedBrowser[] = []): return lines.join("\n"); } +export interface DoctorCheck { + name: string; + ok: boolean; + detail: string; +} + export interface DoctorReport { - checks: Array<{ name: string; ok: boolean; detail: string }>; + checks: DoctorCheck[]; ok: boolean; } +/** + * Daemon vs CLI version. Older daemon: left over from before an upgrade, so + * restart it. Newer daemon: this CLI is the stale one (a second install, or + * an old copy earlier on PATH) — restarting from it would downgrade the daemon. + */ +function versionCheck(daemon: string, cli: string): DoctorCheck { + if (isOlderVersion(daemon, cli)) { + return { + name: "version", + ok: false, + detail: `daemon v${daemon}, CLI v${cli} — run \`reins restart\``, + }; + } + if (isOlderVersion(cli, daemon)) { + return { + name: "version", + ok: false, + detail: `daemon v${daemon} is newer than this CLI (v${cli}) — upgrade it (\`npm i -g @karnstack/reins@latest\`) or check \`which -a reins\` for a second install`, + }; + } + return { name: "version", ok: true, detail: `daemon and CLI both v${cli}` }; +} + /** Diagnostic checks for `reins doctor`. `cliVersion` is compared against the daemon's. */ export function doctorReport( cfg: ReinsConfig, @@ -221,17 +250,8 @@ export function doctorReport( ? `running (v${health.version})` : "not running — starts on demand (`reins tabs`), or run `reins daemon`", }, - // A daemon left over from before an upgrade runs the old code until restarted. ...(health && knownVersion(health.version) && knownVersion(cliVersion) - ? [ - health.version === cliVersion - ? { name: "version", ok: true, detail: `daemon and CLI both v${cliVersion}` } - : { - name: "version", - ok: false, - detail: `daemon v${health.version}, CLI v${cliVersion} — run \`reins restart\``, - }, - ] + ? [versionCheck(health.version, cliVersion)] : []), { name: "browser",