Uh oh!
There was an error while loading. Please reload this page.
feat: dataset management — full feature with tests - #1347
Conversation
Implements complete dataset management lifecycle for AgentCore: Schema & Primitives: - DatasetPrimitive (add/remove/preview) with PREDEFINED_V1 and SIMULATED_V1 schema types - Dataset schema validation (name, schemaType, config.managed.location, kmsKeyArn) - CDK AgentCoreDataset construct (AWS::BedrockAgentCore::Dataset) CLI Commands: - `agentcore add dataset` / `agentcore remove dataset` - `agentcore dataset download` — pull from service to local file - `agentcore dataset publish-version` — publish DRAFT as immutable version - `agentcore dataset remove-version` — delete published version Operations: - Incremental push engine (diff by exampleId, batched API calls with retry) - Post-deploy sync (hash-based change detection, automatic on deploy) - Dataset-driven evaluation (load scenarios, invoke agent, collect spans) - resolve-dataset, wait, status, pull utilities TUI: - Add dataset wizard (name → schema type → description → confirm) - Dataset hub (download/publish/remove-version management) - Dataset picker in batch eval flow with version selection Bug Fixes: - Deploy preflight now recognizes datasets as deployable resources - Removed stale x-amz-expected-bucket-owner header workaround Tests: - 68 unit tests (DatasetPrimitive, AWS client, push engine, pull, publish, resolve, wait, post-deploy sync, session provider, preflight) - 17 integration tests (undeployed commands, validation, scaffolding) - 19 E2E tests (lifecycle, large batch 1000 examples, eval integration)
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Claude Security Review: no high-confidence findings. (run) |
- Replace non-existent `cliCommandRun` import with `runCliCommand` - Replace non-existent `RemovalResult` type with `Result` from lib - Fix error type mismatches (string vs Error object) across dataset primitives, TUI flows, and batch evaluation - Add `result.targetName` guard in status command to prevent undefined index - Remove unused `DEFAULT_ENDPOINT_NAME` import in get-trace.ts - Fix prettier formatting in invoke/action.ts and eval/run-eval.ts - Update asset snapshots and test assertions to match Result<T,E> type
Package TarballHow to installgh release download pr-1347-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.14.2.tgz |
Claude Security Review: the review run failed before completing. See the run for details. |
Coverage Report
|
tejaskash
left a comment
There was a problem hiding this comment.
Nice work overall — clean layering, good test coverage. A few inline notes; only the post-deploy hash one feels like it should block.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Recompute content hash after pushDataset to avoid stale hash causing redundant network calls on every deploy - Replace Promise.all with Promise.allSettled in span-collector so one failed session doesn't abort the entire eval run - Move getDataset to static import in publish.ts (no cycle to dodge) - Add AbortSignal.timeout(30s) to fetch in agentcore-datasets.ts so hung connections don't block indefinitely
Claude Security Review: the review run failed before completing. See the run for details. |
tejaskash
left a comment
There was a problem hiding this comment.
Verified the four blocking/sub-blocking comments are addressed:
- post-deploy-datasets.ts: now re-reads the file after
pushDatasetand hashes the post-push content ✓ - span-collector.ts: switched to
Promise.allSettledso one failure doesn't abort the rest ✓ - publish.ts: dynamic
getDatasetimport promoted to top-level ✓ - agentcore-datasets.ts:
fetch()now usesAbortSignal.timeout(30_000)✓
LGTM.
Claude Security Review: no high-confidence findings. (run) |
padmak30
left a comment
There was a problem hiding this comment.
Some of the back navigation is broken.
- From run -> batch evals -> eval source (dataset & version) -> evaluators and navigating back with escape takes me back to run skipping the eval source step.
- Back-navigation in AddDatasetScreen loses previously typed name.
- DatasetFlow back-nav resets datasets to [], leaving the hub permanently stuck with "No items available" after pressing Esc from any sub-screen.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
* fix(tui): back-navigation bugs in dataset and batch eval flows Three back-navigation issues reported on PR #1347: 1. BatchEvalWizard Escape skips source-picker — onExit passed the top-level exit handler instead of returning to the source-picker state. Now returns to source-picker on first-step back-nav. 2. AddDatasetScreen loses typed name — initialValue always called generateUniqueName() ignoring the stored name state. Now uses the previously submitted name when available. 3. DatasetFlow back-nav resets datasets to [] — all four sub-screen onExit/onCancel handlers hardcoded an empty array. Added loadedDatasets state that persists across flow transitions. * feat(dataset): add --kms-key-arn support to add dataset command and TUI Wire kmsKeyArn through the full add-dataset flow: - CLI: --kms-key-arn flag with ARN validation - TUI: optional step after Description (Enter to skip) - Primitive: writes kmsKeyArn to agentcore.json when provided - CDK construct already passes KmsKeyArn to CloudFormation The field is immutable after dataset creation (createOnly CFN property). * test(dataset): add kmsKeyArn unit and integration tests Cover the kmsKeyArn flow with: - Unit: DatasetPrimitive.add() persists kmsKeyArn when provided, omits when absent - Integ: CLI --kms-key-arn writes to agentcore.json, invalid ARN rejected, absent ARN omitted --------- Co-authored-by: jariy17 <tjariy+jariy17@users.noreply.github.com>
Summary
Implements complete dataset management lifecycle for AgentCore CLI:
add dataset,remove dataset,dataset download,dataset publish-version,dataset remove-versionTest plan