fix(cli): prevent duplicate SessionStart systemMessage render

[dimssu]

Summary

The SessionStart hook's systemMessage was rendering twice in the interactive UI (once with a [node "…"] source annotation, once without), while BeforeAgent/BeforeTool correctly rendered once.

This PR:

1. Removes the redundant direct-render path in AppContainer so the HookSystemMessage event-bus listener is the single source of truth for rendering.
2. Broadens the emission in hookEventHandler.ts to cover both 'json' and 'text' output formats, so plain-text hooks (whose stdout is promoted into systemMessage by convertPlainTextToHookOutput) aren't silently dropped — and as a bonus, plain-text BeforeAgent / BeforeTool hooks now surface their messages too (they previously never rendered anywhere).

Closes #25655.

Details

Root cause of the duplicate: AppContainer's SessionStart-hook effect was emitting result.systemMessage via two paths:

1. Direct path — historyManager.addItem({ type: INFO, text: result.systemMessage }, Date.now()) right after fireSessionStartEvent.
2. Event-bus path — coreEvents.emitHookSystemMessage(...) fires inside hookEventHandler.ts for every hook that returns a systemMessage; AppContainer's HookSystemMessage listener + coreEvents.drainBacklogs() renders it too.

BeforeAgent/BeforeTool only went through the event-bus path, which is why they weren't duplicated.

Initially, removing only the direct-render path introduced a regression for plain-text SessionStart hooks (flagged by @kamal2730 in review — thanks!), because the outputFormat === 'json' gate on the event emission meant plain-text hooks never produced a HookSystemMessage event. Broadening the gate to cover both formats keeps the behavioural contract consistent across JSON and text hooks and removes a latent gap for BeforeAgent/BeforeTool plain-text hooks as well.

Why the eslint-disable was also touched: the effect had an // eslint-disable-next-line react-hooks/exhaustive-deps directive that existed specifically to suppress the warning triggered by the now-removed historyManager.addItem call. With that call gone, the directive becomes an unused-disable and eslint --max-warnings 0 fails on it, so it had to be dropped alongside the addItem block.

Also verified:

• /clear → clearCommand.ts renders via context.ui.addItem AFTER context.ui.clear(), which wipes the event-bus-drained copy. Still exactly one render.
• Non-interactive mode (gemini.tsx) uses writeToStderr with no UI event bus. Unaffected.

Related Issues

Closes #25655

How to Validate

Unit tests (included):

npx vitest run packages/core/src/hooks/hookEventHandler.test.ts
npx vitest run packages/cli/src/ui/AppContainer.test.tsx

New / relevant cases:

• HookEventHandler > systemMessage event emission > emits HookSystemMessage for json-format hook output
• HookEventHandler > systemMessage event emission > emits HookSystemMessage for text-format hook output
• HookEventHandler > systemMessage event emission > does not emit when systemMessage is absent
• AppContainer State Management > SessionStart Hook Rendering > does not render systemMessage directly — guards the direct-render path from coming back.

End-to-end (PTY) repros I ran locally:

1. JSON hook — a SessionStart hook that outputs {"systemMessage": "hello"}.
   • Before PR: banner renders 2× in the same rendered frame.
   • After PR: banner renders 1×.
2. Plain-text hook — a SessionStart hook that just console.logs a banner.
   • Before PR: banner renders 1× (via the now-removed direct-render path).
   • With only the AppContainer removal (intermediate state): banner silently dropped (the regression @kamal2730 called out).
   • After full PR (core gate broadened): banner renders 1× via the event bus.

Build / lint / typecheck:

npm run build
npm run lint
npm run typecheck

All pass locally on macOS.

Pre-Merge Checklist

• Updated relevant documentation and README (if needed) — n/a, behavioral fix only
• Added/updated tests (if needed)
• Noted breaking changes (if any) — none; plain-text BeforeAgent/BeforeTool hooks that previously never surfaced systemMessage will now render it (treated as a bug fix, not a break)
• Validated on required platforms/methods:
  • MacOS
    • npm run
    • npx
    • Docker
    • Podman
    • Seatbelt
  • Windows
    • npm run
    • npx
    • Docker
  • Linux
    • npm run
    • npx
    • Docker

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! 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 addresses a UI bug where system messages from the SessionStart hook were being rendered twice in the interactive CLI. By consolidating the rendering logic to rely solely on the event-bus listener, the application now maintains consistent behavior with other hooks like BeforeAgent and BeforeTool, ensuring a cleaner and more predictable user experience.

Highlights

• Duplicate Message Prevention: Removed the redundant direct-render path for SessionStart system messages in AppContainer to ensure the event-bus listener remains the single source of truth.
• Test Coverage: Added a new unit test in AppContainer.test.tsx to verify that system messages are not rendered directly, preventing future regressions.
• Code Cleanup: Removed a stale eslint-disable comment that was no longer necessary after the logic refactor.

Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[gemini-code-assist]

Code Review

This pull request prevents duplicate rendering of the systemMessage from the SessionStart hook by removing the manual addition to the history manager in AppContainer.tsx, as it is now handled by an event listener. It also includes a new test case to verify this behavior and removes an unnecessary ESLint disable comment in a useEffect hook. I have no feedback to provide.

[kamal2730]

Hey @dimssu, I think there might be a small issue with removing that block.

The outputFormat === 'json' check is specific to hook outputs, but result.systemMessage in AppContainer.tsx comes from a different flow (like SessionStartEvent) and isn’t necessarily tied to hook-based JSON output.

Because of that, relying only on the HookSystemMessage listener might miss some cases where systemMessage is set directly on result. The previous logic ensured those messages were added to history regardless of hooks.

This hook-based handling is here (which explains part of the behavior), and I can also see it being rendered after that:
hookEventHandler.ts (L462–L469)

So removing this could lead to some system messages not being shown. Maybe we should verify whether all systemMessage cases are indeed covered by the hook event before removing it?

[dimssu]

Good catch, you're right. I traced it: plain-text hook outputs go through convertPlainTextToHookOutput in hookRunner.ts, which does set systemMessage, but the emit in hookEventHandler.ts:463 is gated by outputFormat === 'json', so those never reach the listener. My patch would silently drop them.

Side note while I was digging: BeforeAgent/BeforeTool plain-text hooks already don't render their systemMessage anywhere (no direct-render path for those either), so the same latent gap exists for them today.

My thinking: drop the outputFormat === 'json' check in hookEventHandler so the event bus carries both formats. Then the AppContainer removal stays clean, and BeforeAgent/BeforeTool also start surfacing text-hook messages consistently — covers this bug without leaving the adjacent one. Let me know if that sounds reasonable or if you'd rather keep it narrower.

[jacob314]

Please address this and then I will approve. Would also encourage you to get Gemini to write some tests to help verify we aren't missing cases.

[dimssu]

Thanks @jacob314. Just pushed the fix (force-pushed onto the same branch — head is now cb65b25ac).

On the approach: dropped the outputFormat === 'json' gate in hookEventHandler.ts so the HookSystemMessage event fires for both JSON and plain-text hooks. That way the AppContainer direct-render removal no longer regresses plain-text SessionStart hooks, and as a side effect plain-text BeforeAgent / BeforeTool hooks also start surfacing their messages (they silently dropped before).

On tests: added three cases in hookEventHandler.test.ts under a new systemMessage event emission block — JSON-format emission, text-format emission, and the no-systemMessage no-op. The existing AppContainer.test.tsx case that guards the direct-render path from coming back is still there. Also re-ran the PTY repro locally for both a JSON hook and a plain-text hook; both render exactly once now.

Happy to add more coverage if there's a specific case you'd like me to pin down.

[dimssu]

Thanks for the review @jacob314. Addressed in the latest push:

• Dropped the outputFormat === 'json' gate in hookEventHandler.ts so the HookSystemMessage event covers both JSON and plain-text hooks. This fixes the regression @kamal2730 flagged and also closes a latent gap where plain-text BeforeAgent / BeforeTool hooks never surfaced their messages.
• Added 3 unit tests in hookEventHandler.test.ts covering JSON-format emission, text-format emission, and the no-systemMessage no-op.
• Kept the existing AppContainer.test.tsx case that guards against the direct-render path coming back.
• Verified end-to-end with PTY scripts: a JSON SessionStart hook now renders 1× (was 2×), and a plain-text SessionStart hook still renders 1× (would have been dropped with the intermediate fix).

Let me know if you'd like any additional coverage.

[jacob314]