fix(dev): honor explicit --port literally instead of silently offsetting by runtime index (#1079)

[aidandaly24]

Description

#1079

When running multiple agentcore dev runtimes in parallel, the -p/--port value is silently offset by the runtime's index in agentcore.json (basePort + index). agentcore dev -r AgentB -p 8788 actually binds 8789, with no log explaining it. Downstream services/integration tests/CI scripts that target http://localhost:<-p>/invocations silently hit the wrong port or get connection refused; reordering runtimes in agentcore.json silently remaps every developer's ports. Local-dev only — no deployed/production surface.

Fix

Honor explicit --port literally and fail fast on conflict; when -p is implicit (default), keep the index offset but LOG it (mirror the existing command.tsx:417-418 shift message) and document it in --help. Use Commander's getOptionValueSource to distinguish explicit vs default. Apply consistently across the three server-side offset pathways: command.tsx:402 (--logs), web-ui/handlers/start.ts:103 (uiPort+1+index — this path ignores -p entirely today), and useDevServer.ts:157 (TUI path, which currently honors -p with NO offset — divergent). NOTE: command.tsx:262 (invoke) offset is NOT a bug to remove — invoke must target whatever port the server bound; make it consistent with the chosen server semantics, don't strip it. Closed (unmerged) PR #1158 implements this approach and can be revived/rebased; it also preserves getAgentPort as a deprecated wrapper for backwards-compat.

This fix was produced by an automated triage+fix run and validated locally (build + unit suite + symptom reproduction). Opened as a draft for CI and human review; will be bug-bashed before it's marked ready.

Related Issue

Closes #1079

Documentation PR

N/A — bug fix; no devguide change required.

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation update
• Other (please describe):

Testing

How have you tested the change? (full validation in PR comments after bug bash)

• I ran npm run test:unit and npm run test:integ
• I ran npm run typecheck
• I ran npm run lint
• If I modified src/assets/, I ran npm run test:update-snapshots and committed the updated snapshots — N/A, no src/assets/ changes

Checklist

• I have read the CONTRIBUTING document
• I have added any necessary tests that prove my fix is effective or my feature works
• I have updated the documentation accordingly
• I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
• My changes generate no new warnings
• Any dependent changes have been merged and published

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.

[agentcore-devx-automation]

Claude Security Review: no high-confidence findings. (run)

[github-actions]

Package Tarball

aws-agentcore-0.21.0.tgz

How to install

gh release download pr-1661-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.21.0.tgz

[agentcore-cli-automation]

Thanks for picking this up — the core getAgentPort(..., explicit) change is clean and the unit tests for the explicit-honored case are solid. A few issues to address before merge:

1. Missing fail-fast on conflict for explicit -p (HTTP). This is the headline behavior promised in the PR description ("Honor explicit --port literally and fail fast on conflict") but it isn't implemented for HTTP agents — findAvailablePort silently picks a different port. This reproduces the same class of silent-rebind surprise the PR is trying to fix. See inline comments on command.tsx, useDevServer.ts, and handlers/start.ts.
2. Misleading log message when the default offset kicks in (Port X in use as base...) — the port isn't actually in use, it's just being skipped for index reasons.
3. No tests for the new branch in handlers/start.ts (the most subtle logic in the PR).
4. Confusing offset semantics for non-selected runtimes in the web UI when -p is explicit (can produce ports below the user-requested base).
5. Telemetry (non-blocking). Per src/cli/telemetry/README.md, please consider adding a port_explicit: z.boolean().optional() attribute to DevAttrs in src/cli/telemetry/schemas/command-run.ts and setting it from command.tsx. The bug being fixed here is about silent misuse of -p; instrumenting explicit-vs-default would let us validate adoption of the new behavior in the wild.

Details inline.

[agentcore-cli-automation]

Misleading message. This fires when !portExplicit && fixedPort !== port, i.e. the default base plus a non-zero runtime index. The text Port ${port} in use as base, using ${fixedPort} implies a port conflict — but nothing is actually in use; the port is just being skipped because of the runtime index.

The PR description references mirroring an existing "shift message," but the original message at the old command.tsx:417-418 was Port ${fixedPort} in use, using ${actualPort} (about a real availability fallback). The new message conflates two different events.

Suggest something like: Runtime "${config.agentName}" is at index ${idx}; using port ${fixedPort} (pass --port ${fixedPort} explicitly to override).

[aidandaly24]

Fixed in 8c56f8f. Reworded to Runtime "${config.agentName}" is at index ${idx}; using port ${fixedPort} (pass --port ${fixedPort} to override). — it no longer implies a conflict and points at the index + the explicit-override escape hatch.

[agentcore-cli-automation]

Missing fail-fast on explicit-port conflict (HTTP). Two lines below this (the unchanged const actualPort = await findAvailablePort(fixedPort) on the old line 412), HTTP agents will silently bind to whatever findAvailablePort returns. The A2A/MCP throw that follows it doesn't apply.

The PR description states the fix is to "honor explicit --port literally and fail fast on conflict," but for HTTP agents this code path silently rebinds. If a user runs agentcore dev -r AgentB -p 8788 --logs and 8788 is taken, they'll silently bind to 8789 — the exact silent-port-shift behavior this PR is trying to eliminate.

Options:

• Add an explicit branch: when portExplicit && !isA2A && !isMcp && actualPort !== fixedPort, throw a ValidationError (mirroring the A2A/MCP message).
• Alternatively, skip findAvailablePort entirely when portExplicit is true for HTTP and try to bind directly, surfacing EADDRINUSE.

The same fix needs to be applied to useDevServer.ts (around line 186-188) and handlers/start.ts (around line 115).

[aidandaly24]

Fixed in 8c56f8f. Added an explicit-conflict branch in all three paths: command.tsx now throws a ValidationError when isHttp && portExplicit && actualPort !== fixedPort; useDevServer.ts sets error status + returns instead of warning; handlers/start.ts returns { success: false, error } for the selected runtime. So dev -r AgentB -p 8788 --logs with 8788 taken now fails fast instead of silently binding 8789.

[agentcore-cli-automation]

Two concerns with this branch:

1. Unexpected offsets for non-selected agents. When the user runs agentcore dev -p 8788 -r AgentB (AgentB at index 1), selectedIndex=1. The selected agent gets 8788 + (1-1) = 8788 (good). But if the user then clicks AgentA (index 0) in the UI, it tries to bind 8788 + (0-1) = 8787 — below the port the user asked for. And AgentC (index 2) gets 8789, which the user never mentioned. The "relative to selected" offset is non-obvious and produces ports the user didn't request (potentially even privileged ports if -p is low and another agent has a lower index).

   Consider one of:

   • Use agentBasePort + safeAgentIndex for all runtimes (treat the explicit -p as the new base for the project; the selected runtime is no longer guaranteed to bind literally to -p unless it's index 0).
   • Honor -p only for the selected runtime and use the default uiPort + 1 + index allocation for the others.
   • At minimum, document the current behavior clearly so users aren't surprised.

2. No fail-fast on explicit-port conflict. Same issue as command.tsx: findAvailablePort(targetPort) on the next line silently picks a different port. When agentBasePort came from an explicit -p, this should error instead of silently rebinding (at least for the selected runtime where agentBasePort matches the user-typed value).

Also — this branch has no test coverage. Please add a unit test for doStartAgent's port resolution covering: (a) explicit agentBasePort honored for selected runtime, (b) other runtimes' relative offset, (c) fixed-port (A2A/MCP) still wins, (d) explicit-port-conflict behavior.

[aidandaly24]

Fixed in 8c56f8f. Took the second option you suggested: explicit -p is now honored literally only for the selected runtime; every other runtime keeps the default uiPort + 1 + index allocation, so we never produce ports below the requested base or for runtimes the user didn't name. The logic is extracted into a pure resolveAgentTargetPort helper with unit tests in web-ui/__tests__/start-port.test.ts covering: default allocation, agent-not-found, A2A/MCP fixed ports, explicit-honored selected runtime, and non-selected runtimes keeping the default. Explicit-port conflict for the selected runtime now fails fast (see the other thread).

[agentcore-cli-automation]

Missing fail-fast on explicit-port conflict. Resolving fixedPort here is fine, but the downstream code (around the unchanged line 186-188) calls findAvailablePort(fixedPort) and just emits an addLog('warn', ...) when the result differs. When options.portExplicit is true, this is the same silent-port-shift bug the PR claims to fix, just in the TUI path.

When portExplicit && !isRestart && port !== fixedPort, this should set status to error and surface a clear message (mirroring the A2A/MCP error branch above), not just warn.

[aidandaly24]

Fixed in 8c56f8f. In the HTTP branch, when options.portExplicit && !isRestart && port !== fixedPort we now addLog('error', ...), setStatus('error') and return, mirroring the A2A/MCP error branch — no more silent rebind in the TUI path. Restart still reuses the prior port as before.

[agentcore-devx-automation]

Claude Security Review: no high-confidence findings. (run)

[agentcore-cli-automation]

Reviewed the diff against the four touched code paths (command.tsx, useDevServer.ts, handlers/start.ts, config.ts) and the existing review from @agentcore-cli-automation already covers every substantive issue I would raise:

• Misleading Port X in use as base log on command.tsx:414 (the port isn't actually in use).
• Missing fail-fast on explicit-port conflict for HTTP in all three server paths (command.tsx:416, useDevServer.ts:186-188, handlers/start.ts:115) — this is the headline promise of the PR and currently silently rebinds, reproducing the bug being fixed.
• Surprising agentBasePort + (safeAgentIndex - selectedIndex) offset for non-selected runtimes in handlers/start.ts (can yield ports below the user-requested base, or privileged ports).
• No unit coverage for the new branch in handlers/start.ts.
• Missing telemetry (port_explicit attribute on DevAttrs) to validate adoption of the new behavior.

No additional issues to flag. Deferring to the author to address the existing review.

[aidandaly24]

Thanks for the thorough review — all five points addressed in 8c56f8f (on top of the prettier fix in 1ecc5a2):

1. Fail-fast on explicit -p conflict (HTTP) — added in all three paths (command.tsx, useDevServer.ts, handlers/start.ts). An explicit -p that's already taken now errors instead of silently rebinding.
2. Misleading offset log — reworded to name the runtime index and the --port override; no longer implies a conflict.
3. Tests for handlers/start.ts — extracted a pure resolveAgentTargetPort and covered it with unit tests (web-ui/__tests__/start-port.test.ts): default allocation, agent-not-found, A2A/MCP fixed ports, explicit-honored selected runtime, non-selected runtimes keeping the default.
4. Confusing web-UI offset — switched to: explicit -p honored literally only for the selected runtime; all others keep uiPort + 1 + index. No more ports below the requested base or for unnamed runtimes.
5. Telemetry — added port_explicit: z.boolean().optional() to DevAttrs and set it on the dev-server recorder.

Local: prettier clean, eslint (0 errors), typecheck, build, and vitest --project unit for the dev + telemetry suites all green (415 tests).

[agentcore-devx-automation]

Claude Security Review: no high-confidence findings. (run)

[agentcore-cli-automation]

Re-reviewed after 8c56f8f. The five substantive issues raised in the prior review are all addressed:

1. Fail-fast on explicit -p conflict (HTTP) — confirmed in command.tsx:430-434 (ValidationError), useDevServer.ts:190-197 (error log + setStatus + early return), and handlers/start.ts:153-160 (failure result returned before runningAgents is mutated). ✓
2. Misleading offset log — reworded to Runtime "<name>" is at index <idx>; using port <fixedPort> (pass --port <fixedPort> to override). No longer implies a conflict. ✓
3. Non-selected runtime offset surprise — handlers/start.ts now keeps the default uiPort + 1 + index allocation for non-selected runtimes; only the selected runtime honors agentBasePort literally. No more sub-base or unrequested-runtime ports. ✓
4. Test coverage for start.ts — resolveAgentTargetPort extracted as a pure helper and covered by web-ui/__tests__/start-port.test.ts (default allocation, agent-not-found, A2A/MCP fixed, explicit-honored selected, non-selected default). ✓
5. Telemetry — port_explicit: z.boolean().optional() added to DevAttrs and set in the dev-server recorder. ✓

Also verified:

• command.getOptionValueSource(port) === cli || env is the right way to distinguish explicit vs default with Commander. The option has no .env() binding, but treating both as explicit is defensible and future-proof.
• getAgentPort’s new fourth param defaults to false, so the existing two-arg callsites and tests still pass.
• agentBasePort is plumbed end-to-end: command.tsx → runBrowserMode (portExplicit ? port : undefined) → WebUIOptions.agentBasePort → RouteContext.options → resolveAgentTargetPort.
• A2A/MCP framework-fixed ports (9000/8000) still take precedence over an explicit -p in all three paths, which is the correct behavior.

Minor (non-blocking) observations, no changes required:

• In multi-runtime browser mode with no -r but explicit -p, the explicit port is effectively dropped because selectedAgent is undefined. That’s consistent with the new "honor for the selected runtime only" semantics and matches the --logs path which requires -r anyway.
• The port_explicit recorder is only set on the server-start telemetry, not on the invoke/exec branches. That’s fine for adoption tracking since the binding happens on the server path.

LGTM.