fix(extensions): tone down script-only skill advisories
This commit is contained in:
parent
0648509a82
commit
79050cc318
6 changed files with 272 additions and 137 deletions
|
|
@ -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',
|
||||
});
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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<void> {
|
||||
if (!item) return;
|
||||
setDeleteLoading(true);
|
||||
|
|
@ -167,16 +189,30 @@ export const SkillDetailDialog = ({
|
|||
</div>
|
||||
|
||||
{item.issues.length > 0 && (
|
||||
<div className="space-y-2 rounded-md border border-amber-500/30 bg-amber-500/5 p-4">
|
||||
<p className="text-sm font-medium text-amber-700 dark:text-amber-300">
|
||||
Review this skill carefully before using it
|
||||
<div className={`space-y-2 rounded-md border p-4 ${issuesTone?.className ?? ''}`}>
|
||||
<p
|
||||
className={`text-sm font-medium ${
|
||||
issuesTone?.Icon === Info
|
||||
? 'text-blue-700 dark:text-blue-300'
|
||||
: 'text-amber-700 dark:text-amber-300'
|
||||
}`}
|
||||
>
|
||||
{issuesTone?.title}
|
||||
</p>
|
||||
{item.issues.map((issue, index) => (
|
||||
<div
|
||||
key={`${issue.code}-${index}`}
|
||||
className="flex gap-2 text-sm text-amber-700 dark:text-amber-300"
|
||||
className={`flex gap-2 text-sm ${
|
||||
issue.severity === 'info'
|
||||
? 'text-blue-700 dark:text-blue-300'
|
||||
: 'text-amber-700 dark:text-amber-300'
|
||||
}`}
|
||||
>
|
||||
<AlertTriangle className="mt-0.5 size-4 shrink-0" />
|
||||
{issue.severity === 'info' ? (
|
||||
<Info className="mt-0.5 size-4 shrink-0" />
|
||||
) : (
|
||||
<AlertTriangle className="mt-0.5 size-4 shrink-0" />
|
||||
)}
|
||||
<span>{issue.message}</span>
|
||||
</div>
|
||||
))}
|
||||
|
|
|
|||
|
|
@ -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 = ({
|
|||
</Badge>
|
||||
</div>
|
||||
<div className="skills-grid grid grid-cols-1 gap-3 xl:grid-cols-2">
|
||||
{visibleProjectSkills.map((skill) => (
|
||||
<button
|
||||
key={skill.id}
|
||||
type="button"
|
||||
onClick={() => setSelectedSkillId(skill.id)}
|
||||
className={`rounded-xl border p-4 text-left transition-colors ${
|
||||
highlightedSkillId === skill.id
|
||||
? 'border-green-500/50 bg-green-500/10 shadow-[0_0_0_1px_rgba(34,197,94,0.18)]'
|
||||
: 'bg-surface-raised/10 border-border hover:border-border-emphasis'
|
||||
}`}
|
||||
>
|
||||
<div className="flex items-start justify-between gap-3">
|
||||
<div className="min-w-0 space-y-1">
|
||||
<div className="flex flex-wrap items-center gap-2">
|
||||
<h3 className="truncate text-sm font-semibold text-text">{skill.name}</h3>
|
||||
{!skill.isValid && (
|
||||
<Badge
|
||||
variant="outline"
|
||||
className="border-amber-500/40 text-amber-700 dark:text-amber-300"
|
||||
>
|
||||
Needs attention
|
||||
</Badge>
|
||||
)}
|
||||
{visibleProjectSkills.map((skill) => {
|
||||
const primaryIssue = getPrimarySkillIssue(skill);
|
||||
const issueTone = getSkillIssueTone(primaryIssue);
|
||||
const IssueIcon = issueTone.Icon;
|
||||
return (
|
||||
<button
|
||||
key={skill.id}
|
||||
type="button"
|
||||
onClick={() => setSelectedSkillId(skill.id)}
|
||||
className={`rounded-xl border p-4 text-left transition-colors ${
|
||||
highlightedSkillId === skill.id
|
||||
? 'border-green-500/50 bg-green-500/10 shadow-[0_0_0_1px_rgba(34,197,94,0.18)]'
|
||||
: 'bg-surface-raised/10 border-border hover:border-border-emphasis'
|
||||
}`}
|
||||
>
|
||||
<div className="flex items-start justify-between gap-3">
|
||||
<div className="min-w-0 space-y-1">
|
||||
<div className="flex flex-wrap items-center gap-2">
|
||||
<h3 className="truncate text-sm font-semibold text-text">
|
||||
{skill.name}
|
||||
</h3>
|
||||
{!skill.isValid && (
|
||||
<Badge
|
||||
variant="outline"
|
||||
className="border-amber-500/40 text-amber-700 dark:text-amber-300"
|
||||
>
|
||||
Needs attention
|
||||
</Badge>
|
||||
)}
|
||||
</div>
|
||||
<p className="line-clamp-2 text-sm text-text-secondary">
|
||||
{skill.description}
|
||||
</p>
|
||||
</div>
|
||||
<p className="line-clamp-2 text-sm text-text-secondary">
|
||||
{skill.description}
|
||||
</p>
|
||||
<Badge variant="outline">{getScopeLabel(skill)}</Badge>
|
||||
</div>
|
||||
<Badge variant="outline">{getScopeLabel(skill)}</Badge>
|
||||
</div>
|
||||
|
||||
<div className="mt-3 space-y-2 text-xs text-text-muted">
|
||||
<p>{getInvocationLabel(skill)}</p>
|
||||
<p>{getSkillStatus(skill)}</p>
|
||||
</div>
|
||||
|
||||
<div className="mt-3 flex flex-wrap gap-2">
|
||||
<Badge variant="secondary" className="font-normal">
|
||||
Stored in {formatSkillRootKind(skill.rootKind)}
|
||||
</Badge>
|
||||
<Badge variant="outline" className="font-normal">
|
||||
{getSkillAudienceLabel(skill.rootKind)}
|
||||
</Badge>
|
||||
{skill.flags.hasScripts && (
|
||||
<Badge variant="destructive" className="font-normal">
|
||||
Has scripts
|
||||
</Badge>
|
||||
)}
|
||||
{skill.flags.hasReferences && (
|
||||
<Badge variant="secondary" className="font-normal">
|
||||
References
|
||||
</Badge>
|
||||
)}
|
||||
{skill.flags.hasAssets && (
|
||||
<Badge variant="secondary" className="font-normal">
|
||||
Assets
|
||||
</Badge>
|
||||
)}
|
||||
</div>
|
||||
|
||||
{skill.issues.length > 0 && (
|
||||
<div className="mt-3 flex items-start gap-2 rounded-md border border-amber-500/20 bg-amber-500/5 px-3 py-2 text-xs text-amber-700 dark:text-amber-300">
|
||||
<AlertTriangle className="mt-0.5 size-3.5 shrink-0" />
|
||||
<span>{skill.issues[0]?.message}</span>
|
||||
<div className="mt-3 space-y-2 text-xs text-text-muted">
|
||||
<p>{getInvocationLabel(skill)}</p>
|
||||
<p>{getSkillStatus(skill)}</p>
|
||||
</div>
|
||||
)}
|
||||
</button>
|
||||
))}
|
||||
|
||||
<div className="mt-3 flex flex-wrap gap-2">
|
||||
<Badge variant="secondary" className="font-normal">
|
||||
Stored in {formatSkillRootKind(skill.rootKind)}
|
||||
</Badge>
|
||||
<Badge variant="outline" className="font-normal">
|
||||
{getSkillAudienceLabel(skill.rootKind)}
|
||||
</Badge>
|
||||
{skill.flags.hasScripts && (
|
||||
<Badge variant="destructive" className="font-normal">
|
||||
Has scripts
|
||||
</Badge>
|
||||
)}
|
||||
{skill.flags.hasReferences && (
|
||||
<Badge variant="secondary" className="font-normal">
|
||||
References
|
||||
</Badge>
|
||||
)}
|
||||
{skill.flags.hasAssets && (
|
||||
<Badge variant="secondary" className="font-normal">
|
||||
Assets
|
||||
</Badge>
|
||||
)}
|
||||
</div>
|
||||
|
||||
{primaryIssue && (
|
||||
<div
|
||||
className={`mt-3 flex items-start gap-2 rounded-md border px-3 py-2 text-xs ${issueTone.className}`}
|
||||
>
|
||||
<IssueIcon className="mt-0.5 size-3.5 shrink-0" />
|
||||
<span>{primaryIssue.message}</span>
|
||||
</div>
|
||||
)}
|
||||
</button>
|
||||
);
|
||||
})}
|
||||
</div>
|
||||
</section>
|
||||
)}
|
||||
|
|
@ -573,74 +609,83 @@ export const SkillsPanel = ({
|
|||
</Badge>
|
||||
</div>
|
||||
<div className="skills-grid grid grid-cols-1 gap-3 xl:grid-cols-2">
|
||||
{visibleUserSkills.map((skill) => (
|
||||
<button
|
||||
key={skill.id}
|
||||
type="button"
|
||||
onClick={() => setSelectedSkillId(skill.id)}
|
||||
className={`rounded-xl border p-4 text-left transition-colors ${
|
||||
highlightedSkillId === skill.id
|
||||
? 'border-green-500/50 bg-green-500/10 shadow-[0_0_0_1px_rgba(34,197,94,0.18)]'
|
||||
: 'bg-surface-raised/10 border-border hover:border-border-emphasis'
|
||||
}`}
|
||||
>
|
||||
<div className="flex items-start justify-between gap-3">
|
||||
<div className="min-w-0 space-y-1">
|
||||
<div className="flex flex-wrap items-center gap-2">
|
||||
<h3 className="truncate text-sm font-semibold text-text">{skill.name}</h3>
|
||||
{!skill.isValid && (
|
||||
<Badge
|
||||
variant="outline"
|
||||
className="border-amber-500/40 text-amber-700 dark:text-amber-300"
|
||||
>
|
||||
Needs attention
|
||||
</Badge>
|
||||
)}
|
||||
{visibleUserSkills.map((skill) => {
|
||||
const primaryIssue = getPrimarySkillIssue(skill);
|
||||
const issueTone = getSkillIssueTone(primaryIssue);
|
||||
const IssueIcon = issueTone.Icon;
|
||||
return (
|
||||
<button
|
||||
key={skill.id}
|
||||
type="button"
|
||||
onClick={() => setSelectedSkillId(skill.id)}
|
||||
className={`rounded-xl border p-4 text-left transition-colors ${
|
||||
highlightedSkillId === skill.id
|
||||
? 'border-green-500/50 bg-green-500/10 shadow-[0_0_0_1px_rgba(34,197,94,0.18)]'
|
||||
: 'bg-surface-raised/10 border-border hover:border-border-emphasis'
|
||||
}`}
|
||||
>
|
||||
<div className="flex items-start justify-between gap-3">
|
||||
<div className="min-w-0 space-y-1">
|
||||
<div className="flex flex-wrap items-center gap-2">
|
||||
<h3 className="truncate text-sm font-semibold text-text">
|
||||
{skill.name}
|
||||
</h3>
|
||||
{!skill.isValid && (
|
||||
<Badge
|
||||
variant="outline"
|
||||
className="border-amber-500/40 text-amber-700 dark:text-amber-300"
|
||||
>
|
||||
Needs attention
|
||||
</Badge>
|
||||
)}
|
||||
</div>
|
||||
<p className="line-clamp-2 text-sm text-text-secondary">
|
||||
{skill.description}
|
||||
</p>
|
||||
</div>
|
||||
<p className="line-clamp-2 text-sm text-text-secondary">
|
||||
{skill.description}
|
||||
</p>
|
||||
<Badge variant="outline">{getScopeLabel(skill)}</Badge>
|
||||
</div>
|
||||
<Badge variant="outline">{getScopeLabel(skill)}</Badge>
|
||||
</div>
|
||||
|
||||
<div className="mt-3 space-y-2 text-xs text-text-muted">
|
||||
<p>{getInvocationLabel(skill)}</p>
|
||||
<p>{getSkillStatus(skill)}</p>
|
||||
</div>
|
||||
|
||||
<div className="mt-3 flex flex-wrap gap-2">
|
||||
<Badge variant="secondary" className="font-normal">
|
||||
Stored in {formatSkillRootKind(skill.rootKind)}
|
||||
</Badge>
|
||||
<Badge variant="outline" className="font-normal">
|
||||
{getSkillAudienceLabel(skill.rootKind)}
|
||||
</Badge>
|
||||
{skill.flags.hasScripts && (
|
||||
<Badge variant="destructive" className="font-normal">
|
||||
Has scripts
|
||||
</Badge>
|
||||
)}
|
||||
{skill.flags.hasReferences && (
|
||||
<Badge variant="secondary" className="font-normal">
|
||||
References
|
||||
</Badge>
|
||||
)}
|
||||
{skill.flags.hasAssets && (
|
||||
<Badge variant="secondary" className="font-normal">
|
||||
Assets
|
||||
</Badge>
|
||||
)}
|
||||
</div>
|
||||
|
||||
{skill.issues.length > 0 && (
|
||||
<div className="mt-3 flex items-start gap-2 rounded-md border border-amber-500/20 bg-amber-500/5 px-3 py-2 text-xs text-amber-700 dark:text-amber-300">
|
||||
<AlertTriangle className="mt-0.5 size-3.5 shrink-0" />
|
||||
<span>{skill.issues[0]?.message}</span>
|
||||
<div className="mt-3 space-y-2 text-xs text-text-muted">
|
||||
<p>{getInvocationLabel(skill)}</p>
|
||||
<p>{getSkillStatus(skill)}</p>
|
||||
</div>
|
||||
)}
|
||||
</button>
|
||||
))}
|
||||
|
||||
<div className="mt-3 flex flex-wrap gap-2">
|
||||
<Badge variant="secondary" className="font-normal">
|
||||
Stored in {formatSkillRootKind(skill.rootKind)}
|
||||
</Badge>
|
||||
<Badge variant="outline" className="font-normal">
|
||||
{getSkillAudienceLabel(skill.rootKind)}
|
||||
</Badge>
|
||||
{skill.flags.hasScripts && (
|
||||
<Badge variant="destructive" className="font-normal">
|
||||
Has scripts
|
||||
</Badge>
|
||||
)}
|
||||
{skill.flags.hasReferences && (
|
||||
<Badge variant="secondary" className="font-normal">
|
||||
References
|
||||
</Badge>
|
||||
)}
|
||||
{skill.flags.hasAssets && (
|
||||
<Badge variant="secondary" className="font-normal">
|
||||
Assets
|
||||
</Badge>
|
||||
)}
|
||||
</div>
|
||||
|
||||
{primaryIssue && (
|
||||
<div
|
||||
className={`mt-3 flex items-start gap-2 rounded-md border px-3 py-2 text-xs ${issueTone.className}`}
|
||||
>
|
||||
<IssueIcon className="mt-0.5 size-3.5 shrink-0" />
|
||||
<span>{primaryIssue.message}</span>
|
||||
</div>
|
||||
)}
|
||||
</button>
|
||||
);
|
||||
})}
|
||||
</div>
|
||||
</section>
|
||||
)}
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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', () => {
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue