fix: Migration now upgrades v3 editor tools without leaving compile errors#1374
Merged
Conversation
Rewrite V2 EditorDelay.DelayFrame calls to the V3 EditorFrameWaiter API so migrated custom tools no longer keep an unresolved EditorDelay symbol. Cover both direct legacy namespace usage and files that rely on assembly-scoped legacy global usings.
Avoid starting another migration preview immediately after files were rewritten. The wizard now shows the completed no-targets state after a successful migration and only refreshes when the apply step made no changes.
Rewrite V2 EditorWindowCaptureUtility.CaptureWindowAsync calls to the V3 screenshot utility signature so partially migrated custom tools do not keep an unresolved helper reference. Cover both legacy-source and partially migrated ToolContracts-source cases.
Handle public custom tool APIs that moved out of the legacy V2 editor assembly, including screenshot helpers, main-thread switching, timer delay arguments, and ToolInfo metadata. Keep migration target assemblies reachable from predefined Editor assemblies and prevent the migration wizard from immediately starting another scan after a migration run.
Move the migration button path onto the async migration port so project scanning and file writes no longer block the Unity editor thread. Reuse the wizard progress bar during apply, prevent duplicate actions while migration is running, and keep AssetDatabase.Refresh on the main thread after writes finish.
Prefer project-declared DTO names over first-party screenshot DTO rewrites during custom tool migration. Repair already-qualified local UIElementInfo references so rerunning migration fixes files broken by earlier passes.
Keep migrated screenshot capture calls compilable for named cancellation tokens, qualified legacy calls, and ConfigureAwait suffixes.
Keep ignored awaited CaptureWindowAsync statements valid and map non-awaited legacy capture tasks back to texture task results.
Split the migration rule and file-service logic by responsibility so each extracted class stays within a reviewable size while preserving the migration behavior covered by the existing upgrade tests.
Contributor
|
Warning Review limit reached
More reviews will be available in 9 minutes and 14 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 Walkthrough📝 Walkthrough🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (8)
Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationPlanBuilder.cs (2)
22-40: ⚡ Quick winAdd precondition assert for consistency with
CreateAsync.
CreateAsync(line 180) hasDebug.Assert(!string.IsNullOrEmpty(projectRoot), ...)butCreatelacks this precondition check. For consistency with the contract programming conventions in this codebase, add the same assert here.Suggested fix
internal static MigrationPlan Create(string projectRoot) { + Debug.Assert(!string.IsNullOrEmpty(projectRoot), "projectRoot must not be null or empty"); + if (!Directory.Exists(projectRoot)) { throw new DirectoryNotFoundException(projectRoot); }🤖 Prompt for 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. In `@Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationPlanBuilder.cs` around lines 22 - 40, The Create method lacks a precondition assertion that exists in the CreateAsync method for parameter validation. Add a Debug.Assert statement at the beginning of the Create method (before the Directory.Exists check) to verify that the projectRoot parameter is not null or empty, using the same assertion pattern found in CreateAsync at line 180 to maintain consistency with the codebase's contract programming conventions.Source: Learnings
41-122: ⚖️ Poor tradeoffSignificant code duplication between sync and async C# file processing.
The C# file iteration logic (lines 41-122 in
Createand lines 215-302 inCreateAsync) is nearly identical, differing only in cancellation checks and progress reporting. Consider extracting the per-file transformation logic into a shared helper that both methods can call, passing in the source retrieval function.This is a maintainability concern as changes to the migration logic would need to be applied in two places.
Also applies to: 215-302
🤖 Prompt for 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. In `@Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationPlanBuilder.cs` around lines 41 - 122, Extract the duplicated per-file transformation logic from both the Create and CreateAsync methods into a shared helper method. This helper should contain the common logic that iterates through file processing, including the calls to FindNearestAssemblyDirectory, the repeated TryGetValue operations for assembly aliases, and the invocation of ThirdPartyToolMigrationRules.MigrateCSharpSourceForLegacyAssembly. The helper method should accept parameters for the source code, file path, assembly usage data, and other required data, then return the transformation results. Both Create and CreateAsync can then call this helper method while handling their specific concerns of cancellation checks and progress reporting separately, eliminating the need to maintain the same migration logic in two places.Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationCrossFileTimingMigrationPlanner.cs (1)
294-327: 💤 Low valueConsider using a dictionary for O(1) file lookups.
GetPendingMigrationFileContentandUpsertMigrationFileChangeboth perform linear searches through thechangeslist. For projects with many files, this results in O(files × changes) complexity per cross-file iteration. ADictionary<string, int>mapping file paths to indices would provide O(1) lookups.Given the expected size of migration plans (hundreds of files at most based on the PR description), the current approach is acceptable for this PR.
🤖 Prompt for 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. In `@Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationCrossFileTimingMigrationPlanner.cs` around lines 294 - 327, While the review comment notes a potential performance optimization to use a Dictionary<string, int> for O(1) file lookups instead of linear searches in GetPendingMigrationFileContent, the reviewer has explicitly stated this approach is acceptable for the current PR given the expected project size of hundreds of files at most. No action is required for this PR, but for future optimization if needed: refactor the changes list into a dictionary mapping file paths to their corresponding MigrationFileChange objects in GetPendingMigrationFileContent to enable direct lookups instead of iterating through the list, and apply the same optimization pattern to UpsertMigrationFileChange to eliminate the O(files × changes) complexity during cross-file iterations.Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTimingMethodDeclarationRules.cs (1)
322-322: 💤 Low valueConsider caching or pre-building the regex pattern.
A new
Regexinstance withRegexOptions.Compiledis created each timeContainsInterfaceMethodContractInBodyis called. Since method names vary, the pattern cannot be fully pre-compiled, but consider whether building the pattern withoutRegexOptions.Compiledwould be more efficient for one-shot usage, or caching results if the same method name is checked repeatedly.Given that this is called during migration which runs infrequently and the regex is relatively simple, this is a minor concern.
🤖 Prompt for 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. In `@Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTimingMethodDeclarationRules.cs` at line 322, The Regex instance in the methodRegex variable is created with RegexOptions.Compiled each time the method is called, incurring an upfront compilation cost that may not be justified for single-use pattern matching. Since method names vary and this is called during infrequent migrations, consider removing RegexOptions.Compiled from the Regex constructor if the pattern is only matched once per method name, or alternatively implement a caching mechanism (such as a static Dictionary) to store previously compiled regex patterns and reuse them when the same method name is checked repeatedly to improve efficiency.Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationMetadataConstructorRules.cs (1)
9-17: 💤 Low valueUnused imports in this file.
Newtonsoft.Json,Newtonsoft.Json.Linq, and several type aliases (ReplacementRule,TypeReplacementRule,LegacyPlayerLoopTimingParameterDeclaration,RemovedLegacyPlayerLoopTimingParameter,RemovedLegacyPlayerLoopTimingSignature) are imported but not used in this file. This pattern appears across multiple files in this PR, suggesting a shared template.Consider creating a common imports file or trimming unused imports per file to reduce cognitive load during maintenance.
🤖 Prompt for 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. In `@Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationMetadataConstructorRules.cs` around lines 9 - 17, Remove the unused imports and type aliases from the ThirdPartyToolMigrationMetadataConstructorRules.cs file. Specifically, delete the using statements for Newtonsoft.Json and Newtonsoft.Json.Linq, and remove the type alias declarations for ReplacementRule, TypeReplacementRule, LegacyPlayerLoopTimingParameterDeclaration, RemovedLegacyPlayerLoopTimingParameter, and RemovedLegacyPlayerLoopTimingSignature. Keep only the imports and aliases that are actually referenced in the rest of the file to reduce clutter and improve maintainability.Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAssemblyUsageAnalyzer.cs (1)
299-315: ⚡ Quick winUse the shared
CreateMigrationAssemblyUsagefactory to avoid sync/async drift.This return block duplicates assembly-usage materialization logic that is already centralized in
ThirdPartyToolMigrationAssemblyScopedNameMap.CreateMigrationAssemblyUsage(...). Reusing the shared factory keeps the sync and async analyzers aligned when fields evolve.Suggested diff
- return new MigrationAssemblyUsage( + return CreateMigrationAssemblyUsage( asmdefDirectories, assemblyReferenceDirectories, legacyAssemblyDirectories, assemblyScopedLegacyDirectories, assemblyScopedCurrentToolContractsDirectories, assemblyScopedCurrentApplicationDirectories, assemblyScopedCurrentFirstPartyToolsDirectories, - CreateAssemblyScopedLegacyAliasesByDirectory(assemblyScopedLegacyAliasesByDirectory), - CreateAssemblyScopedLegacyAliasesByDirectory(assemblyScopedLegacyToolInfoAliasesByDirectory), - CreateAssemblyScopedNamesByDirectory(assemblyScopedCurrentApplicationAliasesByDirectory), - CreateAssemblyScopedNamesByDirectory(assemblyScopedCurrentFirstPartyToolsAliasesByDirectory), - CreateAssemblyScopedNamesByDirectory(assemblyDeclaredTypeNamesByDirectory), + assemblyScopedLegacyAliasesByDirectory, + assemblyScopedLegacyToolInfoAliasesByDirectory, + assemblyScopedCurrentApplicationAliasesByDirectory, + assemblyScopedCurrentFirstPartyToolsAliasesByDirectory, + assemblyDeclaredTypeNamesByDirectory, toolContractsReferenceAssemblyDirectories, applicationReferenceAssemblyDirectories, domainReferenceAssemblyDirectories, firstPartyScreenshotReferenceAssemblyDirectories);🤖 Prompt for 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. In `@Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAssemblyUsageAnalyzer.cs` around lines 299 - 315, Replace the manual construction of MigrationAssemblyUsage with a call to the shared factory method CreateMigrationAssemblyUsage from ThirdPartyToolMigrationAssemblyScopedNameMap. This will eliminate duplication of the assembly-usage materialization logic and ensure that the sync and async analyzers remain aligned when the MigrationAssemblyUsage fields are modified in the future. Pass all the currently collected parameters and objects to the shared factory method instead of directly instantiating the MigrationAssemblyUsage class.Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationCodeTextMaskBuilder.cs (1)
155-268: 💤 Low valueMissing precondition asserts for consistency.
Methods
FindLineCommentEndIndex,FindBlockCommentEndIndex,FindRegularStringEndIndex,FindVerbatimStringEndIndex, andFindCharLiteralEndIndexlackDebug.Assertpreconditions forsource != nullandstartIndex >= 0, unlike other methods in this file and the codebase convention.🤖 Prompt for 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. In `@Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationCodeTextMaskBuilder.cs` around lines 155 - 268, Add Debug.Assert precondition checks to the following methods for consistency with other methods in this file and codebase convention: FindLineCommentEndIndex, FindBlockCommentEndIndex, FindRegularStringEndIndex, FindVerbatimStringEndIndex, and FindCharLiteralEndIndex. Each method should validate that the source parameter is not null and that the startIndex or quoteIndex parameter is greater than or equal to zero at the beginning of the method, similar to the pattern already used in FindRawStringEndIndex.Source: Learnings
Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationParsingRules.cs (1)
383-454: 💤 Low valueDuplicate helper methods inside
CodeTextMaskstruct.
CountRepeatedCharacter(lines 419-431) andHasRepeatedCharacterAt(lines 433-453) are exact duplicates of the module-level methods at lines 304-316 and 318-338. Consider delegating to the existing module-level helpers to reduce duplication.🤖 Prompt for 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. In `@Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationParsingRules.cs` around lines 383 - 454, The CodeTextMask struct contains duplicate implementations of the CountRepeatedCharacter and HasRepeatedCharacterAt methods that already exist as module-level helpers elsewhere in the file. Remove these duplicate method implementations from inside the CodeTextMask struct and update any internal calls to delegate to the existing module-level versions instead. This will eliminate code duplication and ensure a single source of truth for these helper utilities.
🤖 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/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationArgumentRules.cs`:
- Around line 56-192: The SplitAttributeArguments method does not track
interpolation holes within interpolated strings starting with $". When parsing
such strings (at the check for $"), the code sets isInRegularString to true but
fails to skip the nestingDepth increment/decrement logic for braces that appear
inside interpolation expressions like {value}. To fix this, add a separate flag
to track when you are inside an interpolation expression within a regular
interpolated string, and when isInRegularString is true, handle opening and
closing braces by toggling this interpolation expression flag instead of
updating nestingDepth. This ensures that braces within interpolated string
expressions do not incorrectly affect the nesting depth used for argument
splitting.
In
`@Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAssemblyReferenceResolver.cs`:
- Around line 466-495: The GetRelativePathSegments method and IsSameOrChildPath
method perform string comparisons on raw file paths without normalizing them
first, which can cause mismatches due to mixed path separators or inconsistent
casing. In GetRelativePathSegments, normalize both filePath and projectRoot
before the StartsWith check on line 471. In IsSameOrChildPath, normalize both
childPath and parentPath before performing the StartsWith comparison on line
492-494. Use Path.GetFullPath or implement a normalization helper that converts
all path separators to the OS-specific separator and handles casing consistently
to ensure accurate path ancestry and relative path calculations.
In
`@Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationModels.cs`:
- Around line 14-21: The MigrationFileChange constructor and other methods in
this file use Debug.Assert to validate preconditions but then silently recover
from violations using null-coalescing operators, defeating fail-fast semantics.
Remove the null-coalescing fallbacks (the ?? operators providing default values)
from the Content property assignment in the MigrationFileChange constructor, the
similar patterns in lines 31-37, and the collection assignments on lines
164-194. Instead, directly assign the values without recovery operators,
trusting that the Debug.Assert preconditions will catch invalid inputs early
rather than silently converting them to defaults.
In
`@Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTargetScanner.cs`:
- Around line 45-146: The legacyAssemblyDirectories HashSet is initialized but
never populated in the foreach loop that processes inventory.CSharpFilePaths
before being passed to ContainsFastCSharpSourceMigrationTargetAsync, causing it
to always be empty. Within the foreach loop where other assembly directories are
populated (inside the
ThirdPartyToolMigrationRules.ContainsMigrationCandidateText conditional block),
add checks using ThirdPartyToolMigrationRules methods to detect legacy migration
patterns in the source code and add the assemblyDirectory to
legacyAssemblyDirectories when legacy patterns are found, similar to how
assemblyScopedCurrentDomainDirectories,
assemblyScopedCurrentToolContractsDirectories, and other directories are
populated.
---
Nitpick comments:
In
`@Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAssemblyUsageAnalyzer.cs`:
- Around line 299-315: Replace the manual construction of MigrationAssemblyUsage
with a call to the shared factory method CreateMigrationAssemblyUsage from
ThirdPartyToolMigrationAssemblyScopedNameMap. This will eliminate duplication of
the assembly-usage materialization logic and ensure that the sync and async
analyzers remain aligned when the MigrationAssemblyUsage fields are modified in
the future. Pass all the currently collected parameters and objects to the
shared factory method instead of directly instantiating the
MigrationAssemblyUsage class.
In
`@Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationCodeTextMaskBuilder.cs`:
- Around line 155-268: Add Debug.Assert precondition checks to the following
methods for consistency with other methods in this file and codebase convention:
FindLineCommentEndIndex, FindBlockCommentEndIndex, FindRegularStringEndIndex,
FindVerbatimStringEndIndex, and FindCharLiteralEndIndex. Each method should
validate that the source parameter is not null and that the startIndex or
quoteIndex parameter is greater than or equal to zero at the beginning of the
method, similar to the pattern already used in FindRawStringEndIndex.
In
`@Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationCrossFileTimingMigrationPlanner.cs`:
- Around line 294-327: While the review comment notes a potential performance
optimization to use a Dictionary<string, int> for O(1) file lookups instead of
linear searches in GetPendingMigrationFileContent, the reviewer has explicitly
stated this approach is acceptable for the current PR given the expected project
size of hundreds of files at most. No action is required for this PR, but for
future optimization if needed: refactor the changes list into a dictionary
mapping file paths to their corresponding MigrationFileChange objects in
GetPendingMigrationFileContent to enable direct lookups instead of iterating
through the list, and apply the same optimization pattern to
UpsertMigrationFileChange to eliminate the O(files × changes) complexity during
cross-file iterations.
In
`@Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationMetadataConstructorRules.cs`:
- Around line 9-17: Remove the unused imports and type aliases from the
ThirdPartyToolMigrationMetadataConstructorRules.cs file. Specifically, delete
the using statements for Newtonsoft.Json and Newtonsoft.Json.Linq, and remove
the type alias declarations for ReplacementRule, TypeReplacementRule,
LegacyPlayerLoopTimingParameterDeclaration,
RemovedLegacyPlayerLoopTimingParameter, and
RemovedLegacyPlayerLoopTimingSignature. Keep only the imports and aliases that
are actually referenced in the rest of the file to reduce clutter and improve
maintainability.
In
`@Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationParsingRules.cs`:
- Around line 383-454: The CodeTextMask struct contains duplicate
implementations of the CountRepeatedCharacter and HasRepeatedCharacterAt methods
that already exist as module-level helpers elsewhere in the file. Remove these
duplicate method implementations from inside the CodeTextMask struct and update
any internal calls to delegate to the existing module-level versions instead.
This will eliminate code duplication and ensure a single source of truth for
these helper utilities.
In
`@Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationPlanBuilder.cs`:
- Around line 22-40: The Create method lacks a precondition assertion that
exists in the CreateAsync method for parameter validation. Add a Debug.Assert
statement at the beginning of the Create method (before the Directory.Exists
check) to verify that the projectRoot parameter is not null or empty, using the
same assertion pattern found in CreateAsync at line 180 to maintain consistency
with the codebase's contract programming conventions.
- Around line 41-122: Extract the duplicated per-file transformation logic from
both the Create and CreateAsync methods into a shared helper method. This helper
should contain the common logic that iterates through file processing, including
the calls to FindNearestAssemblyDirectory, the repeated TryGetValue operations
for assembly aliases, and the invocation of
ThirdPartyToolMigrationRules.MigrateCSharpSourceForLegacyAssembly. The helper
method should accept parameters for the source code, file path, assembly usage
data, and other required data, then return the transformation results. Both
Create and CreateAsync can then call this helper method while handling their
specific concerns of cancellation checks and progress reporting separately,
eliminating the need to maintain the same migration logic in two places.
In
`@Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTimingMethodDeclarationRules.cs`:
- Line 322: The Regex instance in the methodRegex variable is created with
RegexOptions.Compiled each time the method is called, incurring an upfront
compilation cost that may not be justified for single-use pattern matching.
Since method names vary and this is called during infrequent migrations,
consider removing RegexOptions.Compiled from the Regex constructor if the
pattern is only matched once per method name, or alternatively implement a
caching mechanism (such as a static Dictionary) to store previously compiled
regex patterns and reuse them when the same method name is checked repeatedly to
improve efficiency.
🪄 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
Run ID: 0839a9cb-03a7-4a2f-987b-80dde3e10a25
⛔ Files ignored due to path filters (53)
Packages/src/Editor/Application/UnityCLILoop.Application.asmdefis excluded by none and included by nonePackages/src/Editor/Domain/UnityCLILoop.Domain.asmdefis excluded by none and included by nonePackages/src/Editor/FirstPartyTools/Screenshot/UnityCLILoop.FirstPartyTools.Screenshot.Editor.asmdefis excluded by none and included by nonePackages/src/Editor/FirstPartyTools/UnityCLILoop.FirstPartyTools.Editor.asmdefis excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAliasRules.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationApiDetectionRules.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationApplicationDetectionRules.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationArgumentRules.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAsmdefMigrationRequirementResolver.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAsmdefRules.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAssemblyReferenceResolver.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAssemblyScopedNameMap.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAssemblyUsageAnalyzer.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAssemblyUsageAsyncAnalyzer.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAttributeRules.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationCSharpRules.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationCodeTextMaskBuilder.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationCodeTextMaskInterpolationRules.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationConstructorArgumentRules.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationCrossFileTimingMigrationPlanner.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationDelayRules.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationDetectionRules.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationDomainDetectionRules.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationEditorDelayRules.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFastAssemblyRequirementCollector.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFastSourceTargetDetector.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFileServiceConstants.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFileWriter.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationMetadataConstructorRules.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationModels.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationParsingRules.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationPlanBuilder.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationProjectFileInventory.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationRegexRewriteRules.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationRuleCatalog.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationScreenshotArgumentRules.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationScreenshotDeconstructionRules.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationScreenshotDetectionRules.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationScreenshotRules.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationSourceFileCache.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTargetScanner.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTimingArgumentRules.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTimingCallerRules.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTimingCleanupRules.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTimingDeclarationRules.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTimingInvocationRules.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTimingMethodBodyRules.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTimingMethodDeclarationRules.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTimingTypeNameRules.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTimingTypeResolutionRules.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTimingTypeScopeRules.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationToolContractDetectionRules.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTypeReplacementRules.cs.metais excluded by none and included by none
📒 Files selected for processing (62)
Assets/Tests/Editor/OnionAssemblyDependencyTests.csAssets/Tests/Editor/ThirdPartyToolMigrationFileServiceTests.csAssets/Tests/Editor/ThirdPartyToolMigrationRulesTests.csAssets/Tests/Editor/ThirdPartyToolMigrationWizardWindowTests.csPackages/src/Editor/Application/UseCases/ThirdPartyToolMigrationUseCase.csPackages/src/Editor/CompositionRoot/UnityCliLoopApplicationRegistration.csPackages/src/Editor/Domain/ThirdPartyToolMigrationData.csPackages/src/Editor/FirstPartyTools/Common/EditorUtility/DomainReloadDisableScopeRecovery.csPackages/src/Editor/FirstPartyTools/FirstPartyToolsEditorStartup.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAliasRules.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationApiDetectionRules.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationApplicationDetectionRules.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationArgumentRules.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAsmdefMigrationRequirementResolver.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAsmdefRules.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAssemblyReferenceResolver.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAssemblyScopedNameMap.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAssemblyUsageAnalyzer.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAssemblyUsageAsyncAnalyzer.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAttributeRules.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationCSharpRules.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationCodeTextMaskBuilder.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationCodeTextMaskInterpolationRules.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationConstructorArgumentRules.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationCrossFileTimingMigrationPlanner.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationDelayRules.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationDetectionRules.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationDomainDetectionRules.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationEditorDelayRules.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFastAssemblyRequirementCollector.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFastSourceTargetDetector.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFileService.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFileServiceConstants.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFileWriter.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationMetadataConstructorRules.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationModels.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationParsingRules.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationPlanBuilder.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationProjectFileInventory.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationRegexRewriteRules.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationRuleCatalog.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationRules.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationScreenshotArgumentRules.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationScreenshotDeconstructionRules.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationScreenshotDetectionRules.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationScreenshotRules.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationSourceFileCache.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTargetScanner.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTimingArgumentRules.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTimingCallerRules.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTimingCleanupRules.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTimingDeclarationRules.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTimingInvocationRules.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTimingMethodBodyRules.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTimingMethodDeclarationRules.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTimingTypeNameRules.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTimingTypeResolutionRules.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTimingTypeScopeRules.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationToolContractDetectionRules.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTypeReplacementRules.csPackages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardWindow.csPackages/src/Editor/ToolContracts/VibeLogger.cs
There was a problem hiding this comment.
9 issues found across 115 files
Note: This PR contains a large number of files. cubic only reviews up to 100 files per PR, so some files may not have been reviewed. cubic prioritizes the most important files to review.
On a pro plan you can use ultrareview for larger PRs.
Re-trigger cubic
Move extension-facing registrar, main-thread, metadata, and screenshot contracts into the public ToolContracts assembly so third-party tools no longer need Application, Domain, or FirstPartyTools auto references. Update migration rules to rewrite current internal namespaces to ToolContracts and add only the ToolContracts asmdef reference for migrated tools.
Extract text, state rules, view rendering, resize measurement, and progress reporting from the migration EditorWindow so the window coordinates lifecycle flow instead of owning every UI concern. Update onion assembly expectations for public ToolContracts types that were moved as part of the v3 migration surface.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Assets/Tests/Editor/OnionAssemblyDependencyTests.cs (1)
1208-1211:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPublic auto-reference whitelist now permits implementation layers.
Including
ApplicationAssemblyNameandDomainAssemblyNameinIsPublicAutoReferencedAssemblyweakens the boundary check inProjectAsmdefs_WhenLoaded_AutoReferenceOnlyPublicAssembliesand can mask unintended autoReferenced leakage from inner layers. Keep this whitelist scoped to truly public assemblies (e.g., ToolContracts and explicitly intended runtime-facing ones), or split the check into separate “public API” vs “allowed exceptions” assertions.🤖 Prompt for 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. In `@Assets/Tests/Editor/OnionAssemblyDependencyTests.cs` around lines 1208 - 1211, The IsPublicAutoReferencedAssembly method currently includes ApplicationAssemblyName and DomainAssemblyName in its whitelist, which are implementation layer assemblies rather than truly public APIs. Remove the string.Equals checks for ApplicationAssemblyName and DomainAssemblyName from this method to restrict the whitelist to only genuinely public assemblies like ToolContractsAssemblyName. This will restore the intended boundary check and prevent masking unintended auto-referenced leakage from inner layers.
🤖 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.
Outside diff comments:
In `@Assets/Tests/Editor/OnionAssemblyDependencyTests.cs`:
- Around line 1208-1211: The IsPublicAutoReferencedAssembly method currently
includes ApplicationAssemblyName and DomainAssemblyName in its whitelist, which
are implementation layer assemblies rather than truly public APIs. Remove the
string.Equals checks for ApplicationAssemblyName and DomainAssemblyName from
this method to restrict the whitelist to only genuinely public assemblies like
ToolContractsAssemblyName. This will restore the intended boundary check and
prevent masking unintended auto-referenced leakage from inner layers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 814c33a2-5596-4bc4-8526-452c352d8c59
⛔ Files ignored due to path filters (5)
Packages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationProgressReporter.cs.metais excluded by none and included by nonePackages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardStateRules.cs.metais excluded by none and included by nonePackages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardText.cs.metais excluded by none and included by nonePackages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardView.cs.metais excluded by none and included by nonePackages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardWindowResizer.cs.metais excluded by none and included by none
📒 Files selected for processing (7)
Assets/Tests/Editor/OnionAssemblyDependencyTests.csPackages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationProgressReporter.csPackages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardStateRules.csPackages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardText.csPackages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardView.csPackages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardWindow.csPackages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardWindowResizer.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- Packages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardWindow.cs
There was a problem hiding this comment.
1 issue found across 12 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Tighten migration parsing and fast-scan behavior so split global usings, interpolated attribute arguments, normalized paths, unreadable asmdef meta files, and fail-fast model contracts behave consistently during V3 migration checks. Keep larger sync/async analyzer consolidation out of this pass because those flows currently differ in cancellation, progress, and cache ownership.
There was a problem hiding this comment.
2 issues found across 17 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Handle interpolated raw string holes while scanning nested literals so raw delimiters inside hole expressions do not close the outer string early. Also fail fast on empty path ancestry arguments before path normalization.
Merged
This was referenced Jun 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
User Impact
Changes
Verification