fix(app): tell the user localhost means the container, not their machine (#1346)

app/src/app/api/connections/[id]/test/route.ts

@@ -11,6 +11,7 @@ import {
   connectionCheckFalseResult,
   connectionTestErrorResult,
 } from "@/lib/connector/connection-test-result";
+import { isContainerised } from "@/lib/connector/is-containerised";
 
 export async function POST(
   _request: Request,
@@ -60,7 +61,12 @@ export async function POST(
         success ? { success: true } : connectionCheckFalseResult(),
       );
     } catch (testError) {
-      return apiSuccess(connectionTestErrorResult(testError));
+      return apiSuccess(
+        connectionTestErrorResult(testError, {
+          uri: credentials.uri,
+          containerised: isContainerised(),
+        }),
+      );
     }
   } catch (error) {
     return handleRouteError(error, "Connection test failed");

app/src/app/api/connections/test-inline/route.ts

@@ -9,6 +9,7 @@ import {
   connectionCheckFalseResult,
   connectionTestErrorResult,
 } from "@/lib/connector/connection-test-result";
+import { isContainerised } from "@/lib/connector/is-containerised";
 
 export async function POST(request: Request) {
   try {
@@ -43,7 +44,12 @@ export async function POST(request: Request) {
         success ? { success: true } : connectionCheckFalseResult(),
       );
     } catch (testError) {
-      return apiSuccess(connectionTestErrorResult(testError));
+      return apiSuccess(
+        connectionTestErrorResult(testError, {
+          uri: config.uri,
+          containerised: isContainerised(),
+        }),
+      );
     }
   } catch (error) {
     return handleRouteError(error, "Connection test failed");

app/src/lib/connector/__tests__/connection-error-classifier.test.ts

@@ -117,3 +117,105 @@ describe("hintForConnectionErrorCode", () => {
     }
   });
 });
+
+// The most common thing a user does after `neoboard demo` is connect their own
+// database. On a Docker install that database is on the HOST, so they type
+// neo4j://localhost:7688 — and localhost inside the app container is the
+// container. The driver says "Could not perform discovery. No routing servers
+// available", which classified as `network`, whose hint told them to verify the
+// host, the port, that the database is running, and their firewall. All four
+// are already correct. There was no thread to pull (#1346).
+describe("loopback from inside a container (#1346)", () => {
+  const DISCOVERY_ERROR =
+    "Could not perform discovery. No routing servers available.";
+
+  it.each([
+    ["localhost", "neo4j://localhost:7688"],
+    ["127.0.0.1", "postgresql://127.0.0.1:5432/app"],
+    ["::1", "neo4j://[::1]:7687"],
+    ["with credentials in the URI", "postgresql://u:p@localhost:5432/app"],
+    ["uppercase host", "neo4j://LOCALHOST:7687"],
+  ])("codes a network failure to %s as container_loopback", (_l, uri) => {
+    expect(
+      classifyConnectionError(DISCOVERY_ERROR, { uri, containerised: true }),
+    ).toBe("container_loopback");
+  });
+
+  it("stays `network` when the app is NOT containerised", () => {
+    // The regression that matters. In local mode the app runs on the host,
+    // where localhost is exactly right — telling that user to use a Docker
+    // hostname would send them somewhere that does not exist.
+    expect(
+      classifyConnectionError(DISCOVERY_ERROR, {
+        uri: "neo4j://localhost:7688",
+        containerised: false,
+      }),
+    ).toBe("network");
+  });
+
+  it.each([
+    ["a remote host", "neo4j://db.example.com:7687"],
+    ["a compose service name", "neo4j://neo4j:7687"],
+    ["a LAN address", "postgresql://192.168.1.50:5432/app"],
+  ])("stays `network` for %s", (_l, uri) => {
+    expect(
+      classifyConnectionError(DISCOVERY_ERROR, { uri, containerised: true }),
+    ).toBe("network");
+  });
+
+  it("does not outrank auth or bad_uri", () => {
+    // Priority order is unchanged: a loopback auth failure is still an auth
+    // failure, and the Docker hint would be a misdiagnosis.
+    expect(
+      classifyConnectionError("Authentication failure", {
+        uri: "neo4j://localhost:7688",
+        containerised: true,
+      }),
+    ).toBe("auth_failed");
+    expect(
+      classifyConnectionError("Invalid URI scheme", {
+        uri: "wat://localhost:7688",
+        containerised: true,
+      }),
+    ).toBe("bad_uri");
+  });
+
+  it.each([
+    ["a malformed URI", "not a uri at all"],
+    ["an empty URI", ""],
+  ])("degrades to `network` for %s rather than throwing", (_l, uri) => {
+    // This runs on an error path. A classifier that throws replaces a bad
+    // message with a 500.
+    expect(() =>
+      classifyConnectionError(DISCOVERY_ERROR, { uri, containerised: true }),
+    ).not.toThrow();
+    expect(
+      classifyConnectionError(DISCOVERY_ERROR, { uri, containerised: true }),
+    ).toBe("network");
+  });
+
+  it("is unchanged when no context is passed at all", () => {
+    expect(classifyConnectionError(DISCOVERY_ERROR)).toBe("network");
+  });
+
+  it("names Docker, the CLI flag, and host.docker.internal in the hint", () => {
+    const hint = hintForConnectionErrorCode("container_loopback");
+    expect(hint).toMatch(/host\.docker\.internal/);
+    expect(hint).toMatch(/container/i);
+    // The hostname only resolves on Linux when the overlay is applied, so the
+    // hint has to say how to apply it.
+    expect(hint).toMatch(/--expose-host/);
+  });
+
+  it("says WHOSE localhost, because the URI is resolved server-side", () => {
+    // The connection is opened by the NeoBoard server, not the browser. On a
+    // deployed instance, a user typing `localhost` means the SERVER's
+    // localhost — and host.docker.internal is the server's host too, not
+    // theirs. A hint saying "not your machine" reads as though their own
+    // laptop were reachable. It is not, and the copy has to say so.
+    const hint = hintForConnectionErrorCode("container_loopback");
+    expect(hint).toMatch(/server/i);
+    expect(hint).toMatch(/your own computer/i);
+    expect(hint).toMatch(/not reachable|cannot see your machine/i);
+  });
+});

