fix(onboard): reject sandbox names starting with a digit and allow retry

[latenighthackathon]

Summary

Names starting with a digit (e.g., 7racii) pass the current validation regex but fail downstream in Kubernetes. The user is then stuck in a loop because the invalid name is persisted in the onboard session and reused on retry — the only recovery is deleting ~/.nemoclaw.

Related Issue

Closes #1120

Changes

• Tightened sandbox name regex from ^[a-z0-9] to ^[a-z] so digit-prefixed names are caught at prompt time
• Replaced process.exit(1) with a retry loop (up to 3 attempts) so users can correct the name interactively
• Added a specific error message when the name starts with a digit ("Names must start with a letter, not a digit")
• Non-interactive mode still exits immediately on invalid input (no retry loop)
• Added regression test verifying digit-prefixed names are rejected

Testing

• Unit tests added
• npm test passes (no local environment)

Executed:

• npx vitest run test/onboard.test.js -t "rejects sandbox names" passes in Docker (Linux)
• Full test suite: 37 passed, 2 failed (pre-existing: sandbox create timeout + openshell CLI dep)
• No new test failures introduced

Checklist

• Follows Conventional Commits
• Commits are signed

Summary by CodeRabbit

• Bug Fixes

  • Tightened sandbox name validation: must start with a lowercase letter and may include lowercase letters, digits, and hyphens; clearer error messages for digit-start vs other format violations. Interactive prompts now limit retries to three, showing "Please try again" only when attempts remain and reporting when attempts are exhausted; non-interactive invalid input exits immediately.

• Tests

  • Added tests verifying accepted and rejected sandbox name patterns.

Signed-off-by: latenighthackathon latenighthackathon@users.noreply.github.com

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Sandbox name validation now requires names to start with a lowercase ASCII letter and match /^[a-z]([a-z0-9-]*[a-z0-9])?$/. Interactive retries are capped at 3 with distinct error messages; non‑interactive invalid input exits immediately.

Changes

Cohort / File(s)	Summary
Sandbox Name Validation & Prompt Flow bin/lib/onboard.js	Tightened sandbox-name regex to require leading lowercase letter and disallow trailing hyphen; replaced infinite while retry with bounded MAX_ATTEMPTS = 3; added distinct messaging for digit-start vs other format violations; non-interactive invalid input immediately exits.
Tests test/onboard.test.js	Added Vitest assertions validating the new name regex (accepts letter-start names, rejects digit-/hyphen-prefixed, trailing-hyphen, and empty); updated source-inspection assertion to expect a bounded retry loop (for/attempts) rather than while (true).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• fix: re-prompt on invalid sandbox name instead of exiting #1173 — Modifies promptValidatedSandboxName retry behavior and validation; closely related changes to the same function and prompt loop.

Poem

🐰 I hopped through prompts, corrected my stride,
Letters lead now — no digits to hide.
Three little nudges, then off we roam,
No endless loops — a tidy path home. 🥕

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main changes: rejecting sandbox names starting with digits and enabling retry functionality.
Linked Issues check	✅ Passed	The pull request fully addresses issue #1120 by validating sandbox names, preventing digit-prefixed names, and enabling interactive retry without requiring manual deletion of session state.
Out of Scope Changes check	✅ Passed	All changes in onboard.js and onboard.test.js are directly related to sandbox name validation and retry logic as specified in the linked issue.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

🧹 Nitpick comments (1)

test/onboard.test.js (1)

443-459: Avoid duplicating the production regex literal in the test.

This test can drift from runtime validation because it redefines the pattern locally. Consider exporting a shared sandbox-name validator (or regex constant) from bin/lib/onboard.js and asserting against that single source of truth.

🤖 Prompt for AI Agents

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

In `@test/onboard.test.js` around lines 443 - 459, The test currently redefines
SANDBOX_NAME_REGEX locally which can drift from production; export the shared
validator (e.g., a named export SANDBOX_NAME_REGEX or a function
validateSandboxName) from bin/lib/onboard.js, update the test to import that
exported symbol instead of defining its own regex, and assert against the
imported constant/function (replace local SANDBOX_NAME_REGEX usage with the
imported symbol or call validateSandboxName) so the test uses the single source
of truth.

