fix: internal hooks handlers stored in globalThis to survive bundle chunk boundaries#30631
Conversation
openclaw-barnacle
bot
added
channel: discord
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1091feff0d
ℹ️ 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".
src/config/types.channels.ts
Outdated
| * Response behavior when an unpaired (unauthorized) user sends a message. | ||
| * @default "silent" | ||
| */ | ||
| unpairedResponse?: UnpairedResponseMode; |
There was a problem hiding this comment.
channels.defaults.unpairedResponse is introduced here, but the runtime schema still rejects it because ChannelsSchema.defaults is .strict() with only groupPolicy and heartbeat allowed (src/config/zod-schema.providers.ts:27-33). That means any config trying to set unpairedResponse fails validation, so operators cannot enable code-only/branded; the code then always falls back to "silent" in getUnpairedResponseMode (src/pairing/unpaired-response.ts:10).
Useful? React with 👍 / 👎.
src/telegram/bot-handlers.ts
Outdated
| const dmAuthorized = await enforceTelegramDmAccess({ | ||
| isGroup: event.isGroup, | ||
| dmPolicy, | ||
| unpairedResponse: getUnpairedResponseMode(loadConfig()), |
There was a problem hiding this comment.
This call path always resolves DM pairing response mode from global defaults via getUnpairedResponseMode(loadConfig()), which reads only channels.defaults (src/pairing/unpaired-response.ts:10) and ignores the newly added channels.telegram(...).unpairedResponse field documented in src/config/types.telegram.ts:157-164. As a result, Telegram/account-level overrides never take effect in DM access checks, so per-account policy tuning is broken.
Useful? React with 👍 / 👎.
Greptile SummaryThis PR addresses the internal hook Major concern: This PR bundles multiple unrelated changes together, which violates single-responsibility principles and makes it difficult to review, test, and potentially rollback individual changes:
Recommendation: Consider splitting this into separate PRs, with each addressing a single concern. This would make the changes easier to review, test independently, and rollback if issues arise. Code quality: The individual changes are well-implemented with appropriate tests. The Confidence Score: 3/5
Last reviewed commit: 1091fef |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
src/hooks/internal-hooks.ts
Outdated
| if (!globalThis.__openclawHookHandlers) { | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| (globalThis as any).__openclawHookHandlers = new Map<string, InternalHookHandler[]>(); | ||
| } | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| return (globalThis as any).__openclawHookHandlers; |
There was a problem hiding this comment.
Consider adding a proper type declaration for the global property instead of using as any:
declare global {
var __openclawHookHandlers: Map<string, InternalHookHandler[]> | undefined;
}This provides better type safety while still allowing the property to be undefined initially.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/hooks/internal-hooks.ts
Line: 115-120
Comment:
Consider adding a proper type declaration for the global property instead of using `as any`:
```typescript
declare global {
var __openclawHookHandlers: Map<string, InternalHookHandler[]> | undefined;
}
```
This provides better type safety while still allowing the property to be undefined initially.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
kevinWangSheng
force-pushed
the
fix/30624-internal-hooks-globalthis
branch
from
1091fef to
efb4c55
Compare
openclaw-barnacle
bot
added
size: XS
and removed
channel: discord
2 participants
Summary
Fixes #30624 - Internal hook
message:sentnever fires due to splithandlersMap between bundle chunks.Root Cause
The internal hook system (handlers Map, registerInternalHook, triggerInternalHook) was defined as a module-level variable. When bundled into multiple chunks, each chunk got its own handlers Map instance, causing hooks registered in one chunk to be invisible to triggers in another chunk.
Solution
Store the handlers Map in globalThis (
globalThis.__openclawHookHandlers) to ensure a single shared instance across all bundle chunks.Testing