Skip to content

fix(cli): map --follow to --tail for openshell logs streaming#1148

Closed
senthilr-nv wants to merge 1 commit intoNVIDIA:mainfrom
senthilr-nv:fix/logs-follow-tail-regression
Closed

fix(cli): map --follow to --tail for openshell logs streaming#1148
senthilr-nv wants to merge 1 commit intoNVIDIA:mainfrom
senthilr-nv:fix/logs-follow-tail-regression

Conversation

@senthilr-nv
Copy link
Copy Markdown
Contributor

@senthilr-nv senthilr-nv commented Mar 31, 2026

Summary

  • nemoclaw <name> logs --follow passes --follow to openshell, but openshell expects --tail for live log streaming
  • Map the user-facing --follow flag to --tail internally, keeping the CLI interface unchanged
  • Update the test to verify the correct flag reaches openshell

This restores the fix from #436 (credit: @paritoshd-nv) which was inadvertently lost during a refactor.

Reproduction (before fix)

$ nemoclaw my-assistant logs --follow
error: unexpected argument '--follow' found
  Command failed (exit 2): openshell logs my-assistant --follow

Test plan

  • npm test — 675 passed, 3 skipped
  • Manual: nemoclaw my-assistant logs --follow streams live logs (DGX Spark, openshell 0.0.12)
  • Manual: nemoclaw my-assistant logs (without --follow) still works

Fixes #1146

Signed-off-by: senthilr-nv senthilr@nvidia.com

Summary by CodeRabbit

  • Bug Fixes
    • Fixed log-follow functionality by correcting the CLI argument translation used when retrieving live sandbox logs.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 31, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 83ff4668-b32f-438b-994d-5098485628af

📥 Commits

Reviewing files that changed from the base of the PR and between 1ba24f6 and e23a775.

📒 Files selected for processing (2)
  • bin/nemoclaw.js
  • test/cli.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/cli.test.js

📝 Walkthrough

Walkthrough

sandboxLogs() in bin/nemoclaw.js now appends --tail (not --follow) to the OpenShell CLI args when follow is requested. The corresponding test in test/cli.test.js was updated to expect --tail.

Changes

Cohort / File(s) Summary
CLI argument correction
bin/nemoclaw.js
Replaced appending --follow with --tail inside sandboxLogs() so the constructed args match OpenShell's expected flag.
Test assertion update
test/cli.test.js
Updated test description and marker assertion to verify openshell is invoked with logs <name> --tail instead of --follow.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐇 I nudged a flag, swapped follow for tail,
One tiny hop and the logs set sail,
Tests winked green, no more fail,
A carrot cheers the little trail! 🥕✨

🚥 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 accurately and concisely describes the main change: mapping the user-facing --follow CLI flag to OpenShell's --tail for logs streaming.
Linked Issues check ✅ Passed The changes fully satisfy issue #1146 requirements: --follow is mapped to --tail in sandboxLogs(), test assertions verify --tail is passed to openshell, and the user-facing flag remains --follow.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the --follow to --tail regression: bin/nemoclaw.js updates the mapping and test/cli.test.js verifies the correct argument is passed.

✏️ 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.

@senthilr-nv senthilr-nv self-assigned this Mar 31, 2026
@senthilr-nv senthilr-nv force-pushed the fix/logs-follow-tail-regression branch from 5f20bd8 to 1ba24f6 Compare March 31, 2026 06:51
NemoClaw advertises --follow as the user-facing flag for live log
streaming, but openshell expects --tail. Ensure the internal call
uses the correct flag while keeping the user-facing interface unchanged.

Fixes NVIDIA#1146

Signed-off-by: senthilr-nv <senthilr@nvidia.com>
@ericksoa
Copy link
Copy Markdown
Contributor

Superseded by #1187, which absorbed the --follow--tail fix along with reboot recovery and connect-based registry repair.

@ericksoa ericksoa closed this Mar 31, 2026
@senthilr-nv senthilr-nv deleted the fix/logs-follow-tail-regression branch March 31, 2026 17:49
kjw3 added a commit that referenced this pull request Mar 31, 2026
## Summary

- restore `nemoclaw <name> logs --follow` by mapping the user-facing
`--follow` flag to the OpenShell `--tail` flag
- add compatibility guidance when the installed OpenShell CLI is too old
for NemoClaw log streaming
- recover local sandbox inventory from the last onboard session so
sandboxes do not appear to disappear after a reboot
- import additional live sandbox names from the current OpenShell
gateway during list recovery
- make `nemoclaw <sandbox-name> connect` recover missing local registry
state directly instead of introducing a separate top-level `reconnect`
command

This absorbs the useful parts of `#1148` and the recovery ideas from
`#960`, but keeps the product surface on the command users already
expect to reach for after a reboot: `nemoclaw <name> connect`.

## Issues

