fix: use named exec for bridge health probe (Fixes #2018)

[deepujain]

Fixes #2018

Summary

checkMessagingBridgeHealth() was calling openshell sandbox exec with the sandbox name as a positional argument. That form silently misses the intended sandbox on current OpenShell, so the Telegram conflict probe never checks the right target.

This change switches the probe to the named-sandbox form that OpenShell expects.

Changes

• src/nemoclaw.ts: changed the bridge-health probe to use openshell sandbox exec -n <sandbox> -- sh -c ....
• test/cli.test.ts: added a CLI regression test that drives nemoclaw status with a fake openshell binary and verifies the exact argv shape used for the bridge-health exec call.

Testing

• npm run build:cli
• npm run typecheck:cli
• npm test -- test/cli.test.ts

Evidence It Works

• The new CLI regression test confirms the status path now invokes sandbox exec -n alpha -- sh -c ....
• The focused CLI suite passes with the new coverage in place.

Notes

• I also ran npm test. In this worktree it still reproduces unrelated existing failures in test/legacy-path-guard.test.ts, src/lib/preflight.test.ts, src/lib/sandbox-version.test.ts, and test/install-preflight.test.ts.

Signed-off-by: Deepak Jain [email protected]

Summary by CodeRabbit

• Chores
  • Adjusted internal messaging-bridge health check and gateway log handling to use a consistent named-sandbox invocation format.
• Documentation
  • Updated documentation for gateway log reading and health-check probe behavior.
• Tests
  • Added a CLI test verifying status/health-check uses the named-sandbox execution form and exits successfully.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 02eb0654-60b6-4fc5-a745-4490bb3cec2e

📥 Commits

Reviewing files that changed from the base of the PR and between 9f3caf4 and 2501719.

📒 Files selected for processing (2)

• src/nemoclaw.ts
• test/cli.test.ts

---

📝 Walkthrough

Walkthrough

Updated messaging-bridge helpers in src/nemoclaw.ts to pass sandbox names to openshell sandbox exec using the named form (-n <sandboxName> --) instead of a positional argument. Added/extended JSDoc and a CLI test confirming the new exec-argument shape for nemoclaw status.

Changes

Cohort / File(s)	Summary
Exec arg fix src/nemoclaw.ts	Rewrote openshell sandbox exec invocations in checkMessagingBridgeHealth and readGatewayLog to use ["sandbox","exec","-n", sandboxName,"--","sh","-c", script]; preserved grep/tail logic, timeout and stdio handling; added/extended JSDoc.
Test coverage test/cli.test.ts	Added CLI dispatch test for nemoclaw status that stubs openshell, creates a temp registry sandbox alpha with messagingChannels, asserts recorded openshell calls use the -n alpha -- form and that the old positional form does not appear.

Sequence Diagram(s)

(Skipped — changes are a targeted bugfix and tests; no new multi-component flow requiring visualization.)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• fix: use named exec for bridge health probe (Fixes #2018) #2037 — Makes the same change switching openshell sandbox exec calls to the named-sandbox form and adds matching CLI tests.

Poem

🐰 I nudged the flag where names belong,

sh -c hops after, tidy and strong.
Tests gave a sniff, then thumped with delight,
Logs now report what was hidden from sight.
A little carrot for fixed command might.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.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 fix: changing the openshell exec call to use the named sandbox form (-n flag) for bridge health probes, which directly addresses issue #2018.
Linked Issues check	✅ Passed	The PR implements the exact fix specified in issue #2018: replacing positional sandbox argument with named form 'sandbox exec -n --' in checkMessagingBridgeHealth() and readGatewayLog(), plus adds regression test confirming the correct argv shape.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing issue #2018: updates to src/nemoclaw.ts messaging bridge health probes and a corresponding test in test/cli.test.ts for regression coverage; no unrelated modifications present.

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

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

Inline comments:
In `@src/nemoclaw.ts`:
- Line 1061: readGatewayLog still calls the sandbox exec command using the old
positional form; update its arg array to use the named namespace flag like the
corrected call at line 1061—i.e. change the invocation in readGatewayLog so the
args become ["sandbox", "exec", "-n", sandboxName, "--", "sh", "-c", script]
(matching the "-n", sandboxName, "--" pattern) instead of passing sandboxName
positionally so the namespace is specified consistently.

🪄 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: 6abf4984-8ac1-472f-9ca6-ad5e627da906

📥 Commits

Reviewing files that changed from the base of the PR and between 9222cae and 5c9195d.

📒 Files selected for processing (2)

• src/nemoclaw.ts
• test/cli.test.ts

[deepujain]

Rebased on current main and addressed the CodeRabbit follow-up. The status helpers now use the named sandbox exec form consistently, and the focused CLI suite passes locally.

[ericksoa]

Clean, focused fix. The -n flag form with -- separator is the correct named-sandbox invocation for OpenShell. Both callsites (bridge health probe and gateway log tail) get the same treatment. Test creates a fake openshell binary and asserts the exact argv shape — solid regression coverage.

CI hasn't run yet (only CodeRabbit completed) — please ensure the checks workflow passes before merging.