fix(cli): Honor explicit --port literally and fail fast on conflict... (#1079)#65
Draft
aidandaly24 wants to merge 1 commit into
Draft
fix(cli): Honor explicit --port literally and fail fast on conflict... (#1079)#65aidandaly24 wants to merge 1 commit into
aidandaly24 wants to merge 1 commit into
Conversation
…ing by runtime index When running multiple `agentcore dev` runtimes, an explicit -p/--port was silently offset by the runtime's index in agentcore.json (basePort + index), so `agentcore dev -r AgentB -p 8788` bound 8789 with no explanation. Clients targeting http://localhost:<-p>/invocations silently hit the wrong port. Use Commander's getOptionValueSource to distinguish explicit vs default -p: honor an explicit --port literally across the --logs, web-ui, and TUI paths; when -p is implicit (default), keep the index offset but log it (mirroring the existing findAvailablePort shift message) and document it in --help. invoke continues to target whatever port the server bound, made consistent with the chosen server semantics. Scope: Surgical, unit-test-verified. tsc --noEmit clean, eslint clean (no new warnings), 1002/1002 unit tests pass across src/cli/operations/dev and src/cli/tui including the updated and new getAgentPort tests. The only failing tests (4 in dev.test.ts) are pre-existing CLI-spawn failures that fail identically on the clean baseline (require a built dist; environmental, unrelated to this change). The web-ui path is a structurally multi-agent server, so for the explicit -p case the SELECTED runtime binds exactly -p while concurrently-served runtimes are offset relative to it -- the minimal way to honor explicit -p there without redesigning the multi-agent port model.
Coverage Report
|
github-actions
Bot
added
agentcore-harness-reviewing
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.
Refs aws#1079
Issues
agentcore devruntimes in parallel, the-p/--portvalue is silently offset by the runtime's index in agentcore.json (basePort + index).agentcore dev -r AgentB -p 8788actually 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.Root cause
getAgentPort() at config.ts:72-76 returns basePort + runtime array index; the --logs path (command.tsx:402) passes the user's -p as basePort and binds the shifted port via createDevServer, while only the secondary findAvailablePort shift is logged (command.tsx:417-418) and help text (command.tsx:174) omits the base-port semantics. Offset locked by config.test.ts:531-532.
The 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 aws#1158 implements this approach and can be revived/rebased; it also preserves getAgentPort as a deprecated wrapper for backwards-compat.
Files touched: src/cli/operations/dev/config.ts:72-76 (getAgentPort); src/cli/commands/dev/command.tsx:174 (help text), :402 (--logs offset), :262 (invoke — keep but make consistent); src/cli/tui/hooks/useDevServer.ts:157; src/cli/operations/dev/web-ui/handlers/start.ts:103; update test src/cli/operations/dev/tests/config.test.ts:531-532
Validation evidence
The fix was verified by reproducing the original symptom and re-running after the change:
Test suite: green.
Staged on the fork as a draft for human review. Promote to aws/agentcore-cli after vetting.