🤖 Prompt for all review comments with AI agents

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

Nitpick comments:
In `@test/onboard.test.js`:
- Around line 443-459: The test currently redefines SANDBOX_NAME_REGEX locally
which can drift from production; export the shared validator (e.g., a named
export SANDBOX_NAME_REGEX or a function validateSandboxName) from
bin/lib/onboard.js, update the test to import that exported symbol instead of
defining its own regex, and assert against the imported constant/function
(replace local SANDBOX_NAME_REGEX usage with the imported symbol or call
validateSandboxName) so the test uses the single source of truth.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6cec9920-7f3f-4eef-9d99-575ac761567c

📥 Commits

Reviewing files that changed from the base of the PR and between 0a97e89 and a351552.

📒 Files selected for processing (2)

• bin/lib/onboard.js
• test/onboard.test.js

[wscurran]

✨ Thanks for submitting this PR with a detailed summary, it proposes to address an onboarding issue by improving sandbox name validation..

[coderabbitai]

🧹 Nitpick comments (1)

bin/lib/onboard.js (1)

2280-2281: Minor inconsistency: error message says "start with a letter or number" but rule is letter-only.

The generic error message claims names can start with "a letter or number," but the regex ^[a-z] requires a letter. While the digit-prefixed case has its own specific message, other invalid cases (e.g., -abc, ABC) would show this slightly misleading text.

💡 Suggested fix for error message accuracy

     } else {
-      console.error("  Names must be lowercase, contain only letters, numbers, and hyphens,");
-      console.error("  and must start and end with a letter or number.");
+      console.error("  Names must be lowercase, contain only letters, numbers, and hyphens,");
+      console.error("  must start with a letter, and end with a letter or number.");
     }

🤖 Prompt for AI Agents

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

In `@bin/lib/onboard.js` around lines 2280 - 2281, The error text printed in
onboard.js is inconsistent with the validation regex (which uses ^[a-z] to
require names to start with a lowercase letter); update the two console.error
lines that describe allowed names so they accurately state that names must start
with a lowercase letter (not "a letter or number") — reference the validation
using the ^[a-z] rule and adjust the message to say something like "Names must
be lowercase, contain only letters, numbers, and hyphens, and must start with a
lowercase letter and end with a letter or number." Ensure the updated text
replaces the existing console.error lines.

🤖 Prompt for all review comments with AI agents

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

Nitpick comments:
In `@bin/lib/onboard.js`:
- Around line 2280-2281: The error text printed in onboard.js is inconsistent
with the validation regex (which uses ^[a-z] to require names to start with a
lowercase letter); update the two console.error lines that describe allowed
names so they accurately state that names must start with a lowercase letter
(not "a letter or number") — reference the validation using the ^[a-z] rule and
adjust the message to say something like "Names must be lowercase, contain only
letters, numbers, and hyphens, and must start with a lowercase letter and end
with a letter or number." Ensure the updated text replaces the existing
console.error lines.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 3ed6025f-380b-4441-8fc7-75ac4eeadb70

📥 Commits

Reviewing files that changed from the base of the PR and between 64e333e and beee1a0.

📒 Files selected for processing (2)

• bin/lib/onboard.js
• test/onboard.test.js

✅ Files skipped from review due to trivial changes (1)

• test/onboard.test.js

[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 `@bin/lib/onboard.js`:
- Around line 2261-2296: Update the test that asserts promptValidatedSandboxName
uses an infinite loop: instead of looking for a while(true) pattern, change the
assertion to verify the new bounded-retry implementation by checking for the
MAX_ATTEMPTS constant or the for-loop pattern (e.g., assert the function body
contains "MAX_ATTEMPTS" or a for loop like "for (let attempt" / "for\\s*\\(let
attempt") and/or that it exits with process.exit(1) after too many attempts;
reference the promptValidatedSandboxName function and MAX_ATTEMPTS in your test
modification so it matches the new behavior.

🪄 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

Run ID: 44ef639f-d66d-47ab-a7db-9a2a8d47cef5

📥 Commits

Reviewing files that changed from the base of the PR and between beee1a0 and 76df03a.

📒 Files selected for processing (1)

• bin/lib/onboard.js