Skip to content

refactor(onboard): centralize reused dashboard metadata - #5130

Merged
cv merged 9 commits into
mainfrom
codex/onboard-reuse-dashboard-flow
Jun 11, 2026
Merged

refactor(onboard): centralize reused dashboard metadata#5130
cv merged 9 commits into
mainfrom
codex/onboard-reuse-dashboard-flow

Conversation

@cv

@cv cv commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

Centralizes the reused-sandbox dashboard update path so interactive and non-interactive sandbox reuse share the same forward, CHAT_UI_URL, Hermes dashboard, and registry metadata handling. This draft PR is stacked on #5128 and keeps behavior unchanged while removing duplicated createSandbox logic.

Related Issue

Refs #3802

Changes

  • Added applyReusedSandboxDashboardState to src/lib/onboard/sandbox-reuse.ts for reused dashboard forward and metadata updates.
  • Replaced the duplicated non-interactive and interactive reuse branches in createSandbox with the shared helper.
  • Added focused coverage for CHAT_UI_URL updates, Hermes dashboard forwarding, reused metadata updates, and gateway registry fields.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Verification

Targeted checks run:

  • npx @biomejs/biome lint src/lib/onboard.ts src/lib/onboard/sandbox-reuse.ts src/lib/onboard/sandbox-reuse.test.ts
  • npm run typecheck:cli
  • npm run build:cli
  • git diff --check
  • npx vitest run src/lib/onboard/sandbox-reuse.test.ts src/lib/onboard/sandbox-registry-metadata.test.ts test/onboard.test.ts test/shellquote-sandbox.test.ts

GitHub validation:


Signed-off-by: Carlos Villela cvillela@nvidia.com

Summary by CodeRabbit

  • Refactor
    • Improved sandbox creation dashboard port allocation logic for more reliable port resolution and URL handling.
    • Enhanced sandbox reuse flow to better manage dashboard state and port forwarding across reused sandbox instances.

@cv cv self-assigned this Jun 10, 2026
@copy-pr-bot

copy-pr-bot Bot commented Jun 10, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: f88c6990-3fa2-469e-88a4-8b5531945c62

📥 Commits

Reviewing files that changed from the base of the PR and between c27d8fa and c06aae6.

📒 Files selected for processing (5)
  • src/lib/onboard.ts
  • src/lib/onboard/dashboard-port.test.ts
  • src/lib/onboard/dashboard-port.ts
  • src/lib/onboard/sandbox-reuse.test.ts
  • src/lib/onboard/sandbox-reuse.ts

📝 Walkthrough

Walkthrough

This PR refactors dashboard port selection and sandbox reuse state management by extracting duplicated logic into dedicated, testable resolver functions. The main onboarding flow is simplified by adopting these helpers in sandbox creation and sandbox reuse paths.

Changes

Dashboard and Sandbox Reuse State Refactoring

Layer / File(s) Summary
Dashboard port resolution contract and implementation
src/lib/onboard/dashboard-port.ts
Defines CreateSandboxDashboardPortInput and CreateSandboxDashboardPortResult types, and implements resolveCreateSandboxDashboardPort to centralize port selection from multiple candidate sources (control UI port, CHAT_UI_URL, persisted/agent ports, defaults), compute availability via findAvailableDashboardPort, emit warnings on mismatch, and construct the resulting chatUiUrl.
Dashboard port resolver test coverage
src/lib/onboard/dashboard-port.test.ts
Comprehensive test suite validating port precedence, URL rewriting with warnings, fallback ordering, URL scheme normalization, and malformed URL error/ignore behavior.
Sandbox reuse dashboard state contract and implementation
src/lib/onboard/sandbox-reuse.ts
Defines ReusedSandboxDashboardForwarding, ReusedSandboxDashboardStateInput, and ReusedSandboxDashboardStateResult interfaces, and implements applyReusedSandboxDashboardState to forward dashboard ports, resolve Hermes dashboard onboarding state, update reused sandbox metadata, and persist registry fields (Hermes settings plus gateway identity).
Sandbox reuse dashboard state test coverage
src/lib/onboard/sandbox-reuse.test.ts
Test suite covering both Hermes-enabled and Hermes-disabled scenarios, validating port forwarding, env mutation, state resolution, metadata updates, and registry field persistence.
Onboard.ts integration of dashboard helpers
src/lib/onboard.ts
Replaces inline dashboard port selection and manual Hermes state management in sandbox creation and reuse fast-paths with calls to resolveCreateSandboxDashboardPort and applyReusedSandboxDashboardState, simplifying both code paths.

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#5128: Introduces the extracted dashboard port resolver function and its initial integration into the onboarding flow.

