diff --git a/mcp-server/src/legal_mcp/services/claude_session.py b/mcp-server/src/legal_mcp/services/claude_session.py index c44916d..d49435a 100644 --- a/mcp-server/src/legal_mcp/services/claude_session.py +++ b/mcp-server/src/legal_mcp/services/claude_session.py @@ -29,6 +29,7 @@ from __future__ import annotations import asyncio import json import logging +import os from legal_mcp.config import parse_llm_json @@ -40,6 +41,32 @@ logger = logging.getLogger(__name__) DEFAULT_TIMEOUT = 1800 LONG_TIMEOUT = 3600 # opus block writing on full case context +# Environment markers a running Claude Code session exports into its child +# processes. When an agent itself runs as ``claude -p`` (e.g. the CEO MCP +# instance launched by Paperclip's claude_local adapter), its MCP server +# inherits these, and a *nested* ``claude -p`` we spawn here inherits them +# too. Stripping them makes every nested invocation launch as a clean +# top-level session, decoupled from the parent's session/project state — +# defensive hardening against the #85 nested-``exit 1`` failure (which could +# not be reproduced in a plain interactive session, so the markers are a +# suspect, not a proven cause). Auth/config vars (CLAUDE_CONFIG_DIR, +# ANTHROPIC_*, PATH, HOME) are kept. See TaskMaster legal-ai #85. +_SESSION_MARKER_PREFIXES = ("CLAUDECODE", "CLAUDE_CODE_", "CLAUDE_AGENT_") +_SESSION_MARKER_EXACT = frozenset({"AI_AGENT", "CLAUDE_EFFORT"}) + + +def _clean_subprocess_env() -> dict[str, str]: + """Copy the current env minus Claude Code session markers. + + Lets a nested ``claude -p`` start fresh instead of detecting it is + already inside a Claude Code session (the #85 nested-exit-1 bug). + """ + env = dict(os.environ) + for key in list(env): + if key in _SESSION_MARKER_EXACT or key.startswith(_SESSION_MARKER_PREFIXES): + del env[key] + return env + async def query( prompt: str, @@ -100,6 +127,8 @@ async def query( stdin=asyncio.subprocess.PIPE, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, + env=_clean_subprocess_env(), + cwd=os.path.expanduser("~"), ) except FileNotFoundError: raise RuntimeError( @@ -125,9 +154,20 @@ async def query( raise RuntimeError(f"Claude CLI timed out after {timeout}s") if proc.returncode != 0: - stderr = stderr_b.decode("utf-8", errors="replace").strip()[:500] or "unknown error" + # Surface the real cause: the CLI sometimes writes its diagnostic to + # stdout (or nowhere) rather than stderr, so a stderr-only message + # collapsed to "unknown error" and hid the actual failure (#85). + stderr = stderr_b.decode("utf-8", errors="replace").strip() + stdout = stdout_b.decode("utf-8", errors="replace").strip() + diagnostic = stderr or stdout or "no output on stderr/stdout" size_info = f"; prompt_len={len(full_prompt):,} chars" if len(full_prompt) > 100_000 else "" - raise RuntimeError(f"Claude CLI failed (exit {proc.returncode}): {stderr}{size_info}") + logger.error( + "claude -p failed (exit %s): stderr=%r stdout=%r", + proc.returncode, stderr[:500], stdout[:500], + ) + raise RuntimeError( + f"Claude CLI failed (exit {proc.returncode}): {diagnostic[:500]}{size_info}" + ) stdout = stdout_b.decode("utf-8", errors="replace").strip() if not stdout: @@ -232,6 +272,7 @@ async def query_streaming( stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, cwd=cwd, + env=_clean_subprocess_env(), ) except FileNotFoundError: yield { diff --git a/mcp-server/tests/test_claude_session.py b/mcp-server/tests/test_claude_session.py new file mode 100644 index 0000000..510d385 --- /dev/null +++ b/mcp-server/tests/test_claude_session.py @@ -0,0 +1,44 @@ +from __future__ import annotations + +import os + +from legal_mcp.services import claude_session as cs + + +def test_clean_env_strips_session_markers(monkeypatch): + """Nested claude -p must not inherit the parent session markers (#85).""" + for k in ( + "CLAUDECODE", + "CLAUDE_CODE_ENTRYPOINT", + "CLAUDE_CODE_SESSION_ID", + "CLAUDE_CODE_EXECPATH", + "CLAUDE_CODE_SSE_PORT", + "CLAUDE_AGENT_SDK_VERSION", + "AI_AGENT", + "CLAUDE_EFFORT", + ): + monkeypatch.setenv(k, "x") + + env = cs._clean_subprocess_env() + + assert "CLAUDECODE" not in env + assert "AI_AGENT" not in env + assert "CLAUDE_EFFORT" not in env + assert not any(k.startswith("CLAUDE_CODE_") for k in env) + assert not any(k.startswith("CLAUDE_AGENT_") for k in env) + + +def test_clean_env_keeps_auth_and_path(monkeypatch): + """Auth/config + PATH/HOME must survive — they are needed by the CLI.""" + monkeypatch.setenv("CLAUDECODE", "1") + monkeypatch.setenv("CLAUDE_CONFIG_DIR", "/home/chaim/.claude") + monkeypatch.setenv("ANTHROPIC_BASE_URL", "https://example") + monkeypatch.setenv("PATH", os.environ.get("PATH", "/usr/bin")) + + env = cs._clean_subprocess_env() + + # CLAUDE_CONFIG_DIR carries credentials — must NOT be stripped. + assert env.get("CLAUDE_CONFIG_DIR") == "/home/chaim/.claude" + assert env.get("ANTHROPIC_BASE_URL") == "https://example" + assert "PATH" in env + assert "CLAUDECODE" not in env