Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/cli/AGENTS.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
```

Expand All@@ -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
Expand All@@ -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.
Expand Down
33 changes: 32 additions & 1 deletion src/cli/__tests__/errors.test.ts
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
import { AgentAlreadyExistsError, toError } from '../../lib';
import {
AgentAlreadyExistsError,
getErrorMessage,
isChangesetInProgressError,
isExpiredTokenError,
Expand DownExpand Up@@ -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]');
});
});
});
21 changes: 11 additions & 10 deletions src/cli/aws/__tests__/transaction-search.test.ts
Original file line numberDiff line numberDiff line change
@@ -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(() => ({
Expand DownExpand Up@@ -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 () => {
Expand All@@ -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 () => {
Expand All@@ -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 () => {
Expand All@@ -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 () => {
Expand Down
2 changes: 1 addition & 1 deletion src/cli/aws/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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,
Expand Down
48 changes: 34 additions & 14 deletions src/cli/aws/transaction-search.ts
Original file line numberDiff line numberDiff line change
@@ -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';
Expand All@@ -14,11 +16,6 @@ import {
XRayClient,
} from '@aws-sdk/client-xray';

export interface TransactionSearchEnableResult {
success: boolean;
error?: string;
}

const RESOURCE_POLICY_NAME = 'TransactionSearchXRayAccess';

/**
Expand All@@ -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)
Expand All@@ -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)
Expand DownExpand Up@@ -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 });
Expand All@@ -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)
Expand All@@ -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 };
Expand Down
32 changes: 0 additions & 32 deletions src/cli/commands/add/types.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
Expand All@@ -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;
Expand All@@ -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;
Expand All@@ -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;
Expand All@@ -139,9 +113,3 @@ export interface AddCredentialOptions {

/** @deprecated Use AddCredentialOptions */
export type AddIdentityOptions = AddCredentialOptions;

export interface AddCredentialResult {
success: boolean;
credentialName?: string;
error?: string;
}
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
refactor: unify result types with discriminated Result<T, E> union by Hweinstock · Pull Request #1125 · aws/agentcore-cli · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/cli/AGENTS.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
```

