fix(cli): accept a bare tool name for veryfront install - #3609
Conversation
`veryfront install agents` silently dropped the positional and fell back to auto-detection, which in a fresh project writes SKILL.md and exits 0. The published installation and coding-agents pages still print that exact line while promising AGENTS.md, so a reader following the docs gets the wrong file with no warning. The arg parser already supports positionals; the install/uninstall spec just never declared one. Declaring `positional: 0` makes `veryfront install agents` resolve the same target as `--target agents`, with the explicit flag still winning when both are given. An unknown positional now fails validation instead of installing something else.
|
Warning Review limit reached
Next review available in: 41 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe install and uninstall commands now accept optional positional targets. Named ChangesInstall target selection
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@cli/commands/install/command-help.ts`:
- Line 42: Update the uninstall command examples near the usage entry in
command-help to include a positional invocation, veryfront uninstall agents,
alongside the existing --target example. Preserve the current usage text and
other examples.
In `@cli/commands/install/install.integration.test.ts`:
- Around line 165-195: Add an integration test in the “bare positional target”
suite that invokes runInstallArgs with an unknown positional target and the
existing noninteractive/force options, then assert the CLI returns exit code 1.
Keep the existing successful-target and --target precedence tests unchanged.
🪄 Autofix
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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f440b205-db5d-464a-98a7-2b6a4456b94b
📒 Files selected for processing (5)
cli/commands/install/command-help.tscli/commands/install/handler.test.tscli/commands/install/handler.tscli/commands/install/install.integration.test.tstests/docs/cli-install-commands.test.ts
Review follow-up: add the uninstall positional example to the command help and an end-to-end case asserting an unknown positional exits 1 without writing any integration file.
`veryfront install not-a-tool` exited 1. The target was only checked inside `installCommand`, where `parseTargetFlag` throws a plain runtime error, so a typo was indistinguishable from an installation failure. AGENTS.md reserves exit 1 for runtime errors and exit 2 for usage and argument errors. Validate the target while parsing arguments instead, so the failure goes through `parseArgsOrThrow` and the router's "Invalid ..." usage path and the message names the valid targets. `--target not-a-tool` takes the same path and also exits 2 now. `parseTargetFlag` keeps its own check for programmatic callers. The positional-target fix this branch originally carried landed on main in #3609, so only the exit-code correction remains here. The end-to-end case #3609 added for an unknown positional asserted exit 1 and is updated to 2.
1 participant
Symptom (reproduced on published 0.1.1229)
veryfront installread its target from--targetonly. A bare positional wasdropped on the floor, the command fell through to auto-detection, and in a fresh
project auto-detection resolves to
SKILL.md. No warning, no error, exit 0. Thesame silent drop hit every documented variant (
veryfront install claude-code,... cursor, ...).That matters because the live docs still print the positional form:
Why the previous doc fix did not make the symptom go away
#3558 (
125f290e2) rewrotedocs/getting-started/installation.mdanddocs/guides/coding-agents.mdin this repo to use--target. That landedand is correct. But
veryfront.com/docs/code/**is served from theveryfront-docsrepo, which receives these pages through a periodic sync commit(
docs: update code docs from veryfront-code@<sha>). The last sync predates#3558, so the live pages still print the broken form and will keep printing it
until the next sync. A docs-only fix therefore could not close this: readers
follow the deployed page, not the source page.
This PR fixes the CLI instead, so the invocation the live docs print does the
right thing regardless of when the sync lands.
Fix
cli/shared/args.tsalready supports positional arg specs; the install specsimply never declared one. One line:
veryfront install agentsnow resolves the same target asveryfront install --target agents.--targetstill wins when both a flag and a positional are given.veryfront install bogus) now fails validation andexits 1 rather than quietly installing something else.
uninstallshares the parser, soveryfront uninstall cursorworks too.veryfront install [tools] [options]with an example.docs/**is left as #3558 wrote it —--targetis still the form torecommend for scripts, and the docs contract test in
tests/docs/cli-install-commands.test.tskeeps enforcing that every documentedcommand actually selects a target. Its header comment is updated, since the
"positional is silently ignored" statement it was written against is no longer
true.
Regression tests (written first, confirmed red)
cli/commands/install/handler.test.ts— bare positional resolves to the target,comma-separated positional,
--targetbeats the positional, no positionalleaves it unset, same for
uninstall.Before the fix:
cli/commands/install/install.integration.test.ts— drives the real CLI binaryin a temp dir and asserts
veryfront install agents --force --no-inputwritesAGENTS.mdand notSKILL.md(plus theclaude-codeand flag-beats-positionalcases). Before the fix both new end-to-end cases failed on the missing
AGENTS.md/.claude/CLAUDE.md.Original symptom, re-run against this build
veryfront install bogus --no-inputnow exits 1 instead of writingSKILL.md.Follow-up owned elsewhere
The
veryfront-docssync still needs to run for the live pages to match thisrepo's docs. After that sync,
https://veryfront.com/docs/code/getting-started/installationandhttps://veryfront.com/docs/code/guides/coding-agentsshould showveryfront install --target agents. With this PR shipped, the currently-livepositional form is no longer wrong either way.
Summary by CodeRabbit
New Features
--targetoptions take precedence over positional targets.Documentation
SKILL.mdcreation.