perf(plugins): extend discovery threading to loader, manifest registry, installed-index, and config contracts by RomneyDa · Pull Request #84283 · openclaw/openclaw

RomneyDa · 2026-05-19T19:38:50Z

Summary

Follow-up to #75451. Adds optional discovery?: PluginDiscoveryResult to the remaining src/plugins/ helpers that still call discoverOpenClawPlugins internally during startup. Pure plumbing — every change is additive, so existing callers are unaffected. Re-submit of #84258 (merged then reverted in #84278) so CI can run to completion before merge.

Changes

File	Change
`loader.ts`	`discovery?` on `PluginLoadOptions`; both `loadOpenClawPlugins` and `loadOpenClawPluginCliRegistry` consult it before falling back to an internal scan.
`manifest-registry.ts`	`discovery?` on `loadPluginManifestRegistry`; used when `candidates?` is not supplied.
`installed-plugin-index-types.ts` + `installed-plugin-index-registry.ts`	`discovery?` on `LoadInstalledPluginIndexParams`; consumed by `resolveInstalledPluginIndexRegistry` when `candidates?` is not supplied.
`config-contracts.ts`	`discovery?` on `resolvePluginConfigContractsById`; threaded into the bundled-fallback closure.
`discovery-threading.test.ts` (new)	6 tests asserting each entry point skips internal discovery when supplied, calls it when not, and prefers explicit `candidates?` over `discovery?` when both are present.

Motivation

