Skip to content

Commit 86abe2d

Browse files
committed
feat: wire telemetry into all remove.* commands
1 parent bd6f841 commit 86abe2d

21 files changed

Lines changed: 153 additions & 58 deletions

integ-tests/add-remove-gateway.test.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
import { createTestProject, runCLI } from '../src/test-utils/index.js';
22
import type { TestProject } from '../src/test-utils/index.js';
3+
import { createTelemetryHelper } from '../src/test-utils/telemetry-helper.js';
34
import { mkdir, readFile, writeFile } from 'node:fs/promises';
45
import { join } from 'node:path';
56
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
67

8+
const telemetry = createTelemetryHelper();
9+
710
async function readProjectConfig(projectPath: string) {
811
return JSON.parse(await readFile(join(projectPath, 'agentcore/agentcore.json'), 'utf-8'));
912
}
@@ -19,6 +22,7 @@ describe('integration: add and remove gateway with external MCP server', () => {
1922

2023
afterAll(async () => {
2124
await project.cleanup();
25+
telemetry.destroy();
2226
});
2327

2428
describe('gateway lifecycle', () => {
@@ -64,7 +68,9 @@ describe('integration: add and remove gateway with external MCP server', () => {
6468
});
6569

6670
it('removes the gateway target', async () => {
67-
const result = await runCLI(['remove', 'gateway-target', '--name', targetName, '--json'], project.projectPath);
71+
const result = await runCLI(['remove', 'gateway-target', '--name', targetName, '--json'], project.projectPath, {
72+
env: telemetry.env,
73+
});
6874

6975
expect(result.exitCode, `stdout: ${result.stdout}, stderr: ${result.stderr}`).toBe(0);
7076
const json = JSON.parse(result.stdout);
@@ -75,10 +81,13 @@ describe('integration: add and remove gateway with external MCP server', () => {
7581
const targets = gateway?.targets ?? [];
7682
const found = targets.find((t: { name: string }) => t.name === targetName);
7783
expect(found, `Target "${targetName}" should be removed`).toBeFalsy();
84+
telemetry.assertMetricEmitted({ command: 'remove.gateway-target', exit_reason: 'success' });
7885
});
7986

8087
it('removes the gateway', async () => {
81-
const result = await runCLI(['remove', 'gateway', '--name', gatewayName, '--json'], project.projectPath);
88+
const result = await runCLI(['remove', 'gateway', '--name', gatewayName, '--json'], project.projectPath, {
89+
env: telemetry.env,
90+
});
8291

8392
expect(result.exitCode, `stdout: ${result.stdout}, stderr: ${result.stderr}`).toBe(0);
8493
const json = JSON.parse(result.stdout);
@@ -88,6 +97,7 @@ describe('integration: add and remove gateway with external MCP server', () => {
8897
const gateways = mcpSpec.agentCoreGateways ?? [];
8998
const found = gateways.find((g: { name: string }) => g.name === gatewayName);
9099
expect(found, `Gateway "${gatewayName}" should be removed`).toBeFalsy();
100+
telemetry.assertMetricEmitted({ command: 'remove.gateway', exit_reason: 'success' });
91101
});
92102
});
93103
});

integ-tests/add-remove-resources.test.ts

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ describe('integration: add and remove resources', () => {
4141
const found = memories!.some((m: Record<string, unknown>) => m.name === memoryName);
4242
expect(found, `Memory "${memoryName}" should be in config`).toBe(true);
4343

44-
// Verify telemetry
4544
telemetry.assertMetricEmitted({ command: 'add.memory', exit_reason: 'success' });
4645
});
4746

@@ -71,7 +70,6 @@ describe('integration: add and remove resources', () => {
7170
expect(episodic!.reflectionNamespaces, 'Should have reflectionNamespaces').toBeDefined();
7271
expect(episodic!.reflectionNamespaces!.length).toBeGreaterThan(0);
7372

74-
// Verify telemetry
7573
telemetry.assertMetricEmitted({
7674
command: 'add.memory',
7775
exit_reason: 'success',
@@ -84,7 +82,9 @@ describe('integration: add and remove resources', () => {
8482
});
8583

8684
it('removes the memory resource', async () => {
87-
const result = await runCLI(['remove', 'memory', '--name', memoryName, '--json'], project.projectPath);
85+
const result = await runCLI(['remove', 'memory', '--name', memoryName, '--json'], project.projectPath, {
86+
env: telemetry.env,
87+
});
8888

8989
expect(result.exitCode, `stdout: ${result.stdout}, stderr: ${result.stderr}`).toBe(0);
9090
const json = JSON.parse(result.stdout);
@@ -95,6 +95,8 @@ describe('integration: add and remove resources', () => {
9595
const memories = (config.memories as Record<string, unknown>[] | undefined) ?? [];
9696
const found = memories.some((m: Record<string, unknown>) => m.name === memoryName);
9797
expect(found, `Memory "${memoryName}" should be removed from config`).toBe(false);
98+
99+
telemetry.assertMetricEmitted({ command: 'remove.memory', exit_reason: 'success' });
98100
});
99101
});
100102

@@ -119,7 +121,6 @@ describe('integration: add and remove resources', () => {
119121
const found = credentials!.some((c: Record<string, unknown>) => c.name === credentialName);
120122
expect(found, `Credential "${credentialName}" should be in config`).toBe(true);
121123

122-
// Verify telemetry
123124
telemetry.assertMetricEmitted({
124125
command: 'add.credential',
125126
exit_reason: 'success',
@@ -128,7 +129,9 @@ describe('integration: add and remove resources', () => {
128129
});
129130

130131
it('removes the credential resource', async () => {
131-
const result = await runCLI(['remove', 'credential', '--name', credentialName, '--json'], project.projectPath);
132+
const result = await runCLI(['remove', 'credential', '--name', credentialName, '--json'], project.projectPath, {
133+
env: telemetry.env,
134+
});
132135

133136
expect(result.exitCode, `stdout: ${result.stdout}, stderr: ${result.stderr}`).toBe(0);
134137
const json = JSON.parse(result.stdout);
@@ -139,6 +142,8 @@ describe('integration: add and remove resources', () => {
139142
const credentials = (config.credentials as Record<string, unknown>[] | undefined) ?? [];
140143
const found = credentials.some((c: Record<string, unknown>) => c.name === credentialName);
141144
expect(found, `Credential "${credentialName}" should be removed from config`).toBe(false);
145+
146+
telemetry.assertMetricEmitted({ command: 'remove.credential', exit_reason: 'success' });
142147
});
143148
});
144149

@@ -162,9 +167,37 @@ describe('integration: add and remove resources', () => {
162167
});
163168

164169
it('removes the policy engine resource', async () => {
165-
const result = await runCLI(['remove', 'policy-engine', '--name', engineName, '--json'], project.projectPath);
170+
const result = await runCLI(['remove', 'policy-engine', '--name', engineName, '--json'], project.projectPath, {
171+
env: telemetry.env,
172+
});
166173

167174
expect(result.exitCode, `stdout: ${result.stdout}, stderr: ${result.stderr}`).toBe(0);
175+
176+
telemetry.assertMetricEmitted({ command: 'remove.policy-engine', exit_reason: 'success' });
177+
});
178+
});
179+
180+
describe('remove failure telemetry', () => {
181+
it('emits failure telemetry for non-existent resource', async () => {
182+
const result = await runCLI(['remove', 'memory', '--name', 'DoesNotExist', '--json'], project.projectPath, {
183+
env: telemetry.env,
184+
});
185+
186+
expect(result.exitCode).toBe(1);
187+
telemetry.assertMetricEmitted({ command: 'remove.memory', exit_reason: 'failure' });
188+
});
189+
});
190+
191+
describe('remove all', () => {
192+
it('resets all schemas and emits telemetry', async () => {
193+
const result = await runCLI(['remove', 'all', '--yes', '--json'], project.projectPath, {
194+
env: telemetry.env,
195+
});
196+
197+
expect(result.exitCode).toBe(0);
198+
const json = JSON.parse(result.stdout);
199+
expect(json.success).toBe(true);
200+
telemetry.assertMetricEmitted({ command: 'remove.all', exit_reason: 'success' });
168201
});
169202
});
170203
});

src/cli/commands/remove/command.tsx

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { ConfigIO } from '../../../lib';
22
import { getErrorMessage } from '../../errors';
3+
import { cliCommandRun } from '../../telemetry/cli-command-run.js';
34
import { COMMAND_DESCRIPTIONS } from '../../tui/copy';
45
import { requireProject, requireTTY } from '../../tui/guards';
56
import { RemoveAllScreen, RemoveFlow } from '../../tui/screens/remove';
@@ -51,9 +52,12 @@ async function handleRemoveAll(_options: RemoveAllOptions): Promise<RemoveResult
5152

5253
async function handleRemoveAllCLI(options: RemoveAllOptions): Promise<void> {
5354
validateRemoveAllOptions(options);
54-
const result = await handleRemoveAll(options);
55-
console.log(JSON.stringify(result));
56-
process.exit(result.success ? 0 : 1);
55+
await cliCommandRun('remove.all', !!options.json, async () => {
56+
const result = await handleRemoveAll(options);
57+
if (!result.success) throw new Error(result.error);
58+
console.log(JSON.stringify(result));
59+
return {};
60+
});
5761
}
5862

5963
export const registerRemove = (program: Command): Command => {

src/cli/primitives/BasePrimitive.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ import { ConfigIO, findConfigRoot } from '../../lib';
22
import type { AgentCoreProjectSpec } from '../../schema';
33
import type { ResourceType } from '../commands/remove/types';
44
import { getErrorMessage } from '../errors';
5+
import { withTuiTelemetry } from '../telemetry/cli-command-run.js';
6+
import type { SubCommand } from '../telemetry/schemas/command-run.js';
57
import { requireTTY } from '../tui/guards/tty';
68
import { SOURCE_CODE_NOTE } from './constants';
79
import type { AddResult, AddScreenComponent, RemovableResource, RemovalPreview, RemovalResult } from './types';
@@ -120,7 +122,11 @@ export abstract class BasePrimitive<
120122
process.exit(1);
121123
}
122124

123-
const result = await this.remove(cliOptions.name);
125+
const result = await withTuiTelemetry<SubCommand<'remove', typeof this.kind>, RemovalResult>(
126+
`remove.${this.kind}`,
127+
{},
128+
() => this.remove(cliOptions.name!)
129+
);
124130
console.log(
125131
JSON.stringify({
126132
success: result.success,

src/cli/primitives/GatewayPrimitive.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import type { AddGatewayOptions as CLIAddGatewayOptions } from '../commands/add/
1212
import { validateAddGatewayOptions } from '../commands/add/validate';
1313
import { getErrorMessage } from '../errors';
1414
import type { RemovalPreview, RemovalResult, SchemaChange } from '../operations/remove/types';
15-
import { cliCommandRun } from '../telemetry/cli-command-run.js';
15+
import { cliCommandRun, withTuiTelemetry } from '../telemetry/cli-command-run.js';
1616
import { AuthorizerType, PolicyEngineMode, standardize } from '../telemetry/schemas/common-shapes.js';
1717
import { requireTTY } from '../tui/guards/tty';
1818
import type { AddGatewayConfig } from '../tui/screens/mcp/types';
@@ -262,7 +262,7 @@ export class GatewayPrimitive extends BasePrimitive<AddGatewayOptions, Removable
262262
process.exit(1);
263263
}
264264

265-
const result = await this.remove(cliOptions.name);
265+
const result = await withTuiTelemetry('remove.gateway', {}, () => this.remove(cliOptions.name!));
266266
console.log(
267267
JSON.stringify({
268268
success: result.success,

src/cli/primitives/GatewayTargetPrimitive.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import { validateAddGatewayTargetOptions } from '../commands/add/validate';
1414
import { getErrorMessage } from '../errors';
1515
import type { RemovableGatewayTarget } from '../operations/remove/remove-gateway-target';
1616
import type { RemovalPreview, RemovalResult, SchemaChange } from '../operations/remove/types';
17-
import { cliCommandRun } from '../telemetry/cli-command-run.js';
17+
import { cliCommandRun, withTuiTelemetry } from '../telemetry/cli-command-run.js';
1818
import {
1919
GATEWAY_TARGET_TYPE_MAP,
2020
GatewayTargetHost,
@@ -510,7 +510,7 @@ export class GatewayTargetPrimitive extends BasePrimitive<AddGatewayTargetOption
510510
process.exit(1);
511511
}
512512

513-
const result = await this.remove(cliOptions.name);
513+
const result = await withTuiTelemetry('remove.gateway-target', {}, () => this.remove(cliOptions.name!));
514514
console.log(
515515
JSON.stringify({
516516
success: result.success,

src/cli/primitives/PolicyEnginePrimitive.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import type { AgentCoreProjectSpec, PolicyEngine } from '../../schema';
33
import { PolicyEngineModeSchema, PolicyEngineSchema } from '../../schema';
44
import { getErrorMessage } from '../errors';
55
import type { RemovalPreview, RemovalResult, SchemaChange } from '../operations/remove/types';
6-
import { cliCommandRun } from '../telemetry/cli-command-run.js';
6+
import { cliCommandRun, withTuiTelemetry } from '../telemetry/cli-command-run.js';
77
import { AttachMode, standardize } from '../telemetry/schemas/common-shapes.js';
88
import { requireTTY } from '../tui/guards/tty';
99
import { BasePrimitive } from './BasePrimitive';
@@ -316,7 +316,7 @@ export class PolicyEnginePrimitive extends BasePrimitive<AddPolicyEngineOptions,
316316
process.exit(1);
317317
}
318318

319-
const result = await this.remove(cliOptions.name);
319+
const result = await withTuiTelemetry('remove.policy-engine', {}, () => this.remove(cliOptions.name!));
320320
if (cliOptions.json) {
321321
console.log(
322322
JSON.stringify({

src/cli/primitives/PolicyPrimitive.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { detectRegion } from '../aws';
55
import { getPolicyGeneration, startPolicyGeneration } from '../aws/policy-generation';
66
import { getErrorMessage } from '../errors';
77
import type { RemovalPreview, RemovalResult, SchemaChange } from '../operations/remove/types';
8-
import { cliCommandRun } from '../telemetry/cli-command-run.js';
8+
import { cliCommandRun, withTuiTelemetry } from '../telemetry/cli-command-run.js';
99
import { ValidationMode, standardize } from '../telemetry/schemas/common-shapes.js';
1010
import { requireTTY } from '../tui/guards/tty';
1111
import { BasePrimitive } from './BasePrimitive';
@@ -400,7 +400,7 @@ export class PolicyPrimitive extends BasePrimitive<AddPolicyOptions, RemovablePo
400400

401401
// Build composite key when --engine is provided for unambiguous removal
402402
const removeKey = cliOptions.engine ? `${cliOptions.engine}/${cliOptions.name}` : cliOptions.name;
403-
const result = await this.remove(removeKey);
403+
const result = await withTuiTelemetry('remove.policy', {}, () => this.remove(removeKey));
404404

405405
if (cliOptions.json) {
406406
console.log(

src/cli/primitives/RuntimeEndpointPrimitive.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { RuntimeEndpointSchema } from '../../schema';
44
import type { ResourceType } from '../commands/remove/types';
55
import { getErrorMessage } from '../errors';
66
import type { RemovalPreview, RemovalResult, SchemaChange } from '../operations/remove/types';
7-
import { cliCommandRun } from '../telemetry/cli-command-run.js';
7+
import { cliCommandRun, withTuiTelemetry } from '../telemetry/cli-command-run.js';
88
import { BasePrimitive } from './BasePrimitive';
99
import { SOURCE_CODE_NOTE } from './constants';
1010
import type { AddResult, AddScreenComponent, RemovableResource } from './types';
@@ -296,7 +296,7 @@ export class RuntimeEndpointPrimitive extends BasePrimitive<AddRuntimeEndpointOp
296296
process.exit(1);
297297
}
298298

299-
const result = await this.remove(cliOptions.name);
299+
const result = await withTuiTelemetry('remove.runtime-endpoint', {}, () => this.remove(cliOptions.name!));
300300
console.log(
301301
JSON.stringify({
302302
success: result.success,

src/cli/telemetry/cli-command-run.ts

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import { getErrorMessage } from '../errors';
2-
import type { AddResult } from '../primitives/types.js';
32
import { TelemetryClientAccessor } from './client-accessor.js';
43
import type { Command, CommandAttrs } from './schemas/command-run.js';
54

5+
/** Standard result shape for operations wrapped by {@link withTuiTelemetry}. */
6+
export type OperationResult = { success: true } | { success: false; error: string };
7+
68
/**
79
* Run a CLI command with telemetry, standardized error output, and process.exit.
810
* The callback should throw on failure and return telemetry attrs on success.
@@ -38,22 +40,29 @@ export async function cliCommandRun<C extends Command>(
3840
}
3941

4042
/**
41-
* Wrap a primitive .add() call with telemetry — used by TUI paths.
42-
* CLI paths use {@link cliCommandRun} instead.
43+
* Wrap a TUI operation with telemetry. Works for any `{ success: boolean }` result type
44+
* (add, remove, etc.). CLI paths use {@link cliCommandRun} instead.
45+
*
46+
* Attrs are only recorded on success — on failure, `withCommandRun` records the
47+
* error classification instead.
48+
*
49+
* @param command Telemetry command key (e.g. 'add.memory', 'remove.agent')
50+
* @param attrs Command-specific telemetry attributes (pass `{}` for NoAttrs commands)
51+
* @param fn The operation to wrap
4352
*/
44-
export async function withAddTelemetry<C extends Command, T extends Record<string, unknown>>(
53+
export async function withTuiTelemetry<C extends Command, R extends OperationResult>(
4554
command: C,
4655
attrs: CommandAttrs<C>,
47-
fn: () => Promise<AddResult<T>>
48-
): Promise<AddResult<T>> {
56+
fn: () => Promise<R>
57+
): Promise<R> {
4958
let client;
5059
try {
5160
client = await TelemetryClientAccessor.get();
5261
} catch {
5362
return fn();
5463
}
5564

56-
let result: AddResult<T> | undefined;
65+
let result: R | undefined;
5766
try {
5867
await client.withCommandRun(command, async () => {
5968
result = await fn();
@@ -64,7 +73,7 @@ export async function withAddTelemetry<C extends Command, T extends Record<strin
6473
// withCommandRun re-throws after recording failure telemetry.
6574
// result is set if fn() ran; if not, fn() itself threw.
6675
if (!result) {
67-
return { success: false, error: getErrorMessage(err) };
76+
return { success: false, error: getErrorMessage(err) } as R;
6877
}
6978
}
7079
return result!;

0 commit comments

Comments
 (0)