← Back to PRs

#20402: Pr/load openclaw plugins async

by ramarnat open 2026-02-18 21:40 View on GitHub →
size: L
## Summary Describe the problem and fix in 2–5 bullets: - Problem: plugin loading in tests needed a non-`jiti` path for JS ESM entrypoints, - Why it matters: test reliability on newer Node runtimes and maintainability of plugin loader behavior. - What changed: added `loadOpenClawPluginsAsync` (native ESM import for JS entrypoints) and DRY’d shared sync/async setup/finalization via shared helpers. - What did NOT change (scope boundary): default sync loader behavior remains; no production-facing API surface changes outside plugin loader internals; no e2e fixture/test/doc files are included in this PR branch. ! lobster-biscuit ! ## Change Type (select all) - [x] Bug fix - [x] Feature - [x] Refactor ## Scope (select all touched areas) - [x] Gateway / orchestration ## Linked Issue/PR - Closes # - Related # ## User-visible / Behavior Changes - None for end users. - Test/runtime internals: new async plugin loader path is available for test harnesses. ## Security Impact (required) - New permissions/capabilities? (`No`) - Secrets/tokens handling changed? (`No`) - New/changed network calls? (`No`) - Command/tool execution surface changed? (`No`) - Data access scope changed? (`No`) - If any `Yes`, explain risk + mitigation: ## Repro + Verification ### Environment - OS: macOS - Runtime/container: Node 25.6.0 (local) - Model/provider: N/A - Integration/channel (if any): N/A - Relevant config (redacted): N/A ### Steps 1. `direnv exec . pnpm vitest run src/plugins/hooks.phase-hooks.test.ts src/plugins/wired-hooks-message.test.ts` 2. Confirm branch diff vs `upstream/main` touches only plugin loader internals. 3. Verify async loader path exists and shared DRY helpers are used by both loaders. ### Expected - Targeted plugin loader tests pass. - Sync and async loaders share common setup/finalization logic. - Async loader uses native ESM import for JS entrypoints. ### Actual - Matches expected. ## Evidence Attach at least one: - [x] Failing test/log before + passing after - [x] Trace/log snippets ## Human Verification (required) What you personally verified (not just CI), and how: - Verified scenarios: targeted tests for plugin hook loading/message flow. - Edge cases checked: async loader rejects TS entrypoints and loads JS via native ESM import path. - What you did **not** verify: full `pnpm test` (workspace has unrelated optional extension dependency failures/OOM outside this scope). ## Compatibility / Migration - Backward compatible? (`Yes`) - Config/env changes? (`No`) - Migration needed? (`No`) - If yes, exact upgrade steps: ## Failure Recovery (if this breaks) - How to disable/revert this change quickly: revert commits on `pr/load-openclaw-plugins-async`. - Files/config to restore: `/Volumes/devel/openclaw-work/openclaw/codex-ikentic-plugin-test-scaffolding/src/plugins/loader.ts` - Known bad symptoms reviewers should watch for: plugin load regressions in tests relying on sync loader defaults. ## Risks and Mitigations - Risk: async loader path diverges from sync behavior over time. - Mitigation: shared helper refactor centralizes common setup/finalization logic used by both loaders. <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR refactors the plugin loader to extract shared logic into helper functions and adds a new async loader (`loadOpenClawPluginsAsync`) that uses native ESM imports for JavaScript entrypoints instead of jiti. **Key changes:** - Extracted 5 helper functions to eliminate duplication: `createPluginLoadContext`, `finalizePluginLoad`, `handlePluginLoadFailure`, `preparePluginCandidate`, and `applyLoadedPluginCandidate` - Added async loader that rejects TypeScript files and uses `import()` with `pathToFileURL` for JS/ESM compatibility - Preserved explicit memory slot `null` values (fixed in head commit 79a6c809) - Both loaders now share the same candidate processing logic through the extracted helpers **Previous issues resolved:** - No longer imports non-existent `command-options.js` module - No longer references non-existent `commandOptions` field on `PluginRecord` The refactor maintains backward compatibility - the sync loader behavior is unchanged, and the async loader is a new export not yet used elsewhere in the codebase. <h3>Confidence Score: 5/5</h3> - This PR is safe to merge - it's a well-executed refactoring with no behavioral changes to existing code - The refactoring successfully eliminates ~260 lines of duplication between sync and async loaders by extracting shared logic into helper functions. The async loader is a new export not yet used in production. Previous compilation issues mentioned in review threads (missing imports, non-existent fields) have been resolved. The memory slot handling fix in the head commit correctly preserves explicit `null` values. All shared logic paths are identical between both loaders, reducing future maintenance risk. - No files require special attention - the refactoring is thorough and correct <sub>Last reviewed commit: 79a6c80</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs