feat(install): add --dry-run and --help flags to install.sh#19
Open
Arvuno wants to merge 3 commits into
Open
Conversation
The install summary was using `ls`/`wc` on the destination filesystem to count agents, skills, and hooks. That undercounts because the kit ships the same content under both `.claude/` and `.codex/`, and the previous counter only walked `.claude/`. The summary also claimed "(Claude Code + Codex)" while only counting Claude. Drive the counts off `$FILES`, the resolver output that the install loop just wrote to `.vc-installed-files`. That output is the authoritative list of what the manifest produced, so the summary now matches reality across both runtimes and is independent of `ls`/`wc` quirks on the host. Smoke-tested the regex on a small fixture: AGENT=3 (matches .claude/agents/* + .codex/agents/*) SKILL=3 (matches SKILL.md anywhere) HOOK=3 (matches top-level *.cjs under any hooks/ dir)
`set -o pipefail` was aborting the installer when a category had no matches. Append `|| true` to the grep stage so `wc -l` can still report 0. Reported-by: CodeRabbit review (PR withkynam#17, major)
The installer unconditionally copies files, creates symlinks, and removes any existing .claude / .codex / .agents directories. There was no way to preview the plan before running it for real, which made the script hard to evaluate in CI smoke checks and awkward for humans who want to see what would change. This change adds two flags: - --dry-run: parse the manifest, count files/skills/hooks, and print the install summary without touching the working tree. Backups, copies, symlink creation, and the .vc-installed-files / .vc-version writes are all skipped. - --help: print usage and exit 0. - Unknown arguments: print an error to stderr and exit 64 (EX_USAGE) so calling scripts can detect bad input. No behavior change for the default invocation (no flags). The exit status from a successful dry-run is 0, matching the success path of the real installer.
📝 WalkthroughWalkthrough
ChangesDry-run mode in install.sh
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
install.sh (1)
230-233:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDry-run prints a false “backed up” success message
When
HAS_EXISTING=trueandDRY_RUN=true, Line 232 says the previous setup was backed up, but no backup was created in dry-run mode. This is misleading for users and automation logs.Suggested patch
if [ "$HAS_EXISTING" = true ]; then echo "" - echo -e " ${YELLOW}Previous setup backed up to ${CYAN}$BACKUP_DIR/${NC}" + if [ "$DRY_RUN" = true ]; then + echo -e " ${YELLOW}Previous setup would be backed up to ${CYAN}$BACKUP_DIR/${NC}" + else + echo -e " ${YELLOW}Previous setup backed up to ${CYAN}$BACKUP_DIR/${NC}" + fi echo -e " ${YELLOW}Your process/ directory was preserved (plans, context, features).${NC}" fi🤖 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 `@install.sh` around lines 230 - 233, The backup success message at line 232 is printed unconditionally when HAS_EXISTING is true, but in dry-run mode no backup is actually created. Add a condition to check if DRY_RUN is not set to true before printing the echo statements that claim the backup was successful. This ensures users only see the backup confirmation message when a backup was actually performed.
🤖 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.
Outside diff comments:
In `@install.sh`:
- Around line 230-233: The backup success message at line 232 is printed
unconditionally when HAS_EXISTING is true, but in dry-run mode no backup is
actually created. Add a condition to check if DRY_RUN is not set to true before
printing the echo statements that claim the backup was successful. This ensures
users only see the backup confirmation message when a backup was actually
performed.
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.
The installer unconditionally copies files, creates symlinks, and removes any existing
.claude/.codex/.agentsdirectories. There was no way to preview the plan before running it for real, which made the script hard to evaluate in CI smoke checks and awkward for humans who want to see what would change.This change adds two flags:
--dry-run: parse the manifest, count files/skills/hooks, and print the install summary without touching the working tree. Backups, copies, symlink creation, and the.vc-installed-files/.vc-versionwrites are all skipped.--help: print usage and exit 0.EX_USAGE) so calling scripts can detect bad input.No behavior change for the default invocation (no flags). The exit status from a successful dry-run is 0, matching the success path of the real installer.
Summary by CodeRabbit
--dry-runmode to preview installation changes without making actual modifications.-h) to display usage information.