fix: uloop skills install without a target flag now refreshes existing skill installs instead of doing nothing

[hatayama]

Summary

• uloop skills install with no target flag now refreshes every target that already holds uloop skills, as its help text already promised. When no target holds them yet, it still prints the target guidance and exits 0.

User Impact

• Before: after updating the package, running uloop skills install with no target flag printed "Please specify at least one target for 'install':" and exited 0 without touching anything, even though the project's .claude/skills already held uloop skills. The help text says "Targets that already contain uloop skills are refreshed automatically, even when their flag is omitted", so the installed copies stayed stale while the command looked successful. Passing any target flag (for example --claude) refreshed every installed target, including the others.
• After: the same command prints Auto-refreshing <target>: an existing uloop skill install was detected there. for each target that holds uloop skills and refreshes them, the same way a flagged install already refreshes the targets it was not asked for.

Cause

Install without any target flag printed the guidance and returned before reaching runSkillsInstall, which is where the detection of already-installed targets runs.

Changes

• runSkillsInstallWithGuidance: with no target flag, run the existing detectInstalledSkillTargets first. If it finds any target, continue into runSkillsInstall, which detects them again (a few stats) and refreshes them. If it finds none, print the guidance and return 0 as before. A detection error is reported in the same form as in runSkillsInstall and returns 1.

What did not change:

• No target holds uloop skills: same guidance, same exit code 0, nothing written.
• uloop skills uninstall still requires a target flag. Uninstall removes files, so it still asks for an explicit target.
• Installs with target flags, --output-dir, and the v3-migration subcommands are unchanged. The help text is unchanged; the command now does what it says.
• One edge case differs: when detection itself fails (for example --global with no resolvable home directory, or a target directory that cannot be read), install without a flag now reports that error with exit 1 instead of printing the guidance. A flagged install already failed the same way.

Tests

New tests in skills_dispatch_test.go:

• TestRunSkillsSubcommandInstallWithoutTargetRefreshesDetectedInstall: install with --claude, overwrite the installed SKILL.md with stale content, then install with no target. Expects exit 0, Auto-refreshing in the output, no guidance, and the source content restored. It failed before the fix: the guidance was printed and the file stayed stale. That failing state is the first commit.
• TestRunSkillsSubcommandInstallWithoutTargetAndNoInstallPrintsGuidance: with nothing installed, install with no target. Expects exit 0, the guidance, no Auto-refreshing, and no SKILL.md anywhere outside the skill source.
• TestRunSkillsSubcommandInstallWithoutTargetReportsDetectionErrors: with --global and a failing home lookup, install with no target. Expects exit 1, the error on stderr, and no guidance. This case does not touch the file system, so it also runs on Windows.

Mutations, each applied to the fix and then reverted:

• Drop the "nothing detected" check, so the command always installs: the no-install guidance test fails, and so does the existing TestTryHandleSkillsRequestPrintsTargetGuidanceWithoutTargets.
• Restore the original early return: the refresh test and the detection-error test fail.
• Swallow the detection error and treat it as "nothing detected": the detection-error test fails.

Verification

This PR targets the integration branch, so the Go CI (build-cli) does not run on it. It will run when the integration branch goes to main. Local results on macOS:

• cd cli/dispatcher && go test ./... -count=1: all packages pass.
• scripts/check-go-cli.sh stopped in cli/common. ipcendpoint's TestOSUnixMetadataReaderReportsRealOwnerAndMode could not create its directory under the system temp directory because the local sandbox denies it (operation not permitted). This PR does not touch cli/common. Before that point, cli/common passed format, vet, and lint (0 issues), and every other cli/common test package passed.
• The script stops at the first failure, so I ran its remaining per-module steps by hand with the same commands (golangci-lint fmt --diff, go vet ./..., golangci-lint run ./..., go test ./...):
  • cli/dispatcher: format, vet, lint (0 issues), and tests all pass.
  • cli/release-automation: format, vet, lint (0 issues), and tests all pass.
  • cli/project-runner: format, vet, and lint (0 issues) pass. One test, TestSendWithTransientConnectionRetryAbortsOnRefusedConnect, cannot bind its Unix socket because the local sandbox denies it (bind: operation not permitted). This PR does not touch cli/project-runner.
• The script's binary rebuild step did not run. I built the darwin-arm64 dispatcher with the same go build flags as scripts/build-go-cli.sh. From the root of a Unity project whose .claude/skills and .agents/skills already hold the generated copies, I ran uloop skills install with no target flag. It exited 0, printed Auto-refreshing Claude Code and Auto-refreshing Common, and reported Skipped: 22 for each target (the copies were already current). git status --porcelain stayed empty.
• Coverage: the dispatcher module measures 94.2% (94.29%) with the baseline's exclusions, against a baseline figure of 94.2. All branches of the changed function are covered.

Release

This is a dispatcher change, so users get it with the next dispatcher release.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 15 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository: hatayama/unity-cli-loop/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 7024cfd1-ecf2-40e3-b96e-623fea724dac

📥 Commits

Reviewing files that changed from the base of the PR and between 6847f03 and 4e3eb7d.

📒 Files selected for processing (1)

• cli/dispatcher/internal/dispatcher/skills_dispatch_test.go

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: hatayama/unity-cli-loop/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 5fbe42fc-f591-49a3-974e-49dc460c21f5

📥 Commits

Reviewing files that changed from the base of the PR and between 370b6ee and 6847f03.

📒 Files selected for processing (2)

• cli/dispatcher/internal/dispatcher/skills_dispatch.go
• cli/dispatcher/internal/dispatcher/skills_dispatch_test.go

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

---

📝 Walkthrough

Walkthrough

When install receives no explicit targets, the command detects installed skill targets. It proceeds with installation when targets are found, prints guidance when none are found, and reports detection errors with status 1.

Changes

Skill Install

Layer / File(s)	Summary
Detect targets and handle install outcomes cli/dispatcher/internal/dispatcher/skills_dispatch.go, cli/dispatcher/internal/dispatcher/skills_dispatch_test.go	The command detects installed targets before printing guidance when no targets are specified. Tests cover refreshing an existing install, guidance when no install is found, and detection errors.

Priority: ⬇️ Low

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 6847f

Running uloop skills install without a target now refreshes existing installs, still prints guidance when none are found, and reports detection errors. No merge-blocking risk was found.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 6847f

The command reuses the existing installer and preserves explicit global and custom-directory options. However, automatic refresh now reaches filesystem writes without target flags, and linked target directories can redirect those writes beyond the selected project. Exposure requires running the install command and remains limited by the caller's filesystem permissions.

Retained concerns

• Low · security · inferred: Implicit refresh inherits a filesystem-containment weakness: a project-controlled ancestor symlink can redirect a detected target to another skill tree accessible to the caller. Detection follows that alias, and replacement and deprecated-skill cleanup operate through it. This mechanism already existed for flagged installs, but the PR newly exposes it through no-target invocations that previously wrote nothing.

Security review details

Security Blast Radius

• inferred — The nominal scope is the detected default target trees under the selected project, or under the user's home when global mode is explicitly selected. Parent-directory aliases can extend physical exposure to other caller-accessible trees. Operations remain constrained to discovered skill names and configured cleanup names; no privilege elevation is established.

Security Findings and Attack Paths

• inferred — A conditional attack path requires control of a project's target-directory ancestor and a local install invocation. An alias to another existing skill tree can satisfy marker detection and direct refresh or cleanup there. This is source-inferred exposure, not a reproduced exploit; agent execution, credential disclosure and broader compromise were not established.

Trust Boundaries and Controls

• observed — Fixed target configuration limits logical destination selection. Global mode remains explicit, and output-directory options are mutually exclusive with target and global options. These controls remain in place, but lexical path construction is not a physical filesystem-containment control.

Resilience and Maintainability Implications

• observed — Detection errors stop the new no-target gate before mutation. Installation errors return nonzero, but may occur after earlier updates or cleanup have become externally visible; the backup restoration mechanism covers only the individual replacement attempt.

Hardening Proposals

• proposed — Define whether automatically detected targets may use linked roots. If containment is intended, use link-aware, race-resistant destination validation before refresh and cleanup rather than treating fixed path components as sufficient containment.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: installing without a target flag now refreshes existing skill installs. It is specific but longer than necessary.
Description check	✅ Passed	The description explains the behavior change, edge cases, tests, and verification. It is directly related to the changeset.
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.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving 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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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