From 79050cc318462414d65a050449c863223ee06dfa Mon Sep 17 00:00:00 2001 From: 777genius Date: Fri, 17 Apr 2026 21:32:04 +0300 Subject: [PATCH] fix(extensions): tone down script-only skill advisories --- .../extensions/skills/SkillMetadataParser.ts | 2 +- .../extensions/skills/SkillDetailDialog.tsx | 48 ++- .../extensions/skills/SkillsPanel.tsx | 303 ++++++++++-------- src/shared/types/extensions/skill.ts | 2 +- .../extensions/SkillMetadataParser.test.ts | 6 + .../skills/SkillDetailDialog.test.ts | 48 +++ 6 files changed, 272 insertions(+), 137 deletions(-) diff --git a/src/main/services/extensions/skills/SkillMetadataParser.ts b/src/main/services/extensions/skills/SkillMetadataParser.ts index 9f93b607..cef82046 100644 --- a/src/main/services/extensions/skills/SkillMetadataParser.ts +++ b/src/main/services/extensions/skills/SkillMetadataParser.ts @@ -117,7 +117,7 @@ export class SkillMetadataParser { code: 'has-scripts', message: 'This skill includes a scripts directory. Review bundled scripts before trusting it.', - severity: 'warning', + severity: 'info', }); } diff --git a/src/renderer/components/extensions/skills/SkillDetailDialog.tsx b/src/renderer/components/extensions/skills/SkillDetailDialog.tsx index 6a0d1d2d..0db53776 100644 --- a/src/renderer/components/extensions/skills/SkillDetailDialog.tsx +++ b/src/renderer/components/extensions/skills/SkillDetailDialog.tsx @@ -24,7 +24,7 @@ import { } from '@renderer/components/ui/dialog'; import { useStore } from '@renderer/store'; import { formatSkillRootKind, getSkillAudienceLabel } from '@shared/utils/skillRoots'; -import { AlertTriangle, ExternalLink, FolderOpen, Pencil, Trash2 } from 'lucide-react'; +import { AlertTriangle, ExternalLink, FolderOpen, Info, Pencil, Trash2 } from 'lucide-react'; import { useShallow } from 'zustand/react/shallow'; import { resolveSkillProjectPath } from './skillProjectUtils'; @@ -81,6 +81,7 @@ export const SkillDetailDialog = ({ const effectiveProjectPath = item ? resolveSkillProjectPath(item.scope, projectPath, item.projectRoot) : (projectPath ?? undefined); + const issuesTone = item?.issues.length ? getIssuesTone(item.issues) : null; function formatScopeLabel(scope: 'user' | 'project'): string { return scope === 'project' ? 'This project only' : 'Your personal skills'; @@ -92,6 +93,27 @@ export const SkillDetailDialog = ({ : 'Runs automatically when it matches the task.'; } + function getIssuesTone(issues: typeof item.issues): { + className: string; + title: string; + Icon: typeof AlertTriangle; + } { + const informationalOnly = issues.every((issue) => issue.severity === 'info'); + if (informationalOnly) { + return { + className: 'border-blue-500/30 bg-blue-500/5', + title: 'This skill includes bundled scripts', + Icon: Info, + }; + } + + return { + className: 'border-amber-500/30 bg-amber-500/5', + title: 'Review this skill carefully before using it', + Icon: AlertTriangle, + }; + } + async function handleDelete(): Promise { if (!item) return; setDeleteLoading(true); @@ -167,16 +189,30 @@ export const SkillDetailDialog = ({ {item.issues.length > 0 && ( -
-

- Review this skill carefully before using it +

+

+ {issuesTone?.title}

{item.issues.map((issue, index) => (
- + {issue.severity === 'info' ? ( + + ) : ( + + )} {issue.message}
))} diff --git a/src/renderer/components/extensions/skills/SkillsPanel.tsx b/src/renderer/components/extensions/skills/SkillsPanel.tsx index 2ecd9078..149bc98a 100644 --- a/src/renderer/components/extensions/skills/SkillsPanel.tsx +++ b/src/renderer/components/extensions/skills/SkillsPanel.tsx @@ -26,6 +26,7 @@ import { CheckCircle2, Clock3, Download, + Info, Plus, Search, } from 'lucide-react'; @@ -39,7 +40,7 @@ import { SkillImportDialog } from './SkillImportDialog'; import { resolveSkillProjectPath } from './skillProjectUtils'; import type { SkillsSortState } from '@renderer/hooks/useExtensionsTabState'; -import type { SkillCatalogItem, SkillDetail } from '@shared/types/extensions'; +import type { SkillCatalogItem, SkillDetail, SkillValidationIssue } from '@shared/types/extensions'; const SUCCESS_BANNER_MS = 2500; const NEW_SKILL_HIGHLIGHT_MS = 4000; @@ -95,6 +96,32 @@ function getSkillStatus(skill: SkillCatalogItem): string { return 'Ready to use'; } +function getPrimarySkillIssue(skill: SkillCatalogItem): SkillValidationIssue | null { + return ( + skill.issues.find((issue) => issue.severity === 'error') ?? + skill.issues.find((issue) => issue.severity === 'warning') ?? + skill.issues[0] ?? + null + ); +} + +function getSkillIssueTone(issue: SkillValidationIssue | null): { + className: string; + Icon: typeof AlertTriangle; +} { + if (issue?.severity === 'info') { + return { + className: 'border-blue-500/20 bg-blue-500/10 text-blue-700 dark:text-blue-300', + Icon: Info, + }; + } + + return { + className: 'border-amber-500/20 bg-amber-500/5 text-amber-700 dark:text-amber-300', + Icon: AlertTriangle, + }; +} + function formatRuntimeAudienceLabel(providerNames: readonly string[]): string { if (providerNames.length === 0) { return 'the configured runtime'; @@ -487,74 +514,83 @@ export const SkillsPanel = ({
- {visibleProjectSkills.map((skill) => ( -
-
-

{getInvocationLabel(skill)}

-

{getSkillStatus(skill)}

-
- -
- - Stored in {formatSkillRootKind(skill.rootKind)} - - - {getSkillAudienceLabel(skill.rootKind)} - - {skill.flags.hasScripts && ( - - Has scripts - - )} - {skill.flags.hasReferences && ( - - References - - )} - {skill.flags.hasAssets && ( - - Assets - - )} -
- - {skill.issues.length > 0 && ( -
- - {skill.issues[0]?.message} +
+

{getInvocationLabel(skill)}

+

{getSkillStatus(skill)}

- )} - - ))} + +
+ + Stored in {formatSkillRootKind(skill.rootKind)} + + + {getSkillAudienceLabel(skill.rootKind)} + + {skill.flags.hasScripts && ( + + Has scripts + + )} + {skill.flags.hasReferences && ( + + References + + )} + {skill.flags.hasAssets && ( + + Assets + + )} +
+ + {primaryIssue && ( +
+ + {primaryIssue.message} +
+ )} + + ); + })}
)} @@ -573,74 +609,83 @@ export const SkillsPanel = ({
- {visibleUserSkills.map((skill) => ( -
-
-

{getInvocationLabel(skill)}

-

{getSkillStatus(skill)}

-
- -
- - Stored in {formatSkillRootKind(skill.rootKind)} - - - {getSkillAudienceLabel(skill.rootKind)} - - {skill.flags.hasScripts && ( - - Has scripts - - )} - {skill.flags.hasReferences && ( - - References - - )} - {skill.flags.hasAssets && ( - - Assets - - )} -
- - {skill.issues.length > 0 && ( -
- - {skill.issues[0]?.message} +
+

{getInvocationLabel(skill)}

+

{getSkillStatus(skill)}

- )} - - ))} + +
+ + Stored in {formatSkillRootKind(skill.rootKind)} + + + {getSkillAudienceLabel(skill.rootKind)} + + {skill.flags.hasScripts && ( + + Has scripts + + )} + {skill.flags.hasReferences && ( + + References + + )} + {skill.flags.hasAssets && ( + + Assets + + )} +
+ + {primaryIssue && ( +
+ + {primaryIssue.message} +
+ )} + + ); + })}
)} diff --git a/src/shared/types/extensions/skill.ts b/src/shared/types/extensions/skill.ts index 9aca8729..d8d038aa 100644 --- a/src/shared/types/extensions/skill.ts +++ b/src/shared/types/extensions/skill.ts @@ -10,7 +10,7 @@ export type SkillSourceType = 'filesystem'; export type SkillInvocationMode = 'auto' | 'manual-only'; -export type SkillIssueSeverity = 'warning' | 'error'; +export type SkillIssueSeverity = 'info' | 'warning' | 'error'; export interface SkillDirectoryFlags { hasScripts: boolean; diff --git a/test/main/services/extensions/SkillMetadataParser.test.ts b/test/main/services/extensions/SkillMetadataParser.test.ts index 013c01ba..53daeebd 100644 --- a/test/main/services/extensions/SkillMetadataParser.test.ts +++ b/test/main/services/extensions/SkillMetadataParser.test.ts @@ -48,6 +48,12 @@ unknown-key: true message: expect.stringContaining('version'), }) ); + expect(item.issues).toContainEqual( + expect.objectContaining({ + code: 'has-scripts', + severity: 'info', + }) + ); }); it('marks missing frontmatter as invalid', () => { diff --git a/test/renderer/components/extensions/skills/SkillDetailDialog.test.ts b/test/renderer/components/extensions/skills/SkillDetailDialog.test.ts index ebc3f2de..986b6a94 100644 --- a/test/renderer/components/extensions/skills/SkillDetailDialog.test.ts +++ b/test/renderer/components/extensions/skills/SkillDetailDialog.test.ts @@ -113,6 +113,7 @@ vi.mock('lucide-react', () => { AlertTriangle: Icon, ExternalLink: Icon, FolderOpen: Icon, + Info: Icon, Pencil: Icon, Trash2: Icon, }; @@ -277,4 +278,51 @@ describe('SkillDetailDialog', () => { await Promise.resolve(); }); }); + + it('renders script-only advisory issues as informational copy', async () => { + const detail = makeDetail({ + flags: { + hasScripts: true, + hasReferences: false, + hasAssets: false, + }, + issues: [ + { + code: 'has-scripts', + message: 'This skill includes a scripts directory. Review bundled scripts before trusting it.', + severity: 'info', + }, + ], + }); + storeState.skillsDetailsById[detail.item.id] = detail; + + const host = document.createElement('div'); + document.body.appendChild(host); + const root = createRoot(host); + + await act(async () => { + root.render( + React.createElement(SkillDetailDialog, { + skillId: detail.item.id, + open: true, + onClose: vi.fn(), + projectPath: '/tmp/project-a', + onEdit: vi.fn(), + onDeleted: vi.fn(), + }) + ); + await Promise.resolve(); + }); + + expect(host.textContent).toContain('This skill includes bundled scripts'); + expect(host.textContent).toContain( + 'This skill includes a scripts directory. Review bundled scripts before trusting it.' + ); + expect(host.textContent).not.toContain('Review this skill carefully before using it'); + + await act(async () => { + root.unmount(); + await Promise.resolve(); + }); + }); });