fix(browse): clarify localhost bind failures in sandboxes#1664
Open
spacegeologist wants to merge 1 commit into
Open
fix(browse): clarify localhost bind failures in sandboxes#1664spacegeologist wants to merge 1 commit into
spacegeologist wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reviewer Guide
browse/src/server.tspreserves the actual localhost bind failure fromnet.createServer().listen()and separates realEADDRINUSEport occupancy from sandbox / OS bind-denial failures such asEPERM.browse/test/findport.test.tsadds regression coverage for both explicitBROWSE_PORTand random-port diagnostics, while keeping the old “port is in use” meaning for trueEADDRINUSE.Why
I have been experimenting with running
gstackinside Codex and noticed thatbrowsecan report random port exhaustion when the underlying issue is actuallya sandbox-denied
127.0.0.1listen()call.browsestartup currently collapses every failed localhost bind attempt into a port-availability message like “No available port after 5 attempts” or “Port ... is in use.” In a sandboxed agent environment, including Codex,listen()can fail withEPERMeven when the sampled port is not occupied. That sends the user and reviewer toward the wrong diagnosis: looking for port exhaustion instead of realizing that localhost binding is blocked by sandbox / OS permissions.This patch keeps the successful path unchanged, but carries the original bind error through the diagnostic path. If the failure is
EADDRINUSE,browsestill reports an occupied port. If the failure is something else, the error now says that localhost binding is likely blocked and suggests allowing localhost binding, settingBROWSE_PORTto an approved port, or running from an unrestricted terminal.Evidence
I hit this while dogfooding
browsefrom a sandboxed Codex session. The installedgstackbinary failed with the old message:After the local build, the same default sandbox scenario reports the actual class of failure:
When localhost binding is approved in the same environment,
browse statusbecomes healthy, which confirms this is a diagnostic clarity issue rather than every random port being occupied.Risk / Scope
EADDRINUSEstill reports the existing occupied-port message.Verification
bun test browse/test/findport.test.tsbun test browse/test/server-factory.test.tsbun run buildgit diff --checkbrowse/dist/browse statusunder the default sandbox now reports theEPERMbind-denial diagnostic instead of the misleading port-exhaustion message.