Repository navigation
fix: gate sandbox registration on creation readiness and guard WSL name truncation #229
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
kjw3
merged 11 commits into
NVIDIA:main
from
brianwtaylor:fix/sandbox-creation-readiness-gate
Mar 19, 2026
Merged
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
8c49271
fix: gate sandbox registration on readiness and surface creation fail…
brianwtaylor 4fd2c6e
fix: use word-boundary match for Ready status and fix timeout
brianwtaylor 68b8e5e
fix: exact-match sandbox name in readiness check
brianwtaylor 7a01e35
test: add readiness gate parsing tests for sandbox creation
brianwtaylor 9f08ced
fix: guard against truncated sandbox names on WSL (fixes #21)
brianwtaylor 7357372
fix: clean up orphaned sandbox on readiness timeout
brianwtaylor 8d4a23b
chore: remove issue references from code comments
brianwtaylor 6d617be
fix: enforce RFC 1123 63-char limit in sandbox name validation
brianwtaylor 5d3b6d0
fix: extract isSandboxReady as shared function and branch cleanup mes…
brianwtaylor 6a1da48
fix: clean up stale gateway and port forward before preflight check
brianwtaylor c43c66a
test: add double-onboard e2e test for stale state recovery
brianwtaylor File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,136 @@ | ||
| // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| const { describe, it } = require("node:test"); | ||
| const assert = require("node:assert/strict"); | ||
|
|
||
| const { buildPolicySetCommand, buildPolicyGetCommand } = require("../bin/lib/policies"); | ||
|
|
||
| // Test the readiness parsing logic extracted for testability | ||
| // These validate the exact matching algorithm from onboard.js createSandbox() | ||
|
|
||
| function isSandboxReady(output, sandboxName) { | ||
| const clean = output.replace(/\x1b\[[0-9;]*m/g, ""); | ||
| return clean.split("\n").some((l) => { | ||
| const cols = l.trim().split(/\s+/); | ||
| return cols[0] === sandboxName && cols.includes("Ready") && !cols.includes("NotReady"); | ||
| }); | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
Resolved
|
||
|
|
||
| describe("sandbox readiness parsing", () => { | ||
| it("detects Ready sandbox", () => { | ||
| assert.ok(isSandboxReady("my-assistant Ready 2m ago", "my-assistant")); | ||
| }); | ||
|
|
||
| it("rejects NotReady sandbox", () => { | ||
| assert.ok(!isSandboxReady("my-assistant NotReady init failed", "my-assistant")); | ||
| }); | ||
|
|
||
| it("rejects empty output", () => { | ||
| assert.ok(!isSandboxReady("No sandboxes found.", "my-assistant")); | ||
| assert.ok(!isSandboxReady("", "my-assistant")); | ||
| }); | ||
|
|
||
| it("strips ANSI escape codes before matching", () => { | ||
| assert.ok(isSandboxReady( | ||
| "\x1b[1mmy-assistant\x1b[0m \x1b[32mReady\x1b[0m 2m ago", | ||
| "my-assistant" | ||
| )); | ||
| }); | ||
|
|
||
| it("rejects ANSI-wrapped NotReady", () => { | ||
| assert.ok(!isSandboxReady( | ||
| "\x1b[1mmy-assistant\x1b[0m \x1b[31mNotReady\x1b[0m crash", | ||
| "my-assistant" | ||
| )); | ||
| }); | ||
|
|
||
| it("exact-matches sandbox name in first column", () => { | ||
| // "my" should NOT match "my-assistant" | ||
| assert.ok(!isSandboxReady("my-assistant Ready 2m ago", "my")); | ||
| }); | ||
|
|
||
| it("does not match sandbox name in non-first column", () => { | ||
| assert.ok(!isSandboxReady("other-box Ready owned-by-my-assistant", "my-assistant")); | ||
| }); | ||
|
|
||
| it("handles multiple sandboxes in output", () => { | ||
| const output = [ | ||
| "NAME STATUS AGE", | ||
| "dev-box NotReady 5m ago", | ||
| "my-assistant Ready 2m ago", | ||
| "staging Ready 10m ago", | ||
| ].join("\n"); | ||
| assert.ok(isSandboxReady(output, "my-assistant")); | ||
| assert.ok(!isSandboxReady(output, "dev-box")); // NotReady | ||
| assert.ok(isSandboxReady(output, "staging")); | ||
| assert.ok(!isSandboxReady(output, "prod")); // not present | ||
| }); | ||
|
|
||
| it("handles Ready sandbox with extra status columns", () => { | ||
| assert.ok(isSandboxReady("my-assistant Ready Running 2m ago 1/1", "my-assistant")); | ||
| }); | ||
|
|
||
| it("rejects when output only contains name in a URL or path", () => { | ||
| assert.ok(!isSandboxReady("Connecting to my-assistant.openshell.internal Ready", "my-assistant")); | ||
| // "my-assistant.openshell.internal" is cols[0], not "my-assistant" | ||
| }); | ||
|
|
||
| it("handles tab-separated output", () => { | ||
| assert.ok(isSandboxReady("my-assistant\tReady\t2m ago", "my-assistant")); | ||
| }); | ||
| }); | ||
|
|
||
| // Regression tests: WSL truncates hyphenated sandbox names during shell | ||
| // argument parsing (e.g. "my-assistant" → "m"). | ||
| describe("WSL sandbox name handling", () => { | ||
| it("buildPolicySetCommand preserves hyphenated sandbox name", () => { | ||
| const cmd = buildPolicySetCommand("/tmp/policy.yaml", "my-assistant"); | ||
| assert.ok(cmd.includes('"my-assistant"'), `Expected quoted name in: ${cmd}`); | ||
| assert.ok(!cmd.includes(' my-assistant '), "Name must be quoted, not bare"); | ||
| }); | ||
|
|
||
| it("buildPolicyGetCommand preserves hyphenated sandbox name", () => { | ||
| const cmd = buildPolicyGetCommand("my-assistant"); | ||
| assert.ok(cmd.includes('"my-assistant"'), `Expected quoted name in: ${cmd}`); | ||
| }); | ||
|
|
||
| it("buildPolicySetCommand preserves multi-hyphen names", () => { | ||
| const cmd = buildPolicySetCommand("/tmp/p.yaml", "my-dev-assistant-v2"); | ||
| assert.ok(cmd.includes('"my-dev-assistant-v2"')); | ||
| }); | ||
|
|
||
| it("buildPolicySetCommand preserves single-char name", () => { | ||
| // If WSL truncates "my-assistant" to "m", the single-char name should | ||
| // still be quoted and passed through unchanged | ||
| const cmd = buildPolicySetCommand("/tmp/p.yaml", "m"); | ||
| assert.ok(cmd.includes('"m"')); | ||
| }); | ||
|
|
||
| it("applyPreset rejects truncated/invalid sandbox name", () => { | ||
| const policies = require("../bin/lib/policies"); | ||
| // Empty name | ||
| assert.throws( | ||
| () => policies.applyPreset("", "npm"), | ||
| /Invalid or truncated sandbox name/ | ||
| ); | ||
| // Name with uppercase (not valid per RFC 1123) | ||
| assert.throws( | ||
| () => policies.applyPreset("My-Assistant", "npm"), | ||
| /Invalid or truncated sandbox name/ | ||
| ); | ||
| // Name starting with hyphen | ||
| assert.throws( | ||
| () => policies.applyPreset("-broken", "npm"), | ||
| /Invalid or truncated sandbox name/ | ||
| ); | ||
| }); | ||
|
|
||
| it("readiness check uses exact match preventing truncated name false-positive", () => { | ||
| // If "my-assistant" was truncated to "m", the readiness check should | ||
| // NOT match a sandbox named "my-assistant" when searching for "m" | ||
| assert.ok(!isSandboxReady("my-assistant Ready 2m ago", "m")); | ||
| assert.ok(!isSandboxReady("my-assistant Ready 2m ago", "my")); | ||
| assert.ok(!isSandboxReady("my-assistant Ready 2m ago", "my-")); | ||
| }); | ||
| }); | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.