fix(onboard): output dashboard URL with token on completion (Fixes #53)

[deepujain]

Summary

Fixes #53. After onboarding completes, the installer showed the dashboard URL without the gateway token, so users could not log in. This change retrieves the token from the sandbox via openclaw dashboard --no-open over SSH and appends it to the dashboard URL in the completion summary.

Changes

• bin/lib/onboard.js:
  • Added parseDashboardUrlFromOutput(output): pure helper that parses the dashboard URL with #token= from openclaw dashboard output; normalizes 127.0.0.1 to localhost. Exported for unit tests.
  • Added getDashboardUrlWithToken(sandboxName): gets SSH config via openshell sandbox ssh-config, runs openclaw dashboard --no-open inside the sandbox via SSH, uses parseDashboardUrlFromOutput on the output, and returns the URL or null. Uses a temp config file and cleans up; returns null on any failure.
  • Updated printDashboard() to call getDashboardUrlWithToken(sandboxName) and display the URL with token when available; otherwise falls back to http://localhost:18789/.
• test/onboard.test.js (new): unit tests for parseDashboardUrlFromOutput — returns URL with token when output contains it, normalizes 127.0.0.1 to localhost, returns null when no token URL or empty/non-string input.

The model identifier and the rest of the summary output are unchanged.

Testing

• npm test passes (44 tests, including 5 new for parseDashboardUrlFromOutput).
• Manual: run ./install.sh through onboarding; the final Dashboard line should show a URL with #token=... when the sandbox is reachable via SSH.

Fixes #53

Summary by CodeRabbit

• New Features

  • Show an authenticated local dashboard URL for sandboxes (includes token when available); otherwise fall back to the local dashboard URL.

• Tests

  • Added tests covering extraction of dashboard tokens from command output, handling of whitespace, multiple matches, and missing or empty output.

[wscurran]

Thanks for enhancing the onboarding process and making it more user-friendly, this will definitely improve the user experience for new users.

[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

Adds token retrieval to onboarding: extracts TOKEN:<value> from command output; writes a temporary sandbox-side script, runs openshell sandbox connect <sandboxName> feeding that script, parses the token, and prints a dashboard URL with #token=<token> when available (falls back to generic URL).

Changes

Cohort / File(s)	Summary
Onboard logic bin/lib/onboard.js	Added parseTokenFromOutput(output) and exported it; added getDashboardToken(sandboxName) which writes a temp script, runs openshell sandbox connect <sandboxName> < <script> with timeout and ignoreError: true, parses token, ensures temp-file cleanup; updated printDashboard(...) to include #token=<token> when present; replaced inline fs/os requires with module-scoped imports.
Tests test/onboard.test.js	Imported parseTokenFromOutput and added unit tests for token extraction, multiple matches, whitespace trimming, absent token, and null/undefined/empty inputs.

Sequence Diagram

sequenceDiagram
    participant Installer as Installer
    participant Runner as openshell CLI
    participant Sandbox as Sandbox

    Installer->>Installer: write temp script that prints TOKEN:<token> from ~/.openclaw/openclaw.json
    Installer->>Runner: run "openshell sandbox connect <sandboxName>" with script fed to stdin (timeout, ignoreError)
    Runner->>Sandbox: execute stdin script inside sandbox
    Sandbox-->>Runner: stdout (may contain "TOKEN:<value>")
    Runner-->>Installer: captured stdout/stderr
    Installer->>Installer: parseTokenFromOutput(output)
    alt token found
        Installer->>Installer: format dashboard URL with token and print
    else no token
        Installer->>Installer: print fallback dashboard URL (http://localhost:18789/)
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐇 I hid a tiny script beneath the sand,

it squeaked out TOKEN: bright and small.
I read the echo with a careful hand,
stitched it to the URL so you can call.
Hop in — the dashboard opens the hall.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly reflects the main change: retrieving and displaying the dashboard token in the completion summary to fix issue #53.
Linked Issues check	✅ Passed	The implementation successfully addresses all core objectives from issue #53: retrieving the dashboard token from the sandbox and including it in the onboarding summary URL displayed to users.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to issue #53: token retrieval infrastructure, dashboard URL generation with tokens, and corresponding unit tests for the new functionality.

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

[deepujain]

Rebased onto latest main to resolve conflicts in bin/lib/onboard.js and test/onboard.test.js. Upstream commented out the Dashboard line and added buildSandboxConfigSyncScript exports/tests; merged both with the dashboard token URL and parseDashboardUrlFromOutput helper. All onboard tests pass.

[coderabbitai]

🧹 Nitpick comments (1)

bin/lib/onboard.js (1)

9-9: Remove redundant inline os require at line 448.

The os module is now imported at the module level (line 9), but there's still an inline const os = require("os"); at line 448 inside createSandbox. This creates unnecessary shadowing. Consider removing the inline require.

Suggested fix at line 448

   // Stage build context
   const { mkdtempSync } = require("fs");
-  const os = require("os");
   const buildCtx = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-build-"));

🤖 Prompt for AI Agents

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

In `@bin/lib/onboard.js` at line 9, There is a redundant inline require of the
"os" module inside createSandbox which shadows the module-level const os
declared at the top; remove the inline `const os = require("os");` from the
createSandbox function so the function uses the already-imported module-level
`os` variable (search for createSandbox to locate the inline require), ensuring
no other references rely on a locally scoped os.

🤖 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`:
- Line 9: There is a redundant inline require of the "os" module inside
createSandbox which shadows the module-level const os declared at the top;
remove the inline `const os = require("os");` from the createSandbox function so
the function uses the already-imported module-level `os` variable (search for
createSandbox to locate the inline require), ensuring no other references rely
on a locally scoped os.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 65853bbe-82b6-4f97-8e17-9ba3e6078771

📥 Commits

Reviewing files that changed from the base of the PR and between dbfd78c and 1634d79.

📒 Files selected for processing (2)

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

[deepujain]

Addressed CodeRabbit review comment

[kjw3]

Thanks for pushing on this. The UX problem is still real and worth fixing, but this PR is now stale against current main, so I don’t think we should merge it as written.

The main gaps are:

• the patch is based on an older onboard/test structure and needs to be updated to fit the current code layout
• the current approach relies on a fairly brittle direct ssh / temp-config flow with StrictHostKeyChecking=no
• I’d like to land this in a way that matches current helper patterns and keeps the dashboard-token retrieval path as tight and safe as possible

If you’re still up for it, please rebase/update this PR against current main and adapt it to the current onboard/test code. If that gets refreshed, I’d be happy to take another look, because the underlying issue is still valid and I’d like to get it merged in a secure way.

Please tag @kjw3 when it’s ready for review again.

[deepujain]

Rebased onto latest main and rewrote the dashboard-token retrieval to use openshell sandbox connect < script (same pattern as setupOpenclaw) instead of the SSH/temp-config flow. The token is now read directly from ~/.openclaw/openclaw.json inside the sandbox, matching nemoclaw-start.sh. No more StrictHostKeyChecking=no. All onboard tests pass.

@kjw3 ready for another look.

[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 `@test/onboard.test.js`:
- Around line 77-79: The test uses a realistic-looking API token string in the
variable out and assertion (see parseTokenFromOutput usage), which triggers
secret scanners; replace the long hex token with a non-secret fixture such as
"TOKEN:TEST_TOKEN" (or a short dummy string) in the out value and update the
expect(parseTokenFromOutput(out)).toBe(...) to match that dummy value so the
test still verifies parsing without exposing secret-like data.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f24bb95b-d1bd-4652-a707-c4adf0e68959

📥 Commits

Reviewing files that changed from the base of the PR and between 41be8a8 and 9a395f1.

📒 Files selected for processing (2)

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

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

• bin/lib/onboard.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 1087-1090: The call to runCapture that runs `openshell sandbox
connect ${shellQuote(sandboxName)} < ${shellQuote(scriptFile)}` can hang
indefinitely; update the runCapture invocation (the one using sandboxName and
scriptFile) to include a timeout option (for example add timeout: 30000)
alongside the existing ignoreError: true so the call becomes runCapture(..., {
ignoreError: true, timeout: 30000 }) to ensure the onboarding flow falls back
promptly if the sandbox connection blocks.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: cbe0c723-afdf-4f94-ba11-3749d06ec523

📥 Commits

Reviewing files that changed from the base of the PR and between 9a395f1 and 91de335.

📒 Files selected for processing (2)

• bin/lib/onboard.js
• 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 1059-1062: The token returned from the trimmed line is not
normalized/URL-encoded, so any fragment-reserved chars or stray whitespace can
break the dashboard URL; update the logic that currently returns
trimmed.slice(6) to first normalize and encode the token (e.g., trim the
extracted value and pass it through a URI-encoding function) and use that
encoded token when composing dashboardUrl (wherever dashboardUrl is
interpolated) so the fragment is safe; apply the same normalization/encoding to
the other occurrence that mirrors the trimmed.startsWith("TOKEN:") handling.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7d38f12d-c2f8-49b7-b841-151886ee0473

📥 Commits

Reviewing files that changed from the base of the PR and between 91de335 and f8d26ba.

📒 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

[deepujain]

Rebased on latest main. Addressed the CodeRabbit review: token is now trimmed and URL-encoded via encodeURIComponent before being interpolated into the dashboard URL fragment. All onboard tests pass. @kjw3 ready for another look!

[roselijack]

Nice approach reusing the writeSandboxConfigSyncFile + stdin pipe pattern — keeps it consistent with setupOpenclaw.

One thing worth a look: the runCapture call in getDashboardToken has a 30-second timeout. Since this runs at the very end of onboarding (summary step), a failure or slow response would silently block for 30 s before falling back to the bare URL. Given that token retrieval is best-effort, a tighter timeout — say 5–10 s — would give a better experience without changing the happy path.

[deepujain]

Good call @roselijack, reduced the timeout from 30s to 10s. Token retrieval is best-effort so no reason to block the summary step that long. All onboard tests pass. @kjw3 ready for review!

[deepujain]

Rebased on latest main . Merged upstream's nimContainer param into printDashboard and picked up the new setupInference credential tests. All 360 tests pass. @kjw3 ready for review!

[brandonpelfrey]

@deepujain just one issue flagged (log failure to delete the script please), and rebase latest. Looks good otherwise.

[deepujain]

Rebased on latest main. Added error logging when temp script cleanup fails (per @brandonpelfrey's feedback). All tests pass. Ready for review!

[deepujain]

Rebased on latest main , 2/3 times in last few minutes. Commits are landing left and right on main :)

[deepujain]

rebased again to solve one more conflict.

[brandonpelfrey]

I've seen #906 from other reviewer has resolved this @deepujain . I'm closing this PR, please reopen if you have a comment. Thank you for your time. We're trying to make sure we avoid this duplicate effort going forward.