test(e2e): add diagnostics, debug tarball, and credential E2E tests - #2243
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdded a new diagnostics end-to-end Bash test script and a guarded nightly CI job to run it; CI uploads logs on failure and the failure-notification job now considers the diagnostics job result. Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI Runner
participant Script as test-diagnostics.sh
participant Host as Host (nemoclaw CLI)
participant Sandbox as Sandbox (onboarded env)
participant Artifacts as GitHub Artifacts
CI->>Script: checkout repo + env (NVIDIA_API_KEY, CI flags)
Script->>Host: ensure `nemoclaw` present (install/adjust PATH)
Script->>Host: run TC-DIAG-04 (version) and TC-DIAG-02 (quick debug)
Script->>Host: run `nemoclaw onboard` (clear lock, provision sandbox)
Script->>Sandbox: read `openclaw.json`, verify model
Script->>Host: run full `nemoclaw debug --output`, extract & scan tarball
Script->>Host: run credentials list/reset checks
Script->>CI: return aggregated result & logs
CI-->>Artifacts: upload `test-diagnostics-*.log` on failure
CI->>CI: notify-on-failure evaluates diagnostics-e2e result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/e2e/test-diagnostics.sh`:
- Line 287: The grep that sets pattern_leaks currently uses
"nvapi-[A-Za-z0-9_-]{20,}" which is too permissive on length and can miss
shorter nvapi- tokens; update the grep regex used in the pattern_leaks
assignment to match the same nvapi redaction regex used in src/lib/debug.ts
(i.e., replace the "{20,}" segment with the length constraint/pattern from the
debug.ts redaction rule) so the test detects the same nvapi token lengths as the
runtime redactor.
- Around line 314-321: The current check uses grep on config_output and then
allows an empty model_name to pass TC-DIAG-05, causing false positives; change
the logic in test-diagnostics.sh to bypass the initial grep and instead parse
config_output with python3 (the existing model_name extraction) and require the
parsed value to be non-empty: call python3 to load json and print
d.get('model','') (or better print d['model'] if you want a hard error when
missing) into model_name, then if [[ -n "$model_name" ]]; do pass "TC-DIAG-05:
Model configured in openclaw.json ($model_name)"; else fail "TC-DIAG-05: missing
or empty model field in openclaw.json" fi, ensuring you reference the variables
config_output and model_name and the test identifier TC-DIAG-05 to locate and
update the code.
🪄 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 Plus
Run ID: f2caf48e-8e3f-42e0-8b2b-e92be2efa33f
📒 Files selected for processing (2)
.github/workflows/nightly-e2e.yamltest/e2e/test-diagnostics.sh
TruongNguyenG
force-pushed
the
feat/e2e-diagnostics
branch
3 times, most recently
from
April 22, 2026 09:52
b26d7d2 to
7ad8224
Compare
TruongNguyenG
force-pushed
the
feat/e2e-diagnostics
branch
from
April 22, 2026 16:02
7ad8224 to
9dd600b
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/nightly-e2e.yaml:
- Around line 452-457: Add NEMOCLAW_RECREATE_SANDBOX=1 to the environment block
for the "Run diagnostics E2E test" job so the diagnostics test rebuilds the
sandbox instead of reusing the install-time one; update the env section where
NVIDIA_API_KEY, NEMOCLAW_NON_INTERACTIVE and
NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE are set to include
NEMOCLAW_RECREATE_SANDBOX: "1" (this is needed because install.sh
--non-interactive / nemoclaw onboard --non-interactive reads that env var and
controls sandbox recreation for test/e2e/test-diagnostics.sh).
In `@test/e2e/test-diagnostics.sh`:
- Around line 303-320: The test currently fails TC-DIAG-05 if the sandbox
openclaw.json lacks a non-empty model field (the model_name extraction and
subsequent fail), but that field isn't authoritative; remove or neutralize the
model field assertion: delete or comment out the model_name extraction block
(the model_name=$(echo "$config_output" | python3 ...) and the if [[ -n
"$model_name" ]] ... else fail ... fi) and keep only the check that
openclaw.json is readable (sandbox_exec + pass for openclaw.json readable);
ensure TC-DIAG-05 no longer fails based on the model field so downstream
host-side `nemoclaw status` can assert model visibility.
- Around line 208-233: The test currently waits indefinitely for the `nemoclaw
debug --quick` call (line using debug_output=$(nemoclaw debug --quick --output
"$output_file" 2>&1) || debug_rc=$?) so enforce the 30s budget at the command
boundary by running the command through a timeout (e.g. `timeout 30s ...`) or
equivalent, capture both stdout/stderr into `debug_output`, set `debug_rc` to a
distinct non-zero value when timeout occurs, and ensure the rest of the logic
(checks on `debug_rc`, `output_file`, and elapsed) still works; update
references to `debug_output`, `debug_rc`, and the command invocation so a
hung/long-running `nemoclaw debug` triggers failure at 30s instead of waiting
for the process to finish.
🪄 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 Plus
Run ID: 703ab754-f3d9-426f-a8dd-e4506669c9e0
📒 Files selected for processing (2)
.github/workflows/nightly-e2e.yamltest/e2e/test-diagnostics.sh
Comment on lines
+303
to
+320
| log " Checking openclaw.json config inside sandbox..." | ||
| local config_output | ||
| config_output=$(sandbox_exec "cat /sandbox/.openclaw/openclaw.json 2>/dev/null" 2>&1) || true | ||
|
|
||
| if [[ -z "$config_output" ]]; then | ||
| fail "TC-DIAG-05: Config" "Could not read openclaw.json inside sandbox" | ||
| return | ||
| fi | ||
|
|
||
| pass "TC-DIAG-05: openclaw.json readable inside sandbox" | ||
|
|
||
| local model_name | ||
| model_name=$(echo "$config_output" | python3 -c "import sys,json; d=json.load(sys.stdin); v=d.get('model'); print(v.strip() if isinstance(v,str) else '')" 2>/dev/null) || true | ||
| if [[ -n "$model_name" ]]; then | ||
| pass "TC-DIAG-05: Model configured in openclaw.json ($model_name)" | ||
| else | ||
| fail "TC-DIAG-05: Model" "No non-empty model field in openclaw.json" | ||
| fi |
Contributor
There was a problem hiding this comment.
Don't fail TC-DIAG-05 on a field that isn't guaranteed to live in openclaw.json.
The host-side nemoclaw status check below is the right place to assert model visibility. Requiring a non-empty model field inside /sandbox/.openclaw/openclaw.json can turn this into a false negative, because the sandbox config file is not the authoritative source for model metadata.
Suggested fix
- local model_name
- model_name=$(echo "$config_output" | python3 -c "import sys,json; d=json.load(sys.stdin); v=d.get('model'); print(v.strip() if isinstance(v,str) else '')" 2>/dev/null) || true
- if [[ -n "$model_name" ]]; then
- pass "TC-DIAG-05: Model configured in openclaw.json ($model_name)"
- else
- fail "TC-DIAG-05: Model" "No non-empty model field in openclaw.json"
- fi
+ if echo "$config_output" | python3 -c "import sys, json; json.load(sys.stdin)" >/dev/null 2>&1; then
+ pass "TC-DIAG-05: openclaw.json is valid JSON"
+ else
+ fail "TC-DIAG-05: Config" "openclaw.json is not valid JSON"
+ fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/e2e/test-diagnostics.sh` around lines 303 - 320, The test currently
fails TC-DIAG-05 if the sandbox openclaw.json lacks a non-empty model field (the
model_name extraction and subsequent fail), but that field isn't authoritative;
remove or neutralize the model field assertion: delete or comment out the
model_name extraction block (the model_name=$(echo "$config_output" | python3
...) and the if [[ -n "$model_name" ]] ... else fail ... fi) and keep only the
check that openclaw.json is readable (sandbox_exec + pass for openclaw.json
readable); ensure TC-DIAG-05 no longer fails based on the model field so
downstream host-side `nemoclaw status` can assert model visibility.
Add test/e2e/test-diagnostics.sh with 5 test cases: - TC-DIAG-04: nemoclaw --version semver output - TC-DIAG-02: nemoclaw debug --quick fast archive - TC-DIAG-01: nemoclaw debug --output tarball + credential sanitization - TC-DIAG-05: sandbox inference config (openclaw.json + nemoclaw status) - TC-DIAG-03: credentials list (no values exposed) Integrates diagnostics-e2e job into nightly-e2e.yaml. Closes NVIDIA#2242 Signed-off-by: Truong Nguyen <tgnguyen@nvidia.com> Made-with: Cursor
TruongNguyenG
force-pushed
the
feat/e2e-diagnostics
branch
from
April 22, 2026 16:17
9dd600b to
df8ab1b
Compare
ericksoa
approved these changes
Apr 22, 2026
ericksoa
left a comment
ericksoa
left a comment
Contributor
There was a problem hiding this comment.
Well-structured diagnostics E2E suite. Credential sanitization test (TC-DIAG-01) is good security coverage. Destructive test correctly ordered last. LGTM.
wscurran
added
area: e2e
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add
test/e2e/test-diagnostics.shwith 5 end-to-end test cases covering CLI version output, debug snapshots, credential sanitization in debug tarballs, sandbox inference config visibility, and credential list safety. Integrates asdiagnostics-e2ejob innightly-e2e.yaml.Related Issue
Closes #2242
Changes
nemoclaw --version, verifies output matches semver pattern and exits with code 0. No sandbox needed.nemoclaw debug --quick --output <path>, verifies a non-empty archive is produced within 30 seconds. No sandbox needed.nemoclaw debug --output <path>, extracts the tarball, greps all files for the real API key value andnvapi-patterns. Verifies no credentials found in any collected file.openclaw.jsoninside the sandbox via SSH, verifies the model field is present. Runsnemoclaw statusfrom the host and verifies the Model field appears in output.nemoclaw credentials list, verifies key names are shown but the real API key value is not exposed. Handles the CI case where the credential store is empty (API key passed via env var).diagnostics-e2ejob tonightly-e2e.yamlwith 45-minute timeout, artifact upload on failure, and wired intonotify-on-failure.Type of Change
Verification
npx prek run --all-filespassesAI Disclosure
Signed-off-by: Truong Nguyen tgnguyen@nvidia.com
Made with Cursor
Summary by CodeRabbit
Tests
Chores