xiaoxue_admin
/
orca
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix: address review findings (#1981)

This commit is contained in:
Jinjing 2026-05-15 13:42:44 -07:00 committed by GitHub
parent b93849a577
commit 12609a50e3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 174 additions and 100 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
src/renderer/src/components/TaskPage.tsx
View File

@ -58,6 +58,7 @@ import RepoDotLabel from '@/components/repo/RepoDotLabel'
import IssueSourceIndicator, { sameGitHubOwnerRepo } from '@/components/github/IssueSourceIndicator'
import IssueSourceSelector, { issueSourceChipClass } from '@/components/github/IssueSourceSelector'
import GitHubRateLimitPill from '@/components/github/GitHubRateLimitPill'
import { reconcileLinearTeamSelection } from '@/components/task-page-linear-team-selection'
import { stripRepoQualifiers } from '../../../shared/task-query'
import GitHubItemDialog from '@/components/GitHubItemDialog'
import GitLabItemDialog from '@/components/GitLabItemDialog'
@ -1078,15 +1079,10 @@ export default function TaskPage(): React.JSX.Element {
return new Set(defaultLinearTeamSelection)
})
// Why: in sticky-all mode, auto-include all teams once the list arrives.
// In explicit-selection mode, the set is already correct from the initializer.
// Why: team IDs belong to one Linear workspace. Switching workspaces while a
// saved subset exists must not leave the task list filtered by stale team IDs.
useEffect(() => {
if (availableTeams.length === 0) {
return
}
if (!defaultLinearTeamSelection) {
setLinearTeamSelection(new Set(availableTeams.map((t) => t.id)))
}
setLinearTeamSelection(reconcileLinearTeamSelection(availableTeams, defaultLinearTeamSelection))
}, [availableTeams, defaultLinearTeamSelection])
const displayedLinearIssues = useMemo(

102
src/renderer/src/components/dashboard/DashboardAgentRow.tsx
View File

@ -1,7 +1,6 @@
import React, { useState, useCallback } from 'react'
import { X, Wrench, ChevronDown } from 'lucide-react'
import { cn } from '@/lib/utils'
import { Tooltip, TooltipTrigger, TooltipContent } from '@/components/ui/tooltip'
import { AgentStateDot, agentStateLabel, type AgentDotState } from '@/components/AgentStateDot'
import { AgentIcon } from '@/lib/agent-catalog'
import { agentTypeToIconAgent, formatAgentTypeLabel } from '@/lib/agent-status'
@ -222,16 +221,12 @@ const DashboardAgentRow = React.memo(function DashboardAgentRow({
bar + right-side dot combo, which double-encoded state. Size md
gives the glyph enough presence for the leading slot without
overpowering the prompt text. */}
<Tooltip>
<TooltipTrigger asChild>
<span className="inline-flex shrink-0 items-center justify-center">
<AgentStateDot state={asDotState(agent.state)} size={stateDotSize} />
</span>
</TooltipTrigger>
<TooltipContent side="top" sideOffset={4}>
{agent.entry.interrupted ? 'Interrupted' : agentStateLabel(asDotState(agent.state))}
</TooltipContent>
</Tooltip>
<span
className="inline-flex shrink-0 items-center justify-center"
title={agent.entry.interrupted ? 'Interrupted' : agentStateLabel(asDotState(agent.state))}
>
<AgentStateDot state={asDotState(agent.state)} size={stateDotSize} />
</span>
{/* Why: identity (Claude/Codex/Gemini/…) sits inline with the prompt
so the reader gets "state → who → what they said" left-to-right
on the top row. The sub-rows (tool step, assistant response) are
@ -239,16 +234,9 @@ const DashboardAgentRow = React.memo(function DashboardAgentRow({
them — keeping the icon only on the prompt row lets the sub-rows
indent under the prompt text cleanly. */}
{!hideIdentityIcon && (
<Tooltip>
<TooltipTrigger asChild>
<span className="inline-flex shrink-0">
<AgentIcon agent={agentTypeToIconAgent(agent.agentType)} size={14} />
</span>
</TooltipTrigger>
<TooltipContent side="top" sideOffset={4}>
{formatAgentTypeLabel(agent.agentType)}
</TooltipContent>
</Tooltip>
<span className="inline-flex shrink-0" title={formatAgentTypeLabel(agent.agentType)}>
<AgentIcon agent={agentTypeToIconAgent(agent.agentType)} size={14} />
</span>
)}
{/* Why: animate between a 1-line clipped height and the content's
natural height using Chromium's `interpolate-size: allow-keywords`
@ -321,27 +309,21 @@ const DashboardAgentRow = React.memo(function DashboardAgentRow({
? formatTimeAgo(startedAt, now)
: null}
</span>
<Tooltip>
<TooltipTrigger asChild>
<button
type="button"
onClick={handleDismiss}
onMouseDown={stopMouseDown}
onKeyDown={stopKeyDown}
className={cn(
'[grid-area:1/1] inline-flex items-center justify-center text-muted-foreground/70 hover:text-foreground',
'opacity-0 transition-opacity duration-150',
'group-hover:opacity-100 focus-visible:opacity-100'
)}
aria-label="Dismiss agent"
>
<X className="size-3.5" />
</button>
</TooltipTrigger>
<TooltipContent side="top" sideOffset={4}>
Dismiss
</TooltipContent>
</Tooltip>
<button
type="button"
onClick={handleDismiss}
onMouseDown={stopMouseDown}
onKeyDown={stopKeyDown}
className={cn(
'[grid-area:1/1] inline-flex items-center justify-center text-muted-foreground/70 hover:text-foreground',
'opacity-0 transition-opacity duration-150',
'group-hover:opacity-100 focus-visible:opacity-100'
)}
aria-label="Dismiss agent"
title="Dismiss"
>
<X className="size-3.5" />
</button>
</span>
)}
{/* Why: when there is no timestamp yet (fresh agent, never
@ -350,27 +332,21 @@ const DashboardAgentRow = React.memo(function DashboardAgentRow({
reachable. Rare path; most rows have a timestamp the moment
they start. */}
{startedAt === null && doneAt === null && (
<Tooltip>
<TooltipTrigger asChild>
<button
type="button"
onClick={handleDismiss}
onMouseDown={stopMouseDown}
onKeyDown={stopKeyDown}
className={cn(
'inline-flex shrink-0 items-center justify-center text-muted-foreground/70 hover:text-foreground',
'opacity-0 transition-opacity duration-150',
'group-hover:opacity-100 focus-visible:opacity-100'
)}
aria-label="Dismiss agent"
>
<X className="size-3.5" />
</button>
</TooltipTrigger>
<TooltipContent side="top" sideOffset={4}>
Dismiss
</TooltipContent>
</Tooltip>
<button
type="button"
onClick={handleDismiss}
onMouseDown={stopMouseDown}
onKeyDown={stopKeyDown}
className={cn(
'inline-flex shrink-0 items-center justify-center text-muted-foreground/70 hover:text-foreground',
'opacity-0 transition-opacity duration-150',
'group-hover:opacity-100 focus-visible:opacity-100'
)}
aria-label="Dismiss agent"
title="Dismiss"
>
<X className="size-3.5" />
</button>
)}
{/* Why: chevron points down when collapsed (content below is
available) and rotates 180° to point up when expanded

2
src/renderer/src/components/sidebar/useWorktreeAgentRows.ts
View File

@ -115,7 +115,7 @@ export function useWorktreeAgentRows(worktreeId: string): DashboardAgentRow[] {
}
out.push(entry)
}
for (const unsupported of Object.values(s.migrationUnsupportedByPtyId)) {
for (const unsupported of Object.values(s.migrationUnsupportedByPtyId ?? {})) {
const entry = migrationUnsupportedToAgentStatusEntry(unsupported)
if (!entry) {
continue

37
src/renderer/src/components/task-page-linear-team-selection.test.ts Normal file
View File

@ -0,0 +1,37 @@
import { describe, expect, it } from 'vitest'
import type { LinearTeam } from '../../../shared/types'
import { reconcileLinearTeamSelection } from './task-page-linear-team-selection'
function team(id: string): LinearTeam {
return {
id,
name: id,
key: id.toUpperCase()
}
}
describe('reconcileLinearTeamSelection', () => {
it('selects every available team when the saved selection is sticky-all', () => {
expect(Array.from(reconcileLinearTeamSelection([team('a'), team('b')], null))).toEqual([
'a',
'b'
])
})
it('preserves saved teams that still exist', () => {
expect(Array.from(reconcileLinearTeamSelection([team('a'), team('b')], ['b']))).toEqual(['b'])
})
it('drops stale saved teams after switching workspaces', () => {
expect(Array.from(reconcileLinearTeamSelection([team('c'), team('d')], ['a', 'd']))).toEqual([
'd'
])
})
it('falls back to all current teams when every saved team is stale', () => {
expect(Array.from(reconcileLinearTeamSelection([team('c'), team('d')], ['a', 'b']))).toEqual([
'c',
'd'
])
})
})

19
src/renderer/src/components/task-page-linear-team-selection.ts Normal file
View File

@ -0,0 +1,19 @@
import type { LinearTeam } from '../../../shared/types'
export function reconcileLinearTeamSelection(
availableTeams: LinearTeam[],
storedSelection: readonly string[] | null | undefined
): ReadonlySet<string> {
const availableIds = availableTeams.map((team) => team.id)
if (availableIds.length === 0) {
return new Set()
}
const availableIdSet = new Set(availableIds)
const validStoredSelection = (storedSelection ?? []).filter((id) => availableIdSet.has(id))
if (validStoredSelection.length > 0) {
return new Set(validStoredSelection)
}
return new Set(availableIds)
}

23
src/renderer/src/components/ui/button.tsx
View File

@ -36,20 +36,21 @@ const buttonVariants = cva(
}
)
function Button({
className,
variant = 'default',
size = 'default',
asChild = false,
...props
}: React.ComponentProps<'button'> &
VariantProps<typeof buttonVariants> & {
asChild?: boolean
}) {
const Button = React.forwardRef<
HTMLButtonElement,
React.ComponentProps<'button'> &
VariantProps<typeof buttonVariants> & {
asChild?: boolean
}
>(function Button(
{ className, variant = 'default', size = 'default', asChild = false, ...props },
ref
) {
const Comp = asChild ? Slot.Root : 'button'
return (
<Comp
ref={ref}
data-slot="button"
data-variant={variant}
data-size={size}
@ -57,6 +58,6 @@ function Button({
{...props}
/>
)
}
})
export { Button, buttonVariants }

33
src/renderer/src/lib/migration-unsupported-agent-entry.test.ts Normal file
View File

@ -0,0 +1,33 @@
import { describe, expect, it } from 'vitest'
import { migrationUnsupportedToAgentStatusEntry } from './migration-unsupported-agent-entry'
import type { MigrationUnsupportedPtyEntry } from '../../../shared/agent-status-types'
describe('migrationUnsupportedToAgentStatusEntry', () => {
it('returns a stable synthetic entry for the same migration record', () => {
const unsupported: MigrationUnsupportedPtyEntry = {
ptyId: 'pty-1',
paneKey: 'tab-1:11111111-1111-4111-8111-111111111111',
reason: 'legacy-numeric-pane-key',
source: 'local',
updatedAt: 1234
}
const first = migrationUnsupportedToAgentStatusEntry(unsupported)
const second = migrationUnsupportedToAgentStatusEntry(unsupported)
expect(second).toBe(first)
expect(first?.updatedAt).toBe(Number.MAX_SAFE_INTEGER)
})
it('caches null for records that cannot be projected to a pane key', () => {
const unsupported: MigrationUnsupportedPtyEntry = {
ptyId: 'pty-1',
reason: 'legacy-numeric-pane-key',
source: 'ssh',
updatedAt: 1234
}
expect(migrationUnsupportedToAgentStatusEntry(unsupported)).toBeNull()
expect(migrationUnsupportedToAgentStatusEntry(unsupported)).toBeNull()
})
})

46
src/renderer/src/lib/migration-unsupported-agent-entry.ts
View File

@ -3,25 +3,37 @@ import type {
MigrationUnsupportedPtyEntry
} from '../../../shared/agent-status-types'
const cachedMigrationUnsupportedEntries = new WeakMap<
MigrationUnsupportedPtyEntry,
AgentStatusEntry | null
>()
export function migrationUnsupportedToAgentStatusEntry(
entry: MigrationUnsupportedPtyEntry
): AgentStatusEntry | null {
if (!entry.paneKey) {
return null
}
const now = Date.now()
return {
state: 'blocked',
prompt: 'Agent unavailable after pane identity migration',
// Why: this is a persistent migration block, not a hook heartbeat. Keep it
// fresh while present so normal stale-status decay does not hide it.
updatedAt: Math.max(entry.updatedAt, now),
stateStartedAt: entry.updatedAt,
agentType: 'unknown',
paneKey: entry.paneKey,
terminalTitle: 'Migration unsupported',
stateHistory: [],
lastAssistantMessage:
'Restart this terminal so Orca can attach a stable UUID pane key to agent hooks.'
const cached = cachedMigrationUnsupportedEntries.get(entry)
if (cached !== undefined) {
return cached
}
const converted: AgentStatusEntry | null = !entry.paneKey
? null
: {
state: 'blocked',
prompt: 'Agent unavailable after pane identity migration',
// Why: this synthetic row represents a persistent migration block. Keep
// it "fresh" without Date.now() so Zustand selectors can return a stable
// cached object for the same store snapshot.
updatedAt: Number.MAX_SAFE_INTEGER,
stateStartedAt: entry.updatedAt,
agentType: 'unknown',
paneKey: entry.paneKey,
terminalTitle: 'Migration unsupported',
stateHistory: [],
lastAssistantMessage:
'Restart this terminal so Orca can attach a stable UUID pane key to agent hooks.'
}
cachedMigrationUnsupportedEntries.set(entry, converted)
return converted
}