Fixes #1146
Fixes #1165
Addresses #990
Addresses #1154

## Why this shape

- `#1148` is the right fix for the logs regression, but too narrow by
itself
- reboot recovery is more than a connect alias; NemoClaw has to rebuild
local inventory before connect can succeed consistently
- users will try `nemoclaw <name> connect` first, so recovery belongs on
that path rather than behind a new top-level command
- `list` now repairs stale or missing local sandbox inventory so the
user sees recoverable state instead of an empty registry after reboot

## Changes

### Logs

- map `nemoclaw logs --follow` to `openshell logs --tail`
- keep `--follow` as the NemoClaw user-facing flag
- print directed upgrade guidance for older or incompatible OpenShell
log subcommands instead of leaving only raw parser failures
- preserve normal SIGINT exit semantics for `logs --follow`

### Reboot recovery

- recover sandbox inventory from `~/.nemoclaw/onboard-session.json` when
local registry is missing or stale
- import additional live sandbox names from the active or recovered
OpenShell gateway during list recovery
- preserve policy presets and partial registries during recovery instead
of flattening them to an empty/default state
- keep registry writes serialized through the existing advisory lock
plus atomic rename path in `bin/lib/registry.js`

### Connect UX

- remove the top-level `nemoclaw reconnect` command
- make `nemoclaw <sandbox-name> connect` attempt recovery before falling
into the unknown-command path
- reuse the existing `sandboxConnect()` flow after recovery instead of
introducing a second connection path

## Cross-platform validation

### mac

- branch/commit: `fix/logs-follow-reconnect-reboot` / `5191d0d`
- commands:
- `/Users/kejones/Git/nvidia/NemoClaw-2/node_modules/.bin/vitest run
test/cli.test.js -t "connect"`
- result: passed (`6` connect-focused tests passed, `26` skipped)

### brev-cpu (`kj-nemoclaw-cpu-test`)

- branch/commit: `origin/fix/logs-follow-reconnect-reboot` / `5191d0d`
- node runtime used explicitly:
`/home/ubuntu/.nvm/versions/node/v22.22.2/bin/node`
- validation ran in a disposable worktree with a fake `openshell` shim
and last-session recovery seed
- result: passed
- verified behavior:
  - `nemoclaw alpha connect` recovered from missing local registry state
  - recovery probed `openshell sandbox list`
  - connect path then executed `openshell sandbox get alpha`
  - connect path then executed `openshell sandbox connect alpha`

### brev-gpu (`kj-nemoclaw-l40s-test`)

- branch/commit: `origin/fix/logs-follow-reconnect-reboot` / `5191d0d`
- node runtime used explicitly:
`/home/ubuntu/.nvm/versions/node/v22.22.1/bin/node`
- validation ran in a disposable worktree with a fake `openshell` shim
and last-session recovery seed
- result: passed
- verified behavior:
  - `nemoclaw alpha connect` recovered from missing local registry state
  - recovery probed `openshell sandbox list`
  - connect path then executed `openshell sandbox get alpha`
  - connect path then executed `openshell sandbox connect alpha`

### spark (`spark-d8c8`)

- branch/commit: `origin/fix/logs-follow-reconnect-reboot` / `5191d0d`
- node runtime used: `v22.22.1`
- validation ran in a disposable worktree with a fake `openshell` shim
and last-session recovery seed
- result: passed
- verified behavior:
  - `nemoclaw alpha connect` recovered from missing local registry state
  - recovery probed `openshell sandbox list`
  - connect path then executed `openshell sandbox get alpha`
  - connect path then executed `openshell sandbox connect alpha`

## Credits

This branch pulls forward ideas from earlier good-faith work, but
reworks them onto the current codebase and recovery model:

- `#1148` by `senthilr-nv`
- provided the correct narrow fix for the `--follow` -> `--tail` logs
regression
- `#960` by `WuKongAI-CMU` (`Peter`)
- provided useful recovery and reconnect test ideas, even though the
final product direction here keeps recovery on `connect`

## Notes

- did not merge `#1148` directly; absorbed its fix and test intent here
- did not keep the top-level `reconnect` command from `#960`; the final
UX puts recovery on `nemoclaw <name> connect`

Signed-off-by: Kevin Jones <kejones@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* OpenShell version compatibility check for logs with clear upgrade
guidance
* Enhanced sandbox discovery that recovers entries from prior onboarding
and live probes
* Automatic sandbox recovery during connect when a sandbox is missing
locally

* **Bug Fixes**
* Corrected logs streaming behavior for follow/non-follow modes and
improved error handling

* **Tests**
* Added CLI tests for logs follow behavior, registry
recovery/persistence, connect recovery, and SIGINT handling
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Aaron Erickson 🦞 <aerickson@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Regression: nemoclaw logs --follow broken again (was fixed in #424)

2 participants