Skip to content

fix(templates): retrieve SUMMARIZATION summaries with an actor-scoped namespace (#665)#1660

Open
aidandaly24 wants to merge 3 commits into
aws:mainfrom
aidandaly24:fix/665
Open

fix(templates): retrieve SUMMARIZATION summaries with an actor-scoped namespace (#665)#1660
aidandaly24 wants to merge 3 commits into
aws:mainfrom
aidandaly24:fix/665

Conversation

@aidandaly24

Copy link
Copy Markdown
Contributor

Description

#665

Agents created/extended with --strategies SUMMARIZATION write per-session conversation summaries correctly, but the generated session.py retrieves them with a per-session namespace, so a new session never sees prior sessions' summaries. Cross-session summary recall silently does nothing; the feature appears write-only to users.

Fix

Re-apply PR #1299's one-line change in all three Strands session.py templates: change the SUMMARIZATION retrieval line at line 31 from f"/summaries/{actor_id}/{session_id}" to f"/summaries/{actor_id}". Run npm run test:update-snapshots to refresh the 3 affected entries in assets.snapshot.test.ts.snap (lines 2264, 3286, 6554). Update docs/memory.md:95 (manual-wiring snippet) and the strategy table at docs/memory.md:160 to /summaries/{actorId} for SUMMARIZATION. Leave the write-side namespaceTemplate (memory.ts:26) and EPISODIC ({session_id} kept intentionally; the issue is scoped to SUMMARIZATION) unchanged. Add a regression test asserting the generated SUMMARIZATION retrieval namespace contains no {session_id}, so a future squash-merge cannot silently revert it again. No SDK or CDK change needed (SDK prefix-retrieval is correct; CDK owns nothing in this path).

This fix was produced by an automated triage+fix run and validated locally (build + unit suite + symptom reproduction). Opened as a draft for CI and human review; will be bug-bashed before it's marked ready.

Related Issue

Closes #665

Documentation PR

N/A — bug fix; no devguide change required.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Other (please describe):

Testing

How have you tested the change? (full validation in PR comments after bug bash)

  • I ran npm run test:unit and npm run test:integ
  • I ran npm run typecheck
  • I ran npm run lint
  • If I modified src/assets/, I ran npm run test:update-snapshots and committed the updated snapshots

Checklist

  • I have read the CONTRIBUTING document
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.

…mespace

Cross-session SUMMARIZATION recall was silently broken: the vended Strands
session.py templates retrieved summaries with a per-session namespace
(/summaries/{actor_id}/{session_id}), which only prefix-matches the current
session and never surfaces prior sessions' summaries. Re-apply PR aws#1299's
one-line fix to use an actor-scoped namespace (/summaries/{actor_id}) in all
three templates (http, agui, a2a), regenerate the asset snapshot, update
docs/memory.md, and add a regression test to prevent another silent revert
(reverted previously by squash release aws#1547).

Fixes aws#665
@github-actions github-actions Bot added size/s PR size: S agentcore-harness-reviewing AgentCore Harness review in progress labels Jun 29, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Jun 29, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Jun 29, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.21.0.tgz

How to install

gh release download pr-1660-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.21.0.tgz

@agentcore-cli-automation agentcore-cli-automation left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. The fix correctly mirrors PR #1299: retrieval moves to the actor-scoped prefix /summaries/{actor_id} so the SDK's hierarchical prefix match surfaces summaries from prior sessions, while writes remain session-scoped via DEFAULT_STRATEGY_NAMESPACE_TEMPLATES.SUMMARIZATION (memory.ts:23) — so storage organization is preserved and only the read scope is widened. All three template flavors (http, agui, a2a) are updated consistently, the snapshots regenerate cleanly, and the docs (manual-wiring snippet + strategy table) are kept in sync.

The new regression test is well-scoped: it reads the real template files and renders them through the same global Handlebars instance the production renderer uses (registering the includes helper via the side-effect import of render.ts), with no excessive mocking. The negative assertion against /summaries/{actor_id}/{session_id} is exactly what's needed to prevent another silent revert.

No telemetry needed (template bug fix, not a new feature). The api-types.ts:158 comment example still references /summaries/{actorId}/{sessionId}, but that's the write/namespaceTemplate namespace which is intentionally unchanged, so the comment remains accurate.

@github-actions github-actions Bot removed the agentcore-harness-reviewing AgentCore Harness review in progress label Jun 29, 2026
@github-actions github-actions Bot added size/s PR size: S and removed size/s PR size: S labels Jun 29, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Jun 29, 2026
@github-actions github-actions Bot added the agentcore-harness-reviewing AgentCore Harness review in progress label Jun 29, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Jun 29, 2026
@agentcore-cli-automation

Copy link
Copy Markdown

Reviewed independently. Confirmed:

  • All three Strands flavor templates (http, agui, a2a) and the corresponding snapshots are updated consistently to actor-scoped retrieval (/summaries/{actor_id}), while the write/storage namespaceTemplate at src/schema/schemas/primitives/memory.ts:26 remains session-scoped — preserving storage organization and only widening the read prefix, exactly as PR fix: scope summaries retrieval to actor for cross-session memory recall #1299 did.
  • The new regression test at src/assets/__tests__/summarization-namespace.test.ts reads the real on-disk templates and renders them through Handlebars (registering the includes helper via the side-effect import of render.ts) with no excessive mocking. The negative assertion against /summaries/{actor_id}/{session_id} directly guards against a future silent revert.
  • Docs (docs/memory.md:95 manual-wiring snippet and the strategy table at :160) are kept in sync. The api-types.ts:147 comment example correctly refers to the write-side namespaceTemplates so it remains accurate.
  • No telemetry needed — this is a template bug fix, not a new feature.

All the concerns I would have flagged have already been covered in the existing approving review. LGTM to merge.

@github-actions github-actions Bot removed the agentcore-harness-reviewing AgentCore Harness review in progress label Jun 29, 2026
@aidandaly24 aidandaly24 marked this pull request as ready for review June 29, 2026 15:47
@aidandaly24 aidandaly24 requested a review from a team June 29, 2026 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/s PR size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Memory SUMMARIZATION strategy not incorporating summaries in subsequent sessions

2 participants