Unity MCP CI Test Improvements #452
Conversation
…_exit(code) and use timers with args\n- Consistent behavior on BrokenPipeError (no immediate exit)\n- Exit code 1 on unexpected exceptions\n\nTests: restore telemetry module after disabling to avoid bleed-over
…e _force_exit(code) and use timers with args\n- Consistent behavior on BrokenPipeError (no immediate exit)\n- Exit code 1 on unexpected exceptions\n\nTests: restore telemetry module after disabling to avoid bleed-over" This reverts commit 74d35d3.
* 'main' of https://github.com/CoplayDev/unity-mcp: Harden PlayMode test runs (CoplayDev#396) Enable the `rmcp_client` feature so it works with Codex CLI (CoplayDev#395) chore: bump version to 8.0.0 HTTP Server, uvx, C# only custom tools (CoplayDev#375) [CUSTOM TOOLS] Roslyn Runtime Compilation Feature (CoplayDev#371)
* 'main' of github.com:dsarno/unity-mcp:
* 'main' of https://github.com/CoplayDev/unity-mcp: chore: bump version to 8.1.4 Fix Claude Windows config and CLI status refresh (CoplayDev#412) chore: bump version to 8.1.3 fix: restrict fastmcp version to avoid potential KeyError (CoplayDev#411) chore: bump version to 8.1.2 Revert project script change chore: bump version to 8.1.1 Fix CLI entry point path in pyproject.toml (CoplayDev#407) Fix manage prefabs (CoplayDev#405) Remove automatic version bumping from README files and switch to unversioned git URLs chore: bump version to 8.1.0 Add distribution settings for Asset Store vs git defaults (CoplayDev#404) Add CodeBuddy CLI configurator (CoplayDev#403) Fix stdio reloads (CoplayDev#402) Simplify MCP client configs (CoplayDev#401) chore: bump version to 8.0.1
* 'main' of github.com:dsarno/unity-mcp:
* 'main' of github.com:dsarno/unity-mcp: Revert "Server: refine shutdown logic per bot feedback\n- Parameterize _force_exit(code) and use timers with args\n- Consistent behavior on BrokenPipeError (no immediate exit)\n- Exit code 1 on unexpected exceptions\n\nTests: restore telemetry module after disabling to avoid bleed-over" Server: refine shutdown logic per bot feedback\n- Parameterize _force_exit(code) and use timers with args\n- Consistent behavior on BrokenPipeError (no immediate exit)\n- Exit code 1 on unexpected exceptions\n\nTests: restore telemetry module after disabling to avoid bleed-over Update github-repo-stats.yml
* unity-tests-fork-backup: (26 commits) diag fixes Cleanup Fixes to improve CI testing Add MCP preflight workflow and update NL suite another try Point MCP server to src/server.py and fix preflight Fix YAML indentation for MCP preflight step Use latest claude sonnet/haiku models in workflow Use available Anthropic models for T pass Write Unity logs to file and list config dir in debug Allow Unity config dir writable for MCP heartbeat/logs Mount Unity cache rw and dump Editor log for MCP debug Allow Unity local share writable for MCP status Fail fast when Unity MCP status file missing Add Unity bridge debug step in CI update yaml Fix CI: share unity-mcp status dir update claude haiku version for NL/T tests Harden NL suite prompts for deterministic anchors ...
WalkthroughAdds CI preflight and expanded NL-suite workflows, a fork-specific Unity test workflow, a Unity editor CI bootstrap entrypoint, environment-configurable stdio bridge status/heartbeat handling and reload behavior, a new server-side file-search tool, port-discovery robustness for reloading instances, and small metadata adjustments. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
Server/src/transport/legacy/port_discovery.py (2)
279-281: Consider moving the import to the top of the file.The
timezoneimport inside the conditional block works but is unconventional. Moving it to the top-level imports improves readability and avoids repeated import overhead if this code path is hit multiple times.-from datetime import datetime +from datetime import datetime, timezoneThen update the usage:
if freshness.tzinfo: - from datetime import timezone now = datetime.now(timezone.utc)
275-290: Consider inverting the condition to eliminate the emptypassblock.The current logic uses
passwith a comment. Inverting the condition improves readability by making the "skip" case explicit.if not is_alive: - # If Unity says it's reloading and the status is fresh, don't drop the instance. - freshness = last_heartbeat or file_mtime - now = datetime.now() - if freshness.tzinfo: - from datetime import timezone - now = datetime.now(timezone.utc) - - age_s = (now - freshness).total_seconds() - - if is_reloading and age_s < 60: - pass # keep it, status="reloading" - else: - logger.debug( - f"Instance {project_name}@{hash_value} has heartbeat but port {port} not responding") - continue + # If Unity says it's reloading and the status is fresh, keep the instance + freshness = last_heartbeat or file_mtime + now = datetime.now() + if freshness.tzinfo: + now = datetime.now(timezone.utc) + + age_s = (now - freshness).total_seconds() + + if not (is_reloading and age_s < 60): + logger.debug( + f"Instance {project_name}@{hash_value} has heartbeat but port {port} not responding") + continue.claude/prompts/nl-unity-suite-nl.md (2)
106-109: Clarify anchor selection logic and fallback behavior for NL-1.Line 107 states "Prefer
GetCurrentTarget()if unique; otherwise use another unique method such asApplyBlend," but the validation step (line 108) doesn't explicitly address what happens if neither preferred anchor is unique or if neither exists. The term "unique" is ambiguous—does it mean "appears exactly once in the file" or "has a distinctive signature"?Recommend adding:
- A definition of "unique" (e.g., "appears exactly once by method signature").
- A fallback strategy if both preferred anchors fail to satisfy uniqueness (e.g., use
find_in_fileto select the first occurrence of the anchor, or abort with clear error).- An explicit validation expectation (similar to NL-2 line 116: "Expect exactly one match; if multiple, fail clearly").
115-119: Consider documenting the regex pattern rationale and testing it against edge cases.The anchor pattern for
Update()is well-crafted for robustness (optional access modifiers, flexible whitespace), but the complexity warrants a brief explanation. For instance:
- Why does it tolerate
(?:public|private|protected|internal)?but Update() is typically public?- Does the pattern correctly exclude method overloads (e.g.,
Update(float))?Recommend adding a comment explaining the tolerance level and test coverage assumptions (e.g., "Assumes single parameterless Update() method per Unity class").
MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs (1)
1022-1028: Consider extracting status directory resolution.The status directory resolution logic (reading
UNITY_MCP_STATUS_DIRwith fallback) is duplicated betweenStop()(lines 466-471) andWriteHeartbeat(). Extracting this to a helper method would reduce duplication and improve maintainability.Apply this diff to extract the logic:
+private static string GetStatusDirectory() +{ + string dir = Environment.GetEnvironmentVariable("UNITY_MCP_STATUS_DIR"); + if (string.IsNullOrWhiteSpace(dir)) + { + dir = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".unity-mcp"); + } + return dir; +} + public static void Stop() { // ... existing code ... try { - string dir = Environment.GetEnvironmentVariable("UNITY_MCP_STATUS_DIR"); - if (string.IsNullOrWhiteSpace(dir)) - { - dir = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".unity-mcp"); - } + string dir = GetStatusDirectory(); string statusFile = Path.Combine(dir, $"unity-mcp-status-{ComputeProjectHash(Application.dataPath)}.json"); // ... rest of Stop logic ... } } public static void WriteHeartbeat(bool reloading, string reason = null) { try { - string dir = Environment.GetEnvironmentVariable("UNITY_MCP_STATUS_DIR"); - if (string.IsNullOrWhiteSpace(dir)) - { - dir = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".unity-mcp"); - } + string dir = GetStatusDirectory(); Directory.CreateDirectory(dir); // ... rest of WriteHeartbeat logic ... } }Server/src/services/tools/find_in_file.py (1)
72-76: Unused parameter acknowledged but consider documenting the intent.The
project_rootparameter is flagged as unused by static analysis (ARG001). The inline comment on line 76 explains it's kept for interface consistency, which is reasonable. Consider using a leading underscore (_project_root) to suppress the linter warning and signal intent more clearly.- project_root: Annotated[str | None, "Optional project root path"] = None, + _project_root: Annotated[str | None, "Optional project root path (reserved for future use)"] = None,Then remove or update the comment on line 76.
.github/workflows/claude-nl-suite.yml (1)
512-525: Preflight check validates CLI loading, not MCP connectivity.The retry loop (lines 512-525) only runs
--helpto verify the CLI loads, not that it can connect to Unity. The actual connectivity is tested by the Python probe above (lines 487-510). Consider adding a comment clarifying this is a CLI sanity check, not a full connectivity test.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.claude/prompts/nl-unity-suite-nl.md(2 hunks).github/workflows/claude-mcp-preflight.yml(1 hunks).github/workflows/claude-nl-suite.yml(17 hunks).github/workflows/github-repo-stats.yml(1 hunks).github/workflows/unity-tests-fork.yml(1 hunks).github/workflows/unity-tests.yml(1 hunks)MCPForUnity/Editor/McpCiBoot.cs(1 hunks)MCPForUnity/Editor/McpCiBoot.cs.meta(1 hunks)MCPForUnity/Editor/Services/StdioBridgeReloadHandler.cs(2 hunks)MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs(2 hunks)Server/src/services/tools/find_in_file.py(1 hunks)Server/src/transport/legacy/port_discovery.py(2 hunks)TestProjects/UnityMCPTests/Assets/Scripts/Editor.meta(0 hunks)
💤 Files with no reviewable changes (1)
- TestProjects/UnityMCPTests/Assets/Scripts/Editor.meta
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 316
File: TestProjects/UnityMCPTests/Assets/Tests/EditMode/Resources.meta:1-8
Timestamp: 2025-10-13T13:27:23.040Z
Learning: UnityMcpBridge is a legacy project kept for backwards compatibility; MCPForUnity is the only active Unity plugin project. GUID collisions between UnityMcpBridge and MCPForUnity are acceptable.
📚 Learning: 2025-10-13T13:27:23.040Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 316
File: TestProjects/UnityMCPTests/Assets/Tests/EditMode/Resources.meta:1-8
Timestamp: 2025-10-13T13:27:23.040Z
Learning: UnityMcpBridge is a legacy project kept for backwards compatibility; MCPForUnity is the only active Unity plugin project. GUID collisions between UnityMcpBridge and MCPForUnity are acceptable.
Applied to files:
MCPForUnity/Editor/Services/StdioBridgeReloadHandler.csMCPForUnity/Editor/McpCiBoot.csMCPForUnity/Editor/McpCiBoot.cs.meta
📚 Learning: 2025-09-04T01:01:11.927Z
Learnt from: dsarno
Repo: CoplayDev/unity-mcp PR: 260
File: UnityMcpBridge/UnityMcpServer~/src/server_version.txt:1-1
Timestamp: 2025-09-04T01:01:11.927Z
Learning: The UnityMcpBridge project is not maintaining changelogs yet, so don't suggest adding changelog entries for version bumps.
Applied to files:
MCPForUnity/Editor/Services/StdioBridgeReloadHandler.csMCPForUnity/Editor/McpCiBoot.cs.meta
📚 Learning: 2025-10-13T13:41:00.086Z
Learnt from: JohanHoltby
Repo: CoplayDev/unity-mcp PR: 309
File: MCPForUnity/Editor/Helpers/ServerInstaller.cs:478-508
Timestamp: 2025-10-13T13:41:00.086Z
Learning: In the MCPForUnityTools feature (MCPForUnity/Editor/Helpers/ServerInstaller.cs), the design intentionally forces users to have only one .py file per MCPForUnityTools folder to keep file tracking simple. Package-style tools (subdirectories with __init__.py) are not supported.
Applied to files:
Server/src/services/tools/find_in_file.pyMCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs
📚 Learning: 2025-11-27T21:09:35.011Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 401
File: MCPForUnity/Editor/Clients/Configurators/VSCodeConfigurator.cs:10-18
Timestamp: 2025-11-27T21:09:35.011Z
Learning: VS Code GitHub Copilot MCP configuration supports mcp.json files placed directly in the Code/User directory: %APPDATA%\Code\User\mcp.json on Windows, ~/Library/Application Support/Code/User/mcp.json on macOS, and ~/.config/Code/User/mcp.json on Linux. This is in addition to workspace-scoped .vscode/mcp.json files.
Applied to files:
MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs
📚 Learning: 2025-09-05T16:22:04.960Z
Learnt from: dsarno
Repo: CoplayDev/unity-mcp PR: 265
File: README.md:204-204
Timestamp: 2025-09-05T16:22:04.960Z
Learning: In the Unity MCP project, the ServerInstaller.cs creates a symlink from ~/Library/AppSupport to ~/Library/Application Support on macOS to mitigate argument parsing and quoting issues in some MCP clients. The README documentation should use the shortened AppSupport path, not the full "Application Support" path with spaces.
Applied to files:
MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs
📚 Learning: 2025-09-05T16:22:04.960Z
Learnt from: dsarno
Repo: CoplayDev/unity-mcp PR: 265
File: README.md:204-204
Timestamp: 2025-09-05T16:22:04.960Z
Learning: In the Unity MCP project, the ServerInstaller.cs creates a symlink from ~/Library/AppSupport to ~/Library/Application Support on macOS to avoid spaces in paths that can cause arg parsing and quoting issues in some MCP clients. The README documentation should use the shortened AppSupport path, not the full "Application Support" path.
Applied to files:
MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs
🧬 Code graph analysis (2)
MCPForUnity/Editor/McpCiBoot.cs (2)
MCPForUnity/Editor/Constants/EditorPrefKeys.cs (1)
EditorPrefKeys(7-48)MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs (2)
StdioBridgeHost(122-171)StartAutoConnect(83-101)
Server/src/services/tools/find_in_file.py (2)
Server/src/transport/unity_transport.py (1)
send_with_unity_instance(78-103)Server/src/transport/legacy/unity_connection.py (1)
async_send_command_with_retry(753-785)
🪛 Ruff (0.14.8)
Server/src/services/tools/find_in_file.py
72-72: Unused function argument: project_root
(ARG001)
🔇 Additional comments (26)
Server/src/transport/legacy/port_discovery.py (1)
34-44: LGTM!Good refactoring to delegate path construction to
get_registry_dir, avoiding duplication. The environment variable override forUNITY_MCP_STATUS_DIRis clean and allows flexible configuration for testing and custom deployments..claude/prompts/nl-unity-suite-nl.md (1)
122-132: NL-3 pattern is safe for the current test file structure.The pattern
(?m)^\\s*}\\s*$works correctly forLongUnityScriptClaudeTest.csbecause the file has a straightforward, single-class structure with no nested classes or properties with block bodies. The last closing brace in the file (line 2037) is indeed the class closing brace at indentation 0, as the pattern assumes.The file's structure—300+ padding methods all at class level—ensures that selecting the final brace-only line reliably targets the class closing brace. If the file structure changes to include nested classes or properties with block bodies, the concern becomes valid and a more specific anchor (such as matching
public classline or using a sentinel comment) would be necessary. For the current test design, no changes are required.Likely an incorrect or invalid review comment.
.github/workflows/github-repo-stats.yml (1)
4-7: LGTM! Scheduled runs disabled.The cron-based schedule trigger has been disabled while preserving manual workflow_dispatch execution. This is appropriate if automated stats collection is being replaced or deferred.
.github/workflows/unity-tests.yml (1)
14-15: LGTM! Fork protection added.The owner guard ensures Unity tests only run on the upstream repository, preventing unauthorized use of Unity licenses and protecting repository secrets on forks. This aligns with the new fork-specific workflow.
MCPForUnity/Editor/McpCiBoot.cs.meta (1)
1-12: LGTM! Standard Unity meta file.The meta file correctly registers the McpCiBoot.cs editor script with Unity, using a unique GUID and standard MonoImporter configuration.
MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs (2)
466-471: LGTM! Configurable status directory for CI.Environment variable support via
UNITY_MCP_STATUS_DIRenables flexible status-file placement in CI/testing environments while maintaining backward compatibility with the default user-profile location.
1018-1018: LGTM! WriteHeartbeat exposed for reload signaling.Making
WriteHeartbeatpublic enablesStdioBridgeReloadHandlerto signal reload status to clients, improving the stdio bridge lifecycle management during domain reloads.MCPForUnity/Editor/Services/StdioBridgeReloadHandler.cs (2)
28-31: LGTM! Dual-check for CI bootstrap compatibility.The additional
StdioBridgeHost.IsRunningcheck ensures reload handling works correctly when the stdio bridge is started directly via CI bootstrap (McpCiBoot.StartStdioForCi) bypassingTransportManagerstate. The inline comment clearly documents the rationale.
40-45: LGTM! Improved reload signaling.The synchronous stop with timeout followed by
WriteHeartbeat(true, "reloading")ensures clients receive reload status even when the bridge is stopped, improving the client experience during domain reloads.MCPForUnity/Editor/McpCiBoot.cs (1)
10-19: LGTM! Clean CI bootstrap entry point.The
StartStdioForCi()method provides a straightforward entry point for CI workflows to initialize the stdio bridge. The defensive try-catch aroundEditorPrefsensures robustness if preferences are unavailable, and callingStdioBridgeHost.StartAutoConnect()is the correct approach for establishing the bridge..github/workflows/unity-tests-fork.yml (6)
19-20: Verify fork owner guard matches intended user.The workflow is guarded to run only when
github.repository_owner == 'dsarno', which should match the PR author and fork owner. Ensure this is the intended configuration.
27-67: LGTM! Comprehensive credential detection.The multi-stage credential detection correctly identifies ULF license files, entitlement-based licensing (EBL) with email/password, and optional serial numbers. The outputs enable flexible license activation strategies downstream.
69-96: LGTM! Robust ULF staging with fallbacks.The ULF staging step handles base64-encoded licenses with a fallback to plain-text, and intelligently detects misplaced entitlement XML files (which should be in the licenses directory instead). This defensive approach improves reliability across different secret configurations.
98-136: LGTM! Flexible Unity activation supporting Pro and named-user EBL.The activation step correctly handles both Pro licenses (with serial) and named-user entitlement-based licensing via Docker host-mount. The fallback to ULF-only execution when entitlements are unavailable ensures the workflow doesn't fail prematurely.
138-159: LGTM! Warm-up step prevents cold-start issues.Running an initial import pass to populate the Library folder before tests prevents cold-start performance issues and improves test reliability.
162-186: LGTM! Test execution with proper license handling.The test step correctly passes the manual license file argument when ULF is available and executes editmode tests with proper result formatting and artifact output.
.github/workflows/claude-mcp-preflight.yml (2)
22-35: LGTM! Flexible dependency installation.The installation step supports both modern
pyproject.tomland legacyrequirements.txtwith clear error handling if neither is found.
37-53: LGTM! No-Unity preflight validation.The preflight step correctly validates the MCP server can initialize in stdio mode without a running Unity instance. The dummy status file and
UNITY_MCP_STATUS_DIRenvironment variable align with the configurability changes inStdioBridgeHost.cs. Using--helpis appropriate for validation without requiring actual Unity connectivity.Server/src/services/tools/find_in_file.py (2)
15-64: LGTM on URI parsing logic.The
_split_urihelper correctly handlesunity://path/,file://, and plain path formats, with proper percent-decoding, UNC path handling, Windows drive-letter normalization, and Assets-relative path extraction.
128-164: LGTM on regex search and result mapping.The match iteration, line number calculation, and excerpt extraction logic is sound. Properly handles edge cases like matches at EOF (line 149-150) and caps results to avoid payload bloat.
.github/workflows/claude-nl-suite.yml (6)
18-18: Runner pinned to specific Ubuntu version.Pinning to
ubuntu-24.04instead ofubuntu-latestis good for reproducibility. However, be aware this will require manual updates when newer runners are needed.
439-448: Shell script generation looks correct.The f-string correctly interpolates
{workspace}and{uv_cmd}while preserving bash constructs ($LOG,$@,$(...)) since Python's f-strings only interpret{}placeholders. The stderr-to-log redirect with stdout for MCP communication is the right approach.
819-822: Good: Relaxed regex patterns handle inconsistent agent output.The patterns
NL-?(\d+)andT-?([A-J])now accept both hyphenated (NL-0,T-A) and non-hyphenated (NL0,TA) variants from agent output, while the normalization logic ensures consistent canonical format with hyphens.
1208-1219: LGTM: Debug log added to artifacts.Including
.unity-mcp/mcp-server-startup-debug.login artifacts aids post-mortem debugging of MCP server startup issues.
277-312: Good observability instrumentation.The debug step provides comprehensive diagnostics: container state, logs (with credential redaction), status directory listings, and port probes. The fail-fast on missing status file prevents wasted CI time.
647-648: Model identifiers are valid Anthropic snapshots. Anthropic publishes dated model IDs (e.g.,claude-sonnet-4-5-20250929) for reproducible behavior across environments, which is the recommended practice for CI/CD workflows. Theclaude-sonnet-4-5-20250929identifier matches its Sep 29, 2025 release date exactly. Theclaude-haiku-4-5-20251001identifier uses Oct 1 as the snapshot date, which aligns with Haiku's Oct 15 announcement timeline. No action needed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
Server/src/services/tools/find_in_file.py (2)
72-76: Silence Ruff ARG001 forproject_rootwhile keeping the interface stable.
project_rootis intentionally unused but will keep triggering Ruff’s ARG001. To preserve the public signature while satisfying linting, consider annotating it with anoqacomment.- project_root: Annotated[str | None, "Optional project root path"] = None, + project_root: Annotated[str | None, "Optional project root path"] = None, # noqa: ARG001 - kept for interface compatibility
152-160: Preserve indentation in line excerpts by avoidingstrip().Using
line_content.strip()removes leading spaces, which can make C# lines harder to recognize in context. Prefer trimming only the trailing newline so indentation is preserved.- "content": line_content.strip(), # detailed match info? + "content": line_content.rstrip("\n"), # preserve indentation, strip only newline
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Server/src/services/tools/find_in_file.py(1 hunks)Server/src/services/tools/manage_script.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-13T13:41:00.086Z
Learnt from: JohanHoltby
Repo: CoplayDev/unity-mcp PR: 309
File: MCPForUnity/Editor/Helpers/ServerInstaller.cs:478-508
Timestamp: 2025-10-13T13:41:00.086Z
Learning: In the MCPForUnityTools feature (MCPForUnity/Editor/Helpers/ServerInstaller.cs), the design intentionally forces users to have only one .py file per MCPForUnityTools folder to keep file tracking simple. Package-style tools (subdirectories with __init__.py) are not supported.
Applied to files:
Server/src/services/tools/find_in_file.py
🧬 Code graph analysis (1)
Server/src/services/tools/find_in_file.py (3)
Server/src/transport/unity_transport.py (1)
send_with_unity_instance(78-103)Server/src/transport/legacy/unity_connection.py (1)
async_send_command_with_retry(753-785)Server/src/services/tools/manage_script.py (1)
_split_uri(14-63)
🪛 Ruff (0.14.8)
Server/src/services/tools/find_in_file.py
72-72: Unused function argument: project_root
(ARG001)
🔇 Additional comments (3)
Server/src/services/tools/manage_script.py (1)
120-129: Encoded contents guard now correctly requires both flags before decoding.The updated condition
if not contents and data.get("contentsEncoded") and data.get("encodedContents"):closes the earlier hole where an absentencodedContentscould lead to decoding an empty string. The fallback on decode failure remains safe, and this matches the pattern used in the other tools.Server/src/services/tools/find_in_file.py (2)
15-64: URI normalization helper looks robust and consistent with manage_script.
_split_uricorrectly normalizesunity://path/...,file://..., and plain paths, handles UNC/file URIs, and derives an Assets-relative directory where possible. This matches the behavior inServer/src/services/tools/manage_script.pyand should give consistent name/path resolution across tools.
98-107: Base64 decoding guard and fallback behavior are sound.The condition
if not contents and data.get("contentsEncoded") and data.get("encodedContents"):ensures you only attempt base64 decoding when both flags are present, preventing silent decoding of an empty string. Falling back tocontents or ""on decode failure is a safe, predictable behavior for the search tool.
1 participant
Bringing the CI testing framework back up-to-date / online, after it broke during the v8 upgrades. Includes some bug fixes to Stdio brdige and logging, and lots of fiddling with the CI workflow file. Re-addded the
find_in_filepython tool as it has disappeared at one point, and claude script editing uses that.