apply.py currently uses the library's download_skill + defang_skill
separately. Could it instead call soliplex_skills.install.install_skill
(resolve/download -> install -> defang in one call) to cut duplication?
Analysis: not a net simplification
install_skill(spec, version, dest, *, installed_by, defang, force, dry_run)
bakes in four policies apply.py deliberately does not share:
- No local-directory path. It always downloads from a spec, so
--room-skill-dir / --docs-skill-dir (offline/dev installs) still need
the copytree + install.defang_skill path that is today's
install_skill. So it adds a second install path rather than removing one.
- Always hits the network -- even for
--dry-run, and to decide
idempotency (downloads to a temp dir before the dry-run check). Today
resolve_published_skill returns early (no network, no temp dir) on a dry
run or when the skill is already present without --force.
- Commit-based
UPGRADED semantics the installer's ADDED/UNCHANGED
summary and --force model don't use.
- Breaks fail-fast ordering.
main() resolves/downloads both skills up
front, then apply() mutates the stack; a download failure leaves the
stack untouched. install_skill fuses download with the write, so a
late failure could leave the stack half-edited.
The genuinely-duplicated pieces (download, defang) already come from the
library; what remains is installer-specific orchestration.
Recommendation
Keep the download_skill + defang_skill split. Revisit only if we decide to
drop --*-skill-dir, accept downloading on --dry-run, and adopt commit-based
UPGRADED semantics -- then install_skill becomes a clean fit.
apply.pycurrently uses the library'sdownload_skill+defang_skillseparately. Could it instead call
soliplex_skills.install.install_skill(resolve/download -> install -> defang in one call) to cut duplication?
Analysis: not a net simplification
install_skill(spec, version, dest, *, installed_by, defang, force, dry_run)bakes in four policies
apply.pydeliberately does not share:--room-skill-dir/--docs-skill-dir(offline/dev installs) still needthe
copytree+install.defang_skillpath that is today'sinstall_skill. So it adds a second install path rather than removing one.--dry-run, and to decideidempotency (downloads to a temp dir before the dry-run check). Today
resolve_published_skillreturns early (no network, no temp dir) on a dryrun or when the skill is already present without
--force.UPGRADEDsemantics the installer'sADDED/UNCHANGEDsummary and
--forcemodel don't use.main()resolves/downloads both skills upfront, then
apply()mutates the stack; a download failure leaves thestack untouched.
install_skillfuses download with the write, so alate failure could leave the stack half-edited.
The genuinely-duplicated pieces (download, defang) already come from the
library; what remains is installer-specific orchestration.
Recommendation
Keep the
download_skill+defang_skillsplit. Revisit only if we decide todrop
--*-skill-dir, accept downloading on--dry-run, and adopt commit-basedUPGRADEDsemantics -- theninstall_skillbecomes a clean fit.