fix: Tighten v3 migration skill guidance and align wizard prompt#1981
Conversation
The skill predates newer V3 commands, so token-level searches now collide with valid V3 syntax: --wait-for-domain-reload is a positive flag on execute-dynamic-code and --include-inactive on find-game-objects. Document those collisions in a dedicated reference section, tell agents to resolve defaults for unlisted first-party booleans via "uloop <command> --help", and drop the repo-internal maintainer note that shipped to third-party projects where SkillTargetInstaller and skills.go do not exist.
The maintainer note removed from the installed migration skill still matters to maintainers, so restate it where the cleanup lists live: the dispatcher's deprecatedSkillNames and the Unity-side SkillTargetInstaller now reference each other and the skill source.
The copyable AI prompt said "Migrate ... removed commands" while the skill itself treats removed commands as report-only unless the user explicitly asks. Reword the prompt so it no longer overrides the skill's rule.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe change updates V2-to-V3 CLI migration guidance, adds command-specific boolean and same-name option rules, separates removed-command reporting, and documents consistency between Unity cleanup names and dispatcher deprecated skill names. ChangesV3 CLI migration guidance
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/src/TemporarySkills`~/v3-cli-invocation-migration/Skill/references/first-party-v2-to-v3.md:
- Around line 81-84: Clarify the “Leave unchanged on” entries in the option
migration table so only already-bare flags remain unchanged on the listed
commands. Explicitly require value-bearing forms such as true/false variants to
follow the Boolean Argument Rules, while preserving the existing bare-flag V3
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 44df7ca1-092b-4cad-87c2-dcc887d56f41
📒 Files selected for processing (5)
Packages/src/Editor/Infrastructure/SkillSetup/SkillTargetInstaller.csPackages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardText.csPackages/src/TemporarySkills~/v3-cli-invocation-migration/Skill/SKILL.mdPackages/src/TemporarySkills~/v3-cli-invocation-migration/Skill/references/first-party-v2-to-v3.mdcli/dispatcher/internal/dispatcher/skills.go
"Leave unchanged on" could be read as skipping value-bearing V2 forms like "--wait-for-domain-reload true" on execute-dynamic-code. State that only the already-bare flag is exempt and value-bearing forms still follow the Boolean Argument Rules. Addresses CodeRabbit review.
Summary
The
v3-cli-invocation-migrationskill (installed into third-party projects via the migration wizard's "Install Migration Skill" button) predates several V3 changes. This PR fixes four accuracy problems found by re-auditing the skill against the current tool schemas and CLI flag-naming logic:--wait-for-domain-reloadand--include-inactiveare listed as V2 migration targets, but they are now valid V3 positive flags onuloop execute-dynamic-codeanduloop find-game-objectsrespectively. Added a "Same-Name Options on Other Commands" reference section and a command-scoping rule so agents do not remove or negate valid V3 usage by token match alone.uloop <command> --helpand read the printeddefault: enabled/default: disabled.SkillTargetInstaller/skills.godeprecatedSkillNamesis meaningless in third-party projects. Removed it from the skill and restated it as code comments above the two cleanup lists.The special-options table itself was verified against
cli/common/tools/default-tools.jsonandcli/common/tooldocs/tool_options.goand needed no changes.Verification
scripts/check-go-cli.sh: format / vet / lint (0 issues) / tests all passdist/darwin-arm64/uloop compile: 0 errors, 0 warningsrg "Installed-skill cleanup names" Packages/src/TemporarySkills~andrg "and removed commands" Packages/src/Editorboth return no hits