fix(hermes): decide broker reuse from ownership

The managed-tool broker decided it could reuse an existing broker from
three conditions, and one of them checked only that the health endpoint
answered. The other two already covered both ownership proofs, so that
branch was reachable only when NemoClaw did not own the listener. It also
set `brokerStartedThisRun`, which feeds `currentBrokerOwned` module-wide,
so one adopting call suppressed the clone preflight's ownership refusal
for the rest of the process.

`planHermesToolGatewayBrokerReuse` now makes that decision in one place,
next to the existing refresh planner. `ensureHermesToolGatewayBroker`
refuses a listener it cannot prove it owns before any path adopts it,
restarts around it, or stages credentials against it, and names the held
port instead of failing silently.

Reuse of a broker started by another NemoClaw process still works through
the recorded pid. A lost pid file with a live broker now reports no usable
broker rather than adopting it, because the two states are
indistinguishable until `/health` authenticates the broker token.

Refs #9304

Signed-off-by: 1PoPTRoN <vrxn.arp1traj@gmail.com>

Author: 1PoPTRoN

src/lib/hermes-tool-gateway-broker.ts

@@ -63,6 +63,8 @@ const HERMES_TOOL_GATEWAY_CONTROL_CONTRACT_PATH = path.join(
 );
 const HERMES_TOOL_GATEWAY_RUNTIME_MISMATCH_RECOVERY =
   "Reauthorize every managed-tool Hermes sandbox, then retry.";
