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

RomneyDa · 2026-05-19T18:33:05Z

Summary

Follow-up to #75451. Extends the function-scoped discovery?: PluginDiscoveryResult threading pattern to the remaining four plugin subsystems that still call discoverOpenClawPlugins internally during startup, plus tests asserting the wiring.

discoverOpenClawPlugins remains stateless. Sharing is function-scoped per src/plugins/CLAUDE.md guidance. Every change is additive (a new optional param) and backward compatible — existing callers are unaffected.

What this PR adds

File	Change
`src/plugins/loader.ts`	Added `discovery?` to `PluginLoadOptions`; both call sites (`loadOpenClawPlugins` and `loadOpenClawPluginCliRegistry`) consult it before doing their own scan.
`src/plugins/manifest-registry.ts`	Added `discovery?` to `loadPluginManifestRegistry` params; used when `candidates?` is not supplied.
`src/plugins/installed-plugin-index-types.ts` + `installed-plugin-index-registry.ts`	Added `discovery?` to `LoadInstalledPluginIndexParams`; `resolveInstalledPluginIndexRegistry` uses it when `candidates?` is not supplied.
`src/plugins/config-contracts.ts`	Added `discovery?` to `resolvePluginConfigContractsById`; threaded into the bundled-fallback closure.
`src/plugins/discovery-threading.test.ts` (new)	6 tests asserting each entry point: (a) skips its internal `discoverOpenClawPlugins` call when `discovery` is supplied, (b) calls it when nothing is supplied, (c) prefers explicit `candidates?` over `discovery?` when both are present.

#75451 covered bundled-capability-runtime, bundled-sources, channel-catalog-registry, and the contracts/registry retry loop. With this PR, all known discoverOpenClawPlugins callers in src/plugins/ accept an optional shared discovery snapshot. Top-level orchestrators can compute one discovery per CLI/TUI/gateway flow and pass it through.

Context: what we measured before #75451 landed

The freeze that motivated this work is a 25–30s TUI startup with the process at R+ 101% CPU, doing 212,119 synchronous JSON reads against only 379 unique paths per launch — each bundled extension's package.json re-read ~1,500 times. CPU profile Bottom-Up was dominated by lstat (24%), open (13%), realpathSync (13% total), with jiti source-transform and GC making up the rest. An event-loop heartbeat probe showed the JS event loop blocked for the full freeze duration. The detailed evidence is on #75451: #75451 (comment)

This PR is necessary but not sufficient to close the freeze on its own. It is preparatory plumbing that lets callers actually realize the sharing. A separate follow-up should:

Identify the top-level orchestration sites where TUI / CLI / gateway startup invokes multiple of these helpers in sequence.
Compute one PluginDiscoveryResult there and thread it through each call.
Investigate the inside-discovery amplifier (~1,500× per-manifest reads inside one scan) and Node require()-driven package.json walks during plugin module loading — those are larger costs than the outer scan dedup addressed here.

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

Verified the 5 pre-existing failures in src/plugins/manifest-registry.test.ts ("suppresses duplicate warnings..." etc.) reproduce on clean main without this PR's changes — unrelated regression.

Behavior addressed: each of the four remaining plugin subsystem helpers in src/plugins/ independently re-invokes discoverOpenClawPlugins during the same startup flow when not explicitly handed a shared discovery snapshot. After #75451 only three helpers + one retry loop had the option to skip the redundant scan; with this PR, all known call sites can.
Real environment tested: macOS, Node 22.22, source checkout. Same setup that produced the 212K-read profile referenced above.
Exact steps or command run after this patch: node scripts/run-vitest.mjs src/plugins/discovery-threading.test.ts; pnpm tsgo --noEmit --project tsconfig.core.json; node scripts/run-oxlint.mjs --quiet src/plugins/{loader,manifest-registry,installed-plugin-index-registry,installed-plugin-index-types,config-contracts,discovery-threading.test}.ts; npx oxfmt --check.
Evidence after fix: 6 new tests pass; lint/format/typecheck clean.
Observed result after fix: each entry point demonstrably skips its internal discovery call when discovery is supplied (asserted by the new test suite via a vi.mock spy on discoverOpenClawPlugins).
What was not tested: end-to-end TUI startup timing on this PR alone. The sharing is preparatory — orchestrator-level threading that supplies the same discovery to multiple helpers in one flow is a separate follow-up. This PR is the structural prerequisite for that work.

clawsweeper · 2026-05-19T18:34:24Z

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 PluginDiscoveryResult threading to bundled plugin runtime, bundled-source, and channel-catalog helpers, then reuses one discovery result across the scoped capability runtime retry loop.

Reproducibility: Do we have a high-confidence way to reproduce the issue? Yes for the redundant discovery shape: current main source shows independent discoverOpenClawPlugins calls in the affected helpers and a retry loop that calls the bundled runtime loader twice. I did not run the TUI startup profiler in this read-only review.

PR rating
Overall: 🦐 gold shrimp
Proof: 🦐 gold shrimp
Patch quality: 🐚 platinum hermit
Summary: The patch is narrow and aligned with the current plugin architecture, but runtime performance proof on the refreshed branch is limited to structural source evidence and reported focused checks.

Rank-up moves:

Add a before/after TUI or gateway startup profile on this branch if maintainers want quantified performance proof before merge.

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 Brave Branchling

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

Rarity: 🥚 common.
Trait: sparkles near resolved comments.
Share on X: post this hatch
Copy: My PR egg hatched a 🥚 common Brave Branchling 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.
How to hatch it: reach status: 👀 ready for maintainer look or status: 🚀 automerge armed; that 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 external contributor proof gate is not applied because this is a MEMBER PR with the maintainer label; the PR body still notes that end-to-end TUI timing was not measured on this branch.

Risk before merge
Why this matters: - The PR body reports focused type, lint, format, and unit checks, but does not include a before/after TUI startup timing run on this refreshed branch, so the performance win is structurally supported rather than quantified here.

Maintainer options:

Decide the mitigation before merge
Review and land the explicit function-scoped discovery-threading change if maintainers accept the scoped performance proof, then track broader startup costs such as loader discovery, manifest reads, and jiti load paths as follow-up work.
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; the remaining action is maintainer review of the scoped performance fix and any desired startup profiling proof.

Security
Cleared: The current diff only threads an in-memory discovery result through plugin helper calls and does not add dependencies, CI changes, secret handling, downloaded code, or broader filesystem permissions.

Review details

Best possible solution:

Review and land the explicit function-scoped discovery-threading change if maintainers accept the scoped performance proof, then track broader startup costs such as loader discovery, manifest reads, and jiti load paths as follow-up work.

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

Do we have a high-confidence way to reproduce the issue? Yes for the redundant discovery shape: current main source shows independent discoverOpenClawPlugins calls in the affected helpers and a retry loop that calls the bundled runtime loader twice. I did not run the TUI startup profiler in this read-only review.

Is this the best way to solve the issue?

Is this the best way to solve the issue? Yes for the narrow issue: explicit per-flow discovery threading matches the plugin boundary guidance better than a hidden low-level JSON or metadata cache. The remaining startup freeze sources called out in the PR should be handled as separate follow-ups.

Label justifications:

P2: This is a bounded plugin startup performance improvement with limited blast radius and no confirmed blocking correctness defect in the current diff.

Acceptance criteria:

git diff --check 9b97e1e 1c9a8d9
node scripts/run-vitest.mjs src/plugins/bundled-capability-runtime.test.ts src/plugins/bundled-sources.test.ts src/plugins/channel-catalog-registry.test.ts
pnpm tsgo --noEmit --project tsconfig.core.json
node scripts/run-oxlint.mjs --quiet src/plugins/bundled-capability-runtime.ts src/plugins/bundled-sources.ts src/plugins/channel-catalog-registry.ts src/plugins/contracts/registry.ts
npx oxfmt --check src/plugins/bundled-capability-runtime.ts src/plugins/bundled-sources.ts src/plugins/channel-catalog-registry.ts src/plugins/contracts/registry.ts

What I checked:

