Skip to content

fix(18-self-improving-agents): repair AgentCore Memory setup for workshop#280

Open
agent-of-mkmeral wants to merge 2 commits into
strands-agents:mainfrom
agent-of-mkmeral:fix/aim308-self-improving-agents-memory-setup
Open

fix(18-self-improving-agents): repair AgentCore Memory setup for workshop#280
agent-of-mkmeral wants to merge 2 commits into
strands-agents:mainfrom
agent-of-mkmeral:fix/aim308-self-improving-agents-memory-setup

Conversation

@agent-of-mkmeral

Copy link
Copy Markdown

Summary

Fixes the Amazon Bedrock AgentCore Memory setup for the python/01-learn/18-self-improving-agents workshop (AIM308), which is currently broken against the live AgentCore CreateMemory API. Every attendee hits a hard failure on Step 3 (memory) — which also blocks Steps 4 and 5 — because create_memory.py is missing a now-required parameter and uses a strategy/namespace shape the API no longer accepts. Also fixes a deploy-config gap and aligns the model label with the model the code actually uses.

Workshop bug context (from end-to-end testing)

Steps 0, 1, 2, 4 work flawlessly. Step 3 fails at create_memory.py:

ParamValidationError: Missing required parameter in input: "eventExpiryDuration"

…and even after adding that, the API rejects the old single-semanticMemoryStrategy shape that packed two namespaces (/facts + /preferences) into one strategy — the current API caps namespaces at 1 per strategy and allows one strategy per type.

What changed (7 files, surgical)

Bug Severity Fix
#1 create_memory.py missing required eventExpiryDuration 🔴 blocker Added eventExpiryDuration=90 (env-overridable via AIM308_EVENT_EXPIRY_DAYS) to all 3 create_memory.py copies (steps 03/04/05).
#2 strategy/namespace shape rejected 🔴 blocker Modeled facts + preferences as two strategy typessemanticMemoryStrategy (/users/{actorId}/facts) + userPreferenceMemoryStrategy (/users/{actorId}/preferences) — each with a single namespace. Satisfies the API's 1-namespace-per-strategy / 1-strategy-per-type limits while keeping both namespaces, so no agent.py changes are needed. This matches the canonical pattern in the AgentCore SDK's own Strands integration README.
#2-followup list_memories paginator key 🔴 Fixed the result key from memorySummariesmemories (matches the botocore paginator config). Kept a .get(..., .get(...)) fallback for backward compatibility.
#3 deploy.sh never injected the model ID 🟡 deploy.sh now writes BEDROCK_AGENTCORE_MODEL_ID into .env.local (default global.anthropic.claude-opus-4-8); the deployed Step-5 runtime previously got MODEL_ID=None.
#4 README / notebook model-label mismatch 🟢 Aligned labels to Claude Opus 4 (global.anthropic.claude-opus-4-8) — the model every agent.py actually hardcodes — across the main README, step-00 README, step-00-hello/agent.py docstring, and notebooks 00/01/02/03/04. (Old labels said "Claude Sonnet 4.5" / "Claude 4.6 Sonnet".)

Review Loop Summary

This PR went through 3 pre-PR review iterations (fresh-context reviewer chain, task-pr-gate Mode A) before being opened:

Iteration Actor Run Findings Addressed
1 implementor 27666225904 Built the core fix: eventExpiryDuration + two-strategy model (semantic facts + userPreference preferences) keeping BOTH namespaces; list_memories key fix; deploy.sh model-id injection; initial README/notebook label cleanup. Validated via py_compile, bash -n, and botocore ParamValidator (new params PASS, old params REJECTED with the exact eventExpiryDuration error).
2 reviewer 27666645205 Re-derived the core fix cold and CONFIRMED it (botocore model + SDK README + ParamValidator). Found & fixed 1 gap: model-label cleanup was incomplete — Sonnet 4.5/4.6 labels still in notebooks 00/01/02/04 + step-00-hello/agent.py docstring while every agent.py hardcodes claude-opus-4-8. Aligned all to Opus 4.
3 reviewer (this run) 27666957859 Re-derived everything cold one final time at the iteration cap; all checks pass, no new regression. Opened this PR.

Test Plan

All validation done at the schema + SDK-doc level (live AgentCore create blocked in CI — see caveat). Re-run cold in iteration 3:

botocore ParamValidator against the live CreateMemory input shape:

=== CreateMemory required members ===
required: ['name', 'eventExpiryDuration']
=== memoryStrategy union members ===
['semanticMemoryStrategy', 'summaryMemoryStrategy', 'userPreferenceMemoryStrategy', 'customMemoryStrategy', 'episodicMemoryStrategy']
=== NEW params validation ===  -> NONE (PASS)
=== OLD params validation ===  -> Missing required parameter in input: "eventExpiryDuration"

→ Proves eventExpiryDuration is genuinely required, the branch's new params pass, and the old params fail with the exact error attendees hit. Both semanticMemoryStrategy and userPreferenceMemoryStrategy are valid sibling union members.

SDK README cross-check (bedrock_agentcore/memory/integrations/strands/README.md): documents all three built-in strategies (summaryMemoryStrategy, userPreferenceMemoryStrategy, semanticMemoryStrategy) coexisting in one create_memory — confirming the two-strategy pattern is canonical.

list_memories paginator result key is memories (matches the fix).

