fix(harness): populate retrievalConfig from memory strategy namespaces#1374
fix(harness): populate retrievalConfig from memory strategy namespaces#1374padmak30 wants to merge 3 commits into
Conversation
Without retrievalConfig in the CreateHarness payload, the harness runtime had no instruction to retrieve from any namespace at inference time — so long-term memory was written correctly but never read back. mapMemory now accepts the resolved Memory spec and derives retrievalConfig by collecting namespaces from all strategies. computeHarnessHash now includes the Memory spec so that changes to strategy namespaces in agentcore.json trigger a redeploy even when harness.json is unchanged.
github-actions
Bot
added
the
agentcore-harness-reviewing
agentcore-devx-automation
Bot
added
the
claude-security-reviewing
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
Coverage Report
|
agentcore-cli-automation
left a comment
agentcore-cli-automation
left a comment
There was a problem hiding this comment.
Thanks for tackling this! The core fix — populating retrievalConfig from the memory's strategy namespaces and including the memory spec in the harness config hash — is on the right track and matches what the runtime needs.
A couple of things to consider before merging:
- EPISODIC
reflectionNamespacesare excluded fromretrievalConfig. This may leave the issue partially unresolved for users with EPISODIC strategies (see inline comment). - No test coverage for the redeploy-on-namespace-change behavior that the PR description calls out as a primary fix (see inline comment on
harness-deployer.ts).
Minor: the memorySpec lookup in harness-deployer.ts only matches by memory.name, so harnesses that reference memory by arn won't get a retrievalConfig derived. This is probably acceptable as a known limitation (we can't introspect external memories), but worth confirming the intent.
github-actions
Bot
removed
the
agentcore-harness-reviewing
buildRetrievalConfig now flattens reflectionNamespaces alongside namespaces for EPISODIC strategies, so cross-session reflection summaries stored at the parent namespace path are retrieved at inference time. Also adds a unit test asserting that mutating strategies[*].namespaces in agentcore.json produces a different configHash and triggers a harness update rather than being silently skipped.
agentcore-devx-automation
Bot
added
the
claude-security-reviewing
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
|
|
||
| const deployedResources = deployedState.targets?.[targetName]?.resources; | ||
| const existingHarness = deployedHarnesses[entry.name]; | ||
| const memorySpec = projectSpec.memories?.find(m => m.name === harnessSpec.memory?.name); |
There was a problem hiding this comment.
Minor / low priority — flagging for awareness rather than as a blocker.
The spec lookup keys solely on harnessSpec.memory?.name. HarnessMemoryRefSchema (src/schema/schemas/primitives/harness.ts:188) makes both name and arn optional, and mapMemory already has an if (memory.arn) branch (harness-mapper.ts:280). For an arn-only ref, find(m => m.name === undefined) returns undefined and the harness ships without retrievalConfig.
That said, no CLI surface produces this shape — agentcore add harness always writes { name: ... }, and there's no --memory-arn flag. The only way to land here is hand-editing harness.json. So this is theoretical for any first-party flow.
Two reasonable resolutions:
- Tighten
HarnessMemoryRefSchemato requirename(drop thearnbranch inmapMemoryalong with it), so the schema reflects what the tooling actually supports. - Leave as-is and add a test asserting that the arn-only branch intentionally omits
retrievalConfig, so a future contributor doesn't read it as a regression.
Either is fine — feel free to defer or close as out-of-scope.
Includes EPISODIC reflectionNamespaces in the retrieval config so the harness runtime searches all relevant memory namespaces at inference time. Also incorporates memorySpec into the deploy hash so namespace changes trigger a harness update. Cherry-picked from #1374.
…refs When a harness references memory by ARN only (no name field), the previous lookup returned undefined, silently omitting retrievalConfig and excluding the memory from the configHash. resolveMemorySpec now walks deployedResources.memories to match by ARN and find the corresponding projectSpec memory, so name-less refs that point at a CLI-managed memory get the same treatment as name-based refs. HarnessMemoryRef is exported from the schema barrel so it can be used as the explicit parameter type on resolveMemorySpec. Adds unit tests for both the ARN-match path and the intentional undefined fallback for genuinely external memories.
agentcore-devx-automation
Bot
added
the
claude-security-reviewing
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
3 participants
Description
Without retrievalConfig in the CreateHarness payload, the harness runtime had no instruction to retrieve from any namespace at inference time — so long-term memory was written correctly but never read back.
mapMemory now accepts the resolved Memory spec and derives retrievalConfig by collecting namespaces from all strategies.
computeHarnessHash now includes the Memory spec so that changes to strategy namespaces in agentcore.json trigger a redeploy even when harness.json is unchanged.
Related Issue
Closes # #1097
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.