feat(cli): support batch skill imports from skill dirs and parent fol…#230
feat(cli): support batch skill imports from skill dirs and parent fol…#230Fascinax wants to merge 16 commits intoPackmindHub:mainfrom
Conversation
…ders - Allow multiple positional paths for \skills add\ (skill dir, file inside skill, or parent dir) - Recursively discover and deduplicate skill directories from parent folders - Add interactive confirmation prompt when importing 2+ skills (TTY only) - Show batch summary and improved error handling for empty/invalid paths - Update documentation and add comprehensive unit tests for new behaviors
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
Hi @Fascinax, thanks for your contribution :-) |
Greptile SummaryThis PR extends Confidence Score: 4/5Safe to merge; no data-loss or security risk, and both previously flagged issues are resolved. The two new comments are P1 UX/diagnostic improvements rather than crash-blocking bugs. Both prior review concerns are addressed in this revision. The two new P1 comments describe edge-case UX degradation (confusing error message for a non-skill file path; missing summary line on cancel-with-failures), not crashes or data loss. Core happy-path and failure-path test coverage is solid. apps/cli/src/application/utils/resolveSkillInputPaths.ts — the fallback return in resolveSkillDirectoryRoot when no enclosing skill directory is found. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[packmind-cli skills add path1 path2...] --> B{skillPaths.length == 0?}
B -- Yes --> C[logError: Missing skill path\nexit 1]
B -- No --> D[resolveSkillPathsForUpload]
D --> E[resolveSkillInputPaths all paths]
E -- EACCES --> F[Fallback: resolve each path individually]
F --> G{Per-path EACCES?}
G -- Yes --> H[Add to resolutionFailures]
G -- No --> I[Add to resolvedSkillPaths]
E -- OK --> J[resolvedSkillPaths collected]
F --> J
J --> K[logResolutionFailures]
K --> L{resolvedSkillPaths empty?}
L -- Yes & failures --> M[exit 1]
L -- Yes & no failures --> N[logError: No skill dirs found\nexit 1]
L -- No --> O{count > 1 AND isTTY?}
O -- No --> Q[Upload skills sequentially]
O -- Yes --> P[confirmBatchUpload prompt]
P -- n/no --> R[logConsole: Import cancelled\nif failures: exit 1]
P -- y/yes --> Q
Q --> S{Upload each skillPath}
S -- success --> T[logUploadResult\nsuccessCount++]
S -- error --> U[logError\nfailureCount++]
T --> V[Next path]
U --> V
V --> W{More paths?}
W -- Yes --> S
W -- No --> X{count > 1 OR failureCount > 0?}
X -- Yes --> Y[logUploadSummary]
X -- No --> Z[Done]
Y --> AA{failureCount > 0?}
AA -- Yes --> BB[exit 1]
AA -- No --> Z
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…uard The 'No skill directories found' guard in addSkillHandler was unreachable because resolveSkillInputPaths always returned at least one path - it fell back to [skillDirectoryRoot] even for directories with no nested skills. Now resolveSkillInputPaths returns an empty array when no skill directories are discovered, making the guard reachable and the error message visible to users instead of a generic API error.
confirmBatchUpload was leaving the Promise unresolvable if stdin emitted 'close' or 'end' before a 'data' event (e.g., Ctrl+D). Now both listeners are registered and cleaned up properly: onData resolves true/false based on user input, while onClose resolves false when stdin closes, ensuring the Promise always settles.
Change nested describe blocks in AddSkillCommand.spec.ts from 'and...' to 'with...' prefix to match the project's testing standard which requires 'when...', 'with...' context blocks for better readability and consistency with other test files in the codebase.
…improve error handling
…b.com/Fascinax/packmind into batch-skill-import-parent-directories
…an up import formatting in AddSkillCommand
merge conflict didnt saved the first time
|
Hello @Fascinax, It turns out we're working actively on the CLI and are introducing a new CLI With that in mind, two things come up:
This mean we can't unfortunately accept your contribution right now, sorry. But we'll definitively integrate your initial idea very soon! |
Alright then, I understand ! Is it still possible to contribute by answering the issue then ? |
I'd recommand to wait few days, let's middle of next week, so that we stabilize the |
Okay ! I will wait till next week then. Thanks for your answer :) |
2 participants
Explanation
packmind-cli skills addnow accepts multiple paths at once. Each path can be a skill directory (containsSKILL.md), a file inside a skill directory, or a parent directory containing several skill directories.The CLI discovers all matching skill directories, deduplicates them, uploads them sequentially, and prints a summary. An interactive confirmation prompt is shown before bulk-importing 2+ skills (skipped in non-TTY/CI environments).
I also added support to ignore specific directories in skill path resolution.
Type of Change
Affected Components
packmind-cliA quick look of the new beahviour :
Testing
Test Details:
resolveSkillInputPaths.spec.ts— direct skill dir, parent dir expansion, file-inside-skill resolution, deduplicationTODO List
Reviewer Notes
strip-ansiandansi-regextotransformIgnorePatterns(ESM modules not handled by default Jest transform)PackmindCliHexaFactoryis mocked in AddSkillCommand.spec.ts to cut theopenESM import chain (consistent with existing test patterns inPackmindCliHexa.spec.ts)process.stdin.once('data', ...)rather thanreadline.createInterfacefor simpler testabilityNotes
To remove the mock, maybe use a global
moduleNameMapperfor open injest.config.ts