From fabe48ef93616b654b1a0ff59f208929007c9381 Mon Sep 17 00:00:00 2001 From: critesjosh Date: Sat, 2 May 2026 01:14:41 +0000 Subject: [PATCH 1/2] feat: npm-version self-check + accurate aztec_status version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the staleness gap that drove this PR's parent dogfood test: the MCP server pulled its displayed version from sync metadata (populated at last `aztec_sync_repos` run, not at boot), so an upgraded package would still report the OLD version under `aztec_status`. Combined with npx's package cache, this made "running an out-of-date MCP" near-undetectable from inside a session. Three changes solve it together: 1. New `src/utils/version-self-check.ts`. At startup the server GETs `https://registry.npmjs.org/@aztec/mcp-server/latest` (2s timeout, fails silently on network/registry error). Compares against the live `MCP_VERSION` (read from package.json by `src/version.ts`). On `outdated`, the result is cached in a module-level singleton so both the InitializeResult `instructions` banner AND the `aztec_status` text can surface it without a second registry hit. 2. Banner content: `instructions` gets a multi-line "UPDATE AVAILABLE" footer that names both versions and lists the `@aztec/mcp-server@latest` upgrade paths for Claude Desktop / Cursor / Codex / Claude Code / global install. Written to be read by the LLM consumer — the model passes it through to the user. `aztec_status` gets a single-line warning so a curious user running diagnostics sees the same signal. 3. `formatStatus` now reads the live `MCP_VERSION` from the `version.ts` import — fixes the staleness bug. Sync-metadata version moves to a contextual annotation that only renders when it differs from live ("(last sync ran under MCP server v1.5.0 — re-run aztec_sync_repos to refresh metadata)"). Always-show ordering: live version, npm-latest comparison, repos dir, sync metadata. README updated: every install snippet (`npx`, `npm install -g`, `.mcp.json` examples) pinned to `@aztec/mcp-server@latest`. Added a prominent note explaining why npx caching makes the bare form a foot-gun. Tests: 264/264. New `tests/utils/version-self-check.test.ts` with 19 cases (semver edge cases incl. pre-release, fetch failure modes, cache lifecycle, banner formatting). Updated `tests/utils/format.test.ts` for the new live-version display + 4 new cases for the upgrade-line behaviors. End-to-end probe over stdio confirmed the banner reaches both `initialize` (instructions) and `tools/call aztec_status` correctly when the running version trails npm-latest. Co-Authored-By: Claude Opus 4.7 (1M context) --- README.md | 12 +- src/index.ts | 18 ++- src/utils/format.ts | 30 ++++- src/utils/version-self-check.ts | 152 +++++++++++++++++++++ tests/utils/format.test.ts | 84 ++++++++++-- tests/utils/version-self-check.test.ts | 180 +++++++++++++++++++++++++ 6 files changed, 461 insertions(+), 15 deletions(-) create mode 100644 src/utils/version-self-check.ts create mode 100644 tests/utils/version-self-check.test.ts diff --git a/README.md b/README.md index 4b8a4fb..8f720a2 100644 --- a/README.md +++ b/README.md @@ -34,16 +34,20 @@ Keys are free, persistent (re-running `/mcp-key` returns the same key), and revo ### With npx (recommended) ```bash -npx @aztec/mcp-server +npx -y @aztec/mcp-server@latest ``` +> **Always pin `@latest`.** npx caches packages aggressively — without `@latest`, you can end up running an old version indefinitely. The `@latest` tag forces npx to check the registry for the current release every run. The server also self-reports an upgrade-available warning at startup if it detects a newer version on npm (see `aztec_status` output). + ### Global install ```bash -npm install -g @aztec/mcp-server +npm install -g @aztec/mcp-server@latest aztec-mcp ``` +To update later: `npm install -g @aztec/mcp-server@latest` (or just rely on the `npx -y @aztec/mcp-server@latest` form, which always fetches current). + ## Configuration ### Claude Code Plugin @@ -179,7 +183,7 @@ Override with the `AZTEC_MCP_REPOS_DIR` environment variable: "mcpServers": { "aztec-mcp": { "command": "npx", - "args": ["-y", "@aztec/mcp-server"], + "args": ["-y", "@aztec/mcp-server@latest"], "env": { "AZTEC_MCP_REPOS_DIR": "/custom/path" } @@ -197,7 +201,7 @@ Set the default Aztec version with the `AZTEC_DEFAULT_VERSION` environment varia "mcpServers": { "aztec-mcp": { "command": "npx", - "args": ["-y", "@aztec/mcp-server"], + "args": ["-y", "@aztec/mcp-server@latest"], "env": { "AZTEC_DEFAULT_VERSION": "v3.0.0-devnet.6-plugin.1" } diff --git a/src/index.ts b/src/index.ts index 1df4beb..7489e55 100644 --- a/src/index.ts +++ b/src/index.ts @@ -47,6 +47,10 @@ import { getSyncState, writeAutoResyncAttempt } from "./utils/sync-metadata.js"; import { getRepoTag } from "./utils/git.js"; import type { Logger } from "./utils/git.js"; import { DocsGPTClient } from "./backends/docsgpt-client.js"; +import { + checkForUpgrade, + formatUpgradeBanner, +} from "./utils/version-self-check.js"; // --------------------------------------------------------------------------- // DocsGPT client — optional, enabled when API_KEY is set @@ -108,6 +112,16 @@ const LOCAL_ONLY_INSTRUCTIONS = "API_KEY in their MCP client config (e.g. .mcp.json, Claude Desktop " + "config, etc.) and restart the server."; +// Check npm registry for a newer release and surface an "outdated" +// banner in the instructions if so. Top-level await: blocks startup +// for at most ~2s on the registry round-trip (with internal timeout) +// — runs once per server process, so the cost amortizes immediately. +// Failure modes (no network, registry down, slow response) all return +// null and produce no banner; the server boots normally. +const upgradeInfo = await checkForUpgrade(MCP_VERSION); +const upgradeBanner = + upgradeInfo && upgradeInfo.outdated ? formatUpgradeBanner(upgradeInfo) : ""; + const server = new Server( { name: "aztec-mcp", @@ -118,7 +132,9 @@ const server = new Server( tools: {}, logging: {}, }, - instructions: docsgptClient ? SEMANTIC_INSTRUCTIONS : LOCAL_ONLY_INSTRUCTIONS, + instructions: + (docsgptClient ? SEMANTIC_INSTRUCTIONS : LOCAL_ONLY_INSTRUCTIONS) + + upgradeBanner, } ); diff --git a/src/utils/format.ts b/src/utils/format.ts index 841e448..048547b 100644 --- a/src/utils/format.ts +++ b/src/utils/format.ts @@ -8,6 +8,11 @@ import type { SyncMetadata } from "./sync-metadata.js"; import type { ErrorLookupResult } from "./error-lookup.js"; import type { SemanticSearchToolResult } from "../tools/search.js"; import type { ErrorLookupToolResult } from "../tools/error-lookup.js"; +import { MCP_VERSION } from "../version.js"; +import { + formatUpgradeStatusLine, + getUpgradeInfo, +} from "./version-self-check.js"; export function formatSyncResult(result: SyncResult): string { const lines = [ @@ -40,12 +45,33 @@ export function formatStatus(status: { const lines = [ "Aztec MCP Server Status", "", - `Repos directory: ${status.reposDir}`, + // Live version read from package.json at module load (see + // ``src/version.ts``). The previous implementation pulled this + // from sync metadata, which was the version that ran the LAST + // sync — stale across upgrades that didn't touch the clones. + `MCP server version: ${MCP_VERSION}`, ]; + // npm-latest comparison done at boot (``checkForUpgrade`` in + // ``src/index.ts``). Prints either "you are up to date" or an + // upgrade-available warning. Empty string when the registry check + // failed at boot, so we stay silent rather than misleading. + const upgradeLine = formatUpgradeStatusLine(getUpgradeInfo()); + if (upgradeLine) { + lines.push(upgradeLine); + } + + lines.push(`Repos directory: ${status.reposDir}`); + if (status.syncMetadata) { lines.push(`Last synced: ${status.syncMetadata.syncedAt}`); - lines.push(`MCP server version: ${status.syncMetadata.mcpVersion}`); + if (status.syncMetadata.mcpVersion !== MCP_VERSION) { + // Only mention this when it differs from the live version — + // otherwise it's just noise that duplicates the line above. + lines.push( + ` (last sync ran under MCP server v${status.syncMetadata.mcpVersion} — re-run aztec_sync_repos to refresh metadata)` + ); + } lines.push(`Aztec version: ${status.syncMetadata.aztecVersion}`); } diff --git a/src/utils/version-self-check.ts b/src/utils/version-self-check.ts new file mode 100644 index 0000000..01c88d3 --- /dev/null +++ b/src/utils/version-self-check.ts @@ -0,0 +1,152 @@ +/** + * Self-check for outdated installs of @aztec/mcp-server. + * + * Why: npx caches packages, and users frequently end up running an old + * version while assuming `npx @aztec/mcp-server` always pulls the latest. + * The result is silently-degraded behavior + bug reports against fixes + * that have already shipped. This module fetches the current latest tag + * from the npm registry at startup, compares against the running + * version, and surfaces a warning into both the MCP `instructions` + * banner (so the LLM tells the user) and `aztec_status` (so a curious + * user running diagnostics also sees it). + * + * Failure modes are silent (registry down, no network, slow response) + * — the check should never block startup or fail the server. Worst + * case: no banner, business as usual. + */ + +const NPM_REGISTRY_URL = "https://registry.npmjs.org/@aztec/mcp-server/latest"; + +export interface UpgradeInfo { + current: string; + latest: string; + outdated: boolean; +} + +let upgradeInfoCache: UpgradeInfo | null = null; + +/** + * Test-only: reset the module-level upgrade cache between tests. + */ +export function _resetUpgradeCache(): void { + upgradeInfoCache = null; +} + +export function setUpgradeInfo(info: UpgradeInfo | null): void { + upgradeInfoCache = info; +} + +export function getUpgradeInfo(): UpgradeInfo | null { + return upgradeInfoCache; +} + +/** + * Fetch the latest published version of @aztec/mcp-server from npm. + * Returns null on any failure (network, timeout, malformed body) — + * never throws, so callers don't have to wrap in try/catch. + */ +export async function fetchLatestNpmVersion( + timeoutMs: number = 2000, + fetchImpl: typeof fetch = fetch +): Promise { + try { + const ctl = new AbortController(); + const timer = setTimeout(() => ctl.abort(), timeoutMs); + const resp = await fetchImpl(NPM_REGISTRY_URL, { signal: ctl.signal }); + clearTimeout(timer); + if (!resp.ok) return null; + const data = await resp.json(); + if (!data || typeof data !== "object") return null; + const v = (data as Record).version; + return typeof v === "string" ? v : null; + } catch { + return null; + } +} + +/** + * Numeric major.minor.patch comparison. Strips a leading ``v`` and + * any pre-release / build suffix (so ``1.20.0-rc.1`` and ``1.20.0`` + * compare equal — we don't want to flag a stable user as outdated + * relative to a pre-release on npm). + */ +export function compareSemver(a: string, b: string): -1 | 0 | 1 { + const parse = (v: string): number[] => { + const core = v.replace(/^v/, "").split("-")[0].split("+")[0]; + return core.split(".").map((p) => { + const n = parseInt(p, 10); + return Number.isFinite(n) ? n : 0; + }); + }; + const ma = parse(a); + const mb = parse(b); + for (let i = 0; i < Math.max(ma.length, mb.length); i++) { + const x = ma[i] ?? 0; + const y = mb[i] ?? 0; + if (x < y) return -1; + if (x > y) return 1; + } + return 0; +} + +/** + * High-level entry: fetch + compare + cache. Returns the populated + * cache entry (also retrievable via ``getUpgradeInfo()``). + */ +export async function checkForUpgrade( + currentVersion: string, + options: { timeoutMs?: number; fetchImpl?: typeof fetch } = {} +): Promise { + const latest = await fetchLatestNpmVersion( + options.timeoutMs, + options.fetchImpl ?? fetch + ); + if (!latest) { + setUpgradeInfo(null); + return null; + } + const info: UpgradeInfo = { + current: currentVersion, + latest, + outdated: compareSemver(currentVersion, latest) < 0, + }; + setUpgradeInfo(info); + return info; +} + +/** + * Format the upgrade warning that gets appended to the MCP server + * instructions banner. The text is consumed by the LLM, not directly + * by a human, so it explains what the LLM should *do*: tell the user + * to update. Listed remediation commands match the install paths + * documented in the README so the LLM can copy-paste them. + */ +export function formatUpgradeBanner(info: UpgradeInfo): string { + return [ + "", + "", + `⚠️ UPDATE AVAILABLE: this MCP server is running v${info.current}, but v${info.latest} is the current release on npm. ` + + `Tell the user they're on an outdated version, and that bug reports about behavior may already be fixed in the latest release. ` + + `To upgrade, ensure their MCP client config uses \`@aztec/mcp-server@latest\` so npx fetches the newest:`, + ` • Claude Desktop / Cursor / Codex: change the args to ` + + `["-y", "@aztec/mcp-server@latest"] in the MCP server config and restart the client.`, + ` • Claude Code: \`claude mcp remove aztec-docs && claude mcp add aztec-docs ... -- npx -y @aztec/mcp-server@latest\``, + ` • If installed globally: \`npm uninstall -g @aztec/mcp-server && npm install -g @aztec/mcp-server@latest\` (or just rely on npx).`, + ].join("\n"); +} + +/** + * Format a one-line upgrade summary suitable for inclusion in the + * ``aztec_status`` output. Returns the empty string when the install + * is current (so the formatter can unconditionally include it). + */ +export function formatUpgradeStatusLine(info: UpgradeInfo | null): string { + if (!info) return ""; + if (!info.outdated) { + return `npm latest: v${info.latest} (you are up to date)`; + } + return ( + `⚠️ UPDATE AVAILABLE: v${info.current} → v${info.latest} on npm. ` + + `Switch your MCP config to \`@aztec/mcp-server@latest\` and restart the client.` + ); +} diff --git a/tests/utils/format.test.ts b/tests/utils/format.test.ts index c0a5441..f61293c 100644 --- a/tests/utils/format.test.ts +++ b/tests/utils/format.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect } from "vitest"; +import { describe, it, expect, beforeEach } from "vitest"; import { formatSyncResult, formatStatus, @@ -7,6 +7,15 @@ import { formatExampleContent, formatFileContent, } from "../../src/utils/format.js"; +import { MCP_VERSION } from "../../src/version.js"; +import { + setUpgradeInfo, + _resetUpgradeCache, +} from "../../src/utils/version-self-check.js"; + +beforeEach(() => { + _resetUpgradeCache(); +}); describe("formatSyncResult", () => { it("shows checkmark for success", () => { @@ -80,29 +89,88 @@ describe("formatStatus", () => { expect(result).toContain("No repositories cloned"); }); - it("displays sync metadata when present", () => { + it("always shows the live MCP version (read from package.json), even without sync metadata", () => { + const result = formatStatus({ + reposDir: "/repos", + repos: [], + }); + // Live version always present — was previously gated behind syncMetadata + expect(result).toContain(`MCP server version: ${MCP_VERSION}`); + // Sync-only fields should remain absent when no metadata + expect(result).not.toContain("Last synced"); + expect(result).not.toContain("Aztec version:"); + }); + + it("when sync-metadata version equals live MCP_VERSION, does not duplicate it", () => { const result = formatStatus({ reposDir: "/repos", repos: [], syncMetadata: { - mcpVersion: "1.5.0", + mcpVersion: MCP_VERSION, syncedAt: "2025-01-01T00:00:00.000Z", aztecVersion: "v1.0.0", }, }); + expect(result).toContain(`MCP server version: ${MCP_VERSION}`); expect(result).toContain("Last synced: 2025-01-01T00:00:00.000Z"); - expect(result).toContain("MCP server version: 1.5.0"); expect(result).toContain("Aztec version: v1.0.0"); + expect(result).not.toContain("last sync ran under MCP server v"); }); - it("omits sync metadata lines when not present", () => { + it("when sync metadata records a different version, surfaces the staleness line", () => { const result = formatStatus({ reposDir: "/repos", repos: [], + syncMetadata: { + mcpVersion: "1.5.0", + syncedAt: "2025-01-01T00:00:00.000Z", + aztecVersion: "v1.0.0", + }, }); - expect(result).not.toContain("Last synced"); - expect(result).not.toContain("MCP server version"); - expect(result).not.toContain("Aztec version"); + // Live live (always present) + expect(result).toContain(`MCP server version: ${MCP_VERSION}`); + // Staleness annotation only when different + expect(result).toContain("last sync ran under MCP server v1.5.0"); + }); + + it("includes upgrade-available warning in status when registry check found a newer version", () => { + setUpgradeInfo({ + current: MCP_VERSION, + latest: "999.0.0", + outdated: true, + }); + const result = formatStatus({ + reposDir: "/repos", + repos: [], + }); + expect(result).toContain("UPDATE AVAILABLE"); + expect(result).toContain("999.0.0"); + expect(result).toContain("@latest"); + }); + + it("includes 'up to date' line in status when registry check confirmed latest", () => { + setUpgradeInfo({ + current: MCP_VERSION, + latest: MCP_VERSION, + outdated: false, + }); + const result = formatStatus({ + reposDir: "/repos", + repos: [], + }); + expect(result).toContain("up to date"); + expect(result).not.toContain("UPDATE AVAILABLE"); + }); + + it("omits the npm-latest line when the registry check failed (no info cached)", () => { + // _resetUpgradeCache() in beforeEach already cleared this, but + // assert the contract explicitly. + const result = formatStatus({ + reposDir: "/repos", + repos: [], + }); + expect(result).not.toContain("npm latest"); + expect(result).not.toContain("UPDATE AVAILABLE"); }); }); diff --git a/tests/utils/version-self-check.test.ts b/tests/utils/version-self-check.test.ts new file mode 100644 index 0000000..bc86a3e --- /dev/null +++ b/tests/utils/version-self-check.test.ts @@ -0,0 +1,180 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +import { + fetchLatestNpmVersion, + compareSemver, + checkForUpgrade, + getUpgradeInfo, + setUpgradeInfo, + formatUpgradeBanner, + formatUpgradeStatusLine, + _resetUpgradeCache, +} from "../../src/utils/version-self-check.js"; + +beforeEach(() => { + _resetUpgradeCache(); + vi.restoreAllMocks(); +}); + +describe("compareSemver", () => { + it("compares core major.minor.patch numerically", () => { + expect(compareSemver("1.2.3", "1.2.4")).toBe(-1); + expect(compareSemver("1.2.4", "1.2.3")).toBe(1); + expect(compareSemver("1.2.3", "1.2.3")).toBe(0); + expect(compareSemver("1.10.0", "1.2.0")).toBe(1); // numeric, not lexicographic + }); + + it("strips leading 'v'", () => { + expect(compareSemver("v1.20.0", "1.20.0")).toBe(0); + expect(compareSemver("v1.20.0", "v1.21.0")).toBe(-1); + }); + + it("ignores pre-release and build suffixes", () => { + expect(compareSemver("1.20.0-rc.1", "1.20.0")).toBe(0); + expect(compareSemver("1.20.0+build.42", "1.20.0")).toBe(0); + expect(compareSemver("1.20.0-alpha", "1.20.0-beta")).toBe(0); + }); + + it("handles missing patch component", () => { + expect(compareSemver("1.20", "1.20.0")).toBe(0); + expect(compareSemver("1.21", "1.20.99")).toBe(1); + }); + + it("handles non-numeric garbage by treating as 0", () => { + expect(compareSemver("abc.def.ghi", "0.0.0")).toBe(0); + }); +}); + +describe("fetchLatestNpmVersion", () => { + it("returns the version string from a valid registry response", async () => { + const fakeFetch: any = vi.fn().mockResolvedValue( + new Response(JSON.stringify({ version: "1.99.0" }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }) + ); + expect(await fetchLatestNpmVersion(2000, fakeFetch)).toBe("1.99.0"); + expect(fakeFetch).toHaveBeenCalledWith( + expect.stringContaining("registry.npmjs.org/@aztec/mcp-server/latest"), + expect.any(Object) + ); + }); + + it("returns null on non-OK response", async () => { + const fakeFetch: any = vi + .fn() + .mockResolvedValue(new Response("not found", { status: 404 })); + expect(await fetchLatestNpmVersion(2000, fakeFetch)).toBeNull(); + }); + + it("returns null on malformed body", async () => { + const fakeFetch: any = vi.fn().mockResolvedValue( + new Response("not json", { + status: 200, + headers: { "Content-Type": "text/plain" }, + }) + ); + expect(await fetchLatestNpmVersion(2000, fakeFetch)).toBeNull(); + }); + + it("returns null on missing version field", async () => { + const fakeFetch: any = vi.fn().mockResolvedValue( + new Response(JSON.stringify({ name: "@aztec/mcp-server" }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }) + ); + expect(await fetchLatestNpmVersion(2000, fakeFetch)).toBeNull(); + }); + + it("returns null on fetch rejection (network error / abort)", async () => { + const fakeFetch: any = vi + .fn() + .mockRejectedValue(new Error("network unreachable")); + expect(await fetchLatestNpmVersion(2000, fakeFetch)).toBeNull(); + }); + + it("never throws, even with garbage rejection types", async () => { + const fakeFetch: any = vi.fn().mockRejectedValue("just a string"); + await expect(fetchLatestNpmVersion(2000, fakeFetch)).resolves.toBeNull(); + }); +}); + +describe("checkForUpgrade", () => { + function fetchReturning(version: string | null): any { + if (version === null) { + return vi.fn().mockResolvedValue(new Response("nope", { status: 500 })); + } + return vi.fn().mockResolvedValue( + new Response(JSON.stringify({ version }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }) + ); + } + + it("populates the cache with outdated=true when running an older version", async () => { + const info = await checkForUpgrade("1.15.0", { fetchImpl: fetchReturning("1.20.0") }); + expect(info).toEqual({ current: "1.15.0", latest: "1.20.0", outdated: true }); + expect(getUpgradeInfo()).toEqual(info); + }); + + it("populates the cache with outdated=false when up-to-date", async () => { + const info = await checkForUpgrade("1.20.0", { fetchImpl: fetchReturning("1.20.0") }); + expect(info).toEqual({ current: "1.20.0", latest: "1.20.0", outdated: false }); + expect(getUpgradeInfo()?.outdated).toBe(false); + }); + + it("clears the cache to null on registry failure", async () => { + setUpgradeInfo({ current: "x", latest: "y", outdated: true }); + const info = await checkForUpgrade("1.20.0", { fetchImpl: fetchReturning(null) }); + expect(info).toBeNull(); + expect(getUpgradeInfo()).toBeNull(); + }); + + it("treats user pre-release as equal to stable (does not flag as outdated)", async () => { + const info = await checkForUpgrade("1.20.0-rc.1", { fetchImpl: fetchReturning("1.20.0") }); + expect(info?.outdated).toBe(false); + }); +}); + +describe("formatUpgradeBanner", () => { + it("includes both versions and the @latest pin guidance", () => { + const banner = formatUpgradeBanner({ + current: "1.15.0", + latest: "1.20.0", + outdated: true, + }); + expect(banner).toContain("v1.15.0"); + expect(banner).toContain("v1.20.0"); + expect(banner).toContain("@aztec/mcp-server@latest"); + expect(banner).toContain("UPDATE AVAILABLE"); + // Surface the install paths the README documents + expect(banner).toContain("Claude Desktop"); + expect(banner).toContain("Claude Code"); + }); +}); + +describe("formatUpgradeStatusLine", () => { + it("returns empty string when no upgrade info available (registry check failed)", () => { + expect(formatUpgradeStatusLine(null)).toBe(""); + }); + + it("returns 'up to date' line when current matches latest", () => { + expect( + formatUpgradeStatusLine({ current: "1.20.0", latest: "1.20.0", outdated: false }) + ).toContain("up to date"); + }); + + it("returns 'UPDATE AVAILABLE' line when outdated", () => { + const line = formatUpgradeStatusLine({ + current: "1.15.0", + latest: "1.20.0", + outdated: true, + }); + expect(line).toContain("UPDATE AVAILABLE"); + expect(line).toContain("v1.15.0"); + expect(line).toContain("v1.20.0"); + expect(line).toContain("@latest"); + }); +}); From 8fece22d7b727e6f59c383535cae66cae7cc81b2 Mon Sep 17 00:00:00 2001 From: critesjosh Date: Sat, 2 May 2026 01:22:34 +0000 Subject: [PATCH 2/2] fix(version-self-check): clear timer in finally + unref to avoid event-loop pin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex review feedback on PR #21. The previous implementation only cleared the abort timer on the success path — when `fetchImpl` rejected (network error, malformed body), the catch block returned null but the timer was left running until it fired. In a long-lived MCP server that's invisible (the timer just expires harmlessly seconds later), but in short-lived processes / tests it would prevent the event loop from exiting cleanly. Two-line fix: - `timer.unref()` (Node-only, optional-chained for portability) so the timer alone never keeps the loop alive. - `clearTimeout(timer)` moved into a `finally` block so it runs on both success and exception paths. Belt-and-braces by design: either fix alone is sufficient, but together they leave no path for a stray timer. 19 version-self-check tests still pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/utils/version-self-check.ts | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/utils/version-self-check.ts b/src/utils/version-self-check.ts index 01c88d3..08bf911 100644 --- a/src/utils/version-self-check.ts +++ b/src/utils/version-self-check.ts @@ -49,11 +49,16 @@ export async function fetchLatestNpmVersion( timeoutMs: number = 2000, fetchImpl: typeof fetch = fetch ): Promise { + const ctl = new AbortController(); + const timer = setTimeout(() => ctl.abort(), timeoutMs); + // `unref` (Node-only) prevents this timer from keeping the event + // loop alive on its own. Critical for short-lived processes and + // tests where a forgotten timer would block exit. Optional-chained + // because `setTimeout` in browser-shaped environments returns a + // primitive number with no `unref` — the optional call is safe. + (timer as unknown as { unref?: () => void }).unref?.(); try { - const ctl = new AbortController(); - const timer = setTimeout(() => ctl.abort(), timeoutMs); const resp = await fetchImpl(NPM_REGISTRY_URL, { signal: ctl.signal }); - clearTimeout(timer); if (!resp.ok) return null; const data = await resp.json(); if (!data || typeof data !== "object") return null; @@ -61,6 +66,12 @@ export async function fetchLatestNpmVersion( return typeof v === "string" ? v : null; } catch { return null; + } finally { + // Always clear: the previous implementation only cleared on the + // success path, leaking the timer when `fetchImpl` rejected + // (network error, CORS, malformed body) before the timeout + // fired. Combined with `unref` above this is belt-and-braces. + clearTimeout(timer); } }