Skip to content

Add AI assistant skill installation#17

Merged
strangesource merged 7 commits into
mainfrom
feature/skills-install
May 26, 2026
Merged

Add AI assistant skill installation#17
strangesource merged 7 commits into
mainfrom
feature/skills-install

Conversation

@strangesource
Copy link
Copy Markdown
Contributor

Summary

  • add local Bitmovin CLI skill installation via bitmovin init and legacy bitmovin skill output
  • add bitmovin skills list/find/add/remove for remote Bitmovin skills
  • discover remote skills from the bitmovin/skills GitHub archive via skills/*/SKILL.md
  • add archive/path validation and tests for skill installation flows

Verification

  • npm run build
  • npm test
  • npm run lint
  • packed and installed the CLI tarball into a temp project, then verified bitmovin skill, bitmovin init, skills list, skills find, and skills add --dry-run

@strangesource strangesource marked this pull request as ready for review May 22, 2026 10:52
@strangesource strangesource self-assigned this May 22, 2026
@lukaskroepfl
Copy link
Copy Markdown
Member

A few directional points before this lands:

1. Rename bitmovin init. init is a heavily overloaded verb (project init, config init, local infra setup). Today the command does one narrow thing — write a SKILL.md into agent skill directories. A user running bitmovin init in a fresh checkout will reasonably expect "set up local state for the CLI", not "install AI assistant files into ~/.claude/skills". Options: bitmovin skills install, bitmovin skills install-cli, or fold it into bitmovin skills add with a --cli/default-target behavior. The top-level init slot is hard to walk back later.

2. Drop the dedicated CLI skill; make the canonical install be the hub skill from bitmovin/skills (the bitmovin.com/skill hub). The skills/bitmovin-cli/SKILL.md in this PR is essentially a hand-curated --help dump. oclif already exposes that surface (bitmovin <topic> --help, help topics, generated README) and agents can navigate it on their own. That makes the CLI skill duplicate-maintenance — every flag added has to be re-written into the markdown, and it'll drift. Genuine question rather than a hard claim: do you have evidence that agents struggle with oclif's help output and specifically need the hand-written cheat sheet? If not, the higher-leverage thing to install is the hub skill that points agents at the CLI + API docs + player/analytics SDKs together. Naming-wise that also resolves the bitmovin skill vs bitmovin skills add --skill bitmovin mismatch — they'd refer to the same thing.

3. Reuse the existing npx skills flow for add/search/remove instead of reimplementing it. We already have npx skills for skill discovery/install. Building a parallel pipeline here (tar dep, archive download, validation, install/remove) duplicates that work and gives us two installers to keep in sync. Could bitmovin skills add/find/list/remove shell out to npx skills … and inherit it? If yes, the CLI's contribution shrinks to "know which Bitmovin skills exist and delegate", and we drop ~600 lines plus the tar dep plus the Node 18→20 engine bump.

Net effect if all three land: bitmovin init → renamed; default install target → hub skill; add/find/remove → thin wrappers over npx skills. The PR shrinks meaningfully and avoids inventing infrastructure that exists elsewhere.

@strangesource
Copy link
Copy Markdown
Contributor Author

Thanks for the thoughtful review — I agree with point 2 that the dedicated CLI skill risks becoming duplicate-maintenance if it remains a hand-curated help dump.

One bit of context: this CLI skill was already present before this PR as the hidden bitmovin skill command with the markdown embedded directly in TypeScript. In this PR I mostly moved that existing content into skills/bitmovin-cli/SKILL.md, which felt like a more convenient and packageable place for the same pattern, and then wired bitmovin init to install it.

Given your point, should we remove the dedicated CLI skill entirely instead of preserving/moving the existing behavior?

@strangesource
Copy link
Copy Markdown
Contributor Author

One more thing to consider @lukaskroepfl:
I agree that duplicating the CLI help in the skill is not very useful. However, I think the skill can still add value if it focuses only on discoverability and decision guidance: making agents aware that the CLI exists, explaining when it should be used, and then pointing them to the CLI help for the actual command details.

I will however revert to the previous behaviour to focus on the core scope of the pr.

@strangesource
Copy link
Copy Markdown
Contributor Author

Update pushed with the follow-up simplifications:

  • Replaced the custom skill installer/archive/download implementation with thin wrappers around npx --yes [email protected].
  • Removed the custom tar dependency and the archive validation/install pipeline from this repo.
  • Removed bitmovin skills find from this PR to keep scope focused on add/list/remove.
  • Removed the dedicated local bitmovin-cli skill and restored the hidden bitmovin skill command to its pre-PR embedded behavior/location.
  • Removed the top-level bitmovin init command so this PR only adds the scoped bitmovin skills ... commands.
  • Kept agent names pass-through to npx skills without mapping.
  • Updated README and tests accordingly.

Verification run locally:

  • npm run build
  • npx vitest run test/lib/skills-npx.test.ts
  • npm run lint

Copy link
Copy Markdown
Member

@lukaskroepfl lukaskroepfl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick turnaround — sorry, I missed your two earlier comments before posting my last follow-up (since deleted). The new shape is exactly the direction I was hoping for:

  • bitmovin skills add/list/remove as thin wrappers around npx [email protected]
  • tar dep and the archive/extract/install pipeline gone ✓
  • bitmovin init removed ✓
  • Scope tightened by dropping find
  • Keeping the hidden bitmovin skill in pre-PR form ✓ — fair scope call; the "skill as discoverability/decision-guidance vs duplicate help" debate is its own conversation and doesn't need to block this.

A couple of small things worth a glance, none blocking:

  • npx.ts pins [email protected] — good for predictability; just worth noting we'll need to bump it deliberately, won't pick up patches automatically.
  • buildSkillsAddArgs uses --global --copy --yes; the --yes shows up twice in the final argv (once for npx, once for skills). Not a bug, just slightly noisy — fine to leave.
  • The ref flag is hidden: true on add/list but documented in --help description as "Git ref to read skills from"; consistent across both, good.

LGTM.

@strangesource strangesource merged commit 320de7b into main May 26, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants