← Back to PRs

#20541: fix(hooks): clear internal hooks before plugins register

by ramarnat open 2026-02-19 02:27 View on GitHub →
gateway size: S
## Summary Describe the problem and fix in 2–5 bullets: - Problem: plugin-registered internal hooks (for example `session:start`) were being cleared during gateway startup when file-based internal hooks were loaded. - Why it matters: plugins that rely on internal hook callbacks silently stop working after startup. - What changed: `clearInternalHooks()` now runs once before plugin loading in `src/gateway/server.impl.ts`; clearing was removed from sidecar hook loading in `src/gateway/server-startup.ts`; regression coverage added in `src/gateway/server-startup.plugin-internal-hooks.test.ts`. - What did NOT change (scope boundary): no changes to hook file format, plugin API, channel logic, or non-startup hook behavior. ## Change Type (select all) - [x] Bug fix ## Scope (select all touched areas) - [x] Gateway / orchestration ## Linked Issue/PR - Closes # - Related #13709 ## User-visible / Behavior Changes - Plugin-registered internal hooks now persist through gateway startup and still fire after internal hook discovery runs. - No config/default changes. ## 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 (local verification), CI failure originally observed on Windows test lane - Runtime/container: Node + pnpm - Model/provider: N/A - Integration/channel (if any): plugin internal hooks - Relevant config (redacted): gateway startup with plugin registering internal hooks ### Steps 1. Start gateway with a plugin that registers an internal hook during registration. 2. Ensure internal hook discovery/load runs at startup. 3. Observe whether plugin-registered hook remains registered/fires. ### Expected - Plugin-registered hooks remain active after startup. ### Actual - After fix, plugin-registered hooks remain active; regression test passes. ## Evidence Attach at least one: - [x] Failing test/log before + passing after ## Human Verification (required) What you personally verified (not just CI), and how: - Verified scenarios: - `pnpm check` passes on branch. - `pnpm test -- src/gateway/server-startup.plugin-internal-hooks.test.ts` passes. - `pnpm test -- src/docker-setup.test.ts` passes (Windows-related test portability nudge included on this branch). - Edge cases checked: - Hook registry is cleared before plugin load, not during sidecar hook discovery. - File-based internal hooks still load. - What you did **not** verify: - Full end-to-end channel matrix/manual runtime validation across all integrations. ## 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 this PR commit(s) from `pr/internal-hooks-clear-before-plugins`. - Files/config to restore: - `src/gateway/server.impl.ts` - `src/gateway/server-startup.ts` - `src/gateway/server-startup.plugin-internal-hooks.test.ts` - Known bad symptoms reviewers should watch for: - Plugin internal hooks not firing after startup. ## Risks and Mitigations - Risk: startup ordering regressions around plugin load vs hook discovery. - Mitigation: focused regression test added for plugin-registered internal hooks surviving startup. - Risk: low-level gateway startup flow changed in hot path. - Mitigation: change is minimal (clear moved earlier; no new behavior branches added). <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR fixes a critical startup ordering bug where plugin-registered internal hooks were being cleared during gateway initialization. The fix moves `clearInternalHooks()` to run once before plugin loading (in `src/gateway/server.impl.ts:247`) and removes the clearing step from the sidecar hook loading phase (in `src/gateway/server-startup.ts:107-108`). **Key changes:** - Moved hook registry clearing to happen before plugins load, ensuring plugin-registered hooks like `session:start` survive startup - Added comprehensive regression test that creates a test plugin, registers an internal hook during plugin initialization, runs the full startup sequence, and verifies the hook still fires - Updated comments to clarify the new execution order and why hooks should not be cleared during file-based hook discovery **How the fix works:** When plugins call `api.registerHook(...)` during registration (via `loadOpenClawPlugins`), those hooks are registered to the internal hook registry using `registerInternalHook()` (`src/plugins/registry.ts:265`). Previously, when `loadInternalHooks()` was called during `startGatewaySidecars`, it would call `clearInternalHooks()` first, wiping out all plugin-registered hooks. Now the clearing happens once at the start of `startGatewayServer`, before any plugins load. <h3>Confidence Score: 5/5</h3> - This PR is safe to merge with minimal risk - it's a well-isolated bug fix with comprehensive test coverage - The fix is surgical and addresses a clear ordering bug. The change is minimal (moving one function call earlier in the startup sequence), well-documented with inline comments, and includes a regression test that validates the exact failure scenario. The test properly simulates plugin registration, hook loading, and verification that plugin hooks survive the startup sequence. - No files require special attention <sub>Last reviewed commit: 76aa9b7</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs