fix(install.sh): guard rm -rf against empty SKILLS_DIR (SC2115)

[moonming]

What

Hardens the rm -rf in install.sh per CodeRabbit / shellcheck SC2115:

-  rm -rf "${SKILLS_DIR}/${name}"
+  rm -rf "${SKILLS_DIR:?}/${name}"

Why

Defense-in-depth: if SKILLS_DIR were ever empty, the old form expands to rm -rf "/${name}" (a root-level path). The empty case is already blocked upstream (empty --dir and SKILLS_DIR are rejected/defaulted), so this can't trigger today — but ${SKILLS_DIR:?} makes it impossible by construction and silences the shellcheck warning. One-character change; behavior is otherwise identical.

Follow-up to the review on #56.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Improved installation safety by preventing the installer from proceeding when the required skills directory location is unset or empty.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 367d2a6d-daa6-4e0f-ae8a-c8542dbad737

📥 Commits

Reviewing files that changed from the base of the PR and between e017085 and 34b78c0.

📒 Files selected for processing (1)

• install.sh

---

📝 Walkthrough

Walkthrough

install.sh now enforces that SKILLS_DIR is set and non-empty before removing existing destination skill directories.

Changes

Install cleanup safety

Layer / File(s)	Summary
Guard destination cleanup install.sh	The destination removal command uses ${SKILLS_DIR:?} parameter expansion to reject unset or null SKILLS_DIR values.

Estimated code review effort: 1 (Trivial) | ~2 minutes

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
E2e Test Quality Review	⚠️ Warning	install.sh has no E2E/integration coverage for the new SKILLS_DIR guard; searches found no tests invoking the installer or the empty-dir boundary.	Add a shell-level integration/E2E test for install.sh that exercises --dir and SKILLS_DIR with temp dirs, including the empty/null boundary and overwrite behavior.

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the change in install.sh by guarding rm -rf against an empty SKILLS_DIR and references the ShellCheck fix.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Security Check	✅ Passed	install.sh only adds a safer ${SKILLS_DIR:?} guard to rm -rf; no logging, secrets, auth, or data persistence paths are affected.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/install-sh-rm-guard

---

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.}