diff --git a/agents/hermes/start.sh b/agents/hermes/start.sh index 4c605d93d04..aa109b5cc8a 100755 --- a/agents/hermes/start.sh +++ b/agents/hermes/start.sh @@ -230,6 +230,28 @@ start_gateway_log_stream() { GATEWAY_LOG_TAIL_PID=$! } +retry_tirith_marker_if_needed() { + local marker="${HERMES_DIR}/.tirith-install-failed" + local reason + + [ -e "$marker" ] || return 0 + if [ -L "$marker" ] || [ ! -f "$marker" ]; then + echo "[tirith-bootstrap] WARNING: unsafe Tirith install marker at ${marker}; not reading it" >&2 + return 0 + fi + + reason="$(head -n 1 "$marker" 2>/dev/null | tr -d '\r\n' || true)" + if [ "$reason" != "download_failed" ]; then + echo "[tirith-bootstrap] WARNING: Tirith install marker reason '${reason:-unknown}' is not retryable; Hermes gateway startup will continue" >&2 + return 0 + fi + + echo "[tirith-bootstrap] download_failed marker present; letting Hermes runtime fallback retry Tirith" >&2 + if ! rm -f "$marker" 2>/dev/null; then + echo "[tirith-bootstrap] WARNING: could not remove retryable Tirith marker; Hermes gateway startup will continue" >&2 + fi +} + # ── socat forwarder ────────────────────────────────────────────── # Hermes API server binds to 127.0.0.1 regardless of config (upstream bug). # OpenShell needs the port accessible on 0.0.0.0 for port forwarding. @@ -602,6 +624,8 @@ if [ "$(id -u)" -ne 0 ]; then exec "${NEMOCLAW_CMD[@]}" fi + retry_tirith_marker_if_needed + prepare_restricted_log /tmp/gateway.log "" 600 # Defence-in-depth: verify /tmp file permissions before launching services. @@ -633,6 +657,7 @@ fi # ── Root path (full privilege separation via setpriv) ────────── +export HERMES_HOME="${HERMES_DIR}" verify_config_integrity "${HERMES_DIR}" "${HERMES_HASH_FILE}" refresh_hermes_provider_placeholders install_configure_guard @@ -642,6 +667,8 @@ if [ ${#NEMOCLAW_CMD[@]} -gt 0 ]; then exec "${STEP_DOWN_PREFIX_SANDBOX[@]}" "${NEMOCLAW_CMD[@]}" fi +retry_tirith_marker_if_needed + # SECURITY: Protect gateway log from sandbox user tampering prepare_restricted_log /tmp/gateway.log gateway:gateway 600 diff --git a/src/lib/agent/onboard.test.ts b/src/lib/agent/onboard.test.ts index 67ce44f25d9..b71a82a83e0 100644 --- a/src/lib/agent/onboard.test.ts +++ b/src/lib/agent/onboard.test.ts @@ -3,7 +3,11 @@ import { describe, it, expect, beforeEach, afterEach, afterAll, vi } from "vitest"; // Import from compiled dist/ so coverage is attributed correctly. -import { printDashboardUi, verifyAgentBinaryAvailable } from "../../../dist/lib/agent/onboard"; +import { + collectHermesStartupDiagnostics, + printDashboardUi, + verifyAgentBinaryAvailable, +} from "../../../dist/lib/agent/onboard"; import type { AgentDefinition } from "./defs"; function makeAgent(overrides: Partial = {}): AgentDefinition { @@ -157,3 +161,60 @@ describe("handleAgentSetup guards", () => { expect(script).toContain("NEMOCLAW_AGENT_BINARY_CHECK:ok"); }); }); + +describe("collectHermesStartupDiagnostics", () => { + it("includes Tirith marker content and binary state when the marker is present", () => { + const runCapture = vi.fn(() => + [ + "tirith marker: download_failed", + "tirith binary: missing (/sandbox/.hermes/bin/tirith)", + "--- tail: /tmp/nemoclaw-start.log ---", + "[tirith-bootstrap] Retrying Tirith install after download_failed marker", + ].join("\n"), + ); + + const diagnostics = collectHermesStartupDiagnostics("alpha", runCapture); + + expect(runCapture).toHaveBeenCalledWith( + [ + "sandbox", + "exec", + "-n", + "alpha", + "--", + "sh", + "-lc", + expect.stringContaining("/sandbox/.hermes/.tirith-install-failed"), + ], + { ignoreError: true }, + ); + expect(diagnostics.join("\n")).toContain("Hermes startup diagnostics:"); + expect(diagnostics.join("\n")).toContain("tirith marker: download_failed"); + expect(diagnostics.join("\n")).toContain( + "tirith binary: missing (/sandbox/.hermes/bin/tirith)", + ); + }); + + it("returns no extra lines when the Tirith marker is absent", () => { + const runCapture = vi.fn(() => "tirith marker: absent\n"); + + expect(collectHermesStartupDiagnostics("alpha", runCapture)).toEqual([]); + }); + + it("redacts sensitive values from log tails", () => { + const slackToken = ["xoxb", "123456789012", "abcdefghijkl"].join("-"); + const runCapture = vi.fn(() => + [ + "tirith marker: download_failed", + "tirith binary: present but not executable (/sandbox/.hermes/bin/tirith)", + "--- tail: /tmp/gateway.log ---", + `SLACK_BOT_TOKEN=${slackToken}`, + ].join("\n"), + ); + + const output = collectHermesStartupDiagnostics("alpha", runCapture).join("\n"); + + expect(output).toContain("SLACK_BOT_TOKEN="); + expect(output).not.toContain(slackToken); + }); +}); diff --git a/src/lib/agent/onboard.ts b/src/lib/agent/onboard.ts index 359766646e8..2446108910a 100644 --- a/src/lib/agent/onboard.ts +++ b/src/lib/agent/onboard.ts @@ -209,6 +209,42 @@ type AgentBinaryAvailability = }; const AGENT_BINARY_CHECK_PREFIX = "NEMOCLAW_AGENT_BINARY_CHECK:"; +const HERMES_TIRITH_MARKER_ABSENT = "tirith marker: absent"; +const HERMES_STARTUP_DIAGNOSTICS_SCRIPT = ` +set +e +marker=/sandbox/.hermes/.tirith-install-failed +if [ ! -e "$marker" ]; then + echo "${HERMES_TIRITH_MARKER_ABSENT}" + exit 0 +fi +if [ -L "$marker" ]; then + echo "tirith marker: symlink (not read)" +else + printf "tirith marker: " + head -c 200 "$marker" 2>/dev/null || printf "unreadable" + printf "\\n" +fi + +tirith=/sandbox/.hermes/bin/tirith +if [ -x "$tirith" ] && [ ! -L "$tirith" ]; then + echo "tirith binary: present executable ($tirith)" +elif [ -e "$tirith" ]; then + echo "tirith binary: present but not executable ($tirith)" +else + echo "tirith binary: missing ($tirith)" +fi + +for log in /tmp/nemoclaw-start.log /tmp/gateway.log; do + if [ -f "$log" ] && [ ! -L "$log" ]; then + echo "--- tail: $log ---" + tail -n 40 "$log" 2>/dev/null || echo "(tail unavailable)" + elif [ -L "$log" ]; then + echo "--- tail: $log skipped (symlink) ---" + else + echo "--- tail: $log unavailable ---" + fi +done +`.trim(); /** * Check whether the selected agent binary is available inside the sandbox. @@ -285,12 +321,49 @@ function describeAgentBinaryFailure( return `${agent.displayName} binary '${executable}' is missing inside sandbox '${sandboxName}'`; } +/** + * Collect read-only Hermes startup diagnostics for Step 7 health timeouts. + * Returns no extra lines when the Tirith marker is absent so non-Tirith + * failures keep the existing terse error shape. + */ +export function collectHermesStartupDiagnostics( + sandboxName: string, + runCaptureOpenshell: OnboardContext["runCaptureOpenshell"], +): string[] { + const output = runCaptureOpenshell( + ["sandbox", "exec", "-n", sandboxName, "--", "sh", "-lc", HERMES_STARTUP_DIAGNOSTICS_SCRIPT], + { ignoreError: true }, + ); + const redactedOutput = String(redact(output ?? "")); + const lines = redactedOutput + .split(/\r?\n/) + .map((line) => line.trimEnd()) + .filter((line) => line.length > 0); + + const markerLine = lines.find((line) => line.startsWith("tirith marker:")); + if (!markerLine || markerLine === HERMES_TIRITH_MARKER_ABSENT) { + return []; + } + return ["Hermes startup diagnostics:", ...lines.slice(0, 140)]; +} + /** * Record and print an agent setup failure before exiting the onboarding flow. */ -function failAgentSetup(sandboxName: string, agent: AgentDefinition, message: string): never { - onboardSession.markStepFailed("agent_setup", message); +function failAgentSetup( + sandboxName: string, + agent: AgentDefinition, + message: string, + details: string[] = [], +): never { + onboardSession.markStepFailed( + "agent_setup", + details.length > 0 ? `${message}\n${details.join("\n")}` : message, + ); console.error(` \u2717 ${message}`); + for (const line of details) { + console.error(` ${line}`); + } console.error(` Check: ${agentCliName(agent)} ${sandboxName} logs --follow`); process.exit(1); } @@ -404,10 +477,15 @@ export async function handleAgentSetup( if (healthy) { console.log(` \u2713 ${agent.displayName} gateway is healthy`); } else { + const diagnostics = + agent.name === "hermes" + ? collectHermesStartupDiagnostics(sandboxName, runCaptureOpenshell) + : []; failAgentSetup( sandboxName, agent, `${agent.displayName} gateway did not respond within ${timeoutSecs}s`, + diagnostics, ); } } else { diff --git a/test/e2e/docs/parity-map.yaml b/test/e2e/docs/parity-map.yaml index 889c91e37bb..1516ce781b1 100644 --- a/test/e2e/docs/parity-map.yaml +++ b/test/e2e/docs/parity-map.yaml @@ -1,3 +1,6 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + scripts: brev-e2e.test.ts: scenario: '' diff --git a/test/hermes-start.test.ts b/test/hermes-start.test.ts new file mode 100644 index 00000000000..261c5fb8780 --- /dev/null +++ b/test/hermes-start.test.ts @@ -0,0 +1,110 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { spawnSync } from "node:child_process"; +import { describe, expect, it } from "vitest"; + +const START_SCRIPT = path.join(import.meta.dirname, "..", "agents", "hermes", "start.sh"); + +function shellQuote(value: string): string { + return `'${value.replace(/'/g, "'\\''")}'`; +} + +function escapeRegExp(value: string): string { + return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + +function extractShellFunctionFromSource(src: string, name: string): string { + const escapedName = escapeRegExp(name); + const match = src.match(new RegExp(`${escapedName}\\(\\) \\{([\\s\\S]*?)^\\}`, "m")); + if (!match) { + throw new Error(`Expected ${name} in agents/hermes/start.sh`); + } + return `${name}() {${match[1]}\n}`; +} + +function runTirithMarkerBootstrap(opts: { + markerReason?: string; + symlinkMarker?: boolean; +}) { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-hermes-tirith-")); + const hermesHome = path.join(tmpDir, ".hermes"); + const marker = path.join(hermesHome, ".tirith-install-failed"); + const target = path.join(tmpDir, "marker-target"); + const scriptPath = path.join(tmpDir, "run.sh"); + + fs.mkdirSync(hermesHome, { recursive: true }); + if (opts.symlinkMarker) { + fs.writeFileSync(target, opts.markerReason ?? "download_failed"); + fs.symlinkSync(target, marker); + } else if (opts.markerReason !== undefined) { + fs.writeFileSync(marker, opts.markerReason); + } + + const src = fs.readFileSync(START_SCRIPT, "utf-8"); + fs.writeFileSync( + scriptPath, + [ + "#!/usr/bin/env bash", + "set -euo pipefail", + extractShellFunctionFromSource(src, "retry_tirith_marker_if_needed"), + `HERMES_DIR=${shellQuote(hermesHome)}`, + "retry_tirith_marker_if_needed", + ].join("\n"), + { mode: 0o700 }, + ); + + try { + const result = spawnSync("bash", [scriptPath], { + encoding: "utf-8", + timeout: 5000, + env: process.env, + }); + return { + result, + markerExists: fs.existsSync(marker), + markerIsSymlink: fs.existsSync(marker) && fs.lstatSync(marker).isSymbolicLink(), + markerContent: fs.existsSync(marker) ? fs.readFileSync(marker, "utf-8") : "", + targetContent: fs.existsSync(target) ? fs.readFileSync(target, "utf-8") : "", + }; + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } +} + +describe("agents/hermes/start.sh Tirith marker bootstrap", () => { + it("removes a retryable download_failed marker so Hermes runtime fallback can retry", () => { + const run = runTirithMarkerBootstrap({ markerReason: "download_failed" }); + + expect(run.result.status).toBe(0); + expect(run.markerExists).toBe(false); + expect(run.result.stderr).toContain( + "download_failed marker present; letting Hermes runtime fallback retry Tirith", + ); + }); + + it("leaves unknown marker reasons untouched", () => { + const run = runTirithMarkerBootstrap({ markerReason: "checksum_failed" }); + + expect(run.result.status).toBe(0); + expect(run.markerExists).toBe(true); + expect(run.markerContent).toBe("checksum_failed"); + expect(run.result.stderr).toContain("is not retryable"); + }); + + it("refuses to read or remove an unsafe symlink marker", () => { + const run = runTirithMarkerBootstrap({ + markerReason: "download_failed", + symlinkMarker: true, + }); + + expect(run.result.status).toBe(0); + expect(run.markerExists).toBe(true); + expect(run.markerIsSymlink).toBe(true); + expect(run.targetContent).toBe("download_failed"); + expect(run.result.stderr).toContain("unsafe Tirith install marker"); + }); +});