fix(setup): allow repeated Windows setup

[lorenzozanee]

What

Allow bmad setup to replace an existing _bmad directory on Windows.

Why

Windows rejects the replacement when the temporary backup directory already exists, causing repeated setup to fail with WinError 183. Fixes #2879

How

• Remove the empty temporary backup directory before renaming the existing runtime.
• Add a regression test that simulates Windows directory-rename behavior.

Testing

The focused regression, setup test suite, repository quality checks, and git diff --check pass. Native Windows execution was not available locally.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 757fb0a8-1f4b-481c-911c-3686cbf9c15a

📥 Commits

Reviewing files that changed from the base of the PR and between 8c0e91e and b10401d.

📒 Files selected for processing (2)

• skills/bmad/scripts/setup.py
• tools/tests/test_bmad_setup.py

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

replace_dir now removes the temporary backup directory before renaming the existing destination. A regression test covers Windows-style rename behavior and verifies replacement and cleanup.

Changes

BMAD directory replacement

Layer / File(s)	Summary
Windows-safe directory replacement skills/bmad/scripts/setup.py, tools/tests/test_bmad_setup.py	replace_dir removes the empty temporary backup directory before renaming the existing destination. The test verifies successful replacement, removal of old contents, and cleanup of backup artifacts.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: alexeyv

Merge Risk: ⚪ Minimal · up to b1040

The change fixes repeated Windows setup replacement failures while preserving existing BMAD configuration and content. It is ready to merge.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely identifies the fix for repeated Windows setup failures. It matches the main change in the pull request.
Description check	✅ Passed	The description directly explains the Windows replacement failure, the cause, the fix, the regression test, and the testing status. It is fully related to the changeset.
Linked Issues check	✅ Passed	The change satisfies #2879. replace_dir() removes the directory created by tempfile.mkdtemp() with backup.rmdir() before it calls dest.rename(backup). This gives Windows an absent rename targe…
Out of Scope Changes check	✅ Passed	The changes are limited to the one-line replace_dir() fix in skills/bmad/scripts/setup.py and its regression test in tools/tests/test_bmad_setup.py. The test directly verifies the Windows failur…

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Confidence Score: 5/5

The PR appears safe to merge; the Windows-specific replacement failure is addressed without changing the surrounding rollback contract.

Summary

This PR makes repeated Windows setup succeed by removing the empty temporary backup directory before using its path as the destination for the existing _bmad runtime.

• Preserves the existing replacement and rollback sequence.
• Adds a focused regression test that emulates Windows rejecting a rename to an existing directory.
• Confirms successful replacement, removal of old content, and cleanup of the temporary backup.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Create unique empty backup directory] --> B[Remove empty backup directory]
    B --> C[Rename existing _bmad to backup path]
    C --> D[Rename staged runtime to _bmad]
    D --> E[Delete backed-up old runtime]
    C -->|Failure| F[Leave existing _bmad intact]
    D -->|Failure| G[Rename backup back to _bmad]

Loading

_{Reviews (1) · Last reviewed commit: "fix(setup): allow repeated Windows setup"}

[alexeyv]

Thank you for this, and for the clear write-up of the cause. Closing as superseded by #2918, which reached dev with #2917.

#2918 fixes the same defect (#2879) a different way: the backup is a name that does not exist yet, so no rename lands on an existing directory and there is nothing to remove first. #2917 also added a native Windows job to CI, so the failure was reproduced on Windows before the fix and passes there after it, which was the evidence this PR could not have.