src/lib/registry.ts (1)

25-26: Keep dashboardPort round-trippable through registration.

SandboxEntry now supports dashboardPort, but registerSandbox(...) currently drops it. Persisting it there keeps the type/storage contract consistent and avoids silent loss if any caller starts setting it during initial registration.

Proposed patch

diff --git a/src/lib/registry.ts b/src/lib/registry.ts
@@
     data.sandboxes[entry.name] = {
       name: entry.name,
@@
       disabledChannels:
         Array.isArray(entry.disabledChannels) && entry.disabledChannels.length > 0
           ? [...entry.disabledChannels]
           : undefined,
+      dashboardPort:
+        Number.isInteger(entry.dashboardPort) &&
+        (entry.dashboardPort as number) >= 1024 &&
+        (entry.dashboardPort as number) <= 65535
+          ? entry.dashboardPort
+          : undefined,
     };

🤖 Prompt for AI Agents

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

In `@src/lib/registry.ts` around lines 25 - 26, registerSandbox currently drops
the new SandboxEntry.dashboardPort field during registration; update the
registerSandbox implementation to persist dashboardPort when creating or merging
the stored SandboxEntry. Specifically, ensure the code paths in registerSandbox
that construct the saved entry (and any merge/update logic) copy
sandbox.dashboardPort into the stored object (preserving nullable/optional
semantics), and update any object spread/assign calls that assemble the entry so
they include dashboardPort alongside existing fields to keep the type/storage
contract round-trippable.

src/lib/ports.test.ts (1)

96-105: Add an upper-bound test for preferred port near 65535.

A boundary regression test here would protect against accidental allocation of out-of-range ports when the preferred port is at/near the max.

Suggested test case

diff --git a/src/lib/ports.test.ts b/src/lib/ports.test.ts
@@
   it("returns null when the 10-port window is exhausted", () => {
@@
     expect(port).toBeNull();
   });
+
+  it("does not allocate ports above 65535", () => {
+    const port = findFreeDashboardPort(65535, {
+      probe: {
+        listForwardPorts: () => [65535],
+        probePortFree: () => true,
+      },
+    });
+    expect(port).toBeNull();
+  });
 });

🤖 Prompt for AI Agents

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

In `@src/lib/ports.test.ts` around lines 96 - 105, Add a boundary test for
findFreeDashboardPort that uses a preferred port at/near 65535 to ensure the
function does not return or consider ports >65535; create a test similar to the
existing "10-port window exhausted" case but set preferred port to 65535 (or
65534) and mock probe.listForwardPorts and probe.probePortFree so all candidate
ports in the search window are "held" or unavailable, then assert the result is
null (or within valid port range) to verify no out-of-range port is returned by
findFreeDashboardPort.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/lib/onboard.ts (2)

3543-3544: ⚠️ Potential issue | 🟠 Major

Use reuseChatUiUrlFor() on this fallback reuse path too.

This is the one remaining reuse branch that still seeds ensureDashboardForward() from chatUiUrl, so a sandbox already recorded on an auto-allocated port can still ratchet upward here when the backup path aborts.

Suggested fix

-        const reusePort = ensureDashboardForward(sandboxName, chatUiUrl);
+        const reuseUrl = reuseChatUiUrlFor(sandboxName, chatUiUrl);
+        const reusePort = ensureDashboardForward(sandboxName, reuseUrl);

🤖 Prompt for AI Agents

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

In `@src/lib/onboard.ts` around lines 3543 - 3544, The fallback path currently
calls ensureDashboardForward(sandboxName, chatUiUrl) and seeds it directly with
chatUiUrl which can cause an auto-allocated port to be overwritten; instead,
call reuseChatUiUrlFor(sandboxName, chatUiUrl) to compute the proper URL/port
reuse value and pass that result into ensureDashboardForward, then update the
registry (registry.updateSandbox) with the returned dashboardPort as
before—i.e., replace the direct chatUiUrl argument with the value from
reuseChatUiUrlFor for the ensureDashboardForward invocation.

---

6099-6134: ⚠️ Potential issue | 🟠 Major

Requested-port probing still misses local listeners.

The availability check only runs when another sandbox already owns requestedPort, and it relies on lsof alone. If --control-ui-port or a stored dashboardPort points at a non-OpenShell listener, or non-root lsof returns empty for a root-owned socket, forward start can fail at Line 6168 while this function still returns and persists the dead port. Probe the requested port before forward start, and use a real bind fallback instead of treating empty lsof output as authoritative. Based on learnings, "Empty lsof string output is NOT treated as authoritative — non-root lsof silently drops root-owned sockets ... Empty lsof output must fall through to the net probe."

Also applies to: 6156-6177

🤖 Prompt for AI Agents

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

In `@src/lib/onboard.ts` around lines 6099 - 6134, The port-probing logic
currently only runs when another sandbox owns requestedPort and treats an empty
lsof result as authoritative; update the flow so the requestedPort is always
probed before calling `forward start` (i.e., call the same probe used by
`findFreeDashboardPort` for `requestedPort` up-front) and change
`isPortBoundLocally` (and the `probePortFree` you pass into
`findFreeDashboardPort`) to return a tri-state/unknown (e.g., null/undefined)
when `runCapture([... "lsof" ...])` yields an empty string so the caller falls
back to a real bind/net-probe instead of assuming free; ensure the
`probePortFree` implementation then attempts an actual bind probe when lsof is
inconclusive so `chosen` never points at a dead/non-bindable port.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes