Skip to content

fix(server-core): report occupied initial port#1247

Merged
omeraplak merged 1 commit intomainfrom
fix/port-conflict-guidance
Apr 25, 2026
Merged

fix(server-core): report occupied initial port#1247
omeraplak merged 1 commit intomainfrom
fix/port-conflict-guidance

Conversation

@omeraplak
Copy link
Copy Markdown
Member

@omeraplak omeraplak commented Apr 25, 2026

PR Checklist

Please check if your PR fulfills the following requirements:

Bugs / Features

What is the current behavior?

When the default or requested server port is already occupied, VoltAgent silently allocates the next available port and starts the server there. This can be surprising, especially when the user explicitly expects the configured port to be used.

What is the new behavior?

VoltAgent now stops when the initial port is occupied and prints guidance for configuring another port, for example:

Port 3141 is already in use. To use another port, pass it to your server config, for example: honoServer({ port: 4310 })

fixes #1236

Notes for reviewers

Validation run:

  • pnpm --filter @voltagent/server-core test -- src/utils/port-manager.spec.ts
  • pnpm --filter @voltagent/server-core typecheck

Summary by cubic

When a specific port is requested, @voltagent/server-core now fails fast if it’s taken and shows a suggested alternative. Calls without an explicit port continue to auto-select the next available port.

  • Bug Fixes

    • Explicit port conflicts (EADDRINUSE/EACCES) now throw with guidance (e.g., “Port 3141 is already in use… try { port: 4310 }”); no silent fallback.
    • Default honoServer() without port keeps fallback behavior (e.g., if 3141 is busy, it uses 4310).
  • Migration

    • If you relied on fallback while passing an explicit port, set another port in your config (e.g., honoServer({ port: 4310 })) or handle the error.

Written for commit 1f1195f. Summary will update on new commits.

Summary by CodeRabbit

  • Bug Fixes

    • When a configured server port is already in use, the server now fails fast and provides actionable guidance to choose a different port instead of silently binding to an alternative. Calls without an explicit port retain prior automatic fallback behavior.
  • Tests

    • Updated allocation tests to expect rejection for occupied explicit ports and to validate concurrency and fallback scenarios.
  • Documentation

    • Added release notes explaining the new port-conflict behavior and example configuration guidance.

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 25, 2026

🦋 Changeset detected

Latest commit: 1f1195f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@voltagent/server-core Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 25, 2026

📝 Walkthrough

Walkthrough

allocatePort now treats an explicit preferred port as authoritative: if that port is occupied or unavailable, allocation fails with an actionable error advising how to configure a different port; implicit/no-port calls retain the prior fallback behavior.

Changes

Cohort / File(s) Summary
Changeset Documentation
\.changeset/port-conflict-guidance.md
Adds a patch changeset documenting the new fail-fast behavior when a configured port is occupied and guidance to configure an alternate port.
Port Manager Implementation
packages/server-core/src/utils/port-manager.ts
allocatePort updated to check/reserve the preferred port first, use a helper to find alternatives while skipping already-seen/unavailable or already-allocated ports, and produce more specific error messages that include the unavailable preferred port and an example honoServer({ port: ... }) configuration.
Port Manager Tests
packages/server-core/src/utils/port-manager.spec.ts
Tests updated to expect rejection with guidance when a preferred/default port is unavailable (EADDRINUSE/EACCES/other), concurrency tests adjusted to assert second concurrent request rejects, plus a new test ensuring allocatePort() without an argument still falls back to an alternative when appropriate.

Sequence Diagram(s)

sequenceDiagram
  participant Caller as VoltAgent / Server setup
  participant PortMgr as PortManager.allocatePort
  participant OS as OS/socket
  participant AllocMap as PortManager (allocations)

  Caller->>PortMgr: request allocatePort(preferredPort?)
  alt preferredPort provided
    PortMgr->>OS: try bind preferredPort
    OS-->>PortMgr: failure (EADDRINUSE / EACCES / unavailable)
    PortMgr->>AllocMap: check already-allocated ports
    alt alternatives exist and caller omitted explicit port
      PortMgr->>OS: try bind alternative port
      OS-->>PortMgr: success
      PortMgr-->>Caller: return allocated alternative port
    else no acceptable alternative
      PortMgr-->>Caller: throw error with guidance (include preferredPort and example)
    end
  else no preferredPort
    PortMgr->>OS: scan and attempt candidate ports (skip allocated/unavailable)
    OS-->>PortMgr: success on candidate
    PortMgr-->>Caller: return allocated port
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I nudged the door of port three-one-four-one,
Found it full, the dance could not be done.
I thumped and said, in a helpful sport:
"Set honoServer({ port: 4310 })" — not a port of sort! 🥕

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: fixing the behavior when an initial port is already occupied, with guidance provided to users.
Description check ✅ Passed The PR description is comprehensive, covering current behavior, new behavior, test validation, and following the template structure with all required checklist items addressed.
Linked Issues check ✅ Passed The PR fully addresses the linked issue #1236 requirements: reports occupied ports with guidance instead of silently switching ports, distinguishes between explicit and default port scenarios.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the port conflict guidance behavior specified in issue #1236; no unrelated modifications detected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/port-conflict-guidance

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@joggrbot

This comment has been minimized.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (2)
packages/server-core/src/utils/port-manager.spec.ts (1)

109-111: Optional: assertions on the exact suggested port are brittle.

These tests pin the message to a specific second port (4310, 3141) which depends on the contents and ordering of getPortsToTry. The concurrent test on Lines 280-282 already adopts a regex pattern (honoServer\(\{ port: \d+ \}\)) that is much more resilient — consider applying the same pattern here to decouple from the candidate list.

Also applies to: 147-149, 182-184, 220-222, 316-318, 490-492

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/server-core/src/utils/port-manager.spec.ts` around lines 109 - 111,
The current tests in packages/server-core/src/utils/port-manager.spec.ts assert
exact error strings from portManager.allocatePort (e.g., the suggested port
"4310"), which is brittle; update these assertions to match a regex like
honoServer\(\{ port: \d+ \}\) instead of a hard-coded number so the tests no
longer depend on getPortsToTry ordering—replace the exact string matches at the
shown sites (including the failing assert around allocatePort(3141) and the
other occurrences referenced in the comment) with regex-based toThrow/toMatch
checks that verify the message contains the honoServer({ port: <digits> })
pattern.
packages/server-core/src/utils/port-manager.ts (1)

98-124: Computing the "alternative port" via real port binds on the failure path is wasteful and racy.

findAvailableAlternativePort actually opens a TCP socket on each candidate (via isPortAvailable), each successful bind incurs a 50 ms setTimeout (line 60-62), and on failure may walk the entire candidate list — all just to populate a string in the error message. Beyond cost, the suggested port is never reserved, so by the time the user reads the message and reconfigures, the port may itself be taken (TOCTOU), and concurrent failures can hand out the same suggestion to multiple processes.

Since this value is purely advisory, consider either:

  • Picking the suggestion synchronously from portsToTry (first candidate ≠ unavailablePort and not in allocatedPorts), without any network probe; or
  • Dropping the concrete number and using a generic example (e.g., honoServer({ port: <another port> })).
♻️ Sketch: synchronous suggestion
-    const alternativePort = await this.findAvailableAlternativePort(initialPort, portsToTry);
-    throw new Error(this.createPortInUseMessage(initialPort, alternativePort));
+    const alternativePort = this.pickAlternativePortSuggestion(initialPort, portsToTry);
+    if (alternativePort === undefined) {
+      throw new Error(
+        `Port ${initialPort} is already in use and no alternative port is available`,
+      );
+    }
+    throw new Error(this.createPortInUseMessage(initialPort, alternativePort));
   }
 
-  private async findAvailableAlternativePort(
-    unavailablePort: number,
-    portsToTry: number[],
-  ): Promise<number> {
-    const seenPorts = new Set<number>([unavailablePort]);
-
-    for (const port of portsToTry) {
-      if (seenPorts.has(port) || this.allocatedPorts.has(port)) {
-        continue;
-      }
-
-      seenPorts.add(port);
-
-      const isAvailable = await this.isPortAvailable(port);
-      if (isAvailable) {
-        return port;
-      }
-    }
-
-    throw new Error(
-      `Port ${unavailablePort} is already in use and no alternative port is available`,
-    );
-  }
+  private pickAlternativePortSuggestion(
+    unavailablePort: number,
+    portsToTry: number[],
+  ): number | undefined {
+    return portsToTry.find(
+      (port) => port !== unavailablePort && !this.allocatedPorts.has(port),
+    );
+  }

Note: this also changes the "no alternative" test on lines 225-242 — it would now fire any time the candidate list is exhausted of non-allocated entries, regardless of OS-level availability, which is arguably more accurate for an advisory.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/server-core/src/utils/port-manager.ts` around lines 98 - 124, The
current findAvailableAlternativePort uses isPortAvailable (which does real TCP
binds) to pick an advisory port; change it to pick synchronously: iterate
portsToTry and return the first port that is not equal to unavailablePort and
not present in this.allocatedPorts (do not call isPortAvailable or perform any
network I/O); if no such candidate exists, either return undefined or throw the
same advisory-style error so createPortInUseMessage can fall back to a generic
suggestion (e.g., "try honoServer({ port: <another port> })"); update use-sites
of findAvailableAlternativePort and any error handling to accept a possibly
undefined alternative and avoid races/TOCTOU.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.changeset/port-conflict-guidance.md:
- Around line 1-7: Update the changeset to explicitly call out the default-port
behavior change or narrow the implementation: either (A) clarify in the
changeset text that honoServer() will now fail-fast when no port is provided (so
consumers know the previous automatic rollover behavior is removed), or (B)
change port-manager.ts so the new fail-fast behavior only applies when a user
explicitly passed a port (keep previous rollover when port is unspecified);
reference the port resolution logic in port-manager.ts and the honoServer()
entry point when making the change, and re-evaluate whether the version bump
should be minor/major rather than patch per the project’s release policy.

In `@packages/server-core/src/utils/port-manager.ts`:
- Around line 86-100: The current allocatePort method throws when the
initialPort is in use even if no preferredPort was provided, changing prior
behavior for the default 3141; update allocatePort so it only fail-fast (throw
the Error from createPortInUseMessage) when preferredPort !== undefined, and
otherwise continue to pick and return an available alternative from
findAvailableAlternativePort and add it to allocatedPorts; locate this logic in
allocatePort and use isPortAvailable, findAvailableAlternativePort,
allocatedPorts and createPortInUseMessage to implement the conditional behavior.
- Around line 126-131: The error message in createPortInUseMessage currently
hardcodes "honoServer({ port: ... })" which couples the core to Hono; change
createPortInUseMessage to either accept an injected snippet/label (e.g., add a
parameter or constructor option like adapterHint or configSnippet) and include
that value in the returned string, or replace the hardcoded text with
framework-neutral wording such as "pass port: ${alternativePort} to your server
configuration"; update any callers (e.g., allocatePort or where allocatePort is
invoked) to pass an adapter-specific snippet when available so adapters
(server-hono, server-elysia, etc.) can supply their own example, otherwise rely
on the neutral message.

---

Nitpick comments:
In `@packages/server-core/src/utils/port-manager.spec.ts`:
- Around line 109-111: The current tests in
packages/server-core/src/utils/port-manager.spec.ts assert exact error strings
from portManager.allocatePort (e.g., the suggested port "4310"), which is
brittle; update these assertions to match a regex like honoServer\(\{ port: \d+
\}\) instead of a hard-coded number so the tests no longer depend on
getPortsToTry ordering—replace the exact string matches at the shown sites
(including the failing assert around allocatePort(3141) and the other
occurrences referenced in the comment) with regex-based toThrow/toMatch checks
that verify the message contains the honoServer({ port: <digits> }) pattern.

In `@packages/server-core/src/utils/port-manager.ts`:
- Around line 98-124: The current findAvailableAlternativePort uses
isPortAvailable (which does real TCP binds) to pick an advisory port; change it
to pick synchronously: iterate portsToTry and return the first port that is not
equal to unavailablePort and not present in this.allocatedPorts (do not call
isPortAvailable or perform any network I/O); if no such candidate exists, either
return undefined or throw the same advisory-style error so
createPortInUseMessage can fall back to a generic suggestion (e.g., "try
honoServer({ port: <another port> })"); update use-sites of
findAvailableAlternativePort and any error handling to accept a possibly
undefined alternative and avoid races/TOCTOU.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 78c51214-c28b-48b9-997b-2fa2b62927cf

📥 Commits

Reviewing files that changed from the base of the PR and between 28ebcfc and a79a309.

📒 Files selected for processing (3)
  • .changeset/port-conflict-guidance.md
  • packages/server-core/src/utils/port-manager.spec.ts
  • packages/server-core/src/utils/port-manager.ts

Comment thread .changeset/port-conflict-guidance.md Outdated
Comment thread packages/server-core/src/utils/port-manager.ts
Comment thread packages/server-core/src/utils/port-manager.ts
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Apr 25, 2026

Deploying voltagent with  Cloudflare Pages  Cloudflare Pages

Latest commit: 1f1195f
Status: ✅  Deploy successful!
Preview URL: https://22d4a266.voltagent.pages.dev
Branch Preview URL: https://fix-port-conflict-guidance.voltagent.pages.dev

View logs

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 3 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/server-core/src/utils/port-manager.ts">

<violation number="1" location="packages/server-core/src/utils/port-manager.ts:99">
P1: `allocatePort()` now throws even when no `preferredPort` was specified and the default port (3141) is busy. Previously this gracefully fell through to the next available port in `portsToTry`, which is the expected behavior for users running multiple agents/services side-by-side without configuring explicit ports. The fail-fast should only apply when the caller explicitly passes a port (`preferredPort !== undefined`).</violation>

<violation number="2" location="packages/server-core/src/utils/port-manager.ts:129">
P2: Avoid framework-specific guidance in shared `server-core` error messages; `honoServer(...)` is incorrect for non-Hono providers.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread packages/server-core/src/utils/port-manager.ts Outdated
Comment thread packages/server-core/src/utils/port-manager.ts Outdated
@omeraplak omeraplak force-pushed the fix/port-conflict-guidance branch from a79a309 to 5d8742d Compare April 25, 2026 12:45
@omeraplak omeraplak force-pushed the fix/port-conflict-guidance branch from 5d8742d to 1f1195f Compare April 25, 2026 12:47
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (2)
packages/server-core/src/utils/port-manager.ts (2)

86-100: ⚠️ Potential issue | 🟡 Minor

Default-port (no preferredPort) case also fails fast — confirm scope.

allocatePort() with no argument now throws when 3141 happens to be busy, whereas previously it would fall through to the next available port. Issue #1236 only frames the bug around an explicitly configured port, so this scope expansion will break workflows that today run multiple agents/services side-by-side without specifying a port. If intentional, please call this out explicitly in the changeset and PR description; otherwise gate the throw on preferredPort !== undefined.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/server-core/src/utils/port-manager.ts` around lines 86 - 100,
allocatePort() currently throws whenever the first candidate port (initialPort)
is busy, even when no preferredPort was supplied; change it so the throw is only
used when preferredPort !== undefined. Specifically, in allocatePort (and using
getPortsToTry, isPortAvailable, findAvailableAlternativePort,
createPortInUseMessage, and allocatedPorts), if preferredPort is undefined then
when initialPort is busy call findAvailableAlternativePort and return that
alternative (adding it to allocatedPorts) instead of throwing; only call
createPortInUseMessage and throw when preferredPort was explicitly provided and
no alternative is available.

126-131: ⚠️ Potential issue | 🟠 Major

honoServer({ port: ... }) hardcoded in framework-agnostic core.

server-core is consumed by multiple adapters (e.g., server-elysia); embedding honoServer in this error misleads users of other adapters. Either inject the snippet/label from the calling adapter or use neutral phrasing (e.g., "pass port: <n> to your server config").

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/server-core/src/utils/port-manager.ts` around lines 126 - 131, The
error message in createPortInUseMessage currently hardcodes "honoServer({ port:
... })", which is framework-specific; update createPortInUseMessage to avoid
adapter-specific text by either accepting an injected example string/label from
the caller (add a parameter like exampleSnippet or adapterHint and use it in the
returned message) or replace the hardcoded line with a neutral phrase such as
"pass port: <n> to your server config" so server-core remains
framework-agnostic; modify callers to pass an adapter-specific snippet only when
needed.
🧹 Nitpick comments (2)
packages/server-core/src/utils/port-manager.spec.ts (1)

80-112: Nit: test name no longer matches the assertion.

"should skip already allocated ports" describes the old behavior of falling through to another port. The test now asserts the second allocatePort(3141) rejects with guidance, which is the opposite of "skip". Consider renaming to something like "should reject when re-allocating an already-allocated port".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/server-core/src/utils/port-manager.spec.ts` around lines 80 - 112,
Rename the test case description to reflect the current assertion: change the
it(...) title that currently reads "should skip already allocated ports" to a
name like "should reject when re-allocating an already-allocated port" so it
matches the behavior asserted for portManager.allocatePort(3141) rejecting on
the second call; update the it(...) string in the test that references
allocatePort to the new descriptive title.
packages/server-core/src/utils/port-manager.ts (1)

98-124: Reconsider scanning for an alternative port purely to format the error.

findAvailableAlternativePort is invoked only to fill in the example in createPortInUseMessage, but it has real cost and edge cases:

  • isPortAvailable waits ~50ms per successful probe (line 60–62), so on a busy host the user waits several hundred ms just to receive the error. This is wasted time on the failure path.
  • The suggested port is not reserved — between the throw and the user editing config + restarting, another process can grab it, so the hint can be misleading.
  • When no alternative exists, the throw at lines 121–123 propagates with the message "Port X is already in use and no alternative port is available" and the user never sees the actionable honoServer({ port: ... }) guidance.

Consider dropping the scan and using a fixed/example placeholder, which keeps the message actionable without probing the network:

♻️ Proposed refactor
-    const alternativePort = await this.findAvailableAlternativePort(initialPort, portsToTry);
-    throw new Error(this.createPortInUseMessage(initialPort, alternativePort));
+    throw new Error(this.createPortInUseMessage(initialPort));
   }
-
-  private async findAvailableAlternativePort(
-    unavailablePort: number,
-    portsToTry: number[],
-  ): Promise<number> {
-    const seenPorts = new Set<number>([unavailablePort]);
-
-    for (const port of portsToTry) {
-      if (seenPorts.has(port) || this.allocatedPorts.has(port)) {
-        continue;
-      }
-
-      seenPorts.add(port);
-
-      const isAvailable = await this.isPortAvailable(port);
-      if (isAvailable) {
-        return port;
-      }
-    }
-
-    throw new Error(
-      `Port ${unavailablePort} is already in use and no alternative port is available`,
-    );
-  }
-
-  private createPortInUseMessage(unavailablePort: number, alternativePort: number): string {
+  private createPortInUseMessage(unavailablePort: number): string {
+    const example = unavailablePort === 4310 ? 3141 : 4310;
     return [
       `Port ${unavailablePort} is already in use.`,
-      `To use another port, pass it to your server config, for example: honoServer({ port: ${alternativePort} })`,
+      `To use another port, pass it to your server config, for example: honoServer({ port: ${example} })`,
     ].join(" ");
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/server-core/src/utils/port-manager.ts` around lines 98 - 124, The
port-manager currently calls findAvailableAlternativePort (which uses
isPortAvailable and probes ports) solely to populate createPortInUseMessage,
causing costly waits and race conditions; remove the runtime port scan and
instead return a deterministic example placeholder (e.g., unavailablePort + 1 or
a fixed example like 3000) when building the error message in
createPortInUseMessage, avoid awaiting isPortAvailable or invoking
findAvailableAlternativePort on the failure path, and ensure the thrown error
always contains the actionable guidance (the honoServer({ port: ... }) example)
even when no actual alternative was probed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@packages/server-core/src/utils/port-manager.ts`:
- Around line 86-100: allocatePort() currently throws whenever the first
candidate port (initialPort) is busy, even when no preferredPort was supplied;
change it so the throw is only used when preferredPort !== undefined.
Specifically, in allocatePort (and using getPortsToTry, isPortAvailable,
findAvailableAlternativePort, createPortInUseMessage, and allocatedPorts), if
preferredPort is undefined then when initialPort is busy call
findAvailableAlternativePort and return that alternative (adding it to
allocatedPorts) instead of throwing; only call createPortInUseMessage and throw
when preferredPort was explicitly provided and no alternative is available.
- Around line 126-131: The error message in createPortInUseMessage currently
hardcodes "honoServer({ port: ... })", which is framework-specific; update
createPortInUseMessage to avoid adapter-specific text by either accepting an
injected example string/label from the caller (add a parameter like
exampleSnippet or adapterHint and use it in the returned message) or replace the
hardcoded line with a neutral phrase such as "pass port: <n> to your server
config" so server-core remains framework-agnostic; modify callers to pass an
adapter-specific snippet only when needed.

---

Nitpick comments:
In `@packages/server-core/src/utils/port-manager.spec.ts`:
- Around line 80-112: Rename the test case description to reflect the current
assertion: change the it(...) title that currently reads "should skip already
allocated ports" to a name like "should reject when re-allocating an
already-allocated port" so it matches the behavior asserted for
portManager.allocatePort(3141) rejecting on the second call; update the it(...)
string in the test that references allocatePort to the new descriptive title.

In `@packages/server-core/src/utils/port-manager.ts`:
- Around line 98-124: The port-manager currently calls
findAvailableAlternativePort (which uses isPortAvailable and probes ports)
solely to populate createPortInUseMessage, causing costly waits and race
conditions; remove the runtime port scan and instead return a deterministic
example placeholder (e.g., unavailablePort + 1 or a fixed example like 3000)
when building the error message in createPortInUseMessage, avoid awaiting
isPortAvailable or invoking findAvailableAlternativePort on the failure path,
and ensure the thrown error always contains the actionable guidance (the
honoServer({ port: ... }) example) even when no actual alternative was probed.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3b58c4b8-4404-4f95-a017-51ec00708faa

📥 Commits

Reviewing files that changed from the base of the PR and between a79a309 and 5d8742d.

📒 Files selected for processing (3)
  • .changeset/port-conflict-guidance.md
  • packages/server-core/src/utils/port-manager.spec.ts
  • packages/server-core/src/utils/port-manager.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • .changeset/port-conflict-guidance.md

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/server-core/src/utils/port-manager.ts (1)

119-141: Optional: probing for an alternative before throwing is not strictly "fail fast".

findAvailableAlternativePort performs real net.createServer().listen(...) probes (each followed by a 50ms delay in isPortAvailable, Line 62) for every candidate in portsToTry until one is available or the list is exhausted — purely to enrich the error message. In a typical config this is fine, but with a long portsToTry on a busy host the failure path can become noticeably slow, which slightly undercuts the "fail fast" framing in the PR description.

If you want to keep the helpful suggestion without the probing cost, one option is to suggest a deterministic fallback (e.g., unavailablePort + 1 or the next entry in portsToTry) without verifying it, and let the user retry. Non-blocking — happy to leave as-is if the tradeoff is intentional.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/server-core/src/utils/port-manager.ts` around lines 119 - 141, The
current findAvailableAlternativePort in port-manager.ts probes every candidate
via isPortAvailable (and awaits a delay) which can be slow; change it to avoid
exhaustive probing on failure by either (A) selecting the first non-seen,
non-allocated candidate from portsToTry (or unavailablePort + 1 if none) and
returning that candidate without calling isPortAvailable, or (B) add an optional
boolean param (e.g., verify = true) to findAvailableAlternativePort and, when
verify is false, skip calls to isPortAvailable and immediately return the first
valid candidate; update the thrown Error to include the suggested fallback port
(suggestedPort) and reference the symbols unavailablePort, portsToTry,
allocatedPorts, isPortAvailable, and findAvailableAlternativePort so callers can
opt into verification if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/server-core/src/utils/port-manager.ts`:
- Around line 138-140: The error message thrown when no alternative port is
available uses "is already in use" but should match the broader "is already in
use or unavailable" wording used elsewhere; update the throw in the port
allocation logic (the throw that references unavailablePort in
packages/server-core/src/utils/port-manager.ts) to say "Port ${unavailablePort}
is already in use or unavailable and no alternative port is available" (or
equivalent phrasing) so it aligns with isPortAvailable's handling of EADDRINUSE,
EACCES, and other unavailable cases.

---

Nitpick comments:
In `@packages/server-core/src/utils/port-manager.ts`:
- Around line 119-141: The current findAvailableAlternativePort in
port-manager.ts probes every candidate via isPortAvailable (and awaits a delay)
which can be slow; change it to avoid exhaustive probing on failure by either
(A) selecting the first non-seen, non-allocated candidate from portsToTry (or
unavailablePort + 1 if none) and returning that candidate without calling
isPortAvailable, or (B) add an optional boolean param (e.g., verify = true) to
findAvailableAlternativePort and, when verify is false, skip calls to
isPortAvailable and immediately return the first valid candidate; update the
thrown Error to include the suggested fallback port (suggestedPort) and
reference the symbols unavailablePort, portsToTry, allocatedPorts,
isPortAvailable, and findAvailableAlternativePort so callers can opt into
verification if needed.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1e6bbaf1-ef47-4b52-86c5-d777ea6d5a68

📥 Commits

Reviewing files that changed from the base of the PR and between 5d8742d and 1f1195f.

📒 Files selected for processing (3)
  • .changeset/port-conflict-guidance.md
  • packages/server-core/src/utils/port-manager.spec.ts
  • packages/server-core/src/utils/port-manager.ts
✅ Files skipped from review due to trivial changes (1)
  • .changeset/port-conflict-guidance.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/server-core/src/utils/port-manager.spec.ts

Comment on lines +138 to +140
throw new Error(
`Port ${unavailablePort} is already in use and no alternative port is available`,
);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Minor: align "no alternative" wording with the in-use wording for consistency.

Line 145 reads Port ${unavailablePort} is already in use or unavailable. (covers both EADDRINUSE and EACCES), but Line 139 only says is already in use…. Since isPortAvailable treats EACCES and unknown errors as unavailable too, the no-alternative path can also be triggered by permission-denied. Consider matching the phrasing so users see consistent terminology regardless of whether an alternative was discoverable.

✏️ Suggested wording tweak
     throw new Error(
-      `Port ${unavailablePort} is already in use and no alternative port is available`,
+      `Port ${unavailablePort} is already in use or unavailable, and no alternative port is available`,
     );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
throw new Error(
`Port ${unavailablePort} is already in use and no alternative port is available`,
);
throw new Error(
`Port ${unavailablePort} is already in use or unavailable, and no alternative port is available`,
);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/server-core/src/utils/port-manager.ts` around lines 138 - 140, The
error message thrown when no alternative port is available uses "is already in
use" but should match the broader "is already in use or unavailable" wording
used elsewhere; update the throw in the port allocation logic (the throw that
references unavailablePort in packages/server-core/src/utils/port-manager.ts) to
say "Port ${unavailablePort} is already in use or unavailable and no alternative
port is available" (or equivalent phrasing) so it aligns with isPortAvailable's
handling of EADDRINUSE, EACCES, and other unavailable cases.

@omeraplak omeraplak merged commit 832f094 into main Apr 25, 2026
23 checks passed
@omeraplak omeraplak deleted the fix/port-conflict-guidance branch April 25, 2026 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] It should be reported that the port is occupied.

1 participant