fix(status): use -n flag for sandbox exec in checkMessagingBridgeHealth

[Sanjays2402]

Summary

checkMessagingBridgeHealth() calls openshell sandbox exec <sandboxName> sh -c <script> with the sandbox name as a positional argument. However, openshell sandbox exec requires the -n/--name flag. Without it, the sandbox name is interpreted as the command to execute, causing exit code 127. The catch block silently swallows this error, so the Telegram 409 conflict degraded warning is never shown in nemoclaw <sandbox> status output.

Related Issue

Fixes #2018

Changes

• src/nemoclaw.ts: changed ["sandbox", "exec", sandboxName, "sh", "-c", script] to ["sandbox", "exec", "-n", sandboxName, "--", "sh", "-c", script].

Type of Change

• Code change (feature, bug fix, or refactor)

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
• make 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)

Manually verified: nemoclaw <sandbox> status now surfaces the Telegram 409 degraded warning instead of silently swallowing exit 127.

AI Disclosure

• AI-assisted — tool: OpenClaw (Claude Opus 4.7 via GitHub Copilot)

Summary by CodeRabbit

• Chores
  • Updated internal command-line argument handling for improved consistency. No user-facing changes.

[coderabbitai]

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: 57a44447-72d5-4130-b3ea-87b84a4bd168

📥 Commits

Reviewing files that changed from the base of the PR and between f446836 and f774bb1.

📒 Files selected for processing (1)

• src/nemoclaw.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/nemoclaw.ts

---

📝 Walkthrough

Walkthrough

Replaced positional sandbox name in openshell sandbox exec calls with explicit -n <sandboxName> and added a -- separator before sh -c <script> in checkMessagingBridgeHealth() and readGatewayLog(); probe behavior and return shapes remain unchanged.

Changes

Cohort / File(s)	Summary
Openshell exec argument fix src/nemoclaw.ts	Updated spawnSync args to use ["sandbox", "exec", "-n", sandboxName, "--", "sh", "-c", script] instead of ["sandbox", "exec", sandboxName, "sh", "-c", script] for both checkMessagingBridgeHealth() and readGatewayLog().

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 The exec was called the wrong way,
Flags missing made the sandbox stray.
With -n and -- now set just so,
Logs reveal what we need to know.
Hopping back, the rabbit's glad to show. 🥕

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

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.
Out of Scope Changes check	❓ Inconclusive	The PR summary indicates updates to both checkMessagingBridgeHealth() and readGatewayLog(), but the PR title and issue #2018 focus only on checkMessagingBridgeHealth(), making the readGatewayLog() changes potentially out of scope.	Clarify whether readGatewayLog() changes are necessary for issue #2018 or represent a separate fix; if separate, verify they're intentional or document the rationale.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately captures the primary change: using the -n flag for sandbox exec in checkMessagingBridgeHealth function.
Linked Issues check	✅ Passed	The PR successfully addresses all coding requirements from issue #2018: correcting the openshell sandbox exec argument format from positional sandboxName to -n flag with -- separator.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

⚔️ Resolve merge conflicts

• Resolve merge conflict in branch fix/messaging-bridge-health-exec-flag

---

_{Review rate limit: 9/10 reviews remaining, refill in 6 minutes.}

_{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`:
- Around line 1075-1078: The readGatewayLog call uses positional sandbox name
which is inconsistent with the working spawnSync invocation at Line 1077; update
the argument array in the readGatewayLog (where spawnSync is invoked) to use the
same `["sandbox", "exec", "-n", sandboxName, "--", "sh", "-c", script]` shape
(and keep the same encoding/timeout/stdio options) so `sandbox exec` always
receives the `-n/--name` flag and avoids silent exit 127; locate the spawnSync
call inside readGatewayLog and replace the positional sandboxName entry with the
`-n` flag before the sandboxName.

🪄 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: 6d87bdb6-1b80-4d84-a573-c39b1ea0e8ce

📥 Commits

Reviewing files that changed from the base of the PR and between 4e6508d and f446836.

📒 Files selected for processing (1)

• src/nemoclaw.ts

[wscurran]

✨ Thanks for submitting this PR that proposes a fix for the openshell sandbox exec command to use the -n flag, which could help improve the reliability of the sandbox execution.

---

Possibly related open issues:

• #2018 [NemoClaw][Brev][Agent] checkMessagingBridgeHealth() passes sandbox name as positional arg to openshell exec, silently failing

[cv]

@Sanjays2402 can you add a DCO, please?

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[Sanjays2402]

Addressed both review items:

1. CodeRabbit (readGatewayLog): applied the same -n flag + -- separator fix to the readGatewayLog() invocation that had the identical broken shape (silent exit 127 hiding gateway log context).
2. @cv (DCO): rebased with --signoff so both commits now carry Signed-off-by.

Force-pushed to fix/messaging-bridge-health-exec-flag.

[ericksoa]

Thanks @Sanjays2402 for digging into this and for the follow-up fix to apply the same argv shape to readGatewayLog.

I resolved the remaining conflict locally against current main and found that the intended changes from this PR are already present there. Both checkMessagingBridgeHealth() and readGatewayLog() now use the correct openshell sandbox exec shape: -n -- sh -c ....

This was superseded by the later log/audit work in #2590, merged as e225dfb, so once rebased this PR has no remaining diff to merge. Closing this as superseded to keep the queue clean.

The linked issue #2018 is already closed; I added a note there asking folks to reopen it if the behavior regresses.