🐰 Dashboard logic hops forth, now neatly tucked,
Reuse state helpers bundled and tested with luck,
The onboard.ts path grows cleaner and bright,
Where extraction meets composition—a refactor done right!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.11% 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 'refactor(onboard): centralize reused dashboard metadata' accurately describes the main change: centralizing dashboard update handling for reused sandboxes through a shared helper function.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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 codex/onboard-reuse-dashboard-flow

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

@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor

Findings: 0 needs attention, 0 worth checking, 0 nice ideas
Since last review: 0 prior items resolved, 4 still apply, 0 new items found

Consider writing more tests for
  • **Runtime validation** — Reusing a ready sandbox in non-interactive createSandbox updates process.env.CHAT_UI_URL to the reallocated loopback dashboard port and persists Hermes dashboard plus gateway registry fields.. The extracted helpers have focused unit coverage, but the changed surface is onboarding host/sandbox forwarding orchestration. Caller-level or runtime validation would provide stronger confidence that the interactive and non-interactive createSandbox reuse branches wire the shared helper correctly.
  • **Runtime validation** — Accepting reuse in interactive createSandbox routes through the shared reuse helper and preserves dashboard forwarding, Hermes forwarding, reused metadata, and registry side effects.. The extracted helpers have focused unit coverage, but the changed surface is onboarding host/sandbox forwarding orchestration. Caller-level or runtime validation would provide stronger confidence that the interactive and non-interactive createSandbox reuse branches wire the shared helper correctly.
  • **Runtime validation** — applyReusedSandboxDashboardState without an updateSandbox seam writes Hermes and gateway fields through the default registry.updateSandbox path against a temp registry entry.. The extracted helpers have focused unit coverage, but the changed surface is onboarding host/sandbox forwarding orchestration. Caller-level or runtime validation would provide stronger confidence that the interactive and non-interactive createSandbox reuse branches wire the shared helper correctly.
  • **Runtime validation** — Issue Umbrella: refactor onboarding into a serializable FSM #3802 reused-dashboard acceptance is covered once deterministic issue body/comment clauses are available.. The extracted helpers have focused unit coverage, but the changed surface is onboarding host/sandbox forwarding orchestration. Caller-level or runtime validation would provide stronger confidence that the interactive and non-interactive createSandbox reuse branches wire the shared helper correctly.
  • **Acceptance clause:** Refs Umbrella: refactor onboarding into a serializable FSM #3802 — add test evidence or identify existing coverage. The deterministic context reported linkedIssues: [] and did not include issue Umbrella: refactor onboarding into a serializable FSM #3802 body or comments. Repository grep only found existing onboard machine vocabulary tests referencing Umbrella: refactor onboarding into a serializable FSM #3802, not acceptance text for this reused-dashboard refactor.

Workflow run details

This is an automated advisory review. A human maintainer must make the final merge decision.

@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: cloud-onboard-e2e, double-onboard-e2e, concurrent-gateway-ports-e2e
Optional E2E: hermes-dashboard-e2e, dashboard-remote-bind-e2e

Dispatch hint: cloud-onboard-e2e,double-onboard-e2e,concurrent-gateway-ports-e2e

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • cloud-onboard-e2e (medium): Required because core onboarding createSandbox behavior changed. This validates a real source install plus non-interactive sandbox creation with dashboard URL/port values baked into the live sandbox path.
  • double-onboard-e2e (high): Required because the PR changes reused sandbox dashboard state. This job explicitly exercises repeat onboarding, healthy sandbox reuse, gateway reuse, stale registry reconciliation, and forward-list diagnostics around reused sandboxes.
  • concurrent-gateway-ports-e2e (high): Required because dashboard port allocation and collision fallback changed. This job creates multiple sandboxes on one host and verifies distinct dashboard ports/URLs and gateway/dashboard segregation.

