fix(extensions): preserve skill project context in dialogs
This commit is contained in:
parent
c02a12b3f6
commit
3e5ec7c173
6 changed files with 350 additions and 10 deletions
|
|
@ -26,6 +26,8 @@ import { useStore } from '@renderer/store';
|
|||
import { AlertTriangle, ExternalLink, FolderOpen, Pencil, Trash2 } from 'lucide-react';
|
||||
import { useShallow } from 'zustand/react/shallow';
|
||||
|
||||
import { resolveSkillProjectPath } from './skillProjectUtils';
|
||||
|
||||
interface SkillDetailDialogProps {
|
||||
skillId: string | null;
|
||||
open: boolean;
|
||||
|
|
@ -58,8 +60,13 @@ export const SkillDetailDialog = ({
|
|||
|
||||
useEffect(() => {
|
||||
if (!open || !skillId) return;
|
||||
void fetchSkillDetail(skillId, projectPath ?? undefined).catch(() => undefined);
|
||||
}, [fetchSkillDetail, open, projectPath, skillId]);
|
||||
void fetchSkillDetail(
|
||||
skillId,
|
||||
detail?.item.scope
|
||||
? resolveSkillProjectPath(detail.item.scope, projectPath, detail.item.projectRoot)
|
||||
: (projectPath ?? undefined)
|
||||
).catch(() => undefined);
|
||||
}, [detail?.item.projectRoot, detail?.item.scope, fetchSkillDetail, open, projectPath, skillId]);
|
||||
|
||||
useEffect(() => {
|
||||
if (!open) {
|
||||
|
|
@ -70,6 +77,9 @@ export const SkillDetailDialog = ({
|
|||
}, [open]);
|
||||
|
||||
const item = detail?.item;
|
||||
const effectiveProjectPath = item
|
||||
? resolveSkillProjectPath(item.scope, projectPath, item.projectRoot)
|
||||
: (projectPath ?? undefined);
|
||||
|
||||
function formatRootKind(rootKind: 'claude' | 'cursor' | 'agents'): string {
|
||||
return `.${rootKind}`;
|
||||
|
|
@ -92,7 +102,7 @@ export const SkillDetailDialog = ({
|
|||
try {
|
||||
await deleteSkill({
|
||||
skillId: item.id,
|
||||
projectPath: projectPath ?? undefined,
|
||||
projectPath: effectiveProjectPath,
|
||||
});
|
||||
setDeleteConfirmOpen(false);
|
||||
onDeleted();
|
||||
|
|
@ -125,7 +135,7 @@ export const SkillDetailDialog = ({
|
|||
variant="outline"
|
||||
size="sm"
|
||||
onClick={() => {
|
||||
void fetchSkillDetail(skillId, projectPath ?? undefined).catch(() => undefined);
|
||||
void fetchSkillDetail(skillId, effectiveProjectPath).catch(() => undefined);
|
||||
}}
|
||||
>
|
||||
Retry
|
||||
|
|
@ -288,7 +298,7 @@ export const SkillDetailDialog = ({
|
|||
<Button
|
||||
variant="outline"
|
||||
size="sm"
|
||||
onClick={() => void api.openPath(item.skillFile, projectPath ?? undefined)}
|
||||
onClick={() => void api.openPath(item.skillFile, effectiveProjectPath)}
|
||||
>
|
||||
<ExternalLink className="mr-1.5 size-3.5" />
|
||||
Open SKILL.md
|
||||
|
|
|
|||
|
|
@ -32,6 +32,7 @@ import {
|
|||
readSkillTemplateContent,
|
||||
updateSkillTemplateFrontmatter,
|
||||
} from './skillDraftUtils';
|
||||
import { resolveSkillProjectPath } from './skillProjectUtils';
|
||||
import { SkillReviewDialog } from './SkillReviewDialog';
|
||||
|
||||
import type {
|
||||
|
|
@ -227,15 +228,31 @@ export const SkillEditorDialog = ({
|
|||
setMutationError(null);
|
||||
}, [detail, mode, open, projectPath]);
|
||||
|
||||
useEffect(() => {
|
||||
if (open && mode === 'create' && scope === 'project' && !projectPath) {
|
||||
setScope('user');
|
||||
}
|
||||
}, [mode, open, projectPath, scope]);
|
||||
|
||||
useEffect(() => {
|
||||
rawContentRef.current = rawContent;
|
||||
}, [rawContent]);
|
||||
|
||||
const effectiveProjectPath = useMemo(
|
||||
() =>
|
||||
resolveSkillProjectPath(
|
||||
scope,
|
||||
projectPath,
|
||||
mode === 'edit' ? detail?.item.projectRoot : undefined
|
||||
),
|
||||
[detail?.item.projectRoot, mode, projectPath, scope]
|
||||
);
|
||||
|
||||
const request = useMemo(
|
||||
() => ({
|
||||
scope,
|
||||
rootKind,
|
||||
projectPath: scope === 'project' ? (projectPath ?? undefined) : undefined,
|
||||
projectPath: effectiveProjectPath,
|
||||
folderName,
|
||||
existingSkillId: mode === 'edit' ? detail?.item.id : undefined,
|
||||
files: buildSkillDraftFiles({
|
||||
|
|
@ -252,10 +269,10 @@ export const SkillEditorDialog = ({
|
|||
includeReferences,
|
||||
includeScripts,
|
||||
mode,
|
||||
projectPath,
|
||||
rawContent,
|
||||
rootKind,
|
||||
scope,
|
||||
effectiveProjectPath,
|
||||
]
|
||||
);
|
||||
const draftFilePaths = useMemo(
|
||||
|
|
@ -285,7 +302,7 @@ export const SkillEditorDialog = ({
|
|||
if (!folderName.trim()) {
|
||||
return 'Choose a folder name for this skill.';
|
||||
}
|
||||
if (scope === 'project' && !projectPath) {
|
||||
if (scope === 'project' && !effectiveProjectPath) {
|
||||
return 'Project skills need an active project.';
|
||||
}
|
||||
return null;
|
||||
|
|
|
|||
|
|
@ -23,6 +23,7 @@ import { useStore } from '@renderer/store';
|
|||
import { FileSearch, FolderOpen, X } from 'lucide-react';
|
||||
|
||||
import { SkillReviewDialog } from './SkillReviewDialog';
|
||||
import { resolveSkillProjectPath } from './skillProjectUtils';
|
||||
|
||||
import type { SkillReviewPreview } from '@shared/types/extensions';
|
||||
|
||||
|
|
@ -125,7 +126,7 @@ export const SkillImportDialog = ({
|
|||
folderName: folderName || undefined,
|
||||
scope,
|
||||
rootKind,
|
||||
projectPath: scope === 'project' ? (projectPath ?? undefined) : undefined,
|
||||
projectPath: resolveSkillProjectPath(scope, projectPath),
|
||||
});
|
||||
setPreview(nextPreview);
|
||||
setReviewOpen(true);
|
||||
|
|
@ -149,7 +150,7 @@ export const SkillImportDialog = ({
|
|||
folderName: folderName || undefined,
|
||||
scope,
|
||||
rootKind,
|
||||
projectPath: scope === 'project' ? (projectPath ?? undefined) : undefined,
|
||||
projectPath: resolveSkillProjectPath(scope, projectPath),
|
||||
reviewPlanId: preview?.planId,
|
||||
});
|
||||
setReviewOpen(false);
|
||||
|
|
|
|||
|
|
@ -0,0 +1,13 @@
|
|||
import type { SkillScope } from '@shared/types/extensions';
|
||||
|
||||
export function resolveSkillProjectPath(
|
||||
scope: SkillScope,
|
||||
currentProjectPath: string | null,
|
||||
itemProjectRoot?: string | null
|
||||
): string | undefined {
|
||||
if (scope !== 'project') {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
return itemProjectRoot ?? currentProjectPath ?? undefined;
|
||||
}
|
||||
|
|
@ -0,0 +1,280 @@
|
|||
import React, { act } from 'react';
|
||||
import { createRoot } from 'react-dom/client';
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import type { SkillDetail } from '@shared/types/extensions';
|
||||
|
||||
interface StoreState {
|
||||
fetchSkillDetail: ReturnType<typeof vi.fn>;
|
||||
deleteSkill: ReturnType<typeof vi.fn>;
|
||||
skillsDetailsById: Record<string, SkillDetail | null | undefined>;
|
||||
skillsDetailLoadingById: Record<string, boolean>;
|
||||
skillsDetailErrorById: Record<string, string | null>;
|
||||
}
|
||||
|
||||
const storeState = {} as StoreState;
|
||||
const openPathMock = vi.fn();
|
||||
const showInFolderMock = vi.fn();
|
||||
|
||||
vi.mock('@renderer/store', () => ({
|
||||
useStore: (selector: (state: StoreState) => unknown) => selector(storeState),
|
||||
}));
|
||||
|
||||
vi.mock('zustand/react/shallow', () => ({
|
||||
useShallow: <T,>(selector: T) => selector,
|
||||
}));
|
||||
|
||||
vi.mock('@renderer/api', () => ({
|
||||
api: {
|
||||
openPath: (...args: unknown[]) => openPathMock(...args),
|
||||
showInFolder: (...args: unknown[]) => showInFolderMock(...args),
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock('@renderer/components/chat/viewers/CodeBlockViewer', () => ({
|
||||
CodeBlockViewer: () => React.createElement('div', null, 'Code'),
|
||||
}));
|
||||
|
||||
vi.mock('@renderer/components/chat/viewers/MarkdownViewer', () => ({
|
||||
MarkdownViewer: () => React.createElement('div', null, 'Markdown'),
|
||||
}));
|
||||
|
||||
vi.mock('@renderer/components/ui/alert-dialog', () => ({
|
||||
AlertDialog: ({
|
||||
open,
|
||||
children,
|
||||
}: React.PropsWithChildren<{
|
||||
open: boolean;
|
||||
onOpenChange?: (next: boolean) => void;
|
||||
}>) => (open ? React.createElement('div', null, children) : null),
|
||||
AlertDialogAction: ({
|
||||
children,
|
||||
onClick,
|
||||
disabled,
|
||||
}: React.PropsWithChildren<{ onClick?: () => void; disabled?: boolean }>) =>
|
||||
React.createElement(
|
||||
'button',
|
||||
{
|
||||
type: 'button',
|
||||
disabled,
|
||||
onClick,
|
||||
},
|
||||
children
|
||||
),
|
||||
AlertDialogCancel: ({ children }: React.PropsWithChildren<{ disabled?: boolean }>) =>
|
||||
React.createElement('button', { type: 'button' }, children),
|
||||
AlertDialogContent: ({ children }: React.PropsWithChildren) => React.createElement('div', null, children),
|
||||
AlertDialogDescription: ({ children }: React.PropsWithChildren) =>
|
||||
React.createElement('p', null, children),
|
||||
AlertDialogFooter: ({ children }: React.PropsWithChildren) => React.createElement('div', null, children),
|
||||
AlertDialogHeader: ({ children }: React.PropsWithChildren) => React.createElement('div', null, children),
|
||||
AlertDialogTitle: ({ children }: React.PropsWithChildren) => React.createElement('h3', null, children),
|
||||
}));
|
||||
|
||||
vi.mock('@renderer/components/ui/badge', () => ({
|
||||
Badge: ({ children }: React.PropsWithChildren) => React.createElement('span', null, children),
|
||||
}));
|
||||
|
||||
vi.mock('@renderer/components/ui/button', () => ({
|
||||
Button: ({
|
||||
children,
|
||||
onClick,
|
||||
type = 'button',
|
||||
disabled,
|
||||
}: React.PropsWithChildren<{
|
||||
onClick?: () => void;
|
||||
type?: 'button' | 'submit' | 'reset';
|
||||
disabled?: boolean;
|
||||
}>) =>
|
||||
React.createElement(
|
||||
'button',
|
||||
{
|
||||
type,
|
||||
disabled,
|
||||
onClick,
|
||||
},
|
||||
children
|
||||
),
|
||||
}));
|
||||
|
||||
vi.mock('@renderer/components/ui/dialog', () => ({
|
||||
Dialog: ({ open, children }: React.PropsWithChildren<{ open: boolean }>) =>
|
||||
open ? React.createElement('div', null, children) : null,
|
||||
DialogContent: ({ children }: React.PropsWithChildren) => React.createElement('div', null, children),
|
||||
DialogDescription: ({ children }: React.PropsWithChildren) =>
|
||||
React.createElement('p', null, children),
|
||||
DialogHeader: ({ children }: React.PropsWithChildren) => React.createElement('div', null, children),
|
||||
DialogTitle: ({ children }: React.PropsWithChildren) => React.createElement('h2', null, children),
|
||||
}));
|
||||
|
||||
vi.mock('lucide-react', () => {
|
||||
const Icon = (props: React.SVGProps<SVGSVGElement>) => React.createElement('svg', props);
|
||||
return {
|
||||
AlertTriangle: Icon,
|
||||
ExternalLink: Icon,
|
||||
FolderOpen: Icon,
|
||||
Pencil: Icon,
|
||||
Trash2: Icon,
|
||||
};
|
||||
});
|
||||
|
||||
import { SkillDetailDialog } from '@renderer/components/extensions/skills/SkillDetailDialog';
|
||||
|
||||
function makeDetail(overrides: Partial<SkillDetail['item']>): SkillDetail {
|
||||
return {
|
||||
item: {
|
||||
id: '/tmp/project-a/.claude/skills/review-helper',
|
||||
sourceType: 'filesystem',
|
||||
name: 'Review Helper',
|
||||
description: 'Helps with code review',
|
||||
folderName: 'review-helper',
|
||||
scope: 'project',
|
||||
rootKind: 'claude',
|
||||
projectRoot: '/tmp/project-a',
|
||||
discoveryRoot: '/tmp/project-a/.claude/skills',
|
||||
skillDir: '/tmp/project-a/.claude/skills/review-helper',
|
||||
skillFile: '/tmp/project-a/.claude/skills/review-helper/SKILL.md',
|
||||
metadata: {},
|
||||
invocationMode: 'auto',
|
||||
flags: {
|
||||
hasScripts: false,
|
||||
hasReferences: false,
|
||||
hasAssets: false,
|
||||
},
|
||||
isValid: true,
|
||||
issues: [],
|
||||
modifiedAt: 1,
|
||||
...overrides,
|
||||
},
|
||||
body: 'body',
|
||||
rawContent: '# Review Helper',
|
||||
rawFrontmatter: null,
|
||||
referencesFiles: [],
|
||||
scriptFiles: [],
|
||||
assetFiles: [],
|
||||
};
|
||||
}
|
||||
|
||||
describe('SkillDetailDialog', () => {
|
||||
beforeEach(() => {
|
||||
vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true);
|
||||
storeState.fetchSkillDetail = vi.fn().mockResolvedValue(undefined);
|
||||
storeState.deleteSkill = vi.fn().mockResolvedValue(undefined);
|
||||
storeState.skillsDetailsById = {};
|
||||
storeState.skillsDetailLoadingById = {};
|
||||
storeState.skillsDetailErrorById = {};
|
||||
openPathMock.mockReset();
|
||||
showInFolderMock.mockReset();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
document.body.innerHTML = '';
|
||||
vi.unstubAllGlobals();
|
||||
});
|
||||
|
||||
it('uses the skill project root for project-scoped open and delete actions', async () => {
|
||||
const detail = makeDetail({});
|
||||
storeState.skillsDetailsById[detail.item.id] = detail;
|
||||
|
||||
const host = document.createElement('div');
|
||||
document.body.appendChild(host);
|
||||
const root = createRoot(host);
|
||||
const onDeleted = vi.fn();
|
||||
|
||||
await act(async () => {
|
||||
root.render(
|
||||
React.createElement(SkillDetailDialog, {
|
||||
skillId: detail.item.id,
|
||||
open: true,
|
||||
onClose: vi.fn(),
|
||||
projectPath: '/tmp/project-b',
|
||||
onEdit: vi.fn(),
|
||||
onDeleted,
|
||||
})
|
||||
);
|
||||
await Promise.resolve();
|
||||
});
|
||||
|
||||
const openButton = Array.from(host.querySelectorAll('button')).find((button) =>
|
||||
button.textContent?.includes('Open SKILL.md')
|
||||
) as HTMLButtonElement;
|
||||
await act(async () => {
|
||||
openButton.click();
|
||||
await Promise.resolve();
|
||||
});
|
||||
|
||||
expect(openPathMock).toHaveBeenCalledWith(detail.item.skillFile, '/tmp/project-a');
|
||||
|
||||
const deleteButton = Array.from(host.querySelectorAll('button')).find((button) =>
|
||||
button.textContent === 'Delete'
|
||||
) as HTMLButtonElement;
|
||||
await act(async () => {
|
||||
deleteButton.click();
|
||||
await Promise.resolve();
|
||||
});
|
||||
|
||||
const confirmDeleteButton = Array.from(host.querySelectorAll('button')).find((button) =>
|
||||
button.textContent === 'Delete Skill'
|
||||
) as HTMLButtonElement;
|
||||
await act(async () => {
|
||||
confirmDeleteButton.click();
|
||||
await Promise.resolve();
|
||||
});
|
||||
|
||||
expect(storeState.deleteSkill).toHaveBeenCalledWith({
|
||||
skillId: detail.item.id,
|
||||
projectPath: '/tmp/project-a',
|
||||
});
|
||||
expect(onDeleted).toHaveBeenCalled();
|
||||
|
||||
await act(async () => {
|
||||
root.unmount();
|
||||
await Promise.resolve();
|
||||
});
|
||||
});
|
||||
|
||||
it('does not forward the current project path for personal skills', async () => {
|
||||
const detail = makeDetail({
|
||||
id: '/Users/me/.claude/skills/review-helper',
|
||||
scope: 'user',
|
||||
projectRoot: null,
|
||||
discoveryRoot: '/Users/me/.claude/skills',
|
||||
skillDir: '/Users/me/.claude/skills/review-helper',
|
||||
skillFile: '/Users/me/.claude/skills/review-helper/SKILL.md',
|
||||
});
|
||||
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-b',
|
||||
onEdit: vi.fn(),
|
||||
onDeleted: vi.fn(),
|
||||
})
|
||||
);
|
||||
await Promise.resolve();
|
||||
});
|
||||
|
||||
const openButton = Array.from(host.querySelectorAll('button')).find((button) =>
|
||||
button.textContent?.includes('Open SKILL.md')
|
||||
) as HTMLButtonElement;
|
||||
await act(async () => {
|
||||
openButton.click();
|
||||
await Promise.resolve();
|
||||
});
|
||||
|
||||
expect(openPathMock).toHaveBeenCalledWith(detail.item.skillFile, undefined);
|
||||
|
||||
await act(async () => {
|
||||
root.unmount();
|
||||
await Promise.resolve();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,19 @@
|
|||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
import { resolveSkillProjectPath } from '../../../../../src/renderer/components/extensions/skills/skillProjectUtils';
|
||||
|
||||
describe('resolveSkillProjectPath', () => {
|
||||
it('returns undefined for user-scoped skills', () => {
|
||||
expect(resolveSkillProjectPath('user', '/tmp/project-a', '/tmp/project-a')).toBeUndefined();
|
||||
});
|
||||
|
||||
it('prefers the skill project root over the current tab project for project-scoped skills', () => {
|
||||
expect(resolveSkillProjectPath('project', '/tmp/project-b', '/tmp/project-a')).toBe(
|
||||
'/tmp/project-a'
|
||||
);
|
||||
});
|
||||
|
||||
it('falls back to the current tab project when a project skill has no embedded root', () => {
|
||||
expect(resolveSkillProjectPath('project', '/tmp/project-a', null)).toBe('/tmp/project-a');
|
||||
});
|
||||
});
|
||||
Loading…
Reference in a new issue