Greptile Summary

This PR fixes a real startup-lifecycle bug: clearInternalHooks() was wiping plugin-registered hooks (e.g., memory-core's gateway:startup dreaming handler) because both file hooks and plugin hooks shared a single registry. The fix separates them into two global-singleton Maps (handlers for file hooks, pluginHandlers for plugin hooks), narrows clearInternalHooks() to file hooks only, adds clearAllInternalHooks() for test isolation, and updates registry.ts to pass { source: \"plugin\" } when registering plugin hooks. The core logic is correct and the changed test files are properly updated.

Confidence Score: 5/5

Safe to merge; the fix is correct and well-tested across the changed files.

The core logic is sound: two separate global-singleton maps, narrowed clearInternalHooks(), correct { source: 'plugin' } wiring in registry.ts, and updated test files. The only finding is a P2 fragility in an untouched file (src/plugins/loader.test.ts) where clearInternalHooks() now has weaker semantics than before — but this is not currently breaking and does not affect the primary fix path.

src/plugins/loader.test.ts — two inline clearInternalHooks() calls should ideally become clearAllInternalHooks() to match the rest of the test suite after this PR's semantic change.

This is a comment left during a code review.
Path: src/plugins/loader.test.ts
Line: 1463-1483

Comment:
**`clearInternalHooks()` no longer provides full isolation here**

After this PR, `clearInternalHooks()` only clears file-based hooks; plugin hooks registered via `{ source: "plugin" }` survive. This test's assertion at line 1481 (`getRegisteredEventKeys().toEqual([])`) passes today because the only other `api.registerHook()` call in this file (line 2354, "rejects duplicate plugin registrations") runs *after* this test. However, if a future test above line 1450 activates a plugin that calls `api.registerHook()`, those plugin hooks would survive the `clearInternalHooks()` at line 1463 and break the assertion.

Consider replacing both `clearInternalHooks()` calls in this test (and importing `clearAllInternalHooks`) to match the same update applied to the other hook test files in this PR:

```suggestion
    clearAllInternalHooks();
    const scoped = loadOpenClawPlugins({
```

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

_{Reviews (1): Last reviewed commit: "test(hooks): update tests to use clearAl..." | Re-trigger Greptile}

Closing this as implemented after Codex review.

Current main already preserves plugin-registered internal hooks across internal hook refreshes, and it also covers the duplicate-registration risk raised in PR review, so there is no remaining core change for PR #64340 to land.

What I checked:

• File-hook refresh is targeted, not global: loadInternalHooks() starts by calling resetLoadedInternalHooks(), which unregisters only the handlers tracked in loadedHookRegistrations; it does not clear the full internal-hook registry. (src/hooks/loader.ts:51, 11804a484ded)
• Current main has an explicit regression test for the reported behavior: src/hooks/loader.test.ts now asserts that a pre-registered gateway:startup hook is still present after loadInternalHooks() runs and still fires once. (src/hooks/loader.test.ts:264, 11804a484ded)
• Startup still exercises the same sequence safely: Gateway startup loads startup plugins first, then loads internal hooks, then dispatches gateway:startup; combined with the targeted reset above, that preserves plugin hook registrations through startup refresh. (src/gateway/server-startup-post-attach.ts:230, 11804a484ded)
• Repeated activating loads already replace old plugin hook registrations: Plugin hook registration first unregisters any prior active registrations for the same hook name before registering the new handlers. (src/plugins/registry.ts:514, 11804a484ded)
• Main covers the PR review’s duplicate-fire concern: src/plugins/loader.test.ts verifies two activating loads of the same plugin still yield exactly one gateway:startup invocation, addressing the review comment that the PR approach could duplicate plugin handlers. (src/plugins/loader.test.ts:2756, 11804a484ded)

So I’m closing this as already implemented rather than keeping a duplicate issue open.

Review notes: reviewed against 11804a484ded; fix evidence: commit 11804a484ded.