Optional E2E

  • hermes-dashboard-e2e (medium): Useful adjacent confidence for Hermes dashboard forwarding and registry metadata. The PR changes reused-sandbox Hermes dashboard field persistence, while this existing job validates Hermes dashboard create/forward behavior but does not fully cover reuse.
  • dashboard-remote-bind-e2e (medium): Optional adjacent dashboard-forward coverage. This guards remote dashboard bind behavior during connect; the PR is mainly create/reuse port resolution, so this is not merge-blocking unless reviewers are concerned about dashboard forwarding regressions.

New E2E recommendations

  • hermes-dashboard-reuse (high): Existing hermes-dashboard-e2e validates initial Hermes dashboard creation, but this PR specifically changes reused-sandbox Hermes dashboard forwarding/registry updates. Add a live E2E that onboards Hermes with dashboard enabled, re-runs onboard without recreate, accepts reuse, and asserts dashboard/API forwards plus registry fields remain correct.
    • Suggested test: Hermes dashboard reuse E2E covering applyReusedSandboxDashboardState on a ready reused sandbox
  • dashboard-port-precedence (medium): Unit tests cover --control-ui-port, CHAT_UI_URL, persisted port, agent forward port, and default precedence, but no existing live E2E appears to verify CHAT_UI_URL/control-ui-port values are propagated consistently into Dockerfile args, sandbox env, readiness probes, registry, and nemoclaw list output.
    • Suggested test: Live onboard dashboard port precedence E2E for CHAT_UI_URL and --control-ui-port propagation
  • host-bound-dashboard-port-conflict (medium): The allocator probes non-OpenShell host listeners and falls back across the dashboard port range. Existing concurrent sandbox coverage exercises OpenShell forward collisions, but a dedicated live guard for a non-OpenShell listener occupying the preferred port would catch regressions in the host-bound path.
    • Suggested test: Dashboard host-bound preferred-port fallback E2E with a local listener on 18789

Dispatch hint

  • Workflow: nightly-e2e.yaml
  • jobs input: cloud-onboard-e2e,double-onboard-e2e,concurrent-gateway-ports-e2e

@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: ubuntu-repo-cloud-openclaw
Optional Vitest E2E scenarios: ubuntu-repo-docker-post-reboot-recovery

Dispatch required Vitest E2E scenarios:

  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-openclaw

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required Vitest E2E scenarios

  • ubuntu-repo-cloud-openclaw: Core onboarding changes affect sandbox dashboard port resolution, CHAT_UI_URL propagation, sandbox creation, and reused sandbox dashboard metadata. The live-supported Ubuntu cloud OpenClaw scenario exercises the primary Docker onboarding path where dashboard port allocation is resolved before sandbox creation.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-openclaw

Optional Vitest E2E scenarios

  • ubuntu-repo-docker-post-reboot-recovery: Optional adjacent coverage for registry/container preservation after onboarding on the same live-supported Ubuntu OpenClaw platform. It is not the primary target for dashboard-port creation/reuse logic, but can provide additional confidence around state preservation paths after sandbox lifecycle mutation.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-docker-post-reboot-recovery

Relevant changed files

  • src/lib/onboard.ts
  • src/lib/onboard/dashboard-port.ts
  • src/lib/onboard/sandbox-reuse.ts

@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 27269640744
Target ref: codex/onboard-reuse-dashboard-flow
Requested jobs: double-onboard-e2e,token-rotation-e2e
Summary: 2 passed, 0 failed, 0 skipped

Job Result
double-onboard-e2e ✅ success
token-rotation-e2e ✅ success

@wscurran wscurran added area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow refactor PR restructures code without intended behavior change labels Jun 10, 2026
@wscurran

Copy link
Copy Markdown
Contributor

@cv cv added the v0.0.64 label Jun 10, 2026
Base automatically changed from codex/onboard-dashboard-port-flow to main June 11, 2026 07:55
@cv
cv marked this pull request as ready for review June 11, 2026 07:57
@cv
cv merged commit dd9d716 into main Jun 11, 2026
42 checks passed
@cv
cv deleted the codex/onboard-reuse-dashboard-flow branch June 11, 2026 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow refactor PR restructures code without intended behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants