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

fix(skills): stop promising a skill update the command cannot deliver (#10436) 

* fix(skills): stop promising a skill update the command cannot deliver

An "Update available" badge could never clear: pressing Update ran
`npx skills update <name> --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.
This commit is contained in:
Brennan Benson 2026-07-24 15:53:22 -07:00 committed by GitHub
parent b600e25fa1
commit e133d93b7b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
22 changed files with 447 additions and 146 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

67
src/main/skills/skill-freshness-eligibility.test.ts
View File

@ -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', () => {

45
src/main/skills/skill-freshness-eligibility.ts
View File

@ -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'))

22
src/main/skills/skill-freshness-inventory.test.ts
View File

@ -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'])
})
})

6
src/renderer/src/components/feature-wall/BrowserUseSkillSetupCard.tsx
View File

@ -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
}
/>
)

20
src/renderer/src/components/floating-terminal/FloatingTerminalOrchestrationDialog.tsx
View File

@ -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({
)}
</IntegrationStatusPill>
) : orchestrationSkillDetected ? (
<IntegrationStatusPill tone="connected">
{translate(
'auto.components.floating.terminal.FloatingTerminalOrchestrationDialog.630c0ac8c8',
'Installed'
)}
</IntegrationStatusPill>
// 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' ? (
<IntegrationStatusPill tone="connected">
{translate(
'auto.components.floating.terminal.FloatingTerminalOrchestrationDialog.630c0ac8c8',
'Installed'
)}
</IntegrationStatusPill>
) : (
<SkillFreshnessStatusPill skillName={ORCHESTRATION_SKILL_NAME} />
)
) : (
<IntegrationStatusPill tone="attention">
{translate(

4
src/renderer/src/components/settings/MobileEmulatorAgentControlRow.tsx
View File

@ -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}
/>
</div>

7
src/renderer/src/components/settings/SettingsSidebar.tsx
View File

@ -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'

42
src/renderer/src/components/skills/SkillFreshnessStatusPill.test.tsx
View File

@ -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<HTMLDivElement> {
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)
})
})

71
src/renderer/src/components/skills/SkillFreshnessStatusPill.tsx
View File

@ -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 (
<IntegrationStatusPill tone="attention">
@ -20,6 +22,16 @@ export function SkillFreshnessStatusPill({ skillName }: { skillName: string }):
</IntegrationStatusPill>
)
}
if (status === 'needs-attention') {
return (
<IntegrationStatusPill tone="attention">
{translate(
'auto.components.skills.SkillFreshnessStatusPill.needsAttention',
'Needs attention'
)}
</IntegrationStatusPill>
)
}
if (status === 'up-to-date') {
return (
<IntegrationStatusPill tone="connected">
@ -33,3 +45,46 @@ export function SkillFreshnessStatusPill({ skillName }: { skillName: string }):
</IntegrationStatusPill>
)
}
// 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 (
<span className="inline-flex items-center gap-2">
{statusPill(status)}
{hasDetails ? (
<Button
// Why: ghost, not link — this sits in a header row rather than inside a
// paragraph, and its hover/focus background is what makes it read as a
// control. No chevron: it opens the review dialog, and the chevron already
// means an in-place expander on that dialog's own details section.
variant="ghost"
size="xs"
className={cn(
'gap-1 px-1.5 text-[11px]',
needsAttention && 'text-amber-500 hover:text-amber-500'
)}
onClick={() => requestSkillFreshnessUpdateDialog()}
>
{needsAttention ? <AlertTriangle className="size-3" /> : null}
{translate('auto.components.skills.SkillFreshnessStatusPill.details', 'Details')}
{/* Why: a chevron is what makes this read as a control at rest rather than a
label. It points right, not down — this opens the review dialog, and a
down chevron already means an in-place expander inside that dialog. */}
<ChevronRight className="size-3" />
</Button>
) : null}
</span>
)
}

71
src/renderer/src/components/skills/skill-freshness-group.tsx
View File

@ -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
}: {

22
src/renderer/src/components/skills/skill-freshness-grouping.test.ts
View File

@ -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<SkillFreshnessInstallation>): string | null =>
groupSkillFreshness(

2
src/renderer/src/components/skills/skill-freshness-grouping.ts
View File

@ -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') {

32
src/renderer/src/components/skills/skill-freshness-skipped-reason.test.ts Normal file
View File

@ -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')
})
})

85
src/renderer/src/components/skills/skill-freshness-skipped-reason.ts Normal file
View File

@ -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.'
)
}
}

7
src/renderer/src/i18n/locales/en.json
View File

@ -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": {

7
src/renderer/src/i18n/locales/es.json
View File

@ -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": {

7
src/renderer/src/i18n/locales/ja.json
View File

@ -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": {

7
src/renderer/src/i18n/locales/ko.json
View File

@ -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": {

7
src/renderer/src/i18n/locales/zh.json
View File

@ -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": {

1
src/renderer/src/lib/settings-navigation-types.ts
View File

@ -8,6 +8,7 @@ export type SettingsNavInstallStatus =
| 'installed'
| 'up-to-date'
| 'update-available'
| 'needs-attention'
| 'checking'
export type SettingsNavTarget =

15
src/renderer/src/lib/skill-freshness-display-status.test.ts
View File

@ -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')
})
})

46
src/renderer/src/lib/skill-freshness-display-status.ts
View File

@ -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'
)
)
}