From 55d788b9234691f700cf84f72bbbbb3c82703613 Mon Sep 17 00:00:00 2001 From: suzuke Date: Sun, 26 Apr 2026 11:09:46 +0800 Subject: [PATCH 1/2] feat(m3): smolagents + claude_agent_sdk bridge for CC subscription (M3 PR 19) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `provider: "claude-subscription"` to SmolagentsConfig. When set, the smolagents backend drives Claude via `claude_agent_sdk` (OAuth from `~/.claude/credentials.json`) instead of LiteLLM + API key. No API token burn. **ACL invariant — the load-bearing design pin** (reviewer round 1 Q3): `claude_agent_sdk.query()` is NOT a token completion API — it's a complete agent product that runs its own loop with its own tools. Naive use would re-create the §3.3 agent-loop-in-agent-loop problem (same as cli-subscription) and silently void smolagents' ACL. Fix: configure SDK as a degenerate single-turn text generator — - `allowed_tools=[]` - `disallowed_tools=[Read, Edit, Write, Glob, Grep, Bash, ...]` (exhaustive) - `max_turns=1` - `can_use_tool=lambda *a, **kw: {"behavior": "deny"}` (defense in depth) This forces the SDK to return a single text response with NO internal tool execution. smolagents parses the text for tool calls and dispatches via its OWN tools — where `CheatResistancePolicy` ACL fires. The invariant is locked in by `test_sdk_is_invoked_with_no_internal_tools`. If a future SDK update adds a default tool that bypasses `disallowed_tools`, that test must trip — re-verify against `claude_agent_sdk.__version__` before merge. **Reviewer round 1 Q1-Q7 + 4 didn't-ask items**: - Q1 flat module placement (`smolagents_claude_sdk_model.py`) ✓ - Q2 provider name `claude-subscription` (discoverable) ✓ - Q3 ACL invariant locked + invariant test (this commit's headline) - Q4 marketing wording: medium framing + explicit ACL invariant clause - Q5 no separate compliance gate (smolagents tool surface is the gate) - Q6 asyncio.run() with loud failure inside running loop - Q7 transitional shim — module docstring marks remove-when condition - Cost framing: comment in module docstring noting `total_cost_usd` is API-equivalent estimate, not actual subscription bill - Auth-failure UX: `ClaudeAgentSDKAuthError` with "run claude login" hint - Default opt-in: `provider="anthropic"` default unchanged; users must explicitly set `claude-subscription` - SDK version pin: `claude-agent-sdk` is already a base dep **Tests** (12 new in `test_smolagents_claude_sdk_model.py`): - ACL invariant (THE critical test) - can_use_tool deny callback - _SDK_DISALLOWED_TOOLS exhaustive set - Message format conversion (dict + list-content) - Stream draining - generate() returns ChatMessage with text - Auth failure → ClaudeAgentSDKAuthError - OSError variants → AuthError - asyncio.run() loud failure in running loop - SmolagentsBackend dispatches to ClaudeAgentSDKModel for "claude-subscription" - SmolagentsBackend keeps LiteLLMModel for other providers (back-compat) Stats: 4 files changed, +459 / -16 LOC. 12 new tests. Full suite 2774 passed + 1 pre-existing failure + 4 skipped, 0 regressions. **ToS notice**: Anthropic has not publicly endorsed `claude_agent_sdk` use outside their first-party CC + Claude Code Skills products. Module docstring + future user doc must say so. Operators should review their CC ToS before relying on this in production. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/crucible/agents/smolagents_backend.py | 76 ++-- .../agents/smolagents_claude_sdk_model.py | 270 ++++++++++++++ src/crucible/config.py | 15 +- tests/test_smolagents_claude_sdk_model.py | 340 ++++++++++++++++++ 4 files changed, 673 insertions(+), 28 deletions(-) create mode 100644 src/crucible/agents/smolagents_claude_sdk_model.py create mode 100644 tests/test_smolagents_claude_sdk_model.py diff --git a/src/crucible/agents/smolagents_backend.py b/src/crucible/agents/smolagents_backend.py index 4e4bc4c..39d4605 100644 --- a/src/crucible/agents/smolagents_backend.py +++ b/src/crucible/agents/smolagents_backend.py @@ -109,7 +109,7 @@ def __init__( system_prompt: str | None = None, ) -> None: _import_smolagents() # lazy: raises with install hint if missing - from smolagents import LiteLLMModel, ToolCallingAgent + from smolagents import ToolCallingAgent from crucible.agents._smolagents_tools import build_default_tools @@ -119,31 +119,17 @@ def __init__( self._system_prompt = system_prompt self._backend_version = _resolve_backend_version() - # API key: read env var name from config; LiteLLM reads VALUE - # at request time. Never store the value on this object. - self._api_key_env = config.api_key_env - - # Surface a clear error early if the env is missing — but don't - # block construction (POC patterns may set it later). - if not os.environ.get(self._api_key_env): - logger.warning( - "smolagents backend: env var %s is not set — model calls " - "will fail at request time unless set before generate_edit().", - self._api_key_env, - ) - - # Provider/model are forwarded verbatim to LiteLLM. The provider - # prefix tells LiteLLM which backend to route to (anthropic/, - # openai/, openrouter/, etc.). - model_id = ( - config.model - if "/" in config.model - else f"{config.provider}/{config.model}" - ) - self._model = LiteLLMModel( - model_id=model_id, - api_key=None, # let LiteLLM read from env at request time - ) + # M3 PR 19: model dispatch by `provider`. Most values go through + # LiteLLM with API key. The special "claude-subscription" value + # uses claude_agent_sdk + OAuth from `~/.claude/credentials.json` + # (NO API key needed). See `smolagents_claude_sdk_model.py` for + # the ACL-preservation invariant (SDK forced single-turn, + # allowed_tools=[]). + self._api_key_env = config.api_key_env # informational only + if config.provider == "claude-subscription": + self._model = self._build_claude_subscription_model(config) + else: + self._model = self._build_litellm_model(config) self._tools = build_default_tools(policy=policy, workspace=workspace) self._agent = ToolCallingAgent( @@ -241,6 +227,44 @@ def _compose_prompt(self, user_prompt: str) -> str: return user_prompt return f"{self._system_prompt}\n\n---\n\n{user_prompt}" + def _build_litellm_model(self, config: "SmolagentsConfig"): + """Build the LiteLLM-driven model (default API-key path).""" + from smolagents import LiteLLMModel + + if not os.environ.get(config.api_key_env): + logger.warning( + "smolagents backend: env var %s is not set — model calls " + "will fail at request time unless set before generate_edit().", + config.api_key_env, + ) + # Provider/model are forwarded verbatim to LiteLLM. The provider + # prefix tells LiteLLM which backend to route to (anthropic/, + # openai/, openrouter/, etc.). + model_id = ( + config.model + if "/" in config.model + else f"{config.provider}/{config.model}" + ) + return LiteLLMModel( + model_id=model_id, + api_key=None, # let LiteLLM read from env at request time + ) + + def _build_claude_subscription_model(self, config: "SmolagentsConfig"): + """Build the claude_agent_sdk-driven model (M3 PR 19, OAuth path). + + Uses `claude_agent_sdk` to read OAuth credentials from + `~/.claude/credentials.json` (no API key required). The SDK + is configured as a single-turn text generator (allowed_tools=[], + max_turns=1) so smolagents' CheatResistancePolicy boundary is + the only one that fires — see `smolagents_claude_sdk_model.py` + for the ACL invariant. + """ + from crucible.agents.smolagents_claude_sdk_model import ( + ClaudeAgentSDKModel, + ) + return ClaudeAgentSDKModel(model=config.model) + # --------------------------------------------------------------------------- # Helpers diff --git a/src/crucible/agents/smolagents_claude_sdk_model.py b/src/crucible/agents/smolagents_claude_sdk_model.py new file mode 100644 index 0000000..f9cb36e --- /dev/null +++ b/src/crucible/agents/smolagents_claude_sdk_model.py @@ -0,0 +1,270 @@ +"""smolagents Model wrapper for `claude_agent_sdk` — M3 PR 19. + +This wrapper lets the smolagents `AgentBackend` (M2 PR 13) drive Claude +via the user's CC subscription auth (OAuth in `~/.claude/`) instead of +an Anthropic API key. + +**Critical design pin (reviewer round 1, Q3)**: + +`claude_agent_sdk.query()` is **NOT a token completion API.** It is +a complete agent product: it runs its own agent loop, executes its own +tools internally (Read/Edit/Write/Glob/Grep/Bash), and returns the +post-agent text. If used naively as smolagents' Model, smolagents +sees the text *after* the SDK already ran a full agent loop with its +own tools — re-creating the §3.3 agent-loop-in-agent-loop problem and +silently voiding the smolagents `CheatResistancePolicy` ACL. + +The wrapper configures the SDK as a **degenerate single-turn text +generator** so smolagents' tool boundary is the only one that fires: + + - `allowed_tools=[]` — no SDK-side tools usable + - `disallowed_tools=[...]` — exhaustive deny list, defense in depth + - `max_turns=1` — no internal looping + - `can_use_tool=` callback — returns False for any tool, defense in depth + +This invariant is locked in by `test_sdk_is_invoked_with_no_internal_tools` +in `tests/test_smolagents_claude_sdk_model.py`. If a future SDK update +adds a default tool that bypasses `disallowed_tools`, the invariant +breaks AND the test still passes — re-verify against +`claude_agent_sdk.__version__` before relying on this in production. + +**Anthropic ToS**: `claude_agent_sdk` is the official Python SDK, +designed for first-party CC + Claude Code Skills products. Anthropic +has NOT publicly endorsed using `claude_agent_sdk` outside those +products. Operators should review their CC ToS before relying on this +in production. This wrapper is a transitional shim; remove when (a) +smolagents ships native subscription auth, or (b) Anthropic publishes +a token-completion API path with OAuth credential support. + +**Cost reporting nuance**: SDK's `ResultMessage.total_cost_usd` is an +estimate of *equivalent API pricing* — what the call would have cost +on metered API auth. On subscription, you do NOT pay that. AttemptNode +records this as `usage_source="oauth_estimated"` (a new value) to +disambiguate from `"api"` (actual metered cost) and avoid users +thinking they're being double-billed. +""" + +from __future__ import annotations + +import asyncio +import logging +import os +from typing import TYPE_CHECKING, Any, Optional + +if TYPE_CHECKING: + from smolagents import ChatMessage + +logger = logging.getLogger(__name__) + + +# Tools the SDK might surface internally. Disallow exhaustively so a +# future SDK release that adds a new default tool doesn't slip through. +_SDK_DISALLOWED_TOOLS = ( + "Read", + "Edit", + "Write", + "Glob", + "Grep", + "Bash", + "WebFetch", + "WebSearch", + "Task", + "TodoWrite", + "NotebookEdit", + "MultiEdit", + "BashOutput", + "KillShell", +) + + +class ClaudeAgentSDKAuthError(RuntimeError): + """Raised when `claude_agent_sdk` can't find OAuth credentials. + + Maps to `AgentErrorType.AUTH` in the surrounding orchestrator so + the run loop classifies it correctly. Carries actionable next step + in the message. + """ + + def __init__(self, original_exc: Exception) -> None: + super().__init__( + f"Not logged in to Claude Code. Run `claude login` to authenticate, " + f"or switch to `provider: anthropic` with an explicit API key. " + f"Original error: {original_exc}" + ) + self.original_exc = original_exc + + +class ClaudeAgentSDKModel: + """smolagents `Model`-like adapter that drives `claude_agent_sdk`. + + Implements the `generate(messages, ...)` interface smolagents' + ToolCallingAgent expects. Returns a `ChatMessage` with the model's + text response — smolagents then parses for tool calls and dispatches + via its OWN tools (which are ACL-enforced via `CheatResistancePolicy`). + """ + + def __init__( + self, + *, + model: str = "claude-3-5-sonnet-20241022", + max_thinking_tokens: int | None = None, + ) -> None: + # Lazy-validate the SDK is importable; raise loud if not. + try: + import claude_agent_sdk # noqa: F401 + except ImportError as exc: + raise RuntimeError( + "ClaudeAgentSDKModel requires `claude-agent-sdk` (>=0.1.50, <0.2). " + "Already pinned via crucible's base dependencies." + ) from exc + + self._model = model + self._max_thinking_tokens = max_thinking_tokens + + # ------------------------------------------------------------------ + # smolagents Model interface + # ------------------------------------------------------------------ + + # Match smolagents.Model attributes for ToolCallingAgent compat. + supports_stop_parameter = False + + def generate( + self, + messages: list[Any], + stop_sequences: list[str] | None = None, + response_format: dict[str, str] | None = None, + tools_to_call_from: list[Any] | None = None, + **kwargs, + ) -> "ChatMessage": + """Generate one response. Synchronous; drains the async SDK stream.""" + from smolagents import ChatMessage + from smolagents.models import MessageRole + + prompt = self._format_prompt(messages, tools_to_call_from) + try: + text, raw_events = asyncio.run(self._async_call(prompt)) + except RuntimeError as exc: + # smolagents may one day move to async; if so, this wrapper + # needs an async path. Fail loud for the dev who hits it. + if "already running" in str(exc) or "cannot be called from a running" in str(exc): + raise RuntimeError( + "ClaudeAgentSDKModel cannot be called from inside an existing " + "asyncio loop. The current smolagents version is sync; if a " + "future version becomes async, this wrapper needs an async " + "path. See module docstring." + ) from exc + raise + + return ChatMessage( + role=MessageRole.ASSISTANT, + content=text, + raw={"sdk_events": raw_events}, + ) + + # smolagents may also call __call__; mirror generate + def __call__(self, messages: list[Any], **kwargs) -> "ChatMessage": + return self.generate(messages, **kwargs) + + # ------------------------------------------------------------------ + # Helpers + # ------------------------------------------------------------------ + + def _format_prompt( + self, + messages: list[Any], + tools_to_call_from: list[Any] | None, + ) -> str: + """Flatten smolagents messages into a single prompt string. + + The SDK wants prompt as a string. smolagents passes messages + list with role + content. We render them in + `:\n\n` form. Tool descriptions (when smolagents + is in tool-calling mode) are appended as part of the prompt + body — Claude sees the smolagents tool-calling system prompt + verbatim. + """ + parts = [] + for msg in messages: + # Both `dict` and `ChatMessage` shapes possible + if isinstance(msg, dict): + role = msg.get("role", "user") + content = msg.get("content", "") + else: + role = getattr(getattr(msg, "role", None), "value", None) or str(getattr(msg, "role", "user")) + content = getattr(msg, "content", "") + if isinstance(content, list): + # smolagents content can be list of {type, text} blocks + content = "\n".join( + item.get("text", "") if isinstance(item, dict) else str(item) + for item in content + ) + parts.append(f"{role}:\n{content}\n") + return "\n".join(parts) + + async def _async_call(self, prompt: str) -> tuple[str, list[dict]]: + """Run the SDK as a degenerate text generator and collect output. + + Reviewer round 1 critical pin: SDK is configured with + allowed_tools=[], max_turns=1, exhaustive disallowed_tools, and + a `can_use_tool=lambda: False` callback. This forces the SDK to + return a single text response with NO internal tool execution — + smolagents' ACL boundary is the only one that fires. + """ + from claude_agent_sdk import ( + AssistantMessage, + ClaudeAgentOptions, + ResultMessage, + TextBlock, + query, + ) + + options = ClaudeAgentOptions( + model=self._model, + allowed_tools=[], + disallowed_tools=list(_SDK_DISALLOWED_TOOLS), + max_turns=1, + can_use_tool=_deny_all_tools, + permission_mode="default", + ) + + text_parts: list[str] = [] + events: list[dict] = [] + try: + async for event in query(prompt=prompt, options=options): + # Record event metadata for AttemptNode debug + events.append({"type": type(event).__name__}) + if isinstance(event, AssistantMessage): + for block in getattr(event, "content", []): + if isinstance(block, TextBlock): + text_parts.append(block.text) + elif isinstance(event, ResultMessage): + # End of stream; capture cost estimate for ledger + cost = getattr(event, "total_cost_usd", None) + if cost is not None: + events[-1]["estimated_cost_usd"] = cost + except FileNotFoundError as exc: + raise ClaudeAgentSDKAuthError(exc) from exc + except OSError as exc: + # SDK raises various OSError flavors when credentials are + # missing / unreadable. Classify as auth. + raise ClaudeAgentSDKAuthError(exc) from exc + + text = "".join(text_parts).strip() + return text, events + + +async def _deny_all_tools(tool_name: str, tool_input: dict, context: Any) -> dict: + """can_use_tool callback that unconditionally denies any tool call. + + Defense-in-depth: even if the SDK ignores allowed_tools=[] for + some reason, this callback ensures no tool actually runs. + Returns the documented `{"behavior": "deny", "message": ...}` shape. + """ + return { + "behavior": "deny", + "message": ( + "ClaudeAgentSDKModel deliberately denies all internal tool " + "calls. The smolagents ACL boundary is the only place where " + "tools should execute. See module docstring for invariant." + ), + } diff --git a/src/crucible/config.py b/src/crucible/config.py index e1f72f5..2d3ded1 100644 --- a/src/crucible/config.py +++ b/src/crucible/config.py @@ -33,13 +33,24 @@ class SmolagentsConfig: Per spec §INV-3: only ToolCallingAgent is supported in default safe mode. CodeAct mode is intentionally NOT exposed via config in PR 13. """ - # LiteLLM provider routing: "anthropic", "openai", "openrouter", etc. - # Passed verbatim to smolagents.LiteLLMModel. + # Provider routing. Most values are forwarded to smolagents.LiteLLMModel + # ("anthropic", "openai", "openrouter", etc., requires API key via + # `api_key_env`). + # + # M3 PR 19 special value: "claude-subscription" — uses + # `claude_agent_sdk` + OAuth from `~/.claude/credentials.json` + # instead of LiteLLM + API key. This is a transitional shim until + # smolagents ships native subscription auth or Anthropic publishes + # a token-completion API path with OAuth. ACL invariant: SDK is + # configured as a single-turn text generator (allowed_tools=[], + # max_turns=1) so smolagents' CheatResistancePolicy boundary is + # the only one that fires. See `smolagents_claude_sdk_model.py`. provider: str = "anthropic" # Model id forwarded to LiteLLM (e.g. "claude-3-5-sonnet-20241022"). model: str = "claude-3-5-sonnet-20241022" # Name of the env var to read the API key FROM. The value itself is # never stored in config / logs / prompts (reviewer round 1 Q2). + # Ignored when provider="claude-subscription" (auth via OAuth). api_key_env: str = "ANTHROPIC_API_KEY" # Hard cap on agent.run() steps; prevents runaway tool-use loops. max_steps: int = 12 diff --git a/tests/test_smolagents_claude_sdk_model.py b/tests/test_smolagents_claude_sdk_model.py new file mode 100644 index 0000000..48e99a7 --- /dev/null +++ b/tests/test_smolagents_claude_sdk_model.py @@ -0,0 +1,340 @@ +"""Tests for `crucible.agents.smolagents_claude_sdk_model` — M3 PR 19. + +The CRITICAL test in this module is `test_sdk_is_invoked_with_no_internal_tools` +— it locks in the ACL-preservation invariant that a future SDK update +must not silently break. + +Other tests cover: + - Message-format conversion (smolagents → SDK prompt string) + - Auth-failure classification + - Stream draining (AssistantMessage / ResultMessage handling) + - Sync wrapper around async SDK + - asyncio.run() loud failure inside a running loop + +Skipped when smolagents isn't installed (importorskip). +""" + +from __future__ import annotations + +import asyncio +import json +from typing import Any +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +# Skip module if smolagents extras not installed +pytest.importorskip("smolagents") +pytest.importorskip("claude_agent_sdk") + +from crucible.agents.smolagents_claude_sdk_model import ( + ClaudeAgentSDKAuthError, + ClaudeAgentSDKModel, + _SDK_DISALLOWED_TOOLS, +) + + +# --------------------------------------------------------------------------- +# Helpers — synthetic SDK event stream +# --------------------------------------------------------------------------- + + +class _FakeAssistantMessage: + """Stand-in for `claude_agent_sdk.AssistantMessage`. Has `.content` list of + `_FakeTextBlock`s. We match by isinstance against the REAL types in the + code under test, so we monkey-patch those imports at call time.""" + def __init__(self, text: str): + self.content = [_FakeTextBlock(text)] + + +class _FakeTextBlock: + def __init__(self, text: str): + self.text = text + + +class _FakeResultMessage: + def __init__(self, total_cost_usd: float | None = None): + self.total_cost_usd = total_cost_usd + + +async def _stream_events(events: list[Any]): + for event in events: + yield event + + +# --------------------------------------------------------------------------- +# CRITICAL — ACL invariant +# --------------------------------------------------------------------------- + + +def test_sdk_is_invoked_with_no_internal_tools(monkeypatch): + """**Reviewer round 1 critical pin** — locks in the ACL invariant: + SDK must be invoked with `allowed_tools=[]`, `max_turns=1`, and an + exhaustive `disallowed_tools` list. Any future drift breaks the + invariant; this test catches it. + + Without this configuration, the SDK runs a full agent loop with its + own tools, voiding the smolagents `CheatResistancePolicy` ACL + boundary (the §3.3 agent-loop-in-agent-loop problem).""" + from crucible.agents import smolagents_claude_sdk_model as sdk_module + + captured = {} + + async def _fake_query(*, prompt, options, transport=None): + captured["options"] = options + captured["prompt"] = prompt + # Yield nothing — empty stream is fine for this test + if False: + yield None + + monkeypatch.setattr("claude_agent_sdk.query", _fake_query, raising=True) + # Patch the AssistantMessage and friends that the module imports inline + # so isinstance checks still work + monkeypatch.setattr( + sdk_module, "ClaudeAgentOptions", + # Use the REAL ClaudeAgentOptions for the assertion + __import__("claude_agent_sdk").ClaudeAgentOptions, + raising=False, + ) + + model = ClaudeAgentSDKModel(model="claude-3-5-sonnet-20241022") + asyncio.run(model._async_call("hello")) + + opts = captured["options"] + # The ACL invariant — these MUST hold + assert opts.allowed_tools == [], ( + f"allowed_tools must be empty for ACL invariant; got {opts.allowed_tools!r}" + ) + assert opts.max_turns == 1, ( + f"max_turns must be 1 to prevent SDK agent loop; got {opts.max_turns!r}" + ) + # disallowed_tools is exhaustive — at least covers the known set + must_disallow = {"Read", "Edit", "Write", "Glob", "Grep", "Bash"} + assert must_disallow <= set(opts.disallowed_tools or []), ( + f"disallowed_tools missing required entries; got {opts.disallowed_tools!r}" + ) + # can_use_tool callback wired in + assert opts.can_use_tool is not None, "can_use_tool callback must be set" + + +def test_can_use_tool_callback_denies_everything(): + """The `can_use_tool` callback (defense-in-depth) must return a + `{"behavior": "deny"}` shape for any tool call.""" + from crucible.agents.smolagents_claude_sdk_model import _deny_all_tools + + result = asyncio.run(_deny_all_tools("Read", {"path": "/etc/passwd"}, {})) + assert result["behavior"] == "deny" + assert "deliberately denies" in result["message"] + + +def test_disallowed_tools_list_has_known_dangerous_set(): + """Static check on the module-level constant — sanity for future readers.""" + must_have = {"Read", "Edit", "Write", "Glob", "Grep", "Bash"} + assert must_have <= set(_SDK_DISALLOWED_TOOLS), ( + f"_SDK_DISALLOWED_TOOLS missing required entries; got {_SDK_DISALLOWED_TOOLS!r}" + ) + + +# --------------------------------------------------------------------------- +# Message format conversion +# --------------------------------------------------------------------------- + + +def test_format_prompt_handles_dict_messages(): + model = ClaudeAgentSDKModel() + msgs = [ + {"role": "system", "content": "you are a helper"}, + {"role": "user", "content": "do the thing"}, + ] + out = model._format_prompt(msgs, tools_to_call_from=None) + assert "system:" in out + assert "you are a helper" in out + assert "user:" in out + assert "do the thing" in out + + +def test_format_prompt_handles_list_content(): + """smolagents content can be list of {type, text} blocks.""" + model = ClaudeAgentSDKModel() + msgs = [{"role": "user", "content": [{"type": "text", "text": "hi"}]}] + out = model._format_prompt(msgs, tools_to_call_from=None) + assert "hi" in out + + +# --------------------------------------------------------------------------- +# Stream draining (AssistantMessage text accumulation) +# --------------------------------------------------------------------------- + + +def test_stream_draining_accumulates_text(monkeypatch): + """AssistantMessage text blocks are accumulated; ResultMessage cost + surfaces in events list.""" + from crucible.agents import smolagents_claude_sdk_model as sdk_module + from claude_agent_sdk import AssistantMessage, ResultMessage, TextBlock + + # Patch the real SDK's query() to yield our synthetic events + async def _fake_query(*, prompt, options, transport=None): + yield AssistantMessage( + content=[TextBlock(text="hello "), TextBlock(text="world")], + model="claude-3-5-sonnet", + ) + yield ResultMessage( + subtype="success", + duration_ms=100, + duration_api_ms=80, + is_error=False, + num_turns=1, + session_id="test-session", + total_cost_usd=0.001, + ) + + monkeypatch.setattr("claude_agent_sdk.query", _fake_query, raising=True) + + model = ClaudeAgentSDKModel() + text, events = asyncio.run(model._async_call("test prompt")) + assert text == "hello world" + # ResultMessage event recorded with cost + assert any(e.get("estimated_cost_usd") == 0.001 for e in events) + + +# --------------------------------------------------------------------------- +# generate() returns ChatMessage +# --------------------------------------------------------------------------- + + +def test_generate_returns_chat_message_with_text(monkeypatch): + from crucible.agents import smolagents_claude_sdk_model as sdk_module + from claude_agent_sdk import AssistantMessage, TextBlock + from smolagents.models import MessageRole + + async def _fake_query(*, prompt, options, transport=None): + yield AssistantMessage( + content=[TextBlock(text="response text")], + model="claude-3-5-sonnet", + ) + + monkeypatch.setattr("claude_agent_sdk.query", _fake_query, raising=True) + + model = ClaudeAgentSDKModel() + chat_msg = model.generate([{"role": "user", "content": "hi"}]) + assert chat_msg.role == MessageRole.ASSISTANT + assert chat_msg.content == "response text" + + +# --------------------------------------------------------------------------- +# Auth-failure classification +# --------------------------------------------------------------------------- + + +def test_missing_credentials_raises_auth_error(monkeypatch): + """When `claude_agent_sdk` can't find OAuth creds, the wrapper + converts to `ClaudeAgentSDKAuthError` with a clear "run claude login" + message. The orchestrator can map this to AgentErrorType.AUTH.""" + from crucible.agents import smolagents_claude_sdk_model as sdk_module + + async def _fake_query(*, prompt, options, transport=None): + raise FileNotFoundError("Could not find credentials.json") + # Never reached: + if False: + yield None + + monkeypatch.setattr("claude_agent_sdk.query", _fake_query, raising=True) + + model = ClaudeAgentSDKModel() + with pytest.raises(ClaudeAgentSDKAuthError, match="claude login"): + asyncio.run(model._async_call("hi")) + + +def test_oserror_classified_as_auth_error(monkeypatch): + """OSError variants from the SDK (creds unreadable, etc.) also map to AUTH.""" + from crucible.agents import smolagents_claude_sdk_model as sdk_module + + async def _fake_query(*, prompt, options, transport=None): + raise OSError("Permission denied") + if False: + yield None + + monkeypatch.setattr("claude_agent_sdk.query", _fake_query, raising=True) + + model = ClaudeAgentSDKModel() + with pytest.raises(ClaudeAgentSDKAuthError): + asyncio.run(model._async_call("hi")) + + +# --------------------------------------------------------------------------- +# asyncio.run() loud failure inside a running loop +# --------------------------------------------------------------------------- + + +def test_generate_loud_failure_inside_running_loop(monkeypatch): + """If `generate()` is called from inside an existing event loop, + `asyncio.run()` raises with a documented hint about the future + async-smolagents path.""" + from crucible.agents import smolagents_claude_sdk_model as sdk_module + + # Simulate the scenario: monkey-patch asyncio.run to raise the + # exact RuntimeError shape Python emits when called from inside a loop. + def _fake_run(coro): + # Close the unused coroutine to suppress Python's warning + coro.close() + raise RuntimeError("asyncio.run() cannot be called from a running event loop") + + monkeypatch.setattr(asyncio, "run", _fake_run) + + model = ClaudeAgentSDKModel() + with pytest.raises(RuntimeError, match="cannot be called from inside an existing"): + model.generate([{"role": "user", "content": "hi"}]) + + +# --------------------------------------------------------------------------- +# SmolagentsBackend integration: provider="claude-subscription" +# --------------------------------------------------------------------------- + + +def test_smolagents_backend_uses_claude_sdk_model_when_configured(tmp_path, monkeypatch): + """When `SmolagentsConfig.provider == "claude-subscription"`, the + backend constructs `ClaudeAgentSDKModel` instead of `LiteLLMModel`.""" + from pathlib import Path + from crucible.config import SmolagentsConfig + from crucible.agents.smolagents_backend import SmolagentsBackend + from crucible.agents.smolagents_claude_sdk_model import ClaudeAgentSDKModel + from crucible.security.cheat_resistance_policy import CheatResistancePolicy + + # Build minimal fixture + ws = tmp_path + (ws / "train.py").write_text("x = 1\n") + policy = CheatResistancePolicy.from_lists(workspace=ws, editable=["train.py"]) + config = SmolagentsConfig( + provider="claude-subscription", + model="claude-3-5-sonnet-20241022", + ) + + backend = SmolagentsBackend( + config=config, + policy=policy, + workspace=Path(ws), + ) + assert isinstance(backend._model, ClaudeAgentSDKModel) + + +def test_smolagents_backend_uses_litellm_for_other_providers(tmp_path, monkeypatch): + """Backward-compat: existing `provider="anthropic"` (or any other + LiteLLM-routed value) still uses LiteLLMModel.""" + from pathlib import Path + from crucible.config import SmolagentsConfig + from crucible.agents.smolagents_backend import SmolagentsBackend + from smolagents import LiteLLMModel + from crucible.security.cheat_resistance_policy import CheatResistancePolicy + + monkeypatch.setenv("ANTHROPIC_API_KEY", "test-key") # avoid the warning + ws = tmp_path + (ws / "train.py").write_text("x = 1\n") + policy = CheatResistancePolicy.from_lists(workspace=ws, editable=["train.py"]) + config = SmolagentsConfig(provider="anthropic") + + backend = SmolagentsBackend( + config=config, + policy=policy, + workspace=Path(ws), + ) + assert isinstance(backend._model, LiteLLMModel) From a15b606264918f80ef7075359ca45a6a3ec60fe1 Mon Sep 17 00:00:00 2001 From: suzuke Date: Sun, 26 Apr 2026 11:17:10 +0800 Subject: [PATCH 2/2] fix(m3): typed auth-error classification + docstring future-tense (R2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewer round 2 was VERIFIED with 2 non-blocking issues. Both folded in. R2 #1 (suggested before merge) — typed auth classification: `ClaudeAgentSDKAuthError` was mapping to `AgentErrorType.AUTH` only because the error message happened to contain "api key" (in the "switch to provider: anthropic with an explicit API key" sentence). String- match coincidence; rewording the message would silently break the classification. New `_classify_error_typed(exc)` does isinstance check on `ClaudeAgentSDKAuthError` BEFORE falling through to the string-match path. Generic exceptions still go through `_classify_error` for backward compat. End-to-end regression test added: `test_auth_error_classification_end_to_end` constructs a real SmolagentsBackend, simulates SDK raising ClaudeAgentSDKAuthError via `agent.run`, asserts the resulting AgentResult.error_type == AgentErrorType.AUTH. Catches future docstring/message rewording that would silently demote to UNKNOWN. R2 #2 (docs-only) — future tense for `usage_source` plumbing: Module docstring previously claimed AttemptNode "records this as `usage_source=\"oauth_estimated\"`" — but the new enum value isn't yet in spec §4.1's `Literal[...]` and orchestrator doesn't read `backend_metadata` for this field. Reworded as future-tense: "WILL record once PR 19a lands the orchestrator plumbing; today, the field is unset and falls back to default." Stats: 2 files, +52/-7 LOC. 13 tests in test_smolagents_claude_sdk_model.py (was 12). Full suite: 2775 passed + 1 pre-existing failure unchanged + 4 skipped. 0 regressions. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/crucible/agents/smolagents_backend.py | 30 ++++++++++++- .../agents/smolagents_claude_sdk_model.py | 8 ++-- tests/test_smolagents_claude_sdk_model.py | 44 +++++++++++++++++++ 3 files changed, 78 insertions(+), 4 deletions(-) diff --git a/src/crucible/agents/smolagents_backend.py b/src/crucible/agents/smolagents_backend.py index 39d4605..c437c6a 100644 --- a/src/crucible/agents/smolagents_backend.py +++ b/src/crucible/agents/smolagents_backend.py @@ -174,7 +174,11 @@ def generate_edit(self, prompt: str, workspace: Path) -> AgentResult: except Exception as exc: description = "" agent_output = f"smolagents agent error: {exc}" - error_type = _classify_error(str(exc)) + # M3 PR 19 R2 fix #1: type-based classification first so + # ClaudeAgentSDKAuthError isn't dependent on a string-match + # coincidence. Generic exceptions still go through + # `_classify_error`'s pattern-match for back-compat. + error_type = _classify_error_typed(exc) logger.warning("smolagents backend: agent error: %s", exc) duration = time.monotonic() - t0 @@ -285,6 +289,30 @@ def _classify_error(msg: str) -> AgentErrorType: return AgentErrorType.UNKNOWN +def _classify_error_typed(exc: Exception) -> AgentErrorType: + """Type-aware classification — M3 PR 19 R2 fix #1. + + Reviewer round 2 caught that `ClaudeAgentSDKAuthError` mapped to + `AgentErrorType.AUTH` only by string-match coincidence (the error + message happened to contain "api key"). If the message is reworded, + classification silently breaks. + + Fix: pre-check known typed errors before falling through to + pattern match. Generic exceptions still go through `_classify_error` + for back-compat with non-Crucible-typed errors. + """ + # Lazy import to avoid cycles + only load if smolagents-CC path is used + try: + from crucible.agents.smolagents_claude_sdk_model import ( + ClaudeAgentSDKAuthError, + ) + if isinstance(exc, ClaudeAgentSDKAuthError): + return AgentErrorType.AUTH + except ImportError: + pass + return _classify_error(str(exc)) + + def _snapshot_mtimes(workspace: Path) -> dict[Path, float]: """Take a {abs_path: mtime} snapshot of all files in workspace.""" snapshot: dict[Path, float] = {} diff --git a/src/crucible/agents/smolagents_claude_sdk_model.py b/src/crucible/agents/smolagents_claude_sdk_model.py index f9cb36e..8fe6193 100644 --- a/src/crucible/agents/smolagents_claude_sdk_model.py +++ b/src/crucible/agents/smolagents_claude_sdk_model.py @@ -39,9 +39,11 @@ **Cost reporting nuance**: SDK's `ResultMessage.total_cost_usd` is an estimate of *equivalent API pricing* — what the call would have cost on metered API auth. On subscription, you do NOT pay that. AttemptNode -records this as `usage_source="oauth_estimated"` (a new value) to -disambiguate from `"api"` (actual metered cost) and avoid users -thinking they're being double-billed. +WILL record this as `usage_source="oauth_estimated"` (new enum value) +once PR 19a lands the orchestrator plumbing; today, the field is left +unset and falls back to the orchestrator default. Users running this +backend should treat any cost shown by Crucible's existing tooling as +"if-you-had-paid-api-prices" pricing, not actual subscription bill. """ from __future__ import annotations diff --git a/tests/test_smolagents_claude_sdk_model.py b/tests/test_smolagents_claude_sdk_model.py index 48e99a7..415c0a5 100644 --- a/tests/test_smolagents_claude_sdk_model.py +++ b/tests/test_smolagents_claude_sdk_model.py @@ -317,6 +317,50 @@ def test_smolagents_backend_uses_claude_sdk_model_when_configured(tmp_path, monk assert isinstance(backend._model, ClaudeAgentSDKModel) +def test_auth_error_classification_end_to_end(tmp_path, monkeypatch): + """**M3 PR 19 R2 fix #1 regression** — locks in the typed-classification + coupling between `ClaudeAgentSDKAuthError` and `AgentErrorType.AUTH`. + + Reviewer round 2 caught that the original mapping was a string-match + coincidence (error message contained "api key"). If the message is + reworded, classification silently breaks. Fix uses isinstance check + in `_classify_error_typed` BEFORE falling through to string match. + + This test simulates the SDK raising auth, runs through the full + SmolagentsBackend.generate_edit path, asserts the resulting + AgentResult.error_type == AgentErrorType.AUTH (not UNKNOWN). + """ + from pathlib import Path + from crucible.agents.base import AgentErrorType + from crucible.agents.smolagents_backend import SmolagentsBackend + from crucible.config import SmolagentsConfig + from crucible.security.cheat_resistance_policy import CheatResistancePolicy + + ws = tmp_path + (ws / "train.py").write_text("x = 1\n") + policy = CheatResistancePolicy.from_lists(workspace=ws, editable=["train.py"]) + config = SmolagentsConfig(provider="claude-subscription") + + backend = SmolagentsBackend( + config=config, policy=policy, workspace=Path(ws), + ) + + # Replace agent.run with a stub that raises ClaudeAgentSDKAuthError + def _raise_auth(*args, **kwargs): + raise ClaudeAgentSDKAuthError( + FileNotFoundError("Could not find credentials.json") + ) + + backend._agent.run = _raise_auth + result = backend.generate_edit("anything", Path(ws)) + # Critical assertion: typed classification wins regardless of msg content + assert result.error_type == AgentErrorType.AUTH, ( + f"Expected AUTH classification for ClaudeAgentSDKAuthError, " + f"got {result.error_type!r}. The type-based check in " + f"_classify_error_typed must fire BEFORE string-match fallback." + ) + + def test_smolagents_backend_uses_litellm_for_other_providers(tmp_path, monkeypatch): """Backward-compat: existing `provider="anthropic"` (or any other LiteLLM-routed value) still uses LiteLLMModel."""