alishahryar1--free-claude-code
f8c21a48f2
## Problem The HTTP adapter owned model routing, provider execution, and runtime-facing contracts, so API handlers depended on provider implementation types. Provider preflight also discovered private request builders dynamically, obscuring the boundary that must fail before streaming begins. ## Changes | Before | After | | --- | --- | | `api/` owned model routing and shared provider execution. | `application/` owns routing and a settings-independent `ProviderExecutor`. | | API handlers accepted `BaseProvider` callbacks. | API handlers consume the narrow structural `ProviderPort`. | | `BaseProvider` discovered `_build_request_body` dynamically. | Both transport families implement explicit abstract preflight, with LM Studio composing context validation. | | Request leases, task control, and provider model metadata had adapter/provider owners. | Application-owned ports and immutable values define those cross-package contracts. | | Boundary direction was implicit. | Architecture contracts and documentation enforce the final dependency direction. | | Package version was `3.4.19`. | Package version is `3.4.20`, with the lockfile updated. | | Coverage followed the old module layout. | Deterministic boundary/preflight regressions and live Messages/Responses smokes cover the new shape. | <!-- greptile_comment --> <details open><summary><h3>Greptile Summary</h3></summary> This PR adds a typed application boundary for provider execution. The main changes are: - New `application` package for routing, execution, ports, and model metadata. - API handlers now call application-owned routing and provider execution. - Provider preflight is now explicit on the transport families. - Runtime API composition now uses a task-control port for `/stop`. - Import-boundary tests, smoke references, docs, version, and lockfile were updated. </details> <h3>Confidence Score: 5/5</h3> This looks safe to merge. No blocking issues found in the changed code. None. <details><summary><h3><a href="https://www.greptile.com/trex"><img alt="T-Rex" src="https://greptile-static-assets.s3.amazonaws.com/trex/trex_green.svg" height="20" align="absmiddle"></a> T-Rex Logs</h3></summary> **What T-Rex did** - Ran a deterministic pytest run for the provider boundary preflight, which completed with exit code 0 and 102 tests passing in 4.77 seconds. - Launched the environment presence check as part of the preflight, which completed with exit code 0 and confirmed OPENCODE\_API\_KEY=\[REDACTED\] matched. - Attempted the live provider smoke test, which completed with exit code 0 and 2 tests skipped due to incomplete smoke configuration. <a href="https://app.greptile.com/trex/runs/14067905/artifacts"><picture><source media="(prefers-color-scheme: dark)" srcset="https://greptile-static-assets.s3.amazonaws.com/badges/ViewAllArtifactsDark.svg?v=4"><source media="(prefers-color-scheme: light)" srcset="https://greptile-static-assets.s3.amazonaws.com/badges/ViewAllArtifacts.svg?v=4"><img alt="View all artifacts" src="https://greptile-static-assets.s3.amazonaws.com/badges/ViewAllArtifacts.svg?v=4"></picture></a> <sub><a href="https://www.greptile.com/trex"><img alt="T-Rex" src="https://greptile-static-assets.s3.amazonaws.com/trex/trex_green.svg" height="14" align="absmiddle"></a> Ran code and verified through T-Rex</sub> </details> <details open><summary><h3>Important Files Changed</h3></summary> | Filename | Overview | |----------|----------| | src/free_claude_code/application/execution.py | Moves shared provider execution into the application layer and keeps eager preflight before token counting and streaming. | | src/free_claude_code/application/ports.py | Adds structural provider, request-runtime, and task-control protocols used across the new boundary. | | src/free_claude_code/api/routes.py | Updates route composition to use the application provider resolver and task-control stop path. | | src/free_claude_code/providers/base.py | Makes provider preflight explicit by requiring subclasses or transport bases to implement it. | | src/free_claude_code/providers/transports/openai_chat/transport.py | Adds OpenAI-chat preflight through the same request-body builder used by streaming. | | src/free_claude_code/providers/transports/anthropic_messages/transport.py | Adds native Messages preflight through the native request-body builder. | | src/free_claude_code/providers/model_listing.py | Keeps provider model-list parsing while moving `ProviderModelInfo` ownership to the application layer. | | src/free_claude_code/runtime/bootstrap.py | Passes the runtime object through the new `tasks` service slot. | | tests/contracts/test_import_boundaries.py | Extends import-boundary tests for the new application package. | </details> <details open><summary><h3>Flowchart</h3></summary> <a href="#gh-light-mode-only"> ```mermaid %%{init: {'theme': 'neutral'}}%% flowchart LR API[api handlers and routes] --> Routing[application.routing] API --> Executor[application.execution] API --> Ports[application.ports] Executor --> ProviderPort[ProviderPort] ProviderPort --> Preflight[preflight_stream] ProviderPort --> Stream[stream_response] Runtime[runtime bootstrap and provider manager] --> Ports Providers[providers] --> Metadata[application.model_metadata] Executor --> Core[core anthropic and trace] Routing --> Config[config settings and model refs] ``` </a> <a href="#gh-dark-mode-only"> ```mermaid %%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart LR API[api handlers and routes] --> Routing[application.routing] API --> Executor[application.execution] API --> Ports[application.ports] Executor --> ProviderPort[ProviderPort] ProviderPort --> Preflight[preflight_stream] ProviderPort --> Stream[stream_response] Runtime[runtime bootstrap and provider manager] --> Ports Providers[providers] --> Metadata[application.model_metadata] Executor --> Core[core anthropic and trace] Routing --> Config[config settings and model refs] ``` </a> </details> <sub>Reviews (1): Last reviewed commit: ["Introduce typed application boundary"](https://github.com/alishahryar1/free-claude-code/commit/4cdcf97231c812c6f568ca3d74af7ce759d7f2dc) | [Re-trigger Greptile](https://app.greptile.com/api/retrigger?id=43462788)</sub> <!-- /greptile_comment -->
217 行
6.9 KiB
Python
217 行
6.9 KiB
Python
"""Tests that API and SSE logging avoid raw sensitive payloads by default."""
|
|
|
|
from unittest.mock import MagicMock, patch
|
|
|
|
import pytest
|
|
from fastapi import HTTPException
|
|
from fastapi.responses import JSONResponse
|
|
|
|
from free_claude_code.api import request_errors
|
|
from free_claude_code.api.handlers import MessagesHandler, TokenCountHandler
|
|
from free_claude_code.application import execution
|
|
from free_claude_code.config.settings import Settings
|
|
from free_claude_code.core.anthropic import AnthropicStreamLedger
|
|
from free_claude_code.core.anthropic.models import Message, MessagesRequest
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_create_message_skips_full_payload_debug_log_by_default():
|
|
settings = Settings()
|
|
assert settings.log_raw_api_payloads is False
|
|
mock_provider = MagicMock()
|
|
|
|
async def fake_stream(*_a, **_kw):
|
|
yield "event: ping\ndata: {}\n\n"
|
|
|
|
mock_provider.stream_response = fake_stream
|
|
service = MessagesHandler(settings, provider_resolver=lambda _: mock_provider)
|
|
|
|
request = MessagesRequest(
|
|
model="claude-3-haiku-20240307",
|
|
max_tokens=10,
|
|
messages=[Message(role="user", content="secret-user-text")],
|
|
)
|
|
|
|
with patch.object(execution.logger, "debug") as mock_debug:
|
|
await service.create(request)
|
|
|
|
full_payload_calls = [
|
|
c
|
|
for c in mock_debug.call_args_list
|
|
if c.args and str(c.args[0]) == "FULL_PAYLOAD [{}]: {}"
|
|
]
|
|
assert not full_payload_calls
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_create_message_logs_full_payload_when_opt_in():
|
|
settings = Settings()
|
|
settings.log_raw_api_payloads = True
|
|
mock_provider = MagicMock()
|
|
|
|
async def fake_stream(*_a, **_kw):
|
|
yield "event: ping\ndata: {}\n\n"
|
|
|
|
mock_provider.stream_response = fake_stream
|
|
service = MessagesHandler(settings, provider_resolver=lambda _: mock_provider)
|
|
request = MessagesRequest(
|
|
model="claude-3-haiku-20240307",
|
|
max_tokens=10,
|
|
messages=[Message(role="user", content="visible")],
|
|
)
|
|
|
|
with patch.object(execution.logger, "debug") as mock_debug:
|
|
await service.create(request)
|
|
|
|
keys = [c.args[0] for c in mock_debug.call_args_list if c.args]
|
|
assert any(k == "FULL_PAYLOAD [{}]: {}" for k in keys)
|
|
|
|
|
|
def test_stream_ledger_default_debug_has_no_serialized_json_content():
|
|
with patch(
|
|
"free_claude_code.core.anthropic.streaming.emitter.logger.debug"
|
|
) as mock_debug:
|
|
ledger = AnthropicStreamLedger("msg_x", "m", 1, log_raw_events=False)
|
|
ledger.message_start()
|
|
|
|
assert mock_debug.call_count == 0
|
|
|
|
|
|
def test_stream_ledger_raw_logging_includes_event_body_when_enabled():
|
|
with patch(
|
|
"free_claude_code.core.anthropic.streaming.emitter.logger.debug"
|
|
) as mock_debug:
|
|
ledger = AnthropicStreamLedger("msg_x", "m", 1, log_raw_events=True)
|
|
ledger.message_start()
|
|
|
|
assert mock_debug.call_count == 1
|
|
message = str(mock_debug.call_args)
|
|
assert "message_start" in message
|
|
assert "role" in message
|
|
|
|
|
|
def _flatten_log_calls(mock_log) -> str:
|
|
parts: list[str] = []
|
|
for call in mock_log.call_args_list:
|
|
parts.extend(str(arg) for arg in call.args)
|
|
parts.append(repr(call.kwargs))
|
|
return " ".join(parts)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_create_message_unexpected_error_default_logs_exclude_exception_text():
|
|
settings = Settings()
|
|
assert settings.log_api_error_tracebacks is False
|
|
secret = "upstream-secret-token-abc"
|
|
|
|
mock_provider = MagicMock()
|
|
|
|
def stream_boom(*_a, **_kw):
|
|
raise RuntimeError(secret)
|
|
|
|
mock_provider.stream_response = stream_boom
|
|
service = MessagesHandler(settings, provider_resolver=lambda _: mock_provider)
|
|
request = MessagesRequest(
|
|
model="claude-3-haiku-20240307",
|
|
max_tokens=10,
|
|
messages=[Message(role="user", content="hi")],
|
|
)
|
|
|
|
with patch.object(request_errors.logger, "error") as log_err:
|
|
response = await service.create(request)
|
|
|
|
blob = _flatten_log_calls(log_err)
|
|
assert secret not in blob
|
|
assert "RuntimeError" in blob
|
|
assert isinstance(response, JSONResponse)
|
|
assert response.status_code == 500
|
|
assert response.headers["x-should-retry"] == "false"
|
|
assert response.body
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_create_message_unexpected_error_terminal_json_ignores_status_code():
|
|
"""Non-provider stream failures must not leak arbitrary HTTP status attributes."""
|
|
|
|
class WeirdError(Exception):
|
|
status_code = 418
|
|
|
|
settings = Settings()
|
|
mock_provider = MagicMock()
|
|
|
|
def stream_boom(*_a, **_kw):
|
|
raise WeirdError("no")
|
|
|
|
mock_provider.stream_response = stream_boom
|
|
service = MessagesHandler(settings, provider_resolver=lambda _: mock_provider)
|
|
request = MessagesRequest(
|
|
model="claude-3-haiku-20240307",
|
|
max_tokens=10,
|
|
messages=[Message(role="user", content="hi")],
|
|
)
|
|
|
|
response = await service.create(request)
|
|
|
|
assert isinstance(response, JSONResponse)
|
|
assert response.status_code == 500
|
|
assert response.headers["x-should-retry"] == "false"
|
|
payload = bytes(response.body).decode("utf-8")
|
|
assert '"type":"api_error"' in payload
|
|
assert '"message":"no"' in payload
|
|
|
|
|
|
def test_parse_cli_event_error_logs_metadata_by_default():
|
|
"""CLI parser must not log raw error text unless LOG_RAW_CLI_DIAGNOSTICS is on."""
|
|
from free_claude_code.messaging.event_parser import parse_cli_event
|
|
|
|
secret = "user-secret-parser-leak-xyz"
|
|
with patch("free_claude_code.messaging.event_parser.logger.info") as log_info:
|
|
parse_cli_event(
|
|
{"type": "error", "error": {"message": secret}}, log_raw_cli=False
|
|
)
|
|
flat = " ".join(str(c) for c in log_info.call_args_list)
|
|
assert secret not in flat
|
|
assert "message_chars" in flat
|
|
|
|
|
|
def test_parse_cli_event_error_logs_text_when_log_raw_cli_enabled():
|
|
from free_claude_code.messaging.event_parser import parse_cli_event
|
|
|
|
secret = "visible-cli-parser-msg"
|
|
with patch("free_claude_code.messaging.event_parser.logger.info") as log_info:
|
|
parse_cli_event(
|
|
{"type": "error", "error": {"message": secret}}, log_raw_cli=True
|
|
)
|
|
flat = " ".join(str(c) for c in log_info.call_args_list)
|
|
assert secret in flat
|
|
|
|
|
|
def test_count_tokens_unexpected_error_default_logs_exclude_exception_text():
|
|
settings = Settings()
|
|
assert settings.log_api_error_tracebacks is False
|
|
secret = "count-tokens-leak-xyz"
|
|
|
|
def boom(*_a, **_kw):
|
|
raise ValueError(secret)
|
|
|
|
service = TokenCountHandler(
|
|
settings,
|
|
token_counter=boom,
|
|
)
|
|
from free_claude_code.core.anthropic.models import TokenCountRequest
|
|
|
|
req = TokenCountRequest(
|
|
model="claude-3-haiku-20240307",
|
|
messages=[Message(role="user", content="x")],
|
|
)
|
|
|
|
with (
|
|
patch.object(request_errors.logger, "error") as log_err,
|
|
pytest.raises(HTTPException),
|
|
):
|
|
service.count(req)
|
|
|
|
blob = _flatten_log_calls(log_err)
|
|
assert secret not in blob
|
|
assert "ValueError" in blob
|