feat: remove bundled clerk-cli skill - #315
Conversation
Install clerk-cli from clerk/skills during init instead of shipping a bundled local skill.
🦋 Changeset detectedLatest commit: 64b406d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Share skills CLI installation helpers from lib now that the standalone skill command directory is gone.
📝 WalkthroughWalkthroughThis PR removes the bundled Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/cli-core/src/commands/init/README.md (1)
70-70: 💤 Low valueStale skill summary in Flow step 16.
Line 193 now documents the
cli/group (clerk-cli), but this step still reads "core + features, plus a framework-specific skill" and omits the CLI skill. Consider aligning the wording for consistency.📝 Suggested wording
-16. Optionally installs Clerk agent skills (core + features, plus a framework-specific skill) via the project's package runner (see [Agent skills install](`#agent-skills-install`)) +16. Optionally installs Clerk agent skills (cli + core + features, plus a framework-specific skill) via the project's package runner (see [Agent skills install](`#agent-skills-install`))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli-core/src/commands/init/README.md` at line 70, Update the stale wording in step 16 of the init README: change "core + features, plus a framework-specific skill" to mention the CLI skill group as well (e.g., "core + features, plus a framework-specific skill and the cli/ (clerk-cli) skill"), so the step text aligns with the documented cli/ group elsewhere in the file; modify the sentence in the same paragraph that currently contains step 16 to include "cli/ (clerk-cli)" and keep the rest of the phrasing intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/cli-core/src/commands/init/README.md`:
- Line 70: Update the stale wording in step 16 of the init README: change "core
+ features, plus a framework-specific skill" to mention the CLI skill group as
well (e.g., "core + features, plus a framework-specific skill and the cli/
(clerk-cli) skill"), so the step text aligns with the documented cli/ group
elsewhere in the file; modify the sentence in the same paragraph that currently
contains step 16 to include "cli/ (clerk-cli)" and keep the rest of the phrasing
intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c427496f-89cb-41f7-937f-898373fcff1e
📒 Files selected for processing (20)
.changeset/remove-clerk-cli-skill.mdREADME.mdpackages/cli-core/src/cli-program.test.tspackages/cli-core/src/cli-program.tspackages/cli-core/src/commands/billing/index.test.tspackages/cli-core/src/commands/billing/index.tspackages/cli-core/src/commands/init/README.mdpackages/cli-core/src/commands/init/skills.test.tspackages/cli-core/src/commands/init/skills.tspackages/cli-core/src/commands/skill/README.mdpackages/cli-core/src/commands/skill/install.test.tspackages/cli-core/src/commands/skill/install.tspackages/cli-core/src/lib/next-steps.tspackages/cli-core/src/lib/skill-detection.tspackages/cli-core/src/lib/skills.test.tspackages/cli-core/src/lib/skills.tsskills/clerk-cli/SKILL.mdskills/clerk-cli/references/agent-mode.mdskills/clerk-cli/references/auth.mdskills/clerk-cli/references/recipes.md
💤 Files with no reviewable changes (10)
- packages/cli-core/src/commands/skill/install.test.ts
- packages/cli-core/src/commands/skill/README.md
- packages/cli-core/src/lib/skill-detection.ts
- skills/clerk-cli/references/recipes.md
- skills/clerk-cli/references/auth.md
- README.md
- packages/cli-core/src/commands/skill/install.ts
- skills/clerk-cli/SKILL.md
- skills/clerk-cli/references/agent-mode.md
- packages/cli-core/src/lib/next-steps.ts
Co-authored-by: Rafael Thayto <rafa.thayto@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/cli-core/src/lib/skills.ts (1)
54-129: ⚡ Quick winAdd tests for
runSkillsAddandresolveSkillsRunner.Only
buildSkillsArgsis covered byskills.test.ts. The two new functions carry the meaningful behavior (spawn success/failure → boolean, no-runner warning path, interactive multi-runner selection) and are untested. Consider adding cases that mockBun.spawn/runner detection to lock in the "never throws, returns false on failure" contract.As per coding guidelines: "If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli-core/src/lib/skills.ts` around lines 54 - 129, Tests are missing for runSkillsAdd and resolveSkillsRunner: add unit tests that (1) for runSkillsAdd mock Bun.spawn to simulate a successful exit (proc.exited resolves 0) and a failure (throws or non-zero exit) and assert the function returns true on success, false on failure and that appropriate log.warn calls happen; (2) for resolveSkillsRunner mock detectAvailableRunners/preferredRunner/runnerForPackageManager and the select prompt to cover the no-runner warning path (available empty → returns null) and the interactive multi-runner selection path (available.length > 1 and interactive true → returns selected Runner), and assert logging behavior; use buildSkillsArgs/runnerCommand identifiers where needed to ensure correct command construction in runSkillsAdd tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli-core/src/lib/skills.ts`:
- Line 63: The displayed recovery hint is missing the per-skill filters: update
the construction of displayCommand (currently const displayCommand =
`${runner.display} skills add ${source}`) to append the same skill filter flags
used when spawning the actual command (the --skill <name> flags or whatever
string/array is built for the spawn) so the manual command mirrors the real one;
locate where the spawn uses the skill flags (the code building the flags passed
alongside source) and include that same flags string/variable in displayCommand
so running the shown `${runner.display} skills add ...` will install only the
requested subset.
---
Nitpick comments:
In `@packages/cli-core/src/lib/skills.ts`:
- Around line 54-129: Tests are missing for runSkillsAdd and
resolveSkillsRunner: add unit tests that (1) for runSkillsAdd mock Bun.spawn to
simulate a successful exit (proc.exited resolves 0) and a failure (throws or
non-zero exit) and assert the function returns true on success, false on failure
and that appropriate log.warn calls happen; (2) for resolveSkillsRunner mock
detectAvailableRunners/preferredRunner/runnerForPackageManager and the select
prompt to cover the no-runner warning path (available empty → returns null) and
the interactive multi-runner selection path (available.length > 1 and
interactive true → returns selected Runner), and assert logging behavior; use
buildSkillsArgs/runnerCommand identifiers where needed to ensure correct command
construction in runSkillsAdd tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1c70d08f-6f3c-4abe-b912-0c3df2cf2220
📒 Files selected for processing (4)
packages/cli-core/src/commands/billing/index.tspackages/cli-core/src/commands/init/skills.tspackages/cli-core/src/lib/skills.test.tspackages/cli-core/src/lib/skills.ts
💤 Files with no reviewable changes (2)
- packages/cli-core/src/commands/billing/index.ts
- packages/cli-core/src/commands/init/skills.ts
| label: string, | ||
| ): Promise<boolean> { | ||
| const command = runnerCommand(runner, buildSkillsArgs(source, skillNames, interactive)); | ||
| const displayCommand = `${runner.display} skills add ${source}`; |
There was a problem hiding this comment.
displayCommand drops the --skill filters, so the manual-recovery hint is wrong.
The spawned command on Line 62 includes the --skill <name> flags, but displayCommand omits them. The warning messages on Lines 79 and 85 tell the user to run this command manually, which would instead install all skills from source rather than the requested subset.
🛠️ Proposed fix to include the skill flags in the displayed command
- const displayCommand = `${runner.display} skills add ${source}`;
+ const displayCommand = `${runner.display} ${buildSkillsArgs(source, skillNames, interactive).join(" ")}`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const displayCommand = `${runner.display} skills add ${source}`; | |
| const displayCommand = `${runner.display} ${buildSkillsArgs(source, skillNames, interactive).join(" ")}`; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli-core/src/lib/skills.ts` at line 63, The displayed recovery hint
is missing the per-skill filters: update the construction of displayCommand
(currently const displayCommand = `${runner.display} skills add ${source}`) to
append the same skill filter flags used when spawning the actual command (the
--skill <name> flags or whatever string/array is built for the spawn) so the
manual command mirrors the real one; locate where the spawn uses the skill flags
(the code building the flags passed alongside source) and include that same
flags string/variable in displayCommand so running the shown `${runner.display}
skills add ...` will install only the requested subset.
clerk/cli#315 removed the bundled skill and its `skill install` command. cli-program.test.ts now asserts the command is not registered, so both references in this skill pointed at something that errors out. - Drop the `clerk skill install` row from the command table. - Point the version banner at `clerk update` instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…it (#400) * fix(users): confirm before creating a user `users create` registers `--yes` as "Skip confirmation prompt" and its `setExamples` recommend passing it, but no prompt existed. The flag was declared on `CreateUserOptions` and never read, so the user was created immediately in every mode. The surrounding code already assumed a prompt: the `catch` handles `UserAbortError` and `isPromptExitError`, neither of which anything inside the `try` could throw. Nearly every existing test threads `yes: true` through to the BAPI call. This wires up the gate those were written against. Human mode now prints the redacted request body and confirms before the POST, matching `clerk api`, `config push`, `unlink`, and `impersonate`. Agent mode is untouched: `isHuman()` is false there, so it never prompts and `--dry-run` stays the safety net. Chose this over dropping the flag because `--help` and the shipped examples have been telling people the prompt exists. Removing `--yes` would break the documented invocations; adding the prompt makes them correct. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(skills): point audit-clerk-skill at clerk/skills The audit that keeps the `clerk-cli` skill in sync with this binary targets `skills/clerk-cli/SKILL.md`. #315 moved the skill out to clerk/skills, so that path has not existed here since. The audit has been checking nothing, which is how the skill drifted into promising confirmation prompts and a guidance-only agent login that neither exist. Retarget it at a `$SKILL_ROOT` clone of clerk/skills, and drop the `{{CLI_VERSION}}` and `clerk skill install` references that went with the bundled copy. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* fix(clerk-cli): correct agent-mode safety-gate and login claims The skill promised safety gates the CLI does not implement. Verified against clerk/cli @ e2224f0. - `users create` claimed "Confirmation prompt unless --yes". There is no confirm in create.ts; it POSTs immediately. `--yes` is registered but never read. - references/auth.md claimed agent-mode login "prints guidance rather than opening a browser". login.ts only early-returns when a session already exists; otherwise it falls through to performOAuthFlow() and openBrowser(). SKILL.md already described this correctly, so the reference file was simply stale. - references/agent-mode.md claimed `unlink`, `config patch`, and `api -X DELETE` all "Require --yes, otherwise error" in agent mode. Only unlink does. The others gate on `isHuman() && !options.yes`, and isHuman() is strictly !isAgent(), so agent mode skips them and the mutation executes with no prompt and no error. Split into two rows. - SKILL.md "Mutations still require --yes" was wrong for the same reason and contradicted the already-correct --dry-run guidance above it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(clerk-cli): drop the removed `clerk skill install` command clerk/cli#315 removed the bundled skill and its `skill install` command. cli-program.test.ts now asserts the command is not registered, so both references in this skill pointed at something that errors out. - Drop the `clerk skill install` row from the command table. - Point the version banner at `clerk update` instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
What
clerk-cliskill files from the CLI package.clerk skill installcommand and its help-tip detection path.clerk initinstalling agent context by requestingclerk-clifromclerk/skillsalongside the default upstream skills.skillsCLI installer helpers intolibforinitand billing to use.Why
The
clerk-cliagent skill now lives in theclerk/skillsrepository, so the CLI no longer needs to ship or stage a bundled copy.clerk initstill installs the skill as part of the upstream agent skills flow, and shared installer helpers no longer need to live under a removed command directory.How
The init installer now treats
clerk-clias part of the upstream default skill list fromclerk/skills. The bundled-skill text imports, staging helper, install command, skill detection helper, and related docs/tests were removed. The remainingskills addrunner helpers moved topackages/cli-core/src/lib/skills.tswith tests beside them.Verification
bun run format:checkbun run lintbun run typecheckbun run testbun run test:e2e:op