feat(carve S3): agent runtime — deterministic issue feedback, build/lint gate, restack workflow

agent/pyproject.toml

@@ -131,6 +131,16 @@ ignore = [
 [tool.pytest.ini_options]
 testpaths = ["tests"]
 pythonpath = ["src"]
+# Non-TTY output ergonomics. In a captured/piped context (the ECS build gate, CI,
+# an agent's Bash tool) pytest's default is a wall of featureless progress dots
+# with the "N passed" summary easy to lose in a tail — live-caught on ABCA-691,
+# where an agent burned ~6 turns re-running the full suite with different flags
+# because it couldn't confirm the result from the dots. ``count`` renders progress
+# as an explicit "N/M passed" so a captured log always shows how far it got and how
+# many passed; ``-ra`` appends a short summary of every non-passing outcome (with
+# reasons) at the end. Both are display-only — they never change test results.
+addopts = "-ra"
+console_output_style = "count"
 # pytest-timeout: hard wall-clock cap PER TEST. A unit test that blocks (an
 # unmocked network/subprocess/Bedrock call with no timeout of its own) otherwise
 # hangs the whole `mise run build` until the 3600s build-verify ceiling kills it,
@@ -143,6 +153,17 @@ pythonpath = ["src"]
 # but the build kept hanging ~55 min until the 3600s ceiling). signal is the real fix.
 timeout = 120
 timeout_method = "signal"
+# faulthandler backstop for hangs signal-timeout CANNOT interrupt. SIGALRM only
+# fires in the MAIN thread during a test's call phase, so a hang in a WORKER
+# thread (a deadlocked Barrier/join), a fixture, or a C-level socket read the
+# main thread never returns from is invisible to it — exactly the ECS-only stall
+# chased across ABCA-684/686/688 and again on the warm-cache run (53 min of
+# silence, signal never fired). This arms faulthandler's dedicated C watchdog
+# thread (immune to the GIL and blocked syscalls) to dump EVERY thread's Python
+# stack after 300s in a test, so the next hang self-reports the exact file:line
+# instead of stalling blind to the 3600s build ceiling. A session-level
+# hard-exit watchdog for hangs OUTSIDE any test lives in tests/conftest.py.
+faulthandler_timeout = 300
 
 [tool.coverage.run]
 branch = true

agent/src/channel_mcp.py

@@ -6,17 +6,33 @@
 start and exposes the server's tools.
 
 Currently wired channels:
-- ``linear``  → Linear hosted MCP (``mcp__linear-server__*`` tools) — functional.
 - ``jira``    → Atlassian Remote MCP entry — a NON-FUNCTIONAL placeholder. It
   is written for forward-compatibility but cannot connect from a headless
   agent (interactive OAuth 2.1 only); live outbound Jira comments go through
   the REST shim in ``jira_reactions.py``. See ``JIRA_MCP_URL`` below + ADR-015.
 
-For all other channel sources this is a no-op: no MCP is written, and the
-SDK sees no channel-specific tools.
+Linear is NOT written here: ABCA runs Linear 100% deterministically (ADR-016
+"Linear is fully deterministic"). There is no Linear MCP — issue text, recent
+comments, and attachments are pre-hydrated at the Lambda tier (the webhook
+processor + ``linear-attachments.ts`` / ``linear-feedback.fetchRecentComments``),
+and outbound reactions / state transitions go through direct GraphQL in
+``linear_reactions.py`` (which reads ``LINEAR_API_TOKEN`` set by config.py —
+independent of this module). The Linear MCP was removed after it proved
+non-functional against a single OAuth app (actor=user data reads error;
+actor=app can't re-consent an installed app).
+
+Beyond WRITING channel entries, this module also ENFORCES the Linear-no-MCP
+guarantee: {@link strip_linear_mcp_servers} removes any Linear MCP server a repo
+may have COMMITTED to its own ``.mcp.json`` before the SDK loads it (the prompt
+prohibition is not a security boundary — the SDK loads ``project`` settings and
+we export ``LINEAR_API_TOKEN``). The pipeline calls it after
+``configure_channel_mcp`` on every task with a repo.
+
+For all other channel sources ``configure_channel_mcp`` is a no-op: no MCP is
+written (though the scrub above still runs).
 
 See: cdk/src/handlers/{linear,jira}-webhook-processor.ts (inbound),
-runner.py (SDK invocation).
+runner.py (SDK invocation), ADR-016 (Linear determinism).
 """
 
 from __future__ import annotations
@@ -30,31 +46,6 @@
 if TYPE_CHECKING:
     from collections.abc import Callable
 
-# ─── Linear ──────────────────────────────────────────────────────────────────
-
-#: Linear MCP endpoint — hosted by Linear, Streamable HTTP transport.
-LINEAR_MCP_URL = "https://mcp.linear.app/mcp"
-
-#: Key name inside ``mcpServers``. Tools surface as
-#: ``mcp__linear-server__*`` in the Agent SDK.
-LINEAR_MCP_SERVER_KEY = "linear-server"
-
-#: Env var name the MCP server entry reads via ``${LINEAR_API_TOKEN}``
-#: placeholder expansion. Populated from the OAuth secret by config.py.
-LINEAR_API_TOKEN_ENV = "LINEAR_API_TOKEN"  # noqa: S105 — env var *name*, not a secret value
-
-
-def _linear_server_entry() -> dict[str, Any]:
-    """Build the `mcpServers` entry for Linear's hosted MCP."""
-    return {
-        "type": "http",
-        "url": LINEAR_MCP_URL,
-        "headers": {
-            "Authorization": f"Bearer ${{{LINEAR_API_TOKEN_ENV}}}",
-        },
-    }
-
-
 # ─── Jira (Atlassian Remote MCP — NON-FUNCTIONAL PLACEHOLDER) ────────────────
 
 #: Atlassian Remote MCP endpoint — Streamable HTTP transport.
@@ -99,7 +90,6 @@ def _jira_server_entry() -> dict[str, Any]:
 #: have a hosted MCP (api, webhook, slack) intentionally have no entry here —
 #: the gate in ``configure_channel_mcp`` short-circuits on missing keys.
 CHANNEL_MCP_BUILDERS: dict[str, tuple[str, Callable[[], dict[str, Any]]]] = {
-    "linear": (LINEAR_MCP_SERVER_KEY, _linear_server_entry),
     "jira": (JIRA_MCP_SERVER_KEY, _jira_server_entry),
 }
 
@@ -124,7 +114,11 @@ def _read_existing_mcp_config(path: str) -> dict[str, Any]:
     return {}
 
 
-def configure_channel_mcp(repo_dir: str, channel_source: str) -> bool:
+def configure_channel_mcp(
+    repo_dir: str,
+    channel_source: str,
+    channel_metadata: dict[str, str] | None = None,
+) -> bool:
     """Write or merge a channel-specific ``.mcp.json`` into ``repo_dir``.
 
     Looks up ``channel_source`` in :data:`CHANNEL_MCP_BUILDERS`:
@@ -136,11 +130,15 @@ def configure_channel_mcp(repo_dir: str, channel_source: str) -> bool:
     Args:
       repo_dir: the cloned-repo working directory the SDK will use as ``cwd``.
       channel_source: inbound channel (``TaskConfig.channel_source``).
+      channel_metadata: per-task channel metadata (``TaskConfig.channel_metadata``).
+        Currently unused by the wired channels (Jira's entry is static); retained
+        for call-site compatibility and future channel builders that need it.
 
     Returns:
       True if a channel MCP entry was (re)written, False otherwise (channel
       unmapped, missing repo_dir, or write failure).
     """
+    _ = channel_metadata  # reserved for future channel builders (see docstring)
     builder_entry = CHANNEL_MCP_BUILDERS.get(channel_source)
     if builder_entry is None:
         return False
@@ -184,3 +182,77 @@ def configure_channel_mcp(repo_dir: str, channel_source: str) -> bool:
             "not MCP",
         )
     return True
+
+
+#: Substrings that mark an ``mcpServers`` entry as a Linear MCP server. Matched
+#: (case-insensitively) against the server KEY and, defensively, the JSON of its
+#: value (url / command / args / headers) so an entry named innocuously
+#: (``"specs": {url:"https://mcp.linear.app/sse"}``) or reading the token
+#: (``${LINEAR_API_TOKEN}``) is caught regardless of its key.
+_LINEAR_MCP_MARKERS = ("linear", "mcp.linear.app", "linear_api_token")
+
+
+def strip_linear_mcp_servers(repo_dir: str) -> int:
+    """Remove any Linear MCP server entry from the repo's ``.mcp.json``.
+
+    ADR-016 ENFORCEMENT (not just convention): Linear is 100% deterministic and
+    the agent must have NO Linear MCP tools. The prompt says so, but a prompt is
+    not a security boundary — a repo could COMMIT a ``.mcp.json`` with a
+    ``linear-server`` entry using ``${LINEAR_API_TOKEN}``; the SDK loads
+    ``project`` settings + we export ``LINEAR_API_TOKEN``, so under
+    ``bypassPermissions`` those tools would authenticate and run. This scrubs any
+    such entry from the on-disk config BEFORE the SDK reads it, on every task
+    with a repo, regardless of channel_source. Jira's own entry (written by
+    ``configure_channel_mcp`` for jira tasks) never matches these markers.
+
+    Returns the number of server entries removed (0 when none / no file).
+    Best-effort: a read/write failure logs and returns 0 (the prompt prohibition
+    + the absence of any platform-written Linear entry still hold).
+    """
+    if not repo_dir or not os.path.isdir(repo_dir):
+        return 0
+    mcp_path = os.path.join(repo_dir, ".mcp.json")
+    config = _read_existing_mcp_config(mcp_path)
+    servers = config.get("mcpServers")
+    if not isinstance(servers, dict) or not servers:
+        return 0
+
+    def _is_linear(key: str, value: object) -> bool:
+        hay = (key + " " + json.dumps(value, default=str)).lower()
+        return any(m in hay for m in _LINEAR_MCP_MARKERS)
+
+    offending = [k for k, v in servers.items() if _is_linear(k, v)]
+    if not offending:
+        return 0
+
+    for k in offending:
+        del servers[k]
+    config["mcpServers"] = servers
+
+    # If the Linear server(s) were the ONLY content, drop the whole file rather
+    # than leave an inert ``{"mcpServers": {}}`` behind — a repo that shipped a
+    # Linear-only .mcp.json ends up with no .mcp.json at all, which is the correct
+    # end state (the SDK then has nothing to load). Keep the file only when other
+    # MCP servers OR other top-level keys survive (a legit non-Linear server, or a
+    # Jira entry the platform just wrote for a jira task).
+    other_top_level_keys = [k for k in config if k != "mcpServers"]
+    try:
+        if not servers and not other_top_level_keys:
+            os.remove(mcp_path)
+            removed_file = True
+        else:
+            with open(mcp_path, "w", encoding="utf-8") as f:
+                json.dump(config, f, indent=2)
+                f.write("\n")
+            removed_file = False
+    except OSError as e:
+        log("ERROR", f"strip_linear_mcp_servers: failed to rewrite/remove {mcp_path}: {e}")
+        return 0
+    log(
+        "WARN",
+        f"Removed {len(offending)} Linear MCP server entr(ies) from {mcp_path} "
+        f"(ADR-016: Linear is deterministic; the agent has no Linear MCP). "
+        f"Keys: {', '.join(offending)}"
+        + ("; deleted the now-empty .mcp.json" if removed_file else ""),
+    )
+    return len(offending)

agent/src/clarification_tool.py

@@ -0,0 +1,74 @@
+"""In-process ``request_clarification`` SDK tool (clarify-before-spend, UX #4).
+
+The customer-caught problem: a vague request like "make it faster" was answered
+by GUESSING (shipping a plausible PR and charging for it) instead of asking what
+was meant. The fix asks the agent to STOP and pose a question when a request is
+too underspecified to implement without guessing.
+
+An earlier cut used a text sentinel the agent had to reproduce verbatim on its
+final line. That proved unreliable live — the model either shipped a guess or
+finished silently without emitting the exact string. A **tool call** is a
+discrete, deterministic event: the model either invokes ``request_clarification``
+or it doesn't, and the runner sees the ``ToolUseBlock`` in the message stream (no
+string-matching, no reproduction). This module defines that tool as an in-process
+SDK MCP server; the runner registers it and captures the question, and the
+pipeline treats a captured question as a hold-and-ask (no build, no PR).
+
+Kept tiny and side-effect-free: the tool just acknowledges the call. The
+authoritative signal is the runner observing the call + its ``question`` arg.
+"""
+
+from __future__ import annotations
+
+from typing import Any
+
+#: The in-process MCP server name. The SDK exposes each tool as
+#: ``mcp__<server>__<tool>``, so the fully-qualified tool name the runner matches
+#: on is :data:`CLARIFICATION_TOOL_NAME`.
+CLARIFICATION_SERVER_NAME = "abca"
+CLARIFICATION_TOOL_NAME = f"mcp__{CLARIFICATION_SERVER_NAME}__request_clarification"
+
+
+def build_clarification_server() -> Any:
+    """Build the in-process SDK MCP server exposing ``request_clarification``.
+
+    Returns the SDK server config dict for ``ClaudeAgentOptions(mcp_servers=...)``,
+    or ``None`` if the SDK is unavailable (defensive — the runner then simply
+    doesn't register it and the marker-based fallback still works).
+    """
+    try:
+        from claude_agent_sdk import create_sdk_mcp_server, tool
+    except ImportError:  # pragma: no cover - SDK always present in the container
+        return None
+
+    @tool(
+        "request_clarification",
+        (
+            "Ask the requester ONE clarifying question and STOP, instead of guessing, "
+            "when the task is too underspecified to implement without picking among "
+            "materially different interpretations (e.g. 'make it faster' with no target "
+            "or slow path named). Calling this opens NO pull request and charges nothing "
+            "for a guess — the platform surfaces your question to the requester. Only "
+            "call it for genuinely ambiguous goal-without-substance requests; a task that "
+            "names what to change is actionable, so just do it."
+        ),
+        {"question": str},
+    )
+    async def request_clarification(args: dict[str, Any]) -> dict[str, Any]:
+        question = str(args.get("question", "")).strip()
+        # The tool's own return is only feedback to the agent; the runner captures
+        # the question from the ToolUseBlock. Tell the agent to stop here.
+        return {
+            "content": [
+                {
+                    "type": "text",
+                    "text": (
+                        "Clarifying question recorded and will be posted to the requester. "
+                        "Do not make code changes or open a PR — end your turn now."
+                        + (f" (question: {question})" if question else "")
+                    ),
+                }
+            ]
+        }
+
+    return create_sdk_mcp_server(name=CLARIFICATION_SERVER_NAME, tools=[request_clarification])