Repository navigation
refactor(plugins): reuse immutable installed-index preparation - #140600
Merged
Merged
Conversation
Contributor
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
steipete
marked this pull request as ready for review
September 7, 2026 01:53
Contributor
|
Codex review: needs maintainer review before merge. Reviewed September 6, 2026, 11:04 PM ET / September 7, 2026, 03:04 UTC. ClawSweeper reviewWhat this changesReuse prepared plugin ownership lookups, model patterns, and parsed install records across repeated reads of an immutable installed inventory. Merge readiness✅ Ready for maintainer review The optimization remains useful: current main still repeats this preparation. No actionable introduced defect was found, and the protected maintainer PR should remain available for normal landing. Priority: P3 Review scores
Verification
How this fits togetherPlugin discovery produces an installed inventory that startup and runtime readers use to select plugins. This change reuses inventory facts while callers continue applying current configuration and policy. flowchart TD
A[Installed plugin inventory] --> B{Deeply immutable?}
B -->|Yes| C[Generation-owned prepared facts]
B -->|No| D[Prepare fresh facts]
C --> E[Caller-owned lookups and records]
D --> E
E --> F[Current configuration and policy]
F --> G[Plugin selection and loading]
Before mergeNone. Agent review detailsSecurityNone. PR surfaceSource -5, Tests +164, Docs +6. Total +165 across 9 files. View PR surface stats
Review metrics
Technical reviewBest possible solution: Keep preparation within the existing cache generation, preserve caller-owned results, and continue evaluating mutable policy at its current owner. Do we have a high-confidence way to reproduce the issue? Not applicable as a bug reproduction: this is an efficiency refactor, and current-main source confirms repeated lookup construction and install-record parsing. Is this the best way to solve the issue? Yes. Extending the existing generation-owned cache avoids a competing cache lifecycle and preserves mutable-input and caller-ownership behavior. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 8005f439db70. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (4 earlier review cycles)
|
steipete
force-pushed
the
codex/plugin-index-prepared-facts
branch
from
September 7, 2026 02:55
4c3cb67 to
f8631d6
Compare
Contributor
Author
|
Merged via squash.
|
github-actions Bot
pushed a commit
to Desicool/openclaw
that referenced
this pull request
Sep 7, 2026
…law#140600) * refactor(plugins): reuse immutable installed-index preparation * refactor(plugins): keep prepared index contracts acyclic
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What Problem This Solves
Plugin startup and scoped runtime lookups rebuild ownership maps, revisit model matching patterns, and parse the same install ledger even after the installed index belongs to an immutable metadata generation.
Why This Change Was Made
Extend the existing generation-owned installed-index fingerprint cache to retain lookup and parsed install-record facts. Consolidate private ownership-map construction while preserving distinct direct-owner, contribution-owner, and exact installed-ID contracts. Types live in the existing installed-index type owner, keeping the import graph acyclic.
Mutable management indexes remain uncached. Callers retain independent method objects and install-record results, including schema-owned nested values. Configuration, trust, request identity, and lifecycle authority remain evaluated by their existing owners.
User Impact
Repeated reads of an immutable installed inventory do less preparation work. Discovery precedence, setup/runtime modes, configuration, public SDK contracts, and persisted data are unchanged. No end-to-end latency improvement is claimed.
Evidence
node scripts/run-vitest.mjs src/plugins/installed-plugin-index-facts.test.ts src/plugins/channel-plugin-ids.test.ts src/agents/harness/runtime-plugin.test.ts— 218 tests passed. Additional discovery/manifest and loader/generation runs passed.node scripts/run-vitest.mjs test/scripts/doctor-config-preflight-plugin-index.built-cli.e2e.test.ts— both isolated stale-index startup and legacy-state restart fixtures passed. Temporary state and free ports were used, and fixture processes were cleaned up.pnpm buildpassed, including all 152 public SDK subpaths and built control-plane/CLI import checks. Madge reported zero cycles; targeted typed lint and complete staged-patch autoreview through P2 passed.node scripts/check-changed.mjspassed on Blacksmith Testboxtbx_01m1wtg15j4z5wdwqfe219tt8nwith delegated exit code 0 (backing run). The one-shot lease was stopped after success; its backing workflow can show cancellation during delegated cleanup.OPENCLAW_LOCAL_CHECK=0 node --import tsx scripts/profile-extension-memory.mts --extension memory-core --skip-combined --concurrency 1— 219.25 MiB peak RSS, 39.09 MiB empty-Node baseline, 180.16 MiB import delta, no timeout or failure. This is a candidate import profile, not a before/after Gateway resource comparison.4cf8d68eec01735eab0f5cf1ff7f0b21e74c0329; stable patch ID and all nine changed-file contents match the fully reviewed and tested implementation. Exact-head CI passed atf8631d6fd6262b815ae35220b67dedf1b8c43548.Production source: 197 additions, 202 deletions, net -5. Tests add 164 lines; docs add 6. Pure type moves contribute no net reduction. No generated files, snapshots, lockfiles, vendored code, or operational tooling are included.