diff --git a/CHANGELOG.md b/CHANGELOG.md index 679d3d4b3..4437a9677 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,19 @@ All notable changes to this project will be documented in this file. +## [Unreleased] + +### Changed +- **Breaking:** `agentcore dev --port ` now honors the value literally + instead of silently offsetting it by the runtime's index in `agentcore.json` + (issue #1079). When `--port` is passed (including `-p 8080`, which matches + the registered default), per-runtime offsetting is disabled and a port + conflict fails fast with a clear error message. When `--port` is omitted, + the previous `basePort + runtimeIndex` auto-allocation continues to apply + but now logs the offset whenever it shifts. Migration: drop the manual + `-p = desiredPort - runtimeIndex` arithmetic from your scripts and pass the + port you actually want. + ## [0.13.1] - 2026-05-06 ### Added diff --git a/src/cli/commands/dev/__tests__/dev.test.ts b/src/cli/commands/dev/__tests__/dev.test.ts index 5b86e647b..efa91af00 100644 --- a/src/cli/commands/dev/__tests__/dev.test.ts +++ b/src/cli/commands/dev/__tests__/dev.test.ts @@ -13,6 +13,12 @@ describe('dev command', () => { expect(result.stdout.includes('--stream'), 'Should show --stream option').toBeTruthy(); expect(result.stdout.includes('--logs'), 'Should show --logs option').toBeTruthy(); expect(result.stdout.includes('8080'), 'Should show default port').toBeTruthy(); + // Stable contract: the --port description must mention "runtime index" + // so users learn the multi-runtime semantics from --help (issue #1079). + expect( + result.stdout.includes('runtime index'), + 'Should document multi-runtime --port semantics (issue #1079)' + ).toBeTruthy(); }); it('does not show --invoke flag', async () => { diff --git a/src/cli/commands/dev/__tests__/port-resolution.test.ts b/src/cli/commands/dev/__tests__/port-resolution.test.ts new file mode 100644 index 000000000..30397c8b4 --- /dev/null +++ b/src/cli/commands/dev/__tests__/port-resolution.test.ts @@ -0,0 +1,292 @@ +import type { AgentCoreProjectSpec, DirectoryPath, FilePath } from '../../../../schema'; +import { computeTargetPort, isPortExplicit, resolveDevPort } from '../port-resolution'; +import { describe, expect, it, vi } from 'vitest'; + +// Helper to cast strings to branded path types for testing +const filePath = (s: string) => s as FilePath; +const dirPath = (s: string) => s as DirectoryPath; + +const makeProject = (...names: string[]): AgentCoreProjectSpec => ({ + name: 'TestProject', + version: 1, + managedBy: 'CDK' as const, + runtimes: names.map(name => ({ + name, + build: 'CodeZip' as const, + runtimeVersion: 'PYTHON_3_12' as const, + entrypoint: filePath('main.py'), + codeLocation: dirPath(`./agents/${name}`), + protocol: 'HTTP' as const, + })), + memories: [], + credentials: [], + evaluators: [], + onlineEvalConfigs: [], + agentCoreGateways: [], + policyEngines: [], + configBundles: [], + abTests: [], + httpGateways: [], +}); + +describe('isPortExplicit', () => { + it('returns true when source is "cli"', () => { + const cmd = { getOptionValueSource: vi.fn(() => 'cli' as const) }; + expect(isPortExplicit(cmd)).toBe(true); + expect(cmd.getOptionValueSource).toHaveBeenCalledWith('port'); + }); + + it('returns true when source is "env" (any non-default counts)', () => { + const cmd = { getOptionValueSource: vi.fn(() => 'env' as const) }; + expect(isPortExplicit(cmd)).toBe(true); + }); + + it('returns true when source is "config"', () => { + const cmd = { getOptionValueSource: vi.fn(() => 'config' as const) }; + expect(isPortExplicit(cmd)).toBe(true); + }); + + it('returns false when source is "default"', () => { + const cmd = { getOptionValueSource: vi.fn(() => 'default' as const) }; + expect(isPortExplicit(cmd)).toBe(false); + }); + + it('returns false and warns when command is undefined', () => { + const onWarn = vi.fn(); + expect(isPortExplicit(undefined, onWarn)).toBe(false); + expect(onWarn).toHaveBeenCalledTimes(1); + expect(onWarn.mock.calls[0]?.[0]).toMatch(/getOptionValueSource unavailable/); + }); + + it('returns false and warns when getOptionValueSource is missing', () => { + const onWarn = vi.fn(); + // Simulate a future Commander upgrade dropping the API. + const cmd = {} as unknown as { getOptionValueSource: () => string }; + expect(isPortExplicit(cmd, onWarn)).toBe(false); + expect(onWarn).toHaveBeenCalledTimes(1); + expect(onWarn.mock.calls[0]?.[0]).toMatch(/getOptionValueSource unavailable/); + }); + + it('uses console.warn by default when onWarn omitted and API missing', () => { + const spy = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + try { + expect(isPortExplicit(undefined)).toBe(false); + expect(spy).toHaveBeenCalledTimes(1); + } finally { + spy.mockRestore(); + } + }); +}); + +describe('resolveDevPort - HTTP protocol', () => { + it('issue #1079: explicit --port 8080 (matching default) honors literally for runtime at index 1', () => { + const project = makeProject('AgentA', 'AgentB'); + // No conflict: availablePort matches the literal explicit port. + const r = resolveDevPort({ + project, + agentName: 'AgentB', + protocol: 'HTTP', + basePort: 8080, + portIsExplicit: true, + availablePort: 8080, + }); + expect(r.port).toBe(8080); + expect(r.offset).toBe(0); + expect(r.conflictError).toBeUndefined(); + expect(r.infoLogs).toEqual([]); + }); + + it('issue #1079: implicit --port (default) auto-offsets and emits an "index" log', () => { + const project = makeProject('AgentA', 'AgentB'); + const r = resolveDevPort({ + project, + agentName: 'AgentB', + protocol: 'HTTP', + basePort: 8080, + portIsExplicit: false, + availablePort: 8081, + }); + expect(r.port).toBe(8081); + expect(r.offset).toBe(1); + expect(r.conflictError).toBeUndefined(); + expect(r.infoLogs.some(l => l.includes('AgentB') && l.includes('index 1') && l.includes('8081'))).toBe(true); + expect(r.infoLogs.some(l => l.includes('pass --port 8081 explicitly to override'))).toBe(true); + }); + + it('issue #1079: explicit --port + conflict yields a conflictError instead of silent shift', () => { + const project = makeProject('AgentA', 'AgentB'); + const r = resolveDevPort({ + project, + agentName: 'AgentB', + protocol: 'HTTP', + basePort: 8788, + portIsExplicit: true, + availablePort: 8793, // simulate conflict — findAvailablePort shifted + }); + expect(r.conflictError).toBeDefined(); + expect(r.conflictError).toMatch(/8788.*in use/); + expect(r.conflictError).toMatch(/Pass a different --port or stop the conflicting process/); + }); + + it('implicit --port + conflict silently shifts and emits "Port X in use, using Y"', () => { + const project = makeProject('AgentA'); + const r = resolveDevPort({ + project, + agentName: 'AgentA', + protocol: 'HTTP', + basePort: 8080, + portIsExplicit: false, + availablePort: 8085, // findAvailablePort shifted + }); + expect(r.port).toBe(8085); + expect(r.conflictError).toBeUndefined(); + expect(r.infoLogs.some(l => l.includes('Port 8080 in use, using 8085'))).toBe(true); + }); + + it('first runtime (index 0) emits no offset log even with implicit port', () => { + const project = makeProject('AgentA', 'AgentB'); + const r = resolveDevPort({ + project, + agentName: 'AgentA', + protocol: 'HTTP', + basePort: 8080, + portIsExplicit: false, + availablePort: 8080, + }); + expect(r.offset).toBe(0); + expect(r.infoLogs).toEqual([]); + }); +}); + +describe('resolveDevPort - A2A/MCP protocols', () => { + it('A2A: ignores basePort/offset and uses 9000', () => { + const project = makeProject('AgentA', 'AgentB'); + const r = resolveDevPort({ + project, + agentName: 'AgentB', + protocol: 'A2A', + basePort: 8080, + portIsExplicit: false, + availablePort: 9000, + }); + expect(r.port).toBe(9000); + // No "index N" log even though AgentB is at index 1 — A2A is gated out. + expect(r.infoLogs.every(l => !l.includes('index'))).toBe(true); + }); + + it('A2A: conflict yields A2A-specific message, not the HTTP one', () => { + const project = makeProject('AgentA', 'AgentB'); + const r = resolveDevPort({ + project, + agentName: 'AgentB', + protocol: 'A2A', + basePort: 8080, + portIsExplicit: true, // explicit flag must NOT trigger the HTTP branch + availablePort: 9001, // conflict + }); + expect(r.conflictError).toBeDefined(); + expect(r.conflictError).toMatch(/A2A agents require port 9000/); + expect(r.conflictError).not.toMatch(/Pass a different --port/); + }); + + it('MCP: ignores basePort/offset and uses 8000', () => { + const project = makeProject('AgentA', 'AgentB'); + const r = resolveDevPort({ + project, + agentName: 'AgentB', + protocol: 'MCP', + basePort: 8080, + portIsExplicit: false, + availablePort: 8000, + }); + expect(r.port).toBe(8000); + expect(r.infoLogs.every(l => !l.includes('index'))).toBe(true); + }); + + it('MCP: conflict yields MCP-specific message, not the HTTP one', () => { + const project = makeProject('AgentA', 'AgentB'); + const r = resolveDevPort({ + project, + agentName: 'AgentB', + protocol: 'MCP', + basePort: 8080, + portIsExplicit: true, + availablePort: 8001, + }); + expect(r.conflictError).toBeDefined(); + expect(r.conflictError).toMatch(/MCP agents require port 8000/); + expect(r.conflictError).not.toMatch(/Pass a different --port/); + }); +}); + +describe('computeTargetPort', () => { + it('agrees with resolveDevPort on targetPort for HTTP+implicit', () => { + const project = makeProject('AgentA', 'AgentB'); + const args = { + project, + agentName: 'AgentB', + protocol: 'HTTP' as const, + basePort: 8080, + portIsExplicit: false, + }; + const target = computeTargetPort(args); + const resolved = resolveDevPort({ ...args, availablePort: target.targetPort }); + expect(resolved.port).toBe(target.targetPort); + expect(resolved.offset).toBe(target.offset); + }); + + it('agrees with resolveDevPort on targetPort for HTTP+explicit', () => { + const project = makeProject('AgentA', 'AgentB'); + const args = { + project, + agentName: 'AgentB', + protocol: 'HTTP' as const, + basePort: 8788, + portIsExplicit: true, + }; + const target = computeTargetPort(args); + expect(target.targetPort).toBe(8788); + expect(target.offset).toBe(0); + }); + + it('A2A always returns 9000 regardless of basePort', () => { + const project = makeProject('AgentA', 'AgentB'); + const target = computeTargetPort({ + project, + agentName: 'AgentB', + protocol: 'A2A', + basePort: 8080, + portIsExplicit: true, + }); + expect(target.targetPort).toBe(9000); + expect(target.offset).toBe(0); + expect(target.infoLogs.every(l => !l.includes('index'))).toBe(true); + }); + + it('MCP always returns 8000 regardless of basePort', () => { + const project = makeProject('AgentA'); + const target = computeTargetPort({ + project, + agentName: 'AgentA', + protocol: 'MCP', + basePort: 9999, + portIsExplicit: false, + }); + expect(target.targetPort).toBe(8000); + expect(target.offset).toBe(0); + }); + + it('emits offset log when HTTP runtime is at index > 0 and port is implicit', () => { + const project = makeProject('AgentA', 'AgentB', 'AgentC'); + const target = computeTargetPort({ + project, + agentName: 'AgentC', + protocol: 'HTTP', + basePort: 8080, + portIsExplicit: false, + }); + expect(target.targetPort).toBe(8082); + expect(target.offset).toBe(2); + expect(target.infoLogs.some(l => l.includes('AgentC') && l.includes('index 2'))).toBe(true); + }); +}); diff --git a/src/cli/commands/dev/browser-mode.ts b/src/cli/commands/dev/browser-mode.ts index c35113e6d..b8593a962 100644 --- a/src/cli/commands/dev/browser-mode.ts +++ b/src/cli/commands/dev/browser-mode.ts @@ -95,6 +95,8 @@ export interface BrowserModeOptions { workingDir: string; project: AgentCoreProjectSpec; port: number; + /** Was --port passed explicitly on the CLI? Controls fail-fast on conflicts (issue #1079). */ + portIsExplicit?: boolean; agentName?: string; /** OTEL env vars to pass to dev servers (set by the dev command when collector is active) */ otelEnvVars?: Record; @@ -123,13 +125,18 @@ export async function launchBrowserDev(): Promise { workingDir, project, port: 8080, + // Intentionally implicit: this entry point is invoked from the TUI + // launcher (no CLI parsing), so there is no user-supplied --port to + // honor literally. Multi-runtime auto-offset behavior applies, matching + // the experience of running `agentcore dev` with no --port flag. + portIsExplicit: false, otelEnvVars, collector, }); } export async function runBrowserMode(opts: BrowserModeOptions): Promise { - const { workingDir, project, agentName, otelEnvVars = {}, collector } = opts; + const { workingDir, project, agentName, port, portIsExplicit, otelEnvVars = {}, collector } = opts; const configRoot = findConfigRoot(workingDir); const { envVars } = await loadDevEnv(workingDir); @@ -170,6 +177,13 @@ export async function runBrowserMode(opts: BrowserModeOptions): Promise { mode: 'dev', agents: agentInfoList, selectedAgent: agentName, + // Only pass basePort when the user explicitly supplied --port. When + // implicit, leaving basePort undefined preserves the legacy + // `uiPort + 1 + index` allocation in handleStart, so HTTP agents + // continue to bind above the web UI's 8081 (avoiding the regression + // where index-1 agents would land on 8081 = web UI port). + basePort: portIsExplicit ? port : undefined, + basePortIsExplicit: portIsExplicit, envVars: mergedEnvVars, getEnvVars: async () => { const { envVars: freshEnvVars } = await loadDevEnv(workingDir); diff --git a/src/cli/commands/dev/command.tsx b/src/cli/commands/dev/command.tsx index eac485d8c..92456edc7 100644 --- a/src/cli/commands/dev/command.tsx +++ b/src/cli/commands/dev/command.tsx @@ -6,7 +6,6 @@ import { callMcpTool, createDevServer, findAvailablePort, - getAgentPort, getDevConfig, getDevSupportedAgents, getEndpointUrl, @@ -16,6 +15,7 @@ import { listMcpTools, loadDevEnv, loadProjectConfig, + resolveAgentPort, } from '../../operations/dev'; import { OtelCollector, startOtelCollector } from '../../operations/dev/otel'; import { FatalError } from '../../tui/components'; @@ -24,6 +24,7 @@ import { COMMAND_DESCRIPTIONS } from '../../tui/copy'; import { requireProject, requireTTY } from '../../tui/guards'; import { parseHeaderFlags } from '../shared/header-utils'; import { runBrowserMode } from './browser-mode'; +import { computeTargetPort, isPortExplicit, resolveDevPort } from './port-resolution'; import type { Command } from '@commander-js/extra-typings'; import { spawn } from 'child_process'; import { Text, render } from 'ink'; @@ -171,7 +172,11 @@ export const registerDev = (program: Command) => { .alias('d') .description(COMMAND_DESCRIPTIONS.dev) .argument('[prompt]', 'Send a prompt to a running dev server [non-interactive]') - .option('-p, --port ', 'Port for development server', '8080') + .option( + '-p, --port ', + 'Base port for development server. With multiple runtimes, the actual port is base + runtime index when --port is omitted; when --port is passed explicitly the value is used literally and a port conflict fails fast.', + '8080' + ) .option('-r, --runtime ', 'Runtime to run or invoke (required if multiple runtimes)') .option('-s, --stream', 'Stream response when invoking [non-interactive]') .option('-l, --logs', 'Run dev server with logs to stdout [non-interactive]') @@ -187,9 +192,16 @@ export const registerDev = (program: Command) => { .option('-b, --no-browser', 'Use terminal TUI instead of web-based chat UI') .option('--no-traces', 'Disable local OTEL trace collection') - .action(async (positionalPrompt: string | undefined, opts) => { + .action(async (positionalPrompt: string | undefined, opts, command) => { try { const port = parseInt(opts.port, 10); + // Detect explicit --port via Commander's getOptionValueSource. See + // ./port-resolution.ts for the rationale and defensive guard. + const portIsExplicit = isPortExplicit(command); + // Note: passing `-p 8080` (matching the registered default) is now + // treated as explicit, which means it disables the runtime-index + // offset and fails fast on conflict. This is the intended behavior of + // issue #1079 and is documented in the --port help text and CHANGELOG. // Parse custom headers let headers: Record | undefined; @@ -227,7 +239,9 @@ export const registerDev = (program: Command) => { let invokePort = port; let targetAgent = invokeProject?.runtimes[0]; if (opts.runtime && invokeProject) { - invokePort = getAgentPort(invokeProject, opts.runtime, port); + // Honor explicit --port literally; otherwise apply the historical + // base + runtime index offset so we hit the auto-allocated port. + invokePort = resolveAgentPort(invokeProject, opts.runtime, port, { explicit: portIsExplicit }).port; targetAgent = invokeProject.runtimes.find(a => a.name === opts.runtime); } else if (invokeProject && invokeProject.runtimes.length > 1 && !opts.runtime) { const names = invokeProject.runtimes.map(a => a.name).join(', '); @@ -325,17 +339,32 @@ export const registerDev = (program: Command) => { // Create logger for log file path const logger = new ExecLogger({ command: 'dev' }); - // Calculate port: A2A/MCP use fixed framework ports, HTTP uses configurable port - const isA2A = config.protocol === 'A2A'; - const isMcp = config.protocol === 'MCP'; - const fixedPort = isA2A ? 9000 : isMcp ? 8000 : getAgentPort(project, config.agentName, port); - const actualPort = await findAvailablePort(fixedPort); - if ((isA2A || isMcp) && actualPort !== fixedPort) { - console.error(`Error: Port ${fixedPort} is in use. ${config.protocol} agents require port ${fixedPort}.`); - process.exit(1); + // Calculate port: A2A/MCP use fixed framework ports, HTTP uses configurable port. + // computeTargetPort + resolveDevPort encapsulate the issue #1079 logic + // (explicit-port honors literally, otherwise base + runtime-index offset + // with a visible log). Unit-tested in __tests__/port-resolution.test.ts. + const { targetPort } = computeTargetPort({ + project, + agentName: config.agentName, + protocol: config.protocol, + basePort: port, + portIsExplicit, + }); + const actualPort = await findAvailablePort(targetPort); + const resolution = resolveDevPort({ + project, + agentName: config.agentName, + protocol: config.protocol, + basePort: port, + portIsExplicit, + availablePort: actualPort, + }); + for (const line of resolution.infoLogs) { + console.log(line); } - if (actualPort !== fixedPort) { - console.log(`Port ${fixedPort} in use, using ${actualPort}`); + if (resolution.conflictError) { + console.error(`Error: ${resolution.conflictError}`); + process.exit(1); } // Get provider info from agent config @@ -403,6 +432,7 @@ export const registerDev = (program: Command) => { }} workingDir={workingDir} port={port} + portIsExplicit={portIsExplicit} agentName={opts.runtime} headers={headers} /> @@ -419,6 +449,7 @@ export const registerDev = (program: Command) => { workingDir, project, port, + portIsExplicit, agentName: opts.runtime, otelEnvVars, collector, diff --git a/src/cli/commands/dev/port-resolution.ts b/src/cli/commands/dev/port-resolution.ts new file mode 100644 index 000000000..f4aafee07 --- /dev/null +++ b/src/cli/commands/dev/port-resolution.ts @@ -0,0 +1,136 @@ +import type { AgentCoreProjectSpec, ProtocolMode } from '../../../schema'; +import { resolveAgentPort } from '../../operations/dev'; +import type { Command } from '@commander-js/extra-typings'; + +/** + * Detect whether --port was passed explicitly on the CLI (or via env/config), + * as opposed to falling back to the registered default. + * + * Commander v14 always provides `command` and `getOptionValueSource` on the + * action callback. The defensive null check is here so a future Commander + * upgrade that removes the API doesn't silently turn every --port value into + * "implicit" (which would re-introduce issue #1079). When the API is missing, + * we emit a warning via `onWarn` so the regression is visible to operators. + * + * Any source other than 'default' counts as explicit so values supplied via + * env/config/CLI are all honored literally. + */ +export function isPortExplicit( + command: Pick | undefined, + onWarn: (message: string) => void = msg => console.warn(msg) +): boolean { + if (!command || typeof command.getOptionValueSource !== 'function') { + onWarn('Warning: Commander command/getOptionValueSource unavailable; --port will be treated as implicit.'); + return false; + } + const source = command.getOptionValueSource('port'); + return source !== undefined && source !== 'default'; +} + +/** Result of {@link computeTargetPort}: where the dev server should *try* to bind. */ +export interface DevTargetPort { + /** The port to pass to `findAvailablePort` before spawning the dev server. */ + targetPort: number; + /** Runtime-index offset applied to basePort (HTTP only; 0 for A2A/MCP). */ + offset: number; + /** Informational log lines to emit before binding (e.g. the offset notice). */ + infoLogs: string[]; +} + +/** Result of {@link resolveDevPort} after `findAvailablePort` has run. */ +export interface DevPortResolution { + /** The port the dev server should bind to. */ + port: number; + /** Runtime-index offset applied to basePort (HTTP only; 0 for A2A/MCP). */ + offset: number; + /** Informational log lines the caller should emit before binding. */ + infoLogs: string[]; + /** If set, the caller should print this error and exit non-zero (port conflict). */ + conflictError?: string; +} + +/** + * Compute the port the dev server should *try* to bind to. Pure, no I/O. + * + * Precedence: + * 1. A2A → 9000, MCP → 8000 (framework-fixed). + * 2. HTTP → `resolveAgentPort` (explicit honors literally; otherwise + * basePort + runtime-index). + * + * Also returns any informational log lines (e.g. the index-offset notice) + * the caller should emit before invoking `findAvailablePort`. + */ +export function computeTargetPort(args: { + project: AgentCoreProjectSpec | null; + agentName: string; + protocol: ProtocolMode; + basePort: number; + portIsExplicit: boolean; +}): DevTargetPort { + const { project, agentName, protocol, basePort, portIsExplicit } = args; + const isA2A = protocol === 'A2A'; + const isMcp = protocol === 'MCP'; + const httpResolution = resolveAgentPort(project, agentName, basePort, { explicit: portIsExplicit }); + const targetPort = isA2A ? 9000 : isMcp ? 8000 : httpResolution.port; + + const infoLogs: string[] = []; + if (!isA2A && !isMcp && httpResolution.offset > 0) { + infoLogs.push( + `Runtime "${agentName}" is at index ${httpResolution.offset}; using port ${targetPort} ` + + `(pass --port ${targetPort} explicitly to override).` + ); + } + + return { targetPort, offset: httpResolution.offset, infoLogs }; +} + +/** + * Pure helper that resolves the dev server's bind port for the non-interactive + * (`--logs`) CLI flow. Composes {@link computeTargetPort} with conflict checks. + * + * Conflict semantics: + * - A2A/MCP: any deviation from the fixed port → conflictError. + * - HTTP + explicit: any deviation from `basePort` → conflictError. + * - HTTP + implicit: silently shifts and emits a "Port X in use, using Y" log. + */ +export function resolveDevPort(args: { + project: AgentCoreProjectSpec | null; + agentName: string; + protocol: ProtocolMode; + basePort: number; + portIsExplicit: boolean; + /** Result from `findAvailablePort(targetPort)`; pass through so this stays pure. */ + availablePort: number; +}): DevPortResolution { + const { protocol, portIsExplicit, availablePort } = args; + const { targetPort, offset, infoLogs } = computeTargetPort(args); + const isA2A = protocol === 'A2A'; + const isMcp = protocol === 'MCP'; + + // Conflict checks + if ((isA2A || isMcp) && availablePort !== targetPort) { + return { + port: targetPort, + offset, + infoLogs, + conflictError: `Port ${targetPort} is in use. ${protocol} agents require port ${targetPort}.`, + }; + } + if (!isA2A && !isMcp && portIsExplicit && availablePort !== targetPort) { + return { + port: targetPort, + offset, + infoLogs, + conflictError: `Port ${targetPort} is in use. Pass a different --port or stop the conflicting process.`, + }; + } + if (availablePort !== targetPort) { + infoLogs.push(`Port ${targetPort} in use, using ${availablePort}`); + } + + return { + port: availablePort, + offset, + infoLogs, + }; +} diff --git a/src/cli/operations/dev/__tests__/config.test.ts b/src/cli/operations/dev/__tests__/config.test.ts index 844ab437c..d00c8bf3a 100644 --- a/src/cli/operations/dev/__tests__/config.test.ts +++ b/src/cli/operations/dev/__tests__/config.test.ts @@ -1,5 +1,5 @@ import type { AgentCoreProjectSpec, DirectoryPath, FilePath } from '../../../../schema'; -import { getAgentPort, getDevConfig, getDevSupportedAgents } from '../config'; +import { getAgentPort, getDevConfig, getDevSupportedAgents, resolveAgentPort } from '../config'; import { describe, expect, it } from 'vitest'; // Helper to cast strings to branded path types for testing @@ -504,6 +504,83 @@ describe('getAgentPort', () => { }); }); +describe('resolveAgentPort', () => { + const makeProject = (...names: string[]): AgentCoreProjectSpec => ({ + name: 'TestProject', + version: 1, + managedBy: 'CDK' as const, + runtimes: names.map(name => ({ + name, + build: 'CodeZip' as const, + runtimeVersion: 'PYTHON_3_12' as const, + entrypoint: filePath('main.py'), + codeLocation: dirPath(`./agents/${name}`), + protocol: 'HTTP' as const, + })), + memories: [], + credentials: [], + evaluators: [], + onlineEvalConfigs: [], + agentCoreGateways: [], + policyEngines: [], + configBundles: [], + abTests: [], + httpGateways: [], + }); + + it('returns basePort with zero offset when project is null', () => { + expect(resolveAgentPort(null, 'any', 8080)).toEqual({ port: 8080, offset: 0 }); + }); + + it('returns basePort + index with offset > 0 when not explicit', () => { + const project = makeProject('A', 'B', 'C'); + expect(resolveAgentPort(project, 'A', 8080)).toEqual({ port: 8080, offset: 0 }); + expect(resolveAgentPort(project, 'B', 8080)).toEqual({ port: 8081, offset: 1 }); + expect(resolveAgentPort(project, 'C', 8080)).toEqual({ port: 8082, offset: 2 }); + }); + + it('honors basePort literally when explicit (no offset, regardless of index)', () => { + const project = makeProject('A', 'B', 'C'); + expect(resolveAgentPort(project, 'A', 8788, { explicit: true })).toEqual({ port: 8788, offset: 0 }); + expect(resolveAgentPort(project, 'B', 8788, { explicit: true })).toEqual({ port: 8788, offset: 0 }); + expect(resolveAgentPort(project, 'C', 8788, { explicit: true })).toEqual({ port: 8788, offset: 0 }); + }); + + it('returns basePort with zero offset when agent is not found', () => { + const project = makeProject('A', 'B'); + expect(resolveAgentPort(project, 'Missing', 9000)).toEqual({ port: 9000, offset: 0 }); + }); + + it('returns basePort with zero offset for empty agent name (locks the index <= 0 guard)', () => { + const project = makeProject('A', 'B'); + // findIndex returns -1, which collapses to offset 0 via the guard. + expect(resolveAgentPort(project, '', 8080)).toEqual({ port: 8080, offset: 0 }); + }); + + it('returns basePort with zero offset when project has zero runtimes', () => { + const project = makeProject(); // no runtimes + expect(resolveAgentPort(project, 'A', 8080)).toEqual({ port: 8080, offset: 0 }); + expect(resolveAgentPort(project, '', 8080)).toEqual({ port: 8080, offset: 0 }); + }); + + it('first runtime (index 0) returns offset 0 (locks index <= 0, not < 0, semantics)', () => { + const project = makeProject('First', 'Second'); + // index === 0 must collapse to offset 0; a regression flipping `<= 0` to + // `< 0` would erroneously return offset = 0 here too (because index is 0, + // and `0 > 0` is false, returning the same result), so this test pairs + // with the AgentB case below. + expect(resolveAgentPort(project, 'First', 8080)).toEqual({ port: 8080, offset: 0 }); + expect(resolveAgentPort(project, 'Second', 8080)).toEqual({ port: 8081, offset: 1 }); + }); + + it('getAgentPort wrapper still returns a number for backwards compat', () => { + const project = makeProject('A', 'B'); + expect(getAgentPort(project, 'A', 8080)).toBe(8080); + expect(getAgentPort(project, 'B', 8080)).toBe(8081); + expect(typeof getAgentPort(project, 'A', 8080)).toBe('number'); + }); +}); + describe('getDevSupportedAgents', () => { it('returns empty array when project is null', () => { expect(getDevSupportedAgents(null)).toEqual([]); diff --git a/src/cli/operations/dev/config.ts b/src/cli/operations/dev/config.ts index fd13637bb..0d2c5a8ab 100644 --- a/src/cli/operations/dev/config.ts +++ b/src/cli/operations/dev/config.ts @@ -79,14 +79,39 @@ export function getDevSupportedAgents(project: AgentCoreProjectSpec | null): Age return project.runtimes.filter(agent => isDevSupported(agent).supported); } +/** + * Resolve the port for a specific agent based on its index in the project. + * + * When `opts.explicit` is true, the user passed `--port` literally on the CLI; + * in that case the basePort is honored as-is (no offset). Otherwise the + * historical behavior is preserved: actual port = basePort + agent index. + * + * Returns both the resolved port and the offset that was applied so callers + * can surface a log line explaining the shift (issue #1079). + */ +export function resolveAgentPort( + project: AgentCoreProjectSpec | null, + agentName: string, + basePort: number, + opts: { explicit?: boolean } = {} +): { port: number; offset: number } { + if (opts.explicit) return { port: basePort, offset: 0 }; + if (!project) return { port: basePort, offset: 0 }; + const index = project.runtimes.findIndex(a => a.name === agentName); + if (index <= 0) return { port: basePort, offset: 0 }; + return { port: basePort + index, offset: index }; +} + /** * Get the port for a specific agent based on its index in the project. - * Base port + agent index = actual port + * Base port + agent index = actual port. + * + * @deprecated Prefer {@link resolveAgentPort} which also returns the offset + * so callers can log the (otherwise silent) port shift. This wrapper is + * kept for backwards compatibility. */ export function getAgentPort(project: AgentCoreProjectSpec | null, agentName: string, basePort: number): number { - if (!project) return basePort; - const index = project.runtimes.findIndex(a => a.name === agentName); - return index >= 0 ? basePort + index : basePort; + return resolveAgentPort(project, agentName, basePort).port; } /** diff --git a/src/cli/operations/dev/index.ts b/src/cli/operations/dev/index.ts index 2522ef21c..9aea84855 100644 --- a/src/cli/operations/dev/index.ts +++ b/src/cli/operations/dev/index.ts @@ -9,7 +9,14 @@ export { type DevServerOptions, } from './server'; -export { getDevConfig, getDevSupportedAgents, getAgentPort, loadProjectConfig, type DevConfig } from './config'; +export { + getDevConfig, + getDevSupportedAgents, + getAgentPort, + resolveAgentPort, + loadProjectConfig, + type DevConfig, +} from './config'; export { ConnectionError, ServerError, invokeAgent, invokeAgentStreaming, invokeForProtocol } from './invoke'; diff --git a/src/cli/operations/dev/web-ui/__tests__/start.test.ts b/src/cli/operations/dev/web-ui/__tests__/start.test.ts new file mode 100644 index 000000000..8016122d6 --- /dev/null +++ b/src/cli/operations/dev/web-ui/__tests__/start.test.ts @@ -0,0 +1,295 @@ +import type { DevConfig } from '../../config'; +import * as serverModule from '../../server'; +import type { RouteContext } from '../handlers/route-context.js'; +import { handleStart } from '../handlers/start.js'; +import type { IncomingMessage, ServerResponse } from 'http'; +import { Readable } from 'node:stream'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +// Mock the server module so we don't actually spawn dev processes or bind ports. +vi.mock('../../server', async () => { + const actual = await vi.importActual('../../server'); + return { + ...actual, + // findAvailablePort: configured per-test via mockImplementation below + findAvailablePort: vi.fn((p: number) => Promise.resolve(p)), + createDevServer: vi.fn(() => ({ + // The handler only null-checks `child`; cast to satisfy the type without + // pretending to satisfy the full ChildProcess contract. + start: vi.fn(() => Promise.resolve({} as never)), + kill: vi.fn(), + })), + }; +}); + +// Also mock waitForServerReady (imported by handlers/start.ts directly from '../../utils'). +// Without this, tests fall through to a real 60s TCP probe loop after createDevServer.start() +// resolves, ballooning the file's runtime to several minutes and creating flakiness if anything +// happens to be listening on the resolved port. +vi.mock('../../utils', async () => { + const actual = await vi.importActual('../../utils'); + return { + ...actual, + waitForServerReady: vi.fn(() => Promise.resolve(true)), + }; +}); + +function mockRes(): ServerResponse & { _status: number; _headers: Record; _body: string } { + const res = { + _status: 0, + _headers: {} as Record, + _body: '', + writeHead(status: number, headers?: Record) { + res._status = status; + if (headers) Object.assign(res._headers, headers); + return res; + }, + setHeader(name: string, value: string) { + res._headers[name] = value; + }, + end(body?: string) { + if (body) res._body = body; + }, + }; + return res as unknown as ServerResponse & { _status: number; _headers: Record; _body: string }; +} + +function mockReq(body: object): IncomingMessage { + const stream = Readable.from([JSON.stringify(body)]); + return Object.assign(stream, { + url: '/api/start', + headers: { host: 'localhost:8081' }, + }) as unknown as IncomingMessage; +} + +function makeDevConfig(overrides: Partial = {}): DevConfig { + return { + agentName: 'AgentB', + module: 'main:app', + directory: '/tmp/agent', + hasConfig: true, + isPython: true, + buildType: 'CodeZip', + protocol: 'HTTP', + ...overrides, + }; +} + +function mockCtx(opts: { + agentNames: string[]; + basePort?: number; + basePortIsExplicit?: boolean; + protocol?: 'HTTP' | 'A2A' | 'MCP'; + onLog?: (level: 'info' | 'warn' | 'error', message: string) => void; +}): RouteContext { + return { + options: { + mode: 'dev', + agents: opts.agentNames.map(name => ({ + name, + buildType: 'CodeZip' as const, + protocol: opts.protocol ?? 'HTTP', + })), + uiPort: 8081, + basePort: opts.basePort, + basePortIsExplicit: opts.basePortIsExplicit, + onLog: opts.onLog, + getDevConfig: (name: string) => makeDevConfig({ agentName: name, protocol: opts.protocol ?? 'HTTP' }), + }, + runningAgents: new Map(), + startingAgents: new Map(), + agentErrors: new Map(), + setCorsHeaders: vi.fn(), + readBody: (req: IncomingMessage) => + new Promise((resolve, reject) => { + const chunks: Buffer[] = []; + req.on('data', (chunk: Buffer | string) => chunks.push(Buffer.from(chunk))); + req.on('end', () => resolve(Buffer.concat(chunks).toString('utf8'))); + req.on('error', reject); + }), + } as unknown as RouteContext; +} + +describe('handleStart - port resolution (issue #1079)', () => { + const findAvailableMock = serverModule.findAvailablePort as unknown as ReturnType; + + beforeEach(() => { + findAvailableMock.mockReset(); + findAvailableMock.mockResolvedValue(0); + findAvailableMock.mockImplementation((p: number) => Promise.resolve(p) as never); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('logs offset when basePort is implicit and agent index > 0', async () => { + const logs: { level: string; msg: string }[] = []; + const ctx = mockCtx({ + agentNames: ['AgentA', 'AgentB'], + basePort: 8080, + basePortIsExplicit: false, + onLog: (level, msg) => logs.push({ level, msg }), + }); + const req = mockReq({ agentName: 'AgentB' }); + const res = mockRes(); + + await handleStart(ctx, req, res); + + // Expect findAvailablePort called with basePort + index = 8081 + expect(findAvailableMock).toHaveBeenCalledWith(8081); + expect(logs.some(l => l.msg.includes('index 1') && l.msg.includes('8081'))).toBe(true); + }); + + it('honors basePort literally when explicit (no offset for index > 0)', async () => { + const logs: { level: string; msg: string }[] = []; + const ctx = mockCtx({ + agentNames: ['AgentA', 'AgentB'], + basePort: 8788, + basePortIsExplicit: true, + onLog: (level, msg) => logs.push({ level, msg }), + }); + const req = mockReq({ agentName: 'AgentB' }); + const res = mockRes(); + + await handleStart(ctx, req, res); + + expect(findAvailableMock).toHaveBeenCalledWith(8788); + expect(logs.some(l => l.msg.includes('index'))).toBe(false); + }); + + it('falls back to uiPort + 1 + index when basePort is undefined', async () => { + const ctx = mockCtx({ + agentNames: ['AgentA', 'AgentB'], + }); + const req = mockReq({ agentName: 'AgentB' }); + const res = mockRes(); + + await handleStart(ctx, req, res); + + // uiPort=8081, +1, +index 1 = 8083 + expect(findAvailableMock).toHaveBeenCalledWith(8083); + }); + + it('returns 500 with explicit-conflict error when basePortIsExplicit and port is in use', async () => { + findAvailableMock.mockImplementationOnce((p: number) => Promise.resolve(p + 5) as never); // simulate conflict + const ctx = mockCtx({ + agentNames: ['AgentA'], + basePort: 8788, + basePortIsExplicit: true, + }); + const req = mockReq({ agentName: 'AgentA' }); + const res = mockRes(); + + await handleStart(ctx, req, res); + + expect(res._status).toBe(500); + const body = JSON.parse(res._body); + expect(body.success).toBe(false); + expect(body.error).toMatch(/8788.*in use/); + }); + + it('explicit-conflict error mentions multi-runtime hint when more than one HTTP agent exists', async () => { + findAvailableMock.mockImplementationOnce((p: number) => Promise.resolve(p + 5) as never); // simulate conflict + const ctx = mockCtx({ + agentNames: ['AgentA', 'AgentB'], + basePort: 8788, + basePortIsExplicit: true, + }); + const req = mockReq({ agentName: 'AgentA' }); + const res = mockRes(); + + await handleStart(ctx, req, res); + + expect(res._status).toBe(500); + const body = JSON.parse(res._body); + expect(body.error).toMatch(/binds all HTTP runtimes to the same port/); + expect(body.error).toMatch(/omit --port to auto-offset/); + }); + + it('ignores basePortIsExplicit when basePort is undefined (programmatic misuse guard)', async () => { + const ctx = mockCtx({ + agentNames: ['AgentA', 'AgentB'], + // basePort intentionally omitted while basePortIsExplicit is set + basePortIsExplicit: true, + }); + const req = mockReq({ agentName: 'AgentB' }); + const res = mockRes(); + + await handleStart(ctx, req, res); + + // Falls back to legacy uiPort+1+index = 8083 (no fail-fast on explicit conflict). + expect(findAvailableMock).toHaveBeenCalledWith(8083); + }); + + it('basePortIsExplicit without basePort + conflict still falls back to legacy log-and-continue (no 500)', async () => { + // Simulate a conflict: findAvailablePort returns a different port. With + // basePortIsExplicit=true but no basePort, the misuse guard must collapse + // back to implicit semantics, which means logging the shift, NOT a 500. + findAvailableMock.mockImplementationOnce((p: number) => Promise.resolve(p + 5) as never); + const logs: { level: string; msg: string }[] = []; + const ctx = mockCtx({ + agentNames: ['AgentA', 'AgentB'], + // basePort intentionally omitted while basePortIsExplicit is set + basePortIsExplicit: true, + onLog: (level, msg) => logs.push({ level, msg }), + }); + const req = mockReq({ agentName: 'AgentB' }); + const res = mockRes(); + + await handleStart(ctx, req, res); + + // Must NOT return the explicit-conflict 500. + expect(res._status).not.toBe(500); + // Must emit the legacy "Port X in use, using Y" log. + expect(logs.some(l => l.msg.includes('Port 8083 in use, using 8088'))).toBe(true); + }); + + it('A2A protocol with basePortIsExplicit=true: no "index N" log; A2A-specific conflict message', async () => { + findAvailableMock.mockImplementationOnce((p: number) => Promise.resolve(p + 1) as never); + const logs: { level: string; msg: string }[] = []; + const ctx = mockCtx({ + agentNames: ['AgentA', 'AgentB'], + basePort: 8080, + basePortIsExplicit: true, + protocol: 'A2A', + onLog: (level, msg) => logs.push({ level, msg }), + }); + const req = mockReq({ agentName: 'AgentB' }); + const res = mockRes(); + + await handleStart(ctx, req, res); + + // A2A pin to 9000 regardless of basePort/index. + expect(findAvailableMock).toHaveBeenCalledWith(9000); + // No HTTP-only "index" offset log should fire for A2A agents. + expect(logs.every(l => !l.msg.includes('index'))).toBe(true); + // Conflict surfaces the A2A-specific message, not the HTTP "Pass a different --port" one. + const body = JSON.parse(res._body); + expect(body.error).toMatch(/A2A agents require port 9000/); + expect(body.error).not.toMatch(/Pass a different --port/); + }); + + it('MCP protocol with basePortIsExplicit=true: no "index N" log; MCP-specific conflict message', async () => { + findAvailableMock.mockImplementationOnce((p: number) => Promise.resolve(p + 1) as never); + const logs: { level: string; msg: string }[] = []; + const ctx = mockCtx({ + agentNames: ['AgentA', 'AgentB'], + basePort: 8080, + basePortIsExplicit: true, + protocol: 'MCP', + onLog: (level, msg) => logs.push({ level, msg }), + }); + const req = mockReq({ agentName: 'AgentB' }); + const res = mockRes(); + + await handleStart(ctx, req, res); + + // MCP pinned to 8000 regardless of basePort/index. + expect(findAvailableMock).toHaveBeenCalledWith(8000); + expect(logs.every(l => !l.msg.includes('index'))).toBe(true); + const body = JSON.parse(res._body); + expect(body.error).toMatch(/MCP agents require port 8000/); + expect(body.error).not.toMatch(/Pass a different --port/); + }); +}); diff --git a/src/cli/operations/dev/web-ui/handlers/start.ts b/src/cli/operations/dev/web-ui/handlers/start.ts index 41857bda1..97d76cf9d 100644 --- a/src/cli/operations/dev/web-ui/handlers/start.ts +++ b/src/cli/operations/dev/web-ui/handlers/start.ts @@ -98,7 +98,39 @@ async function doStartAgent( // we set. So MCP agents always bind to 8000 regardless of environment configuration. const isA2A = config.protocol === 'A2A'; const isMCP = config.protocol === 'MCP'; - const targetPort = isA2A ? 9000 : isMCP ? 8000 : ctx.options.uiPort + 1 + (agentIndex >= 0 ? agentIndex : 0); + // Resolve HTTP target port. Precedence: + // 1. If basePort was explicitly passed via --port: use it literally (no offset). + // 2. If basePort was provided (default): basePort + agentIndex. + // 3. Otherwise: uiPort + 1 + agentIndex (legacy auto-allocation above the UI port). + // basePortIsExplicit is only meaningful when basePort is set (the two are paired + // by command.tsx; guarding here keeps programmatic misuse from triggering the + // confusing combination of "fail-fast + legacy uiPort+1+index"). + const safeIndex = agentIndex >= 0 ? agentIndex : 0; + const basePort = ctx.options.basePort; + const basePortIsExplicit = ctx.options.basePortIsExplicit === true && basePort !== undefined; + let httpTargetPort: number; + let offset: number; + if (basePort !== undefined && basePortIsExplicit) { + httpTargetPort = basePort; + offset = 0; + } else if (basePort !== undefined) { + httpTargetPort = basePort + safeIndex; + offset = safeIndex; + } else { + httpTargetPort = ctx.options.uiPort + 1 + safeIndex; + offset = safeIndex; + } + const targetPort = isA2A ? 9000 : isMCP ? 8000 : httpTargetPort; + + // Surface the index-based offset so it isn't silent (issue #1079). + if (!isA2A && !isMCP && offset > 0) { + onLog?.( + 'info', + `[${agentName}] Runtime is at index ${offset}; using port ${targetPort} ` + + `(pass --port ${targetPort} explicitly to override).` + ); + } + const agentPort = await findAvailablePort(targetPort); if (isA2A && agentPort !== 9000) { return { @@ -116,6 +148,22 @@ async function doStartAgent( error: `Port 8000 is in use. MCP agents require port 8000 (FastMCP default).`, }; } + if (!isA2A && !isMCP && basePortIsExplicit && agentPort !== targetPort) { + // When --port is explicit, all HTTP runtimes bind to the same literal port, + // so only the first can succeed. Surface this clearly when there is more + // than one HTTP runtime configured. + const httpAgentCount = ctx.options.agents.filter(a => a.protocol !== 'A2A' && a.protocol !== 'MCP').length; + const multiRuntimeHint = + httpAgentCount > 1 + ? ' Explicit --port binds all HTTP runtimes to the same port; omit --port to auto-offset per runtime, or stop the conflicting process.' + : ' Pass a different --port or stop the conflicting process.'; + return { + success: false, + name: agentName, + port: 0, + error: `Port ${targetPort} is in use.${multiRuntimeHint}`, + }; + } if (agentPort !== targetPort) { onLog?.('info', `[${agentName}] Port ${targetPort} in use, using ${agentPort}`); } diff --git a/src/cli/operations/dev/web-ui/web-server.ts b/src/cli/operations/dev/web-ui/web-server.ts index 2b20b2d07..c95e8b9a9 100644 --- a/src/cli/operations/dev/web-ui/web-server.ts +++ b/src/cli/operations/dev/web-ui/web-server.ts @@ -131,6 +131,26 @@ export interface WebUIOptions { uiPort: number; /** Available agents (metadata only — servers are started on demand) */ agents: AgentInfo[]; + /** + * Base port for agent dev servers. When provided, agents bind at + * `basePort + agentIndex` (or literally at `basePort` if `basePortIsExplicit`). + * When omitted, falls back to `uiPort + 1 + agentIndex`. + * + * NOTE for external callers: pair this with `basePortIsExplicit` so the + * caller's intent is unambiguous. Passing `basePort` alone preserves the + * historical auto-offset behavior — which is exactly the silent shift + * issue #1079 fixed in CLI flows. If you want the literal value honored + * (and conflicts to fail fast), set `basePortIsExplicit: true`. + */ + basePort?: number; + /** + * Was the base port explicitly passed by the user (e.g. via CLI `--port`)? + * When true, the port is honored literally and a port conflict fails fast + * instead of being silently shifted by the runtime index (issue #1079). + * + * Only meaningful when `basePort` is also set; ignored otherwise. + */ + basePortIsExplicit?: boolean; /** Dev config factory — called when an agent needs to be started. Required for dev mode, unused when onStart is provided. */ getDevConfig?: (agentName: string) => DevConfig | null | Promise; /** Env vars to pass to started agent servers */ diff --git a/src/cli/tui/hooks/useDevServer.ts b/src/cli/tui/hooks/useDevServer.ts index a580b8477..7e15ef35e 100644 --- a/src/cli/tui/hooks/useDevServer.ts +++ b/src/cli/tui/hooks/useDevServer.ts @@ -22,6 +22,7 @@ import { listMcpTools, loadDevEnv, loadProjectConfig, + resolveAgentPort, waitForPort, } from '../../operations/dev'; import { formatMcpToolList } from '../../operations/dev/utils'; @@ -48,6 +49,8 @@ const MAX_LOG_ENTRIES = 50; export function useDevServer(options: { workingDir: string; port: number; + /** Was --port passed explicitly on the CLI? Controls fail-fast on conflicts (issue #1079). */ + portIsExplicit?: boolean; agentName?: string; onReady?: () => void; headers?: Record; @@ -154,7 +157,18 @@ export function useDevServer(options: { // A2A servers always use port 9000, MCP servers use port 8000 (framework defaults, not configurable via env) const isA2A = config.protocol === 'A2A'; const isMcp = config.protocol === 'MCP'; - const fixedPort = isA2A ? 9000 : isMcp ? 8000 : targetPort; + // Resolve HTTP port: honor --port literally when explicit, otherwise apply + // historical base + runtime-index offset and surface it via a log line (issue #1079). + const httpResolution = resolveAgentPort(project, config.agentName, targetPort, { + explicit: options.portIsExplicit, + }); + const fixedPort = isA2A ? 9000 : isMcp ? 8000 : httpResolution.port; + if (!isA2A && !isMcp && httpResolution.offset > 0) { + addLog( + 'info', + `Runtime "${config.agentName}" is at index ${httpResolution.offset}; using port ${fixedPort} (pass --port ${fixedPort} explicitly to override).` + ); + } // On restart, reuse the same port. On initial start, find an available port. // If restart times out waiting for port, fall back to finding a new one. @@ -177,6 +191,36 @@ export function useDevServer(options: { return; } port = fixedPort; + } else if (options.portIsExplicit) { + // User passed --port explicitly: honor it literally and fail fast on + // conflict. Always probe regardless of restart state so a port stolen + // by another process between waitForPort() and the spawn is caught + // here with a clear message rather than surfacing as an opaque + // EADDRINUSE from the dev server. + const available = await findAvailablePort(fixedPort); + if (available !== fixedPort) { + // On initial start (or when the previous instance still holds the + // port), this is a hard conflict — fail fast. + if (!isRestart) { + addLog('error', `Port ${fixedPort} is in use. Pass a different --port or stop the conflicting process.`); + setStatus('error'); + return; + } + // On restart with the previous instance still holding the port, + // emit a clearer message than the generic conflict text. + addLog( + 'error', + `Port ${fixedPort} still held by previous instance. ` + + `Stop the conflicting process or pass a different --port.` + ); + setStatus('error'); + return; + } + // On restart where the prior port has been released and reuse is + // possible, keep using actualPortRef.current to preserve session + // continuity. Otherwise bind to the explicit port we just verified + // is free. + port = isRestart && portFree ? actualPortRef.current : fixedPort; } else { port = isRestart && portFree ? actualPortRef.current : await findAvailablePort(fixedPort); if (!isRestart && port !== fixedPort) { diff --git a/src/cli/tui/screens/dev/DevScreen.tsx b/src/cli/tui/screens/dev/DevScreen.tsx index 0040aaf30..bf7207b9a 100644 --- a/src/cli/tui/screens/dev/DevScreen.tsx +++ b/src/cli/tui/screens/dev/DevScreen.tsx @@ -11,6 +11,8 @@ interface DevScreenProps { onBack: () => void; workingDir?: string; port?: number; + /** Was --port passed explicitly on the CLI? Controls fail-fast on conflicts (issue #1079). */ + portIsExplicit?: boolean; /** Pre-selected agent name (from CLI --agent flag) */ agentName?: string; /** Custom headers to forward to the agent on every invocation */ @@ -203,6 +205,7 @@ export function DevScreen(props: DevScreenProps) { } = useDevServer({ workingDir, port: props.port ?? 8080, + portIsExplicit: props.portIsExplicit, agentName: selectedAgentName, onReady: onServerReady, headers: props.headers,