Skip to content

fix(test): update onboard test mocks to match shell-quoted sandboxName#1566

Merged
ericksoa merged 2 commits intomainfrom
fix/onboard-test-shellquote-matchers
Apr 7, 2026
Merged

fix(test): update onboard test mocks to match shell-quoted sandboxName#1566
ericksoa merged 2 commits intomainfrom
fix/onboard-test-shellquote-matchers

Conversation

@ericksoa
Copy link
Copy Markdown
Contributor

@ericksoa ericksoa commented Apr 7, 2026

Summary

  • Update 6 runCapture mock matchers in test/onboard.test.js to match the shell-quoted sandbox name format introduced by 8f8b4e7
  • The security fix wrapped sandboxName with shellQuote() in exec and DNS proxy commands, but the test mocks still matched the old unquoted format (sandbox exec my-assistant curl vs sandbox exec 'my-assistant' curl)
  • This caused 4 CI failures: 3 timeouts (dashboard readiness loop never matched) and 1 assertion error (null !== 0)

Test plan

  • npx vitest run — all 1126 tests pass (67 files, 0 failures)
  • All pre-commit and pre-push hooks pass

Fixes the CI break introduced by #1392.

Signed-off-by: Aaron Erickson aerickson@nvidia.com

Summary by CodeRabbit

  • Tests
    • Updated test matchers to correctly detect sandbox invocations that use a quoted sandbox name in command strings.
    • Simplified test logic for scanning run/runCapture templates into an equivalent single-line expression, preserving existing violation detection and assertions.

The shell-quoting security fix (8f8b4e7) wrapped sandboxName with
shellQuote() in exec and DNS proxy commands, but the test mocks still
matched the unquoted format causing 4 CI failures (3 timeouts, 1
assertion error).

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 7, 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: 7dacf070-bf3a-44c3-8d23-c871763ad532

📥 Commits

Reviewing files that changed from the base of the PR and between ab32795 and 0f765d1.

📒 Files selected for processing (1)
  • test/shellquote-sandbox.test.js
✅ Files skipped from review due to trivial changes (1)
  • test/shellquote-sandbox.test.js

📝 Walkthrough

Walkthrough

Updated test logic: onboarding test matchers now detect sandbox exec invocations with quoted sandbox names ('my-assistant'), and a shellquote-related test expression was compacted from multi-line to single-line without behavioral change.

Changes

Cohort / File(s) Summary
Test Command Matchers
test/onboard.test.js
Adjusted runner.runCapture matcher conditions to recognize sandbox exec 'my-assistant' (quoted sandbox name) in curl probe command strings instead of the unquoted form.
Shellquote Test Expression
test/shellquote-sandbox.test.js
Refactored a multi-line if into a single-line boolean expression checking template.includes("${sandboxName}") and !template.includes("shellQuote(sandboxName)"); behavior unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A tiny tweak, a quoted name so bright,
Tests now spot the curl in proper light,
Lines folded neat, logic kept the same,
A hop, a glance — the sandbox earned its frame! 🥕

🚥 Pre-merge checks | ✅ 3
✅ 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 clearly and directly summarizes the main change: updating test mocks to handle shell-quoted sandbox names, which aligns perfectly with the changeset's primary purpose.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ 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/onboard-test-shellquote-matchers

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

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@ericksoa ericksoa enabled auto-merge (squash) April 7, 2026 07:41
@ericksoa ericksoa merged commit de36e2a into main Apr 7, 2026
10 checks passed
gemini2026 pushed a commit to gemini2026/NemoClaw that referenced this pull request Apr 14, 2026
NVIDIA#1566)

## Summary

- Update 6 `runCapture` mock matchers in `test/onboard.test.js` to match
the shell-quoted sandbox name format introduced by 8f8b4e7
- The security fix wrapped `sandboxName` with `shellQuote()` in exec and
DNS proxy commands, but the test mocks still matched the old unquoted
format (`sandbox exec my-assistant curl` vs `sandbox exec 'my-assistant'
curl`)
- This caused 4 CI failures: 3 timeouts (dashboard readiness loop never
matched) and 1 assertion error (`null !== 0`)

## Test plan

- [x] `npx vitest run` — all 1126 tests pass (67 files, 0 failures)
- [x] All pre-commit and pre-push hooks pass

Fixes the CI break introduced by NVIDIA#1392.

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>

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

* **Tests**
* Updated test matchers to correctly detect sandbox invocations that use
a quoted sandbox name in command strings.
* Simplified test logic for scanning run/runCapture templates into an
equivalent single-line expression, preserving existing violation
detection and assertions.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-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.

1 participant