Expand All@@ -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
Expand All@@ -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.
Expand Down
33 changes: 32 additions & 1 deletion src/cli/__tests__/errors.test.ts
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
import { AgentAlreadyExistsError, toError } from '../../lib';
import {
AgentAlreadyExistsError,
getErrorMessage,
isChangesetInProgressError,
isExpiredTokenError,
Expand DownExpand Up@@ -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]');
});
});
});
21 changes: 11 additions & 10 deletions src/cli/aws/__tests__/transaction-search.test.ts
Original file line numberDiff line numberDiff line change
@@ -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(() => ({
Expand DownExpand Up@@ -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 () => {
Expand All@@ -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 () => {
Expand All@@ -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 () => {
Expand All@@ -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 () => {
Expand Down
2 changes: 1 addition & 1 deletion src/cli/aws/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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,
Expand Down
48 changes: 34 additions & 14 deletions src/cli/aws/transaction-search.ts
Original file line numberDiff line numberDiff line change
@@ -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';
Expand All@@ -14,11 +16,6 @@ import {
XRayClient,
} from '@aws-sdk/client-xray';

export interface TransactionSearchEnableResult {
success: boolean;
error?: string;
}

const RESOURCE_POLICY_NAME = 'TransactionSearchXRayAccess';

/**
Expand All@@ -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)
Expand All@@ -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)
Expand DownExpand Up@@ -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 });
Expand All@@ -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)
Expand All@@ -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 };
Expand Down
32 changes: 0 additions & 32 deletions src/cli/commands/add/types.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
Expand All@@ -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;
Expand All@@ -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;
Expand All@@ -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;
Expand All@@ -139,9 +113,3 @@ export interface AddCredentialOptions {

/** @deprecated Use AddCredentialOptions */
export type AddIdentityOptions = AddCredentialOptions;

export interface AddCredentialResult {
success: boolean;
credentialName?: string;
error?: string;
}
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' refactor: unify result types with discriminated Result<T, E> union by Hweinstock · Pull Request #1125 · aws/agentcore-cli · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/cli/AGENTS.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
```

Expand All@@ -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
Expand All@@ -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.
Expand Down
33 changes: 32 additions & 1 deletion src/cli/__tests__/errors.test.ts
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
import { AgentAlreadyExistsError, toError } from '../../lib';
import {
AgentAlreadyExistsError,
getErrorMessage,
isChangesetInProgressError,
isExpiredTokenError,
Expand DownExpand Up@@ -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]');
});
});
});
21 changes: 11 additions & 10 deletions src/cli/aws/__tests__/transaction-search.test.ts
Original file line numberDiff line numberDiff line change
@@ -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(() => ({
Expand DownExpand Up@@ -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 () => {
Expand All@@ -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 () => {
Expand All@@ -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 () => {
Expand All@@ -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 () => {
Expand Down
2 changes: 1 addition & 1 deletion src/cli/aws/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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,
Expand Down
48 changes: 34 additions & 14 deletions src/cli/aws/transaction-search.ts
Original file line numberDiff line numberDiff line change
@@ -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';
Expand All@@ -14,11 +16,6 @@ import {
XRayClient,
} from '@aws-sdk/client-xray';

export interface TransactionSearchEnableResult {
success: boolean;
error?: string;
}

const RESOURCE_POLICY_NAME = 'TransactionSearchXRayAccess';

/**
Expand All@@ -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)
Expand All@@ -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)
Expand DownExpand Up@@ -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 });
Expand All@@ -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)
Expand All@@ -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 };
Expand Down
32 changes: 0 additions & 32 deletions src/cli/commands/add/types.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
Expand All@@ -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;
Expand All@@ -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;
Expand All@@ -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;
Expand All@@ -139,9 +113,3 @@ export interface AddCredentialOptions {

/** @deprecated Use AddCredentialOptions */
export type AddIdentityOptions = AddCredentialOptions;

export interface AddCredentialResult {
success: boolean;
credentialName?: string;
error?: string;
}
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' refactor: unify result types with discriminated Result<T, E> union by Hweinstock · Pull Request #1125 · aws/agentcore-cli · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/cli/AGENTS.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
```

Expand All@@ -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
Expand All@@ -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.
Expand Down
33 changes: 32 additions & 1 deletion src/cli/__tests__/errors.test.ts
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
import { AgentAlreadyExistsError, toError } from '../../lib';
import {
AgentAlreadyExistsError,
getErrorMessage,
isChangesetInProgressError,
isExpiredTokenError,
Expand DownExpand Up@@ -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]');
});
});
});
21 changes: 11 additions & 10 deletions src/cli/aws/__tests__/transaction-search.test.ts
Original file line numberDiff line numberDiff line change
@@ -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(() => ({
Expand DownExpand Up@@ -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 () => {
Expand All@@ -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 () => {
Expand All@@ -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 () => {
Expand All@@ -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 () => {
Expand Down
2 changes: 1 addition & 1 deletion src/cli/aws/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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,
Expand Down
48 changes: 34 additions & 14 deletions src/cli/aws/transaction-search.ts
Original file line numberDiff line numberDiff line change
@@ -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';
Expand All@@ -14,11 +16,6 @@ import {
XRayClient,
} from '@aws-sdk/client-xray';

export interface TransactionSearchEnableResult {
success: boolean;
error?: string;
}

const RESOURCE_POLICY_NAME = 'TransactionSearchXRayAccess';

/**
Expand All@@ -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)
Expand All@@ -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)
Expand DownExpand Up@@ -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 });
Expand All@@ -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)
Expand All@@ -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 };
Expand Down
32 changes: 0 additions & 32 deletions src/cli/commands/add/types.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
Expand All@@ -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;
Expand All@@ -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;
Expand All@@ -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;
Expand All@@ -139,9 +113,3 @@ export interface AddCredentialOptions {

/** @deprecated Use AddCredentialOptions */
export type AddIdentityOptions = AddCredentialOptions;

export interface AddCredentialResult {
success: boolean;
credentialName?: string;
error?: string;
}
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' refactor: unify result types with discriminated Result<T, E> union by Hweinstock · Pull Request #1125 · aws/agentcore-cli · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/cli/AGENTS.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
```

Expand All@@ -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
Expand All@@ -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.
Expand Down
33 changes: 32 additions & 1 deletion src/cli/__tests__/errors.test.ts
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
import { AgentAlreadyExistsError, toError } from '../../lib';
import {
AgentAlreadyExistsError,
getErrorMessage,
isChangesetInProgressError,
isExpiredTokenError,
Expand DownExpand Up@@ -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]');
});
});
});
21 changes: 11 additions & 10 deletions src/cli/aws/__tests__/transaction-search.test.ts
Original file line numberDiff line numberDiff line change
@@ -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(() => ({
Expand DownExpand Up@@ -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 () => {
Expand All@@ -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 () => {
Expand All@@ -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 () => {
Expand All@@ -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 () => {
Expand Down
2 changes: 1 addition & 1 deletion src/cli/aws/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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,
Expand Down
48 changes: 34 additions & 14 deletions src/cli/aws/transaction-search.ts
Original file line numberDiff line numberDiff line change
@@ -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';
Expand All@@ -14,11 +16,6 @@ import {
XRayClient,
} from '@aws-sdk/client-xray';

export interface TransactionSearchEnableResult {
success: boolean;
error?: string;
}

const RESOURCE_POLICY_NAME = 'TransactionSearchXRayAccess';

/**
Expand All@@ -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)
Expand All@@ -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)
Expand DownExpand Up@@ -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 });
Expand All@@ -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)
Expand All@@ -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 };
Expand Down
32 changes: 0 additions & 32 deletions src/cli/commands/add/types.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
Expand All@@ -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;
Expand All@@ -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;
Expand All@@ -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;
Expand All@@ -139,9 +113,3 @@ export interface AddCredentialOptions {

/** @deprecated Use AddCredentialOptions */
export type AddIdentityOptions = AddCredentialOptions;

export interface AddCredentialResult {
success: boolean;
credentialName?: string;
error?: string;
}
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' refactor: unify result types with discriminated Result<T, E> union by Hweinstock · Pull Request #1125 · aws/agentcore-cli · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/cli/AGENTS.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
```

Expand All@@ -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
Expand All@@ -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.
Expand Down
33 changes: 32 additions & 1 deletion src/cli/__tests__/errors.test.ts
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
import { AgentAlreadyExistsError, toError } from '../../lib';
import {
AgentAlreadyExistsError,
getErrorMessage,
isChangesetInProgressError,
isExpiredTokenError,
Expand DownExpand Up@@ -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]');
});
});
});
21 changes: 11 additions & 10 deletions src/cli/aws/__tests__/transaction-search.test.ts
Original file line numberDiff line numberDiff line change
@@ -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(() => ({
Expand DownExpand Up@@ -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 () => {
Expand All@@ -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 () => {
Expand All@@ -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 () => {
Expand All@@ -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 () => {
Expand Down
2 changes: 1 addition & 1 deletion src/cli/aws/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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,
Expand Down
48 changes: 34 additions & 14 deletions src/cli/aws/transaction-search.ts
Original file line numberDiff line numberDiff line change
@@ -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';
Expand All@@ -14,11 +16,6 @@ import {
XRayClient,
} from '@aws-sdk/client-xray';

export interface TransactionSearchEnableResult {
success: boolean;
error?: string;
}

const RESOURCE_POLICY_NAME = 'TransactionSearchXRayAccess';

/**
Expand All@@ -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)
Expand All@@ -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)
Expand DownExpand Up@@ -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 });
Expand All@@ -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)
Expand All@@ -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 };
Expand Down
32 changes: 0 additions & 32 deletions src/cli/commands/add/types.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
Expand All@@ -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;
Expand All@@ -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;
Expand All@@ -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;
Expand All@@ -139,9 +113,3 @@ export interface AddCredentialOptions {

/** @deprecated Use AddCredentialOptions */
export type AddIdentityOptions = AddCredentialOptions;

export interface AddCredentialResult {
success: boolean;
credentialName?: string;
error?: string;
}
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' refactor: unify result types with discriminated Result<T, E> union by Hweinstock · Pull Request #1125 · aws/agentcore-cli · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/cli/AGENTS.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
```

Expand All@@ -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
Expand All@@ -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.
Expand Down
33 changes: 32 additions & 1 deletion src/cli/__tests__/errors.test.ts
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
import { AgentAlreadyExistsError, toError } from '../../lib';
import {
AgentAlreadyExistsError,
getErrorMessage,
isChangesetInProgressError,
isExpiredTokenError,
Expand DownExpand Up@@ -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]');
});
});
});
21 changes: 11 additions & 10 deletions src/cli/aws/__tests__/transaction-search.test.ts
Original file line numberDiff line numberDiff line change
@@ -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(() => ({
Expand DownExpand Up@@ -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 () => {
Expand All@@ -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 () => {
Expand All@@ -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 () => {
Expand All@@ -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 () => {
Expand Down
2 changes: 1 addition & 1 deletion src/cli/aws/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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,
Expand Down
48 changes: 34 additions & 14 deletions src/cli/aws/transaction-search.ts
Original file line numberDiff line numberDiff line change
@@ -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';
Expand All@@ -14,11 +16,6 @@ import {
XRayClient,
} from '@aws-sdk/client-xray';

export interface TransactionSearchEnableResult {
success: boolean;
error?: string;
}

const RESOURCE_POLICY_NAME = 'TransactionSearchXRayAccess';

/**
Expand All@@ -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)
Expand All@@ -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)
Expand DownExpand Up@@ -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 });
Expand All@@ -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)
Expand All@@ -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 };
Expand Down
32 changes: 0 additions & 32 deletions src/cli/commands/add/types.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
Expand All@@ -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;
Expand All@@ -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;
Expand All@@ -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;
Expand All@@ -139,9 +113,3 @@ export interface AddCredentialOptions {

/** @deprecated Use AddCredentialOptions */
export type AddIdentityOptions = AddCredentialOptions;

export interface AddCredentialResult {
success: boolean;
credentialName?: string;
error?: string;
}
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); refactor: unify result types with discriminated Result<T, E> union by Hweinstock · Pull Request #1125 · aws/agentcore-cli · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/cli/AGENTS.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
```

Expand All@@ -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
Expand All@@ -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.
Expand Down
33 changes: 32 additions & 1 deletion src/cli/__tests__/errors.test.ts
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
import { AgentAlreadyExistsError, toError } from '../../lib';
import {
AgentAlreadyExistsError,
getErrorMessage,
isChangesetInProgressError,
isExpiredTokenError,
Expand DownExpand Up@@ -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]');
});
});
});
21 changes: 11 additions & 10 deletions src/cli/aws/__tests__/transaction-search.test.ts
Original file line numberDiff line numberDiff line change
@@ -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(() => ({
Expand DownExpand Up@@ -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 () => {
Expand All@@ -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 () => {
Expand All@@ -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 () => {
Expand All@@ -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 () => {
Expand Down
2 changes: 1 addition & 1 deletion src/cli/aws/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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,
Expand Down
48 changes: 34 additions & 14 deletions src/cli/aws/transaction-search.ts
Original file line numberDiff line numberDiff line change
@@ -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';
Expand All@@ -14,11 +16,6 @@ import {
XRayClient,
} from '@aws-sdk/client-xray';

export interface TransactionSearchEnableResult {
success: boolean;
error?: string;
}

const RESOURCE_POLICY_NAME = 'TransactionSearchXRayAccess';

/**
Expand All@@ -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)
Expand All@@ -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)
Expand DownExpand Up@@ -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 });
Expand All@@ -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)
Expand All@@ -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 };
Expand Down
32 changes: 0 additions & 32 deletions src/cli/commands/add/types.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
Expand All@@ -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;
Expand All@@ -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;
Expand All@@ -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;
Expand All@@ -139,9 +113,3 @@ export interface AddCredentialOptions {

/** @deprecated Use AddCredentialOptions */
export type AddIdentityOptions = AddCredentialOptions;

export interface AddCredentialResult {
success: boolean;
credentialName?: string;
error?: string;
}
Loading
Loading