diff --git a/package.json b/package.json index 21f228ff..f595ccd2 100644 --- a/package.json +++ b/package.json @@ -96,6 +96,7 @@ "@dnd-kit/utilities": "^3.2.2", "@fastify/cors": "^11.2.0", "@fastify/static": "^9.0.0", + "@radix-ui/react-alert-dialog": "^1.1.15", "@radix-ui/react-checkbox": "^1.3.3", "@radix-ui/react-collapsible": "^1.1.12", "@radix-ui/react-context-menu": "^2.2.16", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 3a35f350..a77aac85 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -101,6 +101,9 @@ importers: '@fastify/static': specifier: ^9.0.0 version: 9.0.0 + '@radix-ui/react-alert-dialog': + specifier: ^1.1.15 + version: 1.1.15(@types/react-dom@18.3.7(@types/react@18.3.27))(@types/react@18.3.27)(react-dom@18.3.1(react@18.3.1))(react@18.3.1) '@radix-ui/react-checkbox': specifier: ^1.3.3 version: 1.3.3(@types/react-dom@18.3.7(@types/react@18.3.27))(@types/react@18.3.27)(react-dom@18.3.1(react@18.3.1))(react@18.3.1) @@ -1424,6 +1427,19 @@ packages: '@radix-ui/primitive@1.1.3': resolution: {integrity: sha512-JTF99U/6XIjCBo0wqkU5sK10glYe27MRRsfwoiq5zzOEZLHU3A3KCMa5X/azekYRCJ0HlwI0crAXS/5dEHTzDg==} + '@radix-ui/react-alert-dialog@1.1.15': + resolution: {integrity: sha512-oTVLkEw5GpdRe29BqJ0LSDFWI3qu0vR1M0mUkOQWDIUnY/QIkLpgDMWuKxP94c2NAC2LGcgVhG1ImF3jkZ5wXw==} + peerDependencies: + '@types/react': '*' + '@types/react-dom': '*' + react: ^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc + react-dom: ^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc + peerDependenciesMeta: + '@types/react': + optional: true + '@types/react-dom': + optional: true + '@radix-ui/react-arrow@1.1.7': resolution: {integrity: sha512-F+M1tLhO+mlQaOWspE8Wstg+z6PwxwRd8oQ8IXceWz92kfAmalTRf0EjrouQeo7QssEPfCn05B4Ihs1K9WQ/7w==} peerDependencies: @@ -7995,6 +8011,20 @@ snapshots: '@radix-ui/primitive@1.1.3': {} + '@radix-ui/react-alert-dialog@1.1.15(@types/react-dom@18.3.7(@types/react@18.3.27))(@types/react@18.3.27)(react-dom@18.3.1(react@18.3.1))(react@18.3.1)': + dependencies: + '@radix-ui/primitive': 1.1.3 + '@radix-ui/react-compose-refs': 1.1.2(@types/react@18.3.27)(react@18.3.1) + '@radix-ui/react-context': 1.1.2(@types/react@18.3.27)(react@18.3.1) + '@radix-ui/react-dialog': 1.1.15(@types/react-dom@18.3.7(@types/react@18.3.27))(@types/react@18.3.27)(react-dom@18.3.1(react@18.3.1))(react@18.3.1) + '@radix-ui/react-primitive': 2.1.3(@types/react-dom@18.3.7(@types/react@18.3.27))(@types/react@18.3.27)(react-dom@18.3.1(react@18.3.1))(react@18.3.1) + '@radix-ui/react-slot': 1.2.3(@types/react@18.3.27)(react@18.3.1) + react: 18.3.1 + react-dom: 18.3.1(react@18.3.1) + optionalDependencies: + '@types/react': 18.3.27 + '@types/react-dom': 18.3.7(@types/react@18.3.27) + '@radix-ui/react-arrow@1.1.7(@types/react-dom@18.3.7(@types/react@18.3.27))(@types/react@18.3.27)(react-dom@18.3.1(react@18.3.1))(react@18.3.1)': dependencies: '@radix-ui/react-primitive': 2.1.3(@types/react-dom@18.3.7(@types/react@18.3.27))(@types/react@18.3.27)(react-dom@18.3.1(react@18.3.1))(react@18.3.1) diff --git a/src/main/services/extensions/index.ts b/src/main/services/extensions/index.ts index 51d4e6cf..1a8d5d7b 100644 --- a/src/main/services/extensions/index.ts +++ b/src/main/services/extensions/index.ts @@ -21,6 +21,7 @@ export { SkillValidator } from './skills/SkillValidator'; export { SkillsCatalogService } from './skills/SkillsCatalogService'; export { SkillScaffoldService } from './skills/SkillScaffoldService'; export { SkillImportService } from './skills/SkillImportService'; +export { SkillPlanService } from './skills/SkillPlanService'; export { SkillReviewService } from './skills/SkillReviewService'; export { SkillsMutationService } from './skills/SkillsMutationService'; export { SkillsWatcherService } from './skills/SkillsWatcherService'; diff --git a/src/main/services/extensions/skills/SkillImportService.ts b/src/main/services/extensions/skills/SkillImportService.ts index 3e6495ac..b67c0b01 100644 --- a/src/main/services/extensions/skills/SkillImportService.ts +++ b/src/main/services/extensions/skills/SkillImportService.ts @@ -13,6 +13,15 @@ export interface ImportedSkillSourceFile { isBinary: boolean; } +export interface SkillImportInspection { + files: ImportedSkillSourceFile[]; + warnings: string[]; + hiddenEntriesSkipped: number; +} + +const MAX_IMPORT_FILE_COUNT = 200; +const MAX_IMPORT_TOTAL_BYTES = 10 * 1024 * 1024; + export class SkillImportService { constructor(private readonly scanner = new SkillScanner()) {} @@ -36,11 +45,11 @@ export class SkillImportService { return normalizedSourceDir; } - async readSourceFiles(sourceDir: string): Promise { - const entries = await this.walkDirectory(sourceDir); - return Promise.all( - entries.map(async (absolutePath) => { - const relativePath = path.relative(sourceDir, absolutePath).replace(/\\/g, '/'); + async inspectSourceDir(sourceDir: string): Promise { + const normalizedSourceDir = await this.validateSourceDir(sourceDir); + const walked = await this.walkDirectory(normalizedSourceDir); + const files = await Promise.all( + walked.files.map(async ({ absolutePath, relativePath }) => { const binary = await isBinaryFile(absolutePath); return { relativePath, @@ -50,6 +59,31 @@ export class SkillImportService { }; }) ); + + const warnings: string[] = []; + if (walked.hiddenEntriesSkipped > 0) { + warnings.push('Hidden files and folders were skipped during import.'); + } + if (files.some((file) => file.isBinary)) { + warnings.push('This import includes binary files. Binary files will be copied as-is.'); + } + if ( + files.some( + (file) => file.relativePath === 'scripts' || file.relativePath.startsWith('scripts/') + ) + ) { + warnings.push('This import includes scripts. Review them carefully before importing.'); + } + + return { + files, + warnings, + hiddenEntriesSkipped: walked.hiddenEntriesSkipped, + }; + } + + async readSourceFiles(sourceDir: string): Promise { + return (await this.inspectSourceDir(sourceDir)).files; } async writeImportedFiles( @@ -67,17 +101,57 @@ export class SkillImportService { } } - private async walkDirectory(rootDir: string): Promise { - const dirEntries = await fs.readdir(rootDir, { withFileTypes: true }); - const results = await Promise.all( - dirEntries.map(async (entry) => { - const fullPath = path.join(rootDir, entry.name); - if (entry.isDirectory()) { - return this.walkDirectory(fullPath); + private async walkDirectory( + rootDir: string + ): Promise<{ + files: Array<{ absolutePath: string; relativePath: string }>; + hiddenEntriesSkipped: number; + }> { + const allFiles: Array<{ absolutePath: string; relativePath: string }> = []; + let hiddenEntriesSkipped = 0; + let totalBytes = 0; + + const visit = async (currentDir: string): Promise => { + const dirEntries = await fs.readdir(currentDir, { withFileTypes: true }); + for (const entry of dirEntries) { + if (entry.name.startsWith('.')) { + hiddenEntriesSkipped += 1; + continue; } - return [fullPath]; - }) - ); - return results.flat().sort((a, b) => a.localeCompare(b)); + + const fullPath = path.join(currentDir, entry.name); + if (entry.isSymbolicLink()) { + throw new Error('Import source cannot contain symbolic links'); + } + + if (entry.isDirectory()) { + await visit(fullPath); + continue; + } + + const stat = await fs.stat(fullPath); + totalBytes += stat.size; + if (allFiles.length + 1 > MAX_IMPORT_FILE_COUNT) { + throw new Error(`Import source has too many files (max ${MAX_IMPORT_FILE_COUNT})`); + } + if (totalBytes > MAX_IMPORT_TOTAL_BYTES) { + throw new Error( + `Import source is too large (max ${Math.floor(MAX_IMPORT_TOTAL_BYTES / (1024 * 1024))} MB)` + ); + } + + allFiles.push({ + absolutePath: fullPath, + relativePath: path.relative(rootDir, fullPath).replace(/\\/g, '/'), + }); + } + }; + + await visit(rootDir); + + return { + files: allFiles.sort((a, b) => a.relativePath.localeCompare(b.relativePath)), + hiddenEntriesSkipped, + }; } } diff --git a/src/main/services/extensions/skills/SkillPlanService.ts b/src/main/services/extensions/skills/SkillPlanService.ts new file mode 100644 index 00000000..7b357ddf --- /dev/null +++ b/src/main/services/extensions/skills/SkillPlanService.ts @@ -0,0 +1,412 @@ +import * as fs from 'node:fs/promises'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { createHash } from 'node:crypto'; + +import type { + SkillDraftFile, + SkillReviewFileChange, + SkillReviewPreview, + SkillReviewSummary, +} from '@shared/types/extensions'; + +import type { ImportedSkillSourceFile } from './SkillImportService'; + +import { SkillScanner } from './SkillScanner'; + +type SkillPlanInputFile = + | { relativePath: string; isBinary: false; content: string } + | { relativePath: string; isBinary: true; sourceAbsolutePath: string }; + +interface ManagedCurrentFile { + relativePath: string; + absolutePath: string; +} + +interface SkillExecutionChange extends SkillReviewFileChange { + sourceAbsolutePath?: string; +} + +export interface SkillExecutionPlan { + preview: SkillReviewPreview; + changes: SkillExecutionChange[]; +} + +const MANAGED_SUBDIRECTORIES = ['scripts', 'references', 'assets'] as const; +export class SkillPlanService { + constructor(private readonly scanner = new SkillScanner()) {} + + async buildUpsertPlan( + targetSkillDir: string, + files: SkillDraftFile[] + ): Promise { + const desiredFiles: SkillPlanInputFile[] = files.map((file) => ({ + relativePath: file.relativePath, + isBinary: false, + content: file.content, + })); + + return this.buildPlan(targetSkillDir, desiredFiles, 'upsert'); + } + + async buildImportPlan( + targetSkillDir: string, + files: ImportedSkillSourceFile[] + ): Promise { + const desiredFiles: SkillPlanInputFile[] = files.map((file) => + file.isBinary + ? { + relativePath: file.relativePath, + isBinary: true, + sourceAbsolutePath: file.absolutePath, + } + : { + relativePath: file.relativePath, + isBinary: false, + content: file.content ?? '', + } + ); + + return this.buildPlan(targetSkillDir, desiredFiles, 'import'); + } + + async applyPlan(plan: SkillExecutionPlan): Promise { + const backupRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'skill-plan-backup-')); + const createdPaths: string[] = []; + const backups: Array<{ absolutePath: string; backupPath: string }> = []; + + try { + for (const [index, change] of plan.changes.entries()) { + if (change.action !== 'create' && (await this.pathExists(change.absolutePath))) { + const backupPath = path.join(backupRoot, String(index)); + await fs.mkdir(path.dirname(backupPath), { recursive: true }); + await fs.copyFile(change.absolutePath, backupPath); + backups.push({ absolutePath: change.absolutePath, backupPath }); + } + + if (change.action === 'delete') { + await fs.rm(change.absolutePath, { force: true }); + await this.cleanupManagedParents( + path.dirname(change.absolutePath), + plan.preview.targetSkillDir + ); + continue; + } + + await fs.mkdir(path.dirname(change.absolutePath), { recursive: true }); + if (change.isBinary) { + if (!change.sourceAbsolutePath) { + throw new Error(`Missing binary source for ${change.relativePath}`); + } + await fs.copyFile(change.sourceAbsolutePath, change.absolutePath); + } else { + await fs.writeFile(change.absolutePath, change.newContent ?? '', 'utf8'); + } + + if (change.action === 'create') { + createdPaths.push(change.absolutePath); + } + } + + await this.cleanupManagedDirectories(plan.preview.targetSkillDir); + } catch (error) { + await Promise.all( + createdPaths + .slice() + .reverse() + .map(async (absolutePath) => { + await fs.rm(absolutePath, { force: true }); + await this.cleanupManagedParents( + path.dirname(absolutePath), + plan.preview.targetSkillDir + ); + }) + ); + + await Promise.all( + backups + .slice() + .reverse() + .map(async ({ absolutePath, backupPath }) => { + await fs.mkdir(path.dirname(absolutePath), { recursive: true }); + await fs.copyFile(backupPath, absolutePath); + }) + ); + + throw error; + } finally { + await fs.rm(backupRoot, { recursive: true, force: true }); + } + } + + private async buildPlan( + targetSkillDir: string, + desiredFiles: SkillPlanInputFile[], + mode: 'upsert' | 'import' + ): Promise { + const normalizedDesired = this.normalizeDesiredFiles(desiredFiles); + const [currentManagedFiles, allExistingFiles] = await Promise.all([ + this.readCurrentManagedFiles(targetSkillDir), + this.listAllRelativeFiles(targetSkillDir), + ]); + + const changesByRelativePath = new Map(); + + await Promise.all( + normalizedDesired.map(async (file) => { + const absolutePath = path.join(targetSkillDir, file.relativePath); + const existingTextContent = file.isBinary + ? null + : await this.readUtf8IfExists(absolutePath); + const action = (await this.pathExists(absolutePath)) ? 'update' : 'create'; + changesByRelativePath.set(file.relativePath, { + relativePath: file.relativePath, + absolutePath, + action, + oldContent: existingTextContent, + newContent: file.isBinary ? null : file.content, + isBinary: file.isBinary, + sourceAbsolutePath: file.isBinary ? file.sourceAbsolutePath : undefined, + }); + }) + ); + + for (const currentFile of currentManagedFiles.values()) { + if (changesByRelativePath.has(currentFile.relativePath)) { + continue; + } + + const existingTextContent = await this.readUtf8IfExists(currentFile.absolutePath); + changesByRelativePath.set(currentFile.relativePath, { + relativePath: currentFile.relativePath, + absolutePath: currentFile.absolutePath, + action: 'delete', + oldContent: existingTextContent, + newContent: null, + isBinary: false, + }); + } + + const changes = [...changesByRelativePath.values()].sort((a, b) => + a.relativePath.localeCompare(b.relativePath) + ); + const warnings = this.buildWarnings({ + changes, + currentManagedFiles, + allExistingFiles, + desiredFiles: new Set(normalizedDesired.map((file) => file.relativePath)), + mode, + }); + + const summary = changes.reduce( + (acc, change) => { + acc[`${change.action}d` as 'created' | 'updated' | 'deleted'] += 1; + if (change.isBinary) { + acc.binary += 1; + } + return acc; + }, + { created: 0, updated: 0, deleted: 0, binary: 0 } + ); + + const preview: SkillReviewPreview = { + planId: this.buildPlanId(targetSkillDir, changes, warnings), + targetSkillDir, + changes: changes.map(({ sourceAbsolutePath: _sourceAbsolutePath, ...change }) => change), + warnings, + summary, + }; + + return { preview, changes }; + } + + private normalizeDesiredFiles(files: SkillPlanInputFile[]): SkillPlanInputFile[] { + const map = new Map(); + for (const file of files) { + const normalizedPath = path.normalize(file.relativePath).replace(/\\/g, '/'); + map.set(normalizedPath, { ...file, relativePath: normalizedPath }); + } + return [...map.values()].sort((a, b) => a.relativePath.localeCompare(b.relativePath)); + } + + private async readCurrentManagedFiles( + targetSkillDir: string + ): Promise> { + const files = new Map(); + const detectedSkillFile = await this.scanner.detectSkillFile(targetSkillDir); + if (detectedSkillFile) { + files.set(path.basename(detectedSkillFile), { + relativePath: path.basename(detectedSkillFile), + absolutePath: detectedSkillFile, + }); + } + + for (const directory of MANAGED_SUBDIRECTORIES) { + const fullDirectoryPath = path.join(targetSkillDir, directory); + const relativeFiles = await this.listAllRelativeFiles(fullDirectoryPath); + for (const relativePath of relativeFiles) { + const managedRelativePath = `${directory}/${relativePath}`; + files.set(managedRelativePath, { + relativePath: managedRelativePath, + absolutePath: path.join(fullDirectoryPath, relativePath), + }); + } + } + + return files; + } + + private async listAllRelativeFiles(rootDir: string): Promise { + try { + const rootStat = await fs.stat(rootDir); + if (!rootStat.isDirectory()) { + return []; + } + } catch { + return []; + } + + const dirEntries = await fs.readdir(rootDir, { withFileTypes: true }); + const entries = await Promise.all( + dirEntries.map(async (entry) => { + const fullPath = path.join(rootDir, entry.name); + if (entry.isDirectory()) { + const children = await this.listAllRelativeFiles(fullPath); + return children.map((child) => path.join(entry.name, child).replace(/\\/g, '/')); + } + return [entry.name]; + }) + ); + + return entries.flat().sort((a, b) => a.localeCompare(b)); + } + + private buildWarnings({ + changes, + currentManagedFiles, + allExistingFiles, + desiredFiles, + mode, + }: { + changes: SkillExecutionChange[]; + currentManagedFiles: Map; + allExistingFiles: string[]; + desiredFiles: Set; + mode: 'upsert' | 'import'; + }): string[] { + const warnings: string[] = []; + const deleteCount = changes.filter((change) => change.action === 'delete').length; + const updateCount = changes.filter((change) => change.action === 'update').length; + const binaryCount = changes.filter((change) => change.isBinary).length; + + if (deleteCount > 0) { + warnings.push( + deleteCount === 1 + ? '1 managed file will be removed to match this reviewed plan.' + : `${deleteCount} managed files will be removed to match this reviewed plan.` + ); + } + + if (updateCount > 0) { + warnings.push( + updateCount === 1 + ? '1 existing file will be overwritten.' + : `${updateCount} existing files will be overwritten.` + ); + } + + if (binaryCount > 0) { + warnings.push( + binaryCount === 1 + ? '1 binary file will be copied as-is.' + : `${binaryCount} binary files will be copied as-is.` + ); + } + + const managedPaths = new Set(currentManagedFiles.keys()); + const unmanagedFiles = allExistingFiles.filter( + (relativePath) => !managedPaths.has(relativePath) && !desiredFiles.has(relativePath) + ); + if (unmanagedFiles.length > 0) { + warnings.push( + mode === 'import' + ? 'Existing files outside the imported plan will be kept as-is.' + : 'Existing files outside the managed skill set will be kept as-is.' + ); + } + + return warnings; + } + + private buildPlanId( + targetSkillDir: string, + changes: SkillExecutionChange[], + warnings: string[] + ): string { + const hash = createHash('sha256'); + hash.update(targetSkillDir); + hash.update('\n'); + for (const change of changes) { + hash.update( + JSON.stringify({ + relativePath: change.relativePath, + action: change.action, + oldContent: change.oldContent, + newContent: change.newContent, + isBinary: change.isBinary, + sourceAbsolutePath: change.sourceAbsolutePath ?? null, + }) + ); + hash.update('\n'); + } + for (const warning of warnings) { + hash.update(warning); + hash.update('\n'); + } + return hash.digest('hex'); + } + + private async readUtf8IfExists(filePath: string): Promise { + try { + return await fs.readFile(filePath, 'utf8'); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') { + return null; + } + return null; + } + } + + private async pathExists(targetPath: string): Promise { + try { + await fs.stat(targetPath); + return true; + } catch { + return false; + } + } + + private async cleanupManagedDirectories(targetSkillDir: string): Promise { + await Promise.all( + MANAGED_SUBDIRECTORIES.map((directory) => + this.cleanupManagedParents(path.join(targetSkillDir, directory), targetSkillDir) + ) + ); + } + + private async cleanupManagedParents(currentDir: string, targetSkillDir: string): Promise { + let nextDir = currentDir; + while (nextDir.startsWith(targetSkillDir) && nextDir !== targetSkillDir) { + try { + const entries = await fs.readdir(nextDir); + if (entries.length > 0) { + return; + } + await fs.rmdir(nextDir); + } catch { + return; + } + nextDir = path.dirname(nextDir); + } + } +} diff --git a/src/main/services/extensions/skills/SkillsMutationService.ts b/src/main/services/extensions/skills/SkillsMutationService.ts index a0d67541..6cf02a96 100644 --- a/src/main/services/extensions/skills/SkillsMutationService.ts +++ b/src/main/services/extensions/skills/SkillsMutationService.ts @@ -13,7 +13,7 @@ import { shell } from 'electron'; import { isPathWithinRoot, validateFileName } from '@main/utils/pathValidation'; import { SkillImportService } from './SkillImportService'; -import { SkillReviewService } from './SkillReviewService'; +import { SkillPlanService } from './SkillPlanService'; import { SkillScaffoldService } from './SkillScaffoldService'; import { SkillRootsResolver } from './SkillRootsResolver'; import { SkillsCatalogService } from './SkillsCatalogService'; @@ -24,7 +24,7 @@ export class SkillsMutationService { private readonly catalogService = new SkillsCatalogService(), private readonly scaffoldService = new SkillScaffoldService(rootsResolver), private readonly importService = new SkillImportService(), - private readonly reviewService = new SkillReviewService() + private readonly planService = new SkillPlanService() ) {} async previewUpsert(request: SkillUpsertRequest): Promise { @@ -36,15 +36,15 @@ export class SkillsMutationService { request.existingSkillId ); const files = this.scaffoldService.normalizeDraftFiles(request.files); - const changes = await this.reviewService.buildTextChanges(targetSkillDir, files); - return { - targetSkillDir, - changes, - warnings: [], - }; + const plan = await this.planService.buildUpsertPlan(targetSkillDir, files); + return plan.preview; } async applyUpsert(request: SkillUpsertRequest): Promise { + if (!request.reviewPlanId) { + throw new Error('Review the skill changes before saving.'); + } + const targetSkillDir = await this.scaffoldService.resolveUpsertTarget( request.scope, request.rootKind, @@ -53,30 +53,33 @@ export class SkillsMutationService { request.existingSkillId ); const files = this.scaffoldService.normalizeDraftFiles(request.files); - await this.scaffoldService.writeTextFiles(targetSkillDir, files); + const plan = await this.planService.buildUpsertPlan(targetSkillDir, files); + this.assertReviewedPlanMatches(request.reviewPlanId, plan.preview.planId); + await this.planService.applyPlan(plan); return this.catalogService.getDetail(targetSkillDir, request.projectPath); } async previewImport(request: SkillImportRequest): Promise { const { sourceDir, targetSkillDir } = await this.resolveImportTarget(request); - const sourceFiles = await this.importService.readSourceFiles(sourceDir); - const changes = await this.reviewService.buildImportChanges(targetSkillDir, sourceFiles); - const warnings = changes.some((change) => change.isBinary) - ? ['This import includes binary files. Binary files will be copied as-is.'] - : []; - + const inspection = await this.importService.inspectSourceDir(sourceDir); + const plan = await this.planService.buildImportPlan(targetSkillDir, inspection.files); return { - targetSkillDir, - changes, - warnings, + ...plan.preview, + warnings: [...new Set([...inspection.warnings, ...plan.preview.warnings])], }; } async applyImport(request: SkillImportRequest): Promise { + if (!request.reviewPlanId) { + throw new Error('Review the import changes before saving.'); + } + const { sourceDir, targetSkillDir } = await this.resolveImportTarget(request); - const sourceFiles = await this.importService.readSourceFiles(sourceDir); - await this.importService.writeImportedFiles(targetSkillDir, sourceFiles); + const inspection = await this.importService.inspectSourceDir(sourceDir); + const plan = await this.planService.buildImportPlan(targetSkillDir, inspection.files); + this.assertReviewedPlanMatches(request.reviewPlanId, plan.preview.planId); + await this.planService.applyPlan(plan); return this.catalogService.getDetail(targetSkillDir, request.projectPath); } @@ -133,4 +136,12 @@ export class SkillsMutationService { } return normalizedSkillDir; } + + private assertReviewedPlanMatches(reviewPlanId: string, currentPlanId: string): void { + if (reviewPlanId !== currentPlanId) { + throw new Error( + 'The skill files changed after review. Review the latest changes and try again.' + ); + } + } } diff --git a/src/renderer/components/extensions/ExtensionStoreView.tsx b/src/renderer/components/extensions/ExtensionStoreView.tsx index b3c50946..4928deb4 100644 --- a/src/renderer/components/extensions/ExtensionStoreView.tsx +++ b/src/renderer/components/extensions/ExtensionStoreView.tsx @@ -11,7 +11,7 @@ import { Button } from '@renderer/components/ui/button'; import { useTabIdOptional } from '@renderer/contexts/useTabUIContext'; import { useExtensionsTabState } from '@renderer/hooks/useExtensionsTabState'; import { useStore } from '@renderer/store'; -import { Tabs, TabsContent, TabsList, TabsTrigger } from '@renderer/components/ui/tabs'; +import { Tabs, TabsContent, TabsList } from '@renderer/components/ui/tabs'; import { Tooltip, TooltipContent, @@ -21,6 +21,7 @@ import { import { AlertTriangle, BookOpen, Info, Key, Plus, Puzzle, RefreshCw, Server } from 'lucide-react'; import { ApiKeysPanel } from './apikeys/ApiKeysPanel'; +import { ExtensionsSubTabTrigger } from './ExtensionsSubTabTrigger'; import { CustomMcpServerDialog } from './mcp/CustomMcpServerDialog'; import { McpServersPanel } from './mcp/McpServersPanel'; import { PluginsPanel } from './plugins/PluginsPanel'; @@ -57,6 +58,39 @@ export const ExtensionStoreView = (): React.JSX.Element => { () => projects.find((project) => project.id === extensionsTabProjectId)?.name ?? null, [extensionsTabProjectId, projects] ); + const subTabs = useMemo( + () => [ + { + value: 'plugins' as const, + label: 'Plugins', + icon: Puzzle, + description: + 'Small add-ons for Claude. They give the app extra features and integrations you can install when you need them.', + }, + { + value: 'mcp-servers' as const, + label: 'MCP Servers', + icon: Server, + description: + 'Connections to outside tools and apps. They let Claude read data or do actions beyond this app.', + }, + { + value: 'skills' as const, + label: 'Skills', + icon: BookOpen, + description: + 'Ready-made instructions for common jobs. They help Claude do specific tasks better and more consistently.', + }, + { + value: 'api-keys' as const, + label: 'API Keys', + icon: Key, + description: + 'Secret keys for online services. Add them here so plugins, servers, and integrations can connect and work.', + }, + ], + [] + ); // Fetch plugin catalog on mount useEffect(() => { @@ -102,15 +136,15 @@ export const ExtensionStoreView = (): React.JSX.Element => { } return ( -
-
- {/* Header */} -
-
- -

Extensions

-
- + +
+
+ {/* Header */} +
+
+ +

Extensions

+
+
- {/* Sub-tabs */} -
- {/* CLI not installed warning */} - {!cliInstalled && ( -
- - Claude CLI is required to install or uninstall extensions. Install it from Settings. -
- )} - {/* Active sessions warning */} - {hasOngoingSessions && ( -
- - Running sessions won't pick up extension changes until restarted. -
- )} - - tabState.setActiveSubTab(v as 'plugins' | 'mcp-servers' | 'skills' | 'api-keys') - } - > -
- - - - Plugins - - - - MCP Servers - - - - Skills - - - - API Keys - - - {tabState.activeSubTab === 'mcp-servers' && ( - - )} -
+ {/* Sub-tabs */} +
+ {/* CLI not installed warning */} + {!cliInstalled && ( +
+ + Claude CLI is required to install or uninstall extensions. Install it from Settings. +
+ )} + {/* Active sessions warning */} + {hasOngoingSessions && ( +
+ + Running sessions won't pick up extension changes until restarted. +
+ )} + + tabState.setActiveSubTab(v as 'plugins' | 'mcp-servers' | 'skills' | 'api-keys') + } + > +
+ + {subTabs.map((subTab) => ( + + ))} + + {tabState.activeSubTab === 'mcp-servers' && ( + + )} +
- - - + + + - - - + + + - - - + + + - - - - + + + + - {/* Custom MCP server dialog (lifted to store view level) */} - setCustomMcpDialogOpen(false)} - /> + {/* Custom MCP server dialog (lifted to store view level) */} + setCustomMcpDialogOpen(false)} + /> +
-
+ ); }; diff --git a/src/renderer/components/extensions/ExtensionsSubTabTrigger.tsx b/src/renderer/components/extensions/ExtensionsSubTabTrigger.tsx new file mode 100644 index 00000000..6df3b7da --- /dev/null +++ b/src/renderer/components/extensions/ExtensionsSubTabTrigger.tsx @@ -0,0 +1,53 @@ +import type { LucideIcon } from 'lucide-react'; + +import { TabsTrigger } from '@renderer/components/ui/tabs'; +import { Tooltip, TooltipContent, TooltipTrigger } from '@renderer/components/ui/tooltip'; + +import { Info } from 'lucide-react'; + +interface ExtensionsSubTabTriggerProps { + value: 'plugins' | 'mcp-servers' | 'skills' | 'api-keys'; + label: string; + description: string; + icon: LucideIcon; +} + +export const ExtensionsSubTabTrigger = ({ + value, + label, + description, + icon: Icon, +}: ExtensionsSubTabTriggerProps): React.JSX.Element => { + return ( + + + {label} + + + + event.stopPropagation()} + onMouseDown={(event) => event.stopPropagation()} + onKeyDown={(event) => { + if (event.key === 'Enter' || event.key === ' ') { + event.stopPropagation(); + } + }} + className="size-4.5 absolute right-2 top-1 z-10 inline-flex items-center justify-center rounded-full text-text-muted transition-colors hover:bg-[var(--color-surface)] hover:text-text" + > + + + + + {description} + + + + ); +}; diff --git a/src/renderer/components/extensions/plugins/PluginCard.tsx b/src/renderer/components/extensions/plugins/PluginCard.tsx index 33517bc6..b1649255 100644 --- a/src/renderer/components/extensions/plugins/PluginCard.tsx +++ b/src/renderer/components/extensions/plugins/PluginCard.tsx @@ -44,10 +44,10 @@ export const PluginCard = ({ plugin, index, onClick }: PluginCardProps): React.J onClick(plugin.pluginId); } }} - className={`hover:bg-surface-raised/45 relative flex w-full cursor-pointer flex-col gap-3 rounded-xl border p-4 text-left transition-all duration-200 hover:border-border-emphasis hover:shadow-[0_0_12px_rgba(255,255,255,0.02)] focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-[var(--color-border-emphasis)] ${ - baseStriped ? 'bg-surface-raised/10' : 'bg-transparent' - } ${smStriped ? 'sm:bg-surface-raised/10' : 'sm:bg-transparent'} ${ - xlStriped ? 'xl:bg-surface-raised/10' : 'xl:bg-transparent' + className={`relative flex w-full cursor-pointer flex-col gap-3 rounded-xl border p-4 text-left transition-all duration-200 hover:border-border-emphasis hover:bg-white/[0.06] hover:shadow-[0_0_12px_rgba(255,255,255,0.02)] focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-[var(--color-border-emphasis)] ${ + baseStriped ? 'bg-white/[0.045]' : 'bg-white/[0.015]' + } ${smStriped ? 'sm:bg-white/[0.045]' : 'sm:bg-white/[0.015]'} ${ + xlStriped ? 'xl:bg-white/[0.045]' : 'xl:bg-white/[0.015]' } ${ plugin.isInstalled ? 'border-l-2 border-border border-l-emerald-500/35' : 'border-border' }`} diff --git a/src/renderer/components/extensions/skills/SkillCodeEditor.tsx b/src/renderer/components/extensions/skills/SkillCodeEditor.tsx index 776d8a1f..875cf1a3 100644 --- a/src/renderer/components/extensions/skills/SkillCodeEditor.tsx +++ b/src/renderer/components/extensions/skills/SkillCodeEditor.tsx @@ -30,9 +30,16 @@ const skillEditorTheme = EditorView.theme({ interface SkillCodeEditorProps { value: string; onChange: (value: string) => void; + scrollRef?: React.RefObject; + onScroll?: () => void; } -export const SkillCodeEditor = ({ value, onChange }: SkillCodeEditorProps): React.JSX.Element => { +export const SkillCodeEditor = ({ + value, + onChange, + scrollRef, + onScroll, +}: SkillCodeEditorProps): React.JSX.Element => { const containerRef = useRef(null); const viewRef = useRef(null); const onChangeRef = useRef(onChange); @@ -72,13 +79,27 @@ export const SkillCodeEditor = ({ value, onChange }: SkillCodeEditorProps): Reac }); viewRef.current = view; + if (onScroll) { + view.scrollDOM.addEventListener('scroll', onScroll, { passive: true }); + } + if (scrollRef && 'current' in scrollRef) { + const mutableRef = scrollRef as React.MutableRefObject; + mutableRef.current = view.scrollDOM; + } return () => { + if (onScroll) { + view.scrollDOM.removeEventListener('scroll', onScroll); + } + if (scrollRef && 'current' in scrollRef) { + const mutableRef = scrollRef as React.MutableRefObject; + mutableRef.current = null; + } view.destroy(); viewRef.current = null; }; // eslint-disable-next-line react-hooks/exhaustive-deps -- create editor once per mount - }, []); + }, [onScroll, scrollRef]); useEffect(() => { const view = viewRef.current; diff --git a/src/renderer/components/extensions/skills/SkillDetailDialog.tsx b/src/renderer/components/extensions/skills/SkillDetailDialog.tsx index d5327500..a6f8741e 100644 --- a/src/renderer/components/extensions/skills/SkillDetailDialog.tsx +++ b/src/renderer/components/extensions/skills/SkillDetailDialog.tsx @@ -1,8 +1,18 @@ -import { useEffect } from 'react'; +import { useEffect, useState } from 'react'; import { api } from '@renderer/api'; import { CodeBlockViewer } from '@renderer/components/chat/viewers/CodeBlockViewer'; import { MarkdownViewer } from '@renderer/components/chat/viewers/MarkdownViewer'; +import { + AlertDialog, + AlertDialogAction, + AlertDialogCancel, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, +} from '@renderer/components/ui/alert-dialog'; import { Badge } from '@renderer/components/ui/badge'; import { Button } from '@renderer/components/ui/button'; import { @@ -34,18 +44,29 @@ export const SkillDetailDialog = ({ }: SkillDetailDialogProps): React.JSX.Element => { const fetchSkillDetail = useStore((s) => s.fetchSkillDetail); const deleteSkill = useStore((s) => s.deleteSkill); - const skillsMutationLoading = useStore((s) => s.skillsMutationLoading); const detail = useStore((s) => (skillId ? s.skillsDetailsById[skillId] : undefined)); const loading = useStore((s) => skillId ? (s.skillsDetailLoadingById[skillId] ?? false) : false ); + const detailError = useStore((s) => + skillId ? (s.skillsDetailErrorById[skillId] ?? null) : null + ); + const [deleteLoading, setDeleteLoading] = useState(false); + const [deleteError, setDeleteError] = useState(null); + const [deleteConfirmOpen, setDeleteConfirmOpen] = useState(false); useEffect(() => { if (!open || !skillId) return; - if (detail === undefined) { - void fetchSkillDetail(skillId, projectPath ?? undefined); + void fetchSkillDetail(skillId, projectPath ?? undefined).catch(() => undefined); + }, [fetchSkillDetail, open, projectPath, skillId]); + + useEffect(() => { + if (!open) { + setDeleteError(null); + setDeleteLoading(false); + setDeleteConfirmOpen(false); } - }, [detail, fetchSkillDetail, open, projectPath, skillId]); + }, [open]); const item = detail?.item; @@ -53,16 +74,32 @@ export const SkillDetailDialog = ({ return `.${rootKind}`; } + function formatScopeLabel(scope: 'user' | 'project'): string { + return scope === 'project' ? 'This project only' : 'Your personal skills'; + } + + function formatInvocationLabel(invocationMode: 'auto' | 'manual-only'): string { + return invocationMode === 'manual-only' + ? 'Claude will only use this when you explicitly ask for it.' + : 'Claude can pick this automatically when it matches the task.'; + } + async function handleDelete(): Promise { if (!item) return; - const confirmed = window.confirm(`Delete skill "${item.name}"? It will be moved to Trash.`); - if (!confirmed) return; - - await deleteSkill({ - skillId: item.id, - projectPath: projectPath ?? undefined, - }); - onDeleted(); + setDeleteLoading(true); + setDeleteError(null); + try { + await deleteSkill({ + skillId: item.id, + projectPath: projectPath ?? undefined, + }); + setDeleteConfirmOpen(false); + onDeleted(); + } catch (error) { + setDeleteError(error instanceof Error ? error.message : 'Failed to delete skill'); + } finally { + setDeleteLoading(false); + } } return ( @@ -79,7 +116,24 @@ export const SkillDetailDialog = ({

Loading skill details...

)} - {!loading && detail === null && ( + {!loading && detailError && ( +
+

{detailError}

+ {skillId && ( + + )} +
+ )} + + {!loading && !detailError && detail === null && (
Unable to load this skill.
@@ -87,17 +141,27 @@ export const SkillDetailDialog = ({ {!loading && detail && item && (
+ {deleteError && ( +
+ {deleteError} +
+ )}
- {item.scope} - {formatRootKind(item.rootKind)} - {item.invocationMode} - {item.flags.hasScripts && scripts} - {item.flags.hasReferences && references} - {item.flags.hasAssets && assets} + {formatScopeLabel(item.scope)} + Stored in {formatRootKind(item.rootKind)} + + {item.invocationMode === 'manual-only' ? 'Manual use' : 'Auto use'} + + {item.flags.hasScripts && Has scripts} + {item.flags.hasReferences && References} + {item.flags.hasAssets && Assets}
{item.issues.length > 0 && (
+

+ Review this skill carefully before using it +

{item.issues.map((issue, index) => (
)} +
+
+

+ Who can use it +

+

{formatScopeLabel(item.scope)}

+
+
+

+ How Claude uses it +

+

{formatInvocationLabel(item.invocationMode)}

+
+
+

+ What comes with it +

+

+ {[ + item.flags.hasReferences ? 'references' : null, + item.flags.hasScripts ? 'scripts' : null, + item.flags.hasAssets ? 'assets' : null, + ] + .filter(Boolean) + .join(', ') || 'Just the skill instructions'} +

+
+
+
- -
@@ -153,15 +230,9 @@ export const SkillDetailDialog = ({
- -
-

Path

+

Stored at

{item.skillDir}

@@ -198,11 +269,61 @@ export const SkillDetailDialog = ({
)}
+ +
+ + Advanced file details + +
+
+ + +
+ +
+
)} + + + + + Delete skill? + + {item + ? `Delete "${item.name}" and move it to Trash? You can restore it later from Trash if needed.` + : 'Delete this skill and move it to Trash?'} + + + + Cancel + void handleDelete()} disabled={deleteLoading}> + {deleteLoading ? 'Deleting...' : 'Delete Skill'} + + + + ); }; diff --git a/src/renderer/components/extensions/skills/SkillEditorDialog.tsx b/src/renderer/components/extensions/skills/SkillEditorDialog.tsx index ecccd3d0..8e66ac6c 100644 --- a/src/renderer/components/extensions/skills/SkillEditorDialog.tsx +++ b/src/renderer/components/extensions/skills/SkillEditorDialog.tsx @@ -8,7 +8,6 @@ import { Dialog, DialogContent, DialogDescription, - DialogFooter, DialogHeader, DialogTitle, } from '@renderer/components/ui/dialog'; @@ -21,6 +20,8 @@ import { SelectTrigger, SelectValue, } from '@renderer/components/ui/select'; +import { Textarea } from '@renderer/components/ui/textarea'; +import { useMarkdownScrollSync } from '@renderer/hooks/useMarkdownScrollSync'; import { useStore } from '@renderer/store'; import { FileSearch, RotateCcw, X } from 'lucide-react'; @@ -29,12 +30,11 @@ import { SkillReviewDialog } from './SkillReviewDialog'; import { buildSkillDraftFiles, buildSkillTemplate, - readSkillTemplateInput, + readSkillTemplateContent, updateSkillTemplateFrontmatter, } from './skillDraftUtils'; import type { - SkillCatalogItem, SkillDetail, SkillInvocationMode, SkillReviewPreview, @@ -60,6 +60,16 @@ function parseInitialDescription(detail: SkillDetail | null): string { return detail?.item.description ?? ''; } +function toSuggestedFolderName(value: string): string { + return value + .normalize('NFKD') + .replace(/[^\x00-\x7F]/g, '') + .toLowerCase() + .replace(/[^a-z0-9]+/g, '-') + .replace(/^-+|-+$/g, '') + .slice(0, 80); +} + export const SkillEditorDialog = ({ open, mode, @@ -70,11 +80,10 @@ export const SkillEditorDialog = ({ onSaved, }: SkillEditorDialogProps): React.JSX.Element => { const containerRef = useRef(null); + const editorScrollRef = useRef(null); const rawContentRef = useRef(''); const previewSkillUpsert = useStore((s) => s.previewSkillUpsert); const applySkillUpsert = useStore((s) => s.applySkillUpsert); - const skillsMutationLoading = useStore((s) => s.skillsMutationLoading); - const skillsMutationError = useStore((s) => s.skillsMutationError); const [scope, setScope] = useState<'user' | 'project'>('user'); const [rootKind, setRootKind] = useState<'claude' | 'cursor' | 'agents'>('claude'); @@ -84,17 +93,31 @@ export const SkillEditorDialog = ({ const [license, setLicense] = useState(''); const [compatibility, setCompatibility] = useState(''); const [invocationMode, setInvocationMode] = useState('auto'); + const [whenToUse, setWhenToUse] = useState(''); + const [steps, setSteps] = useState(''); + const [notes, setNotes] = useState(''); const [includeScripts, setIncludeScripts] = useState(false); const [includeReferences, setIncludeReferences] = useState(false); const [includeAssets, setIncludeAssets] = useState(false); const [rawContent, setRawContent] = useState(''); + const [folderNameEdited, setFolderNameEdited] = useState(false); + const [customMarkdownDetected, setCustomMarkdownDetected] = useState(false); const [manualRawEdit, setManualRawEdit] = useState(false); + const [showAdvancedEditor, setShowAdvancedEditor] = useState(false); const [splitRatio, setSplitRatio] = useState(0.52); const [isResizing, setIsResizing] = useState(false); const [reviewPreview, setReviewPreview] = useState(null); const [reviewOpen, setReviewOpen] = useState(false); + const [reviewLoading, setReviewLoading] = useState(false); + const [saveLoading, setSaveLoading] = useState(false); + const [mutationError, setMutationError] = useState(null); + const scrollSync = useMarkdownScrollSync( + showAdvancedEditor, + detail?.item.id ?? (mode === 'create' ? 'create-skill' : 'edit-skill'), + { editorScrollRef } + ); - const applyMetadataToRawContent = useCallback( + const applyFormToRawContent = useCallback( ( nextValues: Partial<{ name: string; @@ -102,6 +125,9 @@ export const SkillEditorDialog = ({ license: string; compatibility: string; invocationMode: SkillInvocationMode; + whenToUse: string; + steps: string; + notes: string; }> ) => { const merged = { @@ -110,17 +136,31 @@ export const SkillEditorDialog = ({ license, compatibility, invocationMode, + whenToUse, + steps, + notes, ...nextValues, }; const nextRawContent = - mode === 'create' && !manualRawEdit + !manualRawEdit && !customMarkdownDetected ? buildSkillTemplate(merged) : updateSkillTemplateFrontmatter(rawContentRef.current, merged); rawContentRef.current = nextRawContent; setRawContent(nextRawContent); }, - [compatibility, description, invocationMode, license, manualRawEdit, mode, name] + [ + compatibility, + description, + invocationMode, + license, + manualRawEdit, + customMarkdownDetected, + name, + notes, + steps, + whenToUse, + ] ); useEffect(() => { @@ -135,6 +175,9 @@ export const SkillEditorDialog = ({ const nextLicense = item?.license ?? ''; const nextCompatibility = item?.compatibility ?? ''; const nextInvocationMode = item?.invocationMode ?? 'auto'; + const nextWhenToUse = 'Use this skill when the task matches these conditions.'; + const nextSteps = '1. Describe the first step.\n2. Describe the second step.'; + const nextNotes = '- Add caveats, review rules, or references.'; const nextRawContent = detail?.rawContent ?? buildSkillTemplate({ @@ -143,12 +186,18 @@ export const SkillEditorDialog = ({ license: nextLicense, compatibility: nextCompatibility, invocationMode: nextInvocationMode, + whenToUse: nextWhenToUse, + steps: nextSteps, + notes: nextNotes, }); - const rawInput = readSkillTemplateInput(nextRawContent); + const rawInput = readSkillTemplateContent(nextRawContent); + const suggestedFolderName = toSuggestedFolderName(nextName || 'New Skill'); + const hasCustomMarkdown = mode === 'edit' && rawInput.hasUnstructuredBody; setScope(nextScope); setRootKind(nextRootKind); - setFolderName(nextFolderName || nextName || ''); + setFolderName(nextFolderName || suggestedFolderName || nextName || ''); + setFolderNameEdited(Boolean(item?.folderName)); setName(rawInput.name || nextName || 'New Skill'); setDescription( rawInput.description || nextDescription || 'Describe what this skill helps with.' @@ -156,14 +205,26 @@ export const SkillEditorDialog = ({ setLicense(rawInput.license ?? nextLicense); setCompatibility(rawInput.compatibility ?? nextCompatibility); setInvocationMode(rawInput.invocationMode ?? nextInvocationMode); + setWhenToUse( + hasCustomMarkdown + ? (rawInput.bodyMarkdown ?? nextRawContent) + : (rawInput.whenToUse ?? nextWhenToUse) + ); + setSteps(hasCustomMarkdown ? '' : (rawInput.steps ?? nextSteps)); + setNotes(hasCustomMarkdown ? '' : (rawInput.notes ?? nextNotes)); setIncludeScripts(item?.flags.hasScripts ?? false); setIncludeReferences(item?.flags.hasReferences ?? false); setIncludeAssets(item?.flags.hasAssets ?? false); + setCustomMarkdownDetected(hasCustomMarkdown); rawContentRef.current = nextRawContent; setRawContent(nextRawContent); setManualRawEdit(false); + setShowAdvancedEditor(hasCustomMarkdown); setReviewPreview(null); setReviewOpen(false); + setReviewLoading(false); + setSaveLoading(false); + setMutationError(null); }, [detail, mode, open, projectPath]); useEffect(() => { @@ -207,11 +268,28 @@ export const SkillEditorDialog = ({ ); const canUseProjectScope = Boolean(projectPath); + const instructionsLocked = manualRawEdit || customMarkdownDetected; const title = mode === 'create' ? 'Create skill' : 'Edit skill'; const descriptionText = mode === 'create' - ? 'Draft a new local skill, review the filesystem changes, then save it into a supported skill root.' - : 'Update the selected skill and review the resulting file changes before saving.'; + ? 'Describe the workflow in plain language, review the files that will be created, then save it.' + : 'Update this skill, review the resulting file changes, then save it.'; + + function validateBeforeReview(): string | null { + if (!name.trim()) { + return 'Add a skill name so people know what this workflow is for.'; + } + if (!description.trim()) { + return 'Add a short description so it is clear what this skill helps with.'; + } + if (!folderName.trim()) { + return 'Choose a folder name for this skill.'; + } + if (scope === 'project' && !projectPath) { + return 'Project skills need an active project.'; + } + return null; + } const handleMouseMove = useCallback((event: MouseEvent): void => { const container = containerRef.current; @@ -242,16 +320,40 @@ export const SkillEditorDialog = ({ }, [handleMouseMove, handleMouseUp, isResizing]); async function handleReview(): Promise { - const preview = await previewSkillUpsert(request); - setReviewPreview(preview); - setReviewOpen(true); + const validationError = validateBeforeReview(); + if (validationError) { + setMutationError(validationError); + return; + } + setReviewLoading(true); + setMutationError(null); + try { + const preview = await previewSkillUpsert(request); + setReviewPreview(preview); + setReviewOpen(true); + } catch (error) { + setMutationError(error instanceof Error ? error.message : 'Failed to review skill changes'); + } finally { + setReviewLoading(false); + } } async function handleConfirmSave(): Promise { - const saved = await applySkillUpsert(request); - setReviewOpen(false); - onSaved(saved?.item.id ?? detail?.item.id ?? null); - onClose(); + setSaveLoading(true); + setMutationError(null); + try { + const saved = await applySkillUpsert({ + ...request, + reviewPlanId: reviewPreview?.planId, + }); + setReviewOpen(false); + onSaved(saved?.item.id ?? detail?.item.id ?? null); + onClose(); + } catch (error) { + setMutationError(error instanceof Error ? error.message : 'Failed to save skill'); + } finally { + setSaveLoading(false); + } } return ( @@ -266,9 +368,17 @@ export const SkillEditorDialog = ({
+
+

1. Basics

+

+ Give this skill a clear name, choose who can use it, and decide where it should + live. +

+
+
- + @@ -313,27 +423,36 @@ export const SkillEditorDialog = ({ setFolderName(event.target.value)} + onChange={(event) => { + setFolderNameEdited(true); + setFolderName(event.target.value); + }} disabled={mode === 'edit'} /> + {mode === 'create' && ( +

+ We suggest this automatically from the skill name so review works right + away. +

+ )}
- +
@@ -348,7 +467,10 @@ export const SkillEditorDialog = ({ onChange={(event) => { const nextValue = event.target.value; setName(nextValue); - applyMetadataToRawContent({ name: nextValue }); + if (mode === 'create' && !folderNameEdited) { + setFolderName(toSuggestedFolderName(nextValue || 'New Skill')); + } + applyFormToRawContent({ name: nextValue }); }} placeholder="Write concise skill name" /> @@ -361,7 +483,7 @@ export const SkillEditorDialog = ({ onChange={(event) => { const nextValue = event.target.value; setLicense(nextValue); - applyMetadataToRawContent({ license: nextValue }); + applyFormToRawContent({ license: nextValue }); }} placeholder="MIT" /> @@ -377,7 +499,7 @@ export const SkillEditorDialog = ({ onChange={(event) => { const nextValue = event.target.value; setDescription(nextValue); - applyMetadataToRawContent({ description: nextValue }); + applyFormToRawContent({ description: nextValue }); }} placeholder="What this skill helps with" /> @@ -390,13 +512,90 @@ export const SkillEditorDialog = ({ onChange={(event) => { const nextValue = event.target.value; setCompatibility(nextValue); - applyMetadataToRawContent({ compatibility: nextValue }); + applyFormToRawContent({ compatibility: nextValue }); }} placeholder="claude-code, cursor" />
+ {!customMarkdownDetected && ( + <> +
+

2. Instructions

+

+ These sections generate the skill file for you, so you do not need to edit + markdown unless you want to. +

+
+ +
+
+ +