Skip to content

fix(cli): add sudo hint to debug dmesg-restricted message#4384

Open
jason-ma-nv wants to merge 3 commits into
mainfrom
fix/4366-dmesg-restricted-sudo-hint
Open

fix(cli): add sudo hint to debug dmesg-restricted message#4384
jason-ma-nv wants to merge 3 commits into
mainfrom
fix/4366-dmesg-restricted-sudo-hint

Conversation

@jason-ma-nv
Copy link
Copy Markdown
Contributor

@jason-ma-nv jason-ma-nv commented May 28, 2026

Summary

nemoclaw debug running as a non-root user with kernel.dmesg_restrict=1 already explains in the Kernel Messages section that dmesg is restricted, but it does not tell the user how to include kernel logs. Add a "Re-run with sudo nemoclaw debug" hint to the skipped message so users and triagers see a concrete next step, matching the spec in the issue's Suggested Fix. The optional TTY-aware sudo -n dmesg fallback in the same Suggested Fix is intentionally out of scope for this minimum-viable change.

Related Issue

Fixes #4366.

Changes

  • src/lib/diagnostics/debug.ts: extend dmesgRestrictedMessage to append a second line — Re-run with `sudo nemoclaw debug` to include kernel logs in this report. — to the parenthetical block, and export the function so the wording can be unit-tested.
  • src/lib/diagnostics/debug.test.ts: two new #4366 tests asserting the message still explains why kernel messages were skipped and that it contains the sudo re-run hint.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Verification

  • npx prek run --all-files passes
  • npm test passes
  • Tests added or updated for new or changed behavior
  • No secrets, API keys, or credentials committed
  • Docs updated for user-facing behavior changes
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Jason Ma jama@nvidia.com

Summary by CodeRabbit

  • New Features

    • Enhanced error messaging when kernel log collection is unavailable due to access restrictions, now includes a clear explanation and guidance on using elevated permissions to retry.
  • Tests

    • Added verification for diagnostic messages when kernel log access is restricted.

Review Change Stack

