fix(onboard): improve gateway startup diagnostics (#1605)

[Dongni-Yang]

Summary

• Remove 240-char truncation on gateway start output; split on newlines first, apply `compactText` per-line, and print each line indented under a header — fixes the `\naiting` rendering artifact caused by embedded `\r` overwriting the terminal cursor
• Auto-capture `openshell doctor logs` in the final-failure catch block so users see the actual k3s/flannel/AppArmor errors without having to run a separate command manually
• Export `startGateway` and add a test that verifies doctor log output appears in stderr when the gateway fails
• Redact all new output paths with `redact()` + ANSI stripping before printing (addresses CodeRabbit security review)
• Redact `upsertProvider` and `setupInference` stderr — `openshell provider create/update` and `openshell inference set` run with credential env in scope; their stderr was passed through `compactText()` without `redact()`, same class of bug as API Key Displayed in Plaintext During Retry Flow - IssueFinder - SN 02 #1423

Note: The underlying gateway startup failure on Ubuntu 24.04 (overlayfs/AppArmor/flannel incompatibility reported in #1605) is an OpenShell upstream issue. This PR improves diagnostics so users and maintainers can see the actual error — it does not fix the root cause. Issue #1605 should remain open until an upstream fix or workaround lands.

Improves diagnostics for #1605

Test plan

• `npm test` passes (only pre-existing `runtime-shell.test.ts` failures)
• New test `prints doctor logs automatically when gateway fails to start (NemoClaw Onboard Error #1605)` passes
• Manual: `nemoclaw onboard` on a system where the gateway fails now shows `Gateway logs:` section with k3s output before the troubleshooting hints

Signed-off-by: Dongni Yang dongniy@nvidia.com

🤖 Generated with Claude Code

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Gateway startup failures now redact captured output, strip ANSI sequences per-line, and print an indented multi-line block; on final failure the flow best-effort runs openshell doctor logs --name nemoclaw, redacts/cleans and prints those logs; startGateway is exported.

Changes

Cohort / File(s)	Summary
Gateway Startup Diagnostics src/lib/onboard.ts	Add ANSI_RE to strip ANSI escape sequences; redact combined stderr/stdout before compaction; when gateway start returns early, split output into lines, strip ANSI per-line, remove empties, indent and log as a multi-line block; on final failure with exitOnFailure best-effort run openshell doctor logs --name nemoclaw, redact/clean and print logs line-by-line; export startGateway via module.exports.
Test Coverage test/onboard.test.ts	Add test "prints doctor logs automatically when gateway fails to start (#1605)" that spawns Node with a PATH-shadowed openshell shim (only doctor logs succeeds), patches p-retry and env to force immediate failure/skip health polling, asserts process exits non-zero and stderr contains "Gateway logs:" and diagnostic content (e.g., "OOMKilled").

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Onboard as Onboard (startGateway)
  participant Gateway as Local Gateway Process
  participant CLI as openshell CLI
  participant Logger as StdErr / Logger

  Onboard->>Gateway: spawn/start gateway
  Gateway-->>Onboard: returns non-zero with combined stdout/stderr
  Onboard->>Onboard: redact, split lines, strip ANSI, filter empties, indent
  Onboard->>Logger: print indented multi-line startup output
  Onboard->>CLI: run "openshell doctor logs --name nemoclaw" (best-effort)
  CLI-->>Onboard: doctor logs output
  Onboard->>Onboard: redact/strip doctor logs per-line
  Onboard->>Logger: print "Gateway logs:" and cleaned doctor logs
  Onboard->>Logger: continue with fallback troubleshooting output

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I nibbled noisy lines at night,
Hid the colors, set them right,
Split the hops and cleared the jive,
Fetched doctor logs to help revive.
Tiny rabbit cheers — cleaned and alive!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the primary change: improving gateway startup diagnostics by addressing output truncation and adding automatic error log capture.
Linked Issues check	✅ Passed	The PR addresses all core coding requirements from issue #1605: displaying full gateway logs without truncation, fixing carriage return rendering artifacts by splitting on newlines, redacting sensitive output, automatically capturing doctor logs on failure, and exporting startGateway with a corresponding test.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to issue #1605: onboard.ts modifications handle gateway diagnostics and security redaction, test additions verify doctor log capture, and no unrelated refactoring or feature additions are present.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

test/onboard.test.ts (1)

833-905: Add deterministic cleanup for the temporary test directory.

Line 835 creates tmpDir, but this test never removes it. Please wrap the body in try/finally and clean it up to prevent temp-dir buildup in long/local CI runs.

Proposed patch

   it("prints doctor logs automatically when gateway fails to start (`#1605`)", () => {
     const repoRoot = path.join(import.meta.dirname, "..");
     const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-gateway-diag-"));
@@
-    fs.mkdirSync(fakeBin, { recursive: true });
+    try {
+      fs.mkdirSync(fakeBin, { recursive: true });
@@
-    assert.ok(
-      result.stderr.includes("OOMKilled"),
-      `expected doctor log output in stderr:\n${result.stderr}`,
-    );
+      assert.ok(
+        result.stderr.includes("OOMKilled"),
+        `expected doctor log output in stderr:\n${result.stderr}`,
+      );
+    } finally {
+      fs.rmSync(tmpDir, { recursive: true, force: true });
+    }
   });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/onboard.test.ts` around lines 833 - 905, Wrap the test body that begins
after creating tmpDir (created via fs.mkdtempSync and used to build fakeBin and
scriptPath) in a try/finally so temporary files are always removed; in the
finally block call fs.rmSync(tmpDir, { recursive: true, force: true }) (or
equivalent cleanup) after the spawnSync completes to remove fakeBin, scriptPath
and tmpDir, ensuring deterministic cleanup even on assertion failures.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/lib/onboard.ts`:
- Around line 2091-2097: The printed gateway/doctor logs are currently emitted
raw and may leak secrets; before converting startResult.output (and the similar
doctor output around the 2137–2145 block) to a string and splitting, pass the
raw text through the redact(...) sanitizer and then use compactText/redact
output for subsequent .split("\n"), .map and the console.log; update the code
paths that reference startResult.output and the analogous doctor output so they
call redact(...) first and then proceed with compactText and join to ensure
sensitive data is sanitized before printing.

---

Nitpick comments:
In `@test/onboard.test.ts`:
- Around line 833-905: Wrap the test body that begins after creating tmpDir
(created via fs.mkdtempSync and used to build fakeBin and scriptPath) in a
try/finally so temporary files are always removed; in the finally block call
fs.rmSync(tmpDir, { recursive: true, force: true }) (or equivalent cleanup)
after the spawnSync completes to remove fakeBin, scriptPath and tmpDir, ensuring
deterministic cleanup even on assertion failures.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6f5c710e-13c2-4b91-93ee-110e03dc9ac7

📥 Commits

Reviewing files that changed from the base of the PR and between d4aac4c and e4e3bef.

📒 Files selected for processing (2)

• src/lib/onboard.ts
• test/onboard.test.ts

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/lib/onboard.ts`:
- Around line 26-27: The ANSI_RE constant only removes SGR color codes and
leaves other ANSI control sequences (CSI, OSC, erase-line, cursor movements)
intact; update the ANSI_RE definition to use a more comprehensive regex that
matches all standard ANSI escape sequences (including CSI sequences like ESC [
... (not just those ending in m), OSC sequences starting with ESC ] ... BEL or
ESC \, and other single-character ESC sequences) so terminal control codes are
fully stripped before printing; locate and replace the ANSI_RE constant in
onboard.ts (symbol: ANSI_RE) with the expanded pattern and ensure any code paths
using ANSI_RE (e.g., diagnostic/gateway output) continue to call it.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d0d0b060-7e71-4ca7-8484-490f7c899b29

📥 Commits

Reviewing files that changed from the base of the PR and between 7c81c4e and ae6c21d.

📒 Files selected for processing (1)

• src/lib/onboard.ts

[coderabbitai]

♻️ Duplicate comments (1)

src/lib/onboard.ts (1)

26-27: ⚠️ Potential issue | 🟠 Major

Broaden ANSI stripping beyond SGR color codes

Line 27 only strips ESC[..m sequences. CSI cursor/erase and OSC sequences can still leak into log output and cause rendering artifacts in the new diagnostics paths.

🔧 Proposed fix

-const ANSI_RE = /\x1b\[[0-9;]*m/g;
+// Match CSI, OSC, and single-character ANSI escape sequences.
+const ANSI_RE = /\x1B(?:\[[0-?]*[ -/]*[`@-`~]|\][^\x07]*(?:\x07|\x1B\\)|[`@-Z`\\-_])/g;

#!/bin/bash
# Verify current sanitizer misses non-SGR ANSI control sequences.
python - <<'PY'
import re, pathlib
src = pathlib.Path("src/lib/onboard.ts").read_text()
print("SGR-only regex present:", r"const ANSI_RE = /\x1b\[[0-9;]*m/g;" in src)

pat = re.compile(r'\x1b\[[0-9;]*m')
samples = {
    "SGR color": "\x1b[31mred\x1b[0m",
    "Erase line": "\x1b[2Kline",
    "Cursor up": "\x1b[1Aup",
    "OSC title": "\x1b]0;title\x07text",
}
for name, value in samples.items():
    cleaned = pat.sub("", value)
    print(f"{name:11} -> {cleaned.encode()}")
PY

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/onboard.ts` around lines 26 - 27, The current ANSI_RE only strips SGR
sequences (ESC [ ... m) so CSI cursor/erase sequences and OSC sequences still
leak; update the ANSI_RE constant in onboard.ts to a broader pattern that
removes CSI sequences and OSC/STRING sequences (e.g., match ESC [ ... final-byte
not just 'm', ESC ] ... BEL, and other single-byte ESC sequences), replace the
existing /\x1b\[[0-9;]*m/g with a comprehensive regex that covers CSI and OSC
(and stray ESC sequences) to sanitize logs (ensure you update the ANSI_RE
identifier and run the existing tests or the provided sanitizer check to verify
samples "Erase line", "Cursor up", and "OSC title" are stripped).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/lib/onboard.ts`:
- Around line 26-27: The current ANSI_RE only strips SGR sequences (ESC [ ... m)
so CSI cursor/erase sequences and OSC sequences still leak; update the ANSI_RE
constant in onboard.ts to a broader pattern that removes CSI sequences and
OSC/STRING sequences (e.g., match ESC [ ... final-byte not just 'm', ESC ] ...
BEL, and other single-byte ESC sequences), replace the existing
/\x1b\[[0-9;]*m/g with a comprehensive regex that covers CSI and OSC (and stray
ESC sequences) to sanitize logs (ensure you update the ANSI_RE identifier and
run the existing tests or the provided sanitizer check to verify samples "Erase
line", "Cursor up", and "OSC title" are stripped).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e3426ca6-f846-49b8-85b2-52d0c5882c03

📥 Commits

Reviewing files that changed from the base of the PR and between ae6c21d and 096421c.

📒 Files selected for processing (1)

• src/lib/onboard.ts

[wscurran]

✨ Thanks for submitting this PR, which proposes a fix for a bug with the gateway startup process and may improve the overall quality of the codebase.

---

Possibly related open issues:

• #1423 API Key Displayed in Plaintext During Retry Flow - IssueFinder - SN 02
• #1605 NemoClaw Onboard Error

[ericksoa]

LGTM — diagnostics and redaction improvements look solid. Approving as-is.

Two items to address in a follow-up PR:

1. Missing timeout on openshell doctor logs — The runCaptureOpenshell(["doctor", "logs", ...]) call in the final-failure catch block has no timeout. If k3s is wedged, this will block the exit path indefinitely and make the failure experience worse. Please add a timeout (e.g., timeout: 10_000 in the opts).

2. Unbounded gateway start output — The old code truncated to 240 chars; the new code prints all lines. On a gateway that emits hundreds of lines, this could produce very noisy output. Consider capping to ~50 lines.