Sync upstream main 20250830 132219

[dsarno]

Summary by CodeRabbit

• New Features

  • Powerful Unity script editing tools (text and structured edits) with validation and atomic batches.
  • Framed MCP protocol for more reliable connections and ping/pong health checks.
  • macOS config path support and improved path handling; added logging helper.
  • Editor action to retrieve project root.
  • New Unity test script asset for NL/T scenarios.

• CI/Chores

  • Added full and mini NL/T GitHub Actions workflows; JUnit post-processor to mark environment failures as skipped.
  • Version bumps and ignore updates.

• Documentation

  • Added NL/T suite specs and CI usage guidance; expanded tool descriptions.

• Tests

  • Extensive coverage for framing, URI handling, edits, resources, and options forwarding.

[coderabbitai]

Walkthrough

Adds CI workflows and prompts for a Unity NL/T editing suite, introduces framed MCP transport and tooling (text/structured edits, resources), extends editor helpers and logging, adds a long Unity test script, new Python server tools/config, a JUnit post-processor, and extensive tests. Versions bumped to 3.1.x; some legacy files removed/ignored.

Changes

Cohort / File(s)	Summary
CI Workflows & Orchestration .github/workflows/claude-nl-suite.yml, .github/workflows/claude-nl-suite-mini.yml	New GitHub Actions to run Unity NL/T suites with MCP bridge, artifact handling, summaries, and gating on secrets.
JUnit Post-Processing .github/scripts/mark_skipped.py	New script converts environment/precondition failures in JUnit XML to skipped; updates testsuite counts.
Prompts & Docs (NL/T Suite) .claude/prompts/nl-unity-suite-full.md, .claude/prompts/nl-unity-claude-tests-mini.md, README-DEV.md	Adds CI agent contracts and developer docs for NL/T workflows, reporting, and targets.
Readme Tools README.md	Documents new tools: apply_text_edits, script_apply_edits, validate_script.
Unity Test Asset TestProjects/UnityMCPTests/Assets/Scripts/LongUnityScriptClaudeTest.cs, ...LongUnityScriptClaudeTest.cs.meta	Adds large MonoBehaviour script with target/animation scaffolding for NL tests and associated meta.
Unity Test Project Adjustments TestProjects/UnityMCPTests/Packages/packages-lock.json (deleted), .../ProjectSettings/.../Settings.json	Removes lockfile; trims two metadata fields from code coverage settings.
Repo Ignore .gitignore	Ignores Unity test project lockfile.
Editor Models & Clients UnityMcpBridge/Editor/Models/McpClient.cs, UnityMcpBridge/Editor/Data/McpClients.cs	Adds macConfigPath and populates macOS config paths for known clients.
Editor Helpers (Logging/Config/Detection/Responses) UnityMcpBridge/Editor/Helpers/McpLog.cs(+meta), ConfigJsonBuilder.cs, PackageDetector.cs(+meta), Response.cs, ServerInstaller.cs	New logging helper; macOS path remap for Cursor; one-time legacy detection/auto-install; error payload includes code; installer gains version-aware install, legacy cleanup, macOS symlink, path helpers, and GetServerPath().
Editor Bridge (Transport/Windowing) Editor/MCPForUnityBridge.cs, Editor/Windows/MCPForUnityEditorWindow.cs, Editor/Windows/ManualConfigEditorWindow.cs, Editor/Windows/VSCodeManualSetupWindow.cs	Introduces framed, length-prefixed protocol with handshake and timeouts; richer startup/shutdown logs; project root/config handling updates; VSCode guidance switches to mcp.json; macOS path selection split from Linux.
Server Python: Transport & Config UnityMcpBridge/UnityMcpServer~/src/unity_connection.py, .../config.py, .../pyproject.toml, .../pyrightconfig.json, .../server_version.txt	Adds framed handshake/IO with heartbeats and locking; new config fields for framing behavior; bumps version to 3.1.0/3.1.1; adds Pyright config and version file.
Server Python: Tools Registration .../tools/__init__.py	Switches to logging; registers new tools (script edits, resources, gameobject, asset); exposes resource wrappers.
Server Python: Script Tools .../tools/manage_script.py, .../tools/manage_script_edits.py	Adds URI normalization, apply_text_edits, create/delete/validate/capabilities/get_sha; routes legacy update; new structured/text edit tool with previews, preconditions, validation.
Server Python: Resource Tools .../tools/resource_tools.py	New safe URI-based list/read/find API under Assets with project root discovery.
Server Python: Minor .../tools/manage_asset.py	Adds trailing newline (no logic change).
Utilities & Tests test_unity_socket_framing.py, tests/*	Adds transport, tooling, normalization, resource, and placeholder tests; stdout guard for server code.
Misc Docs & Formatting claude-chunk.md (removed), mcp_source.py	Removes macOS troubleshooting doc; adds trailing newline.
Version Bumps UnityMcpBridge/package.json	Bumps package version to 3.1.0.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor GitHubActions as GitHub Actions
  participant Docker as Unity Container (MCP Bridge)
  participant Unity as Unity Editor Bridge
  participant ClaudeAction as Claude Code Action
  participant ServerPy as MCP Server (Python)

  GitHubActions->>Docker: Start container with bridge entrypoint
  Docker->>Unity: Launch Editor + Bridge
  Unity-->>GitHubActions: Write heartbeat/status

  GitHubActions->>ClaudeAction: Run NL/T job with MCP config
  ClaudeAction->>ServerPy: Register tools (script/resource/asset/scene/...)
  ClaudeAction->>Unity: manage_editor.get_project_root (framed)
  Unity-->>ClaudeAction: response {projectRoot}

  loop For each NL/T test
    ClaudeAction->>Unity: manage_script.apply_text_edits / edit (framed)
    Unity-->>ClaudeAction: response {success|error, sha, diagnostics}
    ClaudeAction-->>GitHubActions: Append JUnit fragment + MD
  end

  GitHubActions->>GitHubActions: Merge/normalize JUnit, mark skipped
  GitHubActions-->>GitHubActions: Publish reports/artifacts
  GitHubActions->>Docker: Stop container

Loading

sequenceDiagram
  autonumber
  participant Client as MCP Client (Python)
  participant Socket as TCP Socket
  participant Editor as Unity Bridge

  Client->>Socket: connect()
  Editor-->>Client: "WELCOME UNITY-MCP 1 FRAMING=1\n"
  Client->>Editor: [8-byte len][JSON command]
  Editor->>Editor: Validate, route command
  alt ping fast-path
    Editor-->>Client: [8-byte len]["pong"]
  else normal response
    Editor-->>Client: [8-byte len][JSON result]
  end
  note over Client,Editor: Heartbeats allowed as zero-length frames (bounded)

Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120+ minutes

Possibly related PRs

• CI: add NL suite full + mini workflows (sanitized logs, normalized JU… #55 — Also adds Claude NL/T CI workflows and orchestration; overlaps with workflows introduced here.
• feat: add initial script edit tests #3 — Touches manage_script and apply_text_edits tooling; directly related to the script-edit changes.
• CI: add Claude NL suite workflow, prompt, and long Unity test script … #30 — Adds NL/T Unity test suite components; aligns with prompts and long test script added here.

Suggested labels

codex

Poem

Hop-hop, I wire the bridge so neat,
Frames go by with timed heartbeat.
Scripts now dance to edits’ tune,
Tests bloom nightly, artifact moon.
JUnit whispers, “skipped, not fail!”
3.1 sails on carrot-gale.
— (^._.^)ﾉ🐇

✨ Finishing Touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch sync-upstream-main-20250830-132219

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

• Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

• Visit our Status Page to check the current availability of CodeRabbit.
• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[greptile-apps]

Greptile Summary

This pull request represents a major synchronization from upstream main that significantly enhances the Unity MCP Bridge system with substantial new functionality and improvements. The changes introduce a comprehensive upgrade from version 3.0.0 to 3.1.0/3.1.1, implementing advanced text editing capabilities, robust CI testing infrastructure, and enhanced cross-platform compatibility.

Core Functionality Enhancements:
The PR introduces sophisticated script editing tools (apply_text_edits, script_apply_edits, validate_script) that provide precise, range-based text modifications with SHA-256 precondition checking, overlap detection, and atomic batching. These tools support both LSP-style ranges and index-based editing, with intelligent normalization between coordinate systems.

Protocol Improvements:
A major upgrade implements strict framed communication protocol replacing legacy text-based messaging. The new binary protocol uses 64MB frame limits with big-endian headers, includes heartbeat mechanisms, and provides enhanced timeout handling for improved reliability.

CI/Testing Infrastructure:
Extensive automated testing capabilities are added via GitHub Actions workflows that spin up headless Unity environments, execute Claude-based natural language editing tests, and generate comprehensive JUnit reports. The testing suite includes both 'mini' and 'full' variants for different CI scenarios.

Cross-Platform Enhancements:
Significant improvements for macOS compatibility include dedicated configuration paths for all MCP clients, symlink creation to handle spaces in Application Support paths, and enhanced path resolution logic. The system now properly distinguishes between Windows, Linux, and macOS requirements.

Security and Reliability:
Robust security measures are implemented including path traversal protection, symlink attack prevention, Assets directory access restrictions, and comprehensive input validation. The system includes atomic file operations with backup/recovery mechanisms and debounced refresh systems.

Developer Experience:
New tooling includes centralized logging via McpLog class, comprehensive debug capabilities, package detection with automatic legacy cleanup, resource wrapper tools for MCP clients, and enhanced error handling with structured response codes.

Important Files Changed

Click to expand file changes

Filename	Score	Overview
UnityMcpBridge/Editor/Tools/ManageScript.cs	3/5	Major refactoring adding sophisticated text editing, security protections, and structured editing with potential complexity/compatibility concerns
UnityMcpBridge/Editor/MCPForUnityBridge.cs	4/5	Implements strict framed I/O protocol with binary communication, timeouts, and enhanced reliability
UnityMcpBridge/Editor/Helpers/ServerInstaller.cs	3/5	Comprehensive installation system with version management, legacy cleanup, and aggressive process killing operations
UnityMcpBridge/UnityMcpServer~/src/tools/manage_script_edits.py	4/5	New comprehensive Python script editing system with complex routing logic and extensive validation
UnityMcpBridge/UnityMcpServer~/src/tools/manage_script.py	4/5	Major refactoring expanding from 29 to 562 lines with advanced text editing and URI parsing capabilities
UnityMcpBridge/UnityMcpServer~/src/unity_connection.py	4/5	Implements framed protocol with strict handshake, thread safety, and enhanced error handling
UnityMcpBridge/Editor/Windows/MCPForUnityEditorWindow.cs	4/5	Significant refactoring with enhanced debug logging, framed protocol verification, and improved configuration handling
.github/workflows/claude-nl-suite.yml	4/5	Comprehensive CI workflow for automated Unity MCP testing with Claude integration and extensive error handling
.github/workflows/claude-nl-suite-mini.yml	4/5	Streamlined CI workflow variant for faster Unity MCP testing with proper secret handling and diagnostics
UnityMcpBridge/UnityMcpServer~/src/tools/resource_tools.py	4/5	New resource wrapper tools with comprehensive path resolution and security restrictions
UnityMcpBridge/Editor/Data/McpClients.cs	4/5	Adds macOS configuration paths for all MCP clients with proper Application Support directory handling
UnityMcpBridge/Editor/Helpers/ConfigJsonBuilder.cs	4/5	Adds macOS-specific path handling with symlink workarounds for Cursor compatibility
UnityMcpBridge/Editor/Helpers/PackageDetector.cs	4/5	New automatic legacy installation detection with version-scoped checks and cleanup operations
UnityMcpBridge/Editor/Windows/VSCodeManualSetupWindow.cs	4/5	Updates configuration workflow from settings.json to mcp.json format following VSCode changes
.claude/prompts/nl-unity-suite-full.md	4/5	Comprehensive CI agent contract for Unity testing with detailed specifications and potential complexity issues
.claude/prompts/nl-unity-claude-tests-mini.md	4/5	Simplified Unity testing prompt for faster CI validation with autonomous operation requirements
tests/test_regex_delete_guard.py	4/5	Comprehensive test coverage for regex validation preventing structural code damage
tests/test_transport_framing.py	4/5	Critical transport protocol testing with handshake validation and frame-based communication
tests/test_resources_api.py	4/5	Security-focused testing for file access patterns and directory traversal protection
UnityMcpBridge/Editor/Windows/ManualConfigEditorWindow.cs	4/5	Improved platform-specific configuration handling with macOS/Linux separation and fallback mechanisms
README-DEV.md	4/5	Comprehensive documentation for new CI testing workflow with detailed instructions and debugging guidance
UnityMcpBridge/Editor/Helpers/Response.cs	4/5	Enhanced error response structure adding machine-parsable 'code' field while maintaining compatibility
UnityMcpBridge/UnityMcpServer~/src/config.py	4/5	Adds framed protocol configuration parameters for heartbeat timeout and frame limiting
test_unity_socket_framing.py	4/5	New test script for Unity's socket framing protocol with large payload testing capabilities
TestProjects/UnityMCPTests/Packages/packages-lock.json	4/5	Removed lock file to enable fresh package resolution during upstream sync
TestProjects/UnityMCPTests/ProjectSettings/boot.config	4/5	Emptied Unity boot configuration file to use default settings for standardized test environment
claude-chunk.md	4/5	Removed macOS Claude CLI troubleshooting documentation, potentially impacting user support
tests/test_edit_normalization_and_noop.py	4/5	Tests for text edit normalization and no-op detection with some incomplete assertions
tests/test_edit_strict_and_warnings.py	4/5	Tests strict validation behavior for zero-based index handling with proper warning generation
tests/test_script_tools.py	4/5	Comprehensive unit tests for script and asset management tools with mock infrastructure
tests/test_logging_stdout.py	4/5	Static analysis tests ensuring clean logging practices using AST parsing
tests/test_get_sha.py	4/5	Unit tests for SHA functionality with URI parsing validation
tests/test_manage_script_uri.py	4/5	URI parsing tests covering Unity, file, and plain path formats
.github/scripts/mark_skipped.py	4/5	Post-processing script converting environmental test failures to skipped status
UnityMcpBridge/Editor/Tools/ManageEditor.cs	5/5	Safe addition of get_project_root command with proper error handling and path normalization
README.md	5/5	Documentation update for new script editing tools with clear descriptions
UnityMcpBridge/package.json	5/5	Standard version bump from 3.0.0 to 3.1.0 as part of upstream sync
UnityMcpBridge/UnityMcpServer~/src/pyproject.toml	5/5	Version synchronization from 3.0.0 to 3.1.0 aligning with Unity package version
UnityMcpBridge/UnityMcpServer~/src/server_version.txt	5/5	New version tracking file containing '3.1.1' for server installation management
UnityMcpBridge/Editor/Models/McpClient.cs	5/5	Simple addition of macConfigPath field for macOS configuration support
UnityMcpBridge/Editor/Helpers/McpLog.cs	5/5	New centralized logging utility with consistent formatting and debug mode control
TestProjects/UnityMCPTests/Assets/Scripts/LongUnityScriptClaudeTest.cs	5/5	New test script for Claude natural language editing with extensive padding methods
tests/test_script_editing.py	5/5	Placeholder test structure defining comprehensive testing scope for script editing features
UnityMcpBridge/UnityMcpServer~/src/pyrightconfig.json	5/5	Standard Pyright configuration for improved Python development experience
mcp_source.py	5/5	Minor formatting fix adding trailing newline
.gitignore	5/5	Standard addition excluding Unity test project lock files
UnityMcpBridge/UnityMcpServer~/src/tools/manage_asset.py	5/5	Cosmetic formatting change adding trailing newline
TestProjects/UnityMCPTests/ProjectSettings/Packages/com.unity.testtools.codecoverage/Settings.json	5/5	Configuration cleanup removing optional metadata fields
UnityMcpBridge/Editor/Helpers/PackageDetector.cs.meta	5/5	Standard Unity meta file for PackageDetector.cs asset tracking
UnityMcpBridge/Editor/Helpers/McpLog.cs.meta	5/5	Standard Unity meta file for McpLog.cs asset tracking
TestProjects/UnityMCPTests/Assets/Scripts/LongUnityScriptClaudeTest.cs.meta	5/5	Standard Unity meta file for test script asset tracking
UnityMcpBridge/UnityMcpServer~/src/tools/init.py	4/5	Adds new tool registrations and logging capabilities with priority ordering for script edits

Confidence score: 4/5

• This PR represents a major feature upgrade with comprehensive improvements but includes complex changes that require careful validation
• Score reflects significant new functionality with robust testing but concerns around complexity and potential breaking changes in core components
• Pay close attention to ManageScript.cs and ServerInstaller.cs for their extensive modifications and potential impact on existing workflows

Sequence Diagram

sequenceDiagram
    participant User
    participant GitHub as GitHub Actions
    participant Unity as Unity Container
    participant MCP as MCP Server
    participant Claude as Claude Agent
    participant Scripts as Unity Scripts

    User->>GitHub: Trigger workflow dispatch
    GitHub->>GitHub: Setup environment (Python, uv, Unity)
    GitHub->>Unity: Start Unity container with MCP Bridge
    Unity->>Unity: Initialize MCP Bridge on dynamic port
    Unity->>GitHub: Write status to ~/.unity-mcp/status.json
    
    GitHub->>MCP: Install and start MCP server
    MCP->>Unity: Connect via framed protocol
    Unity->>MCP: Send handshake "FRAMING=1"
    MCP->>Unity: Verify connection with ping/pong
    
    GitHub->>Claude: Launch Claude agent with MCP config
    Claude->>MCP: Connect via stdio transport
    Claude->>Claude: Discover available tools and capabilities
    
    Note over Claude: Execute NL test suite
    Claude->>MCP: list_resources(pattern="*.cs", under="Assets")
    MCP->>Unity: Forward request
    Unity->>MCP: Return script list
    MCP->>Claude: Return available scripts
    
    Claude->>MCP: read_resource("unity://path/Assets/.../LongScript.cs")
    MCP->>Unity: Forward read request
    Unity->>Scripts: Read file contents
    Scripts->>Unity: Return file contents
    Unity->>MCP: Return script content
    MCP->>Claude: Return script content
    
    loop For each test (NL-1 through NL-4, T-A through T-J)
        Claude->>MCP: apply_text_edits or script_apply_edits
        MCP->>Unity: Forward edit request with precondition SHA
        Unity->>Unity: Validate precondition and apply edits
        Unity->>Scripts: Write modified script
        Unity->>Unity: Schedule asset refresh and compilation
        Unity->>MCP: Return success/failure
        MCP->>Claude: Return edit result
        
        Claude->>MCP: validate_script(level="standard")
        MCP->>Unity: Forward validation request
        Unity->>Unity: Run syntax and structure checks
        Unity->>MCP: Return validation diagnostics
        MCP->>Claude: Return validation results
        
        Claude->>Claude: Write test results to reports/*.xml
        
        Claude->>MCP: Restore script from snapshot
        MCP->>Unity: Apply restore operation
        Unity->>Scripts: Restore original content
    end
    
    Claude->>GitHub: Write JUnit XML and Markdown reports
    GitHub->>GitHub: Normalize and publish test results
    GitHub->>GitHub: Upload artifacts and create job summary
    GitHub->>Unity: Stop Unity container

Loading

_{52 files reviewed, 27 comments}

_{Edit Code Review Bot Settings | Greptile}

[greptile-apps]

style: missing newline at end of file - Unity typically expects meta files to end with a newline character

[greptile-apps]

logic: Potential issue: first_match_text could remain None if the first matching node has no text content, but a later node does have text. Consider capturing text from any matching node, not just the first.

[greptile-apps]

style: The unusual formatting with line breaks inside the ternary operator hurts readability and doesn't follow typical C# conventions.

Suggested change

	displayPath = string.IsNullOrEmpty(mcpClient.macConfigPath)
	? configPath
	: mcpClient.macConfigPath;
	displayPath = string.IsNullOrEmpty(mcpClient.macConfigPath)
	? configPath
	: mcpClient.macConfigPath;

[greptile-apps]

style: Missing blank line between MCP Connection Debugging section and Workflow section creates formatting inconsistency

Suggested change

	- In CI, the job tails Unity logs (redacted for serial/license/password/token) and prints socket/status JSON diagnostics if startup fails.
	## Workflow
	- In CI, the job tails Unity logs (redacted for serial/license/password/token) and prints socket/status JSON diagnostics if startup fails.
	## Workflow

[greptile-apps]

style: System.Runtime.InteropServices is imported but not used in this file

Suggested change

	using System.Runtime.InteropServices;
	using System;
	using System.Collections.Generic;
	using System.IO;
	using MCPForUnity.Editor.Models;

[greptile-apps]

style: duplicate import pytest statements

Suggested change

	import pytest
	import sys
	from pathlib import Path
	import pytest
	import pytest
	import sys
	from pathlib import Path

[greptile-apps]

style: Header guard prevents editing before first 'using' statement but could be overly restrictive for valid use cases like adding new using statements

[greptile-apps]

style: Magic number regex with hardcoded threshold of 5 matches - this heuristic may produce false positives and should be configurable

[greptile-apps]

style: Class definition should follow PEP 8 style with proper spacing

[greptile-apps]

logic: debug preview imports difflib but doesn't use it, and the preview logic is incomplete - should either implement or remove

[coderabbitai]

Actionable comments posted: 10

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)

UnityMcpBridge/Editor/Models/McpClient.cs (1)

13-29: Fix NRE in Error status handling (SetStatus ➜ GetStatusDisplayString).

Calling SetStatus(Error) without errorDetails dereferences a null configStatus via GetStatusDisplayString. Make Error handling self‑contained and null‑safe.

Apply this diff:

         public string GetStatusDisplayString()
         {
             return status switch
             {
                 McpStatus.NotConfigured => "Not Configured",
                 McpStatus.Configured => "Configured",
                 McpStatus.Running => "Running",
                 McpStatus.Connected => "Connected",
                 McpStatus.IncorrectPath => "Incorrect Path",
                 McpStatus.CommunicationError => "Communication Error",
                 McpStatus.NoResponse => "No Response",
                 McpStatus.UnsupportedOS => "Unsupported OS",
                 McpStatus.MissingConfig => "Missing MCPForUnity Config",
-                McpStatus.Error => configStatus.StartsWith("Error:") ? configStatus : "Error",
+                McpStatus.Error => (!string.IsNullOrEmpty(configStatus) && configStatus.StartsWith("Error:")) ? configStatus : "Error",
                 _ => "Unknown",
             };
         }

         // Helper method to set both status enum and string for backward compatibility
         public void SetStatus(McpStatus newStatus, string errorDetails = null)
         {
             status = newStatus;

-            if (newStatus == McpStatus.Error && !string.IsNullOrEmpty(errorDetails))
-            {
-                configStatus = $"Error: {errorDetails}";
-            }
-            else
-            {
-                configStatus = GetStatusDisplayString();
-            }
+            if (newStatus == McpStatus.Error)
+            {
+                configStatus = !string.IsNullOrEmpty(errorDetails) ? $"Error: {errorDetails}" : "Error";
+                return;
+            }
+            configStatus = GetStatusDisplayString();
         }

Also applies to: 32-45

UnityMcpBridge/Editor/Windows/ManualConfigEditorWindow.cs (1)

173-191: Open file robustly (expand ~ and env vars; handle missing file).

Current Process.Start will fail for paths containing ~ or when the file doesn’t exist. Expand and guard; fall back to revealing the containing folder.

Apply this diff:

-            if (
-                GUILayout.Button(
-                    "Open File",
-                    copyButtonStyle,
-                    GUILayout.Height(25),
-                    GUILayout.Width(100)
-                )
-            )
-            {
-                // Open the file using the system's default application
-                System.Diagnostics.Process.Start(
-                    new System.Diagnostics.ProcessStartInfo
-                    {
-                        FileName = displayPath,
-                        UseShellExecute = true,
-                    }
-                );
-            }
+            if (GUILayout.Button("Open File", copyButtonStyle, GUILayout.Height(25), GUILayout.Width(100)))
+            {
+                var pathToOpen = ExpandPath(displayPath);
+                if (System.IO.File.Exists(pathToOpen))
+                {
+                    System.Diagnostics.Process.Start(new System.Diagnostics.ProcessStartInfo
+                    {
+                        FileName = pathToOpen,
+                        UseShellExecute = true,
+                    });
+                }
+                else
+                {
+                    var dir = System.IO.Path.GetDirectoryName(pathToOpen);
+#if UNITY_EDITOR
+                    if (!string.IsNullOrEmpty(dir))
+                        EditorUtility.RevealInFinder(dir);
+                    else
+                        EditorUtility.DisplayDialog("Path not found", $"Could not find: {pathToOpen}", "OK");
+#endif
+                }
+            }

Add this helper and using directives outside the snippet:

// Add at top:
using System;
using System.IO;

// Add inside class:
private static string ExpandPath(string path)
{
    if (string.IsNullOrWhiteSpace(path)) return path;
    var expanded = Environment.ExpandEnvironmentVariables(path);
    if (expanded.StartsWith("~"))
    {
        var home = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile);
        expanded = Path.Combine(home, expanded.Substring(1).TrimStart('/', '\\'));
    }
    return expanded;
}

UnityMcpBridge/Editor/Windows/VSCodeManualSetupWindow.cs (1)

114-133: Fix Linux path: non‑Windows falls into macOS path currently.
Else branch hardcodes Library/Application Support, which is wrong on Linux (should be ~/.config/Code/User). Add explicit macOS vs Linux handling.

-            if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
+            if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
             {
                 displayPath = System.IO.Path.Combine(
                     System.Environment.GetFolderPath(System.Environment.SpecialFolder.ApplicationData),
                     "Code",
                     "User",
                     "mcp.json"
                 );
             }
-            else 
+            else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
             {
                 displayPath = System.IO.Path.Combine(
                     System.Environment.GetFolderPath(System.Environment.SpecialFolder.UserProfile),
                     "Library",
                     "Application Support",
                     "Code",
                     "User",
                     "mcp.json"
                 );
             }
+            else // Linux and others
+            {
+                displayPath = System.IO.Path.Combine(
+                    System.Environment.GetFolderPath(System.Environment.SpecialFolder.UserProfile),
+                    ".config",
+                    "Code",
+                    "User",
+                    "mcp.json"
+                );
+            }

UnityMcpBridge/Editor/Windows/MCPForUnityEditorWindow.cs (1)

1492-1504: Dev-mode detection checks only legacy package id

IsDevelopmentMode searches for "com.justinpbarnett.unity-mcp" only; miss current id "com.coplaydev.unity-mcp". Include both to ensure dev mode is detected.

- if (manifestContent.IndexOf("\"com.justinpbarnett.unity-mcp\"", StringComparison.OrdinalIgnoreCase) >= 0)
+ if (manifestContent.IndexOf("\"com.justinpbarnett.unity-mcp\"", StringComparison.OrdinalIgnoreCase) >= 0
+  || manifestContent.IndexOf("\"com.coplaydev.unity-mcp\"", StringComparison.OrdinalIgnoreCase) >= 0)
 {
-   int idx = manifestContent.IndexOf("com.justinpbarnett.unity-mcp", StringComparison.OrdinalIgnoreCase);
+   int idx = manifestContent.IndexOf("com.coplaydev.unity-mcp", StringComparison.OrdinalIgnoreCase);
+   if (idx < 0) idx = manifestContent.IndexOf("com.justinpbarnett.unity-mcp", StringComparison.OrdinalIgnoreCase);
    // Crude but effective: check for "file:" in the same line/value
    if (manifestContent.IndexOf("file:", idx, StringComparison.OrdinalIgnoreCase) >= 0
        && manifestContent.IndexOf("\n", idx, StringComparison.OrdinalIgnoreCase) > manifestContent.IndexOf("file:", idx, StringComparison.OrdinalIgnoreCase))
    {
      return true;
    }
 }

🧹 Nitpick comments (92)

mcp_source.py (3)

39-47: Broaden SSH origin normalization and ensure .git suffix.

Handle ssh://git@github.com/... and enforce https URL with .git to avoid Unity UPM resolution issues.

 def normalize_origin_to_https(url: str) -> str:
-    """Map common SSH origin forms to https for Unity's git URL scheme."""
-    if url.startswith("git@github.com:"):
-        owner_repo = url.split(":", 1)[1]
-        if owner_repo.endswith(".git"):
-            owner_repo = owner_repo[:-4]
-        return f"https://github.com/{owner_repo}.git"
-    # already https or file: etc.
-    return url
+    """Map common SSH origin forms to https for Unity's git URL scheme."""
+    m = re.match(r'^(?:ssh://)?git@github\.com[:/](?P<owner>[^/]+)/(?P<repo>[^/]+?)(?:\.git)?$', url)
+    if m:
+        return f"https://github.com/{m.group('owner')}/{m.group('repo')}.git"
+    # already https or file: etc.
+    return url if url.endswith(".git") or not url.startswith("https://github.com/") else url + ".git"

---

94-109: Tighten option URLs: use constant for path, ensure .git on origin, and prefer file URI.

• Keep upstream path in sync via BRIDGE_SUBPATH.
• If origin lacks .git, append it.
• Use Path.as_uri() for the local file URI (clearer across OSes). Please verify Unity accepts file:/// URIs in your environment.

 def build_options(repo_root: pathlib.Path, branch: str, origin_https: str):
-    upstream = "git+https://github.com/CoplayDev/unity-mcp.git?path=/UnityMcpBridge"
+    upstream = f"git+https://github.com/CoplayDev/unity-mcp.git?path=/{BRIDGE_SUBPATH}"
     # Ensure origin is https
-    origin = origin_https
+    origin = origin_https if origin_https.endswith(".git") else (origin_https + ".git" if origin_https.startswith("https://github.com/") else origin_https)
@@
-        ("[2] Remote current branch", f"{origin_remote}?path=/{BRIDGE_SUBPATH}#{branch}"),
-        ("[3] Local workspace", f"file:{(repo_root / BRIDGE_SUBPATH).as_posix()}"),
+        ("[2] Remote current branch", f"{origin_remote}?path=/{BRIDGE_SUBPATH}#{branch}"),
+        ("[3] Local workspace", (repo_root / BRIDGE_SUBPATH).as_uri()),

---

62-64: Handle detached HEAD when deriving branch.

Fallback to commit SHA so option [2] remains usable outside branch tips.

 def detect_branch(repo: pathlib.Path) -> str:
-    return run_git(repo, "rev-parse", "--abbrev-ref", "HEAD")
+    name = run_git(repo, "rev-parse", "--abbrev-ref", "HEAD")
+    if name == "HEAD":
+        # Detached HEAD: use short SHA
+        return run_git(repo, "rev-parse", "--short", "HEAD")
+    return name

UnityMcpBridge/UnityMcpServer~/src/pyrightconfig.json (2)

5-10: Broaden analysis scope and add ignores.

If tests live outside src, Pyright won’t analyze them from this location. Also add common excludes to cut noise.

Apply:

 {
   "typeCheckingMode": "basic",
   "reportMissingImports": "warning",
   "pythonVersion": "3.11",
+  "exclude": [
+    "**/__pycache__",
+    "**/.venv",
+    "**/node_modules",
+    "**/.mypy_cache",
+    "**/build",
+    "**/dist"
+  ],
   "executionEnvironments": [
     {
       "root": ".",
       "pythonVersion": "3.11"
-    }
+    }
+    // Add another env here if tests are outside this folder, e.g. {"root": "../tests", "pythonVersion": "3.11", "extraPaths": ["./"] }
   ]
 }

