Skip to content

Conversation

@avrum
Copy link
Collaborator

@avrum avrum commented Feb 8, 2026

fix(main): fix model switching — duplicate events, broken sends, and lost context

Problem

Switching models in the model picker caused three issues:

  1. Duplicate messages — every response appeared 2×, 3×, up to N× in the chat, worsening with each subsequent model switch
  2. Broken sends — if the model switch failed, the tab's session ID became undefined and all subsequent messages failed with Session not found: undefined
  3. Lost context — after switching, the new model had no knowledge of the prior conversation

Root Cause

PR #274 (a6f0230) refactored copilot:setModel to use client.resumeSession() instead of destroy() + createNewSession(), intending to preserve conversation context across model changes. Inspection of the @github/copilot-sdk source revealed this approach
was fundamentally broken:

  1. resumeSession() does not pass the model parameter to the server (client.js:367). Only createSession() sends model in the session.create RPC call (client.js:307). The model was never actually changing on the server side.
  2. Each resumeSession() call accumulates a server-side event subscription on the connection without releasing the previous one. The server sends N copies of every event after N resume calls — confirmed by logs showing event count grow by exactly +1 per
    switch (3×→4×→5×→6×→7×).
  3. The session.destroy() call before resumeSession() was contradictory — it killed the server-side session, so the subsequent resumeSession() failed with "Session not found". After the failure, the session was already deleted from the local map, leaving
    the tab with a stale session ID. On the next attempt, the handler returned { model } without a sessionId, setting the tab ID to undefined.

Changes

src/main/main.ts

Model switching — copilot:setModel handler:

  • Replaced resumeSession() with destroy() + createNewSession(). destroy() cleanly releases the server-side event subscription, and createNewSession() properly passes the model via the session.create RPC.
  • Before destroying the old session, captures the full conversation history via session.getMessages() and injects it into the new session's systemMessage as a ## Previous Conversation Context section, giving the new model continuity. Gracefully degrades
    if message retrieval fails.

Event forwarding consolidation:

  • Replaced 3 duplicate inline session.on() handlers (early startup resume, late startup resume, resumePreviousSession) with the shared registerSessionEventForwarding() function. Eliminates ~160 lines of duplicated code and ensures all code paths get
    consistent event handling — the inline handlers were missing session.error, session.usage_info, and session.compaction_* events.

Event handler lifecycle management:

  • registerSessionEventForwarding() now returns the SDK's unsubscribe function (from session.on()), stored as unsubscribeEvents in SessionState.
  • Unsubscribe is called during session cleanup: model changes, resumeDisconnectedSession(), and session destruction.
  • Added a stale handler guard — the event handler verifies that the session object it was registered on is still the active session in the sessions map before processing events, as a safety net against leaked handlers.

src/renderer/App.tsx

  • handleModelChange now validates that the response contains a sessionId before updating tab state, preventing the tab from being assigned undefined as its ID.

Testing

  • npm run build ✅
  • npm test — 384 tests passed ✅
  • Manual verification: model switching works on first click, no duplicate messages after repeated switches, new model receives conversation context from previous session

maxtara
maxtara previously approved these changes Feb 9, 2026
…story parameter and enhancing session resumption
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.

2 participants