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/__tests__/connector/container-host.test.ts

@@ -0,0 +1,65 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+
+const mockIsContainerised = vi.fn();
+vi.mock("@/lib/connector/is-containerised", () => ({
+  isContainerised: () => mockIsContainerised(),
+}));
+
+import { resolveContainerHost } from "@/lib/connector/container-host";
+
+beforeEach(() => vi.clearAllMocks());
+
+describe("resolveContainerHost (#1346)", () => {
+  describe("inside a container", () => {
+    beforeEach(() => mockIsContainerised.mockReturnValue(true));
+
+    it.each([
+      ["neo4j://localhost:7688", "neo4j://host.docker.internal:7688"],
+      ["bolt://127.0.0.1:7687", "bolt://host.docker.internal:7687"],
+      ["neo4j://LOCALHOST:7687", "neo4j://host.docker.internal:7687"],
+    ])("rewrites %s", (input, expected) => {
+      expect(resolveContainerHost(input)).toBe(expected);
+    });
+
+    it("keeps credentials, port, path and query intact", () => {
+      // A rewrite that drops the password or the database name trades one
+      // failure for a more confusing one.
+      expect(
+        resolveContainerHost(
+          "postgresql://u:p%40ss@localhost:5432/app?sslmode=require",
+        ),
+      ).toBe(
+        "postgresql://u:p%40ss@host.docker.internal:5432/app?sslmode=require",
+      );
+    });
+
+    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"],
+      [
+        "a host merely containing 'localhost'",
+        "neo4j://my-localhost.example.com:7687",
+      ],
+    ])("leaves %s alone", (_l, uri) => {
+      expect(resolveContainerHost(uri)).toBe(uri);
+    });
+
+    it.each([
+      ["an unparseable uri", "not a uri"],
+      ["an empty string", ""],
+    ])("returns %s unchanged rather than throwing", (_l, uri) => {
+      expect(() => resolveContainerHost(uri)).not.toThrow();
+      expect(resolveContainerHost(uri)).toBe(uri);
+    });
+  });
+
+  it("leaves loopback alone when NOT containerised", () => {
+    // Local mode runs on the host, where localhost is exactly right. Rewriting
+    // it there would break a working setup.
+    mockIsContainerised.mockReturnValue(false);
+    expect(resolveContainerHost("neo4j://localhost:7688")).toBe(
+      "neo4j://localhost:7688",
+    );
+  });
+});

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.",
 };