feat: Make Unity tools easier to extend and maintain#1063
Merged
Conversation
Move the public tool extension surface into a dedicated ToolContracts assembly so first-party and extension tools share the same contract boundary. Remove manual built-in tool registration and cover the shared registry path with focused EditMode tests.
Drop the display description field from the UnityCLILoop tool attribute and runtime catalogs because skills now own agent-facing tool guidance. This keeps the tool contract focused on registration and execution metadata.
Move low-dependency bundled tools into the first-party plugin assembly so the physical onion split starts with a low-risk slice. The new registry coverage proves those bundled plugin tools still register as first-party while depending only on the public tool contract surface.
Drop the MCP-era ping and debug-sleep tools because they are not part of the extension-facing tool surface and no current CLI path depends on them. Keep get-version for now as a CLI readiness bridge command and document the future split between internal bridge commands and public tool registration.
Keep get-version available for CLI readiness checks without exposing it through the extension-facing tool registry. Route the command before normal tool execution and document the internal bridge boundary for future CLI-only commands.
Focus-window is handled by the native Go CLI before tool dispatch, so the Unity-side registration stub only kept an obsolete tool surface alive. Remove the stub and document that FirstPartyTools remains only as a future plugin boundary until real bundled tools move there.
Keep get-tool-details available for CLI list and sync without registering the catalog command as an extension-facing Unity tool. Route the catalog request through the internal bridge command path and keep registry tests covering the hidden command surface.
Lock the current onion assembly boundaries in tests so future moves cannot accidentally introduce presentation, infrastructure, or composition-root dependencies in the wrong direction. Reference Domain from Application and include first-party tools in the composition root wiring surface to match the intended layer graph.
Use control-play-mode as the first bundled plugin example so first-party tools can prove the same attribute registration path as third-party tools without referencing the application assembly. Keep the tool and its skill definition under UnityCLILoop.FirstPartyTools.Editor while preserving the ToolContracts-only asmdef boundary.
Add toolName metadata to the moved control-play-mode skill so tool-scoped skill synchronization continues to identify it after the first-party plugin move. Cover the first-party skill source path with a regression test and keep the generated skill copies in sync with the source definition.
Verify the Hello World sample executes through the same UnityCLILoop typed contract path as bundled tools, and record the facade prerequisite before moving presentation code.
Introduce a CLI setup facade so settings and setup windows no longer call installer internals directly. This prepares the presentation layer move without exposing Application internals across module boundaries.
Use UnityCliLoopSettingsWindow naming for the settings UI files, docs, and tests so the presentation surface no longer exposes the legacy MCP window name. Route skill setup actions through SkillSetupApplicationFacade while preserving the onion dependency guard for UI code.
Keep presentation-facing settings code behind ToolSettingsApplicationFacade so the UI no longer depends directly on tool settings persistence, registry warmup, or catalog internals before the physical presentation move.
Compile the settings, setup, and server editor windows under UnityCLILoop.Presentation after routing their setup and tool-settings dependencies through application facades. Keep recordings UI in the application assembly until its feature-internal dependencies have a proper facade.
Keep side-effect-free policy values in UnityCLILoop.Domain so application, presentation, and shared services depend inward on the same domain definitions instead of carrying them in adapter-oriented folders.
Route the recordings editor window through an Application facade before moving it into the Presentation assembly, so the view no longer depends on recorder, replayer, overlay, or file-helper internals. Keep the onion boundary enforced with tests that reject EditorWindow implementations under the legacy UI folder and direct Presentation references to recording service internals.
The communication log utility had no production callers and kept the old MCP naming alive under the legacy UI folder. Remove the dead logger, its transient settings, and the remaining UI directory so presentation code is no longer mixed with obsolete MCP-era support code.
Use UnityCliLoop naming for shared editor UI constants so presentation and error-handling code no longer depend on a legacy MCP-facing type name.
Use UnityCliLoopEditorSettings for editor state persistence so non-protocol settings code no longer carries the legacy MCP type name.
Use UnityCliLoop naming for the editor domain reload state provider because it stores editor process state rather than protocol-facing MCP behavior.
Use UnityCliLoopPathResolver for project and package path resolution because this utility is part of Unity CLI Loop naming, not MCP protocol behavior.
Use UnityCliLoopVersion because the package version belongs to the Unity CLI Loop platform name, not the MCP protocol implementation.
Use UnityCliLoopSecurityChecker and UnityCliLoopSecurityException because execution security is platform policy, not MCP protocol code. Remove unused command compatibility members while touching the policy boundary.
Keep the editor domain reload provider and its registration under Unity CLI Loop naming so non-protocol platform code no longer exposes the legacy MCP name.
Use UnityCliLoopLogType for GetLogs filtering because log categories are part of the Unity CLI Loop tool contract, not MCP protocol code.
Use UnityCliLoopConstants for protocol-independent package paths, command names, and limits so shared platform code no longer exposes the legacy MCP type name.
Use UnityCliLoopServerConfig for project IPC and JSON-RPC bridge settings because these constants are Unity CLI Loop infrastructure, not MCP extension contracts.
Delete obsolete comments that pointed at removed MCP-era session and logging types so production source no longer teaches outdated architecture names.
Use UnityCliLoopBridgeServer, UnityCliLoopServerController, and related lifecycle use cases because the project IPC server belongs to the Unity CLI Loop infrastructure naming surface.
Clean up broad imports left by the layer namespace split so files only depend on namespaces whose symbols they actually use.
Strip namespace imports that were retained because comments and string literals confused the previous cleanup pass.
Keep the metadata validation dependencies next to the first-party tool that owns them instead of exposing them as top-level package plugins.
Expose the bundled-tool startup entrypoint directly so composition root no longer needs InternalsVisibleTo access to initialize first-party tools.
Document class-level responsibilities consistently so the refactored assembly layout is easier to scan without exposing more internal implementation detail.
Document interface-level boundaries consistently with the class summaries so assembly and module contracts are easier to scan.
Trim stale constants and static helpers from the shared contract surface so the remaining values reflect current package behavior.
Drop dead constants and unreferenced helper types left behind after the tool contract cleanup so the public surface only exposes active APIs.
Build a repository-local analysis tool that reconstructs Unity asmdef projects in Roslyn and reports unused production symbols without adding dependencies to the Unity package runtime. The scanner supports configurable scope, member/local detection, test-only classification, and Unity/reflection keeper reporting.
Use the new Roslyn dead code scanner to prune private implementation details that have no production or non-production references, while teaching the scanner to preserve RuntimeInitializeOnLoadMethod entry points.
Prune production-unreferenced ExecuteDynamicCode helper classes and their stale tests. Also keep types that host Unity-invoked members out of the dead-code candidate list so scanner output stays actionable.
Remove public-facing methods, enum members, and helper types that scanner found without production or test references. This trims legacy debug and compatibility surfaces before deciding on higher-risk DTO and contract members.
Replace the unused prewarm use case and telemetry surface with a module-local startup prewarmer that is actually requested during first-party tool startup, preserving first-request performance while removing stale service reset and capability plumbing.
Move long-form tool parameter guidance out of runtime schema attributes so the CLI contract stays structural and skill files remain the source of agent-facing instructions.
Drop IDE-only browsing metadata from the runtime schema because it does not affect CLI schema generation and keeps the tool contract focused on structural parameters.
Restore friendly dynamic-code errors, first-party runtime reset behavior, and internal bridge response versions found during the review loop. Update custom-tool documentation and the Go package-root probe to match the current first-party tool layout.
Allow execute-dynamic-code missing-return recovery to retry even when the compiler reports wrapped updated code, and reset startup prewarm request state after transient incomplete attempts. Tighten the static facade guard so target-typed new initializers cannot hide mutable static fields.
Keep execute-dynamic-code recovery from rewriting raw class or namespace input after compiler-shaped diagnostics. The retry path now only appends return null for top-level script snippets where the shorthand return behavior applies.
There was a problem hiding this comment.
1 issue found across 1305 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/FirstPartyTools/ClearConsole/Skill/SKILL.md">
<violation number="1" location="Packages/src/Editor/FirstPartyTools/ClearConsole/Skill/SKILL.md:48">
P2: This documentation change weakens the clear-console output contract: it no longer states that failure details are in `Message` while `ErrorMessage` stays empty.
(Based on your team's feedback about keeping clear-console failure details in `Message` and `ErrorMessage` empty.) [FEEDBACK_USED]</violation>
</file>
Note: This PR contains a large number of files. cubic only reviews up to 75 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.
Keep the generated skill documentation aligned with the tool contract: clear-console failure details are read from Message while ErrorMessage remains empty for this tool.
There was a problem hiding this comment.
1 issue found across 45 files (changes from recent commits).
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/FirstPartyTools/ExecuteDynamicCode/DynamicCodeFriendlyErrorConverter.cs">
<violation number="1" location="Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeFriendlyErrorConverter.cs:13">
P2: Use `CompilationErrors` as the first source when deriving friendly error guidance; relying only on `ErrorMessage`/`Logs` misses compiler diagnostics and degrades failures to generic messages.
(Based on your team's feedback about checking `CompilationErrors` first for execute-dynamic-code failures.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Read structured CompilationErrors before fallback error text so execute-dynamic-code can keep known compiler failures mapped to friendly CLI guidance even when the pattern is absent from logs.
This was referenced May 5, 2026
Merged
Closed
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
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
Notes