diff --git a/.github/workflows/sync-from-public.yml b/.github/workflows/sync-from-public.yml index e38a4d625..127257392 100644 --- a/.github/workflows/sync-from-public.yml +++ b/.github/workflows/sync-from-public.yml @@ -37,8 +37,7 @@ jobs: - name: Sync main with public/main run: | - git checkout main - git reset --hard origin/main + git checkout -B main origin/main # Check if public/main is already merged if git merge-base --is-ancestor public/main HEAD; then @@ -137,8 +136,7 @@ jobs: - name: Sync preview with public/preview run: | - git checkout preview - git reset --hard origin/preview + git checkout -B preview origin/preview # Check if public/preview is already merged if git merge-base --is-ancestor public/preview HEAD; then diff --git a/integ-tests/create-edge-cases.test.ts b/integ-tests/create-edge-cases.test.ts index d1bbcf056..5b2d299fa 100644 --- a/integ-tests/create-edge-cases.test.ts +++ b/integ-tests/create-edge-cases.test.ts @@ -1,5 +1,6 @@ /* eslint-disable security/detect-non-literal-fs-filename */ import { exists, prereqs, runCLI } from '../src/test-utils/index.js'; +import { createTelemetryHelper } from '../src/test-utils/telemetry-helper.js'; import { randomUUID } from 'node:crypto'; import { mkdir, rm } from 'node:fs/promises'; import { tmpdir } from 'node:os'; @@ -9,18 +10,21 @@ import { afterAll, beforeAll, describe, expect, it } from 'vitest'; describe.skipIf(!prereqs.npm || !prereqs.git)('integration: create edge cases', () => { let testDir: string; + const telemetry = createTelemetryHelper(); + beforeAll(async () => { testDir = join(tmpdir(), `agentcore-integ-edge-${randomUUID()}`); await mkdir(testDir, { recursive: true }); }); afterAll(async () => { + telemetry.destroy(); await rm(testDir, { recursive: true, force: true }); }); describe('reserved names', () => { it('rejects reserved name "Test"', async () => { - const result = await runCLI(['create', '--name', 'Test', '--json'], testDir); + const result = await runCLI(['create', '--name', 'Test', '--json'], testDir, { env: telemetry.env }); expect(result.exitCode).toBe(1); const json = JSON.parse(result.stdout); @@ -30,6 +34,13 @@ describe.skipIf(!prereqs.npm || !prereqs.git)('integration: create edge cases', json.error.toLowerCase().includes('reserved') || json.error.toLowerCase().includes('conflict'), `Error should mention reserved/conflict: ${json.error}` ).toBeTruthy(); + + telemetry.assertMetricEmitted({ + command: 'create', + exit_reason: 'failure', + language: 'python', + has_agent: 'true', + }); }); it('rejects reserved name "bedrock"', async () => { @@ -121,12 +132,21 @@ describe.skipIf(!prereqs.npm || !prereqs.git)('integration: create edge cases', describe('flag interactions', () => { it('--defaults creates project with default settings', async () => { const name = `Def${Date.now().toString().slice(-6)}`; - const result = await runCLI(['create', '--name', name, '--defaults', '--json'], testDir); + const result = await runCLI(['create', '--name', name, '--defaults', '--json'], testDir, { env: telemetry.env }); expect(result.exitCode, `stderr: ${result.stderr}`).toBe(0); const json = JSON.parse(result.stdout); expect(json.success).toBe(true); expect(json.projectPath).toBeTruthy(); + + telemetry.assertMetricEmitted({ + command: 'create', + exit_reason: 'success', + language: 'python', + framework: 'strands', + model_provider: 'bedrock', + has_agent: 'true', + }); }); it('--dry-run shows what would be created without writing files', async () => { diff --git a/integ-tests/create-frameworks.test.ts b/integ-tests/create-frameworks.test.ts index 82bbc0871..1be05d237 100644 --- a/integ-tests/create-frameworks.test.ts +++ b/integ-tests/create-frameworks.test.ts @@ -1,4 +1,5 @@ import { exists, prereqs, readProjectConfig, runCLI } from '../src/test-utils/index.js'; +import { createTelemetryHelper } from '../src/test-utils/telemetry-helper.js'; import { randomUUID } from 'node:crypto'; import { mkdir, readFile, rm } from 'node:fs/promises'; import { tmpdir } from 'node:os'; @@ -8,12 +9,15 @@ import { afterAll, beforeAll, describe, expect, it } from 'vitest'; describe.skipIf(!prereqs.npm || !prereqs.git)('integration: create with different frameworks', () => { let testDir: string; + const telemetry = createTelemetryHelper(); + beforeAll(async () => { testDir = join(tmpdir(), `agentcore-integ-frameworks-${randomUUID()}`); await mkdir(testDir, { recursive: true }); }); afterAll(async () => { + telemetry.destroy(); await rm(testDir, { recursive: true, force: true }); }); @@ -34,7 +38,8 @@ describe.skipIf(!prereqs.npm || !prereqs.git)('integration: create with differen 'none', '--json', ], - testDir + testDir, + { env: telemetry.env } ); expect(result.exitCode, `stderr: ${result.stderr}`).toBe(0); @@ -60,6 +65,15 @@ describe.skipIf(!prereqs.npm || !prereqs.git)('integration: create with differen expect(agents).toBeDefined(); expect(agents.length).toBe(1); expect(agents[0]!.name).toBe(agentName); + + telemetry.assertMetricEmitted({ + command: 'create', + exit_reason: 'success', + language: 'python', + framework: 'langchain_langgraph', + model_provider: 'bedrock', + has_agent: 'true', + }); }); it('creates GoogleADK project with Gemini provider', async () => { diff --git a/package-lock.json b/package-lock.json index 96fa23e72..cf1b80903 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4213,6 +4213,8 @@ "license": "Apache-2.0", "dependencies": { "@opentelemetry/core": "2.7.0", +<<<<<<< HEAD +======= "@opentelemetry/semantic-conventions": "^1.29.0" }, "engines": { @@ -4256,6 +4258,70 @@ "@opentelemetry/api": ">=1.9.0 <1.10.0" } }, + "node_modules/@opentelemetry/exporter-metrics-otlp-http/node_modules/@opentelemetry/sdk-trace-base": { + "version": "2.7.0", + "resolved": "https://registry.npmjs.org/@opentelemetry/sdk-trace-base/-/sdk-trace-base-2.7.0.tgz", + "integrity": "sha512-Yg9zEXJB50DLVLpsKPk7NmNqlPlS+OvqhJGh0A8oawIOTPOwlm4eXs9BMJV7L79lvEwI+dWtAj+YjTyddV336A==", + "license": "Apache-2.0", + "dependencies": { + "@opentelemetry/core": "2.7.0", + "@opentelemetry/resources": "2.7.0", +>>>>>>> origin/main + "@opentelemetry/semantic-conventions": "^1.29.0" + }, + "engines": { + "node": "^18.19.0 || >=20.6.0" + }, + "peerDependencies": { + "@opentelemetry/api": ">=1.3.0 <1.10.0" + } + }, +<<<<<<< HEAD + "node_modules/@opentelemetry/exporter-metrics-otlp-http/node_modules/@opentelemetry/sdk-logs": { + "version": "0.215.0", + "resolved": "https://registry.npmjs.org/@opentelemetry/sdk-logs/-/sdk-logs-0.215.0.tgz", + "integrity": "sha512-y3ucOmphzc4vgBTyIGchs+N/1rkACmoka8QalT2z1LBNM232Z17zMYayHcMl+dgMoOadZ0b72UZv7mDtqy1cFA==", + "license": "Apache-2.0", + "dependencies": { + "@opentelemetry/api-logs": "0.215.0", + "@opentelemetry/core": "2.7.0", + "@opentelemetry/resources": "2.7.0", + "@opentelemetry/semantic-conventions": "^1.29.0" +======= + "node_modules/@opentelemetry/exporter-metrics-otlp-http/node_modules/protobufjs": { + "version": "8.2.0", + "resolved": "https://registry.npmjs.org/protobufjs/-/protobufjs-8.2.0.tgz", + "integrity": "sha512-oI+GC9iPxrQEr6wragljFKH46/r3rNsm6eg7F2fp6kBUMnf6/mesDRdBuF4gK+OyaKJ8N4C1B9s9cCeYdqFikg==", + "hasInstallScript": true, + "license": "BSD-3-Clause", + "dependencies": { + "@types/node": ">=13.7.0", + "long": "^5.0.0" +>>>>>>> origin/main + }, + "engines": { + "node": "^18.19.0 || >=20.6.0" + }, + "peerDependencies": { + "@opentelemetry/api": ">=1.4.0 <1.10.0" + } + }, + "node_modules/@opentelemetry/exporter-metrics-otlp-http/node_modules/@opentelemetry/sdk-metrics": { + "version": "2.7.0", + "resolved": "https://registry.npmjs.org/@opentelemetry/sdk-metrics/-/sdk-metrics-2.7.0.tgz", + "integrity": "sha512-Vd7h95av/LYRsAVN7wbprvvJnHkq7swMXAo7Uad0Uxf9jl6NSReLa0JNivrcc5BVIx/vl2t+cgdVQQbnVhsR9w==", + "license": "Apache-2.0", + "dependencies": { + "@opentelemetry/core": "2.7.0", + "@opentelemetry/resources": "2.7.0" + }, + "engines": { + "node": "^18.19.0 || >=20.6.0" + }, + "peerDependencies": { + "@opentelemetry/api": ">=1.9.0 <1.10.0" + } + }, "node_modules/@opentelemetry/exporter-metrics-otlp-http/node_modules/@opentelemetry/sdk-trace-base": { "version": "2.7.0", "resolved": "https://registry.npmjs.org/@opentelemetry/sdk-trace-base/-/sdk-trace-base-2.7.0.tgz", @@ -4344,6 +4410,8 @@ "license": "Apache-2.0", "dependencies": { "@opentelemetry/core": "2.7.0", +<<<<<<< HEAD +======= "@opentelemetry/semantic-conventions": "^1.29.0" }, "engines": { @@ -4387,6 +4455,70 @@ "@opentelemetry/api": ">=1.9.0 <1.10.0" } }, + "node_modules/@opentelemetry/otlp-exporter-base/node_modules/@opentelemetry/sdk-trace-base": { + "version": "2.7.0", + "resolved": "https://registry.npmjs.org/@opentelemetry/sdk-trace-base/-/sdk-trace-base-2.7.0.tgz", + "integrity": "sha512-Yg9zEXJB50DLVLpsKPk7NmNqlPlS+OvqhJGh0A8oawIOTPOwlm4eXs9BMJV7L79lvEwI+dWtAj+YjTyddV336A==", + "license": "Apache-2.0", + "dependencies": { + "@opentelemetry/core": "2.7.0", + "@opentelemetry/resources": "2.7.0", +>>>>>>> origin/main + "@opentelemetry/semantic-conventions": "^1.29.0" + }, + "engines": { + "node": "^18.19.0 || >=20.6.0" + }, + "peerDependencies": { + "@opentelemetry/api": ">=1.3.0 <1.10.0" + } + }, +<<<<<<< HEAD + "node_modules/@opentelemetry/otlp-exporter-base/node_modules/@opentelemetry/sdk-logs": { + "version": "0.215.0", + "resolved": "https://registry.npmjs.org/@opentelemetry/sdk-logs/-/sdk-logs-0.215.0.tgz", + "integrity": "sha512-y3ucOmphzc4vgBTyIGchs+N/1rkACmoka8QalT2z1LBNM232Z17zMYayHcMl+dgMoOadZ0b72UZv7mDtqy1cFA==", + "license": "Apache-2.0", + "dependencies": { + "@opentelemetry/api-logs": "0.215.0", + "@opentelemetry/core": "2.7.0", + "@opentelemetry/resources": "2.7.0", + "@opentelemetry/semantic-conventions": "^1.29.0" +======= + "node_modules/@opentelemetry/otlp-exporter-base/node_modules/protobufjs": { + "version": "8.2.0", + "resolved": "https://registry.npmjs.org/protobufjs/-/protobufjs-8.2.0.tgz", + "integrity": "sha512-oI+GC9iPxrQEr6wragljFKH46/r3rNsm6eg7F2fp6kBUMnf6/mesDRdBuF4gK+OyaKJ8N4C1B9s9cCeYdqFikg==", + "hasInstallScript": true, + "license": "BSD-3-Clause", + "dependencies": { + "@types/node": ">=13.7.0", + "long": "^5.0.0" +>>>>>>> origin/main + }, + "engines": { + "node": "^18.19.0 || >=20.6.0" + }, + "peerDependencies": { + "@opentelemetry/api": ">=1.4.0 <1.10.0" + } + }, + "node_modules/@opentelemetry/otlp-exporter-base/node_modules/@opentelemetry/sdk-metrics": { + "version": "2.7.0", + "resolved": "https://registry.npmjs.org/@opentelemetry/sdk-metrics/-/sdk-metrics-2.7.0.tgz", + "integrity": "sha512-Vd7h95av/LYRsAVN7wbprvvJnHkq7swMXAo7Uad0Uxf9jl6NSReLa0JNivrcc5BVIx/vl2t+cgdVQQbnVhsR9w==", + "license": "Apache-2.0", + "dependencies": { + "@opentelemetry/core": "2.7.0", + "@opentelemetry/resources": "2.7.0" + }, + "engines": { + "node": "^18.19.0 || >=20.6.0" + }, + "peerDependencies": { + "@opentelemetry/api": ">=1.9.0 <1.10.0" + } + }, "node_modules/@opentelemetry/otlp-exporter-base/node_modules/@opentelemetry/sdk-trace-base": { "version": "2.7.0", "resolved": "https://registry.npmjs.org/@opentelemetry/sdk-trace-base/-/sdk-trace-base-2.7.0.tgz", @@ -4648,6 +4780,73 @@ "node": ">=18" } }, +<<<<<<< HEAD +======= + "node_modules/@protobufjs/aspromise": { + "version": "1.1.2", + "resolved": "https://registry.npmjs.org/@protobufjs/aspromise/-/aspromise-1.1.2.tgz", + "integrity": "sha512-j+gKExEuLmKwvz3OgROXtrJ2UG2x8Ch2YZUxahh+s1F2HZ+wAceUNLkvy6zKCPVRkU++ZWQrdxsUeQXmcg4uoQ==", + "license": "BSD-3-Clause" + }, + "node_modules/@protobufjs/base64": { + "version": "1.1.2", + "resolved": "https://registry.npmjs.org/@protobufjs/base64/-/base64-1.1.2.tgz", + "integrity": "sha512-AZkcAA5vnN/v4PDqKyMR5lx7hZttPDgClv83E//FMNhR2TMcLUhfRUBHCmSl0oi9zMgDDqRUJkSxO3wm85+XLg==", + "license": "BSD-3-Clause" + }, + "node_modules/@protobufjs/codegen": { + "version": "2.0.5", + "resolved": "https://registry.npmjs.org/@protobufjs/codegen/-/codegen-2.0.5.tgz", + "integrity": "sha512-zgXFLzW3Ap33e6d0Wlj4MGIm6Ce8O89n/apUaGNB/jx+hw+ruWEp7EwGUshdLKVRCxZW12fp9r40E1mQrf/34g==", + "license": "BSD-3-Clause" + }, + "node_modules/@protobufjs/eventemitter": { + "version": "1.1.0", + "resolved": "https://registry.npmjs.org/@protobufjs/eventemitter/-/eventemitter-1.1.0.tgz", + "integrity": "sha512-j9ednRT81vYJ9OfVuXG6ERSTdEL1xVsNgqpkxMsbIabzSo3goCjDIveeGv5d03om39ML71RdmrGNjG5SReBP/Q==", + "license": "BSD-3-Clause" + }, + "node_modules/@protobufjs/fetch": { + "version": "1.1.0", + "resolved": "https://registry.npmjs.org/@protobufjs/fetch/-/fetch-1.1.0.tgz", + "integrity": "sha512-lljVXpqXebpsijW71PZaCYeIcE5on1w5DlQy5WH6GLbFryLUrBD4932W/E2BSpfRJWseIL4v/KPgBFxDOIdKpQ==", + "license": "BSD-3-Clause", + "dependencies": { + "@protobufjs/aspromise": "^1.1.1", + "@protobufjs/inquire": "^1.1.0" + } + }, + "node_modules/@protobufjs/float": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/@protobufjs/float/-/float-1.0.2.tgz", + "integrity": "sha512-Ddb+kVXlXst9d+R9PfTIxh1EdNkgoRe5tOX6t01f1lYWOvJnSPDBlG241QLzcyPdoNTsblLUdujGSE4RzrTZGQ==", + "license": "BSD-3-Clause" + }, + "node_modules/@protobufjs/inquire": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/@protobufjs/inquire/-/inquire-1.1.1.tgz", + "integrity": "sha512-mnzgDV26ueAvk7rsbt9L7bE0SuAoqyuys/sMMrmVcN5x9VsxpcG3rqAUSgDyLp0UZlmNfIbQ4fHfCtreVBk8Ew==", + "license": "BSD-3-Clause" + }, + "node_modules/@protobufjs/path": { + "version": "1.1.2", + "resolved": "https://registry.npmjs.org/@protobufjs/path/-/path-1.1.2.tgz", + "integrity": "sha512-6JOcJ5Tm08dOHAbdR3GrvP+yUUfkjG5ePsHYczMFLq3ZmMkAD98cDgcT2iA1lJ9NVwFd4tH/iSSoe44YWkltEA==", + "license": "BSD-3-Clause" + }, + "node_modules/@protobufjs/pool": { + "version": "1.1.0", + "resolved": "https://registry.npmjs.org/@protobufjs/pool/-/pool-1.1.0.tgz", + "integrity": "sha512-0kELaGSIDBKvcgS4zkjz1PeddatrjYcmMWOlAuAPwAeccUrPHdUqo/J6LiymHHEiJT5NrF1UVwxY14f+fy4WQw==", + "license": "BSD-3-Clause" + }, + "node_modules/@protobufjs/utf8": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/@protobufjs/utf8/-/utf8-1.1.1.tgz", + "integrity": "sha512-oOAWABowe8EAbMyWKM0tYDKi8Yaox52D+HWZhAIJqQXbqe0xI/GV7FhLWqlEKreMkfDjshR5FKgi3mnle0h6Eg==", + "license": "BSD-3-Clause" + }, +>>>>>>> origin/main "node_modules/@rolldown/binding-android-arm64": { "version": "1.0.0-rc.17", "resolved": "https://registry.npmjs.org/@rolldown/binding-android-arm64/-/binding-android-arm64-1.0.0-rc.17.tgz", @@ -13762,12 +13961,31 @@ } }, "node_modules/protobufjs": { +<<<<<<< HEAD "version": "8.0.3", "resolved": "https://registry.npmjs.org/protobufjs/-/protobufjs-8.0.3.tgz", "integrity": "sha512-LBYnMWkKLB8fE/ljROPDbCl7mgLSlI+oBe1fAAr5MTqFg4TIi0tYrVVurJvQggOjnUYMQtEZBjrej59ojMNTHQ==", "hasInstallScript": true, "license": "BSD-3-Clause", "dependencies": { +======= + "version": "7.5.8", + "resolved": "https://registry.npmjs.org/protobufjs/-/protobufjs-7.5.8.tgz", + "integrity": "sha512-dvpCIeLPbXZS/Ete7yLaO7RenOdken2NHKykBXbsaGxZT0UTltcarBciw+A78SRQs9iMAAVpsYA+l8b1hTePIA==", + "hasInstallScript": true, + "license": "BSD-3-Clause", + "dependencies": { + "@protobufjs/aspromise": "^1.1.2", + "@protobufjs/base64": "^1.1.2", + "@protobufjs/codegen": "^2.0.5", + "@protobufjs/eventemitter": "^1.1.0", + "@protobufjs/fetch": "^1.1.0", + "@protobufjs/float": "^1.0.2", + "@protobufjs/inquire": "^1.1.1", + "@protobufjs/path": "^1.1.2", + "@protobufjs/pool": "^1.1.0", + "@protobufjs/utf8": "^1.1.1", +>>>>>>> origin/main "@types/node": ">=13.7.0", "long": "^5.0.0" }, diff --git a/package.json b/package.json index 5f040158e..cd4273f2d 100644 --- a/package.json +++ b/package.json @@ -151,15 +151,23 @@ "minimatch": "GHSA-7r86-cg39-jmmj, GHSA-23c5-xmqv-rm74: minimatch 10.0.0-10.2.2 has ReDoS vulnerabilities. Multiple transitive deps (eslint, typescript-eslint, eslint-plugin-import, eslint-plugin-react, prettier-plugin-sort-imports, aws-cdk-lib) pin older versions. Remove this override once upstream packages update their minimatch dependency to >=10.2.3.", "glob": "glob <12 is deprecated and emits npm install warnings (https://github.com/isaacs/node-glob). Pulled in transitively via archiver-utils@5.0.2 (latest), which still pins glob@^10.0.0. archiver-utils only uses glob.sync(pattern, options), which remains compatible in glob@13. Remove this override once archiver-utils updates its glob dependency.", "fast-xml-parser": "GHSA-8gc5-j5rx-235r, GHSA-jp2q-39xq-3w4g: fast-xml-parser <=5.5.6 has entity expansion bypass (CVE-2026-33036, CVE-2026-33349). Transitive via @aws-sdk/xml-builder. Remove once @aws-sdk updates to fast-xml-parser >=5.5.7.", +<<<<<<< HEAD "@aws-sdk/xml-builder": "aws/aws-sdk-js-v3#7867: @aws-sdk/xml-builder <3.972.14 does not configure maxTotalExpansions on fast-xml-parser, causing 'Entity expansion limit exceeded' on large CloudFormation responses. Remove once @aws-sdk/client-* deps are bumped past 3.972.14.", "protobufjs": "GHSA-q6x5-8v7m-xcrf, GHSA-2pr8-phx7-x9h3: protobufjs <=8.0.1 has overlong UTF-8 decoding and DoS vulnerabilities. Transitive via @opentelemetry/otlp-transformer. Remove once @opentelemetry updates." +======= + "@aws-sdk/xml-builder": "aws/aws-sdk-js-v3#7867: @aws-sdk/xml-builder <3.972.14 does not configure maxTotalExpansions on fast-xml-parser, causing 'Entity expansion limit exceeded' on large CloudFormation responses. Remove once @aws-sdk/client-* deps are bumped past 3.972.14." +>>>>>>> origin/main }, "overrides": { "minimatch": "10.2.4", "glob": "^13.0.0", "fast-xml-parser": "5.5.7", +<<<<<<< HEAD "@aws-sdk/xml-builder": "3.972.15", "protobufjs": "8.0.3" +======= + "@aws-sdk/xml-builder": "3.972.15" +>>>>>>> origin/main }, "engines": { "node": ">=20" diff --git a/src/cli/AGENTS.md b/src/cli/AGENTS.md index 55512bb47..9c9a3a921 100644 --- a/src/cli/AGENTS.md +++ b/src/cli/AGENTS.md @@ -82,7 +82,7 @@ primitives/ ├── registry.ts # Singleton instances + ALL_PRIMITIVES array ├── credential-utils.ts # Shared credential env var name computation ├── constants.ts # SOURCE_CODE_NOTE and other shared constants -├── types.ts # AddResult, RemovableResource, RemovalResult, etc. +├── types.ts # RemovableResource, AddScreenComponent, etc. └── index.ts # Barrel exports ``` @@ -92,8 +92,8 @@ Every primitive extends `BasePrimitive` 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` +- `remove(name)` — remove a resource, returns `Result` - `previewRemove(name)` — preview what removal will do - `getRemovable()` — list resources available for removal - `registerCommands(addCmd, removeCmd)` — register CLI subcommands @@ -119,7 +119,8 @@ BasePrimitive provides shared helpers: - **Absorb, don't wrap.** Each primitive owns its logic directly. Do not create facade files that delegate to primitives. - **No backward-compatibility shims.** This is a CLI, not a library. If the CLI functions the same, delete old files. -- **Use `{ success, error? }` result format** throughout (never `{ ok, error }`). See `AddResult` and `RemovalResult`. +- **Use the discriminated `Result` 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. diff --git a/src/cli/__tests__/errors.test.ts b/src/cli/__tests__/errors.test.ts index 4a8d2c1cf..663cd3508 100644 --- a/src/cli/__tests__/errors.test.ts +++ b/src/cli/__tests__/errors.test.ts @@ -1,5 +1,5 @@ +import { AgentAlreadyExistsError, toError } from '../../lib'; import { - AgentAlreadyExistsError, getErrorMessage, isChangesetInProgressError, isExpiredTokenError, @@ -204,4 +204,35 @@ describe('errors', () => { expect(isChangesetInProgressError({})).toBe(false); }); }); + + describe('toError', () => { + it('returns Error instance as-is', () => { + const err = new Error('original'); + expect(toError(err)).toBe(err); + }); + + it('wraps string in Error', () => { + const result = toError('string error'); + expect(result).toBeInstanceOf(Error); + expect(result.message).toBe('string error'); + }); + + it('handles null', () => { + const result = toError(null); + expect(result).toBeInstanceOf(Error); + expect(result.message).toBe('null'); + }); + + it('handles undefined', () => { + const result = toError(undefined); + expect(result).toBeInstanceOf(Error); + expect(result.message).toBe('undefined'); + }); + + it('handles non-Error objects', () => { + const result = toError({ code: 42 }); + expect(result).toBeInstanceOf(Error); + expect(result.message).toBe('[object Object]'); + }); + }); }); diff --git a/src/cli/aws/__tests__/transaction-search.test.ts b/src/cli/aws/__tests__/transaction-search.test.ts index 67ccd0ac6..307063a61 100644 --- a/src/cli/aws/__tests__/transaction-search.test.ts +++ b/src/cli/aws/__tests__/transaction-search.test.ts @@ -1,4 +1,5 @@ import { enableTransactionSearch } from '../transaction-search.js'; +import assert from 'node:assert'; import { beforeEach, describe, expect, it, vi } from 'vitest'; const { mockAppSignalsSend, mockLogsSend, mockXRaySend } = vi.hoisted(() => ({ @@ -162,8 +163,8 @@ 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 () => { @@ -171,8 +172,8 @@ describe('enableTransactionSearch', () => { const result = await enableTransactionSearch('us-east-1', '123456789012'); - expect(result.success).toBe(false); - expect(result.error).toContain('Failed to enable Application Signals'); + assert(!result.success); + expect(result.error.message).toContain('Failed to enable Application Signals'); }); it('returns error when CloudWatch Logs policy fails with AccessDenied', async () => { @@ -183,8 +184,8 @@ describe('enableTransactionSearch', () => { const result = await enableTransactionSearch('us-east-1', '123456789012'); - expect(result.success).toBe(false); - expect(result.error).toContain('Insufficient permissions to configure CloudWatch Logs policy'); + assert(!result.success); + expect(result.error.message).toContain('Insufficient permissions to configure CloudWatch Logs policy'); }); it('returns error when trace destination fails', async () => { @@ -194,8 +195,8 @@ describe('enableTransactionSearch', () => { const result = await enableTransactionSearch('us-east-1', '123456789012'); - expect(result.success).toBe(false); - expect(result.error).toContain('Failed to configure trace destination'); + assert(!result.success); + expect(result.error.message).toContain('Failed to configure trace destination'); }); it('returns error when indexing rule update fails', async () => { @@ -214,8 +215,8 @@ describe('enableTransactionSearch', () => { const result = await enableTransactionSearch('us-east-1', '123456789012'); - expect(result.success).toBe(false); - expect(result.error).toContain('Failed to configure indexing rules'); + assert(!result.success); + expect(result.error.message).toContain('Failed to configure indexing rules'); }); it('does not proceed to later steps when an earlier step fails', async () => { diff --git a/src/cli/aws/index.ts b/src/cli/aws/index.ts index d306a80e1..1948d0efa 100644 --- a/src/cli/aws/index.ts +++ b/src/cli/aws/index.ts @@ -17,7 +17,7 @@ export { type GetAgentRuntimeStatusOptions, } from './agentcore-control'; export { streamLogs, searchLogs, type LogEvent, type StreamLogsOptions, type SearchLogsOptions } from './cloudwatch'; -export { enableTransactionSearch, type TransactionSearchEnableResult } from './transaction-search'; +export { enableTransactionSearch } from './transaction-search'; export { startPolicyGeneration, getPolicyGeneration, diff --git a/src/cli/aws/transaction-search.ts b/src/cli/aws/transaction-search.ts index 3a3b53fa6..0556c1fbd 100644 --- a/src/cli/aws/transaction-search.ts +++ b/src/cli/aws/transaction-search.ts @@ -1,3 +1,5 @@ +import { AccessDeniedError } from '../../lib'; +import type { Result } from '../../lib/result'; import { getErrorMessage, isAccessDeniedError } from '../errors'; import { getCredentialProvider } from './account'; import { arnPrefix } from './partition'; @@ -14,11 +16,6 @@ import { XRayClient, } from '@aws-sdk/client-xray'; -export interface TransactionSearchEnableResult { - success: boolean; - error?: string; -} - const RESOURCE_POLICY_NAME = 'TransactionSearchXRayAccess'; /** @@ -34,7 +31,7 @@ export async function enableTransactionSearch( region: string, accountId: string, indexPercentage = 100 -): Promise { +): Promise { const credentials = getCredentialProvider(); // Step 1: Enable Application Signals (creates service-linked role, idempotent) @@ -44,9 +41,14 @@ export async function enableTransactionSearch( } catch (err: unknown) { const message = getErrorMessage(err); if (isAccessDeniedError(err)) { - return { success: false, error: `Insufficient permissions to enable Application Signals: ${message}` }; + return { + success: false, + error: new AccessDeniedError(`Insufficient permissions to enable Application Signals: ${message}`, { + cause: err, + }), + }; } - return { success: false, error: `Failed to enable Application Signals: ${message}` }; + return { success: false, error: new Error(`Failed to enable Application Signals: ${message}`, { cause: err }) }; } // Step 2: Create CloudWatch Logs resource policy for X-Ray (if needed) @@ -80,9 +82,17 @@ export async function enableTransactionSearch( } catch (err: unknown) { const message = getErrorMessage(err); if (isAccessDeniedError(err)) { - return { success: false, error: `Insufficient permissions to configure CloudWatch Logs policy: ${message}` }; + return { + success: false, + error: new AccessDeniedError(`Insufficient permissions to configure CloudWatch Logs policy: ${message}`, { + cause: err, + }), + }; } - return { success: false, error: `Failed to configure CloudWatch Logs policy: ${message}` }; + return { + success: false, + error: new Error(`Failed to configure CloudWatch Logs policy: ${message}`, { cause: err }), + }; } const xrayClient = new XRayClient({ region, credentials }); @@ -96,9 +106,14 @@ export async function enableTransactionSearch( } catch (err: unknown) { const message = getErrorMessage(err); if (isAccessDeniedError(err)) { - return { success: false, error: `Insufficient permissions to configure trace destination: ${message}` }; + return { + success: false, + error: new AccessDeniedError(`Insufficient permissions to configure trace destination: ${message}`, { + cause: err, + }), + }; } - return { success: false, error: `Failed to configure trace destination: ${message}` }; + return { success: false, error: new Error(`Failed to configure trace destination: ${message}`, { cause: err }) }; } // Step 4: Set indexing to 100% on the built-in Default rule (always exists, idempotent) @@ -112,9 +127,14 @@ export async function enableTransactionSearch( } catch (err: unknown) { const message = getErrorMessage(err); if (isAccessDeniedError(err)) { - return { success: false, error: `Insufficient permissions to configure indexing rules: ${message}` }; + return { + success: false, + error: new AccessDeniedError(`Insufficient permissions to configure indexing rules: ${message}`, { + cause: err, + }), + }; } - return { success: false, error: `Failed to configure indexing rules: ${message}` }; + return { success: false, error: new Error(`Failed to configure indexing rules: ${message}`, { cause: err }) }; } return { success: true }; diff --git a/src/cli/commands/add/types.ts b/src/cli/commands/add/types.ts index 47e7c0ad9..cbf92ed63 100644 --- a/src/cli/commands/add/types.ts +++ b/src/cli/commands/add/types.ts @@ -41,13 +41,6 @@ export interface AddAgentOptions extends VpcOptions { json?: boolean; } -export interface AddAgentResult { - success: boolean; - agentName?: string; - agentPath?: string; - error?: string; -} - // Gateway types export interface AddGatewayOptions { name?: string; @@ -68,12 +61,6 @@ export interface AddGatewayOptions { json?: boolean; } -export interface AddGatewayResult { - success: boolean; - gatewayName?: string; - error?: string; -} - // Gateway Target types export interface AddGatewayTargetOptions { name?: string; @@ -100,6 +87,7 @@ export interface AddGatewayTargetOptions { json?: boolean; } +<<<<<<< HEAD export interface AddGatewayTargetResult { success: boolean; toolName?: string; @@ -145,6 +133,8 @@ export interface AddHarnessCliOptions { json?: boolean; } +======= +>>>>>>> origin/main // Memory types (v2: no owner/user concept) export interface AddMemoryOptions { name?: string; @@ -157,12 +147,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; @@ -177,9 +161,3 @@ export interface AddCredentialOptions { /** @deprecated Use AddCredentialOptions */ export type AddIdentityOptions = AddCredentialOptions; - -export interface AddCredentialResult { - success: boolean; - credentialName?: string; - error?: string; -} diff --git a/src/cli/commands/create/action.ts b/src/cli/commands/create/action.ts index a00397f38..0f97de294 100644 --- a/src/cli/commands/create/action.ts +++ b/src/cli/commands/create/action.ts @@ -1,4 +1,13 @@ -import { APP_DIR, CONFIG_DIR, ConfigIO, setEnvVar, setSessionProjectRoot } from '../../../lib'; +import { + APP_DIR, + CONFIG_DIR, + ConfigIO, + DependencyCheckError, + GitInitError, + setEnvVar, + setSessionProjectRoot, + toError, +} from '../../../lib'; import type { BuildType, DeployedState, @@ -8,7 +17,6 @@ import type { SDKFramework, TargetLanguage, } from '../../../schema'; -import { getErrorMessage } from '../../errors'; import { checkCreateDependencies } from '../../external-requirements'; import { initGitRepo, setupPythonProject, writeEnvFile, writeGitignore } from '../../operations'; import { createConfigBundleForAgent } from '../../operations/agent/config-bundle-defaults'; @@ -57,7 +65,11 @@ export async function createProject(options: CreateProjectOptions): Promise 0 ? depWarnings : undefined, + }; } } @@ -93,7 +105,11 @@ export async function createProject(options: CreateProjectOptions): Promise 0 ? depWarnings : undefined, + }; } onProgress?.('Initialize git repository', 'done'); } @@ -104,7 +120,7 @@ export async function createProject(options: CreateProjectOptions): Promise 0 ? depWarnings : undefined, }; } catch (err) { - return { success: false, error: getErrorMessage(err), warnings: depWarnings }; + return { success: false, error: toError(err), warnings: depWarnings.length > 0 ? depWarnings : undefined }; } } @@ -174,7 +190,11 @@ export async function createProjectWithAgent(options: CreateWithAgentOptions): P // Fail on errors if (!depCheck.passed) { - return { success: false, error: depCheck.errors.join('\n'), warnings: depWarnings }; + return { + success: false, + error: new DependencyCheckError(depCheck.errors), + warnings: depWarnings.length > 0 ? depWarnings : undefined, + }; } // First create the base project (skip dependency check since we already did it) @@ -207,7 +227,11 @@ export async function createProjectWithAgent(options: CreateWithAgentOptions): P }); if (!importResult.success) { onProgress?.('Import agent from Bedrock', 'error'); - return { success: false, error: importResult.error, warnings: depWarnings }; + return { + success: false, + error: importResult.error, + warnings: depWarnings.length > 0 ? depWarnings : undefined, + }; } onProgress?.('Import agent from Bedrock', 'done'); return { @@ -217,7 +241,7 @@ export async function createProjectWithAgent(options: CreateWithAgentOptions): P warnings: depWarnings.length > 0 ? depWarnings : undefined, }; } catch (err) { - return { success: false, error: getErrorMessage(err), warnings: depWarnings }; + return { success: false, error: toError(err), warnings: depWarnings.length > 0 ? depWarnings : undefined }; } } @@ -310,7 +334,7 @@ export async function createProjectWithAgent(options: CreateWithAgentOptions): P warnings: depWarnings.length > 0 ? depWarnings : undefined, }; } catch (err) { - return { success: false, error: getErrorMessage(err), warnings: depWarnings }; + return { success: false, error: toError(err), warnings: depWarnings.length > 0 ? depWarnings : undefined }; } } diff --git a/src/cli/commands/create/command.tsx b/src/cli/commands/create/command.tsx index 9be7e7bf0..57ef36445 100644 --- a/src/cli/commands/create/command.tsx +++ b/src/cli/commands/create/command.tsx @@ -1,4 +1,4 @@ -import { getWorkingDirectory } from '../../../lib'; +import { getWorkingDirectory, serializeResult } from '../../../lib'; import type { BuildType, HarnessModelProvider, @@ -10,7 +10,22 @@ import type { } from '../../../schema'; import { LIFECYCLE_TIMEOUT_MAX, LIFECYCLE_TIMEOUT_MIN } from '../../../schema'; import { getErrorMessage } from '../../errors'; +<<<<<<< HEAD import { harnessPrimitive } from '../../primitives/registry'; +======= +import { runCliCommand } from '../../telemetry/cli-command-run.js'; +import { + AgentType, + Build, + Framework, + Language, + Memory, + ModelProvider as ModelProviderEnum, + NetworkMode as NetworkModeEnum, + Protocol, + standardize, +} from '../../telemetry/schemas/common-shapes.js'; +>>>>>>> origin/main import { COMMAND_DESCRIPTIONS } from '../../tui/copy'; import { requireTTY } from '../../tui/guards'; import { CreateScreen } from '../../tui/screens/create'; @@ -213,22 +228,25 @@ async function handleCreateAgentCLI(options: CreateOptions): Promise { const name = options.name ?? options.projectName; const projectName = options.projectName ?? name; - const validation = validateCreateOptions(options, cwd); - if (!validation.valid) { - if (options.json) { - console.log(JSON.stringify({ success: false, error: validation.error })); - } else { - console.error(validation.error); - } - process.exit(1); - } - - // Handle dry-run mode + // Handle dry-run mode (no telemetry for dry-run) if (options.dryRun) { +<<<<<<< HEAD const result = getDryRunInfo({ name: name!, projectName: projectName!, cwd, language: options.language }); +======= + const validation = validateCreateOptions(options, cwd); + if (!validation.valid) { + if (options.json) { + console.log(JSON.stringify({ success: false, error: validation.error })); + } else { + console.error(validation.error); + } + process.exit(1); + } + const result = getDryRunInfo({ name: name!, projectName, cwd, language: options.language }); +>>>>>>> origin/main if (options.json) { - console.log(JSON.stringify(result)); - } else { + console.log(JSON.stringify(serializeResult(result))); + } else if (result.success) { console.log('Dry run - would create:'); for (const path of result.wouldCreate ?? []) { console.log(` ${path}`); @@ -237,21 +255,96 @@ async function handleCreateAgentCLI(options: CreateOptions): Promise { process.exit(0); } - const green = '\x1b[32m'; - const reset = '\x1b[0m'; + const knownAttrs = { + language: standardize(Language, options.language), + framework: standardize(Framework, options.framework), + model_provider: standardize(ModelProviderEnum, options.modelProvider), + memory: standardize(Memory, options.memory ?? 'none'), + protocol: standardize(Protocol, options.protocol ?? 'http'), + build: standardize(Build, options.build ?? 'codezip'), + agent_type: standardize(AgentType, options.type ?? 'create'), + network_mode: standardize(NetworkModeEnum, options.networkMode ?? 'public'), + has_agent: options.agent !== false, + }; - // Progress callback for real-time output - const onProgress: ProgressCallback | undefined = options.json - ? undefined - : (step, status) => { - if (status === 'done') { - console.log(`${green}[done]${reset} ${step}`); - } else if (status === 'error') { - console.log(`\x1b[31m[error]${reset} ${step}`); + await runCliCommand( + 'create', + !!options.json, + async () => { + const validation = validateCreateOptions(options, cwd); + if (!validation.valid) { + throw new Error(validation.error); + } + const green = '\x1b[32m'; + const reset = '\x1b[0m'; + + // Progress callback for real-time output + const onProgress: ProgressCallback | undefined = options.json + ? undefined + : (step, status) => { + if (status === 'done') { + console.log(`${green}[done]${reset} ${step}`); + } else if (status === 'error') { + console.log(`\x1b[31m[error]${reset} ${step}`); + } + // 'start' is silent - we only show when done + }; + + // Commander.js --no-agent sets agent=false, not noAgent=true + const skipAgent = options.agent === false; + + const result = skipAgent + ? await createProject({ + name: projectName!, + cwd, + skipGit: options.skipGit, + skipInstall: options.skipInstall, + onProgress, + }) + : await createProjectWithAgent({ + name: name!, + projectName, + cwd, + type: options.type as 'create' | 'import' | undefined, + buildType: (options.build as BuildType) ?? 'CodeZip', + language: (options.language as TargetLanguage) ?? (options.type === 'import' ? 'Python' : undefined), + framework: options.framework as SDKFramework | undefined, + modelProvider: options.modelProvider as ModelProvider | undefined, + apiKey: options.apiKey, + memory: (options.memory as 'none' | 'shortTerm' | 'longAndShortTerm') ?? 'none', + protocol: options.protocol as ProtocolMode | undefined, + agentId: options.agentId, + agentAliasId: options.agentAliasId, + region: options.region, + networkMode: options.networkMode as NetworkMode | undefined, + subnets: parseCommaSeparatedList(options.subnets), + securityGroups: parseCommaSeparatedList(options.securityGroups), + idleTimeout: options.idleTimeout ? Number(options.idleTimeout) : undefined, + maxLifetime: options.maxLifetime ? Number(options.maxLifetime) : undefined, + sessionStorageMountPath: options.sessionStorageMountPath, + withConfigBundle: options.withConfigBundle, + skipGit: options.skipGit, + skipInstall: options.skipInstall, + skipPythonSetup: options.skipPythonSetup, + onProgress, + }); + + if (!result.success) { + throw result.error; + } + + if (options.json) { + console.log(JSON.stringify(result)); + } else { + printCreateSummary(projectName!, result.agentName, options.language, options.framework); + if (options.skipInstall) { + console.log( + "\nDependency installation was skipped. Run 'npm install' in agentcore/cdk/ and 'uv sync' in your agent directory manually." + ); } - // 'start' is silent - we only show when done - }; + } +<<<<<<< HEAD // Commander.js --no-agent sets agent=false, not noAgent=true const skipAgent = options.agent === false; @@ -305,6 +398,12 @@ async function handleCreateAgentCLI(options: CreateOptions): Promise { } process.exit(result.success ? 0 : 1); +======= + return knownAttrs; + }, + knownAttrs + ); +>>>>>>> origin/main } export const registerCreate = (program: Command) => { diff --git a/src/cli/commands/create/types.ts b/src/cli/commands/create/types.ts index a46a85885..72547679c 100644 --- a/src/cli/commands/create/types.ts +++ b/src/cli/commands/create/types.ts @@ -1,3 +1,4 @@ +import type { Result } from '../../../lib/result'; import type { VpcOptions } from '../shared/vpc-utils'; export interface CreateOptions extends VpcOptions { @@ -37,12 +38,9 @@ export interface CreateOptions extends VpcOptions { truncationStrategy?: string; } -export interface CreateResult { - success: boolean; +export type CreateResult = Result<{ projectPath?: string; agentName?: string; - error?: string; dryRun?: boolean; wouldCreate?: string[]; - warnings?: string[]; -} +}> & { warnings?: string[] }; diff --git a/src/cli/commands/deploy/actions.ts b/src/cli/commands/deploy/actions.ts index f7210a9b2..204aa8e63 100644 --- a/src/cli/commands/deploy/actions.ts +++ b/src/cli/commands/deploy/actions.ts @@ -1,5 +1,10 @@ +<<<<<<< HEAD import { ConfigIO, SecureCredentials } from '../../../lib'; import type { AgentCoreMcpSpec, DeployedState, HarnessDeployedState } from '../../../schema'; +======= +import { ConfigIO, ResourceNotFoundError, SecureCredentials, ValidationError, toError } from '../../../lib'; +import type { AgentCoreMcpSpec, DeployedState } from '../../../schema'; +>>>>>>> origin/main import { applyTargetRegionToEnv } from '../../aws'; import { validateAwsCredentials } from '../../aws/account'; import type { DeployMessage } from '../../cdk/toolkit-lib'; @@ -91,7 +96,7 @@ export async function handleDeploy(options: ValidatedDeployOptions): Promise>>>>>> origin/main import { getErrorMessage } from '../../errors'; import { withCommandRunTelemetry } from '../../telemetry/cli-command-run.js'; import { COMMAND_DESCRIPTIONS } from '../../tui/copy'; @@ -50,10 +54,17 @@ async function handleDeployCLI(options: DeployOptions): Promise { const { deployResult } = await withCommandRunTelemetry('deploy', attrs, async () => { const result = await executeDeploy(options).catch( +<<<<<<< HEAD (e): DeployResult => ({ success: false, error: getErrorMessage(e) }) ); if (!result.success) { return { success: false as const, error: result.error ?? 'Deploy failed', deployResult: result }; +======= + (e): DeployResult => ({ success: false, error: e instanceof Error ? e : new Error(getErrorMessage(e)) }) + ); + if (!result.success) { + return { success: false as const, error: result.error, deployResult: result }; +>>>>>>> origin/main } return { success: true as const, deployResult: result }; }); @@ -61,9 +72,15 @@ async function handleDeployCLI(options: DeployOptions): Promise { // ALL output happens here, after telemetry if (!deployResult.success) { if (options.json) { +<<<<<<< HEAD console.log(JSON.stringify(deployResult)); } else { console.error(deployResult.error); +======= + console.log(JSON.stringify(serializeResult(deployResult))); + } else { + console.error(deployResult.error.message); +>>>>>>> origin/main if (deployResult.logPath) { console.error(`Log: ${deployResult.logPath}`); } @@ -71,7 +88,11 @@ async function handleDeployCLI(options: DeployOptions): Promise { process.exit(1); } +<<<<<<< HEAD printDeployResult(deployResult as DeployResult & { success: true }, options); +======= + printDeployResult(deployResult, options); +>>>>>>> origin/main if (deployResult.postDeployWarnings && deployResult.postDeployWarnings.length > 0) { process.exit(2); diff --git a/src/cli/commands/deploy/types.ts b/src/cli/commands/deploy/types.ts index 44cdc7847..a29b6ef8e 100644 --- a/src/cli/commands/deploy/types.ts +++ b/src/cli/commands/deploy/types.ts @@ -1,3 +1,5 @@ +import type { Result } from '../../../lib/result'; + export interface DeployOptions { target?: string; yes?: boolean; @@ -8,21 +10,16 @@ export interface DeployOptions { diff?: boolean; } -export interface DeployResult { - success: boolean; +export type DeployResult = Result<{ targetName?: string; stackName?: string; outputs?: Record; - logPath?: string; nextSteps?: string[]; notes?: string[]; postDeployWarnings?: string[]; - error?: string; -} +}> & { logPath?: string }; -export interface PreflightResult { - success: boolean; +export type PreflightResult = Result<{ stackNames?: string[]; needsBootstrap?: boolean; - error?: string; -} +}>; diff --git a/src/cli/commands/dev/browser-mode.ts b/src/cli/commands/dev/browser-mode.ts index 77342e7f1..4212c209e 100644 --- a/src/cli/commands/dev/browser-mode.ts +++ b/src/cli/commands/dev/browser-mode.ts @@ -68,24 +68,26 @@ async function resolveDeployedHandlers( result.onListMemoryRecords = async (memoryName, namespace, strategyId) => { const memory = memories.find(m => m.name === memoryName); if (!memory) return { success: false, error: `Memory "${memoryName}" not found in deployed state` }; - return listMemoryRecords({ + const res = await listMemoryRecords({ region: memory.region, memoryId: memory.memoryId, namespace, memoryStrategyId: strategyId, }); + return res.success ? res : { success: false as const, error: res.error.message }; }; result.onRetrieveMemoryRecords = async (memoryName, namespace, searchQuery, strategyId) => { const memory = memories.find(m => m.name === memoryName); if (!memory) return { success: false, error: `Memory "${memoryName}" not found in deployed state` }; - return retrieveMemoryRecords({ + const res = await retrieveMemoryRecords({ region: memory.region, memoryId: memory.memoryId, namespace, searchQuery, memoryStrategyId: strategyId, }); + return res.success ? res : { success: false as const, error: res.error.message }; }; } @@ -254,13 +256,14 @@ export async function runBrowserMode(opts: BrowserModeOptions): Promise { const context = await loadDeployedProjectConfig(configIO); const resolved = await resolveAgentOrHarness(context, { runtime: agentName, harness: harnessName }); if (!resolved.success) return { success: false, error: resolved.error }; - return listTraces({ + const res = await listTraces({ region: resolved.agent.region, runtimeId: resolved.agent.runtimeId, agentName: resolved.agent.agentName, startTime, endTime, }); + return res.success ? res : { success: false as const, error: res.error.message }; } catch (err) { return { success: false, @@ -274,7 +277,7 @@ export async function runBrowserMode(opts: BrowserModeOptions): Promise { const context = await loadDeployedProjectConfig(configIO); const resolved = await resolveAgentOrHarness(context, { runtime: agentName, harness: harnessName }); if (!resolved.success) return { success: false, error: resolved.error }; - return fetchTraceRecords({ + const res = await fetchTraceRecords({ region: resolved.agent.region, runtimeId: resolved.agent.runtimeId, traceId, @@ -282,6 +285,7 @@ export async function runBrowserMode(opts: BrowserModeOptions): Promise { endTime, includeSpans: true, }); + return res.success ? res : { success: false as const, error: res.error.message }; } catch (err) { return { success: false, diff --git a/src/cli/commands/eval/command.tsx b/src/cli/commands/eval/command.tsx index 9dff195c4..4a086584b 100644 --- a/src/cli/commands/eval/command.tsx +++ b/src/cli/commands/eval/command.tsx @@ -1,3 +1,4 @@ +import { serializeResult } from '../../../lib'; import { getErrorMessage } from '../../errors'; import { handleListEvalRuns } from '../../operations/eval'; import { getResultsPath } from '../../operations/eval/storage'; @@ -27,13 +28,13 @@ export const registerEval = (program: Command) => { }); if (cliOptions.json) { - console.log(JSON.stringify(result)); + console.log(JSON.stringify(serializeResult(result))); process.exit(result.success ? 0 : 1); return; } if (!result.success) { - render({result.error}); + render({result.error.message}); process.exit(1); } diff --git a/src/cli/commands/import/__tests__/idempotency.test.ts b/src/cli/commands/import/__tests__/idempotency.test.ts index 4b1938e28..7cf7648aa 100644 --- a/src/cli/commands/import/__tests__/idempotency.test.ts +++ b/src/cli/commands/import/__tests__/idempotency.test.ts @@ -10,6 +10,7 @@ // ── Import the function under test AFTER mocks ──────────────────────────────── import { handleImport } from '../actions'; import type { ParsedStarterToolkitConfig } from '../types'; +import assert from 'node:assert'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; // ── Hoisted mock fns (available inside vi.mock factories) ───────────────────── @@ -76,7 +77,20 @@ const { vi.mock('../../../../lib', () => ({ APP_DIR: 'app', ConfigIO: MockConfigIOClass, + NoProjectError: class NoProjectError extends Error { + constructor(msg?: string) { + super(msg ?? 'No agentcore project found'); + this.name = 'NoProjectError'; + } + }, + ValidationError: class extends Error { + constructor(m: string) { + super(m); + this.name = 'ValidationError'; + } + }, findConfigRoot: (...args: unknown[]) => mockFindConfigRoot(...args), + toError: (err: unknown) => (err instanceof Error ? err : new Error(String(err))), })); vi.mock('../../../aws/account', () => ({ @@ -279,7 +293,7 @@ describe('Import Idempotency (Test Group 7)', () => { const result = await handleImport({ source: '/tmp/config.yaml' }); - expect(result.success).toBe(true); + assert(result.success); expect(result.importedAgents).toContain('my-agent'); expect(result.importedMemories).toContain('my-memory'); @@ -393,7 +407,7 @@ describe('Import Idempotency (Test Group 7)', () => { const result = await handleImport({ source: '/tmp/config.yaml' }); - expect(result.success).toBe(true); + assert(result.success); expect(result.importedAgents).toEqual([]); expect(result.importedMemories).toEqual([]); }); @@ -610,8 +624,8 @@ describe('Import Idempotency (Test Group 7)', () => { const result = await handleImport({ source: '/tmp/config.yaml' }); - expect(result.success).toBe(false); - expect(result.error).toContain('No agents found'); + assert(!result.success); + expect(result.error.message).toContain('No agents found'); }); it('returns error when no project found', async () => { @@ -619,8 +633,8 @@ describe('Import Idempotency (Test Group 7)', () => { const result = await handleImport({ source: '/tmp/config.yaml' }); - expect(result.success).toBe(false); - expect(result.error).toContain('No agentcore project found'); + assert(!result.success); + expect(result.error.message).toContain('No agentcore project found'); }); }); diff --git a/src/cli/commands/import/__tests__/import-gateway-flow.test.ts b/src/cli/commands/import/__tests__/import-gateway-flow.test.ts index 183a6e63f..34803e8eb 100644 --- a/src/cli/commands/import/__tests__/import-gateway-flow.test.ts +++ b/src/cli/commands/import/__tests__/import-gateway-flow.test.ts @@ -11,6 +11,7 @@ * - Non-READY gateway warning */ import { handleImportGateway } from '../import-gateway'; +import assert from 'node:assert'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; // ── Hoisted mock fns ──────────────────────────────────────────────────────── @@ -56,6 +57,12 @@ const { vi.mock('../../../../lib', () => ({ APP_DIR: 'app', ConfigIO: MockConfigIOClass, + NoProjectError: class NoProjectError extends Error { + constructor(msg?: string) { + super(msg ?? 'No agentcore project found'); + this.name = 'NoProjectError'; + } + }, findConfigRoot: (...args: unknown[]) => mockFindConfigRoot(...args), })); @@ -177,7 +184,7 @@ describe('handleImportGateway', () => { it('successfully imports a gateway with --arn', async () => { const result = await handleImportGateway({ arn: GATEWAY_ARN }); - expect(result.success).toBe(true); + assert(result.success); expect(result.resourceId).toBe(GATEWAY_ID); expect(result.resourceType).toBe('gateway'); expect(result.resourceName).toBe(GATEWAY_NAME); @@ -199,8 +206,8 @@ describe('handleImportGateway', () => { const result = await handleImportGateway({ arn: GATEWAY_ARN }); - expect(result.success).toBe(false); - expect(result.error).toBe('Phase 2 failed'); + assert(!result.success); + expect(result.error.message).toBe('Phase 2 failed'); // First call = write merged config, second call = rollback expect(mockConfigIOInstance.writeProjectSpec).toHaveBeenCalledTimes(2); @@ -213,8 +220,8 @@ describe('handleImportGateway', () => { const result = await handleImportGateway({ arn: GATEWAY_ARN }); - expect(result.success).toBe(false); - expect(result.error).toContain('Could not find logical ID'); + assert(!result.success); + expect(result.error.message).toContain('Could not find logical ID'); // First call = write merged config, second call = rollback expect(mockConfigIOInstance.writeProjectSpec).toHaveBeenCalledTimes(2); @@ -231,8 +238,8 @@ describe('handleImportGateway', () => { const result = await handleImportGateway({ arn: GATEWAY_ARN }); - expect(result.success).toBe(false); - expect(result.error).toContain('already exists'); + assert(!result.success); + expect(result.error.message).toContain('already exists'); expect(mockConfigIOInstance.writeProjectSpec).not.toHaveBeenCalled(); }); @@ -254,7 +261,7 @@ describe('handleImportGateway', () => { const result = await handleImportGateway({ arn: GATEWAY_ARN }); - expect(result.success).toBe(true); + assert(result.success); expect(result.resourceName).toBe('ExistingGateway'); expect(mockConfigIOInstance.writeProjectSpec).toHaveBeenCalledTimes(1); expect(mockExecuteCdkImportPipeline).toHaveBeenCalled(); @@ -278,7 +285,7 @@ describe('handleImportGateway', () => { const result = await handleImportGateway({ arn: GATEWAY_ARN, name: 'myCustomName' }); - expect(result.success).toBe(true); + assert(result.success); expect(result.resourceName).toBe('myCustomName'); expect(mockConfigIOInstance.writeProjectSpec).toHaveBeenCalledTimes(1); expect(mockExecuteCdkImportPipeline).toHaveBeenCalled(); @@ -302,8 +309,8 @@ describe('handleImportGateway', () => { const result = await handleImportGateway({ arn: GATEWAY_ARN }); - expect(result.success).toBe(false); - expect(result.error).toContain('already exists'); + assert(!result.success); + expect(result.error.message).toContain('already exists'); }); }); @@ -315,15 +322,15 @@ describe('handleImportGateway', () => { const result = await handleImportGateway({ arn: GATEWAY_ARN }); - expect(result.success).toBe(false); - expect(result.error).toContain('Invalid name'); + assert(!result.success); + expect(result.error.message).toContain('Invalid name'); expect(mockConfigIOInstance.writeProjectSpec).not.toHaveBeenCalled(); }); it('uses --name override with original resourceName preserved', async () => { const result = await handleImportGateway({ arn: GATEWAY_ARN, name: 'myCustomName' }); - expect(result.success).toBe(true); + assert(result.success); expect(result.resourceName).toBe('myCustomName'); const writtenSpec = mockConfigIOInstance.writeProjectSpec.mock.calls[0]![0]; @@ -343,7 +350,7 @@ describe('handleImportGateway', () => { const result = await handleImportGateway({}); - expect(result.success).toBe(true); + assert(result.success); expect(result.resourceId).toBe(GATEWAY_ID); expect(mockGetGatewayDetail).toHaveBeenCalledWith({ region: REGION, gatewayId: GATEWAY_ID }); }); @@ -356,8 +363,8 @@ describe('handleImportGateway', () => { const result = await handleImportGateway({}); - expect(result.success).toBe(false); - expect(result.error).toContain('Multiple gateways found'); + assert(!result.success); + expect(result.error.message).toContain('Multiple gateways found'); }); it('fails when no gateways exist and no --arn', async () => { @@ -365,8 +372,8 @@ describe('handleImportGateway', () => { const result = await handleImportGateway({}); - expect(result.success).toBe(false); - expect(result.error).toContain('No gateways found'); + assert(!result.success); + expect(result.error.message).toContain('No gateways found'); }); }); @@ -399,7 +406,7 @@ describe('handleImportGateway', () => { onProgress: (msg: string) => progressMessages.push(msg), }); - expect(result.success).toBe(true); + assert(result.success); // Verify warning about unmapped targets expect(progressMessages.some(m => m.includes('1 target(s) could not be mapped'))).toBe(true); @@ -414,7 +421,7 @@ describe('handleImportGateway', () => { onProgress: (msg: string) => progressMessages.push(msg), }); - expect(result.success).toBe(true); + assert(result.success); expect(progressMessages.some(m => m.includes('CREATING') && m.includes('not READY'))).toBe(true); }); }); diff --git a/src/cli/commands/import/__tests__/import-no-deploy.test.ts b/src/cli/commands/import/__tests__/import-no-deploy.test.ts index f0b84303f..001c551be 100644 --- a/src/cli/commands/import/__tests__/import-no-deploy.test.ts +++ b/src/cli/commands/import/__tests__/import-no-deploy.test.ts @@ -5,6 +5,7 @@ * that were created but never deployed (no agent_id/memory_id in YAML). */ import { parseStarterToolkitYaml } from '../yaml-parser.js'; +import assert from 'node:assert'; import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; @@ -30,6 +31,12 @@ vi.mock('../../../../lib', () => ({ readDeployedState = mockReadDeployedState; writeDeployedState = mockWriteDeployedState; }, + NoProjectError: class NoProjectError extends Error { + constructor(msg?: string) { + super(msg ?? 'No agentcore project found'); + this.name = 'NoProjectError'; + } + }, findConfigRoot: (...args: unknown[]) => mockFindConfigRoot(...args), })); @@ -488,7 +495,7 @@ agents: onProgress: (msg: string) => progressMessages.push(msg), }); - expect(result.success).toBe(true); + assert(result.success); expect(result.importedAgents).toEqual([]); expect(result.importedMemories).toEqual([]); expect(result.stackName).toBeDefined(); @@ -645,6 +652,7 @@ agents: const { handleImport } = await import('../actions.js'); const result = await handleImport({ source: yamlPath }); + assert(result.success); expect(result.stackName).toBe('AgentCore-myproject-default'); }); }); @@ -724,7 +732,7 @@ agents: // No physical IDs means target resolution is skipped entirely. // The import succeeds -- config merge + source copy still happen. - expect(result.success).toBe(true); + assert(result.success); expect(result.importedAgents).toEqual([]); expect(result.importedMemories).toEqual([]); }); @@ -768,7 +776,7 @@ agents: const { handleImport } = await import('../actions.js'); const result = await handleImport({ source: yamlPath }); - expect(result.success).toBe(true); + assert(result.success); expect(result.importedAgents).toEqual([]); expect(result.importedMemories).toEqual([]); }); @@ -812,7 +820,7 @@ agents: const { handleImport } = await import('../actions.js'); const result = await handleImport({ source: yamlPath }); - expect(result.success).toBe(true); + assert(result.success); // No physical IDs means target is not written to disk expect(mockWriteAWSDeploymentTargets).not.toHaveBeenCalled(); // But the stackName should still be computed using 'default' fallback diff --git a/src/cli/commands/import/__tests__/import-runtime-handler.test.ts b/src/cli/commands/import/__tests__/import-runtime-handler.test.ts index b4fb7de90..d8926feb9 100644 --- a/src/cli/commands/import/__tests__/import-runtime-handler.test.ts +++ b/src/cli/commands/import/__tests__/import-runtime-handler.test.ts @@ -10,6 +10,7 @@ * - Fails when runtime name already exists in project */ import { handleImportRuntime } from '../import-runtime'; +import assert from 'node:assert'; import { afterEach, describe, expect, it, vi } from 'vitest'; // ── Mock dependencies ──────────────────────────────────────────────────────── @@ -25,7 +26,7 @@ const mockParseAndValidateArn = vi.fn(); const mockFindResourceInDeployedState = vi.fn(); const mockFailResult = vi.fn((...args: unknown[]) => ({ success: false, - error: args[1] as string, + error: new Error(args[1] as string), resourceType: args[2] as string, resourceName: args[3] as string, logPath: 'test.log', @@ -205,8 +206,9 @@ describe('handleImportRuntime', () => { }); expect(result.success).toBe(false); - expect(result.error).toContain('Could not determine entrypoint'); - expect(result.error).toContain('--entrypoint'); + assert(!result.success); + expect(result.error.message).toContain('Could not determine entrypoint'); + expect(result.error.message).toContain('--entrypoint'); }); it('fails with clear error when entryPoint is undefined', async () => { @@ -231,7 +233,8 @@ describe('handleImportRuntime', () => { }); expect(result.success).toBe(false); - expect(result.error).toContain('Could not determine entrypoint'); + assert(!result.success); + expect(result.error.message).toContain('Could not determine entrypoint'); }); it('fails with clear error when entryPoint is empty array', async () => { @@ -256,7 +259,8 @@ describe('handleImportRuntime', () => { }); expect(result.success).toBe(false); - expect(result.error).toContain('Could not determine entrypoint'); + assert(!result.success); + expect(result.error.message).toContain('Could not determine entrypoint'); }); it('uses --entrypoint flag when provided, bypassing auto-detection', async () => { @@ -369,7 +373,8 @@ describe('handleImportRuntime', () => { }); expect(result.success).toBe(false); - expect(result.error).toContain('Multiple runtimes found'); + assert(!result.success); + expect(result.error.message).toContain('Multiple runtimes found'); }); it('errors when no runtimes exist', async () => { @@ -382,7 +387,8 @@ describe('handleImportRuntime', () => { }); expect(result.success).toBe(false); - expect(result.error).toContain('No runtimes found'); + assert(!result.success); + expect(result.error.message).toContain('No runtimes found'); }); }); @@ -546,7 +552,8 @@ describe('handleImportRuntime', () => { }); expect(result.success).toBe(false); - expect(result.error).toContain('--code'); + assert(!result.success); + expect(result.error.message).toContain('--code'); }); it('fails when source path does not exist', async () => { @@ -574,7 +581,8 @@ describe('handleImportRuntime', () => { }); expect(result.success).toBe(false); - expect(result.error).toContain('does not exist'); + assert(!result.success); + expect(result.error.message).toContain('does not exist'); }); it('fails when runtime name already exists in project', async () => { @@ -606,7 +614,8 @@ describe('handleImportRuntime', () => { }); expect(result.success).toBe(false); - expect(result.error).toContain('already exists'); + assert(!result.success); + expect(result.error.message).toContain('already exists'); }); }); }); diff --git a/src/cli/commands/import/__tests__/jwt-authorizer.test.ts b/src/cli/commands/import/__tests__/jwt-authorizer.test.ts index 024c592df..1da1bb33a 100644 --- a/src/cli/commands/import/__tests__/jwt-authorizer.test.ts +++ b/src/cli/commands/import/__tests__/jwt-authorizer.test.ts @@ -27,7 +27,20 @@ vi.mock('../../../../lib', () => ({ readDeployedState = mockReadDeployedState; writeDeployedState = mockWriteDeployedState; }, + NoProjectError: class NoProjectError extends Error { + constructor(msg?: string) { + super(msg ?? 'No agentcore project found'); + this.name = 'NoProjectError'; + } + }, + ValidationError: class extends Error { + constructor(m: string) { + super(m); + this.name = 'ValidationError'; + } + }, findConfigRoot: (...args: unknown[]) => mockFindConfigRoot(...args), + toError: (err: unknown) => (err instanceof Error ? err : new Error(String(err))), })); vi.mock('../../../aws/account', () => ({ diff --git a/src/cli/commands/import/__tests__/phase-failure-rollback.test.ts b/src/cli/commands/import/__tests__/phase-failure-rollback.test.ts index 149d65faf..f18f3cd71 100644 --- a/src/cli/commands/import/__tests__/phase-failure-rollback.test.ts +++ b/src/cli/commands/import/__tests__/phase-failure-rollback.test.ts @@ -7,6 +7,7 @@ */ import { handleImport } from '../actions'; import type { ParsedStarterToolkitConfig } from '../types'; +import assert from 'node:assert'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; // ── Hoisted mock fns ──────────────────────────────────────────────────────── @@ -73,7 +74,20 @@ const { vi.mock('../../../../lib', () => ({ APP_DIR: 'app', ConfigIO: MockConfigIOClass, + NoProjectError: class NoProjectError extends Error { + constructor(msg?: string) { + super(msg ?? 'No agentcore project found'); + this.name = 'NoProjectError'; + } + }, + ValidationError: class ValidationError extends Error { + constructor(msg: string) { + super(msg); + this.name = 'ValidationError'; + } + }, findConfigRoot: (...args: unknown[]) => mockFindConfigRoot(...args), + toError: (err: unknown) => (err instanceof Error ? err : new Error(String(err))), })); vi.mock('../../../aws/account', () => ({ @@ -249,8 +263,8 @@ describe('Config Rollback on Import Failure', () => { const result = await handleImport({ source: '/tmp/config.yaml' }); - expect(result.success).toBe(false); - expect(result.error).toContain('Phase 1 failed'); + assert(!result.success); + expect(result.error.message).toContain('Phase 1 failed'); // First call = merge write, second call = rollback with original (empty) runtimes expect(mockConfigIOInstance.writeProjectSpec).toHaveBeenCalledTimes(2); @@ -265,8 +279,8 @@ describe('Config Rollback on Import Failure', () => { const result = await handleImport({ source: '/tmp/config.yaml' }); - expect(result.success).toBe(false); - expect(result.error).toContain('Phase 2 failed'); + assert(!result.success); + expect(result.error.message).toContain('Phase 2 failed'); expect(mockConfigIOInstance.writeProjectSpec).toHaveBeenCalledTimes(2); const rollbackData = mockConfigIOInstance.writeProjectSpec.mock.calls[1]![0]; @@ -280,8 +294,8 @@ describe('Config Rollback on Import Failure', () => { const result = await handleImport({ source: '/tmp/config.yaml' }); - expect(result.success).toBe(false); - expect(result.error).toContain('CDK build failed'); + assert(!result.success); + expect(result.error.message).toContain('CDK build failed'); expect(mockConfigIOInstance.writeProjectSpec).toHaveBeenCalledTimes(2); const rollbackData = mockConfigIOInstance.writeProjectSpec.mock.calls[1]![0]; @@ -294,7 +308,7 @@ describe('Config Rollback on Import Failure', () => { const result = await handleImport({ source: '/tmp/config.yaml' }); - expect(result.success).toBe(true); + assert(result.success); // Only one write: the merge write expect(mockConfigIOInstance.writeProjectSpec).toHaveBeenCalledTimes(1); }); @@ -311,8 +325,8 @@ describe('Config Rollback on Import Failure', () => { const result = await handleImport({ source: '/tmp/config.yaml' }); - expect(result.success).toBe(false); - expect(result.error).toContain('No agents found'); + assert(!result.success); + expect(result.error.message).toContain('No agents found'); // Config was never written, so no rollback expect(mockConfigIOInstance.writeProjectSpec).not.toHaveBeenCalled(); }); @@ -338,8 +352,8 @@ describe('Config Rollback on Import Failure', () => { const result = await handleImport({ source: '/tmp/config.yaml' }); - expect(result.success).toBe(false); - expect(result.error).toContain('Could not read deployed template'); + assert(!result.success); + expect(result.error.message).toContain('Could not read deployed template'); expect(mockConfigIOInstance.writeProjectSpec).toHaveBeenCalledTimes(2); const rollbackData = mockConfigIOInstance.writeProjectSpec.mock.calls[1]![0]; diff --git a/src/cli/commands/import/actions.ts b/src/cli/commands/import/actions.ts index 71eb70f83..94323e30c 100644 --- a/src/cli/commands/import/actions.ts +++ b/src/cli/commands/import/actions.ts @@ -1,4 +1,4 @@ -import { APP_DIR, ConfigIO, findConfigRoot } from '../../../lib'; +import { APP_DIR, ConfigIO, NoProjectError, ValidationError, findConfigRoot, toError } from '../../../lib'; import type { AgentCoreProjectSpec, AgentCoreRegion, @@ -124,9 +124,10 @@ export async function handleImport(options: ImportOptions): Promise` first, then run import from inside the project.'; - logger.endStep('error', error); + const error = new NoProjectError( + 'No agentcore project found in the current directory.\nRun `agentcore create ` first, then run import from inside the project.' + ); + logger.endStep('error', error.message); logger.finalize(false); return { success: false, @@ -160,7 +161,7 @@ export async function handleImport(options: ImportOptions): Promise { console.log(`Log: ${result.logPath}`); } } else { - console.error(`\n\x1b[31m[error]${reset} Import failed: ${result.error}`); + console.error(`\n\x1b[31m[error]${reset} Import failed: ${result.error.message}`); if (result.logPath) { console.error(`Log: ${result.logPath}`); } diff --git a/src/cli/commands/import/import-evaluator.ts b/src/cli/commands/import/import-evaluator.ts index 447f1316c..914a50dfa 100644 --- a/src/cli/commands/import/import-evaluator.ts +++ b/src/cli/commands/import/import-evaluator.ts @@ -152,7 +152,7 @@ export function registerImportEvaluator(importCmd: Command): void { console.log(` ID: ${result.resourceId}`); console.log(''); } else { - console.error(`\n${ANSI.red}[error]${ANSI.reset} ${result.error}`); + console.error(`\n${ANSI.red}[error]${ANSI.reset} ${result.error.message}`); if (result.logPath) { console.error(`Log: ${result.logPath}`); } diff --git a/src/cli/commands/import/import-gateway.ts b/src/cli/commands/import/import-gateway.ts index 3c2384e03..5a7822210 100644 --- a/src/cli/commands/import/import-gateway.ts +++ b/src/cli/commands/import/import-gateway.ts @@ -1,3 +1,4 @@ +import { toError } from '../../../lib'; import type { AgentCoreGateway, AgentCoreGatewayTarget, @@ -612,7 +613,7 @@ export async function handleImportGateway(options: ImportResourceOptions): Promi logger.finalize(false); return { success: false, - error: pipelineResult.error, + error: new Error(pipelineResult.error ?? 'Pipeline failed'), resourceType: 'gateway', resourceName: localName, logPath: logger.getRelativeLogPath(), @@ -638,7 +639,7 @@ export async function handleImportGateway(options: ImportResourceOptions): Promi } return { success: false, - error: message, + error: toError(err), resourceType: 'gateway', resourceName: options.name ?? '', logPath: importCtx?.logger.getRelativeLogPath(), @@ -680,7 +681,7 @@ export function registerImportGateway(importCmd: Command): void { console.log(` agentcore fetch access ${ANSI.dim}Get gateway URL and token${ANSI.reset}`); console.log(''); } else { - console.error(`\n${ANSI.red}[error]${ANSI.reset} ${result.error}`); + console.error(`\n${ANSI.red}[error]${ANSI.reset} ${result.error.message}`); if (result.logPath) { console.error(`Log: ${result.logPath}`); } diff --git a/src/cli/commands/import/import-memory.ts b/src/cli/commands/import/import-memory.ts index 2362d1353..02213c9f1 100644 --- a/src/cli/commands/import/import-memory.ts +++ b/src/cli/commands/import/import-memory.ts @@ -121,7 +121,7 @@ export function registerImportMemory(importCmd: Command): void { console.log(` ID: ${result.resourceId}`); console.log(''); } else { - console.error(`\n${ANSI.red}[error]${ANSI.reset} ${result.error}`); + console.error(`\n${ANSI.red}[error]${ANSI.reset} ${result.error.message}`); if (result.logPath) { console.error(`Log: ${result.logPath}`); } diff --git a/src/cli/commands/import/import-online-eval.ts b/src/cli/commands/import/import-online-eval.ts index 99935ecdf..b43fa174a 100644 --- a/src/cli/commands/import/import-online-eval.ts +++ b/src/cli/commands/import/import-online-eval.ts @@ -209,7 +209,7 @@ export function registerImportOnlineEval(importCmd: Command): void { console.log(` ID: ${result.resourceId}`); console.log(''); } else { - console.error(`\n${ANSI.red}[error]${ANSI.reset} ${result.error}`); + console.error(`\n${ANSI.red}[error]${ANSI.reset} ${result.error.message}`); if (result.logPath) { console.error(`Log: ${result.logPath}`); } diff --git a/src/cli/commands/import/import-runtime.ts b/src/cli/commands/import/import-runtime.ts index b1921d546..ff6ba0640 100644 --- a/src/cli/commands/import/import-runtime.ts +++ b/src/cli/commands/import/import-runtime.ts @@ -225,7 +225,7 @@ export function registerImportRuntime(importCmd: Command): void { console.log(` agentcore invoke ${ANSI.dim}Test your agent${ANSI.reset}`); console.log(''); } else { - console.error(`\n${ANSI.red}[error]${ANSI.reset} ${result.error}`); + console.error(`\n${ANSI.red}[error]${ANSI.reset} ${result.error.message}`); if (result.logPath) { console.error(`Log: ${result.logPath}`); } diff --git a/src/cli/commands/import/import-utils.ts b/src/cli/commands/import/import-utils.ts index 5829bbc6c..e29ada5ee 100644 --- a/src/cli/commands/import/import-utils.ts +++ b/src/cli/commands/import/import-utils.ts @@ -67,7 +67,7 @@ export function failResult( logger.finalize(false); return { success: false, - error, + error: new Error(error), resourceType, resourceName, logPath: logger.getRelativeLogPath(), diff --git a/src/cli/commands/import/resource-import.ts b/src/cli/commands/import/resource-import.ts index a21a160ec..ad543fd48 100644 --- a/src/cli/commands/import/resource-import.ts +++ b/src/cli/commands/import/resource-import.ts @@ -1,3 +1,4 @@ +import { toError } from '../../../lib'; import type { AgentCoreProjectSpec } from '../../../schema'; import { NAME_REGEX } from './constants'; import { executeCdkImportPipeline } from './import-pipeline'; @@ -228,7 +229,7 @@ export async function executeResourceImport( logger.finalize(false); return { success: false, - error: pipelineResult.error, + error: new Error(pipelineResult.error ?? 'Pipeline failed'), resourceType: descriptor.resourceType, resourceName: localName, logPath: logger.getRelativeLogPath(), @@ -254,7 +255,7 @@ export async function executeResourceImport( } return { success: false, - error: message, + error: toError(err), resourceType: descriptor.resourceType, resourceName: options.name ?? '', logPath: importCtx?.logger.getRelativeLogPath(), diff --git a/src/cli/commands/import/types.ts b/src/cli/commands/import/types.ts index 5d99c79c1..032f92793 100644 --- a/src/cli/commands/import/types.ts +++ b/src/cli/commands/import/types.ts @@ -1,3 +1,4 @@ +import type { Result } from '../../../lib/result'; import type { AgentCoreProjectSpec, AuthorizerConfig, @@ -89,27 +90,19 @@ export interface ResourceToImport { /** * Result of the import command. */ -export interface ImportResult { - success: boolean; - error?: string; +export type ImportResult = Result<{ projectSpec?: AgentCoreProjectSpec; importedAgents?: string[]; importedMemories?: string[]; stackName?: string; - logPath?: string; -} +}> & { logPath?: string }; /** * Result for single-resource import (runtime, memory, evaluator, etc.). */ -export interface ImportResourceResult { - success: boolean; - error?: string; - resourceType: ImportableResourceType; - resourceName: string; +export type ImportResourceResult = Result<{ resourceId?: string; - logPath?: string; -} +}> & { resourceType: ImportableResourceType; resourceName: string; logPath?: string }; /** * Options shared across import subcommands. diff --git a/src/cli/commands/invoke/action.ts b/src/cli/commands/invoke/action.ts index 4c5501b6f..23038eca1 100644 --- a/src/cli/commands/invoke/action.ts +++ b/src/cli/commands/invoke/action.ts @@ -1,4 +1,4 @@ -import { ConfigIO } from '../../../lib'; +import { ConfigIO, ResourceNotFoundError, ValidationError } from '../../../lib'; import type { AgentCoreProjectSpec, AwsDeploymentTargets, DeployedState } from '../../../schema'; import { buildAguiRunInput, @@ -55,20 +55,31 @@ export async function handleInvoke(context: InvokeContext, options: InvokeOption // Resolve target const targetNames = Object.keys(deployedState.targets); if (targetNames.length === 0) { - return { success: false, error: 'No deployed targets found. Run `agentcore deploy` first.' }; + return { + success: false, + error: new ResourceNotFoundError('No deployed targets found. Run `agentcore deploy` first.'), + }; } const selectedTargetName = options.targetName ?? targetNames[0]!; if (options.targetName && !targetNames.includes(options.targetName)) { - return { success: false, error: `Target '${options.targetName}' not found. Available: ${targetNames.join(', ')}` }; + return { + success: false, + error: new ResourceNotFoundError( + `Target '${options.targetName}' not found. Available: ${targetNames.join(', ')}` + ), + }; } const targetState = deployedState.targets[selectedTargetName]; const targetConfig = awsTargets.find(t => t.name === selectedTargetName); if (!targetConfig) { - return { success: false, error: `Target config '${selectedTargetName}' not found in aws-targets` }; + return { + success: false, + error: new ResourceNotFoundError(`Target config '${selectedTargetName}' not found in aws-targets`), + }; } // ── Route to harness or runtime ───────────────────────────────────────── @@ -94,24 +105,34 @@ export async function handleInvoke(context: InvokeContext, options: InvokeOption // ── Runtime invoke path ──────────────────────────────────────────────── if (project.runtimes.length === 0) { +<<<<<<< HEAD return { success: false, error: 'No runtimes or harnesses defined in configuration' }; +======= + return { success: false, error: new ValidationError('No agents defined in configuration') }; +>>>>>>> origin/main } // Resolve agent const agentNames = project.runtimes.map(a => a.name); if (!options.agentName && project.runtimes.length > 1) { - return { success: false, error: `Multiple runtimes found. Use --runtime to specify one: ${agentNames.join(', ')}` }; + return { + success: false, + error: new ValidationError(`Multiple runtimes found. Use --runtime to specify one: ${agentNames.join(', ')}`), + }; } const agentSpec = options.agentName ? project.runtimes.find(a => a.name === options.agentName) : project.runtimes[0]; if (options.agentName && !agentSpec) { - return { success: false, error: `Agent '${options.agentName}' not found. Available: ${agentNames.join(', ')}` }; + return { + success: false, + error: new ResourceNotFoundError(`Agent '${options.agentName}' not found. Available: ${agentNames.join(', ')}`), + }; } if (!agentSpec) { - return { success: false, error: 'No agents defined in configuration' }; + return { success: false, error: new ValidationError('No agents defined in configuration') }; } // Warn about VPC mode endpoint requirements @@ -125,7 +146,10 @@ export async function handleInvoke(context: InvokeContext, options: InvokeOption const agentState = targetState?.resources?.runtimes?.[agentSpec.name]; if (!agentState) { - return { success: false, error: `Agent '${agentSpec.name}' is not deployed to target '${selectedTargetName}'` }; + return { + success: false, + error: new ValidationError(`Agent '${agentSpec.name}' is not deployed to target '${selectedTargetName}'`), + }; } // Build config bundle baggage if a bundle is associated with this agent @@ -152,13 +176,18 @@ export async function handleInvoke(context: InvokeContext, options: InvokeOption } catch (err) { return { success: false, - error: `CUSTOM_JWT agent requires a bearer token. Auto-fetch failed: ${err instanceof Error ? err.message : String(err)}\nProvide one manually with --bearer-token.`, + error: new ValidationError( + `CUSTOM_JWT agent requires a bearer token. Auto-fetch failed: ${err instanceof Error ? err.message : String(err)}\nProvide one manually with --bearer-token.`, + { cause: err } + ), }; } } else { return { success: false, - error: `Agent '${agentSpec.name}' is configured for CUSTOM_JWT but no bearer token is available.\nEither provide --bearer-token or re-add the agent with --client-id and --client-secret to enable auto-fetch.`, + error: new ValidationError( + `Agent '${agentSpec.name}' is configured for CUSTOM_JWT but no bearer token is available.\nEither provide --bearer-token or re-add the agent with --client-id and --client-secret to enable auto-fetch.` + ), }; } } @@ -181,7 +210,7 @@ export async function handleInvoke(context: InvokeContext, options: InvokeOption }); const command = options.prompt; if (!command) { - return { success: false, error: '--exec requires a command (prompt)' }; + return { success: false, error: new ValidationError('--exec requires a command (prompt)') }; } logger.logPrompt(command, options.sessionId, options.userId); @@ -229,8 +258,18 @@ export async function handleInvoke(context: InvokeContext, options: InvokeOption logger.logResponse(stdout || stderr || `exit code: ${exitCode}`); if (options.json) { + if (exitCode === 0) { + return { + success: true, + agentName: agentSpec.name, + targetName: selectedTargetName, + response: JSON.stringify({ stdout, stderr, exitCode, status }), + logFilePath: logger.logFilePath, + }; + } return { - success: exitCode === 0, + success: false, + error: new Error(`Command exited with code ${exitCode}`), agentName: agentSpec.name, targetName: selectedTargetName, response: JSON.stringify({ stdout, stderr, exitCode, status }), @@ -241,9 +280,9 @@ export async function handleInvoke(context: InvokeContext, options: InvokeOption if (exitCode === undefined) { return { success: false, + error: new Error('Command stream ended without exit code'), agentName: agentSpec.name, targetName: selectedTargetName, - error: 'Command stream ended without exit code', logFilePath: logger.logFilePath, }; } @@ -251,9 +290,10 @@ export async function handleInvoke(context: InvokeContext, options: InvokeOption if (exitCode !== 0) { return { success: false, + error: new Error(`Command exited with code ${exitCode}${status === 'TIMED_OUT' ? ' (timed out)' : ''}`), agentName: agentSpec.name, targetName: selectedTargetName, - error: `Command exited with code ${exitCode}${status === 'TIMED_OUT' ? ' (timed out)' : ''}`, + response: JSON.stringify({ stdout, stderr, exitCode, status }), logFilePath: logger.logFilePath, }; } @@ -295,7 +335,9 @@ export async function handleInvoke(context: InvokeContext, options: InvokeOption } catch (err) { return { success: false, - error: `Failed to list MCP tools: ${err instanceof Error ? err.message : String(err)}`, + error: new Error(`Failed to list MCP tools: ${err instanceof Error ? err.message : String(err)}`, { + cause: err, + }), }; } } @@ -305,7 +347,7 @@ export async function handleInvoke(context: InvokeContext, options: InvokeOption if (!options.tool) { return { success: false, - error: 'MCP call-tool requires --tool . Use "list-tools" to see available tools.', + error: new Error('MCP call-tool requires --tool . Use "list-tools" to see available tools.'), }; } let args: Record = {}; @@ -313,7 +355,7 @@ export async function handleInvoke(context: InvokeContext, options: InvokeOption try { args = JSON.parse(options.input) as Record; } catch { - return { success: false, error: `Invalid JSON for --input: ${options.input}` }; + return { success: false, error: new ValidationError(`Invalid JSON for --input: ${options.input}`) }; } } try { @@ -329,7 +371,9 @@ export async function handleInvoke(context: InvokeContext, options: InvokeOption } catch (err) { return { success: false, - error: `Failed to call MCP tool: ${err instanceof Error ? err.message : String(err)}`, + error: new Error(`Failed to call MCP tool: ${err instanceof Error ? err.message : String(err)}`, { + cause: err, + }), }; } } @@ -337,14 +381,15 @@ export async function handleInvoke(context: InvokeContext, options: InvokeOption if (!options.prompt) { return { success: false, - error: - 'MCP agents require a command. Usage:\n agentcore invoke list-tools\n agentcore invoke call-tool --tool --input \'{"arg": "value"}\'', + error: new ValidationError( + 'MCP agents require a command. Usage:\n agentcore invoke list-tools\n agentcore invoke call-tool --tool --input \'{"arg": "value"}\'' + ), }; } } if (!options.prompt) { - return { success: false, error: 'No prompt provided. Usage: agentcore invoke "your prompt"' }; + return { success: false, error: new ValidationError('No prompt provided. Usage: agentcore invoke "your prompt"') }; } // A2A protocol handling — send JSON-RPC message/send via InvokeAgentRuntime @@ -377,7 +422,10 @@ export async function handleInvoke(context: InvokeContext, options: InvokeOption response, }; } catch (err) { - return { success: false, error: `A2A invoke failed: ${err instanceof Error ? err.message : String(err)}` }; + return { + success: false, + error: new Error(`A2A invoke failed: ${err instanceof Error ? err.message : String(err)}`, { cause: err }), + }; } } @@ -422,8 +470,20 @@ export async function handleInvoke(context: InvokeContext, options: InvokeOption logger.logResponse(response); + if (hasError) { + return { + success: false, + error: new Error(response), + agentName: agentSpec.name, + targetName: selectedTargetName, + response, + sessionId: aguiResult.sessionId, + logFilePath: logger.logFilePath, + }; + } + return { - success: !hasError, + success: true, agentName: agentSpec.name, targetName: selectedTargetName, response, @@ -432,7 +492,10 @@ export async function handleInvoke(context: InvokeContext, options: InvokeOption }; } catch (err) { logger.logError(err, 'AGUI invoke failed'); - return { success: false, error: `AGUI invoke failed: ${err instanceof Error ? err.message : String(err)}` }; + return { + success: false, + error: new Error(`AGUI invoke failed: ${err instanceof Error ? err.message : String(err)}`, { cause: err }), + }; } } diff --git a/src/cli/commands/invoke/command.tsx b/src/cli/commands/invoke/command.tsx index 390fe7b2e..9503d080b 100644 --- a/src/cli/commands/invoke/command.tsx +++ b/src/cli/commands/invoke/command.tsx @@ -1,3 +1,4 @@ +import { serializeResult } from '../../../lib'; import { getErrorMessage } from '../../errors'; import { COMMAND_DESCRIPTIONS } from '../../tui/copy'; import { requireProject, requireTTY } from '../../tui/guards'; @@ -79,7 +80,7 @@ async function handleInvokeCLI(options: InvokeOptions): Promise { } if (options.json) { - console.log(JSON.stringify(result)); + console.log(JSON.stringify(serializeResult(result))); } else if (options.stream) { // Streaming already wrote to stdout, just show session and log path if (result.sessionId) { @@ -94,7 +95,7 @@ async function handleInvokeCLI(options: InvokeOptions): Promise { if (result.success && result.response) { console.log(result.response); } else if (!result.success && result.error) { - console.error(result.error); + console.error(result.error.message); } if (result.sessionId) { console.error(`\nSession: ${result.sessionId}`); diff --git a/src/cli/commands/invoke/types.ts b/src/cli/commands/invoke/types.ts index d88dc5f69..31798b3de 100644 --- a/src/cli/commands/invoke/types.ts +++ b/src/cli/commands/invoke/types.ts @@ -1,3 +1,5 @@ +import type { Result } from '../../../lib/result'; + export interface InvokeOptions { agentName?: string; harnessName?: string; @@ -51,12 +53,10 @@ export interface InvokeOptions { actorId?: string; } -export interface InvokeResult { - success: boolean; +export type InvokeResult = Result & { + logFilePath?: string; agentName?: string; targetName?: string; response?: string; sessionId?: string; - error?: string; - logFilePath?: string; -} +}; diff --git a/src/cli/commands/logs/__tests__/action.test.ts b/src/cli/commands/logs/__tests__/action.test.ts index 842fa333a..fbaac8ee8 100644 --- a/src/cli/commands/logs/__tests__/action.test.ts +++ b/src/cli/commands/logs/__tests__/action.test.ts @@ -134,9 +134,9 @@ describe('resolveAgentContext', () => { const result = await resolveAgentContext(context, {}); expect(result.success).toBe(false); if (!result.success) { - expect(result.error).toContain('Multiple runtimes found'); - expect(result.error).toContain('AgentA'); - expect(result.error).toContain('AgentB'); + expect(result.error.message).toContain('Multiple runtimes found'); + expect(result.error.message).toContain('AgentA'); + expect(result.error.message).toContain('AgentB'); } }); @@ -208,7 +208,7 @@ describe('resolveAgentContext', () => { const result = await resolveAgentContext(makeContext(), { runtime: 'UnknownAgent' }); expect(result.success).toBe(false); if (!result.success) { - expect(result.error).toContain("Runtime 'UnknownAgent' not found"); + expect(result.error.message).toContain("Runtime 'UnknownAgent' not found"); } }); @@ -234,7 +234,11 @@ describe('resolveAgentContext', () => { const result = await resolveAgentContext(context, {}); expect(result.success).toBe(false); if (!result.success) { +<<<<<<< HEAD expect(result.error).toContain('No runtimes or harnesses defined'); +======= + expect(result.error.message).toContain('No runtimes defined'); +>>>>>>> origin/main } }); @@ -253,7 +257,7 @@ describe('resolveAgentContext', () => { const result = await resolveAgentContext(context, {}); expect(result.success).toBe(false); if (!result.success) { - expect(result.error).toContain('is not deployed'); + expect(result.error.message).toContain('is not deployed'); } }); }); diff --git a/src/cli/commands/logs/action.ts b/src/cli/commands/logs/action.ts index 26d48f6f0..448d1ad48 100644 --- a/src/cli/commands/logs/action.ts +++ b/src/cli/commands/logs/action.ts @@ -1,3 +1,5 @@ +import { ResourceNotFoundError, ValidationError } from '../../../lib'; +import type { Result } from '../../../lib/result'; import { parseTimeString } from '../../../lib/utils'; import { searchLogs, streamLogs } from '../../aws/cloudwatch'; import { DEFAULT_ENDPOINT_NAME } from '../../constants'; @@ -17,11 +19,6 @@ export interface AgentContext { logGroupName: string; } -export interface LogsResult { - success: boolean; - error?: string; -} - /** * Detect whether to stream or search based on options */ @@ -50,10 +47,15 @@ export function formatLogLine(event: { timestamp: number; message: string }, jso export async function resolveAgentContext( context: DeployedProjectConfig, options: LogsOptions +<<<<<<< HEAD ): Promise<{ success: true; agentContext: AgentContext } | { success: false; error: string }> { const result = await resolveAgentOrHarness(context, options); +======= +): Result<{ agentContext: AgentContext }> { + const result = resolveAgent(context, options); +>>>>>>> origin/main if (!result.success) { - return { success: false, error: result.error }; + return { success: false, error: new ResourceNotFoundError(result.error) }; } const { agent } = result; const endpointName = DEFAULT_ENDPOINT_NAME; @@ -74,12 +76,12 @@ export async function resolveAgentContext( /** * Main logs handler */ -export async function handleLogs(options: LogsOptions): Promise { +export async function handleLogs(options: LogsOptions): Promise { // Validate level early if (options.level && !VALID_LEVELS.includes(options.level.toLowerCase())) { return { success: false, - error: `Invalid log level: "${options.level}". Valid levels: ${VALID_LEVELS.join(', ')}`, + error: new ValidationError(`Invalid log level: "${options.level}". Valid levels: ${VALID_LEVELS.join(', ')}`), }; } @@ -97,7 +99,7 @@ export async function handleLogs(options: LogsOptions): Promise { try { filterPattern = buildFilterPattern({ level: options.level, query: options.query }); } catch (err) { - return { success: false, error: (err as Error).message }; + return { success: false, error: err instanceof Error ? err : new Error(String(err)) }; } const mode = detectMode(options); @@ -144,7 +146,9 @@ export async function handleLogs(options: LogsOptions): Promise { if (errorName === 'ResourceNotFoundException') { return { success: false, - error: `No logs found for agent '${agentContext.agentName}'. Has the agent been invoked?`, + error: new ResourceNotFoundError( + `No logs found for agent '${agentContext.agentName}'. Has the agent been invoked?` + ), }; } diff --git a/src/cli/commands/logs/command.tsx b/src/cli/commands/logs/command.tsx index 12728d995..3beb18346 100644 --- a/src/cli/commands/logs/command.tsx +++ b/src/cli/commands/logs/command.tsx @@ -35,7 +35,7 @@ export const registerLogs = (program: Command) => { const result = await handleLogs(cliOptions); if (!result.success) { - render({result.error}); + render({result.error.message}); process.exit(1); } } catch (error) { @@ -60,7 +60,7 @@ export const registerLogs = (program: Command) => { const result = await handleLogsEval(cliOptions); if (!result.success) { - render({result.error}); + render({result.error.message}); process.exit(1); } } catch (error) { diff --git a/src/cli/commands/pause/command.tsx b/src/cli/commands/pause/command.tsx index e99e266aa..c60f87158 100644 --- a/src/cli/commands/pause/command.tsx +++ b/src/cli/commands/pause/command.tsx @@ -1,4 +1,4 @@ -import { ConfigIO } from '../../../lib'; +import { ConfigIO, serializeResult } from '../../../lib'; import { listABTests, updateABTest } from '../../aws/agentcore-ab-tests'; import { stopBatchEvaluation } from '../../aws/agentcore-batch-evaluation'; import { getErrorMessage } from '../../errors'; @@ -52,12 +52,12 @@ function registerOnlineEvalSubcommand(parent: Command, action: 'pause' | 'resume const result = await handlePauseResume(options, action); if (cliOptions.json) { - console.log(JSON.stringify(result)); + console.log(JSON.stringify(serializeResult(result))); } else if (result.success) { const displayName = cliOptions.arn ? result.configId : name; console.log(`${pastTense} online eval config "${displayName}" (status: ${result.executionStatus})`); } else { - render({result.error}); + render({result.error.message}); } process.exit(result.success ? 0 : 1); diff --git a/src/cli/commands/remove/command.tsx b/src/cli/commands/remove/command.tsx index 3705c523f..5a03209a6 100644 --- a/src/cli/commands/remove/command.tsx +++ b/src/cli/commands/remove/command.tsx @@ -1,4 +1,4 @@ -import { ConfigIO } from '../../../lib'; +import { ConfigIO, serializeResult, toError } from '../../../lib'; import { getErrorMessage } from '../../errors'; import { COMMAND_DESCRIPTIONS } from '../../tui/copy'; import { requireProject, requireTTY } from '../../tui/guards'; @@ -49,15 +49,24 @@ async function handleRemoveAll(_options: RemoveAllOptions): Promise { validateRemoveAllOptions(options); +<<<<<<< HEAD const result = await handleRemoveAll(options); console.log(JSON.stringify(result)); process.exit(result.success ? 0 : 1); +======= + await runCliCommand('remove.all', !!options.json, async () => { + const result = await handleRemoveAll(options); + if (!result.success) throw result.error; + console.log(JSON.stringify(serializeResult(result))); + return {}; + }); +>>>>>>> origin/main } export const registerRemove = (program: Command): Command => { diff --git a/src/cli/commands/remove/types.ts b/src/cli/commands/remove/types.ts index 06089bfeb..e2ab9b09f 100644 --- a/src/cli/commands/remove/types.ts +++ b/src/cli/commands/remove/types.ts @@ -1,3 +1,5 @@ +import type { Result } from '../../../lib/result'; + export type ResourceType = | 'agent' | 'gateway' @@ -26,11 +28,9 @@ export interface RemoveAllOptions { json?: boolean; } -export interface RemoveResult { - success: boolean; +export type RemoveResult = Result<{ resourceType?: ResourceType; resourceName?: string; message?: string; note?: string; - error?: string; -} +}>; diff --git a/src/cli/commands/run/command.tsx b/src/cli/commands/run/command.tsx index e5ba8ca59..09b18358d 100644 --- a/src/cli/commands/run/command.tsx +++ b/src/cli/commands/run/command.tsx @@ -1,3 +1,4 @@ +import { serializeResult } from '../../../lib'; import type { RecommendationType } from '../../aws/agentcore-recommendation'; import { getErrorMessage } from '../../errors'; import { handleRunEval } from '../../operations/eval'; @@ -25,7 +26,7 @@ const RECOMMENDATION_TYPE_MAP: Record = { }; function formatRunOutput(result: Awaited>): void { - if (!result.run) return; + if (!result.success) return; const { run } = result; const date = new Date(run.timestamp).toLocaleString([], { @@ -56,7 +57,7 @@ function formatRunOutput(result: Awaited>): voi for (const r of run.results) { const score = r.aggregateScore.toFixed(2); - const errors = r.sessionScores.filter(s => s.errorMessage).length; + const errors = r.sessionScores.filter((s: { errorMessage?: string }) => s.errorMessage).length; const errorSuffix = errors > 0 ? ` (${errors} errors)` : ''; console.log(` ${r.evaluator}: ${score}${errorSuffix}`); } @@ -146,12 +147,12 @@ export const registerRun = (program: Command) => { const result = await handleRunEval(options); if (cliOptions.json) { - console.log(JSON.stringify(result)); + console.log(JSON.stringify(serializeResult(result))); } else if (result.success) { formatRunOutput(result); } else { formatRunOutput(result); - render({result.error}); + render({result.error.message}); } process.exit(result.success ? 0 : 1); @@ -240,11 +241,11 @@ export const registerRun = (program: Command) => { } if (cliOptions.json) { - console.log(JSON.stringify(result)); + console.log(JSON.stringify(serializeResult(result))); } else if (result.success) { formatBatchEvalOutput(result); } else { - render({result.error}); + render({result.error.message}); if (result.logFilePath) { console.error(`\nLog: ${result.logFilePath}`); } @@ -401,9 +402,9 @@ export const registerRun = (program: Command) => { if (!result.success) { if (cliOptions.json) { - console.log(JSON.stringify(result)); + console.log(JSON.stringify(serializeResult(result))); } else { - render({result.error}); + render({result.error.message}); if (result.logFilePath) { console.error(`\nLog: ${result.logFilePath}`); } @@ -428,7 +429,7 @@ export const registerRun = (program: Command) => { } if (cliOptions.json) { - console.log(JSON.stringify(result)); + console.log(JSON.stringify(serializeResult(result))); } else { console.log(`\nRecommendation ID: ${result.recommendationId}`); @@ -467,7 +468,7 @@ export const registerRun = (program: Command) => { ); console.log(`Local config for "${cliOptions.bundleName}" has been updated to match.`); } else { - console.log(`\nCould not sync config bundle: ${applyResult.error}`); + console.log(`\nCould not sync config bundle: ${applyResult.error.message}`); } } catch { // Non-fatal — user can manually sync diff --git a/src/cli/commands/shared/__tests__/header-utils.test.ts b/src/cli/commands/shared/__tests__/header-utils.test.ts index 640a3a9b0..6e29da9ba 100644 --- a/src/cli/commands/shared/__tests__/header-utils.test.ts +++ b/src/cli/commands/shared/__tests__/header-utils.test.ts @@ -32,11 +32,26 @@ describe('normalizeHeaderName', () => { ); }); - it('auto-prefixes a bare suffix like "MyHeader"', () => { + it('passes through X- prefixed headers unchanged', () => { + expect(normalizeHeaderName('X-Api-Key')).toBe('X-Api-Key'); + expect(normalizeHeaderName('X-Custom-Signature')).toBe('X-Custom-Signature'); + expect(normalizeHeaderName('X-Request-Id')).toBe('X-Request-Id'); + }); + + it('canonicalizes Runtime-Custom- prefix casing but preserves suffix as-typed', () => { + expect(normalizeHeaderName('x-amzn-bedrock-agentcore-runtime-custom-myheader')).toBe( + 'X-Amzn-Bedrock-AgentCore-Runtime-Custom-myheader' + ); + expect(normalizeHeaderName('X-AMZN-BEDROCK-AGENTCORE-RUNTIME-CUSTOM-MyHeader')).toBe( + 'X-Amzn-Bedrock-AgentCore-Runtime-Custom-MyHeader' + ); + }); + + it('auto-prefixes a bare suffix like "MyHeader" (no X- prefix, backward compat)', () => { expect(normalizeHeaderName('MyHeader')).toBe('X-Amzn-Bedrock-AgentCore-Runtime-Custom-MyHeader'); }); - it('auto-prefixes suffix with hyphens like "My-Custom-Header"', () => { + it('auto-prefixes suffix with hyphens like "My-Custom-Header" (no X- prefix)', () => { expect(normalizeHeaderName('My-Custom-Header')).toBe('X-Amzn-Bedrock-AgentCore-Runtime-Custom-My-Custom-Header'); }); }); @@ -59,6 +74,11 @@ describe('parseAndNormalizeHeaders', () => { ]); }); + it('passes through X- prefixed headers without auto-prefixing', () => { + const result = parseAndNormalizeHeaders('X-Api-Key, X-Custom-Signature, authorization'); + expect(result).toEqual(['X-Api-Key', 'X-Custom-Signature', 'Authorization']); + }); + it('deduplicates after normalization', () => { const result = parseAndNormalizeHeaders('MyHeader, X-Amzn-Bedrock-AgentCore-Runtime-Custom-MyHeader'); expect(result).toEqual(['X-Amzn-Bedrock-AgentCore-Runtime-Custom-MyHeader']); @@ -69,13 +89,14 @@ describe('parseAndNormalizeHeaders', () => { expect(result).toEqual(['Authorization']); }); + it('deduplicates case-insensitively for X- headers', () => { + const result = parseAndNormalizeHeaders('X-Api-Key, x-api-key'); + expect(result).toEqual(['X-Api-Key']); + }); + it('trims whitespace around values', () => { - const result = parseAndNormalizeHeaders(' MyHeader , authorization , Another-Header '); - expect(result).toEqual([ - 'X-Amzn-Bedrock-AgentCore-Runtime-Custom-MyHeader', - 'Authorization', - 'X-Amzn-Bedrock-AgentCore-Runtime-Custom-Another-Header', - ]); + const result = parseAndNormalizeHeaders(' MyHeader , authorization , X-Api-Key '); + expect(result).toEqual(['X-Amzn-Bedrock-AgentCore-Runtime-Custom-MyHeader', 'Authorization', 'X-Api-Key']); }); }); @@ -98,12 +119,37 @@ describe('validateHeaderAllowlist', () => { expect(validateHeaderAllowlist('authorization')).toEqual({ success: true }); }); + it('returns success for X- prefixed headers from AWS docs', () => { + expect(validateHeaderAllowlist('X-Api-Key')).toEqual({ success: true }); + expect(validateHeaderAllowlist('X-Custom-Signature')).toEqual({ success: true }); + }); + it('returns success for mixed valid headers', () => { - expect(validateHeaderAllowlist('Authorization, MyHeader, X-Amzn-Bedrock-AgentCore-Runtime-Custom-Another')).toEqual( + expect(validateHeaderAllowlist('Authorization, X-Api-Key, X-Amzn-Bedrock-AgentCore-Runtime-Custom-UserId')).toEqual( { success: true } ); }); + it('returns success for headers with underscores', () => { + expect(validateHeaderAllowlist('X-My_Custom_Header')).toEqual({ success: true }); + }); + + it('returns error for x-amz- prefixed headers', () => { + const result = validateHeaderAllowlist('x-amz-security-token'); + expect(result.success).toBe(false); + expect(result.error).toContain('reserved for AWS request signing'); + }); + + it('returns error for x-amzn- prefixed headers (not Runtime-Custom-)', () => { + const result = validateHeaderAllowlist('x-amzn-trace-id'); + expect(result.success).toBe(false); + expect(result.error).toContain('x-amzn-'); + }); + + it('returns success for X-Amzn-Bedrock-AgentCore-Runtime-Custom- headers', () => { + expect(validateHeaderAllowlist('X-Amzn-Bedrock-AgentCore-Runtime-Custom-UserId')).toEqual({ success: true }); + }); + it('returns error when exceeding max 20 headers', () => { const headers = Array.from({ length: 21 }, (_, i) => `Header${i}`).join(', '); const result = validateHeaderAllowlist(headers); @@ -119,13 +165,19 @@ describe('validateHeaderAllowlist', () => { it('returns error for header names containing whitespace', () => { const result = validateHeaderAllowlist('My Header'); expect(result.success).toBe(false); - expect(result.error).toContain('Invalid header name'); + expect(result.error).toContain('must contain only'); }); it('returns error for header names with special characters', () => { const result = validateHeaderAllowlist('My@Header'); expect(result.success).toBe(false); - expect(result.error).toContain('Invalid header name'); + expect(result.error).toContain('must contain only'); + }); + + it('returns error for header with dots', () => { + const result = validateHeaderAllowlist('My.Header'); + expect(result.success).toBe(false); + expect(result.error).toContain('must contain only'); }); }); @@ -137,6 +189,13 @@ describe('parseHeaderFlag', () => { }); }); + it('parses X- prefixed header without auto-prefixing', () => { + expect(parseHeaderFlag('X-Api-Key: my-key')).toEqual({ + name: 'X-Api-Key', + value: 'my-key', + }); + }); + it('parses "Key:Value" format without space', () => { expect(parseHeaderFlag('MyHeader:some-value')).toEqual({ name: 'X-Amzn-Bedrock-AgentCore-Runtime-Custom-MyHeader', @@ -183,6 +242,14 @@ describe('parseHeaderFlags', () => { }); }); + it('parses X- prefixed headers without prefixing', () => { + const result = parseHeaderFlags(['X-Api-Key: key123', 'X-Custom-Signature: sha256=abc']); + expect(result).toEqual({ + 'X-Api-Key': 'key123', + 'X-Custom-Signature': 'sha256=abc', + }); + }); + it('returns empty object for empty array', () => { expect(parseHeaderFlags([])).toEqual({}); }); diff --git a/src/cli/commands/shared/header-utils.ts b/src/cli/commands/shared/header-utils.ts index 6791d1647..76a9981ee 100644 --- a/src/cli/commands/shared/header-utils.ts +++ b/src/cli/commands/shared/header-utils.ts @@ -1,18 +1,22 @@ import { HEADER_ALLOWLIST_PREFIX as HEADER_ALLOWLIST_PREFIX_FROM_SCHEMA, + HEADER_NAME_PATTERN as HEADER_NAME_PATTERN_FROM_SCHEMA, MAX_HEADER_ALLOWLIST_SIZE as MAX_HEADER_ALLOWLIST_SIZE_FROM_SCHEMA, + checkAllowlistHeader, } from '../../../schema/schemas/agent-env'; export const HEADER_ALLOWLIST_PREFIX = HEADER_ALLOWLIST_PREFIX_FROM_SCHEMA; +export const HEADER_NAME_PATTERN = HEADER_NAME_PATTERN_FROM_SCHEMA; export const MAX_HEADER_ALLOWLIST_SIZE = MAX_HEADER_ALLOWLIST_SIZE_FROM_SCHEMA; -const HEADER_NAME_PATTERN = /^[A-Za-z0-9-]+$/; - /** * Normalize a header name according to AgentCore Runtime rules: * - "Authorization" (case-insensitive) -> "Authorization" - * - Headers already starting with the prefix (case-insensitive) -> canonical prefix + original suffix - * - Other headers -> prepend the prefix + * - Headers starting with X-Amzn-Bedrock-AgentCore-Runtime-Custom- (case-insensitive) -> + * canonical prefix casing + original suffix + * - Any other X- prefixed header (e.g. X-Api-Key, X-Custom-Signature) -> pass through unchanged + * - Bare suffixes without X- prefix (e.g. MyHeader) -> auto-prefix with Runtime-Custom- for + * backward compatibility */ export function normalizeHeaderName(input: string): string { if (input.toLowerCase() === 'authorization') { @@ -21,11 +25,15 @@ export function normalizeHeaderName(input: string): string { if (input.toLowerCase().startsWith(HEADER_ALLOWLIST_PREFIX.toLowerCase())) { return `${HEADER_ALLOWLIST_PREFIX}${input.slice(HEADER_ALLOWLIST_PREFIX.length)}`; } + if (/^x-/i.test(input)) { + return input; + } return `${HEADER_ALLOWLIST_PREFIX}${input}`; } /** * Parse a comma-separated string of header names, normalize each, and deduplicate. + * Deduplication is case-insensitive per AWS docs. * Returns an array of normalized header names. */ export function parseAndNormalizeHeaders(input: string): string[] { @@ -35,7 +43,16 @@ export function parseAndNormalizeHeaders(input: string): string[] { .filter(Boolean) .map(normalizeHeaderName); - return Array.from(new Set(headers)); + const seen = new Set(); + const result: string[] = []; + for (const header of headers) { + const lower = header.toLowerCase(); + if (!seen.has(lower)) { + seen.add(lower); + result.push(header); + } + } + return result; } /** @@ -52,20 +69,18 @@ export function validateHeaderAllowlist(value: string): { success: boolean; erro .split(',') .map(s => s.trim()) .filter(Boolean); + for (const name of rawNames) { - if (!HEADER_NAME_PATTERN.test(name)) { - return { - success: false, - error: `Invalid header name "${name}". Header names may only contain letters, numbers, and hyphens.`, - }; + const error = checkAllowlistHeader(name); + if (error) { + return { success: false, error }; } } - const headers = parseAndNormalizeHeaders(value); - if (headers.length > MAX_HEADER_ALLOWLIST_SIZE) { + if (rawNames.length > MAX_HEADER_ALLOWLIST_SIZE) { return { success: false, - error: `Header allowlist cannot exceed ${MAX_HEADER_ALLOWLIST_SIZE} headers. Provided: ${headers.length}`, + error: `Header allowlist cannot exceed ${MAX_HEADER_ALLOWLIST_SIZE} headers. Provided: ${rawNames.length}`, }; } diff --git a/src/cli/commands/status/__tests__/action.test.ts b/src/cli/commands/status/__tests__/action.test.ts index 1c3a089dc..f8c94c6bc 100644 --- a/src/cli/commands/status/__tests__/action.test.ts +++ b/src/cli/commands/status/__tests__/action.test.ts @@ -1,7 +1,8 @@ import type { AgentCoreProjectSpec, DeployedResourceState } from '../../../../schema/index.js'; import { computeResourceStatuses, handleProjectStatus } from '../action.js'; -import type { StatusContext } from '../action.js'; +import type { ResourceStatusEntry, StatusContext } from '../action.js'; import { buildRuntimeInvocationUrl } from '../constants.js'; +import assert from 'node:assert'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; const mockGetAgentRuntimeStatus = vi.fn(); @@ -508,9 +509,11 @@ describe('handleProjectStatus — live enrichment', () => { const result = await handleProjectStatus(makeContext()); - expect(result.success).toBe(true); + assert(result.success); - const evalEntry = result.resources.find(r => r.resourceType === 'evaluator' && r.name === 'MyEval'); + const evalEntry = result.resources.find( + (r: ResourceStatusEntry) => r.resourceType === 'evaluator' && r.name === 'MyEval' + ); expect(evalEntry).toBeDefined(); expect(evalEntry!.detail).toContain('ACTIVE'); @@ -536,9 +539,11 @@ describe('handleProjectStatus — live enrichment', () => { const result = await handleProjectStatus(makeContext()); - expect(result.success).toBe(true); + assert(result.success); - const configEntry = result.resources.find(r => r.resourceType === 'online-eval' && r.name === 'MyConfig'); + const configEntry = result.resources.find( + (r: ResourceStatusEntry) => r.resourceType === 'online-eval' && r.name === 'MyConfig' + ); expect(configEntry).toBeDefined(); expect(configEntry!.detail).toContain('ACTIVE'); expect(configEntry!.detail).toContain('ENABLED'); @@ -560,9 +565,11 @@ describe('handleProjectStatus — live enrichment', () => { const result = await handleProjectStatus(makeContext()); - expect(result.success).toBe(true); + assert(result.success); - const evalEntry = result.resources.find(r => r.resourceType === 'evaluator' && r.name === 'MyEval'); + const evalEntry = result.resources.find( + (r: ResourceStatusEntry) => r.resourceType === 'evaluator' && r.name === 'MyEval' + ); expect(evalEntry).toBeDefined(); expect(evalEntry!.error).toBe('AccessDenied'); }); @@ -578,9 +585,11 @@ describe('handleProjectStatus — live enrichment', () => { const result = await handleProjectStatus(makeContext()); - expect(result.success).toBe(true); + assert(result.success); - const configEntry = result.resources.find(r => r.resourceType === 'online-eval' && r.name === 'MyConfig'); + const configEntry = result.resources.find( + (r: ResourceStatusEntry) => r.resourceType === 'online-eval' && r.name === 'MyConfig' + ); expect(configEntry).toBeDefined(); expect(configEntry!.error).toBe('ResourceNotFound'); }); @@ -624,9 +633,11 @@ describe('handleProjectStatus — live enrichment', () => { const result = await handleProjectStatus(ctx); - expect(result.success).toBe(true); + assert(result.success); - const evalEntry = result.resources.find(r => r.resourceType === 'evaluator' && r.name === 'MyEval'); + const evalEntry = result.resources.find( + (r: ResourceStatusEntry) => r.resourceType === 'evaluator' && r.name === 'MyEval' + ); expect(evalEntry).toBeDefined(); expect(evalEntry!.deploymentState).toBe('local-only'); expect(mockGetEvaluator).not.toHaveBeenCalled(); @@ -697,8 +708,10 @@ describe('handleProjectStatus — invocation URL enrichment', () => { const result = await handleProjectStatus(ctx); - expect(result.success).toBe(true); - const agentEntry = result.resources.find(r => r.resourceType === 'agent' && r.name === 'MyAgent'); + assert(result.success); + const agentEntry = result.resources.find( + (r: ResourceStatusEntry) => r.resourceType === 'agent' && r.name === 'MyAgent' + ); expect(agentEntry).toBeDefined(); expect(agentEntry!.invocationUrl).toBe( `https://bedrock-agentcore.us-east-1.amazonaws.com/runtimes/${encodeURIComponent(runtimeArn)}/invocations` @@ -723,8 +736,10 @@ describe('handleProjectStatus — invocation URL enrichment', () => { const result = await handleProjectStatus(ctx); - expect(result.success).toBe(true); - const agentEntry = result.resources.find(r => r.resourceType === 'agent' && r.name === 'LocalAgent'); + assert(result.success); + const agentEntry = result.resources.find( + (r: ResourceStatusEntry) => r.resourceType === 'agent' && r.name === 'LocalAgent' + ); expect(agentEntry).toBeDefined(); expect(agentEntry!.invocationUrl).toBeUndefined(); }); @@ -758,8 +773,10 @@ describe('handleProjectStatus — invocation URL enrichment', () => { const result = await handleProjectStatus(ctx); - expect(result.success).toBe(true); - const agentEntry = result.resources.find(r => r.resourceType === 'agent' && r.name === 'FailAgent'); + assert(result.success); + const agentEntry = result.resources.find( + (r: ResourceStatusEntry) => r.resourceType === 'agent' && r.name === 'FailAgent' + ); expect(agentEntry).toBeDefined(); expect(agentEntry!.error).toBe('Timeout'); expect(agentEntry!.invocationUrl).toBe( @@ -798,8 +815,10 @@ describe('handleProjectStatus — invocation URL enrichment', () => { const result = await handleProjectStatus(ctx); - expect(result.success).toBe(true); - const agentEntry = result.resources.find(r => r.resourceType === 'agent' && r.name === 'OldAgent'); + assert(result.success); + const agentEntry = result.resources.find( + (r: ResourceStatusEntry) => r.resourceType === 'agent' && r.name === 'OldAgent' + ); expect(agentEntry).toBeDefined(); expect(agentEntry!.deploymentState).toBe('pending-removal'); expect(agentEntry!.invocationUrl).toBeUndefined(); diff --git a/src/cli/commands/status/action.ts b/src/cli/commands/status/action.ts index 1ef72aaca..be3c7cacb 100644 --- a/src/cli/commands/status/action.ts +++ b/src/cli/commands/status/action.ts @@ -1,4 +1,5 @@ -import { ConfigIO } from '../../../lib'; +import { ConfigIO, ResourceNotFoundError, toError } from '../../../lib'; +import type { Result } from '../../../lib/result'; import type { AgentCoreProjectSpec, AwsDeploymentTargets, DeployedResourceState, DeployedState } from '../../../schema'; import { getAgentRuntimeStatus } from '../../aws'; import { getEvaluator, getOnlineEvaluationConfig } from '../../aws/agentcore-control'; @@ -34,15 +35,10 @@ export interface ResourceStatusEntry { invocationUrl?: string; } -export interface ProjectStatusResult { - success: boolean; - projectName: string; - targetName: string; +export type ProjectStatusResult = Result<{ targetRegion?: string; resources: ResourceStatusEntry[]; - error?: string; - logPath?: string; -} +}> & { projectName?: string; targetName?: string; logPath?: string; resources?: ResourceStatusEntry[] }; export interface StatusContext { project: AgentCoreProjectSpec; @@ -50,14 +46,11 @@ export interface StatusContext { awsTargets: AwsDeploymentTargets; } -export interface RuntimeLookupResult { - success: boolean; +export type RuntimeLookupResult = Result<{ targetName?: string; runtimeId?: string; runtimeStatus?: string; - error?: string; - logPath?: string; -} +}> & { logPath?: string }; /** * Loads configuration required for status check. @@ -343,10 +336,10 @@ export async function handleProjectStatus( logger.finalize(false); return { success: false, + error: new Error(error), projectName: project.name, targetName: options.targetName, resources: [], - error, logPath: logger.getRelativeLogPath(), }; } @@ -550,7 +543,7 @@ export async function handleRuntimeLookup( const error = 'No deployment targets found. Run `agentcore create` first.'; logger.endStep('error', error); logger.finalize(false); - return { success: false, error, logPath: logger.getRelativeLogPath() }; + return { success: false, error: new ResourceNotFoundError(error), logPath: logger.getRelativeLogPath() }; } const selectedTargetName = options.targetName ?? targetNames[0]!; @@ -559,7 +552,7 @@ export async function handleRuntimeLookup( const error = `Target '${options.targetName}' not found. Available: ${targetNames.join(', ')}`; logger.endStep('error', error); logger.finalize(false); - return { success: false, error, logPath: logger.getRelativeLogPath() }; + return { success: false, error: new ResourceNotFoundError(error), logPath: logger.getRelativeLogPath() }; } const targetConfig = awsTargets.find(target => target.name === selectedTargetName); @@ -568,7 +561,7 @@ export async function handleRuntimeLookup( const error = `Target config '${selectedTargetName}' not found in aws-targets`; logger.endStep('error', error); logger.finalize(false); - return { success: false, error, logPath: logger.getRelativeLogPath() }; + return { success: false, error: new ResourceNotFoundError(error), logPath: logger.getRelativeLogPath() }; } logger.log(`Target: ${selectedTargetName} (${targetConfig.region})`); @@ -596,6 +589,6 @@ export async function handleRuntimeLookup( const errorMsg = getErrorMessage(error); logger.endStep('error', errorMsg); logger.finalize(false); - return { success: false, error: errorMsg, logPath: logger.getRelativeLogPath() }; + return { success: false, error: toError(error), logPath: logger.getRelativeLogPath() }; } } diff --git a/src/cli/commands/status/command.tsx b/src/cli/commands/status/command.tsx index bd997a999..06df70e61 100644 --- a/src/cli/commands/status/command.tsx +++ b/src/cli/commands/status/command.tsx @@ -1,3 +1,4 @@ +import { serializeResult } from '../../../lib'; import { getErrorMessage } from '../../errors'; import { COMMAND_DESCRIPTIONS } from '../../tui/copy'; import { requireProject } from '../../tui/guards'; @@ -101,12 +102,12 @@ export const registerStatus = (program: Command) => { }); if (cliOptions.json) { - console.log(JSON.stringify(result, null, 2)); + console.log(JSON.stringify(serializeResult(result), null, 2)); return; } if (!result.success) { - render({result.error}); + render({result.error.message}); return; } @@ -127,13 +128,17 @@ export const registerStatus = (program: Command) => { }); if (cliOptions.json) { - const filtered = filterResources(result.resources, cliOptions); - console.log(JSON.stringify({ ...result, resources: filtered }, null, 2)); + if (result.success) { + const filtered = filterResources(result.resources, cliOptions); + console.log(JSON.stringify({ ...result, resources: filtered }, null, 2)); + } else { + console.log(JSON.stringify(serializeResult(result), null, 2)); + } return; } if (!result.success) { - render({result.error}); + render({result.error.message}); return; } @@ -155,7 +160,7 @@ export const registerStatus = (program: Command) => { render( - AgentCore Status (target: {result.targetName || 'No target configured'} + AgentCore Status (target: {result.targetName ?? 'No target configured'} {result.targetRegion ? `, ${result.targetRegion}` : ''}) diff --git a/src/cli/commands/traces/action.ts b/src/cli/commands/traces/action.ts index 719245480..024548a17 100644 --- a/src/cli/commands/traces/action.ts +++ b/src/cli/commands/traces/action.ts @@ -1,17 +1,16 @@ +import { ResourceNotFoundError, ValidationError } from '../../../lib'; +import type { Result } from '../../../lib/result'; import { parseTimeString } from '../../../lib/utils'; import type { DeployedProjectConfig } from '../../operations/resolve-agent'; import { resolveAgentOrHarness } from '../../operations/resolve-agent'; import { buildTraceConsoleUrl, getTrace, listTraces } from '../../operations/traces'; import type { TracesGetOptions, TracesListOptions } from './types'; -export interface TracesListResult { - success: boolean; +export type TracesListResult = Result<{ agentName?: string; targetName?: string; - consoleUrl?: string; - traces?: { traceId: string; timestamp: string; sessionId?: string }[]; - error?: string; -} + traces: { traceId: string; timestamp: string; sessionId?: string }[]; +}> & { consoleUrl?: string }; export async function handleTracesList( context: DeployedProjectConfig, @@ -19,7 +18,7 @@ export async function handleTracesList( ): Promise { const resolved = await resolveAgentOrHarness(context, options); if (!resolved.success) { - return { success: false, error: resolved.error }; + return { success: false, error: new ResourceNotFoundError(resolved.error) }; } const { agent } = resolved; @@ -33,7 +32,7 @@ export async function handleTracesList( const limit = options.limit ? parseInt(options.limit, 10) : 20; if (isNaN(limit)) { - return { success: false, error: '--limit must be a number' }; + return { success: false, error: new ValidationError('--limit must be a number') }; } // Parse time options @@ -68,14 +67,11 @@ export async function handleTracesList( }; } -export interface TracesGetResult { - success: boolean; +export type TracesGetResult = Result<{ agentName?: string; targetName?: string; - consoleUrl?: string; filePath?: string; - error?: string; -} +}> & { consoleUrl?: string }; export async function handleTracesGet( context: DeployedProjectConfig, @@ -84,7 +80,7 @@ export async function handleTracesGet( ): Promise { const resolved = await resolveAgentOrHarness(context, options); if (!resolved.success) { - return { success: false, error: resolved.error }; + return { success: false, error: new ResourceNotFoundError(resolved.error) }; } const { agent } = resolved; diff --git a/src/cli/commands/traces/command.tsx b/src/cli/commands/traces/command.tsx index a25b8a613..0f2a11075 100644 --- a/src/cli/commands/traces/command.tsx +++ b/src/cli/commands/traces/command.tsx @@ -38,7 +38,7 @@ export const registerTraces = (program: Command) => { if (!result.success) { render( - Error: {result.error} + Error: {result.error.message} {result.consoleUrl && Console: {result.consoleUrl}} ); @@ -111,7 +111,7 @@ export const registerTraces = (program: Command) => { if (!result.success) { render( - Error: {result.error} + Error: {result.error.message} {result.consoleUrl && Console: {result.consoleUrl}} ); diff --git a/src/cli/commands/validate/__tests__/action.test.ts b/src/cli/commands/validate/__tests__/action.test.ts index a27894f64..6944f9132 100644 --- a/src/cli/commands/validate/__tests__/action.test.ts +++ b/src/cli/commands/validate/__tests__/action.test.ts @@ -1,4 +1,5 @@ import { handleValidate } from '../action.js'; +import assert from 'node:assert'; import { afterEach, describe, expect, it, vi } from 'vitest'; const { @@ -77,7 +78,8 @@ describe('handleValidate', () => { const result = await handleValidate({}); expect(result.success).toBe(false); - expect(result.error).toContain('No agentcore project found'); + assert(!result.success); + expect(result.error.message).toContain('No agentcore project found'); }); it('returns success when all configs are valid', async () => { @@ -98,7 +100,8 @@ describe('handleValidate', () => { const result = await handleValidate({}); expect(result.success).toBe(false); - expect(result.error).toContain('invalid project'); + assert(!result.success); + expect(result.error.message).toContain('invalid project'); }); it('returns error when AWS targets fails', async () => { @@ -109,7 +112,8 @@ describe('handleValidate', () => { const result = await handleValidate({}); expect(result.success).toBe(false); - expect(result.error).toContain('bad targets'); + assert(!result.success); + expect(result.error.message).toContain('bad targets'); }); it('validates state file when it exists', async () => { @@ -135,7 +139,8 @@ describe('handleValidate', () => { const result = await handleValidate({}); expect(result.success).toBe(false); - expect(result.error).toContain('bad state'); + assert(!result.success); + expect(result.error.message).toContain('bad state'); }); it('uses custom directory when provided', async () => { @@ -158,7 +163,8 @@ describe('handleValidate', () => { const result = await handleValidate({}); expect(result.success).toBe(false); - expect(result.error).toBe('field "name" is required'); + assert(!result.success); + expect(result.error.message).toBe('field "name" is required'); }); it('formats ConfigParseError with cause', async () => { @@ -169,8 +175,9 @@ describe('handleValidate', () => { const result = await handleValidate({}); expect(result.success).toBe(false); - expect(result.error).toContain('Invalid JSON in agentcore.json'); - expect(result.error).toContain('Unexpected token'); + assert(!result.success); + expect(result.error.message).toContain('Invalid JSON in agentcore.json'); + expect(result.error.message).toContain('Unexpected token'); }); it('formats ConfigReadError with cause', async () => { @@ -183,8 +190,9 @@ describe('handleValidate', () => { const result = await handleValidate({}); expect(result.success).toBe(false); - expect(result.error).toContain('Failed to read agentcore.json'); - expect(result.error).toContain('EACCES'); + assert(!result.success); + expect(result.error.message).toContain('Failed to read agentcore.json'); + expect(result.error.message).toContain('EACCES'); }); it('formats ConfigNotFoundError with file name', async () => { @@ -195,7 +203,8 @@ describe('handleValidate', () => { const result = await handleValidate({}); expect(result.success).toBe(false); - expect(result.error).toBe('Required file not found: agentcore.json'); + assert(!result.success); + expect(result.error.message).toBe('Required file not found: agentcore.json'); }); it('formats non-Error values as strings', async () => { @@ -205,7 +214,8 @@ describe('handleValidate', () => { const result = await handleValidate({}); expect(result.success).toBe(false); - expect(result.error).toBe('string error'); + assert(!result.success); + expect(result.error.message).toBe('string error'); }); describe('harness validation', () => { diff --git a/src/cli/commands/validate/action.ts b/src/cli/commands/validate/action.ts index 410056750..b5d384c89 100644 --- a/src/cli/commands/validate/action.ts +++ b/src/cli/commands/validate/action.ts @@ -7,22 +7,21 @@ import { NoProjectError, findConfigRoot, } from '../../../lib'; +<<<<<<< HEAD import type { AgentCoreProjectSpec } from '../../../schema'; +======= +import type { Result } from '../../../lib/result'; +>>>>>>> origin/main export interface ValidateOptions { directory?: string; } -export interface ValidateResult { - success: boolean; - error?: string; -} - /** * Validates all AgentCore schema files in the project. * Returns a binary success/fail result with an error message if validation fails. */ -export async function handleValidate(options: ValidateOptions): Promise { +export async function handleValidate(options: ValidateOptions): Promise { const baseDir = options.directory ?? process.cwd(); // Check if project exists @@ -30,7 +29,7 @@ export async function handleValidate(options: ValidateOptions): Promise { render(Valid); process.exit(0); } else { - render({result.error}); + render({result.error.message}); process.exit(1); } }); diff --git a/src/cli/commands/validate/index.ts b/src/cli/commands/validate/index.ts index cecbb46ac..067c35e56 100644 --- a/src/cli/commands/validate/index.ts +++ b/src/cli/commands/validate/index.ts @@ -1,2 +1,2 @@ export { registerValidate } from './command'; -export { handleValidate, type ValidateOptions, type ValidateResult } from './action'; +export { handleValidate, type ValidateOptions } from './action'; diff --git a/src/cli/errors.ts b/src/cli/errors.ts index 9e7e9397d..99c59c051 100644 --- a/src/cli/errors.ts +++ b/src/cli/errors.ts @@ -1,13 +1,3 @@ -/** - * Error thrown when an agent with the same name already exists. - */ -export class AgentAlreadyExistsError extends Error { - constructor(agentName: string) { - super(`An agent named "${agentName}" already exists in the schema.`); - this.name = 'AgentAlreadyExistsError'; - } -} - /** * Converts an unknown error to a string message. * Handles Error instances and other thrown values consistently. diff --git a/src/cli/operations/agent/generate/write-agent-to-project.ts b/src/cli/operations/agent/generate/write-agent-to-project.ts index 728236e54..cfa946ce1 100644 --- a/src/cli/operations/agent/generate/write-agent-to-project.ts +++ b/src/cli/operations/agent/generate/write-agent-to-project.ts @@ -1,7 +1,6 @@ -import { ConfigIO, requireConfigRoot } from '../../../../lib'; +import { AgentAlreadyExistsError, ConfigIO, requireConfigRoot } from '../../../../lib'; import type { AgentCoreProjectSpec } from '../../../../schema'; import { SCHEMA_VERSION } from '../../../constants'; -import { AgentAlreadyExistsError } from '../../../errors'; import type { CredentialStrategy } from '../../../primitives/CredentialPrimitive'; import type { GenerateConfig } from '../../../tui/screens/generate/types'; import { mapGenerateConfigToAgent, mapGenerateInputToMemories, mapModelProviderToCredentials } from './schema-mapper'; diff --git a/src/cli/operations/agent/import/index.ts b/src/cli/operations/agent/import/index.ts index e49d9e3c0..25fb6299f 100644 --- a/src/cli/operations/agent/import/index.ts +++ b/src/cli/operations/agent/import/index.ts @@ -3,10 +3,9 @@ * Provides executeImportAgent() as the shared handler called by both * the TUI hook (useAddAgent) and the non-interactive CLI (AgentPrimitive). */ -import { APP_DIR, ConfigIO } from '../../../../lib'; +import { APP_DIR, ConfigIO, toError } from '../../../../lib'; import type { RuntimeAuthorizerType, SDKFramework } from '../../../../schema'; import { getBedrockAgentConfig } from '../../../aws/bedrock-import'; -import { getErrorMessage } from '../../../errors'; import type { JwtConfigOptions } from '../../../primitives/auth-utils'; import { createManagedOAuthCredential } from '../../../primitives/auth-utils'; import type { AddResult } from '../../../primitives/types'; @@ -120,6 +119,6 @@ export async function executeImportAgent( return { success: true, agentName: name, agentPath }; } catch (err) { - return { success: false, error: getErrorMessage(err) }; + return { success: false, error: toError(err) }; } } diff --git a/src/cli/operations/deploy/__tests__/post-deploy-observability.test.ts b/src/cli/operations/deploy/__tests__/post-deploy-observability.test.ts index ba069f29e..8fc549624 100644 --- a/src/cli/operations/deploy/__tests__/post-deploy-observability.test.ts +++ b/src/cli/operations/deploy/__tests__/post-deploy-observability.test.ts @@ -70,7 +70,7 @@ describe('setupTransactionSearch', () => { }); it('propagates error from enableTransactionSearch', async () => { - mockEnableTransactionSearch.mockResolvedValue({ success: false, error: 'Insufficient permissions' }); + mockEnableTransactionSearch.mockResolvedValue({ success: false, error: new Error('Insufficient permissions') }); const result = await setupTransactionSearch({ region: 'us-east-1', @@ -78,7 +78,10 @@ describe('setupTransactionSearch', () => { agentNames: ['agent-1'], }); - expect(result).toEqual({ success: false, error: 'Insufficient permissions' }); + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error.message).toBe('Insufficient permissions'); + } }); it('triggers when hasGateways is true and agentNames is empty', async () => { diff --git a/src/cli/operations/deploy/index.ts b/src/cli/operations/deploy/index.ts index 332f0ca2d..93cdb6353 100644 --- a/src/cli/operations/deploy/index.ts +++ b/src/cli/operations/deploy/index.ts @@ -39,11 +39,10 @@ export { type DeployedTarget, type DiscoverDeployedResult, type DestroyTargetOptions, - type StackTeardownResult, } from './teardown'; // Post-deploy observability setup -export { setupTransactionSearch, type TransactionSearchSetupResult } from './post-deploy-observability'; +export { setupTransactionSearch } from './post-deploy-observability'; // Post-deploy HTTP gateways export { diff --git a/src/cli/operations/deploy/post-deploy-observability.ts b/src/cli/operations/deploy/post-deploy-observability.ts index 0616a65dc..4ee613c42 100644 --- a/src/cli/operations/deploy/post-deploy-observability.ts +++ b/src/cli/operations/deploy/post-deploy-observability.ts @@ -1,3 +1,4 @@ +import type { Result } from '../../../lib/result'; import { readGlobalConfigSync } from '../../../lib/schemas/io/global-config'; import { enableTransactionSearch } from '../../aws/transaction-search'; @@ -8,11 +9,6 @@ export interface TransactionSearchSetupOptions { hasGateways?: boolean; } -export interface TransactionSearchSetupResult { - success: boolean; - error?: string; -} - /** * Post-deploy step: enable CloudWatch Transaction Search (Application Signals + * resource policy + CloudWatchLogs destination + 100% indexing). @@ -22,9 +18,7 @@ export interface TransactionSearchSetupResult { * * This is a non-blocking best-effort operation — failures do not fail the deploy. */ -export async function setupTransactionSearch( - options: TransactionSearchSetupOptions -): Promise { +export async function setupTransactionSearch(options: TransactionSearchSetupOptions): Promise { const { region, accountId, agentNames } = options; if (agentNames.length === 0 && !options.hasGateways) { diff --git a/src/cli/operations/deploy/teardown.ts b/src/cli/operations/deploy/teardown.ts index 2e38f2576..b7c39c49b 100644 --- a/src/cli/operations/deploy/teardown.ts +++ b/src/cli/operations/deploy/teardown.ts @@ -1,4 +1,5 @@ import { CONFIG_DIR, ConfigIO } from '../../../lib'; +import type { Result } from '../../../lib/result'; import type { AwsDeploymentTarget } from '../../../schema'; import { withTargetRegion } from '../../aws'; import { deleteConfigurationBundle } from '../../aws/agentcore-config-bundles'; @@ -99,16 +100,11 @@ export function getCdkProjectDir(cwd?: string): string { return join(baseDir, CONFIG_DIR, 'cdk'); } -export interface StackTeardownResult { - success: boolean; - error?: string; -} - /** * Perform full stack teardown for a target: destroy CloudFormation stack, * remove deployed-state entry, and remove the target from aws-targets.json. */ -export async function performStackTeardown(targetName: string): Promise { +export async function performStackTeardown(targetName: string): Promise { const cdkProjectDir = getCdkProjectDir(); const configIO = new ConfigIO(); diff --git a/src/cli/operations/eval/__tests__/list-eval-runs.test.ts b/src/cli/operations/eval/__tests__/list-eval-runs.test.ts index c9a71a8cf..1c1c48f71 100644 --- a/src/cli/operations/eval/__tests__/list-eval-runs.test.ts +++ b/src/cli/operations/eval/__tests__/list-eval-runs.test.ts @@ -1,5 +1,6 @@ import { handleListEvalRuns } from '../list-eval-runs.js'; import type { EvalRunResult } from '../types.js'; +import assert from 'node:assert'; import { afterEach, describe, expect, it, vi } from 'vitest'; const mockListEvalRuns = vi.fn(); @@ -28,7 +29,7 @@ describe('handleListEvalRuns', () => { const result = handleListEvalRuns({}); - expect(result.success).toBe(true); + assert(result.success); expect(result.runs).toHaveLength(2); }); @@ -42,9 +43,9 @@ describe('handleListEvalRuns', () => { const result = handleListEvalRuns({ agent: 'agent-a' }); - expect(result.success).toBe(true); + assert(result.success); expect(result.runs).toHaveLength(2); - expect(result.runs!.every(r => r.agent === 'agent-a')).toBe(true); + expect(result.runs.every((r: EvalRunResult) => r.agent === 'agent-a')).toBe(true); }); it('limits the number of results', () => { @@ -57,7 +58,7 @@ describe('handleListEvalRuns', () => { const result = handleListEvalRuns({ limit: 2 }); - expect(result.success).toBe(true); + assert(result.success); expect(result.runs).toHaveLength(2); }); @@ -72,9 +73,10 @@ describe('handleListEvalRuns', () => { const result = handleListEvalRuns({ agent: 'a', limit: 2 }); + assert(result.success); expect(result.runs).toHaveLength(2); - expect(result.runs![0]!.timestamp).toBe('2025-01-15T10:00:00.000Z'); - expect(result.runs![1]!.timestamp).toBe('2025-01-15T12:00:00.000Z'); + expect(result.runs[0]!.timestamp).toBe('2025-01-15T10:00:00.000Z'); + expect(result.runs[1]!.timestamp).toBe('2025-01-15T12:00:00.000Z'); }); it('returns empty array when no runs exist', () => { @@ -82,7 +84,7 @@ describe('handleListEvalRuns', () => { const result = handleListEvalRuns({}); - expect(result.success).toBe(true); + assert(result.success); expect(result.runs).toEqual([]); }); @@ -93,9 +95,8 @@ describe('handleListEvalRuns', () => { const result = handleListEvalRuns({}); - expect(result.success).toBe(false); - expect(result.error).toBe('disk error'); - expect(result.runs).toBeUndefined(); + assert(!result.success); + expect(result.error.message).toBe('disk error'); }); it('handles non-Error thrown values', () => { @@ -105,7 +106,7 @@ describe('handleListEvalRuns', () => { const result = handleListEvalRuns({}); - expect(result.success).toBe(false); - expect(result.error).toBe('42'); + assert(!result.success); + expect(result.error.message).toBe('42'); }); }); diff --git a/src/cli/operations/eval/__tests__/logs-eval.test.ts b/src/cli/operations/eval/__tests__/logs-eval.test.ts index 5411d842e..a363941fd 100644 --- a/src/cli/operations/eval/__tests__/logs-eval.test.ts +++ b/src/cli/operations/eval/__tests__/logs-eval.test.ts @@ -1,4 +1,5 @@ import { handleLogsEval } from '../logs-eval.js'; +import assert from 'node:assert'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; const mockLoadDeployedProjectConfig = vi.fn(); @@ -100,8 +101,8 @@ describe('handleLogsEval', () => { const result = await handleLogsEval({}); - expect(result.success).toBe(false); - expect(result.error).toBe('No agents defined'); + assert(!result.success); + expect(result.error.message).toBe('No agents defined'); }); it('returns error when no online eval configs exist for the agent', async () => { @@ -111,8 +112,8 @@ describe('handleLogsEval', () => { const result = await handleLogsEval({}); - expect(result.success).toBe(false); - expect(result.error).toContain('No deployed online eval configs found'); + assert(!result.success); + expect(result.error.message).toContain('No deployed online eval configs found'); }); it('returns error when online eval configs exist but none are deployed', async () => { @@ -122,8 +123,8 @@ describe('handleLogsEval', () => { const result = await handleLogsEval({}); - expect(result.success).toBe(false); - expect(result.error).toContain('No deployed online eval configs found'); + assert(!result.success); + expect(result.error.message).toContain('No deployed online eval configs found'); }); it('searches logs with time range when --since is specified', async () => { diff --git a/src/cli/operations/eval/__tests__/pause-resume.test.ts b/src/cli/operations/eval/__tests__/pause-resume.test.ts index 834525675..9a6291a3d 100644 --- a/src/cli/operations/eval/__tests__/pause-resume.test.ts +++ b/src/cli/operations/eval/__tests__/pause-resume.test.ts @@ -1,4 +1,5 @@ import { handlePauseResume } from '../pause-resume.js'; +import assert from 'node:assert'; import { afterEach, describe, expect, it, vi } from 'vitest'; const mockLoadDeployedProjectConfig = vi.fn(); @@ -46,7 +47,7 @@ describe('handlePauseResume', () => { const result = await handlePauseResume({ name: 'my-config' }, 'pause'); - expect(result.success).toBe(true); + assert(result.success); expect(result.executionStatus).toBe('DISABLED'); expect(mockUpdateOnlineEvalExecutionStatus).toHaveBeenCalledWith({ region: 'us-east-1', @@ -65,7 +66,7 @@ describe('handlePauseResume', () => { const result = await handlePauseResume({ name: 'my-config' }, 'resume'); - expect(result.success).toBe(true); + assert(result.success); expect(result.executionStatus).toBe('ENABLED'); expect(mockUpdateOnlineEvalExecutionStatus).toHaveBeenCalledWith({ region: 'us-east-1', @@ -83,8 +84,8 @@ describe('handlePauseResume', () => { const result = await handlePauseResume({ name: 'my-config' }, 'pause'); - expect(result.success).toBe(false); - expect(result.error).toContain('No deployed targets found'); + assert(!result.success); + expect(result.error.message).toContain('No deployed targets found'); }); it('returns error when config name is not found in deployed state', async () => { @@ -92,9 +93,9 @@ describe('handlePauseResume', () => { const result = await handlePauseResume({ name: 'missing-config' }, 'pause'); - expect(result.success).toBe(false); - expect(result.error).toContain('missing-config'); - expect(result.error).toContain('not found'); + assert(!result.success); + expect(result.error.message).toContain('missing-config'); + expect(result.error.message).toContain('not found'); }); it('returns error when target config is missing from aws-targets', async () => { @@ -105,9 +106,9 @@ describe('handlePauseResume', () => { const result = await handlePauseResume({ name: 'my-config' }, 'pause'); - expect(result.success).toBe(false); - expect(result.error).toContain('Target config'); - expect(result.error).toContain('not found'); + assert(!result.success); + expect(result.error.message).toContain('Target config'); + expect(result.error.message).toContain('not found'); }); it('returns error when the SDK call fails', async () => { @@ -116,8 +117,8 @@ describe('handlePauseResume', () => { const result = await handlePauseResume({ name: 'my-config' }, 'pause'); - expect(result.success).toBe(false); - expect(result.error).toBe('Service unavailable'); + assert(!result.success); + expect(result.error.message).toBe('Service unavailable'); }); describe('ARN mode', () => { @@ -131,7 +132,7 @@ describe('handlePauseResume', () => { const arn = 'arn:aws:bedrock-agentcore:us-west-2:123456789012:online-evaluation-config/my-cfg-id'; const result = await handlePauseResume({ name: '', arn }, 'pause'); - expect(result.success).toBe(true); + assert(result.success); expect(result.executionStatus).toBe('DISABLED'); expect(mockLoadDeployedProjectConfig).not.toHaveBeenCalled(); expect(mockUpdateOnlineEvalExecutionStatus).toHaveBeenCalledWith({ @@ -151,7 +152,7 @@ describe('handlePauseResume', () => { const arn = 'arn:aws:bedrock-agentcore:us-west-2:123456789012:online-evaluation-config/my-cfg-id'; const result = await handlePauseResume({ name: '', arn, region: 'eu-west-1' }, 'resume'); - expect(result.success).toBe(true); + assert(result.success); expect(result.executionStatus).toBe('ENABLED'); expect(mockUpdateOnlineEvalExecutionStatus).toHaveBeenCalledWith({ region: 'eu-west-1', @@ -163,16 +164,16 @@ describe('handlePauseResume', () => { it('returns error for invalid ARN', async () => { const result = await handlePauseResume({ name: '', arn: 'not-an-arn' }, 'pause'); - expect(result.success).toBe(false); - expect(result.error).toContain('Invalid online eval config ARN'); + assert(!result.success); + expect(result.error.message).toContain('Invalid online eval config ARN'); }); it('returns error when config ID cannot be extracted from ARN', async () => { const arn = 'arn:aws:bedrock-agentcore:us-east-1:123456789012:some-other-resource/foo'; const result = await handlePauseResume({ name: '', arn }, 'pause'); - expect(result.success).toBe(false); - expect(result.error).toContain('Could not extract config ID'); + assert(!result.success); + expect(result.error.message).toContain('Could not extract config ID'); }); }); }); diff --git a/src/cli/operations/eval/__tests__/run-eval.test.ts b/src/cli/operations/eval/__tests__/run-eval.test.ts index 39314af69..7b6279b28 100644 --- a/src/cli/operations/eval/__tests__/run-eval.test.ts +++ b/src/cli/operations/eval/__tests__/run-eval.test.ts @@ -1,4 +1,5 @@ import { handleRunEval } from '../run-eval.js'; +import assert from 'node:assert'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; // ─── Mocks ──────────────────────────────────────────────────────────────────── @@ -150,8 +151,8 @@ describe('handleRunEval', () => { const result = await handleRunEval({ evaluator: ['Builtin.GoalSuccessRate'], days: 7 }); - expect(result.success).toBe(false); - expect(result.error).toBe('No agents defined'); + assert(!result.success); + expect(result.error.message).toBe('No agents defined'); }); it('returns error when a custom evaluator is not found in deployed state', async () => { @@ -170,9 +171,9 @@ describe('handleRunEval', () => { const result = await handleRunEval({ evaluator: ['MissingEval'], days: 7 }); - expect(result.success).toBe(false); - expect(result.error).toContain('MissingEval'); - expect(result.error).toContain('not found in deployed state'); + assert(!result.success); + expect(result.error.message).toContain('MissingEval'); + expect(result.error.message).toContain('not found in deployed state'); }); it('resolves builtin evaluators without deployed state lookup', async () => { @@ -195,7 +196,8 @@ describe('handleRunEval', () => { const result = await handleRunEval({ evaluator: ['Builtin.GoalSuccessRate'], days: 7 }); // Fails because no spans, but NOT because evaluator wasn't found - expect(result.error).toContain('No session spans found'); + assert(!result.success); + expect(result.error.message).toContain('No session spans found'); }); it('resolves custom evaluator name to deployed evaluator ID', async () => { @@ -278,9 +280,9 @@ describe('handleRunEval', () => { const result = await handleRunEval({ evaluator: ['Builtin.GoalSuccessRate'], days: 7 }); - expect(result.success).toBe(false); - expect(result.error).toContain('No session spans found'); - expect(result.error).toContain('my-agent'); + assert(!result.success); + expect(result.error.message).toContain('No session spans found'); + expect(result.error.message).toContain('my-agent'); }); // ─── Successful evaluation ──────────────────────────────────────────────── @@ -324,12 +326,12 @@ describe('handleRunEval', () => { const result = await handleRunEval({ evaluator: ['Builtin.GoalSuccessRate'], days: 7 }); - expect(result.success).toBe(true); + assert(result.success); expect(result.run).toBeDefined(); - expect(result.run!.sessionCount).toBe(2); - expect(result.run!.results).toHaveLength(1); + expect(result.run.sessionCount).toBe(2); + expect(result.run.results).toHaveLength(1); - const evalResult = result.run!.results[0]!; + const evalResult = result.run.results[0]!; expect(evalResult.aggregateScore).toBe(3.0); // (4 + 2) / 2 expect(evalResult.sessionScores).toHaveLength(2); expect(evalResult.tokenUsage).toEqual({ inputTokens: 180, outputTokens: 90, totalTokens: 270 }); @@ -361,8 +363,8 @@ describe('handleRunEval', () => { const result = await handleRunEval({ evaluator: ['Builtin.GoalSuccessRate'], days: 7 }); - expect(result.success).toBe(true); - const evalResult = result.run!.results[0]!; + assert(result.success); + const evalResult = result.run.results[0]!; // Only the non-errored session (value 5.0) should be in the aggregate expect(evalResult.aggregateScore).toBe(5.0); expect(evalResult.sessionScores).toHaveLength(2); @@ -391,7 +393,7 @@ describe('handleRunEval', () => { const result = await handleRunEval({ evaluator: ['Builtin.GoalSuccessRate'], days: 7 }); - expect(result.success).toBe(true); + assert(result.success); expect(mockSaveEvalRun).toHaveBeenCalled(); expect(mockWriteFileSync).not.toHaveBeenCalled(); expect(result.filePath).toBe('/tmp/eval-results/eval_2025-01-15_10-00-00.json'); @@ -422,7 +424,7 @@ describe('handleRunEval', () => { output: '/tmp/my-output.json', }); - expect(result.success).toBe(true); + assert(result.success); expect(mockWriteFileSync).toHaveBeenCalledWith('/tmp/my-output.json', expect.any(String)); expect(mockSaveEvalRun).not.toHaveBeenCalled(); expect(result.filePath).toBe('/tmp/my-output.json'); @@ -461,12 +463,12 @@ describe('handleRunEval', () => { days: 7, }); - expect(result.success).toBe(true); - expect(result.run!.results).toHaveLength(2); - expect(result.run!.results[0]!.evaluator).toBe('Builtin.GoalSuccessRate'); - expect(result.run!.results[0]!.aggregateScore).toBe(0.9); - expect(result.run!.results[1]!.evaluator).toBe('CustomEval'); - expect(result.run!.results[1]!.aggregateScore).toBe(4.5); + assert(result.success); + expect(result.run.results).toHaveLength(2); + expect(result.run.results[0]!.evaluator).toBe('Builtin.GoalSuccessRate'); + expect(result.run.results[0]!.aggregateScore).toBe(0.9); + expect(result.run.results[1]!.evaluator).toBe('CustomEval'); + expect(result.run.results[1]!.aggregateScore).toBe(4.5); }); // ─── ARN mode ───────────────────────────────────────────────────────────── @@ -484,8 +486,8 @@ describe('handleRunEval', () => { days: 3, }); - expect(result.success).toBe(true); - expect(result.run!.agent).toBe('rt-arn-test'); + assert(result.success); + expect(result.run.agent).toBe('rt-arn-test'); expect(mockLoadDeployedProjectConfig).not.toHaveBeenCalled(); expect(mockResolveAgent).not.toHaveBeenCalled(); }); @@ -532,8 +534,8 @@ describe('handleRunEval', () => { days: 7, }); - expect(result.success).toBe(false); - expect(result.error).toContain('Invalid agent runtime ARN'); + assert(!result.success); + expect(result.error.message).toContain('Invalid agent runtime ARN'); }); it('rejects custom evaluator names in ARN mode', async () => { @@ -543,8 +545,8 @@ describe('handleRunEval', () => { days: 7, }); - expect(result.success).toBe(false); - expect(result.error).toContain('cannot be resolved in ARN mode'); + assert(!result.success); + expect(result.error.message).toContain('cannot be resolved in ARN mode'); }); it('saves to cwd in ARN mode when no --output is specified', async () => { @@ -559,7 +561,7 @@ describe('handleRunEval', () => { days: 7, }); - expect(result.success).toBe(true); + assert(result.success); // Should write to cwd, not call saveEvalRun (which requires a project) expect(mockSaveEvalRun).not.toHaveBeenCalled(); expect(mockWriteFileSync).toHaveBeenCalledWith( @@ -582,7 +584,7 @@ describe('handleRunEval', () => { output: '/tmp/custom-eval.json', }); - expect(result.success).toBe(true); + assert(result.success); expect(mockWriteFileSync).toHaveBeenCalledWith('/tmp/custom-eval.json', expect.any(String)); expect(result.filePath).toBe('/tmp/custom-eval.json'); }); @@ -594,8 +596,8 @@ describe('handleRunEval', () => { days: 7, }); - expect(result.success).toBe(false); - expect(result.error).toContain('No evaluators specified'); + assert(!result.success); + expect(result.error.message).toContain('No evaluators specified'); }); // ─── Endpoint selection ────────────────────────────────────────────────── @@ -1199,8 +1201,8 @@ describe('handleRunEval', () => { expectedTrajectory: ['tool_1', 'tool_2'], }); - expect(result.success).toBe(true); - expect(result.run!.referenceInputs).toEqual({ + assert(result.success); + expect(result.run.referenceInputs).toEqual({ expectedTrajectory: ['tool_1', 'tool_2'], }); }); @@ -1215,7 +1217,7 @@ describe('handleRunEval', () => { assertions: ['Agent should greet user'], }); - expect(result.success).toBe(false); - expect(result.error).toContain('require exactly one session'); + assert(!result.success); + expect(result.error.message).toContain('require exactly one session'); }); }); diff --git a/src/cli/operations/eval/index.ts b/src/cli/operations/eval/index.ts index de5999569..66efbbb21 100644 --- a/src/cli/operations/eval/index.ts +++ b/src/cli/operations/eval/index.ts @@ -5,6 +5,5 @@ export type { ListEvalRunsResult } from './list-eval-runs'; export { handlePauseResume } from './pause-resume'; export type { PauseResumeResult } from './pause-resume'; export { handleLogsEval } from './logs-eval'; -export type { LogsEvalResult } from './logs-eval'; export type { EvalRunResult, RunEvalOptions, ListEvalRunsOptions, OnlineEvalActionOptions, SessionInfo } from './types'; export type { LogsEvalOptions } from './logs-eval'; diff --git a/src/cli/operations/eval/list-eval-runs.ts b/src/cli/operations/eval/list-eval-runs.ts index 66b0ed528..9b79a9ebc 100644 --- a/src/cli/operations/eval/list-eval-runs.ts +++ b/src/cli/operations/eval/list-eval-runs.ts @@ -1,12 +1,9 @@ -import { getErrorMessage } from '../../errors'; +import { toError } from '../../../lib'; +import type { Result } from '../../../lib/result'; import { listEvalRuns } from './storage'; import type { EvalRunResult, ListEvalRunsOptions } from './types'; -export interface ListEvalRunsResult { - success: boolean; - error?: string; - runs?: EvalRunResult[]; -} +export type ListEvalRunsResult = Result<{ runs: EvalRunResult[] }>; export function handleListEvalRuns(options: ListEvalRunsOptions): ListEvalRunsResult { try { @@ -22,6 +19,6 @@ export function handleListEvalRuns(options: ListEvalRunsOptions): ListEvalRunsRe return { success: true, runs }; } catch (err) { - return { success: false, error: getErrorMessage(err) }; + return { success: false, error: toError(err) }; } } diff --git a/src/cli/operations/eval/logs-eval.ts b/src/cli/operations/eval/logs-eval.ts index 430103a44..70df24eb4 100644 --- a/src/cli/operations/eval/logs-eval.ts +++ b/src/cli/operations/eval/logs-eval.ts @@ -1,3 +1,5 @@ +import { ResourceNotFoundError, ValidationError } from '../../../lib'; +import type { Result } from '../../../lib/result'; import { parseTimeString } from '../../../lib/utils'; import { getOnlineEvaluationConfig } from '../../aws/agentcore-control'; import { searchLogs, streamLogs } from '../../aws/cloudwatch'; @@ -13,11 +15,6 @@ export interface LogsEvalOptions { follow?: boolean; } -export interface LogsEvalResult { - success: boolean; - error?: string; -} - function formatLogLine(event: { timestamp: number; message: string }, json: boolean): string { if (json) { return JSON.stringify({ timestamp: new Date(event.timestamp).toISOString(), message: event.message }); @@ -70,12 +67,12 @@ async function resolveEvalLogGroups( return results; } -export async function handleLogsEval(options: LogsEvalOptions): Promise { +export async function handleLogsEval(options: LogsEvalOptions): Promise { const context = await loadDeployedProjectConfig(); const agentResult = resolveAgent(context, { runtime: options.agent }); if (!agentResult.success) { - return { success: false, error: agentResult.error }; + return { success: false, error: new ResourceNotFoundError(agentResult.error) }; } const { agent } = agentResult; @@ -85,7 +82,9 @@ export async function handleLogsEval(options: LogsEvalOptions): Promise; async function resolveOnlineEvalConfig( configName: string @@ -88,7 +85,7 @@ export async function handlePauseResume( ): Promise { const resolution = await resolveConfig(options); if (!resolution.success) { - return resolution; + return { success: false, error: new ResourceNotFoundError(resolution.error) }; } const executionStatus: OnlineEvalExecutionStatus = action === 'pause' ? 'DISABLED' : 'ENABLED'; @@ -106,6 +103,6 @@ export async function handlePauseResume( executionStatus: result.executionStatus, }; } catch (err) { - return { success: false, error: (err as Error).message }; + return { success: false, error: toError(err) }; } } diff --git a/src/cli/operations/eval/run-batch-evaluation.ts b/src/cli/operations/eval/run-batch-evaluation.ts index 0962f4e0a..436cace71 100644 --- a/src/cli/operations/eval/run-batch-evaluation.ts +++ b/src/cli/operations/eval/run-batch-evaluation.ts @@ -6,7 +6,8 @@ * 4. Poll GetBatchEvaluation until terminal status * 5. Return results */ -import { ConfigIO } from '../../../lib'; +import { ConfigIO, ResourceNotFoundError, ValidationError, toError } from '../../../lib'; +import type { Result } from '../../../lib/result'; import type { DeployedState } from '../../../schema'; import { generateClientToken, getBatchEvaluation, startBatchEvaluation } from '../../aws/agentcore-batch-evaluation'; import type { @@ -54,9 +55,7 @@ export interface BatchEvaluationResult { error?: string; } -export interface RunBatchEvaluationCommandResult { - success: boolean; - error?: string; +export type RunBatchEvaluationCommandResult = Result & { batchEvaluationId?: string; name?: string; status?: string; @@ -65,7 +64,7 @@ export interface RunBatchEvaluationCommandResult { startedAt?: string; completedAt?: string; logFilePath?: string; -} +}; // ============================================================================ // Constants @@ -116,7 +115,7 @@ export async function runBatchEvaluationCommand( logger?.log(error, 'error'); logger?.endStep('error', error); logger?.finalize(false); - return { success: false, error, results: [], logFilePath: logger?.logFilePath }; + return { success: false, error: new ResourceNotFoundError(error), results: [], logFilePath: logger?.logFilePath }; } const runtimeId = agentState.runtimeId; @@ -152,7 +151,9 @@ export async function runBatchEvaluationCommand( if (!/^[a-zA-Z][a-zA-Z0-9_]{0,47}$/.test(options.name)) { return { success: false, - error: `Batch evaluation name must start with a letter and contain only letters, digits, and underscores (max 48 chars). Got: "${options.name}"`, + error: new ValidationError( + `Batch evaluation name must start with a letter and contain only letters, digits, and underscores (max 48 chars). Got: "${options.name}"` + ), results: [], logFilePath: logger?.logFilePath, }; @@ -242,7 +243,7 @@ export async function runBatchEvaluationCommand( logger?.finalize(false); return { success: false, - error, + error: new Error(error), batchEvaluationId: startResult.batchEvaluationId, name: evalName, status: current.status, @@ -287,7 +288,7 @@ export async function runBatchEvaluationCommand( const error = err instanceof Error ? err.message : String(err); logger?.log(error, 'error'); logger?.finalize(false); - return { success: false, error, results: [], logFilePath: logger?.logFilePath }; + return { success: false, error: toError(err), results: [], logFilePath: logger?.logFilePath }; } } diff --git a/src/cli/operations/eval/run-eval.ts b/src/cli/operations/eval/run-eval.ts index 90cd519c7..8ba5cb307 100644 --- a/src/cli/operations/eval/run-eval.ts +++ b/src/cli/operations/eval/run-eval.ts @@ -1,3 +1,5 @@ +import { ResourceNotFoundError, ValidationError } from '../../../lib'; +import type { Result } from '../../../lib/result'; import { getCredentialProvider } from '../../aws'; import { evaluate } from '../../aws/agentcore'; import type { EvaluationReferenceInput } from '../../aws/agentcore'; @@ -551,12 +553,7 @@ async function fetchSessionSpans(opts: FetchSpansOptions): Promise; export async function handleRunEval(options: RunEvalOptions): Promise { let resolution: ResolveResult; @@ -569,7 +566,7 @@ export async function handleRunEval(options: RunEvalOptions): Promise ({ configExists = mockConfigExists; }, requireConfigRoot: () => '/project/agentcore', + findConfigRoot: () => '/project/agentcore', + toError: (err: unknown) => (err instanceof Error ? err : new Error(String(err))), + serializeResult: (r: unknown) => r, + ResourceNotFoundError: class extends Error { + constructor(m: string) { + super(m); + this.name = 'ResourceNotFoundError'; + } + }, + APP_DIR: 'app', + MCP_APP_SUBDIR: 'mcp', })); const computeDefaultGatewayEnvVarName = (name: string) => GatewayPrimitive.computeDefaultGatewayEnvVarName(name); @@ -200,7 +211,10 @@ describe('GatewayPrimitive.add (createGateway)', () => { }); expect(result).toEqual( - expect.objectContaining({ success: false, error: expect.stringContaining('Gateway "dup-gw" already exists') }) + expect.objectContaining({ + success: false, + error: expect.objectContaining({ message: expect.stringContaining('Gateway "dup-gw" already exists') }), + }) ); }); diff --git a/src/cli/operations/memory/__tests__/create-memory.test.ts b/src/cli/operations/memory/__tests__/create-memory.test.ts index a0b8077c4..ad9cef2c8 100644 --- a/src/cli/operations/memory/__tests__/create-memory.test.ts +++ b/src/cli/operations/memory/__tests__/create-memory.test.ts @@ -15,6 +15,15 @@ vi.mock('../../../../lib/index.js', () => ({ readProjectSpec = mockReadProjectSpec; writeProjectSpec = mockWriteProjectSpec; }, + findConfigRoot: () => '/fake/root', + toError: (err: unknown) => (err instanceof Error ? err : new Error(String(err))), + serializeResult: (r: unknown) => r, + ResourceNotFoundError: class extends Error { + constructor(m: string) { + super(m); + this.name = 'ResourceNotFoundError'; + } + }, })); const makeProject = (memoryNames: string[]) => ({ @@ -86,7 +95,7 @@ describe('add', () => { expiry: 30, }); - expect(result).toEqual(expect.objectContaining({ success: false, error: expect.any(String) })); + expect(result).toEqual(expect.objectContaining({ success: false, error: expect.any(Error) })); expect(mockWriteProjectSpec).not.toHaveBeenCalled(); }); @@ -96,7 +105,10 @@ describe('add', () => { const result = await primitive.add({ name: 'Existing', strategies: '', expiry: 30 }); expect(result).toEqual( - expect.objectContaining({ success: false, error: expect.stringContaining('Memory "Existing" already exists') }) + expect.objectContaining({ + success: false, + error: expect.objectContaining({ message: expect.stringContaining('Memory "Existing" already exists') }), + }) ); }); }); diff --git a/src/cli/operations/memory/list-memory-records.ts b/src/cli/operations/memory/list-memory-records.ts index 5d0c53828..a30a1cfab 100644 --- a/src/cli/operations/memory/list-memory-records.ts +++ b/src/cli/operations/memory/list-memory-records.ts @@ -1,5 +1,12 @@ +<<<<<<< HEAD import { createAgentCoreClient } from '../../aws'; import { ListMemoryRecordsCommand } from '@aws-sdk/client-bedrock-agentcore'; +======= +import { ResourceNotFoundError, toError } from '../../../lib'; +import type { Result } from '../../../lib/result'; +import { getCredentialProvider } from '../../aws'; +import { BedrockAgentCoreClient, ListMemoryRecordsCommand } from '@aws-sdk/client-bedrock-agentcore'; +>>>>>>> origin/main export interface MemoryRecordEntry { memoryRecordId: string; @@ -20,12 +27,7 @@ export interface ListMemoryRecordsOptions { nextToken?: string; } -export interface ListMemoryRecordsResult { - success: boolean; - records?: MemoryRecordEntry[]; - nextToken?: string; - error?: string; -} +export type ListMemoryRecordsResult = Result<{ records: MemoryRecordEntry[]; nextToken?: string }>; /** * Lists memory records for a deployed memory resource via the AWS SDK. @@ -60,8 +62,11 @@ export async function listMemoryRecords(options: ListMemoryRecordsOptions): Prom } catch (error: unknown) { const err = error as Error; if (err.name === 'ResourceNotFoundException') { - return { success: false, error: `Memory '${memoryId}' not found. It may not have been deployed yet.` }; + return { + success: false, + error: new ResourceNotFoundError(`Memory '${memoryId}' not found. It may not have been deployed yet.`), + }; } - return { success: false, error: err.message ?? String(error) }; + return { success: false, error: toError(error) }; } } diff --git a/src/cli/operations/memory/retrieve-memory-records.ts b/src/cli/operations/memory/retrieve-memory-records.ts index 720a45287..071b68bf4 100644 --- a/src/cli/operations/memory/retrieve-memory-records.ts +++ b/src/cli/operations/memory/retrieve-memory-records.ts @@ -1,4 +1,10 @@ +<<<<<<< HEAD import { createAgentCoreClient } from '../../aws'; +======= +import { ResourceNotFoundError, toError } from '../../../lib'; +import type { Result } from '../../../lib/result'; +import { getCredentialProvider } from '../../aws'; +>>>>>>> origin/main import type { MemoryRecordEntry } from './list-memory-records'; import { RetrieveMemoryRecordsCommand } from '@aws-sdk/client-bedrock-agentcore'; @@ -13,12 +19,7 @@ export interface RetrieveMemoryRecordsOptions { nextToken?: string; } -export interface RetrieveMemoryRecordsResult { - success: boolean; - records?: MemoryRecordEntry[]; - nextToken?: string; - error?: string; -} +export type RetrieveMemoryRecordsResult = Result<{ records: MemoryRecordEntry[]; nextToken?: string }>; /** * Searches memory records using semantic retrieval via the AWS SDK. @@ -59,8 +60,11 @@ export async function retrieveMemoryRecords( } catch (error: unknown) { const err = error as Error; if (err.name === 'ResourceNotFoundException') { - return { success: false, error: `Memory '${memoryId}' not found. It may not have been deployed yet.` }; + return { + success: false, + error: new ResourceNotFoundError(`Memory '${memoryId}' not found. It may not have been deployed yet.`), + }; } - return { success: false, error: err.message ?? String(error) }; + return { success: false, error: toError(error) }; } } diff --git a/src/cli/operations/recommendation/__tests__/apply-to-bundle.test.ts b/src/cli/operations/recommendation/__tests__/apply-to-bundle.test.ts index 5e0fb668a..981f00a75 100644 --- a/src/cli/operations/recommendation/__tests__/apply-to-bundle.test.ts +++ b/src/cli/operations/recommendation/__tests__/apply-to-bundle.test.ts @@ -1,6 +1,7 @@ import type { ConfigIO } from '../../../../lib'; import type { RecommendationResult } from '../../../aws/agentcore-recommendation'; import { applyRecommendationToBundle } from '../apply-to-bundle'; +import assert from 'node:assert'; import { describe, expect, it, vi } from 'vitest'; const { RUNTIME_ARN, BUNDLE_ARN, NEW_VERSION_ID } = vi.hoisted(() => ({ @@ -98,7 +99,7 @@ describe('applyRecommendationToBundle', () => { configIO ); - expect(applyResult.success).toBe(true); + assert(applyResult.success); expect(applyResult.newVersionId).toBe(NEW_VERSION_ID); // Verify spec was written with server components @@ -132,7 +133,7 @@ describe('applyRecommendationToBundle', () => { configIO ); - expect(applyResult.success).toBe(true); + assert(applyResult.success); expect(applyResult.newVersionId).toBe(NEW_VERSION_ID); }); @@ -152,7 +153,7 @@ describe('applyRecommendationToBundle', () => { configIO ); - expect(applyResult.success).toBe(true); + assert(applyResult.success); expect(applyResult.newVersionId).toBe(NEW_VERSION_ID); }); @@ -171,8 +172,8 @@ describe('applyRecommendationToBundle', () => { configIO ); - expect(applyResult.success).toBe(false); - expect(applyResult.error).toContain('does not contain a new config bundle version'); + assert(!applyResult.success); + expect(applyResult.error.message).toContain('does not contain a new config bundle version'); expect(writeSpecSpy).not.toHaveBeenCalled(); }); @@ -192,8 +193,8 @@ describe('applyRecommendationToBundle', () => { configIO ); - expect(applyResult.success).toBe(false); - expect(applyResult.error).toContain('NonExistent'); + assert(!applyResult.success); + expect(applyResult.error.message).toContain('NonExistent'); expect(writeSpecSpy).not.toHaveBeenCalled(); }); }); diff --git a/src/cli/operations/recommendation/__tests__/recommendation-storage.test.ts b/src/cli/operations/recommendation/__tests__/recommendation-storage.test.ts index f6a60b6e8..55c5b6eae 100644 --- a/src/cli/operations/recommendation/__tests__/recommendation-storage.test.ts +++ b/src/cli/operations/recommendation/__tests__/recommendation-storage.test.ts @@ -17,7 +17,9 @@ function makeTmpDir(): string { return dir; } -function makeResult(overrides: Partial = {}): RunRecommendationCommandResult { +function makeResult( + overrides: Partial> = {} +): RunRecommendationCommandResult { return { success: true, recommendationId: 'rec-123', diff --git a/src/cli/operations/recommendation/__tests__/run-recommendation.test.ts b/src/cli/operations/recommendation/__tests__/run-recommendation.test.ts index b26a59b32..765ee6692 100644 --- a/src/cli/operations/recommendation/__tests__/run-recommendation.test.ts +++ b/src/cli/operations/recommendation/__tests__/run-recommendation.test.ts @@ -1,4 +1,5 @@ import { runRecommendationCommand } from '../run-recommendation'; +import assert from 'node:assert'; import { beforeEach, describe, expect, it, vi } from 'vitest'; // Mock dependencies — paths are relative to the file under test (run-recommendation.ts) @@ -29,6 +30,25 @@ vi.mock('../../../../lib', () => ({ readDeployedState = mockReadDeployedState; resolveAWSDeploymentTargets = vi.fn().mockResolvedValue([{ region: 'us-east-1' }]); }, + toError: (err: unknown) => (err instanceof Error ? err : new Error(String(err))), + ResourceNotFoundError: class extends Error { + constructor(m: string) { + super(m); + this.name = 'ResourceNotFoundError'; + } + }, + ValidationError: class extends Error { + constructor(m: string) { + super(m); + this.name = 'ValidationError'; + } + }, + TimeoutError: class extends Error { + constructor(m: string) { + super(m); + this.name = 'TimeoutError'; + } + }, })); vi.mock('../../../aws/region', () => ({ @@ -71,9 +91,9 @@ describe('runRecommendationCommand', () => { traceSource: 'cloudwatch', }); - expect(result.success).toBe(false); - expect(result.error).toContain('NonExistentAgent'); - expect(result.error).toContain('not deployed'); + assert(!result.success); + expect(result.error.message).toContain('NonExistentAgent'); + expect(result.error.message).toContain('not deployed'); }); it('returns error when evaluator cannot be resolved', async () => { @@ -86,9 +106,9 @@ describe('runRecommendationCommand', () => { traceSource: 'cloudwatch', }); - expect(result.success).toBe(false); - expect(result.error).toContain('UnknownEvaluator'); - expect(result.error).toContain('not found'); + assert(!result.success); + expect(result.error.message).toContain('UnknownEvaluator'); + expect(result.error.message).toContain('not found'); }); it('returns result on COMPLETED status', async () => { @@ -123,7 +143,7 @@ describe('runRecommendationCommand', () => { pollIntervalMs: 0, }); - expect(result.success).toBe(true); + assert(result.success); expect(result.recommendationId).toBe('rec-001'); expect(result.status).toBe('COMPLETED'); expect(result.result?.systemPromptRecommendationResult?.recommendedSystemPrompt).toBe('Optimized prompt'); @@ -153,8 +173,8 @@ describe('runRecommendationCommand', () => { pollIntervalMs: 0, }); - expect(result.success).toBe(false); - expect(result.error).toContain('FAILED'); + assert(!result.success); + expect(result.error.message).toContain('FAILED'); expect(result.recommendationId).toBe('rec-002'); }); @@ -287,8 +307,8 @@ describe('runRecommendationCommand', () => { traceSource: 'cloudwatch', }); - expect(result.success).toBe(false); - expect(result.error).toContain('API timeout'); + assert(!result.success); + expect(result.error.message).toContain('API timeout'); }); it('retries transient poll failures and succeeds', async () => { @@ -345,10 +365,10 @@ describe('runRecommendationCommand', () => { pollIntervalMs: 0, }); - expect(result.success).toBe(false); - expect(result.error).toContain('consecutive errors'); - expect(result.error).toContain('fetch failed'); - expect(result.error).toContain('rec-retry-fail'); + assert(!result.success); + expect(result.error.message).toContain('consecutive errors'); + expect(result.error.message).toContain('fetch failed'); + expect(result.error.message).toContain('rec-retry-fail'); expect(mockGetRecommendation).toHaveBeenCalledTimes(3); }); @@ -377,9 +397,9 @@ describe('runRecommendationCommand', () => { maxPollDurationMs: 0, // Immediately timeout }); - expect(result.success).toBe(false); - expect(result.error).toContain('Polling timed out'); - expect(result.error).toContain('rec-timeout'); + assert(!result.success); + expect(result.error.message).toContain('Polling timed out'); + expect(result.error.message).toContain('rec-timeout'); }); it('reads system prompt from file when inputSource is file', async () => { @@ -538,8 +558,8 @@ describe('runRecommendationCommand', () => { pollIntervalMs: 0, }); - expect(result.success).toBe(false); - expect(result.error).toContain('No spans found'); + assert(!result.success); + expect(result.error.message).toContain('No spans found'); }); it('derives service name from runtimeId by stripping hash suffix', async () => { @@ -664,10 +684,10 @@ describe('runRecommendationCommand', () => { pollIntervalMs: 0, }); - expect(result.success).toBe(false); - expect(result.error).toContain('Insufficient trace data'); - expect(result.error).toContain('INSUFFICIENT_DATA'); - expect(result.error).toContain('Not enough traces'); + assert(!result.success); + expect(result.error.message).toContain('Insufficient trace data'); + expect(result.error.message).toContain('INSUFFICIENT_DATA'); + expect(result.error.message).toContain('Not enough traces'); // Request IDs are logged to file only, not included in the error message }); diff --git a/src/cli/operations/recommendation/apply-to-bundle.ts b/src/cli/operations/recommendation/apply-to-bundle.ts index bf9060d10..26d0f00c6 100644 --- a/src/cli/operations/recommendation/apply-to-bundle.ts +++ b/src/cli/operations/recommendation/apply-to-bundle.ts @@ -9,7 +9,8 @@ * This module fetches that new version via GetConfigurationBundleVersion and * updates the local agentcore.json components to match the server state. */ -import { ConfigIO } from '../../../lib'; +import { ConfigIO, ResourceNotFoundError } from '../../../lib'; +import type { Result } from '../../../lib/result'; import { getConfigurationBundleVersion } from '../../aws/agentcore-config-bundles'; import type { RecommendationResult } from '../../aws/agentcore-recommendation'; @@ -24,12 +25,7 @@ export interface ApplyRecommendationOptions { region: string; } -export interface ApplyRecommendationResult { - success: boolean; - error?: string; - /** New version ID that was synced from the server */ - newVersionId?: string; -} +export type ApplyRecommendationResult = Result<{ newVersionId?: string }>; /** * Extract the bundleId from a bundle ARN. @@ -58,8 +54,9 @@ export async function applyRecommendationToBundle( if (!resultBundle) { return { success: false, - error: - 'Recommendation result does not contain a new config bundle version. The server may not have applied the recommendation to the bundle.', + error: new Error( + 'Recommendation result does not contain a new config bundle version. The server may not have applied the recommendation to the bundle.' + ), }; } @@ -67,7 +64,7 @@ export async function applyRecommendationToBundle( if (!bundleId) { return { success: false, - error: `Could not extract bundle ID from ARN: ${resultBundle.bundleArn}`, + error: new Error(`Could not extract bundle ID from ARN: ${resultBundle.bundleArn}`), }; } @@ -107,7 +104,7 @@ export async function applyRecommendationToBundle( if (!bundle) { return { success: false, - error: `Config bundle "${identifier}" not found in agentcore.json.`, + error: new ResourceNotFoundError(`Config bundle "${identifier}" not found in agentcore.json.`), }; } diff --git a/src/cli/operations/recommendation/recommendation-storage.ts b/src/cli/operations/recommendation/recommendation-storage.ts index ad8aa7160..2049535e3 100644 --- a/src/cli/operations/recommendation/recommendation-storage.ts +++ b/src/cli/operations/recommendation/recommendation-storage.ts @@ -43,9 +43,9 @@ export function saveRecommendationRun( agent, evaluators, status: result.status ?? 'unknown', - startedAt: result.startedAt, - completedAt: result.completedAt, - result: result.result, + startedAt: result.success ? result.startedAt : undefined, + completedAt: result.success ? result.completedAt : undefined, + result: result.success ? result.result : undefined, }; writeFileSync(filePath, JSON.stringify(record, null, 2)); diff --git a/src/cli/operations/recommendation/run-recommendation.ts b/src/cli/operations/recommendation/run-recommendation.ts index 0423cfe32..d277c01cb 100644 --- a/src/cli/operations/recommendation/run-recommendation.ts +++ b/src/cli/operations/recommendation/run-recommendation.ts @@ -6,7 +6,7 @@ * 4. Poll GetRecommendation until terminal status * 5. Return result with optimized artifact */ -import { ConfigIO } from '../../../lib'; +import { ConfigIO, ResourceNotFoundError, TimeoutError, ValidationError, toError } from '../../../lib'; import type { DeployedState } from '../../../schema'; import type { RecommendationConfig, @@ -60,7 +60,7 @@ export async function runRecommendationCommand( logger?.finalize(false); return { success: false, - error: `Agent "${options.agent}" not deployed. Run \`agentcore deploy\` first.`, + error: new Error(`Agent "${options.agent}" not deployed. Run \`agentcore deploy\` first.`), logFilePath: logger?.logFilePath, }; } @@ -73,7 +73,9 @@ export async function runRecommendationCommand( if (!evaluatorId) { return { success: false, - error: `Evaluator "${evaluator}" not found in deployed state. Use a Builtin.* name, a full ARN, or deploy a custom evaluator first.`, + error: new Error( + `Evaluator "${evaluator}" not found in deployed state. Use a Builtin.* name, a full ARN, or deploy a custom evaluator first.` + ), logFilePath: logger?.logFilePath, }; } @@ -82,7 +84,7 @@ export async function runRecommendationCommand( if (options.type === 'SYSTEM_PROMPT_RECOMMENDATION' && evaluatorIds.length !== 1) { return { success: false, - error: 'System prompt recommendations require exactly one evaluator.', + error: new ValidationError('System prompt recommendations require exactly one evaluator.'), logFilePath: logger?.logFilePath, }; } @@ -105,7 +107,9 @@ export async function runRecommendationCommand( ) { return { success: false, - error: 'System prompt content is required. Provide via --inline, --prompt-file, or --bundle-name.', + error: new ValidationError( + 'System prompt content is required. Provide via --inline, --prompt-file, or --bundle-name.' + ), logFilePath: logger?.logFilePath, }; } @@ -132,7 +136,9 @@ export async function runRecommendationCommand( if (!bundleArn) { return { success: false, - error: `Config bundle "${options.bundleName}" not found in deployed state. Run \`agentcore deploy\` first.`, + error: new ResourceNotFoundError( + `Config bundle "${options.bundleName}" not found in deployed state. Run \`agentcore deploy\` first.` + ), logFilePath: logger?.logFilePath, }; } @@ -230,7 +236,9 @@ export async function runRecommendationCommand( logger?.finalize(false); return { success: false, - error: `Polling timed out after ${Math.round(maxDurationMs / 60000)} minutes. The recommendation may still be running server-side.\nRecommendation ID: ${startResult.recommendationId}`, + error: new TimeoutError( + `Polling timed out after ${Math.round(maxDurationMs / 60000)} minutes. The recommendation may still be running server-side.\nRecommendation ID: ${startResult.recommendationId}` + ), recommendationId: startResult.recommendationId, status: currentStatus, logFilePath: logger?.logFilePath, @@ -255,7 +263,9 @@ export async function runRecommendationCommand( logger?.finalize(false); return { success: false, - error: `Polling failed after ${MAX_POLL_RETRIES} consecutive errors: ${pollErrMsg}\nThe recommendation may still be running server-side.\nRecommendation ID: ${startResult.recommendationId}`, + error: new TimeoutError( + `Polling failed after ${MAX_POLL_RETRIES} consecutive errors: ${pollErrMsg}\nThe recommendation may still be running server-side.\nRecommendation ID: ${startResult.recommendationId}` + ), recommendationId: startResult.recommendationId, status: currentStatus, logFilePath: logger?.logFilePath, @@ -303,9 +313,11 @@ export async function runRecommendationCommand( return { success: false, - error: failureDetails - ? `Recommendation failed: ${failureDetails}` - : `Recommendation finished with status: ${currentStatus}`, + error: new Error( + failureDetails + ? `Recommendation failed: ${failureDetails}` + : `Recommendation finished with status: ${currentStatus}` + ), recommendationId: startResult.recommendationId, status: currentStatus, logFilePath: logger?.logFilePath, @@ -319,7 +331,7 @@ export async function runRecommendationCommand( logger?.finalize(false); return { success: false, - error: `Recommendation ended with unexpected status: ${currentStatus}`, + error: new Error(`Recommendation ended with unexpected status: ${currentStatus}`), recommendationId: startResult.recommendationId, status: currentStatus, logFilePath: logger?.logFilePath, @@ -331,7 +343,7 @@ export async function runRecommendationCommand( logger?.finalize(false); return { success: false, - error: errorMsg, + error: toError(err), logFilePath: logger?.logFilePath, }; } diff --git a/src/cli/operations/recommendation/types.ts b/src/cli/operations/recommendation/types.ts index 426ba84a8..487681a2e 100644 --- a/src/cli/operations/recommendation/types.ts +++ b/src/cli/operations/recommendation/types.ts @@ -1,6 +1,7 @@ /** * Shared types for the recommendation feature. */ +import type { Result } from '../../../lib/result'; import type { RecommendationResult, RecommendationType } from '../../aws/agentcore-recommendation'; export type { RecommendationType } from '../../aws/agentcore-recommendation'; @@ -56,17 +57,9 @@ export interface RunRecommendationCommandOptions { onStarted?: (info: { recommendationId: string; region: string }) => void; } -export interface RunRecommendationCommandResult { - success: boolean; - error?: string; - recommendationId?: string; - status?: string; - /** The recommendation result from the API (populated on COMPLETED) */ +export type RunRecommendationCommandResult = Result<{ result?: RecommendationResult; - /** Resolved AWS region used for the recommendation */ region?: string; startedAt?: string; completedAt?: string; - /** Path to the execution log file */ - logFilePath?: string; -} +}> & { recommendationId?: string; status?: string; logFilePath?: string }; diff --git a/src/cli/operations/remove/__tests__/remove-agent-ops.test.ts b/src/cli/operations/remove/__tests__/remove-agent-ops.test.ts index fb2b9fedc..ece57dcb9 100644 --- a/src/cli/operations/remove/__tests__/remove-agent-ops.test.ts +++ b/src/cli/operations/remove/__tests__/remove-agent-ops.test.ts @@ -1,3 +1,4 @@ +import { ResourceNotFoundError } from '../../../../lib'; import { AgentPrimitive } from '../../../primitives/AgentPrimitive.js'; import { afterEach, describe, expect, it, vi } from 'vitest'; @@ -15,6 +16,29 @@ vi.mock('../../../../lib/index.js', () => ({ readProjectSpec = mockReadProjectSpec; writeProjectSpec = mockWriteProjectSpec; }, + findConfigRoot: () => '/fake/root', + toError: (err: unknown) => (err instanceof Error ? err : new Error(String(err))), + serializeResult: (r: unknown) => r, + ResourceNotFoundError: class extends Error { + constructor(m: string) { + super(m); + this.name = 'ResourceNotFoundError'; + } + }, + ConflictError: class extends Error { + constructor(m: string) { + super(m); + this.name = 'ConflictError'; + } + }, + NoProjectError: class extends Error { + constructor(m: string) { + super(m); + this.name = 'NoProjectError'; + } + }, + APP_DIR: 'app', + setEnvVar: vi.fn().mockResolvedValue(undefined), })); const makeProject = (agentNames: string[]) => ({ @@ -87,7 +111,7 @@ describe('remove', () => { const result = await primitive.remove('Missing'); - expect(result).toEqual({ success: false, error: 'Agent "Missing" not found.' }); + expect(result).toEqual({ success: false, error: new ResourceNotFoundError('Agent "Missing" not found.') }); }); it('returns error on exception', async () => { @@ -95,6 +119,6 @@ describe('remove', () => { const result = await primitive.remove('Agent1'); - expect(result).toEqual({ success: false, error: 'read fail' }); + expect(result).toEqual({ success: false, error: new Error('read fail') }); }); }); diff --git a/src/cli/operations/remove/__tests__/remove-gateway-ops.test.ts b/src/cli/operations/remove/__tests__/remove-gateway-ops.test.ts index 54293df5a..d44a4bb3c 100644 --- a/src/cli/operations/remove/__tests__/remove-gateway-ops.test.ts +++ b/src/cli/operations/remove/__tests__/remove-gateway-ops.test.ts @@ -1,3 +1,4 @@ +import { ResourceNotFoundError } from '../../../../lib'; import { GatewayPrimitive } from '../../../primitives/GatewayPrimitive.js'; import { afterEach, describe, expect, it, vi } from 'vitest'; @@ -11,6 +12,15 @@ vi.mock('../../../../lib/index.js', () => ({ writeProjectSpec = mockWriteProjectSpec; configExists = mockConfigExists; }, + findConfigRoot: () => '/fake/root', + toError: (err: unknown) => (err instanceof Error ? err : new Error(String(err))), + serializeResult: (r: unknown) => r, + ResourceNotFoundError: class extends Error { + constructor(m: string) { + super(m); + this.name = 'ResourceNotFoundError'; + } + }, })); const makeMcpSpec = (gatewayNames: string[], targetsPerGateway = 0) => ({ @@ -101,7 +111,7 @@ describe('remove', () => { const result = await primitive.remove('missing'); - expect(result).toEqual({ success: false, error: 'Gateway "missing" not found.' }); + expect(result).toEqual({ success: false, error: new ResourceNotFoundError('Gateway "missing" not found.') }); }); it('returns error on exception', async () => { @@ -109,6 +119,6 @@ describe('remove', () => { const result = await primitive.remove('gw1'); - expect(result).toEqual({ success: false, error: 'read fail' }); + expect(result).toEqual({ success: false, error: new Error('read fail') }); }); }); diff --git a/src/cli/operations/remove/__tests__/remove-identity-ops.test.ts b/src/cli/operations/remove/__tests__/remove-identity-ops.test.ts index 1b1bbb8e7..f34cee7f3 100644 --- a/src/cli/operations/remove/__tests__/remove-identity-ops.test.ts +++ b/src/cli/operations/remove/__tests__/remove-identity-ops.test.ts @@ -1,3 +1,4 @@ +import { ResourceNotFoundError } from '../../../../lib'; import { CredentialPrimitive } from '../../../primitives/CredentialPrimitive.js'; import { afterEach, describe, expect, it, vi } from 'vitest'; @@ -16,6 +17,23 @@ vi.mock('../../../../lib/index.js', () => ({ writeProjectSpec = mockWriteProjectSpec; configExists = vi.fn().mockReturnValue(false); }, + findConfigRoot: () => '/fake/root', + toError: (err: unknown) => (err instanceof Error ? err : new Error(String(err))), + serializeResult: (r: unknown) => r, + ResourceNotFoundError: class extends Error { + constructor(m: string) { + super(m); + this.name = 'ResourceNotFoundError'; + } + }, + ConflictError: class extends Error { + constructor(m: string) { + super(m); + this.name = 'ConflictError'; + } + }, + setEnvVar: vi.fn().mockResolvedValue(undefined), + getEnvVar: vi.fn(), })); const makeProject = ( @@ -93,7 +111,7 @@ describe('remove', () => { const result = await primitive.remove('Missing'); - expect(result).toEqual({ success: false, error: 'Credential "Missing" not found.' }); + expect(result).toEqual({ success: false, error: new ResourceNotFoundError('Credential "Missing" not found.') }); }); it('returns error on exception', async () => { @@ -101,6 +119,6 @@ describe('remove', () => { const result = await primitive.remove('Cred1'); - expect(result).toEqual({ success: false, error: 'read fail' }); + expect(result).toEqual({ success: false, error: new Error('read fail') }); }); }); diff --git a/src/cli/operations/remove/__tests__/remove-memory-ops.test.ts b/src/cli/operations/remove/__tests__/remove-memory-ops.test.ts index d42bedb94..29b485c15 100644 --- a/src/cli/operations/remove/__tests__/remove-memory-ops.test.ts +++ b/src/cli/operations/remove/__tests__/remove-memory-ops.test.ts @@ -1,3 +1,4 @@ +import { ResourceNotFoundError } from '../../../../lib'; import { MemoryPrimitive } from '../../../primitives/MemoryPrimitive.js'; import { afterEach, describe, expect, it, vi } from 'vitest'; @@ -15,6 +16,15 @@ vi.mock('../../../../lib/index.js', () => ({ readProjectSpec = mockReadProjectSpec; writeProjectSpec = mockWriteProjectSpec; }, + findConfigRoot: () => '/fake/root', + toError: (err: unknown) => (err instanceof Error ? err : new Error(String(err))), + serializeResult: (r: unknown) => r, + ResourceNotFoundError: class extends Error { + constructor(m: string) { + super(m); + this.name = 'ResourceNotFoundError'; + } + }, })); const makeProject = (memoryNames: string[]) => ({ @@ -84,7 +94,7 @@ describe('remove', () => { const result = await primitive.remove('Missing'); - expect(result).toEqual({ success: false, error: 'Memory "Missing" not found.' }); + expect(result).toEqual({ success: false, error: new ResourceNotFoundError('Memory "Missing" not found.') }); }); it('returns error on exception', async () => { @@ -92,6 +102,6 @@ describe('remove', () => { const result = await primitive.remove('Mem1'); - expect(result).toEqual({ success: false, error: 'read fail' }); + expect(result).toEqual({ success: false, error: new Error('read fail') }); }); }); diff --git a/src/cli/operations/remove/remove-gateway-target.ts b/src/cli/operations/remove/remove-gateway-target.ts index 0ceebf7eb..fa84d8b70 100644 --- a/src/cli/operations/remove/remove-gateway-target.ts +++ b/src/cli/operations/remove/remove-gateway-target.ts @@ -1,6 +1,7 @@ -import { ConfigIO } from '../../../lib'; +import { ConfigIO, ResourceNotFoundError, toError } from '../../../lib'; +import type { Result } from '../../../lib/result'; import type { AgentCoreCliMcpDefs, AgentCoreMcpSpec } from '../../../schema'; -import type { RemovalPreview, RemovalResult, SchemaChange } from './types'; +import type { RemovalPreview, SchemaChange } from './types'; import { existsSync } from 'fs'; import { rm } from 'fs/promises'; import { join } from 'path'; @@ -155,7 +156,7 @@ function computeRemovedToolMcpDefs( /** * Remove a gateway target from the project. */ -export async function removeGatewayTarget(tool: RemovableGatewayTarget): Promise { +export async function removeGatewayTarget(tool: RemovableGatewayTarget): Promise { try { const configIO = new ConfigIO(); const project = await configIO.readProjectSpec(); @@ -172,11 +173,14 @@ export async function removeGatewayTarget(tool: RemovableGatewayTarget): Promise const gateway = mcpSpec.agentCoreGateways.find(g => g.name === tool.gatewayName); if (!gateway) { - return { success: false, error: `Gateway "${tool.gatewayName}" not found.` }; + return { success: false, error: new ResourceNotFoundError(`Gateway "${tool.gatewayName}" not found.`) }; } const target = gateway.targets.find(t => t.name === tool.name); if (!target) { - return { success: false, error: `Target "${tool.name}" not found in gateway "${tool.gatewayName}".` }; + return { + success: false, + error: new ResourceNotFoundError(`Target "${tool.name}" not found in gateway "${tool.gatewayName}".`), + }; } if (target.compute?.implementation && 'path' in target.compute.implementation) { toolPath = target.compute.implementation.path; @@ -200,7 +204,6 @@ export async function removeGatewayTarget(tool: RemovableGatewayTarget): Promise return { success: true }; } catch (err) { - const message = err instanceof Error ? err.message : 'Unknown error'; - return { success: false, error: message }; + return { success: false, error: toError(err) }; } } diff --git a/src/cli/operations/remove/types.ts b/src/cli/operations/remove/types.ts index 2194a66f7..66c2af228 100644 --- a/src/cli/operations/remove/types.ts +++ b/src/cli/operations/remove/types.ts @@ -21,11 +21,6 @@ export interface RemovalPreview { schemaChanges: SchemaChange[]; } -/** - * Result of a removal operation. - */ -export type RemovalResult = { success: true } | { success: false; error: string }; - /** * Snapshot of all schemas before removal (for diff computation). */ diff --git a/src/cli/operations/traces/__tests__/get-trace.test.ts b/src/cli/operations/traces/__tests__/get-trace.test.ts index 8f833cc0a..3498f6ebb 100644 --- a/src/cli/operations/traces/__tests__/get-trace.test.ts +++ b/src/cli/operations/traces/__tests__/get-trace.test.ts @@ -1,5 +1,6 @@ import { fetchTraceRecords, getTrace } from '../get-trace'; import type { FetchTraceRecordsOptions } from '../types'; +import assert from 'node:assert'; import { existsSync, mkdtempSync, readFileSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -57,14 +58,14 @@ describe('fetchTraceRecords', () => { const result = await fetchTraceRecords(baseOptions); - expect(result.success).toBe(true); + assert(result.success); expect(result.records).toHaveLength(2); - expect(result.records![0]).toEqual({ + expect(result.records[0]).toEqual({ '@timestamp': '2024-01-01T00:00:00Z', '@message': { traceId: 'abc123', spanId: 'span1' }, '@ptr': 'ptr-value-1', }); - expect(result.records![1]).toEqual({ + expect(result.records[1]).toEqual({ '@timestamp': '2024-01-01T00:00:01Z', '@message': { traceId: 'abc123', spanId: 'span2' }, }); @@ -76,8 +77,8 @@ describe('fetchTraceRecords', () => { traceId: 'invalid!@#$', }); - expect(result.success).toBe(false); - expect(result.error).toContain('Invalid trace ID format'); + assert(!result.success); + expect(result.error.message).toContain('Invalid trace ID format'); expect(mockSend).not.toHaveBeenCalled(); }); @@ -89,8 +90,8 @@ describe('fetchTraceRecords', () => { const result = await fetchTraceRecords(baseOptions); - expect(result.success).toBe(false); - expect(result.error).toContain('No trace data found'); + assert(!result.success); + expect(result.error.message).toContain('No trace data found'); }); it('returns error when query fails to start', async () => { @@ -98,8 +99,8 @@ describe('fetchTraceRecords', () => { const result = await fetchTraceRecords(baseOptions); - expect(result.success).toBe(false); - expect(result.error).toContain('Failed to start CloudWatch Logs Insights query'); + assert(!result.success); + expect(result.error.message).toContain('Failed to start CloudWatch Logs Insights query'); }); it('returns error when query status is Failed', async () => { @@ -107,8 +108,8 @@ describe('fetchTraceRecords', () => { const result = await fetchTraceRecords(baseOptions); - expect(result.success).toBe(false); - expect(result.error).toContain('failed'); + assert(!result.success); + expect(result.error.message).toContain('failed'); }); it('preserves @ptr when present in CloudWatch response', async () => { @@ -125,9 +126,9 @@ describe('fetchTraceRecords', () => { const result = await fetchTraceRecords(baseOptions); - expect(result.success).toBe(true); + assert(result.success); expect(result.records).toHaveLength(1); - expect(result.records![0]!['@ptr']).toBe('cw-ptr-123'); + expect(result.records[0]!['@ptr']).toBe('cw-ptr-123'); }); it('omits @ptr when not present in CloudWatch response', async () => { @@ -143,8 +144,8 @@ describe('fetchTraceRecords', () => { const result = await fetchTraceRecords(baseOptions); - expect(result.success).toBe(true); - expect(result.records![0]).not.toHaveProperty('@ptr'); + assert(result.success); + expect(result.records[0]).not.toHaveProperty('@ptr'); }); it('handles non-JSON @message gracefully', async () => { @@ -160,9 +161,9 @@ describe('fetchTraceRecords', () => { const result = await fetchTraceRecords(baseOptions); - expect(result.success).toBe(true); + assert(result.success); expect(result.records).toHaveLength(1); - expect(result.records![0]!['@message']).toBe('plain text message'); + expect(result.records[0]!['@message']).toBe('plain text message'); }); it('handles ResourceNotFoundException', async () => { @@ -172,9 +173,9 @@ describe('fetchTraceRecords', () => { const result = await fetchTraceRecords(baseOptions); - expect(result.success).toBe(false); - expect(result.error).toContain('Log group'); - expect(result.error).toContain('not found'); + assert(!result.success); + expect(result.error.message).toContain('Log group'); + expect(result.error.message).toContain('not found'); }); }); @@ -212,7 +213,7 @@ describe('getTrace', () => { endTime: 2000000, }); - expect(result.success).toBe(true); + assert(result.success); expect(result.filePath).toContain('test-trace.json'); const content = JSON.parse(readFileSync(outputPath, 'utf-8')); expect(content).toHaveLength(1); @@ -231,8 +232,8 @@ describe('getTrace', () => { endTime: 2000000, }); - expect(result.success).toBe(false); - expect(result.error).toContain('Invalid trace ID format'); + assert(!result.success); + expect(result.error.message).toContain('Invalid trace ID format'); expect(existsSync(outputPath)).toBe(false); }); }); diff --git a/src/cli/operations/traces/__tests__/list-traces.test.ts b/src/cli/operations/traces/__tests__/list-traces.test.ts index 0bbe884de..ab6c8b9b9 100644 --- a/src/cli/operations/traces/__tests__/list-traces.test.ts +++ b/src/cli/operations/traces/__tests__/list-traces.test.ts @@ -1,5 +1,7 @@ +import { ResourceNotFoundError } from '../../../../lib'; import { listTraces } from '../list-traces'; import type { ListTracesOptions } from '../types'; +import assert from 'node:assert'; import { afterEach, describe, expect, it, vi } from 'vitest'; const { mockRunInsightsQuery } = vi.hoisted(() => ({ @@ -38,15 +40,15 @@ describe('listTraces', () => { const result = await listTraces(baseOptions); - expect(result.success).toBe(true); + assert(result.success); expect(result.traces).toHaveLength(2); - expect(result.traces![0]).toEqual({ + expect(result.traces[0]).toEqual({ traceId: 'trace-1', timestamp: '2024-01-01T00:05:00Z', sessionId: 'sess-1', spanCount: '12', }); - expect(result.traces![1]).toEqual({ + expect(result.traces[1]).toEqual({ traceId: 'trace-2', timestamp: '2024-01-01T00:03:00Z', sessionId: undefined, @@ -66,9 +68,9 @@ describe('listTraces', () => { const result = await listTraces(baseOptions); - expect(result.success).toBe(true); + assert(result.success); expect(result.traces).toHaveLength(1); - expect(result.traces![0]!.traceId).toBe('trace-1'); + expect(result.traces[0]!.traceId).toBe('trace-1'); }); it('falls back to firstSeen when lastSeen is missing', async () => { @@ -79,8 +81,8 @@ describe('listTraces', () => { const result = await listTraces(baseOptions); - expect(result.success).toBe(true); - expect(result.traces![0]!.timestamp).toBe('2024-01-01T00:00:00Z'); + assert(result.success); + expect(result.traces[0]!.timestamp).toBe('2024-01-01T00:00:00Z'); }); it('returns empty traces for empty query results', async () => { @@ -91,20 +93,20 @@ describe('listTraces', () => { const result = await listTraces(baseOptions); - expect(result.success).toBe(true); + assert(result.success); expect(result.traces).toHaveLength(0); }); it('propagates errors from runInsightsQuery', async () => { mockRunInsightsQuery.mockResolvedValueOnce({ success: false, - error: 'Log group not found', + error: new ResourceNotFoundError('Log group not found'), }); const result = await listTraces(baseOptions); - expect(result.success).toBe(false); - expect(result.error).toBe('Log group not found'); + assert(!result.success); + expect(result.error.message).toBe('Log group not found'); }); it('passes correct log group name and default limit', async () => { diff --git a/src/cli/operations/traces/get-trace.ts b/src/cli/operations/traces/get-trace.ts index a87f10a65..153e67f5c 100644 --- a/src/cli/operations/traces/get-trace.ts +++ b/src/cli/operations/traces/get-trace.ts @@ -1,3 +1,5 @@ +import { ResourceNotFoundError, ValidationError } from '../../../lib'; +import type { Result } from '../../../lib/result'; import { DEFAULT_ENDPOINT_NAME } from '../../constants'; import { runInsightsQuery } from './insights-query'; import type { @@ -23,9 +25,12 @@ async function fetchSpans( traceId: string, startTime?: number, endTime?: number -): Promise<{ success: boolean; spans?: CloudWatchSpanRecord[]; error?: string }> { +): Promise> { if (!TRACE_ID_PATTERN.test(traceId)) { - return { success: false, error: 'Invalid trace ID format. Expected a hex string (e.g., abc123def456).' }; + return { + success: false, + error: new ValidationError('Invalid trace ID format. Expected a hex string (e.g., abc123def456).'), + }; } const result = await runInsightsQuery({ @@ -50,7 +55,7 @@ async function fetchSpans( if (!result.success) return { success: false, error: result.error }; - const spans: CloudWatchSpanRecord[] = (result.rows ?? []) + const spans: CloudWatchSpanRecord[] = result.rows .filter(row => row.traceId && row.spanId) .map(row => ({ traceId: row.traceId!, @@ -82,7 +87,10 @@ export async function fetchTraceRecords(options: FetchTraceRecordsOptions): Prom const { region, runtimeId, traceId, includeSpans } = options; if (!TRACE_ID_PATTERN.test(traceId)) { - return { success: false, error: 'Invalid trace ID format. Expected a hex string (e.g., abc123def456).' }; + return { + success: false, + error: new ValidationError('Invalid trace ID format. Expected a hex string (e.g., abc123def456).'), + }; } const [recordsResult, spansResult] = await Promise.all([ @@ -103,10 +111,10 @@ export async function fetchTraceRecords(options: FetchTraceRecordsOptions): Prom return { success: false, error: recordsResult.error }; } - const traceData = recordsResult.rows ?? []; + const traceData = recordsResult.rows; - if (traceData.length === 0 && (!spansResult || (spansResult.spans ?? []).length === 0)) { - return { success: false, error: `No trace data found for trace ID: ${traceId}` }; + if (traceData.length === 0 && (!spansResult || !spansResult.success || spansResult.spans.length === 0)) { + return { success: false, error: new ResourceNotFoundError(`No trace data found for trace ID: ${traceId}`) }; } const records: CloudWatchTraceRecord[] = traceData.map(entry => { @@ -129,11 +137,9 @@ export async function fetchTraceRecords(options: FetchTraceRecordsOptions): Prom return record; }); - const result: FetchTraceRecordsResult = { success: true, records }; - - if (spansResult?.success && spansResult.spans) { - result.spans = spansResult.spans; - } + const result: FetchTraceRecordsResult = spansResult?.success + ? { success: true, records, spans: spansResult.spans } + : { success: true, records }; return result; } @@ -146,7 +152,10 @@ export async function getTrace(options: GetTraceOptions): Promise { diff --git a/src/cli/operations/traces/insights-query.ts b/src/cli/operations/traces/insights-query.ts index 5a4da2031..5d9c30b3a 100644 --- a/src/cli/operations/traces/insights-query.ts +++ b/src/cli/operations/traces/insights-query.ts @@ -1,3 +1,5 @@ +import { ResourceNotFoundError, TimeoutError, toError } from '../../../lib'; +import type { Result } from '../../../lib/result'; import { getCredentialProvider } from '../../aws'; import { CloudWatchLogsClient, GetQueryResultsCommand, StartQueryCommand } from '@aws-sdk/client-cloudwatch-logs'; @@ -11,11 +13,7 @@ export interface InsightsQueryOptions { endTime?: number; } -export interface InsightsQueryResult { - success: boolean; - rows?: Record[]; - error?: string; -} +export type InsightsQueryResult = Result<{ rows: Record[] }>; async function pollQueryResults(client: CloudWatchLogsClient, queryId: string): Promise { for (let i = 0; i < 60; i++) { @@ -26,7 +24,7 @@ async function pollQueryResults(client: CloudWatchLogsClient, queryId: string): if (status === 'Complete' || status === 'Failed' || status === 'Cancelled') { if (status !== 'Complete') { - return { success: false, error: `Query ${status.toLowerCase()}` }; + return { success: false, error: new Error(`Query ${status.toLowerCase()}`) }; } const rows = (queryResults.results ?? []).map(row => { @@ -42,7 +40,7 @@ async function pollQueryResults(client: CloudWatchLogsClient, queryId: string): } } - return { success: false, error: 'Query timed out after 60 seconds' }; + return { success: false, error: new TimeoutError('Query timed out after 60 seconds') }; } export async function runInsightsQuery(options: InsightsQueryOptions): Promise { @@ -68,7 +66,7 @@ export async function runInsightsQuery(options: InsightsQueryOptions): Promise((acc, row) => { + const traces = result.rows.reduce((acc, row) => { if (row.traceId) { acc.push({ traceId: row.traceId, diff --git a/src/cli/operations/traces/types.ts b/src/cli/operations/traces/types.ts index fae88a83b..95afe1a1e 100644 --- a/src/cli/operations/traces/types.ts +++ b/src/cli/operations/traces/types.ts @@ -1,3 +1,5 @@ +import type { Result } from '../../../lib/result'; + export interface CloudWatchTraceRecord { '@timestamp': string; '@message': unknown; @@ -31,12 +33,10 @@ export interface FetchTraceRecordsOptions { includeSpans?: boolean; } -export interface FetchTraceRecordsResult { - success: boolean; - records?: CloudWatchTraceRecord[]; +export type FetchTraceRecordsResult = Result<{ + records: CloudWatchTraceRecord[]; spans?: CloudWatchSpanRecord[]; - error?: string; -} +}>; export interface GetTraceOptions { region: string; @@ -48,11 +48,7 @@ export interface GetTraceOptions { endTime?: number; } -export interface GetTraceResult { - success: boolean; - filePath?: string; - error?: string; -} +export type GetTraceResult = Result<{ filePath: string }>; export interface TraceEntry { traceId: string; @@ -70,8 +66,4 @@ export interface ListTracesOptions { endTime?: number; } -export interface ListTracesResult { - success: boolean; - traces?: TraceEntry[]; - error?: string; -} +export type ListTracesResult = Result<{ traces: TraceEntry[] }>; diff --git a/src/cli/primitives/ABTestPrimitive.ts b/src/cli/primitives/ABTestPrimitive.ts index dc1852c1f..7dc2a7fd4 100644 --- a/src/cli/primitives/ABTestPrimitive.ts +++ b/src/cli/primitives/ABTestPrimitive.ts @@ -1,8 +1,9 @@ -import { findConfigRoot } from '../../lib'; +import { ResourceNotFoundError, findConfigRoot, serializeResult, toError } from '../../lib'; +import type { Result } from '../../lib/result'; import type { ABTest } from '../../schema/schemas/primitives/ab-test'; import { ABTestSchema } from '../../schema/schemas/primitives/ab-test'; import { getErrorMessage } from '../errors'; -import type { RemovalPreview, RemovalResult, SchemaChange } from '../operations/remove/types'; +import type { RemovalPreview, SchemaChange } from '../operations/remove/types'; import { withCommandRunTelemetry } from '../telemetry/cli-command-run.js'; import { requireTTY } from '../tui/guards/tty'; import { BasePrimitive } from './BasePrimitive'; @@ -66,17 +67,17 @@ export class ABTestPrimitive extends BasePrimitive { + async remove(testName: string, options?: { deleteGateway?: boolean }): Promise { try { const project = await this.readProjectSpec(); const index = (project.abTests ?? []).findIndex(t => t.name === testName); if (index === -1) { - return { success: false, error: `AB test "${testName}" not found.` }; + return { success: false, error: new ResourceNotFoundError(`AB test "${testName}" not found.`) }; } const removedTest = project.abTests[index]!; @@ -124,7 +125,7 @@ export class ABTestPrimitive extends BasePrimitive { + async remove(agentName: string): Promise { try { const project = await this.readProjectSpec(); const agentIndex = project.runtimes.findIndex(a => a.name === agentName); if (agentIndex === -1) { - return { success: false, error: `Agent "${agentName}" not found.` }; + return { success: false, error: new ResourceNotFoundError(`Agent "${agentName}" not found.`) }; } // Remove agent (credentials preserved for potential reuse) @@ -164,8 +177,7 @@ export class AgentPrimitive extends BasePrimitive', 'OAuth client secret [non-interactive]') .option( '--request-header-allowlist ', - 'Comma-separated list of custom header names to allow (auto-prefixed with X-Amzn-Bedrock-AgentCore-Runtime-Custom-) [non-interactive]' + 'Comma-separated list of header names to allow. X-prefixed names (e.g. Authorization, X-Api-Key, X-Custom-Signature) pass through unchanged; bare names without X- prefix are auto-prefixed with X-Amzn-Bedrock-AgentCore-Runtime-Custom- for backward compatibility. [non-interactive]' ) .option( '--idle-timeout ', @@ -333,11 +345,11 @@ export class AgentPrimitive extends BasePrimitive; + abstract remove(name: string): Promise; /** * Preview what will be removed. @@ -120,7 +121,15 @@ export abstract class BasePrimitive< process.exit(1); } +<<<<<<< HEAD const result = await this.remove(cliOptions.name); +======= + const result = await withCommandRunTelemetry, Result>( + `remove.${this.kind}`, + {}, + () => this.remove(cliOptions.name!) + ); +>>>>>>> origin/main console.log( JSON.stringify({ success: result.success, @@ -128,7 +137,7 @@ export abstract class BasePrimitive< resourceName: cliOptions.name, message: result.success ? `Removed ${this.label.toLowerCase()} '${cliOptions.name}'` : undefined, note: result.success ? SOURCE_CODE_NOTE : undefined, - error: !result.success ? result.error : undefined, + error: !result.success ? result.error.message : undefined, }) ); process.exit(result.success ? 0 : 1); diff --git a/src/cli/primitives/ConfigBundlePrimitive.ts b/src/cli/primitives/ConfigBundlePrimitive.ts index 77f26205b..92c798d8f 100644 --- a/src/cli/primitives/ConfigBundlePrimitive.ts +++ b/src/cli/primitives/ConfigBundlePrimitive.ts @@ -1,8 +1,9 @@ -import { findConfigRoot } from '../../lib'; +import { ResourceNotFoundError, findConfigRoot, serializeResult, toError } from '../../lib'; +import type { Result } from '../../lib/result'; import type { ConfigBundle } from '../../schema'; import { ConfigBundleSchema } from '../../schema'; import { getErrorMessage } from '../errors'; -import type { RemovalPreview, RemovalResult, SchemaChange } from '../operations/remove/types'; +import type { RemovalPreview, SchemaChange } from '../operations/remove/types'; import { BasePrimitive } from './BasePrimitive'; import type { AddResult, AddScreenComponent, RemovableResource } from './types'; import type { Command } from '@commander-js/extra-typings'; @@ -37,17 +38,17 @@ export class ConfigBundlePrimitive extends BasePrimitive { + async remove(bundleName: string): Promise { try { const project = await this.readProjectSpec(); const index = (project.configBundles ?? []).findIndex(b => b.name === bundleName); if (index === -1) { - return { success: false, error: `Configuration bundle "${bundleName}" not found.` }; + return { success: false, error: new ResourceNotFoundError(`Configuration bundle "${bundleName}" not found.`) }; } project.configBundles.splice(index, 1); @@ -55,7 +56,7 @@ export class ConfigBundlePrimitive extends BasePrimitive { + async remove(credentialName: string, options?: { force?: boolean }): Promise { try { const project = await this.readProjectSpec(); const credentialIndex = project.credentials.findIndex(c => c.name === credentialName); if (credentialIndex === -1) { - return { success: false, error: `Credential "${credentialName}" not found.` }; + return { success: false, error: new ResourceNotFoundError(`Credential "${credentialName}" not found.`) }; } const credential = project.credentials[credentialIndex]!; @@ -96,7 +105,9 @@ export class CredentialPrimitive extends BasePrimitive t.name).join(', '); return { success: false, - error: `Credential "${credentialName}" is referenced by gateway target(s): ${targetList}. Use force to override.`, + error: new ConflictError( + `Credential "${credentialName}" is referenced by gateway target(s): ${targetList}. Use force to override.` + ), }; } @@ -115,8 +128,7 @@ export class CredentialPrimitive extends BasePrimitive { + async remove(evaluatorName: string): Promise { try { const project = await this.readProjectSpec(); const index = project.evaluators.findIndex(e => e.name === evaluatorName); if (index === -1) { - return { success: false, error: `Evaluator "${evaluatorName}" not found.` }; + return { success: false, error: new ResourceNotFoundError(`Evaluator "${evaluatorName}" not found.`) }; } // Warn if referenced by online eval configs @@ -77,7 +78,9 @@ export class EvaluatorPrimitive extends BasePrimitive c.name).join(', '); return { success: false, - error: `Evaluator "${evaluatorName}" is referenced by online eval config(s): ${configNames}. Remove those references first.`, + error: new ConflictError( + `Evaluator "${evaluatorName}" is referenced by online eval config(s): ${configNames}. Remove those references first.` + ), }; } @@ -97,7 +100,7 @@ export class EvaluatorPrimitive extends BasePrimitive { + async remove(gatewayName: string): Promise { try { const project = await this.readProjectSpec(); const mcpSpec = extractMcpSpec(project); const gateway = mcpSpec.agentCoreGateways.find(g => g.name === gatewayName); if (!gateway) { - return { success: false, error: `Gateway "${gatewayName}" not found.` }; + return { success: false, error: new ResourceNotFoundError(`Gateway "${gatewayName}" not found.`) }; } const newMcpSpec = this.computeRemovedGatewayMcpSpec(mcpSpec, gatewayName); @@ -86,8 +87,7 @@ export class GatewayPrimitive extends BasePrimitive { + async remove(name: string): Promise { // Find the target by name to get its gateway info const tools = await this.getRemovable(); const tool = tools.find(t => t.name === name); if (!tool) { - return { success: false, error: `Gateway target "${name}" not found.` }; + return { success: false, error: new ResourceNotFoundError(`Gateway target "${name}" not found.`) }; } return this.removeGatewayTarget(tool); } @@ -183,7 +192,7 @@ export class GatewayTargetPrimitive extends BasePrimitive { + async removeGatewayTarget(tool: RemovableGatewayTarget): Promise { try { const project = await this.readProjectSpec(); const mcpSpec = extractMcpSpec(project); @@ -195,11 +204,14 @@ export class GatewayTargetPrimitive extends BasePrimitive g.name === tool.gatewayName); if (!gateway) { - return { success: false, error: `Gateway "${tool.gatewayName}" not found.` }; + return { success: false, error: new ResourceNotFoundError(`Gateway "${tool.gatewayName}" not found.`) }; } const target = gateway.targets.find(t => t.name === tool.name); if (!target) { - return { success: false, error: `Target "${tool.name}" not found in gateway "${tool.gatewayName}".` }; + return { + success: false, + error: new ResourceNotFoundError(`Target "${tool.name}" not found in gateway "${tool.gatewayName}".`), + }; } if (target.compute?.implementation && 'path' in target.compute.implementation) { toolPath = target.compute.implementation.path; @@ -223,8 +235,7 @@ export class GatewayTargetPrimitive extends BasePrimitive { + async remove(memoryName: string): Promise { try { const project = await this.readProjectSpec(); const memoryIndex = project.memories.findIndex(m => m.name === memoryName); if (memoryIndex === -1) { - return { success: false, error: `Memory "${memoryName}" not found.` }; + return { success: false, error: new ResourceNotFoundError(`Memory "${memoryName}" not found.`) }; } project.memories.splice(memoryIndex, 1); @@ -101,8 +102,7 @@ export class MemoryPrimitive extends BasePrimitive { + async remove(configName: string): Promise { try { const project = await this.readProjectSpec(); const index = project.onlineEvalConfigs.findIndex(c => c.name === configName); if (index === -1) { - return { success: false, error: `Online eval config "${configName}" not found.` }; + return { success: false, error: new ResourceNotFoundError(`Online eval config "${configName}" not found.`) }; } project.onlineEvalConfigs.splice(index, 1); @@ -52,7 +53,7 @@ export class OnlineEvalConfigPrimitive extends BasePrimitive { + async remove(engineName: string): Promise { try { const project = await this.readProjectSpec(); const index = project.policyEngines.findIndex(e => e.name === engineName); if (index === -1) { - return { success: false, error: `Policy engine "${engineName}" not found.` }; + return { success: false, error: new ResourceNotFoundError(`Policy engine "${engineName}" not found.`) }; } project.policyEngines.splice(index, 1); @@ -70,8 +71,7 @@ export class PolicyEnginePrimitive extends BasePrimitive 1) { return { success: false, - error: 'Only one of --statement, --source, or --generate can be provided.', + error: new ValidationError('Only one of --statement, --source, or --generate can be provided.'), }; } @@ -48,7 +49,7 @@ export class PolicyPrimitive extends BasePrimitive e.name === options.engine); if (!engine) { - return { success: false, error: `Policy engine "${options.engine}" not found.` }; + return { success: false, error: new ResourceNotFoundError(`Policy engine "${options.engine}" not found.`) }; } this.checkDuplicate(engine.policies, options.name, 'Policy'); @@ -57,11 +58,11 @@ export class PolicyPrimitive extends BasePrimitive to specify one.', + error: new ResourceNotFoundError( + 'No deployed gateway found. Policy generation requires a deployed gateway. Use --gateway to specify one.' + ), }; } @@ -123,7 +130,10 @@ export class PolicyPrimitive extends BasePrimitive { + async remove(nameOrCompositeKey: string, engineName?: string): Promise { try { const project = await this.readProjectSpec(); @@ -167,7 +177,9 @@ export class PolicyPrimitive extends BasePrimitive 1) { return { success: false, - error: `Policy "${resolvedPolicy}" exists in multiple engines: ${matchingEngines.map(e => e.name).join(', ')}. Use --engine to specify which one.`, + error: new ValidationError( + `Policy "${resolvedPolicy}" exists in multiple engines: ${matchingEngines.map(e => e.name).join(', ')}. Use --engine to specify which one.` + ), }; } } @@ -185,10 +197,12 @@ export class PolicyPrimitive extends BasePrimitive>>>>>> origin/main import { BasePrimitive } from './BasePrimitive'; import { SOURCE_CODE_NOTE } from './constants'; import type { AddResult, AddScreenComponent, RemovableResource } from './types'; @@ -46,7 +59,7 @@ export class RuntimeEndpointPrimitive extends BasePrimitive a.name === options.runtime); if (!runtime) { - return { success: false, error: `Runtime "${options.runtime}" not found.` }; + return { success: false, error: new ResourceNotFoundError(`Runtime "${options.runtime}" not found.`) }; } // Initialize endpoints dictionary if needed @@ -56,14 +69,14 @@ export class RuntimeEndpointPrimitive extends BasePrimitive deployedRuntime.runtimeVersion) { return { success: false, - error: `Version ${version} exceeds latest deployed version ${deployedRuntime.runtimeVersion} for runtime "${options.runtime}".`, + error: new ValidationError( + `Version ${version} exceeds latest deployed version ${deployedRuntime.runtimeVersion} for runtime "${options.runtime}".` + ), }; } } @@ -104,11 +119,11 @@ export class RuntimeEndpointPrimitive extends BasePrimitive { + async remove(name: string): Promise { try { const project = await this.readProjectSpec(); @@ -119,7 +134,7 @@ export class RuntimeEndpointPrimitive extends BasePrimitive r.name === runtimeName); if (!runtime?.endpoints?.[endpointName]) { - return { success: false, error: `Runtime endpoint "${name}" not found.` }; + return { success: false, error: new ResourceNotFoundError(`Runtime endpoint "${name}" not found.`) }; } delete runtime.endpoints[endpointName]; if (Object.keys(runtime.endpoints).length === 0) { @@ -141,9 +156,9 @@ export class RuntimeEndpointPrimitive extends BasePrimitive ({ writeProjectSpec = mockWriteProjectSpec; }, findConfigRoot: () => '/fake/root', + toError: (err: unknown) => (err instanceof Error ? err : new Error(String(err))), + serializeResult: (r: unknown) => r, + ResourceNotFoundError: class extends Error { + constructor(m: string) { + super(m); + this.name = 'ResourceNotFoundError'; + } + }, })); function makeProject(abTests: { name: string; gatewayRef?: string }[] = []) { @@ -121,7 +129,10 @@ describe('ABTestPrimitive', () => { const result = await primitive.add(validOptions); expect(result).toEqual( - expect.objectContaining({ success: false, error: expect.stringContaining('already exists') }) + expect.objectContaining({ + success: false, + error: expect.objectContaining({ message: expect.stringContaining('already exists') }), + }) ); }); @@ -130,7 +141,7 @@ describe('ABTestPrimitive', () => { const result = await primitive.add(validOptions); - expect(result).toEqual(expect.objectContaining({ success: false, error: 'disk read error' })); + expect(result).toEqual(expect.objectContaining({ success: false, error: new Error('disk read error') })); }); it('returns error when writeProjectSpec fails', async () => { @@ -139,7 +150,7 @@ describe('ABTestPrimitive', () => { const result = await primitive.add(validOptions); - expect(result).toEqual(expect.objectContaining({ success: false, error: 'disk write error' })); + expect(result).toEqual(expect.objectContaining({ success: false, error: new Error('disk write error') })); }); it('returns error when variant weights do not sum to 100', async () => { @@ -175,8 +186,8 @@ describe('ABTestPrimitive', () => { expect(result.success).toBe(false); if (!result.success) { - expect(result.error).toContain('NonExistent'); - expect(result.error).toContain('not found'); + expect(result.error.message).toContain('NonExistent'); + expect(result.error.message).toContain('not found'); } }); @@ -187,7 +198,7 @@ describe('ABTestPrimitive', () => { expect(result.success).toBe(false); if (!result.success) { - expect(result.error).toBe('io error'); + expect(result.error.message).toBe('io error'); } }); diff --git a/src/cli/primitives/__tests__/BasePrimitive.test.ts b/src/cli/primitives/__tests__/BasePrimitive.test.ts index 830cd4244..f29252bf0 100644 --- a/src/cli/primitives/__tests__/BasePrimitive.test.ts +++ b/src/cli/primitives/__tests__/BasePrimitive.test.ts @@ -1,5 +1,6 @@ +import type { Result } from '../../../lib/result'; import { BasePrimitive } from '../BasePrimitive'; -import type { AddResult, AddScreenComponent, RemovableResource, RemovalPreview, RemovalResult } from '../types'; +import type { AddResult, AddScreenComponent, RemovableResource, RemovalPreview } from '../types'; import type { Command } from '@commander-js/extra-typings'; import { describe, expect, it } from 'vitest'; import { z } from 'zod'; @@ -14,7 +15,7 @@ class StubPrimitive extends BasePrimitive { return Promise.resolve({ success: true }); } - remove(_name: string): Promise { + remove(_name: string): Promise { return Promise.resolve({ success: true }); } diff --git a/src/cli/primitives/__tests__/EvaluatorPrimitive.test.ts b/src/cli/primitives/__tests__/EvaluatorPrimitive.test.ts index b41545d20..a521e6a78 100644 --- a/src/cli/primitives/__tests__/EvaluatorPrimitive.test.ts +++ b/src/cli/primitives/__tests__/EvaluatorPrimitive.test.ts @@ -11,6 +11,20 @@ vi.mock('../../../lib/index.js', () => ({ writeProjectSpec = mockWriteProjectSpec; }, findConfigRoot: () => '/fake/root', + toError: (err: unknown) => (err instanceof Error ? err : new Error(String(err))), + serializeResult: (r: unknown) => r, + ResourceNotFoundError: class extends Error { + constructor(m: string) { + super(m); + this.name = 'ResourceNotFoundError'; + } + }, + ConflictError: class extends Error { + constructor(m: string) { + super(m); + this.name = 'ConflictError'; + } + }, })); const validConfig: EvaluatorConfig = { @@ -108,7 +122,10 @@ describe('EvaluatorPrimitive', () => { }); expect(result).toEqual( - expect.objectContaining({ success: false, error: expect.stringContaining('already exists') }) + expect.objectContaining({ + success: false, + error: expect.objectContaining({ message: expect.stringContaining('already exists') }), + }) ); }); @@ -121,7 +138,7 @@ describe('EvaluatorPrimitive', () => { config: validConfig, }); - expect(result).toEqual(expect.objectContaining({ success: false, error: 'disk read error' })); + expect(result).toEqual(expect.objectContaining({ success: false, error: new Error('disk read error') })); }); }); @@ -145,8 +162,8 @@ describe('EvaluatorPrimitive', () => { expect(result.success).toBe(false); if (!result.success) { - expect(result.error).toContain('NonExistent'); - expect(result.error).toContain('not found'); + expect(result.error.message).toContain('NonExistent'); + expect(result.error.message).toContain('not found'); } }); @@ -159,8 +176,8 @@ describe('EvaluatorPrimitive', () => { expect(result.success).toBe(false); if (!result.success) { - expect(result.error).toContain('referenced by online eval config'); - expect(result.error).toContain('MyOnlineConfig'); + expect(result.error.message).toContain('referenced by online eval config'); + expect(result.error.message).toContain('MyOnlineConfig'); } expect(mockWriteProjectSpec).not.toHaveBeenCalled(); }); @@ -172,7 +189,7 @@ describe('EvaluatorPrimitive', () => { expect(result.success).toBe(false); if (!result.success) { - expect(result.error).toBe('io error'); + expect(result.error.message).toBe('io error'); } }); }); diff --git a/src/cli/primitives/__tests__/GatewayPrimitive.test.ts b/src/cli/primitives/__tests__/GatewayPrimitive.test.ts index f812ca380..09ae5fabe 100644 --- a/src/cli/primitives/__tests__/GatewayPrimitive.test.ts +++ b/src/cli/primitives/__tests__/GatewayPrimitive.test.ts @@ -35,6 +35,14 @@ vi.mock('../../../lib', () => { ConfigIO: MockConfigIO, findConfigRoot: vi.fn().mockReturnValue('/fake/root'), setEnvVar: vi.fn().mockResolvedValue(undefined), + toError: (err: unknown) => (err instanceof Error ? err : new Error(String(err))), + serializeResult: (r: unknown) => r, + ResourceNotFoundError: class extends Error { + constructor(m: string) { + super(m); + this.name = 'ResourceNotFoundError'; + } + }, }; }); diff --git a/src/cli/primitives/__tests__/OnlineEvalConfigPrimitive.test.ts b/src/cli/primitives/__tests__/OnlineEvalConfigPrimitive.test.ts index c81160a6c..81f13b40a 100644 --- a/src/cli/primitives/__tests__/OnlineEvalConfigPrimitive.test.ts +++ b/src/cli/primitives/__tests__/OnlineEvalConfigPrimitive.test.ts @@ -10,6 +10,14 @@ vi.mock('../../../lib/index.js', () => ({ writeProjectSpec = mockWriteProjectSpec; }, findConfigRoot: () => '/fake/root', + toError: (err: unknown) => (err instanceof Error ? err : new Error(String(err))), + serializeResult: (r: unknown) => r, + ResourceNotFoundError: class extends Error { + constructor(m: string) { + super(m); + this.name = 'ResourceNotFoundError'; + } + }, })); function makeProject( @@ -126,7 +134,10 @@ describe('OnlineEvalConfigPrimitive', () => { }); expect(result).toEqual( - expect.objectContaining({ success: false, error: expect.stringContaining('already exists') }) + expect.objectContaining({ + success: false, + error: expect.objectContaining({ message: expect.stringContaining('already exists') }), + }) ); }); @@ -140,7 +151,7 @@ describe('OnlineEvalConfigPrimitive', () => { samplingRate: 10, }); - expect(result).toEqual(expect.objectContaining({ success: false, error: 'no project' })); + expect(result).toEqual(expect.objectContaining({ success: false, error: new Error('no project') })); }); }); @@ -169,8 +180,8 @@ describe('OnlineEvalConfigPrimitive', () => { expect(result.success).toBe(false); if (!result.success) { - expect(result.error).toContain('NonExistent'); - expect(result.error).toContain('not found'); + expect(result.error.message).toContain('NonExistent'); + expect(result.error.message).toContain('not found'); } }); @@ -181,7 +192,7 @@ describe('OnlineEvalConfigPrimitive', () => { expect(result.success).toBe(false); if (!result.success) { - expect(result.error).toBe('io error'); + expect(result.error.message).toBe('io error'); } }); }); diff --git a/src/cli/primitives/__tests__/RuntimeEndpointPrimitive.test.ts b/src/cli/primitives/__tests__/RuntimeEndpointPrimitive.test.ts index 46fe426f5..1dc4d9e73 100644 --- a/src/cli/primitives/__tests__/RuntimeEndpointPrimitive.test.ts +++ b/src/cli/primitives/__tests__/RuntimeEndpointPrimitive.test.ts @@ -14,6 +14,26 @@ vi.mock('../../../lib/index.js', () => ({ readDeployedState = mockReadDeployedState; }, findConfigRoot: () => '/fake/root', + toError: (err: unknown) => (err instanceof Error ? err : new Error(String(err))), + serializeResult: (r: unknown) => r, + ResourceNotFoundError: class extends Error { + constructor(m: string) { + super(m); + this.name = 'ResourceNotFoundError'; + } + }, + ConflictError: class extends Error { + constructor(m: string) { + super(m); + this.name = 'ConflictError'; + } + }, + ValidationError: class extends Error { + constructor(m: string) { + super(m); + this.name = 'ValidationError'; + } + }, })); function makeProject( @@ -87,7 +107,12 @@ describe('RuntimeEndpointPrimitive', () => { endpoint: 'prod', }); - expect(result).toEqual(expect.objectContaining({ success: false, error: expect.stringContaining('not found') })); + expect(result).toEqual( + expect.objectContaining({ + success: false, + error: expect.objectContaining({ message: expect.stringContaining('not found') }), + }) + ); }); it('returns error when endpoint already exists', async () => { @@ -100,7 +125,10 @@ describe('RuntimeEndpointPrimitive', () => { }); expect(result).toEqual( - expect.objectContaining({ success: false, error: expect.stringContaining('already exists') }) + expect.objectContaining({ + success: false, + error: expect.objectContaining({ message: expect.stringContaining('already exists') }), + }) ); }); @@ -134,7 +162,10 @@ describe('RuntimeEndpointPrimitive', () => { }); expect(result).toEqual( - expect.objectContaining({ success: false, error: expect.stringContaining('positive integer') }) + expect.objectContaining({ + success: false, + error: expect.objectContaining({ message: expect.stringContaining('positive integer') }), + }) ); }); @@ -181,7 +212,10 @@ describe('RuntimeEndpointPrimitive', () => { }); expect(result).toEqual( - expect.objectContaining({ success: false, error: expect.stringContaining('exceeds latest deployed version') }) + expect.objectContaining({ + success: false, + error: expect.objectContaining({ message: expect.stringContaining('exceeds latest deployed version') }), + }) ); }); }); @@ -223,7 +257,12 @@ describe('RuntimeEndpointPrimitive', () => { const result = await primitive.remove('MyRuntime/nonexistent'); - expect(result).toEqual(expect.objectContaining({ success: false, error: expect.stringContaining('not found') })); + expect(result).toEqual( + expect.objectContaining({ + success: false, + error: expect.objectContaining({ message: expect.stringContaining('not found') }), + }) + ); }); it('cleans up empty endpoints dict after removing last endpoint', async () => { diff --git a/src/cli/primitives/index.ts b/src/cli/primitives/index.ts index 83bff4bbb..457fbd4ec 100644 --- a/src/cli/primitives/index.ts +++ b/src/cli/primitives/index.ts @@ -26,5 +26,9 @@ export { getPrimitive, } from './registry'; export { SOURCE_CODE_NOTE } from './constants'; +<<<<<<< HEAD export type { AddResult, AddScreenComponent, RemovableResource, RemovalPreview, RemovalResult } from './types'; export type { AddHarnessOptions } from './HarnessPrimitive'; +======= +export type { AddResult, AddScreenComponent, RemovableResource, RemovalPreview, Result } from './types'; +>>>>>>> origin/main diff --git a/src/cli/primitives/types.ts b/src/cli/primitives/types.ts index 73842c972..90885ed7f 100644 --- a/src/cli/primitives/types.ts +++ b/src/cli/primitives/types.ts @@ -1,14 +1,11 @@ -import type { RemovalPreview, RemovalResult } from '../operations/remove/types'; +import type { Result } from '../../lib/result'; +import type { RemovalPreview } from '../operations/remove/types'; import type { ComponentType } from 'react'; -/** - * Result of an add operation. - * Use the generic parameter to type extra fields on the success branch: - * AddResult<{ agentName: string }> → success branch has typed agentName - */ -export type AddResult = Record> = - | ({ success: true; message?: string } & T) - | { success: false; error: string }; +export type { Result }; + +/** @deprecated Use Result directly */ +export type AddResult = Record> = Result; /** * Represents a resource that can be removed. @@ -21,7 +18,7 @@ export interface RemovableResource { /** * Re-export removal types from shared types. */ -export type { RemovalPreview, RemovalResult }; +export type { RemovalPreview }; /** * Screen component type for TUI add flows. diff --git a/src/cli/telemetry/README.md b/src/cli/telemetry/README.md index 7d83ef6dc..00ec3e633 100644 --- a/src/cli/telemetry/README.md +++ b/src/cli/telemetry/README.md @@ -58,6 +58,7 @@ async function withCommandRunTelemetry` (from `src/lib/result.ts`) + +**Behavior:** + +- On success: records `attrs` with `exit_reason: 'success'`, returns the result. +- On failure/throw: records `attrs` with `exit_reason: 'failure'`, returns `{ success: false, error }`. +- If telemetry is unavailable: runs `fn()` untracked. + +Since `attrs` are passed upfront, they are always recorded — even on failure. + +>>>>>>> origin/main **Example with attributes:** ```ts @@ -95,6 +108,25 @@ await runCliCommand('add.widget', !!options.json, async () => { }); ``` +<<<<<<< HEAD +======= +To record attrs on failure, pass `knownAttrs` as the fourth argument: + +```ts +const knownAttrs = { widget_type: standardize(WidgetType, options.type), count: options.items.length }; +await runCliCommand( + 'add.widget', + !!options.json, + async () => { + const result = await widgetPrimitive.add(options); + if (!result.success) throw new Error(result.error); + return knownAttrs; + }, + knownAttrs +); +``` + +>>>>>>> origin/main ## Key Points - Telemetry never crashes the CLI — `standardize()` falls back gracefully, `resilientParse` defaults invalid fields to diff --git a/src/cli/telemetry/__tests__/client.test.ts b/src/cli/telemetry/__tests__/client.test.ts index 96adebafc..cfba2a0c2 100644 --- a/src/cli/telemetry/__tests__/client.test.ts +++ b/src/cli/telemetry/__tests__/client.test.ts @@ -173,5 +173,54 @@ describe('TelemetryClient', () => { exit_reason: 'cancel', }); }); + + it('records fallbackAttrs on failure when provided', async () => { + const sink = new InMemorySink(); + const client = new TelemetryClient(sink); + + await expect( + client.withCommandRun( + 'create', + async () => { + throw new Error('validation failed'); + }, + { + language: 'python', + framework: 'strands', + model_provider: 'bedrock', + memory: 'none', + protocol: 'http', + build: 'codezip', + agent_type: 'create', + network_mode: 'public', + has_agent: true, + } + ) + ).rejects.toThrow('validation failed'); + + expect(sink.metrics).toHaveLength(1); + expect(sink.metrics[0]!.attrs).toMatchObject({ + exit_reason: 'failure', + error_name: 'UnknownError', + language: 'python', + framework: 'strands', + model_provider: 'bedrock', + has_agent: 'true', + }); + }); + + it('records empty attrs on failure when fallbackAttrs not provided', async () => { + const sink = new InMemorySink(); + const client = new TelemetryClient(sink); + + await expect( + client.withCommandRun('deploy', async () => { + throw new Error('boom'); + }) + ).rejects.toThrow('boom'); + + expect(sink.metrics).toHaveLength(1); + expect(sink.metrics[0]!.attrs.language).toBeUndefined(); + }); }); }); diff --git a/src/cli/telemetry/cli-command-run.ts b/src/cli/telemetry/cli-command-run.ts index f04347dc5..2fabd39fe 100644 --- a/src/cli/telemetry/cli-command-run.ts +++ b/src/cli/telemetry/cli-command-run.ts @@ -1,9 +1,9 @@ +import type { Result } from '../../lib/result'; import { getErrorMessage } from '../errors'; import { TelemetryClientAccessor } from './client-accessor.js'; import type { Command, CommandAttrs } from './schemas/command-run.js'; -// TODO: Replace with a generic Result type that preserves the original error object. -export type OperationResult = { success: true } | { success: false; error: string }; +export type OperationResult = Result; async function getTelemetryClient() { try { @@ -31,18 +31,22 @@ export async function withCommandRunTelemetry { - result = await fn(); - if (!result.success) throw new Error(result.error); - return attrs; - }); + await client.withCommandRun( + command, + async () => { + result = await fn(); + if (!result.success) throw result.error; + return attrs; + }, + attrs + ); } catch (e) { // withCommandRun re-throws after recording failure telemetry. // If result was set, fn() returned a failure result — return it directly. // If not, fn() itself threw — convert to a failure result so callers // that don't wrap in try/catch (e.g. TUI hooks) don't leak unhandled rejections. if (!result) { - return { success: false, error: getErrorMessage(e) } as R; + return { success: false, error: e instanceof Error ? e : new Error(getErrorMessage(e)) } as R; } } return result!; @@ -52,11 +56,13 @@ export async function withCommandRunTelemetry( command: C, json: boolean, - fn: () => Promise> + fn: () => Promise>, + knownAttrs?: Partial> ): Promise { try { const client = await getTelemetryClient(); @@ -64,7 +70,7 @@ export async function runCliCommand( await fn(); process.exit(0); } - await client.withCommandRun(command, fn); + await client.withCommandRun(command, fn, knownAttrs); process.exit(0); } catch (error) { if (json) { diff --git a/src/cli/telemetry/client.ts b/src/cli/telemetry/client.ts index 91dffd94f..2341231ea 100644 --- a/src/cli/telemetry/client.ts +++ b/src/cli/telemetry/client.ts @@ -26,7 +26,8 @@ export class TelemetryClient { */ async withCommandRun( command: C, - fn: () => CommandAttrs | typeof CANCELLED | Promise | typeof CANCELLED> + fn: () => CommandAttrs | typeof CANCELLED | Promise | typeof CANCELLED>, + fallbackAttrs?: Partial> ): Promise { const start = performance.now(); try { @@ -43,7 +44,7 @@ export class TelemetryClient { error_name: classifyError(err), is_user_error: isUserError(err), }; - this.recordCommandRun(command, failureResult, {}, Math.round(performance.now() - start)); + this.recordCommandRun(command, failureResult, fallbackAttrs ?? {}, Math.round(performance.now() - start)); throw err; } finally { try { @@ -75,9 +76,8 @@ export class TelemetryClient { // Validate command attrs resiliently: invalid fields default to 'unknown' // instead of dropping the entire metric. - // On failure/cancel the callback attrs are empty so validation is skipped. const validatedAttrs = - result.exit_reason !== 'failure' && result.exit_reason !== 'cancel' + Object.keys(attrs as Record).length > 0 ? resilientParse(COMMAND_SCHEMAS[command], attrs as Record) : attrs; diff --git a/src/cli/telemetry/error-classification.ts b/src/cli/telemetry/error-classification.ts index 0cc5d060d..1f5627699 100644 --- a/src/cli/telemetry/error-classification.ts +++ b/src/cli/telemetry/error-classification.ts @@ -9,6 +9,7 @@ const CONFIG_ERRORS = new Set([ 'ConfigReadError', 'ConfigWriteError', 'ConfigParseError', + 'ValidationError', ]); const PACKAGING_ERRORS = new Set([ 'PackagingError', @@ -16,11 +17,13 @@ const PACKAGING_ERRORS = new Set([ 'MissingProjectFileError', 'UnsupportedLanguageError', 'ArtifactSizeError', + 'DependencyCheckError', ]); const CREDENTIAL_ERRORS = new Set([ 'AwsCredentialsError', 'AccessDeniedException', 'AccessDenied', + 'AccessDeniedError', 'ExpiredToken', 'ExpiredTokenException', 'TokenRefreshRequired', @@ -36,6 +39,8 @@ const SERVICE_ERRORS = new Set([ 'ValidationException', 'ConflictException', 'ResourceAlreadyExistsException', + 'ResourceNotFoundError', + 'ConflictError', ]); const USER_CATEGORIES = new Set(['ConfigError', 'CredentialsError', 'ProjectError']); diff --git a/src/cli/tui/hooks/__tests__/useRemove.test.tsx b/src/cli/tui/hooks/__tests__/useRemove.test.tsx index ef2a68dda..e0ed7f9ac 100644 --- a/src/cli/tui/hooks/__tests__/useRemove.test.tsx +++ b/src/cli/tui/hooks/__tests__/useRemove.test.tsx @@ -281,7 +281,7 @@ describe('useRemoveAgent', () => { }); it('calls removeAgent and shows failure result', async () => { - mockAgentRemove.mockResolvedValue({ success: false, error: 'Not found' }); + mockAgentRemove.mockResolvedValue({ success: false, error: new Error('Not found') }); const { lastFrame } = render(); await delay(); diff --git a/src/cli/tui/hooks/useCreateABTest.ts b/src/cli/tui/hooks/useCreateABTest.ts index e54666074..89c36715e 100644 --- a/src/cli/tui/hooks/useCreateABTest.ts +++ b/src/cli/tui/hooks/useCreateABTest.ts @@ -43,7 +43,7 @@ export function useCreateABTest() { enableOnCreate: config.enableOnCreate, }); if (!addResult.success) { - throw new Error(addResult.error ?? 'Failed to create AB test'); + throw new Error(addResult.error?.message ?? 'Failed to create AB test'); } setStatus({ state: 'success' }); return { ok: true as const, testName: config.name }; @@ -59,7 +59,7 @@ export function useCreateABTest() { try { const addResult = await abTestPrimitive.addTargetBased(config); if (!addResult.success) { - throw new Error(addResult.error ?? 'Failed to create target-based AB test'); + throw new Error(addResult.error?.message ?? 'Failed to create target-based AB test'); } setStatus({ state: 'success' }); return { ok: true as const, testName: config.name }; diff --git a/src/cli/tui/hooks/useCreateConfigBundle.ts b/src/cli/tui/hooks/useCreateConfigBundle.ts index 864501eed..f243b8e4d 100644 --- a/src/cli/tui/hooks/useCreateConfigBundle.ts +++ b/src/cli/tui/hooks/useCreateConfigBundle.ts @@ -25,7 +25,7 @@ export function useCreateConfigBundle() { commitMessage: config.commitMessage, }); if (!addResult.success) { - throw new Error(addResult.error ?? 'Failed to create configuration bundle'); + throw new Error(addResult.error?.message ?? 'Failed to create configuration bundle'); } setStatus({ state: 'success' }); return { ok: true as const, bundleName: config.name }; diff --git a/src/cli/tui/hooks/useCreateEvaluator.ts b/src/cli/tui/hooks/useCreateEvaluator.ts index 5336cdc82..86cf7c4be 100644 --- a/src/cli/tui/hooks/useCreateEvaluator.ts +++ b/src/cli/tui/hooks/useCreateEvaluator.ts @@ -34,7 +34,7 @@ export function useCreateEvaluator() { }) ); if (!addResult.success) { - throw new Error(addResult.error ?? 'Failed to create evaluator'); + throw new Error(addResult.error?.message ?? 'Failed to create evaluator'); } setStatus({ state: 'success' }); return { ok: true as const, evaluatorName: config.name, codePath: addResult.codePath }; diff --git a/src/cli/tui/hooks/useCreateMcp.ts b/src/cli/tui/hooks/useCreateMcp.ts index 60900bc63..db541c76e 100644 --- a/src/cli/tui/hooks/useCreateMcp.ts +++ b/src/cli/tui/hooks/useCreateMcp.ts @@ -53,7 +53,7 @@ export function useCreateGateway() { }) ); if (!addResult.success) { - throw new Error(addResult.error ?? 'Failed to create gateway'); + throw new Error(addResult.error?.message ?? 'Failed to create gateway'); } const result: CreateGatewayResult = { name: config.name }; setStatus({ state: 'success', result }); diff --git a/src/cli/tui/hooks/useCreateMemory.ts b/src/cli/tui/hooks/useCreateMemory.ts index f125b2615..cf891c4f9 100644 --- a/src/cli/tui/hooks/useCreateMemory.ts +++ b/src/cli/tui/hooks/useCreateMemory.ts @@ -45,7 +45,7 @@ export function useCreateMemory() { }) ); if (!addResult.success) { - throw new Error(addResult.error ?? 'Failed to create memory'); + throw new Error(addResult.error?.message ?? 'Failed to create memory'); } // Read back the memory object const configIO = new ConfigIO(); diff --git a/src/cli/tui/hooks/useCreateOnlineEval.ts b/src/cli/tui/hooks/useCreateOnlineEval.ts index 381e6d136..0fa53f56b 100644 --- a/src/cli/tui/hooks/useCreateOnlineEval.ts +++ b/src/cli/tui/hooks/useCreateOnlineEval.ts @@ -38,7 +38,7 @@ export function useCreateOnlineEval() { }) ); if (!addResult.success) { - throw new Error(addResult.error ?? 'Failed to create online eval config'); + throw new Error(addResult.error?.message ?? 'Failed to create online eval config'); } setStatus({ state: 'success' }); return { ok: true as const, configName: config.name }; diff --git a/src/cli/tui/hooks/useRemove.ts b/src/cli/tui/hooks/useRemove.ts index db1061dd4..e8628f9ac 100644 --- a/src/cli/tui/hooks/useRemove.ts +++ b/src/cli/tui/hooks/useRemove.ts @@ -1,6 +1,7 @@ +import type { Result } from '../../../lib/result'; import type { ResourceType } from '../../commands/remove/types'; import { RemoveLogger } from '../../logging'; -import type { RemovableGatewayTarget, RemovalPreview, RemovalResult } from '../../operations/remove'; +import type { RemovableGatewayTarget, RemovalPreview } from '../../operations/remove'; import type { RemovableCredential } from '../../primitives/CredentialPrimitive'; import type { RemovableMemory } from '../../primitives/MemoryPrimitive'; import type { RemovablePolicyResource } from '../../primitives/PolicyPrimitive'; @@ -61,7 +62,7 @@ function useRemovableResources(loader: () => Promise) { * All useRemove* hooks delegate to this. */ function useRemoveResource( - removeFn: (id: TIdentifier) => Promise, + removeFn: (id: TIdentifier) => Promise, resourceType: ResourceType, getResourceName: (id: TIdentifier) => string ) { @@ -75,7 +76,15 @@ function useRemoveResource( const remove = useCallback(async (id: TIdentifier, preview?: RemovalPreview): Promise => { setState({ isLoading: true, result: null }); +<<<<<<< HEAD const result = await removeFnRef.current(id); +======= + const result = await withCommandRunTelemetry, Result>( + `remove.${resourceTypeRef.current}`, + {}, + () => removeFnRef.current(id) + ); +>>>>>>> origin/main setState({ isLoading: false, result }); let logPath: string | undefined; @@ -84,7 +93,7 @@ function useRemoveResource( resourceType: resourceTypeRef.current, resourceName: getNameRef.current(id), }); - logger.logRemoval(preview, result.success, result.success ? undefined : result.error); + logger.logRemoval(preview, result.success, result.success ? undefined : result.error?.message); logPath = logger.getAbsoluteLogPath(); setLogFilePath(logPath); } @@ -304,10 +313,10 @@ export function useRemovalPreview() { interface RemovalState { isLoading: boolean; - result: RemovalResult | null; + result: Result | null; } -type RemoveResult = RemovalResult & { logFilePath?: string }; +type RemoveResult = Result & { logFilePath?: string }; export function useRemoveAgent() { return useRemoveResource( diff --git a/src/cli/tui/screens/agent/AddAgentScreen.tsx b/src/cli/tui/screens/agent/AddAgentScreen.tsx index c8961c065..5072e8ccb 100644 --- a/src/cli/tui/screens/agent/AddAgentScreen.tsx +++ b/src/cli/tui/screens/agent/AddAgentScreen.tsx @@ -1181,8 +1181,8 @@ export function AddAgentScreen({ existingAgentNames, onComplete, onExit }: AddAg /> - Enter header suffixes or full names. We auto-prefix with X-Amzn-Bedrock-AgentCore-Runtime-Custom- if - needed. 'Authorization' is also accepted. + Enter header names (e.g. Authorization, X-Api-Key, X-Custom-Signature). Bare names without X- prefix are + auto-prefixed with X-Amzn-Bedrock-AgentCore-Runtime-Custom- for backward compatibility. diff --git a/src/cli/tui/screens/agent/useAddAgent.ts b/src/cli/tui/screens/agent/useAddAgent.ts index aeac9e442..2c3539dd4 100644 --- a/src/cli/tui/screens/agent/useAddAgent.ts +++ b/src/cli/tui/screens/agent/useAddAgent.ts @@ -1,4 +1,12 @@ -import { APP_DIR, ConfigIO, NoProjectError, findConfigRoot, setEnvVar } from '../../../../lib'; +import { + APP_DIR, + AgentAlreadyExistsError, + ConfigIO, + NoProjectError, + type Result, + findConfigRoot, + setEnvVar, +} from '../../../../lib'; import type { AgentEnvSpec, DirectoryPath, FilePath } from '../../../../schema'; import { type PythonSetupResult, setupPythonProject } from '../../../operations'; import { createConfigBundleForAgent } from '../../../operations/agent/config-bundle-defaults'; @@ -165,7 +173,7 @@ export function useAddAgent() { () => addAgentInner(config) ); if (!result.success) { - return { ok: false, error: result.error }; + return { ok: false, error: result.error.message }; } return result.outcome; } finally { @@ -180,26 +188,24 @@ export function useAddAgent() { return { addAgent, isLoading, reset }; } -type AddAgentInnerResult = - | { success: true; outcome: AddAgentCreateResult | AddAgentByoResult } - | { success: false; error: string }; +type AddAgentInnerResult = Result<{ outcome: AddAgentCreateResult | AddAgentByoResult }>; async function addAgentInner(config: AddAgentConfig): Promise { const configBaseDir = findConfigRoot(); if (!configBaseDir) { - return { success: false, error: new NoProjectError().message }; + return { success: false, error: new NoProjectError() }; } const configIO = new ConfigIO({ baseDir: configBaseDir }); if (!configIO.configExists('project')) { - return { success: false, error: new NoProjectError().message }; + return { success: false, error: new NoProjectError() }; } const project = await configIO.readProjectSpec(); const existingAgent = project.runtimes.find(agent => agent.name === config.name); if (existingAgent) { - return { success: false, error: `Agent "${config.name}" already exists in this project.` }; + return { success: false, error: new AgentAlreadyExistsError(config.name) }; } let outcome: AddAgentCreateResult | AddAgentByoResult | AddAgentError; @@ -212,7 +218,7 @@ async function addAgentInner(config: AddAgentConfig): Promise => { +======= + const run = async (): Promise<{ success: true } | { success: false; error: Error }> => { +>>>>>>> origin/main // Project root is now cwd/projectName (creating a new directory) const projectRoot = join(cwd, projectName); const configBaseDir = join(projectRoot, CONFIG_DIR); @@ -298,7 +310,11 @@ export function useCreateFlow(cwd: string): CreateFlowState { logger.endStep('error', errMsg); updateStep(stepIndex, { status: 'error', error: errMsg }); logger.finalize(false); +<<<<<<< HEAD return { success: false, error: errMsg }; +======= + return { success: false, error: new Error(errMsg) }; +>>>>>>> origin/main } // Step: Add agent to project (if addAgentConfig is set) @@ -417,7 +433,7 @@ export function useCreateFlow(cwd: string): CreateFlowState { sessionStorageMountPath: addAgentConfig.sessionStorageMountPath, }); if (!importResult.success) { - throw new Error(importResult.error ?? 'Import failed'); + throw new Error(importResult.error?.message ?? 'Import failed'); } } else { // BYO path: just write config to project (no file generation) @@ -486,7 +502,11 @@ export function useCreateFlow(cwd: string): CreateFlowState { logger.endStep('error', errMsg); updateStep(stepIndex, { status: 'error', error: errMsg }); logger.finalize(false); +<<<<<<< HEAD return { success: false, error: errMsg }; +======= + return { success: false, error: new Error(errMsg) }; +>>>>>>> origin/main } // Step: Set up Python environment (if Python and create path) @@ -588,7 +608,11 @@ export function useCreateFlow(cwd: string): CreateFlowState { logger.endStep('error', errMsg); updateStep(stepIndex, { status: 'error', error: errMsg }); logger.finalize(false); +<<<<<<< HEAD return { success: false, error: errMsg }; +======= + return { success: false, error: new Error(errMsg) }; +>>>>>>> origin/main } // Step: Initialize git repository @@ -600,7 +624,11 @@ export function useCreateFlow(cwd: string): CreateFlowState { logger.endStep('error', gitResult.message); updateStep(stepIndex, { status: 'error', error: gitResult.message }); logger.finalize(false); +<<<<<<< HEAD return { success: false, error: gitResult.message ?? 'Git initialization failed' }; +======= + return { success: false, error: new GitInitError(gitResult.message ?? 'Git initialization failed') }; +>>>>>>> origin/main } else if (gitResult.status === 'skipped') { logger.endStep('warn', gitResult.message); updateStep(stepIndex, { status: 'success', warn: gitResult.message }); @@ -625,7 +653,11 @@ export function useCreateFlow(cwd: string): CreateFlowState { } return prev; }); +<<<<<<< HEAD return { success: false, error: errMsg }; +======= + return { success: false, error: new Error(errMsg) }; +>>>>>>> origin/main } }; diff --git a/src/cli/tui/screens/deploy/useDeployFlow.ts b/src/cli/tui/screens/deploy/useDeployFlow.ts index 5d77a5543..d23b84df5 100644 --- a/src/cli/tui/screens/deploy/useDeployFlow.ts +++ b/src/cli/tui/screens/deploy/useDeployFlow.ts @@ -25,7 +25,11 @@ import { } from '../../../operations/deploy/post-deploy-config-bundles'; import { setupHttpGateways } from '../../../operations/deploy/post-deploy-http-gateways'; import { enableOnlineEvalConfigs } from '../../../operations/deploy/post-deploy-online-evals'; +<<<<<<< HEAD import { type OperationResult, withCommandRunTelemetry } from '../../../telemetry/cli-command-run.js'; +======= +import { withCommandRunTelemetry } from '../../../telemetry/cli-command-run.js'; +>>>>>>> origin/main import { type StackDiffSummary, type Step, @@ -607,7 +611,11 @@ export function useDeployFlow(options: DeployFlowOptions = {}): DeployFlowState const attrs = context ? computeDeployAttrs(context.projectSpec, 'deploy') : { ...DEFAULT_DEPLOY_ATTRS }; +<<<<<<< HEAD const run = async (): Promise => { +======= + const run = async (): Promise<{ success: true } | { success: false; error: Error }> => { +>>>>>>> origin/main // Run diff before deploy to capture pre-deploy differences if (!isDiffRunningRef.current) { isDiffRunningRef.current = true; @@ -709,7 +717,7 @@ export function useDeployFlow(options: DeployFlowOptions = {}): DeployFlowState if (targetName) { const teardown = await performStackTeardown(targetName); if (!teardown.success) { - throw new Error(`Stack teardown failed: ${teardown.error}`); + throw new Error(`Stack teardown failed: ${teardown.error.message}`); } } } else { @@ -734,8 +742,8 @@ export function useDeployFlow(options: DeployFlowOptions = {}): DeployFlowState agentNames, hasGateways, }); - if (tsResult.error) { - logger.log(`Transaction search setup warning: ${tsResult.error}`, 'warn'); + if (!tsResult.success) { + logger.log(`Transaction search setup warning: ${tsResult.error.message}`, 'warn'); } else { setDeployNotes(prev => [ ...prev, @@ -787,7 +795,11 @@ export function useDeployFlow(options: DeployFlowOptions = {}): DeployFlowState error: logger.getFailureMessage('Publish assets'), })); } +<<<<<<< HEAD return { success: false, error: errorMsg }; +======= + return { success: false, error: err instanceof Error ? err : new Error(errorMsg) } as const; +>>>>>>> origin/main } finally { // Disable verbose output and clear callback after deploy switchableIoHost?.setVerbose(false); @@ -826,7 +838,11 @@ export function useDeployFlow(options: DeployFlowOptions = {}): DeployFlowState ? computeDeployAttrs(context.projectSpec, 'diff') : { ...DEFAULT_DEPLOY_ATTRS, mode: 'diff' as const }; +<<<<<<< HEAD const run = async (): Promise => { +======= + const run = async (): Promise<{ success: true } | { success: false; error: Error }> => { +>>>>>>> origin/main setDiffStep(prev => ({ ...prev, status: 'running' })); setShouldStartDeploy(false); setDiffSummaries([]); @@ -862,7 +878,11 @@ export function useDeployFlow(options: DeployFlowOptions = {}): DeployFlowState status: 'error', error: logger.getFailureMessage('Run CDK diff'), })); +<<<<<<< HEAD return { success: false, error: errorMsg }; +======= + return { success: false, error: err instanceof Error ? err : new Error(errorMsg) }; +>>>>>>> origin/main } finally { switchableIoHost?.setVerbose(false); switchableIoHost?.setOnRawMessage(null); diff --git a/src/cli/tui/screens/eval/EvalScreen.tsx b/src/cli/tui/screens/eval/EvalScreen.tsx index 3f57999d9..eb33557fc 100644 --- a/src/cli/tui/screens/eval/EvalScreen.tsx +++ b/src/cli/tui/screens/eval/EvalScreen.tsx @@ -339,7 +339,7 @@ export function EvalScreen({ onExit }: EvalScreenProps) { } const result = handleListEvalRuns({}); if (!result.success) { - setState({ phase: 'error', runs: [], error: result.error ?? 'Unknown error' }); + setState({ phase: 'error', runs: [], error: result.error?.message ?? 'Unknown error' }); return; } setState({ phase: 'loaded', runs: result.runs ?? [], error: null }); diff --git a/src/cli/tui/screens/generate/GenerateWizardUI.tsx b/src/cli/tui/screens/generate/GenerateWizardUI.tsx index 9c6c79599..2677858de 100644 --- a/src/cli/tui/screens/generate/GenerateWizardUI.tsx +++ b/src/cli/tui/screens/generate/GenerateWizardUI.tsx @@ -307,8 +307,8 @@ export function GenerateWizardUI({ /> - Enter header suffixes or full names. We auto-prefix with X-Amzn-Bedrock-AgentCore-Runtime-Custom- if - needed. 'Authorization' is also accepted. + Enter header names (e.g. Authorization, X-Api-Key, X-Custom-Signature). Bare names without X- prefix are + auto-prefixed with X-Amzn-Bedrock-AgentCore-Runtime-Custom- for backward compatibility. diff --git a/src/cli/tui/screens/identity/useCreateIdentity.ts b/src/cli/tui/screens/identity/useCreateIdentity.ts index c9293665b..031f30d42 100644 --- a/src/cli/tui/screens/identity/useCreateIdentity.ts +++ b/src/cli/tui/screens/identity/useCreateIdentity.ts @@ -25,7 +25,7 @@ export function useCreateIdentity() { () => credentialPrimitive.add(config) ); if (!result.success) { - throw new Error(result.error ?? 'Failed to create credential'); + throw new Error(result.error?.message ?? 'Failed to create credential'); } // Read back the credential object const configIO = new ConfigIO(); diff --git a/src/cli/tui/screens/import/ImportFlow.tsx b/src/cli/tui/screens/import/ImportFlow.tsx index ed82962d3..0a0843f7e 100644 --- a/src/cli/tui/screens/import/ImportFlow.tsx +++ b/src/cli/tui/screens/import/ImportFlow.tsx @@ -151,7 +151,7 @@ export function ImportFlow({ onBack, onNavigate }: ImportFlowProps) { Name: {result.resourceName} - {result.resourceId && ( + {'resourceId' in result && result.resourceId && ( ID: {result.resourceId} diff --git a/src/cli/tui/screens/import/ImportProgressScreen.tsx b/src/cli/tui/screens/import/ImportProgressScreen.tsx index 3771ffbab..a0f0229cf 100644 --- a/src/cli/tui/screens/import/ImportProgressScreen.tsx +++ b/src/cli/tui/screens/import/ImportProgressScreen.tsx @@ -59,9 +59,9 @@ export function ImportProgressScreen({ onSuccess(result); } else { setSteps(prev => - prev.map(s => (s.status === 'running' ? { ...s, status: 'error', error: result.error } : s)) + prev.map(s => (s.status === 'running' ? { ...s, status: 'error', error: result.error.message } : s)) ); - onError(result.error ?? 'Import failed'); + onError(result.error.message ?? 'Import failed'); } } else { // Starter toolkit @@ -75,9 +75,9 @@ export function ImportProgressScreen({ onSuccess(result); } else { setSteps(prev => - prev.map(s => (s.status === 'running' ? { ...s, status: 'error', error: result.error } : s)) + prev.map(s => (s.status === 'running' ? { ...s, status: 'error', error: result.error.message } : s)) ); - onError(result.error ?? 'Import failed'); + onError(result.error.message ?? 'Import failed'); } } }; diff --git a/src/cli/tui/screens/online-eval/OnlineEvalDashboard.tsx b/src/cli/tui/screens/online-eval/OnlineEvalDashboard.tsx index 03961473e..bfc0e7ed1 100644 --- a/src/cli/tui/screens/online-eval/OnlineEvalDashboard.tsx +++ b/src/cli/tui/screens/online-eval/OnlineEvalDashboard.tsx @@ -156,7 +156,7 @@ export function OnlineEvalDashboard({ onExit }: OnlineEvalDashboardProps) { setState(prev => ({ ...prev, phase: 'toggling' })); void handlePauseResume({ name: item.name }, action).then(result => { if (!result.success) { - setState(prev => ({ ...prev, phase: 'loaded', error: result.error ?? 'Toggle failed' })); + setState(prev => ({ ...prev, phase: 'loaded', error: result.error?.message ?? 'Toggle failed' })); return; } return fetchDashboardConfigs().then(configs => { diff --git a/src/cli/tui/screens/policy/AddPolicyFlow.tsx b/src/cli/tui/screens/policy/AddPolicyFlow.tsx index 8238d84c8..54a180dd0 100644 --- a/src/cli/tui/screens/policy/AddPolicyFlow.tsx +++ b/src/cli/tui/screens/policy/AddPolicyFlow.tsx @@ -139,7 +139,7 @@ export function AddPolicyFlow({ isInteractive = true, onExit, onBack, onDev, onD () => policyEnginePrimitive.add({ name: engineName }) ); if (!result.success) { - setFlow({ name: 'error', message: result.error }); + setFlow({ name: 'error', message: result.error.message }); return; } setEngineNames(prev => [...prev, engineName]); @@ -184,7 +184,7 @@ export function AddPolicyFlow({ isInteractive = true, onExit, onBack, onDev, onD setPolicyNames(prev => [...prev, config.name]); setFlow({ name: 'policy-success', policyName: config.name, engineName: config.engine }); } else { - setFlow({ name: 'error', message: result.error }); + setFlow({ name: 'error', message: result.error.message }); } }, []); diff --git a/src/cli/tui/screens/recommendation/RecommendationFlow.tsx b/src/cli/tui/screens/recommendation/RecommendationFlow.tsx index b28344df3..b044d5464 100644 --- a/src/cli/tui/screens/recommendation/RecommendationFlow.tsx +++ b/src/cli/tui/screens/recommendation/RecommendationFlow.tsx @@ -33,7 +33,12 @@ type FlowState = recommendationId?: string; region?: string; } - | { name: 'results'; result: RunRecommendationCommandResult; config: RecommendationWizardConfig; filePath?: string } + | { + name: 'results'; + result: Extract; + config: RecommendationWizardConfig; + filePath?: string; + } | { name: 'creds-error'; message: string } | { name: 'error'; message: string; logFilePath?: string }; @@ -194,13 +199,17 @@ export function RecommendationFlow({ onExit }: RecommendationFlowProps) { setFlow(prev => { if (prev.name !== 'running') return prev; const steps = prev.steps.map(s => - s.status === 'running' ? { ...s, status: 'error' as const, error: result.error } : s + s.status === 'running' ? { ...s, status: 'error' as const, error: result.error.message } : s ); return { ...prev, steps }; }); await new Promise(resolve => setTimeout(resolve, 2000)); if (cancelled) return; - setFlow({ name: 'error', message: result.error ?? 'Recommendation failed', logFilePath: result.logFilePath }); + setFlow({ + name: 'error', + message: result.error?.message ?? 'Recommendation failed', + logFilePath: result.logFilePath, + }); return; } @@ -333,7 +342,7 @@ export function RecommendationFlow({ onExit }: RecommendationFlowProps) { // ───────────────────────────────────────────────────────────────────────────── interface ResultsViewProps { - result: RunRecommendationCommandResult; + result: Extract; config: RecommendationWizardConfig; filePath?: string; onRunAnother: () => void; @@ -369,7 +378,7 @@ function ResultsView({ result, config, filePath, onRunAnother, onExit }: Results message: `New bundle version (${applyResult.newVersionId}) created with recommended changes. Local config updated.`, }); } else { - setApplyStatus({ applied: false, message: applyResult.error ?? 'Unknown error' }); + setApplyStatus({ applied: false, message: applyResult.error?.message ?? 'Unknown error' }); } } catch (err) { setApplyStatus({ applied: false, message: getErrorMessage(err) }); diff --git a/src/cli/tui/screens/remove/RemoveFlow.tsx b/src/cli/tui/screens/remove/RemoveFlow.tsx index 6ad51fb47..bfdd26d4f 100644 --- a/src/cli/tui/screens/remove/RemoveFlow.tsx +++ b/src/cli/tui/screens/remove/RemoveFlow.tsx @@ -347,7 +347,7 @@ export function RemoveFlow({ if (removeResult.success) { setFlow({ name: 'agent-success', agentName }); } else { - setFlow({ name: 'error', message: removeResult.error }); + setFlow({ name: 'error', message: removeResult.error.message }); } } else { setFlow({ name: 'confirm-agent', agentName, preview: result.preview }); @@ -391,7 +391,7 @@ export function RemoveFlow({ if (removeResult.success) { setFlow({ name: 'gateway-success', gatewayName }); } else { - setFlow({ name: 'error', message: removeResult.error }); + setFlow({ name: 'error', message: removeResult.error.message }); } } else { setFlow({ name: 'confirm-gateway', gatewayName, preview: result.preview }); @@ -413,7 +413,7 @@ export function RemoveFlow({ if (removeResult.success) { setFlow({ name: 'tool-success', toolName: tool.name }); } else { - setFlow({ name: 'error', message: removeResult.error }); + setFlow({ name: 'error', message: removeResult.error.message }); } } else { setFlow({ name: 'confirm-gateway-target', tool, preview: result.preview }); @@ -435,7 +435,7 @@ export function RemoveFlow({ if (removeResult.success) { setFlow({ name: 'memory-success', memoryName }); } else { - setFlow({ name: 'error', message: removeResult.error }); + setFlow({ name: 'error', message: removeResult.error.message }); } } else { setFlow({ name: 'confirm-memory', memoryName, preview: result.preview }); @@ -457,7 +457,7 @@ export function RemoveFlow({ if (removeResult.success) { setFlow({ name: 'identity-success', identityName }); } else { - setFlow({ name: 'error', message: removeResult.error }); + setFlow({ name: 'error', message: removeResult.error.message }); } } else { setFlow({ name: 'confirm-identity', identityName, preview: result.preview }); @@ -479,7 +479,7 @@ export function RemoveFlow({ if (removeResult.success) { setFlow({ name: 'evaluator-success', evaluatorName }); } else { - setFlow({ name: 'error', message: removeResult.error }); + setFlow({ name: 'error', message: removeResult.error.message }); } } else { setFlow({ name: 'confirm-evaluator', evaluatorName, preview: result.preview }); @@ -501,7 +501,7 @@ export function RemoveFlow({ if (removeResult.success) { setFlow({ name: 'online-eval-success', configName }); } else { - setFlow({ name: 'error', message: removeResult.error }); + setFlow({ name: 'error', message: removeResult.error.message }); } } else { setFlow({ name: 'confirm-online-eval', configName, preview: result.preview }); @@ -523,7 +523,7 @@ export function RemoveFlow({ if (removeResult.success) { setFlow({ name: 'policy-engine-success', engineName }); } else { - setFlow({ name: 'error', message: removeResult.error }); + setFlow({ name: 'error', message: removeResult.error.message }); } } else { setFlow({ name: 'confirm-policy-engine', engineName, preview: result.preview }); @@ -548,7 +548,7 @@ export function RemoveFlow({ if (removeResult.success) { setFlow({ name: 'policy-success', policyName }); } else { - setFlow({ name: 'error', message: removeResult.error }); + setFlow({ name: 'error', message: removeResult.error.message }); } } else { setFlow({ name: 'confirm-policy', compositeKey, policyName, preview: result.preview }); @@ -570,7 +570,7 @@ export function RemoveFlow({ if (removeResult.success) { setFlow({ name: 'config-bundle-success', bundleName }); } else { - setFlow({ name: 'error', message: removeResult.error }); + setFlow({ name: 'error', message: removeResult.error.message }); } } else { setFlow({ name: 'confirm-config-bundle', bundleName, preview: result.preview }); @@ -592,7 +592,7 @@ export function RemoveFlow({ if (removeResult.success) { setFlow({ name: 'ab-test-success', testName }); } else { - setFlow({ name: 'error', message: removeResult.error }); + setFlow({ name: 'error', message: removeResult.error.message }); } } else { setFlow({ name: 'confirm-ab-test', testName, preview: result.preview }); @@ -614,7 +614,7 @@ export function RemoveFlow({ if (removeResult.success) { setFlow({ name: 'runtime-endpoint-success', endpointName }); } else { - setFlow({ name: 'error', message: removeResult.error }); + setFlow({ name: 'error', message: removeResult.error.message }); } } else { setFlow({ name: 'confirm-runtime-endpoint', endpointName, preview: result.preview }); @@ -700,7 +700,7 @@ export function RemoveFlow({ if (result.success) { pendingResultRef.current = { name: 'agent-success', agentName, logFilePath: result.logFilePath }; } else { - pendingResultRef.current = { name: 'error', message: result.error }; + pendingResultRef.current = { name: 'error', message: result.error.message }; } setResultReady(true); }, @@ -732,7 +732,7 @@ export function RemoveFlow({ if (result.success) { pendingResultRef.current = { name: 'gateway-success', gatewayName, logFilePath: result.logFilePath }; } else { - pendingResultRef.current = { name: 'error', message: result.error }; + pendingResultRef.current = { name: 'error', message: result.error.message }; } setResultReady(true); }, @@ -748,7 +748,7 @@ export function RemoveFlow({ if (result.success) { pendingResultRef.current = { name: 'tool-success', toolName: tool.name, logFilePath: result.logFilePath }; } else { - pendingResultRef.current = { name: 'error', message: result.error }; + pendingResultRef.current = { name: 'error', message: result.error.message }; } setResultReady(true); }, @@ -764,7 +764,7 @@ export function RemoveFlow({ if (result.success) { pendingResultRef.current = { name: 'memory-success', memoryName, logFilePath: result.logFilePath }; } else { - pendingResultRef.current = { name: 'error', message: result.error }; + pendingResultRef.current = { name: 'error', message: result.error.message }; } setResultReady(true); }, @@ -780,7 +780,7 @@ export function RemoveFlow({ if (result.success) { pendingResultRef.current = { name: 'identity-success', identityName, logFilePath: result.logFilePath }; } else { - pendingResultRef.current = { name: 'error', message: result.error }; + pendingResultRef.current = { name: 'error', message: result.error.message }; } setResultReady(true); }, @@ -796,7 +796,7 @@ export function RemoveFlow({ if (result.success) { pendingResultRef.current = { name: 'evaluator-success', evaluatorName, logFilePath: result.logFilePath }; } else { - pendingResultRef.current = { name: 'error', message: result.error }; + pendingResultRef.current = { name: 'error', message: result.error.message }; } setResultReady(true); }, @@ -812,7 +812,7 @@ export function RemoveFlow({ if (result.success) { pendingResultRef.current = { name: 'online-eval-success', configName, logFilePath: result.logFilePath }; } else { - pendingResultRef.current = { name: 'error', message: result.error }; + pendingResultRef.current = { name: 'error', message: result.error.message }; } setResultReady(true); }, @@ -828,7 +828,7 @@ export function RemoveFlow({ if (result.success) { pendingResultRef.current = { name: 'policy-engine-success', engineName, logFilePath: result.logFilePath }; } else { - pendingResultRef.current = { name: 'error', message: result.error }; + pendingResultRef.current = { name: 'error', message: result.error.message }; } setResultReady(true); }, @@ -844,7 +844,7 @@ export function RemoveFlow({ if (result.success) { pendingResultRef.current = { name: 'policy-success', policyName, logFilePath: result.logFilePath }; } else { - pendingResultRef.current = { name: 'error', message: result.error }; + pendingResultRef.current = { name: 'error', message: result.error.message }; } setResultReady(true); }, @@ -860,7 +860,7 @@ export function RemoveFlow({ if (result.success) { pendingResultRef.current = { name: 'config-bundle-success', bundleName, logFilePath: result.logFilePath }; } else { - pendingResultRef.current = { name: 'error', message: result.error }; + pendingResultRef.current = { name: 'error', message: result.error.message }; } setResultReady(true); }, @@ -876,7 +876,7 @@ export function RemoveFlow({ if (result.success) { pendingResultRef.current = { name: 'ab-test-success', testName, logFilePath: result.logFilePath }; } else { - pendingResultRef.current = { name: 'error', message: result.error }; + pendingResultRef.current = { name: 'error', message: result.error.message }; } setResultReady(true); }, @@ -892,7 +892,7 @@ export function RemoveFlow({ if (result.success) { pendingResultRef.current = { name: 'runtime-endpoint-success', endpointName, logFilePath: result.logFilePath }; } else { - pendingResultRef.current = { name: 'error', message: result.error }; + pendingResultRef.current = { name: 'error', message: result.error.message }; } setResultReady(true); }, diff --git a/src/cli/tui/screens/remove/useRemoveFlow.ts b/src/cli/tui/screens/remove/useRemoveFlow.ts index 96ddd2f64..36062d520 100644 --- a/src/cli/tui/screens/remove/useRemoveFlow.ts +++ b/src/cli/tui/screens/remove/useRemoveFlow.ts @@ -1,7 +1,7 @@ import { ConfigIO, getWorkingDirectory } from '../../../../lib'; +import type { Result } from '../../../../lib/result'; import { findStack } from '../../../cloudformation/stack-discovery'; import { getErrorMessage } from '../../../errors'; -import type { RemovalResult } from '../../../operations/remove/types'; import { createDefaultProjectSpec } from '../../../project'; import { withCommandRunTelemetry } from '../../../telemetry/cli-command-run.js'; import { type Step, areStepsComplete, hasStepError } from '../../components'; @@ -151,7 +151,7 @@ export function useRemoveFlow({ force, dryRun }: RemoveFlowOptions): RemoveFlowS const res = await withCommandRunTelemetry( 'remove.all', {}, - (): Promise => + (): Promise => withMinDuration(async () => { const configIO = new ConfigIO(); @@ -164,7 +164,7 @@ export function useRemoveFlow({ force, dryRun }: RemoveFlowOptions): RemoveFlowS return { success: true }; }) ); - if (!res.success) throw new Error(res.error); + if (!res.success) throw res.error; updateStep(0, { status: 'success' }); } catch (err) { updateStep(0, { status: 'error', error: getErrorMessage(err) }); diff --git a/src/cli/tui/screens/run-eval/RunBatchEvalFlow.tsx b/src/cli/tui/screens/run-eval/RunBatchEvalFlow.tsx index 38c03c150..659d2034e 100644 --- a/src/cli/tui/screens/run-eval/RunBatchEvalFlow.tsx +++ b/src/cli/tui/screens/run-eval/RunBatchEvalFlow.tsx @@ -260,7 +260,7 @@ export function RunBatchEvalFlow({ onExit }: RunBatchEvalFlowProps) { setFlow(prev => { if (prev.name !== 'running') return prev; const steps = prev.steps.map(s => - s.status === 'running' ? { ...s, status: 'error' as const, error: result.error } : s + s.status === 'running' ? { ...s, status: 'error' as const, error: result.error.message } : s ); return { ...prev, steps }; }); @@ -268,7 +268,7 @@ export function RunBatchEvalFlow({ onExit }: RunBatchEvalFlowProps) { if (cancelled) return; setFlow({ name: 'error', - message: result.error ?? 'Batch evaluation failed', + message: result.error?.message ?? 'Batch evaluation failed', logFilePath: result.logFilePath, }); return; diff --git a/src/cli/tui/screens/run-eval/RunEvalFlow.tsx b/src/cli/tui/screens/run-eval/RunEvalFlow.tsx index 6b4ced516..231589b2d 100644 --- a/src/cli/tui/screens/run-eval/RunEvalFlow.tsx +++ b/src/cli/tui/screens/run-eval/RunEvalFlow.tsx @@ -20,7 +20,7 @@ type FlowState = | { name: 'loading' } | { name: 'wizard'; data: RunEvalFlowData } | { name: 'running'; config: RunEvalConfig } - | { name: 'results'; result: RunEvalResult; run: EvalRunResult } + | { name: 'results'; result: RunEvalResult; run: EvalRunResult; filePath: string } | { name: 'creds-error'; message: string } | { name: 'error'; message: string }; @@ -145,12 +145,12 @@ export function RunEvalFlow({ onExit, onViewRuns }: RunEvalFlowProps) { if (cancelled) return; - if (!result.success || !result.run) { - setFlow({ name: 'error', message: result.error ?? 'Evaluation failed' }); + if (!result.success) { + setFlow({ name: 'error', message: result.error.message }); return; } - setFlow({ name: 'results', result, run: result.run }); + setFlow({ name: 'results', result, run: result.run, filePath: result.filePath }); } catch (err) { if (!cancelled) setFlow({ name: 'error', message: getErrorMessage(err) }); } @@ -196,7 +196,7 @@ export function RunEvalFlow({ onExit, onViewRuns }: RunEvalFlowProps) { return ( setFlow({ name: 'loading' })} onViewRuns={onViewRuns} onExit={onExit} diff --git a/src/cli/tui/screens/runtime-endpoint/AddRuntimeEndpointFlow.tsx b/src/cli/tui/screens/runtime-endpoint/AddRuntimeEndpointFlow.tsx index 77c07b2e8..77f818124 100644 --- a/src/cli/tui/screens/runtime-endpoint/AddRuntimeEndpointFlow.tsx +++ b/src/cli/tui/screens/runtime-endpoint/AddRuntimeEndpointFlow.tsx @@ -95,7 +95,7 @@ export function AddRuntimeEndpointFlow({ }); return; } - setFlow({ name: 'error', message: result.error ?? 'Unknown error' }); + setFlow({ name: 'error', message: result.error?.message ?? 'Unknown error' }); }); }, []); diff --git a/src/cli/tui/screens/status/useStatusFlow.ts b/src/cli/tui/screens/status/useStatusFlow.ts index 2260e1e82..d54ea6e79 100644 --- a/src/cli/tui/screens/status/useStatusFlow.ts +++ b/src/cli/tui/screens/status/useStatusFlow.ts @@ -99,7 +99,7 @@ export function useStatusFlow() { ...prev, phase: 'ready', statusesLoaded: true, - statusesError: result.error, + statusesError: result.error?.message, })); return; } diff --git a/src/lib/errors/index.ts b/src/lib/errors/index.ts index f03c2281a..2acd167c8 100644 --- a/src/lib/errors/index.ts +++ b/src/lib/errors/index.ts @@ -1 +1,2 @@ export * from './config'; +export * from './types'; diff --git a/src/lib/errors/types.ts b/src/lib/errors/types.ts new file mode 100644 index 000000000..f0a05106c --- /dev/null +++ b/src/lib/errors/types.ts @@ -0,0 +1,94 @@ +/** + * Error indicating no agentcore project was found in the working directory. + */ +export class NoProjectError extends Error { + constructor(message?: string) { + super(message ?? 'No agentcore project found. Run "agentcore create" first.'); + this.name = 'NoProjectError'; + } +} + +/** + * Error thrown when an agent with the same name already exists. + */ +export class AgentAlreadyExistsError extends Error { + constructor(agentName: string) { + super(`An agent named "${agentName}" already exists in the schema.`); + this.name = 'AgentAlreadyExistsError'; + } +} + +/** + * Error indicating an AWS permissions failure (AccessDenied / AccessDeniedException). + */ +export class AccessDeniedError extends Error { + constructor(message: string, options?: ErrorOptions) { + super(message, options); + this.name = 'AccessDeniedError'; + } +} + +/** + * Error indicating missing system dependencies required for an operation. + */ +export class DependencyCheckError extends Error { + constructor(errors: string[]) { + super(errors.join('\n')); + this.name = 'DependencyCheckError'; + } +} + +/** + * Error indicating git repository initialization failed. + */ +export class GitInitError extends Error { + constructor(message: string) { + super(message); + this.name = 'GitInitError'; + } +} + +/** + * Error indicating a referenced resource could not be found. + */ +export class ResourceNotFoundError extends Error { + constructor(message: string) { + super(message); + this.name = 'ResourceNotFoundError'; + } +} + +export class ValidationError extends Error { + constructor(message: string, options?: ErrorOptions) { + super(message, options); + this.name = 'ValidationError'; + } +} + +/** + * Converts an unknown thrown value to an Error instance. + * Use in catch blocks to ensure the error field is always an Error object. + */ +export function toError(err: unknown): Error { + return err instanceof Error ? err : new Error(String(err)); +} + +/** + * Error indicating a resource conflict (e.g., already exists). + */ +export class ConflictError extends Error { + constructor(message: string) { + super(message); + this.name = 'ConflictError'; + } +} + +/** + * Error indicating an operation timed out or exceeded retry limits. + */ +export class TimeoutError extends Error { + constructor(message: string) { + super(message); + this.name = 'TimeoutError'; + } +} diff --git a/src/lib/result.test.ts b/src/lib/result.test.ts new file mode 100644 index 000000000..9ecdd2bad --- /dev/null +++ b/src/lib/result.test.ts @@ -0,0 +1,19 @@ +import { serializeResult } from './result'; +import { describe, expect, it } from 'vitest'; + +describe('serializeResult', () => { + it('passes through success results unchanged', () => { + const result = { success: true as const, name: 'test' }; + expect(serializeResult(result)).toEqual({ success: true, name: 'test' }); + }); + + it('converts error.message to string on failure', () => { + const result = { success: false as const, error: new Error('something broke') }; + expect(serializeResult(result)).toEqual({ success: false, error: 'something broke' }); + }); + + it('preserves extra fields on failure branch', () => { + const result = { success: false as const, error: new Error('fail'), logPath: '/tmp/log' }; + expect(serializeResult(result)).toEqual({ success: false, error: 'fail', logPath: '/tmp/log' }); + }); +}); diff --git a/src/lib/schemas/io/__tests__/config-io.test.ts b/src/lib/schemas/io/__tests__/config-io.test.ts index ffe829fa0..ffb5e3c29 100644 --- a/src/lib/schemas/io/__tests__/config-io.test.ts +++ b/src/lib/schemas/io/__tests__/config-io.test.ts @@ -1,6 +1,6 @@ +import { NoProjectError } from '../../../errors'; import { ConfigNotFoundError, ConfigParseError, ConfigValidationError } from '../../../errors/config.js'; import { ConfigIO } from '../config-io.js'; -import { NoProjectError } from '../path-resolver.js'; import { randomUUID } from 'node:crypto'; import { existsSync, mkdirSync, writeFileSync } from 'node:fs'; import { mkdir, rm, unlink, writeFile } from 'node:fs/promises'; diff --git a/src/lib/schemas/io/__tests__/path-resolver.test.ts b/src/lib/schemas/io/__tests__/path-resolver.test.ts index 70bff97e2..320d797fd 100644 --- a/src/lib/schemas/io/__tests__/path-resolver.test.ts +++ b/src/lib/schemas/io/__tests__/path-resolver.test.ts @@ -1,5 +1,5 @@ +import { NoProjectError } from '../../../errors'; import { - NoProjectError, PathResolver, findConfigRoot, findProjectRoot, diff --git a/src/lib/schemas/io/config-io.ts b/src/lib/schemas/io/config-io.ts index e4b2740e4..9e954593e 100644 --- a/src/lib/schemas/io/config-io.ts +++ b/src/lib/schemas/io/config-io.ts @@ -20,8 +20,9 @@ import { ConfigValidationError, ConfigWriteError, } from '../../errors'; +import { NoProjectError } from '../../errors'; import { detectAwsAccount } from '../../utils'; -import { NoProjectError, type PathConfig, PathResolver, findConfigRoot } from './path-resolver'; +import { type PathConfig, PathResolver, findConfigRoot } from './path-resolver'; import { loadSharedConfigFiles } from '@smithy/shared-ini-file-loader'; import { existsSync } from 'fs'; import { mkdir, readFile, writeFile } from 'fs/promises'; diff --git a/src/lib/schemas/io/index.ts b/src/lib/schemas/io/index.ts index 212468ffe..e5915541e 100644 --- a/src/lib/schemas/io/index.ts +++ b/src/lib/schemas/io/index.ts @@ -7,7 +7,6 @@ export { getSessionProjectRoot, getWorkingDirectory, requireConfigRoot, - NoProjectError, type PathConfig, } from './path-resolver'; export { ConfigIO, createConfigIO, getSchemaUrlForVersion } from './config-io'; diff --git a/src/lib/schemas/io/path-resolver.ts b/src/lib/schemas/io/path-resolver.ts index d79e54e89..e31bb2498 100644 --- a/src/lib/schemas/io/path-resolver.ts +++ b/src/lib/schemas/io/path-resolver.ts @@ -1,20 +1,14 @@ +<<<<<<< HEAD import { APP_DIR, CLI_LOGS_DIR, CLI_SYSTEM_DIR, CONFIG_DIR, CONFIG_FILES as _CONFIG_FILES } from '../../constants'; +======= +import { CLI_LOGS_DIR, CLI_SYSTEM_DIR, CONFIG_DIR, CONFIG_FILES as _CONFIG_FILES } from '../../constants'; +import { NoProjectError } from '../../errors'; +>>>>>>> origin/main import { existsSync } from 'fs'; import { dirname, join } from 'path'; -// Re-export for backward compatibility export const CONFIG_FILES = _CONFIG_FILES; -/** - * Error thrown when no AgentCore project is found. - */ -export class NoProjectError extends Error { - constructor(message?: string) { - super(message ?? 'No agentcore project found. Run "agentcore create" first.'); - this.name = 'NoProjectError'; - } -} - /** * Get the working directory where the user invoked the CLI. * diff --git a/src/schema/schemas/agent-env.ts b/src/schema/schemas/agent-env.ts index 789109a38..3745ad2d7 100644 --- a/src/schema/schemas/agent-env.ts +++ b/src/schema/schemas/agent-env.ts @@ -125,20 +125,44 @@ export type NetworkConfig = z.infer; /** * Allowed request headers for the runtime. - * Each header must be 'Authorization' or start with 'X-Amzn-Bedrock-AgentCore-Runtime-Custom-'. + * Per https://docs.aws.amazon.com/bedrock-agentcore/latest/devguide/runtime-header-allowlist.html + * any valid HTTP header name (alphanumeric, hyphens, underscores) may be allow-listed, + * provided it is not structurally reserved (x-amz-*, x-amzn-* except Runtime-Custom-*). * Maximum 20 headers. */ export const HEADER_ALLOWLIST_PREFIX = 'X-Amzn-Bedrock-AgentCore-Runtime-Custom-'; +export const HEADER_NAME_PATTERN = /^[A-Za-z0-9\-_]+$/; export const MAX_HEADER_ALLOWLIST_SIZE = 20; +/** + * Validate a single allowlist header name. Returns null if valid, or a specific + * error message describing which rule the input violated. + * + * Note: 'x-amz-' and 'x-amzn-' are disjoint prefixes (position 5 differs: '-' vs 'n'), + * so the two checks below are independent. + */ +export function checkAllowlistHeader(val: string): string | null { + if (!HEADER_NAME_PATTERN.test(val)) { + return `Header name "${val}" must contain only alphanumeric characters, hyphens, and underscores.`; + } + const lower = val.toLowerCase(); + if (lower.startsWith('x-amz-')) { + return `Header "${val}" is not allowed. Headers starting with "x-amz-" are reserved for AWS request signing.`; + } + if (lower.startsWith('x-amzn-') && !lower.startsWith('x-amzn-bedrock-agentcore-runtime-custom-')) { + return `Header "${val}" is not allowed. Headers starting with "x-amzn-" are reserved, except for "X-Amzn-Bedrock-AgentCore-Runtime-Custom-*".`; + } + return null; +} + export const RequestHeaderAllowlistSchema = z .array( - z - .string() - .refine( - val => val === 'Authorization' || val.startsWith(HEADER_ALLOWLIST_PREFIX), - `Must be "Authorization" or start with "${HEADER_ALLOWLIST_PREFIX}"` - ) + z.string().superRefine((val, ctx) => { + const error = checkAllowlistHeader(val); + if (error) { + ctx.addIssue({ code: z.ZodIssueCode.custom, message: error }); + } + }) ) .max(MAX_HEADER_ALLOWLIST_SIZE, `Maximum ${MAX_HEADER_ALLOWLIST_SIZE} headers allowed`); diff --git a/src/schema/schemas/agentcore-project.ts b/src/schema/schemas/agentcore-project.ts index c5480bec7..8c0e219c9 100644 --- a/src/schema/schemas/agentcore-project.ts +++ b/src/schema/schemas/agentcore-project.ts @@ -17,7 +17,10 @@ import { EvaluatorNameSchema, KmsKeyArnSchema, } from './primitives/evaluator'; +<<<<<<< HEAD import { HarnessNameSchema } from './primitives/harness'; +======= +>>>>>>> origin/main import { HttpGatewaySchema } from './primitives/http-gateway'; import { DEFAULT_EPISODIC_REFLECTION_NAMESPACES,