fix(tunnel): surface daemon login failures + 401 status hint

src/commands/login.ts

@@ -31,9 +31,16 @@ export const loginCommand = new Command("login")
     // Step 2: best-effort daemon login. The hsh-tunneld daemon owns its
     // own token (it does its own gateway round-trip via IPC — we never
     // hand it the CLI's token, preserving the daemon-owns-token trust
-    // model). If the daemon isn't installed/running/configured this is
-    // a silent no-op, so a tunnel-less setup sees no change in behavior.
+    // model).
+    //
+    // If the daemon is genuinely not installed this is a silent no-op.
+    // But if the daemon IS installed and we fail to update its token
+    // (unreachable, unconfigured, login error), loginDaemon returns
+    // false and has already warned the user — we propagate a non-zero
+    // exit so the failure is visible in scripts and shells. The CLI
+    // login itself already succeeded, so this only flags the daemon leg.
     if (opts.tunnel) {
-      await loginDaemon({ browser: opts.browser, timeout: 180, optional: true });
+      const ok = await loginDaemon({ browser: opts.browser, timeout: 180, optional: true });
+      if (!ok) process.exitCode = 1;
     }
   });

src/commands/tunnel.ts

@@ -98,6 +98,18 @@ const statusSub = new Command("status")
         ...(s.last_error ? { "Last error": chalk.red(s.last_error) } : {}),
       });
       console.log();
+
+      // The daemon reports "authenticated" whenever it merely HOLDS a
+      // token — it can't know the token is valid until it dials the
+      // gateway. When that dial 401s (expired/rejected token) the only
+      // signal is a buried last_error string. Detect it and surface a
+      // concrete next step, otherwise the user sees "authenticated" +
+      // an opaque "401 Unauthorized" and has no idea the fix is to log
+      // in again.
+      if (isAuthError(s.last_error)) {
+        warn("The daemon's saved login has expired or was rejected by the gateway.");
+        info("Re-authenticate the daemon with:  hsh tunnel login");
+      }
     } catch (err) {
       renderApiError(err);
       process.exitCode = 1;
@@ -263,34 +275,96 @@ const loginSub = new Command("login")
  * auth method and run the matching OIDC or local-auth path, then report
  * the resulting tunnel state.
  *
- * `optional` controls the contract for the "daemon not usable" cases
- * (not installed, not running, no api_url configured):
+ * `optional` controls the contract for the "daemon not usable" cases:
  *
- *   - optional=false (interactive `hsh tunnel login`): these are hard
- *     errors — the user explicitly targeted the daemon, so we tell them
- *     exactly what to fix and return false.
+ *   - optional=false (interactive `hsh tunnel login`): every failure is
+ *     a hard error — the user explicitly targeted the daemon, so we
+ *     tell them exactly what to fix and return false.
  *
  *   - optional=true  (best-effort follow-up from the unified
- *     `hsh login`): these are silently skipped and return true. The
- *     user asked to log the *CLI* in; the daemon is a bonus, not a
- *     requirement, and a missing daemon must not fail `hsh login`.
+ *     `hsh login`): we ONLY skip silently when the daemon is genuinely
+ *     absent (no IPC socket on disk). When the daemon is installed but
+ *     unreachable, unconfigured, or its login fails, we WARN and return
+ *     false — silently swallowing those cases was the bug that left the
+ *     daemon on a stale/expired token while `hsh login` claimed success.
+ *
+ * Returns true only when the daemon login actually succeeded OR the
+ * daemon is genuinely absent (optional=true). Returns false on any real
+ * failure so the caller can set a non-zero exit code.
+ */
+/** Human-readable reason from a TunnelUnavailableError (or any error). */
+function unavailableReason(err: unknown): string {
+  if (err instanceof TunnelUnavailableError) {
+    return `${err.reason} (${err.message})`;
+  }
+  return errMessage(err);
+}
+
+function errMessage(err: unknown): string {
+  return err instanceof Error ? err.message : String(err);
+}
+
+/**
+ * Heuristic: does the daemon's last_error indicate the saved token was
+ * rejected by the gateway (as opposed to a network/DNS/route failure)?
  *
- * Returns true when the daemon login succeeded OR was legitimately
- * skipped (optional=true), false on a real failure.
+ * The daemon surfaces gateway HTTP failures verbatim in last_error
+ * (e.g. "bring up: fetch serverinfo: GET .../api/serverinfo returned
+ * 401 Unauthorized: {...access denied...}"). We match on the canonical
+ * auth signals — a 401 status or the gateway's "access denied" body —
+ * so `hsh tunnel status` can point the user at `hsh tunnel login`
+ * instead of leaving them staring at a raw HTTP error.
  */
