refactor: unify result types with discriminated Result<T, E> union (#1125)

* refactor: unify result types with discriminated Result<T, E> union

Introduce a shared Result<T, E> type (inspired by Rust's Result) that
replaces ad-hoc { success: boolean; error?: string } patterns across
the codebase.

Key changes:
- Add src/lib/types.ts with Result<T, E> discriminated union type
- Add toError() helper in src/cli/errors.ts for catch blocks
- Migrate all command, operation, and primitive result types to Result<T>
- Error field is now Error (not string) on the failure branch
- Data fields only exist on the success branch (proper narrowing)
- Update all consumers to narrow before accessing branch-specific fields
- Update test assertions to match new Error objects and add narrowing

* docs: update AGENTS.md and telemetry README to reflect Result<T, E> type

Author: Hweinstock

src/cli/AGENTS.md

@@ -82,7 +82,7 @@ primitives/
 ├── registry.ts                # Singleton instances + ALL_PRIMITIVES array
 ├── credential-utils.ts        # Shared credential env var name computation
 ├── constants.ts               # SOURCE_CODE_NOTE and other shared constants
-├── types.ts                   # AddResult, RemovableResource, RemovalResult, etc.
+├── types.ts                   # RemovableResource, AddScreenComponent, etc.
 └── index.ts                   # Barrel exports
 ```
 
@@ -92,8 +92,8 @@ Every primitive extends `BasePrimitive<TAddOptions, TRemovable>` and implements:
 
 - `kind` — resource identifier (`'agent'`, `'memory'`, `'identity'`, `'gateway'`, `'mcp-tool'`)
 - `label` — human-readable name (`'Agent'`, `'Memory'`, `'Identity'`)
-- `add(options)` — create a resource, returns `AddResult`
-- `remove(name)` — remove a resource, returns `RemovalResult`
+- `add(options)` — create a resource, returns `Result<T>`
+- `remove(name)` — remove a resource, returns `Result`
 - `previewRemove(name)` — preview what removal will do
 - `getRemovable()` — list resources available for removal
 - `registerCommands(addCmd, removeCmd)` — register CLI subcommands
@@ -119,7 +119,8 @@ BasePrimitive provides shared helpers:
 - **Absorb, don't wrap.** Each primitive owns its logic directly. Do not create facade files that delegate to
   primitives.
 - **No backward-compatibility shims.** This is a CLI, not a library. If the CLI functions the same, delete old files.
-- **Use `{ success, error? }` result format** throughout (never `{ ok, error }`). See `AddResult` and `RemovalResult`.
+- **Use the discriminated `Result<T, E>` union** from `src/lib/result.ts` throughout. See typed error classes in
+  `src/lib/errors/types.ts`.
 - **Dynamic imports for ink/React only.** TUI components (ink, react, screen components) must be dynamically imported
   inside Commander action handlers to prevent esbuild async module propagation issues. All other imports go at the top
   of the file. See the esbuild section below.

src/cli/__tests__/errors.test.ts

@@ -1,5 +1,5 @@
+import { AgentAlreadyExistsError, toError } from '../../lib';
 import {
-  AgentAlreadyExistsError,
   getErrorMessage,
   isChangesetInProgressError,
   isExpiredTokenError,
@@ -204,4 +204,35 @@ describe('errors', () => {
       expect(isChangesetInProgressError({})).toBe(false);
     });
   });
+
+  describe('toError', () => {
+    it('returns Error instance as-is', () => {
+      const err = new Error('original');
+      expect(toError(err)).toBe(err);
+    });
+
+    it('wraps string in Error', () => {
+      const result = toError('string error');
+      expect(result).toBeInstanceOf(Error);
+      expect(result.message).toBe('string error');
+    });
+
+    it('handles null', () => {
+      const result = toError(null);
+      expect(result).toBeInstanceOf(Error);
+      expect(result.message).toBe('null');
+    });
+
+    it('handles undefined', () => {
+      const result = toError(undefined);
+      expect(result).toBeInstanceOf(Error);
+      expect(result.message).toBe('undefined');
+    });
+
+    it('handles non-Error objects', () => {
+      const result = toError({ code: 42 });
+      expect(result).toBeInstanceOf(Error);
+      expect(result.message).toBe('[object Object]');
+    });
+  });
 });

src/cli/aws/__tests__/transaction-search.test.ts

@@ -1,4 +1,5 @@
 import { enableTransactionSearch } from '../transaction-search.js';
+import assert from 'node:assert';
 import { beforeEach, describe, expect, it, vi } from 'vitest';
 
 const { mockAppSignalsSend, mockLogsSend, mockXRaySend } = vi.hoisted(() => ({
@@ -162,17 +163,17 @@ describe('enableTransactionSearch', () => {
 
       const result = await enableTransactionSearch('us-east-1', '123456789012');
 
-      expect(result.success).toBe(false);
-      expect(result.error).toContain('Insufficient permissions to enable Application Signals');
+      assert(!result.success);
+      expect(result.error.message).toContain('Insufficient permissions to enable Application Signals');
     });
 
     it('returns error when Application Signals fails with generic error', async () => {
       mockAppSignalsSend.mockRejectedValue(new Error('Service unavailable'));
 
       const result = await enableTransactionSearch('us-east-1', '123456789012');
 
-      expect(result.success).toBe(false);
-      expect(result.error).toContain('Failed to enable Application Signals');
+      assert(!result.success);
+      expect(result.error.message).toContain('Failed to enable Application Signals');
     });
 
     it('returns error when CloudWatch Logs policy fails with AccessDenied', async () => {
@@ -183,8 +184,8 @@ describe('enableTransactionSearch', () => {
 
       const result = await enableTransactionSearch('us-east-1', '123456789012');
 
-      expect(result.success).toBe(false);
-      expect(result.error).toContain('Insufficient permissions to configure CloudWatch Logs policy');
+      assert(!result.success);
+      expect(result.error.message).toContain('Insufficient permissions to configure CloudWatch Logs policy');
     });
 
     it('returns error when trace destination fails', async () => {
@@ -194,8 +195,8 @@ describe('enableTransactionSearch', () => {
 
       const result = await enableTransactionSearch('us-east-1', '123456789012');
 
-      expect(result.success).toBe(false);
-      expect(result.error).toContain('Failed to configure trace destination');
+      assert(!result.success);
+      expect(result.error.message).toContain('Failed to configure trace destination');
     });
 
     it('returns error when indexing rule update fails', async () => {
@@ -214,8 +215,8 @@ describe('enableTransactionSearch', () => {
 
       const result = await enableTransactionSearch('us-east-1', '123456789012');
 
-      expect(result.success).toBe(false);
-      expect(result.error).toContain('Failed to configure indexing rules');
+      assert(!result.success);
+      expect(result.error.message).toContain('Failed to configure indexing rules');
     });
 
     it('does not proceed to later steps when an earlier step fails', async () => {

src/cli/aws/index.ts

@@ -17,7 +17,7 @@ export {
   type GetAgentRuntimeStatusOptions,
 } from './agentcore-control';
 export { streamLogs, searchLogs, type LogEvent, type StreamLogsOptions, type SearchLogsOptions } from './cloudwatch';
-export { enableTransactionSearch, type TransactionSearchEnableResult } from './transaction-search';
+export { enableTransactionSearch } from './transaction-search';
 export {
   startPolicyGeneration,
   getPolicyGeneration,

src/cli/aws/transaction-search.ts

@@ -1,3 +1,5 @@
+import { AccessDeniedError } from '../../lib';
+import type { Result } from '../../lib/result';
 import { getErrorMessage, isAccessDeniedError } from '../errors';
 import { getCredentialProvider } from './account';
 import { arnPrefix } from './partition';
@@ -14,11 +16,6 @@ import {
   XRayClient,
 } from '@aws-sdk/client-xray';
 
-export interface TransactionSearchEnableResult {
-  success: boolean;
-  error?: string;
-}
-
 const RESOURCE_POLICY_NAME = 'TransactionSearchXRayAccess';
 
 /**
@@ -34,7 +31,7 @@ export async function enableTransactionSearch(
   region: string,
   accountId: string,
   indexPercentage = 100
-): Promise<TransactionSearchEnableResult> {
+): Promise<Result> {
   const credentials = getCredentialProvider();
 
   // Step 1: Enable Application Signals (creates service-linked role, idempotent)
@@ -44,9 +41,14 @@ export async function enableTransactionSearch(
   } catch (err: unknown) {
     const message = getErrorMessage(err);
     if (isAccessDeniedError(err)) {
-      return { success: false, error: `Insufficient permissions to enable Application Signals: ${message}` };
+      return {
+        success: false,
+        error: new AccessDeniedError(`Insufficient permissions to enable Application Signals: ${message}`, {
+          cause: err,
+        }),
+      };
     }
-    return { success: false, error: `Failed to enable Application Signals: ${message}` };
+    return { success: false, error: new Error(`Failed to enable Application Signals: ${message}`, { cause: err }) };
   }
 
   // Step 2: Create CloudWatch Logs resource policy for X-Ray (if needed)
@@ -80,9 +82,17 @@ export async function enableTransactionSearch(
   } catch (err: unknown) {
     const message = getErrorMessage(err);
     if (isAccessDeniedError(err)) {
-      return { success: false, error: `Insufficient permissions to configure CloudWatch Logs policy: ${message}` };
+      return {
+        success: false,
+        error: new AccessDeniedError(`Insufficient permissions to configure CloudWatch Logs policy: ${message}`, {
+          cause: err,
+        }),
+      };
     }
-    return { success: false, error: `Failed to configure CloudWatch Logs policy: ${message}` };
+    return {
+      success: false,
+      error: new Error(`Failed to configure CloudWatch Logs policy: ${message}`, { cause: err }),
+    };
   }
 
   const xrayClient = new XRayClient({ region, credentials });
@@ -96,9 +106,14 @@ export async function enableTransactionSearch(
   } catch (err: unknown) {
     const message = getErrorMessage(err);
     if (isAccessDeniedError(err)) {
-      return { success: false, error: `Insufficient permissions to configure trace destination: ${message}` };
+      return {
+        success: false,
+        error: new AccessDeniedError(`Insufficient permissions to configure trace destination: ${message}`, {
+          cause: err,
+        }),
+      };
     }
-    return { success: false, error: `Failed to configure trace destination: ${message}` };
+    return { success: false, error: new Error(`Failed to configure trace destination: ${message}`, { cause: err }) };
   }
 
   // Step 4: Set indexing to 100% on the built-in Default rule (always exists, idempotent)
@@ -112,9 +127,14 @@ export async function enableTransactionSearch(
   } catch (err: unknown) {
     const message = getErrorMessage(err);
     if (isAccessDeniedError(err)) {
-      return { success: false, error: `Insufficient permissions to configure indexing rules: ${message}` };
+      return {
+        success: false,
+        error: new AccessDeniedError(`Insufficient permissions to configure indexing rules: ${message}`, {
+          cause: err,
+        }),
+      };
     }
-    return { success: false, error: `Failed to configure indexing rules: ${message}` };
+    return { success: false, error: new Error(`Failed to configure indexing rules: ${message}`, { cause: err }) };
   }
 
   return { success: true };

src/cli/commands/add/types.ts

@@ -41,13 +41,6 @@ export interface AddAgentOptions extends VpcOptions {
   json?: boolean;
 }
 
-export interface AddAgentResult {
-  success: boolean;
-  agentName?: string;
-  agentPath?: string;
-  error?: string;
-}
-
 // Gateway types
 export interface AddGatewayOptions {
   name?: string;
@@ -68,12 +61,6 @@ export interface AddGatewayOptions {
   json?: boolean;
 }
 
-export interface AddGatewayResult {
-  success: boolean;
-  gatewayName?: string;
-  error?: string;
-}
-
 // Gateway Target types
 export interface AddGatewayTargetOptions {
   name?: string;
@@ -100,13 +87,6 @@ export interface AddGatewayTargetOptions {
   json?: boolean;
 }
 
-export interface AddGatewayTargetResult {
-  success: boolean;
-  toolName?: string;
-  sourcePath?: string;
-  error?: string;
-}
-
 // Memory types (v2: no owner/user concept)
 export interface AddMemoryOptions {
   name?: string;
@@ -119,12 +99,6 @@ export interface AddMemoryOptions {
   json?: boolean;
 }
 
-export interface AddMemoryResult {
-  success: boolean;
-  memoryName?: string;
-  error?: string;
-}
-
 // Credential types (v2: credential, no owner/user concept)
 export interface AddCredentialOptions {
   name?: string;
@@ -139,9 +113,3 @@ export interface AddCredentialOptions {
 
 /** @deprecated Use AddCredentialOptions */
 export type AddIdentityOptions = AddCredentialOptions;
-
-export interface AddCredentialResult {
-  success: boolean;
-  credentialName?: string;
-  error?: string;
-}