Protected maintainer workflow state: The provided live GitHub context shows authorAssociation MEMBER and the maintainer label, so this cleanup review should not auto-close the PR and should leave explicit maintainer handling in place.
Current merge effect is limited to four plugin files: The synthetic merge commit changes only bundled capability runtime, bundled sources, channel catalog registry, and contract registry files, matching the current PR body and pullFiles context. (1c9a8d9cf942)
PR code uses function-scoped discovery sharing: The merged PR version creates one discovery result inside loadScopedCapabilityRuntimeRegistryEntries and passes it to loadBundledCapabilityRuntimeRegistry for both retry attempts. (src/plugins/contracts/registry.ts:259, 1c9a8d9cf942)
Plugin boundary guidance supports explicit per-flow ownership: The scoped plugin guide allows metadata to stay fresh unless a caller owns an explicit snapshot, lookup table, or manifest registry for the current flow, and forbids persistent metadata caches; this PR follows the explicit current-flow shape. (src/plugins/AGENTS.md:22, 9b97e1ef2fd2)
Current main still has independent discovery calls: Current main calls discoverOpenClawPlugins directly inside the three helpers and inside the retry loop path, so the PR is not already implemented on main. (src/plugins/bundled-capability-runtime.ts:235, 9b97e1ef2fd2)
Related issue defines the same fix shape: The related issue [Perf]: Redundant filesystem walks in plugin discovery during startup #82308 asks for callers that already hold PluginDiscoveryResult to pass it through while keeping discoverOpenClawPlugins stateless; the current PR implements that central approach.

Likely related people:

Vincent Koc: Recent commits touched contract registry fast paths, runtime boot trimming, and bundled capability runtime cache behavior in the same runtime-loading area. (role: recent area contributor; confidence: high; commits: 48c4a026dd65, 8b5030447a44, 60ea8e9a1cd8; files: src/plugins/contracts/registry.ts, src/plugins/contracts/registry.retry.test.ts, src/plugins/bundled-capability-runtime.ts)
Ayaan Zaidi: Recent plugin discovery history includes a discovery cache refactor in src/plugins/discovery.ts and its tests, which is adjacent to the discovery freshness and sharing boundary this PR touches. (role: recent discovery contributor; confidence: medium; commits: 9da4d5f5df5d; files: src/plugins/discovery.ts, src/plugins/discovery.test.ts)
SebTardif: The related closed PR authored the same function-scoped discovery-threading approach and is explicitly credited by this PR as the refreshed implementation source. (role: prior implementation author; confidence: high; commits: 44991371bda5; files: src/plugins/bundled-capability-runtime.ts, src/plugins/bundled-sources.ts, src/plugins/channel-catalog-registry.ts)

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

RomneyDa · 2026-05-19T18:35:38Z

Existing work in this area — for reviewer awareness

After opening this PR I searched for related issues/PRs and found significant existing work that reviewers should consider:

Issue #82308 — "[Perf]: Redundant filesystem walks in plugin discovery during startup" (open, May 15 2026)

Direct match for the same root cause. The issue states an explicit design constraint which conflicts with the approach in this PR:

discoverOpenClawPlugins() must remain stateless with no hidden cache. Discovery sharing should be function-scoped, not module-level.

A ClawSweeper review on the issue reinforces:

The plugin boundary guide says metadata should stay fresh unless a caller owns an explicit snapshot or registry for the current flow, and it rejects persistent discovery caches.

This PR's Map<absolutePath, parsedValue> is at the fs reader layer (one level below discovery), not the discovery layer itself — but because plugin manifests are among the files being read, it effectively functions as a manifest-content cache, which the design constraint appears to forbid.

PR #75451 — "perf: thread explicit plugin discovery through contracts registry" (open, May 1 2026, author @SebTardif)

The architecturally-aligned fix for #82308. Threads an optional discovery?: PluginDiscoveryResult parameter through four functions and lets the retry loop in contracts/registry.ts share one walk across attempts. 22 additions / 15 deletions — narrow and surgical. Closes #82308.

How these relate to this PR

	#75451	This PR (#84258)
