fix(active-memory): expose memory tools to recall runs#74592
Conversation
|
Codex review: needs maintainer review before merge. What this changes: The PR threads embedded-run Maintainer follow-up before merge: No repair job is needed: this active contributor PR has no concrete review finding for automation to fix, and the remaining action is exact-head CI completion plus normal maintainer merge handling. Security review: Security review cleared: The diff changes runtime tool materialization but stays bounded by explicit allowlists and gateway-bindable plugin registry behavior, with no dependency, workflow, secret, publishing, or downloaded-code changes. Review detailsBest possible solution: Land this focused generic fix once exact-head checks pass, preserving the existing core/plugin boundary: runtime Do we have a high-confidence way to reproduce the issue? Yes. The linked issue gives concrete steps, and source inspection confirms the current-main path: Active Memory allowlists Memory Core tool names, Memory Core owns those tools as plugin registrations, and the embedded runner filters after tool construction without using runtime Is this the best way to solve the issue? Yes. The PR fixes the narrow root cause without hardcoding Memory Core IDs in shared core behavior, and the final embedded-run allowlist still bounds the callable tool set after plugin materialization. What I checked:
Likely related people:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 54f44ec3215d. |
|
Reviewed the current head again. The fix direction is right: carry the embedded run Still blocked on mergeability: GitHub reports Separately, I added a doctor-side warning for the related |
c29e54d to
4ba08c9
Compare
|
Landed as Review result: this fixes the Active Memory + Memory Core composition bug by preserving runtime plugin tool allowlists through the embedded attempt tool-run context, so recall runs can materialize the Memory Core plugin tools instead of ending with an empty callable set. Validation:
Thanks @vyctorbrzezowski and @LaFleurAdvertising. |
Fix Active Memory recall runs so plugin tool allowlists from composed Memory Core agents flow into embedded tool execution, restoring callable memory plugin tools during recall.\n\nCo-authored-by: vyctorbrzezowski <vyctorbrzezowski@users.noreply.github.com>
Fix Active Memory recall runs so plugin tool allowlists from composed Memory Core agents flow into embedded tool execution, restoring callable memory plugin tools during recall.\n\nCo-authored-by: vyctorbrzezowski <vyctorbrzezowski@users.noreply.github.com>
Summary
memory_searchandmemory_getshould remain callable when Memory Core is installed and Active Memory scopes the embedded run to memory tools.toolsAllownow contributes to generic plugin tool materialization, and Active Memory opts into gateway subagent binding for the embedded recall run.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause (if applicable)
toolsAllowwas not also used to materialize matching plugin tools before that final filter.memory_searchandmemory_getas plugin tools.Regression Test Plan (if applicable)
extensions/active-memory/index.test.ts,src/agents/pi-embedded-runner/run/attempt.test.ts,src/agents/pi-tools.create-openclaw-coding-tools.test.tssrc/plugins/tools.optional.test.tscovers optional plugin tools and gateway-bindable registry behavior.User-visible / Behavior Changes
Active Memory recall runs can now call Memory Core plugin tools when Memory Core is installed and those tools are explicitly allowlisted for the embedded run.
Diagram (if applicable)
Security Impact (required)
Yes/No) NoYes/No) NoYes/No) NoYes/No) YesYes/No) NoYes, explain risk + mitigation: embedded runs can materialize only plugin tools already included in the explicit runtime allowlist and gateway-bindable registry; the final allowlist filter still applies.Repro + Verification
Environment
Steps
bootstrapContextMode: "lightweight"andtoolsAllow: ["memory_recall", "memory_search", "memory_get"].Expected
memory_searchandmemory_getremain callable when registered by Memory Core and explicitly allowlisted by the embedded run.Actual
Evidence
Human Verification (required)
What you personally verified (not just CI), and how:
pnpm test extensions/active-memory/index.test.ts src/agents/pi-embedded-runner/run/attempt.test.ts src/plugins/tools.optional.test.ts src/agents/pi-tools.create-openclaw-coding-tools.test.tspnpm exec oxfmt --check --threads=1 extensions/active-memory/index.ts extensions/active-memory/index.test.ts src/agents/pi-embedded-runner/run/attempt.tool-run-context.ts src/agents/pi-embedded-runner/run/attempt.test.ts src/agents/pi-tools.ts src/agents/pi-tools.create-openclaw-coding-tools.test.ts src/agents/test-helpers/fast-openclaw-tools.ts CHANGELOG.mdgit diff --checkcodex review --base origin/maintoolsAllowis presentpnpm check:changedcurrently fails in extension test typecheck on a pre-existing duplicate-property error inextensions/memory-wiki/src/claim-health.test.tsthat is present onorigin/main.Review Conversations
Compatibility / Migration
Yes/No) YesYes/No) NoYes/No) NoRisks and Mitigations