fix: address CodeRabbit review findings

RoyougiShiki · RoyougiShiki · commit e940e904e3c0 · 2026-08-12T12:44:52.000+08:00
- ProcessArgumentListCompat: correct Windows command-line quoting (backslash
  runs doubled only before quotes/at end; verbatim pass-through when unquoted)
- EncryptedFileKeyStore: implement PBKDF2-HMAC-SHA256 manually on 2020.3
  (netstandard2.1 lacks 4-arg Rfc2898DeriveBytes) so key derivation stays
  byte-identical with 2021.2+; 3-arg SHA1 would break existing ciphertext MAC
- CompatDropdownField: SetValueWithoutNotify no longer dispatches change events
- MCPForUnityEditorWindow: 2020.3 package add/remove serialized via queue
  (legacy PM accepts one in-flight request per operation)
- BuildSettingsHelper: architecture mapping fixed (0 = None, not x86_64)
- ManageBuild: 'server' subtarget rejected on &lt;2021.2 instead of silent player
- docs: correct packages-lock.json description; test project: align
  test-framework 1.1.31, portable verify_compile.cmd (auto-detect Unity, exit
  codes, consistent log), README command block
diff --git a/MCPForUnity/Editor/Helpers/ProcessArgumentListCompat.cs b/MCPForUnity/Editor/Helpers/ProcessArgumentListCompat.cs
@@ -30,14 +30,41 @@ private static string Quote(string arg)
                 return "\"\"";
             }
 
+            // Only quote when necessary; otherwise pass through verbatim so
+            // backslashes in paths (e.g. C:\Program Files\...) are preserved.
             if (arg.IndexOfAny(new[] { ' ', '\t', '"' }) < 0)
             {
                 return arg;
             }
 
-            // Escape embedded quotes the way Windows CreateProcess expects:
-            // backslashes before a quote are doubled, then the quote escaped.
-            return "\"" + arg.Replace("\\", "\\\\").Replace("\"", "\\\"") + "\"";
+            // Standard Windows command-line quoting (the algorithm used by
+            // .NET's ArgumentList / CommandLineToArgvW): wrap in quotes; for every
+            // run of backslashes, double them only when immediately followed by a
+            // quote or at the very end of the argument (before the closing quote).
+            var sb = new System.Text.StringBuilder();
+            sb.Append('"');
+            int backslashes = 0;
+            foreach (char ch in arg)
+            {
+                if (ch == '\\')
+                {
+                    backslashes++;
+                    continue;
+                }
+                if (ch == '"')
+                {
+                    sb.Append('\\', backslashes * 2 + 1);
+                    sb.Append('"');
+                    backslashes = 0;
+                    continue;
+                }
+                sb.Append('\\', backslashes);
+                backslashes = 0;
+                sb.Append(ch);
+            }
+            sb.Append('\\', backslashes * 2);
+            sb.Append('"');
+            return sb.ToString();
         }
     }
 }
diff --git a/MCPForUnity/Editor/Security/SecureKeyStore/EncryptedFileKeyStore.cs b/MCPForUnity/Editor/Security/SecureKeyStore/EncryptedFileKeyStore.cs
@@ -93,16 +93,65 @@ private void DeriveKeys(out byte[] encKey, out byte[] macKey)
             byte[] master = LoadOrCreate(Path.Combine(_dir, "secret.bin"), 32);
             byte[] salt = LoadOrCreate(Path.Combine(_dir, "salt.bin"), 16);
             string password = Convert.ToBase64String(master) + "|" + MachineId();
-            using (var kdf = new Rfc2898DeriveBytes(password, salt, Iterations))
+#if UNITY_2021_2_OR_NEWER
+            using (var kdf = new Rfc2898DeriveBytes(password, salt, Iterations, HashAlgorithmName.SHA256))
             {
                 byte[] material = kdf.GetBytes(64);
                 encKey = new byte[32];
                 macKey = new byte[32];
                 Buffer.BlockCopy(material, 0, encKey, 0, 32);
                 Buffer.BlockCopy(material, 32, macKey, 0, 32);
             }
+#else
+            // Unity 2020.3 (netstandard2.1) has no 4-arg Rfc2898DeriveBytes overload, so
+            // implement PBKDF2-HMAC-SHA256 (RFC 2898) manually to keep key derivation
+            // byte-identical with the 2021.2+ path (a 3-arg SHA1 derivation would make
+            // existing ciphertext fail MAC validation).
+            byte[] material = Pbkdf2Sha256(password, salt, Iterations, 64);
+            encKey = new byte[32];
+            macKey = new byte[32];
+            Buffer.BlockCopy(material, 0, encKey, 0, 32);
+            Buffer.BlockCopy(material, 32, macKey, 0, 32);
+#endif
         }
 
+#if !UNITY_2021_2_OR_NEWER
+        /// <summary>PBKDF2 with HMAC-SHA256 (RFC 2898), matching the .NET 4-arg Rfc2898DeriveBytes output.</summary>
+        private static byte[] Pbkdf2Sha256(string password, byte[] salt, int iterations, int numBytes)
+        {
+            var prf = new System.Security.Cryptography.HMACSHA256(
+                System.Text.Encoding.UTF8.GetBytes(password));
+            int hLen = prf.HashSize / 8;
+            int blocks = (numBytes + hLen - 1) / hLen;
+
+            var output = new byte[blocks * hLen];
+            var saltPlusOne = new byte[salt.Length + 4];
+
+            for (int block = 1; block <= blocks; block++)
+            {
+                Buffer.BlockCopy(salt, 0, saltPlusOne, 0, salt.Length);
+                saltPlusOne[salt.Length] = (byte)((block >> 24) & 0xFF);
+                saltPlusOne[salt.Length + 1] = (byte)((block >> 16) & 0xFF);
+                saltPlusOne[salt.Length + 2] = (byte)((block >> 8) & 0xFF);
+                saltPlusOne[salt.Length + 3] = (byte)(block & 0xFF);
+
+                byte[] u = prf.ComputeHash(saltPlusOne);
+                byte[] t = (byte[])u.Clone();
+                for (int i = 1; i < iterations; i++)
+                {
+                    u = prf.ComputeHash(u);
+                    for (int j = 0; j < hLen; j++) t[j] ^= u[j];
+                }
+                Buffer.BlockCopy(t, 0, output, (block - 1) * hLen, hLen);
+            }
+
+            prf.Dispose();
+            var result = new byte[numBytes];
+            Buffer.BlockCopy(output, 0, result, 0, numBytes);
+            return result;
+        }
+#endif
+
         private byte[] Encrypt(byte[] plaintext)
         {
             DeriveKeys(out byte[] encKey, out byte[] macKey);
diff --git a/MCPForUnity/Editor/Tools/Build/BuildSettingsHelper.cs b/MCPForUnity/Editor/Tools/Build/BuildSettingsHelper.cs
@@ -26,7 +26,9 @@ public static object ReadProperty(string property, BuildTargetGroup namedTarget)
                     return new { property, value = PlayerSettings.GetScriptingDefineSymbolsForGroup(namedTarget) };
                 case "architecture":
                     var arch = PlayerSettings.GetArchitecture(namedTarget);
-                    string archName = arch switch { 0 => "x86_64", 1 => "arm64", 2 => "universal", _ => "unknown" };
+                    // GetArchitecture returns the BuildTargetGroup's architecture setting,
+                    // where 0 means None (not x86_64); see PlayerSettings.SetArchitecture docs.
+                    string archName = arch switch { 0 => "none", 1 => "arm64", 2 => "universal", _ => "unknown" };
                     return new { property, value = archName, raw = arch };
                 default:
                     return null;
@@ -66,15 +68,14 @@ public static string WriteProperty(string property, string value, BuildTargetGro
                     case "architecture":
                         int arch = value.ToLowerInvariant() switch
                         {
-                            "x86_64" => 0,
                             "none" => 0,
                             "default" => 0,
                             "arm64" => 1,
                             "universal" => 2,
                             _ => -1
                         };
                         if (arch < 0)
-                            return $"Unknown architecture '{value}'. Valid: x86_64, arm64, universal";
+                            return $"Unknown architecture '{value}'. Valid: none, arm64, universal";
                         PlayerSettings.SetArchitecture(namedTarget, arch);
                         return null;
                     default:
diff --git a/MCPForUnity/Editor/Tools/ManageBuild.cs b/MCPForUnity/Editor/Tools/ManageBuild.cs
@@ -250,6 +250,12 @@ private static object HandlePlatform(ToolParams p)
                     EditorUserBuildSettings.standaloneBuildSubtarget = StandaloneBuildSubtarget.Server;
                 else if (subtargetLower == "player")
                     EditorUserBuildSettings.standaloneBuildSubtarget = StandaloneBuildSubtarget.Player;
+#else
+                // Unity 2020.3 has no server subtarget; fail loudly instead of silently
+                // building the player variant.
+                string subtargetLower = subtargetStr.ToLowerInvariant();
+                if (subtargetLower == "server")
+                    return new ErrorResponse("subtarget 'server' requires Unity 2021.2 or newer (StandaloneBuildSubtarget API).");
 #endif
             }
 
diff --git a/MCPForUnity/Editor/Windows/Components/CompatDropdownField.cs b/MCPForUnity/Editor/Windows/Components/CompatDropdownField.cs
@@ -87,8 +87,7 @@ public int index
                 else if (clamped < 0) clamped = 0;
                 else if (clamped >= m_Choices.Count) clamped = m_Choices.Count - 1;
                 if (clamped == m_Index) return;
-                m_Index = clamped;
-                UpdateValueFromIndex();
+                SetValueWithoutNotify(clamped >= 0 && clamped < m_Choices.Count ? m_Choices[clamped] : null);
             }
         }
 
@@ -110,10 +109,10 @@ public string value
         public void SetValueWithoutNotify(string newValue)
         {
             int newIndex = m_Choices.IndexOf(newValue);
-            if (newIndex >= 0)
+            if (newIndex >= 0 && newIndex != m_Index)
             {
                 m_Index = newIndex;
-                UpdateValueFromIndex();
+                m_Value = newIndex < m_Choices.Count ? m_Choices[newIndex] : null;
             }
         }
 
diff --git a/MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs b/MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs
@@ -1058,12 +1058,10 @@ private static void BatchUpmAdd(string[] packageIds, Action onComplete = null)
             var request = UnityEditor.PackageManager.Client.AddAndRemove(packageIds, null);
             PollUpmRequest(request, "install", onComplete);
 #else
-            UnityEditor.PackageManager.Requests.AddRequest request = null;
-            foreach (var id in packageIds)
-            {
-                request = UnityEditor.PackageManager.Client.Add(id);
-            }
-            PollUpmAddRequest(request, onComplete);
+            // Unity 2020.3: queue adds serially — PackageManager accepts one in-flight
+            // request per operation on legacy editors; starting all at once and polling
+            // only the last one would leave earlier packages' results unchecked.
+            QueueUpmAdds(new Queue<string>(packageIds), onComplete);
 #endif
         }
 
@@ -1074,12 +1072,8 @@ private static void BatchUpmRemove(string[] packageIds, Action onComplete = null
             var request = UnityEditor.PackageManager.Client.AddAndRemove(null, packageIds);
             PollUpmRequest(request, "remove", onComplete);
 #else
-            UnityEditor.PackageManager.Requests.RemoveRequest request = null;
-            foreach (var id in packageIds)
-            {
-                request = UnityEditor.PackageManager.Client.Remove(id);
-            }
-            PollUpmRemoveRequest(request, onComplete);
+            // Unity 2020.3: queue removes serially (see BatchUpmAdd).
+            QueueUpmRemoves(new Queue<string>(packageIds), onComplete);
 #endif
         }
 
@@ -1139,6 +1133,67 @@ private static void PollUpmRemoveRequest(UnityEditor.PackageManager.Requests.Rem
             };
             EditorApplication.update += pollCallback;
         }
+
+        private static void QueueUpmAdds(Queue<string> queue, Action onComplete)
+        {
+            if (queue.Count == 0)
+            {
+                EditorUtility.ClearProgressBar();
+                onComplete?.Invoke();
+                return;
+            }
+            string id = queue.Dequeue();
+            var request = UnityEditor.PackageManager.Client.Add(id);
+            EditorApplication.CallbackFunction poll = null;
+            poll = () =>
+            {
+                if (!request.IsCompleted) return;
+                EditorApplication.update -= poll;
+                if (request.Status == UnityEditor.PackageManager.StatusCode.Success)
+                {
+                    Debug.Log($"[MCP] Package {id} installed.");
+                    QueueUpmAdds(queue, onComplete);
+                }
+                else
+                {
+                    EditorUtility.ClearProgressBar();
+                    Debug.LogError($"[MCP] Package {id} install failed: {request.Error?.message}");
+                    onComplete?.Invoke();
+                }
+            };
+            EditorApplication.update += poll;
+        }
+
+        private static void QueueUpmRemoves(Queue<string> queue, Action onComplete)
+        {
+            if (queue.Count == 0)
+            {
+                EditorUtility.ClearProgressBar();
+                onComplete?.Invoke();
+                return;
+            }
+            string id = queue.Dequeue();
+            var request = UnityEditor.PackageManager.Client.Remove(id);
+            EditorApplication.CallbackFunction poll = null;
+            poll = () =>
+            {
+                if (!request.IsCompleted) return;
+                EditorApplication.update -= poll;
+                if (request.Status == UnityEditor.PackageManager.StatusCode.Success)
+                {
+                    Debug.Log($"[MCP] Package {id} removed.");
+                    QueueUpmRemoves(queue, onComplete);
+                }
+                else
+                {
+                    EditorUtility.ClearProgressBar();
+                    Debug.LogError($"[MCP] Package {id} remove failed: {request.Error?.message}");
+                    onComplete?.Invoke();
+                }
+            };
+            EditorApplication.update += poll;
+        }
+
 #endif
 
         private static void UninstallRoslyn()
diff --git a/TestProjects/Unity2020Compat/Packages/manifest.json b/TestProjects/Unity2020Compat/Packages/manifest.json
@@ -5,7 +5,7 @@
     "com.unity.ide.rider": "2.0.7",
     "com.unity.ide.visualstudio": "2.0.12",
     "com.unity.ide.vscode": "1.2.4",
-    "com.unity.test-framework": "1.1.29",
+    "com.unity.test-framework": "1.1.31",
     "com.unity.textmeshpro": "3.0.6",
     "com.unity.timeline": "1.4.8",
     "com.unity.toolchain.win-x86_64-linux-x86_64": "2.0.11",
@@ -43,4 +43,4 @@
     "com.unity.modules.wind": "1.0.0",
     "com.unity.modules.xr": "1.0.0"
   }
-}
+}
diff --git a/TestProjects/Unity2020Compat/Packages/packages-lock.json b/TestProjects/Unity2020Compat/Packages/packages-lock.json
@@ -409,4 +409,4 @@
       }
     }
   }
-}
+}
diff --git a/TestProjects/Unity2020Compat/README.md b/TestProjects/Unity2020Compat/README.md
@@ -10,7 +10,8 @@
 
 ```cmd
 cd /d <repo>\TestProjects\Unity2020Compat
-"C:\SoftWare\Unity\2020.3.24f1\Editor\Unity.exe" -batchmode -nographics -quit -projectPath "%~dp0" -logFile compile.log
+verify_compile.cmd                       rem auto-detects Unity 2020.3
+verify_compile.cmd C:\path\to\Unity.exe  rem or pass the editor explicitly
 ```
 
 通过标准：`compile.log` 中无 `error CS`，结尾出现 `Exiting batchmode successfully now!`。
diff --git a/TestProjects/Unity2020Compat/verify_compile.cmd b/TestProjects/Unity2020Compat/verify_compile.cmd
@@ -1,4 +1,66 @@
 @echo off
-cd /d F:\AIProjects\Unity-shikiMCP\TestProjects\Unity2020Compat
-"C:\SoftWare\Unity\2020.3.24f1\Editor\Unity.exe" -batchmode -nographics -quit -projectPath "F:\AIProjects\Unity-shikiMCP\TestProjects\Unity2020Compat" -logFile "F:\AIProjects\Unity-shikiMCP\TestProjects\Unity2020Compat\compile_check.log"
-echo UNITY_EXIT=%ERRORLEVEL%
+rem ============================================================
+rem  Unity 2020.3 compile verification for MCP for Unity
+rem
+rem  Usage:
+rem    verify_compile.cmd                (uses UNITY_EDITOR env var or auto-detect)
+rem    verify_compile.cmd <path\to\Unity.exe>
+rem
+rem  Exit codes: 0 = compile OK, 1 = compile errors found, 2 = Unity not found
+rem ============================================================
+setlocal
+
+set "PROJECT_DIR=%~dp0"
+set "LOG_FILE=%PROJECT_DIR%compile_check.log"
+set "UNITY_EXE="
+
+if not "%~1"=="" (
+    set "UNITY_EXE=%~1"
+) else if defined UNITY_EDITOR (
+    set "UNITY_EXE=%UNITY_EDITOR%"
+) else (
+    for %%E in (
+        "C:\SoftWare\Unity\2020.3.24f1\Editor\Unity.exe"
+        "C:\Program Files\Unity\Hub\Editor\2020.3.*\Editor\Unity.exe"
+        "%ProgramFiles%\Unity\Hub\Editor\2020.3.*\Editor\Unity.exe"
+    ) do (
+        if not defined UNITY_EXE if exist %%E set "UNITY_EXE=%%E"
+    )
+)
+
+if not defined UNITY_EXE (
+    echo [verify] Unity 2020.3 executable not found.
+    echo [verify] Pass the path:  verify_compile.cmd C:\path\to\Unity.exe
+    exit /b 2
+)
+if not exist "%UNITY_EXE%" (
+    echo [verify] Unity executable not found: %UNITY_EXE%
+    exit /b 2
+)
+
+echo [verify] Unity: %UNITY_EXE%
+echo [verify] Project: %PROJECT_DIR%
+del /q "%LOG_FILE%" 2>nul
+
+"%UNITY_EXE%" -batchmode -nographics -quit -projectPath "%PROJECT_DIR%" -logFile "%LOG_FILE%"
+set "UNITY_STATUS=%ERRORLEVEL%"
+
+if not exist "%LOG_FILE%" (
+    echo [verify] No log file produced (Unity may have failed to start).
+    exit /b 1
+)
+
+findstr /c:"error CS" "%LOG_FILE%" >nul
+if not errorlevel 1 (
+    echo [verify] FAILED: compiler errors found in %LOG_FILE%
+    exit /b 1
+)
+
+findstr /c:"Exiting batchmode successfully now!" "%LOG_FILE%" >nul
+if errorlevel 1 (
+    echo [verify] FAILED: Unity did not exit cleanly (status %UNITY_STATUS%).
+    exit /b 1
+)
+
+echo [verify] PASS: 0 compiler errors, clean batchmode exit.
+exit /b 0
diff --git a/docs/UNITY_2020_3_COMPAT.md b/docs/UNITY_2020_3_COMPAT.md
@@ -52,7 +52,7 @@
 | `PrefabStage`/`PrefabStageUtility` 命名空间 | `UnityEditor.Experimental.SceneManagement` | 8 文件 |
 | `StandaloneBuildSubtarget`/`subtarget` | 返回 0（仅 Player，2020.3 无 Server 子目标） | Build 系列 |
 | `BuildOptions.CleanBuildCache` | 降级为空操作（2020.3 无此选项） | BuildRunner |
-| `PackageInfo.GetAllRegisteredPackages` | `Client.List(true)` 同步轮询 | 3 文件 |
+| `PackageInfo.GetAllRegisteredPackages` | 新增 `RegisteredPackageInfo`：2020.3 同步解析权威 `Packages/packages-lock.json`（纯文件 IO，无死锁；`Client.List` 轮询会死锁编辑器） | 3 文件 |
 | `AddAndRemoveRequest` | `Client.Add`/`Client.Remove` 逐个 | MCPForUnityEditorWindow |
 | `ShaderPropertyType.Int` | 2020.3 枚举无 Int（用 Float/Range 分支覆盖） | SkyboxOps |
 | `ProfilerCategory.FileIO/VirtualTexturing` | 映射 Loading/Render | CounterOps |

Original file line number	Diff line number	Diff line change
`@@ -87,8 +87,7 @@ public int index`
`87`	`87`	`else if (clamped < 0) clamped = 0;`
`88`	`88`	`else if (clamped >= m_Choices.Count) clamped = m_Choices.Count - 1;`
`89`	`89`	`if (clamped == m_Index) return;`
`90`		`- m_Index = clamped;`
`91`		`- UpdateValueFromIndex();`
	`90`	`+ SetValueWithoutNotify(clamped >= 0 && clamped < m_Choices.Count ? m_Choices[clamped] : null);`
`92`	`91`	`}`
`93`	`92`	`}`
`94`	`93`
`@@ -110,10 +109,10 @@ public string value`
`110`	`109`	`public void SetValueWithoutNotify(string newValue)`
`111`	`110`	`{`
`112`	`111`	`int newIndex = m_Choices.IndexOf(newValue);`
`113`		`- if (newIndex >= 0)`
	`112`	`+ if (newIndex >= 0 && newIndex != m_Index)`
`114`	`113`	`{`
`115`	`114`	`m_Index = newIndex;`
`116`		`- UpdateValueFromIndex();`
	`115`	`+ m_Value = newIndex < m_Choices.Count ? m_Choices[newIndex] : null;`
`117`	`116`	`}`
`118`	`117`	`}`
`119`	`118`
Original file line number	Diff line number	Diff line change
`@@ -409,4 +409,4 @@`
`409`	`409`	`}`
`410`	`410`	`}`
`411`	`411`	`}`
`412`		`-}`
	`412`	`+}`