fix(cli): detect tar failure in debug --output and set non-zero exit code by kagura-agent · Pull Request #1770 · NVIDIA/NemoClaw

kagura-agent · 2026-04-11T00:39:56Z

Problem

nemoclaw debug --output /nonexistent/path/debug.tar.gz prints "Tarball written to ..." and exits with code 0 even when tar fails. Users see a success message despite no tarball being created.

Fixes #1765

Root Cause

createTarball() called spawnSync("tar", ...) without checking the return status. The success message was unconditionally printed.

Fix

Check spawnSync result .status and .signal
On failure: print a red error message and set process.exitCode = 1
On signal kill (e.g. timeout): report the signal name

Testing

Added 2 regression tests for createTarball:
- Invalid output path → process.exitCode === 1
- Valid output path → tarball created, no error
All existing tests pass (npx vitest run src/lib/debug.test.ts — 10/10)
tsc clean, eslint clean, prettier clean

Signed-off-by: kagura-agent kagura.chen28@gmail.com

Summary by CodeRabbit

Bug Fixes
- Tarball creation now detects failures, emits a clear error message, sets the process exit code, and avoids showing misleading “Done…” instructions on failure.
Tests
- Added tests for tarball creation success and failure, verifying exit-code behavior and temporary-file cleanup.

coderabbitai · 2026-04-11T00:40:12Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 2f7349b7-ff4f-412b-987f-f4798bea30fb

📥 Commits

Reviewing files that changed from the base of the PR and between de1911a and ba6af85.

📒 Files selected for processing (1)

src/lib/debug.ts

🚧 Files skipped from review as they are similar to previous changes (1)

src/lib/debug.ts

📝 Walkthrough

Walkthrough

createTarball was exported and changed to return a boolean, inspect the spawned tar result, log errors and set process.exitCode = 1 on failure; runDebug conditionally prints final instructions only when tar succeeds. Tests for createTarball were added and process.exitCode handling was included in setup/teardown.

Changes

Cohort / File(s)	Summary
Tarball logic & error reporting `src/lib/debug.ts`	Added `RED` color and `error()` helper. Exported `createTarball(collectDir, output): boolean`. Capture `spawnSync("tar", ...)` result, detect non-zero exit or signal, log error, set `process.exitCode = 1`, and return `false` on failure. `runDebug` prints completion text only when tar creation succeeds.
Tests for createTarball `src/lib/debug.test.ts`	Added `describe("createTarball")` tests. Use `beforeEach`/`afterEach` to reset `process.exitCode` and clean temp dirs. Tests assert invalid output returns `false` and sets exit code; valid output returns `true` and writes the tarball file.

Sequence Diagram(s)

sequenceDiagram
    participant CLI as CLI (runDebug)
    participant Debug as createTarball
    participant Tar as tar (child)
    participant Console as Console

    CLI->>Debug: createTarball(collectDir, output)
    Debug->>Tar: spawnSync tar -czf output -C collectDir .
    Tar-->>Debug: exitCode / signal
    alt exitCode == 0
        Debug-->>CLI: return true
        CLI->>Console: print "[debug] Tarball written..." and instructions
    else failure (non-zero or signal)
        Debug->>Console: error("[debug] Tarball creation failed: ...")
        Debug-->>CLI: return false (set process.exitCode = 1)
    end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I dug through logs with whiskers bright,
Found tar that failed in dead of night.
Now crimson warnings softly sing,
Exit codes hop on proper spring. 📦🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title accurately describes the main change: detecting tar failure and setting a non-zero exit code in the debug command's tarball creation functionality.
Linked Issues check	✅ Passed	All coding requirements from issue `#1765` are met: tar failure detection, error reporting, non-zero exit code (1) on failure, and proper success messaging only on success.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to addressing the tar failure detection issue; no extraneous modifications were introduced.

_{✏️ 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

🧹 Nitpick comments (1)

src/lib/debug.ts (1)

435-447: Return a boolean from createTarball so callers can suppress success-style footer text on failure.

Right now createTarball only mutates process.exitCode; runDebug still prints completion guidance afterward. Returning an explicit success flag makes the control flow clearer and avoids mixed messaging.

Proposed refactor

-export function createTarball(collectDir: string, output: string): void {
+export function createTarball(collectDir: string, output: string): boolean {
   const result = spawnSync("tar", ["czf", output, "-C", dirname(collectDir), basename(collectDir)], {
     stdio: "inherit",
     timeout: 60_000,
   });
   if (result.status !== 0 || result.signal) {
     const reason = result.signal
       ? `killed by signal ${result.signal}`
       : `exited with code ${result.status ?? "unknown"}`;
     error(`Failed to create tarball at ${output} (tar ${reason})`);
     process.exitCode = 1;
-    return;
+    return false;
   }
   info(`Tarball written to ${output}`);
   warn(
     "Known secrets are auto-redacted, but please review for any remaining sensitive data before sharing.",
   );
   info("Attach this file to your GitHub issue.");
+  return true;
 }

-    if (output) {
-      createTarball(collectDir, output);
-    }
+    const tarOk = output ? createTarball(collectDir, output) : true;
 
     console.log("");
-    info("Done. If filing a bug, run with --output and attach the tarball to your issue:");
-    info("  nemoclaw debug --output /tmp/nemoclaw-debug.tar.gz");
+    if (tarOk) {
+      info("Done. If filing a bug, run with --output and attach the tarball to your issue:");
+      info("  nemoclaw debug --output /tmp/nemoclaw-debug.tar.gz");
+    } else {
+      warn("Diagnostics were collected, but tarball creation failed.");
+    }

🤖 Prompt for AI Agents

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

In `@src/lib/debug.ts` around lines 435 - 447, The createTarball function
currently only sets process.exitCode on failure; change its signature to return
a boolean (true on success, false on failure), keep the existing spawnSync logic
and error logging but return false where you set process.exitCode = 1 (and
return true at the end on success). Update callers (e.g., runDebug) to capture
the boolean result from createTarball and use it to suppress any post-run
success/footer messages when false. Ensure the function name createTarball
remains and only the return type/returns plus caller checks are modified.

🤖 Prompt for all review comments with AI agents

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

Nitpick comments:
In `@src/lib/debug.ts`:
- Around line 435-447: The createTarball function currently only sets
process.exitCode on failure; change its signature to return a boolean (true on
success, false on failure), keep the existing spawnSync logic and error logging
but return false where you set process.exitCode = 1 (and return true at the end
on success). Update callers (e.g., runDebug) to capture the boolean result from
createTarball and use it to suppress any post-run success/footer messages when
false. Ensure the function name createTarball remains and only the return
type/returns plus caller checks are modified.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9a26f6c6-98b9-4013-b666-3e6e1918e76d

📥 Commits

Reviewing files that changed from the base of the PR and between b409821 and cbb0fa2.

📒 Files selected for processing (2)

src/lib/debug.test.ts
src/lib/debug.ts

…code (NVIDIA#1765) `nemoclaw debug --output <invalid-path>` would print 'Tarball written to ...' and exit 0 even when tar failed. Now checks spawnSync return status and signal, reports the error, and sets process.exitCode = 1. Changes: - Check spawnSync result.status and result.signal in createTarball - Report error with red-colored message on failure - Set process.exitCode = 1 so the command exits non-zero - Export createTarball for testability - Add regression tests for both success and failure paths Signed-off-by: kagura-agent <kagura.chen28@gmail.com>

kagura-agent · 2026-04-11T00:45:04Z

@coderabbitai Good suggestion — updated createTarball to return a boolean. runDebug now uses the return value to suppress the success footer when tarball creation fails. Force-pushed with the change.

coderabbitai · 2026-04-11T00:47:09Z

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check current state of createTarball and runDebug in debug.ts
rg -n "createTarball|runDebug|exitCode|return" --type=ts -A2 -B2

Length of output: 297756

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate files =="
fd -a --strip-cwd-prefix "^debug\.ts$" src || true
fd -a --strip-cwd-prefix "^debug-command\.ts$" src || true

echo "== show src/lib/debug.ts =="
sed -n '1,240p' src/lib/debug.ts || true

echo "== show src/lib/debug-command.ts =="
sed -n '1,260p' src/lib/debug-command.ts || true

echo "== confirm tests reference boolean return =="
sed -n '55,100p' src/lib/debug.test.ts || true

Length of output: 11156

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== file length =="
wc -l src/lib/debug.ts || true

echo "== grep key functions =="
rg -nP '^\s*(export\s+)?function\s+(createTarball|runDebug)\b|Tarball written|footer|createTarball\(' src/lib/debug.ts -n -C3 || true

echo "== show rest of src/lib/debug.ts from line 240 =="
sed -n '240,999p' src/lib/debug.ts || true

Length of output: 10021

@kagura-agent Awesome—thanks for the quick follow-up. I pulled the force-push and verified:

createTarball now returns boolean and sets process.exitCode = 1 on failure (Line 435; failure path Lines 445–447). It logs a clear reason, and only on success prints “Tarball written…” plus the redaction reminder (Lines 448–452).
runDebug uses tarballOk to gate the success footer, so the footer is suppressed on failure (Lines 497–502; runDebug defined at Line 460).
Tests exercise the boolean + exitCode behavior (src/lib/debug.test.ts, Lines 69–85 in the shown slice).

Looks good to me.

Optional nit for later: consider mapping signal termination to a distinct exit code (128 + signal) for parity with uninstall-command, but keeping 1 is perfectly fine since you already include the signal detail in the error.

wscurran · 2026-04-13T15:18:12Z

✨ Thanks for submitting this PR, which proposes a fix for a bug where tarball creation failure is not detected and a non-zero exit code is not set.

Possibly related open issues:

#1765 [All Platforms][Nemoclaw] nemoclaw debug --output ` reports "Tarball written to ..." even when tar fails; exit code remains 0

prekshivyas

Good fix — tar failures are now detected and reported correctly. Note: this will conflict with #1775 (already approved) around the same completion message block. Merge #1775 first, then rebase this. @kagura-agent can you rebase onto main after #1775 lands?

Refresh user-facing docs against the 34 commits merged between v0.0.17 and v0.0.18. Highlights: - Replace the Ollama 0.0.0.0 binding guidance with the new authenticated reverse proxy on 127.0.0.1:11435 (#1922). - Document the compatible-endpoint provider defaulting to /v1/chat/completions and the NEMOCLAW_PREFERRED_API=openai-responses opt-in (#1984). - Add the new nemoclaw upgrade-sandboxes command with --check, --auto, and --yes flags (#1943). - Note the cross-sandbox messaging overlap warning and 409 detection in nemoclaw <name> status (#1953). - Document the messaging-token rotation auto-rebuild flow (#1967). - Cover new troubleshooting entries for the Ollama auth proxy, IPv6 localhost resolution, orphan SSH port-forward cleanup on re-onboard, and rotated messaging credentials (#1978, #1950). - Note tar failure exit code for nemoclaw debug --output (#1770) and the orphaned openshell process cleanup in nemoclaw uninstall (#1940). Also: - Extend docs/.docs-skip to exclude the experimental sandbox-mgmt shields and config commands (#1976). - Fix a sphinx-autobuild infinite rebuild loop in docs/conf.py by writing docs/project.json only when its contents change. - Bump the docs version switcher preferred entry to 0.0.18. - Regenerate nemoclaw-user-* agent skills from docs/. Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> Made-with: Cursor

## Summary Refresh user-facing documentation against the 34 commits merged between v0.0.17 and v0.0.18, bump the docs version switcher to v0.0.18, and fix a `sphinx-autobuild` infinite-rebuild loop triggered by `docs/conf.py`. ## Changes - **Ollama authenticated reverse proxy** (#1922): Replace the `0.0.0.0:11434` guidance in `docs/inference/use-local-inference.md` with the new token-gated proxy on `127.0.0.1:11435`, including persisted token, health-check exemption, and sandbox provider wiring. Replace the matching troubleshooting entry in `docs/reference/troubleshooting.md`. - **Compatible-endpoint default API path** (#1984): Document that the compatible-endpoint provider now defaults to `/v1/chat/completions` and update `NEMOCLAW_PREFERRED_API` to describe `openai-responses` as the opt-in instead of `openai-completions`. Updates in `use-local-inference.md`, `switch-inference-providers.md`, and `troubleshooting.md`. - **`nemoclaw upgrade-sandboxes` command** (#1943): Add a new reference entry in `docs/reference/commands.md` covering `--check`, `--auto`, and `--yes` flags. - **Messaging token rotation auto-rebuild** (#1967, #1953): Note the automatic rebuild behavior and cross-sandbox overlap warning in `docs/deployment/set-up-telegram-bridge.md`, `commands.md`, and `troubleshooting.md`. - **Other troubleshooting additions**: - `localhost` → `127.0.0.1` IPv6 note (#1978) - Orphan SSH port-forward cleanup on re-onboard (#1950) - Orphan `openshell` process cleanup in `nemoclaw uninstall` (#1940) - Non-zero exit on tar failure in `nemoclaw debug --output` (#1770) - **Skip list**: Extend `docs/.docs-skip` to exclude the experimental sandbox-mgmt shields and config commands feature (#1976), which was explicitly merged as not-yet-documented. - **Build stability**: `docs/conf.py` now writes `docs/project.json` only when contents change, so `make docs-live` / `sphinx-autobuild` no longer detects its own generated file as a source change and enters an infinite rebuild loop. - **Version switcher**: Bump `docs/versions1.json` and `docs/project.json` preferred entry to v0.0.18 so this refresh renders under the new version. - **Agent skills**: Regenerate `nemoclaw-user-*` skills from `docs/` with `scripts/docs-to-skills.py`. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Verification - [x] `npx prek run --all-files` passes (ran via pre-commit hook on staged files) - [ ] `npm test` passes - [ ] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [x] Docs updated for user-facing behavior changes - [x] `make docs` builds without warnings (doc changes only) - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) ## AI Disclosure - [x] AI-assisted — tool: Cursor --- Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> Made with [Cursor](https://cursor.com)  ## Summary by CodeRabbit ## Release Notes * **New Features** * Added `nemoclaw upgrade-sandboxes` command to rebuild sandboxes when base-image digests change. * Introduced authenticated reverse proxy for local Ollama inference with token-based access control. * Automatic sandbox backup, recreation, and restore when messaging credentials are updated. * Cross-sandbox messaging token overlap detection with status warnings. * **Improvements** * Compatible-endpoint provider now defaults to `/v1/chat/completions` API path. * Enhanced troubleshooting documentation with new diagnostics sections. * **Documentation** * Updated onboarding and configuration guides. * Expanded version documentation to 0.0.18.  Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>

coderabbitai Bot reviewed Apr 11, 2026

View reviewed changes

kagura-agent force-pushed the fix/debug-tarball-exit-code branch from cbb0fa2 to de1911a Compare April 11, 2026 00:44

wscurran added bug Something fails against expected or documented behavior NemoClaw CLI labels Apr 13, 2026

cv added the v0.0.18 label Apr 16, 2026

prekshivyas self-assigned this Apr 16, 2026

prekshivyas approved these changes Apr 16, 2026

View reviewed changes

prekshivyas and others added 2 commits April 16, 2026 12:54

Merge branch 'main' into fix/debug-tarball-exit-code

ba6af85

Merge branch 'main' into fix/debug-tarball-exit-code

e0ce7f9

ericksoa merged commit f7a3c33 into NVIDIA:main Apr 16, 2026
9 checks passed

miyoungc mentioned this pull request Apr 17, 2026

docs: catch up documentation for v0.0.18 changes #2033

Merged

13 tasks

wscurran added area: cli Command line interface, flags, terminal UX, or output bug-fix PR fixes a bug or regression and removed NemoClaw CLI bug Something fails against expected or documented behavior labels Jun 3, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(cli): detect tar failure in debug --output and set non-zero exit code#1770

fix(cli): detect tar failure in debug --output and set non-zero exit code#1770
ericksoa merged 3 commits into
NVIDIA:mainfrom
kagura-agent:fix/debug-tarball-exit-code

kagura-agent commented Apr 11, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Apr 11, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

kagura-agent commented Apr 11, 2026

Uh oh!

coderabbitai Bot commented Apr 11, 2026

Uh oh!

wscurran commented Apr 13, 2026

Uh oh!

prekshivyas left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

kagura-agent commented Apr 11, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Root Cause

Fix

Testing

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Apr 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

kagura-agent commented Apr 11, 2026

Uh oh!

coderabbitai Bot commented Apr 11, 2026

Uh oh!

wscurran commented Apr 13, 2026

Uh oh!

prekshivyas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kagura-agent commented Apr 11, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 11, 2026 •

edited

Loading