mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
perf: always show project names and remove notification scans (#20931)
* perf: avoid repeated agent scans when labeling notifications * perf: always label notifications and remove project counting * fix: qualify the notification project group by its folder's host Folder notifications resolved the folder host-aware, then looked its project group up by bare ID. The owner index fails a bare ID closed when two hosts publish the same group ID, so a remote folder lost the project name the catalog already had. Also drops the identity rescans that recovered display fields: the catalog finders now return the caller's row type, matching findIndexedRepoOwnerForHost. Updates the idle-arbitration expectation that still asserted the removed hasMultipleActiveRepos flag.
This commit is contained in:
@@ -87,7 +87,7 @@ function formatNotificationWorktreeContext(args: NotificationDispatchRequest): s
|
||||
NOTIFICATION_TITLE_CONTEXT_MAX_LENGTH
|
||||
)
|
||||
const repoLabel = normalizeNotificationText(args.repoLabel, NOTIFICATION_TITLE_CONTEXT_MAX_LENGTH)
|
||||
if (args.hasMultipleActiveRepos && repoLabel && worktreeLabel) {
|
||||
if (repoLabel && worktreeLabel) {
|
||||
return normalizeNotificationText(
|
||||
`${repoLabel} / ${worktreeLabel}`,
|
||||
NOTIFICATION_TITLE_CONTEXT_MAX_LENGTH
|
||||
|
||||
@@ -96,43 +96,6 @@ describe('registerNotificationHandlers', () => {
|
||||
)
|
||||
).toEqual({ delivered: true })
|
||||
|
||||
expect(notificationCtorMock).toHaveBeenCalledWith(
|
||||
expectedNativeNotificationOptions({
|
||||
title: 'feat/notis - Codex finished',
|
||||
body: 'Updated the notification body.'
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
it('includes the repo name when multiple repos are active', async () => {
|
||||
registerNotificationHandlers({
|
||||
getSettings: () => ({
|
||||
notifications: {
|
||||
enabled: true,
|
||||
agentTaskComplete: true,
|
||||
terminalBell: false,
|
||||
suppressWhenFocused: true
|
||||
}
|
||||
})
|
||||
} as never)
|
||||
|
||||
const handler = getDispatchHandler()
|
||||
expect(
|
||||
await handler(
|
||||
{},
|
||||
{
|
||||
source: 'agent-task-complete',
|
||||
worktreeId: 'repo::wt1',
|
||||
worktreeLabel: 'feat/notis',
|
||||
repoLabel: 'orca',
|
||||
hasMultipleActiveRepos: true,
|
||||
agentType: 'codex',
|
||||
agentState: 'done',
|
||||
agentLastAssistantMessage: 'Updated the notification body.'
|
||||
}
|
||||
)
|
||||
).toEqual({ delivered: true })
|
||||
|
||||
expect(notificationCtorMock).toHaveBeenCalledWith(
|
||||
expectedNativeNotificationOptions({
|
||||
title: 'orca / feat/notis - Codex finished',
|
||||
@@ -141,6 +104,46 @@ describe('registerNotificationHandlers', () => {
|
||||
)
|
||||
})
|
||||
|
||||
it.each([true, false, undefined])(
|
||||
'includes the repo name regardless of the legacy multiple-repo flag (%s)',
|
||||
async (hasMultipleActiveRepos) => {
|
||||
registerNotificationHandlers({
|
||||
getSettings: () => ({
|
||||
notifications: {
|
||||
enabled: true,
|
||||
agentTaskComplete: true,
|
||||
terminalBell: false,
|
||||
suppressWhenFocused: true
|
||||
}
|
||||
})
|
||||
} as never)
|
||||
|
||||
const handler = getDispatchHandler()
|
||||
expect(
|
||||
await handler(
|
||||
{},
|
||||
{
|
||||
source: 'agent-task-complete',
|
||||
worktreeId: 'repo::wt1',
|
||||
worktreeLabel: 'feat/notis',
|
||||
repoLabel: 'orca',
|
||||
hasMultipleActiveRepos,
|
||||
agentType: 'codex',
|
||||
agentState: 'done',
|
||||
agentLastAssistantMessage: 'Updated the notification body.'
|
||||
}
|
||||
)
|
||||
).toEqual({ delivered: true })
|
||||
|
||||
expect(notificationCtorMock).toHaveBeenCalledWith(
|
||||
expectedNativeNotificationOptions({
|
||||
title: 'orca / feat/notis - Codex finished',
|
||||
body: 'Updated the notification body.'
|
||||
})
|
||||
)
|
||||
}
|
||||
)
|
||||
|
||||
it('keeps a readable body when no assistant response was captured', async () => {
|
||||
registerNotificationHandlers({
|
||||
getSettings: () => ({
|
||||
|
||||
@@ -218,7 +218,6 @@ describe('connectPanePty', () => {
|
||||
worktreeId: 'wt-1',
|
||||
repoLabel: 'orca',
|
||||
worktreeLabel: 'feat/notis',
|
||||
hasMultipleActiveRepos: true,
|
||||
terminalTitle: '* Claude done',
|
||||
agentType: 'claude',
|
||||
agentState: 'done',
|
||||
|
||||
@@ -0,0 +1,160 @@
|
||||
// @vitest-environment happy-dom
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants'
|
||||
import { useAppStore } from '@/store'
|
||||
import { makeFolderWorkspace, makeWorktree } from '@/store/slices/worktrees-slice-test-fixtures'
|
||||
import { getNotificationWorkspaceLabels } from './terminal-notification-state'
|
||||
|
||||
function stateWithWorkspace() {
|
||||
return {
|
||||
...useAppStore.getInitialState(),
|
||||
worktreesByRepo: { repo: [makeWorktree({ id: 'wt', repoId: 'repo', displayName: 'Feature' })] },
|
||||
repos: [
|
||||
{
|
||||
id: 'repo',
|
||||
displayName: 'Orca',
|
||||
path: '/orca',
|
||||
connectionId: null,
|
||||
badgeColor: 'blue',
|
||||
addedAt: 0
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
|
||||
describe('notification workspace labels', () => {
|
||||
it('includes the only project without reading agent inventories', () => {
|
||||
const state = stateWithWorkspace()
|
||||
Object.defineProperty(state, 'agentStatusByPaneKey', {
|
||||
get() {
|
||||
throw new Error('agent scan')
|
||||
}
|
||||
})
|
||||
Object.defineProperty(state, 'retainedAgentsByPaneKey', {
|
||||
get() {
|
||||
throw new Error('retained scan')
|
||||
}
|
||||
})
|
||||
expect(getNotificationWorkspaceLabels(state, 'wt')).toEqual({
|
||||
repoLabel: 'Orca',
|
||||
worktreeLabel: 'Feature'
|
||||
})
|
||||
expect(getNotificationWorkspaceLabels(state, 'worktree:wt')).toEqual({
|
||||
repoLabel: 'Orca',
|
||||
worktreeLabel: 'Feature'
|
||||
})
|
||||
})
|
||||
|
||||
it('keeps labels for remote Git workspaces', () => {
|
||||
const state = stateWithWorkspace()
|
||||
state.worktreesByRepo.repo = [
|
||||
makeWorktree({
|
||||
id: 'remote',
|
||||
repoId: 'repo',
|
||||
hostId: 'ssh:server',
|
||||
displayName: 'Remote feature'
|
||||
})
|
||||
]
|
||||
expect(getNotificationWorkspaceLabels(state, 'remote')).toEqual({
|
||||
repoLabel: 'Orca',
|
||||
worktreeLabel: 'Remote feature'
|
||||
})
|
||||
})
|
||||
|
||||
it.each([undefined, 'ssh:server'] as const)(
|
||||
'resolves folder and project names on host %s',
|
||||
(executionHostId) => {
|
||||
const state = stateWithWorkspace()
|
||||
state.folderWorkspaces = [
|
||||
makeFolderWorkspace({
|
||||
id: 'folder-id',
|
||||
projectGroupId: 'group',
|
||||
name: 'Website',
|
||||
executionHostId
|
||||
})
|
||||
]
|
||||
state.projectGroups = [
|
||||
{
|
||||
id: 'group',
|
||||
name: 'Personal',
|
||||
executionHostId,
|
||||
parentPath: null,
|
||||
parentGroupId: null,
|
||||
createdFrom: 'manual',
|
||||
tabOrder: 0,
|
||||
isCollapsed: false,
|
||||
color: null,
|
||||
createdAt: 0,
|
||||
updatedAt: 0
|
||||
}
|
||||
]
|
||||
expect(getNotificationWorkspaceLabels(state, 'folder:folder-id')).toEqual({
|
||||
repoLabel: 'Personal',
|
||||
worktreeLabel: 'Website'
|
||||
})
|
||||
state.projectGroups = []
|
||||
expect(getNotificationWorkspaceLabels(state, 'folder:folder-id')).toEqual({
|
||||
repoLabel: undefined,
|
||||
worktreeLabel: 'Website'
|
||||
})
|
||||
}
|
||||
)
|
||||
|
||||
it.each([false, true])(
|
||||
'qualifies project groups by the folder host (legacy SSH: %s)',
|
||||
(legacy) => {
|
||||
const state = stateWithWorkspace()
|
||||
state.folderWorkspaces = [
|
||||
makeFolderWorkspace({
|
||||
id: 'remote-folder',
|
||||
name: 'Remote folder',
|
||||
projectGroupId: 'shared',
|
||||
...(legacy ? { connectionId: 'server' } : { executionHostId: 'ssh:server' as const })
|
||||
})
|
||||
]
|
||||
state.projectGroups = (['local', 'ssh:server'] as const).map((executionHostId) => ({
|
||||
id: 'shared',
|
||||
name: executionHostId === 'local' ? 'Local group' : 'Remote group',
|
||||
executionHostId,
|
||||
parentPath: null,
|
||||
parentGroupId: null,
|
||||
createdFrom: 'manual' as const,
|
||||
tabOrder: 0,
|
||||
isCollapsed: false,
|
||||
color: null,
|
||||
createdAt: 0,
|
||||
updatedAt: 0
|
||||
}))
|
||||
expect(getNotificationWorkspaceLabels(state, 'folder:remote-folder')).toEqual({
|
||||
repoLabel: 'Remote group',
|
||||
worktreeLabel: 'Remote folder'
|
||||
})
|
||||
}
|
||||
)
|
||||
|
||||
it('does not pick an arbitrary folder when hosts have conflicting records', () => {
|
||||
const state = stateWithWorkspace()
|
||||
state.folderWorkspaces = (['ssh:a', 'ssh:b'] as const).map((executionHostId) =>
|
||||
makeFolderWorkspace({ id: 'duplicate', name: executionHostId, executionHostId })
|
||||
)
|
||||
expect(getNotificationWorkspaceLabels(state, 'folder:duplicate', 'Terminal')).toEqual({
|
||||
repoLabel: undefined,
|
||||
worktreeLabel: 'Terminal'
|
||||
})
|
||||
})
|
||||
|
||||
it.each(['folder:missing', 'missing-worktree', FLOATING_TERMINAL_WORKTREE_ID])(
|
||||
'uses readable fallbacks for %s',
|
||||
(id) => {
|
||||
const state = stateWithWorkspace()
|
||||
expect(getNotificationWorkspaceLabels(state, id, 'My terminal')).toEqual({
|
||||
repoLabel: undefined,
|
||||
worktreeLabel: 'My terminal'
|
||||
})
|
||||
expect(getNotificationWorkspaceLabels(state, id, ' ')).toEqual({
|
||||
repoLabel: undefined,
|
||||
worktreeLabel: 'workspace'
|
||||
})
|
||||
}
|
||||
)
|
||||
})
|
||||
@@ -1,7 +1,11 @@
|
||||
import { isExplicitAgentStatusFresh } from '@/lib/agent-status'
|
||||
import type { useAppStore } from '@/store'
|
||||
import { getWorktreeMapFromState } from '@/store/selectors'
|
||||
import { AGENT_STATUS_STALE_AFTER_MS } from '../../../../shared/agent-status-types'
|
||||
import { getRepoMapFromState, getWorktreeMapFromState } from '@/store/selectors'
|
||||
import {
|
||||
findIndexedFolderWorkspaceOwner,
|
||||
findIndexedProjectGroupOwner,
|
||||
getCatalogOwnerHostId
|
||||
} from '@/lib/worktree-runtime-owner-index'
|
||||
import { parseWorkspaceKey } from '../../../../shared/workspace-scope'
|
||||
import { parsePaneKey } from '../../../../shared/stable-pane-id'
|
||||
import type { TerminalPaneLayoutNode } from '../../../../shared/terminal-tab-types'
|
||||
|
||||
@@ -144,74 +148,31 @@ export function isCurrentKnownPaneKey(
|
||||
return ptyHints.length === 0 || ptyHints.some((ptyId) => !isSuppressedPtyHint(state, ptyId))
|
||||
}
|
||||
|
||||
function hasActiveWorktreeState(state: StoreSnapshot, worktreeId: string): boolean {
|
||||
if (hasLivePtyForWorktree(state, worktreeId)) {
|
||||
return true
|
||||
export function getNotificationWorkspaceLabels(
|
||||
state: StoreSnapshot,
|
||||
workspaceId: string,
|
||||
terminalTitle?: string
|
||||
): { repoLabel?: string; worktreeLabel: string } {
|
||||
const scope = parseWorkspaceKey(workspaceId)
|
||||
const fallback = terminalTitle?.trim() || 'workspace'
|
||||
if (scope?.type === 'folder') {
|
||||
const folder = findIndexedFolderWorkspaceOwner(state.folderWorkspaces, scope.folderWorkspaceId)
|
||||
// The group ID is only unique per host, so qualify it with the folder's own host.
|
||||
const group =
|
||||
folder &&
|
||||
findIndexedProjectGroupOwner(
|
||||
state.projectGroups,
|
||||
folder.projectGroupId,
|
||||
getCatalogOwnerHostId(folder)
|
||||
)
|
||||
return { repoLabel: group?.name, worktreeLabel: folder?.name || fallback }
|
||||
}
|
||||
|
||||
if ((state.browserTabsByWorktree?.[worktreeId] ?? []).length > 0) {
|
||||
return true
|
||||
const worktree = getWorktreeMapFromState(state).get(
|
||||
scope?.type === 'worktree' ? scope.worktreeId : workspaceId
|
||||
)
|
||||
const repo = worktree ? getRepoMapFromState(state).get(worktree.repoId) : undefined
|
||||
return {
|
||||
repoLabel: repo?.displayName,
|
||||
worktreeLabel: worktree?.displayName || worktree?.branch || fallback
|
||||
}
|
||||
|
||||
const worktree = getWorktreeMapFromState(state).get(worktreeId)
|
||||
if (worktree?.workspaceStatus === 'in-progress') {
|
||||
return true
|
||||
}
|
||||
|
||||
if (
|
||||
Object.values(state.retainedAgentsByPaneKey ?? {}).some(
|
||||
(agent) => agent.worktreeId === worktreeId
|
||||
)
|
||||
) {
|
||||
return true
|
||||
}
|
||||
|
||||
const tabs = state.tabsByWorktree[worktreeId] ?? []
|
||||
const tabIds = new Set(tabs.map((tab) => tab.id))
|
||||
if (tabIds.size === 0) {
|
||||
return false
|
||||
}
|
||||
|
||||
const now = Date.now()
|
||||
return Object.values(state.agentStatusByPaneKey ?? {}).some((entry) => {
|
||||
const tabId = getPaneKeyTabId(entry.paneKey)
|
||||
return (
|
||||
tabId !== null &&
|
||||
tabIds.has(tabId) &&
|
||||
isExplicitAgentStatusFresh(entry, now, AGENT_STATUS_STALE_AFTER_MS)
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
function countReposWithWorktrees(state: StoreSnapshot): number {
|
||||
let count = 0
|
||||
for (const worktrees of Object.values(state.worktreesByRepo)) {
|
||||
if (worktrees.length > 0) {
|
||||
count += 1
|
||||
}
|
||||
}
|
||||
return count
|
||||
}
|
||||
|
||||
export function countReposNeedingNotificationDisambiguation(state: StoreSnapshot): number {
|
||||
const activeRepoIds = new Set<string>()
|
||||
const worktreeMap = getWorktreeMapFromState(state)
|
||||
for (const worktreeId of Object.keys(state.tabsByWorktree)) {
|
||||
if (!hasActiveWorktreeState(state, worktreeId)) {
|
||||
continue
|
||||
}
|
||||
const repoId = worktreeMap.get(worktreeId)?.repoId
|
||||
if (repoId) {
|
||||
activeRepoIds.add(repoId)
|
||||
}
|
||||
}
|
||||
for (const [repoId, worktrees] of Object.entries(state.worktreesByRepo)) {
|
||||
if (activeRepoIds.has(repoId)) {
|
||||
continue
|
||||
}
|
||||
if (worktrees.some((worktree) => hasActiveWorktreeState(state, worktree.id))) {
|
||||
activeRepoIds.add(repoId)
|
||||
}
|
||||
}
|
||||
return Math.max(activeRepoIds.size, countReposWithWorktrees(state))
|
||||
}
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
import { useCallback } from 'react'
|
||||
import { useAppStore } from '@/store'
|
||||
import { resolveCommittedTitleAgentType } from '@/lib/pane-agent-evidence'
|
||||
import { getRepoMapFromState, getWorktreeMapFromState } from '@/store/selectors'
|
||||
import { playDesktopNotificationSound } from '@/lib/desktop-notification-sound'
|
||||
import { showBlockedNotificationFallbackToast } from '@/lib/blocked-notification-fallback'
|
||||
import { buildAgentNotificationId } from '../../../../shared/agent-notification-id'
|
||||
@@ -15,7 +14,7 @@ import type {
|
||||
AgentCompletionDispatchMeta,
|
||||
AgentCompletionStatusSnapshot
|
||||
} from './agent-completion-coordinator-types'
|
||||
import { countReposNeedingNotificationDisambiguation } from './terminal-notification-state'
|
||||
import { getNotificationWorkspaceLabels } from './terminal-notification-state'
|
||||
import { createTerminalAttentionSurface } from './terminal-attention-surface'
|
||||
import {
|
||||
applyAgentAttention,
|
||||
@@ -134,13 +133,6 @@ export function dispatchTerminalNotification(
|
||||
|
||||
// Desktop settings are applied in main after independent mobile delivery.
|
||||
|
||||
// Why: prefer worktree.repoId over string-parsing the worktreeId. The
|
||||
// `${repoId}::${path}` format is an implementation detail of id
|
||||
// construction; coupling the notification dispatcher to it would silently
|
||||
// drop the repo label if that format ever changes. The worktree object
|
||||
// itself is the source of truth for its owning repo.
|
||||
const worktree = getWorktreeMapFromState(state).get(worktreeId)
|
||||
const repo = worktree ? getRepoMapFromState(state).get(worktree.repoId) : null
|
||||
const customSoundId = state.settings?.notifications?.customSoundId ?? 'system'
|
||||
const customSoundVolume = state.settings?.notifications?.customSoundVolume ?? null
|
||||
// Why: pane keys are reused across turns. A rich OS notification must not
|
||||
@@ -175,9 +167,7 @@ export function dispatchTerminalNotification(
|
||||
...(notificationId ? { notificationId } : {}),
|
||||
worktreeId: request.workspaceId,
|
||||
paneKey: request.subjectKey ?? undefined,
|
||||
repoLabel: repo?.displayName,
|
||||
worktreeLabel: worktree?.displayName || worktree?.branch || worktreeId,
|
||||
hasMultipleActiveRepos: countReposNeedingNotificationDisambiguation(state) > 1,
|
||||
...getNotificationWorkspaceLabels(state, request.workspaceId, event.terminalTitle),
|
||||
terminalTitle: event.terminalTitle,
|
||||
isActiveWorktree: request.workspaceIsActive,
|
||||
...agentSnapshot
|
||||
|
||||
@@ -279,11 +279,11 @@ export function findIndexedRepoOwnerForHost<T extends RepoOwnerRecord>(
|
||||
return resolution?.kind === 'resolved' ? (resolution.owner as T) : null
|
||||
}
|
||||
|
||||
export function findIndexedFolderWorkspaceOwner(
|
||||
folderWorkspaces: readonly FolderWorkspaceOwnerRecord[] | undefined,
|
||||
export function findIndexedFolderWorkspaceOwner<T extends FolderWorkspaceOwnerRecord>(
|
||||
folderWorkspaces: readonly T[] | undefined,
|
||||
folderWorkspaceId: string,
|
||||
executionHostId?: ExecutionHostId
|
||||
): FolderWorkspaceOwnerRecord | null {
|
||||
): T | null {
|
||||
if (!folderWorkspaces) {
|
||||
return null
|
||||
}
|
||||
@@ -295,14 +295,15 @@ export function findIndexedFolderWorkspaceOwner(
|
||||
const resolution = index.get(
|
||||
executionHostId ? `${folderWorkspaceId}\0${executionHostId}` : folderWorkspaceId
|
||||
)
|
||||
return resolution?.kind === 'resolved' ? resolution.owner : null
|
||||
// The cache is keyed by this exact array, so its owner retains the caller's row type.
|
||||
return resolution?.kind === 'resolved' ? (resolution.owner as T) : null
|
||||
}
|
||||
|
||||
export function findIndexedProjectGroupOwner(
|
||||
projectGroups: readonly ProjectGroupOwnerRecord[] | undefined,
|
||||
export function findIndexedProjectGroupOwner<T extends ProjectGroupOwnerRecord>(
|
||||
projectGroups: readonly T[] | undefined,
|
||||
projectGroupId: string,
|
||||
executionHostId?: ExecutionHostId
|
||||
): ProjectGroupOwnerRecord | null {
|
||||
): T | null {
|
||||
if (!projectGroups) {
|
||||
return null
|
||||
}
|
||||
@@ -314,5 +315,6 @@ export function findIndexedProjectGroupOwner(
|
||||
const resolution = index.get(
|
||||
executionHostId ? `${projectGroupId}\0${executionHostId}` : projectGroupId
|
||||
)
|
||||
return resolution?.kind === 'resolved' ? resolution.owner : null
|
||||
// The cache is keyed by this exact array, so its owner retains the caller's row type.
|
||||
return resolution?.kind === 'resolved' ? (resolution.owner as T) : null
|
||||
}
|
||||
|
||||
@@ -33,6 +33,7 @@ export type NotificationDispatchRequest = {
|
||||
paneKey?: string
|
||||
repoLabel?: string
|
||||
worktreeLabel?: string
|
||||
/** Legacy senders may still provide this; project labels are now always shown. */
|
||||
hasMultipleActiveRepos?: boolean
|
||||
terminalTitle?: string
|
||||
isActiveWorktree?: boolean
|
||||
|
||||
Reference in New Issue
Block a user