chore: Setup and installer maintenance now use explicit helper names#1595
Conversation
Remove the no-token compile wrapper and rename R2-7 overload targets so each behavior variant has a distinct method name. Add a source guard to keep the targeted overloads from returning.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThis PR performs three independent internal refactors: CompileController.TryCompileAsync now requires an explicit CancellationToken (default overload removed), NativeCliInstaller splits install command building into remote and local-package-path variants, and skill installation detection APIs are renamed into layout-specific and any-layout helpers across SkillInstallLayout, SkillInstallationDetector, SkillTargetDetector, and the settings window. A new static-analysis guard test is added. ChangesEditor Tooling Refactors
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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.
1 issue found across 10 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Packages/src/Editor/Infrastructure/CLI/NativeCliInstaller.cs">
<violation number="1" location="Packages/src/Editor/Infrastructure/CLI/NativeCliInstaller.cs:39">
P3: The R2-7 overload guard in `StaticFacadeStateGuardTests.cs` still references the old method name `"BuildInstallCommand"` for `NativeCliInstaller.cs`. Since the methods were renamed to `BuildRemoteInstallCommand` and `BuildInstallCommandWithPackagePath`, the guard now silently passes without checking anything for this file — it searches for a method that no longer exists. Consider updating the guard entry to track the new method names so the test continues to protect against overload regressions.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Track the renamed R2-7 helper methods and fail the guard when a configured target disappears, preventing silent pass-through after future renames.
Keep the public SkillInstallationDetector overloads stable while retaining explicit internal helpers for the R2-7 overload cleanup.
Summary
User Impact
Changes
CancellationToken.Noneexplicitly.Verification
dist/darwin-arm64/uloop compile --project-path <PROJECT_ROOT>dist/darwin-arm64/uloop run-tests --project-path <PROJECT_ROOT> --test-mode EditMode --filter-type regex --filter-value "StaticFacadeStateGuardTests|SkillInstallLayoutTests|NativeCliInstallerTests"