fix: stream sandbox logs via tail (Fixes #253)

[deepujain]

Summary

nemoclaw <sandbox> logs no longer calls the unsupported openshell sandbox logs subcommand. It now streams the sandbox gateway log by connecting into the sandbox and running tail directly, which works across affected OpenShell versions.

Changes

• bin/nemoclaw.js - replaced the logs dispatch with openshell sandbox connect <name> -- tail -n 200 /tmp/gateway.log, and added -f when --follow is requested.
• test/cli.test.js - updated the logs test to assert the new sandbox connect ... tail invocation and added coverage for --follow.

Testing

• npx vitest run test/cli.test.js passes (24 passed).
• npm test still has 2 pre-existing timeout failures in test/install-preflight.test.js and test/uninstall.test.js, unrelated to this change.

Fixes #253

Summary by CodeRabbit

• Refactor
  • Improved the internal implementation of sandbox log viewing to maintain consistent behavior and performance. The --follow flag continues to work as expected for live log streaming.

[coderabbitai]

📝 Walkthrough

Walkthrough

The change replaces an unrecognized openshell logs subcommand with a working implementation that uses openshell sandbox connect to tail the gateway log file directly, supporting both default and follow mode.

Changes

Cohort / File(s)	Summary
Log Command Implementation bin/nemoclaw.js	Introduced buildSandboxLogsArgs() helper to construct arguments for sandbox connect with tail -n 200 /tmp/gateway.log, optionally adding -f flag when follow mode is enabled. Updated sandboxLogs() to use this new approach instead of the unrecognized openshell logs subcommand.
Log Command Tests test/cli.test.js	Updated existing test to verify the new sandbox connect command format with tail -n 200. Added new test to confirm --follow flag is correctly propagated as the -f argument to tail.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A logs command lost in the mist,
Till openshell help we'd dismissed,
Now sandbox connect brings the gist,
Tail-ing logs with -f twist,
Gateway streams—bug fixed, we've kissed! 🐰

🚥 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 title 'fix: stream sandbox logs via tail (Fixes #253)' clearly summarizes the main change—replacing an unsupported openshell logs command with a tail-based streaming approach.
Linked Issues check	✅ Passed	The PR fully addresses issue #253 by implementing sandbox log viewing via 'sandbox connect' and 'tail -n 200 /tmp/gateway.log', supporting both one-shot and follow modes.
Out of Scope Changes check	✅ Passed	All changes in bin/nemoclaw.js and test/cli.test.js are directly scoped to fixing the logs subcommand functionality with no unrelated modifications detected.

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

test/cli.test.js (1)

141-148: Harden log-routing assertions to avoid false positives.

toContain(...) can still pass if extra unintended args are appended. Consider asserting exact argv tokens (and explicitly no -f in non-follow mode).

Proposed test tightening

-    expect(fs.readFileSync(markerFile, "utf8")).toContain("sandbox connect alpha -- tail -n 200 /tmp/gateway.log");
+    const argv = fs.readFileSync(markerFile, "utf8").trim().split(/\s+/);
+    expect(argv).toEqual(["sandbox", "connect", "alpha", "--", "tail", "-n", "200", "/tmp/gateway.log"]);
+    expect(argv).not.toContain("-f");
...
-    expect(fs.readFileSync(markerFile, "utf8")).toContain("sandbox connect alpha -- tail -n 200 -f /tmp/gateway.log");
+    const followArgv = fs.readFileSync(markerFile, "utf8").trim().split(/\s+/);
+    expect(followArgv).toEqual(["sandbox", "connect", "alpha", "--", "tail", "-n", "200", "-f", "/tmp/gateway.log"]);

Also applies to: 184-190

🤖 Prompt for AI Agents

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

In `@test/cli.test.js` around lines 141 - 148, The test currently uses
expect(...).toContain(...) which can give false positives; update the assertion
to parse the recorded command from markerFile (read via
fs.readFileSync(markerFile, "utf8")), split it into argv tokens (e.g., by
whitespace while preserving quoted tokens) and assert the token array equals the
exact expected argv array for the non-follow case (e.g.,
["sandbox","connect","alpha","--","tail","-n","200","/tmp/gateway.log"]) and
also explicitly assert that the "-f" flag is not present; apply the same
tightening to the other similar assertion around the runWithEnv usage at the
later block (the 184-190 case) so both tests validate exact tokens rather than
substring containment.

🤖 Prompt for all review comments with AI agents

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

Nitpick comments:
In `@test/cli.test.js`:
- Around line 141-148: The test currently uses expect(...).toContain(...) which
can give false positives; update the assertion to parse the recorded command
from markerFile (read via fs.readFileSync(markerFile, "utf8")), split it into
argv tokens (e.g., by whitespace while preserving quoted tokens) and assert the
token array equals the exact expected argv array for the non-follow case (e.g.,
["sandbox","connect","alpha","--","tail","-n","200","/tmp/gateway.log"]) and
also explicitly assert that the "-f" flag is not present; apply the same
tightening to the other similar assertion around the runWithEnv usage at the
later block (the 184-190 case) so both tests validate exact tokens rather than
substring containment.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d90c901d-27cb-48e5-a728-82314e810cfe

📥 Commits

Reviewing files that changed from the base of the PR and between eb4ba8c and 3acd02a.

📒 Files selected for processing (2)

• bin/nemoclaw.js
• test/cli.test.js