fix: persist provider on default conversation to prevent tab flickering

[arnestrickmann]

Summary

Fixes a bug where conversation tabs would flicker or show incorrect agent names because the default conversation's provider field was not being saved atomically during creation.

Changes

• DatabaseService.getOrCreateDefaultConversation: Accept optional provider parameter and persist it when creating new conversations. Backfills provider on existing conversations that were saved as null.
• dbIpc.ts: Update IPC signature to pass { taskId, provider } object instead of bare taskId.
• ChatInterface.tsx:
  • Pass task's agent as provider when calling getOrCreateDefaultConversation, eliminating the separate saveConversation round-trip
  • Add cancellation cleanup to the conversation-loading effect to prevent stale state updates on rapid task switches
  • Replace || agent fallbacks with ?? 'claude' to avoid leaking the current agent selection into unrelated conversation tabs
• taskCreationService.ts: Pass provider when creating default conversation during task creation with issue context

Test plan

• Create a new task with a non-Claude agent — verify the tab shows the correct agent name immediately
• Switch rapidly between tasks — verify no flickering or stale conversation state
• Open an existing task that was created before this change — verify provider is backfilled correctly
• Delete a conversation tab — verify correct terminal is disposed

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
docs	Ready	Preview, Comment	Mar 6, 2026 4:47am

[greptile-apps]

Greptile Summary

This PR fixes tab flickering caused by the default conversation's provider field not being persisted atomically on creation, requiring a second round-trip saveConversation call that could be observed as an intermediate render. The fix is applied across the full stack: the DatabaseService now accepts and saves provider in a single call (and backfills existing null rows), the IPC handler is updated to pass the new argument object, and ChatInterface is tightened up with a cancellation guard on the async effect plus consistent ?? 'claude' fallbacks for tab rendering.

Key changes and findings:

• DatabaseService.getOrCreateDefaultConversation: Solid implementation — all three return paths (disabled DB, existing row, new row) correctly handle provider ?? null, and the backfill update is safe because concurrent writers would set the same value.
• dbIpc.ts / taskCreationService.ts: Straightforward signature updates; no issues.
• ChatInterface.tsx (line 208): The || agent fallback leaks the previous task's agent value for tasks without agentId into the database, defeating the PR's intent to eliminate stale-agent persistence. Should use ?? 'claude' instead.
• ChatInterface.tsx (line 224): The || taskAgent fallback contradicts the "provider guaranteed" comment above it and should use ?? for consistency with the overall pattern.

Confidence Score: 3/5

• PR introduces correct core fixes (atomic provider persistence, cancellation guard) but contains two unresolved logic issues in ChatInterface.tsx that cause wrong agent values to be persisted for legacy tasks.
• The two verified issues on lines 208 and 224 are real logic bugs: (1) line 208 uses || agent which captures the previous task's agent for tasks without agentId—the opposite of the PR's intent; (2) line 224's || fallback contradicts its preceding comment. These should be fixed before merging. The rest of the PR (DatabaseService, dbIpc, taskCreationService) is solid.
• src/renderer/components/ChatInterface.tsx — lines 208 and 224 both need fallback operator changes from || to ??.

Sequence Diagram

sequenceDiagram
    participant UI as ChatInterface (renderer)
    participant IPC as dbIpc (main)
    participant DB as DatabaseService (main)

    Note over UI: task.id or task.agentId changes
    UI->>UI: set cancelled=false
    UI->>IPC: getConversations(task.id)
    IPC->>DB: getConversations(task.id)
    DB-->>UI: conversations[]

    alt conversations exist
        UI->>UI: setConversations / setActiveConversationId / setAgent
        opt no active conversation found
            UI->>IPC: setActiveConversation({ taskId, conversationId })
        end
        UI->>UI: if (!cancelled) setConversationsLoaded(true)
    else no conversations (first load / legacy task)
        UI->>UI: taskAgent = task.agentId || agent (stale!)
        UI->>IPC: getOrCreateDefaultConversation({ taskId, provider: taskAgent })
        IPC->>DB: getOrCreateDefaultConversation(taskId, provider)

        alt existing row with null provider → backfill
            DB->>DB: UPDATE conversations SET provider=? WHERE id=?
            DB-->>UI: conversation (provider backfilled)
        else existing row with provider
            DB-->>UI: conversation (unchanged)
        else no row → create
            DB->>DB: saveConversation({ id, taskId, provider, isMain, isActive })
            DB-->>UI: conversation (provider saved atomically)
        end

        UI->>UI: if (!cancelled) setConversations / setAgent / setConversationsLoaded
    end

    Note over UI: cleanup: cancelled=true on unmount/re-run

Loading

_{Last reviewed commit: 9b3764a}

[greptile-apps]

Stale agent still leaks via || agent fallback

The PR description states a goal of "replacing || agent fallbacks with ?? 'claude' to avoid leaking the current agent selection into unrelated conversation tabs", but this line still uses || agent as a fallback for tasks without task.agentId.

Because agent is intentionally excluded from the effect's dependency array (line 234: // agent is intentionally not included as a dependency), it captures the agent value from the previous task at effect-run time. For tasks created before agentId was introduced (i.e. task.agentId is null/undefined), this causes the old task's agent to be atomically saved as the provider for the new task's default conversation — exactly the stale-agent leak the PR aims to fix.

Using ?? 'claude' here is consistent with every other fallback changed in this PR and avoids persisting a potentially wrong provider to the database.

Suggested change

	const taskAgent = (task.agentId || agent) as string;
	const taskAgent = (task.agentId ?? 'claude') as string;

[greptile-apps]

Fallback contradicts "guaranteed provider" comment

The comment directly above (line 215) reads "Provider is guaranteed by getOrCreateDefaultConversation (saves atomically)", yet this line still falls back to taskAgent via ||. These two statements are in tension:

• If the guarantee holds (provider is always non-null when taskAgent is truthy, which it always is here), the || taskAgent branch is dead code.
• If the guarantee does not hold for some edge case, the comment is misleading.

Using ?? over || is also more precise: || would replace an empty string "" with taskAgent, while ?? only replaces null/undefined, which is consistent with the Conversation type's nullable provider field.

Suggested change

	setAgent((defaultConversation.provider || taskAgent) as Agent);
	setAgent((defaultConversation.provider ?? taskAgent) as Agent);