diff --git a/src/renderer/src/components/skills/SkillFreshnessUpdateDialog.test.tsx b/src/renderer/src/components/skills/SkillFreshnessUpdateDialog.test.tsx index fe02400a11b..e9d2ef9ad58 100644 --- a/src/renderer/src/components/skills/SkillFreshnessUpdateDialog.test.tsx +++ b/src/renderer/src/components/skills/SkillFreshnessUpdateDialog.test.tsx @@ -49,15 +49,22 @@ vi.mock('@/components/ui/dialog', () => ({ })) vi.mock('@/components/ui/collapsible', () => ({ + // Forward data-* props: the unified skill row tags its Collapsible root with + // the identifiers the assertions below read. Collapsible: ({ children, - defaultOpen = false + defaultOpen = false, + ...rest }: { children?: ReactNode defaultOpen?: boolean - }) => { + } & Record) => { const [open] = useState(defaultOpen) - return
{children}
+ return ( +
+ {children} +
+ ) }, CollapsibleTrigger: ({ children }: { children?: ReactNode }) =>
{children}
, CollapsibleContent: ({ children }: { children?: ReactNode }) =>
{children}
@@ -214,7 +221,7 @@ describe('SkillFreshnessUpdateDialog', () => { expect(container?.textContent).toContain('keeps running in the background') expect(container?.querySelector('[role="progressbar"]')).not.toBeNull() expect( - container?.querySelector('[data-skill-result="orca-cli"]')?.getAttribute('data-status') + container?.querySelector('[data-skill-row="orca-cli"]')?.getAttribute('data-state-label') ).toBe('pending') }) @@ -240,7 +247,7 @@ describe('SkillFreshnessUpdateDialog', () => { expect(container?.textContent).toContain('Updated 1 skill') expect( - container?.querySelector('[data-skill-result="orca-cli"]')?.getAttribute('data-status') + container?.querySelector('[data-skill-row="orca-cli"]')?.getAttribute('data-state-label') ).toBe('done') expect(findButton('Done')).toBeDefined() // The re-scan is what makes the result trustworthy, so it must be requested. @@ -267,15 +274,65 @@ describe('SkillFreshnessUpdateDialog', () => { expect(container?.textContent).toContain('Updated 1 of 2 skills') expect( - container?.querySelector('[data-skill-result="orca-cli"]')?.getAttribute('data-status') + container?.querySelector('[data-skill-row="orca-cli"]')?.getAttribute('data-state-label') ).toBe('done') expect( - container?.querySelector('[data-skill-result="orchestration"]')?.getAttribute('data-status') + container?.querySelector('[data-skill-row="orchestration"]')?.getAttribute('data-state-label') ).toBe('failed') expect(container?.textContent).toContain('skills update exited with code 1') expect(findButton('Retry')).toBeDefined() }) + it('keeps the same row elements across the whole run instead of swapping layouts', async () => { + await renderDialog() + await openViaRequest() + const before = container?.querySelector('[data-skill-row="orca-cli"]') + expect(before?.getAttribute('data-state-label')).toBe('available') + + await emitRun({ state: 'running', names: ['orca-cli'], startedAt: 1, output: '' }) + const during = container?.querySelector('[data-skill-row="orca-cli"]') + expect(during).toBe(before) + expect(during?.getAttribute('data-state-label')).toBe('pending') + + // The re-scan makes the skill current, which would otherwise drop it from + // the group list and blank the row out mid-transition. + mocks.inventory = { + schemaVersion: 1, + installations: [placement('orca-cli', { status: 'current', installedReleaseRevision: 2 })], + eligibleUpdateNames: [], + scannedAt: 5 + } + await emitRun({ state: 'success', names: ['orca-cli'], finishedAt: 2, output: 'done' }) + await rerender() + const after = container?.querySelector('[data-skill-row="orca-cli"]') + expect(after).toBe(before) + expect(after?.getAttribute('data-state-label')).toBe('done') + }) + + it('collapses each skill’s locations behind its own disclosure', async () => { + mocks.inventory = { + schemaVersion: 1, + installations: [ + placement('orca-cli'), + placement('orca-cli', { + rootId: 'plugin', + topology: 'plugin-cache', + status: 'inaccessible' + }) + ], + eligibleUpdateNames: ['orca-cli'], + scannedAt: 1 + } + await renderDialog() + await openViaRequest() + + const row = container?.querySelector('[data-skill-row="orca-cli"]') + expect(row).not.toBeNull() + // Closed by default — the paths are behind the row's own trigger. + expect(row?.getAttribute('data-collapsible-open')).toBe('false') + expect(container?.textContent).toContain('2 locations') + }) + it('shows the captured log verbatim without parsing it', async () => { await renderDialog() await openViaRequest() diff --git a/src/renderer/src/components/skills/SkillFreshnessUpdateDialog.tsx b/src/renderer/src/components/skills/SkillFreshnessUpdateDialog.tsx index 50724730c58..b6892a6904f 100644 --- a/src/renderer/src/components/skills/SkillFreshnessUpdateDialog.tsx +++ b/src/renderer/src/components/skills/SkillFreshnessUpdateDialog.tsx @@ -15,8 +15,7 @@ import { import { Collapsible, CollapsibleContent, CollapsibleTrigger } from '@/components/ui/collapsible' import { TooltipProvider } from '@/components/ui/tooltip' import { groupSkillFreshness } from './skill-freshness-grouping' -import { SkillFreshnessGroup } from './skill-freshness-group' -import { SkillUpdateResultRows } from './skill-update-result-rows' +import { SkillUpdateRow } from './SkillUpdateRow' import { SummaryHeadline, summarizeInventory } from './skill-freshness-summary-headline' import { acknowledgeSkillUpdateRun, @@ -67,16 +66,39 @@ export function SkillFreshnessUpdateDialog(): React.JSX.Element { const [copied, setCopied] = useState(false) const inventory = state.inventory const eligibleNames = useMemo(() => inventory?.eligibleUpdateNames ?? [], [inventory]) + const isRunning = run.state === 'running' + const showResult = run.state === 'success' || run.state === 'error' + const runNames = useMemo(() => (run.state === 'idle' ? [] : run.names), [run]) const groups = useMemo( () => - inventory ? groupSkillFreshness(inventory.installations, inventory.eligibleUpdateNames) : [], - [inventory] + inventory + ? groupSkillFreshness(inventory.installations, inventory.eligibleUpdateNames, runNames) + : [], + [inventory, runNames] ) const hasBlockedGroup = groups.some((group) => group.status === 'cannot-update') const blockedCount = groups.filter((group) => group.status === 'cannot-update').length const summaryKind = summarizeInventory(inventory, hasBlockedGroup) - const isRunning = run.state === 'running' - const showResult = run.state === 'success' || run.state === 'error' + + // Why: one row list for every state. The rows are identical objects across the + // transition, so pressing Update changes each row's leading icon in place + // instead of swapping the dialog's body for a different component. + const rows = useMemo(() => { + const failed = new Set(run.state === 'error' ? run.failedNames : []) + const inRun = new Set(runNames) + return groups.map((group) => { + if (inRun.has(group.name)) { + if (isRunning) { + return { group, state: 'pending' as const } + } + return { group, state: failed.has(group.name) ? ('failed' as const) : ('done' as const) } + } + return { + group, + state: group.status === 'cannot-update' ? ('blocked' as const) : ('available' as const) + } + }) + }, [groups, isRunning, run, runNames]) const handleOpenChange = (next: boolean): void => { if (next) { @@ -193,27 +215,27 @@ export function SkillFreshnessUpdateDialog(): React.JSX.Element { )} {isRunning ? ( - <> - {/* Indeterminate on purpose: the CLI reports no parseable progress. */} -
-
-
- - + // Indeterminate on purpose: the CLI reports no parseable progress. +
+
+
) : null} - {showResult ? ( - + {rows.length > 0 ? ( +
+ + {rows.map((row) => ( + + ))} + +
) : null} {run.state === 'error' ? ( @@ -252,16 +274,6 @@ export function SkillFreshnessUpdateDialog(): React.JSX.Element { {isRunning || showResult ? : null} - {!isRunning && !showResult && groups.length > 0 ? ( -
- - {groups.map((group) => ( - - ))} - -
- ) : null} - {isRunning || showResult ? ( diff --git a/src/renderer/src/components/skills/SkillUpdateRow.tsx b/src/renderer/src/components/skills/SkillUpdateRow.tsx new file mode 100644 index 00000000000..84d7c76af67 --- /dev/null +++ b/src/renderer/src/components/skills/SkillUpdateRow.tsx @@ -0,0 +1,140 @@ +import { CheckCircle2, ChevronDown, Circle, XCircle } from 'lucide-react' +import type { SkillFreshnessGroupModel } from './skill-freshness-grouping' +import { translate } from '@/i18n/i18n' +import { Badge } from '@/components/ui/badge' +import { Collapsible, CollapsibleContent, CollapsibleTrigger } from '@/components/ui/collapsible' +import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' +import { chipLabel, chipTooltip } from './skill-location-chip-copy' +import { skippedReason } from './skill-freshness-skipped-reason' + +export type SkillRowState = 'available' | 'blocked' | 'pending' | 'done' | 'failed' + +/** + * The single status slot, sitting between the name and the location count. + * + * Why one slot rather than a leading icon column: a leading icon has nothing to + * show in the resting state, and reserving the box for it just indents every + * name past an empty gap. Swapping badge for icon in place keeps the name + * flush-left and the row's right edge fixed in every state. + */ +function StateSlot({ state }: { state: SkillRowState }): React.JSX.Element { + switch (state) { + case 'done': + return + case 'failed': + return + case 'pending': + return + case 'blocked': + return ( + + {translate('auto.components.skills.SkillFreshnessRow.statusCantUpdate', 'Skipped')} + + ) + case 'available': + return ( + + {translate( + 'auto.components.skills.SkillFreshnessRow.statusUpdateAvailable', + 'Update available' + )} + + ) + } +} + +/** + * One skill in the update dialog, used unchanged in every state. + * + * The header keeps identical geometry from "update available" through running to + * the result — only the status slot's contents change — so pressing Update + * doesn't replace the dialog's layout with a different one. Locations live + * behind a per-skill disclosure instead of being dumped inline, because a skill + * with several plugin-cache copies otherwise buries the actions. + */ +export function SkillUpdateRow({ + group, + state +}: { + group: SkillFreshnessGroupModel + state: SkillRowState +}): React.JSX.Element { + const locationCount = group.locations.length + return ( + + + + {group.name} + + + + {locationCount === 1 + ? translate('auto.components.skills.SkillUpdateRow.oneLocation', '1 location') + : translate( + 'auto.components.skills.SkillUpdateRow.manyLocations', + '{{value0}} locations', + { value0: locationCount } + )} + + + + + {state === 'failed' ? ( +

+ {translate( + 'auto.components.skills.SkillUpdateResultRows.stillOutdated', + 'Still out of date after the update ran.' + )} +

+ ) : null} + + +
+ {state === 'blocked' ? ( +

+ {skippedReason(group.locations)} +

+ ) : null} + {group.locations.map((location) => ( +
+ {/* Why: plugin-cache paths nest arbitrarily deep. Without an explicit + shrink basis the unbreakable string sets the dialog's width and + pushes the footer actions off-screen. */} + + {location.path} + + {location.chip ? ( + + + + {chipLabel(location.chip)} + + + + {chipTooltip(location.chip)} + + + ) : null} +
+ ))} +
+
+
+ ) +} diff --git a/src/renderer/src/components/skills/skill-freshness-grouping.ts b/src/renderer/src/components/skills/skill-freshness-grouping.ts index b33dd1f49fe..8d89c6ddd0d 100644 --- a/src/renderer/src/components/skills/skill-freshness-grouping.ts +++ b/src/renderer/src/components/skills/skill-freshness-grouping.ts @@ -60,12 +60,19 @@ export function locationChip(installation: SkillFreshnessInstallation): SkillLoc * update disposition. Only skills with an out-of-date official copy are returned — * up-to-date, unrecognized-only, and unreadable-only skills have nothing to change * here, so they are omitted entirely. + * + * `alwaysIncludeNames` overrides that filter. A successful update makes every + * targeted skill current, which would otherwise drop its row the instant the + * re-scan lands — the dialog passes the running/finished run's names so the same + * rows stay put from "update available" through to the result. */ export function groupSkillFreshness( installations: readonly SkillFreshnessInstallation[], - eligibleUpdateNames: readonly string[] + eligibleUpdateNames: readonly string[], + alwaysIncludeNames: readonly string[] = [] ): SkillFreshnessGroupModel[] { const eligible = new Set(eligibleUpdateNames) + const pinned = new Set(alwaysIncludeNames) const byName = new Map() for (const installation of installations) { const entries = byName.get(installation.name) ?? [] @@ -74,7 +81,7 @@ export function groupSkillFreshness( } const groups: SkillFreshnessGroupModel[] = [] for (const [name, entries] of byName) { - if (!entries.some((entry) => entry.status === 'outdated')) { + if (!pinned.has(name) && !entries.some((entry) => entry.status === 'outdated')) { continue } const locations = entries diff --git a/src/renderer/src/components/skills/skill-location-chip-copy.ts b/src/renderer/src/components/skills/skill-location-chip-copy.ts new file mode 100644 index 00000000000..82e709665cc --- /dev/null +++ b/src/renderer/src/components/skills/skill-location-chip-copy.ts @@ -0,0 +1,77 @@ +import type { SkillLocationChip } from './skill-freshness-grouping' +import { translate } from '@/i18n/i18n' + +export function chipLabel(chip: SkillLocationChip): string { + switch (chip) { + case 'current': + return translate('auto.components.skills.SkillFreshnessRow.chipCurrent', 'Current') + case 'unrecognized': + return translate('auto.components.skills.SkillFreshnessRow.chipUnrecognized', 'Unrecognized') + case 'inaccessible': + return translate('auto.components.skills.SkillFreshnessRow.chipInaccessible', 'Inaccessible') + case 'duplicate': + return translate('auto.components.skills.SkillFreshnessRow.chipDuplicate', 'Duplicate') + case 'external-link': + return translate('auto.components.skills.SkillFreshnessRow.chipExternalLink', 'External link') + case 'broken-link': + return translate('auto.components.skills.SkillFreshnessRow.chipBrokenLink', 'Broken link') + case 'read-only': + return translate('auto.components.skills.SkillFreshnessRow.chipReadOnly', 'Read only') + case 'in-a-repo': + return translate('auto.components.skills.SkillFreshnessRow.chipInRepo', 'In a repo') + case 'plugin-cache': + return translate('auto.components.skills.SkillFreshnessRow.chipPluginCache', 'Plugin cache') + } +} + +// Why: chips describe only what a location *is*; the effect on the update +// command lives in the per-skill sentence, so the two never say it twice. +export function chipTooltip(chip: SkillLocationChip): string { + switch (chip) { + case 'current': + return translate( + 'auto.components.skills.SkillFreshnessRow.tipCurrent', + 'This copy matches the current official version.' + ) + case 'unrecognized': + return translate( + 'auto.components.skills.SkillFreshnessRow.tipUnrecognized', + 'This copy doesn’t match any official version — it may be modified, or a different skill with the same name.' + ) + case 'inaccessible': + return translate( + 'auto.components.skills.SkillFreshnessRow.tipInaccessible', + 'Orca couldn’t read this copy (a permissions or file error).' + ) + case 'duplicate': + return translate( + 'auto.components.skills.SkillFreshnessRow.tipDuplicate', + 'A separate copy of this skill, installed apart from the main one.' + ) + case 'external-link': + return translate( + 'auto.components.skills.SkillFreshnessRow.tipExternalLink', + 'A shortcut pointing outside Orca’s skill folders.' + ) + case 'broken-link': + return translate( + 'auto.components.skills.SkillFreshnessRow.tipBrokenLink', + 'A shortcut to something that no longer exists.' + ) + case 'read-only': + return translate( + 'auto.components.skills.SkillFreshnessRow.tipReadOnly', + 'This copy is in a read-only location.' + ) + case 'in-a-repo': + return translate( + 'auto.components.skills.SkillFreshnessRow.tipInRepo', + 'This copy lives inside a project, not your global skills.' + ) + case 'plugin-cache': + return translate( + 'auto.components.skills.SkillFreshnessRow.tipPluginCache', + 'This copy is managed by a plugin.' + ) + } +} diff --git a/src/renderer/src/components/skills/skill-freshness-group.tsx b/src/renderer/src/components/skills/skill-location-chip-copy.tsx similarity index 100% rename from src/renderer/src/components/skills/skill-freshness-group.tsx rename to src/renderer/src/components/skills/skill-location-chip-copy.tsx diff --git a/src/renderer/src/components/skills/skill-update-result-rows.tsx b/src/renderer/src/components/skills/skill-update-result-rows.tsx deleted file mode 100644 index 1ce9a059f79..00000000000 --- a/src/renderer/src/components/skills/skill-update-result-rows.tsx +++ /dev/null @@ -1,63 +0,0 @@ -import { CheckCircle2, Circle, XCircle } from 'lucide-react' -import { translate } from '@/i18n/i18n' - -export type SkillRunRowStatus = 'pending' | 'done' | 'failed' - -function statusIcon(status: SkillRunRowStatus): React.JSX.Element { - if (status === 'done') { - return - } - if (status === 'failed') { - return - } - return -} - -/** - * Per-skill outcome rows for a finished or in-flight run. - * - * Rendered from the run itself rather than the freshness groups: a successful - * update makes every skill current, which drops it out of `groupSkillFreshness` - * entirely, so the groups are empty exactly when the results matter most. - */ -export function SkillUpdateResultRows({ - names, - failedNames, - pending -}: { - names: readonly string[] - failedNames?: readonly string[] - pending?: boolean -}): React.JSX.Element { - const failed = new Set(failedNames ?? []) - return ( -
- {names.map((name) => { - const status: SkillRunRowStatus = pending ? 'pending' : failed.has(name) ? 'failed' : 'done' - return ( -
- {statusIcon(status)} -
-
{name}
- {status === 'failed' ? ( -
- {translate( - 'auto.components.skills.SkillUpdateResultRows.stillOutdated', - 'Still out of date after the update ran.' - )} -
- ) : null} -
-
- ) - })} -
- ) -} diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 678121c6103..c40e23dae27 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -3815,6 +3815,10 @@ "SkillUpdateResultRows": { "stillOutdated": "Still out of date after the update ran." }, + "SkillUpdateRow": { + "oneLocation": "1 location", + "manyLocations": "{{value0}} locations" + }, "SkillFreshnessStatusPill": { "updateAvailable": "Update available", "upToDate": "Up to date", diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index 35a75574c61..663635cb5e4 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -3798,6 +3798,10 @@ }, "SkillUpdateResultRows": { "stillOutdated": "Still out of date after the update ran." + }, + "SkillUpdateRow": { + "oneLocation": "1 location", + "manyLocations": "{{value0}} locations" } }, "sidebar": { diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index 25fca7831ed..5fd59120510 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -3798,6 +3798,10 @@ }, "SkillUpdateResultRows": { "stillOutdated": "Still out of date after the update ran." + }, + "SkillUpdateRow": { + "oneLocation": "1 location", + "manyLocations": "{{value0}} locations" } }, "sidebar": { diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index 78efeeccb2a..b0b0e04ddff 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -3798,6 +3798,10 @@ }, "SkillUpdateResultRows": { "stillOutdated": "Still out of date after the update ran." + }, + "SkillUpdateRow": { + "oneLocation": "1 location", + "manyLocations": "{{value0}} locations" } }, "sidebar": { diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index 0cdc1fb7027..66aac7cd6c3 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -3798,6 +3798,10 @@ }, "SkillUpdateResultRows": { "stillOutdated": "Still out of date after the update ran." + }, + "SkillUpdateRow": { + "oneLocation": "1 location", + "manyLocations": "{{value0}} locations" } }, "sidebar": {