feat(evaluator): add kmsKeyArn support for custom evaluator#994
Conversation
Coverage Report
|
ba4c55c to
fe5cd4b
Compare
|
Field naming inconsistency: Other primitives in both Additionally, CFN uses Options:
Please confirm the field name with the companion CDK PR before merging, since whatever is in |
|
Unsafe cast in
kmsKeyArn: (response as unknown as Record<string, unknown>).kmsKeyArn as string | undefined,This double-cast bypasses TypeScript's checking of the SDK response type. Compare with how Given that CFN exposes this as Options:
Either way, please verify by inspecting the actual |
|
KMS key ARN validation regex is too restrictive The regex used in both will reject several valid KMS key ARN formats:
Additionally, duplicating the same regex in two places risks them drifting; consider extracting to a shared helper (e.g. Options:
|
|
|
Reviewed the PR. The three existing automation comments (field naming |
fe5cd4b to
9bc2d70
Compare
Package TarballHow to installnpm install https://github.com/aws/agentcore-cli/releases/download/pr-994-tarball/aws-agentcore-0.13.1.tgz |
The lock file was out of sync after dependency bumps on main were merged, causing npm ci to fail in CI.
Reverts @opentelemetry/exporter-metrics-otlp-http ^0.217.0 back to ^0.214.0 and secretlint ^13.0.0 back to ^12.2.0 — these were accidentally included in the feature commit from unmerged dependabot PRs and introduce high-severity protobufjs vulnerabilities. Restores fast-xml-parser and @aws-sdk/xml-builder overrides that were also inadvertently removed. Fixes Prettier formatting on agentcore-project.ts import lines.
ed5651a to
64421a9
Compare
Cherry-picks non-breaking changes from main: - docs: add telemetry instrumentation guide (#1197) - chore: replace PAT tokens with GitHub App token (#1198) - fix: add batch eval, recommendation, CloudWatch Logs permissions (#1113) - feat(evaluator): add kmsKeyArn support (#994) - chore: replace github.token with GitHub App token (#1210) - fix: sync-preview workflow rewrite (#1078) Skipped (requires dedicated effort due to Result type refactor): - refactor: unify result types with Result<T, E> (#1125) - feat: instrument telemetry for create/deploy (#1202, #1206) - feat: record command attrs on failure (#1204)
Cherry-picks non-breaking changes from main: - docs: add telemetry instrumentation guide (#1197) - chore: replace PAT tokens with GitHub App token (#1198) - fix: add batch eval, recommendation, CloudWatch Logs permissions (#1113) - feat(evaluator): add kmsKeyArn support (#994) - chore: replace github.token with GitHub App token (#1210) - fix: sync-preview workflow rewrite (#1078) Skipped (requires dedicated effort due to Result type refactor): - refactor: unify result types with Result<T, E> (#1125) - feat: instrument telemetry for create/deploy (#1202, #1206) - feat: record command attrs on failure (#1204)
Cherry-picks non-breaking changes from main: - docs: add telemetry instrumentation guide (#1197) - chore: replace PAT tokens with GitHub App token (#1198) - fix: add batch eval, recommendation, CloudWatch Logs permissions (#1113) - feat(evaluator): add kmsKeyArn support (#994) - chore: replace github.token with GitHub App token (#1210) - fix: sync-preview workflow rewrite (#1078) Skipped (requires dedicated effort due to Result type refactor): - refactor: unify result types with Result<T, E> (#1125) - feat: instrument telemetry for create/deploy (#1202, #1206) - feat: record command attrs on failure (#1204)
* docs: add telemetry instrumentation guide (#1197) * chore: replace PAT tokens with GitHub App token (#1198) Replace secrets.PAT_TOKEN and secrets.AUTOMATION_ACCOUNT_PAT_TOKEN with short-lived tokens generated by the agentcore-devx-automation GitHub App (ID: 3637953) via actions/create-github-app-token@v1. This improves security by using ephemeral tokens scoped to the installation rather than long-lived personal access tokens. Requires adding repo variable APP_ID=3637953 and repo secret APP_PRIVATE_KEY with the app's RSA private key. * fix: add batch eval, recommendation, and CloudWatch Logs write permissions to docs (#1113) * feat: instrument telemetry for create command (CLI + TUI) (#1202) Add telemetry recording to the create command in both CLI and TUI paths: - CLI: wrap handleCreateCLI with runCliCommand to emit CreateAttrs on success/failure - TUI: wrap useCreateFlow's run() with withCommandRunTelemetry - Add telemetry assertions to existing integration tests (frameworks + edge cases) * chore: replace all github.token/GITHUB_TOKEN with GitHub App token Replaces all occurrences of \${{ github.token }} and \${{ secrets.GITHUB_TOKEN }} across .github/workflows/ with a per-job GitHub App token generated via actions/create-github-app-token@v1 using vars.APP_ID and secrets.APP_PRIVATE_KEY. * fix: bump versions to resolve security audit failure * revert: keep pr-title.yml using GITHUB_TOKEN (read-only access sufficient) * feat(evaluator): add kmsKeyArn support for custom evaluator (#994) * feat(evaluator): Add kmsKeyArn support for custom evaluator * fix: sync package-lock.json with package.json The lock file was out of sync after dependency bumps on main were merged, causing npm ci to fail in CI. * fix: revert unrelated dep bumps and fix formatting Reverts @opentelemetry/exporter-metrics-otlp-http ^0.217.0 back to ^0.214.0 and secretlint ^13.0.0 back to ^12.2.0 — these were accidentally included in the feature commit from unmerged dependabot PRs and introduce high-severity protobufjs vulnerabilities. Restores fast-xml-parser and @aws-sdk/xml-builder overrides that were also inadvertently removed. Fixes Prettier formatting on agentcore-project.ts import lines. * fix: sync package-lock.json with updated dependencies --------- Co-authored-by: notgitika <gitijh@gmail.com> * refactor: unify result types with discriminated Result<T, E> union (#1125) * refactor: unify result types with discriminated Result<T, E> union Introduce a shared Result<T, E> type (inspired by Rust's Result) that replaces ad-hoc { success: boolean; error?: string } patterns across the codebase. Key changes: - Add src/lib/types.ts with Result<T, E> discriminated union type - Add toError() helper in src/cli/errors.ts for catch blocks - Migrate all command, operation, and primitive result types to Result<T> - Error field is now Error (not string) on the failure branch - Data fields only exist on the success branch (proper narrowing) - Update all consumers to narrow before accessing branch-specific fields - Update test assertions to match new Error objects and add narrowing * docs: update AGENTS.md and telemetry README to reflect Result<T, E> type * feat: record command attrs on telemetry failure via fallbackAttrs (#1204) * feat: record command attrs on telemetry failure via fallbackAttrs Add optional fallbackAttrs parameter to client.withCommandRun so command-specific attributes are recorded even when the callback throws. - client.ts: accept fallbackAttrs, use on failure instead of {} - client.ts: run resilientParse on all non-empty attrs (not just success) - cli-command-run.ts: withCommandRunTelemetry passes attrs as fallbackAttrs - cli-command-run.ts: runCliCommand accepts optional knownAttrs param - command.tsx: extract knownAttrs upfront, pass to runCliCommand - client.test.ts: add unit tests for fallbackAttrs behavior - create-edge-cases.test.ts: assert attrs present on failure entry * chore: rebase onto mainline * fix: sync-preview pushes directly on clean merge, PRs only on conflict (#1078) * fix: sync-preview workflow restores version instead of ignoring files Instead of keeping preview's entire package.json/package-lock.json (which discards new deps, scripts, etc. from main), accept main's content and surgically restore only the version field to preview's value after merge. * fix: push directly to preview on clean merge via GitHub App bypass Use agentcore-devx-automation app token to bypass branch protection and push directly when the merge is clean (or only version conflicts). Only creates a PR when there are real conflicts in other files. * chore: use app-slug instead of app-id for token generation * fix: address review feedback on sync-preview workflow - Pass PREVIEW_VERSION via env var instead of string interpolation in node -e scripts (safer against special chars) - Make git add of package-lock.json conditional on file existence to match the earlier -f guard - Replace loose title search for dedup with headRefName prefix filter to avoid false positives from unrelated PRs - Clarify why package.json/package-lock.json are special-cased (preview carries a different version string that needs preserving) * fix: restore preview-owned files after sync merge Adds a step to restore schemas/agentcore.schema.v1.json and CHANGELOG.md to preview's versions after merging main. These files are auto-generated during preview releases — schema-check CI rejects direct modifications to schemas/, and CHANGELOG.md tracks preview releases separately. * fix: use app-id instead of app-slug for GitHub App token Aligns with the pattern in PR #1210 and ci-failure-issue.yml. * feat: instrument telemetry for deploy command (CLI + TUI) (#1206) * chore: sync safe commits from main into preview Cherry-picks non-breaking changes from main: - docs: add telemetry instrumentation guide (#1197) - chore: replace PAT tokens with GitHub App token (#1198) - fix: add batch eval, recommendation, CloudWatch Logs permissions (#1113) - feat(evaluator): add kmsKeyArn support (#994) - chore: replace github.token with GitHub App token (#1210) - fix: sync-preview workflow rewrite (#1078) Skipped (requires dedicated effort due to Result type refactor): - refactor: unify result types with Result<T, E> (#1125) - feat: instrument telemetry for create/deploy (#1202, #1206) - feat: record command attrs on failure (#1204) * chore: merge main into preview with Result refactor Brings in all remaining main commits including: - refactor: unify result types with Result<T, E> (#1125) - feat: instrument telemetry for deploy command (#1206) - feat: record command attrs on failure (#1204) - feat: instrument telemetry for create command (#1202) Adapts preview's deploy flow to use OperationResult with string errors for compatibility with the telemetry layer. --------- Co-authored-by: Hweinstock <42325418+Hweinstock@users.noreply.github.com> Co-authored-by: Aidan Daly <99039782+aidandaly24@users.noreply.github.com> Co-authored-by: Gitika <53349492+notgitika@users.noreply.github.com> Co-authored-by: Aidan Daly <aidandal@amazon.com> Co-authored-by: Harrison Weinstock <hkobew@amazon.com> Co-authored-by: aws-aditya21 <saivenki@amazon.com> Co-authored-by: notgitika <gitijh@gmail.com>
Description
Add optional
kmsKeyArnsupport for custom evaluators, enabling customers to specify a KMS key for evaluator encryption at rest.Changes (9 files)
src/schema/schemas/agentcore-project.ts— addedkmsKeyArntoEvaluatorSchemasrc/cli/primitives/EvaluatorPrimitive.ts— added--kms-key-arnCLI flag, updated options, action handler, andcreateEvaluatorsrc/cli/tui/screens/evaluator/types.ts— addedkms-key-arnstep andkmsKeyArntoAddEvaluatorConfigsrc/cli/tui/screens/evaluator/useAddEvaluatorWizard.ts— added KMS state, callback, wired into all wizard flowssrc/cli/tui/screens/evaluator/AddEvaluatorScreen.tsx— added KMS key ARN text input and confirm fieldsrc/cli/tui/hooks/useCreateEvaluator.ts— passeskmsKeyArnthrough to primitivesrc/cli/commands/import/import-evaluator.ts—toEvaluatorSpecforwardskmsKeyArnfrom API responsesrc/cli/aws/agentcore-control.ts— addedkmsKeyArntoGetEvaluatorResultsrc/cli/commands/import/__tests__/import-evaluator.test.ts— added tests forkmsKeyArnforwardingCompanion PR
CDK constructs: https://github.com/aws/agentcore-l3-cdk-constructs/pull/184
CFN support for
EncryptionKeyArnonAWS::BedrockAgentCore::Evaluatoris assumed ready per service team confirmation.Closes #
Type of Change
Testing
How have you tested the change?
npm run test:unitandnpm run test:integnpm run typechecknpm run lintsrc/assets/, I rannpm run test:update-snapshotsand committed the updated snapshotsChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.