From 1775d83cf634d276774a37da65a565412612ee04 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 28 Jul 2026 01:00:10 -0700 Subject: [PATCH] fix(skills): tell the user how to fix a skill the updater cannot converge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A skill the update command provably cannot move showed one sentence — "Orca left this skill out of the update command." — which names no cause and no remedy. It is the message for the only state #11110 exists to create, and it is a dead end. No location is at fault in that state: the copy is an ordinary out-of-date one the update would happily write. What is wrong is the updater's own record of it. Since `skills update` compares that record against the source and never reads disk, a record that is missing, or that already names the version it was meant to fetch, makes the command report "up to date" and write nothing. Only a reinstall rewrites it, so the sentence now hands over that command for the skill in question. A copy that is *ahead* of this build reached the same bare state, where the new advice would quietly roll it back. It gets its own chip and sentence instead. --- .../src/components/skills/SkillUpdateRow.tsx | 2 +- .../skills/skill-freshness-grouping.test.ts | 3 ++ .../skills/skill-freshness-grouping.ts | 12 +++++-- .../skill-freshness-skipped-reason.test.ts | 26 +++++++++++++- .../skills/skill-freshness-skipped-reason.ts | 35 ++++++++++++++++--- .../skills/skill-location-chip-copy.ts | 7 ++++ src/renderer/src/i18n/locales/en.json | 6 +++- src/renderer/src/i18n/locales/es.json | 6 +++- src/renderer/src/i18n/locales/ja.json | 6 +++- src/renderer/src/i18n/locales/ko.json | 6 +++- src/renderer/src/i18n/locales/zh.json | 6 +++- 11 files changed, 100 insertions(+), 15 deletions(-) diff --git a/src/renderer/src/components/skills/SkillUpdateRow.tsx b/src/renderer/src/components/skills/SkillUpdateRow.tsx index 3f069be5b54..d6d95f34783 100644 --- a/src/renderer/src/components/skills/SkillUpdateRow.tsx +++ b/src/renderer/src/components/skills/SkillUpdateRow.tsx @@ -118,7 +118,7 @@ export function SkillUpdateRow({ nor on a mount-time `defaultOpen` that a re-scan can't re-fire. */} {state === 'blocked' ? (

- {skippedReason(group.locations)} + {skippedReason(group.locations, group.name)}

) : null} 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 c4913f678da..39bb58cc4cb 100644 --- a/src/renderer/src/components/skills/skill-freshness-grouping.test.ts +++ b/src/renderer/src/components/skills/skill-freshness-grouping.test.ts @@ -164,6 +164,9 @@ describe('groupSkillFreshness', () => { expect(chipFor(at('g', { topology: 'repo-scope' }))).toBe('in-a-repo') expect(chipFor(at('h', { topology: 'plugin-cache' }))).toBe('plugin-cache') expect(chipFor(at('i', { status: 'current', topology: 'provider-alias' }))).toBe('current') + // Why: a bare chip is what the reason copy reads as "behind, and a reinstall fixes + // it". A copy that is ahead must be told apart, or that advice rolls it back. + expect(chipFor(at('k', { status: 'newer-known', topology: 'canonical-copy' }))).toBe('newer') expect(chipFor(at('j', { status: 'unrecognized', topology: 'plugin-cache' }))).toBe( 'plugin-cache' ) diff --git a/src/renderer/src/components/skills/skill-freshness-grouping.ts b/src/renderer/src/components/skills/skill-freshness-grouping.ts index 2a306b44c72..79d39b31aff 100644 --- a/src/renderer/src/components/skills/skill-freshness-grouping.ts +++ b/src/renderer/src/components/skills/skill-freshness-grouping.ts @@ -7,6 +7,7 @@ export type SkillGroupStatus = 'update-available' | 'cannot-update' export type SkillLocationChip = | 'current' + | 'newer' | 'unrecognized' | 'inaccessible' | 'duplicate' @@ -53,9 +54,14 @@ export function locationChip(installation: SkillFreshnessInstallation): SkillLoc return 'plugin-cache' case 'canonical-copy': case 'provider-alias': - // Why: a supported location only needs a chip when it's already up to date, - // to explain why the update won't touch it; the out-of-date main copy is bare. - return installation.status === 'current' ? 'current' : null + // Why: a supported location only needs a chip when the update won't touch it — + // already current, or ahead of what this build knows. The out-of-date main copy + // is bare, and 'newer' is what keeps a bare chip meaning "behind, and fixable": + // reinstalling a copy that is ahead would quietly roll the user back. + if (installation.status === 'current') { + return 'current' + } + return installation.status === 'newer-known' ? 'newer' : null } } 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 index 16ef8cb0876..664a4883f84 100644 --- a/src/renderer/src/components/skills/skill-freshness-skipped-reason.test.ts +++ b/src/renderer/src/components/skills/skill-freshness-skipped-reason.test.ts @@ -25,7 +25,31 @@ describe('skippedReason', () => { expect(skippedReason([row('duplicate'), row('read-only')])).toContain('read-only location') }) - it('falls back to the generic sentence when nothing is blocking', () => { + it('hands over the reinstall command when no location is at fault', () => { + // Why: the only way to reach this with a bare out-of-date copy is the updater's own + // record, which `skills update` can never converge — so the sentence has to give the + // one command that does, not report a skip the user cannot act on. + const reason = skippedReason([row(null)], 'orchestration') + expect(reason).toContain('reports the skill as already up to date') + expect(reason).toContain( + 'npx skills add https://github.com/stablyai/orca --skill orchestration --global' + ) + }) + + it('never offers the reinstall for a copy that is ahead of this build', () => { + // Why: reinstalling a newer copy rolls the user back to what this build ships. + const reason = skippedReason([row('newer')], 'orchestration') + expect(reason).toContain('later version') + expect(reason).not.toContain('skills add') + }) + + it('keeps a placement blocker ahead of the record advice', () => { + expect(skippedReason([row(null), row('unrecognized')], 'orchestration')).toContain( + 'doesn’t match the official version' + ) + }) + + it('falls back to the generic sentence when no skill name is available', () => { 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 index 05af44e1a63..0e9c01c01f1 100644 --- a/src/renderer/src/components/skills/skill-freshness-skipped-reason.ts +++ b/src/renderer/src/components/skills/skill-freshness-skipped-reason.ts @@ -1,3 +1,4 @@ +import { buildAgentFeatureSkillInstallCommand } from '../../../../shared/agent-feature-install-commands' import type { SkillLocationChip, SkillLocationRow } from './skill-freshness-grouping' import { translate } from '@/i18n/i18n' @@ -8,6 +9,9 @@ const SKIPPED_REASON_PRIORITY: SkillLocationChip[] = [ 'unrecognized', 'read-only', 'inaccessible', + // Why: above the placement chips — a copy that is ahead of this build must never + // fall through to the reinstall advice below, which would roll it back. + 'newer', 'in-a-repo', 'plugin-cache', 'external-link', @@ -29,10 +33,18 @@ function blockingChip(locations: readonly SkillLocationRow[]): SkillLocationChip * 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. + * + * `skillName` is only used for the no-chip case, where the fault is not a placement at + * all but the updater's own record of this skill, and the remedy has to name it. */ -export function skippedReason(locations: readonly SkillLocationRow[]): string { +export function skippedReason(locations: readonly SkillLocationRow[], skillName?: string): string { const chip = blockingChip(locations) switch (chip) { + case 'newer': + return translate( + 'auto.components.skills.SkillFreshnessRow.skippedReasonNewer', + 'This copy is a later version than the one this build of Orca ships, so Orca left it alone rather than roll it back. Updating Orca will bring the two back in line.' + ) case 'unrecognized': return translate( 'auto.components.skills.SkillFreshnessRow.skippedReasonUnrecognized', @@ -75,9 +87,22 @@ export function skippedReason(locations: readonly SkillLocationRow[]): string { ) case 'current': case undefined: - return translate( - 'auto.components.skills.SkillFreshnessRow.cantUpdateReason', - 'Orca left this skill out of the update command.' - ) + // Why: no location is at fault here — the copy is an ordinary out-of-date one the + // update would happily write. What is wrong is the updater's own record of it: + // `skills update` decides what to do by comparing that record against the source + // and never reads disk, so when the record is missing or already names the version + // it was meant to fetch, the command reports "up to date" and writes nothing. No + // retry converges it; only a reinstall rewrites the record, which is why the + // sentence has to hand over the command rather than say the update was skipped. + return skillName + ? translate( + 'auto.components.skills.SkillFreshnessRow.skippedReasonStaleRecord', + 'The skills updater has no usable record of this copy, so it reports the skill as already up to date and changes nothing. Reinstall it to bring the record back in line: {{value0}}', + { value0: buildAgentFeatureSkillInstallCommand([skillName]) } + ) + : translate( + 'auto.components.skills.SkillFreshnessRow.cantUpdateReason', + 'Orca left this skill out of the update command.' + ) } } diff --git a/src/renderer/src/components/skills/skill-location-chip-copy.ts b/src/renderer/src/components/skills/skill-location-chip-copy.ts index 82e709665cc..fdee4dce2a8 100644 --- a/src/renderer/src/components/skills/skill-location-chip-copy.ts +++ b/src/renderer/src/components/skills/skill-location-chip-copy.ts @@ -5,6 +5,8 @@ export function chipLabel(chip: SkillLocationChip): string { switch (chip) { case 'current': return translate('auto.components.skills.SkillFreshnessRow.chipCurrent', 'Current') + case 'newer': + return translate('auto.components.skills.SkillFreshnessRow.chipNewer', 'Newer') case 'unrecognized': return translate('auto.components.skills.SkillFreshnessRow.chipUnrecognized', 'Unrecognized') case 'inaccessible': @@ -33,6 +35,11 @@ export function chipTooltip(chip: SkillLocationChip): string { 'auto.components.skills.SkillFreshnessRow.tipCurrent', 'This copy matches the current official version.' ) + case 'newer': + return translate( + 'auto.components.skills.SkillFreshnessRow.tipNewer', + 'This copy is a later version than the one this build of Orca ships.' + ) case 'unrecognized': return translate( 'auto.components.skills.SkillFreshnessRow.tipUnrecognized', diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 252253011c2..ede4b7fb99c 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -3820,7 +3820,11 @@ "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.", - "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." + "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.", + "skippedReasonNewer": "This copy is a later version than the one this build of Orca ships, so Orca left it alone rather than roll it back. Updating Orca will bring the two back in line.", + "skippedReasonStaleRecord": "The skills updater has no usable record of this copy, so it reports the skill as already up to date and changes nothing. Reinstall it to bring the record back in line: {{value0}}", + "chipNewer": "Newer", + "tipNewer": "This copy is a later version than the one this build of Orca ships." }, "SkillFreshnessUpdateDialog": { "title": "Update skills", diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index 302f24f7bb1..166318ba1c6 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -3797,7 +3797,11 @@ "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.", - "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." + "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.", + "skippedReasonNewer": "This copy is a later version than the one this build of Orca ships, so Orca left it alone rather than roll it back. Updating Orca will bring the two back in line.", + "skippedReasonStaleRecord": "The skills updater has no usable record of this copy, so it reports the skill as already up to date and changes nothing. Reinstall it to bring the record back in line: {{value0}}", + "chipNewer": "Newer", + "tipNewer": "This copy is a later version than the one this build of Orca ships." }, "SkillFreshnessUpdateDialog": { "title": "Actualizar skills", diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index a3b5555a865..8302a306811 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -3797,7 +3797,11 @@ "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.", - "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." + "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.", + "skippedReasonNewer": "This copy is a later version than the one this build of Orca ships, so Orca left it alone rather than roll it back. Updating Orca will bring the two back in line.", + "skippedReasonStaleRecord": "The skills updater has no usable record of this copy, so it reports the skill as already up to date and changes nothing. Reinstall it to bring the record back in line: {{value0}}", + "chipNewer": "Newer", + "tipNewer": "This copy is a later version than the one this build of Orca ships." }, "SkillFreshnessUpdateDialog": { "title": "スキルを更新", diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index 91878a71e44..5124dcbdb65 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -3797,7 +3797,11 @@ "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.", - "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." + "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.", + "skippedReasonNewer": "This copy is a later version than the one this build of Orca ships, so Orca left it alone rather than roll it back. Updating Orca will bring the two back in line.", + "skippedReasonStaleRecord": "The skills updater has no usable record of this copy, so it reports the skill as already up to date and changes nothing. Reinstall it to bring the record back in line: {{value0}}", + "chipNewer": "Newer", + "tipNewer": "This copy is a later version than the one this build of Orca ships." }, "SkillFreshnessUpdateDialog": { "title": "스킬 업데이트", diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index 70c2933abff..d7139b19eba 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -3797,7 +3797,11 @@ "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.", - "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." + "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.", + "skippedReasonNewer": "This copy is a later version than the one this build of Orca ships, so Orca left it alone rather than roll it back. Updating Orca will bring the two back in line.", + "skippedReasonStaleRecord": "The skills updater has no usable record of this copy, so it reports the skill as already up to date and changes nothing. Reinstall it to bring the record back in line: {{value0}}", + "chipNewer": "Newer", + "tipNewer": "This copy is a later version than the one this build of Orca ships." }, "SkillFreshnessUpdateDialog": { "title": "更新技能",