Skip to content

fix(init-uninstall): uninstall removes --claude-md artifacts on Windows#392

Open
aeppling wants to merge 5 commits intodevelopfrom
fix/address-issue-384
Open

fix(init-uninstall): uninstall removes --claude-md artifacts on Windows#392
aeppling wants to merge 5 commits intodevelopfrom
fix/address-issue-384

Conversation

@aeppling
Copy link
Copy Markdown
Contributor

@aeppling aeppling commented Mar 6, 2026

Fix for : #384

Core fix (src/init.rs — uninstall()):

  • Added step after existing @RTK.md removal: checks for <!-- rtk-instructions marker in CLAUDE.md and removes the block using the existing remove_rtk_block() helper
  • Handles both artifacts in a single pass (covers edge case where both @RTK.md and the instructions block exist, e.g. after running both rtk init -g and rtk init -g --claude-md)
  • If CLAUDE.md becomes empty after cleanup, deletes the file

Hardening:

  • Extracted RTK_BLOCK_START and RTK_BLOCK_END constants — all marker matching across uninstall(), upsert_rtk_block(), remove_rtk_block(), patch_claude_md(), and show_config() now uses these instead of duplicated string literals
  • When uninstall finds nothing to remove, prints which paths were checked to help users diagnose path issues
  • resolve_claude_dir() error message now shows %USERPROFILE% on Windows instead of $HOME

@aeppling
Copy link
Copy Markdown
Contributor Author

aeppling commented Mar 6, 2026

Summary

  • rtk init -g --uninstall now removes the block from CLAUDE.md, which is the only artifact
    installed on Windows (where rtk init -g falls back to --claude-md mode)
  • Previously, uninstall only looked for Unix-mode artifacts (hook file, RTK.md, @RTK.md reference, settings.json entry), so
    on Windows it reported "RTK was not installed (nothing to remove)" even though RTK instructions were present in CLAUDE.md

@aeppling aeppling changed the title fix(uninstall): uninstall removes --claude-md artifacts on Windows fix(init-uninstall): uninstall removes --claude-md artifacts on Windows Mar 6, 2026
aeppling added 2 commits March 7, 2026 11:23
  Core fix (src/init.rs — uninstall()):
  - Added step 3b after existing @RTK.md removal: checks for <!-- rtk-instructions marker in CLAUDE.md and removes the block
  using the existing remove_rtk_block() helper
  - Handles both artifacts in a single pass (covers edge case where both @RTK.md and the instructions block exist, e.g. after
  running both rtk init -g and rtk init -g --claude-md)
  - If CLAUDE.md becomes empty after cleanup, deletes the file

  Hardening:
  - Extracted RTK_BLOCK_START and RTK_BLOCK_END constants — all marker matching across uninstall(), upsert_rtk_block(),
  remove_rtk_block(), patch_claude_md(), and show_config() now uses these instead of duplicated string literals
  - When uninstall finds nothing to remove, prints which paths were checked to help users diagnose path issues
  - resolve_claude_dir() error message now shows %USERPROFILE% on Windows instead of $HOME

Signed-off-by: Adrien Eppling <adrien.eppling@supinfo.com>
@aeppling aeppling force-pushed the fix/address-issue-384 branch from 1cac797 to 8fc0e4d Compare March 7, 2026 12:14
@pszymkowiak pszymkowiak force-pushed the develop branch 4 times, most recently from dc20fc4 to cc93afc Compare March 10, 2026 19:59
@pszymkowiak
Copy link
Copy Markdown
Collaborator

Thanks @aeppling! Clean fix — reusing remove_rtk_block() and extracting the block markers as constants is the right approach.

Two things to address:

Must fix:

  1. remove_rtk_block() drops trailing newline — now that uninstall() writes the result directly to disk, the file loses its final \n. Add format!("{}\n", before) in the return path when after is empty.

  2. Integration tests duplicate production logic (test_uninstall_integration_claude_md_only, test_uninstall_integration_preserves_user_content) — they copy the if contains(RTK_BLOCK_START) block instead of calling remove_rtk_block() directly. If the real function changes, these tests won't catch it.

Nice to fix:

  1. CHANGELOG entry should go under [Unreleased], not [0.27.2].

  2. test_uninstall_handles_both_artifacts creates a TempDir but never uses it — can be a pure in-memory test.

Also, could you enable "Allow edits by maintainers" on this PR? It would let us help with small fixes if needed.

Thanks!

@aeppling
Copy link
Copy Markdown
Contributor Author

Fix 1

src/init.rs:1009: before.to_string() → format!("{}\n", before) — files now keep their trailing newline
after RTK block removal.

Fix 2

src/init.rs:1677-1736: Both "integration" tests simplified to call remove_rtk_block() directly on
strings. Removes the duplicated orchestration logic (marker checks, file I/O, removed vec) that was
copied from uninstall(). The existing TempDir tests at lines 1617 and 1633 already cover file I/O
behavior.

Fix 3

CHANGELOG.md: Moved the #384 entry from the second [Unreleased] section into the first (canonical) one,
and removed the duplicate.

Skipped 4

This test is already a pure in-memory test — it doesn't create a TempDir. You may have been thinking
of the neighboring tests (test_uninstall_removes_rtk_instructions_block,
test_uninstall_preserves_non_rtk_content) which do use one.

Signed-off-by: aesoft <43991222+aeppling@users.noreply.github.com>
@aeppling aeppling force-pushed the fix/address-issue-384 branch from 7042ca9 to 69a9c64 Compare March 13, 2026 14:32
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Mar 20, 2026

CLA assistant check
All committers have signed the CLA.

@aeppling
Copy link
Copy Markdown
Contributor Author

Hey

We are cleaning up the codebase and improving the project structure for better onboarding. As part of this effort, PR #826 reorganizes src/ from a flat layout into subfolders.

No logic changes — only file moves and import path updates.

What you need to do

Rebase your branch on develop when receiving this comment:

git fetch origin && git rebase origin/develop

Git detects renames automatically. If you get import conflicts, update the paths:

use crate::git;        // now: use crate::cmds::git::git;
use crate::tracking;   // now: use crate::core::tracking;
use crate::config;     // now: use crate::core::config;
use crate::init;       // now: use crate::hooks::init;
use crate::gain;       // now: use crate::analytics::gain;

Need help rebasing? Tag @aeppling

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.

4 participants