+const HERMES_TOOL_GATEWAY_UNOWNED_LISTENER_RECOVERY =
+  "Stop the process holding that port, then retry.";
 const HERMES_TOOL_GATEWAY_CONTROL_CLIENT_SOURCE = [
   'const http = require("node:http");',
   "const [socketPath, route, timeoutValue] = process.argv.slice(1);",
@@ -730,6 +732,36 @@ function planHermesToolGatewayBrokerRefresh({
   return "start-or-restart";
 }
 
+/**
+ * Decide whether the listener already on HERMES_TOOL_GATEWAY_PORT may be
+ * adopted as this run's broker.
+ *
+ * sourceOfTruth: This is the only place that converts broker reachability into
+ * a reuse decision for the no-credential path.
+ * invalidState: `/health` is unauthenticated and binds a fixed port, so a
+ * reachable endpoint proves liveness only. Ownership must come from the pid
+ * file or from a broker this process started; reachability must never stand in
+ * for identity, and must never latch `brokerStartedThisRun` for later callers.
+ * regressionTest: test/hermes-tool-gateway-broker-unowned-listener.test.ts.
+ * removalCondition: remove the "refuse-unowned-listener" outcome only when the
+ * broker authenticates `/health` with the per-sandbox broker token, so that a
+ * probe can prove identity instead of reachability.
+ */
+function planHermesToolGatewayBrokerReuse({
+  brokerHealthy,
+  currentBrokerOwned,
+  forceRestart = false,
+  hashMatches,
+}) {
+  if (brokerHealthy && !currentBrokerOwned) {
+    return "refuse-unowned-listener";
+  }
+  if (!forceRestart && hashMatches && brokerHealthy) {
+    return "reuse-current";
+  }
+  return "no-usable-broker";
+}
+
 function ensureHermesToolGatewayBroker(options = {}) {
   const refreshToken =
     typeof options.refreshToken === "string" && options.refreshToken.trim()
@@ -739,7 +771,27 @@ function ensureHermesToolGatewayBroker(options = {}) {
   const hashMatches = readBrokerHash() === desiredHash;
   const pid = readPid();
   const currentBrokerOwned = isHermesToolGatewayBrokerProcess(pid) || brokerStartedThisRun;
-  const currentBrokerHealthy = currentBrokerOwned && isHermesToolGatewayBrokerHealthy();
+  const brokerHealthy = isHermesToolGatewayBrokerHealthy();
+  const currentBrokerHealthy = currentBrokerOwned && brokerHealthy;
+  const reusePlan = planHermesToolGatewayBrokerReuse({
+    brokerHealthy,
+    currentBrokerOwned,
+    forceRestart: options.forceRestart,
+    hashMatches,
+  });
+  // Refuse before any path can adopt, restart around, or stage credentials
+  // against a listener NemoClaw cannot prove it owns. The clone preflight
+  // already rejects this state; every entry point has to agree, because a
+  // single adopting path latches `brokerStartedThisRun` and thereby satisfies
+  // the ownership test for the rest of the process.
+  if (reusePlan === "refuse-unowned-listener") {
+    console.error(
+      "Hermes managed-tool broker health endpoint is not owned by NemoClaw; " +
+        `refusing to reuse the listener on port ${HERMES_TOOL_GATEWAY_PORT}. ` +
+        HERMES_TOOL_GATEWAY_UNOWNED_LISTENER_RECOVERY,
+    );
+    return false;
+  }
   if (options.startWithoutCredential) {
     if (currentBrokerHealthy) {
       return hashMatches && fs.existsSync(HERMES_TOOL_GATEWAY_CONTROL_SOCKET_PATH);
@@ -799,24 +851,10 @@ function ensureHermesToolGatewayBroker(options = {}) {
     return false;
   }
 
-  if (
-    !options.forceRestart &&
-    hashMatches &&
-    brokerStartedThisRun &&
-    isHermesToolGatewayBrokerHealthy()
-  ) {
-    return true;
-  }
-  if (
-    !options.forceRestart &&
-    hashMatches &&
-    isHermesToolGatewayBrokerProcess(pid) &&
-    isHermesToolGatewayBrokerHealthy()
-  ) {
-    brokerStartedThisRun = true;
-    return true;
-  }
-  if (!options.forceRestart && hashMatches && isHermesToolGatewayBrokerHealthy()) {
+  // `currentBrokerOwned` already covers both ownership proofs the three former
+  // branches tested separately (`brokerStartedThisRun` and a live broker pid),
+  // and the unowned case returned above, so reuse is one decision.
+  if (reusePlan === "reuse-current") {
     brokerStartedThisRun = true;
     return true;
   }
@@ -913,6 +951,7 @@ module.exports = {
   discardHermesToolGatewayCloneBinding,
   bindHermesToolGatewayCloneProviderState,
   planHermesToolGatewayBrokerRefresh,
+  planHermesToolGatewayBrokerReuse,
   isHermesToolGatewayBrokerHealthy,
   killStaleHermesToolGatewayBroker,
   ensureHermesToolGatewayBroker,

test/helpers/hermes-tool-gateway-broker-ownership-fixture.ts

@@ -0,0 +1,56 @@
+// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+//
+// Support for the broker ownership regression. The branching setup lives here
+// so the test bodies stay linear, which `tools/growth-guardrails/test-conditionals.mts`
+// requires of every `*.test.ts` file.
+
+import http from "node:http";
+
+/** A listener that answers the broker health probe and serves nothing else. */
+export function createUnownedHealthListener(): http.Server {
+  return http.createServer((req, res) => {
+    if (req.url === "/health") {
+      res.writeHead(200, { "Content-Type": "application/json" });
+      res.end(JSON.stringify({ ok: true, services: [] }));
+      return;
+    }
+    res.writeHead(404).end();
+  });
+}
+
+export function listenOn(server: http.Server, port: number): Promise<void> {
+  return new Promise((resolve, reject) => {
+    server.once("error", reject);
+    server.listen(port, "127.0.0.1", () => resolve());
+  });
+}
+
+export function closeServer(server: http.Server): Promise<void> {
+  return new Promise((resolve) => server.close(() => resolve()));
+}
+
+/** Wait for a killed broker to release the fixed managed-tool gateway port. */
+export async function waitForPortFree(port: number): Promise<void> {
+  const deadline = Date.now() + 10_000;
+  while (Date.now() < deadline) {
+    const probe = http.createServer();
+    const free = await new Promise<boolean>((resolve) => {
+      probe.once("error", () => resolve(false));
+      probe.listen(port, "127.0.0.1", () => resolve(true));
+    });
+    await closeServer(probe);
+    if (free) return;
+    await new Promise((resolve) => setTimeout(resolve, 100));
+  }
+  throw new Error(`port ${port} was still held after the staged broker was killed`);
+}
+
+/** Restore a captured environment value, including the previously-unset case. */
+export function restoreEnv(key: string, previous: string | undefined): void {
+  if (previous === undefined) {
+    delete process.env[key];
+    return;
+  }
+  process.env[key] = previous;
+}

test/hermes-tool-gateway-broker-unowned-listener.test.ts

@@ -0,0 +1,222 @@
+// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+import fs from "node:fs";
+import { createRequire } from "node:module";
+import os from "node:os";
+import path from "node:path";
+import {
+  closeServer,
+  createUnownedHealthListener,
+  listenOn,
+  restoreEnv,
+  waitForPortFree,
+} from "./helpers/hermes-tool-gateway-broker-ownership-fixture";
+import { describe, expect, test as it } from "./helpers/owned-test-resources";
+import { testTimeout } from "./helpers/timeouts";
+
+const require = createRequire(import.meta.url);
+const BROKER_WRAPPER = path.join(
+  import.meta.dirname,
+  "..",
+  "src",
+  "lib",
+  "hermes-tool-gateway-broker.ts",
+);
+
+const BROKER_TEST_TIMEOUT_MS = testTimeout(45_000);
+
+/** Every reuse input that must still refuse a listener NemoClaw does not own. */
+const UNOWNED_LISTENER_CASES = [
+  { label: "a matching hash", forceRestart: false, hashMatches: true },
+  { label: "a mismatched hash", forceRestart: false, hashMatches: false },
+  { label: "a restart request", forceRestart: true, hashMatches: true },
+  { label: "a restart request and a mismatched hash", forceRestart: true, hashMatches: false },
+];
+
+/** Every reuse input that resolves to an ordinary restart when the port is dead. */
+const UNREACHABLE_PORT_CASES = [
+  { label: "an unowned broker and a matching hash", currentBrokerOwned: false, forceRestart: false, hashMatches: true },
+  { label: "an unowned broker and a mismatched hash", currentBrokerOwned: false, forceRestart: false, hashMatches: false },
+  { label: "an unowned broker and a restart request", currentBrokerOwned: false, forceRestart: true, hashMatches: true },
+  { label: "an unowned broker, a restart request, and a mismatched hash", currentBrokerOwned: false, forceRestart: true, hashMatches: false },
+  { label: "an owned broker and a matching hash", currentBrokerOwned: true, forceRestart: false, hashMatches: true },
+  { label: "an owned broker and a mismatched hash", currentBrokerOwned: true, forceRestart: false, hashMatches: false },
+  { label: "an owned broker and a restart request", currentBrokerOwned: true, forceRestart: true, hashMatches: true },
+  { label: "an owned broker, a restart request, and a mismatched hash", currentBrokerOwned: true, forceRestart: true, hashMatches: false },
+];
+
+function loadBrokerWithHome(home: string) {
+  process.env.HOME = home;
+  // The module resolves its credential paths at load time, and
+  // `brokerStartedThisRun` is module state, so each case needs a fresh copy.
+  delete require.cache[require.resolve(BROKER_WRAPPER)];
+  return require(BROKER_WRAPPER);
+}
+
+describe("Hermes managed-tool broker ownership", () => {
+  it("refuses to adopt a healthy listener it does not own", () => {
+    const broker = require(BROKER_WRAPPER);
+
+    expect(
+      broker.planHermesToolGatewayBrokerReuse({
+        brokerHealthy: true,
+        currentBrokerOwned: false,
+        forceRestart: false,
+        hashMatches: true,
+      }),
+    ).toBe("refuse-unowned-listener");
+  });
+
+  // Ownership is the only input that may admit a listener. A mismatched hash or
+  // a restart request must not downgrade the refusal into an ordinary "no
+  // broker" outcome, because only the refusal names the held port.
+  it.each(UNOWNED_LISTENER_CASES)(
+    "keeps refusing an unowned listener with $label",
+    ({ forceRestart, hashMatches }) => {
+      const broker = require(BROKER_WRAPPER);
+
+      expect(
+        broker.planHermesToolGatewayBrokerReuse({
+          brokerHealthy: true,
+          currentBrokerOwned: false,
+          forceRestart,
+          hashMatches,
+        }),
+      ).toBe("refuse-unowned-listener");
+    },
+  );
+
+  it("treats an omitted restart request as no restart", () => {
+    const broker = require(BROKER_WRAPPER);
+
+    // `ensureHermesToolGatewayBroker` forwards `options.forceRestart`, which is
+    // undefined whenever a caller omits it, so the default is the production
+    // path rather than a convenience.
+    expect(
+      broker.planHermesToolGatewayBrokerReuse({
+        brokerHealthy: true,
+        currentBrokerOwned: true,
+        hashMatches: true,
+      }),
+    ).toBe("reuse-current");
+    expect(
+      broker.planHermesToolGatewayBrokerReuse({
+        brokerHealthy: true,
+        currentBrokerOwned: false,
+        hashMatches: true,
+      }),
+    ).toBe("refuse-unowned-listener");
+  });
+
+  it("adopts a healthy listener it owns when the runtime hash still matches", () => {
+    const broker = require(BROKER_WRAPPER);
+
+    expect(
+      broker.planHermesToolGatewayBrokerReuse({
+        brokerHealthy: true,
+        currentBrokerOwned: true,
+        forceRestart: false,
+        hashMatches: true,
+      }),
+    ).toBe("reuse-current");
+  });
+
+  it("declines an owned broker when the runtime hash moved or a restart was requested", () => {
+    const broker = require(BROKER_WRAPPER);
+
+    expect(
+      broker.planHermesToolGatewayBrokerReuse({
+        brokerHealthy: true,
+        currentBrokerOwned: true,
+        forceRestart: false,
+        hashMatches: false,
+      }),
+    ).toBe("no-usable-broker");
+    expect(
+      broker.planHermesToolGatewayBrokerReuse({
+        brokerHealthy: true,
+        currentBrokerOwned: true,
+        forceRestart: true,
+        hashMatches: true,
+      }),
+    ).toBe("no-usable-broker");
+    expect(
+      broker.planHermesToolGatewayBrokerReuse({
+        brokerHealthy: true,
+        currentBrokerOwned: true,
+        forceRestart: true,
+        hashMatches: false,
+      }),
+    ).toBe("no-usable-broker");
+  });
+
+  // An unreachable port cannot be refused as unowned; it is the ordinary
+  // restart case whether or not a prior broker was ours.
+  it.each(UNREACHABLE_PORT_CASES)(
+    "reports no usable broker when nothing answers on the port with $label",
+    ({ currentBrokerOwned, forceRestart, hashMatches }) => {
+      const broker = require(BROKER_WRAPPER);
+
+      expect(
+        broker.planHermesToolGatewayBrokerReuse({
+          brokerHealthy: false,
+          currentBrokerOwned,
+          forceRestart,
+          hashMatches,
+        }),
+      ).toBe("no-usable-broker");
+    },
+  );
+
+  it(
+    "reports no broker when a foreign listener answers health after a stale hash is left behind",
+    async ({ skip }) => {
+      const previousHome = process.env.HOME;
+      const home = fs.mkdtempSync(path.join(os.tmpdir(), "nc-broker-unowned-"));
+      const impostor = createUnownedHealthListener();
+
+      try {
+        const staging = loadBrokerWithHome(home);
+        const credsDir = path.dirname(staging.HERMES_TOOL_GATEWAY_STATE_DIR);
+        const hashPath = path.join(credsDir, "hermes-tool-gateway-broker.hash");
+        const pidPath = path.join(credsDir, "hermes-tool-gateway-broker.pid");
+
+        // Let the module write its own runtime hash rather than recomputing it
+        // here. A hand-built hash would stop matching the moment the runtime
+        // inputs change, and this case would then pass for the wrong reason.
+        // The hash is written when the broker is spawned, so it exists whether
+        // or not that broker went on to become healthy.
+        staging.ensureHermesToolGatewayBroker({ startWithoutCredential: true });
+        const runtimeHash = fs.readFileSync(hashPath, "utf8");
+
+        staging.killStaleHermesToolGatewayBroker();
+        await waitForPortFree(staging.HERMES_TOOL_GATEWAY_PORT);
+
+        // A broker that crashed leaves its runtime hash behind while the pid
+        // goes away. That is the exact state this regression covers.
+        fs.writeFileSync(hashPath, runtimeHash, { mode: 0o600 });
+        expect(fs.existsSync(pidPath)).toBe(false);
+
+        const settled = loadBrokerWithHome(home);
+        await listenOn(impostor, settled.HERMES_TOOL_GATEWAY_PORT);
+        // The probe shells out to curl. Where a harness blocks loopback HTTP
+        // for subprocesses, no listener can be observed as healthy and this
+        // case would assert nothing, so report it as skipped instead of passed.
+        skip(
+          !settled.isHermesToolGatewayBrokerHealthy(),
+          "curl cannot read a loopback response in this environment",
+        );
+
+        // Reachability alone must not answer "broker ready".
+        expect(settled.ensureHermesToolGatewayBroker({})).toBe(false);
+      } finally {
+        await closeServer(impostor);
+        restoreEnv("HOME", previousHome);
+        delete require.cache[require.resolve(BROKER_WRAPPER)];
+        fs.rmSync(home, { recursive: true, force: true });
+      }
+    },
+    BROKER_TEST_TIMEOUT_MS,
+  );
+});