From b35fcdae41ca8e4c34dfe958944b62af72c7fec8 Mon Sep 17 00:00:00 2001 From: Santos Date: Sat, 1 Aug 2026 17:35:17 +0800 Subject: [PATCH] fix(import): separate type advice from variant identity --- CHANGELOG.md | 1 + .../desktop/src/App.import-candidates.test.js | 100 ++++++++++++------ apps/desktop/src/cardLayout.test.js | 6 ++ apps/desktop/src/components/importReview.jsx | 60 +++++++---- apps/desktop/src/importCandidates.js | 57 ++++++++-- apps/desktop/src/previewData.js | 73 +++++++------ apps/desktop/src/styles.css | 11 ++ crates/skillbox-core/src/import.rs | 27 ++++- crates/skillbox-core/src/tests.rs | 72 +++++++++---- crates/skillbox-core/src/types.rs | 5 + docs/architecture.md | 4 +- docs/implementation-status.md | 2 +- docs/workflows.md | 8 +- 13 files changed, 299 insertions(+), 127 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ca0674..a8d5892 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ version tags such as `v0.3.0`. ## Unreleased - Group Import Review candidates by normalized skill name, keep equivalent copies as locations of one Rust-qualified variant, require explicit selection when same-name variants differ, and submit at most one source per skill. +- Keep mixed User/Remote import suggestions inside one content-equivalent variant and require an explicit classification choice instead of manufacturing duplicate variants. - Add explicit inbound user-skills Git updates through Check remote, reviewed repository-wide diffs, and preview-confirmed fast-forward apply. diff --git a/apps/desktop/src/App.import-candidates.test.js b/apps/desktop/src/App.import-candidates.test.js index 9fe7818..9cb91eb 100644 --- a/apps/desktop/src/App.import-candidates.test.js +++ b/apps/desktop/src/App.import-candidates.test.js @@ -13,6 +13,7 @@ import { selectedImportCandidates, selectImportCandidateVariant, toggleImportCandidateGroupSelection, + updateImportCandidateGroupType, visibleImportCandidates, workspaceSkillTabs } from './importCandidates.js'; @@ -150,64 +151,93 @@ test('normalizes grouped import candidate source paths', () => { ]); }); -test('normalizes Rust-owned import groups without inferring variant equivalence', () => { +test('normalizes Rust-owned mixed type suggestions without splitting one content variant', () => { const group = normalizeImportCandidateGroup({ - id: 'skill-hyperframes', - name: 'hyperframes', + id: 'skill-general-video', + name: 'general-video', description: 'Create product videos.', usage_count: 8, - requires_review: true, - selected_variant_id: null, + requires_review: false, + selected_variant_id: 'variant-shared', variants: [ { - id: 'variant-user', + id: 'variant-shared', + requires_type_review: true, + selected_type: null, + suggested_types: ['user', 'remote'], candidate: { - name: 'hyperframes', - source_path: '/Users/example/.agents/skills/hyperframes', + name: 'general-video', + source_path: '/Users/example/.agents/skills/general-video', suggested_type: 'user', - import_status: 'importable' + import_status: 'importable', + is_selected: false }, locations: [ { - source_path: '/Users/example/.agents/skills/hyperframes', - real_path: '/Users/example/src/hyperframes', - is_symlink: true, - symlink_target_path: '/Users/example/src/hyperframes' + source_path: '/Users/example/.agents/skills/general-video', + real_path: '/Users/example/.agents/skills/general-video', + suggested_type: 'user', + suggestion_reason: 'inside ~/.agents/skills' + }, + { + source_path: '/Users/example/.claude/skills/general-video', + real_path: '/Users/example/.claude/skills/general-video', + suggested_type: 'user', + suggestion_reason: 'Needs confirm' }, { - source_path: '/Users/example/.cursor/skills/hyperframes', - real_path: '/Users/example/src/hyperframes', + source_path: '/Users/example/.cursor/skills/general-video', + real_path: '/Users/example/.claude/skills/general-video', is_symlink: true, - symlink_target_path: '/Users/example/src/hyperframes' - } - ] - }, - { - id: 'variant-remote', - candidate: { - name: 'hyperframes', - source_path: '/Users/example/.codex/skills/hyperframes', - suggested_type: 'remote', - import_status: 'importable' - }, - locations: [ + symlink_target_path: '/Users/example/.claude/skills/general-video', + suggested_type: 'user', + suggestion_reason: 'Needs confirm' + }, { - source_path: '/Users/example/.codex/skills/hyperframes', - real_path: '/Users/example/src/hyperframes', + source_path: '/Users/example/.codex/skills/general-video', + real_path: '/Users/example/.claude/skills/general-video', is_symlink: true, - symlink_target_path: '/Users/example/src/hyperframes' + symlink_target_path: '/Users/example/.claude/skills/general-video', + suggested_type: 'remote', + suggestion_reason: 'inside ~/.codex/skills' } ] } ] }); - assert.equal(group.variants.length, 2); - assert.equal(importCandidateGroupLocationCount(group), 3); - assert.equal(group.requiresReview, true); - assert.equal(group.selectedVariantId, null); + assert.equal(group.variants.length, 1); + assert.equal(importCandidateGroupLocationCount(group), 4); + assert.equal(group.requiresReview, false); + assert.equal(group.selectedVariantId, 'variant-shared'); + assert.equal(group.variants[0].requiresTypeReview, true); + assert.equal(group.variants[0].selectedType, null); + assert.deepEqual(group.variants[0].locations.map((location) => location.sourcePath), [ + '/Users/example/.agents/skills/general-video', + '/Users/example/.claude/skills/general-video', + '/Users/example/.cursor/skills/general-video', + '/Users/example/.codex/skills/general-video' + ]); + assert.deepEqual( + group.variants[0].locations + .filter((location) => location.isSymlink) + .map((location) => location.symlinkTargetPath), + [ + '/Users/example/.claude/skills/general-video', + '/Users/example/.claude/skills/general-video' + ] + ); assert.equal(group.isSelected, false); assert.deepEqual(selectedImportCandidates([group]), []); + + const classified = updateImportCandidateGroupType([group], group.id, 'remote'); + assert.deepEqual(importRequestItems(selectedImportCandidates(classified)), [ + { + source_path: '/Users/example/.agents/skills/general-video', + skill_type: 'remote', + deploy_back_to_source: true + } + ]); }); test('variant review requires an explicit Rust variant choice and submits one primary', () => { diff --git a/apps/desktop/src/cardLayout.test.js b/apps/desktop/src/cardLayout.test.js index 63d7377..39a400f 100644 --- a/apps/desktop/src/cardLayout.test.js +++ b/apps/desktop/src/cardLayout.test.js @@ -442,9 +442,15 @@ test('import candidate groups disclose locations and use radio variant selection assert.match(groupSource, /aria-expanded=\{expanded\}/); assert.match(groupSource, /Found in \{locationCount\}/); assert.match(groupSource, /type="radio"/); + assert.match(groupSource, /group\.variants\.length > 1/); assert.match(groupSource, /name=\{`\$\{group\.id\}-variant`\}/); assert.match(groupSource, /onSelectVariant\(group, variant\)/); assert.match(groupSource, /Source: \{compactPath\(location\.symlinkTargetPath \|\| location\.realPath\)\}/); + assert.match(groupSource, /Mixed type suggestions/); + assert.match(groupSource, /Choose User or Remote before importing this skill/); + assert.match(groupSource, /selectedVariant\?\.selectedType === 'user'/); + assert.match(groupSource, /selectedVariant\?\.selectedType === 'remote'/); + assert.match(groupSource, /disabled=\{!canClassifyImportCandidateGroup\(group\)\}/); assert.match(disclosureRule, /display:\s*inline-flex;/); assert.match(locationRule, /grid-template-columns:\s*96px minmax\(0,\s*1fr\);/); }); diff --git a/apps/desktop/src/components/importReview.jsx b/apps/desktop/src/components/importReview.jsx index b547937..25e5fee 100644 --- a/apps/desktop/src/components/importReview.jsx +++ b/apps/desktop/src/components/importReview.jsx @@ -3,13 +3,15 @@ import { ChevronDown, MapPin, Search } from 'lucide-react'; import codexAppIcon from '../assets/codex-app-icon.png'; import codexCliIcon from '../assets/codex-cli-icon.png'; import { + canClassifyImportCandidateGroup, filterImportCandidateGroups, filterImportCandidateGroupsByQuery, importCandidateGroupLocationCount, importCandidateGroupStatus, importCandidateGroupTabs, isSelectableImportCandidateGroup, - selectedImportCandidate + selectedImportCandidate, + selectedImportCandidateVariant } from '../importCandidates.js'; import { candidateImportSourcePaths, @@ -352,6 +354,10 @@ function WorkspaceSkillTabs({ activeTab, tabs, onTabChange }) { function CandidateGroupCard({ group, onSelectVariant, onToggleSelected, onTypeChange }) { const [expanded, setExpanded] = useState(false); const candidate = selectedImportCandidate(group) || group.variants[0]?.candidate; + const selectedVariant = selectedImportCandidateVariant(group); + const needsTypeChoice = Boolean( + selectedVariant?.requiresTypeReview && !selectedVariant.selectedType + ); const status = importCandidateGroupStatus(group); const locationCount = importCandidateGroupLocationCount(group); const disclosureId = `${group.id}-locations`; @@ -378,8 +384,12 @@ function CandidateGroupCard({ group, onSelectVariant, onToggleSelected, onTypeCh {status.system ? System : null} {status.conflict ? Conflict : null} {group.requiresReview && !group.selectedVariantId ? Needs review : null} + {needsTypeChoice ? Mixed type suggestions : null} {group.description || 'No description in SKILL.md'} + {needsTypeChoice ? ( +

Choose User or Remote before importing this skill.

+ ) : null}