MAESTRO: Address PR #486 review comments from CodeRabbit and Greptile

- TerminalView: Guard duplicate PTY spawns with in-flight ref; fix
  missing useCallback deps (onTabPidChange, onTabStateChange,
  sessionSshRemoteConfig); prevent repeated loading message writes
  per tab; pass sessionSshRemoteConfig through to spawnTerminalTab IPC
- TabBar/TerminalTabItem: Add tabIndex, role, aria-selected, onFocus,
  onBlur, onKeyDown for full keyboard accessibility
- TabBar auto-scroll: include activeTerminalTabId and inputMode so
  terminal tabs scroll into view on selection
- TabBar unread filter: gate terminal tabs by inputMode === 'terminal'
  to prevent leaking them into AI-mode filtered view
- TerminalSearchBar: add type/aria-label to icon buttons; clear
  terminal engine search state on bar close or empty query; guard
  Enter navigation when query is empty
- TerminalTabRenameModal: sync value with currentName/isOpen via
  useEffect to fix stale state when modal reopens for a new tab
- useMainKeyboardHandler: scope Ctrl-passthrough to macOS only to
  avoid breaking Ctrl+F/W/K on Windows/Linux; always preventDefault on
  closeTab shortcut in terminal mode regardless of tab count
- tabStore: validate close before kill to prevent orphaned PTY when
  closeTerminalTabHelper refuses (last-tab guard)
- useDebouncedPersistence: validate and normalize activeTerminalTabId
  against cleaned terminal tabs before persisting
- terminalTabHelpers: always overwrite exitCode on state transitions to
  clear stale values from prior process runs

Author: pedramamini

src/renderer/components/TabBar.tsx

