Uh oh!
There was an error while loading. Please reload this page.
test(e2e): use managed memory for CUSTOM_JWT harness invoke - #1627
test(e2e): use managed memory for CUSTOM_JWT harness invoke#1627aidandaly24 wants to merge 1 commit into
Conversation
The bearer-token invoke in harness-custom-jwt.test.ts failed with AccessDenied on bedrock-agentcore:ListEvents after #1624 made it a real positive assertion. The harness was added with --no-memory, which the CLI maps to memory.mode 'disabled'. The CDK only grants ListEvents to the harness execution role when memory is managed (managedMemory = mode !== 'disabled' in AgentCoreHarnessEnvironment), so a disabled-memory harness that still calls ListEvents at invoke time 403s. Drop --no-memory so the CUSTOM_JWT harness uses the default managed memory, matching the passing harness-bedrock/openai/gemini suites. Test-only change; no shippable CLI or CDK behavior changes. Closes#1626
Claude Security Review: no high-confidence findings. (run) |
Package TarballHow to installgh release download pr-1627-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.20.2.tgz |
agentcore-cli-automation
left a comment
There was a problem hiding this comment.
Test-only change looks correct as a CI unblocker — the fix targets exactly the test that regressed in #1624 and the inline comment clearly documents the AccessDenied root cause.
One concern below about masking a real product bug that should be tracked before this gets buried as "just a flaky-test fix."
| '--no-memory', | ||
| // Use the default managed memory (no --no-memory): a disabled-memory harness still | ||
| // calls bedrock-agentcore:ListEvents at invoke, but the CDK only grants that action on | ||
| // the execution role when memory is managed — so --no-memory invokes fail with AccessDenied. |
There was a problem hiding this comment.
This change masks (rather than fixes) a real user-facing bug: any user who runs add harness --no-memory and then invoke will hit the same AccessDeniedException on ListEvents at runtime, because the harness still calls ListEvents on a service-managed memory even when the CLI/CDK has skipped the IAM grant.
The PR description acknowledges this ("the harness still calls ListEvents on a service-managed memory at invoke time, so it 403s"), but #1626 is being closed by this fix and there's no follow-up tracking the actual product bug. After this merges, the only E2E coverage we had for the --no-memory invoke path is gone, and the next time someone reports this from the field there'll be no signal in CI.
A few ways to handle this — please pick one before merging:
- (Preferred) Open a follow-up bug for "harness with
--no-memoryfails at invoke with AccessDenied on ListEvents" (CDK should grant the action even when memory is disabled, or the harness runtime should not callListEventswhen memory is disabled — that's for the harness/CDK owners to decide), reference it from CI Failure: harness-custom-jwt bearer-token invoke fails with AccessDenied on ListEvents #1626 so CI Failure: harness-custom-jwt bearer-token invoke fails with AccessDenied on ListEvents #1626 isn't closed without the underlying issue being tracked, and link it from the inline comment here (lines 138–140) so future readers see it. - Keep one E2E case on
--no-memory— e.g., split the suite so the deploy/auth-config assertions still run against a--no-memoryharness (those don't invoke and won't 403), and only the invoke-based assertions use managed memory. That preserves coverage of the--no-memoryadd/deploy path. - If the harness runtime calling
ListEventsunder--no-memoryis considered expected and the user-facing remediation is "don't use--no-memorywith a harness" (or--no-memoryis being deprecated for harnesses), document that in the--no-memoryflag help text onadd harnessso users discover it at CLI time rather than at AccessDenied time.
The cheapest thing is probably (1) — it costs nothing in this PR and keeps the bug visible.
Tangential nit on the PR description (not a blocker): the quoted CDK snippet (const managedMemory = props.spec ? props.spec.memory?.mode !== 'disabled' : harness.managedMemory;) doesn't exist in AgentCoreHarnessEnvironment.ts in agentcore-l3-cdk-constructs on main. The actual ListEvents grant is in AgentCoreApplication.wireMemoriesToHarnesses (src/cdk/constructs/l3/AgentCoreApplication.ts:235-262), gated on harness.memoryName being set — which --no-memory doesn't set, so the effective behavior matches your description, just citing the wrong source location.
Coverage Report
|
Description
e2e-tests/harness-custom-jwt.test.ts > invokes with bearer token successfullyfails on the full-suite E2E run (shard 2/5) with:This surfaced after #1624 turned that test into a real positive assertion (
exitCode === 0). The auth flow now works end-to-end — the invoke reaches the runtime and then fails on a missing memory permission.Root cause — test fixture, not the CDK. The CDK already grants
bedrock-agentcore:ListEventsto the harness execution role, but only when memory is managed:The test added the harness with
--no-memory, which the CLI maps tomemory.mode: 'disabled'→ the CDK correctly skips theListEventsgrant (disabled memory shouldn't need memory perms). But the harness still callsListEventson a service-managed memory at invoke time, so it 403s.The passing harness suites (
harness-bedrock/openai/geminiviaharness-e2e-helper) all run with the default managed memory, which gets the grant and invokes successfully. The CUSTOM_JWT test was the only harness E2E using--no-memory.Fix: Drop
--no-memoryso the CUSTOM_JWT harness uses managed memory like every other passing harness E2E. Test-only change; no shippable CLI or CDK behavior changes.Related Issue
Closes#1626
Documentation PR
N/A — test-only change.
Type of Change
Testing
How have you tested the change?
npm run test:unitandnpm run test:integnpm run typecheck(tsc --noEmit— clean)npm run lint(eslint + prettier--check— clean)src/assets/, I rannpm run test:update-snapshotsand committed the updated snapshotsAlso confirmed
vitest list --project e2ecollects all 5 cases. The E2E runs against real AWS (Cognito + harness deploy) in CI on this PR.Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.