Namespace consistency: retrieval_config in all three agent.py (steps 03/04/05) and notebook 03 references both /users/{actorId}/facts and /users/{actorId}/preferences, exactly matching the two namespaces create_memory.py creates.

Structural checks:

md5sum (3× create_memory.py)  -> all identical (b0d27fd7f9817a77daf7cd2babec1965)
py_compile (all .py)          -> OK
bash -n deploy.sh             -> OK
JSON-validate (all notebooks) -> OK
grep Sonnet|4.5|4.6 (code/md/ipynb) -> 0 matches
grep facts_and_preferences (old shape) -> 0 matches
model_id across all agent.py  -> consistent global.anthropic.claude-opus-4-8

⚠️ Caveat — NOT live-validated in CI

This fix has NOT been live-validated against AgentCore in this environment. The CI role (StrandsActionRole) gets a blanket ForbiddenException on all bedrock-agentcore-control operations, so a memory could not be created here. The fix is validated at the schema + SDK-documentation level only.

👉 Before the workshop, someone should run create_memory.py once with real AgentCore access to confirm the memory goes ACTIVE end-to-end. (An earlier manual e2e run in a different environment did confirm store+recall works once the memory exists.)

Remaining Findings (filed as inline review comments)

  • 🟢 The rendered assets in images/*.svg and casts/*.svg/.cast still display "Claude 4.6 Sonnet". These are pre-generated/recorded artifacts (not editable source), low priority — flagged for a future regen, not blocking.

strands-agent and others added 2 commits June 17, 2026 04:51
…shop

The step-3 memory setup (create_memory.py, shared by steps 3-5) failed
against the current AgentCore CreateMemory API, blocking steps 3, 4, and 5:

- Add required eventExpiryDuration (CreateMemory now requires it; omitting
  it raises 'Missing required parameter: eventExpiryDuration').
- Model facts and preferences as TWO strategy types (semanticMemoryStrategy
  + userPreferenceMemoryStrategy), each with a single namespace. The API
  caps a strategy at one namespace and one strategy per type, so the prior
  two-namespace single-semantic-strategy shape was rejected. This keeps the
  existing /facts and /preferences namespaces, so the three agents'
  retrieval_config stays valid (no agent.py changes needed).
- Harden list_memories pagination key (memories vs memorySummaries).

Also:
- deploy.sh now injects BEDROCK_AGENTCORE_MODEL_ID into .env.local (step-5
  agent.py reads it; without it the deployed runtime got MODEL_ID=None).
- README/notebook: align model label with the model actually used
  (Claude Opus 4 / global.anthropic.claude-opus-4-8).
The PR's stated goal strands-agents#3 was to align model labels with the actual hardcoded
MODEL_ID = global.anthropic.claude-opus-4-8 used by every agent.py. Iteration 1
only fixed 3 of the occurrences; this pass fixes the rest:
- notebooks/00,01,02,04: '# Claude Sonnet 4.5' comment -> neutral SSM-param note
  (matching the already-fixed notebooks/03_memory.ipynb)
- notebooks/00 prose: 'Claude Sonnet 4.5' -> 'Claude Opus 4'
- code/step-00-hello/agent.py docstring: 'Claude 4.6 Sonnet' -> 'Claude Opus 4'

Core memory fix from iteration 1 re-verified against botocore model + SDK README.
@agent-of-mkmeral

Copy link
Copy Markdown
Author

🟢 Remaining finding (non-blocking, not in this diff)

The rendered/pre-generated assets still display the old "Claude 4.6 Sonnet" label — these aren't editable source so they're out of scope for this surgical fix, but flagging for a future regeneration:

  • python/01-learn/18-self-improving-agents/images/hero.svg (L143): …60-minute builders session · Claude 4.6 Sonnet · Amazon Bedrock AgentCore
  • images/agent-loop.svg, images/module-1-hello.svg, images/module-7-final.svg
  • casts/00-hello.svg, casts/00-hello.cast (asciinema recording)

All source files (README, notebooks, agent.py) are aligned to Claude Opus 4 (global.anthropic.claude-opus-4-8) — the model the code actually uses. Only the baked image/cast artifacts retain the old label. Low priority; suggest regenerating the SVGs/casts in a follow-up.

@github-actions

Copy link
Copy Markdown

Latest scan for commit: adae428 | Updated: 2026-06-17 09:31:32 UTC

✅ Security Scan Report (PR Files Only)

Scanned Files

  • python/01-learn/18-self-improving-agents/README.md
  • python/01-learn/18-self-improving-agents/code/step-00-hello/README.md
  • python/01-learn/18-self-improving-agents/code/step-00-hello/agent.py
  • python/01-learn/18-self-improving-agents/code/step-03-memory/create_memory.py
  • python/01-learn/18-self-improving-agents/code/step-04-ambient/create_memory.py
  • python/01-learn/18-self-improving-agents/code/step-05-deploy/create_memory.py
  • python/01-learn/18-self-improving-agents/code/step-05-deploy/deploy.sh
  • python/01-learn/18-self-improving-agents/notebooks/00_hello_agent.ipynb
  • python/01-learn/18-self-improving-agents/notebooks/01_self_extending.ipynb
  • python/01-learn/18-self-improving-agents/notebooks/02_self_modifying.ipynb
  • ... and 2 more files

Security Scan Results

Critical High Medium Low Info
0 0 0 0 0

Threshold: High

No security issues detected in your changes. Great job!

This scan only covers files changed in this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants