fix: skills install handles .claude path - #145
Merged
Merged
Conversation
`parallel-cli skills install` only ever downloaded SKILL.md, so any skill shipping bundled resources landed incomplete. `migrate-to-parallel` installed 1 of its 10 files, leaving its SKILL.md pointing at 7 reference documents and a scan script that were never fetched. The CDN already published everything needed: each index entry carries a manifest_url, and the per-skill manifest lists every file with a URL, sha256, and size. _skills_from_index discarded manifest_url and the install loop hard coded a single SKILL.md write. Resolve each skill's full file list from its manifest and write every entry, preserving relative paths. Verify sha256 when the manifest supplies one, and reject absolute or parent-traversing paths so a manifest cannot write outside the skill directory. Indexes without a manifest_url, and manifests with an empty file list, keep the previous SKILL.md-only behavior, so custom indexes set via PARALLEL_SKILLS_INDEX_URL still install. Install and reinstall now report the file count so a truncated install is visible instead of silent. Verified against the live CDN: all 11 skills install to 20 files, byte identical to the 0.8.0 plugin release. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`parallel-cli skills install` wrote only to `.agents/skills`. Claude Code does not read that path — it discovers skills under `.claude/skills`, plugin directories, and managed policy dirs, and nothing else. Every skill installed by this CLI was therefore invisible to Claude Code, silently: the install reported success and wrote a complete, correct tree the agent never loaded. `.agents/skills` stays canonical and stays first. When a Claude Code configuration directory is present — `CLAUDE_CONFIG_DIR` or `~/.claude` for a global install, `<root>/.claude` for `--project` — the same skills are written to its `skills/` subdirectory too. Absence of that directory means no Claude Code on this machine, so nothing extra is created. An explicit `PARALLEL_SKILLS_GLOBAL_DIR` still targets exactly one directory. install/uninstall/reinstall now operate over a list of directories. Files are downloaded once and written to each target, so a mid-download failure leaves no location half-installed, and targets that resolve to the same path are written once — a symlinked `~/.claude/skills` would otherwise have its tree and manifest rewritten twice. Each directory carries its own manifest, so uninstall reconciles them independently and still leaves unmanaged skills alone. Results report `install_dirs`. The former singular `install_dir` is dropped rather than kept as an alias: it would always be `install_dirs[0]`, so any caller reading it would silently miss the Claude Code location. Nothing in either repo consumed it. Verified against the live CDN in a sandboxed HOME: with `.claude` present, all 11 skills install to both locations, 21 files each, byte-identical trees; uninstall clears both; without `.claude`, only `.agents/skills` is written and no `.claude` directory is created. A fresh Claude Code session loads all 11. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The 0.8.1 bump (#144) updated pyproject.toml, npm/package.json, __init__.py, and the cloud function requirements, but left uv.lock pinning the editable parallel-web-tools entry at 0.7.1. Any uv sync regenerates it, so the lock has been showing as dirty in working trees since that release. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Install unconditionally rmtree'd any directory matching a skill name before writing, so a user's own skill was destroyed if it happened to share a name with one of ours — then recorded in the manifest, so the following uninstall deleted what was left. Two of the published skills are named `result` and `status`, which is exactly the kind of name someone picks for a personal skill. Harmless while `.agents/skills` was the only target, since in practice nothing but this CLI writes there. Installing into `.claude/skills` changed that: it is where people keep hand-written skills. Each directory's manifest already records what we installed there, so it can answer the ownership question. A skill directory that exists but is absent from the manifest belongs to someone else: skip it, report it in `skipped_skills`, and leave it out of the manifest so uninstall cannot claim it later. A directory that is in the manifest is our own copy, and replacing it stays an upgrade. Skips are per directory, so a name can install into `.agents/skills` and be skipped in `.claude/skills`. `installed_skills` now reports what was actually written rather than what was requested. Also fixes two defects found while reviewing this path: Manifest paths were validated with POSIX rules only, so `C:/outside/payload` passed and pathlib then joined it on Windows by discarding the skill directory entirely, letting a hostile index write anywhere writable. Release builds include a windows-x64 target, so this was reachable. Both path flavours are now rejected, plus a resolved-containment backstop at write time that does not depend on anticipating each platform's join semantics. `reinstall_skills` consumed a one-shot iterable during uninstall and then raised on the empty remainder, after the existing skills had already been removed. Targets are normalized once and reused. Verified against the live CDN: a planted `~/.claude/skills/status` keeps its contents through install and uninstall, is reported as skipped, and the same skill still installs into `.agents/skills`; an unrelated personal skill is untouched throughout. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The ownership manifest was written after a directory's skills were on disk, so an install interrupted between the two — Ctrl-C, full disk — left skill directories that no manifest claimed. The next install read them as a user's own work and skipped them, reporting success, and uninstall ignored them for the same reason. Nothing short of deleting the directory by hand recovered. The collision guard introduced that trap: before it, the next install simply overwrote the partial tree. Ownership is now decided first, then recorded, then the files are written. A crashed install leaves directories this CLI still recognizes as its own, so the next install replaces them and uninstall can remove them. Deciding before recording is what keeps a user's skill out of the manifest even when a later write in the same run fails. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
2 fixes for
skills install: