Skip to content

fix(desktop): fix notification indicator not showing on hook events#456

Merged
Kitenite merged 1 commit intomainfrom
debug-indicator-notifs
Dec 21, 2025
Merged

fix(desktop): fix notification indicator not showing on hook events#456
Kitenite merged 1 commit intomainfrom
debug-indicator-notifs

Conversation

@Kitenite
Copy link
Copy Markdown
Collaborator

@Kitenite Kitenite commented Dec 21, 2025

Summary

  • Fix tRPC subscription using async generator pattern which doesn't work with trpc-electron IPC transport - changed to use observable pattern from @trpc/server/observable
  • Fix stale closure issue in useAgentHookListener where activeWorkspace was captured at subscription creation time instead of using current value via ref
  • Update AGENTS.md with tRPC subscription best practices to prevent this issue in the future

Test plan

  • Run Claude Code in a terminal pane and let it complete a task
  • Verify the red notification indicator appears on the tab when the agent completes (if not viewing that pane)
  • Verify clicking the native notification focuses the correct tab/pane

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation

    • Fixed typo in interprocess communication terminology
    • Added tRPC Subscriptions implementation guidance with code examples
  • Bug Fixes

    • Improved reliability of subscription event delivery in notifications
    • Fixed workspace context detection issue in agent hook listener

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 21, 2025

Warning

Rate limit exceeded

@Kitenite has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 22 minutes and 49 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between e68389e and 3e8b495.

📒 Files selected for processing (3)
  • apps/desktop/AGENTS.md (1 hunks)
  • apps/desktop/src/lib/trpc/routers/notifications.ts (2 hunks)
  • apps/desktop/src/renderer/stores/tabs/useAgentHookListener.ts (3 hunks)

Walkthrough

The pull request refactors tRPC subscriptions from async generators to the observable pattern, adds documentation and examples for this approach, and fixes a stale closure issue in the notification hook listener by using a React ref to track the active workspace.

Changes

Cohort / File(s) Change Summary
Documentation
apps/desktop/AGENTS.md
Corrects typo ("communnication" → "communication"); adds new "tRPC Subscriptions" section with guidance on using @trpc/server/observable for trpc-electron IPC transport; includes correct example using observable with emit.next and cleanup, and a wrong example showing async generators do not work; adds note on tsconfig.json alias usage.
Notification Subscription Refactor
apps/desktop/src/lib/trpc/routers/notifications.ts
Replaces async generator-based subscription with observable pattern; removes internal queue and polling loop; introduces per-event listeners that push events via emit.next for agent-complete and focus-tab events; replaces try/finally lifecycle with simple teardown function for listener cleanup; imports observable from @trpc/server/observable.
React Hook Closure Fix
apps/desktop/src/renderer/stores/tabs/useAgentHookListener.ts
Adds useRef import and introduces activeWorkspaceRef to hold current active workspace; replaces direct activeWorkspace?.id access with activeWorkspaceRef.current?.id in subscription condition to prevent stale closure issues.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10–15 minutes

  • notifications.ts: Verify observable pattern implementation, listener registration, and proper cleanup logic when subscription tears down.
  • useAgentHookListener.ts: Confirm ref usage correctly captures active workspace and eliminates stale closure in the subscription callback.

Possibly related PRs

Poem

🐰 Observable streams flow clean and bright,
No stale closures lurking in the night,
Refs hold fast where workspaces are born,
From async gens to patterns reborn!
Hop along, dear tRPC, to async no more!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main fix: addressing a notification indicator issue triggered by hook events, which is the primary purpose of this PR.
Description check ✅ Passed The PR description covers the main changes, root causes, and testing approach, though it deviates from the template structure with a custom Summary and Test plan format instead of the specified template sections.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Kitenite Kitenite force-pushed the debug-indicator-notifs branch 2 times, most recently from 05c1f99 to 3e54678 Compare December 21, 2025 04:33
- Fix tRPC subscription using async generator pattern which doesn't work
  with trpc-electron IPC transport. Changed to use observable pattern.
- Fix stale closure issue in useAgentHookListener where activeWorkspace
  was captured at subscription creation time instead of using current value.
- Update AGENTS.md with tRPC subscription best practices.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@Kitenite Kitenite force-pushed the debug-indicator-notifs branch from 3e54678 to 3e8b495 Compare December 21, 2025 04:35
@Kitenite Kitenite merged commit 4457d7e into main Dec 21, 2025
5 checks passed
@Kitenite Kitenite deleted the debug-indicator-notifs branch December 21, 2025 04:39
@github-actions
Copy link
Copy Markdown
Contributor

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

Service Status
Neon Database (Neon) ⚠️

Thank you for your contribution! 🎉


Preview resources have been processed for cleanup

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant