feat(hooks): Hook Event Handling - #9097
Conversation
Summary of ChangesHello @Edilmo, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a robust and extensible hook system into the CLI. The core of this system is the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive hook system, which is a significant and valuable addition. The architecture with a planner, runner, aggregator, and translator is well-structured. The code is generally of high quality with good test coverage for the new components.
I've identified a few high-severity issues that should be addressed:
- A bug in telemetry logging that omits crucial hook input data.
- A command parsing issue in the hook runner that will break hooks with quoted arguments.
- Missing test coverage for the sequential hook execution logic.
Addressing these points will improve the observability, correctness, and robustness of the new hook system.
ghost
removed
the
ae91549 to
5fab2eb
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a robust, message-bus-oriented hook event handling system, which is a great step towards decoupling integrations. The implementation is comprehensive, including planning, execution, aggregation, and telemetry for hooks. The new HookEventHandler and the request-response pattern on the MessageBus are well-designed. My review focuses on improving type safety in a few key areas to prevent potential runtime errors from malformed message payloads. I've identified some unsafe type assertions and duplicated logic that could be refactored for better robustness and maintainability.
Merge latest upstream changes from google-gemini/gemini-cli main branch. ## Merge Conflict Resolution **File:** packages/core/src/services/chatCompressionService.ts **Conflict:** Both branches modified the compression service API: - main: Migrated to new model config system (getBaseLlmClient, modelConfigKey) - feat/deliberate-compaction: Added goal-focused compression (userGoal parameter) **Resolution:** Combined both changes: - Used new model config API from main (getBaseLlmClient, modelStringToModelConfigAlias, abortSignal) - Preserved goal-focused enhancement (getChatCompressionPrompt(userGoal)) - Result: New API + goal awareness working together **Additional Changes:** - Updated chatCompressionService.test.ts to use new BaseLlmClient API - Changed all test mocks from getContentGenerator to getBaseLlmClient - Added BaseLlmClient import and proper type casts - Fixed linting: changed messagesPreserved/messagesCompressed to const - Build passes successfully ## Upstream Changes Merged From google-gemini/gemini-cli main (95693e2..d14779b): - feat(core): Land bool for alternate system prompt (google-gemini#13764) - feat(hooks): Hook Agent Lifecycle Integration (google-gemini#9105) - feat(hooks): Hook Event Handling (google-gemini#9097) - fix: Minor improvements to configs and getPackageJson (google-gemini#12510) - feat(hooks): Hook Telemetry Infrastructure (google-gemini#9082) - feat(core): Migrate chatCompressionService to model configs (google-gemini#12863) - Add session subtask in /stats command (google-gemini#13750) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
4 participants
TLDR
Creates HookEventHandler that coordinates the complete hook
execution pipeline via MessageBus, achieving complete decoupling of integration
points from hook system internals.
Part of full hook system implementation here
Dive Deeper
This is the central coordinator implementing a
MessageBus-mediated architecture. HookEventHandler subscribes to
HOOK_EXECUTION_REQUESTmessages, enriches them with base fields (session_id,cwd, timestamp), translates SDK types using HookTranslator, coordinates the
pipeline (planning → execution → aggregation → telemetry), and publishes full
HOOK_EXECUTION_RESPONSEmessages. The MessageBusrequest()method providesRPC-style communication with internal correlation ID management. The
createHookOutput()factory ensures proper type reconstruction from responses.Integration points achieve complete decoupling - they only know about MessageBus
and pass event-specific data.
Reviewer Test Plan
Testing Matrix
Linked issues / bugs
Closes #9096