From 6681bd8e87254c8d8797e0799203f19ffc60e5f3 Mon Sep 17 00:00:00 2001 From: Harrison Weinstock Date: Mon, 25 May 2026 17:29:24 +0000 Subject: [PATCH 1/7] feat(telemetry): allow dynamic attribute merging --- src/cli/telemetry/__tests__/client.test.ts | 109 +++++++++++++++++++++ src/cli/telemetry/cli-command-run.ts | 46 +++++---- 2 files changed, 134 insertions(+), 21 deletions(-) diff --git a/src/cli/telemetry/__tests__/client.test.ts b/src/cli/telemetry/__tests__/client.test.ts index 949ae80ac..e0dd59e79 100644 --- a/src/cli/telemetry/__tests__/client.test.ts +++ b/src/cli/telemetry/__tests__/client.test.ts @@ -166,4 +166,113 @@ describe('withCommandRunTelemetry', () => { error_name: 'UnknownError', }); }); + + describe('_telemetryAttrs', () => { + it('returned _telemetryAttrs override upfront attrs on success', async () => { + await withCommandRunTelemetry( + 'dev', + { + dev_action: 'server', + ui_mode: 'terminal', + has_stream: false, + agent_protocol: 'http', + invoke_count: 0, + }, + async () => ({ + success: true as const, + _telemetryAttrs: { + dev_action: 'invoke', + ui_mode: 'browser', + has_stream: true, + agent_protocol: 'a2a', + invoke_count: 5, + }, + }) + ); + + expect(sink.metrics).toHaveLength(1); + expect(sink.metrics[0]!.attrs).toMatchObject({ + dev_action: 'invoke', + ui_mode: 'browser', + has_stream: 'true', + agent_protocol: 'a2a', + invoke_count: 5, + }); + }); + + it('returned _telemetryAttrs override upfront attrs on failure result', async () => { + await withCommandRunTelemetry( + 'dev', + { + dev_action: 'server', + ui_mode: 'terminal', + has_stream: false, + agent_protocol: 'http', + invoke_count: 0, + }, + async () => ({ + success: false as const, + error: new Error('port in use'), + _telemetryAttrs: { + dev_action: 'server', + ui_mode: 'terminal', + has_stream: false, + agent_protocol: 'mcp', + invoke_count: 0, + }, + }) + ); + + expect(sink.metrics).toHaveLength(1); + expect(sink.metrics[0]!.attrs).toMatchObject({ + exit_reason: 'failure', + agent_protocol: 'mcp', + }); + }); + + it('falls back to upfront attrs when _telemetryAttrs is not returned', async () => { + await withCommandRunTelemetry( + 'dev', + { + dev_action: 'server', + ui_mode: 'terminal', + has_stream: false, + agent_protocol: 'http', + invoke_count: 0, + }, + async () => ({ success: true as const }) + ); + + expect(sink.metrics).toHaveLength(1); + expect(sink.metrics[0]!.attrs).toMatchObject({ + agent_protocol: 'http', + dev_action: 'server', + }); + }); + + it('does not leak _telemetryAttrs to the caller', async () => { + const result = await withCommandRunTelemetry( + 'dev', + { + dev_action: 'server', + ui_mode: 'terminal', + has_stream: false, + agent_protocol: 'http', + invoke_count: 0, + }, + async () => ({ + success: true as const, + _telemetryAttrs: { + dev_action: 'server', + ui_mode: 'terminal', + has_stream: false, + agent_protocol: 'mcp', + invoke_count: 0, + }, + }) + ); + + expect('_telemetryAttrs' in result).toBe(false); + }); + }); }); diff --git a/src/cli/telemetry/cli-command-run.ts b/src/cli/telemetry/cli-command-run.ts index 0ed290027..73156b505 100644 --- a/src/cli/telemetry/cli-command-run.ts +++ b/src/cli/telemetry/cli-command-run.ts @@ -78,37 +78,41 @@ async function trackCommandRun( * is returned to the caller. If the callback throws, telemetry is recorded and * the exception is converted to a result type such that callers do not need to handle result + try/catch. * If telemetry is unavailable, the callback runs untracked. + * + * The callback may return `_telemetryAttrs` to override or supplement the upfront `attrs`. + * Returned attrs are merged with the upfront attrs (returned takes priority). + * On throw, only the upfront attrs are used since the callback never produced a value. */ export async function withCommandRunTelemetry( command: C, attrs: CommandAttrs, - fn: () => R | Promise + fn: () => (R & { _telemetryAttrs?: CommandAttrs }) | Promise }> ): Promise { const client = await getTelemetryClient(); - - let result: R | undefined; + const start = performance.now(); try { - if (!client) return fn(); - await trackCommandRun( - client, - command, - async () => { - result = await fn(); - if (!result.success) throw result.error; - return attrs; - }, - attrs - ); + const result = await fn(); + if (client) { + const merged = result._telemetryAttrs ? { ...attrs, ...result._telemetryAttrs } : attrs; + const durationMs = Math.round(performance.now() - start); + if (!result.success) { + const { category, source } = classifyError(result.error); + recordCommandRun(client, command, { exit_reason: 'failure', error_name: category, error_source: source }, merged, durationMs); + } else { + recordCommandRun(client, command, { exit_reason: 'success' }, merged, durationMs); + } + } + const { _telemetryAttrs, ...cleanResult } = result; + return cleanResult as R; } catch (e) { - // trackCommandRun re-throws after recording failure telemetry. - // If result was set, fn() returned a failure result — return it directly. - // If not, fn() itself threw — convert to a failure result so callers - // that don't wrap in try/catch (e.g. TUI hooks) don't leak unhandled rejections. - if (!result) { - return { success: false, error: e instanceof Error ? e : new Error(getErrorMessage(e)) } as R; + if (client) { + const { category, source } = classifyError(e); + recordCommandRun(client, command, { exit_reason: 'failure', error_name: category, error_source: source }, attrs, Math.round(performance.now() - start)); } + return { success: false, error: e instanceof Error ? e : new Error(getErrorMessage(e)) } as R; + } finally { + await client?.flush(); } - return result!; } /** From dc2d620177373ca07ec10e90e5a9627faa901089 Mon Sep 17 00:00:00 2001 From: Harrison Weinstock Date: Mon, 25 May 2026 17:50:18 +0000 Subject: [PATCH 2/7] refactor: avoid killing the process on validation errors --- src/cli/commands/dev/command.tsx | 40 ++++++++++++++------------------ 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/src/cli/commands/dev/command.tsx b/src/cli/commands/dev/command.tsx index 0f67615fc..1d5f71a98 100644 --- a/src/cli/commands/dev/command.tsx +++ b/src/cli/commands/dev/command.tsx @@ -1,5 +1,6 @@ import { ConnectionError, + NoProjectError, ResourceNotFoundError, type Result, ValidationError, @@ -28,7 +29,6 @@ import { OtelCollector, startOtelCollector } from '../../operations/dev/otel'; import { withCommandRunTelemetry } from '../../telemetry/cli-command-run.js'; import { TelemetryClientAccessor } from '../../telemetry/client-accessor.js'; import { AgentProtocol, standardize } from '../../telemetry/schemas/common-shapes.js'; -import { FatalError } from '../../tui/components'; import { LayoutProvider } from '../../tui/context'; import { COMMAND_DESCRIPTIONS } from '../../tui/copy'; import { requireProject, requireTTY } from '../../tui/guards'; @@ -198,17 +198,16 @@ export const registerDev = (program: Command) => { // Exec mode: run shell command in the dev container if (opts.exec) { if (!positionalPrompt) { - console.error('A command is required with --exec. Usage: agentcore dev --exec "whoami"'); - process.exit(1); + throw new ValidationError('A command is required with --exec. Usage: agentcore dev --exec "whoami"'); } const workingDir = getWorkingDirectory(); const project = await loadProjectConfig(workingDir); const agentName = opts.runtime ?? project?.runtimes[0]?.name ?? 'unknown'; const targetAgent = project?.runtimes.find(a => a.name === agentName); if (targetAgent?.build !== 'Container') { - console.error('Error: --exec is only supported for Container build agents.'); - console.error('For CodeZip agents, use your terminal to run commands directly.'); - process.exit(1); + throw new ValidationError( + '--exec is only supported for Container build agents. For CodeZip agents, use your terminal to run commands directly.' + ); } const containerName = `agentcore-dev-${agentName}`.toLowerCase(); const execResult = await withCommandRunTelemetry( @@ -243,9 +242,9 @@ export const registerDev = (program: Command) => { 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(', '); - console.error(`Error: Multiple runtimes found. Use --runtime to specify which one.`); - console.error(`Available: ${names}`); - process.exit(1); + throw new ValidationError( + `Multiple runtimes found. Use --runtime to specify which one. Available: ${names}` + ); } const protocol = targetAgent?.protocol ?? 'HTTP'; @@ -294,13 +293,11 @@ export const registerDev = (program: Command) => { const project = await loadProjectConfig(workingDir); if (!project) { - render(); - process.exit(1); + throw new NoProjectError(); } if (!project.runtimes || project.runtimes.length === 0) { - render(); - process.exit(1); + throw new ValidationError('No agents defined in project. Run `agentcore add agent` to fix this.'); } // Warn about VPC mode limitations in local dev @@ -313,8 +310,7 @@ export const registerDev = (program: Command) => { const supportedAgents = getDevSupportedAgents(project); if (supportedAgents.length === 0) { - render(); - process.exit(1); + throw new ValidationError('No agents support dev mode. Dev mode requires an agent with an entrypoint.'); } // Start local OTEL collector so agent traces are captured in dev mode. @@ -335,9 +331,9 @@ export const registerDev = (program: Command) => { // Require --agent if multiple agents if (project.runtimes.length > 1 && !opts.runtime) { const names = project.runtimes.map(a => a.name).join(', '); - console.error(`Error: Multiple runtimes found. Use --runtime to specify which one.`); - console.error(`Available: ${names}`); - process.exit(1); + throw new ValidationError( + `Multiple runtimes found. Use --runtime to specify which one. Available: ${names}` + ); } const agentName = opts.runtime ?? project.runtimes[0]?.name; @@ -346,8 +342,7 @@ export const registerDev = (program: Command) => { const config = getDevConfig(workingDir, project, configRoot ?? undefined, agentName); if (!config) { - console.error('Error: No dev-supported agents found.'); - process.exit(1); + throw new ValidationError('No dev-supported agents found.'); } // Create logger for log file path @@ -359,8 +354,9 @@ export const registerDev = (program: Command) => { 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); + throw new ValidationError( + `Port ${fixedPort} is in use. ${config.protocol} agents require port ${fixedPort}.` + ); } if (actualPort !== fixedPort) { console.log(`Port ${fixedPort} in use, using ${actualPort}`); From 5e1bc2d8c823d1a6c9af041acf98cad89459ec20 Mon Sep 17 00:00:00 2001 From: Harrison Weinstock Date: Mon, 25 May 2026 19:04:03 +0000 Subject: [PATCH 3/7] fix(telemetry): capture dev validation failures by wrapping at top-level --- integ-tests/dev-server.test.ts | 36 ++- src/cli/commands/dev/command.tsx | 394 ++++++++++++++++--------------- 2 files changed, 234 insertions(+), 196 deletions(-) diff --git a/integ-tests/dev-server.test.ts b/integ-tests/dev-server.test.ts index b1608f6c0..df07b7a42 100644 --- a/integ-tests/dev-server.test.ts +++ b/integ-tests/dev-server.test.ts @@ -88,7 +88,7 @@ describe('integration: dev server', () => { }); it.skipIf(!hasNpm || !hasGit || !hasUv)( - 'starts dev server and responds to health check', + 'starts dev server, responds to health check, and emits telemetry', async () => { expect(projectPath, 'Project should have been created').toBeTruthy(); @@ -98,12 +98,21 @@ describe('integration: dev server', () => { devProcess = spawn('node', [cliPath, 'dev', '--port', String(port), '--logs'], { cwd: projectPath, stdio: 'pipe', - env: { ...process.env, INIT_CWD: undefined }, + env: { ...process.env, INIT_CWD: undefined, ...telemetry.env }, }); const serverReady = await waitForServer(port, 20000); expect(serverReady, 'Dev server should respond to ping within 20s').toBeTruthy(); + // Verify telemetry was emitted for the server startup (before blocking) + telemetry.assertMetricEmitted({ + command: 'dev', + dev_action: 'server', + ui_mode: 'terminal', + exit_reason: 'success', + }); + telemetry.clearEntries(); + // Invoke the running server and verify telemetry const invokeResult = await runCLI(['dev', 'hello', '--port', String(port)], projectPath, { env: telemetry.env, @@ -135,4 +144,27 @@ describe('integration: dev server', () => { }, 30000 ); + + it.skipIf(!hasNpm || !hasGit || !hasUv)( + 'exits with error when runtime not found and emits failure telemetry', + async () => { + expect(projectPath, 'Project should have been created').toBeTruthy(); + + telemetry.clearEntries(); + const result = await runCLI(['dev', '--logs', '--runtime', 'nonexistent-agent'], projectPath, { + env: telemetry.env, + }); + + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain('nonexistent-agent'); + expect(result.stderr).toContain('not found'); + + telemetry.assertMetricEmitted({ + command: 'dev', + dev_action: 'server', + exit_reason: 'failure', + }); + }, + 15000 + ); }); diff --git a/src/cli/commands/dev/command.tsx b/src/cli/commands/dev/command.tsx index 1d5f71a98..2ea2af85b 100644 --- a/src/cli/commands/dev/command.tsx +++ b/src/cli/commands/dev/command.tsx @@ -2,7 +2,6 @@ import { ConnectionError, NoProjectError, ResourceNotFoundError, - type Result, ValidationError, findConfigRoot, getWorkingDirectory, @@ -27,11 +26,10 @@ import { } from '../../operations/dev'; import { OtelCollector, startOtelCollector } from '../../operations/dev/otel'; import { withCommandRunTelemetry } from '../../telemetry/cli-command-run.js'; -import { TelemetryClientAccessor } from '../../telemetry/client-accessor.js'; import { AgentProtocol, standardize } from '../../telemetry/schemas/common-shapes.js'; import { LayoutProvider } from '../../tui/context'; import { COMMAND_DESCRIPTIONS } from '../../tui/copy'; -import { requireProject, requireTTY } from '../../tui/guards'; +import { requireTTY } from '../../tui/guards'; import { parseHeaderFlags } from '../shared/header-utils'; import { runBrowserMode } from './browser-mode'; import type { Command } from '@commander-js/extra-typings'; @@ -197,73 +195,80 @@ export const registerDev = (program: Command) => { // Exec mode: run shell command in the dev container if (opts.exec) { - if (!positionalPrompt) { - throw new ValidationError('A command is required with --exec. Usage: agentcore dev --exec "whoami"'); - } - const workingDir = getWorkingDirectory(); - const project = await loadProjectConfig(workingDir); - const agentName = opts.runtime ?? project?.runtimes[0]?.name ?? 'unknown'; - const targetAgent = project?.runtimes.find(a => a.name === agentName); - if (targetAgent?.build !== 'Container') { - throw new ValidationError( - '--exec is only supported for Container build agents. For CodeZip agents, use your terminal to run commands directly.' - ); - } - const containerName = `agentcore-dev-${agentName}`.toLowerCase(); const execResult = await withCommandRunTelemetry( 'dev', { dev_action: 'exec' as const, ui_mode: 'terminal' as const, has_stream: false, - agent_protocol: standardize(AgentProtocol, (targetAgent?.protocol ?? 'http').toLowerCase()), + agent_protocol: 'http' as const, invoke_count: 0, }, - async (): Promise => { + async () => { + if (!positionalPrompt) { + throw new ValidationError('A command is required with --exec. Usage: agentcore dev --exec "whoami"'); + } + const workingDir = getWorkingDirectory(); + const project = await loadProjectConfig(workingDir); + const agentName = opts.runtime ?? project?.runtimes[0]?.name ?? 'unknown'; + const targetAgent = project?.runtimes.find(a => a.name === agentName); + if (targetAgent?.build !== 'Container') { + throw new ValidationError( + '--exec is only supported for Container build agents. For CodeZip agents, use your terminal to run commands directly.' + ); + } + const containerName = `agentcore-dev-${agentName}`.toLowerCase(); await execInContainer(positionalPrompt, containerName); - return { success: true }; + return { + success: true as const, + _telemetryAttrs: { + dev_action: 'exec' as const, + ui_mode: 'terminal' as const, + has_stream: false, + agent_protocol: standardize(AgentProtocol, (targetAgent?.protocol ?? 'http').toLowerCase()), + invoke_count: 0, + }, + }; } ); - if (!execResult.success) throw execResult.error; + // TODO: Remove cast once withCommandRunTelemetry's return type is narrowed + if (!execResult.success) throw (execResult as unknown as { error: Error }).error; return; } // If a prompt is provided, invoke a running dev server const invokePrompt = positionalPrompt; if (invokePrompt !== undefined) { - const workingDir = getWorkingDirectory(); - const invokeProject = await loadProjectConfig(workingDir); - - // Determine which agent/port to invoke - let invokePort = port; - let targetAgent = invokeProject?.runtimes[0]; - if (opts.runtime && invokeProject) { - invokePort = getAgentPort(invokeProject, opts.runtime, 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(', '); - throw new ValidationError( - `Multiple runtimes found. Use --runtime to specify which one. Available: ${names}` - ); - } - - const protocol = targetAgent?.protocol ?? 'HTTP'; - - // Override port for protocols with fixed framework ports - if (protocol === 'A2A') invokePort = 9000; - else if (protocol === 'MCP') invokePort = 8000; - const invokeResult = await withCommandRunTelemetry( 'dev', { dev_action: 'invoke' as const, ui_mode: 'terminal' as const, has_stream: opts.stream ?? false, - agent_protocol: standardize(AgentProtocol, protocol.toLowerCase()), + agent_protocol: 'http' as const, invoke_count: 1, }, - async (): Promise => { - // Protocol-aware dispatch + async () => { + const workingDir = getWorkingDirectory(); + const invokeProject = await loadProjectConfig(workingDir); + + let invokePort = port; + let targetAgent = invokeProject?.runtimes[0]; + if (opts.runtime && invokeProject) { + invokePort = getAgentPort(invokeProject, opts.runtime, 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(', '); + throw new ValidationError( + `Multiple runtimes found. Use --runtime to specify which one. Available: ${names}` + ); + } + + const protocol = targetAgent?.protocol ?? 'HTTP'; + + if (protocol === 'A2A') invokePort = 9000; + else if (protocol === 'MCP') invokePort = 8000; + if (protocol === 'MCP') { await handleMcpInvoke(invokePort, invokePrompt, opts.tool, opts.input, headers); } else if (protocol === 'A2A') { @@ -280,114 +285,116 @@ export const registerDev = (program: Command) => { } else { await invokeDevServer(invokePort, invokePrompt, opts.stream ?? false, headers); } - return { success: true }; + + return { + success: true as const, + _telemetryAttrs: { + dev_action: 'invoke' as const, + ui_mode: 'terminal' as const, + has_stream: opts.stream ?? false, + agent_protocol: standardize(AgentProtocol, protocol.toLowerCase()), + invoke_count: 1, + }, + }; } ); - if (!invokeResult.success) throw invokeResult.error; + // TODO: Remove cast once withCommandRunTelemetry's return type is narrowed + if (!invokeResult.success) throw (invokeResult as unknown as { error: Error }).error; return; } - requireProject(); - const workingDir = getWorkingDirectory(); - const project = await loadProjectConfig(workingDir); - if (!project) { - throw new NoProjectError(); - } + const serverResult = await withCommandRunTelemetry( + 'dev', + { + dev_action: 'server' as const, + ui_mode: 'terminal' as const, + has_stream: false, + agent_protocol: 'http' as const, + invoke_count: 0, + }, + async () => { + const project = await loadProjectConfig(workingDir); + if (!project) { + throw new NoProjectError(); + } + if (!project.runtimes || project.runtimes.length === 0) { + throw new ValidationError('No agents defined in project. Run `agentcore add agent` to fix this.'); + } - if (!project.runtimes || project.runtimes.length === 0) { - throw new ValidationError('No agents defined in project. Run `agentcore add agent` to fix this.'); - } + const targetDevAgent = opts.runtime + ? project.runtimes.find(a => a.name === opts.runtime) + : project.runtimes[0]; + if (targetDevAgent?.networkMode === 'VPC') { + console.log( + '\x1b[33mWarning: This agent uses VPC network mode. Local dev server runs outside your VPC. Network behavior may differ from deployed environment.\x1b[0m\n' + ); + } - // Warn about VPC mode limitations in local dev - const targetDevAgent = opts.runtime ? project.runtimes.find(a => a.name === opts.runtime) : project.runtimes[0]; - if (targetDevAgent?.networkMode === 'VPC') { - console.log( - '\x1b[33mWarning: This agent uses VPC network mode. Local dev server runs outside your VPC. Network behavior may differ from deployed environment.\x1b[0m\n' - ); - } + const supportedAgents = getDevSupportedAgents(project); + if (supportedAgents.length === 0) { + throw new ValidationError('No agents support dev mode. Dev mode requires an agent with an entrypoint.'); + } - const supportedAgents = getDevSupportedAgents(project); - if (supportedAgents.length === 0) { - throw new ValidationError('No agents support dev mode. Dev mode requires an agent with an entrypoint.'); - } + const configRoot = findConfigRoot(workingDir); + let otelEnvVars: Record = {}; + let collector: OtelCollector | undefined; - // Start local OTEL collector so agent traces are captured in dev mode. - // Persists traces to .cli/traces/ so they survive dev server restarts. - const configRoot = findConfigRoot(workingDir); - let otelEnvVars: Record = {}; - let collector: OtelCollector | undefined; - - if (opts.traces !== false) { - const persistTracesDir = path.join(configRoot ?? workingDir, '.cli', 'traces'); - const otelResult = await startOtelCollector(persistTracesDir); - collector = otelResult.collector; - otelEnvVars = otelResult.otelEnvVars; - } + if (opts.traces !== false) { + const persistTracesDir = path.join(configRoot ?? workingDir, '.cli', 'traces'); + const otelResult = await startOtelCollector(persistTracesDir); + collector = otelResult.collector; + otelEnvVars = otelResult.otelEnvVars; + } - // If --logs provided, run non-interactive mode - if (opts.logs) { - // Require --agent if multiple agents - if (project.runtimes.length > 1 && !opts.runtime) { - const names = project.runtimes.map(a => a.name).join(', '); - throw new ValidationError( - `Multiple runtimes found. Use --runtime to specify which one. Available: ${names}` - ); - } + // --logs: non-interactive server mode + if (opts.logs) { + if (project.runtimes.length > 1 && !opts.runtime) { + const names = project.runtimes.map(a => a.name).join(', '); + throw new ValidationError( + `Multiple runtimes found. Use --runtime to specify which one. Available: ${names}` + ); + } - const agentName = opts.runtime ?? project.runtimes[0]?.name; - const { envVars } = await loadDevEnv(workingDir); - const mergedEnvVars = { ...envVars, ...otelEnvVars }; - const config = getDevConfig(workingDir, project, configRoot ?? undefined, agentName); + const agentName = opts.runtime ?? project.runtimes[0]?.name; + const { envVars } = await loadDevEnv(workingDir); + const mergedEnvVars = { ...envVars, ...otelEnvVars }; + const config = getDevConfig(workingDir, project, configRoot ?? undefined, agentName); - if (!config) { - throw new ValidationError('No dev-supported agents found.'); - } + if (!config) { + throw new ValidationError('No dev-supported agents found.'); + } - // 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) { - throw new ValidationError( - `Port ${fixedPort} is in use. ${config.protocol} agents require port ${fixedPort}.` - ); - } - if (actualPort !== fixedPort) { - console.log(`Port ${fixedPort} in use, using ${actualPort}`); - } + 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) { + throw new ValidationError( + `Port ${fixedPort} is in use. ${config.protocol} agents require port ${fixedPort}.` + ); + } - // Get provider info from agent config - const providerInfo = '(see agent code)'; + const logger = new ExecLogger({ command: 'dev' }); - console.log(`Starting dev server...`); - console.log(`Agent: ${config.agentName}`); - if (config.protocol !== 'MCP') { - console.log(`Provider: ${providerInfo}`); - } - if (config.protocol !== 'HTTP') { - console.log(`Protocol: ${config.protocol}`); - } - console.log(`Server: ${getEndpointUrl(actualPort, config.protocol)}`); - console.log(`Log: ${logger.getRelativeLogPath()}`); - console.log(`Press Ctrl+C to stop\n`); + if (actualPort !== fixedPort) { + console.log(`Port ${fixedPort} in use, using ${actualPort}`); + } - const devResult = await withCommandRunTelemetry( - 'dev', - { - dev_action: 'server' as const, - ui_mode: 'terminal' as const, - has_stream: false, - agent_protocol: standardize(AgentProtocol, (config.protocol ?? 'http').toLowerCase()), - invoke_count: 0, - }, - async (): Promise => { - await new Promise((resolve, reject) => { + console.log(`Starting dev server...`); + console.log(`Agent: ${config.agentName}`); + if (config.protocol !== 'MCP') { + console.log(`Provider: (see agent code)`); + } + if (config.protocol !== 'HTTP') { + console.log(`Protocol: ${config.protocol}`); + } + console.log(`Server: ${getEndpointUrl(actualPort, config.protocol)}`); + console.log(`Log: ${logger.getRelativeLogPath()}`); + console.log(`Press Ctrl+C to stop\n`); + + const serverPromise = new Promise((resolve, reject) => { const devCallbacks = { onLog: (level: string, msg: string) => { const prefix = level === 'error' ? '❌' : level === 'warn' ? '⚠️' : '→'; @@ -418,34 +425,30 @@ export const registerDev = (program: Command) => { server.kill(); }); }); - return { success: true as const }; + + return { + success: true as const, + blockingPromise: serverPromise, + _telemetryAttrs: { + dev_action: 'server' as const, + ui_mode: 'terminal' as const, + has_stream: false, + agent_protocol: standardize(AgentProtocol, config.protocol.toLowerCase()), + invoke_count: 0, + }, + }; } - ); - if (!devResult.success) throw devResult.error; - process.exit(0); - } - // If --no-browser provided, launch terminal TUI mode - if (!opts.browser) { - requireTTY(); - // Enter alternate screen buffer for fullscreen mode - process.stdout.write(ENTER_ALT_SCREEN); + // --no-browser: terminal TUI mode + if (!opts.browser) { + requireTTY(); + process.stdout.write(ENTER_ALT_SCREEN); - const exitAltScreen = () => { - process.stdout.write(EXIT_ALT_SCREEN); - process.stdout.write(SHOW_CURSOR); - }; + const exitAltScreen = () => { + process.stdout.write(EXIT_ALT_SCREEN); + process.stdout.write(SHOW_CURSOR); + }; - const tuiResult = await withCommandRunTelemetry( - 'dev', - { - dev_action: 'server' as const, - ui_mode: 'terminal' as const, - has_stream: false, - agent_protocol: standardize(AgentProtocol, (targetDevAgent?.protocol ?? 'http').toLowerCase()), - invoke_count: 0, - }, - async (): Promise => { const { DevScreen } = await import('../../tui/screens/dev/DevScreen'); const { unmount, waitUntilExit } = render( @@ -462,44 +465,47 @@ export const registerDev = (program: Command) => { ); - await waitUntilExit(); - exitAltScreen(); - return { success: true }; + return { + success: true as const, + blockingPromise: waitUntilExit().finally(() => { + exitAltScreen(); + collector?.stop(); + }), + _telemetryAttrs: { + dev_action: 'server' as const, + ui_mode: 'terminal' as const, + has_stream: false, + agent_protocol: standardize(AgentProtocol, (targetDevAgent?.protocol ?? 'http').toLowerCase()), + invoke_count: 0, + }, + }; } - ); - if (!tuiResult.success) throw tuiResult.error; - collector?.stop(); - process.exit(0); - } - // Default: launch web UI in browser - // NOTE: Do not copy this pattern. runBrowserMode blocks forever (internal - // await new Promise(() => {})) so we cannot use withCommandRunTelemetry here. - // We emit telemetry eagerly before the blocking call. - { - const client = await TelemetryClientAccessor.get().catch(() => undefined); - if (client) { - client.emit('cli.command_run', 0, { - command_group: 'dev', - command: 'dev', - exit_reason: 'success', - dev_action: 'server', - ui_mode: 'browser', - has_stream: false, - agent_protocol: standardize(AgentProtocol, (targetDevAgent?.protocol ?? 'http').toLowerCase()), - invoke_count: 0, - }); - await client.flush(); + // Default: browser mode (blocks forever) + return { + success: true as const, + blockingPromise: runBrowserMode({ + workingDir, + project, + port, + agentName: opts.runtime, + otelEnvVars, + collector, + }), + _telemetryAttrs: { + dev_action: 'server' as const, + ui_mode: 'browser' as const, + has_stream: false, + agent_protocol: standardize(AgentProtocol, (targetDevAgent?.protocol ?? 'http').toLowerCase()), + invoke_count: 0, + }, + }; } - await runBrowserMode({ - workingDir, - project, - port, - agentName: opts.runtime, - otelEnvVars, - collector, - }); - } + ); + // TODO: Remove cast once withCommandRunTelemetry's return type is narrowed + if (!serverResult.success) throw (serverResult as unknown as { error: Error }).error; + if ('blockingPromise' in serverResult) await serverResult.blockingPromise; + process.exit(0); } catch (error) { console.error(`Error: ${getErrorMessage(error)}`); process.exit(1); From 6f559ab97707d459e1f5d5d6ef16a981db2a5169 Mon Sep 17 00:00:00 2001 From: Harrison Weinstock Date: Tue, 26 May 2026 13:12:06 +0000 Subject: [PATCH 4/7] fix: avoid defaulting to http protocol on telemetry failures --- src/cli/commands/dev/command.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cli/commands/dev/command.tsx b/src/cli/commands/dev/command.tsx index 2ea2af85b..085b8c5c4 100644 --- a/src/cli/commands/dev/command.tsx +++ b/src/cli/commands/dev/command.tsx @@ -201,7 +201,7 @@ export const registerDev = (program: Command) => { dev_action: 'exec' as const, ui_mode: 'terminal' as const, has_stream: false, - agent_protocol: 'http' as const, + agent_protocol: standardize(AgentProtocol, 'unknown'), invoke_count: 0, }, async () => { @@ -245,7 +245,7 @@ export const registerDev = (program: Command) => { dev_action: 'invoke' as const, ui_mode: 'terminal' as const, has_stream: opts.stream ?? false, - agent_protocol: 'http' as const, + agent_protocol: standardize(AgentProtocol, 'unknown'), invoke_count: 1, }, async () => { @@ -311,7 +311,7 @@ export const registerDev = (program: Command) => { dev_action: 'server' as const, ui_mode: 'terminal' as const, has_stream: false, - agent_protocol: 'http' as const, + agent_protocol: standardize(AgentProtocol, 'unknown'), invoke_count: 0, }, async () => { From e41c7169c51b66451933457f73b3b86cfde1ef0f Mon Sep 17 00:00:00 2001 From: Harrison Weinstock Date: Tue, 26 May 2026 13:22:34 +0000 Subject: [PATCH 5/7] fix: remove redundant property check --- src/cli/commands/dev/command.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cli/commands/dev/command.tsx b/src/cli/commands/dev/command.tsx index 085b8c5c4..6ff5450bf 100644 --- a/src/cli/commands/dev/command.tsx +++ b/src/cli/commands/dev/command.tsx @@ -504,7 +504,7 @@ export const registerDev = (program: Command) => { ); // TODO: Remove cast once withCommandRunTelemetry's return type is narrowed if (!serverResult.success) throw (serverResult as unknown as { error: Error }).error; - if ('blockingPromise' in serverResult) await serverResult.blockingPromise; + await serverResult.blockingPromise; process.exit(0); } catch (error) { console.error(`Error: ${getErrorMessage(error)}`); From fd69292291aed52b5257dfc079dee6dbc0d60c91 Mon Sep 17 00:00:00 2001 From: Harrison Weinstock Date: Tue, 26 May 2026 13:33:48 +0000 Subject: [PATCH 6/7] feat(telemetry): support partial attribute overrides --- src/cli/telemetry/__tests__/client.test.ts | 34 ++++++++++++++++++++-- src/cli/telemetry/cli-command-run.ts | 20 +++++++++++-- 2 files changed, 48 insertions(+), 6 deletions(-) diff --git a/src/cli/telemetry/__tests__/client.test.ts b/src/cli/telemetry/__tests__/client.test.ts index e0dd59e79..57b305089 100644 --- a/src/cli/telemetry/__tests__/client.test.ts +++ b/src/cli/telemetry/__tests__/client.test.ts @@ -186,7 +186,7 @@ describe('withCommandRunTelemetry', () => { has_stream: true, agent_protocol: 'a2a', invoke_count: 5, - }, + } as const, }) ); @@ -219,7 +219,7 @@ describe('withCommandRunTelemetry', () => { has_stream: false, agent_protocol: 'mcp', invoke_count: 0, - }, + } as const, }) ); @@ -268,11 +268,39 @@ describe('withCommandRunTelemetry', () => { has_stream: false, agent_protocol: 'mcp', invoke_count: 0, - }, + } as const, }) ); expect('_telemetryAttrs' in result).toBe(false); }); + + it('partially returned _telemetryAttrs merge with upfront attrs preserving non-overlapping keys', async () => { + await withCommandRunTelemetry( + 'dev', + { + dev_action: 'server', + ui_mode: 'terminal', + has_stream: false, + agent_protocol: 'http', + invoke_count: 0, + }, + async () => ({ + success: true as const, + _telemetryAttrs: { + agent_protocol: 'mcp', + } as const, + }) + ); + + expect(sink.metrics).toHaveLength(1); + expect(sink.metrics[0]!.attrs).toMatchObject({ + dev_action: 'server', + ui_mode: 'terminal', + has_stream: 'false', + agent_protocol: 'mcp', + invoke_count: 0, + }); + }); }); }); diff --git a/src/cli/telemetry/cli-command-run.ts b/src/cli/telemetry/cli-command-run.ts index 73156b505..9b9e0be77 100644 --- a/src/cli/telemetry/cli-command-run.ts +++ b/src/cli/telemetry/cli-command-run.ts @@ -86,7 +86,9 @@ async function trackCommandRun( export async function withCommandRunTelemetry( command: C, attrs: CommandAttrs, - fn: () => (R & { _telemetryAttrs?: CommandAttrs }) | Promise }> + fn: () => + | (R & { _telemetryAttrs?: Partial> }) + | Promise> }> ): Promise { const client = await getTelemetryClient(); const start = performance.now(); @@ -97,7 +99,13 @@ export async function withCommandRunTelemetry Date: Tue, 26 May 2026 12:35:57 -0400 Subject: [PATCH 7/7] refactor: leverage mutable recorder in favor of immutable returns --- src/cli/commands/dev/command.tsx | 68 ++++++------------- src/cli/telemetry/__tests__/client.test.ts | 79 ++++++++++------------ src/cli/telemetry/attribute-recorder.ts | 16 +++++ src/cli/telemetry/cli-command-run.ts | 29 ++++---- 4 files changed, 87 insertions(+), 105 deletions(-) create mode 100644 src/cli/telemetry/attribute-recorder.ts diff --git a/src/cli/commands/dev/command.tsx b/src/cli/commands/dev/command.tsx index 6ff5450bf..49411646d 100644 --- a/src/cli/commands/dev/command.tsx +++ b/src/cli/commands/dev/command.tsx @@ -204,7 +204,7 @@ export const registerDev = (program: Command) => { agent_protocol: standardize(AgentProtocol, 'unknown'), invoke_count: 0, }, - async () => { + async recorder => { if (!positionalPrompt) { throw new ValidationError('A command is required with --exec. Usage: agentcore dev --exec "whoami"'); } @@ -217,18 +217,12 @@ export const registerDev = (program: Command) => { '--exec is only supported for Container build agents. For CodeZip agents, use your terminal to run commands directly.' ); } + recorder.set({ + agent_protocol: standardize(AgentProtocol, (targetAgent?.protocol ?? 'http').toLowerCase()), + }); const containerName = `agentcore-dev-${agentName}`.toLowerCase(); await execInContainer(positionalPrompt, containerName); - return { - success: true as const, - _telemetryAttrs: { - dev_action: 'exec' as const, - ui_mode: 'terminal' as const, - has_stream: false, - agent_protocol: standardize(AgentProtocol, (targetAgent?.protocol ?? 'http').toLowerCase()), - invoke_count: 0, - }, - }; + return { success: true as const }; } ); // TODO: Remove cast once withCommandRunTelemetry's return type is narrowed @@ -248,7 +242,7 @@ export const registerDev = (program: Command) => { agent_protocol: standardize(AgentProtocol, 'unknown'), invoke_count: 1, }, - async () => { + async recorder => { const workingDir = getWorkingDirectory(); const invokeProject = await loadProjectConfig(workingDir); @@ -265,6 +259,9 @@ export const registerDev = (program: Command) => { } const protocol = targetAgent?.protocol ?? 'HTTP'; + recorder.set({ + agent_protocol: standardize(AgentProtocol, protocol.toLowerCase()), + }); if (protocol === 'A2A') invokePort = 9000; else if (protocol === 'MCP') invokePort = 8000; @@ -286,16 +283,7 @@ export const registerDev = (program: Command) => { await invokeDevServer(invokePort, invokePrompt, opts.stream ?? false, headers); } - return { - success: true as const, - _telemetryAttrs: { - dev_action: 'invoke' as const, - ui_mode: 'terminal' as const, - has_stream: opts.stream ?? false, - agent_protocol: standardize(AgentProtocol, protocol.toLowerCase()), - invoke_count: 1, - }, - }; + return { success: true as const }; } ); // TODO: Remove cast once withCommandRunTelemetry's return type is narrowed @@ -314,7 +302,7 @@ export const registerDev = (program: Command) => { agent_protocol: standardize(AgentProtocol, 'unknown'), invoke_count: 0, }, - async () => { + async recorder => { const project = await loadProjectConfig(workingDir); if (!project) { throw new NoProjectError(); @@ -366,6 +354,10 @@ export const registerDev = (program: Command) => { throw new ValidationError('No dev-supported agents found.'); } + recorder.set({ + agent_protocol: standardize(AgentProtocol, config.protocol.toLowerCase()), + }); + const isA2A = config.protocol === 'A2A'; const isMcp = config.protocol === 'MCP'; const fixedPort = isA2A ? 9000 : isMcp ? 8000 : getAgentPort(project, config.agentName, port); @@ -426,18 +418,11 @@ export const registerDev = (program: Command) => { }); }); - return { - success: true as const, - blockingPromise: serverPromise, - _telemetryAttrs: { - dev_action: 'server' as const, - ui_mode: 'terminal' as const, - has_stream: false, - agent_protocol: standardize(AgentProtocol, config.protocol.toLowerCase()), - invoke_count: 0, - }, - }; + return { success: true as const, blockingPromise: serverPromise }; } + recorder.set({ + agent_protocol: standardize(AgentProtocol, (targetDevAgent?.protocol ?? 'http').toLowerCase()), + }); // --no-browser: terminal TUI mode if (!opts.browser) { @@ -471,17 +456,11 @@ export const registerDev = (program: Command) => { exitAltScreen(); collector?.stop(); }), - _telemetryAttrs: { - dev_action: 'server' as const, - ui_mode: 'terminal' as const, - has_stream: false, - agent_protocol: standardize(AgentProtocol, (targetDevAgent?.protocol ?? 'http').toLowerCase()), - invoke_count: 0, - }, }; } // Default: browser mode (blocks forever) + recorder.set({ ui_mode: 'browser' as const }); return { success: true as const, blockingPromise: runBrowserMode({ @@ -492,13 +471,6 @@ export const registerDev = (program: Command) => { otelEnvVars, collector, }), - _telemetryAttrs: { - dev_action: 'server' as const, - ui_mode: 'browser' as const, - has_stream: false, - agent_protocol: standardize(AgentProtocol, (targetDevAgent?.protocol ?? 'http').toLowerCase()), - invoke_count: 0, - }, }; } ); diff --git a/src/cli/telemetry/__tests__/client.test.ts b/src/cli/telemetry/__tests__/client.test.ts index 57b305089..24263e6ad 100644 --- a/src/cli/telemetry/__tests__/client.test.ts +++ b/src/cli/telemetry/__tests__/client.test.ts @@ -167,8 +167,8 @@ describe('withCommandRunTelemetry', () => { }); }); - describe('_telemetryAttrs', () => { - it('returned _telemetryAttrs override upfront attrs on success', async () => { + describe('AttributeRecorder', () => { + it('recorder.set() overrides initial attributes on success', async () => { await withCommandRunTelemetry( 'dev', { @@ -178,16 +178,16 @@ describe('withCommandRunTelemetry', () => { agent_protocol: 'http', invoke_count: 0, }, - async () => ({ - success: true as const, - _telemetryAttrs: { + async recorder => { + recorder.set({ dev_action: 'invoke', ui_mode: 'browser', has_stream: true, agent_protocol: 'a2a', invoke_count: 5, - } as const, - }) + }); + return { success: true as const }; + } ); expect(sink.metrics).toHaveLength(1); @@ -200,7 +200,7 @@ describe('withCommandRunTelemetry', () => { }); }); - it('returned _telemetryAttrs override upfront attrs on failure result', async () => { + it('recorder.set() overrides initial attributes on failure result', async () => { await withCommandRunTelemetry( 'dev', { @@ -210,17 +210,12 @@ describe('withCommandRunTelemetry', () => { agent_protocol: 'http', invoke_count: 0, }, - async () => ({ - success: false as const, - error: new Error('port in use'), - _telemetryAttrs: { - dev_action: 'server', - ui_mode: 'terminal', - has_stream: false, + async recorder => { + recorder.set({ agent_protocol: 'mcp', - invoke_count: 0, - } as const, - }) + }); + return { success: false as const, error: new Error('port in use') }; + } ); expect(sink.metrics).toHaveLength(1); @@ -230,7 +225,7 @@ describe('withCommandRunTelemetry', () => { }); }); - it('falls back to upfront attrs when _telemetryAttrs is not returned', async () => { + it('uses initial attributes when recorder.set() is never called', async () => { await withCommandRunTelemetry( 'dev', { @@ -250,8 +245,8 @@ describe('withCommandRunTelemetry', () => { }); }); - it('does not leak _telemetryAttrs to the caller', async () => { - const result = await withCommandRunTelemetry( + it('partial recorder.set() merges with initial attributes preserving non-overlapping keys', async () => { + await withCommandRunTelemetry( 'dev', { dev_action: 'server', @@ -260,22 +255,25 @@ describe('withCommandRunTelemetry', () => { agent_protocol: 'http', invoke_count: 0, }, - async () => ({ - success: true as const, - _telemetryAttrs: { - dev_action: 'server', - ui_mode: 'terminal', - has_stream: false, + async recorder => { + recorder.set({ agent_protocol: 'mcp', - invoke_count: 0, - } as const, - }) + }); + return { success: true as const }; + } ); - expect('_telemetryAttrs' in result).toBe(false); + expect(sink.metrics).toHaveLength(1); + expect(sink.metrics[0]!.attrs).toMatchObject({ + dev_action: 'server', + ui_mode: 'terminal', + has_stream: 'false', + agent_protocol: 'mcp', + invoke_count: 0, + }); }); - it('partially returned _telemetryAttrs merge with upfront attrs preserving non-overlapping keys', async () => { + it('recorder.set() called before throw is preserved in telemetry', async () => { await withCommandRunTelemetry( 'dev', { @@ -285,21 +283,16 @@ describe('withCommandRunTelemetry', () => { agent_protocol: 'http', invoke_count: 0, }, - async () => ({ - success: true as const, - _telemetryAttrs: { - agent_protocol: 'mcp', - } as const, - }) + async recorder => { + recorder.set({ agent_protocol: 'a2a' }); + throw new Error('crash'); + } ); expect(sink.metrics).toHaveLength(1); expect(sink.metrics[0]!.attrs).toMatchObject({ - dev_action: 'server', - ui_mode: 'terminal', - has_stream: 'false', - agent_protocol: 'mcp', - invoke_count: 0, + exit_reason: 'failure', + agent_protocol: 'a2a', }); }); }); diff --git a/src/cli/telemetry/attribute-recorder.ts b/src/cli/telemetry/attribute-recorder.ts new file mode 100644 index 000000000..36047c8a9 --- /dev/null +++ b/src/cli/telemetry/attribute-recorder.ts @@ -0,0 +1,16 @@ +export interface AttributeRecorder> { + set(attrs: Pick): void; + get(): Partial; +} + +export function createAttributeRecorder>(): AttributeRecorder { + let recorded: Partial = {}; + return { + set(attrs: Pick) { + recorded = { ...recorded, ...attrs }; + }, + get() { + return recorded; + }, + }; +} diff --git a/src/cli/telemetry/cli-command-run.ts b/src/cli/telemetry/cli-command-run.ts index 9b9e0be77..d60c0e8fd 100644 --- a/src/cli/telemetry/cli-command-run.ts +++ b/src/cli/telemetry/cli-command-run.ts @@ -1,5 +1,6 @@ import type { Result } from '../../lib/result'; import { getErrorMessage } from '../errors'; +import { type AttributeRecorder, createAttributeRecorder } from './attribute-recorder.js'; import { TelemetryClientAccessor } from './client-accessor.js'; import { TelemetryClient } from './client.js'; import { classifyError } from './error.js'; @@ -7,6 +8,8 @@ import { COMMAND_SCHEMAS, type Command, type CommandAttrs, deriveCommandGroup } import { type CommandResult, CommandResultSchema, resilientParse } from './schemas/common-shapes.js'; import { performance } from 'perf_hooks'; +export type { AttributeRecorder } from './attribute-recorder.js'; + async function getTelemetryClient() { try { return await TelemetryClientAccessor.get(); @@ -79,23 +82,22 @@ async function trackCommandRun( * the exception is converted to a result type such that callers do not need to handle result + try/catch. * If telemetry is unavailable, the callback runs untracked. * - * The callback may return `_telemetryAttrs` to override or supplement the upfront `attrs`. - * Returned attrs are merged with the upfront attrs (returned takes priority). - * On throw, only the upfront attrs are used since the callback never produced a value. + * The callback receives an AttributeRecorder to dynamically set or override attributes. + * Initial attributes are seeded into the recorder; the callback may call recorder.set() + * to override or supplement them at any point during execution. */ export async function withCommandRunTelemetry( command: C, - attrs: CommandAttrs, - fn: () => - | (R & { _telemetryAttrs?: Partial> }) - | Promise> }> + attributes: CommandAttrs, + fn: (recorder: AttributeRecorder>) => R | Promise ): Promise { const client = await getTelemetryClient(); + const recorder = createAttributeRecorder>(); + recorder.set(attributes); const start = performance.now(); try { - const result = await fn(); + const result = await fn(recorder); if (client) { - const merged = result._telemetryAttrs ? { ...attrs, ...result._telemetryAttrs } : attrs; const durationMs = Math.round(performance.now() - start); if (!result.success) { const { category, source } = classifyError(result.error); @@ -103,15 +105,14 @@ export async function withCommandRunTelemetry