Skip to content

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

Merged
ericksoa merged 2 commits into
mainfrom
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 into
mainfrom
fix/onboard-test-shellquote-matchers

Conversation

@ericksoa

@ericksoa ericksoa commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

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

coderabbitai Bot commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

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>
@wscurran wscurran added the bug-fix PR fixes a bug or regression label Jun 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants