fix(acp): complete lifecycle review repairs

scripts/install.sh

@@ -2280,9 +2280,131 @@ maybe_install_openshell_during_install() {
   install_nemoclaw_openshell_gateway_user_service
 }
 
+is_installer_managed_cli_shim() {
+  local shim_path="${1:-}" cli_bin="${2:-}" canonical_cli_path="${3:-}"
+  local line path_line node_dir path_dir exec_line shim_cli_path
+  local path_prefix path_middle path_suffix exec_prefix exec_suffix
+  local -a lines=()
+
+  [[ -n "$shim_path" && -n "$cli_bin" && -n "$canonical_cli_path" ]] || return 1
+  [[ -f "$shim_path" && ! -L "$shim_path" && -e "$canonical_cli_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='" "$@"'
+  exec_line="${lines[2]}"
+  [[ "$exec_line" == "$exec_prefix"*"$exec_suffix" ]] || return 1
+  shim_cli_path="${exec_line#"$exec_prefix"}"
+  shim_cli_path="${shim_cli_path%"$exec_suffix"}"
+  [[ "$shim_cli_path" == */"$cli_bin" && -e "$shim_cli_path" ]] || return 1
+  [[ "$shim_cli_path" -ef "$canonical_cli_path" ]]
+}
+
+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 before_identity after_identity
+  _NEMOCLAW_ACP_SHIM_REPLACE_IDENTITY=""
+  [[ "$cli_bin" == "nemoclaw-acp" ]] || return 0
+  shim_path="${NEMOCLAW_SHIM_DIR}/${cli_bin}"
+
+  if [[ ! -e "$shim_path" && ! -L "$shim_path" ]]; then
+    _NEMOCLAW_ACP_SHIM_REPLACE_IDENTITY="absent"
+    return 0
+  fi
+  before_identity="$(cli_shim_entry_identity "$shim_path")" \
+    || error "Installation stopped because NemoClaw could not inspect $shim_path without following symbolic links. NemoClaw left it unchanged."
+  if ! is_installer_managed_cli_shim "$shim_path" "$cli_bin" "$cli_path" \
+    && ! is_npm_managed_nemoclaw_acp_link "$shim_path"; then
+    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."
+  fi
+  after_identity="$(cli_shim_entry_identity "$shim_path")" \
+    || error "Installation stopped because NemoClaw could not recheck $shim_path. NemoClaw left it unchanged."
+  [[ "$before_identity" == "$after_identity" ]] \
+    || error "Installation stopped because $shim_path changed while NemoClaw checked it. NemoClaw left the current path unchanged. Rerun the installer."
+  _NEMOCLAW_ACP_SHIM_REPLACE_IDENTITY="$before_identity"
+}
+
+cli_shim_entry_identity() {
+  local shim_path="${1:-}" identity
+  [[ -n "$shim_path" ]] || return 1
+  if [[ ! -e "$shim_path" && ! -L "$shim_path" ]]; then
+    printf 'absent'
+    return 0
+  fi
+  if identity="$(stat -c '%F:%d:%i' -- "$shim_path" 2>/dev/null)"; then
+    :
+  elif identity="$(stat -f '%HT:%d:%i' "$shim_path" 2>/dev/null)"; then
+    :
+  else
+    return 1
+  fi
+  printf '%s' "$identity"
+}
+
+assert_nemoclaw_acp_shim_unchanged() {
+  local cli_bin="${1:-}" cli_path="${2:-}" expected_identity="${3:-}" shim_path current_identity
+  [[ "$cli_bin" == "nemoclaw-acp" ]] || return 0
+  shim_path="${NEMOCLAW_SHIM_DIR}/${cli_bin}"
+  current_identity="$(cli_shim_entry_identity "$shim_path")" \
+    || error "Installation stopped because NemoClaw could not recheck $shim_path. NemoClaw left it unchanged."
+  [[ -n "$expected_identity" && "$current_identity" == "$expected_identity" ]] \
+    || error "Installation stopped because $shim_path changed while NemoClaw prepared its shim. NemoClaw left the current path unchanged. Rerun the installer."
+  if [[ "$expected_identity" != "absent" ]]; then
+    is_installer_managed_cli_shim "$shim_path" "$cli_bin" "$cli_path" \
+      || is_npm_managed_nemoclaw_acp_link "$shim_path" \
+      || error "Installation stopped because $shim_path is no longer a NemoClaw-managed shim. NemoClaw left it unchanged. Rerun the installer."
+  fi
+}
+
+preflight_nemoclaw_acp_shim() {
+  local shim_path="${NEMOCLAW_SHIM_DIR}/nemoclaw-acp" npm_bin="" cli_path=""
+  [[ -e "$shim_path" || -L "$shim_path" ]] || return 0
+  npm_bin="$(resolve_npm_bin)" \
+    || error "Installation stopped because NemoClaw could not resolve the active npm prefix to verify $shim_path. NemoClaw left it unchanged. Fix the npm configuration, then rerun the installer."
+  cli_path="${npm_bin:+${npm_bin}/nemoclaw-acp}"
+  if [[ -n "$cli_path" && "$cli_path" != "$shim_path" && -e "$cli_path" &&
+    "$cli_path" -ef "$shim_path" ]]; then
+    return 0
+  fi
+  is_installer_managed_cli_shim "$shim_path" "nemoclaw-acp" "$cli_path" && 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
+  local npm_bin shim_path node_path node_dir cli_path expected_shim temp_shim=""
+  local replace_identity=""
   npm_bin="$(resolve_npm_bin)" || true
   shim_path="${NEMOCLAW_SHIM_DIR}/${cli_bin}"
 
@@ -2305,7 +2427,21 @@ ensure_cli_shim() {
   # 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" && -e "$shim_path" && "$cli_path" -ef "$shim_path" ]]; then
+    refresh_path
+    ensure_local_bin_in_profile
+    return 0
+  fi
+
+  assert_nemoclaw_acp_shim_replaceable "$cli_bin" "$cli_path"
+  replace_identity="${_NEMOCLAW_ACP_SHIM_REPLACE_IDENTITY:-}"
+  if [[ "$cli_path" == "$shim_path" ]]; then
+    refresh_path
+    ensure_local_bin_in_profile
+    return 0
+  fi
+  if [[ "$cli_bin" == "nemoclaw-acp" && "$replace_identity" != "absent" ]]; then
+    assert_nemoclaw_acp_shim_unchanged "$cli_bin" "$cli_path" "$replace_identity"
     refresh_path
     ensure_local_bin_in_profile
     return 0
@@ -2319,15 +2455,36 @@ exec "$cli_path" "\$@"
 EOF
   )"
 
-  if [[ -x "$shim_path" ]] && cmp -s "$shim_path" <(printf '%s\n' "$expected_shim"); then
+  if [[ "$cli_bin" != "nemoclaw-acp" && -x "$shim_path" ]] \
+    && cmp -s "$shim_path" <(printf '%s\n' "$expected_shim"); then
     refresh_path
     ensure_local_bin_in_profile
     return 0
   fi
 
   mkdir -p "$NEMOCLAW_SHIM_DIR"
-  printf '%s\n' "$expected_shim" >"$shim_path"
-  chmod +x "$shim_path"
+  temp_shim="$(mktemp "${shim_path}.tmp.XXXXXX")" \
+    || error "Could not create a temporary shim beside $shim_path."
+  _cleanup_files+=("$temp_shim")
+  if ! printf '%s\n' "$expected_shim" >"$temp_shim" || ! chmod 755 "$temp_shim"; then
+    rm -f "$temp_shim"
+    error "Could not prepare the user-local shim for $cli_bin."
+  fi
+  assert_nemoclaw_acp_shim_unchanged "$cli_bin" "$cli_path" "$replace_identity"
+  if [[ "$cli_bin" == "nemoclaw-acp" ]]; then
+    if ! ln "$temp_shim" "$shim_path"; then
+      rm -f "$temp_shim"
+      if [[ -e "$shim_path" || -L "$shim_path" ]]; then
+        error "Installation stopped because $shim_path changed while NemoClaw published its shim. NemoClaw left the current path unchanged. Rerun the installer."
+      fi
+      error "Could not publish the user-local shim at $shim_path."
+    fi
+    rm -f "$temp_shim" \
+      || error "NemoClaw published $shim_path but could not remove its temporary shim."
+  elif ! mv -f -- "$temp_shim" "$shim_path"; then
+    rm -f "$temp_shim"
+    error "Could not publish the user-local shim at $shim_path."
+  fi
   refresh_path
   ensure_local_bin_in_profile
   info "Created user-local shim at $shim_path"
@@ -2336,6 +2493,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
@@ -2794,6 +2952,7 @@ install_nemoclaw() {
   local repo_root package_json
   repo_root="$(resolve_repo_root)"
   package_json="${repo_root}/package.json"
+  preflight_nemoclaw_acp_shim
   # Tell prepare not to run npm link — the installer handles linking explicitly.
   export NEMOCLAW_INSTALLING=1
 
@@ -6548,6 +6707,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",