Skip to content

fix(plugins): scope install slot selection#75640

Merged
vincentkoc merged 2 commits into
mainfrom
qa-plugin-install-stress
May 1, 2026
Merged

fix(plugins): scope install slot selection#75640
vincentkoc merged 2 commits into
mainfrom
qa-plugin-install-stress

Conversation

@vincentkoc

Copy link
Copy Markdown
Member

Summary

  • scope plugin install/enable slot selection to the selected plugin manifest instead of building the broad plugin snapshot
  • use a selected-plugin runtime diagnostics fallback only when manifest metadata omits kind
  • add install persistence coverage proving sibling runtimes are not loaded for slot cleanup
  • update changelog with Thanks @vincentkoc

Bug / behavior

Realistic plugin install and enable paths were loading broad plugin status/runtime state just to update exclusive slots. In the stress probe, a single synthetic local plugin install spent most of its time in slot selection because the CLI imported every plugin runtime.

Live evidence

Before this patch, node .artifacts/qa-plugin-install-probe.mjs 1 took 112.875s with max RSS about 751MB, with lifecycle trace showing slot selection at 108.0s and runtime plugin registry load at 106.6s.

After this patch, configured memory-lancedb install completed in 7.22s with max RSS about 319MB, slot selection at 140.49ms, and no all-plugin runtime load. Synthetic local 3-install stress completed in 26.487s total, avg 8.829s, max RSS about 421MB.

Validation

  • Crabbox cbx_3d17082efb24 / coral-hermit
  • run_be45800925ef: remote changed lanes verified only CHANGELOG.md, src/cli/plugins-cli-test-helpers.ts, src/cli/plugins-command-helpers.ts, and src/cli/plugins-install-persist.test.ts
  • run_f6bc41c854f4: pnpm test:serial src/cli/plugins-install-persist.test.ts src/plugins/slots.test.ts passed, 30 tests
  • run_681df459a0f0: pnpm check:changed passed

@vincentkoc vincentkoc self-assigned this May 1, 2026
@openclaw-barnacle openclaw-barnacle Bot added cli CLI command changes size: M maintainer Maintainer-authored PR labels May 1, 2026
@clawsweeper

clawsweeper Bot commented May 1, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

What this changes:

The PR scopes plugin install and enable slot selection to selected-plugin manifest metadata with a selected-plugin runtime fallback, updates CLI test helpers and install-persistence coverage, and adds a changelog entry.

Maintainer follow-up before merge:

This is an open maintainer-labeled implementation PR with no discrete automated repair blocker from the review; the next action is maintainer review plus mergeability/check handling, not a ClawSweeper fix PR.

Security review:

Security review cleared: The diff changes CLI/plugin slot lookup scope plus tests and changelog only; it does not add dependency sources, workflow permissions, package resolution changes, secret handling, or new install-time code execution paths.

Review details

Best possible solution:

Finish maintainer review, resolve mergeability/check gating, and land the scoped manifest-first/runtime-fallback implementation with its install-persistence coverage and changelog entry if the branch remains green.

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

Yes. Current main inspection provides a high-confidence reproduction path: enabled plugin installs and plugins enable call applySlotSelectionForPlugin, which builds a broad plugin snapshot and can load unscoped runtime diagnostics; the PR body adds timing traces showing the observed install slowdown before and after the change.

Is this the best way to solve the issue?

Yes, with maintainer sign-off. The PR uses existing scoped manifest pluginIds and diagnostics onlyPluginIds contracts, keeps runtime kind lookup as a selected-plugin legacy fallback, and avoids adding a new plugin API or broad core behavior.

What I checked:

  • Protected PR state: Provided GitHub context shows this PR is open, ready for review, authored by vincentkoc with authorAssociation=MEMBER, labeled maintainer, and currently mergeable=false, so conservative cleanup must not close it. (cb217a7e5a63)
  • Current main broad slot path: Current main's applySlotSelectionForPlugin builds a broad plugin snapshot and can call buildPluginDiagnosticsReport without onlyPluginIds before applying slot selection. (src/cli/plugins-command-helpers.ts:50, 3a24a25f4b97)
  • Install and enable paths reach slot selection: persistPluginInstall calls applySlotSelectionForPlugin during enabled installs, and the plugins enable command calls the same helper after enablePluginInConfig. (src/cli/plugins-install-persist.ts:91, 3a24a25f4b97)
  • Existing scoped manifest contract: loadPluginManifestRegistryForPluginRegistry already accepts pluginIds and forwards them into loadPluginManifestRegistryForInstalledIndex, which filters candidates before manifest loading. (src/plugins/plugin-registry-contributions.ts:279, 3a24a25f4b97)
  • Existing scoped runtime contract: buildPluginDiagnosticsReport already accepts onlyPluginIds and passes that scope to loadOpenClawPlugins, matching the PR's selected-plugin runtime fallback direction. (src/plugins/status.ts:298, 3a24a25f4b97)
  • PR diff scope: The public PR diff replaces broad snapshot status loading with selected-plugin manifest registry loading and onlyPluginIds diagnostics fallback, updates tests to assert sibling runtimes stay loaded/enabled, and adds the required changelog line. (src/cli/plugins-command-helpers.ts:1, cb217a7e5a63)

Likely related people:

  • vincentkoc: Local blame and path history show the slot-selection helper, install-persist slot call, slot implementation, and related tests all trace to bcaf980 by Vincent Koc; the same maintainer authored this focused PR branch. (role: introduced behavior and recent maintainer; confidence: high; commits: bcaf9800157b; files: src/cli/plugins-command-helpers.ts, src/cli/plugins-install-persist.ts, src/cli/plugins-install-persist.test.ts)

Remaining risk / open question:

  • Provided GitHub context reports mergeable=false, so branch mergeability and required checks still need the normal maintainer gate before landing.

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

@vincentkoc
vincentkoc marked this pull request as ready for review May 1, 2026 12:59
@vincentkoc
vincentkoc force-pushed the qa-plugin-install-stress branch from cb217a7 to 70095f0 Compare May 1, 2026 13:08
@vincentkoc
vincentkoc merged commit 70095f0 into main May 1, 2026
19 checks passed
@vincentkoc
vincentkoc deleted the qa-plugin-install-stress branch May 1, 2026 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cli CLI command changes maintainer Maintainer-authored PR size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant