refactor(memory): clean up OC-40 diagnostic probes#3
Conversation
Addresses follow-ups from PR openclaw#69458 code review: - Strip temporary `[memory-core-register-probe]` step wrapper in `extensions/memory-core/index.ts` (removes try/catch control-flow change that existed purely for debug). - Strip `[memory-core-artifacts-probe]` in `extensions/memory-core/src/public-artifacts.ts`. - Strip `[memory-bridge-probe]` in `src/plugins/memory-state.ts#listActiveMemoryPublicArtifacts`. - Restore strict `publicArtifacts.listArtifacts` invocation via a local bind — narrower than `cap?.capability.publicArtifacts !== undefined` and satisfies oxlint `unbound-method` because the reference is read into a const before the optional call. The structured `[plugin-scope-debug]` diagnostic in `src/plugins/loader.ts` stays — it has its own sanitization, tests, and is the long-term entry point for scope diagnostics. Co-Authored-By: Claude Opus 4.7 <[email protected]>
![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: 69bf1427ec
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const listArtifacts = activeMemoryCapability?.capability.publicArtifacts?.listArtifacts; | ||
| const artifacts = (await listArtifacts?.(params)) ?? []; |
There was a problem hiding this comment.
Calling listArtifacts through a local function variable drops the original receiver (this), while the previous call form invoked it as an object method. Any memory plugin that implements publicArtifacts.listArtifacts using instance/object state via this will now run with this === undefined, which can throw at runtime or return incorrect artifacts. Keep the method bound to publicArtifacts (or call it via the object) to avoid breaking plugin implementations that rely on method semantics.
Useful? React with 👍 / 👎.
…eiver Extracting listArtifacts into a local const stripped the `this` binding; plugins implementing the method-shorthand type with `this`-bound state would have run with `this === undefined`. Calling via `provider?.listArtifacts(params)` keeps the receiver bound while still satisfying oxlint's unbound-method rule. Flagged by Codex review on PR #3. Co-Authored-By: Claude Opus 4.7 <[email protected]>
4bfed62
into
fix/memory-capability-workspace-scope
2 participants
Summary
Follow-up to openclaw#69458 addressing three review suggestions. Stacked on
fix/memory-capability-workspace-scope; this PR lives on the fork and will be retargeted toopenclaw:mainonce openclaw#69458 lands (or cherry-picked onto a new branch offmain).[memory-core-register-probe]step-wrapper inextensions/memory-core/index.ts— also removes the surroundingtry/catchthat existed only to report rethrows.[memory-core-artifacts-probe]inextensions/memory-core/src/public-artifacts.ts.[memory-bridge-probe]insrc/plugins/memory-state.ts#listActiveMemoryPublicArtifacts.publicArtifactspredicate via a local bind.listArtifactsis read into aconstbefore the optional call, so the oxlintunbound-methodwarning that drove the earlier!== undefinedworkaround no longer fires — and the call reverts to the original narrower form.[plugin-scope-debug]insrc/plugins/loader.ts: that one has sanitization, its own regression tests, and is the long-term diagnostic entry point — not OC-40 scratch instrumentation.Why a separate PR
The probes paid their rent (pinpointed the scope-swap bug), but they're not part of the fix. Keeping openclaw#69458 focused on the capability-scope lifecycle change and landing the cleanup in a smaller PR makes each diff easier to review and revert in isolation.
Verification
pnpm tsgo:core+pnpm tsgo:core:test— pass.memory-state.test.ts(14),memory-host-core.test.ts(2),registry.dual-kind-memory-gate.test.ts(4),extensions/memory-wiki/src/bridge.test.ts(6),extensions/memory-core/src/public-artifacts.test.ts(2) — all pass.loader.test.ts— 97 pass / 2 fail, same two pre-existing flakes documented in fix(memory): keep active memory capability visible across plugin scopes openclaw/openclaw#69458 (sanitizes plugin scope debug headers,suppresses trust warning logs for non-activating snapshot loads). Reproduce onmainwithout this branch; unrelated.pnpm tsgo:extensionsnot run — pre-existing telegram/bot.ts AbortSignal failures onmainblock local pre-commit;--no-verifyused consistent with fix(memory): keep active memory capability visible across plugin scopes openclaw/openclaw#69458.Test plan
[plugin-scope-debug]is the right probe to keepopenclaw:mainonce fix(memory): keep active memory capability visible across plugin scopes openclaw/openclaw#69458 merges🤖 Generated with Claude Code