From e133d93b7bb951b25aefd0c292136fff38960a98 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 24 Jul 2026 15:53:22 -0700 Subject: [PATCH] fix(skills): stop promising a skill update the command cannot deliver (#10436) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(skills): stop promising a skill update the command cannot deliver An "Update available" badge could never clear: pressing Update ran `npx skills update --global`, which reported "All global skills are up to date" and wrote nothing, while the badge stayed on. Freshness marked a name updatable whenever ANY placement was outdated, including a standalone duplicate in an agent home. The global command only converges the canonical copy and its symlink aliases, so a stale duplicate kept the badge lit with no command that could clear it. - Count only reliably-convergent placements toward the update promise, so a stale duplicate no longer advertises an update that cannot land. - Give the duplicate chip a real skipped-reason instead of falling through to the generic sentence, naming the copy and how to resolve it. - Surface the existing freshness review dialog from the setup rails via a Details link, so a blocked or duplicate copy is explainable where the user actually sees the badge. Covers orchestration, Computer Use, Ephemeral VMs, Linear, the CLI section, the floating orchestration modal, the Browser Use card, and the Mobile Emulator row. * fix(skills): say when a skill copy needs attention instead of reading as all-clear A skill with an out-of-date copy the update command cannot reach rendered as a green "Installed" pill. That is honest about the main copy but reads as all-clear, so real drift in an agent home stayed invisible — the user had no reason to suspect there was anything to click. - Add a needs-attention display status (amber) for a placement that is not current but has no eligible update: stale duplicates, edited copies, read-only, inaccessible, and broken or external links. Presence-only stays green, and an unscanned inventory stays quiet so nothing flashes amber on launch. - State the reason inline on the setup rails, so the cause is readable without opening the review dialog; Details still opens the full per-location list. - Extract the skipped-reason sentence into its own module so the rails and the dialog share one source and can never drift apart. - Carry the state through the settings sidebar badge so the nav and the card cannot disagree. * fix(skills): give the inline skill warning something to point at The shared reason sentences are deictic — "this copy", "the copy here" — because they were written for the review dialog, where the location rows they describe sit directly beneath them. On the setup rails there are no rows, so "this" referred to nothing and the sentence read as if it were about the skill itself. Name the offending paths above the sentence on the rails, so the referent is present before the wording that depends on it. Every copy sharing the blocking reason is listed, not just the first, so resolving one does not leave the badge unexplained. The dialog keeps its existing wording and rows unchanged. * fix(skills): stop withholding the update over copies it cannot reach Eligibility is now decided purely over the placements the global command actually converges — the canonical copy and its symlink aliases. A project skill, plugin cache, standalone duplicate, or unreadable copy in another agent's home used to withhold the update from the whole name. `skills update --global` provably never writes any of them, so that refused work the command could have done over a copy that was never at stake. A blocked *convergent* copy still withholds it: that is the placement the command writes to, and overwriting it is the real data-loss case. Also drop the inline reason from the setup rails. The reasons are written to sit beside the location rows they describe, so on a card they had nothing to point at and could only ever name one cause. The rails now mark Details with a warning icon when a copy needs the user's own hands, and the dialog does the explaining with every location and cause it knows about. * fix(skills): make the skill Details affordance read as a control The review link rendered as bare text, so nothing but hover said it could be clicked — the badge told the user something was wrong and then gave them no visible way in. Use the ghost variant so it carries a hover/focus background and a real hit target instead of a zero-padding text run, and add a chevron so it reads as a control at rest. The chevron points right, not down: this opens the review dialog, while a down chevron already means the in-place expander inside that dialog. --- .../skill-freshness-eligibility.test.ts | 67 ++++++++++++--- .../skills/skill-freshness-eligibility.ts | 45 +++++----- .../skills/skill-freshness-inventory.test.ts | 22 +++-- .../feature-wall/BrowserUseSkillSetupCard.tsx | 6 ++ .../FloatingTerminalOrchestrationDialog.tsx | 20 +++-- .../MobileEmulatorAgentControlRow.tsx | 4 + .../components/settings/SettingsSidebar.tsx | 7 +- .../skills/SkillFreshnessStatusPill.test.tsx | 42 +++++++-- .../skills/SkillFreshnessStatusPill.tsx | 71 ++++++++++++++-- .../skills/skill-freshness-group.tsx | 71 +--------------- .../skills/skill-freshness-grouping.test.ts | 22 +++++ .../skills/skill-freshness-grouping.ts | 2 +- .../skill-freshness-skipped-reason.test.ts | 32 +++++++ .../skills/skill-freshness-skipped-reason.ts | 85 +++++++++++++++++++ src/renderer/src/i18n/locales/en.json | 7 +- src/renderer/src/i18n/locales/es.json | 7 +- src/renderer/src/i18n/locales/ja.json | 7 +- src/renderer/src/i18n/locales/ko.json | 7 +- src/renderer/src/i18n/locales/zh.json | 7 +- .../src/lib/settings-navigation-types.ts | 1 + .../skill-freshness-display-status.test.ts | 15 +++- .../src/lib/skill-freshness-display-status.ts | 46 ++++++++-- 22 files changed, 447 insertions(+), 146 deletions(-) create mode 100644 src/renderer/src/components/skills/skill-freshness-skipped-reason.test.ts create mode 100644 src/renderer/src/components/skills/skill-freshness-skipped-reason.ts diff --git a/src/main/skills/skill-freshness-eligibility.test.ts b/src/main/skills/skill-freshness-eligibility.test.ts index e37541975..cd8262986 100644 --- a/src/main/skills/skill-freshness-eligibility.test.ts +++ b/src/main/skills/skill-freshness-eligibility.test.ts @@ -55,14 +55,39 @@ describe('skill freshness name-scoped update eligibility', () => { ['current', 'read-only'], ['current', 'repo-scope'], ['current', 'plugin-cache'] - ] as const)('poisons a name for a %s placement in %s topology', (status, topology) => { - expect( - eligibleSkillUpdateNames([ - placement('orca-cli'), - placement('orca-cli', { id: `poison-${status}-${topology}`, status, topology }) - ]) - ).toEqual([]) - }) + ] as const)( + 'still updates the canonical copy despite a %s placement in %s topology', + (status, topology) => { + // Why: `--global` provably never writes these placements, so withholding the + // update over one refuses work the command could do to a copy that is never at + // stake. The canonical copy converges and the outlier is reported separately. + expect( + eligibleSkillUpdateNames([ + placement('orca-cli'), + placement('orca-cli', { id: `outlier-${status}-${topology}`, status, topology }) + ]) + ).toEqual(['orca-cli']) + } + ) + + it.each(['unrecognized', 'inaccessible', 'newer-known'] as const)( + 'withholds the update when the convergent copy itself is %s', + (status) => { + // Why: this is the placement the command writes to, so overwriting it is the + // real data-loss case the rail exists to avoid. + expect( + eligibleSkillUpdateNames([ + placement('orca-cli', { id: 'blocked-canonical', status }), + placement('orca-cli', { + id: 'orca-cli-claude', + rootId: 'home-claude', + topology: 'provider-alias', + status: 'outdated' + }) + ]) + ).toEqual([]) + } + ) it('still updates the canonical copy when a clean standalone duplicate exists', () => { // Why: a duplicate no longer omits the whole name — the canonical copy converges @@ -82,6 +107,26 @@ describe('skill freshness name-scoped update eligibility', () => { ).toEqual(['orca-cli']) }) + it('does not promise an update when only an unreachable duplicate is outdated', () => { + // Why: `--global` converges the canonical copy and its aliases only. Offering the + // name here advertises an update the command reports as already up to date, so the + // badge could never clear; the dialog explains the duplicate as skipped instead. + expect( + eligibleSkillUpdateNames([ + placement('orchestration', { status: 'current' }), + placement('orchestration', { + id: 'orchestration-factory', + rootId: 'home-factory', + unresolvedPath: '/home/.factory/skills/orchestration', + resolvedPath: '/home/.factory/skills/orchestration', + physicalIdentity: 'physical-orchestration-factory', + topology: 'independent-copy', + status: 'outdated' + }) + ]) + ).toEqual([]) + }) + it('does not offer a skill that exists only as a standalone copy', () => { // Why: with no canonical or alias to anchor `--global`, the command has no // reliable target, so a duplicate-only skill stays unoffered. @@ -98,7 +143,9 @@ describe('skill freshness name-scoped update eligibility', () => { ).toEqual([]) }) - it('does not offer an all-current name or let another safe name hide a poisoned one', () => { + it('scopes each name independently and leaves an all-current name alone', () => { + // Why: a project copy is never written by `--global`, so it does not speak for + // the global one — while a name whose convergent copy is current stays unoffered. expect( eligibleSkillUpdateNames([ placement('computer-use', { status: 'current' }), @@ -109,7 +156,7 @@ describe('skill freshness name-scoped update eligibility', () => { topology: 'repo-scope' }) ]) - ).toEqual([]) + ).toEqual(['orchestration']) }) it('builds only an explicit, deterministic global command', () => { diff --git a/src/main/skills/skill-freshness-eligibility.ts b/src/main/skills/skill-freshness-eligibility.ts index f4eb27a9f..76e216116 100644 --- a/src/main/skills/skill-freshness-eligibility.ts +++ b/src/main/skills/skill-freshness-eligibility.ts @@ -3,6 +3,17 @@ import { type SkillFreshnessInstallation } from '../../shared/skill-freshness' +/** + * Names the global update command can actually converge. + * + * Eligibility is decided purely over the placements that command touches — the + * canonical copy and its symlink aliases. Copies it provably leaves alone (standalone + * duplicates, project skills, plugin caches, links out of tree) neither authorize an + * update nor withhold one: the badge would otherwise promise work the command cannot + * do, or refuse work it could, over a copy that is never at stake either way. A + * blocked *convergent* copy still withholds it, because that is the placement the + * command would write to and overwriting it is the real data-loss case. + */ export function eligibleSkillUpdateNames( installations: readonly SkillFreshnessInstallation[] ): string[] { @@ -14,27 +25,23 @@ export function eligibleSkillUpdateNames( } const eligible: string[] = [] - for (const [name, entries] of byName) { - const hasOutdated = entries.some((entry) => entry.status === 'outdated') - const everyPlacementIsOfficialAndUpdatable = entries.every( - (entry) => - (entry.status === 'current' || entry.status === 'outdated') && - // Why: the rail reliably converges the canonical copy and its symlink aliases. - // A standalone duplicate no longer blocks the whole name — the canonical copy - // still updates and the duplicate row is flagged as maybe-not-reached — while - // data-loss topologies (unrecognized/read-only/etc.) still poison via these checks. - (SUPPORTED_GLOBAL_SKILL_TOPOLOGIES.has(entry.topology) || - entry.topology === 'independent-copy') && - Boolean(entry.resolvedPath && entry.physicalIdentity) - ) - // Why: only offer the global command when a reliably-convergent placement anchors it, - // so a skill that exists solely as a standalone copy never draws a command that could - // no-op or error against a canonical install that isn't there. - const hasReliableTarget = entries.some((entry) => + for (const [, entries] of byName) { + const convergent = entries.filter((entry) => SUPPORTED_GLOBAL_SKILL_TOPOLOGIES.has(entry.topology) ) - if (hasOutdated && everyPlacementIsOfficialAndUpdatable && hasReliableTarget) { - eligible.push(name) + // Why: without a convergent placement the command has no anchor, so it would + // no-op or error against a canonical install that isn't there. + if (convergent.length === 0) { + continue + } + const hasOutdated = convergent.some((entry) => entry.status === 'outdated') + const everyConvergentCopyIsSafeToWrite = convergent.every( + (entry) => + (entry.status === 'current' || entry.status === 'outdated') && + Boolean(entry.resolvedPath && entry.physicalIdentity) + ) + if (hasOutdated && everyConvergentCopyIsSafeToWrite) { + eligible.push(entries[0].name) } } return eligible.sort((left, right) => left.localeCompare(right, 'en')) diff --git a/src/main/skills/skill-freshness-inventory.test.ts b/src/main/skills/skill-freshness-inventory.test.ts index 1570430ad..2b010639e 100644 --- a/src/main/skills/skill-freshness-inventory.test.ts +++ b/src/main/skills/skill-freshness-inventory.test.ts @@ -192,7 +192,7 @@ describe('read-only skill freshness inventory', () => { ) it.runIf(process.platform !== 'win32')( - 'deduplicates aliases within an unsupported topology without hiding its poison', + 'deduplicates aliases within an unsupported topology while still updating the canonical copy', async () => { const test = await fixture() await test.writeSkill(join(test.homeDir, '.agents', 'skills'), test.oldMarkdown) @@ -217,11 +217,11 @@ describe('read-only skill freshness inventory', () => { expect( inventory.installations.filter((entry) => entry.topology === 'repo-scope') ).toHaveLength(1) - expect(inventory.eligibleUpdateNames).toEqual([]) + expect(inventory.eligibleUpdateNames).toEqual(['orca-cli']) } ) - it('keeps inaccessible placements visible and lets them poison the name', async () => { + it('keeps an unreadable foreign-home placement visible without withholding the update', async () => { const test = await fixture() await test.writeSkill(join(test.homeDir, '.agents', 'skills'), test.oldMarkdown) const inaccessiblePath = join(test.homeDir, '.codex', 'skills', 'orca-cli') @@ -243,7 +243,9 @@ describe('read-only skill freshness inventory', () => { 'outdated', 'inaccessible' ]) - expect(inventory.eligibleUpdateNames).toEqual([]) + // Why: `--global` never writes another agent's home, so an unreadable copy there + // cannot be harmed by the update and must not withhold it from the canonical copy. + expect(inventory.eligibleUpdateNames).toEqual(['orca-cli']) }) it('does not lose an inaccessible known repository placement', async () => { @@ -274,14 +276,14 @@ describe('read-only skill freshness inventory', () => { }) ]) ) - expect(inventory.eligibleUpdateNames).toEqual([]) + expect(inventory.eligibleUpdateNames).toEqual(['orca-cli']) }) it.each([ ['repo', 'repo-scope'], ['plugin', 'plugin-cache'] ] as const)( - 'keeps an official %s placement informational and name-poisoning', + 'keeps an official %s placement informational without withholding the update', async (kind, topology) => { const test = await fixture() await test.writeSkill(join(test.homeDir, '.agents', 'skills'), test.oldMarkdown) @@ -305,7 +307,7 @@ describe('read-only skill freshness inventory', () => { }) expect(inventory.installations.some((entry) => entry.topology === topology)).toBe(true) - expect(inventory.eligibleUpdateNames).toEqual([]) + expect(inventory.eligibleUpdateNames).toEqual(['orca-cli']) } ) @@ -351,7 +353,7 @@ describe('read-only skill freshness inventory', () => { }) }) - it('withholds updates when stored repositories exceed the probe budget', async () => { + it('reports the repository scan limit without withholding the global update', async () => { const test = await fixture() await test.writeSkill(join(test.homeDir, '.agents', 'skills'), test.oldMarkdown) const repos = Array.from( @@ -371,6 +373,8 @@ describe('read-only skill freshness inventory', () => { expect.objectContaining({ errorCategory: 'repository-scan-limit', status: 'inaccessible' }) ]) ) - expect(inventory.eligibleUpdateNames).toEqual([]) + // Why: unscanned repositories only ever hold project skills, which the global + // command does not touch, so the limit is reported without blocking the update. + expect(inventory.eligibleUpdateNames).toEqual(['orca-cli']) }) }) diff --git a/src/renderer/src/components/feature-wall/BrowserUseSkillSetupCard.tsx b/src/renderer/src/components/feature-wall/BrowserUseSkillSetupCard.tsx index b8b1cd583..87e733f76 100644 --- a/src/renderer/src/components/feature-wall/BrowserUseSkillSetupCard.tsx +++ b/src/renderer/src/components/feature-wall/BrowserUseSkillSetupCard.tsx @@ -1,6 +1,7 @@ import type { JSX } from 'react' import { ORCA_CLI_SKILL_INSTALL_COMMAND, + ORCA_CLI_SKILL_NAME, ORCA_CLI_SKILL_UPDATE_COMMAND } from '@/lib/agent-feature-install-commands' import { @@ -74,6 +75,11 @@ export function BrowserUseSkillSetupCard(props: { onBeforeOpenTerminal={handleBeforeOpenTerminal} showRecheckWhenInstalled={false} onRecheck={skill.refresh} + // Why: the local-host-only freshness scan cannot vouch for a WSL runtime, + // so fall back to the presence-only pill there (mirrors the settings cards). + freshnessSkillName={ + activeSkillRuntime.agentRuntime?.runtime === 'wsl' ? undefined : ORCA_CLI_SKILL_NAME + } /> ) diff --git a/src/renderer/src/components/floating-terminal/FloatingTerminalOrchestrationDialog.tsx b/src/renderer/src/components/floating-terminal/FloatingTerminalOrchestrationDialog.tsx index abfc314d6..6f9aed3f2 100644 --- a/src/renderer/src/components/floating-terminal/FloatingTerminalOrchestrationDialog.tsx +++ b/src/renderer/src/components/floating-terminal/FloatingTerminalOrchestrationDialog.tsx @@ -8,6 +8,7 @@ import { } from '@/components/ui/dialog' import { AgentSkillSetupPanel } from '@/components/settings/AgentSkillSetupPanel' import { IntegrationStatusPill } from '@/components/integration-status-pill' +import { SkillFreshnessStatusPill } from '@/components/skills/SkillFreshnessStatusPill' import { ORCHESTRATION_SKILL_NAME } from '@/lib/agent-feature-install-commands' import { AGENT_SKILL_CLI_PREREQUISITE_NOTICE, @@ -94,12 +95,19 @@ export function FloatingTerminalOrchestrationDialog({ )} ) : orchestrationSkillDetected ? ( - - {translate( - 'auto.components.floating.terminal.FloatingTerminalOrchestrationDialog.630c0ac8c8', - 'Installed' - )} - + // Why: the modal owns the status pill, so it must carry the same + // freshness signal — and route to the same review dialog — as the + // settings card for this skill; WSL falls back to presence-only. + activeSkillRuntime.agentRuntime?.runtime === 'wsl' ? ( + + {translate( + 'auto.components.floating.terminal.FloatingTerminalOrchestrationDialog.630c0ac8c8', + 'Installed' + )} + + ) : ( + + ) ) : ( {translate( diff --git a/src/renderer/src/components/settings/MobileEmulatorAgentControlRow.tsx b/src/renderer/src/components/settings/MobileEmulatorAgentControlRow.tsx index 17946134b..ec1403f6a 100644 --- a/src/renderer/src/components/settings/MobileEmulatorAgentControlRow.tsx +++ b/src/renderer/src/components/settings/MobileEmulatorAgentControlRow.tsx @@ -1,6 +1,7 @@ import { Import, Loader2 } from 'lucide-react' import { ORCA_CLI_SKILL_INSTALL_COMMAND, + ORCA_CLI_SKILL_NAME, ORCA_CLI_SKILL_UPDATE_COMMAND } from '@/lib/agent-feature-install-commands' import { @@ -171,6 +172,9 @@ export function MobileEmulatorAgentControlRow(): React.JSX.Element { await ensureOrcaCliAvailableForAgentSkillTerminal() }} onRecheck={setup.refreshCliSkill} + // Why: this row builds its commands for the local host only, so the + // local-host freshness scan can vouch for the copy it points at. + freshnessSkillName={ORCA_CLI_SKILL_NAME} /> diff --git a/src/renderer/src/components/settings/SettingsSidebar.tsx b/src/renderer/src/components/settings/SettingsSidebar.tsx index 54908713b..d9a0093f0 100644 --- a/src/renderer/src/components/settings/SettingsSidebar.tsx +++ b/src/renderer/src/components/settings/SettingsSidebar.tsx @@ -162,6 +162,11 @@ export function SettingsSidebar({ 'auto.components.skills.SkillFreshnessStatusPill.updateAvailable', 'Update available' ) + case 'needs-attention': + return translate( + 'auto.components.skills.SkillFreshnessStatusPill.needsAttention', + 'Needs attention' + ) case 'checking': return translate('auto.components.settings.AgentSkillSetupPanel.68a468752e', 'Checking...') } @@ -171,7 +176,7 @@ export function SettingsSidebar({ 'ml-auto shrink-0 rounded-full border px-1.5 py-0.5 text-[10px] font-medium leading-none', status === 'installed' || status === 'up-to-date' ? 'border-status-success-border bg-status-success-background text-status-success' - : status === 'update-available' + : status === 'update-available' || status === 'needs-attention' ? 'border-amber-500/40 bg-amber-500/10 text-amber-700 dark:text-amber-300' : status === 'install' ? 'border-foreground/15 bg-foreground/10 text-foreground' diff --git a/src/renderer/src/components/skills/SkillFreshnessStatusPill.test.tsx b/src/renderer/src/components/skills/SkillFreshnessStatusPill.test.tsx index 785b67aaf..e3b1d1e01 100644 --- a/src/renderer/src/components/skills/SkillFreshnessStatusPill.test.tsx +++ b/src/renderer/src/components/skills/SkillFreshnessStatusPill.test.tsx @@ -5,6 +5,7 @@ import { createRoot, type Root } from 'react-dom/client' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { SkillFreshnessInventory } from '../../../../shared/skill-freshness' import { SkillFreshnessStatusPill } from './SkillFreshnessStatusPill' +import { consumeSkillFreshnessUpdateDialogRequest } from './skill-freshness-update-dialog' const mocks = vi.hoisted(() => ({ inventory: null as SkillFreshnessInventory | null @@ -19,6 +20,14 @@ vi.mock('@/hooks/useSkillFreshness', () => ({ }) })) +function detailsButton(container: HTMLDivElement): HTMLButtonElement | null { + return container.querySelector('[data-slot="button"]') +} + +function pillText(container: HTMLDivElement): string { + return (container.textContent ?? '').replace(detailsButton(container)?.textContent ?? '', '') +} + function inventory( entries: { name: string; status: 'current' | 'outdated' | 'unrecognized' }[], eligibleUpdateNames: string[] @@ -66,6 +75,8 @@ async function renderPill(skillName: string): Promise { describe('SkillFreshnessStatusPill', () => { beforeEach(() => { mocks.inventory = null + // Why: the dialog request is module-level state shared across tests. + consumeSkillFreshnessUpdateDialogRequest() }) afterEach(async () => { @@ -80,16 +91,21 @@ describe('SkillFreshnessStatusPill', () => { it('shows Update available for an eligible outdated skill', async () => { mocks.inventory = inventory([{ name: 'orca-cli', status: 'outdated' }], ['orca-cli']) - expect((await renderPill('orca-cli')).textContent).toBe('Update available') + const rendered = await renderPill('orca-cli') + expect(pillText(rendered)).toBe('Update available') + expect(detailsButton(rendered)?.textContent).toBe('Details') }) it('shows Up to date when every placement is current', async () => { mocks.inventory = inventory([{ name: 'orca-cli', status: 'current' }], []) - expect((await renderPill('orca-cli')).textContent).toBe('Up to date') + const rendered = await renderPill('orca-cli') + expect(pillText(rendered)).toBe('Up to date') + // Why: nothing is out of date, so the review dialog would have no row to show. + expect(detailsButton(rendered)).toBeNull() }) - it('falls back to Installed for a blocked outdated placement', async () => { + it('flags a blocked outdated placement instead of reading as all-clear', async () => { mocks.inventory = inventory( [ { name: 'orca-cli', status: 'outdated' }, @@ -98,10 +114,26 @@ describe('SkillFreshnessStatusPill', () => { [] ) - expect((await renderPill('orca-cli')).textContent).toBe('Installed') + const rendered = await renderPill('orca-cli') + // Why: a green pill over a copy the update cannot reach hides real drift. + expect(pillText(rendered)).toBe('Needs attention') + expect(detailsButton(rendered)?.textContent).toBe('Details') }) it('falls back to Installed before the inventory loads', async () => { - expect((await renderPill('orca-cli')).textContent).toBe('Installed') + const rendered = await renderPill('orca-cli') + expect(pillText(rendered)).toBe('Installed') + expect(detailsButton(rendered)).toBeNull() + }) + + it('opens the freshness review dialog from Details', async () => { + mocks.inventory = inventory([{ name: 'orca-cli', status: 'outdated' }], ['orca-cli']) + const rendered = await renderPill('orca-cli') + + await act(async () => { + detailsButton(rendered)?.click() + }) + + expect(consumeSkillFreshnessUpdateDialogRequest()).toBe(true) }) }) diff --git a/src/renderer/src/components/skills/SkillFreshnessStatusPill.tsx b/src/renderer/src/components/skills/SkillFreshnessStatusPill.tsx index d71bf0d8b..6329622a1 100644 --- a/src/renderer/src/components/skills/SkillFreshnessStatusPill.tsx +++ b/src/renderer/src/components/skills/SkillFreshnessStatusPill.tsx @@ -1,15 +1,17 @@ import { useSkillFreshness } from '@/hooks/useSkillFreshness' import { translate } from '@/i18n/i18n' +import { Button } from '@/components/ui/button' import { IntegrationStatusPill } from '@/components/integration-status-pill' -import { getSkillFreshnessDisplayStatus } from '@/lib/skill-freshness-display-status' +import { cn } from '@/lib/utils' +import { AlertTriangle, ChevronRight } from 'lucide-react' +import { + getSkillFreshnessDisplayStatus, + hasSkillCopyNeedingAttention, + type SkillFreshnessDisplayStatus +} from '@/lib/skill-freshness-display-status' +import { requestSkillFreshnessUpdateDialog } from './skill-freshness-update-dialog' -// Why: the setup rails' Installed pill is presence-only; when freshness knows a -// safe update exists (or that every copy is current) the pill should say so. -// Falls back to plain Installed for blocked/unrecognized copies so an unsafe -// placement is never advertised as updatable here. -export function SkillFreshnessStatusPill({ skillName }: { skillName: string }): React.JSX.Element { - const { inventory } = useSkillFreshness() - const status = getSkillFreshnessDisplayStatus(inventory, skillName) +function statusPill(status: SkillFreshnessDisplayStatus): React.JSX.Element { if (status === 'update-available') { return ( @@ -20,6 +22,16 @@ export function SkillFreshnessStatusPill({ skillName }: { skillName: string }): ) } + if (status === 'needs-attention') { + return ( + + {translate( + 'auto.components.skills.SkillFreshnessStatusPill.needsAttention', + 'Needs attention' + )} + + ) + } if (status === 'up-to-date') { return ( @@ -33,3 +45,46 @@ export function SkillFreshnessStatusPill({ skillName }: { skillName: string }): ) } + +// Why: the setup rails' Installed pill is presence-only. Freshness knows more — that +// a safe update exists, that every copy is current, or that a copy is out of date +// somewhere the update cannot reach — and green must never stand in for that last +// case, which is real drift the user would otherwise have no way to see. +export function SkillFreshnessStatusPill({ skillName }: { skillName: string }): React.JSX.Element { + const { inventory } = useSkillFreshness() + const status = getSkillFreshnessDisplayStatus(inventory, skillName) + // Why: the dialog lists every placement, so Details is offered whenever a placement + // is what drove the status — an available update, or a copy that blocked one. + const hasDetails = status === 'update-available' || status === 'needs-attention' + // Why: the badge alone can't say which copies are wrong, and the reasons only read + // correctly beside the locations they describe. Marking the way in is enough here — + // the dialog does the explaining, with every location and cause it knows about. + const needsAttention = hasSkillCopyNeedingAttention(inventory, skillName) + return ( + + {statusPill(status)} + {hasDetails ? ( + + ) : null} + + ) +} diff --git a/src/renderer/src/components/skills/skill-freshness-group.tsx b/src/renderer/src/components/skills/skill-freshness-group.tsx index 514acad74..977af3482 100644 --- a/src/renderer/src/components/skills/skill-freshness-group.tsx +++ b/src/renderer/src/components/skills/skill-freshness-group.tsx @@ -1,11 +1,8 @@ -import type { - SkillFreshnessGroupModel, - SkillLocationChip, - SkillLocationRow -} from './skill-freshness-grouping' +import type { SkillFreshnessGroupModel, SkillLocationChip } from './skill-freshness-grouping' import { translate } from '@/i18n/i18n' import { Badge } from '@/components/ui/badge' import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' +import { skippedReason } from './skill-freshness-skipped-reason' function chipLabel(chip: SkillLocationChip): string { switch (chip) { @@ -82,70 +79,6 @@ function chipTooltip(chip: SkillLocationChip): string { } } -// Why: a skill is skipped for one concrete reason; lead with the highest-priority -// blocking placement so the sentence explains the real cause (an edited copy is -// more useful to surface than a downstream symptom). -const SKIPPED_REASON_PRIORITY: SkillLocationChip[] = [ - 'unrecognized', - 'read-only', - 'inaccessible', - 'in-a-repo', - 'plugin-cache', - 'external-link', - 'broken-link' -] - -function skippedReason(locations: readonly SkillLocationRow[]): string { - const present = new Set(locations.map((location) => location.chip)) - const chip = SKIPPED_REASON_PRIORITY.find((candidate) => present.has(candidate)) - switch (chip) { - case 'unrecognized': - return translate( - 'auto.components.skills.SkillFreshnessRow.skippedReasonUnrecognized', - 'The copy here doesn’t match the official version — it may be modified, or a different skill with the same name. Orca left it out of the update so it won’t overwrite it. Remove it if you want Orca to update this skill.' - ) - case 'read-only': - return translate( - 'auto.components.skills.SkillFreshnessRow.skippedReasonReadOnly', - 'This copy is in a read-only location, so Orca left it out of the update. Change its permissions to let Orca update it.' - ) - case 'inaccessible': - return translate( - 'auto.components.skills.SkillFreshnessRow.skippedReasonInaccessible', - 'Orca couldn’t read this copy, so it left the skill out of the update.' - ) - case 'in-a-repo': - return translate( - 'auto.components.skills.SkillFreshnessRow.skippedReasonInRepo', - 'This is a project skill, not a global one — Orca only updates your global skills, so it left this out of the update.' - ) - case 'plugin-cache': - return translate( - 'auto.components.skills.SkillFreshnessRow.skippedReasonPluginCache', - 'A plugin manages this skill, so Orca left it out of the update — update the plugin instead.' - ) - case 'external-link': - return translate( - 'auto.components.skills.SkillFreshnessRow.skippedReasonExternalLink', - 'This copy is a shortcut pointing outside Orca’s skill folders, so Orca left it out of the update.' - ) - case 'broken-link': - return translate( - 'auto.components.skills.SkillFreshnessRow.skippedReasonBrokenLink', - 'This copy is a shortcut to something that no longer exists, so Orca left it out — you can safely delete it.' - ) - // Why: 'current'/'duplicate' are non-blocking chips, and an empty priority - // list is possible; all fall through to the generic skipped message. - case 'current': - case 'duplicate': - case undefined: - return translate( - 'auto.components.skills.SkillFreshnessRow.cantUpdateReason', - 'Orca left this skill out of the update command.' - ) - } -} - export function SkillFreshnessGroup({ group }: { diff --git a/src/renderer/src/components/skills/skill-freshness-grouping.test.ts b/src/renderer/src/components/skills/skill-freshness-grouping.test.ts index 03368ec07..3a89ae1c2 100644 --- a/src/renderer/src/components/skills/skill-freshness-grouping.test.ts +++ b/src/renderer/src/components/skills/skill-freshness-grouping.test.ts @@ -73,6 +73,28 @@ describe('groupSkillFreshness', () => { ]) }) + it('blocks a skill whose only outdated copy is an unreachable duplicate', () => { + // Why: the global command reports "already up to date" here, so the group must + // read as skipped with the duplicate flagged rather than promising an update. + const groups = groupSkillFreshness( + [ + placement('orchestration', { status: 'current' }), + placement('orchestration', { + rootId: 'home-factory', + unresolvedPath: '/home/.factory/skills/orchestration', + topology: 'independent-copy' + }) + ], + [] + ) + expect(groups).toHaveLength(1) + expect(groups[0]?.status).toBe('cannot-update') + expect(groups[0]?.locations).toEqual([ + { id: expect.any(String), path: '/home/.agents/skills/orchestration', chip: 'current' }, + { id: expect.any(String), path: '/home/.factory/skills/orchestration', chip: 'duplicate' } + ]) + }) + it('prefers a location status over its topology and maps every topology to a chip', () => { const chipFor = (overrides: Partial): string | null => groupSkillFreshness( diff --git a/src/renderer/src/components/skills/skill-freshness-grouping.ts b/src/renderer/src/components/skills/skill-freshness-grouping.ts index db76800fd..b33dd1f49 100644 --- a/src/renderer/src/components/skills/skill-freshness-grouping.ts +++ b/src/renderer/src/components/skills/skill-freshness-grouping.ts @@ -25,7 +25,7 @@ export type SkillFreshnessGroupModel = { locations: SkillLocationRow[] } -function locationChip(installation: SkillFreshnessInstallation): SkillLocationChip | null { +export function locationChip(installation: SkillFreshnessInstallation): SkillLocationChip | null { // Why: a location's own status wins over its topology — "the contents don't // match" is more useful to the user than "it's a duplicate". if (installation.status === 'unrecognized') { diff --git a/src/renderer/src/components/skills/skill-freshness-skipped-reason.test.ts b/src/renderer/src/components/skills/skill-freshness-skipped-reason.test.ts new file mode 100644 index 000000000..16ef8cb08 --- /dev/null +++ b/src/renderer/src/components/skills/skill-freshness-skipped-reason.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, it } from 'vitest' +import type { SkillLocationRow } from './skill-freshness-grouping' +import { skippedReason } from './skill-freshness-skipped-reason' + +function row( + chip: SkillLocationRow['chip'], + path = `/home/.agents/skills/${chip}` +): SkillLocationRow { + return { id: `row-${chip}-${path}`, path, chip } +} + +describe('skippedReason', () => { + it('names the stale duplicate the global command cannot reach', () => { + const reason = skippedReason([row('current'), row('duplicate')]) + expect(reason).toContain('separate copy') + expect(reason).toContain('only refreshes the main copy') + }) + + it('leads with the harder blocker when several placements are off', () => { + // Why: an edited copy is the real cause; the duplicate is the lesser symptom, and + // telling the user to remove the duplicate would not unblock the update. + expect(skippedReason([row('duplicate'), row('unrecognized')])).toContain( + 'doesn’t match the official version' + ) + expect(skippedReason([row('duplicate'), row('read-only')])).toContain('read-only location') + }) + + it('falls back to the generic sentence when nothing is blocking', () => { + expect(skippedReason([row('current')])).toContain('left this skill out of the update') + expect(skippedReason([])).toContain('left this skill out of the update') + }) +}) diff --git a/src/renderer/src/components/skills/skill-freshness-skipped-reason.ts b/src/renderer/src/components/skills/skill-freshness-skipped-reason.ts new file mode 100644 index 000000000..2d693b691 --- /dev/null +++ b/src/renderer/src/components/skills/skill-freshness-skipped-reason.ts @@ -0,0 +1,85 @@ +import type { SkillLocationChip, SkillLocationRow } from './skill-freshness-grouping' +import { translate } from '@/i18n/i18n' + +// Why: a skill is skipped for one concrete reason; lead with the highest-priority +// blocking placement so the sentence explains the real cause (an edited copy is +// more useful to surface than a downstream symptom). +const SKIPPED_REASON_PRIORITY: SkillLocationChip[] = [ + 'unrecognized', + 'read-only', + 'inaccessible', + 'in-a-repo', + 'plugin-cache', + 'external-link', + 'broken-link', + // Why: lowest priority — a stale duplicate only explains the skip once no + // harder blocker is present, since the others describe a more specific cause. + 'duplicate' +] + +function blockingChip(locations: readonly SkillLocationRow[]): SkillLocationChip | undefined { + const present = new Set(locations.map((location) => location.chip)) + return SKIPPED_REASON_PRIORITY.find((candidate) => present.has(candidate)) +} + +/** + * The one sentence that explains why an update won't reach a skill. Shared by the + * review dialog and the setup rails so the badge and the dialog can never disagree. + * + * The wording is deictic ("this copy") on purpose: it is only ever rendered beside the + * location rows it describes, which is why the setup rails link into the dialog rather + * than repeating a sentence that would have nothing to point at. + */ +export function skippedReason(locations: readonly SkillLocationRow[]): string { + const chip = blockingChip(locations) + switch (chip) { + case 'unrecognized': + return translate( + 'auto.components.skills.SkillFreshnessRow.skippedReasonUnrecognized', + 'The copy here doesn’t match the official version — it may be modified, or a different skill with the same name. Orca left it out of the update so it won’t overwrite it. Remove it if you want Orca to update this skill.' + ) + case 'read-only': + return translate( + 'auto.components.skills.SkillFreshnessRow.skippedReasonReadOnly', + 'This copy is in a read-only location, so Orca left it out of the update. Change its permissions to let Orca update it.' + ) + case 'inaccessible': + return translate( + 'auto.components.skills.SkillFreshnessRow.skippedReasonInaccessible', + 'Orca couldn’t read this copy, so it left the skill out of the update.' + ) + case 'in-a-repo': + return translate( + 'auto.components.skills.SkillFreshnessRow.skippedReasonInRepo', + 'This is a project skill, not a global one — Orca only updates your global skills, so it left this out of the update.' + ) + case 'plugin-cache': + return translate( + 'auto.components.skills.SkillFreshnessRow.skippedReasonPluginCache', + 'A plugin manages this skill, so Orca left it out of the update — update the plugin instead.' + ) + case 'external-link': + return translate( + 'auto.components.skills.SkillFreshnessRow.skippedReasonExternalLink', + 'This copy is a shortcut pointing outside Orca’s skill folders, so Orca left it out of the update.' + ) + case 'broken-link': + return translate( + 'auto.components.skills.SkillFreshnessRow.skippedReasonBrokenLink', + 'This copy is a shortcut to something that no longer exists, so Orca left it out — you can safely delete it.' + ) + case 'duplicate': + return translate( + 'auto.components.skills.SkillFreshnessRow.skippedReasonDuplicate', + 'This is a separate copy, so the update won’t reach it — the command only refreshes the main copy. Remove this copy, then reinstall the skill so this location follows the main one.' + ) + // Why: 'current' is non-blocking and an empty priority list is possible; + // both fall through to the generic skipped message. + case 'current': + case undefined: + return translate( + 'auto.components.skills.SkillFreshnessRow.cantUpdateReason', + 'Orca left this skill out of the update command.' + ) + } +} diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index e32011ff6..0e13ddeab 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -3748,7 +3748,8 @@ "tipBrokenLink": "A shortcut to something that no longer exists.", "tipReadOnly": "This copy is in a read-only location.", "tipInRepo": "This copy lives inside a project, not your global skills.", - "tipPluginCache": "This copy is managed by a plugin." + "tipPluginCache": "This copy is managed by a plugin.", + "skippedReasonDuplicate": "This is a separate copy, so the update won’t reach it — the command only refreshes the main copy. Remove this copy, then reinstall the skill so this location follows the main one." }, "SkillFreshnessUpdateDialog": { "title": "Update skills", @@ -3769,7 +3770,9 @@ "SkillFreshnessStatusPill": { "updateAvailable": "Update available", "upToDate": "Up to date", - "installed": "Installed" + "installed": "Installed", + "details": "Details", + "needsAttention": "Needs attention" } }, "sidebar": { diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index f698b2ab1..a6f65f6df 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -3725,7 +3725,8 @@ "skippedReasonInRepo": "This is a project skill, not a global one — Orca only updates your global skills, so it left this out of the update.", "skippedReasonPluginCache": "A plugin manages this skill, so Orca left it out of the update — update the plugin instead.", "skippedReasonExternalLink": "This copy is a shortcut pointing outside Orca’s skill folders, so Orca left it out of the update.", - "skippedReasonBrokenLink": "This copy is a shortcut to something that no longer exists, so Orca left it out — you can safely delete it." + "skippedReasonBrokenLink": "This copy is a shortcut to something that no longer exists, so Orca left it out — you can safely delete it.", + "skippedReasonDuplicate": "This is a separate copy, so the update won’t reach it — the command only refreshes the main copy. Remove this copy, then reinstall the skill so this location follows the main one." }, "SkillFreshnessUpdateDialog": { "title": "Actualizar skills", @@ -3746,7 +3747,9 @@ "SkillFreshnessStatusPill": { "updateAvailable": "Actualización disponible", "upToDate": "Actualizado", - "installed": "Instalado" + "installed": "Instalado", + "details": "Details", + "needsAttention": "Needs attention" } }, "sidebar": { diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index 5ffc4a375..89e694716 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -3725,7 +3725,8 @@ "skippedReasonInRepo": "This is a project skill, not a global one — Orca only updates your global skills, so it left this out of the update.", "skippedReasonPluginCache": "A plugin manages this skill, so Orca left it out of the update — update the plugin instead.", "skippedReasonExternalLink": "This copy is a shortcut pointing outside Orca’s skill folders, so Orca left it out of the update.", - "skippedReasonBrokenLink": "This copy is a shortcut to something that no longer exists, so Orca left it out — you can safely delete it." + "skippedReasonBrokenLink": "This copy is a shortcut to something that no longer exists, so Orca left it out — you can safely delete it.", + "skippedReasonDuplicate": "This is a separate copy, so the update won’t reach it — the command only refreshes the main copy. Remove this copy, then reinstall the skill so this location follows the main one." }, "SkillFreshnessUpdateDialog": { "title": "スキルを更新", @@ -3746,7 +3747,9 @@ "SkillFreshnessStatusPill": { "updateAvailable": "更新があります", "upToDate": "最新です", - "installed": "インストール済み" + "installed": "インストール済み", + "details": "Details", + "needsAttention": "Needs attention" } }, "sidebar": { diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index 14d816407..16a465d66 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -3725,7 +3725,8 @@ "skippedReasonInRepo": "This is a project skill, not a global one — Orca only updates your global skills, so it left this out of the update.", "skippedReasonPluginCache": "A plugin manages this skill, so Orca left it out of the update — update the plugin instead.", "skippedReasonExternalLink": "This copy is a shortcut pointing outside Orca’s skill folders, so Orca left it out of the update.", - "skippedReasonBrokenLink": "This copy is a shortcut to something that no longer exists, so Orca left it out — you can safely delete it." + "skippedReasonBrokenLink": "This copy is a shortcut to something that no longer exists, so Orca left it out — you can safely delete it.", + "skippedReasonDuplicate": "This is a separate copy, so the update won’t reach it — the command only refreshes the main copy. Remove this copy, then reinstall the skill so this location follows the main one." }, "SkillFreshnessUpdateDialog": { "title": "스킬 업데이트", @@ -3746,7 +3747,9 @@ "SkillFreshnessStatusPill": { "updateAvailable": "업데이트 가능", "upToDate": "최신 상태", - "installed": "설치됨" + "installed": "설치됨", + "details": "Details", + "needsAttention": "Needs attention" } }, "sidebar": { diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index bca48c084..7b33271bc 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -3725,7 +3725,8 @@ "skippedReasonInRepo": "This is a project skill, not a global one — Orca only updates your global skills, so it left this out of the update.", "skippedReasonPluginCache": "A plugin manages this skill, so Orca left it out of the update — update the plugin instead.", "skippedReasonExternalLink": "This copy is a shortcut pointing outside Orca’s skill folders, so Orca left it out of the update.", - "skippedReasonBrokenLink": "This copy is a shortcut to something that no longer exists, so Orca left it out — you can safely delete it." + "skippedReasonBrokenLink": "This copy is a shortcut to something that no longer exists, so Orca left it out — you can safely delete it.", + "skippedReasonDuplicate": "This is a separate copy, so the update won’t reach it — the command only refreshes the main copy. Remove this copy, then reinstall the skill so this location follows the main one." }, "SkillFreshnessUpdateDialog": { "title": "更新技能", @@ -3746,7 +3747,9 @@ "SkillFreshnessStatusPill": { "updateAvailable": "有可用更新", "upToDate": "已是最新", - "installed": "已安装" + "installed": "已安装", + "details": "Details", + "needsAttention": "Needs attention" } }, "sidebar": { diff --git a/src/renderer/src/lib/settings-navigation-types.ts b/src/renderer/src/lib/settings-navigation-types.ts index f3409e690..7ce835e69 100644 --- a/src/renderer/src/lib/settings-navigation-types.ts +++ b/src/renderer/src/lib/settings-navigation-types.ts @@ -8,6 +8,7 @@ export type SettingsNavInstallStatus = | 'installed' | 'up-to-date' | 'update-available' + | 'needs-attention' | 'checking' export type SettingsNavTarget = diff --git a/src/renderer/src/lib/skill-freshness-display-status.test.ts b/src/renderer/src/lib/skill-freshness-display-status.test.ts index 9d244007d..15b25aa4a 100644 --- a/src/renderer/src/lib/skill-freshness-display-status.test.ts +++ b/src/renderer/src/lib/skill-freshness-display-status.test.ts @@ -56,14 +56,23 @@ describe('getSkillFreshnessDisplayStatus', () => { it.each([ ['before the inventory loads', null], - ['when the inventory has no matching placement', inventory([])], + ['when the inventory has no matching placement', inventory([])] + ])('reports presence only %s', (_scenario, value) => { + // Why: with nothing scanned there is no drift to claim, and flashing attention + // on every launch before the first scan would train the user to ignore it. + expect(getSkillFreshnessDisplayStatus(value, SKILL_NAME)).toBe('installed') + }) + + it.each([ [ 'when any placement is unrecognized', inventory([placement('current'), placement('unrecognized', 1)]) ], ['when a placement is inaccessible', inventory([placement('inaccessible')])], ['when an outdated placement is not eligible', inventory([placement('outdated')])] - ])('falls back to installed %s', (_scenario, value) => { - expect(getSkillFreshnessDisplayStatus(value, SKILL_NAME)).toBe('installed') + ])('reports needs attention %s', (_scenario, value) => { + // Why: no eligible update is not proof a copy is fine. Green here would read as + // all-clear over drift the update command cannot reach and the user cannot see. + expect(getSkillFreshnessDisplayStatus(value, SKILL_NAME)).toBe('needs-attention') }) }) diff --git a/src/renderer/src/lib/skill-freshness-display-status.ts b/src/renderer/src/lib/skill-freshness-display-status.ts index 252640226..581c281a8 100644 --- a/src/renderer/src/lib/skill-freshness-display-status.ts +++ b/src/renderer/src/lib/skill-freshness-display-status.ts @@ -1,6 +1,13 @@ -import type { SkillFreshnessInventory } from '../../../shared/skill-freshness' +import { + SUPPORTED_GLOBAL_SKILL_TOPOLOGIES, + type SkillFreshnessInventory +} from '../../../shared/skill-freshness' -export type SkillFreshnessDisplayStatus = 'installed' | 'up-to-date' | 'update-available' +export type SkillFreshnessDisplayStatus = + | 'installed' + | 'up-to-date' + | 'update-available' + | 'needs-attention' export function getSkillFreshnessDisplayStatus( inventory: SkillFreshnessInventory | null, @@ -11,15 +18,44 @@ export function getSkillFreshnessDisplayStatus( } let hasPlacement = false + let hasBlockedCopy = false for (const installation of inventory?.installations ?? []) { if (installation.name !== skillName) { continue } hasPlacement = true - // No eligible update is not proof that a blocked or unrecognized copy is current. if (installation.status !== 'current') { - return 'installed' + hasBlockedCopy = true } } - return hasPlacement ? 'up-to-date' : 'installed' + // Why: with no scan yet (or nothing found) the only honest answer is presence. + // Reporting attention here would flash amber on every launch before the first scan. + if (!hasPlacement) { + return 'installed' + } + // Why: no eligible update is not proof a copy is fine — it can equally mean a copy + // is out of date somewhere the update command cannot reach. Saying "Installed" there + // reads as all-clear and hides real drift, so that case gets its own attention state. + return hasBlockedCopy ? 'needs-attention' : 'up-to-date' +} + +/** + * Whether a copy needs the user's own hands — it is not current, and running the update + * would not resolve it. This is what marks the review affordance as carrying a problem + * rather than a routine update, so the badge can stay a badge and the dialog explains. + */ +export function hasSkillCopyNeedingAttention( + inventory: SkillFreshnessInventory | null, + skillName: string +): boolean { + return (inventory?.installations ?? []).some( + (installation) => + installation.name === skillName && + installation.status !== 'current' && + // Why: an out-of-date copy the command converges is ordinary work, not a problem. + !( + SUPPORTED_GLOBAL_SKILL_TOPOLOGIES.has(installation.topology) && + installation.status === 'outdated' + ) + ) }