mcp: extract the shared request/completion choreography into McpRequest

  The preempt → install → complete/abandon ritual duplicated between
  ClickEntityTool/McpPointerEventSystem and WalkTool/McpInputOverrideSystem now
  lives in one place: IMcpRequest<TResult> marks a request component and the
  McpRequest helper owns SendAsync (preempting a pending request), CompleteAndRemove
  (copy out, structural removal, then resolve the awaiter) and AbandonAsync
  (tool-side timeout cleanup). McpMovementOverride joins the contract via an
  AsyncUnit completion.
  McpPointerEventIntent is now a genuinely immutable request: readonly fields set
  through the constructor, AimPoint + HasExplicitAimPoint merged into a nullable
  Vector3, and the Deadline field is gone — the system-side deadline duplicated the
  tool-side .Timeout(), so the tool watchdog is now the single timeout. As a result
  click_entity times out after exactly timeoutSec (previously timeoutSec + 2s
  grace) and a timeout always surfaces as an MCP error instead of sometimes an
  in-band hit:false.

  Tests: McpRequestShould covers install, preemption, complete-and-remove and a
  missing completion; the deadline test is removed with the mechanism.

Author: popuz

.claude/agents/mcp-server-engineer.md

@@ -32,8 +32,8 @@ Everything lives in `Explorer/Assets/DCL/McpServer/`, its own `DCL.McpServer` as
 | Plugin | `Systems/McpServerPlugin.cs` | Builds the tool registry in `InjectToWorld` (needs `GlobalPluginArguments.PlayerEntity/SkyboxEntity`), starts/disposes the server, wires the scene-log tap (`DiagnosticsContainer.AddDebugConsoleHandler`) |
 | Core (transport + protocol + contract) | `Core/McpHttpServer.cs`, `McpJsonRpcDispatcher.cs`, `McpTool.cs`, `McpToolsRegistry.cs`, `McpToolResult.cs`, `McpToolAnnotations.cs`, `McpJsonSchema.cs`, `McpWireEnum.cs` | `HttpListener` on `http://127.0.0.1:{port}/unity-explorer-mcp` (the URL template is single-sourced in `IDecentralandUrlsSource.LOCAL_MCP_ENDPOINT_URL`); POST → dispatch, GET → 405, Origin allowlist, 1 MB body cap; JSON-RPC 2.0 over Streamable HTTP (spec 2025-06-18), tools-only capability |
 | Tools | `Tools/*.cs` (one class per tool, 16) | The agent-facing surface |
-| Components | `Components/` — `McpMovementOverride`, `McpPointerClickIntent` | ECS intents for the input-driving tools |
-| Systems | `Systems/McpInputOverrideSystem.cs` (held movement), `Systems/McpPointerClickSystem.cs` (synthetic entity clicks) | Per-frame/pipeline-integrated drivers |
+| Components | `Components/` — `McpMovementOverride`, `McpPointerEventIntent` | ECS intents for the input-driving tools |
+| Systems | `Systems/McpInputOverrideSystem.cs` (held movement), `Systems/McpPointerEventSystem.cs` (synthetic pointer press/release delivery; `ClickEntityTool` composes a click from two intents) | Per-frame/pipeline-integrated drivers |
 | Utils | `Utils/SceneLogBuffer.cs`, `JObjectExtensions.cs` | Log tap buffer, args parsing |
 
 Registration: `DynamicWorldContainer.CreateAsync`, gated on `FeaturesRegistry` `FeatureId.MCP_SERVER` = `appArgs.HasFlag(MCP) || appArgs.HasFlag(MCP_PORT)` — so `--mcp-port` alone implies `--mcp` (presence check; an invalid port value still enables the server and falls back to 8123). Flags accepted from CLI **or** deep link by user decision — do not add CLI-only enforcement without being asked. Log category: `ReportCategory.MCP`.
@@ -92,6 +92,6 @@ Forbidden: `git commit`, `git push`, `git merge`, `git rebase`
 
 ## Roadmap context
 
-Milestone 2 (pointer clicks) SHIPPED 2026-07-05 as `click_entity` — implemented via **semantic injection**, not the originally-scoped synthetic `InputSystem` device: `McpPointerClickSystem` raycasts camera→target, mirrors the distance gate, and fills the entity's `PBPointerEvents.AppendPointerEventResultsIntent` so the unmodified `WritePointerEventResultsSystem` emits a byte-identical `PBPointerEventsResult`. Zero production interaction code changed; approach recorded in `~/.claude/plans/wondrous-forging-fox.md`.
+Milestone 2 (pointer clicks) SHIPPED 2026-07-05 as `click_entity` — implemented via **semantic injection**, not the originally-scoped synthetic `InputSystem` device: `McpPointerEventSystem` (née McpPointerClickSystem) raycasts camera→target, mirrors the distance gate, and fills the entity's `PBPointerEvents.AppendPointerEventResultsIntent` so the unmodified `WritePointerEventResultsSystem` emits a byte-identical `PBPointerEventsResult`. Zero production interaction code changed; approach recorded in `~/.claude/plans/wondrous-forging-fox.md`.
 
 Current "Wanted tools" head: **recover_scene** — force-recreate a scene that dropped out of `ScenesCache` (`get_scene_state` → `scene: null`, the LSD hard-wedge from rapid saves where every existing reload path needs the cached facade). Implementation lead is in the skill's Wanted tools entry.

Explorer/Assets/DCL/McpServer/Components/McpMovementOverride.cs

@@ -1,5 +1,6 @@
 using Cysharp.Threading.Tasks;
 using DCL.CharacterMotion.Components;
+using DCL.McpServer.Core;
 using UnityEngine;
 
 namespace DCL.McpServer.Components
@@ -8,7 +9,7 @@ namespace DCL.McpServer.Components
     ///     Held movement input requested by the MCP walk tool. While present on the player entity,
     ///     <see cref="McpInputOverrideSystem" /> re-asserts it into <see cref="MovementInputComponent" /> every frame.
     /// </summary>
-    public struct McpMovementOverride
+    public struct McpEcsMovementOverride : IMcpEcsRequest<AsyncUnit>
     {
         /// <summary>
         ///     Normalized camera-relative axes (x = strafe, y = forward).
@@ -30,6 +31,6 @@ public struct McpMovementOverride
         /// <summary>
         ///     Completed by the system when the hold expires or is preempted by a newer request.
         /// </summary>
-        public UniTaskCompletionSource? Completion;
+        public UniTaskCompletionSource<AsyncUnit>? Completion { get; set; }
     }
 }

Explorer/Assets/DCL/McpServer/Components/McpPointerEventIntent.cs

@@ -1,5 +1,6 @@
 using Cysharp.Threading.Tasks;
 using DCL.ECSComponents;
+using DCL.McpServer.Core;
 using UnityEngine;
 using RaycastHit = UnityEngine.RaycastHit;
 
@@ -8,31 +9,42 @@ namespace DCL.McpServer.Components
     /// <summary>
     ///     Present on the player entity while a single agent-requested pointer event awaits delivery.
     ///     McpPointerEventSystem validates the aim with a physics raycast, appends the event to the target's
-    ///     <see cref="PBPointerEvents.AppendPointerEventResultsIntent" /> and removes the component. The intent
-    ///     is written once and never mutated; a full click is composed by the click_entity tool from two intents —
-    ///     a press, then a release carrying the press <see cref="Press" /> handoff.
+    ///     <see cref="PBPointerEvents.AppendPointerEventResultsIntent" /> and removes the component. The request
+    ///     is immutable; a full click is composed by the click_entity tool from two intents — a press, then a
+    ///     release carrying the press <see cref="Press" /> handoff. A request the simulation never picks up is
+    ///     removed by the tool-side timeout.
     /// </summary>
-    public struct McpPointerEventIntent
+    public struct McpEcsPointerEventIntent : IMcpEcsRequest<McpPointerClickResult>
     {
         /// <summary>Arch entity id in the current scene world; -1 when aiming at an explicit world point.</summary>
-        public int TargetEntityId;
-        public Vector3 AimPoint;
-        public bool HasExplicitAimPoint;
-        public InputAction Button;
+        public readonly int TargetEntityId;
+
+        /// <summary>Explicit world-space aim point; when null the aim is the target's collider center.</summary>
+        public readonly Vector3? AimPoint;
+
+        public readonly InputAction Button;
 
         /// <summary>PetDown or PetUp.</summary>
-        public PointerEventType EventType;
+        public readonly PointerEventType EventType;
 
         /// <summary>
         ///     Set on the release leg of a click: the press this release must stay ordered after. Delivery waits
         ///     until the scene has advanced past the press tick, is bound to the world that received the press,
         ///     and falls back to the press-frame hit when the fresh ray no longer reaches the target.
         /// </summary>
-        public McpPressHandoff? Press;
+        public readonly McpPressHandoff? Press;
+
+        public UniTaskCompletionSource<McpPointerClickResult>? Completion { get; set; }
 
-        /// <summary>Time.time after which delivery is abandoned.</summary>
-        public float Deadline;
-        public UniTaskCompletionSource<McpPointerClickResult>? Completion;
+        public McpEcsPointerEventIntent(int targetEntityId, Vector3? aimPoint, InputAction button, PointerEventType eventType, McpPressHandoff? press = null)
+        {
+            TargetEntityId = targetEntityId;
+            AimPoint = aimPoint;
+            Button = button;
+            EventType = eventType;
+            Press = press;
+            Completion = null;
+        }
     }
 
     /// <summary>

Explorer/Assets/DCL/McpServer/Core/McpRequest.cs

@@ -0,0 +1,68 @@
+using Arch.Core;
+using Cysharp.Threading.Tasks;
+using Utility.Arch;
+
+namespace DCL.McpServer.Core
+{
+    /// <summary>
+    ///     A request component written onto an entity by an MCP tool and fulfilled by a consuming system.
+    ///     The shared install/complete/abandon choreography lives in <see cref="McpRequest" />.
+    /// </summary>
+    public interface IMcpEcsRequest<TResult>
+    {
+        /// <summary>
+        ///     Completed exactly once: with the outcome by the fulfilling system, or with a preemption result
+        ///     when a newer request replaces this one before it is fulfilled.
+        /// </summary>
+        UniTaskCompletionSource<TResult>? Completion { get; set; }
+    }
+
+    /// <summary>
+    ///     The request/response choreography shared by MCP intent components: a tool installs the request
+    ///     (preempting a pending one), the consuming system completes and removes it, and the tool-side timeout
+    ///     abandons a request the simulation never picked up.
+    /// </summary>
+    public static class McpRequest
+    {
+        /// <summary>
+        ///     Installs the request on the entity and returns the task its consuming system will complete.
+        ///     A pending request of the same type is preempted: its awaiter is released with
+        ///     <paramref name="preemptedResult" /> before the component is replaced. Main thread only.
+        /// </summary>
+        public static UniTask<TResult> SendAsync<TIntent, TResult>(World world, Entity entity, TIntent request, TResult preemptedResult)
+            where TIntent : struct, IMcpEcsRequest<TResult>
+        {
+            if (world.TryGet(entity, out TIntent existing))
+                existing.Completion?.TrySetResult(preemptedResult);
+
+            var completion = new UniTaskCompletionSource<TResult>();
+            request.Completion = completion;
+            world.AddOrSet(entity, request);
+
+            return completion.Task;
+        }
+
+        /// <summary>
+        ///     Removes the request component, then releases its awaiter with <paramref name="result" />.
+        ///     Takes the request by copy and removes before completing, so the awaiter's continuation observes
+        ///     the entity without the component; the caller must be done with any refs into the entity, as the
+        ///     removal is a structural change.
+        /// </summary>
+        public static void CompleteAndRemove<TIntent, TResult>(World world, Entity entity, TIntent request, TResult result)
+            where TIntent : struct, IMcpEcsRequest<TResult>
+        {
+            UniTaskCompletionSource<TResult>? completion = request.Completion;
+            world.Remove<TIntent>(entity);
+            completion?.TrySetResult(result);
+        }
+
+        /// <summary>Drops a request the simulation never completed (tool-side timeout). Safe to call from any thread.</summary>
+        public static async UniTask AbandonAsync<TIntent>(World world, Entity entity) where TIntent : struct
+        {
+            await UniTask.SwitchToMainThread();
+
+            if (world.Has<TIntent>(entity))
+                world.Remove<TIntent>(entity);
+        }
+    }
+}

Explorer/Assets/DCL/McpServer/Core/McpRequest.cs.meta

@@ -7,13 +7,14 @@
 using DCL.Diagnostics;
 using DCL.Input;
 using DCL.McpServer.Components;
+using DCL.McpServer.Core;
 using ECS.Abstract;
 using UnityEngine;
 
 namespace DCL.McpServer.Systems
 {
     /// <summary>
-    ///     While an <see cref="McpMovementOverride" /> is present on the player entity, re-asserts its axes into
+    ///     While an <see cref="McpEcsMovementOverride" /> is present on the player entity, re-asserts its axes into
     ///     <see cref="MovementInputComponent" /> after the real-input systems have written it, so an agent-requested
     ///     walk survives the per-frame overwrite performed by <see cref="UpdateInputMovementSystem" />.
     /// </summary>
@@ -40,24 +41,24 @@ public override void Initialize()
 
         protected override void Update(float t)
         {
-            ref McpMovementOverride movementOverride = ref World.TryGetRef<McpMovementOverride>(playerEntity, out bool overrideExists);
+            ref McpEcsMovementOverride ecsMovementOverride = ref World.TryGetRef<McpEcsMovementOverride>(playerEntity, out bool overrideExists);
 
             if (!overrideExists)
                 return;
 
             ref MovementInputComponent movement = ref World.TryGetRef<MovementInputComponent>(playerEntity, out bool hasMovement);
 
-            if (UnityEngine.Time.time < movementOverride.EndTime)
+            if (UnityEngine.Time.time < ecsMovementOverride.EndTime)
             {
                 if (hasMovement)
                 {
-                    movement.Axes = movementOverride.Axes;
-                    movement.Kind = movementOverride.Kind;
+                    movement.Axes = ecsMovementOverride.Axes;
+                    movement.Kind = ecsMovementOverride.Kind;
                 }
 
-                if (movementOverride.JumpRequested)
+                if (ecsMovementOverride.JumpRequested)
                 {
-                    movementOverride.JumpRequested = false;
+                    ecsMovementOverride.JumpRequested = false;
 
                     ref JumpInputComponent jump = ref World.TryGetRef<JumpInputComponent>(playerEntity, out bool hasJump);
 
@@ -67,17 +68,14 @@ protected override void Update(float t)
             }
             else
             {
-                UniTaskCompletionSource? completion = movementOverride.Completion;
-
                 if (hasMovement)
                 {
                     movement.Axes = Vector2.zero;
                     movement.Kind = MovementKind.IDLE;
                 }
 
-                // Structural change only after all outstanding component refs are done.
-                World.Remove<McpMovementOverride>(playerEntity);
-                completion?.TrySetResult();
+                // The override is copied out before the structural removal; no component refs are touched afterwards.
+                McpRequest.CompleteAndRemove(World, playerEntity, ecsMovementOverride, AsyncUnit.Default);
             }
         }
     }