fix(acp): complete lifecycle review repairs

scripts/install.sh

@@ -2219,6 +2219,71 @@ maybe_install_openshell_during_install() {
   install_nemoclaw_openshell_gateway_user_service
 }
 
+is_installer_managed_cli_shim() {
+  local shim_path="${1:-}" cli_bin="${2:-}"
+  local line path_line node_dir path_dir
+  local path_prefix path_middle path_suffix exec_prefix exec_suffix
+  local -a lines=()
+
+  [[ -n "$shim_path" && -n "$cli_bin" && -f "$shim_path" && ! -L "$shim_path" ]] || return 1
+  while IFS= read -r line || [[ -n "$line" ]]; do
+    lines[${#lines[@]}]="$line"
+    [[ "${#lines[@]}" -le 3 ]] || return 1
+  done <"$shim_path" || return 1
+  [[ "${#lines[@]}" -eq 3 && "${lines[0]}" == "#!/usr/bin/env bash" ]] || return 1
+
+  path_line="${lines[1]}"
+  path_prefix='export PATH="'
+  # shellcheck disable=SC2016 # Match the literal $PATH emitted by the managed shim.
+  path_suffix=':$PATH"'
+  if [[ "$path_line" == "$path_prefix"*"$path_suffix" ]]; then
+    path_dir="${path_line#"$path_prefix"}"
+    path_dir="${path_dir%"$path_suffix"}"
+    [[ -n "$path_dir" ]] || return 1
+  else
+    # shellcheck disable=SC2016 # Match the literal command substitution emitted by the shim.
+    path_prefix='[[ "$(command -v node 2>/dev/null)" == "'
+    path_middle='/node" ]] || export PATH="'
+    [[ "$path_line" == "$path_prefix"*"$path_middle"*"$path_suffix" ]] || return 1
+    node_dir="${path_line#"$path_prefix"}"
+    node_dir="${node_dir%%"$path_middle"*}"
+    path_dir="${path_line#*"$path_middle"}"
+    path_dir="${path_dir%"$path_suffix"}"
+    [[ -n "$node_dir" && "$node_dir" == "$path_dir" ]] || return 1
+  fi
+
+  exec_prefix='exec "'
+  exec_suffix="/${cli_bin}\" \"\$@\""
+  [[ "${lines[2]}" == "$exec_prefix"*"$exec_suffix" ]]
+}
+
+is_npm_managed_nemoclaw_acp_link() {
+  local shim_path="${1:-}" link_target
+  [[ -n "$shim_path" && -L "$shim_path" ]] || return 1
+  link_target="$(readlink "$shim_path")" || return 1
+  [[ "$link_target" == "../lib/node_modules/nemoclaw/dist/lib/acp/main.js" ]]
+}
+
+assert_nemoclaw_acp_shim_replaceable() {
+  local cli_bin="${1:-}" cli_path="${2:-}" shim_path
+  [[ "$cli_bin" == "nemoclaw-acp" ]] || return 0
+  shim_path="${NEMOCLAW_SHIM_DIR}/${cli_bin}"
+
+  [[ -e "$shim_path" || -L "$shim_path" ]] || return 0
+  [[ "$cli_path" == "$shim_path" ]] && return 0
+  is_installer_managed_cli_shim "$shim_path" "$cli_bin" && return 0
+
+  error "Installation stopped because $shim_path already exists and is not a NemoClaw-managed shim. NemoClaw left it unchanged. Move or remove that path, then rerun the installer."
+}
+
+preflight_nemoclaw_acp_shim() {
+  local shim_path="${NEMOCLAW_SHIM_DIR}/nemoclaw-acp"
+  [[ -e "$shim_path" || -L "$shim_path" ]] || return 0
+  is_installer_managed_cli_shim "$shim_path" "nemoclaw-acp" && return 0
+  is_npm_managed_nemoclaw_acp_link "$shim_path" && return 0
+  error "Installation stopped because $shim_path already exists and is not a NemoClaw-managed shim. NemoClaw left it unchanged. Move or remove that path, then rerun the installer."
+}
+
 ensure_cli_shim() {
   local cli_bin="${1:-$_CLI_BIN}"
   local npm_bin shim_path node_path node_dir cli_path expected_shim
@@ -2240,11 +2305,13 @@ ensure_cli_shim() {
   fi
   node_dir="$(dirname "$node_path")"
 
+  assert_nemoclaw_acp_shim_replaceable "$cli_bin" "$cli_path"
+
   # If npm placed the binary at the same path as the shim target (e.g. when
   # npm_config_prefix=$HOME/.local), writing a shim would overwrite the real
   # binary with a script that exec's itself — an infinite loop.  In that case
   # the binary is already where it needs to be; skip shim creation.
-  if [[ "$cli_path" -ef "$shim_path" ]]; then
+  if [[ "$cli_path" == "$shim_path" ]]; then
     refresh_path
     ensure_local_bin_in_profile
     return 0
@@ -2275,6 +2342,7 @@ EOF
 
 ensure_nemoclaw_shim() {
   local cli_bin status=0
+  preflight_nemoclaw_acp_shim
   ensure_cli_shim "$_CLI_BIN" || status=$?
   for cli_bin in nemoclaw nemoclaw-acp nemohermes nemo-deepagents; do
     [[ "$cli_bin" == "$_CLI_BIN" ]] && continue
@@ -6372,6 +6440,7 @@ install_nemoclaw_before_onboarding() {
   # `nemoclaw onboard` (the install-ollama / install-vllm branches).
   # install.sh stays focused on dependency setup.
   fix_npm_permissions
+  preflight_nemoclaw_acp_shim
   preinstall_backup_and_retire_legacy_gateway
   install_nemoclaw
   verify_nemoclaw

src/lib/adapters/openshell/client.ts

@@ -13,6 +13,7 @@ import {
 import { redirectInheritedChildStdoutToStderr } from "../../cli/stdout-guard";
 import { buildSubprocessEnv } from "../../subprocess-env";
 import { processTreeBoundedOpenshellInvocation } from "./process-tree-timeout";
+import { captureSandboxSshConfig } from "./sandbox-ssh-config-capture";
 import { classifyManagedGatewayEndpointBinding } from "../../../../nemoclaw/dist/shared/openshell-gateway-endpoint-boundary.cjs";
 
 export { classifyManagedGatewayEndpointBinding };
@@ -308,46 +309,17 @@ export function captureOpenshellCommand(
   };
 }
 
-/**
- * Insert `-g <gateway>` after the subcommand pair, matching the placement
- * `gatewayScopedArgs` already uses in `actions/sandbox/gateway-state.ts`.
- * Duplicated rather than imported: an adapter must not depend on the actions
- * layer.
- */
-function gatewayScopedArgs(args: string[], gatewayName?: string): string[] {
-  if (!gatewayName) return args;
-  return [...args.slice(0, 2), "-g", gatewayName, ...args.slice(2)];
-}
-
 export function captureSandboxSshConfigCommand(
   binary: string,
   sandboxName: string,
   opts: CaptureSandboxSshConfigOptions = {},
 ): CaptureOpenshellResult {
   const { gatewayName, ...spawnOpts } = opts;
-  const sandboxGet = captureOpenshellCommand(
-    binary,
-    gatewayScopedArgs(["sandbox", "get", sandboxName], gatewayName),
-    {
+  return captureSandboxSshConfig(sandboxName, gatewayName, (args, { includeStderr }) =>
+    captureOpenshellCommand(binary, args, {
       ...spawnOpts,
-      ignoreError: true,
-      includeStderr: true,
-    },
-  );
-  if (sandboxGet.status !== 0) {
-    const output = sandboxGet.output || `failed to query sandbox '${sandboxName}'`;
-    const sandboxMissing = /\bnot[- ]?found\b/i.test(output);
-    return {
-      ...sandboxGet,
-      output: sandboxMissing ? `sandbox '${sandboxName}' not found` : output,
-    };
-  }
-  // Pin every hop to the same gateway so `get` and `ssh-config` cannot
-  // disagree about which one owns the sandbox.
-  return captureOpenshellCommand(
-    binary,
-    gatewayScopedArgs(["sandbox", "ssh-config", sandboxName], gatewayName),
-    spawnOpts,
+      ...(includeStderr ? { ignoreError: true, includeStderr: true } : {}),
+    }),
   );
 }
 

src/lib/adapters/openshell/hermes-acp-ssh-cli.test.ts

@@ -176,7 +176,8 @@ describe("CLI Hermes ACP SSH transport", () => {
     expect(probe.at(-1)).toContain('m.version("hermes-agent")');
     expect(probe.at(-1)).toContain('m.version("agent-client-protocol")');
     expect(session.at(-1)).toBe("/usr/local/bin/hermes-acp");
-    expect(session).not.toContain("sh");
+    expect(session.slice(0, -1)).not.toContain("sh");
+    expect(session.slice(0, -1)).not.toContain("/bin/sh");
   });
 
   it("forwards duplex bytes with backpressure and keeps stderr out of ACP output", async () => {
@@ -322,6 +323,29 @@ describe("CLI Hermes ACP SSH transport", () => {
     expect(fixture.cleanup).toHaveBeenCalledOnce();
   });
 
+  it("returns the SIGTERM exit code when AbortSignal cancels the compatibility probe", async () => {
+    const probe = fakeChild();
+    const fixture = harness([probe]);
+    const controller = new AbortController();
+    const io = streams();
+    const pending = fixture.transport.run({
+      gatewayName: "nemoclaw",
+      sandboxName: "alpha",
+      signal: controller.signal,
+      streams: io.value,
+    });
+    await vi.waitFor(() => expect(fixture.spawnSsh).toHaveBeenCalledOnce());
+    controller.abort();
+
+    await expect(pending).resolves.toMatchObject({
+      kind: "failed",
+      error: { kind: "cancelled" },
+      exitCode: 143,
+    });
+    expect(probe.kill).toHaveBeenCalledWith("SIGTERM");
+    expect(fixture.cleanup).toHaveBeenCalledOnce();
+  });
+
   it.each([
     ["SIGTERM" as const, 143],
     ["SIGINT" as const, 130],

src/lib/adapters/openshell/hermes-acp-ssh-cli.ts

@@ -20,6 +20,7 @@ import { isValidName } from "../../sandbox-name-contract";
 import { isSshTransportFailure } from "../../state/ssh-transport";
 import { resolveOpenshellBinaryOrNull } from "./resolve-shared";
 import { buildOpenShellRuntimeSelectionEnv } from "./runtime-selection";
+import { captureSandboxSshConfig } from "./sandbox-ssh-config-capture";
 import { OPENSHELL_DEFAULT_WORKSPACE } from "./sandbox-ssh-host";
 import {
   HERMES_ACP_EXECUTABLE,
@@ -297,7 +298,7 @@ async function runProbe(
     return failure(
       "cancelled",
       "The Hermes ACP compatibility probe was cancelled.",
-      requestedSignal === "SIGTERM" ? 143 : 130,
+      requestedSignal === "SIGINT" ? 130 : 143,
     );
   }
   if (outputOverflow) {
@@ -449,33 +450,6 @@ function buildRuntimeEnv(gatewayName: string): NodeJS.ProcessEnv {
   return environment;
 }
 
-function captureConfig(
-  binary: string,
-  request: HermesAcpSshRequest,
-  environment: NodeJS.ProcessEnv,
-  capture: CaptureOpenShell,
-): string | null {
-  const options = {
-    env: environment,
-    ignoreError: true,
-    includeStreams: true,
-    maxBuffer: SSH_CONFIG_MAX_BYTES,
-    openshellBinary: binary,
-    replaceEnv: true,
-    timeout: ACP_SETUP_TIMEOUT_MS,
-  } as const;
-  const sandbox = capture(
-    ["sandbox", "get", "-g", request.gatewayName, request.sandboxName],
-    options,
-  );
-  if (sandbox.status !== 0 || sandbox.error) return null;
-  const result = capture(
-    ["sandbox", "ssh-config", "-g", request.gatewayName, request.sandboxName],
-    options,
-  );
-  return result.status === 0 && !result.error && result.output.trim() ? result.output : null;
-}
-
 function parseExactVersion(output: string): string | null {
   const matches = [...output.matchAll(/(?:^|[^0-9.])(\d+\.\d+\.\d+)(?![0-9.])/gu)]
     .map((match) => match[1])
@@ -568,12 +542,24 @@ export function createCliHermesAcpSshTransport(
           69,
         );
       }
-      const config = captureConfig(
-        binaries.openshell,
-        request,
-        environment,
-        deps.captureOpenShell ?? captureOpenShell,
+      const configResult = captureSandboxSshConfig(
+        request.sandboxName,
+        request.gatewayName,
+        (args) =>
+          (deps.captureOpenShell ?? captureOpenShell)(args, {
+            env: environment,
+            ignoreError: true,
+            includeStreams: true,
+            maxBuffer: SSH_CONFIG_MAX_BYTES,
+            openshellBinary: binaries.openshell,
+            replaceEnv: true,
+            timeout: ACP_SETUP_TIMEOUT_MS,
+          }),
       );
+      const config =
+        configResult.status === 0 && !configResult.error && configResult.output.trim()
+          ? configResult.output
+          : null;
       if (!config) {
         return failure(
           "transport",

src/lib/adapters/openshell/sandbox-ssh-config-capture.ts

@@ -0,0 +1,40 @@
+// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+export type SandboxSshConfigCaptureResult = Readonly<{
+  error?: Error;
+  output: string;
+  status: number | null;
+}>;
+
+type CaptureSandboxCommand = (
+  args: string[],
+  options: Readonly<{ includeStderr: boolean }>,
+) => SandboxSshConfigCaptureResult;
+
+function gatewayScopedArgs(args: string[], gatewayName?: string): string[] {
+  if (!gatewayName) return args;
+  return [...args.slice(0, 2), "-g", gatewayName, ...args.slice(2)];
+}
+
+/** Capture one sandbox's SSH configuration after confirming it exists on the same gateway. */
+export function captureSandboxSshConfig(
+  sandboxName: string,
+  gatewayName: string | undefined,
+  capture: CaptureSandboxCommand,
+): SandboxSshConfigCaptureResult {
+  const sandboxGet = capture(gatewayScopedArgs(["sandbox", "get", sandboxName], gatewayName), {
+    includeStderr: true,
+  });
+  if (sandboxGet.status !== 0 || sandboxGet.error) {
+    const output = sandboxGet.output || `failed to query sandbox '${sandboxName}'`;
+    const sandboxMissing = /\bnot[- ]?found\b/iu.test(output);
+    return {
+      ...sandboxGet,
+      output: sandboxMissing ? `sandbox '${sandboxName}' not found` : output,
+    };
+  }
+  return capture(gatewayScopedArgs(["sandbox", "ssh-config", sandboxName], gatewayName), {
+    includeStderr: false,
+  });
+}

src/lib/gateway-runtime-action.test.ts

@@ -281,6 +281,34 @@ describe("gateway-runtime-action per-sandbox gateway routing", () => {
       expect(process.env.OPENSHELL_GATEWAY).toBe("nemoclaw-8090");
     });
 
+    it("reports a redacted startup failure only after recovery remains unhealthy", async () => {
+      const output = {
+        error: vi.fn(),
+        log: vi.fn(),
+        step: vi.fn(),
+        warn: vi.fn(),
+      };
+      captureSpy.mockReturnValue({
+        status: 0,
+        output: "Status: Disconnected\nGateway: nemoclaw\n",
+      });
+      runSpy.mockReturnValue({ status: 0 } as never);
+      startGatewaySpy.mockRejectedValueOnce(
+        new Error("gateway start failed with Authorization: Bearer recovery-secret"),
+      );
+
+      const result = await gatewayRuntime.recoverNamedGatewayRuntime({
+        gatewayName: "nemoclaw-8090",
+        output,
+      });
+
+      expect(result).toMatchObject({ recovered: false, attempted: true });
+      expect(output.error).toHaveBeenCalledOnce();
+      expect(output.error.mock.calls[0]?.[0]).toContain("OpenShell gateway recovery failed");
+      expect(output.error.mock.calls[0]?.[0]).toContain("<REDACTED>");
+      expect(output.error.mock.calls[0]?.[0]).not.toContain("recovery-secret");
+    });
+
     it("keeps recovery probes and startup on the frozen OpenShell target (#10514)", async () => {
       vi.stubEnv("OPENSHELL_GATEWAY", "hostile-gateway");
       vi.stubEnv("OPENSHELL_WORKSPACE", "hostile-workspace");