[shaobin1991]

Description

Fix Stop Server reporting success on Windows while the Python server continues running. A successful non-forced taskkill did not reliably stop the headless server in the live reproduction.

Have the Stop Server button request cooperative shutdown over local HTTP, using the existing per-launch instance token. Wait asynchronously for cooperative exit and verify the process has stopped before reporting success. Use guarded process termination when the endpoint is unavailable or the acknowledged shutdown times out.

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change
• Documentation update
• Refactoring (no functional changes)
• Test update

Changes Made

• Add POST /_unity/shutdown, restricted to loopback clients with the matching Unity launch token. Reject requests in remote-hosted mode or without a managed-launch token. No MCP tool or action is added.
• Send the acknowledgement before raising an in-process SIGINT. This follows the Uvicorn shutdown path and unwinds FastMCP's outer lifespan. Deduplicate repeated requests so they do not trigger Uvicorn's forced-exit behavior.
• Wait asynchronously for up to eight seconds only after the managed server acknowledges the request with its expected PID. Revalidate the launch token before escalating a stuck process, even after its listener closes.
• For older servers without the endpoint, retain the existing identity checks and use forced Windows process-tree termination directly. Allow up to one second to confirm forced exit, returning immediately once the process is gone, instead of trusting the command result. Existing synchronous restart/Editor-quit cleanup paths also use this process fallback.
• Use the managed Windows process-existence API, conservatively treating inspection errors as still running. Preserve protected-PID checks and the existing Unix process-termination fallback.

Compatibility / Package Source

• Unity versions tested: 2021.3.45f2 and 6000.0.47f1 on Windows.
• Package source: isolated file: test package based on upstream beta ed4e205d31a545bac3686d5e19210e67959aeddd.
• Python tests: 3.10.20 and 3.14.4 with uv sync --locked --extra dev; live server checks used Python 3.12.10 with FastMCP 3.0.2.
• Git package lock hash: not applicable to file: packages.

Testing/Screenshots/Recordings

• Full Python tests/, including tests/integration: 1603 passed, 3 skipped on both Python 3.10 and 3.14.
• CI tooling tests (tools/tests/): 205 passed on each Python version.
• Targeted Unity EditMode tests: 18 passed on each tested Editor version.
• Unity PlayMode tests: not run; this change concerns Editor/server lifecycle handling.
• Package import/compile check
• Not applicable

tools/check-unity-versions.ps1: 1 passed, 0 failed, 3 skipped. Unity 2021.3.45f2 compiled successfully in the repository's test project. Exact matrix versions 2022.3.62f1, 6000.0.75f1 and 6000.4.8f1 are not installed locally, so this does not validate the complete Unity matrix.

Local Windows validation invoked the actual Stop Server callback in a disposable project on port 9078:

Scenario	Observed result
Cooperative shutdown, including a connected Editor session	About 0.8-1.3 seconds; lifespan cleanup completed; no taskkill calls
Older upstream server without the shutdown endpoint	Final run: about 3.20 seconds; one forced fallback; no eight-second grace wait
Server acknowledges shutdown but deliberately ignores the signal	Final run: about 10.00 seconds including the eight-second grace period; one forced fallback; Editor updates continued during the grace period

Each callback run confirmed process exit, port release and the button returning to Start Server. Separate headless Python runs also confirmed lifespan cleanup across three cooperative shutdown cycles. Test preferences were isolated from existing Editors; tracing is not included in the change.

Regression coverage includes a replaced PID file during shutdown and delayed exit after forced termination. Both regression cases failed before their corresponding fixes and pass afterward on both tested Editors.

Documentation Updates

• I have added/removed/modified tools or resources

Related Issues

Fixes #1278

Additional Notes

Windows shutdown strategy: I considered non-forced taskkill and a terminate-wait-force sequence similar to the existing macOS approach. In local Windows testing, non-forced taskkill reported success while the same Python process remained alive and continued serving HTTP requests. Waiting for that attempt before forcing termination would add an unnecessary timeout in those cases.

The primary path therefore uses an authenticated loopback request to call Python's signal.raise_signal inside the server process. This triggers the installed SIGINT handler, following Uvicorn's shutdown path and unwinding FastMCP's enclosing lifespan manager. Using forced termination as the primary mechanism would skip that cleanup. Forced termination remains a fallback when cooperative shutdown is unavailable or fails, and process exit is verified before reporting success. The eight-second grace period starts only after the target server acknowledges the shutdown request with the expected PID.

Summary by CodeRabbit

• Bug Fixes
  • Stopping the local server now requests a graceful shutdown first, helping the Editor remain responsive while the server exits.
  • If the server does not respond or stop, the app uses a guarded fallback and verifies the server’s identity before acting.
  • Improved process detection and termination behavior on Windows.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 896920cb-07ee-4070-915c-8894e5f40beb

📥 Commits

Reviewing files that changed from the base of the PR and between ed4e205 and 1736100.

📒 Files selected for processing: 13

• MCPForUnity/Editor/Services/IServerManagementService.cs
• MCPForUnity/Editor/Services/Server/IProcessDetector.cs
• MCPForUnity/Editor/Services/Server/IProcessTerminator.cs
• MCPForUnity/Editor/Services/Server/ProcessDetector.cs
• MCPForUnity/Editor/Services/Server/ProcessTerminator.cs
• MCPForUnity/Editor/Services/ServerManagementService.cs
• MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs
• Server/src/main.py
• Server/tests/test_server_shutdown.py
• TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Server/ProcessTerminatorWindowsTests.cs
• TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Server/ProcessTerminatorWindowsTests.cs.meta
• TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Server/ServerShutdownTests.cs
• TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Server/ServerShutdownTests.cs.meta

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

