fix(plugins): preserve plugin-registered internal hooks across gateway startup by carbaj03 · Pull Request #29515 · openclaw/openclaw

carbaj03 · 2026-02-28T06:36:41Z

Summary

Fix plugin hooks (e.g. agent:bootstrap) being wiped during gateway startup: clearInternalHooks() in startGatewaySidecars destroyed hooks registered by plugins during loadGatewayPlugins
Store the handler function in PluginHookRegistration so it survives the clear, then re-register plugin hooks after loadInternalHooks()
Without this fix, plugins cannot use api.registerHook("agent:bootstrap", handler) — the only way to inject content into the system prompt once per session

Test plan

pnpm tsgo — no new type errors
pnpm vitest run src/hooks/internal-hooks.test.ts — 30/30 pass (includes new test)
Gateway restart with plugin using registerHook("agent:bootstrap") → "re-registered 1 plugin hook" in logs

🤖 Generated with Claude Code

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 84415f5a9a

ℹ️ 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".

chatgpt-codex-connector · 2026-02-28T06:39:36Z

src/gateway/server-startup.ts

+      for (const hook of params.pluginRegistry.hooks) {
+        if (hook.handler) {
+          for (const event of hook.events) {
+            registerInternalHook(event, hook.handler);


Preserve register:false when restoring plugin hooks

This loop re-registers every plugin hook that has a stored handler, but api.registerHook(..., { register: false }) is explicitly supported in src/plugins/registry.ts (the initial registration path returns early when opts?.register === false). Because the register intent is not stored and checked here, hooks that were intentionally metadata-only become active after startup, so their handlers run unexpectedly once clearInternalHooks() has happened and this restore block executes.

Useful? React with 👍 / 👎.

Fixed in 33742f6. Hooks registered with opts.register = false now store handler: undefined in the registry, so the re-registration loop in startGatewaySidecars skips them (the existing if (hook.handler) guard handles it). Added a dedicated test case for this edge case.

greptile-apps · 2026-02-28T06:41:35Z

Greptile Summary

This PR fixes a real lifecycle bug where clearInternalHooks() in startGatewaySidecars was destroying hook registrations that plugins had set up during loadGatewayPlugins, making api.registerHook("agent:bootstrap", handler) silently inoperative. The approach — storing the handler in PluginHookRegistration and replaying it after loadInternalHooks — is clean and fits the existing architecture well.

Key issues:

Logic inconsistency with opts.register = false: handler is stored unconditionally in the pushed PluginHookRegistration, even when the guard (!hookSystemEnabled || opts?.register === false) would have blocked initial registration. The re-registration loop in server-startup.ts then registers those handlers unconditionally (only gating on hook.handler truthiness), so a plugin that opts out via register: false will have its hook silently re-registered on every gateway restart.
Test coverage gap: The new test validates the internal-hooks primitive in isolation but doesn't exercise the registry → startGatewaySidecars path, so the opts.register = false issue would not be caught by the test suite.

Confidence Score: 3/5

The core fix is correct and safe for the described use case, but there is a logic gap around opts.register = false that could cause unintended hook registrations on gateway restart.
Score of 3 reflects that the PR fixes a genuine bug and the approach is architecturally sound, but the unconditional storage of handler ignores the opts.register = false opt-out semantics. This could result in hooks that were never meant to fire being called on every gateway startup after the first restart. The test validates the happy path but misses this edge case.
src/plugins/registry.ts (lines 254-260) — the handler assignment needs to be conditional on whether registration was actually intended.

_{Last reviewed commit: 84415f5}

greptile-apps

_{3 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-28T06:41:42Z

src/hooks/internal-hooks.test.ts

+  describe("plugin hook re-registration after clear", () => {
+    it("should survive clearInternalHooks when re-registered from registry", async () => {
+      const handler = vi.fn();
+
+      // Simulate plugin registering a hook during loadGatewayPlugins
+      registerInternalHook("agent:bootstrap", handler);
+
+      // Simulate startGatewaySidecars clearing all hooks
+      clearInternalHooks();
+
+      // Handler should be gone
+      const event = createInternalHookEvent("agent", "bootstrap", "test-session", {
+        workspaceDir: "/tmp",
+        bootstrapFiles: [],
+      });
+      await triggerInternalHook(event);
+      expect(handler).not.toHaveBeenCalled();
+
+      // Simulate re-registration from plugin registry (the fix)
+      registerInternalHook("agent:bootstrap", handler);
+
+      // Now handler should fire
+      await triggerInternalHook(event);
+      expect(handler).toHaveBeenCalledOnce();
+      expect(handler).toHaveBeenCalledWith(event);
+    });
+  });


Test exercises raw primitives, not the full re-registration path

The test directly calls registerInternalHook / clearInternalHooks / registerInternalHook again — essentially verifying that the underlying hook map works, which was already covered by earlier tests. It doesn't exercise the actual code path introduced in this PR (startGatewaySidecars iterating pluginRegistry.hooks and calling registerInternalHook).

As a result, the opts.register = false edge-case described in the sibling comment is untested, and any regression in the re-registration loop in server-startup.ts would go undetected by this suite. Consider adding an integration-level test that goes through the createPluginRegistry → registerHook → re-registration loop, or at minimum add a case where handler is undefined (as it would be for opts.register = false hooks) and confirm it is skipped.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/hooks/internal-hooks.test.ts Line: 459-485 Comment: **Test exercises raw primitives, not the full re-registration path** The test directly calls `registerInternalHook` / `clearInternalHooks` / `registerInternalHook` again — essentially verifying that the underlying hook map works, which was already covered by earlier tests. It doesn't exercise the actual code path introduced in this PR (`startGatewaySidecars` iterating `pluginRegistry.hooks` and calling `registerInternalHook`). As a result, the `opts.register = false` edge-case described in the sibling comment is untested, and any regression in the re-registration loop in `server-startup.ts` would go undetected by this suite. Consider adding an integration-level test that goes through the `createPluginRegistry` → `registerHook` → re-registration loop, or at minimum add a case where `handler` is `undefined` (as it would be for `opts.register = false` hooks) and confirm it is skipped. How can I resolve this? If you propose a fix, please make it concise.

Addressed in 33742f6. Added a test case that simulates the re-registration loop with a handler: undefined hook (as produced by register: false) alongside a real handler, and verifies only the real handler fires. The registry fix ensures handler is stored as undefined when register: false, so metadata-only hooks stay inactive.

greptile-apps · 2026-02-28T06:41:44Z

Additional Comments (1)

src/plugins/registry.ts
handler stored unconditionally, ignoring opts?.register === false

The handler is pushed into registry.hooks unconditionally, even when the early-return guard at line 263 would have prevented registration (!hookSystemEnabled || opts?.register === false). This means that during re-registration in startGatewaySidecars, any hook a plugin author explicitly opted out of via opts.register = false will be silently re-registered anyway — violating their stated intent.

The fix is to only store handler when registration was actually intended:

const shouldRegister = hookSystemEnabled && opts?.register !== false;

registry.hooks.push({
  pluginId: record.id,
  entry: hookEntry,
  events: normalizedEvents,
  source: record.source,
  handler: shouldRegister ? handler : undefined,
});

Alternatively, store shouldRegister as a boolean field in PluginHookRegistration and check it in startGatewaySidecars.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/plugins/registry.ts
Line: 254-260

Comment:
**`handler` stored unconditionally, ignoring `opts?.register === false`**

The `handler` is pushed into `registry.hooks` unconditionally, even when the early-return guard at line 263 would have prevented registration (`!hookSystemEnabled || opts?.register === false`). This means that during re-registration in `startGatewaySidecars`, any hook a plugin author explicitly opted out of via `opts.register = false` will be silently re-registered anyway — violating their stated intent.

The fix is to only store `handler` when registration was actually intended:

```ts
const shouldRegister = hookSystemEnabled && opts?.register !== false;

registry.hooks.push({
  pluginId: record.id,
  entry: hookEntry,
  events: normalizedEvents,
  source: record.source,
  handler: shouldRegister ? handler : undefined,
});
```

Alternatively, store `shouldRegister` as a boolean field in `PluginHookRegistration` and check it in `startGatewaySidecars`.

How can I resolve this? If you propose a fix, please make it concise.

openclaw-barnacle · 2026-03-06T04:06:12Z

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

Workaround for OpenClaw Gateway startup order issue. Problem: api.registerHook() registers hooks before clearInternalHooks() runs, causing plugin hooks to be cleared during Gateway startup. Solution: Register the hook in service.start() by directly accessing globalThis.__openclaw_internal_hook_handlers__, which runs after clearInternalHooks() has completed. This workaround will be obsolete once openclaw/openclaw#29515 is merged and released.

carbaj03 · 2026-03-06T11:43:35Z

Still active. All bot review concerns addressed (see inline replies). CI is green. Ready for maintainer review.

carbaj03 · 2026-03-07T06:58:18Z

👋 Friendly ping — this PR has been green for over a week now and is ready for review.

We're preparing to launch our app and this fix is in the critical path. Would really appreciate a review when you get a chance. Thanks!

carbaj03 · 2026-03-09T06:58:07Z

CI is green, all bot review concerns addressed.

This fixes a real lifecycle bug: clearInternalHooks() in startGatewaySidecars destroys hooks that plugins registered during loadGatewayPlugins. Without this fix, api.registerHook("agent:bootstrap", handler) — the only way for plugins to inject content into the system prompt — is silently broken.

Minimal fix: store the handler reference in the plugin registry and re-register after the clear. 3 files, 2 tests. Happy to address any feedback.

carbaj03 · 2026-03-09T07:03:16Z

@vincentkoc would you have a chance to take a look when you get a moment?

carbaj03 · 2026-03-11T05:35:21Z

@joshavant @altaywtf — friendly ping, this PR is CI-green and ready for review. We're preparing for our app launch and this fix is in the critical path. Would appreciate a look when you get a chance!

carbaj03 · 2026-03-14T09:33:25Z

@tyler6204 @vincentkoc — this is a small fix (86 lines, 3 files) for a plugin lifecycle bug: clearInternalHooks() in startGatewaySidecars destroys hooks that plugins registered during loadGatewayPlugins. Without this, api.registerHook("agent:bootstrap", handler) is broken — the only way for plugins to inject content into the system prompt once per session.

CI is green, tests included. Would really appreciate a review!

carbaj03 · 2026-03-16T04:56:26Z

Hi @vincentkoc and @steipete — gentle ping on this one. This PR preserves plugin-registered internal hooks across gateway restarts (fixes hook loss on reload). All CI checks pass. Would love a human reviewer when you have a moment. Thanks! 🙏

carbaj03 · 2026-03-16T16:19:52Z

Bumping this for visibility — this PR and #31828 (canvas broadcast to all connected nodes) are a pair, both green with 23/23 CI checks passing.

supersantux expressed interest on Discord today and mentioned he'd follow up. Both are small and non-breaking — would really appreciate a maintainer pass when you get a chance. 🙏

…y startup clearInternalHooks() in startGatewaySidecars wiped plugin hooks registered during loadGatewayPlugins. Store the handler function in PluginHookRegistration and re-register plugin hooks after loadInternalHooks(). Co-Authored-By: Claude Opus 4.6 <[email protected]>

Hooks registered with opts.register=false are metadata-only. Previously the handler was stored unconditionally, causing these hooks to become active after clearInternalHooks() re-registration in startGatewaySidecars. Co-Authored-By: Claude Opus 4.6 <[email protected]>

carbaj03 · 2026-04-01T05:54:39Z

Closing — stale with merge conflicts and no reviews. Will re-submit on a fresh branch if still needed.

openclaw-barnacle bot added gateway Gateway runtime size: S labels Feb 28, 2026

chatgpt-codex-connector bot reviewed Feb 28, 2026

View reviewed changes

greptile-apps bot reviewed Feb 28, 2026

View reviewed changes

carbaj03 mentioned this pull request Feb 28, 2026

feat(canvas): add A2UI v0.8 wire protocol to a2ui_push tool description #29074

Closed

This was referenced Mar 3, 2026

fix: preserve plugin-registered internal hooks across gateway startup #26184

Closed

fix(hooks): preserve plugin-registered hooks across gateway startup reload #26242

Closed

openclaw-barnacle bot added the stale Marked as stale due to inactivity label Mar 6, 2026

Disaster-Terminator mentioned this pull request Mar 6, 2026

fix: register session-memory hook after clearInternalHooks() CortexReach/memory-lancedb-pro#62

Closed

carbaj03 force-pushed the fix/preserve-plugin-hooks-across-startup branch from 33742f6 to f260f1d Compare March 13, 2026 05:28

carbaj03 mentioned this pull request Mar 16, 2026

feat(canvas): broadcast UI commands to all connected nodes #31828

Closed

5 tasks

ACV and others added 2 commits March 16, 2026 17:35

carbaj03 force-pushed the fix/preserve-plugin-hooks-across-startup branch from f260f1d to 34c62d8 Compare March 16, 2026 16:35

openclaw-barnacle bot removed the stale Marked as stale due to inactivity label Mar 28, 2026

carbaj03 closed this Apr 1, 2026

Uh oh!

Conversation

carbaj03 commented Feb 28, 2026

Summary

Test plan

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

carbaj03 Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot commented Feb 28, 2026

Greptile Summary

Confidence Score: 3/5

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

carbaj03 Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot commented Feb 28, 2026

Uh oh!

openclaw-barnacle bot commented Mar 6, 2026

Uh oh!

carbaj03 commented Mar 6, 2026

Uh oh!

carbaj03 commented Mar 7, 2026

Uh oh!

carbaj03 commented Mar 9, 2026

Uh oh!

carbaj03 commented Mar 9, 2026

Uh oh!

carbaj03 commented Mar 11, 2026

Uh oh!

carbaj03 commented Mar 14, 2026

Uh oh!

carbaj03 commented Mar 16, 2026

Uh oh!

carbaj03 commented Mar 16, 2026

Uh oh!

carbaj03 commented Apr 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant