feat(plugins): accept slot owner records#88507
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed June 27, 2026, 10:50 AM ET / 14:50 UTC. Summary PR surface: Source +59, Tests +312, Docs +4. Total +375 across 33 files. Reproducibility: yes. for the blocking defects: the PR diff shows duplicate imports, and source inspection shows object-form memory slots can reach a raw string comparison in Active Memory. For the broader requested behavior, this is a config-contract feature rather than a current-main bug report. Review metrics: 1 noteworthy metric.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Repair the branch so every raw slot reader that can receive Do we have a high-confidence way to reproduce the issue? Yes for the blocking defects: the PR diff shows duplicate imports, and source inspection shows object-form memory slots can reach a raw string comparison in Active Memory. For the broader requested behavior, this is a config-contract feature rather than a current-main bug report. Is this the best way to solve the issue? No. A shared read helper is a good core shape, but the current branch is incomplete because it accepts object-form slot values before all relevant raw config readers and docs are aligned. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 4010b81a77f4. Label changesLabel justifications:
Evidence reviewedPR surface: Source +59, Tests +312, Docs +4. Total +375 across 33 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
This pull request has been automatically marked as stale due to inactivity. |
Summary
This PR implements the read-side normalization layer proposed in #70823:
plugins.slots.memoryandplugins.slots.contextEnginecan now be read as either a bare plugin id string or an object-form ownership record with anownerfield.What problem does this PR solve?
plugins.slots.memory = "memory-core".plugins.slots.<slot>.owner#70823: recording slot ownership provenance without breaking existing configs.What changes?
PluginSlotOwnerRecordandPluginSlotValueconfig types.plugins.slots.memoryandplugins.slots.contextEngineaccept either"plugin-id"or{ "owner": "plugin-id", ...metadata }.resolvePluginSlotOwner(value)as the shared read-side helper for slot owner normalization.ownerfield.What is intentionally out of scope?
{ owner, claimed_at, claimed_by_version }records in this PR.--forcebehavior. Silent overwrite prevention remains a follow-up to RFC: machine-readable memory-slot ownership —plugins.slots.<slot>.owner#70823.memoryandcontextEngineslots.Intended outcome:
Linked context
Closes #70823.
Directly related work:
Supporting related PRs and issues:
Problem / Root Cause
Before this change, slot readers commonly assumed
plugins.slots.<slot>was a string. That assumption is exactly what #70823 needs to retire.The problem is not just schema shape. If only the schema accepts object-form records, but runtime paths keep reading raw values directly, then object-form configs fail in subtle places:
legacy;none;This PR makes the read-side contract real by routing those consumers through one shared helper.
What Changed
src/config/types.plugins.tsPluginSlotOwnerRecord.PluginSlotValue = string | PluginSlotOwnerRecord.plugins.slots.memoryandplugins.slots.contextEngine.src/config/zod-schema.tsowner.src/plugins/slots.tsresolvePluginSlotOwner(value).noneas the slot-disabled sentinel.Runtime/config consumers now normalize slot owners before comparison:
Documentation now states that slot values may be string ids or
{ owner: "<plugin-id>" }records.Merge-Order Compatibility
This PR is deliberately a groundwork PR, not a competing architecture.
If this lands before #86210:
PluginSlotsConfig/schema withmemory.recall,memory.compaction,memory.capture,memory.dreaming, andmemory.userModelusing the samePluginSlotValueshape.resolvePluginSlotOwner()at new role-slot read sites instead of adding new local normalization.If #86210 lands before this:
memoryandcontextEngine.If #83637 lands before or after this:
contextEngineownership where it reads that slot, without changing per-agent compaction semantics.Follow-up intended after this PR:
Real behavior proof
plugins.slots.memoryandplugins.slots.contextEnginerecords should behave the same as bare string slot owners across config parsing, validation, doctor checks, startup planning, runtime memory/context-engine resolution, provider auth trust checks, plugin lifecycle cleanup, and plugin ID migration.b87f4a710c6de665bffcc6c58245d9950a021926(feat(plugins): accept slot owner records), a final single-commit squash on top of forkmainat26913e60a42e84dd256a682c5239be57f346e244.27585fb987df8d6c68886566a6280e755a507456. The final squash preserves the intended branch content as one commit, but exact-head proof has not yet been rerun forb87f4a710c6de665bffcc6c58245d9950a021926./home/kklouzal/.openclaw/workspace/repos/openclaw; branchfeat/plugin-slot-owner-records; proof head27585fb987df8d6c68886566a6280e755a507456; local Node/Vitest/OpenClaw repo toolchain.nonebehavior, participate in context-engine and memory runtime resolution, validate missing plugin references correctly, trust selected workspace context-engine plugins for provider auth only when the normalized owner matches, reset/update selected slots during uninstall/update, and preserve extra object metadata when plugin ID migration only changesowner.plugins.slots.<slot>.owner#70823 documents the pre-change limitation: slot ownership was only machine-readable through direct string comparison, leaving no durable place for ownership provenance and forcing each consumer/plugin to repeat brittle raw-slot checks.Tests and validation
Commands run:
Regression coverage added or updated:
none.memoryandcontextEngine.contextEngine.memory.owner.Compatibility and Risk
Backward compatibility:
"none"memory-slot behavior remains valid.memory-core; context engine defaults tolegacy.Compatibility changes reviewers should inspect:
Primary risks:
plugins.slots.<slot>.owner#70823.Risk mitigation:
Current Branch State
mainb87f4a710c6de665bffcc6c58245d9950a021926feat(plugins): accept slot owner recordsmainat26913e60a42e84dd256a682c5239be57f346e244.git diff --stat origin/main...fork/feat/plugin-slot-owner-records: 33 files changed, 450 insertions, 75 deletions.git rev-list --left-right --count origin/main...fork/feat/plugin-slot-owner-records:1 1because the branch is based on the pinned green fork-main commit one commit behind upstream.fork/main.git diff --check origin/main...fork/feat/plugin-slot-owner-records: passed.