---

2-4: Optionally raise type-checking over time.

Consider “standard” (or stricter for src only) once the codebase settles; keep “basic” for tests.

TestProjects/UnityMCPTests/Assets/Scripts/LongUnityScriptClaudeTest.cs.meta (1)

1-2: Unity .meta seems minimal; verify Unity won’t auto-regenerate it.

Typical .cs .meta files include a MonoImporter block. Please open the project once in Unity and confirm this file isn’t rewritten. If Unity regenerates it, commit the regenerated version to avoid churn.

UnityMcpBridge/package.json (1)

3-3: Version bump OK; double-check cross-version alignment (3.1.0 vs 3.1.1).

package.json is at 3.1.0 while server_version.txt is 3.1.1 per PR notes. If this mismatch is intentional (client vs server), please document it; otherwise align versions and update any release notes.

UnityMcpBridge/Editor/Helpers/PackageDetector.cs.meta (1)

1-2: Meta file is minimal and missing trailing newline.

Add a trailing newline and verify Unity won’t regenerate this with a MonoImporter block. If it does, commit the Unity-generated version.

UnityMcpBridge/UnityMcpServer~/src/tools/manage_asset.py (1)

79-79: Remove unused import and normalize return in manage_asset.py

• At the top of src/tools/manage_asset.py, remove the unused import time.
• On line 79, replace

  return result if isinstance(result, dict) else {"success": False, "message": str(result)}

  with

  return result if isinstance(result, dict) and "success" in result else {"success": False, "error": str(result)}

(Optional) For consistency across all tools, consider standardizing failure responses to use an "error" key instead of "message".

.gitignore (1)

36-38: LGTM—good call ignoring macOS cruft and the Unity test lockfile.

Optional: consider also ignoring common Unity project folders under TestProjects/UnityMCPTests to prevent accidental noise (Library/, Temp/, Logs/, Obj/). Only if those dirs can appear locally for these tests.

UnityMcpBridge/UnityMcpServer~/src/config.py (1)

20-22: Add guardrails for framed_receive settings
Include a __post_init__ in ServerConfig to validate your new fields:

 def __post_init__(self):
     if self.framed_receive_timeout <= 0:
         raise ValueError("framed_receive_timeout must be > 0")
     if self.max_heartbeat_frames < 0:
         raise ValueError("max_heartbeat_frames must be >= 0")

These fields are consumed in unity_connection.py (lines 129 and 137), so this will surface misconfigurations early.

UnityMcpBridge/Editor/Helpers/ConfigJsonBuilder.cs (2)

66-73: Prefer path-segment aware replacement over string Contains/Replace.

The string replace works but is brittle if the path contains the text elsewhere. Consider splitting into segments and swapping the exact segment, or verifying prefix with StartsWith before replacing.

- if (directory.Contains(canonical))
- {
-     var candidate = directory.Replace(canonical, symlinkSeg).Replace('\\', '/');
+ if (directory.Replace('\\','/').StartsWith(canonical, StringComparison.Ordinal))
+ {
+     var candidate = symlinkSeg + directory.Replace('\\','/').Substring(canonical.Length);
      if (System.IO.Directory.Exists(candidate))
      {
          effectiveDir = candidate;
      }
- }
+ }

---

57-59: Gate macOS remap to Cursor explicitly. Replace the non-VSCode client check with an explicit mcpType == McpTypes.Cursor to avoid affecting other clients:

- bool isCursor = !isVSCode && (client == null || client.mcpType != McpTypes.VSCode);
+ bool isCursor = client != null && client.mcpType == McpTypes.Cursor;

tests/test_logging_stdout.py (3)

22-24: Consider xfail instead of a hard skip so regressions surface when unskipped.

Marking as xfail keeps the test visible in reports and encourages eventual enablement.

-@pytest.mark.skip(reason="TODO: ensure server logs only to stderr and rotating file")
+@pytest.mark.xfail(reason="TODO: ensure server logs only to stderr and rotating file", strict=False)

---

67-68: Ruff S101 in tests—silence rule for this file or test dir.

Asserts are idiomatic in pytest. Either disable S101 for tests in your ruff config or add a per-file ignore.

Options:

• pyproject.toml:
  [tool.ruff.per-file-ignores]
  "tests/**.py" = ["S101"]
• Or at top of this file:

  ruff: noqa: S101

---

47-66: Catches print and sys.stdout.write—nice. Consider alias coverage later.

Non-blocking: this misses “import sys as s; s.stdout.write(...)”. Fine for now.

tests/test_script_editing.py (1)

4-6: Avoid XPASS for placeholder tests; skip module instead of xfail+pass.

As written, these pass bodies will report XPASS (green) under xfail(strict=False), which can hide missing coverage. Prefer a module-level skip until implemented.

Apply this minimal change at the top:

 import pytest

+# Pending: scaffold only; enable when concrete assertions exist
+pytestmark = pytest.mark.skip(reason="pending: implement script-editing tests")
+
 @pytest.mark.xfail(strict=False, reason="pending: create new script, validate, apply edits, build and compile scene")
 def test_script_edit_happy_path():
-    pass
+    ...

Then drop the per-test xfail marks or leave them as comments for future TODOs.

Also applies to: 9-12, 14-17, 19-22, 24-27, 29-32, 34-36

.github/scripts/mark_skipped.py (3)

67-69: Keep root aggregate counts consistent.

You recompute per-suite tallies but leave the root aggregates stale. Update them when root.tag == "testsuites".

     for ts in suites:
         ...
         ts.set("skipped", str(skipped))

-    if changed:
+    # Update root aggregate counts when applicable
+    if root.tag == "testsuites":
+        total = {"tests": 0, "failures": 0, "errors": 0, "skipped": 0}
+        for ts in suites:
+            for k in total.keys():
+                total[k] += int(ts.get(k, "0"))
+        for k, v in total.items():
+            root.set(k, str(v))
+
+    if changed:
         tree.write(path, encoding="utf-8", xml_declaration=True)

Also applies to: 92-99, 100-105

---

17-30: Pattern scope check.

Keywords like “permission” and “approval” may be overbroad and could mask genuine failures. Consider scoping with surrounding context (e.g., “PermissionPrompt”, “autoApprove=” tokens) or anchoring to full lines where possible.

---

84-91: Preserve provenance in .

Include the original node tag (“failure”/“error”) to aid triage.

-                skip = ET.SubElement(case, "skipped")
+                skip = ET.SubElement(case, "skipped")
                 skip.set("message", reason)
-                skip.text = first_match_text or reason
+                skip.set("from", nodes[0].tag)
+                skip.text = first_match_text or reason

test_unity_socket_framing.py (2)

65-76: Read a full greeting line; narrow exception types.

Single recv(256) risks truncating “FRAMING=1”. Read a line with a short timeout and avoid bare Exception.

-        s.settimeout(2)
-        # Read optional greeting
-        try:
-            greeting = s.recv(256)
-        except Exception:
-            greeting = b""
+        s.settimeout(2)
+        try:
+            f = s.makefile("rb", buffering=0)
+            greeting = f.readline(256)
+        except (OSError, socket.timeout):
+            greeting = b""

---

31-47: Legacy read loop can spin decoding full buffer repeatedly.

Minor: consider validating JSON only when the last byte suggests an object end (e.g., ‘}’ or ‘]’) to cut redundant parses under large payloads.

TestProjects/UnityMCPTests/Assets/Scripts/LongUnityScriptClaudeTest.cs (1)

25-29: Optional: prefer [SerializeField] private fields for tunables.

Keeps inspector-visible while preserving encapsulation; not required for test asset.

-    public float maxReachDistance = 2f;
-    public float maxHorizontalDistance = 1.0f;
-    public float maxVerticalDistance = 1.0f;
+    [SerializeField] private float maxReachDistance = 2f;
+    [SerializeField] private float maxHorizontalDistance = 1.0f;
+    [SerializeField] private float maxVerticalDistance = 1.0f;

Expose read-only properties if needed by tests.

.claude/prompts/nl-unity-suite-full.md (3)

57-63: Specify code-fence language for JSON snippet.

Helps linters and readability.

-```json
+```json
 {"op":"regex_replace",
  "pattern":"(?s)(\\r?\\n\\s*\\})\\s*$",
  "replacement":"\\n    // Tail test A\\n    // Tail test B\\n    // Tail test C\\1"}

---

`71-73`: **Add language to anchor_insert example.**

```diff
-```json
+```json
 {"op":"anchor_insert","afterMethodName":"GetCurrentTarget","text":"private int __TempHelper(int a,int b)=>a+b;\\n"}

---

`171-176`: **Anchor wording vs code: don’t require “public” on Update().**

The test target currently defines `private void Update()`. Recommend rewording to “above Update()” (no access qualifier) to avoid brittle matches.

</blockquote></details>
<details>
<summary>.claude/prompts/nl-unity-claude-tests-mini.md (3)</summary><blockquote>

`22-25`: **Clarify env var(s) for project_root override.**

You mention “the server will also accept the absolute path passed via env” but don’t name the key(s). Please specify the exact environment variable(s) the server honors so CI authors can wire this reliably.


I can update the doc once you confirm the correct env var name(s).

---

`8-18`: **Tighten phrasing + minor grammar nits.**

Small copyedits improve clarity and consistency (hyphenation and parentheticals).

Apply this diff:

```diff
-3) **Perform a small set of realistic edits** using minimal, precise changes (not full-file rewrites). Examples of small edits you may choose from (pick 3–6 total):
+3) **Perform a small set of realistic edits** using minimal, precise changes (not full‑file rewrites). Examples (pick 3–6 total):

-   - Append an end‑of‑class utility method (e.g., formatting or clamping helper).
+   - Append an end‑of‑class utility method (e.g., a formatting or clamping helper).

-   - Optionally include one idempotency/no‑op check (re‑apply an edit and confirm nothing breaks).
+   - Optionally include one idempotency (no‑op) check (reapply an edit and confirm nothing breaks).

-4) **Validate your edits.** Re‑read the modified regions and verify the changes exist, compile‑risk is low, and surrounding structure remains intact.
+4) **Validate your edits.** Reread the modified regions and verify the changes exist, compile risk is low, and surrounding structure remains intact.

-5) **Report results.** Produce both:
+5) **Report results.** Produce both of the following:

---

29-33: State suite/testcase naming rules explicitly.

To avoid CI/JUnit parsers misgrouping results, restate the exact suite and testcase naming constraints in one place (already implied but easy to miss).

Apply this diff:

-## Output Requirements (match NL suite conventions)
+## Output Requirements (match NL suite conventions)
+Exact naming (required for CI parsers):
+- Test suite name: `UnityMCP.NL`
+- One `<testcase>` per sub‑test; use concise, unique testcase names.

UnityMcpBridge/UnityMcpServer~/src/server_version.txt (1)

1-1: Add trailing newline.

POSIX text files should end with a newline for tooling friendliness.

Apply this diff:

-3.1.1
+3.1.1
+

UnityMcpBridge/Editor/Models/McpClient.cs (1)

7-7: Document new macConfigPath field.

Add a short summary to aid discoverability and future serialization.

Apply this diff:

-        public string macConfigPath;
+        /// <summary>Absolute path to the MCP client's config file on macOS (if different from Windows/Linux defaults).</summary>
+        public string macConfigPath;

UnityMcpBridge/Editor/Windows/ManualConfigEditorWindow.cs (2)

119-126: Treat whitespace‑only macConfigPath as empty.

Use IsNullOrWhiteSpace to avoid selecting paths that are effectively empty.

Apply this diff:

-                else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
+                else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
                 {
-                    displayPath = string.IsNullOrEmpty(mcpClient.macConfigPath)
-
-                        ? configPath
-
-                        : mcpClient.macConfigPath;
+                    displayPath = string.IsNullOrWhiteSpace(mcpClient.macConfigPath)
+                        ? configPath
+                        : mcpClient.macConfigPath;
                 }

---

141-148: Improve long-path readability.

SelectableLabel avoids clipping and supports easy selection for copy.

Apply this diff:

-            EditorGUILayout.TextField(
-                displayPath,
-                pathStyle,
-                GUILayout.Height(EditorGUIUtility.singleLineHeight)
-            );
+            EditorGUILayout.SelectableLabel(
+                displayPath ?? string.Empty,
+                pathStyle,
+                GUILayout.Height(EditorGUIUtility.singleLineHeight * 2)
+            );

README.md (1)

46-48: Fix list indentation (MD007) for new tool bullets.

Unindent to match surrounding list and satisfy markdownlint.

Apply this diff:

-  *   `apply_text_edits`: Precise text edits with precondition hashes and atomic multi-edit batches.
-  *   `script_apply_edits`: Structured C# method/class edits (insert/replace/delete) with safer boundaries.
-  *   `validate_script`: Fast validation (basic/standard) to catch syntax/structure issues before/after writes.
+* `apply_text_edits`: Precise text edits with precondition hashes and atomic multi‑edit batches.
+* `script_apply_edits`: Structured C# method/class edits (insert/replace/delete) with safer boundaries.
+* `validate_script`: Fast validation (basic/standard) to catch syntax/structure issues before/after writes.

UnityMcpBridge/Editor/Tools/ManageEditor.cs (2)

171-188: Harden GetProjectRoot and simplify path derivation

Use Path.GetDirectoryName for clarity and add an existence check; keep normalized slashes.

-            try
+            try
             {
-                // Application.dataPath points to <Project>/Assets
-                string assetsPath = Application.dataPath.Replace('\\', '/');
-                string projectRoot = Directory.GetParent(assetsPath)?.FullName.Replace('\\', '/');
+                // Application.dataPath points to <Project>/Assets
+                var projectRootRaw = Path.GetDirectoryName(Application.dataPath);
+                string projectRoot = projectRootRaw?.Replace('\\', '/');
                 if (string.IsNullOrEmpty(projectRoot))
                 {
                     return Response.Error("Could not determine project root from Application.dataPath");
                 }
+                if (!Directory.Exists(projectRoot))
+                {
+                    return Response.Error($"Resolved project root does not exist: {projectRoot}");
+                }
                 return Response.Success("Project root resolved.", new { projectRoot });
             }

---

143-144: Avoid drift in the “Unknown action” help text

Minor: consider centralizing the supported-actions list to prevent mismatches when adding new actions in the future.

README-DEV.md (1)

69-104: Polish wording and header spacing in CI section

Tighten phrasing and add a blank line before the next header to satisfy linters.

-## CI Test Workflow (GitHub Actions)
+## CI Test Workflow (GitHub Actions)

 We provide a CI job to run a Natural Language Editing mini-suite against the Unity test project. It spins up a headless Unity container and connects via the MCP bridge.

 - Trigger: Workflow dispatch (`Claude NL suite (Unity live)`).
 - Image: `UNITY_IMAGE` (UnityCI) pulled by tag; the job resolves a digest at runtime. Logs are sanitized.
 - Reports: JUnit at `reports/junit-nl-suite.xml`, Markdown at `reports/junit-nl-suite.md`.
-- Publishing: JUnit is normalized to `reports/junit-for-actions.xml` and published; artifacts upload all files under `reports/`.
+- Publishing: JUnit is normalized to `reports/junit-for-actions.xml` and published.
+- Artifacts: the job uploads all files under `reports/`.

 ### Test target script
 - The repo includes a long, standalone C# script used to exercise larger edits and windows:
   - `TestProjects/UnityMCPTests/Assets/Scripts/LongUnityScriptClaudeTest.cs`
   Use this file locally and in CI to validate multi-edit batches, anchor inserts, and windowed reads on a sizable script.

 ### Add a new NL test
 - Edit `.claude/prompts/nl-unity-claude-tests-mini.md` (or `nl-unity-suite-full.md` for the larger suite).
 - Follow the conventions: single `<testsuite>` root, one `<testcase>` per sub-test, end system-out with `VERDICT: PASS|FAIL`.
 - Keep edits minimal and reversible; include evidence windows and compact diffs.

 ### Run the suite
 1) Push your branch, then manually run the workflow from the Actions tab.
 2) The job writes reports into `reports/` and uploads artifacts.
 3) The “JUnit Test Report” check summarizes results; open the Job Summary for full markdown.

 ### View results
 - Job Summary: inline markdown summary of the run on the Actions tab in GitHub
-- Check: “JUnit Test Report” on the PR/commit.
+- Check: “JUnit Test Report” on the PR or commit.
 - Artifacts: `claude-nl-suite-artifacts` includes XML and MD.

 ### MCP Connection Debugging
 - *Enable debug logs* in the Unity MCP window (inside the Editor) to view connection status, auto-setup results, and MCP client paths. It shows:
   - bridge startup/port, client connections, strict framing negotiation, and parsed frames
   - auto-config path detection (Windows/macOS/Linux), uv/claude resolution, and surfaced errors
 - In CI, the job tails Unity logs (redacted for serial/license/password/token) and prints socket/status JSON diagnostics if startup fails.
+
 ## Workflow

UnityMcpBridge/Editor/Data/McpClients.cs (2)

22-26: macOS config paths added—looks right; consider using SpecialFolder.ApplicationData for mac app-support cases

The additions map to expected locations. Optional: for mac entries under Library/Application Support (Claude Desktop, VS Code), you could use Environment.SpecialFolder.ApplicationData to avoid stringing “Library/Application Support”.

Also applies to: 43-46, 64-69, 89-95, 118-125, 147-152

---

137-137: Fix numbering typo in comment

Sequence jumps back to “3)”. Update to “6) Kiro” to avoid confusion.

-            // 3) Kiro
+            // 6) Kiro

tests/test_edit_strict_and_warnings.py (4)

15-18: Style: avoid one‑liner class body for readability.
Expanding this helps linting (E701) and consistency across tests.

-class _Dummy: pass
+class _Dummy:
+    pass

---

36-39: Tighten DummyMCP decorator and silence Ruff ARG00x/E702.
Split statements and underscore unused decorator args.

-class DummyMCP:
-    def __init__(self): self.tools = {}
-    def tool(self, *args, **kwargs):
-        def deco(fn): self.tools[fn.__name__] = fn; return fn
-        return deco
+class DummyMCP:
+    def __init__(self):
+        self.tools = {}
+
+    def tool(self, *_args, **_kwargs):
+        def deco(fn):
+            self.tools[fn.__name__] = fn
+            return fn
+        return deco

---

52-56: Silence unused-arg warnings in test double.

-    def fake_send(cmd, params):
+    def fake_send(_cmd, _params):
         # Simulate Unity path returning minimal success
         return {"success": True}

---

70-83: Assert strict-mode short-circuits backend call.
Make the stub fail if invoked to prove early error path.

-    def fake_send(cmd, params):
-        return {"success": True}
+    def fake_send(_cmd, _params):
+        raise AssertionError("send_command_with_retry should not be called in strict=True path")

UnityMcpBridge/Editor/Windows/VSCodeManualSetupWindow.cs (1)

181-190: Harden “Open File” with try/catch.
Avoid Editor exceptions when file is missing or no handler exists; log a warning.

-            {
-                // Open the file using the system's default application
-                System.Diagnostics.Process.Start(
-                    new System.Diagnostics.ProcessStartInfo
-                    {
-                        FileName = displayPath,
-                        UseShellExecute = true,
-                    }
-                );
-            }
+            {
+                try
+                {
+                    // Open the file using the system's default application
+                    System.Diagnostics.Process.Start(
+                        new System.Diagnostics.ProcessStartInfo
+                        {
+                            FileName = displayPath,
+                            UseShellExecute = true,
+                        }
+                    );
+                }
+                catch (System.Exception e)
+                {
+                    Debug.LogWarning($"Failed to open '{displayPath}': {e.Message}");
+                }
+            }

tests/test_manage_script_uri.py (6)

28-30: Style: expand one‑liner class.
Removes E701 and improves readability.

-class _Dummy: pass
+class _Dummy:
+    pass

---

46-51: Silence unused decorator args and split statements.

-    def tool(self, *args, **kwargs):  # ignore decorator kwargs like description
-        def _decorator(fn):
-            self.tools[fn.__name__] = fn
-            return fn
-        return _decorator
+    def tool(self, *_args, **_kwargs):  # ignore decorator kwargs like description
+        def _decorator(fn):
+            self.tools[fn.__name__] = fn
+            return fn
+        return _decorator

---

67-71: Use underscores for unused args in stub.

-    def fake_send(cmd, params):  # capture params and return success
+    def fake_send(_cmd, _params):  # capture params and return success
         captured['cmd'] = cmd
         captured['params'] = params
         return {"success": True, "message": "ok"}

---

96-100: Ditto: underscores for unused args.

-    def fake_send(cmd, params):
+    def fake_send(_cmd, _params):
         captured['cmd'] = cmd
         captured['params'] = params
         return {"success": True, "message": "ok"}

---

114-117: Ditto: underscores for unused args.

-    def fake_send(cmd, params):
+    def fake_send(_cmd, _params):
         captured['params'] = params
         return {"success": True, "message": "ok"}

---

83-91: Optional: add UNC path coverage.
Cover file://server/share cases that _split_uri handles (UNC semantics).

Add this test:

@pytest.mark.parametrize(
    "uri, expected_name, expected_path",
    [
        ("file://fileserver/Share/UnityProj/Assets/Scripts/Net.cs", "Net", "Assets/Scripts"),
    ],
)
def test_split_uri_unc(monkeypatch, uri, expected_name, expected_path):
    tools = _register_tools()
    captured = {}
    def fake_send(_cmd, _params):
        captured['params'] = _params
        return {"success": True, "message": "ok"}
    monkeypatch.setattr(manage_script, "send_command_with_retry", fake_send)
    fn = tools['apply_text_edits']
    fn(DummyCtx(), uri=uri, edits=[], precondition_sha256=None)
    assert captured['params']['name'] == expected_name
    assert captured['params']['path'] == expected_path

tests/test_get_sha.py (1)

42-47: Silence unused decorator args and split statements.

-    def tool(self, *args, **kwargs):
-        def deco(fn):
-            self.tools[fn.__name__] = fn
-            return fn
-        return deco
+    def tool(self, *_args, **_kwargs):
+        def deco(fn):
+            self.tools[fn.__name__] = fn
+            return fn
+        return deco

tests/test_edit_normalization_and_noop.py (3)

71-75: Add missing assertions for index-based normalization (and capture the apply call).

Without appending non-read calls to calls, the second half of the test doesn’t verify normalization of index ranges. Capture the apply call and assert the computed 1-based span.

@@
-    def fake_read(cmd, params):
-        if params.get("action") == "read":
-            return {"success": True, "data": {"contents": "hello\n"}}
-        return {"success": True}
+    def fake_read(cmd, params):
+        if params.get("action") == "read":
+            return {"success": True, "data": {"contents": "hello\n"}}
+        # capture non-read (apply_text_edits) invocations
+        calls.append(params)
+        return {"success": True}
@@
-    # last call is apply_text_edits
-    
+    # last call is apply_text_edits; verify 0-based index normalized to 1:1 insertion
+    p = calls[-1]
+    e = p["edits"][0]
+    assert e["startLine"] == 1 and e["startCol"] == 1 and e["endLine"] == 1 and e["endCol"] == 1
+    assert e["newText"] == "// idx\n"

Also applies to: 77-79

---

96-96: Remove unused manage_script_edits registration.

tools_struct is not used; drop to keep the test focused and quiet static analysis.

-    tools_struct = DummyMCP(); manage_script_edits.register_manage_script_edits_tools(tools_struct)

---

15-18: Fix Ruff E701/E702/ARG002 nits in test helpers.

Reformat one-liners and mark unused decorator args to keep CI linters quiet.

@@
-class _Dummy: pass
+class _Dummy:
+    pass
@@
-class DummyMCP:
-    def __init__(self): self.tools = {}
-    def tool(self, *args, **kwargs):
-        def deco(fn): self.tools[fn.__name__] = fn; return fn
-        return deco
+class DummyMCP:
+    def __init__(self):
+        self.tools = {}
+    def tool(self, *_, **__):
+        def deco(fn):
+            self.tools[fn.__name__] = fn
+            return fn
+        return deco

Also applies to: 35-39

tests/test_resources_api.py (4)

6-6: Drop duplicate import.

Duplicate import pytest is harmless but flagged.

-import pytest

---

29-33: Silence unused decorator args.

Rename unused *args, **kwargs to conventional throwaways to satisfy linters.

-    def tool(self, *args, **kwargs):  # accept kwargs like description
+    def tool(self, *_, **__):  # accept kwargs like description

---

42-42: Remove unused fixture argument.

monkeypatch isn’t used.

-def test_resource_list_filters_and_rejects_traversal(resource_tools, tmp_path, monkeypatch):
+def test_resource_list_filters_and_rejects_traversal(resource_tools, tmp_path):

---

55-57: Narrow broad exception on symlink creation.

Catch OSError only; avoid swallowing unexpected issues.

-    except Exception:
-        # Some platforms may not allow symlinks in tests; ignore
-        pass
+    except OSError:
+        # Some platforms may not allow symlinks in tests; ignore
+        pass

UnityMcpBridge/Editor/Helpers/PackageDetector.cs (1)

50-54: Prefer Debug.LogException for richer diagnostics.

You already capture the exception; logging the full stack helps field triage.

-                        if (!string.IsNullOrEmpty(error))
-                        {
-                            Debug.LogWarning($"MCP for Unity: Auto-detect on load failed: {capturedEx}");
-                            // Alternatively: Debug.LogException(capturedEx);
-                        }
+                        if (!string.IsNullOrEmpty(error))
+                        {
+                            Debug.LogException(capturedEx);
+                        }

tests/test_regex_delete_guard.py (2)

49-87: Assert no writes occur on validation failure.

Capture non-read invocations to ensure the regex guard prevents any write RPCs.

@@
-    tools = setup_tools()
-    apply = tools["script_apply_edits"]
+    tools = setup_tools()
+    apply = tools["script_apply_edits"]
+    writes = []
@@
-    def fake_send(cmd, params):
+    def fake_send(cmd, params):
         # Only the initial read should be invoked; provide contents
         if cmd == "manage_script" and params.get("action") == "read":
             return {"success": True, "data": {"contents": contents}}
-        # If preflight failed as intended, no write should be attempted; return a marker if called
-        return {"success": True, "message": "SHOULD_NOT_WRITE"}
+        # Track unexpected writes
+        writes.append(params)
+        return {"success": True, "message": "SHOULD_NOT_WRITE"}
@@
     assert resp.get("code") == "validation_failed"
     data = resp.get("data", {})
     assert data.get("status") == "validation_failed"
     # Helpful hint to prefer structured delete
     assert "delete_method" in (data.get("hint") or "")
+    # Ensure no writes were attempted
+    assert not writes

---

16-16: Fix Ruff E701/E702/ARG002 nits in test helpers.

Reformat one-liners and mark unused decorator args.

@@
-class _D: pass
+class _D:
+    pass
@@
-class DummyMCP:
-    def __init__(self): self.tools = {}
-    def tool(self, *args, **kwargs):
-        def deco(fn): self.tools[fn.__name__] = fn; return fn
-        return deco
+class DummyMCP:
+    def __init__(self):
+        self.tools = {}
+    def tool(self, *_, **__):
+        def deco(fn):
+            self.tools[fn.__name__] = fn
+            return fn
+        return deco

Also applies to: 37-40

tests/test_transport_framing.py (3)

1-1: Tighten exception handling and formatting; add contextlib.suppress.

Replace bare try/except-pass with suppress and fix E701/E702. This keeps logs clean without masking unexpected errors.

@@
-import sys
+import sys
+import contextlib
@@
-            except Exception:
-                pass
+            except Exception:
+                pass  # keep the thread alive for the test; server may close early
@@
-        try:
-            conn.close()
-        except Exception:
-            pass
+        with contextlib.suppress(Exception):
+            conn.close()
@@
-                    except Exception:
+                    except Exception:
                         peek = b"\x00"
@@
-        finally:
-            try: conn.close()
-            except Exception: pass
-            sock.close()
+        finally:
+            with contextlib.suppress(Exception):
+                conn.close()
+            sock.close()

Also applies to: 63-64, 68-69, 100-100, 184-186

---

33-33: Reduce port reuse flakiness.

Enable SO_REUSEADDR on test servers.

@@
-    sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+    sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+    sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
@@
-    sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+    sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+    sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
@@
-    sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+    sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+    sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)

Also applies to: 81-81, 164-164

---

162-162: Remove redundant inline imports.

Modules are already imported at file top.

-    import socket, struct, threading, time

.github/workflows/claude-nl-suite-mini.yml (3)

21-31: Good timeout configuration, but consider adjusting conditional check.

The 60-minute timeout is reasonable for NL test suites. However, the conditional check on line 21 is redundant since this workflow only triggers on workflow_dispatch (line 4).

You can simplify by removing the redundant condition:

-    if: github.event_name == 'workflow_dispatch'
     runs-on: ubuntu-latest

---

206-206: Fix trailing whitespace issues.

YAMLlint correctly identified trailing spaces that should be removed.

-          JSON
-          
+          JSON
+

-            fi
-        
+            fi
+

-          fail_on_parse_error: true
-        
+          fail_on_parse_error: true
+

Also applies to: 331-331, 343-343

---

163-165: Wrap JSON decoding in try/except and refactor port discovery
The current one-liner aborts on malformed JSON (or missing unity_port) and silently returns '', skipping any subsequent valid files. Extract this into a small Python loop or script that catches json.JSONDecodeError and missing-key errors per file to skip invalid entries and improve reliability and maintainability.

UnityMcpBridge/Editor/Helpers/ServerInstaller.cs (2)

49-58: Server installation logic is well-structured, but version file write could fail silently.

The installation correctly copies the server and attempts to write the version file, but failures are silently caught.

Consider logging version file write failures:

-    try { File.WriteAllText(Path.Combine(destSrc, VersionFileName), embeddedVer ?? "unknown"); } catch { }
+    try 
+    { 
+        File.WriteAllText(Path.Combine(destSrc, VersionFileName), embeddedVer ?? "unknown"); 
+    } 
+    catch (Exception ex) 
+    { 
+        McpLog.Warn($"Failed to write version file: {ex.Message}");
+    }

---

321-353: Tighten pgrep pattern to avoid false positives

• Anchor and escape the directory argument, e.g.
  Arguments = $"-f '^uv\\s.*--directory\\s{Regex.Escape(serverSrcPath)}$'"
• Or restrict to the exact executable name with pgrep -x uv plus a directory filter
• Alternatively, record launched UV PIDs (e.g. via a lock file) and kill those directly instead of scanning all processes

UnityMcpBridge/Editor/MCPForUnityBridge.cs (1)

559-560: Fix inconsistent spacing in condition.

-             if (payloadLen > MaxFrameBytes)
+            if (payloadLen > MaxFrameBytes)

UnityMcpBridge/Editor/Windows/MCPForUnityEditorWindow.cs (5)

258-294: Minor: avoid heavy resolver work on UI thread during LogDebugPrefsState

ReadEmbeddedVersionOrFallback can traverse packages; run it lazily or via EditorApplication.delayCall to avoid editor hitches when enabling logs.

- string embeddedVer = ReadEmbeddedVersionOrFallback();
+ string embeddedVer = "pending";
+ EditorApplication.delayCall += () => {
+   try { var v = ReadEmbeddedVersionOrFallback(); McpLog.Info($"Embedded version: {v}", always:false); } catch {}
+ };

---

801-866: Framing helpers: add basic upper-bound check and zero-length guard consistency