@@ -1782,15 +1782,29 @@ const TerminalTabItem = memo(function TerminalTabItem({
 		<div
 			ref={setTabRef}
 			data-tab-id={tab.id}
+			tabIndex={0}
+			role="tab"
+			aria-selected={isActive}
 			className={`
         relative flex items-center gap-1.5 px-3 py-1.5 cursor-pointer
-        transition-all duration-150 select-none shrink-0
+        transition-all duration-150 select-none shrink-0 outline-none
         ${isDragging ? 'opacity-50' : ''}
         ${isDragOver ? 'ring-2 ring-inset' : ''}
       `}
 			style={tabStyle}
 			title={tab.cwd ? `${tab.shellType} — ${tab.cwd}` : tab.shellType}
 			onClick={handleTabSelect}
+			onFocus={handleMouseEnter}
+			onBlur={() => {
+				handleMouseLeave();
+				setOverlayOpen(false);
+			}}
+			onKeyDown={(e) => {
+				if (e.key === 'Enter' || e.key === ' ') {
+					e.preventDefault();
+					handleTabSelect();
+				}
+			}}
 			onDoubleClick={handleDoubleClick}
 			onMouseDown={handleMouseDown}
 			onMouseEnter={handleMouseEnter}
@@ -2056,8 +2070,11 @@ function TabBarInner({
 		requestAnimationFrame(() => {
 			requestAnimationFrame(() => {
 				const container = tabBarRef.current;
-				// When a file tab is active, scroll to it; otherwise scroll to the active AI tab
-				const targetTabId = activeFileTabId || activeTabId;
+				// Scroll to the currently active tab across AI/file/terminal modes
+				const targetTabId =
+					inputMode === 'terminal'
+						? activeTerminalTabId || activeTabId
+						: activeFileTabId || activeTabId;
 				const tabElement = container?.querySelector(
 					`[data-tab-id="${targetTabId}"]`
 				) as HTMLElement | null;
@@ -2083,7 +2100,7 @@ function TabBarInner({
 				}
 			});
 		});
-	}, [activeTabId, activeFileTabId, activeTabName, showUnreadOnly]);
+	}, [activeTabId, activeFileTabId, activeTerminalTabId, inputMode, activeTabName, showUnreadOnly]);
 
 	// Can always close tabs - closing the last one creates a fresh new tab
 	const canClose = true;
@@ -2108,10 +2125,10 @@ function TabBarInner({
 			if (ut.type === 'file') {
 				return ut.id === activeFileTabId;
 			}
-			// Terminal tabs: only show if active
-			return ut.id === activeTerminalTabId;
+			// Terminal tabs: only show if active in terminal mode
+			return inputMode === 'terminal' && ut.id === activeTerminalTabId;
 		});
-	}, [unifiedTabs, showUnreadOnly, activeTabId, activeFileTabId, activeTerminalTabId]);
+	}, [unifiedTabs, showUnreadOnly, activeTabId, activeFileTabId, activeTerminalTabId, inputMode]);
 
 	const handleDragStart = useCallback((tabId: string, e: React.DragEvent) => {
 		e.dataTransfer.effectAllowed = 'move';

src/renderer/components/TerminalSearchBar.tsx

@@ -27,18 +27,21 @@ export const TerminalSearchBar = memo(function TerminalSearchBar({
 	const { registerLayer, unregisterLayer } = useLayerStack();
 	const onCloseRef = useRef(onClose);
 	onCloseRef.current = onClose;
+	const onSearchRef = useRef(onSearch);
+	onSearchRef.current = onSearch;
 
-	// Auto-focus input when opened
+	// Auto-focus input when opened; clear search state in terminal engine when closed
 	useEffect(() => {
 		if (isOpen) {
 			const timer = setTimeout(() => {
 				inputRef.current?.focus();
 			}, 0);
 			return () => clearTimeout(timer);
 		} else {
-			// Reset query when closed
+			// Reset local state and clear any active search highlight in the terminal engine
 			setQuery('');
 			setHasResults(true);
+			onSearchRef.current('');
 		}
 	}, [isOpen]);
 
@@ -65,12 +68,14 @@ export const TerminalSearchBar = memo(function TerminalSearchBar({
 			const found = onSearch(value);
 			setHasResults(found);
 		} else {
+			// Clear search highlight in terminal engine when query is emptied
+			onSearch('');
 			setHasResults(true);
 		}
 	};
 
 	const handleKeyDown = (e: React.KeyboardEvent<HTMLInputElement>) => {
-		if (e.key === 'Enter') {
+		if (e.key === 'Enter' && query) {
 			e.preventDefault();
 			if (e.shiftKey) {
 				const found = onSearchPrevious();
@@ -125,6 +130,8 @@ export const TerminalSearchBar = memo(function TerminalSearchBar({
 				</span>
 			)}
 			<button
+				type="button"
+				aria-label="Previous match"
 				onClick={handlePrevious}
 				disabled={!query}
 				title="Previous match (Shift+Enter)"
@@ -134,6 +141,8 @@ export const TerminalSearchBar = memo(function TerminalSearchBar({
 				<ChevronUp size={14} />
 			</button>
 			<button
+				type="button"
+				aria-label="Next match"
 				onClick={handleNext}
 				disabled={!query}
 				title="Next match (Enter)"
@@ -143,6 +152,8 @@ export const TerminalSearchBar = memo(function TerminalSearchBar({
 				<ChevronDown size={14} />
 			</button>
 			<button
+				type="button"
+				aria-label="Close search"
 				onClick={onClose}
 				title="Close (Escape)"
 				className="p-0.5 rounded opacity-70 hover:opacity-100"

src/renderer/components/TerminalTabRenameModal.tsx

@@ -1,4 +1,4 @@
-import React, { memo, useRef, useState } from 'react';
+import React, { memo, useRef, useState, useEffect } from 'react';
 import type { Theme } from '../types';
 import { MODAL_PRIORITIES } from '../constants/modalPriorities';
 import { Modal, ModalFooter } from './ui/Modal';
@@ -22,6 +22,13 @@ export const TerminalTabRenameModal = memo(function TerminalTabRenameModal(
 	const inputRef = useRef<HTMLInputElement>(null);
 	const [value, setValue] = useState(currentName ?? '');
 
+	// Sync input value when modal reopens for a different tab
+	useEffect(() => {
+		if (isOpen) {
+			setValue(currentName ?? '');
+		}
+	}, [isOpen, currentName]);
+
 	if (!isOpen) return null;
 
 	const handleSave = () => {

src/renderer/components/TerminalView.tsx

@@ -1,4 +1,4 @@
-import { memo, forwardRef, useImperativeHandle, useRef, useEffect, useCallback } from 'react';
+import { memo, forwardRef, useImperativeHandle, useRef, useEffect, useCallback, useState } from 'react';
 import { AlertCircle } from 'lucide-react';
 import { XTerminal, XTerminalHandle } from './XTerminal';
 import { TerminalSearchBar } from './TerminalSearchBar';
@@ -58,6 +58,10 @@ export const TerminalView = memo(
 		const terminalRefs = useRef<Map<string, XTerminalHandle>>(new Map());
 		// Track previous tab states to detect transitions (for exit message)
 		const prevTabStatesRef = useRef<Map<string, TerminalTab['state']>>(new Map());
+		// In-flight spawn guard: set of tabIds currently waiting for a PTY PID
+		const spawnInFlightRef = useRef<Set<string>>(new Set());
+		// Track which tabs have already had the loading message written to avoid duplicates
+		const loadingWrittenRef = useRef<Set<string>>(new Set());
 
 		const activeTab = getActiveTerminalTab(session);
 
@@ -94,8 +98,12 @@ export const TerminalView = memo(
 		// Shared spawn function — used both on mount and for retry
 		const spawnPtyForTab = useCallback(
 			(tab: TerminalTab) => {
-				const terminalSessionId = getTerminalSessionId(session.id, tab.id);
 				const tabId = tab.id;
+				// Guard: skip if a spawn is already in flight for this tab
+				if (spawnInFlightRef.current.has(tabId)) return;
+				spawnInFlightRef.current.add(tabId);
+
+				const terminalSessionId = getTerminalSessionId(session.id, tabId);
 
 				window.maestro.process
 					.spawnTerminalTab({
@@ -104,6 +112,7 @@ export const TerminalView = memo(
 						shell: defaultShell || undefined,
 						shellArgs,
 						shellEnvVars,
+						sessionSshRemoteConfig: session.sessionSshRemoteConfig,
 					})
 					.then((result) => {
 						if (result.success) {
@@ -114,9 +123,12 @@ export const TerminalView = memo(
 					})
 					.catch(() => {
 						onTabStateChange(tabId, 'exited', 1);
+					})
+					.finally(() => {
+						spawnInFlightRef.current.delete(tabId);
 					});
 			},
-			[session.id, session.cwd, defaultShell, shellArgs, shellEnvVars]
+			[session.id, session.cwd, session.sessionSshRemoteConfig, defaultShell, shellArgs, shellEnvVars, onTabPidChange, onTabStateChange]
 		);
 
 		// Spawn PTY when active tab changes and has no PID yet
@@ -125,7 +137,7 @@ export const TerminalView = memo(
 				return;
 			}
 			spawnPtyForTab(activeTab);
-		}, [activeTab?.id]);
+		}, [activeTab?.id, spawnPtyForTab]);
 
 		// Focus the active terminal when the active tab changes
 		useEffect(() => {
@@ -260,14 +272,16 @@ export const TerminalView = memo(
 										ref={(handle) => {
 											if (handle) {
 												terminalRefs.current.set(tab.id, handle);
-												// Write loading indicator when terminal mounts with no PTY yet
-												if (tab.pid === 0 && tab.state === 'idle') {
+												// Write loading indicator once per idle cycle — guard prevents duplicate writes on re-renders
+												if (tab.pid === 0 && tab.state === 'idle' && !loadingWrittenRef.current.has(tab.id)) {
+													loadingWrittenRef.current.add(tab.id);
 													setTimeout(() => {
 														handle.write('\x1b[2mStarting terminal...\x1b[0m');
 													}, 0);
 												}
 											} else {
 												terminalRefs.current.delete(tab.id);
+												loadingWrittenRef.current.delete(tab.id);
 											}
 										}}
 										sessionId={terminalSessionId}

src/renderer/hooks/keyboard/useMainKeyboardHandler.ts

@@ -84,9 +84,13 @@ export function useMainKeyboardHandler(): UseMainKeyboardHandlerReturn {
 
 			// CRITICAL: When in terminal mode, let xterm.js handle Ctrl+[A-Z] control sequences.
 			// These include Ctrl+C (SIGINT), Ctrl+D (EOF), Ctrl+Z (suspend), Ctrl+\ (quit), etc.
-			// Only intercept Meta (Cmd) key combos from terminal mode — those are Maestro shortcuts.
-			// Exception: Ctrl+Shift+` always creates a new terminal tab regardless of mode.
+			// On macOS, Ctrl is used for terminal control sequences; Cmd (Meta) is for Maestro shortcuts.
+			// On Windows/Linux, Ctrl doubles as the modifier for Maestro shortcuts (Ctrl+F, Ctrl+W, etc.)
+			// so we only bypass for macOS to avoid breaking cross-platform app shortcuts.
+			// Exception: Ctrl+Shift+` always creates a new terminal tab regardless of mode/platform.
+			const isMac = navigator.platform.toUpperCase().includes('MAC');
 			if (
+				isMac &&
 				ctx.activeSession?.inputMode === 'terminal' &&
 				e.ctrlKey &&
 				!e.metaKey &&
@@ -843,10 +847,13 @@ export function useMainKeyboardHandler(): UseMainKeyboardHandlerReturn {
 				);
 
 				// Cmd+W: Close the active terminal tab (only if more than one exists)
-				if (ctx.isTabShortcut(e, 'closeTab') && terminalTabs.length > 1 && activeTerminalTabId) {
+				// Always preventDefault to prevent native window-close when only one tab remains
+				if (ctx.isTabShortcut(e, 'closeTab')) {
 					e.preventDefault();
-					ctx.handleCloseTerminalTab(activeTerminalTabId);
-					trackShortcut('closeTab');
+					if (terminalTabs.length > 1 && activeTerminalTabId) {
+						ctx.handleCloseTerminalTab(activeTerminalTabId);
+						trackShortcut('closeTab');
+					}
 				}
 
 				// Cmd+Shift+] — Navigate to next terminal tab

src/renderer/hooks/utils/useDebouncedPersistence.ts

@@ -104,12 +104,21 @@ const prepareSessionForPersistence = (session: Session): Session => {
 		exitCode: undefined,
 	}));
 
+	// Validate activeTerminalTabId against the cleaned terminal tabs list
+	const activeTerminalTabExists = cleanedTerminalTabs.some(
+		(tab) => tab.id === session.activeTerminalTabId
+	);
+	const newActiveTerminalTabId = activeTerminalTabExists
+		? session.activeTerminalTabId
+		: cleanedTerminalTabs[0]?.id ?? null;
+
 	return {
 		...sessionWithoutRuntimeFields,
 		aiTabs: truncatedTabs,
 		activeTabId: newActiveTabId,
 		// Reset terminal tab runtime state
 		terminalTabs: cleanedTerminalTabs,
+		activeTerminalTabId: newActiveTerminalTabId,
 		// Reset runtime-only session state - processes don't survive app restart
 		state: 'idle',
 		busySource: undefined,

src/renderer/stores/tabStore.ts

@@ -489,9 +489,11 @@ export const useTabStore = create<TabStore>()((set) => ({
 	closeTerminalTab: (tabId) => {
 		const session = getActiveSession();
 		if (!session) return;
-		// Kill the PTY process before removing the tab
-		window.maestro.process.kill(getTerminalSessionId(session.id, tabId));
+		// Validate close first — closeTerminalTabHelper refuses to close the last tab
 		const updatedSession = closeTerminalTabHelper(session, tabId);
+		if (updatedSession === session) return; // Close was rejected (e.g., last tab)
+		// Kill the PTY process only after confirming the tab will be removed
+		window.maestro.process.kill(getTerminalSessionId(session.id, tabId));
 		updateActiveSession(updatedSession);
 	},
 

src/renderer/utils/terminalTabHelpers.ts

@@ -286,7 +286,7 @@ export function updateTerminalTabState(
 		...session,
 		terminalTabs: terminalTabs.map((tab) =>
 			tab.id === tabId
-				? { ...tab, state, ...(exitCode !== undefined ? { exitCode } : {}) }
+				? { ...tab, state, exitCode }
 				: tab
 		),
 	};