feat: make parsing resilient to individual failures (#1062)

Author: Hweinstock

src/cli/telemetry/__tests__/client.test.ts

@@ -116,18 +116,49 @@ describe('TelemetryClient', () => {
       expect(sink.metrics[0]!.attrs.check_only).toBe('true');
     });
 
-    it('silently drops invalid success payloads', async () => {
+    it('publishes metric with unknown defaults for incomplete success payloads', async () => {
       const sink = new InMemorySink();
       const client = new TelemetryClient(sink);
 
-      // Missing required attrs for 'create' — should silently drop
+      // Missing required attrs for 'create' — should still publish with 'unknown' defaults
       await client.withCommandRun(
         'create',
         // @ts-expect-error — intentionally incomplete
         async () => ({ language: 'python' })
       );
 
-      expect(sink.metrics).toHaveLength(0);
+      expect(sink.metrics).toHaveLength(1);
+      expect(sink.metrics[0]!.attrs).toMatchObject({
+        exit_reason: 'success',
+        language: 'python',
+        framework: 'unknown',
+        model_provider: 'unknown',
+      });
+    });
+
+    it('defaults invalid attrs to unknown while preserving valid ones', async () => {
+      const sink = new InMemorySink();
+      const client = new TelemetryClient(sink);
+
+      await client.withCommandRun(
+        'create',
+        // @ts-expect-error — intentionally invalid enum value
+        async () => ({
+          language: 'rust', // invalid enum
+          framework: 'strands',
+          model_provider: 'bedrock',
+          memory: 'shortterm',
+          protocol: 'mcp',
+          build: 'codezip',
+          agent_type: 'create',
+          network_mode: 'public',
+          has_agent: true,
+        })
+      );
+
+      expect(sink.metrics).toHaveLength(1);
+      expect(sink.metrics[0]!.attrs.language).toBe('unknown');
+      expect(sink.metrics[0]!.attrs.framework).toBe('strands');
     });
 
     it('records cancel when callback returns CANCELLED', async () => {

src/cli/telemetry/client.ts

@@ -1,6 +1,6 @@
 import { classifyError, isUserError } from './error-classification.js';
 import { COMMAND_SCHEMAS, type Command, type CommandAttrs, deriveCommandGroup } from './schemas/command-run.js';
-import { type CommandResult, CommandResultSchema } from './schemas/common-shapes.js';
+import { type CommandResult, CommandResultSchema, resilientParse } from './schemas/common-shapes.js';
 import type { MetricSink } from './sinks/metric-sink.js';
 import { performance } from 'perf_hooks';
 
@@ -69,17 +69,24 @@ export class TelemetryClient {
     durationMs: number
   ): void {
     try {
+      // CommandResult is built internally — hard parse is intentional since
+      // a metric without a valid exit_reason is meaningless.
       CommandResultSchema.parse(result);
-      if (result.exit_reason !== 'failure' && result.exit_reason !== 'cancel') {
-        COMMAND_SCHEMAS[command].parse(attrs);
-      }
+
+      // Validate command attrs resiliently: invalid fields default to 'unknown'
+      // instead of dropping the entire metric.
+      // On failure/cancel the callback attrs are empty so validation is skipped.
+      const validatedAttrs =
+        result.exit_reason !== 'failure' && result.exit_reason !== 'cancel'
+          ? resilientParse(COMMAND_SCHEMAS[command], attrs as Record<string, unknown>)
+          : attrs;
 
       const otelAttrs: Record<string, string | number> = {
         command_group: deriveCommandGroup(command),
         command,
       };
 
-      for (const obj of [result, attrs]) {
+      for (const obj of [result, validatedAttrs]) {
         for (const [k, v] of Object.entries(obj)) {
           if (typeof v === 'boolean') {
             otelAttrs[k] = String(v);

src/cli/telemetry/schemas/__tests__/command-run.test.ts

@@ -1,6 +1,6 @@
 import { COMMAND_SCHEMAS, type Command, type CommandAttrs, deriveCommandGroup } from '../command-run';
 import { ResourceAttributesSchema } from '../common-attributes';
-import { CommandResultSchema } from '../common-shapes';
+import { CommandResultSchema, resilientParse } from '../common-shapes';
 import { describe, expect, expectTypeOf, it } from 'vitest';
 import { z } from 'zod';
 
@@ -170,3 +170,55 @@ describe('type safety', () => {
     }
   });
 });
+
+describe('resilientParse', () => {
+  it('passes valid attrs through unchanged', () => {
+    const attrs = {
+      language: 'python',
+      framework: 'strands',
+      model_provider: 'bedrock',
+      memory: 'shortterm',
+      protocol: 'mcp',
+      build: 'codezip',
+      agent_type: 'create',
+      network_mode: 'public',
+      has_agent: true,
+    };
+    expect(resilientParse(COMMAND_SCHEMAS.create, attrs)).toEqual(attrs);
+  });
+
+  it('defaults a single invalid enum field to unknown', () => {
+    const attrs = {
+      language: 'rust', // invalid
+      framework: 'strands',
+      model_provider: 'bedrock',
+      memory: 'shortterm',
+      protocol: 'mcp',
+      build: 'codezip',
+      agent_type: 'create',
+      network_mode: 'public',
+      has_agent: true,
+    };
+    const result = resilientParse(COMMAND_SCHEMAS.create, attrs);
+    expect(result.language).toBe('unknown');
+    expect(result.framework).toBe('strands');
+  });
+
+  it('defaults missing required fields to unknown', () => {
+    const result = resilientParse(COMMAND_SCHEMAS.create, { language: 'python' });
+    expect(result.language).toBe('python');
+    expect(result.framework).toBe('unknown');
+    expect(result.model_provider).toBe('unknown');
+  });
+
+  it('defaults all fields to unknown when all are invalid', () => {
+    const result = resilientParse(COMMAND_SCHEMAS.create, {});
+    for (const value of Object.values(result)) {
+      expect(value).toBe('unknown');
+    }
+  });
+
+  it('returns empty object for no-attrs schemas', () => {
+    expect(resilientParse(COMMAND_SCHEMAS['telemetry.disable'], {})).toEqual({});
+  });
+});

src/cli/telemetry/schemas/common-shapes.ts

@@ -8,6 +8,24 @@ export function safeSchema<T extends Record<string, SafeField>>(shape: T) {
   return z.object(shape);
 }
 
+/**
+ * Validate each field in a schema individually, defaulting to 'unknown' on failure.
+ * This ensures a single invalid attribute never blocks the entire metric from being published.
+ * Keys in attrs not present in the schema are omitted from the result.
+ */
+export function resilientParse(
+  schema: z.ZodObject<z.ZodRawShape>,
+  attrs: Record<string, unknown>
+): Record<string, unknown> {
+  const result: Record<string, unknown> = {};
+  for (const key of Object.keys(schema.shape)) {
+    const field = schema.shape[key] as z.ZodType;
+    const parsed = field.safeParse(attrs[key]);
+    result[key] = parsed.success ? parsed.data : 'unknown';
+  }
+  return result;
+}
+
 // Primitive types
 export const Count = z.number().int().nonnegative();