app/src/lib/connector/__tests__/connection-test-result.test.ts

@@ -32,4 +32,35 @@ describe("connection-test-result (#1043)", () => {
     expect(r.success).toBe(false);
     expect(r.code).toBe("unknown");
   });
+
+  // The URI has to reach the classifier for it to spot a Docker networking
+  // miss — the route already has it, and passing it is the whole wiring (#1346).
+  it("passes the URI and container flag through to the classifier", () => {
+    expect(
+      connectionTestErrorResult(
+        new Error("Could not perform discovery. No routing servers available."),
+        { uri: "neo4j://localhost:7688", containerised: true },
+      ).code,
+    ).toBe("container_loopback");
+  });
+
+  it("still classifies as network when no context is given", () => {
+    // Both call sites must keep working unchanged if the context is absent.
+    expect(
+      connectionTestErrorResult(
+        new Error("Could not perform discovery. No routing servers available."),
+      ).code,
+    ).toBe("network");
+  });
+
+  it("never echoes the URI into the user-facing error", () => {
+    // A URI can carry a password. The classifier reads it; the result must not
+    // carry it back out.
+    const r = connectionTestErrorResult(new Error("ECONNREFUSED"), {
+      uri: "postgresql://admin:hunter2@localhost:5432/app",
+      containerised: true,
+    });
+    expect(JSON.stringify(r)).not.toContain("hunter2");
+    expect(JSON.stringify(r)).not.toContain("localhost:5432");
+  });
 });

app/src/lib/connector/__tests__/is-containerised.test.ts