+export function isAuthError(lastError: string | undefined): boolean {
+  if (!lastError) return false;
+  const e = lastError.toLowerCase();
+  return (
+    e.includes("401") ||
+    e.includes("unauthorized") ||
+    e.includes("access denied") ||
+    e.includes("token is expired") ||
+    e.includes("token expired")
+  );
+}
+
 export async function loginDaemon(opts: {
   browser: boolean;
   timeout: number;
   optional: boolean;
 }): Promise<boolean> {
+  // The `optional` (best-effort) contract distinguishes two very
+  // different "daemon not usable" situations:
+  //
+  //   - Genuinely ABSENT: no IPC socket on disk → the daemon isn't
+  //     installed/running. Skipping silently is correct; a tunnel-less
+  //     setup must not see noise from `hsh login`.
+  //
+  //   - Present but UNREACHABLE or its login fails: the socket exists
+  //     (daemon installed) but we can't read its control token, can't
+  //     connect, or the daemon-side login errors. Skipping silently
+  //     here is the bug that let `hsh login` leave the daemon on a
+  //     stale/expired token while reporting success — so we WARN and
+  //     fail loudly, pointing the user at `hsh tunnel login`.
+  //
+  // The discriminator is whether the socket file exists.
+  const daemonPresent = resolveSocketPath().exists;
+  const skipOrFail = (reason: string): boolean => {
+    if (opts.optional && !daemonPresent) {
+      // Truly not installed — silent best-effort skip.
+      return true;
+    }
+    warn(`Tunnel daemon login was not completed: ${reason}`);
+    info("Your CLI is logged in, but the tunnel daemon still holds its previous token.");
+    info("Run `hsh tunnel login` to update the daemon.");
+    return false;
+  };
+
   let client: TunnelClient;
   try {
     client = TunnelClient.connect();
   } catch (err) {
     if (opts.optional) {
-      // Daemon not installed/running — skip silently. The CLI login
-      // already succeeded; the tunnel is simply not part of this setup.
-      return true;
+      return skipOrFail(unavailableReason(err));
     }
     renderUnavailable(err);
     return false;
@@ -304,19 +378,20 @@ export async function loginDaemon(opts: {
     const cfg = await client.config();
     if (!cfg.api_url) {
       if (opts.optional) {
-        // Best-effort: nudge but don't fail. The user can wire the
-        // daemon up later with `hsh tunnel config set api-url`.
-        info("Tunnel daemon detected but not configured — skipping tunnel login.");
-        info("Configure it with: hsh tunnel config set api-url <url>");
-        return true;
+        // Daemon is reachable but unconfigured. This is a real
+        // "couldn't update the daemon" case, not an absent daemon, so
+        // surface it rather than swallow it.
+        warn("Tunnel daemon is installed but has no api_url configured.");
+        info("Configure it with: hsh tunnel config set api-url <url>, then run hsh tunnel login.");
+        return false;
       }
       error("Daemon has no api_url configured.");
       info("Set it first: hsh tunnel config set api-url <url>");
       return false;
     }
     apiUrl = cfg.api_url;
   } catch (err) {
-    if (opts.optional) return true;
+    if (opts.optional) return skipOrFail(errMessage(err));
     renderApiError(err);
     return false;
   }
@@ -329,7 +404,7 @@ export async function loginDaemon(opts: {
   try {
     serverInfo = await getPublicServerInfo(apiUrl);
   } catch (err) {
-    if (opts.optional) return true;
+    if (opts.optional) return skipOrFail(errMessage(err));
     error(`Could not detect auth method: ${(err as Error).message}`);
     info(`Verify the api-url: hsh tunnel config get`);
     return false;

tests/login-daemon.test.ts

@@ -0,0 +1,73 @@
+/**
+ * tests/login-daemon.test.ts — regression tests for the unified
+ * `hsh login` daemon leg (loginDaemon).
+ *
+ * The bug being guarded: with optional=true, loginDaemon used to skip
+ * silently whenever the daemon couldn't be reached — including when the
+ * daemon WAS installed but its control token was unreadable. That left
+ * the daemon on a stale/expired token while `hsh login` reported
+ * success, forcing users to run `hsh tunnel login` manually.
+ *
+ * The fix: only skip silently when the daemon is GENUINELY ABSENT (no
+ * IPC socket on disk). When the socket exists but the login can't be
+ * completed, loginDaemon must return false (caller exits non-zero).
+ *
+ * We drive the real loginDaemon and steer the "is the daemon present"
+ * discriminator via the HSH_TUNNELD_SOCKET env override that
+ * resolveSocketPath honors. No real daemon is ever contacted: when the
+ * socket path points at a non-socket file, TunnelClient.connect throws
+ * before any network I/O (no control token), which is exactly the
+ * "present but unusable" branch we want to assert on.
+ */
+
+import { afterEach, beforeEach, describe, expect, test } from "bun:test";
+import { mkdtempSync, rmSync, writeFileSync } from "fs";
+import { tmpdir } from "os";
+import { join } from "path";
+
+import { loginDaemon } from "../src/commands/tunnel.ts";
+
+let tmp: string;
+const savedSocket = process.env.HSH_TUNNELD_SOCKET;
+const savedToken = process.env.HSH_TUNNELD_TOKEN_FILE;
+
+beforeEach(() => {
+  tmp = mkdtempSync(join(tmpdir(), "hsh-login-daemon-"));
+});
+
+afterEach(() => {
+  if (savedSocket === undefined) delete process.env.HSH_TUNNELD_SOCKET;
+  else process.env.HSH_TUNNELD_SOCKET = savedSocket;
+  if (savedToken === undefined) delete process.env.HSH_TUNNELD_TOKEN_FILE;
+  else process.env.HSH_TUNNELD_TOKEN_FILE = savedToken;
+  rmSync(tmp, { recursive: true, force: true });
+});
+
+describe("loginDaemon optional contract", () => {
+  test("genuinely absent daemon (no socket) → silent skip, returns true", async () => {
+    // Point at a socket path that does not exist on disk.
+    process.env.HSH_TUNNELD_SOCKET = join(tmp, "does-not-exist.sock");
+    delete process.env.HSH_TUNNELD_TOKEN_FILE;
+
+    const ok = await loginDaemon({ browser: false, timeout: 1, optional: true });
+    // Absent daemon must NOT fail `hsh login`.
+    expect(ok).toBe(true);
+  });
+
+  test("installed daemon we can't authenticate against → returns false (no silent skip)", async () => {
+    // Create a real file at the socket path so resolveSocketPath reports
+    // exists=true (daemon "present"), but provide no readable control
+    // token so connect() throws — the exact "present but unusable" case
+    // that previously skipped silently.
+    const sockPath = join(tmp, "hsh.sock");
+    writeFileSync(sockPath, "");
+    process.env.HSH_TUNNELD_SOCKET = sockPath;
+    // Token file points at a missing path → connect() throws no-token.
+    process.env.HSH_TUNNELD_TOKEN_FILE = join(tmp, "missing-token");
+
+    const ok = await loginDaemon({ browser: false, timeout: 1, optional: true });
+    // Present-but-unusable must surface as a failure so the caller
+    // exits non-zero and the user knows to run `hsh tunnel login`.
+    expect(ok).toBe(false);
+  });
+});

tests/tunnel-status-auth-hint.test.ts

@@ -0,0 +1,43 @@
+/**
+ * tests/tunnel-status-auth-hint.test.ts — unit tests for isAuthError,
+ * the heuristic that decides whether `hsh tunnel status` should hint
+ * "re-run hsh tunnel login" based on the daemon's last_error.
+ *
+ * The daemon reports "authenticated" whenever it merely holds a token;
+ * a rejected/expired token only shows up as a buried HTTP error in
+ * last_error. isAuthError extracts the auth signal so the UI can give a
+ * concrete next step.
+ */
+
+import { describe, expect, test } from "bun:test";
+
+import { isAuthError } from "../src/commands/tunnel.ts";
+
+describe("isAuthError", () => {
+  test("matches the real serverinfo 401 last_error", () => {
+    const real =
+      'bring up: fetch serverinfo: GET https://sandbox.hoop.dev/api/serverinfo ' +
+      'returned 401 Unauthorized: {"message":"access denied"}';
+    expect(isAuthError(real)).toBe(true);
+  });
+
+  test("matches a bare 401 / unauthorized / access denied / expired", () => {
+    expect(isAuthError("got 401 from gateway")).toBe(true);
+    expect(isAuthError("Unauthorized")).toBe(true);
+    expect(isAuthError("access denied")).toBe(true);
+    expect(isAuthError("token is expired")).toBe(true);
+    expect(isAuthError("token expired")).toBe(true);
+  });
+
+  test("does NOT match non-auth failures", () => {
+    expect(isAuthError("dial tcp: connection refused")).toBe(false);
+    expect(isAuthError("configure routes: permission denied")).toBe(false);
+    expect(isAuthError("fetch serverinfo: context deadline exceeded")).toBe(false);
+    expect(isAuthError("no tunnelable connections found for this user")).toBe(false);
+  });
+
+  test("returns false for empty / undefined", () => {
+    expect(isAuthError(undefined)).toBe(false);
+    expect(isAuthError("")).toBe(false);
+  });
+});