The change adds cooperative shutdown for the local HTTP server. Unity sends an authenticated shutdown request and waits for the process to exit, with guarded termination as a fallback. Windows process detection and termination behavior also change.

Changes: Local server shutdown

Layer / File(s)	Summary
Process detection and termination MCPForUnity/Editor/Services/Server/IProcessDetector.cs, MCPForUnity/Editor/Services/Server/IProcessTerminator.cs, MCPForUnity/Editor/Services/Server/ProcessDetector.cs, MCPForUnity/Editor/Services/Server/ProcessTerminator.cs, TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Server/ProcessTerminatorWindowsTests.cs, TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Server/ProcessTerminatorWindowsTests.cs.meta	Windows process detection now checks the PID directly. Windows termination uses forced taskkill and reports whether the process exits within one second. Shared wait helpers poll for process exit. Windows tests cover detection, protected PIDs, and termination results.
Authenticated shutdown endpoint Server/src/main.py, Server/tests/test_server_shutdown.py	The server adds POST /_unity/shutdown. It rejects requests that fail the local-host, token, or signal-handler checks. Accepted requests return the process ID and schedule SIGINT. Tests cover accepted and rejected requests, missing handlers, and repeated requests.
Unity asynchronous stop flow MCPForUnity/Editor/Services/IServerManagementService.cs, MCPForUnity/Editor/Services/ServerManagementService.cs, MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs, TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Server/ServerShutdownTests.cs, TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Server/ServerShutdownTests.cs.meta	Unity now awaits the asynchronous stop path. The service validates the stored process identity, requests cooperative shutdown, waits for exit, and uses guarded termination if needed. Tests cover cooperative exit, identity changes, fallback behavior, and tracking cleanup.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant McpConnectionSection
  participant ServerManagementService
  participant UnityShutdownRoute
  participant ServerProcess
  McpConnectionSection->>ServerManagementService: await StopLocalHttpServerAsync()
  ServerManagementService->>UnityShutdownRoute: POST shutdown with instance token
  UnityShutdownRoute-->>ServerManagementService: 202 Accepted with process ID
  UnityShutdownRoute->>ServerProcess: schedule SIGINT after response
  ServerManagementService->>ServerManagementService: poll for process exit
  ServerManagementService->>ServerManagementService: recheck identity before termination

Loading

Suggested reviewers: dsarno

Merge Risk: ⚪ Minimal · up to 17361

No actionable issue is established from the available evidence. The shutdown change is mergeable after the normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 8.97% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 78 functions across 11 files. (2 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly and concisely describes the primary change: cooperative local server shutdown with process-exit verification.
Description check		The description follows the required template, identifies the bug, implementation, compatibility details, tests, limitations, and related issue. It is complete and directly aligned with the changes.
Linked Issues check		[#1278] The issue requires that Stop Server stops the local Python server on Windows, and that the button returns to Start Server. The reviewed head implements this in MCPForUnity/Editor/Services/Serv…
Out of Scope Changes check		All changed areas connect to the Stop Server objective of [#1278]. Server/src/main.py adds the loopback, token-guarded /_unity/shutdown endpoint that the Unity stop path calls. IServerManagementServic…

Full details: Linked Issues check

Explanation

[#1278] The issue requires that Stop Server stops the local Python server on Windows, and that the button returns to Start Server. The reviewed head implements this in MCPForUnity/Editor/Services/ServerManagementService.cs. StopLocalHttpServerAsync sends an authenticated POST to /_unity/shutdown, using the per-launch token and the expected PID. It waits up to eight seconds through WaitForServerExitAsync. If the process still exists, it revalidates the token and terminates the process. It clears PID tracking only when the handshake still matches. It returns true only after the wait or the guarded termination completes. McpConnectionSection.cs awaits this method, so the button result reflects the outcome. The fallback path (ProcessTerminator and ProcessDetector) now checks actual process exit, not the taskkill exit code. This closes the reported case in which a non-forced taskkill reported success while the server kept running. The server-side endpoint is described in the supplied summary and its tests in Server/tests/test_server_shutdown.py. Those details could not be checked against source, because the main.py diff was unavailable at the reviewed head. The Unity-side fallback alone covers the issue requirement, so this verdict does not depend on that gap. Automated Unity EditMode tests (ServerShutdownTests.cs, ProcessTerminatorWindowsTests.cs) cover the cooperative, replaced-PID, stuck-server, and older-server paths. Live Windows runs, as reported by the author, show process exit and button recovery. No coding requirement is unmet.

Full details: Out of Scope Changes check

Explanation

All changed areas connect to the Stop Server objective of [#1278]. Server/src/main.py adds the loopback, token-guarded /_unity/shutdown endpoint that the Unity stop path calls. IServerManagementService.cs adds StopLocalHttpServerAsync, and McpConnectionSection.cs awaits it. ServerManagementService.cs adds the cooperative stop path and changes TryProcessCommandLineContainsInstanceToken to internal virtual so tests can override it. ProcessDetector.cs and ProcessTerminator.cs change the Windows fallback to verify actual exit, which is the stated defect. The Unix path change moves the graceful-kill wait into WaitForProcessExit with the same escalation. The PR description states this is a refactor that keeps the Unix fallback. The restart and Editor-quit cleanup paths now use the same process fallback. The PR description discloses this, and it applies the same stale-process fix to those paths. The added tests and .meta files support the new behavior. No unrelated feature or tool change appears. The PR states that no MCP tool or action is added. The main.py diff could not be read, so the endpoint details rely on the supplied summary.

Full details: Docstring Coverage

Explanation

Docstring coverage is 8.97% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 78 functions across 11 files. (2 skipped: 2 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

_{Comment @coderabbitai help to get the list of available commands.}