fix(onboard): keep unprovable forward ports occupied instead of failing allocation

[sandeepstele]

Outcome

A dashboard or Hermes API port whose OpenShell forward ownership cannot be proven is now treated as occupied, and allocation moves on to the next port. Before this change, one such port made allocation fail with Cannot allocate dashboard port: NemoClaw could not prove OpenShell forward ownership., even when free ports remained in the range. Observations that fail for any other reason (timeout, transport, command, validation) still stop allocation.

Reason

findAvailablePortInRangeFromObservations threw on the first indeterminate observation, before scanning the range. Its own contract says: "Foreign, indeterminate, and missing observations stay occupied so allocation never converts uncertainty into permission to bind." observedForwardPortAvailability already maps such a port to blocked, which the allocator records as "unverified OpenShell forward ownership".

In #11979, on a shared host, a listener that belongs to another user is invisible to non-root lsof but reachable, so the adapter reports that one port as ownership-indeterminate. That port then blocks the whole dashboard range, and the Hermes API port range has the same problem.

Related issues

Refs #11979. This fixes the allocator side. If a host's lsof makes every port unverifiable, for example through stderr warnings, onboarding still stops. It now does so with the range-exhausted error that lists each unverified port, instead of a single opaque ownership message. Adapter-side detection is unchanged.

Changes

• src/lib/onboard/dashboard-port.ts: throw only for an indeterminate observation whose error kind is not ownership. Ownership-indeterminate ports keep their existing blocked → occupied handling. The Hermes API allocator (hermes-api-port.ts) uses the same function, so it gets the same fix.
• src/lib/onboard/dashboard-port.test.ts:
  • The test that locked in the old throw now asserts that the next free port is allocated.
  • New: an all-unverified range throws the range-exhausted error naming each port.
  • New: a timeout observation still stops allocation.
• src/lib/onboard/hermes-api-port.test.ts: the matching Hermes test now expects the next free port.

Verification

• npx vitest run src/lib/onboard/dashboard-port.test.ts src/lib/onboard/hermes-api-port.test.ts: 79 passed. With main's dashboard-port.ts, the three new or updated expectations fail.
• npx vitest run src/lib/onboard/: 565 files, 9529 passed, 1 skipped.
• npm run typecheck:cli: passed.
• Pre-commit hooks passed.
• The diff contains no secrets, API keys or credentials.

Review notes

• Sensitive path: src/lib/onboard/**. No independent pre-publication review exists. The author self-reviewed the change and checked the tests against main's allocator, so it is awaiting maintainer review.
• cc @rsliter, as the author of the fail-closed forward ownership in feat(openshell): centralize forward lifecycle ownership #11594. This inverts the "blocks allocation when ownership is indeterminate" tests, and it keeps the port-level uncertainty fail-closed: an unprovable port is never bound.
• Open question: if the sandbox's own preferred (persisted) dashboard port becomes unprovable, allocation now moves to another free port rather than stopping. If maintainers would rather stop in that case, a port === preferredPort guard is a small follow-up.
• This PR comes from a fork, so openshell-sdk-package fails with "The reviewed SDK is available only to same-repository pull requests".

---

Signed-off-by: Sandeep Satheesh sandeep.sath7@gmail.com

Summary by CodeRabbit

• Bug Fixes
  • Port allocation now skips ports whose ownership cannot be verified and checks for the next available port in the configured range.
  • Allocation still stops when a port’s status is indeterminate due to a timeout, and exhaustion errors identify unverified ports.

[copy-pr-bot]

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]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: NVIDIA/NemoClaw/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: ec062571-39e3-4dcc-a7ea-9dccf41e691b

📥 Commits

Reviewing files that changed from the base of the PR and between 1aca55c and 9af8d39.

📒 Files selected for processing (3)

• src/lib/onboard/dashboard-port.test.ts
• src/lib/onboard/dashboard-port.ts
• src/lib/onboard/hermes-api-port.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

The allocator now skips ownership-indeterminate observations during its early failure check. Tests cover selecting the next absent port and retaining failures for exhausted ranges and timeout observations.

Changes

Port Allocation

Layer / File(s)	Summary
Port selection and validation src/lib/onboard/dashboard-port.ts, src/lib/onboard/dashboard-port.test.ts, src/lib/onboard/hermes-api-port.test.ts	The allocator continues checking ports after an ownership-indeterminate observation. Tests verify selecting the next absent port, reporting unverified ports when the range is exhausted, and preserving timeout errors.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: rsliter

Merge Risk: ⚪ Minimal · up to 9af8d

The allocator can now try later free ports without claiming ports whose ownership is uncertain, while other indeterminate results still stop allocation. No material merge-blocking risk is evident.

🚥 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 concisely describes the main change: unprovable forward ports remain occupied, while allocation continues instead of failing immediately.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files.
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.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

_{Comment @coderabbitai help to get the list of available commands.}

[sandeepstele]

@rsliter Thanks for #12484. It builds on the #11963 measurements and cuts the dashboard scan to one authority check per batch.

This PR complements it. #12484 makes the observation fast. This one keeps a single port whose ownership cannot be proven from blocking the whole range: the port is treated as occupied, and allocation continues. That is the #11979 case, where another user's listener is invisible to non-root lsof.

I checked it against current main (cb7315d), which includes #12484. It merges cleanly, and dashboard-port.test.ts, hermes-api-port.test.ts and forward-cli.test.ts pass (162 tests).

When you have a moment, could a vetter run /ok to test 9af8d39c4ab21f04cbd65182c121002bc502c197? I'm happy to adjust the preferred-port behaviour noted in the description.