fix(plugins): include memory slot plugin in primary wiki CLI scope#67063
fix(plugins): include memory slot plugin in primary wiki CLI scope#67063sahilsatralkar wants to merge 2 commits into
Conversation
Greptile SummaryThis PR fixes a false-negative wiki bridge health report where Confidence Score: 5/5Safe to merge; all findings are non-blocking style observations. The fix is logically correct: the early-return on empty pluginIds preserves existing full-loading behavior, the "none" sentinel and dedup via Set are handled properly, and the new tests cover the primary edge cases. The only observation is a minor mismatch between the PR title ("wiki scope") and the implementation (all primary scopes), which is an intentional design choice documented in the PR description but worth a code comment for future readers. No files require special attention. Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/plugins/cli-registry-loader.ts
Line: 105-119
Comment:
**Slot injection scope is broader than wiki**
`withPrimaryMemorySlotPluginId` is applied unconditionally to every non-empty primary-scoped load, not just `wiki`. Any command whose activation planner returns a non-empty list (e.g., `send`, `memory`, etc.) will also pick up the configured memory slot plugin when this path runs. The PR description acknowledges this as intentional ("primary-command scoped CLI plugin loading now appends the configured memory slot plugin id"), but the PR title ("primary wiki CLI scope") and the linked issue suggest the intent was wiki-specific. Worth a clarifying comment if general injection is the deliberate long-term direction, so future readers don't treat it as a wiki-only guard.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix(plugins): include memory slot plugin..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69dfbe4501
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 073e8d5857
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| workspaceDir: context.workspaceDir, | ||
| env: context.env, | ||
| }); | ||
| return withPrimaryMemorySlotPluginId(plannedPluginIds, context.activationSourceConfig); |
There was a problem hiding this comment.
resolvePrimaryCommandPluginIds now unconditionally appends the configured/default memory slot plugin whenever any primary command matches activation planning, so scoped loads for unrelated plugin commands (for example a non-memory primary) now include memory-core or a custom memory slot plugin. This widens command-scoped loading and makes unrelated commands pay memory-plugin startup cost or surface memory-plugin load failures even when the command does not use wiki/memory bridging; the slot append should be gated to memory-dependent primaries (or to plans that already include memory-bridge owners).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Ignoring this P2 issue for now, unless Maintainers deem it necessary
|
Adding a repro datapoint from a separate install because this looks very close to the bug I just traced on OpenClaw 2026.4.15 (macOS, npm global install). What I saw locally:
The useful runtime probe was:
So this matches the theory in this PR very closely: the wiki command path can load narrowly enough that the active memory slot plugin does not register its capability before bridge status/import asks for public artifacts. From this repro, it does not look like bad bridge config or missing files on disk. |
|
Related work from PRtags group Title: Open PR duplicate: memory-wiki CLI loads memory runtime for artifacts
|
|
Codex automated review: keeping this open. Keep this PR open. Current main still does not include the proposed primary-scope memory slot append or its regression tests, and Memory Wiki bridge/status paths still depend on an active memory public-artifact capability. The PR remains relevant to a reproduced bug, but the patch is stale and needs maintainer reconciliation with overlapping fixes, especially the open maintainer PR #72615. Best possible solution: Keep this PR open as a related implementation candidate. Rebase it against current main, either narrow the memory-slot append to memory-dependent primary scopes or adopt the Gateway RPC direction from #72615, and require regression coverage for What I checked:
Remaining risk / open question:
Codex Review notes: model gpt-5.5, reasoning high; reviewed against e962381dbf1e. |
3 participants
Summary
Describe the problem and fix in 2–5 bullets:
If this PR fixes a plugin beta-release blocker, title it
fix(<plugin-id>): beta blocker - <summary>and link the matchingBeta blocker: <plugin-name> - <summary>issue labeledbeta-blocker. Contributors cannot label PRs, so the title is the PR-side signal for maintainers and automation.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause (if applicable)
For bug fixes or regressions, explain why this happened, not just what changed. Otherwise write
N/A. If the cause is unclear, writeUnknown.Regression Test Plan (if applicable)
For bug fixes or regressions, name the smallest reliable test coverage that should catch this. Otherwise write
N/A.plugins.slots.memory plugin id.
User-visible / Behavior Changes
List user-visible changes (including defaults/config).
If none, write
None.Diagram (if applicable)
For UI changes or non-trivial logic flows, include a small ASCII diagram reviewers can scan quickly. Otherwise write
N/A.Before:
[wiki primary scoped load] -> [loads memory-wiki only] -> [no active memory runtime capability] -> [artifact count 0 + false warning]
After:
[wiki primary scoped load] -> [loads memory-wiki + configured memory slot plugin] -> [memory public artifacts registered] -> [correct count + no false warning]
Security Impact (required)
execution; mitigation is strict config-driven inclusion only when primary scope is already active, none is respected, ids are deduped/sorted, and regression tests cover scope boundaries.
Repro + Verification
Environment
memory/day-1.md, memory/.dreams/events.jsonl
Steps
Expected
Actual
Evidence
Attach at least one:
Human Verification (required)
What you personally verified (not just CI), and how:
Review Conversations
If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.
Compatibility / Migration
Risks and Mitigations
List only real risks for this PR. Add/remove entries as needed. If none, write
None.preserved; dedupe and deterministic ordering; regression tests added in src/plugins/cli.test.ts.
Built with Codex