From bf9254fead8772380cd53eb1477e1c7632f8b6ce Mon Sep 17 00:00:00 2001 From: B <6723574+louisgv@users.noreply.github.com> Date: Mon, 18 May 2026 03:07:51 +0000 Subject: [PATCH 1/2] fix(hermes): probe for dashboard subcommand before launching (#3407) The root cause of issue #3407 is that some hermes installs (older versions or partial installs on DigitalOcean) lack the `dashboard` subcommand entirely. The previous code launched `hermes dashboard` unconditionally and waited 60s for a timeout, producing a confusing argparse error. Add a capability probe (`hermes --help | grep dashboard`) before attempting the launch. If the subcommand is missing, bail immediately with a clear message instead of waiting for the timeout. Also surface the actual error message from runServer failures in the warning. Fixes #3407 Agent: code-health Co-Authored-By: Claude Sonnet 4.6 --- packages/cli/package.json | 2 +- .../src/__tests__/hermes-dashboard.test.ts | 46 +++++++++++++++++++ packages/cli/src/shared/agent-setup.ts | 10 +++- 3 files changed, 56 insertions(+), 2 deletions(-) diff --git a/packages/cli/package.json b/packages/cli/package.json index 7ddfc6674..2d64862a0 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -1,6 +1,6 @@ { "name": "@openrouter/spawn", - "version": "1.0.44", + "version": "1.0.45", "type": "module", "bin": { "spawn": "cli.js" diff --git a/packages/cli/src/__tests__/hermes-dashboard.test.ts b/packages/cli/src/__tests__/hermes-dashboard.test.ts index aee3f755f..4101e3e1f 100644 --- a/packages/cli/src/__tests__/hermes-dashboard.test.ts +++ b/packages/cli/src/__tests__/hermes-dashboard.test.ts @@ -6,6 +6,7 @@ */ import { afterEach, beforeEach, describe, expect, it, mock, spyOn } from "bun:test"; +import { isString } from "@openrouter/spawn-shared"; import { mockClackPrompts } from "./test-helpers"; // ── Mock @clack/prompts (must be before importing agent-setup) ────────── @@ -97,4 +98,49 @@ describe("startHermesDashboard", () => { expect(capturedScript).not.toContain("/etc/systemd/system/"); expect(capturedScript).not.toContain("crontab"); }); + + it("probes hermes --help for 'dashboard' subcommand before launching (issue #3407)", () => { + // If the installed hermes lacks the dashboard subcommand, we should bail + // early instead of launching and waiting 60s for a timeout. + expect(capturedScript).toContain("--help"); + expect(capturedScript).toContain("does not support the dashboard subcommand"); + // The probe must come BEFORE the setsid/nohup launch. + const probeIdx = capturedScript.indexOf("--help"); + const launchIdx = capturedScript.indexOf("setsid"); + expect(probeIdx).toBeLessThan(launchIdx); + }); +}); + +describe("startHermesDashboard — failure surfacing", () => { + let stderrSpy: ReturnType; + let warnings: string[]; + + beforeEach(() => { + warnings = []; + stderrSpy = spyOn(process.stderr, "write").mockImplementation((chunk) => { + const text = isString(chunk) ? chunk : new TextDecoder().decode(chunk); + warnings.push(text); + return true; + }); + }); + + afterEach(() => { + stderrSpy.mockRestore(); + }); + + it("includes the runServer error message in the warning so users can grep it", async () => { + const failing: CloudRunner = { + runServer: mock(async () => { + throw new Error("run_server failed (exit 1): hermes dashboard ..."); + }), + uploadFile: mock(async () => {}), + downloadFile: mock(async () => {}), + }; + // Should NOT throw — dashboard failure is non-fatal. + await startHermesDashboard(failing); + const combined = warnings.join(""); + // Surfaces the underlying cause, not a generic message. + expect(combined).toContain("run_server failed (exit 1)"); + expect(combined).toContain("TUI still available"); + }); }); diff --git a/packages/cli/src/shared/agent-setup.ts b/packages/cli/src/shared/agent-setup.ts index 50eb8d8da..f951257b0 100644 --- a/packages/cli/src/shared/agent-setup.ts +++ b/packages/cli/src/shared/agent-setup.ts @@ -728,6 +728,13 @@ export async function startHermesDashboard(runner: CloudRunner): Promise { hermesPath, `if ${portCheck}; then echo "Hermes dashboard already running on :9119"; exit 0; fi`, "_hermes_bin=$(command -v hermes) || { echo 'hermes not found in PATH' >&2; exit 1; }", + // Capability probe: bail early if the installed hermes doesn't have the + // `dashboard` subcommand (issue #3407 — older versions or partial installs + // lack it, causing a confusing argparse error after a 60s timeout). + 'if ! "$_hermes_bin" --help 2>&1 | grep -q "^[[:space:]]*dashboard"; then', + ' echo "hermes does not support the dashboard subcommand — skipping" >&2', + " exit 1", + "fi", // --no-open: we're on a remote VM, don't try to spawn a browser there. // --host 127.0.0.1: loopback-only; the SSH tunnel is how the user reaches it. "if command -v setsid >/dev/null 2>&1; then", @@ -749,7 +756,8 @@ export async function startHermesDashboard(runner: CloudRunner): Promise { logInfo("Hermes web dashboard started on :9119"); } else { // Non-fatal: the TUI still works even if the dashboard didn't come up. - logWarn("Hermes web dashboard failed to start — TUI still available"); + // Surface the error so users see the real cause (e.g. missing subcommand). + logWarn(`Hermes web dashboard failed to start — TUI still available (${getErrorMessage(result.error)})`); } } From 76f8b941e67389aeb22beb7705934610b1ea02f7 Mon Sep 17 00:00:00 2001 From: B <6723574+louisgv@users.noreply.github.com> Date: Mon, 18 May 2026 03:32:30 +0000 Subject: [PATCH 2/2] fix(tests): use URL-based mock routing to prevent fetch pollution across test files The digitalocean-token and hetzner-cov tests used callCount-based fetch mock routing, which broke when concurrent test files shared globalThis.fetch. Switch to URL-based routing so mock responses are determined by the request URL, not the order of all fetch calls across the process. Agent: code-health Co-Authored-By: Claude Sonnet 4.6 --- .../src/__tests__/digitalocean-token.test.ts | 24 +++--- .../cli/src/__tests__/hetzner-cov.test.ts | 84 ++++++++++--------- 2 files changed, 55 insertions(+), 53 deletions(-) diff --git a/packages/cli/src/__tests__/digitalocean-token.test.ts b/packages/cli/src/__tests__/digitalocean-token.test.ts index e0d329129..36dfbd63b 100644 --- a/packages/cli/src/__tests__/digitalocean-token.test.ts +++ b/packages/cli/src/__tests__/digitalocean-token.test.ts @@ -88,23 +88,20 @@ describe("doApi 401 OAuth recovery", () => { it("attempts OAuth recovery on 401 before throwing", async () => { state.token = "expired-token"; - let callCount = 0; + let apiCalls = 0; + let oauthCalls = 0; globalThis.fetch = mock((url: string | URL | Request) => { - callCount++; const urlStr = String(url); - // First call: the actual API call returning 401 - if (callCount === 1) { - return Promise.resolve( - new Response("Unauthorized", { - status: 401, - }), - ); - } - // Second call: OAuth connectivity check — fail it so tryDoOAuth returns null quickly + // OAuth connectivity check — fail it so tryDoOAuth returns null quickly // (avoids starting a real Bun.serve OAuth server) if (urlStr.includes("cloud.digitalocean.com")) { + oauthCalls++; return Promise.reject(new Error("network unavailable")); } + // Track only DO API calls (other test files may share globalThis.fetch) + if (urlStr.includes("api.digitalocean.com")) { + apiCalls++; + } return Promise.resolve( new Response("Unauthorized", { status: 401, @@ -114,8 +111,9 @@ describe("doApi 401 OAuth recovery", () => { // OAuth recovery fails (connectivity check fails), so doApi throws the 401 await expect(doApi("GET", "/account", undefined, 1)).rejects.toThrow("DigitalOcean API error 401"); - // Verify recovery was attempted: 1 API call + 1 connectivity check = 2 - expect(callCount).toBe(2); + // Verify recovery was attempted: 1 API call + 1 OAuth connectivity check + expect(apiCalls).toBe(1); + expect(oauthCalls).toBe(1); }); it("succeeds after OAuth recovery provides a new token", async () => { diff --git a/packages/cli/src/__tests__/hetzner-cov.test.ts b/packages/cli/src/__tests__/hetzner-cov.test.ts index ed1c050f1..3fe8fe3fa 100644 --- a/packages/cli/src/__tests__/hetzner-cov.test.ts +++ b/packages/cli/src/__tests__/hetzner-cov.test.ts @@ -585,21 +585,14 @@ describe("hetzner/createServer", () => { }, }, }; - let callCount = 0; - global.fetch = mock(() => { - callCount++; - if (callCount <= 1) { - // Token validation - return Promise.resolve( - new Response( - JSON.stringify({ - servers: [], - }), - ), - ); - } - if (callCount <= 2) { - // SSH keys + // Route by URL + method to avoid mock pollution from concurrent test files. + // POST /servers is called twice: first fails with resource_limit_exceeded, + // second succeeds after orphaned IP cleanup. + let serverPostCount = 0; + global.fetch = mock((url: string | URL | Request, init?: RequestInit) => { + const urlStr = String(url); + const method = init?.method ?? "GET"; + if (urlStr.includes("/ssh_keys")) { return Promise.resolve( new Response( JSON.stringify({ @@ -608,23 +601,15 @@ describe("hetzner/createServer", () => { ), ); } - if (callCount <= 3) { - // First create attempt — resource_limit_exceeded (HTTP 403) + if (urlStr.includes("/primary_ips/")) { + // Delete orphaned IP return Promise.resolve( - new Response( - JSON.stringify({ - error: { - code: "resource_limit_exceeded", - message: "primary_ip_limit", - }, - }), - { - status: 403, - }, - ), + new Response("", { + status: 204, + }), ); } - if (callCount <= 4) { + if (urlStr.includes("/primary_ips")) { // List primary IPs for cleanup return Promise.resolve( new Response( @@ -645,23 +630,42 @@ describe("hetzner/createServer", () => { ), ); } - if (callCount <= 5) { - // Delete orphaned IP 100 - return Promise.resolve( - new Response("", { - status: 204, - }), - ); + if (urlStr.includes("/servers") && method === "POST") { + serverPostCount++; + if (serverPostCount === 1) { + // First create attempt — resource_limit_exceeded (HTTP 403) + return Promise.resolve( + new Response( + JSON.stringify({ + error: { + code: "resource_limit_exceeded", + message: "primary_ip_limit", + }, + }), + { + status: 403, + }, + ), + ); + } + // Retry create — success + return Promise.resolve(new Response(JSON.stringify(serverResp))); } - // Retry create — success - return Promise.resolve(new Response(JSON.stringify(serverResp))); + // Default: token validation (GET /servers) and other GETs + return Promise.resolve( + new Response( + JSON.stringify({ + servers: [], + }), + ), + ); }); const { ensureHcloudToken, createServer } = await import("../hetzner/hetzner"); await ensureHcloudToken(); const conn = await createServer("test-retry", "cx23", "fsn1"); expect(conn.ip).toBe("10.0.0.5"); - // Should have called: token(1), ssh_keys(2), create-fail(3), list-ips(4), delete-ip(5), create-ok(6) - expect(callCount).toBeGreaterThanOrEqual(6); + // POST /servers was called twice: once failing, once succeeding + expect(serverPostCount).toBe(2); }); it("throws with guidance when resource limit hit and no orphaned IPs to clean", async () => {