fix: use 127.0.0.1 as default hostname for HTTP server#499
fix: use 127.0.0.1 as default hostname for HTTP server#499Avital Ben-Natan (dewdad) wants to merge 1 commit intobrowseros-ai:mainfrom
Conversation
On Windows, Bun.serve() with hostname '0.0.0.0' fails in compiled binaries with a misleading 'Is port N in use?' error. This changes the default hostname to '127.0.0.1' which is sufficient since the MCP server only needs local connections. The '0.0.0.0' binding is preserved when --allow-remote-in-mcp is explicitly enabled. Fixes browseros-ai#498
|
Thank you for your contribution! Before we can merge this PR, we need you to sign our Contributor License Agreement. To sign the CLA, please add a comment to this PR with the following text: You only need to sign once. After signing, this check will pass automatically. Troubleshooting
|
Greptile SummaryThis PR fixes a Windows-specific crash loop in
Confidence Score: 5/5Safe to merge — the fix is targeted, correct, and a security improvement; only a minor log message accuracy issue remains. All remaining findings are P2 style suggestions. The core logic — aligning No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant Main as main.ts (Application.start)
participant Server as server.ts (createHttpServer)
participant Probe as assertPortAvailable (node:net)
participant Bun as Bun.serve()
Main->>Main: mcpAllowRemote ? '0.0.0.0' : '127.0.0.1'
Main->>Server: createHttpServer({ port, host })
Server->>Probe: assertPortAvailable(port, host)
Note over Probe: probe.listen({ port, host, exclusive: true })<br/>host is now the SAME as Bun.serve() will use
Probe-->>Server: resolve (port is free)
Server->>Bun: Bun.serve({ port, hostname: host })
Note over Bun: Previously: assertPortAvailable used '127.0.0.1'<br/>but Bun.serve used '0.0.0.0' → Windows crash
Bun-->>Server: server instance
Server-->>Main: { app, server, config }
Prompt To Fix All With AIThis is a comment left during a code review.
Path: apps/server/src/main.ts
Line: 106-111
Comment:
**Log messages hardcode `127.0.0.1` when binding to `0.0.0.0`**
When `--allow-remote-in-mcp` is enabled the server binds to `0.0.0.0`, but the startup log messages still unconditionally print `127.0.0.1`. This can mislead operators who need to know the actual bound address for remote-access scenarios.
```suggestion
const boundHost = this.config.mcpAllowRemote ? '0.0.0.0' : '127.0.0.1'
logger.info(
`HTTP server listening on http://${boundHost}:${this.config.serverPort}`,
)
logger.info(
`Health endpoint: http://${boundHost}:${this.config.serverPort}/health`,
)
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix: use 127.0.0.1 as default hostname f..." | Re-trigger Greptile |
Summary
Bun.serve()failing on Windows compiled binaries when binding to0.0.0.00.0.0.0to127.0.0.10.0.0.0binding when--allow-remote-in-mcpis explicitly enabledassertPortAvailableprobe hostname with actualBun.serve()hostnameProblem
On Windows,
browseros_server.exe(Bun compiled binary) crash-loops with:The error repeats indefinitely (5,300+ crash-restart cycles observed in logs). The "port 1455" is actually a Windows socket error code leaking into Bun's error message — the port itself is not in use.
Root cause:
Bun.serve({ hostname: '0.0.0.0' })fails in compiled binaries on Windows. TheassertPortAvailableprobe succeeds because it binds to127.0.0.1, but thenBun.serve()attempts0.0.0.0which triggers the failure.Related: oven-sh/bun#12127
Fix
server.ts: Defaulthostchanged from'0.0.0.0'to'127.0.0.1'server.ts:assertPortAvailable()now accepts and uses the samehostparameter asBun.serve()(was hardcoded to'127.0.0.1')main.ts: Host selection respects--allow-remote-in-mcpflag:this.config.mcpAllowRemote ? '0.0.0.0' : '127.0.0.1'This is also a security improvement — the MCP server only needs localhost connections by default, so binding to
0.0.0.0was unnecessarily broad.Testing
127.0.0.1it should bind successfullyFixes #498