fix(chat): remove confirmation modal when closing chat tab

[arnestrickmann]

Summary

• Remove the DeleteChatModal confirmation dialog that appeared when closing a chat tab
• Inline the chat deletion logic directly into handleCloseChat, making tab close instant
• Delete the now-unused DeleteChatModal component

Motivation

The confirmation modal added unnecessary friction when closing chat tabs. Since chats can be easily recreated and this is a lightweight action, the extra confirmation step was more disruptive than helpful.

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
docs	Ready	Preview, Comment	Mar 3, 2026 11:59pm

[greptile-apps]

Greptile Summary

This PR removes the DeleteChatModal confirmation dialog and inlines the chat deletion logic directly into handleCloseChat, making tab close instant. The change is well-motivated and the code refactor is clean, but two concerns were found:

• No busy-state guard: The handleCloseChat function does not check busyByConversationId[conversationId] before proceeding. Clicking ✕ on a tab that is actively running will immediately dispose the terminal process and permanently delete the conversation and all its messages — with no warning. The old confirmation modal at least surfaced the irreversibility of the action.
• Unguarded async operations: The async DB calls (deleteConversation, getConversations, setActiveConversation) are not wrapped in a try/catch. A failure after terminalSessionRegistry.dispose() leaves the terminal gone but the conversation still in the DB, and the error surfaces as an unhandled promise rejection since the callers don't await or .catch() the result.

Confidence Score: 3/5

• Safe to merge if the team accepts instant deletion of busy chats; the busy-state guard omission is a meaningful UX regression that can cause silent data loss.
• The inlining of logic is correct and the dead code is cleanly removed, but the missing busy-state check introduces a regression where an actively-running chat can be irreversibly deleted mid-generation with no confirmation or warning. The lack of error handling around the DB operations also risks leaving app state inconsistent on failure.
• src/renderer/components/ChatInterface.tsx — specifically the handleCloseChat callback around lines 517–560.

Important Files Changed

Filename	Overview
src/renderer/components/ChatInterface.tsx	Inlines deletion logic from the removed modal into handleCloseChat; however, lacks a busy-state guard (chat can be deleted while actively running) and has no error handling around the async DB operations, which can leave state inconsistent on failure.
src/renderer/components/DeleteChatModal.tsx	File deleted entirely as part of the modal removal — clean deletion with no dangling references.

Sequence Diagram

sequenceDiagram
    actor User
    participant Tab as Chat Tab (UI)
    participant HC as handleCloseChat
    participant TR as terminalSessionRegistry
    participant DB as rpc.db
    participant State as React State

    User->>Tab: Click ✕ (close button)
    Tab->>HC: handleCloseChat(conversationId)
    alt conversations.length <= 1
        HC-->>User: toast "Cannot Close"
    else
        Note over HC: ⚠️ No busy-state check here
        HC->>TR: dispose(terminalId)
        HC->>DB: deleteConversation(conversationId)
        HC->>DB: getConversations(taskId)
        DB-->>HC: updatedConversations
        HC->>State: setConversations(updatedConversations)
        alt deleted chat was active
            HC->>DB: setActiveConversation(newActive)
            HC->>State: setActiveConversationId / setAgent
        end
        HC->>Window: dispatchEvent('emdash:conversations-changed')
    end

Loading

_{Last reviewed commit: 9cdd14c}

[greptile-apps]

No guard against closing a busy/running chat

The handleCloseChat function now immediately disposes the terminal and deletes the conversation without checking whether the chat is actively processing a request. When busyByConversationId[conversationId] === true, clicking the ✕ button will:

1. Dispose the running terminal process mid-generation
2. Permanently delete all conversation messages from the DB

Before this PR, the confirmation modal at least surfaced the warning "This will permanently delete this chat and all its messages. This action cannot be undone." — giving the user a chance to cancel. Now, there is no protection at all for a busy chat.

Consider adding a busy-state guard at the top of the function:

Suggested change

	const handleCloseChat = useCallback(
	async (conversationId: string) => {
	if (conversations.length <= 1) {
	toast({
	title: 'Cannot Close',
	description: 'Cannot close the last chat',
	variant: 'destructive',
	});
	return;
	}
	if (busyByConversationId[conversationId]) {
	toast({
	title: 'Cannot Close',
	description: 'Cannot close a chat while it is running',
	variant: 'destructive',
	});
	return;
	}

[greptile-apps]

No error handling around async DB operations

If rpc.db.deleteConversation or rpc.db.getConversations throws, the function exits after the terminal has already been disposed — leaving the conversation still in the DB while its terminal session is gone. Additionally, since handleCloseChat is now async and is called from click/keyboard event handlers and a window event listener without .catch(), any rejection becomes an unhandled promise rejection.

Wrapping the body in a try/catch (and reverting the terminal dispose on failure) would keep state consistent:

const terminalToDispose = makePtyId(convAgent, 'chat', conversationId);
terminalSessionRegistry.dispose(terminalToDispose);
try {
  await rpc.db.deleteConversation(conversationId);
  // ... rest of logic
} catch (err) {
  // Re-register or notify; at minimum surface the error
  toast({ title: 'Error', description: 'Failed to close chat', variant: 'destructive' });
}