When `kernel.dmesg_restrict=1` and the user runs `nemoclaw debug` as
non-root, the Kernel Messages section explained why the section was
skipped but did not tell the user how to include kernel logs anyway.
Extend `dmesgRestrictedMessage` to append a "Re-run with `sudo
nemoclaw debug`" hint so users and triagers see a concrete next step,
matching the spec in the bug's Suggested Fix.

The TTY-aware `sudo -n dmesg` fallback from the same Suggested Fix is
intentionally out of scope for this minimum-viable change.

Export `dmesgRestrictedMessage` so the wording can be pinned by a
unit test.

Fixes #4366.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Jason Ma <jama@nvidia.com>
@jason-ma-nv jason-ma-nv self-assigned this May 28, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 957575c7-b6c0-4a46-b5d3-3a72288c2184

📥 Commits

Reviewing files that changed from the base of the PR and between 1daf081 and ad97b46.

📒 Files selected for processing (2)
  • src/lib/diagnostics/debug.test.ts
  • src/lib/diagnostics/debug.ts

📝 Walkthrough

Walkthrough

This PR exports the dmesgRestrictedMessage function and enhances its output with a clearer user-facing message that explains kernel logs were skipped due to dmesg restrictions and includes a hint to re-run with sudo nemoclaw debug to include kernel logs. Tests verify the new message format.

Changes

dmesg-Restricted Message Export and Testing

Layer / File(s) Summary
Export and enhance dmesg-restricted message
src/lib/diagnostics/debug.ts
dmesgRestrictedMessage changes from internal helper to exported function and returns a multi-line guidance string that includes the restriction reason and a sudo nemoclaw debug re-run hint.
Test dmesg-restricted message output
src/lib/diagnostics/debug.test.ts
Import of dmesgRestrictedMessage and new vitest suite with two test cases that verify the message includes the kernel-skip explanation, the provided restriction reason, and the sudo re-run hint.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#3854: Both PRs change src/lib/diagnostics/debug.ts and its debug tests to improve handling of restricted/unreadable dmesg output—moving from raw permission errors to context-aware "kernel messages skipped" messaging (including sudo re-run hints).
  • NVIDIA/NemoClaw#3890: Both PRs modify the same src/lib/diagnostics/debug.ts "restricted dmesg" codepath—one adds/exports the user-facing dmesgRestrictedMessage and tests its exact multi-line output, while the other exports/parameterizes the restriction-detection helper and adds tests that drive the restricted-dmesg explanation.

Suggested labels

NemoClaw CLI, fix, v0.0.51

Suggested reviewers

  • jyaunches
  • ericksoa

Poem

🐰 A rabbit hops through debug logs with glee,
Exporting messages so users can see—
"Run sudo again!" the message now cries,
Kernel logs hidden from non-root eyes.
With tests that confirm the hint shines so clear! 📋

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.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 title clearly summarizes the main change: adding a sudo hint to the dmesg-restricted message in the debug output, which directly addresses the core requirement of issue #4366.
Linked Issues check ✅ Passed The PR successfully implements the primary objective from issue #4366: adding a sudo re-run hint to the dmesg-restricted message to guide users on how to include kernel logs.
Out of Scope Changes check ✅ Passed All changes are directly scoped to the linked issue #4366: exporting the function, extending the message with the sudo hint, and adding corresponding unit tests. No unrelated modifications detected.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/4366-dmesg-restricted-sudo-hint

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 28, 2026

E2E Advisor Recommendation

Required E2E: diagnostics-e2e
Optional E2E: None

Dispatch hint: diagnostics-e2e

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • diagnostics-e2e (medium): Runs the existing diagnostics E2E covering nemoclaw debug --quick, nemoclaw debug --output, debug tarball creation, and credential redaction. This is the closest existing E2E for changes in src/lib/diagnostics/debug.ts and should verify the user-facing debug command still works end-to-end.

Optional E2E

  • None.

New E2E recommendations

  • diagnostics-debug-cli (medium): Existing diagnostics E2E exercises debug tarball creation but does not appear to assert the restricted-dmesg path or the new sudo nemoclaw debug re-run hint. Add a hermetic case that forces /proc/sys/kernel/dmesg_restrict-style behavior or stubs dmesg permission denial and verifies dmesg.txt contains the restricted-access explanation without leaking secrets.
    • Suggested test: Extend test/e2e/test-diagnostics.sh with a TC-DIAG restricted-dmesg assertion for the skipped kernel logs message and sudo re-run hint.

Dispatch hint

  • Workflow: E2E / Nightly
  • jobs input: diagnostics-e2e

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 28, 2026

E2E Scenario Advisor Recommendation

Required scenario E2E: None
Optional scenario E2E: None

Workflow run

Full scenario advisor summary

E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required scenario E2E

  • None. No scenario workflow, scenario metadata, scenario runtime, or validation-suite files changed.

Optional scenario E2E

  • None.

Relevant changed files

  • None.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 28, 2026

PR Review Advisor

Findings: 0 needs attention, 2 worth checking, 0 nice ideas
Since last review: 0 prior items resolved, 2 still apply, 0 new items found

Review findings

🛠️ Needs attention

  • None.

🔎 Worth checking

  • Source-of-truth review needed: Restricted dmesg recovery guidance: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: `dmesgRestrictedMessage()` is localized and hardcodes `sudo nemoclaw debug`; `collectDmesg()` calls it without passing `DebugOptions.quick` or `output`, so the hint cannot reflect the user's original command.
  • Tighten the sudo debug rerun hint (src/lib/diagnostics/debug.ts:160): The restricted-dmesg recovery message now gives users a concrete next step, but it always recommends `sudo nemoclaw debug`. That can nudge users who ran a scoped command such as `nemoclaw debug --quick` to rerun the full diagnostics collector as root, potentially collecting broader host, container, network, process, Docker, OpenShell, and kernel data than intended. It also does not preserve an output workflow or warn at the hint site that privileged diagnostics/kernel logs may include sensitive data.
    • Recommendation: Build the rerun hint from the current debug options where possible, preserving relevant flags such as `--quick` and output guidance. Add wording near the sudo hint that privileged diagnostics and kernel logs may contain sensitive data and should be reviewed before sharing. Add a regression test for the option-aware message path.
    • Evidence: The helper hardcodes `Re-run with `sudo nemoclaw debug` to include kernel logs in this report.)`. Issue [All Platforms][CLI&UX] nemoclaw debug: dmesg-restricted kernel section still not labeled as clearly as spec (no sudo hint) #4366's reproduction and example use `nemoclaw debug --quick` and `sudo nemoclaw debug --quick`. The existing sensitive-data warning is emitted by `createTarball()` only when an output tarball is written, not when this sudo recovery hint is shown.

🌱 Nice ideas

  • None.
Since last review details

Current findings:

  • Source-of-truth review needed: Restricted dmesg recovery guidance: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: `dmesgRestrictedMessage()` is localized and hardcodes `sudo nemoclaw debug`; `collectDmesg()` calls it without passing `DebugOptions.quick` or `output`, so the hint cannot reflect the user's original command.
  • Tighten the sudo debug rerun hint (src/lib/diagnostics/debug.ts:160): The restricted-dmesg recovery message now gives users a concrete next step, but it always recommends `sudo nemoclaw debug`. That can nudge users who ran a scoped command such as `nemoclaw debug --quick` to rerun the full diagnostics collector as root, potentially collecting broader host, container, network, process, Docker, OpenShell, and kernel data than intended. It also does not preserve an output workflow or warn at the hint site that privileged diagnostics/kernel logs may include sensitive data.
    • Recommendation: Build the rerun hint from the current debug options where possible, preserving relevant flags such as `--quick` and output guidance. Add wording near the sudo hint that privileged diagnostics and kernel logs may contain sensitive data and should be reviewed before sharing. Add a regression test for the option-aware message path.
    • Evidence: The helper hardcodes `Re-run with `sudo nemoclaw debug` to include kernel logs in this report.)`. Issue [All Platforms][CLI&UX] nemoclaw debug: dmesg-restricted kernel section still not labeled as clearly as spec (no sudo hint) #4366's reproduction and example use `nemoclaw debug --quick` and `sudo nemoclaw debug --quick`. The existing sensitive-data warning is emitted by `createTarball()` only when an output tarball is written, not when this sudo recovery hint is shown.

Workflow run details

This is an automated advisory review. A human maintainer must make the final merge decision.

@jason-ma-nv jason-ma-nv added the v0.0.54 Release target label May 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v0.0.54 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[All Platforms][CLI&UX] nemoclaw debug: dmesg-restricted kernel section still not labeled as clearly as spec (no sudo hint)

1 participant