ReadFrameUtf8 enforces zero-length/size checks; mirror a max frame bound for defense-in-depth (match bridge MaxFrameBytes) to avoid large allocations if peer misbehaves.

- byte[] header = ReadExact(stream, 8, timeoutMs);
+ byte[] header = ReadExact(stream, 8, timeoutMs);
  ulong len = ((ulong)header[0] << 56)
@@
- if (len == 0UL) throw new IOException("Zero-length frames are not allowed");
- if (len > int.MaxValue) throw new IOException("Frame too large");
+ if (len == 0UL) throw new IOException("Zero-length frames are not allowed");
+ const ulong MaxFrameBytes = 8UL * 1024UL * 1024UL; // keep in sync with bridge
+ if (len > MaxFrameBytes || len > (ulong)int.MaxValue) throw new IOException($"Frame too large: {len}");

---

1163-1367: Config write: ensure parent directory and harden backup/replace paths

WriteToConfig assumes parent dir exists; safe to create it here too because this method can be called from multiple places. Also, consider using ValidateUvBinarySafe when persisting UvPath.

 JsonSerializerSettings jsonSettings = new() { Formatting = Formatting.Indented };
+// Ensure config directory exists (idempotent)
+try { Directory.CreateDirectory(Path.GetDirectoryName(configPath)); } catch {}

@@
- if (IsValidUv(uvPath)) UnityEditor.EditorPrefs.SetString("MCPForUnity.UvPath", uvPath);
+ if (ValidateUvBinarySafe(uvPath)) UnityEditor.EditorPrefs.SetString("MCPForUnity.UvPath", uvPath);

---

1256-1271: macOS path normalization: edge-case with redundant slashes

If suffix begins with a slash, Path.Combine will drop earlier segments. Normalize suffix to trim any leading separator.

- string suffix = norm.Substring(idx + "/.local/share/".Length); // UnityMCP/...
+ string suffix = norm.Substring(idx + "/.local/share/".Length).TrimStart('/');

---

168-221: Layout math: clamp reduces column width aggressively on small editor widths

On small windows, 220f min combined with two columns + padding can force horizontal scroll. Consider Mathf.Max with available width check or switch to GUILayout.MinWidth only.

UnityMcpBridge/UnityMcpServer~/src/tools/manage_script.py (2)

116-121: Key mismatch when decoding read response

Guard both contentsEncoded/encodedContents consistently to avoid None decode if only one flag is present.

- if not contents and data.get("contentsEncoded"):
-     try:
-         contents = base64.b64decode(data.get("encodedContents", "").encode("utf-8")).decode("utf-8", "replace")
+ if not contents and (data.get("contentsEncoded") and data.get("encodedContents")):
+     try:
+         contents = base64.b64decode((data.get("encodedContents") or "").encode("utf-8")).decode("utf-8", "replace")
      except Exception:
          contents = contents or ""

---

275-293: Remove unused locals in debug_preview path

lines and prev are unused; keep the preview path minimal.

-                import difflib
-                # Apply locally to preview final result
-                lines = []
-                # Build an indexable original from a read if we normalized from read; otherwise skip
-                prev = ""
+                import difflib  # kept for future enhancement

UnityMcpBridge/UnityMcpServer~/src/tools/manage_script_edits.py (1)

751-789: Preview flow: OK; minor diff truncation idiom

Ruff suggests building list with unpack; optional.

-                if len(diff) > 800:
-                    diff = diff[:800] + ["... (diff truncated) ..."]
+                if len(diff) > 800:
+                    diff = [*diff[:800], "... (diff truncated) ..."]

UnityMcpBridge/UnityMcpServer~/src/tools/resource_tools.py (1)

111-168: list_resources: symlink-escape guard + file caps look good

For large trees, consider switching rglob to iterdir()+stack for better early break, but current cap/filters are fine.

.github/workflows/claude-nl-suite.yml (1)

140-179: Wait-for-bridge: resilient; minor: avoid duplicating json.load per file

The inline Python reads each status file twice in a single expression. Read once per file to cut I/O.

- PORT=$(python3 -c "import os,glob,json,sys,time; b=os.path.expanduser('~/.unity-mcp'); fs=sorted(glob.glob(os.path.join(b,'unity-mcp-status-*.json')), key=os.path.getmtime, reverse=True); print(next((json.load(open(f,'r',encoding='utf-8')).get('unity_port') for f in fs if time.time()-os.path.getmtime(f)<=300 and json.load(open(f,'r',encoding='utf-8')).get('unity_port')), '' ))" 2>/dev/null || true)
+ PORT=$(python3 - <<'PY' 2>/dev/null || true
+ import os,glob,json,time,sys
+ b=os.path.expanduser('~/.unity-mcp')
+ fs=sorted(glob.glob(os.path.join(b,'unity-mcp-status-*.json')), key=os.path.getmtime, reverse=True)
+ port=''
+ now=time.time()
+ for f in fs:
+   try:
+     if now-os.path.getmtime(f) > 300: continue
+     with open(f,'r',encoding='utf-8') as fh:
+       d=json.load(fh); p=d.get('unity_port')
+       if p: port=str(p); break
+   except Exception: pass
+ print(port)
+ PY
+ )

UnityMcpBridge/Editor/Tools/ManageScript.cs (12)

54-107: Assets path hardening is solid; consider two small tweaks

• Consider treating an empty relDir as “Assets” rather than defaulting to “Scripts” to avoid surprising callers who only pass name (keeps API neutral to folder choice).
• The symlink guard skips non-existent ancestors. If a non-existent leaf is under a symlinked ancestor, this still relies on that ancestor existing to detect the link; that’s fine but worth a short comment.

Apply this doc/comment diff for clarity:

-/// Resolves a directory under Assets/, preventing traversal and escaping.
-/// Returns fullPathDir on disk and canonical 'Assets/...' relative path.
+/// Resolves a directory under Assets/, preventing traversal and escaping.
+/// Returns fullPathDir on disk and canonical 'Assets/...' relative path.
+/// Note: Non-existent leaf directories are allowed; symlink checks are best-effort on existing ancestors.

---

212-245: Validation API: minor parsing robustness

Parsing diagnostics via Regex is fine. If Roslyn adds more location formats, consider passing structured diagnostics directly when available to avoid regex parsing of messages.

---

252-278: get_sha: robust; minor length calculation fallback nit

Computing length via UTF8 without BOM is good; the FI fallback covers non-UTF8 oddities. Consider always reporting both logical lengthBytes (UTF-8) and fileSizeBytes (FileInfo.Length) to help clients diagnose encoding differences.

---

482-797: apply_text_edits: strong safeguards; a couple of edge cases

• Header guard blocks edits before first using; this is protective but can prevent legitimate header/comment/license updates. Consider an opt-in flag (e.g., options.allowHeaderEdits=true) with additional checks.
• Roslyn formatting (when enabled) changes content post-apply, altering the returned SHA. That’s fine, but surface this in docs so clients expect a different hash than the local edited buffer.

Apply this minimal, backward-compatible flag:

- private static object ApplyTextEdits(
+ private static object ApplyTextEdits(
   string fullPath,
   string relativePath,
   string name,
   JArray edits,
   string preconditionSha256,
   string refreshModeFromCaller = null,
-  string validateMode = null)
+  string validateMode = null,
+  bool allowHeaderEdits = false)
 {
   ...
-  foreach (var sp in spans)
+  foreach (var sp in spans)
   {
-    if (sp.start < headerBoundary)
+    if (!allowHeaderEdits && sp.start < headerBoundary)
     {
       return Response.Error("using_guard", new { status = "using_guard", hint = "Refusing to edit before the first 'using'. Use anchor_insert near a method or a structured edit." });
     }
   }

And thread through allowHeaderEdits from options if present.

---

799-832: Line/column indexing: comment is inaccurate re “code points”

The implementation advances per UTF-16 code unit, not Unicode code points. Update the comment to avoid confusion.

-// 1-based line/col to absolute index (0-based), col positions are counted in code points
+// 1-based line/col to absolute index (0-based); columns are counted in UTF-16 code units

---

913-942: Scoped balance tolerance could hide genuine underflows

Returning true when counters are -1 tolerates context but can mask missing opens inside the window. Consider >=0 for stricter “scoped” checks, or make tolerance a parameter.

---

957-960: Delete uses synchronous Refresh; align with debounced model

Synchronous AssetDatabase.Refresh may stall the editor and is inconsistent with the debounced strategy used elsewhere. Prefer scheduling a refresh/import for the deleted asset.

- AssetDatabase.Refresh();
- return Response.Success(
-     $"Script '{Path.GetFileName(relativePath)}' moved to trash successfully.",
-     new { deleted = true }
- );
+ ManageScriptRefreshHelpers.ScheduleScriptRefresh(relativePath);
+ return Response.Success(
+     $"Script '{Path.GetFileName(relativePath)}' moved to trash successfully.",
+     new { deleted = true, scheduledRefresh = true }
+ );

---

976-1446: Structured edits: strong feature set; a few heuristics to revisit

• anchor_insert duplicate guard uses the file “name” as class; if the class name differs from the file name, duplicates won’t be detected. Consider optional className in anchor ops for scoping the duplicate scan.
• Method header regex is permissive; to reduce false positives, add word boundaries around methodName.

- string pattern =
+ string pattern =
   @"(?m)^[\t ]*(?:\[[^\]]+\][\t ]*)*[\t ]*" +
   @"(?:(?:public|private|protected|internal|static|virtual|override|sealed|async|extern|unsafe|new|partial|readonly|volatile|event|abstract|ref|in|out)\s+)*" +
-  rtPattern + @"[\t ]+" + namePattern + @"\s*(?:<[^>]+>)?\s*\(" + paramsPattern + @"\)";
+  rtPattern + @"[\t ]+\b" + namePattern + @"\b\s*(?:<[^>]+>)?\s*\(" + paramsPattern + @"\)";

---

1479-1531: Class span: balanced fallback is pragmatic; tighten token match

IndexOfClassToken uses a simple “class {name}” substring; use word boundaries to avoid matching prefixes/suffixes (e.g., MyClassy).

-private static int IndexOfClassToken(string s, string className)
-{
-    // simple token search; could be tightened with Regex for word boundaries
-    var pattern = "class " + className;
-    return s.IndexOf(pattern, StringComparison.Ordinal);
-}
+private static int IndexOfClassToken(string s, string className)
+{
+    var rx = new Regex($@"\bclass\s+{Regex.Escape(className)}\b", RegexOptions.CultureInvariant);
+    var m = rx.Match(s);
+    return m.Success ? m.Index : -1;
+}

---

2279-2336: Reference cache should use UTC

Cache expiry uses DateTime.Now. Using UtcNow avoids DST/time-shift surprises.

- if (_cachedReferences != null && DateTime.Now - _cacheTime < CacheExpiry)
+ if (_cachedReferences != null && DateTime.UtcNow - _cacheTime < CacheExpiry)
     return _cachedReferences;
 ...
- _cacheTime = DateTime.Now;
+ _cacheTime = DateTime.UtcNow;

---

2428-2437: Heuristic regexes are fine; consider ignoring strings/comments

Magic number and method-length heuristics can false-positive inside strings/comments. If noise becomes an issue, run these scans on a tokenized view or strip strings/comments first.

---

2549-2615: Debounced refresh: add safety around ImportAsset and compilation request

Wrap ImportAsset/RequestScriptCompilation in try/catch to avoid breaking the debounce loop if a single import throws (locked file, transient IO).

- foreach (var p in toImport)
-     AssetDatabase.ImportAsset(p, ImportAssetOptions.ForceUpdate);
+ foreach (var p in toImport)
+ {
+     try { AssetDatabase.ImportAsset(p, ImportAssetOptions.ForceUpdate); }
+     catch (Exception ex) { Debug.LogWarning($"ImportAsset failed for {p}: {ex.Message}"); }
+ }
 #if UNITY_EDITOR
- UnityEditor.Compilation.CompilationPipeline.RequestScriptCompilation();
+ try { UnityEditor.Compilation.CompilationPipeline.RequestScriptCompilation(); }
+ catch (Exception ex) { Debug.LogWarning($"RequestScriptCompilation failed: {ex.Message}"); }
 #endif

[coderabbitai]

🛠️ Refactor suggestion

Harden XML parsing against XXE; use defusedxml.

CI can ingest untrusted XML; switch to defusedxml to avoid known xml.etree attacks.

-import xml.etree.ElementTree as ET
+from defusedxml import ElementTree as ET

Ensure defusedxml is added to the runtime environment (e.g., requirements/runner image).

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	import xml.etree.ElementTree as ET
	from defusedxml import ElementTree as ET

🤖 Prompt for AI Agents

In .github/scripts/mark_skipped.py around lines 15 to 16, the script imports
xml.etree.ElementTree which is vulnerable to XXE; replace it with
defusedxml.ElementTree (import defusedxml.ElementTree as ET) so parsing is
hardened against external entity attacks, and ensure defusedxml is added to the
runtime dependencies (e.g., requirements.txt or the CI/runner image) so the
module is available at execution time.

[coderabbitai]

💡 Verification agent

🧩 Analysis chain

YAML lint: trailing spaces and indentation inconsistencies

Numerous trailing-space and indentation warnings (yamllint). Recommend running yamllint/prettier to normalize. Functionally fine, but cleaning helps diffs and prevents accidental syntax slips.

Run:

---

🏁 Script executed:

#!/bin/bash
yamllint .github/workflows/claude-nl-suite.yml || true

Length of output: 7639

---

Fix YAML style issues in .github/workflows/claude-nl-suite.yml
Detected by yamllint: missing document start (---), truthy values instead of booleans, numerous trailing spaces, incorrect indentations, lines exceeding 80 chars, and no newline at EOF. Run yamllint or Prettier to normalize the file and clear these warnings.

🧰 Tools

🪛 YAMLlint (1.37.1)

[error] 5-5: trailing spaces

(trailing-spaces)

---

[error] 9-9: trailing spaces

(trailing-spaces)

---

[error] 13-13: trailing spaces

(trailing-spaces)

---

[error] 18-18: trailing spaces

(trailing-spaces)

---

[warning] 21-21: wrong indentation: expected 8 but found 6

(indentation)

---

[warning] 25-25: wrong indentation: expected 10 but found 8

(indentation)

---

[warning] 26-26: too many spaces after colon

(colons)

---

[error] 27-27: trailing spaces

(trailing-spaces)

---

[warning] 30-30: wrong indentation: expected 10 but found 8

(indentation)

---

[warning] 33-33: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 33-33: too many spaces after colon

(colons)

---

[warning] 34-34: too many spaces after colon

(colons)

---

[warning] 36-36: too many spaces after colon

(colons)

---

[error] 46-46: trailing spaces

(trailing-spaces)

---

[warning] 49-49: wrong indentation: expected 14 but found 12

(indentation)

---

[error] 50-50: trailing spaces

(trailing-spaces)

---

[warning] 54-54: wrong indentation: expected 14 but found 12

(indentation)

---

[error] 55-55: trailing spaces

(trailing-spaces)

---

[error] 73-73: trailing spaces

(trailing-spaces)

---

[warning] 79-79: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 79-79: too many spaces after colon

(colons)

---

[warning] 80-80: too many spaces after colon

(colons)

---

[warning] 82-82: too many spaces after colon

(colons)

---

[warning] 84-84: wrong indentation: expected 14 but found 12

(indentation)

---

[error] 88-88: trailing spaces

(trailing-spaces)

---

[error] 95-95: trailing spaces

(trailing-spaces)

---

[error] 102-102: trailing spaces

(trailing-spaces)

---

[warning] 107-107: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 107-107: too many spaces after colon

(colons)

---

[warning] 109-109: too many spaces after colon

(colons)

---

[error] 139-139: trailing spaces

(trailing-spaces)

---

[error] 179-179: trailing spaces

(trailing-spaces)

---

[error] 201-201: trailing spaces

(trailing-spaces)

---

[error] 208-208: trailing spaces

(trailing-spaces)

---

[error] 221-221: trailing spaces

(trailing-spaces)

---

[error] 267-267: trailing spaces

(trailing-spaces)

---

[error] 268-268: trailing spaces

(trailing-spaces)

---

[warning] 275-275: wrong indentation: expected 14 but found 12

(indentation)

---

[error] 294-294: trailing spaces

(trailing-spaces)

---

[error] 343-343: trailing spaces

(trailing-spaces)

---

[error] 443-443: trailing spaces

(trailing-spaces)

---

[error] 455-455: trailing spaces

(trailing-spaces)

---

[error] 467-467: trailing spaces

(trailing-spaces)

---

[error] 480-480: trailing spaces

(trailing-spaces)

---

[error] 497-497: trailing spaces

(trailing-spaces)

---

[error] 513-513: trailing spaces

(trailing-spaces)

---

[warning] 518-518: wrong indentation: expected 14 but found 12

(indentation)

---

[error] 524-524: trailing spaces

(trailing-spaces)

---

[warning] 529-529: wrong indentation: expected 14 but found 12

(indentation)

---

[error] 536-536: trailing spaces

(trailing-spaces)

---

[error] 543-543: no new line character at the end of file

(new-line-at-end-of-file)

---

[error] 543-543: trailing spaces

(trailing-spaces)

🤖 Prompt for AI Agents

In .github/workflows/claude-nl-suite.yml (lines 1-543): the file has YAML style
issues flagged by yamllint — add a document start marker (---) at the top,
replace truthy/quoted values with proper YAML booleans (true/false without
quotes) where applicable, remove trailing whitespace, fix inconsistent
indentation to a consistent 2-space style, wrap or shorten lines exceeding 80
characters, and ensure the file ends with a single newline; run yamllint and/or
Prettier/YAML formatter and apply their suggestions to normalize the file.

[coderabbitai]

🛠️ Refactor suggestion

Ensure body size never exceeds MAX_FRAME; compute after serialization.

Current “reserve 4096” can still overshoot when JSON overhead is larger, causing server disconnects. Compute baseline JSON size and cap filler accordingly.

-    # Cap filler to stay within framing limit (reserve small overhead for JSON)
-    safe_max = max(1, MAX_FRAME - 4096)
-    filler_len = min(SIZE_MB * 1024 * 1024, safe_max)
-    body = {
+    # Compute baseline JSON size, then cap filler so body_bytes <= MAX_FRAME
+    req_bytes = SIZE_MB * 1024 * 1024
+    base = {
         "type": "read_console",
         "params": {
             "action": "get",
             "types": ["all"],
             "count": 1000,
             "format": "detailed",
             "includeStacktrace": True,
-            "filterText": FILL * filler_len
+            "filterText": ""
         }
-    }
-    body_bytes = json.dumps(body, ensure_ascii=False).encode("utf-8")
+    }
+    base_len = len(json.dumps(base, ensure_ascii=False).encode("utf-8"))
+    avail = max(1, MAX_FRAME - base_len)
+    filler_len = min(req_bytes, avail)
+    body = base
+    body["params"]["filterText"] = FILL * filler_len
+    body_bytes = json.dumps(body, ensure_ascii=False).encode("utf-8")
+    if len(body_bytes) > MAX_FRAME:
+        raise RuntimeError(f"serialized request exceeds MAX_FRAME ({len(body_bytes)} > {MAX_FRAME})")

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	# Cap filler to stay within framing limit (reserve small overhead for JSON)
	safe_max = max(1, MAX_FRAME - 4096)
	filler_len = min(SIZE_MB * 1024 * 1024, safe_max)
	body = {
	"type": "read_console",
	"params": {
	"action": "get",
	"types": ["all"],
	"count": 1000,
	"format": "detailed",
	"includeStacktrace": True,
	"filterText": FILL * filler_len
	}
	}
	body_bytes = json.dumps(body, ensure_ascii=False).encode("utf-8")
	# Compute baseline JSON size, then cap filler so body_bytes <= MAX_FRAME
	req_bytes = SIZE_MB * 1024 * 1024
	base = {
	"type": "read_console",
	"params": {
	"action": "get",
	"types": ["all"],
	"count": 1000,
	"format": "detailed",
	"includeStacktrace": True,
	"filterText": ""
	}
	}
	base_len = len(json.dumps(base, ensure_ascii=False).encode("utf-8"))
	avail = max(1, MAX_FRAME - base_len)
	filler_len = min(req_bytes, avail)
	body = base
	body["params"]["filterText"] = FILL * filler_len
	body_bytes = json.dumps(body, ensure_ascii=False).encode("utf-8")
	if len(body_bytes) > MAX_FRAME:
	raise RuntimeError(f"serialized request exceeds MAX_FRAME ({len(body_bytes)} > {MAX_FRAME})")

🤖 Prompt for AI Agents

In test_unity_socket_framing.py around lines 49 to 64, the current approach
reserves a fixed 4096 bytes and computes filler length before JSON serialization
which can still exceed MAX_FRAME; instead serialize a minimal body first (e.g.,
with an empty or single-char filler) to measure actual overhead, compute
allowed_filler_bytes = max(1, MAX_FRAME - overhead_bytes), then set filler_len =
min(requested_filler_bytes, allowed_filler_bytes), rebuild the body with that
capped filler and re-serialize to body_bytes so the final serialized payload
will never exceed MAX_FRAME.

[coderabbitai]

🛠️ Refactor suggestion

Unify legacy-root detection with ServerInstaller to cover Windows and avoid drift.

Current list omits Windows AppData path and duplicates logic. Delegate to ServerInstaller.GetLegacyRootsForDetection().

-        private static bool LegacyRootsExist()
-        {
-            try
-            {
-                string home = System.Environment.GetFolderPath(System.Environment.SpecialFolder.UserProfile) ?? string.Empty;
-                string[] roots =
-                {
-                    System.IO.Path.Combine(home, ".config", "UnityMCP", "UnityMcpServer", "src"),
-                    System.IO.Path.Combine(home, ".local", "share", "UnityMCP", "UnityMcpServer", "src")
-                };
-                foreach (var r in roots)
-                {
-                    try { if (System.IO.File.Exists(System.IO.Path.Combine(r, "server.py"))) return true; } catch { }
-                }
-            }
-            catch { }
-            return false;
-        }
+        private static bool LegacyRootsExist()
+        {
+            try
+            {
+                foreach (var root in ServerInstaller.GetLegacyRootsForDetection())
+                {
+                    try
+                    {
+                        var src = System.IO.Path.Combine(root, "src");
+                        if (System.IO.File.Exists(System.IO.Path.Combine(src, "server.py")))
+                            return true;
+                    }
+                    catch { }
+                }
+            }
+            catch { }
+            return false;
+        }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	private static bool LegacyRootsExist()
	{
	try
	{
	string home = System.Environment.GetFolderPath(System.Environment.SpecialFolder.UserProfile) ?? string.Empty;
	string[] roots =
	{
	System.IO.Path.Combine(home, ".config", "UnityMCP", "UnityMcpServer", "src"),
	System.IO.Path.Combine(home, ".local", "share", "UnityMCP", "UnityMcpServer", "src")
	};
	foreach (var r in roots)
	{
	try { if (System.IO.File.Exists(System.IO.Path.Combine(r, "server.py"))) return true; } catch { }
	}
	}
	catch { }
	return false;
	}
	private static bool LegacyRootsExist()
	{
	try
	{
	foreach (var root in ServerInstaller.GetLegacyRootsForDetection())
	{
	try
	{
	var src = System.IO.Path.Combine(root, "src");
	if (System.IO.File.Exists(System.IO.Path.Combine(src, "server.py")))
	return true;
	}
	catch { }
	}
	}
	catch { }
	return false;
	}

🤖 Prompt for AI Agents

In UnityMcpBridge/Editor/Helpers/PackageDetector.cs around lines 88 to 105, the
LegacyRootsExist method duplicates legacy-root logic and omits Windows AppData;
replace its hard-coded roots with a call to
ServerInstaller.GetLegacyRootsForDetection() and iterate those returned paths to
check for server.py (using Path.Combine and File.Exists), swallowing only
per-path exceptions if necessary and returning true on first match, otherwise
return false; do not reintroduce platform-specific hard-coded paths here so
ServerInstaller remains the single source of truth.

[coderabbitai]

🛠️ Refactor suggestion

UI-freeze risk: synchronous socket I/O with 30s frame timeout on main thread

VerifyBridgePing does blocking reads/writes (up to 30s). When invoked from AutoFirstRunSetup/RunSetupNow on the UI thread, this can freeze the editor. Reduce timeouts and/or offload to a background Task with a short overall budget (2–3s).

- const int ConnectTimeoutMs = 1000;
- const int FrameTimeoutMs = 30000; // match bridge frame I/O timeout
+ const int ConnectTimeoutMs = 1000;
+ const int FrameTimeoutMs = 3000; // keep UI snappy; bridge has longer internal timeout

And consider:

- lastBridgeVerifiedOk = VerifyBridgePing(MCPForUnityBridge.GetCurrentPort());
+ System.Threading.Tasks.Task.Run(() => VerifyBridgePing(MCPForUnityBridge.GetCurrentPort()))
+   .ContinueWith(t => { lastBridgeVerifiedOk = t.Result; Repaint(); }, TaskScheduler.FromCurrentSynchronizationContext());

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents

UnityMcpBridge/Editor/Windows/MCPForUnityEditorWindow.cs lines 758-799:
VerifyBridgePing performs synchronous blocking socket I/O (ConnectAsync.Wait +
blocking Read/Write with a 30s frame timeout) which can freeze the Unity editor
UI; change it to run on a background Task with an overall short budget (2–3s)
and use async socket/stream APIs with CancellationToken timeouts (avoid
Task.Wait and blocking reads), or reduce individual timeouts to a few seconds if
immediate refactor to async is not possible; ensure ConnectAsync is awaited with
a timeout via Task.WhenAny or CancellationToken, use
NetworkStream.ReadAsync/WriteAsync with timeouts, and marshal results back to
the caller so UI callers call the async method and handle timeout/failure
without blocking the main thread.

[coderabbitai]

💡 Verification agent

🧩 Analysis chain

Version mismatch vs. server_version.txt—confirm intent.

Package shows 3.1.0 while server_version.txt (elsewhere in PR) is 3.1.1. If semver separation is intentional (package vs. internal server), ignore; otherwise align to one source of truth (e.g., read from a single version file or setuptools-scm).

Run to confirm divergence and where it’s used:

---

🏁 Script executed:

#!/bin/bash
rg -n "version\s*=\s*\"3\.1\." -g '!**/node_modules/**'
rg -n "server_version\.txt|3\.1\." -g '!**/node_modules/**'

Length of output: 1188

---

Align package versions with server_version.txt
pyproject.toml (UnityMcpBridge/UnityMcpServer~/src/pyproject.toml) and package.json (UnityMcpBridge/package.json) are set to 3.1.0, while server_version.txt is 3.1.1. Bump both package files to 3.1.1 (or revert the server version) to maintain a single source of truth.

🤖 Prompt for AI Agents

In UnityMcpBridge/UnityMcpServer~/src/pyproject.toml around line 3 the version
is "3.1.0" which is inconsistent with server_version.txt (3.1.1); update the
version to "3.1.1" to match the single source of truth, and also update
UnityMcpBridge/package.json to "3.1.1" (or alternatively change
server_version.txt back to 3.1.0) so all three files use the same version.

[coderabbitai]

🛠️ Refactor suggestion

Don’t fully suppress missing import diagnostics.

"reportMissingImports": "none" hides packaging/venv issues that become runtime errors. Recommend at least “warning” (and “error” in CI).

Apply:

-  "reportMissingImports": "none",
+  "reportMissingImports": "warning",

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	"reportMissingImports": "none",
	"reportMissingImports": "warning",

🤖 Prompt for AI Agents

In UnityMcpBridge/UnityMcpServer~/src/pyrightconfig.json around line 3, the
project currently sets "reportMissingImports": "none" which silences
missing-import diagnostics; change this to "warning" so local development
surfaces packaging/venv import issues, and ensure CI runs with stricter checks
(override to "error" via a CI-specific pyright config or CLI flag) so missing
imports fail the build.

[coderabbitai]

💡 Verification agent

🧩 Analysis chain

Verify version matrix alignment (server vs package).

server_version.txt is 3.1.1. Please confirm this intentionally differs from Unity package and Python package versions (seen elsewhere as 3.1.0), or bump all to a single semantic version.

Run this script to diff versions across the repo:

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail
echo "Embedded server_version.txt:"
sed -n '1p' UnityMcpBridge/UnityMcpServer~/src/server_version.txt || true

echo -e "\nUnity package.json versions:"
rg -nP --glob 'UnityMcpBridge/**/package.json' '"version"\s*:\s*"\d+\.\d+\.\d+"' -n -S -C0 || true

echo -e "\nPython pyproject versions:"
rg -nP '^\s*version\s*=\s*"\d+\.\d+\.\d+"' UnityMcpBridge/UnityMcpServer~/src/pyproject.toml -n -S || true

echo -e "\nAll explicit 3.x tags referenced:"
rg -nS '3\.\d+\.\d+' -g '!**/dist/**' -g '!**/build/**' -g '!**/.git/**' || true

Length of output: 2552

---

Align semantic versions across server, Unity, and Python packages
server_version.txt is 3.1.1, but UnityMcpBridge/package.json and UnityMcpBridge/UnityMcpServer~/src/pyproject.toml are still at 3.1.0. Update both to 3.1.1 for consistency.

🤖 Prompt for AI Agents

In UnityMcpBridge/UnityMcpServer~/src/server_version.txt line 1 the version is
3.1.1 while UnityMcpBridge/package.json and
UnityMcpBridge/UnityMcpServer~/src/pyproject.toml still show 3.1.0; update the
"version" field in package.json and the version entry in pyproject.toml to 3.1.1
to keep all packages aligned with server_version.txt, ensure semantic version
syntax matches (3.1.1) and save both files.

[coderabbitai]

🛠️ Refactor suggestion

replace_range: add column bound clamping to avoid slicing beyond line ends

Current index_of uses (col-1) without checking per-line length. Clamp start/end columns to [1..len(line)+1] to prevent mis-slices on short lines.

         elif op == "replace_range":
@@
-            def index_of(line: int, col: int) -> int:
-                if line <= len(lines):
-                    return sum(len(l) for l in lines[: line - 1]) + (col - 1)
-                return sum(len(l) for l in lines)
+            def index_of(line: int, col: int) -> int:
+                if line <= len(lines):
+                    base = sum(len(l) for l in lines[: line - 1])
+                    max_col = len(lines[line - 1].rstrip('\n').rstrip('\r')) + 1
+                    c = max(1, min(col, max_col))
+                    return base + (c - 1)
+                return sum(len(l) for l in lines)

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	elif op == "replace_range":
	start_line = int(edit.get("startLine", 1))
	start_col = int(edit.get("startCol", 1))
	end_line = int(edit.get("endLine", start_line))
	end_col = int(edit.get("endCol", 1))
	replacement = edit.get("text", "")
	lines = text.splitlines(keepends=True)
	max_line = len(lines) + 1 # 1-based, exclusive end
	if (start_line < 1 or end_line < start_line or end_line > max_line
	or start_col < 1 or end_col < 1):
	raise RuntimeError("replace_range out of bounds")
	def index_of(line: int, col: int) -> int:
	if line <= len(lines):
	return sum(len(l) for l in lines[: line - 1]) + (col - 1)
	return sum(len(l) for l in lines)
	a = index_of(start_line, start_col)
	b = index_of(end_line, end_col)
	text = text[:a] + replacement + text[b:]
	elif op == "regex_replace":
	elif op == "replace_range":
	start_line = int(edit.get("startLine", 1))
	start_col = int(edit.get("startCol", 1))
	end_line = int(edit.get("endLine", start_line))
	end_col = int(edit.get("endCol", 1))
	replacement = edit.get("text", "")
	lines = text.splitlines(keepends=True)
	max_line = len(lines) + 1 # 1-based, exclusive end
	if (start_line < 1 or end_line < start_line or end_line > max_line
	or start_col < 1 or end_col < 1):
	raise RuntimeError("replace_range out of bounds")
	def index_of(line: int, col: int) -> int:
	if line <= len(lines):
	base = sum(len(l) for l in lines[: line - 1])
	max_col = len(lines[line - 1].rstrip('\n').rstrip('\r')) + 1
	c = max(1, min(col, max_col))
	return base + (c - 1)
	return sum(len(l) for l in lines)
	a = index_of(start_line, start_col)
	b = index_of(end_line, end_col)
	text = text[:a] + replacement + text[b:]
	elif op == "regex_replace":

🧰 Tools

🪛 Ruff (0.12.2)

59-59: Avoid specifying long messages outside the exception class

(TRY003)

---

61-61: Function definition does not bind loop variable lines

(B023)

---

62-62: Ambiguous variable name: l

(E741)

---

62-62: Function definition does not bind loop variable lines

(B023)

---

63-63: Ambiguous variable name: l

(E741)

---

63-63: Function definition does not bind loop variable lines

(B023)

🤖 Prompt for AI Agents

In UnityMcpBridge/UnityMcpServer~/src/tools/manage_script_edits.py around lines
49 to 67, the index_of calculation for replace_range fails to clamp column
values to a line's length which can produce incorrect slice indices; fix it by
clamping start_col and end_col to the valid range [1, len(line)+1] before
computing the character index (use len(lines[line-1]) for lines that exist,
default to 0 for non-existent lines), apply the clamp in index_of (or compute
per-line clamped cols right before summing), and ensure validation/slicing uses
these clamped values so replacements never slice beyond a line end.

[coderabbitai]

🛠️ Refactor suggestion

_split_uri: ensure Assets/ guard for apply_text_edits path, not just create/delete/validate

Currently delete_script/validate_script enforce Assets/, but apply_text_edits does not. Add a guard after _split_uri to prevent edits outside project scope.

 def apply_text_edits(
     ctx: Context,
     uri: str,
     edits: List[Dict[str, Any]],
     precondition_sha256: str | None = None,
     strict: bool | None = None,
     options: Dict[str, Any] | None = None,
 ) -> Dict[str, Any]:
     """Apply small text edits to a C# script identified by URI."""
-    name, directory = _split_uri(uri)
+    name, directory = _split_uri(uri)
+    if not directory or directory.split("/")[0].lower() != "assets":
+        return {"success": False, "code": "path_outside_assets", "message": "URI must resolve under 'Assets/'."}

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	def _split_uri(uri: str) -> tuple[str, str]:
	"""Split an incoming URI or path into (name, directory) suitable for Unity.
	Rules:
	- unity://path/Assets/... → keep as Assets-relative (after decode/normalize)
	- file://... → percent-decode, normalize, strip host and leading slashes,
	then, if any 'Assets' segment exists, return path relative to that 'Assets' root.
	Otherwise, fall back to original name/dir behavior.
	- plain paths → decode/normalize separators; if they contain an 'Assets' segment,
	return relative to 'Assets'.
	"""
	raw_path: str
	if uri.startswith("unity://path/"):
	raw_path = uri[len("unity://path/") :]
	elif uri.startswith("file://"):
	parsed = urlparse(uri)
	host = (parsed.netloc or "").strip()
	p = parsed.path or ""
	# UNC: file://server/share/... -> //server/share/...
	if host and host.lower() != "localhost":
	p = f"//{host}{p}"
	# Use percent-decoded path, preserving leading slashes
	raw_path = unquote(p)
	else:
	raw_path = uri
	# Percent-decode any residual encodings and normalize separators
	raw_path = unquote(raw_path).replace("\\", "/")
	# Strip leading slash only for Windows drive-letter forms like "/C:/..."
	if os.name == "nt" and len(raw_path) >= 3 and raw_path[0] == "/" and raw_path[2] == ":":
	raw_path = raw_path[1:]
	# Normalize path (collapse ../, ./)
	norm = os.path.normpath(raw_path).replace("\\", "/")
	# If an 'Assets' segment exists, compute path relative to it (case-insensitive)
	parts = [p for p in norm.split("/") if p not in ("", ".")]
	idx = next((i for i, seg in enumerate(parts) if seg.lower() == "assets"), None)
	assets_rel = "/".join(parts[idx:]) if idx is not None else None
	effective_path = assets_rel if assets_rel else norm
	# For POSIX absolute paths outside Assets, drop the leading '/'
	# to return a clean relative-like directory (e.g., '/tmp' -> 'tmp').
	if effective_path.startswith("/"):
	effective_path = effective_path[1:]
	name = os.path.splitext(os.path.basename(effective_path))[0]
	directory = os.path.dirname(effective_path)
	return name, directory
	def apply_text_edits(
	ctx: Context,
	uri: str,
	edits: List[Dict[str, Any]],
	precondition_sha256: str | None = None,
	strict: bool | None = None,
	options: Dict[str, Any] | None = None,
	) -> Dict[str, Any]:
	"""Apply small text edits to a C# script identified by URI."""
	name, directory = _split_uri(uri)
	if not directory or directory.split("/")[0].lower() != "assets":
	return {
	"success": False,
	"code": "path_outside_assets",
	"message": "URI must resolve under 'Assets/'."
	}
	# ... existing logic for applying edits follows ...

🤖 Prompt for AI Agents

In UnityMcpBridge/UnityMcpServer~/src/tools/manage_script.py around lines 12 to
61, _split_uri can return paths outside the Unity project (i.e., not under an
Assets/ segment) and currently only create/delete/validate paths are guarded;
you must add a check immediately after calling _split_uri in the
apply_text_edits handler to ensure the returned effective path is inside Assets
(e.g., verify the directory or name/directory combined begins with or was
derived from an Assets segment), and if not, reject the operation by raising a
descriptive exception or returning an error response consistent with other
handlers (same error type/message used by delete_script/validate_script). Ensure
the new guard uses the same case-insensitive Assets detection logic as
_split_uri and mirrors existing error-handling behavior so apply_text_edits
cannot modify files outside the project scope.