From f89a0b25cf63daebe97c96082f1a8e47f0ccb09e Mon Sep 17 00:00:00 2001 From: brennanb2025 Date: Mon, 22 Jun 2026 16:06:30 -0700 Subject: [PATCH] Address managed skill PR feedback Co-authored-by: Orca --- src/main/index.ts | 3 +- .../managed-skill-discovery-candidate.ts | 2 ++ .../skills/managed-skill-global-candidate.ts | 8 ++++- src/main/skills/managed-skill-lockfile.ts | 2 ++ ...ntSkillSetupPrompt.update-command.test.tsx | 10 ++++-- .../ManagedAgentSkillSetupDialogHost.tsx | 36 ++++++++++++++++--- .../hooks/installed-agent-skill-discovery.ts | 2 +- src/renderer/src/web/web-preload-api.ts | 1 + src/shared/agent-feature-install-commands.ts | 2 ++ src/shared/constants.ts | 1 + 10 files changed, 56 insertions(+), 11 deletions(-) diff --git a/src/main/index.ts b/src/main/index.ts index 4587fa29ac0..3a7b81a9d8d 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -9,6 +9,7 @@ import { app, BrowserWindow, ipcMain, nativeTheme } from 'electron' import { electronApp, is } from '@electron-toolkit/utils' import * as QRCode from 'qrcode' import { Store, initDataPath } from './persistence' +import { shouldEmitManagedAgentSkillFallback } from '../shared/skills' import { applyAppIcon } from './app-icon' import { StatsCollector, initStatsPath } from './stats/collector' import { ClaudeUsageStore, initClaudeUsagePath } from './claude-usage/store' @@ -1643,7 +1644,7 @@ app.whenReady().then(async () => { ...(remoteRuntime ? { remoteRuntime } : {}), ...(!remoteRuntime && discoveryTarget ? { discoveryTarget } : {}) }) - if (result.status === 'fallback') { + if (result.status === 'fallback' && shouldEmitManagedAgentSkillFallback(result)) { sendManagedSkillFallback(result) } else if (result.status === 'updated') { sendManagedSkillUpdated(result) diff --git a/src/main/skills/managed-skill-discovery-candidate.ts b/src/main/skills/managed-skill-discovery-candidate.ts index f8e3e2448eb..113eb55b7d8 100644 --- a/src/main/skills/managed-skill-discovery-candidate.ts +++ b/src/main/skills/managed-skill-discovery-candidate.ts @@ -5,6 +5,8 @@ export function isDiscoveredManagedSkill( skill: DiscoveredSkill, skillName: ManagedAgentSkillName ): boolean { + // Why: frontmatter names can be display labels, while the folder basename is + // the install identity used by the skills CLI. const expected = normalizeSkillName(skillName) return ( normalizeSkillName(skill.name) === expected || diff --git a/src/main/skills/managed-skill-global-candidate.ts b/src/main/skills/managed-skill-global-candidate.ts index 3b25ea23c6f..bce66084514 100644 --- a/src/main/skills/managed-skill-global-candidate.ts +++ b/src/main/skills/managed-skill-global-candidate.ts @@ -68,7 +68,7 @@ export function selectSingleGlobalManagedSkillCandidate(args: { fallback: { reason: 'project-install', scope: 'project' } } } - if (bundledOrPluginCandidates.length > 0 || homeCandidates.length !== 1) { + if (bundledOrPluginCandidates.length > 0) { return { status: 'fallback', fallback: { @@ -82,6 +82,12 @@ export function selectSingleGlobalManagedSkillCandidate(args: { } } } + if (homeCandidates.length !== 1) { + return { + status: 'fallback', + fallback: { reason: 'ambiguous-install', scope: 'global' } + } + } const candidate = homeCandidates[0] if (candidate.directoryIsSymlink || candidate.skillFileIsSymlink) { diff --git a/src/main/skills/managed-skill-lockfile.ts b/src/main/skills/managed-skill-lockfile.ts index 7d83ea7e508..31213212045 100644 --- a/src/main/skills/managed-skill-lockfile.ts +++ b/src/main/skills/managed-skill-lockfile.ts @@ -10,6 +10,7 @@ type ManagedSkillLockEntry = { skillFolderHash?: unknown } +// Why: managed-skill readiness validation currently trusts lockfile schema v3 only. const SUPPORTED_LOCKFILE_SCHEMA_VERSION = 3 export async function readManagedSkillLockEntry(args: { @@ -43,6 +44,7 @@ export async function readManagedSkillLockEntry(args: { if (!entry) { return { ok: false, reason: 'lock-entry-missing' } } + // Why: only trust lock entries tied to Orca-managed source and canonical skill path. if (!isManagedLockEntryForSkill(entry, args.skillName)) { return { ok: false, reason: 'lock-entry-unmanaged-source' } } diff --git a/src/renderer/src/components/sidebar/LinearAgentSkillSetupPrompt.update-command.test.tsx b/src/renderer/src/components/sidebar/LinearAgentSkillSetupPrompt.update-command.test.tsx index 7ff473b49da..63d720a3870 100644 --- a/src/renderer/src/components/sidebar/LinearAgentSkillSetupPrompt.update-command.test.tsx +++ b/src/renderer/src/components/sidebar/LinearAgentSkillSetupPrompt.update-command.test.tsx @@ -6,6 +6,10 @@ import { createRoot, type Root } from 'react-dom/client' import type { CliInstallStatus } from '../../../../shared/cli-install-types' import type { DiscoveredSkill } from '../../../../shared/skills' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + LINEAR_TICKETS_SKILL_UPDATE_COMMAND, + ORCA_LINEAR_SKILL_UPDATE_COMMAND +} from '@/lib/agent-feature-install-commands' import { LinearAgentSkillSetupPrompt, _linearAgentSkillSetupPromptInternalsForTests @@ -156,7 +160,7 @@ describe('LinearAgentSkillSetupPrompt update command', () => { await renderPrompt() expect(mocks.panelProps.at(-1)).toEqual( - expect.objectContaining({ installedCommand: 'npx skills update orca-linear --global' }) + expect.objectContaining({ installedCommand: ORCA_LINEAR_SKILL_UPDATE_COMMAND }) ) }) @@ -166,7 +170,7 @@ describe('LinearAgentSkillSetupPrompt update command', () => { await renderPrompt() expect(mocks.panelProps.at(-1)).toEqual( - expect.objectContaining({ installedCommand: 'npx skills update linear-tickets --global' }) + expect.objectContaining({ installedCommand: LINEAR_TICKETS_SKILL_UPDATE_COMMAND }) ) }) @@ -176,7 +180,7 @@ describe('LinearAgentSkillSetupPrompt update command', () => { await renderPrompt() expect(mocks.panelProps.at(-1)).toEqual( - expect.objectContaining({ installedCommand: 'npx skills update orca-linear --global' }) + expect.objectContaining({ installedCommand: ORCA_LINEAR_SKILL_UPDATE_COMMAND }) ) }) }) diff --git a/src/renderer/src/components/skills/ManagedAgentSkillSetupDialogHost.tsx b/src/renderer/src/components/skills/ManagedAgentSkillSetupDialogHost.tsx index 34beef335ed..b23c3a065f3 100644 --- a/src/renderer/src/components/skills/ManagedAgentSkillSetupDialogHost.tsx +++ b/src/renderer/src/components/skills/ManagedAgentSkillSetupDialogHost.tsx @@ -91,22 +91,42 @@ export function ManagedAgentSkillSetupDialogHost(): React.JSX.Element | null { ) const recheck = useCallback(async (): Promise => { - if (!active) { + if (!active || rechecking) { return } + const recheckUiKey = active.uiKey setRechecking(true) try { const result = await window.api.skills.ensureManagedReady({ ...active.request, force: true }) if (result.status === 'fallback') { - setDialogState((current) => replaceActiveAfterManagedAgentSkillRecheck(current, result)) + setDialogState((current) => { + if (current.active?.uiKey !== recheckUiKey) { + return current + } + const previousKey = current.active.uiKey + const next = replaceActiveAfterManagedAgentSkillRecheck(current, result) + if (next.active?.uiKey !== previousKey) { + queuedKeysRef.current.delete(previousKey) + if (next.active) { + queuedKeysRef.current.add(next.active.uiKey) + } + } + return next + }) return } + setDialogState((current) => { + if (current.active?.uiKey !== recheckUiKey) { + return current + } + queuedKeysRef.current.delete(recheckUiKey) + return advanceManagedAgentSkillFallbackQueue(current) + }) notifyInstalledAgentSkillsChanged() - advanceQueue() } finally { setRechecking(false) } - }, [active, advanceQueue]) + }, [active, rechecking]) if (!active) { return null @@ -188,7 +208,13 @@ export function ManagedAgentSkillSetupDialogHost(): React.JSX.Element | null { /> ) : (
-