fix(plugins): reuse active registry for sub-agent tool resolution#56240
Conversation
|
This directly addresses a sub-agent tool resolution ordering issue — plugin-registered tools missing the resolution window because createOpenClawTools() runs before loadOpenClawPlugins() in the sub-agent startup flow. We have confirmed this issue affects multi-agent workflows where plugin tools (e.g. memory plugins) are unavailable in sub-agent sessions. The registry reuse approach is the right fix. Size M, but the fix is targeted. LGTM, requesting maintainer review. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d1cffa371
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Greptile SummaryReview Summary (PR #56240)Confidence: 4/5 — The production fix is correct and well-reasoned, but one P1 test issue should be addressed before merge. P1 — Inverted assertion in the cold-start test (
|
| Filename | Overview |
|---|---|
| src/plugins/loader.ts | Adds isGatewayScopedLoad() predicate and a fallback path in resolveRuntimePluginRegistry() that returns the active registry for non-gateway-scoped callers when cache keys differ. Logic is sound for the stated use case; one design caveat around config/workspace compatibility noted. |
| src/plugins/loader.test.ts | Adds comprehensive regression tests for the new fallback path; one cold-start test has an inverted assertion — loadOpenClawPlugins always activates the registry so getActivePluginRegistry() will not be null after the call. |
Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/plugins/loader.test.ts
Line: 3808-3817
Comment:
**Incorrect assertion — active registry will not be null after `loadOpenClawPlugins`**
`resolveRuntimePluginRegistry(toolOptions)` falls through to `loadOpenClawPlugins(options)`. `loadOpenClawPlugins` always calls `activatePluginRegistry(registry, cacheKey)` — i.e. `setActivePluginRegistry(registry)` — whenever `shouldActivate` is true (the default, since `toolOptions` has no `activate: false`). The activation fires unconditionally after the discovery loop (line 1432 in `loader.ts`):
```typescript
if (shouldActivate) {
activatePluginRegistry(registry, cacheKey);
}
return registry;
```
Consequently `getActivePluginRegistry()` returns the freshly-created registry, not `null`. The assertion is inverted:
```suggestion
expect(getActivePluginRegistry()).not.toBeNull();
expect(getActivePluginRegistry()).toBe(result);
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/plugins/loader.ts
Line: 359-365
Comment:
**Fallback ignores `config`/`workspaceDir` compatibility when returning the active registry**
`getCompatibleActivePluginRegistry(options)` already returned `undefined`, meaning the options' cache key does not match the active registry's key. The new fallback then calls `getActivePluginRegistry()` unconditionally for every non-gateway-scoped caller — without re-checking `workspaceDir`, `config`, or `runtimeOptions`.
This is safe today because sub-agents share one gateway process and the same `workspaceDir`/`config`. But a future caller that omits gateway-specific fields while passing a different `workspaceDir` or `config` would silently receive the wrong registry. Consider adding a comment documenting why non-matching workspace/config is acceptable for this path, or narrow the guard to also verify those components match.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "refactor: simplify plugin registry resol..." | Re-trigger Greptile
7d1cffa to
9361b1b
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9361b1bcef
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
a15bcb0 to
308ceba
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 308ceba46a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
308ceba to
f6cdf38
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6cdf38e56
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
f6cdf38 to
1f48c58
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f48c583ec
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (!isGatewayScopedLoad(options)) { | ||
| const active = getActivePluginRegistry(); | ||
| if (active) { | ||
| defaultLogger().debug("resolve-registry: returning active via non-gateway-scoped fallback"); | ||
| return active; |
There was a problem hiding this comment.
Preserve workspace-specific reloads in registry fallback
This fallback returns the process-wide active registry for any non-gateway-scoped call even when the requested workspaceDir differs, so loadOpenClawPlugins(options) is skipped and workspace-specific discovery never runs for that caller. In multi-agent runs where a non-default agent has its own workspace, plugin tools/providers from that agent’s <workspace>/.openclaw/extensions can be silently missing because tool resolution reuses the startup registry instead of loading for the requested context. Fresh evidence for re-raising: resolvePluginSourceRoots/discoverOpenClawPlugins still key discovery off workspaceDir, and createOpenClawTools explicitly resolves per-session workspaces to keep plugin loading workspace-stable.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Same concern as all prior Codex P1 comments — addressed in the inline safety comment. See reply on comment 3004704388 for the full rationale.
1f48c58 to
05cefb3
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05cefb32d3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (!isGatewayScopedLoad(options)) { | ||
| const active = getActivePluginRegistry(); | ||
| if (active) { | ||
| defaultLogger().debug("resolve-registry: returning active via non-gateway-scoped fallback"); | ||
| return active; |
There was a problem hiding this comment.
Gate active fallback on workspace/config cache-key parity
Returning the process-wide active registry whenever !isGatewayScopedLoad(options) ignores cache-key mismatches for other compatibility inputs, so a call with different workspaceDir/config/env can skip loadOpenClawPlugins(options) and reuse stale plugin state. Fresh evidence for re-raising this: subagent lifecycle paths pass per-run workspaces into plugin bootstrap (src/agents/subagent-registry.ts:194-198), and plugin discovery still keys off options.workspaceDir (src/plugins/loader.ts:986-995), so workspace-local plugins/hooks/tools for non-default agent workspaces can be silently missed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Same concern as all prior Codex P1 comments — addressed in the inline safety comment. See reply on comment 3004704388 for the full rationale.
05cefb3 to
678b72c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 678b72c42f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (!isGatewayScopedLoad(options)) { | ||
| const active = getActivePluginRegistry(); | ||
| if (active) { | ||
| defaultLogger().debug("resolve-registry: returning active via non-gateway-scoped fallback"); | ||
| return active; |
There was a problem hiding this comment.
Honor workspace/config cache-key mismatches on fallback
This fallback returns getActivePluginRegistry() for any non-gateway-scoped call even after getCompatibleActivePluginRegistry(options) has already proven the cache key is different, so callers requesting a different load context silently skip loadOpenClawPlugins(options). That can drop workspace-local plugins/tools for subagent flows that pass a per-run workspace (for example via ensureRuntimePluginsLoaded(...)) because plugin discovery roots are derived from workspaceDir (including <workspaceDir>/.openclaw/extensions). Fresh evidence for re-raising: spawned/subagent runs can resolve a target-agent workspace in src/agents/spawned-context.ts (resolveSpawnedWorkspaceInheritance), and loader roots still include workspace-specific extension paths in src/plugins/roots.ts.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Same concern as all prior Codex P1 comments — addressed in the inline safety comment. See reply on comment 3004704388 for the full rationale.
678b72c to
51b083a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 51b083afc7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (!isGatewayScopedLoad(options)) { | ||
| const active = getActivePluginRegistry(); | ||
| if (active) { | ||
| defaultLogger().debug("resolve-registry: returning active via non-gateway-scoped fallback"); | ||
| return active; |
There was a problem hiding this comment.
Respect config mismatch before reusing active registry
The non-gateway fallback now returns the process-wide active registry even after getCompatibleActivePluginRegistry(options) has already detected a cache-key mismatch, which prevents intentional config-based reloads. A concrete affected path is resolvePluginCapabilityProviders: when the active registry has no capability providers, it builds a compat config and calls resolveRuntimePluginRegistry({ config: ... }) (src/plugins/capability-provider-runtime.ts:75-82) specifically to load bundled capability plugins; with this change, that call can be short-circuited back to the startup registry, so speech/media/image provider discovery stays empty instead of loading providers for the requested config.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Same concern as all prior Codex P1 comments — addressed in the inline safety comment. See reply on comment 3004704388 for the full rationale.
51b083a to
7c5d5a0
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c5d5a0780
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (!isGatewayScopedLoad(options)) { | ||
| const active = getActivePluginRegistry(); | ||
| if (active) { | ||
| defaultLogger().debug("resolve-registry: returning active via non-gateway-scoped fallback"); | ||
| return active; |
There was a problem hiding this comment.
Honor config mismatch before reusing active registry
Do not return the process-wide active registry here when cache keys differ for a non-gateway-scoped call, because some callers rely on that mismatch to force a targeted reload. In resolvePluginCapabilityProviders (src/plugins/capability-provider-runtime.ts:75-82), when the active registry has no capability providers it builds a compat config and calls resolveRuntimePluginRegistry({ config: ... }) specifically to load bundled capability plugins; this fallback now short-circuits that path and returns the startup registry unchanged, leaving speech/media/image provider lists empty in gateway flows (for example tts.providers / tts.setProvider) whenever startup loaded a scoped registry without those providers.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Same concern as all prior Codex P1 comments — addressed in the inline safety comment. See reply on comment 3004704388 for the full rationale.
7c5d5a0 to
05d5710
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05d57103bf
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (!isGatewayScopedLoad(options)) { | ||
| const active = getActivePluginRegistry(); | ||
| if (active) { | ||
| defaultLogger().debug("resolve-registry: returning active via non-gateway-scoped fallback"); | ||
| return active; |
There was a problem hiding this comment.
Skip active-registry fallback when compat config is requested
Do not return the process-wide active registry for every non-gateway-scoped mismatch here, because some callers intentionally pass a different config to force a targeted reload. In src/plugins/capability-provider-runtime.ts, resolvePluginCapabilityProviders falls back to resolveRuntimePluginRegistry({ config: ... }) when the active registry has no capability providers; this branch now returns getActivePluginRegistry() instead of loading that compat config, so speech/media/image provider discovery can stay empty in gateway processes that started with a scoped plugin set.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Same concern as all prior Codex P1 comments on this fallback path — addressed in the inline safety comment.
The single-gateway-per-process model ensures sub-agent callers share the same config, workspaceDir, and env as the gateway. Plugin discovery is config-driven (not workspace-driven), and the gateway's active registry is a superset that already contains all activated plugins. A fresh loadOpenClawPlugins without onlyPluginIds would create a registry with unactivated plugins AND replace the fully-initialized one — strictly worse.
See the expanded safety comment in the code for the full invariant documentation.
…ller-shape, and cold-start tests Add 11 regression tests covering: - R1: Per-field isolation (coreGatewayHandlers, includeSetupOnlyChannelPlugins, preferSetupRuntimeForChannelPlugins each independently prevent fallback; empty onlyPluginIds[] treated as non-gateway-scoped) - R2: Caller-shape regression (tools.ts, memory-runtime.ts, channel-resolution.ts shapes fall back; web-search-providers.runtime.ts with onlyPluginIds does not) - R3: Cold-start path (null active registry falls through to loadOpenClawPlugins) Add debug logging to resolveRuntimePluginRegistry recording which exit path was taken (no-options, cache-key-match, non-gateway-scoped fallback, fresh load).
- Fix cold-start test assertion: loadOpenClawPlugins always activates the registry (shouldActivate defaults to true), so getActivePluginRegistry() is not null after the call. Updated assertion to match actual behavior. - Add safety comment documenting why the non-gateway-scoped fallback is safe despite cache-key mismatch: single-gateway-per-process model means sub-agents share workspaceDir, config, and env with the gateway.
…meouts Test isGatewayScopedLoad directly instead of going through the full resolveRuntimePluginRegistry path which triggers expensive plugin discovery. This fixes the includeSetupOnlyChannelPlugins test timing out in CI while providing more precise coverage of the predicate.
cf99ea0 to
388b14e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 388b14efe4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (params.allowGatewaySubagentBinding) { | ||
| return getActivePluginRegistry() ?? resolveRuntimePluginRegistry(params.loadOptions); |
There was a problem hiding this comment.
Preserve cache-key checks for gateway-bindable tool registry
This fast path returns getActivePluginRegistry() for any gateway-bindable call and skips resolveRuntimePluginRegistry(loadOptions), which bypasses the loader’s compatibility keying on runtimeSubagentMode (src/plugins/loader.ts includes it in the cache key). Fresh evidence in this commit is that resolvePluginTools now never reaches loader compatibility checks when allowGatewaySubagentBinding is true and an active registry exists. If the active registry was loaded in default mode, plugin tools are built against a runtime where subagent stays unavailable (src/plugins/runtime/index.ts), so plugin tools that depend on gateway subagent APIs can fail at runtime instead of getting a gateway-bindable reload.
Useful? React with 👍 / 👎.
…thanks @GodsBoy) * fix(plugins): reuse active registry for sub-agent tool resolution * test(plugins): harden resolveRuntimePluginRegistry with per-field, caller-shape, and cold-start tests Add 11 regression tests covering: - R1: Per-field isolation (coreGatewayHandlers, includeSetupOnlyChannelPlugins, preferSetupRuntimeForChannelPlugins each independently prevent fallback; empty onlyPluginIds[] treated as non-gateway-scoped) - R2: Caller-shape regression (tools.ts, memory-runtime.ts, channel-resolution.ts shapes fall back; web-search-providers.runtime.ts with onlyPluginIds does not) - R3: Cold-start path (null active registry falls through to loadOpenClawPlugins) Add debug logging to resolveRuntimePluginRegistry recording which exit path was taken (no-options, cache-key-match, non-gateway-scoped fallback, fresh load). * refactor: simplify plugin registry resolution tests and trim happy-path debug logs * fix(plugins): address review comments on registry fallback - Fix cold-start test assertion: loadOpenClawPlugins always activates the registry (shouldActivate defaults to true), so getActivePluginRegistry() is not null after the call. Updated assertion to match actual behavior. - Add safety comment documenting why the non-gateway-scoped fallback is safe despite cache-key mismatch: single-gateway-per-process model means sub-agents share workspaceDir, config, and env with the gateway. * test(plugins): restructure per-field isolation tests to avoid load timeouts Test isGatewayScopedLoad directly instead of going through the full resolveRuntimePluginRegistry path which triggers expensive plugin discovery. This fixes the includeSetupOnlyChannelPlugins test timing out in CI while providing more precise coverage of the predicate. * fix(plugins): expand safety comment to address startup-scoped registry concern * fix(plugins): scope subagent registry reuse to tool loading --------- Co-authored-by: Ayaan Zaidi <hi@obviy.us>
…thanks @GodsBoy) * fix(plugins): reuse active registry for sub-agent tool resolution * test(plugins): harden resolveRuntimePluginRegistry with per-field, caller-shape, and cold-start tests Add 11 regression tests covering: - R1: Per-field isolation (coreGatewayHandlers, includeSetupOnlyChannelPlugins, preferSetupRuntimeForChannelPlugins each independently prevent fallback; empty onlyPluginIds[] treated as non-gateway-scoped) - R2: Caller-shape regression (tools.ts, memory-runtime.ts, channel-resolution.ts shapes fall back; web-search-providers.runtime.ts with onlyPluginIds does not) - R3: Cold-start path (null active registry falls through to loadOpenClawPlugins) Add debug logging to resolveRuntimePluginRegistry recording which exit path was taken (no-options, cache-key-match, non-gateway-scoped fallback, fresh load). * refactor: simplify plugin registry resolution tests and trim happy-path debug logs * fix(plugins): address review comments on registry fallback - Fix cold-start test assertion: loadOpenClawPlugins always activates the registry (shouldActivate defaults to true), so getActivePluginRegistry() is not null after the call. Updated assertion to match actual behavior. - Add safety comment documenting why the non-gateway-scoped fallback is safe despite cache-key mismatch: single-gateway-per-process model means sub-agents share workspaceDir, config, and env with the gateway. * test(plugins): restructure per-field isolation tests to avoid load timeouts Test isGatewayScopedLoad directly instead of going through the full resolveRuntimePluginRegistry path which triggers expensive plugin discovery. This fixes the includeSetupOnlyChannelPlugins test timing out in CI while providing more precise coverage of the predicate. * fix(plugins): expand safety comment to address startup-scoped registry concern * fix(plugins): scope subagent registry reuse to tool loading --------- Co-authored-by: Ayaan Zaidi <hi@obviy.us>
…thanks @GodsBoy) * fix(plugins): reuse active registry for sub-agent tool resolution * test(plugins): harden resolveRuntimePluginRegistry with per-field, caller-shape, and cold-start tests Add 11 regression tests covering: - R1: Per-field isolation (coreGatewayHandlers, includeSetupOnlyChannelPlugins, preferSetupRuntimeForChannelPlugins each independently prevent fallback; empty onlyPluginIds[] treated as non-gateway-scoped) - R2: Caller-shape regression (tools.ts, memory-runtime.ts, channel-resolution.ts shapes fall back; web-search-providers.runtime.ts with onlyPluginIds does not) - R3: Cold-start path (null active registry falls through to loadOpenClawPlugins) Add debug logging to resolveRuntimePluginRegistry recording which exit path was taken (no-options, cache-key-match, non-gateway-scoped fallback, fresh load). * refactor: simplify plugin registry resolution tests and trim happy-path debug logs * fix(plugins): address review comments on registry fallback - Fix cold-start test assertion: loadOpenClawPlugins always activates the registry (shouldActivate defaults to true), so getActivePluginRegistry() is not null after the call. Updated assertion to match actual behavior. - Add safety comment documenting why the non-gateway-scoped fallback is safe despite cache-key mismatch: single-gateway-per-process model means sub-agents share workspaceDir, config, and env with the gateway. * test(plugins): restructure per-field isolation tests to avoid load timeouts Test isGatewayScopedLoad directly instead of going through the full resolveRuntimePluginRegistry path which triggers expensive plugin discovery. This fixes the includeSetupOnlyChannelPlugins test timing out in CI while providing more precise coverage of the predicate. * fix(plugins): expand safety comment to address startup-scoped registry concern * fix(plugins): scope subagent registry reuse to tool loading --------- Co-authored-by: Ayaan Zaidi <hi@obviy.us>
…thanks @GodsBoy) * fix(plugins): reuse active registry for sub-agent tool resolution * test(plugins): harden resolveRuntimePluginRegistry with per-field, caller-shape, and cold-start tests Add 11 regression tests covering: - R1: Per-field isolation (coreGatewayHandlers, includeSetupOnlyChannelPlugins, preferSetupRuntimeForChannelPlugins each independently prevent fallback; empty onlyPluginIds[] treated as non-gateway-scoped) - R2: Caller-shape regression (tools.ts, memory-runtime.ts, channel-resolution.ts shapes fall back; web-search-providers.runtime.ts with onlyPluginIds does not) - R3: Cold-start path (null active registry falls through to loadOpenClawPlugins) Add debug logging to resolveRuntimePluginRegistry recording which exit path was taken (no-options, cache-key-match, non-gateway-scoped fallback, fresh load). * refactor: simplify plugin registry resolution tests and trim happy-path debug logs * fix(plugins): address review comments on registry fallback - Fix cold-start test assertion: loadOpenClawPlugins always activates the registry (shouldActivate defaults to true), so getActivePluginRegistry() is not null after the call. Updated assertion to match actual behavior. - Add safety comment documenting why the non-gateway-scoped fallback is safe despite cache-key mismatch: single-gateway-per-process model means sub-agents share workspaceDir, config, and env with the gateway. * test(plugins): restructure per-field isolation tests to avoid load timeouts Test isGatewayScopedLoad directly instead of going through the full resolveRuntimePluginRegistry path which triggers expensive plugin discovery. This fixes the includeSetupOnlyChannelPlugins test timing out in CI while providing more precise coverage of the predicate. * fix(plugins): expand safety comment to address startup-scoped registry concern * fix(plugins): scope subagent registry reuse to tool loading --------- Co-authored-by: Ayaan Zaidi <hi@obviy.us>
…thanks @GodsBoy) * fix(plugins): reuse active registry for sub-agent tool resolution * test(plugins): harden resolveRuntimePluginRegistry with per-field, caller-shape, and cold-start tests Add 11 regression tests covering: - R1: Per-field isolation (coreGatewayHandlers, includeSetupOnlyChannelPlugins, preferSetupRuntimeForChannelPlugins each independently prevent fallback; empty onlyPluginIds[] treated as non-gateway-scoped) - R2: Caller-shape regression (tools.ts, memory-runtime.ts, channel-resolution.ts shapes fall back; web-search-providers.runtime.ts with onlyPluginIds does not) - R3: Cold-start path (null active registry falls through to loadOpenClawPlugins) Add debug logging to resolveRuntimePluginRegistry recording which exit path was taken (no-options, cache-key-match, non-gateway-scoped fallback, fresh load). * refactor: simplify plugin registry resolution tests and trim happy-path debug logs * fix(plugins): address review comments on registry fallback - Fix cold-start test assertion: loadOpenClawPlugins always activates the registry (shouldActivate defaults to true), so getActivePluginRegistry() is not null after the call. Updated assertion to match actual behavior. - Add safety comment documenting why the non-gateway-scoped fallback is safe despite cache-key mismatch: single-gateway-per-process model means sub-agents share workspaceDir, config, and env with the gateway. * test(plugins): restructure per-field isolation tests to avoid load timeouts Test isGatewayScopedLoad directly instead of going through the full resolveRuntimePluginRegistry path which triggers expensive plugin discovery. This fixes the includeSetupOnlyChannelPlugins test timing out in CI while providing more precise coverage of the predicate. * fix(plugins): expand safety comment to address startup-scoped registry concern * fix(plugins): scope subagent registry reuse to tool loading --------- Co-authored-by: Ayaan Zaidi <hi@obviy.us>
…thanks @GodsBoy) * fix(plugins): reuse active registry for sub-agent tool resolution * test(plugins): harden resolveRuntimePluginRegistry with per-field, caller-shape, and cold-start tests Add 11 regression tests covering: - R1: Per-field isolation (coreGatewayHandlers, includeSetupOnlyChannelPlugins, preferSetupRuntimeForChannelPlugins each independently prevent fallback; empty onlyPluginIds[] treated as non-gateway-scoped) - R2: Caller-shape regression (tools.ts, memory-runtime.ts, channel-resolution.ts shapes fall back; web-search-providers.runtime.ts with onlyPluginIds does not) - R3: Cold-start path (null active registry falls through to loadOpenClawPlugins) Add debug logging to resolveRuntimePluginRegistry recording which exit path was taken (no-options, cache-key-match, non-gateway-scoped fallback, fresh load). * refactor: simplify plugin registry resolution tests and trim happy-path debug logs * fix(plugins): address review comments on registry fallback - Fix cold-start test assertion: loadOpenClawPlugins always activates the registry (shouldActivate defaults to true), so getActivePluginRegistry() is not null after the call. Updated assertion to match actual behavior. - Add safety comment documenting why the non-gateway-scoped fallback is safe despite cache-key mismatch: single-gateway-per-process model means sub-agents share workspaceDir, config, and env with the gateway. * test(plugins): restructure per-field isolation tests to avoid load timeouts Test isGatewayScopedLoad directly instead of going through the full resolveRuntimePluginRegistry path which triggers expensive plugin discovery. This fixes the includeSetupOnlyChannelPlugins test timing out in CI while providing more precise coverage of the predicate. * fix(plugins): expand safety comment to address startup-scoped registry concern * fix(plugins): scope subagent registry reuse to tool loading --------- Co-authored-by: Ayaan Zaidi <hi@obviy.us>
…thanks @GodsBoy) * fix(plugins): reuse active registry for sub-agent tool resolution * test(plugins): harden resolveRuntimePluginRegistry with per-field, caller-shape, and cold-start tests Add 11 regression tests covering: - R1: Per-field isolation (coreGatewayHandlers, includeSetupOnlyChannelPlugins, preferSetupRuntimeForChannelPlugins each independently prevent fallback; empty onlyPluginIds[] treated as non-gateway-scoped) - R2: Caller-shape regression (tools.ts, memory-runtime.ts, channel-resolution.ts shapes fall back; web-search-providers.runtime.ts with onlyPluginIds does not) - R3: Cold-start path (null active registry falls through to loadOpenClawPlugins) Add debug logging to resolveRuntimePluginRegistry recording which exit path was taken (no-options, cache-key-match, non-gateway-scoped fallback, fresh load). * refactor: simplify plugin registry resolution tests and trim happy-path debug logs * fix(plugins): address review comments on registry fallback - Fix cold-start test assertion: loadOpenClawPlugins always activates the registry (shouldActivate defaults to true), so getActivePluginRegistry() is not null after the call. Updated assertion to match actual behavior. - Add safety comment documenting why the non-gateway-scoped fallback is safe despite cache-key mismatch: single-gateway-per-process model means sub-agents share workspaceDir, config, and env with the gateway. * test(plugins): restructure per-field isolation tests to avoid load timeouts Test isGatewayScopedLoad directly instead of going through the full resolveRuntimePluginRegistry path which triggers expensive plugin discovery. This fixes the includeSetupOnlyChannelPlugins test timing out in CI while providing more precise coverage of the predicate. * fix(plugins): expand safety comment to address startup-scoped registry concern * fix(plugins): scope subagent registry reuse to tool loading --------- Co-authored-by: Ayaan Zaidi <hi@obviy.us>
…thanks @GodsBoy) * fix(plugins): reuse active registry for sub-agent tool resolution * test(plugins): harden resolveRuntimePluginRegistry with per-field, caller-shape, and cold-start tests Add 11 regression tests covering: - R1: Per-field isolation (coreGatewayHandlers, includeSetupOnlyChannelPlugins, preferSetupRuntimeForChannelPlugins each independently prevent fallback; empty onlyPluginIds[] treated as non-gateway-scoped) - R2: Caller-shape regression (tools.ts, memory-runtime.ts, channel-resolution.ts shapes fall back; web-search-providers.runtime.ts with onlyPluginIds does not) - R3: Cold-start path (null active registry falls through to loadOpenClawPlugins) Add debug logging to resolveRuntimePluginRegistry recording which exit path was taken (no-options, cache-key-match, non-gateway-scoped fallback, fresh load). * refactor: simplify plugin registry resolution tests and trim happy-path debug logs * fix(plugins): address review comments on registry fallback - Fix cold-start test assertion: loadOpenClawPlugins always activates the registry (shouldActivate defaults to true), so getActivePluginRegistry() is not null after the call. Updated assertion to match actual behavior. - Add safety comment documenting why the non-gateway-scoped fallback is safe despite cache-key mismatch: single-gateway-per-process model means sub-agents share workspaceDir, config, and env with the gateway. * test(plugins): restructure per-field isolation tests to avoid load timeouts Test isGatewayScopedLoad directly instead of going through the full resolveRuntimePluginRegistry path which triggers expensive plugin discovery. This fixes the includeSetupOnlyChannelPlugins test timing out in CI while providing more precise coverage of the predicate. * fix(plugins): expand safety comment to address startup-scoped registry concern * fix(plugins): scope subagent registry reuse to tool loading --------- Co-authored-by: Ayaan Zaidi <hi@obviy.us>
Summary
When a sub-agent session starts,
createOpenClawTools()resolves the tool list beforeloadOpenClawPlugins()has run for that session context. Plugin-registered tools (e.g. honcho memory tools) miss the resolution window entirely and appear as "unknown entries" in the allowlist.The gateway start flow works correctly because plugins load during
gateway_startbefore any session resolves tools — the active registry already has everything.Root Cause
resolveRuntimePluginRegistry()compares cache keys to decide whether to reuse the active registry. Sub-agent callers pass different options (noonlyPluginIds, nocoreGatewayHandlers) which produces a different cache key — so the active registry is bypassed and a freshloadOpenClawPlugins()is triggered too late.Fix
Add an
isGatewayScopedLoad()check: when the caller does not restrict the plugin set with gateway-specific fields (onlyPluginIds,coreGatewayHandlers,includeSetupOnlyChannelPlugins,preferSetupRuntimeForChannelPlugins), the active registry set during gateway startup is a safe superset. Reuse it directly instead of falling through to a redundant load.This is the minimal change — one new predicate function and a fallback path in
resolveRuntimePluginRegistry().Changes
src/plugins/loader.ts— addisGatewayScopedLoad()helper; updateresolveRuntimePluginRegistry()to fall back to active registry for non-gateway-scoped callerssrc/plugins/loader.test.ts— two new test cases covering the fallback and the non-fallback pathsTest Plan
pnpm tsgo— 0 type errorspnpm lint— 0 warnings/errorspnpm build— cleanFixes #56208