mirror of
https://github.com/stablyai/orca.git
synced 2026-10-23 08:03:39 +00:00
fix(ui): gate Floating Workspace auto-ack on panel visibility
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.
This commit is contained in:
@@ -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<typeof createTestStore> {
|
||||
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<typeof createTestStore>,
|
||||
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)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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<string, string | null>
|
||||
},
|
||||
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)
|
||||
}
|
||||
}, [])
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user