feat(init): add --dry-run flag to preview changes without writing#1032
Conversation
|
suppressed misleading success messages like "Gemini CLI hook installed (global)." and "Restart Gemini CLI. Test with: git status" that were printing even in dry-run, and printed: [dry-run] Nothing written. instead |
65f5fd1 to
3fa0323
Compare
|
Hey @hed0rah , thanks for the contribution and keeping this up to date. Few things to be check : Merge conflicts with developThis PR is based on a develop version that used hook scripts (rtk-rewrite.sh). Develop has since moved to binary commands (rtk hook claude/rtk hook cursor). Key functions from this PR no longer exist on develop:
Resolution: rebase on current develop, drop the removed functions, and thread dry_run into the new ones (patch_settings_json_command, migrate_old_hook_script, install_cursor_hooks). Bugs to fixPlease check following issues to ensure a clean feature implementation:
Fix: wrap success blocks in if !dry_run { ... }.
Missing testsNo tests exercise dry_run=true. At minimum add tests verifying:
Minor
|
Rebased onto current develop (binary-command era). Threads dry_run through patch_settings_json_command, migrate_old_hook_script, install_cursor_hooks, run_kilocode_mode, run_antigravity_mode, run_gemini, run_copilot, and uninstall. Fixes from PR rtk-ai#1032 review: - --uninstall --dry-run no longer deletes files (uninstall() now takes dry_run, every fs::remove_file / fs::write / atomic_write guarded) - Success messages ("installed", "configured", "Restart ...") gated on !dry_run in run_default_mode, run_hook_only_mode, run_codex_mode, run_copilot, install_cursor_hooks, run_gemini - prompt_telemetry_consent() skipped in dry-run - integrity::store_hash() in run_gemini guarded - KiloCode and Antigravity modes now accept dry_run - PatchResult::WouldPatch variant added for patch_settings_json_command - [dry-run] Nothing written. footer printed by every sub-mode - write_if_changed uses atomic_write (not fs::write) Added integrity::hash_path_for() public wrapper so dry-run can check sidecar existence without the destructive remove_hash. Tests: write_if_changed(dry_run=true) creates nothing; run_codex_mode_with_paths(dry_run=true) creates neither RTK.md nor AGENTS.md. 1596 tests pass, clippy clean.
3fa0323 to
21a069a
Compare
|
Rebased onto current develop and addressed all review feedback. Force-pushed the branch. History is replaced since the old PR commits predate the hook-script to binary-command refactor. Addressed
Tests
Also manually smoke-tested: Ready for re-review. |
|
When --dry-run was added, the signatures of run_default_mode, run_hook_only_mode, run_claude_md_mode, and uninstall each gained a trailing dry_run: bool parameter, but the test calls in src/hooks/init.rs (lines 4233, 4255, 4256, 4272-73, 4285, 4292, 4309, 4327) were never updated. |
…removals
The two remove_file calls flagged by semgrep already existed pre-PR.
Wrapping them in if dry_run { print } else { remove_file } shifted
their context enough that --baseline-commit re-attributed them as new.
The rule's own message says deletion is expected in hooks/init cleanup.
aeppling
left a comment
There was a problem hiding this comment.
Review: PR #1032 feat(init): add --dry-run flag
Thanks for the rebase and V2 fixes, the filesystem guard coverage is thorough. I built the branch (8b4a146), ran 23 manual tests across all init/uninstall modes, and the full test suite (1609 pass). Every install and uninstall dry-run correctly prevents writes.
One bug and two blocking items need to be addressed before merge, plus one architectural change request.
Bug to fix (blocking)
Double [dry-run] Nothing written. footer with --agent cursor
Repro: rtk init -g --agent cursor --auto-patch --dry-run
Output:
[dry-run] would create RTK.md: ~/.claude/RTK.md
[dry-run] would patch settings.json: ~/.claude/settings.json
[dry-run] would create global filters template: ~/.config/rtk/filters.toml
[dry-run] Nothing written. <-- first
[dry-run] would patch Cursor hooks.json: ~/.cursor/hooks.json
[dry-run] Nothing written. <-- second (duplicate)
Cause: run_default_mode() prints its own footer, then run() calls install_cursor_hooks() which prints another.
Fix: Move print_dry_run_footer() out of sub-modes and into run() as the single exit point, after all mode+cursor work completes.
Documentation (blocking)
CONTRIBUTING.md requires documentation for new features. Please add a short section to docs/guide/getting-started/quick-start.md covering --dry-run: what it does, how to use it, and its interaction with -v for content preview.
Architecture: Replace parameter threading with InitContext struct (blocking)
The PR threads dry_run: bool through 36 function signatures, the same 36 that already carry verbose: u8. This works but doesn't scale: PR #1493 (--only/--skip) would add more parameters to these same 36 signatures, and there are 10+ open PRs adding agent modes that each need to thread all params.
RTK already uses this pattern, see RunOptions in src/core/runner.rs:18:
#[derive(Default)]
pub struct RunOptions<'a> {
pub tee_label: Option<&'a str>,
pub filter_stdout_only: bool,
pub skip_filter_on_failure: bool,
pub no_trailing_newline: bool,
}Apply the same pattern to init:
#[derive(Clone, Copy, Default)]
pub struct InitContext {
pub verbose: u8,
pub dry_run: bool,
}Every signature changes from:
fn write_if_changed(path: &Path, content: &str, name: &str, verbose: u8, dry_run: bool) -> Result<bool>
fn run_default_mode(global: bool, patch_mode: PatchMode, verbose: u8, install_opencode: bool, dry_run: bool) -> Result<()>
fn patch_cursor_hooks_json(path: &Path, verbose: u8, dry_run: bool) -> Result<bool>to:
fn write_if_changed(path: &Path, content: &str, name: &str, ctx: InitContext) -> Result<bool>
fn run_default_mode(global: bool, patch_mode: PatchMode, install_opencode: bool, ctx: InitContext) -> Result<()>
fn patch_cursor_hooks_json(path: &Path, ctx: InitContext) -> Result<bool>Construction at call site:
let ctx = InitContext { verbose: cli.verbose, dry_run };
hooks::init::run(global, install_claude, ..., ctx)?;Why this matters now:
verbose: u8already pollutes 36 signatures, this PR doubles the problem#[derive(Copy, Clone, Default)]means zero allocation, passable by value- Adding future flags (
--only,--skip,--quiet) is one struct field instead of 36+ signature changes - Every open PR adding an agent mode benefits immediately, they just accept
ctx: InitContext - This is a mechanical refactor (find-replace), no logic changes
Minor (non-blocking)
--show --dry-run: Silently ignores--dry-run. Consider#[arg(conflicts_with = "show")]ondry_run.- Missing tests: No test for uninstall dry-run path (the #1 bug from V1), no test for default mode dry-run. The 3 existing tests cover the primitive, but the most destructive paths deserve regression tests.
Addresses three review items from PR rtk-ai#1032: - Bundle verbose+dry_run into a Clone+Copy InitContext struct (mirrors RunOptions in src/core/runner.rs). Collapses 25+ function signatures that already carried both fields and makes future flags one struct field instead of N signature changes. - Emit "[dry-run] Nothing written." exactly once from the top-level run() and uninstall() exit points instead of from every sub-mode. Fixes the double footer when --agent cursor combined with default mode. - Reject --show with --dry-run via clap conflicts_with rather than silently ignoring --dry-run. - Add regression tests for run_default_mode and uninstall dry-run paths using the existing with_claude_dir_override scaffolding.
Adds a "Preview without writing" subsection under Step 1 covering the --dry-run flag, -v interaction for content preview, that telemetry consent is skipped, and the --show conflict. Required by CONTRIBUTING.md section 4 (new features need documentation).
|
@aeppling thanks for the thorough review. Pushed two commits addressing all five items. Blocking (refactor): Blocking (bug): Same commit. Removed all Blocking (docs): Minor (conflicts_with): Same code commit. Minor (missing tests): Same code commit. Added Full gate: |
|
LGTM , thanks for contributing to RTK ! |
Summary
--dry-runtortk initso users can preview exactly what files would be created or modified before committing to a global install[dry-run] would write: <abs_path>instead of touching the filesystem--verbose(-v) to also print the full file contents that would be written, making it useful for auditing or CI validationTest plan
rtk init -g --dry-run— prints paths, no files created, verify withrtk init --showstill showing "not installed"rtk init -g --dry-run -v— same as above but also prints file contents inlinertk init -gthenrtk init -g --dry-run— "already up to date" shown for existing files instead of silently skippingrtk init --agent cline --dry-run— shows abs path to.clinerulesin current dirrtk init --agent windsurf --dry-run— same for.windsurfrulesrtk init -g --gemini --dry-run— shows hook file + settings.json patch--dry-run