TUI startup profiled at R+ 101% CPU doing 212,119 sync JSON reads against 379 unique paths (~1,500× per bundled extension's package.json). Evidence on #75451.

This PR is plumbing only — nothing in this diff passes a shared discovery yet. The user-visible win requires a separate orchestrator-level follow-up that computes one discovery per CLI/TUI/gateway flow and threads it through. Larger amplifiers (inside-discovery manifest re-reads, require()-driven package.json walks during plugin module load) are outside this approach entirely.

Test plan

node scripts/run-vitest.mjs src/plugins/discovery-threading.test.ts — 6/6 pass
pnpm tsgo --noEmit --project tsconfig.core.json — clean
node scripts/run-oxlint.mjs --quiet on touched files — 0 warnings, 0 errors
npx oxfmt --check — clean
Full CI to complete (the missing step from perf(plugins): extend discovery threading to loader, manifest registry, installed-index, and config contracts #84258).

Pre-existing failures in src/plugins/manifest-registry.test.ts reproduce on clean main without this PR's changes — unrelated.

…y, installed-index, and config contracts Follow-up to #75451. Threads optional discovery?: PluginDiscoveryResult through the remaining helpers that still call discoverOpenClawPlugins internally during startup: - loadOpenClawPlugins / loadOpenClawPluginCliRegistry (src/plugins/loader.ts): add discovery? to PluginLoadOptions and consult it before falling back to an internal scan at both call sites. - loadPluginManifestRegistry (src/plugins/manifest-registry.ts): accept discovery? as a more ergonomic alternative to the existing candidates? / diagnostics? pair; candidates? still wins when both are supplied. - resolveInstalledPluginIndexRegistry (src/plugins/installed-plugin-index-registry.ts): add discovery? to LoadInstalledPluginIndexParams and use it when candidates aren't supplied. - resolvePluginConfigContractsById (src/plugins/config-contracts.ts): add discovery? and thread it into the bundled-fallback discovery call. Add discovery-threading.test.ts asserting each entry point skips its internal discoverOpenClawPlugins call when discovery is supplied, calls it when nothing is supplied, and prefers explicit candidates over discovery when both are present (6 tests, all pass). discoverOpenClawPlugins remains stateless; sharing is function-scoped per src/plugins/CLAUDE.md guidance. Backward compatible: every change is additive (new optional param).

clawsweeper · 2026-05-19T19:40:27Z

Codex review: needs maintainer review before merge.

Workflow note: Future ClawSweeper reviews update this same comment in place.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Summary
The PR adds optional shared plugin discovery parameters to plugin loader, manifest registry, installed-index, and config-contract helper paths, plus focused discovery-threading tests.

Reproducibility: yes. at source level: current main still has direct discoverOpenClawPlugins calls in the helper paths this PR targets. I did not run runtime profiling or tests because this review was read-only.

PR rating
Overall: 🐚 platinum hermit
Proof: 🌊 off-meta tidepool
Patch quality: 🐚 platinum hermit
Summary: Small additive plumbing with no blocking findings; confidence is mostly limited by read-only validation and the lack of direct tests for two touched helper paths.

Rank-up moves:

none

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

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.

PR egg
✨ Hatched: 🥚 common Pearl Test Hopper

       /\  .---.  /\         
      /  \/     \/  \        
     /   ( -   - )   \       
    |       ._.       |      
    |   /|  ===  |\   |      
     \  \|______/|/  /       
      '._  `--'  _.'         
         '-.__.-'            
       _/|_|  |_|\_          
      /__|      |__\         
       .-----------.         
      '-------------'

Rarity: 🥚 common.
Trait: stacks clean commits.
Image traits: location workflow harbor; accessory shell-shaped keyboard; palette cobalt, lime, and pearl; mood bright-eyed; pose curling around a status light; shell brushed metal shell; lighting gentle morning glow; background miniature CI buoys.
How to hatch it: once this PR reaches status: 👀 ready for maintainer look or status: 🚀 automerge armed, the PR author or a maintainer can comment @clawsweeper hatch to turn this ASCII egg into its generated creature image.
Share on X: post this hatch
Copy: My PR egg hatched a 🥚 common Pearl Test Hopper in ClawSweeper.

What is this egg doing here?

Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
Hatchable usually means sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness.
The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

Real behavior proof
Not applicable: The contributor proof gate does not apply to this MEMBER maintainer-labeled PR; the body includes local test/type/lint/format proof but no end-to-end startup timing proof.

Risk before merge
Why this matters: - I did not run the PR's test plan in this read-only review; merge should rely on CI or maintainer-run focused test/type/lint/format proof.

The added test file directly exercises manifest-registry and installed-index threading, while loader and config-contract threading are supported by source review and typecheck rather than direct focused tests.
This plumbing does not itself prove a TUI startup timing improvement; the user-visible performance win depends on a follow-up that computes and threads one discovery per startup flow.

Maintainer options:

Decide the mitigation before merge
Land the additive helper-level discovery threading after CI/maintainer review, keeping orchestrator-level sharing and deeper read-count fixes in their separate follow-up PRs.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge
No automated repair is indicated; this is a protected maintainer-labeled PR with no concrete code defect found, so the remaining action is maintainer review plus CI/profiling judgment.

Security
Cleared: The diff only touches internal TypeScript plugin helper plumbing and a unit test, with no dependency, CI, secret, install, or release-surface changes found.

Review details

Best possible solution:

Land the additive helper-level discovery threading after CI/maintainer review, keeping orchestrator-level sharing and deeper read-count fixes in their separate follow-up PRs.

Do we have a high-confidence way to reproduce the issue?

Yes, at source level: current main still has direct discoverOpenClawPlugins calls in the helper paths this PR targets. I did not run runtime profiling or tests because this review was read-only.

Is this the best way to solve the issue?

Yes for the helper layer: optional caller-supplied discovery matches the existing function-scoped pattern and avoids persistent caches. It is intentionally not the full startup-performance fix, which belongs in the separate orchestration and inner-scan follow-ups.

Label justifications:

P2: This is a normal-priority plugin startup performance improvement with limited blast radius and no emergency runtime failure shown by the diff.

What I checked:

PR head loader plumbing: The PR head adds discovery?: PluginDiscoveryResult to PluginLoadOptions and uses options.discovery before falling back to discoverOpenClawPlugins in both loadOpenClawPlugins and loadOpenClawPluginCliRegistry. (src/plugins/loader.ts:205, 6b6254db8fd3)
PR head registry plumbing: The PR head preserves the existing candidates-first path and otherwise lets loadPluginManifestRegistry use a supplied discovery result instead of doing its own discovery scan. (src/plugins/manifest-registry.ts:923, 6b6254db8fd3)
PR head installed-index plumbing: The PR head adds discovery?: PluginDiscoveryResult to LoadInstalledPluginIndexParams and uses it in resolveInstalledPluginIndexRegistry when explicit candidates are not supplied. (src/plugins/installed-plugin-index-registry.ts:28, 6b6254db8fd3)
PR head config-contract fallback: The PR head threads the optional discovery result into bundled config-contract fallback discovery, still filtering to bundled candidates before loading the manifest registry. (src/plugins/config-contracts.ts:117, 6b6254db8fd3)
Focused test coverage: The added discovery-threading test covers manifest registry and installed-index skip/fallback/candidates-precedence behavior; loader and config-contract paths remain covered by source review and typechecking rather than direct focused assertions in this file. (src/plugins/discovery-threading.test.ts:26, 6b6254db8fd3)
Runtime-loader fast path unchanged on current main: Current main still returns before loading when an active registry satisfies the requested scope, and the PR diff has no runtime-registry-loader changes, so the prior eager-runtime-discovery finding does not apply to this head. (src/plugins/runtime/runtime-registry-loader.ts:159, a059309a9f9a)

Likely related people:

RomneyDa: The current main blame for the reverted discovery-call lines and the prior merged/reverted attempt both point to the same recent loader/registry performance work. (role: recent area contributor; confidence: high; commits: f5f0b2c7c9e0, 3d96111a5afe; files: src/plugins/loader.ts, src/plugins/manifest-registry.ts, src/plugins/installed-plugin-index-registry.ts)
SebTardif: The merged predecessor PR added the function-scoped explicit-discovery pattern that this PR extends to remaining plugin helpers. (role: adjacent pattern introducer; confidence: high; commits: 28beea9e881c; files: src/plugins/bundled-capability-runtime.ts, src/plugins/bundled-sources.ts, src/plugins/channel-catalog-registry.ts)
Galin Iliev: The shallow current-main blame shows the surrounding loader, manifest registry, installed-index, and config-contract implementations carried by the grafted current-main commit, so this is an adjacent routing signal rather than precise authorship. (role: current code carrier; confidence: medium; commits: 57ec361682e2; files: src/plugins/loader.ts, src/plugins/manifest-registry.ts, src/plugins/installed-plugin-index-registry.ts)

Codex review notes: model gpt-5.5, reasoning high; reviewed against a059309a9f9a.

RomneyDa · 2026-05-19T22:04:52Z

@clawsweeper re-review

clawsweeper · 2026-05-19T22:06:54Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

State: Complete
Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
Run: https://github.com/openclaw/clawsweeper/actions/runs/26128220743
Updated: 2026-05-19T22:13:35.715Z

RomneyDa · 2026-05-19T23:21:22Z

Re: the prior ClawSweeper review on head 8b4574931e — both P2 findings were resolved by force-pushing the branch to drop the 2 commits that introduced them, returning the PR to the pure-plumbing scope its body advertises.

P2: eager discovery scan in runtime-registry-loader.ts:142
Dropped commit 476e29b76f ("orchestrate shared discovery at runtime registry loader"). That commit added an unconditional sharedDiscovery = discoverOpenClawPlugins(...) at the top of ensurePluginRegistryLoaded, before the active-registry early-returns — regressing the scoped/already-loaded fast path from 0 scans to 1 per call. The lazy runtime-loader threading belongs in a separate follow-up.

P2: dead discovery? param in channel-presence-policy.ts:454
Dropped commit 8b4574931e ("thread shared discovery through scope resolvers"). The discovery? param was accepted by resolveScopedChannelOwnerPluginIds but never forwarded into loadInstalledChannelManifestRecords → loadPluginManifestRegistryForPluginRegistry, so the configured-channel / onlyChannelIds flows would have paid both the eager scan and the old manifest scan. Honoring that param requires plumbing further down — out of scope here.

Follow-up cleanup (6b6254db8f)
Dropped four verbose JSDoc blocks on the new discovery? fields; the field name plus surrounding context already explained them.

Current head 6b6254db8f is now equivalent (modulo the JSDoc removal) to the previously-merged f5f0b2c7c9 (#84258) which already passed CI before its revert. The lazy runtime-loader discovery, proper channel-scope threading, and the inner package.json manifest-cache amplifier all live in #84302, which is now stacked on this branch with the P1 cache-trust-key concern from its own ClawSweeper review fixed.

…y, installed-index, and config contracts (openclaw#84283) * perf(plugins): extend discovery threading to loader, manifest registry, installed-index, and config contracts Follow-up to openclaw#75451. Threads optional discovery?: PluginDiscoveryResult through the remaining helpers that still call discoverOpenClawPlugins internally during startup: - loadOpenClawPlugins / loadOpenClawPluginCliRegistry (src/plugins/loader.ts): add discovery? to PluginLoadOptions and consult it before falling back to an internal scan at both call sites. - loadPluginManifestRegistry (src/plugins/manifest-registry.ts): accept discovery? as a more ergonomic alternative to the existing candidates? / diagnostics? pair; candidates? still wins when both are supplied. - resolveInstalledPluginIndexRegistry (src/plugins/installed-plugin-index-registry.ts): add discovery? to LoadInstalledPluginIndexParams and use it when candidates aren't supplied. - resolvePluginConfigContractsById (src/plugins/config-contracts.ts): add discovery? and thread it into the bundled-fallback discovery call. Add discovery-threading.test.ts asserting each entry point skips its internal discoverOpenClawPlugins call when discovery is supplied, calls it when nothing is supplied, and prefers explicit candidates over discovery when both are present (6 tests, all pass). discoverOpenClawPlugins remains stateless; sharing is function-scoped per src/plugins/CLAUDE.md guidance. Backward compatible: every change is additive (new optional param). * perf(plugins): drop verbose JSDoc from discovery? params

openclaw-barnacle Bot added size: S maintainer Maintainer-authored PR labels May 19, 2026

This was referenced May 19, 2026

perf(plugins): scan-scoped package.json cache in discovery #84302

Merged

perf(plugins): memoize isOfficialInstalledCodexPluginModulePath by modulePath #84318

Closed

RomneyDa force-pushed the dallin-perf-discovery-threading-pt2 branch from 8b45749 to 87fef16 Compare May 19, 2026 21:59

RomneyDa mentioned this pull request May 19, 2026

perf(plugins): memoize installed plugin index across redundant startup callers #84339

Closed

perf(plugins): drop verbose JSDoc from discovery? params

6b6254d

RomneyDa mentioned this pull request May 19, 2026

perf(infra): cache sync JSON reads with stat validation #84351

Closed

RomneyDa merged commit 88d8d6a into main May 19, 2026
118 of 122 checks passed

RomneyDa deleted the dallin-perf-discovery-threading-pt2 branch May 19, 2026 23:22

github-actions Bot mentioned this pull request May 20, 2026

📡 Upstream Digest — 2026-05-20 02:34 UTC curtismercier/openclaw-mods#901

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

perf(plugins): extend discovery threading to loader, manifest registry, installed-index, and config contracts#84283

perf(plugins): extend discovery threading to loader, manifest registry, installed-index, and config contracts#84283
RomneyDa merged 2 commits into
mainfrom
dallin-perf-discovery-threading-pt2

RomneyDa commented May 19, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented May 19, 2026 •

edited

Loading

Uh oh!

RomneyDa commented May 19, 2026

Uh oh!

clawsweeper Bot commented May 19, 2026 •

edited

Loading

Uh oh!

RomneyDa commented May 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

RomneyDa commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Motivation

Test plan

Uh oh!

clawsweeper Bot commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RomneyDa commented May 19, 2026

Uh oh!

clawsweeper Bot commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RomneyDa commented May 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

RomneyDa commented May 19, 2026 •

edited

Loading

clawsweeper Bot commented May 19, 2026 •

edited

Loading

clawsweeper Bot commented May 19, 2026 •

edited

Loading