feat: extract in-process MCP action layer, concurrent task dispatch, and spawn/reconcile hardening

- Extract device action execution into artemis/mcp action layer: typed ActionResult, capability manifest with startup validation, AdbActuator as the single physical implementation, in-memory ActionSession (replaces stdio subprocess MCP in validator), and McpActionExecutor preserving the legacy executor contract. adb_server keeps its external stdio/SSE surface unchanged and now reuses the actuator implementation; import-time side effects moved into configure_stdio_mode(). - Capability-gated prompt assembly (prompt_assembly.py): tool enums and teaching segments render per available backend tools; full-set output is byte-identical to the previous static text (locked by tests). - Admin console task queue: serial worker replaced by a per-device concurrent dispatcher (ARTEMIS_CONCURRENCY_MODE=global restores serial semantics), per-session cancellation, strict device binding on enqueue, unified stdio piping with tee to trace stdout.log, and shutdown cleanup of all active runs. Fix _pid_exists exception order so PermissionError is treated as alive. - mcp_server reliability: spawn watchdog kills runners stuck before interpreter startup and releases device reservations; task state reconcile uses psutil liveness, prefers runner-reported pids, and adds a 45s startup grace to stop false failure reports on Windows; conversation_id is now optional end-to-end; explicit device_serial is validated at submission. - CLI install now writes ~/.claude/rules/artemis.md for Claude Code and migrates/removes the legacy CLAUDE.md injection block; ~/.claude.json is never rewritten when it fails to parse. - Tests: Windows-portable stdio lifecycle and path assertions; new suites for action layer, prompt assembly, spawn watchdog, and task reconcile (895 passed, 3 skipped).
2026-08-27 18:25:27 -07:00
1 parent 60312abbb0
commit cbdca1497c
@@ -0,0 +1,294 @@
+# Copyright 2026 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Prompt <-> manifest consistency and actuator contract tests.
+
+The scan below re-derives the manifest classification from the prompt sources, so
+adding a tool name to a prompt without classifying it in
+``artemis/mcp/action_manifest.py`` fails here rather than at runtime.
+"""
+
+import json
+from pathlib import Path
+import re
+
+import pytest
+
+from artemis.core.tool_declaration import ToolDeclaration
+from artemis.mcp.action_manifest import (
+    ActuatorContractError,
+    AGENT_ROLES,
+    ALL_KNOWN_TOOLS,
+    BACKEND_INDEPENDENT_TOOLS,
+    DEVICE_ACTIONS,
+    ExtensionTool,
+    INTERNAL_ACTIONS,
+    OPTIONAL_ACTIONS,
+    REQUIRED_ACTIONS,
+    available_device_actions,
+    filter_declarations,
+    validate_actuator,
+)
+
+REPO_ROOT = Path(__file__).resolve().parents[3]
+
+# Prompt sources belonging to the three agents this manifest governs (plus the
+# Validator's safety net). Other agents (log_analyzer, image_processor, ...) have their
+# own tool universes and are deliberately out of scope.
+SCOPED_PROMPT_FILES = [
+    "artemis/agents/flash/flash_runner.md",
+    "artemis/agents/validator/failure_analyzer.md",
+    "artemis/agents/validator/target_disappeared_analyzer.md",
+    "artemis/agents/validator/pixel_target_disappeared_analyzer.md",
+    "artemis/agents/validator/pixel_safety_net.md",
+]
+
+# Backtick-quoted identifiers in scoped prompts that are not tools: parameter names and
+# note keys. Anything new landing here should be a conscious decision.
+NON_TOOL_IDENTIFIERS = frozenset({"analysis", "task_plan"})
+
+_TOOL_REF = re.compile(r"(?:`([a-z][a-z0-9_]{2,})`|\b([a-z][a-z0-9_]{2,})\()")
+
+
+def _scoped_prompt_sources() -> dict[str, str]:
+    """Collects all in-scope prompt texts, keyed by a short source label."""
+    sources: dict[str, str] = {}
+    for rel in SCOPED_PROMPT_FILES:
+        path = REPO_ROOT / rel
+        sources[path.name] = path.read_text(encoding="utf-8")
+
+    operator_json = REPO_ROOT / "artemis/agents/operator/operator.json"
+    for key, text in json.loads(operator_json.read_text(encoding="utf-8")).items():
+        sources[f"operator.json:{key}"] = text
+
+    # Only the prompt string constants from prompts.py -- scanning the module as text
+    # would pick up Python code (`len(`, `getattr(` ...).
+    from artemis.agents.operator import prompts as operator_prompts
+
+    for name in dir(operator_prompts):
+        value = getattr(operator_prompts, name)
+        if name.startswith("_") and isinstance(value, str) and len(value) > 80:
+            sources[f"prompts.py:{name}"] = value
+
+    return sources
+
+
+def _referenced_tools() -> dict[str, set[str]]:
+    """Returns {identifier: source labels} for every tool-shaped reference in scope."""
+    found: dict[str, set[str]] = {}
+    for label, text in _scoped_prompt_sources().items():
+        for match in _TOOL_REF.finditer(text):
+            name = match.group(1) or match.group(2)
+            found.setdefault(name, set()).add(label)
+    return found
+
+
+# --- Prompt <-> manifest invariants --------------------------------------------------
+
+
+def test_scan_finds_prompt_sources():
+    """The scan must cover all expected sources; silent scope loss would void the suite."""
+    sources = _scoped_prompt_sources()
+    assert "flash_runner.md" in sources
+    assert "operator.json:main_template" in sources
+    assert "operator.json:troubleshooter_template" in sources
+    assert any(label.startswith("prompts.py:") for label in sources)
+
+
+def test_every_prompt_tool_reference_is_classified():
+    """Every tool referenced by an in-scope prompt must be known to the manifest."""
+    unclassified = {
+        name: sorted(labels)
+        for name, labels in _referenced_tools().items()
+        if name not in ALL_KNOWN_TOOLS and name not in NON_TOOL_IDENTIFIERS
+    }
+    assert not unclassified, (
+        "Prompts reference tools the manifest does not classify. Add each to the "
+        f"appropriate set in artemis/mcp/action_manifest.py: {unclassified}"
+    )
+
+
+def test_required_actions_are_structurally_referenced():
+    """A REQUIRED action must actually appear in a prompt; otherwise demote it."""
+    referenced = set(_referenced_tools())
+    unreferenced = REQUIRED_ACTIONS - referenced
+    assert not unreferenced, (
+        f"REQUIRED_ACTIONS not referenced by any prompt: {sorted(unreferenced)}. "
+        "Required status exists only to protect structural prompt dependencies."
+    )
+
+
+def test_internal_actions_never_appear_in_prompts():
+    """INTERNAL actions are adapter plumbing and must stay invisible to every model."""
+    leaked = INTERNAL_ACTIONS & set(_referenced_tools())
+    assert not leaked, f"Internal actions leaked into prompts: {sorted(leaked)}"
+
+
+def test_manifest_sets_are_disjoint():
+    sets = {
+        "REQUIRED_ACTIONS": REQUIRED_ACTIONS,
+        "OPTIONAL_ACTIONS": OPTIONAL_ACTIONS,
+        "INTERNAL_ACTIONS": INTERNAL_ACTIONS,
+        "BACKEND_INDEPENDENT_TOOLS": BACKEND_INDEPENDENT_TOOLS,
+    }
+    names = list(sets)
+    for i, a in enumerate(names):
+        for b in names[i + 1 :]:
+            overlap = sets[a] & sets[b]
+            assert not overlap, f"{a} and {b} overlap: {sorted(overlap)}"
+
+
+# --- Actuator contract ---------------------------------------------------------------
+
+
+class _StubActuator:
+    """Minimal actuator test double with configurable capabilities and extensions."""
+
+    def __init__(self, caps=None, exts=None):
+        self._caps = frozenset(caps if caps is not None else DEVICE_ACTIONS)
+        self._exts = list(exts or [])
+
+    def capabilities(self):
+        return self._caps
+
+    def extensions(self):
+        return self._exts
+
+
+def _extension(**overrides) -> ExtensionTool:
+    async def _noop(**_kwargs):
+        return "ok"
+
+    defaults = dict(
+        name="calibrate_arm",
+        description=(
+            "Runs the robot arm's touch calibration sweep. Use before the first "
+            "physical action of a session, or after any missed tap."
+        ),
+        input_schema={"type": "object", "properties": {}},
+        handler=_noop,
+        targets=frozenset({"operator", "flash"}),
+    )
+    defaults.update(overrides)
+    return ExtensionTool(**defaults)
+
+
+def test_full_actuator_passes_validation():
+    validate_actuator(_StubActuator())
+
+
+def test_minimal_actuator_passes_validation():
+    """REQUIRED + INTERNAL alone is a legal backend; every optional action may be absent."""
+    validate_actuator(_StubActuator(caps=REQUIRED_ACTIONS | INTERNAL_ACTIONS))
+
+
+def test_missing_required_action_fails_fast():
+    caps = (DEVICE_ACTIONS - {"click_sequence"})
+    with pytest.raises(ActuatorContractError, match="click_sequence"):
+        validate_actuator(_StubActuator(caps=caps))
+
+
+def test_missing_internal_action_fails_fast():
+    caps = DEVICE_ACTIONS - {"observe_screen"}
+    with pytest.raises(ActuatorContractError, match="observe_screen"):
+        validate_actuator(_StubActuator(caps=caps))
+
+
+def test_unknown_capability_is_rejected():
+    """Backend-specific tools must go through extensions(), not capabilities()."""
+    with pytest.raises(ActuatorContractError, match="calibrate_arm"):
+        validate_actuator(_StubActuator(caps=DEVICE_ACTIONS | {"calibrate_arm"}))
+
+
+def test_extension_name_collision_fails_fast():
+    with pytest.raises(ActuatorContractError, match="collides"):
+        validate_actuator(_StubActuator(exts=[_extension(name="click")]))
+
+
+def test_duplicate_extension_names_fail_fast():
+    with pytest.raises(ActuatorContractError, match="more than once"):
+        validate_actuator(_StubActuator(exts=[_extension(), _extension()]))
+
+
+def test_extension_with_thin_description_is_rejected():
+    """Extensions get no prompt-level teaching; the description carries the contract."""
+    with pytest.raises(ActuatorContractError, match="description"):
+        validate_actuator(_StubActuator(exts=[_extension(description="Calibrates.")]))
+
+
+def test_extension_with_unknown_target_is_rejected():
+    with pytest.raises(ActuatorContractError, match="unknown agent role"):
+        validate_actuator(_StubActuator(exts=[_extension(targets=frozenset({"planner"}))]))
+
+
+# --- Surface derivation --------------------------------------------------------------
+
+
+def _declarations(*names: str) -> list[ToolDeclaration]:
+    return [
+        ToolDeclaration(name=n, description=f"{n} tool", parameters={"type": "object"})
+        for n in names
+    ]
+
+
+def test_available_device_actions_excludes_internal():
+    actions = available_device_actions(_StubActuator())
+    assert actions == REQUIRED_ACTIONS | OPTIONAL_ACTIONS
+    assert not actions & INTERNAL_ACTIONS
+
+
+def test_filter_drops_unimplemented_optional_actions():
+    actuator = _StubActuator(caps=DEVICE_ACTIONS - {"swipe", "manage_app"})
+    decls = _declarations("click", "swipe", "manage_app", "read_note")
+    kept = [d.name for d in filter_declarations(decls, actuator, agent="flash")]
+    assert kept == ["click", "read_note"]
+
+
+def test_filter_passes_through_unclassified_agent_tools():
+    """Agent-specific tools the manifest does not know must never be gated."""
+    actuator = _StubActuator(caps=REQUIRED_ACTIONS | INTERNAL_ACTIONS)
+    decls = _declarations("click_sequence", "reply_to_checker", "some_bespoke_tool")
+    kept = [d.name for d in filter_declarations(decls, actuator, agent="operator")]
+    assert kept == ["click_sequence", "reply_to_checker", "some_bespoke_tool"]
+
+
+def test_filter_appends_extensions_for_targeted_agents_only():
+    actuator = _StubActuator(exts=[_extension(targets=frozenset({"flash"}))])
+    decls = _declarations("click")
+
+    flash = [d.name for d in filter_declarations(decls, actuator, agent="flash")]
+    operator = [d.name for d in filter_declarations(decls, actuator, agent="operator")]
+
+    assert flash == ["click", "calibrate_arm"]
+    assert operator == ["click"]
+
+
+def test_filter_extension_declaration_shape():
+    """Extension declarations must be indistinguishable in shape from known ones."""
+    actuator = _StubActuator(exts=[_extension()])
+    decls = filter_declarations([], actuator, agent="operator")
+    assert len(decls) == 1
+    ext_decl = decls[0]
+    assert isinstance(ext_decl, ToolDeclaration)
+    assert ext_decl.name == "calibrate_arm"
+    assert ext_decl.parameters == {"type": "object", "properties": {}}
+
+
+def test_filter_rejects_unknown_agent_role():
+    with pytest.raises(ValueError, match="Unknown agent role"):
+        filter_declarations([], _StubActuator(), agent="planner")
+
+
+def test_all_agent_roles_are_covered():
+    assert AGENT_ROLES == {"operator", "flash", "failure_analyzer", "validator"}
@@ -0,0 +1,156 @@
+# Copyright 2026 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""End-to-end tests of the unified action server over the in-memory transport."""
+
+import pytest
+
+from artemis.mcp.action_manifest import (
+    ActuatorContractError,
+    DEVICE_ACTIONS,
+    INTERNAL_ACTIONS,
+    REQUIRED_ACTIONS,
+    ExtensionTool,
+)
+from artemis.mcp.action_server import build_action_server
+from artemis.mcp.action_session import ActionSession
+from artemis.mcp.actuators import MockActuator
+
+pytestmark = pytest.mark.asyncio
+
+
+async def _session_for(actuator) -> ActionSession:
+    return await ActionSession(build_action_server(actuator)).start()
+
+
+async def test_click_end_to_end():
+    actuator = MockActuator(width=1000, height=2000)
+    session = await _session_for(actuator)
+    try:
+        res = await session.call("click", {"target": [500, 500]})
+        assert res.ok
+        assert res.message == "Clicked at [500, 500] (normalized) successfully."
+        assert actuator.action_history[-1]["action"] == "tap"
+        assert actuator.action_history[-1]["x"] == 500
+        assert actuator.action_history[-1]["y"] == 1000
+    finally:
+        await session.aclose()
+
+
+async def test_device_refusal_is_not_a_protocol_error():
+    """A failed action reaches the caller as ok=False, never as an exception."""
+    actuator = MockActuator()
+    session = await _session_for(actuator)
+    try:
+        res = await session.call("press_key", {"key": ""})
+        assert not res.ok
+        assert res.code.value == "INVALID_ARGS"
+        assert res.message == "Error executing key press ''."
+    finally:
+        await session.aclose()
+
+
+async def test_partial_actuator_hides_optional_tools():
+    """An unimplemented optional tool does not exist anywhere on the surface."""
+    minimal = MockActuator(capabilities=REQUIRED_ACTIONS | INTERNAL_ACTIONS)
+    session = await _session_for(minimal)
+    try:
+        listed = {t.name for t in (await session._session.list_tools()).tools}
+        assert "click_sequence" in listed
+        assert "observe_screen" in listed
+        for absent in ("click", "swipe", "manage_app", "wait_for_text", "open_link"):
+            assert absent not in listed
+    finally:
+        await session.aclose()
+
+
+async def test_missing_required_action_fails_at_build_time():
+    broken = MockActuator(capabilities=DEVICE_ACTIONS - {"click_sequence"})
+    with pytest.raises(ActuatorContractError, match="click_sequence"):
+        build_action_server(broken)
+
+
+async def test_extension_tool_is_served_and_callable():
+    async def calibrate(sweep_speed: int = 1) -> str:
+        return f"calibrated at speed {sweep_speed}"
+
+    ext = ExtensionTool(
+        name="calibrate_arm",
+        description=(
+            "Runs the robot arm's touch calibration sweep. Use before the first "
+            "physical action of a session, or after any missed tap."
+        ),
+        input_schema={"type": "object", "properties": {}},
+        handler=calibrate,
+        targets=frozenset({"flash"}),
+    )
+    actuator = MockActuator(extensions=[ext])
+    session = await _session_for(actuator)
+    try:
+        listed = {t.name for t in (await session._session.list_tools()).tools}
+        assert "calibrate_arm" in listed
+        result = await session.call_raw("calibrate_arm", {"sweep_speed": 3})
+        text = next(b.text for b in result.content if b.type == "text")
+        assert text == "calibrated at speed 3"
+    finally:
+        await session.aclose()
+
+
+async def test_extension_name_collision_fails_at_build_time():
+    async def bogus() -> str:
+        return "x"
+
+    ext = ExtensionTool(
+        name="click",
+        description="An extension improperly reusing a canonical tool name, which "
+        "must be rejected at build time.",
+        input_schema={"type": "object"},
+        handler=bogus,
+    )
+    with pytest.raises(ActuatorContractError, match="collides"):
+        build_action_server(MockActuator(extensions=[ext]))
+
+
+async def test_unknown_tool_raises_protocol_error():
+    session = await _session_for(MockActuator())
+    try:
+        with pytest.raises(RuntimeError, match="not_a_tool"):
+            await session.call_raw("not_a_tool", {})
+    finally:
+        await session.aclose()
+
+
+async def test_screenshot_and_hierarchy_helpers():
+    session = await _session_for(MockActuator(width=1000, height=2000))
+    try:
+        b64 = await session.screenshot_b64()
+        assert isinstance(b64, str) and len(b64) > 0
+        elements = await session.ui_hierarchy()
+        assert elements is not None
+    finally:
+        await session.aclose()
+
+
+async def test_structured_content_carries_real_status():
+    """The wire result exposes ok/code so no consumer ever sniffs substrings again."""
+    session = await _session_for(MockActuator())
+    try:
+        raw = await session.call_raw("click", {"target": [10, 10]})
+        sc = raw.structuredContent
+        assert sc["ok"] is True
+        assert sc["code"] == "OK"
+        assert sc["action"] == "click"
+        assert raw.isError is False
+    finally:
+        await session.aclose()
@@ -0,0 +1,188 @@
+# Copyright 2026 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""ActionSession lifecycle tests, including the anyio same-task exit hazard."""
+
+import asyncio
+
+import pytest
+
+from artemis.mcp.action_server import build_action_server
+from artemis.mcp.action_session import ActionSession, get_action_session
+from artemis.mcp.actuators import MockActuator
+
+pytestmark = pytest.mark.asyncio
+
+
+async def test_start_and_close():
+    session = ActionSession(build_action_server(MockActuator()))
+    await session.start()
+    assert session.started
+    await session.aclose()
+    assert not session.started
+
+
+async def test_close_from_different_task():
+    """The regression guard for the anyio cancel-scope same-task rule.
+
+    The in-memory transport's task group is entered inside the owner task; closing
+    from any *other* task must not raise ``RuntimeError: Attempted to exit cancel
+    scope in a different task``.
+    """
+    session = ActionSession(build_action_server(MockActuator()))
+    await session.start()
+
+    async def _closer():
+        await session.aclose()
+
+    await asyncio.create_task(_closer())
+    assert not session.started
+
+
+async def test_caller_timeout_does_not_brick_the_session():
+    """Regression: cancelling a caller must never corrupt the shared transport.
+
+    Observed live on-device: ``asyncio.wait_for(session.ui_hierarchy(), 1.0)`` around
+    a slow hierarchy fetch cancelled ``call_tool`` mid-flight, corrupting the
+    in-memory streams -- every later call failed with an empty-message
+    ``BrokenResourceError`` and the server never received another request. The owner
+    request queue makes an abandoned caller harmless; this test pins that down.
+    """
+    actuator = MockActuator(width=1000, height=2000)
+
+    async def _slow_get_ui_elements():
+        await asyncio.sleep(2.0)
+        return []
+
+    actuator.get_ui_elements = _slow_get_ui_elements
+    session = await ActionSession(build_action_server(actuator)).start()
+    try:
+        with pytest.raises(asyncio.TimeoutError):
+            await asyncio.wait_for(session.ui_hierarchy(), timeout=0.1)
+
+        # The session must still be fully functional afterwards.
+        result = await session.call("click", {"target": [500, 500]})
+        assert result.ok
+        taps = [h for h in actuator.action_history if h["action"] == "tap"]
+        assert len(taps) == 1
+    finally:
+        await session.aclose()
+
+
+async def test_per_call_timeout_fires_server_side_and_session_survives():
+    """A per-call ``timeout`` runs inside the server tool, never on the transport.
+
+    mcp 1.29's ``read_timeout_seconds`` is NOT safe on the in-memory transport: a
+    late response to a timed-out request crashes the client receive loop
+    (``_handle_response`` sends into the request's already-closed response stream) --
+    observed live on-device as a session bricked after a single clean
+    ``McpError`` timeout. Server-side timeouts produce an ordinary error payload
+    and no late response, so the session stays healthy by construction.
+    """
+    actuator = MockActuator(width=1000, height=2000)
+
+    async def _slow_get_ui_elements():
+        await asyncio.sleep(2.0)
+        return []
+
+    actuator.get_ui_elements = _slow_get_ui_elements
+    session = await ActionSession(build_action_server(actuator)).start()
+    try:
+        with pytest.raises(RuntimeError, match="timed out"):
+            await session.ui_hierarchy(timeout=0.1)
+
+        result = await session.call("click", {"target": [500, 500]})
+        assert result.ok
+    finally:
+        await session.aclose()
+
+
+async def test_transport_death_retires_session_for_rebuild():
+    """A dead transport must flip ``started`` so get_action_session rebuilds.
+
+    Without this, one transport failure leaves the rest of a run failing on a
+    corpse (observed live: 24 minutes of empty-message ``ClosedResourceError``).
+    """
+    import anyio
+
+    session = await ActionSession(build_action_server(MockActuator())).start()
+    real_call_tool = session._session.call_tool
+
+    async def _dying_call_tool(name, args, **kw):
+        raise anyio.ClosedResourceError()
+
+    session._session.call_tool = _dying_call_tool
+    with pytest.raises(anyio.ClosedResourceError):
+        await session.call("click", {"target": [1, 1]})
+
+    # Owner loop retires; the session reports itself unusable. The owner task may
+    # finish with an exception group from the transport teardown -- aclose() is the
+    # API that swallows that, so here we just wait for `started` to flip.
+    for _ in range(50):
+        if not session.started:
+            break
+        await asyncio.sleep(0.1)
+    assert not session.started
+
+    class Ctx:
+        action_session = session
+
+    fresh = await get_action_session(Ctx(), actuator=MockActuator())
+    assert fresh is not session
+    assert fresh.started
+    await fresh.aclose()
+    del real_call_tool
+
+
+async def test_calls_are_serialized_by_the_lock():
+    """Two concurrent calls must not interleave on the single device."""
+    actuator = MockActuator(width=1000, height=2000)
+    session = await ActionSession(build_action_server(actuator)).start()
+    try:
+        await asyncio.gather(
+            session.call("click", {"target": [100, 100]}),
+            session.call("click", {"target": [900, 900]}),
+        )
+        taps = [h for h in actuator.action_history if h["action"] == "tap"]
+        assert len(taps) == 2
+    finally:
+        await session.aclose()
+
+
+async def test_call_before_start_raises():
+    session = ActionSession(build_action_server(MockActuator()))
+    with pytest.raises(RuntimeError, match="not started"):
+        await session.call("click", {"target": [1, 1]})
+
+
+async def test_double_start_is_idempotent():
+    session = ActionSession(build_action_server(MockActuator()))
+    await session.start()
+    owner = session._owner
+    await session.start()
+    assert session._owner is owner
+    await session.aclose()
+
+
+async def test_get_action_session_caches_on_ctx():
+    class Ctx:
+        action_session = None
+
+    ctx = Ctx()
+    actuator = MockActuator()
+    session = await get_action_session(ctx, actuator=actuator)
+    assert ctx.action_session is session
+    again = await get_action_session(ctx)
+    assert again is session
+    await session.aclose()
@@ -0,0 +1,146 @@
+# Copyright 2026 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Actuator layer tests: coordinate conversion, message parity, driver dispatch."""
+
+import pytest
+
+from artemis.mcp.action_manifest import DEVICE_ACTIONS
+from artemis.mcp.action_types import ActionCode
+from artemis.mcp.actuators import Actuator, AdbActuator, MockActuator
+
+
+@pytest.fixture
+def actuator() -> MockActuator:
+    return MockActuator(width=1000, height=2000)
+
+
+def test_mock_actuator_satisfies_protocol(actuator):
+    assert isinstance(actuator, Actuator)
+    assert isinstance(actuator, AdbActuator)
+    assert actuator.capabilities() == DEVICE_ACTIONS
+    assert actuator.extensions() == []
+
+
+@pytest.mark.asyncio
+async def test_click_converts_normalized_to_pixels(actuator):
+    res = await actuator.click(500, 500)
+    assert res.ok and res.code is ActionCode.OK
+    # Historical wording is part of the contract: transcripts and traces assert on it.
+    assert res.message == "Clicked at [500, 500] (normalized) successfully."
+    assert res.normalized_coordinates == [500, 500]
+    assert actuator.action_history[-1] == {
+        "action": "tap",
+        "x": 500,
+        "y": 1000,
+        "times": 1,
+        "duration_ms": 100,
+    }
+
+
+@pytest.mark.asyncio
+async def test_click_sequence_taps_each_point(actuator):
+    res = await actuator.click_sequence([(100, 100), (900, 900)], delay_ms=1)
+    assert res.ok
+    assert res.message == (
+        "Sequence clicked successfully: Tapped at [100, 200]; Tapped at [900, 1800]"
+    )
+    taps = [h for h in actuator.action_history if h["action"] == "tap"]
+    assert [(t["x"], t["y"]) for t in taps] == [(100, 200), (900, 1800)]
+
+
+@pytest.mark.asyncio
+async def test_long_press_message_and_duration(actuator):
+    res = await actuator.long_press(250, 750, duration_ms=1500)
+    assert res.ok
+    assert res.message == (
+        "Long pressed at [250, 750] (normalized) for 1500ms successfully."
+    )
+    assert res.duration_ms == 1500
+
+
+@pytest.mark.asyncio
+async def test_swipe_reports_normalized_endpoints(actuator):
+    res = await actuator.swipe((500, 800), (500, 200), 400)
+    assert res.ok
+    assert res.message == "Swipe completed successfully. Swiped from [500, 800] to [500, 200]."
+    assert res.normalized_coordinates == [500, 800, 500, 200]
+
+
+@pytest.mark.asyncio
+async def test_press_key_forwards_unknown_keycode_to_driver(actuator):
+    # Arbitrary keycodes (KEYCODE_* names, numeric codes) pass through to the driver,
+    # matching the historical adb_server behavior of accepting any Android key event.
+    res = await actuator.press_key("KEYCODE_DPAD_DOWN")
+    assert res.ok
+    assert res.message == "Executed key press 'KEYCODE_DPAD_DOWN'."
+    assert actuator.action_history[-1]["action"] == "press_key"
+
+
+@pytest.mark.asyncio
+async def test_press_key_rejects_empty_key(actuator):
+    res = await actuator.press_key("")
+    assert not res.ok
+    assert res.code is ActionCode.INVALID_ARGS
+
+
+@pytest.mark.asyncio
+async def test_input_text_append_moves_cursor_and_preserves_text(actuator, monkeypatch):
+    monkeypatch.setattr("asyncio.sleep", _instant_sleep)
+    res = await actuator.input_text("hello", (500, 300), clear_exist=False)
+    assert res.ok
+    assert res.message == "Executed typing 'hello'."
+    actions = [h["action"] for h in actuator.action_history]
+    # Append semantics: focus tap, cursor-to-end (KEYCODE_MOVE_END), then type without
+    # clearing — matching the historical adb_server focus_and_input_text contract.
+    assert actions == ["tap", "press_key", "input_text"]
+    typed = actuator.action_history[-1]
+    assert typed["clear_existing"] is False
+    cursor = actuator.action_history[-2]
+    assert cursor["key"] == "123"
+
+
+@pytest.mark.asyncio
+async def test_input_text_clear_erases_then_types_without_reclearing(actuator, monkeypatch):
+    monkeypatch.setattr("asyncio.sleep", _instant_sleep)
+    res = await actuator.input_text("hello", (500, 300), clear_exist=True)
+    assert res.ok
+    actions = [h["action"] for h in actuator.action_history]
+    # The controller's full clear runs as a shell keyevent batch on this driver.
+    assert actions == ["tap", "execute_shell", "input_text"]
+    assert actuator.action_history[-1]["clear_existing"] is False
+
+
+async def _instant_sleep(_delay, *args, **kwargs):
+    return None
+
+
+@pytest.mark.asyncio
+async def test_wait_for_delay(actuator):
+    res = await actuator.wait_for_delay(1)
+    assert res.ok
+    assert res.message == "Waited for 1ms successfully."
+
+
+def test_partial_capabilities_are_reported():
+    partial = MockActuator(capabilities=frozenset({"click_sequence"}))
+    assert partial.capabilities() == frozenset({"click_sequence"})
+
+
+@pytest.mark.asyncio
+async def test_observation_primitives(actuator):
+    shot = await actuator.take_screenshot()
+    assert isinstance(shot, str) and len(shot) > 0
+    data = await actuator.get_screen_data()
+    assert data.width == 1000 and data.height == 2000
@@ -20,8 +20,10 @@ subprocess stdin isolation against pipe hijacking, and stdout purity.
 import io
 import json
 import os
+import queue
 import subprocess
 import sys
+import threading
 import time
 from unittest.mock import MagicMock, patch

@@ -34,6 +36,31 @@ from artemis.utils.logger import get_logger
 from mcp_server.utils import device_utils, env_utils


+def _readline_with_timeout(pipe, timeout: float) -> str | None:
+    """Read one line from a subprocess pipe with a timeout, portably.
+
+    select.select only supports sockets on Windows, so poll via a reader thread
+    instead of selecting on the pipe handle.
+    """
+    result: queue.Queue = queue.Queue()
+
+    def _reader():
+        try:
+            result.put(pipe.readline())
+        except Exception:
+            result.put(b"")
+
+    reader = threading.Thread(target=_reader, daemon=True)
+    reader.start()
+    try:
+        line = result.get(timeout=timeout)
+    except queue.Empty:
+        return None
+    if not line:
+        return None
+    return line.decode("utf-8").strip()
+
+
 def test_mcp_stdio_handshake_immediate_input():
    """Verify that MCP server over stdio responds to an immediate initialize request

@@ -51,6 +78,24 @@ def test_mcp_stdio_handshake_immediate_input():
            "PYTHONUNBUFFERED": "1",
            "PATH": os.environ.get("PATH", ""),
            "PYTHONPATH": project_root,
+            # Windows: the interpreter's socket stack (Winsock/_overlapped),
+            # Path.home(), and tempfile handling need these system variables.
+            **{
+                key: os.environ[key]
+                for key in (
+                    "SYSTEMROOT",
+                    "SYSTEMDRIVE",
+                    "WINDIR",
+                    "TEMP",
+                    "TMP",
+                    "USERPROFILE",
+                    "HOMEDRIVE",
+                    "HOMEPATH",
+                    "APPDATA",
+                    "LOCALAPPDATA",
+                )
+                if key in os.environ
+            },
        },
        cwd=project_root,
    )
@@ -71,15 +116,7 @@ def test_mcp_stdio_handshake_immediate_input():
        p.stdin.flush()

        # Read initialize response with a strict timeout
-        start = time.time()
-        init_resp_line = None
-        while time.time() - start < 6.0:
-            import select
-
-            r, _, _ = select.select([p.stdout], [], [], 0.2)
-            if r:
-                init_resp_line = p.stdout.readline().decode("utf-8").strip()
-                break
+        init_resp_line = _readline_with_timeout(p.stdout, 6.0)

        assert init_resp_line is not None, "MCP server failed to respond to initialize request within 6 seconds (deadlock detected)!"
        init_data = json.loads(init_resp_line)
@@ -93,15 +130,7 @@ def test_mcp_stdio_handshake_immediate_input():
        p.stdin.write(json.dumps(tools_req).encode("utf-8") + b"\n")
        p.stdin.flush()

-        tools_resp_line = None
-        start = time.time()
-        while time.time() - start < 4.0:
-            import select
-
-            r, _, _ = select.select([p.stdout], [], [], 0.2)
-            if r:
-                tools_resp_line = p.stdout.readline().decode("utf-8").strip()
-                break
+        tools_resp_line = _readline_with_timeout(p.stdout, 4.0)

        assert tools_resp_line is not None, "MCP server failed to respond to tools/list request!"
        tools_data = json.loads(tools_resp_line)
@@ -0,0 +1,97 @@
+"""Tests for the spawn watchdog guarding mobile_run_task's standalone fallback."""
+
+import json
+import os
+from unittest.mock import patch
+
+from mcp_server.tools import task_runner as spawn_tool
+
+
+def _write_status(trace_dir: str, trace_id: str, status: str) -> None:
+    os.makedirs(trace_dir, exist_ok=True)
+    with open(os.path.join(trace_dir, "status.json"), "w", encoding="utf-8") as f:
+        json.dump({"trace_id": trace_id, "status": status}, f)
+
+
+def _patch_trace_dir(tmp_path):
+    return patch.object(spawn_tool.trace_store, "TRACES_DIR", str(tmp_path))
+
+
+def test_watchdog_passes_when_runner_creates_logs(tmp_path):
+    trace_id = "wd-boot-ok"
+    trace_dir = tmp_path / trace_id
+    _write_status(str(trace_dir), trace_id, "running")
+    (trace_dir / "stdout.log").write_text("booted", encoding="utf-8")
+
+    with _patch_trace_dir(tmp_path), patch.object(spawn_tool, "_kill_process_tree") as kill:
+        assert spawn_tool._watch_spawn(
+            trace_id, pid=999999, queue_ticket="t", conversation_id=None,
+            deadline_seconds=0.2, poll_interval=0.05,
+        )
+    kill.assert_not_called()
+
+
+def test_watchdog_skips_already_terminal_task(tmp_path):
+    trace_id = "wd-cancelled"
+    trace_dir = tmp_path / trace_id
+    _write_status(str(trace_dir), trace_id, "cancelled")
+
+    with _patch_trace_dir(tmp_path), patch.object(spawn_tool, "_kill_process_tree") as kill:
+        assert spawn_tool._watch_spawn(
+            trace_id, pid=999999, queue_ticket="t", conversation_id=None,
+            deadline_seconds=0.2, poll_interval=0.05,
+        )
+    kill.assert_not_called()
+
+
+def test_watchdog_kills_and_fails_hung_runner(tmp_path):
+    trace_id = "wd-hung"
+    trace_dir = tmp_path / trace_id
+    _write_status(str(trace_dir), trace_id, "running")
+
+    with (
+        _patch_trace_dir(tmp_path),
+        patch.object(spawn_tool, "_kill_process_tree") as kill,
+        patch.object(spawn_tool.DeviceExecutionLock, "cancel_reservation") as cancel,
+        patch.object(spawn_tool, "notify") as notifier,
+    ):
+        assert not spawn_tool._watch_spawn(
+            trace_id, pid=424242, queue_ticket="ticket-1", conversation_id="conv-1",
+            deadline_seconds=0.2, poll_interval=0.05,
+        )
+
+    kill.assert_called_once_with(424242)
+    cancel.assert_called_once_with("ticket-1")
+    notifier.assert_called_once()
+
+    with open(trace_dir / "status.json", encoding="utf-8") as f:
+        status = json.load(f)
+    assert status["status"] == "failed"
+    assert "no logs" in status.get("error", "")
+
+
+def test_watchdog_tolerates_logs_appearing_at_deadline(tmp_path):
+    """Logs that appear between the last poll and the deadline must not cause a kill."""
+    trace_id = "wd-late-boot"
+    trace_dir = tmp_path / trace_id
+    _write_status(str(trace_dir), trace_id, "running")
+
+    real_read_status = spawn_tool.trace_store.read_status
+    calls = {"n": 0}
+
+    def read_status_and_create_log(tid):
+        calls["n"] += 1
+        if calls["n"] == 1:
+            (trace_dir / "stderr.log").write_text("late boot", encoding="utf-8")
+        return real_read_status(tid)
+
+    with (
+        _patch_trace_dir(tmp_path),
+        patch.object(spawn_tool.trace_store, "read_status", side_effect=read_status_and_create_log),
+        patch.object(spawn_tool, "_kill_process_tree") as kill,
+    ):
+        assert spawn_tool._watch_spawn(
+            trace_id, pid=999999, queue_ticket="t", conversation_id=None,
+            deadline_seconds=0.15, poll_interval=0.2,
+        )
+    kill.assert_not_called()
@@ -0,0 +1,133 @@
+# Copyright 2026 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Regression tests for mobile_manage_task's state reconciliation.
+
+Guards the invariants restored after the reconcile rewrite:
+- a DB terminal verdict never overrides a status.json that already reached its
+  own terminal state (a completed task must keep its result),
+- except to correct a previously liveness-inferred failure,
+- and a task without a recorded pid (daemon dispatch) is never inferred dead.
+"""
+
+import sqlite3
+import time
+
+import pytest
+
+import mcp_server.tools.task_manager as task_manager
+from mcp_server.tools.task_manager import _LIVENESS_FAILURE_ERROR, _reconcile_task_state
+
+
+@pytest.fixture
+def no_persistence(monkeypatch):
+    """Keep reconciliation in-memory: no DB, no status writes, no lock queries."""
+    monkeypatch.setattr(task_manager, "_find_data_engine_db", lambda: None)
+    monkeypatch.setattr(task_manager, "_session_tracked_by_lock", lambda _tid: False)
+    monkeypatch.setattr(task_manager.trace_store, "write_status", lambda *_a, **_k: None)
+    monkeypatch.setattr(
+        task_manager.trace_store, "update_trace_status", lambda *_a, **_k: None
+    )
+    monkeypatch.setattr(task_manager, "notify", lambda *_a, **_k: None)
+
+
+def _db_with_session(tmp_path, trace_id: str, status: str, pid: int | None = 4242):
+    db_path = tmp_path / "artemis.db"
+    conn = sqlite3.connect(db_path)
+    conn.execute(
+        "CREATE TABLE sessions (session_id TEXT, status TEXT, pid INTEGER, start_time REAL)"
+    )
+    conn.execute(
+        "INSERT INTO sessions VALUES (?, ?, ?, ?)", (trace_id, status, pid, time.time())
+    )
+    conn.commit()
+    conn.close()
+    return str(db_path)
+
+
+@pytest.fixture
+def use_db(tmp_path, monkeypatch, no_persistence):
+    def _install(trace_id: str, status: str, pid: int | None = 4242):
+        db_path = _db_with_session(tmp_path, trace_id, status, pid)
+        monkeypatch.setattr(task_manager, "_find_data_engine_db", lambda: db_path)
+
+    return _install
+
+
+def test_db_failed_never_overrides_completed_status(use_db):
+    use_db("t1", "failed")
+    status_data = {"status": "completed", "result": "the answer", "pid": 111}
+
+    current_status, _pid, is_alive = _reconcile_task_state("t1", status_data)
+
+    assert current_status == "completed"
+    assert status_data["status"] == "completed"
+    assert status_data["result"] == "the answer"
+    assert is_alive is False
+
+
+def test_db_verdict_corrects_liveness_inferred_failure(use_db):
+    use_db("t2", "completed")
+    status_data = {"status": "failed", "error": _LIVENESS_FAILURE_ERROR, "pid": 111}
+
+    current_status, _pid, _is_alive = _reconcile_task_state("t2", status_data)
+
+    assert current_status == "completed"
+    assert status_data["error"] is None
+
+
+def test_db_never_overrides_user_cancellation(use_db):
+    use_db("t3", "completed")
+    status_data = {"status": "cancelled", "pid": 111}
+
+    current_status, _pid, _is_alive = _reconcile_task_state("t3", status_data)
+
+    assert current_status == "cancelled"
+
+
+def test_pidless_running_task_is_assumed_alive(no_persistence):
+    # Daemon dispatch writes no pid into status.json; liveness must not be
+    # inferred even long past the startup grace window.
+    status_data = {"status": "running", "start_time": time.time() - 3600}
+
+    current_status, _pid, is_alive = _reconcile_task_state("t4", status_data)
+
+    assert current_status == "running"
+    assert is_alive is True
+    assert status_data.get("error") is None
+
+
+def test_dead_pid_past_grace_is_marked_failed(no_persistence, monkeypatch):
+    monkeypatch.setattr(task_manager, "_pid_alive", lambda _pid: False)
+    status_data = {
+        "status": "running",
+        "pid": 99999,
+        "start_time": time.time() - 3600,
+    }
+
+    current_status, _pid, is_alive = _reconcile_task_state("t5", status_data)
+
+    assert current_status == "failed"
+    assert status_data["error"] == _LIVENESS_FAILURE_ERROR
+    assert is_alive is False
+
+
+def test_dead_pid_within_grace_is_assumed_alive(no_persistence, monkeypatch):
+    monkeypatch.setattr(task_manager, "_pid_alive", lambda _pid: False)
+    status_data = {"status": "running", "pid": 99999, "start_time": time.time()}
+
+    current_status, _pid, is_alive = _reconcile_task_state("t6", status_data)
+
+    assert current_status == "running"
+    assert is_alive is True
@@ -14,6 +14,7 @@

 """Unit tests for MCP trace store."""

+import os
 import shutil
 import tempfile
 import uuid
@@ -80,6 +81,6 @@ def test_read_nonexistent_status(temp_trace_env):
 def test_trace_paths(temp_trace_env):
    trace_id = "test-paths-id"
    trace_dir = trace_store.get_trace_dir(trace_id)
-    assert trace_store.get_trace_notes_dir(trace_id) == f"{trace_dir}/notes"
-    assert trace_store.get_trace_stdout_log_path(trace_id) == f"{trace_dir}/stdout.log"
-    assert trace_store.get_trace_stderr_log_path(trace_id) == f"{trace_dir}/stderr.log"
+    assert trace_store.get_trace_notes_dir(trace_id) == os.path.join(trace_dir, "notes")
+    assert trace_store.get_trace_stdout_log_path(trace_id) == os.path.join(trace_dir, "stdout.log")
+    assert trace_store.get_trace_stderr_log_path(trace_id) == os.path.join(trace_dir, "stderr.log")