diff --git a/packages/mcp/src/cli/init.ts b/packages/mcp/src/cli/init.ts index 5f52516..31eea6b 100644 --- a/packages/mcp/src/cli/init.ts +++ b/packages/mcp/src/cli/init.ts @@ -16,6 +16,8 @@ import { AGENTS_DIR, getInstalledVersion, getLatestVersion, + isOnline, + isSkillsCliAvailable, detectPackageManager, globalInstallCommand, formatShellCommand, @@ -387,7 +389,20 @@ export async function init(args: string[]): Promise { type SkillsMethod = "default" | "interactive" | "manual"; let skillsMethod: SkillsMethod; - if (nonInteractive) { + const online = await isOnline(); + const offlineWithCache = !online && isSkillsCliAvailable(); + const skillsCliReady = online || offlineWithCache; + + if (!skillsCliReady) { + p.log.warn( + pc.yellow("You appear to be offline. ") + + "Automatic skills installation requires a network connection." + ); + } + + if (!skillsCliReady) { + skillsMethod = "manual"; + } else if (nonInteractive) { skillsMethod = "default"; } else { p.log.message(pc.dim(" Use arrow keys to move, enter to confirm.")); @@ -451,7 +466,9 @@ export async function init(args: string[]): Promise { skillsArgs.push("--skill", "*", "-y"); } - p.log.info(`Running: ${pc.dim("npx")} ${pc.cyan(skillsArgs.join(" "))}`); + const npxArgs = offlineWithCache ? ["--no-install", ...skillsArgs] : skillsArgs; + + p.log.info(`Running: ${pc.dim("npx")} ${pc.cyan(npxArgs.join(" "))}`); const spinner = p.spinner(); if (skillsMethod === "default") { @@ -460,7 +477,7 @@ export async function init(args: string[]): Promise { try { const skillsCwd = scope === "custom" ? customRoot : undefined; - await runNpxSkills(skillsArgs, skillsMethod === "interactive", skillsCwd); + await runNpxSkills(npxArgs, skillsMethod === "interactive", skillsCwd); if (skillsMethod === "default") { spinner.stop("Skills installed."); } diff --git a/packages/mcp/src/cli/utils.ts b/packages/mcp/src/cli/utils.ts index 167ace6..d1706e3 100644 --- a/packages/mcp/src/cli/utils.ts +++ b/packages/mcp/src/cli/utils.ts @@ -1,5 +1,6 @@ import * as fs from "node:fs"; import * as path from "node:path"; +import * as dns from "node:dns"; import { execSync } from "node:child_process"; import { PACKAGE_NAME, NPM_REGISTRY } from "./constants.js"; import { parse as parseToml, stringify as stringifyToml } from "smol-toml"; @@ -128,13 +129,46 @@ export function getInstalledVersion(): string | null { } } +const PROBE_TIMEOUT_MS = 3_000; + export function getLatestVersion(): string { const result = execSync(`npm view ${PACKAGE_NAME} version --registry ${NPM_REGISTRY}`, { encoding: "utf8", + timeout: PROBE_TIMEOUT_MS, }); return result.trim(); } +export function isSkillsCliAvailable(): boolean { + try { + execSync("npx --no-install skills --version", { + stdio: ["ignore", "ignore", "ignore"], + timeout: PROBE_TIMEOUT_MS, + }); + return true; + } catch { + return false; + } +} + +export async function isOnline(timeoutMs = PROBE_TIMEOUT_MS): Promise { + let host: string; + try { + host = new URL(NPM_REGISTRY).hostname; + } catch { + return false; + } + + return new Promise((resolve) => { + const timer = setTimeout(() => resolve(false), timeoutMs); + timer.unref(); + dns.lookup(host, (err) => { + clearTimeout(timer); + resolve(!err); + }); + }); +} + // ── Package manager detection ───────────────────────────────────────────────── export type PackageManager = "npm" | "yarn" | "pnpm" | "bun"; diff --git a/packages/mcp/test/cli/utils.test.ts b/packages/mcp/test/cli/utils.test.ts index d4eb114..c1bd928 100644 --- a/packages/mcp/test/cli/utils.test.ts +++ b/packages/mcp/test/cli/utils.test.ts @@ -1,7 +1,36 @@ -import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import * as fs from "node:fs"; import * as path from "node:path"; import * as os from "node:os"; + +// ── Module mocks ───────────────────────────────────────────────────────────── +// These are hoisted so `vi.mock` can reference them. They let the network- +// dependent helpers (`isOnline`, `isSkillsCliAvailable`) be tested +// deterministically without touching DNS or spawning `npx`. + +const { dnsLookupMock, execSyncMock } = vi.hoisted(() => ({ + dnsLookupMock: vi.fn(), + execSyncMock: vi.fn(), +})); + +vi.mock("node:dns", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + default: { ...actual, lookup: dnsLookupMock }, + lookup: dnsLookupMock, + }; +}); + +vi.mock("node:child_process", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + default: { ...actual, execSync: execSyncMock }, + execSync: execSyncMock, + }; +}); + import { readJson, writeJson, @@ -11,11 +40,14 @@ import { globalInstallCommand, globalUninstallCommand, formatShellCommand, + isOnline, + isSkillsCliAvailable, resolveProjectRoot, SKILLS_DIR, RULES_DIR, AGENTS_DIR, } from "../../src/cli/utils.js"; +import { NPM_REGISTRY } from "../../src/cli/constants.js"; let tmpDir: string; @@ -269,3 +301,118 @@ describe("bundled paths", () => { expect(AGENTS_DIR).toMatch(/agents$/); }); }); + +// ── isOnline ────────────────────────────────────────────────────────────────── +// `isOnline` wraps `dns.lookup` with a timeout. All tests below run against +// the mocked `dns.lookup` set up at the top of this file — they never touch +// real DNS, which keeps them deterministic on offline runners and CI machines +// that deny outbound network access. + +describe("isOnline", () => { + beforeEach(() => { + dnsLookupMock.mockReset(); + }); + + it("returns true when DNS resolution succeeds", async () => { + dnsLookupMock.mockImplementation((_host: string, callback: (err: Error | null) => void) => { + setImmediate(() => callback(null)); + }); + + await expect(isOnline()).resolves.toBe(true); + expect(dnsLookupMock).toHaveBeenCalledTimes(1); + }); + + it("returns false when DNS resolution errors", async () => { + dnsLookupMock.mockImplementation((_host: string, callback: (err: Error | null) => void) => { + setImmediate(() => callback(Object.assign(new Error("ENOTFOUND"), { code: "ENOTFOUND" }))); + }); + + await expect(isOnline()).resolves.toBe(false); + }); + + it("returns false when DNS never responds before the timeout", async () => { + dnsLookupMock.mockImplementation(() => { + // Never invoke the callback — simulate a hanging DNS query. + }); + + const start = Date.now(); + const result = await isOnline(30); + const elapsed = Date.now() - start; + + expect(result).toBe(false); + expect(elapsed).toBeGreaterThanOrEqual(25); + expect(elapsed).toBeLessThan(500); + }); + + it("looks up the hostname from NPM_REGISTRY", async () => { + const expectedHost = new URL(NPM_REGISTRY).hostname; + dnsLookupMock.mockImplementation((_host: string, callback: (err: Error | null) => void) => { + setImmediate(() => callback(null)); + }); + + await isOnline(); + + expect(dnsLookupMock).toHaveBeenCalledWith(expectedHost, expect.any(Function)); + }); + + it("settles once even if DNS responds after the timeout has already fired", async () => { + let dnsCallback: ((err: Error | null) => void) | null = null; + dnsLookupMock.mockImplementation((_host: string, callback: (err: Error | null) => void) => { + dnsCallback = callback; + }); + + const result = await isOnline(10); + expect(result).toBe(false); + + // Late DNS callback must not throw, log, or re-resolve the already- + // settled promise. This mirrors what happens when DNS responds after + // we have already given up waiting. + expect(() => dnsCallback?.(null)).not.toThrow(); + }); +}); + +// ── isSkillsCliAvailable ───────────────────────────────────────────────────── + +describe("isSkillsCliAvailable", () => { + beforeEach(() => { + execSyncMock.mockReset(); + }); + + it("returns true when `npx --no-install skills --version` exits successfully", () => { + execSyncMock.mockReturnValue(Buffer.from("0.1.0\n")); + + expect(isSkillsCliAvailable()).toBe(true); + expect(execSyncMock).toHaveBeenCalledTimes(1); + const [cmd] = execSyncMock.mock.calls[0]!; + expect(cmd).toBe("npx --no-install skills --version"); + }); + + it("returns false when the probe throws (skills CLI not in npx cache)", () => { + execSyncMock.mockImplementation(() => { + throw new Error("command failed"); + }); + + expect(isSkillsCliAvailable()).toBe(false); + }); + + it("fully silences stdio so nothing leaks to the terminal", () => { + execSyncMock.mockReturnValue(Buffer.from("")); + + isSkillsCliAvailable(); + + const opts = execSyncMock.mock.calls[0]![1] as + | { stdio?: [unknown, unknown, unknown] } + | undefined; + expect(opts?.stdio).toEqual(["ignore", "ignore", "ignore"]); + }); + + it("passes a timeout so a wedged npx cannot hang init forever", () => { + execSyncMock.mockReturnValue(Buffer.from("")); + + isSkillsCliAvailable(); + + const opts = execSyncMock.mock.calls[0]![1] as { timeout?: number } | undefined; + expect(typeof opts?.timeout).toBe("number"); + expect(opts!.timeout!).toBeGreaterThan(0); + }); +});