From d459b975136e5cea17f1a5a7b4bead2154c80acd Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 21 Aug 2026 23:04:13 -0700 Subject: [PATCH] fix(ui): gate Floating Workspace auto-ack on panel visibility MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The floating-workspace scan added in the previous commit had no visibility gate. The panel stays mounted while closed (use-floating-workspace-panel: shouldMountPanel), so its layout still resolves an active leaf and the hook acked a floating agent completion the moment it landed — silently killing the minimized toggle's attention dot (selectFloatingWorkspaceHasUnread), which is the only "unseen floating activity" signal a closed panel has. - Scan the floating tab only while the panel is actually visible (isFloatingWorkspacePanelVisible), so a closed panel keeps its dot. - Move the activeView filter onto the main-worktree target only: the panel is an overlay above every view, so it must ack from the activity/tasks views too. - Carry the owning worktree with each target instead of re-deriving it by tab id, and keep the first entry on a tab-id collision, so a duplicate id can no longer clear the wrong worktree's unread. Drops the `as string[]` cast. - Re-scan on TOGGLE_FLOATING_TERMINAL_EVENT (next frame, after aria-hidden commits) since panel open/closed is React state the store never sees — opening onto an already-active completed tab now acks. - Cover the new resolveAutoAckTabTargets helper, including the closed-panel regression asserted against selectFloatingWorkspaceHasUnread. --- .../src/hooks/useAutoAckViewedAgent.test.ts | 124 ++++++++++++++++++ .../src/hooks/useAutoAckViewedAgent.ts | 78 ++++++++--- 2 files changed, 187 insertions(+), 15 deletions(-) diff --git a/src/renderer/src/hooks/useAutoAckViewedAgent.test.ts b/src/renderer/src/hooks/useAutoAckViewedAgent.test.ts index e396a7e5fa2..d461dd13b90 100644 --- a/src/renderer/src/hooks/useAutoAckViewedAgent.test.ts +++ b/src/renderer/src/hooks/useAutoAckViewedAgent.test.ts @@ -3,10 +3,13 @@ import { acknowledgeViewedAgentAttention, computeAutoAckTargets, computeViewedAgentCompletionPaneKey, + resolveAutoAckTabTargets, shouldClearViewedAgentWorktreeUnread } from './useAutoAckViewedAgent' import { createTestStore, makeTab } from '../store/slices/store-test-helpers' +import { selectFloatingWorkspaceHasUnread } from '../store/selectors' import type { RetainedAgentEntry } from '../store/slices/agent-status' +import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../shared/constants' import { makePaneKey } from '../../../shared/stable-pane-id' const CODEX_LEAF_ID = '11111111-1111-4111-8111-111111111111' @@ -379,3 +382,124 @@ describe('shouldClearViewedAgentWorktreeUnread', () => { ).toBe(false) }) }) + +describe('resolveAutoAckTabTargets', () => { + const FLOATING_TAB_ID = 'tab-floating' + const baseState = { + activeView: 'terminal', + activeTabId: 'tab-1', + activeWorktreeId: 'wt-1', + activeTabIdByWorktree: { + 'wt-1': 'tab-1', + [FLOATING_TERMINAL_WORKTREE_ID]: FLOATING_TAB_ID + } + } + + it('scans the floating tab alongside the main tab while the panel is visible', () => { + expect(resolveAutoAckTabTargets(baseState, { floatingPanelVisible: true })).toEqual([ + { tabId: 'tab-1', worktreeId: 'wt-1' }, + { tabId: FLOATING_TAB_ID, worktreeId: FLOATING_TERMINAL_WORKTREE_ID } + ]) + }) + + it('skips the floating tab while the panel is closed', () => { + expect(resolveAutoAckTabTargets(baseState, { floatingPanelVisible: false })).toEqual([ + { tabId: 'tab-1', worktreeId: 'wt-1' } + ]) + }) + + it('scans the floating tab outside the terminal view because the panel overlays every view', () => { + expect( + resolveAutoAckTabTargets( + { ...baseState, activeView: 'activity' }, + { floatingPanelVisible: true } + ) + ).toEqual([{ tabId: FLOATING_TAB_ID, worktreeId: FLOATING_TERMINAL_WORKTREE_ID }]) + }) + + it('scans nothing outside the terminal view with the panel closed', () => { + expect( + resolveAutoAckTabTargets( + { ...baseState, activeView: 'activity' }, + { floatingPanelVisible: false } + ) + ).toEqual([]) + }) + + it('keeps the real worktree when one tab id is claimed by both worktrees', () => { + expect( + resolveAutoAckTabTargets( + { ...baseState, activeTabId: FLOATING_TAB_ID }, + { floatingPanelVisible: true } + ) + ).toEqual([{ tabId: FLOATING_TAB_ID, worktreeId: 'wt-1' }]) + }) +}) + +// Why: the minimized toggle's attention dot is the only signal a closed floating panel has, so a +// hidden panel must never auto-ack (selectFloatingWorkspaceHasUnread → FloatingTerminalToggleButton). +describe('floating workspace auto-ack against the attention dot', () => { + const FLOATING_TAB_ID = 'tab-floating' + const floatingPaneKey = makePaneKey(FLOATING_TAB_ID, CODEX_LEAF_ID) + + function seedFloatingCompletion(): ReturnType { + const store = createTestStore() + store.setState({ + activeView: 'terminal', + activeTabId: 'tab-1', + activeWorktreeId: 'wt-1', + activeTabIdByWorktree: { + 'wt-1': 'tab-1', + [FLOATING_TERMINAL_WORKTREE_ID]: FLOATING_TAB_ID + }, + tabsByWorktree: { + 'wt-1': [makeTab({ id: 'tab-1', worktreeId: 'wt-1' })], + [FLOATING_TERMINAL_WORKTREE_ID]: [ + makeTab({ id: FLOATING_TAB_ID, worktreeId: FLOATING_TERMINAL_WORKTREE_ID }) + ] + } + }) + store.getState().markAgentCompletionPaneUnread(floatingPaneKey) + return store + } + + function runAutoAckScan( + store: ReturnType, + floatingPanelVisible: boolean + ): void { + const state = store.getState() + for (const target of resolveAutoAckTabTargets(state, { floatingPanelVisible })) { + const activePaneKey = computeViewedAgentCompletionPaneKey(state, target.tabId, CODEX_LEAF_ID) + const paneKeysToClear = new Set(activePaneKey ? [activePaneKey] : []) + acknowledgeViewedAgentAttention(store.getState(), { + activeWorktreeId: shouldClearViewedAgentWorktreeUnread(store.getState(), { + activeWorktreeId: target.worktreeId, + activeTabId: target.tabId, + paneKeysToClear + }) + ? target.worktreeId + : null, + activeTabId: target.tabId, + paneKeys: [], + activePaneKey + }) + } + } + + it('keeps the attention dot lit while the panel is closed', () => { + const store = seedFloatingCompletion() + expect(selectFloatingWorkspaceHasUnread(store.getState())).toBe(true) + + runAutoAckScan(store, false) + + expect(selectFloatingWorkspaceHasUnread(store.getState())).toBe(true) + }) + + it('clears the attention dot once the panel is visible', () => { + const store = seedFloatingCompletion() + + runAutoAckScan(store, true) + + expect(selectFloatingWorkspaceHasUnread(store.getState())).toBe(false) + }) +}) diff --git a/src/renderer/src/hooks/useAutoAckViewedAgent.ts b/src/renderer/src/hooks/useAutoAckViewedAgent.ts index 71c1ffd1930..25fbcbce50d 100644 --- a/src/renderer/src/hooks/useAutoAckViewedAgent.ts +++ b/src/renderer/src/hooks/useAutoAckViewedAgent.ts @@ -1,5 +1,7 @@ import { useEffect } from 'react' import { useAppStore } from '@/store' +import { TOGGLE_FLOATING_TERMINAL_EVENT } from '@/lib/floating-terminal' +import { isFloatingWorkspacePanelVisible } from '@/lib/floating-workspace-terminal-actions' import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../shared/constants' import type { AgentStatusEntry } from '../../../shared/agent-status-types' import type { RetainedAgentEntry } from '@/store/slices/agent-status' @@ -146,6 +148,39 @@ export function acknowledgeViewedAgentAttention( } } +export type AutoAckTabTarget = { tabId: string; worktreeId: string | null } + +/** + * Tabs whose visible pane counts as "seen" right now, each paired with the worktree that owns it. + * + * Why the floating workspace is gated on panel visibility rather than `activeView`: the panel is an + * overlay that sits above every view and stays mounted while closed, and its active tab never + * becomes the global `activeTabId` — so neither the view nor the tab id can stand in for "on screen". + */ +export function resolveAutoAckTabTargets( + state: { + activeView: string + activeTabId: string | null + activeWorktreeId: string | null + activeTabIdByWorktree: Record + }, + options: { floatingPanelVisible: boolean } +): AutoAckTabTarget[] { + const targets: AutoAckTabTarget[] = [] + if (state.activeView === 'terminal' && state.activeTabId) { + targets.push({ tabId: state.activeTabId, worktreeId: state.activeWorktreeId }) + } + if (options.floatingPanelVisible) { + const floatingTabId = state.activeTabIdByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? null + // Why first-wins on a tab-id collision: tab ids can be claimed by two worktrees + // (see active-tab-owner-worktree), and acking under the wrong one strands its unread dot. + if (floatingTabId && !targets.some((target) => target.tabId === floatingTabId)) { + targets.push({ tabId: floatingTabId, worktreeId: FLOATING_TERMINAL_WORKTREE_ID }) + } + } + return targets +} + // Auto-ack an agent row as "seen" when the user is already on its tab, so the dashboard/Dock don't stay bold for an event they watched happen. // Scans live + retained maps: Codex's title-revert (pty-connection.ts:onAgentExited) migrates `done` rows to retained mid-race — see docs/codex-agent-row-bold-stuck.md. export function useAutoAckViewedAgent(): void { @@ -161,11 +196,13 @@ export function useAutoAckViewedAgent(): void { let lastLayouts: unknown = undefined let lastUnreadAgentCompletionPanes: unknown = undefined - const maybeAck = (): void => { + // `force` re-scans after a signal the store never sees: panel open/closed is React-local state. + const maybeAck = (options?: { force?: boolean }): void => { const s = useAppStore.getState() const floatingWorkspaceActiveTabId = s.activeTabIdByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? null if ( + !options?.force && s.activeView === lastActiveView && s.activeTabId === lastActiveTabId && floatingWorkspaceActiveTabId === lastFloatingWorkspaceActiveTabId && @@ -178,9 +215,6 @@ export function useAutoAckViewedAgent(): void { return } - if (s.activeView !== 'terminal') { - return - } // Why: tab-active only proxies "seen"; gate on window visible+focused so away-time transitions don't silently clear the bold signal. if (typeof document !== 'undefined') { if (document.visibilityState !== 'visible') { @@ -190,10 +224,11 @@ export function useAutoAckViewedAgent(): void { return } } - const activeTabId = s.activeTabId - // Why: also check Floating Workspace tab even if it's not the active worktree, so agents there get acknowledged when visible. - const ackTabIds = [activeTabId, floatingWorkspaceActiveTabId].filter(Boolean) as string[] - if (ackTabIds.length === 0) { + // Why below the gates: resolving the floating target costs a DOM query, and most store writes never get here. + const targets = resolveAutoAckTabTargets(s, { + floatingPanelVisible: typeof document !== 'undefined' && isFloatingWorkspacePanelVisible() + }) + if (targets.length === 0) { return } // Why: advance refs only after gates pass, else the diff is consumed and a gated-out transition never re-acks when focus returns. @@ -206,7 +241,8 @@ export function useAutoAckViewedAgent(): void { lastLayouts = s.terminalLayoutsByTabId lastUnreadAgentCompletionPanes = s.unreadAgentCompletionPanes - for (const tabId of ackTabIds) { + for (const target of targets) { + const tabId = target.tabId const activeLeafId = resolveActiveLeafId(s, tabId) const toAck = computeAutoAckTargets(s, tabId, activeLeafId) const activePaneKey = computeViewedAgentCompletionPaneKey(s, tabId, activeLeafId) @@ -215,11 +251,7 @@ export function useAutoAckViewedAgent(): void { if (activePaneKey) { paneKeysToClear.add(activePaneKey) } - // Why: for Floating Workspace tabs, still use activeWorktreeId to avoid clearing unseen signals in other worktrees. - const worktreeId = - tabId === floatingWorkspaceActiveTabId - ? FLOATING_TERMINAL_WORKTREE_ID - : s.activeWorktreeId + const worktreeId = target.worktreeId acknowledgeViewedAgentAttention(s, { activeWorktreeId: shouldClearViewedAgentWorktreeUnread(s, { activeWorktreeId: worktreeId, @@ -238,16 +270,32 @@ export function useAutoAckViewedAgent(): void { // Why: run once on mount to catch a restored session that already has agents on the visible tab. maybeAck() // Subscribe to all store changes; the ref-equality guard above skips unrelated updates. - const unsubscribe = useAppStore.subscribe(maybeAck) + const unsubscribe = useAppStore.subscribe(() => maybeAck()) // Why: focus/visibility don't flow through zustand, so re-run the scan on these DOM events when focus returns. const onVisibility = (): void => maybeAck() const onFocus = (): void => maybeAck() + // Why the frame: the toggle event fires before React commits, so the panel's aria-hidden still reads stale. + let floatingToggleFrame: number | null = null + const onFloatingToggle = (): void => { + if (floatingToggleFrame !== null) { + cancelAnimationFrame(floatingToggleFrame) + } + floatingToggleFrame = requestAnimationFrame(() => { + floatingToggleFrame = null + maybeAck({ force: true }) + }) + } document.addEventListener('visibilitychange', onVisibility) window.addEventListener('focus', onFocus) + window.addEventListener(TOGGLE_FLOATING_TERMINAL_EVENT, onFloatingToggle) return () => { unsubscribe() + if (floatingToggleFrame !== null) { + cancelAnimationFrame(floatingToggleFrame) + } document.removeEventListener('visibilitychange', onVisibility) window.removeEventListener('focus', onFocus) + window.removeEventListener(TOGGLE_FLOATING_TERMINAL_EVENT, onFloatingToggle) } }, []) }