fix(ai): verify OS port availability in assign_port to prevent cross-engine data-plane crosstalk (#1558)#1566
Open
meetp06 wants to merge 1 commit into
Conversation
) TaskServer.assign_port() allocated data/debug ports by consulting only its own in-memory `_allocated_ports` list, never checking whether the port was actually free on the host. When two engines run on one machine (two workspaces, or an IDE engine plus a CLI engine), both default to `base_port=20000` and both hand out 20000/20001 first. The second engine's node host then dials `ws://localhost:{data_port}/task/data` and connects to the *first* engine's data server, so node hosts silently talk to the wrong engine's data plane. It surfaces as a misleading downstream error (e.g. a vector-store/DNS failure), not a port collision. Add a real bind test: `_is_port_free()` attempts to bind `127.0.0.1:port` (no SO_REUSEADDR, so an in-use port fails), and `assign_port()` now skips a candidate unless it is free both in `_allocated_ports` and on the host. Two engines on the same machine can no longer hand out the same port. Tests: stub the host check in the existing logic tests so they stay deterministic, and add coverage for the new behaviour — an OS-busy port is skipped, the real bind-check reports bound/free correctly, and end-to-end assign_port skips a port held by a live socket. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
meetp06
requested review from
Rod-Christensen,
jmaionchi and
stepmikhaylov
as code owners
July 13, 2026 22:47
Contributor
🤖 Internal: Discord sync markerAuto-managed by the Discord notification workflow. Stores the linked Discord message ID and forum thread ID. Do not edit or delete. |
Contributor
📝 WalkthroughWalkthroughTaskServer port allocation now checks both in-process allocations and actual TCP bind availability on ChangesTask server port allocation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant TaskServer
participant HostSocket
participant AllocatedPorts
TaskServer->>HostSocket: Probe 127.0.0.1:<port>
HostSocket-->>TaskServer: Report bind availability
TaskServer->>AllocatedPorts: Record available port
TaskServer-->>TaskServer: Return port
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ai/src/ai/modules/task/task_server.py`:
- Around line 699-727: Replace the probe-and-close behavior in _is_port_free and
the associated assign_port flow with a reservation that remains held until the
child listener binds, or pass the reserved socket file descriptor to that
listener. Ensure concurrent engines cannot receive the same port between
checking availability and child startup, while preserving the existing port
allocation behavior for unavailable ports.
- Around line 720-727: Update _is_port_free and the surrounding port-allocation
loop to handle out-of-range ports above 65535: catch OverflowError alongside
OSError or clamp the search range before socket.bind. Ensure exhausted searches
continue to the existing RuntimeError('No available ports...') path rather than
propagating the overflow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0a3d9be2-b348-4c69-aee3-54740e962681
📒 Files selected for processing (2)
packages/ai/src/ai/modules/task/task_server.pypackages/ai/tests/ai/modules/task/test_task_server.py
Comment on lines
+699
to
+727
| @staticmethod | ||
| def _is_port_free(port: int) -> bool: | ||
| """ | ||
| Check whether a TCP port is actually free on the host. | ||
|
|
||
| The in-memory ``_allocated_ports`` list only knows about ports handed | ||
| out by *this* engine process. It cannot see ports held by another | ||
| engine (e.g. a second workspace, or an IDE engine alongside a CLI | ||
| engine) or any unrelated process. Two engines both defaulting to | ||
| ``base_port=20000`` would otherwise both hand out 20000/20001, and the | ||
| second engine's node host would silently dial the first engine's data | ||
| plane (see #1558). | ||
|
|
||
| Attempt a real bind on ``127.0.0.1:port`` to confirm availability. | ||
|
|
||
| Args: | ||
| port: TCP port number to probe. | ||
|
|
||
| Returns: | ||
| True if the port could be bound (free), False if it is in use. | ||
| """ | ||
| with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: | ||
| try: | ||
| # No SO_REUSEADDR: we want the bind to fail if another | ||
| # process (or engine) is already holding this port. | ||
| sock.bind(('127.0.0.1', port)) | ||
| return True | ||
| except OSError: | ||
| return False |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the relevant file and inspect the surrounding port-allocation code.
python3 - <<'PY'
from pathlib import Path
p = Path("packages/ai/src/ai/modules/task/task_server.py")
print("exists:", p.exists())
print("lines:", sum(1 for _ in p.open()))
PY
sed -n '640,780p' packages/ai/src/ai/modules/task/task_server.py
# Find the assign_port implementation and nearby callers/usages.
rg -n "def assign_port|_is_port_free|allocated_ports|base_port|socket\.bind|port" packages/ai/src/ai/modules/task/task_server.pyRepository: rocketride-org/rocketride-server
Length of output: 9074
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show where assign_port() is called and how the returned port is used.
rg -n "assign_port\(" packages/ai/src/ai/modules/task
# Inspect the launch/startup path around the call sites.
python3 - <<'PY'
from pathlib import Path
p = Path("packages/ai/src/ai/modules/task/task_server.py")
lines = p.read_text().splitlines()
for needle in ["assign_port(", "release_port(", "socket.bind(", "port = self.assign_port", "self.assign_port()"]:
hits = [i+1 for i,l in enumerate(lines) if needle in l]
print(needle, hits)
PYRepository: rocketride-org/rocketride-server
Length of output: 586
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the task engine code around the port assignment sites.
sed -n '1460,1535p' packages/ai/src/ai/modules/task/task_engine.py
# Show any direct socket/server binds in the task engine file.
rg -n "bind\\(|listen\\(|serve|start_server|create_server|open_connection|_data_port|_debug_port" packages/ai/src/ai/modules/task/task_engine.pyRepository: rocketride-org/rocketride-server
Length of output: 6022
Hold the port reservation until the child binds
_is_port_free() only probes and closes the socket, so assign_port() can still hand out the same port to two concurrent engines before either child process binds it. Reserving the socket through startup or handing the fd to the listener is the safer fix.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ai/src/ai/modules/task/task_server.py` around lines 699 - 727,
Replace the probe-and-close behavior in _is_port_free and the associated
assign_port flow with a reservation that remains held until the child listener
binds, or pass the reserved socket file descriptor to that listener. Ensure
concurrent engines cannot receive the same port between checking availability
and child startup, while preserving the existing port allocation behavior for
unavailable ports.
Comment on lines
+720
to
+727
| with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: | ||
| try: | ||
| # No SO_REUSEADDR: we want the bind to fail if another | ||
| # process (or engine) is already holding this port. | ||
| sock.bind(('127.0.0.1', port)) | ||
| return True | ||
| except OSError: | ||
| return False |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant file and nearby symbols first.
ast-grep outline packages/ai/src/ai/modules/task/task_server.py --view expanded
# Read the relevant region around the port probe and allocator.
sed -n '700,780p' packages/ai/src/ai/modules/task/task_server.py
# Inspect the referenced test region.
sed -n '180,260p' packages/ai/src/ai/modules/task/test_task_server.py
# Search for other call sites or guards around port allocation.
rg -n "_is_port_free|assign_port|base_port|65535|OverflowError" packages/ai/src/ai/modules/taskRepository: rocketride-org/rocketride-server
Length of output: 5692
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the test file and inspect port-allocation tests.
fd -a "test_task_server.py" packages
rg -n "assign_port|_is_port_free|base_port|ephemeral|port" packages/ai/src/ai/modules/task -g '*test*'
# Read the likely test file if found.
test_file="$(fd -a "test_task_server.py" packages | head -n 1 || true)"
if [ -n "$test_file" ]; then
wc -l "$test_file"
sed -n '1,280p' "$test_file"
fi
# Probe socket behavior for out-of-range ports.
python3 - <<'PY'
import socket
for port in [65535, 65536, 70000, -1]:
try:
with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s:
s.bind(("127.0.0.1", port))
print(port, "OK")
except Exception as e:
print(port, type(e).__name__, e)
PYRepository: rocketride-org/rocketride-server
Length of output: 243
🏁 Script executed:
#!/bin/bash
set -euo pipefail
test_file="packages/ai/tests/ai/modules/task/test_task_server.py"
# Inspect the relevant test area.
wc -l "$test_file"
sed -n '1,280p' "$test_file"
# Confirm the socket exception type for out-of-range ports.
python3 - <<'PY'
import socket
for port in [65535, 65536, 70000, -1]:
try:
with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s:
s.bind(("127.0.0.1", port))
print(port, "OK")
except Exception as e:
print(port, type(e).__name__, str(e))
PYRepository: rocketride-org/rocketride-server
Length of output: 10327
Guard _is_port_free against OverflowError
packages/ai/src/ai/modules/task/task_server.py:700-727 can walk past 65535 when base_port is high, and socket.bind() raises OverflowError there. Catch it (or clamp the search range) so port allocation still falls through to RuntimeError('No available ports...') instead of crashing.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ai/src/ai/modules/task/task_server.py` around lines 720 - 727,
Update _is_port_free and the surrounding port-allocation loop to handle
out-of-range ports above 65535: catch OverflowError alongside OSError or clamp
the search range before socket.bind. Ensure exhausted searches continue to the
existing RuntimeError('No available ports...') path rather than propagating the
overflow.
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
1 participant
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.
Fixes #1558
Problem
TaskServer.assign_port()allocated data/debug ports by consulting only its own in-memory_allocated_portslist — it never verified the port was free on the host. When two engines run on one machine (two workspaces, or an IDE engine + a CLI engine), both default tobase_port=20000and both hand out20000/20001first. The second engine's node host then dialsws://localhost:{data_port}/task/dataand connects to the first engine's data server — node hosts silently talk to the wrong engine's data plane.It surfaces as a misleading downstream error (in the report, a vector-store/DNS failure at
store.py:440), not a port collision — costing hours of misdirected debugging. Any machine running two engines is exposed.Fix
Add a real bind test before accepting a port (the approach suggested in the issue):
_is_port_free(port)— attempts to bind127.0.0.1:port. NoSO_REUSEADDR, so a port already held by another process/engine fails the bind and is reported busy.assign_port()— a candidate is now skipped unless it is free both in_allocated_portsand on the host OS.Two engines on the same machine can no longer be handed the same port, so the cross-engine data-plane connection can't happen.
Tests
test_assign_port_skips_os_busy_port— a port free in_allocated_portsbut busy on the host is skipped.test_is_port_free_detects_bound_port— the real bind-check reports a bound port busy and a released port free.test_assign_port_real_bind_skips_held_port— end-to-end:assign_portskips a port held by a live socket.Verification (run locally)
ruff check— All checks passedruff format --check— already formattedpytestsuite needs theaipackage environment; covered by CI.Notes
🤖 Generated with Claude Code
Summary by CodeRabbit