Layer	Function-scoped threading in plugin discovery callers	Process-scoped memo at the `tryReadJsonSync` fs reader
Aligned with `src/plugins/CLAUDE.md` "no hidden cache" guidance	Yes	No (likely conflicts)
Scope of fix	4 specific callers (`bundled-capability-runtime`, `bundled-sources`, `channel-catalog-registry`, `contracts/registry`)	All sync JSON reads in-process
Symptom coverage	Targeted	Broad — catches any caller that re-reads manifests
Risk	Low (additive optional param)	Higher (changes manifest-read semantics if cached value becomes stale)

Other related-looking reports that may share this root cause:

[Regression]: OpenClaw 2026.5.12 is dramatically slower than 2026.5.7 (QQBot / status / overall responsiveness) #82264 "[Regression]: OpenClaw 2026.5.12 is dramatically slower than 2026.5.7"
[Bug]: OpenClaw on native Windows getting notably slower and slower with each new version??? #76884 "OpenClaw on native Windows getting notably slower and slower with each new version???"

Recommendation for reviewers

Given the design-constraint conflict, I'd defer to maintainers on the right path:

Prefer perf: thread explicit plugin discovery through contracts registry #75451 (architecturally correct, matches the issue's stated constraint) and close this PR — would still leave the loader's own discovery calls and any other untouched callers re-reading manifests, but is the cleaner direction.
Land both, treating this PR as a defense-in-depth hotfix that protects against any caller (including future ones) that bypasses snapshot sharing. The OPENCLAW_DISABLE_JSON_READ_CACHE=1 escape hatch and clearJsonReadCache() API exist for hot-reload scenarios that need fresh reads.
Reject this PR as conflicting with src/plugins/CLAUDE.md's metadata-cache prohibition; rely on perf: thread explicit plugin discovery through contracts registry #75451 plus future per-caller threading.

I have a working CPU profile and per-call instrumentation that reproduces the 212K-reads-from-380-paths pattern from the PR description, including breakdowns of which files are re-read most. Happy to share that data if it helps the decision.

…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).

…y, installed-index, and config contracts (openclaw#84258) 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).

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

clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels May 19, 2026

RomneyDa force-pushed the dallin-performance-audit branch from 84fb71f to d6b8f46 Compare May 19, 2026 18:59

openclaw-barnacle Bot added size: XS and removed size: S labels May 19, 2026

RomneyDa changed the title ~~perf(infra): memoize sync JSON reads to deflate TUI startup~~ perf(plugins): thread explicit discovery through capability runtime + retry loop May 19, 2026

RomneyDa mentioned this pull request May 19, 2026

perf: thread explicit plugin discovery through contracts registry #75451

Merged

RomneyDa force-pushed the dallin-performance-audit branch from d6b8f46 to 1e076ab Compare May 19, 2026 19:34

openclaw-barnacle Bot added size: S and removed size: XS labels May 19, 2026

RomneyDa changed the title ~~perf(plugins): thread explicit discovery through capability runtime + retry loop~~ perf(plugins): extend discovery threading to loader, manifest registry, installed-index, and config contracts May 19, 2026

RomneyDa merged commit f5f0b2c into main May 19, 2026
40 of 42 checks passed

RomneyDa deleted the dallin-performance-audit branch May 19, 2026 19:35

This was referenced May 19, 2026

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

Merged

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

Merged

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

📡 Upstream Digest — 2026-05-19 19:52 UTC curtismercier/openclaw-mods#899

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#84258

perf(plugins): extend discovery threading to loader, manifest registry, installed-index, and config contracts#84258
RomneyDa merged 1 commit into
mainfrom
dallin-performance-audit

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!

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

What this PR adds

Context: what we measured before #75451 landed

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

Existing work in this area — for reviewer awareness

Issue #82308 — "[Perf]: Redundant filesystem walks in plugin discovery during startup" (open, May 15 2026)

PR #75451 — "perf: thread explicit plugin discovery through contracts registry" (open, May 1 2026, author @SebTardif)

How these relate to this PR

Recommendation for reviewers

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