Skip to content

fix(desktop): fix chat infinite re-render and model selector logo#1283

Merged
AviPeltz merged 3 commits into
mainfrom
avipeltz/chat-bug
Feb 7, 2026
Merged

fix(desktop): fix chat infinite re-render and model selector logo#1283
AviPeltz merged 3 commits into
mainfrom
avipeltz/chat-bug

Conversation

@AviPeltz
Copy link
Copy Markdown
Collaborator

@AviPeltz AviPeltz commented Feb 7, 2026

Summary

  • Fix "Maximum update depth exceeded" crash when opening chat — tRPC mutation objects (startSession, stopSession) were in useEffect dependency arrays and get recreated every render, causing an infinite loop. Moved them behind refs and replaced the dual-path connect logic with a single reactive effect gated on sessionReady state + config.proxyUrl.
  • Fix model selector logos not loading — https://models.dev was missing from the Content Security Policy img-src directive, so Electron blocked the external SVG fetches.

Test plan

  • Open the chat pane — should load without crashing
  • Verify the Anthropic logo appears in the model selector trigger and dropdown items
  • Switch sessions / workspaces and confirm chat reconnects correctly

Summary by CodeRabbit

  • Bug Fixes

    • More reliable chat session lifecycle: session startup and connection gating have been stabilized to reduce missed or premature connections.
  • Chores

Fix "Maximum update depth exceeded" crash when opening chat by moving
tRPC mutation objects behind refs so useEffect deps stay stable.

Add https://models.dev to CSP img-src so model selector logos load.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Feb 7, 2026

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Updates Content-Security-Policy to allow https://models.dev for images and refactors ChatInterface session lifecycle to use refs and an explicit sessionReady flag, decoupling connection logic from mutation isSuccess state.

Changes

Cohort / File(s) Summary
CSP Configuration
apps/desktop/src/renderer/index.html
Added https://models.dev to the CSP img-src / image-src directives (2 lines changed).
ChatInterface session lifecycle
apps/desktop/src/renderer/screens/main/components/.../ChatInterface.tsx
Introduced sessionReady state; replaced direct mutation usage with startSessionRef / stopSessionRef; effects now reset sessionReady on mount, use refs for mutate/cleanup, removed startSession/stopSession from dependency arrays; connection logic now triggers when sessionReady is true and config.proxyUrl exists.

Sequence Diagram(s)

sequenceDiagram
  participant UI as Client (ChatInterface)
  participant API as Session API (start/stop mutations)
  participant Proxy as Proxy/Connection (config.proxyUrl)

  UI->>API: startSessionRef.mutate(sessionId, cwd)
  API-->>UI: start success
  UI->>UI: set sessionReady = true
  alt config.proxyUrl exists and sessionReady
    UI->>Proxy: establish connection to proxyUrl
    Proxy-->>UI: connection established
  else no proxy or not ready
    UI-->>UI: wait for sessionReady and proxyUrl
  end
  Note right of UI: on unmount -> stopSessionRef.mutate(...)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

"I nibble on code and hop with glee,
Sessions wake up and images roam free,
Refs hold the keys, connections take flight,
A rabbit's cheer for changes done right! 🐇✨"

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title directly addresses both main fixes: the chat infinite re-render issue and the model selector logo problem, accurately reflecting the changeset.
Description check ✅ Passed The description covers the key changes and includes a test plan, but lacks explicit Type of Change checkbox selection and Related Issues section.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch avipeltz/chat-bug

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.

Replace dual-path doConnect (onSuccess + ref-based useEffect) with
a single useEffect that watches sessionReady state + config.proxyUrl.
@AviPeltz AviPeltz merged commit dab2822 into main Feb 7, 2026
5 of 6 checks passed
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Feb 7, 2026

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

  • ⚠️ Neon database branch
  • ⚠️ Electric Fly.io app

Thank you for your contribution! 🎉

@Kitenite Kitenite deleted the avipeltz/chat-bug branch February 9, 2026 01:18
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