@@ -0,0 +1,44 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+
+const mockExistsSync = vi.fn();
+vi.mock("node:fs", () => ({ existsSync: (p: string) => mockExistsSync(p) }));
+
+import { isContainerised, _resetContainerisedCache } from "../is-containerised";
+
+beforeEach(() => {
+  vi.clearAllMocks();
+  _resetContainerisedCache();
+});
+
+describe("isContainerised (#1346)", () => {
+  it("reports true when /.dockerenv exists", () => {
+    mockExistsSync.mockReturnValue(true);
+    expect(isContainerised()).toBe(true);
+    expect(mockExistsSync).toHaveBeenCalledWith("/.dockerenv");
+  });
+
+  it("reports false on a host install", () => {
+    // The case that keeps the hint honest: a local-mode user pointing at
+    // localhost is correct, and must not be told to use a Docker hostname.
+    mockExistsSync.mockReturnValue(false);
+    expect(isContainerised()).toBe(false);
+  });
+
+  it("checks the filesystem only once", () => {
+    // Called from an error path; the answer cannot change while the process
+    // runs, so a stat per failed connection test would be pure waste.
+    mockExistsSync.mockReturnValue(true);
+    isContainerised();
+    isContainerised();
+    isContainerised();
+    expect(mockExistsSync).toHaveBeenCalledTimes(1);
+  });
+
+  it("caches false as firmly as true", () => {
+    // `cached ??= …` treats a cached false as unset if written naively.
+    mockExistsSync.mockReturnValue(false);
+    expect(isContainerised()).toBe(false);
+    expect(isContainerised()).toBe(false);
+    expect(mockExistsSync).toHaveBeenCalledTimes(1);
+  });
+});

app/src/lib/connector/connection-error-classifier.ts

@@ -15,8 +15,44 @@ export type ConnectionErrorCode =
   | "auth_failed"
   | "network"
   | "bad_uri"
+  /** A loopback host, unreachable because we are inside a container (#1346). */
+  | "container_loopback"
   | "unknown";
 
+/** What the classifier needs beyond the message to spot a Docker networking miss. */
+export interface ConnectionErrorContext {
+  /** The URI the user entered. */
+  uri?: string;
+  /** Whether the app itself is running inside a container. */
+  containerised?: boolean;
+}
+
+/**
+ * Is this URI pointed at the machine it is running on?
+ *
+ * Parsed, not substring-matched: "myhost-localhost.example.com" contains
+ * "localhost" and is not loopback. Returns false for anything unparseable —
+ * this runs on an error path, where a throw would replace a bad message with
+ * a 500, and a malformed URI is already better served by `bad_uri`.
+ */
+function isLoopbackUri(uri: string | undefined): boolean {
+  if (!uri) return false;
+  let host: string;
+  try {
+    host = new URL(uri).hostname.toLowerCase();
+  } catch {
+    return false;
+  }
+  // URL strips the brackets from [::1]; both forms normalise to "::1".
+  return (
+    host === "localhost" ||
+    host === "127.0.0.1" ||
+    host === "::1" ||
+    host === "[::1]" ||
+    host.endsWith(".localhost")
+  );
+}
+
 /**
  * Shown when a connector's check returns false *without* throwing — there's no
  * driver message to classify, so the old "Connection check returned false" was
@@ -77,13 +113,27 @@ function containsAny(text: string, keywords: string[]): boolean {
  * Why auth above network: failed auth attempts can be reported on top of
  * transient network warnings; the user's first step is to fix credentials.
  */
