fix(ports): probe all address families in ensurePortAvailable by wangwllu · Pull Request #94415 · openclaw/openclaw

wangwllu · 2026-06-18T03:51:17Z

Summary

ensurePortAvailable did its preflight with a bare tryListenOnPort, which binds the IPv6 wildcard (::) on dual-stack hosts. An occupant listening only on 127.0.0.1 (IPv4-only) is invisible to that probe, so the preflight reports a busy port as free.

Downstream, Chrome takes that "free" port via --remote-debugging-port and binds 127.0.0.1 for real, collides with the existing IPv4-only occupant, and the CDP HTTP endpoint never comes up — surfacing as a misleading CDP 401 handshake failure (the symptom behind #94379).

The fix routes the preflight through checkPortInUse, which probes all four address endpoints — 127.0.0.1, 0.0.0.0, ::1, :: — so an occupant on any family is caught early with a proper PortInUseError. The heavy lsof/ss diagnostics in inspectPortUsage stay where they were: in handlePortError, on failure only. Net cost is one new export and four lightweight socket probes at startup.

Root cause

Probe before: single listen() on the IPv6 wildcard ::
Miss: an IPv4-only occupant bound to 127.0.0.1 does not conflict with a :: bind on a dual-stack host
Consequence: preflight returns "free" → Chrome binds 127.0.0.1 → EADDRINUSE at the socket layer → CDP endpoint fails → misleading 401

Real behavior proof

Behavior or issue addressed: On a dual-stack host the old preflight binds the IPv6 wildcard :: and never notices an occupant bound only to 127.0.0.1, so it green-lights a port that is actually taken. Chrome then binds 127.0.0.1 via --remote-debugging-port, collides, and the CDP endpoint fails its handshake with a misleading 401 ([Bug]: ensurePortAvailable() pre-flight misses IPv4-only loopback occupant → Chrome CDP bind fails with misleading "HTTP 401" #94379).
Real environment tested: macOS Darwin 25.4.0 (arm64), Node v26.0.0, dual-stack loopback. Reproduced by binding a real IPv4-only occupant on 127.0.0.1, then exercising the old probe (listen ::) and the new probe (listen 127.0.0.1, one of the address families checkPortInUse now covers) against that same port.
Exact steps or command run after this patch: node /tmp/repro-94379.mjs — bind an IPv4-only occupant on 127.0.0.1:<p>, then attempt the old-style :: wildcard bind and the new-style 127.0.0.1 bind on <p>, printing what each returns.
Evidence after fix: copied live terminal output from that run:

occupant bound IPv4-only on 127.0.0.1:61780
OLD preflight (listen ::):           SUCCEEDED -> reports port 61780 FREE  [FALSE NEGATIVE]
NEW preflight (listen 127.0.0.1):    EADDRINUSE -> detected as BUSY -> PortInUseError  [CORRECT]

Observed result after fix: the new preflight probes 127.0.0.1 (among all families) and gets EADDRINUSE against the IPv4-only occupant → classifies the port as busy → throws PortInUseError, so the taken port is no longer let through. The old logic bound :: successfully on the same scenario and wrongly reported the port free.
What was not tested: did not drive a full end-to-end Chrome --remote-debugging-port launch (the root cause lives at the socket address-family layer and is reproduced/fixed there). The sibling isPortBusy in src/cli/ports.ts shares the same flaw and is left for a separate follow-up.

Scope note

A sibling isPortBusy (src/cli/ports.ts, the gateway --force kill-port guard) shares the same bare-listen flaw but on a different surface (force-kill flow, not CDP preflight) with lower harm. Left untouched to keep this PR scoped to #94379; happy to follow up separately.

Fixes #94379

A bare listen binds the IPv6 wildcard on dual-stack hosts and misses an IPv4-only occupant, so the preflight reports a busy port as free. Chrome then binds 127.0.0.1 via --remote-debugging-port, collides, and the CDP HTTP endpoint fails its handshake with a misleading 401. Route the preflight through checkPortInUse, which probes 127.0.0.1, 0.0.0.0, ::1 and :: so an occupant on any family is caught early with a proper PortInUseError. Heavy lsof/ss diagnostics stay in handlePortError, on failure only. Fixes openclaw#94379 Co-Authored-By: Claude <[email protected]>

wangwllu · 2026-06-18T03:51:26Z

@clawsweeper re-review

clawsweeper · 2026-06-18T03:51:28Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

State: Failed
Detail: The targeted re-review did not finish cleanly. Check the workflow run for details.
Run: https://github.com/openclaw/clawsweeper/actions/runs/27735533507
Updated: 2026-06-18T04:13:36.114Z

NianJiuZst · 2026-06-18T04:02:22Z

PR #94415 Review

Non-issues

tryListenOnHost (line 622) sets exclusive: true and treats EADDRNOTAVAIL/EAFNOSUPPORT as "skip" — handles IPv4-only / IPv6-only hosts gracefully ✓
checkPortInUse returns "unknown" (not "busy") when all probes are ambiguous — old tryListenOnPort would have thrown in that case; new code treats it as free. This is a slight behavior relaxation (could let a port through on a weird system), but it matches the existing inspectPortUsage semantics at line 660 and is the conservative call for a preflight guard. Documented in your PR body.
Test is in the un-prefixed describe("ports helpers") block (not describeUnix), which is correct because the test binds only to 127.0.0.1 and works on Windows too.

Minor suggestions (non-blocking)

Follow-up for cli/ports.ts:132-143. Same fix path — isPortBusy should call checkPortInUse instead of tryListenOnPort({port, exclusive: true}). Not blocking this PR, but worth filing as a separate issue so it doesn't get lost. Lower harm (kills a stale process, doesn't silently let Chrome bind on a busy port) but same code smell.
Optional: positive-path test. Existing test only covers the busy case. A "free port returns without throwing" test for ensurePortAvailable would lock the behavior in. Trivial to add — 5 lines. Optional.
Test name nit. Current name: "ensurePortAvailable rejects an IPv4-only occupant on a dual-stack host". Consider "regression: #94379 — IPv4-only occupant is detected" to make the issue link explicit. Cosmetic.

Lock in that ensurePortAvailable resolves for a genuinely free port, and rename the IPv4-only case to reference the issue it guards. Co-Authored-By: Claude <[email protected]>

wangwllu · 2026-06-18T04:22:04Z

Thanks for the review @NianJiuZst — addressed all three:

Nit 2 (positive-path test): added ensurePortAvailable resolves when the port is free in src/infra/ports.test.ts (df50b3b).
Nit 3 (issue-tagged name): renamed the IPv4-only case to regression: #94379 — ensurePortAvailable detects an IPv4-only occupant on a dual-stack host (df50b3b).
Suggestion 1 (sibling isPortBusy): filed as a separate issue — isPortBusy preflight misses IPv4-only occupant (same address-family flaw as #94379) #94426 — to keep this PR scoped, as you suggested.

15/15 unit tests pass; oxlint clean.

clawsweeper · 2026-06-18T05:43:28Z

Codex review: needs maintainer review before merge. Reviewed June 18, 2026, 8:55 AM ET / 12:55 UTC.

Summary
The PR exports checkPortInUse, routes ensurePortAvailable through that multi-family probe, and adds focused port availability regression tests.

PR surface: Source +4, Tests +41. Total +45 across 3 files.

Reproducibility: yes. by source and supplied proof: current main uses a hostless tryListenOnPort({ port }) before the loopback Chrome CDP launch, and the PR body includes terminal output showing the old false-free probe and new EADDRINUSE detection. I did not run a live macOS dual-stack reproduction in this read-only review.

Review metrics: 1 noteworthy metric.

SDK-exported helper behavior: 1 changed. ensurePortAvailable is exported through the plugin SDK security runtime, so maintainers should consciously accept the public-helper behavior change before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #94379
Summary: This PR is a candidate fix for the canonical managed-Chrome CDP port-preflight bug; a separate issue tracks the same address-family flaw on the gateway force-kill helper.

Members:

canonical: [Bug]: ensurePortAvailable() pre-flight misses IPv4-only loopback occupant → Chrome CDP bind fails with misleading "HTTP 401" #94379 - The PR body uses closing syntax for this browser CDP preflight bug and implements the reported ensurePortAvailable path.
fixed_by_candidate: fix(infra): probe 127.0.0.1 in ensurePortAvailable to detect IPv4-only occupants #94394 - This open sibling PR targets the same ensurePortAvailable browser CDP bug with a narrower IPv4-loopback probe.
superseded: fix(chrome): probe IPv4 loopback in ensurePortAvailable to match Chrome CDP bind #94390 - This closed unmerged PR was an earlier attempt at the same browser CDP preflight fix.
partial_overlap: isPortBusy preflight misses IPv4-only occupant (same address-family flaw as #94379) #94426 - This issue tracks the same address-family flaw on src/cli/ports.ts, a separate gateway --force caller surface.
partial_overlap: fix(cli): route isPortBusy through checkPortInUse to detect IPv4-only occupants #94456 - This open PR is a candidate fix for the isPortBusy sibling issue, not the browser CDP preflight fixed here.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Risk before merge

[P1] ensurePortAvailable is exported through src/plugin-sdk/security-runtime.ts, so merging intentionally changes an SDK-visible helper from missing some address-family conflicts to failing earlier with PortInUseError or an availability-verification error.
[P1] There are multiple open candidate PRs around the same root cause; maintainers should pick one canonical ensurePortAvailable fix and keep the isPortBusy sibling under its separate issue.

Maintainer options:

Accept the SDK-visible fail-fast change (recommended)
Proceed with this PR once required checks pass if maintainers agree ensurePortAvailable should now report address-family conflicts that shipped releases missed.
Choose a browser-only preflight seam
Pause this PR if maintainers want to avoid changing the SDK-exported helper contract and instead fix only the managed Chrome launch path.

Next step before merge

No automated repair is indicated; maintainers should choose this PR or another canonical candidate and accept the SDK-visible fail-fast behavior before merge.

Security
Cleared: The diff only changes local TCP port probing and tests; it does not touch secrets, CI, dependencies, package metadata, downloads, or broader code execution paths.

Review details

Best possible solution:

Land this narrow multi-family CDP preflight fix, or an equivalent canonical PR, after maintainer acceptance of the SDK-visible fail-fast behavior; keep isPortBusy work tracked separately under #94426.

Do we have a high-confidence way to reproduce the issue?

Yes by source and supplied proof: current main uses a hostless tryListenOnPort({ port }) before the loopback Chrome CDP launch, and the PR body includes terminal output showing the old false-free probe and new EADDRINUSE detection. I did not run a live macOS dual-stack reproduction in this read-only review.

Is this the best way to solve the issue?

Yes: reusing the existing multi-family checkPortInUse helper is a better fix than adding a one-host browser workaround, and the latest head preserves fail-fast behavior for unverifiable ports. The gateway --force sibling should remain separate because it is a different caller surface with its own issue.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 00a06eab4485.

Label changes

Label changes:

add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦞 diamond lobster, so this older rating label is no longer current.

Label justifications:

P2: This is a focused browser startup bugfix with a clear source path, supplied proof, and limited blast radius.
merge-risk: 🚨 compatibility: The PR changes behavior of an SDK-exported port helper by reporting address-family conflicts earlier than shipped releases did.
rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body supplies copied terminal output from a real macOS socket repro showing the old false-free probe and the new EADDRINUSE detection path after the patch.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body supplies copied terminal output from a real macOS socket repro showing the old false-free probe and the new EADDRINUSE detection path after the patch.

Evidence reviewed

PR surface:

Source +4, Tests +41. Total +45 across 3 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	2	15	11	+4
Tests	1	41	0	+41
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	3	56	11	+45

What I checked:

Current main still has the reported behavior: ensurePortAvailable still calls tryListenOnPort({ port }) without a host on current main, so the preflight can still use Node's host-omitted listen behavior. (src/infra/ports.ts:42, 00a06eab4485)
PR head uses the shared multi-family probe: At the PR head, ensurePortAvailable calls checkPortInUse(port), throws PortInUseError for busy, and fails fast for unknown. (src/infra/ports.ts:42, 07238df4bf83)
Shared helper probes the relevant address families: The reused helper probes 127.0.0.1, 0.0.0.0, ::1, and ::, returning busy on the first address-family conflict. (src/infra/ports-inspect.ts:637, 07238df4bf83)
Browser caller matches the reported CDP path: Managed Chrome startup calls ensurePortAvailable(profile.cdpPort) before spawning Chrome with --remote-debugging-port=${profile.cdpPort}. (extensions/browser/src/browser/chrome.ts:637, 00a06eab4485)
Latest release has not shipped the fix: The latest release tag still calls tryListenOnPort({ port }) in ensurePortAvailable. (src/infra/ports.ts:42, 844f405ac1be)
Dependency contract checked: Node's server.listen([port[, host...]]) documentation says omitting host listens on unspecified IPv6 :: when IPv6 is available, or 0.0.0.0 otherwise; that supports the address-family mismatch behind the report. (nodejs.org)

Likely related people:

suboss87: git blame points the current ensurePortAvailable, checkPortInUse, browser preflight caller, and SDK export lines to the current-main squash commit from fix(slack): remove socket reconnect attempt cap so gateway stays connected indefinitely #73162. (role: introduced or carried current implementation; confidence: medium; commits: 01dcaba78dde; files: src/infra/ports.ts, src/infra/ports-inspect.ts, extensions/browser/src/browser/chrome.ts)
steipete: The current-main commit was merged by steipete and lists Reviewed-by: @steipete; recent history also shows adjacent port test coverage work by Peter Steinberger. (role: reviewer/merger and adjacent contributor; confidence: medium; commits: 01dcaba78dde, f95c09b6f2cd, cab2f891e7a9; files: src/infra/ports.test.ts, src/infra/ports.ts, extensions/browser/src/browser/chrome.ts)
NianJiuZst: The PR discussion includes a focused review identifying the same address-family invariant and the separate isPortBusy follow-up scope. (role: review context; confidence: low; files: src/infra/ports.ts, src/infra/ports-inspect.ts, src/cli/ports.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

xuwei-xy · 2026-06-18T05:49:30Z

I think the fix would involve adjusting test. Would that work for your use case?

ensurePortAvailable routed through checkPortInUse only threw on "busy", silently treating "unknown" (a bind probe that failed with something other than EADDRINUSE, e.g. EACCES/EINVAL) as available. The original bare-listen path rethrew such errors; restore that fail-fast and add an unknown-path regression guard.

steipete · 2026-06-18T13:32:01Z

Superseded by #94394, landed as 4ec9d4a. The landed fix preserves the generic wildcard probe and scopes IPv4 plus configured-host checks to managed Chrome, with focused tests and real Gateway/browser CLI proof.

Thank you @wangwllu for investigating #94379 and contributing a fix.

openclaw-barnacle Bot added size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 18, 2026

clawsweeper Bot mentioned this pull request Jun 18, 2026

fix(ports): specify IPv4 loopback host in ensurePortAvailable #94399

Closed

openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 18, 2026

test(ports): add positive-path guard and tag openclaw#94379 regression

df50b3b

Lock in that ensurePortAvailable resolves for a genuinely free port, and rename the IPv4-only case to reference the issue it guards. Co-Authored-By: Claude <[email protected]>

openclaw-barnacle Bot added size: S and removed size: XS labels Jun 18, 2026

wangwllu mentioned this pull request Jun 18, 2026

isPortBusy preflight misses IPv4-only occupant (same address-family flaw as #94379) #94426

Closed

Pandah97 mentioned this pull request Jun 18, 2026

fix(cli): route isPortBusy through checkPortInUse to detect IPv4-only occupants #94456

Closed

RichChen01 mentioned this pull request Jun 18, 2026

fix: isPortBusy preflight misses IPv4-only occupant #94463

Closed

lzyyzznl mentioned this pull request Jun 18, 2026

fix: #94426 isPortBusy probes all address families via checkPortInUse #94471

Closed

This was referenced Jun 18, 2026

fix(infra): probe 127.0.0.1 in ensurePortAvailable to detect IPv4-only occupants #94394

Merged

fix(cli): add IPv4 loopback host to isPortBusy #94448

Closed

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 18, 2026

clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jun 18, 2026

steipete closed this Jun 18, 2026

sunlit-deng mentioned this pull request Jun 19, 2026

fix(ports): route isPortBusy through checkPortInUse to catch IPv4-only occupants #94949

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(ports): probe all address families in ensurePortAvailable#94415

fix(ports): probe all address families in ensurePortAvailable#94415
wangwllu wants to merge 3 commits into
openclaw:mainfrom
wangwllu:fix-94379-cdp-port-preflight

wangwllu commented Jun 18, 2026 •

edited

Loading

Uh oh!

wangwllu commented Jun 18, 2026

Uh oh!

clawsweeper Bot commented Jun 18, 2026 •

edited

Loading

Uh oh!

NianJiuZst commented Jun 18, 2026

Uh oh!

wangwllu commented Jun 18, 2026

Uh oh!

clawsweeper Bot commented Jun 18, 2026 •

edited

Loading

Uh oh!

xuwei-xy commented Jun 18, 2026

Uh oh!

steipete commented Jun 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Uh oh!

Conversation

wangwllu commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root cause

Real behavior proof

Scope note

Uh oh!

wangwllu commented Jun 18, 2026

Uh oh!

clawsweeper Bot commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NianJiuZst commented Jun 18, 2026

PR #94415 Review

Non-issues

Minor suggestions (non-blocking)

Uh oh!

wangwllu commented Jun 18, 2026

Uh oh!

clawsweeper Bot commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xuwei-xy commented Jun 18, 2026

Uh oh!

steipete commented Jun 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

wangwllu commented Jun 18, 2026 •

edited

Loading

clawsweeper Bot commented Jun 18, 2026 •

edited

Loading

clawsweeper Bot commented Jun 18, 2026 •

edited

Loading