-export function classifyConnectionError(message: string): ConnectionErrorCode {
+export function classifyConnectionError(
+  message: string,
+  context?: ConnectionErrorContext,
+): ConnectionErrorCode {
   if (!message) return "unknown";
   const m = message.toLowerCase();
 
   if (containsAny(m, BAD_URI_KEYWORDS)) return "bad_uri";
   if (containsAny(m, AUTH_KEYWORDS)) return "auth_failed";
-  if (containsAny(m, NETWORK_KEYWORDS)) return "network";
+  if (containsAny(m, NETWORK_KEYWORDS)) {
+    // Narrowing a network failure, never overriding auth or bad_uri: a
+    // loopback auth failure is still an auth failure, and pointing at Docker
+    // there would be a misdiagnosis.
+    //
+    // The containerised check is what keeps this honest. In local mode the app
+    // runs on the host, where localhost is exactly right — that user must not
+    // be sent to a Docker hostname that does not exist for them.
+    return context?.containerised && isLoopbackUri(context.uri)
+      ? "container_loopback"
+      : "network";
+  }
   return "unknown";
 }
 
@@ -94,6 +144,8 @@ const HINTS: Record<ConnectionErrorCode, string> = {
     "The server is unreachable. Verify the host and port, confirm the database is running, and check that no firewall is blocking the connection.",
   bad_uri:
     "The connection URI looks malformed. Confirm the scheme (e.g. `bolt://` or `neo4j+s://` for Neo4j, `postgresql://` for PostgreSQL) and that the host/port are present.",
+  container_loopback:
+    "The connection is opened by the NeoBoard **server**, not by your browser — so `localhost` means the machine NeoBoard runs on, and right now that is the container it runs inside. If the database is on that same host, restart NeoBoard with `neoboard start --full --expose-host` and use `host.docker.internal` in place of `localhost` (e.g. `neo4j://host.docker.internal:7687`). A database in the same Docker network is reached by its service name. If the database is on **your own computer** and NeoBoard is deployed elsewhere, it is not reachable at all — the server cannot see your machine; expose it at a routable address first.",
   unknown:
     "Connection test failed for an unrecognised reason. Check the server logs for more detail.",
 };

app/src/lib/connector/connection-test-result.ts

@@ -3,6 +3,7 @@ import {
   classifyConnectionError,
   CONNECTION_CHECK_FALSE_MESSAGE,
   type ConnectionErrorCode,
+  type ConnectionErrorContext,
 } from "@/lib/connector/connection-error-classifier";
 
 /**
@@ -28,11 +29,14 @@ export function connectionCheckFalseResult(): ConnectionTestResult {
 /** A thrown driver error — classify for a targeted hint, then sanitize for display. */
 export function connectionTestErrorResult(
   thrown: unknown,
+  context?: ConnectionErrorContext,
 ): ConnectionTestResult {
   const rawMessage =
     thrown instanceof Error ? thrown.message : "Connection test failed";
   // Classify BEFORE sanitization — the classifier needs the raw driver text.
-  const code = classifyConnectionError(rawMessage);
+  // The context is read here and never returned: a URI can carry a password,
+  // so it informs the code and goes no further (#1346).
+  const code = classifyConnectionError(rawMessage, context);
   const error = sanitizeErrorMessage(rawMessage, "Connection test failed");
   return { success: false, code, error };
 }

app/src/lib/connector/is-containerised.ts

@@ -0,0 +1,25 @@
+import { existsSync } from "node:fs";
+
+/**
+ * Is this process running inside a container?
+ *
+ * Used to tell two identical-looking failures apart: a connection to
+ * `localhost` that fails from the host is a real network problem, while the
+ * same failure from inside a container usually means `localhost` resolved to
+ * the container rather than the user's machine (#1346).
+ *
+ * `/.dockerenv` is written by the Docker runtime and is the cheap, stable
+ * signal. Evaluated once — the answer cannot change while the process runs,
+ * and this is called from an error path.
+ */
+let cached: boolean | undefined;
+
+export function isContainerised(): boolean {
+  cached ??= existsSync("/.dockerenv");
+  return cached;
+}
+
+/** @internal — test-only, since the answer is cached for the process lifetime. */
+export function _resetContainerisedCache(): void {
+  cached = undefined;
+}

cli/src/__tests__/commands/start.test.ts

@@ -89,7 +89,10 @@ describe("runStart", () => {
 
   it("starts DB containers (not full stack) in docker mode", async () => {
     await runStart();
-    expect(mockComposeUp).toHaveBeenCalledWith({ full: false });
+    expect(mockComposeUp).toHaveBeenCalledWith({
+      full: false,
+      exposeHost: false,
+    });
   });
 
   it("skips composeUp in local mode", async () => {
@@ -270,4 +273,14 @@ describe("runStart", () => {
     const lines = mockBanner.mock.calls[0][0];
     expect(lines.some((l) => l.includes("docker/.env"))).toBe(false);
   });
+
+  it("passes --expose-host through to compose (#1346)", () => {
+    // Off by default above; on only when asked for.
+    return runStart({ full: true, exposeHost: true }).then(() => {
+      expect(mockComposeUp).toHaveBeenCalledWith({
+        full: true,
+        exposeHost: true,
+      });
+    });
+  });
 });