feat(providers): ModelSession interface + AnthropicSession adapter

[dimakis]

Summary

• Defines ModelSession interface — provider-agnostic abstraction for the agentic loop (replaces Agent SDK's query())
• ModelSession.turn(messages) handles one LLM call, streams back normalized StreamEvents — Mitzo owns the loop
• AnthropicSession implementation speaks Anthropic Messages API format, routes through praxis-proxy on :9090 for provider translation
• Full type definitions: StreamEvent, ConversationMessage, ContentBlock, ToolDefinition, ModelSessionConfig
• Exported from @mitzo/harness for server consumption

Architecture:

Mitzo agentic loop → ModelSession.turn() → praxis-proxy (:9090) → Claude/GPT/GLM

This is Phase 1a of the provider-agnostic architecture (mgmt#215). Next: build the agentic loop that calls ModelSession.turn() repeatedly, replacing the SDK's query().

Test plan

• 11 unit tests covering constructor, streaming, text/tool_use blocks, chunked SSE, error handling, headers
• Harness package compiles cleanly (tsc --noEmit)
• Praxis-proxy verified running as launchd daemon on :9090

🤖 Generated with Claude Code

[dimakis]

Centaur Review

Found 7 issue(s) (3 warning).

packages/harness/src/providers/anthropic-session.ts

Clean interface design with good SSE edge-case coverage. Main concerns: the thinking/max_tokens comment contradicts the code (potential API bug), unchecked type assertion on parsed SSE events, and constructor tests that don't verify the routing they claim to test.

• 🟡 bugs (L105): Comment says "Anthropic requires removing max_tokens" when thinking is enabled, but the code never removes max_tokens from the body. Either the code is missing delete body.max_tokens (real bug), or the comment is wrong and the current Anthropic API accepts both (misleading comment). As-is, this will either send an invalid request when thinking is used, or confuse future maintainers. [fixable]
• 🟡 unsafe_assumptions (L57): parsed as StreamEvent is an unchecked type assertion. If the API sends a payload with a matching type field but missing required properties (e.g., a message_start without message.usage), it passes through and will crash downstream. Consider a lightweight shape check (e.g., verify parsed.message exists for message_start). [fixable]
• 🔵 unsafe_assumptions (L78): API key silently falls back to 'dummy' when neither options.apiKey nor ANTHROPIC_API_KEY is set. Requests to the real Anthropic API will fail with a confusing 401 instead of a clear configuration error. Consider throwing or logging a warning when routing to the real API without a key. [fixable]

packages/harness/__tests__/anthropic-session.test.ts

Clean interface design with good SSE edge-case coverage. Main concerns: the thinking/max_tokens comment contradicts the code (potential API bug), unchecked type assertion on parsed SSE events, and constructor tests that don't verify the routing they claim to test.

• 🟡 missing_tests: Constructor tests for praxis proxy (line 61) and explicit baseUrl (line 68) only assert session.provider === 'anthropic', which is true for every AnthropicSession regardless of config. They don't verify the actual URL used. The 'sends correct request body' test covers URL indirectly, but these two constructor tests are effectively no-ops — they prove the constructor doesn't throw, not that proxy/URL routing works. [fixable]
• 🔵 missing_tests: No test for the thinking config path. When config.thinking is set, the request body should include the thinking block. This is the only untested branch in turn()'s body construction, and it interacts with the max_tokens issue noted in the implementation. [fixable]
• 🔵 missing_tests: createAnthropicSession factory function is exported in the public API but has no test coverage. A single test calling it and verifying the return type would suffice. [fixable]
• 🔵 missing_tests: No test for fetch-level failures (network error where fetch itself rejects, vs. the tested HTTP 429 case where fetch resolves with an error status). The error path would surface differently — an unhandled rejection vs. a thrown 'Anthropic API error' message. [fixable]

[dimakis]

🟡 bugs: Comment says "Anthropic requires removing max_tokens" when thinking is enabled, but the code never removes max_tokens from the body. Either the code is missing delete body.max_tokens (real bug), or the comment is wrong and the current Anthropic API accepts both (misleading comment). As-is, this will either send an invalid request when thinking is used, or confuse future maintainers. [fixable]

[dimakis]

🟡 unsafe_assumptions: parsed as StreamEvent is an unchecked type assertion. If the API sends a payload with a matching type field but missing required properties (e.g., a message_start without message.usage), it passes through and will crash downstream. Consider a lightweight shape check (e.g., verify parsed.message exists for message_start). [fixable]

[dimakis]

🔵 unsafe_assumptions: API key silently falls back to 'dummy' when neither options.apiKey nor ANTHROPIC_API_KEY is set. Requests to the real Anthropic API will fail with a confusing 401 instead of a clear configuration error. Consider throwing or logging a warning when routing to the real API without a key. [fixable]

[dimakis]

Centaur Review

Found 5 issue(s) (1 critical) (2 warning).

packages/harness/src/providers/anthropic-session.ts

The thinking-mode path has a documented-but-unimplemented requirement (max_tokens must be removed), which will cause API errors when extended thinking is used. Otherwise a clean, well-tested adapter with good SSE chunking coverage.

• 🔴 bugs (L105): The comment on line 108 documents that max_tokens must be removed when thinking is enabled, but the code never does it. max_tokens (set on line 91) remains in the request body alongside thinking, which will cause an Anthropic API error. Add delete body.max_tokens; inside the if (this.config.thinking) block. [fixable]
• 🟡 unsafe_assumptions (L78): Falling back to 'dummy' when no API key is provided will silently send requests that return 401. Consider throwing at construction time (or at least logging a warning) when no key is available and baseUrl points to the real Anthropic API, so misconfiguration is caught early rather than at the first turn() call. [fixable]

packages/harness/__tests__/anthropic-session.test.ts

The thinking-mode path has a documented-but-unimplemented requirement (max_tokens must be removed), which will cause API errors when extended thinking is used. Otherwise a clean, well-tested adapter with good SSE chunking coverage.

• 🟡 missing_tests: No test for the thinking config path. Given the bug above (max_tokens not removed), a test asserting the correct request body when thinking is enabled would have caught it. Should verify body.thinking is set and body.max_tokens is absent. [fixable]
• 🔵 missing_tests: No test for AbortSignal cancellation (config.signal). A test that aborts mid-stream and verifies the reader is released would cover an important edge case for the agentic loop's interrupt flow. [fixable]
• 🔵 style (L62): Constructor tests for praxis proxy (line 62) and explicit baseUrl (line 69) only assert provider === 'anthropic' — they don't verify the resolved URL. Consider making a turn() call and checking fetchSpy.mock.calls[0][0] to confirm the URL resolution logic actually works, or expose the resolved URL for testing. [fixable]

[dimakis]

🔴 bugs: The comment on line 108 documents that max_tokens must be removed when thinking is enabled, but the code never does it. max_tokens (set on line 91) remains in the request body alongside thinking, which will cause an Anthropic API error. Add delete body.max_tokens; inside the if (this.config.thinking) block. [fixable]

[dimakis]

🟡 unsafe_assumptions: Falling back to 'dummy' when no API key is provided will silently send requests that return 401. Consider throwing at construction time (or at least logging a warning) when no key is available and baseUrl points to the real Anthropic API, so misconfiguration is caught early rather than at the first turn() call. [fixable]

[dimakis]

🔵 style: Constructor tests for praxis proxy (line 62) and explicit baseUrl (line 69) only assert provider === 'anthropic' — they don't verify the resolved URL. Consider making a turn() call and checking fetchSpy.mock.calls[0][0] to confirm the URL resolution logic actually works, or expose the resolved URL for testing. [fixable]

[dimakis]

Centaur Review

Found 7 issue(s) (3 warning).

packages/harness/src/providers/anthropic-session.ts

Solid foundation for provider-agnostic sessions. Main concerns: a misleading comment in the thinking config path that could cause a future bug, a silent fallback to a dummy API key that masks misconfiguration, and missing test coverage for the thinking and abort-signal code paths.

• 🟡 bugs (L107): Misleading comment will cause a future bug. The comment says "Anthropic requires removing max_tokens" when thinking is enabled, but the code doesn't do it — and shouldn't, because the API actually requires max_tokens as the total output budget alongside thinking.budget_tokens. A future contributor following this comment's guidance would break thinking mode. Either delete the comment or correct it to explain the relationship (max_tokens = total budget, budget_tokens = thinking portion). [fixable]
• 🟡 unsafe_assumptions (L82): API key falls back to the literal string 'dummy' when neither options.apiKey nor ANTHROPIC_API_KEY is set. This silently sends requests with an invalid key, producing a confusing 401 instead of failing fast at construction time. Consider throwing an error if no key is available (or at minimum logging a warning), so misconfigured sessions are caught early. [fixable]
• 🔵 style (L1): The 10-line file-level doc comment is verbose for a project whose CLAUDE.md says "default to writing no comments" and "never write multi-paragraph docstrings or multi-line comment blocks." The module's purpose is already clear from the class name and file name. Consider reducing to a single line or removing entirely. [fixable]
• 🔵 bugs (L158): SSE parser resets currentEventType to '' immediately after processing a data: line, which means multi-line data: fields (valid per SSE spec — concatenated with newlines) would lose their event type context on the second line. Anthropic's API doesn't use multi-line data fields today, so this is theoretical, but if praxis-proxy emits them the parser will silently drop events. [fixable]

packages/harness/__tests__/anthropic-session.test.ts

Solid foundation for provider-agnostic sessions. Main concerns: a misleading comment in the thinking config path that could cause a future bug, a silent fallback to a dummy API key that masks misconfiguration, and missing test coverage for the thinking and abort-signal code paths.

• 🟡 missing_tests: No test for the thinking config path. The implementation conditionally sets body.thinking (line 105-109 of anthropic-session.ts) but this branch is never exercised in tests. A test should verify that when thinking: { type: 'enabled', budget_tokens: 1000 } is set, the request body includes the thinking block. [fixable]
• 🔵 missing_tests: No test for AbortSignal cancellation. ModelSessionConfig.signal is passed through to fetch, but no test verifies that aborting mid-stream cancels the request and propagates the error. This is a key production path for session interruption. [fixable]
• 🔵 missing_tests: No test for network-level failures (fetch rejection). The only error test covers HTTP 429 responses. A test where fetch itself throws (e.g., ERR_CONNECTION_REFUSED) would verify the error propagation path for when the API host is unreachable. [fixable]

[dimakis]

🟡 bugs: Misleading comment will cause a future bug. The comment says "Anthropic requires removing max_tokens" when thinking is enabled, but the code doesn't do it — and shouldn't, because the API actually requires max_tokens as the total output budget alongside thinking.budget_tokens. A future contributor following this comment's guidance would break thinking mode. Either delete the comment or correct it to explain the relationship (max_tokens = total budget, budget_tokens = thinking portion). [fixable]

[dimakis]

🟡 unsafe_assumptions: API key falls back to the literal string 'dummy' when neither options.apiKey nor ANTHROPIC_API_KEY is set. This silently sends requests with an invalid key, producing a confusing 401 instead of failing fast at construction time. Consider throwing an error if no key is available (or at minimum logging a warning), so misconfigured sessions are caught early. [fixable]

[dimakis]

🔵 style: The 10-line file-level doc comment is verbose for a project whose CLAUDE.md says "default to writing no comments" and "never write multi-paragraph docstrings or multi-line comment blocks." The module's purpose is already clear from the class name and file name. Consider reducing to a single line or removing entirely. [fixable]

[dimakis]

🔵 bugs: SSE parser resets currentEventType to '' immediately after processing a data: line, which means multi-line data: fields (valid per SSE spec — concatenated with newlines) would lose their event type context on the second line. Anthropic's API doesn't use multi-line data fields today, so this is theoretical, but if praxis-proxy emits them the parser will silently drop events. [fixable]

[dimakis]

Centaur Review

Found 5 issue(s) (1 warning).

packages/harness/__tests__/anthropic-session.test.ts

Well-structured provider abstraction with solid SSE parsing and good test coverage of the happy path. Main gap is incomplete validation depth in parseSSE for message_delta/message_start usage fields, plus missing tests for thinking blocks, abort signals, and parse error paths.

• 🔵 missing_tests: No test coverage for thinking content blocks (content_block_start with type: 'thinking' and content_block_delta with type: 'thinking_delta'). These event types are defined in session-types.ts and wired through parseSSE validation, but never exercised by tests. [fixable]
• 🔵 missing_tests: No test for AbortSignal cancellation. ModelSessionConfig.signal is passed through to fetch() but no test verifies that aborting mid-stream works correctly (reader cleanup, error propagation). [fixable]
• 🔵 missing_tests: No test for parseSSE returning null on malformed data (invalid JSON, missing required fields, unknown event types). The function has significant validation logic that is only exercised on the happy path. A targeted unit test for parseSSE edge cases would strengthen confidence. [fixable]

packages/harness/src/index.ts

Well-structured provider abstraction with solid SSE parsing and good test coverage of the happy path. Main gap is incomplete validation depth in parseSSE for message_delta/message_start usage fields, plus missing tests for thinking blocks, abort signals, and parse error paths.

• 🔵 regressions (L98): Export gap: providers/index.ts exports individual event types (MessageStartEvent, ContentBlockStartEvent, ContentBlockDeltaEvent, ContentBlockStopEvent, MessageDeltaEvent) but the main barrel src/index.ts does not re-export them. Consumers using @mitzo/harness can only access the StreamEvent union — if they need to type a handler for a specific event, they must import from the deep path. Either re-export them or explicitly omit them from providers/index.ts to signal intent. [fixable]

packages/harness/src/providers/anthropic-session.ts

Well-structured provider abstraction with solid SSE parsing and good test coverage of the happy path. Main gap is incomplete validation depth in parseSSE for message_delta/message_start usage fields, plus missing tests for thinking blocks, abort signals, and parse error paths.

• 🟡 bugs (L72): parseSSE validates message_delta by checking only parsed.delta, but the MessageDeltaEvent type also requires usage: { output_tokens: number }. If the API (or praxis-proxy) sends a message_delta without usage, the type assertion passes but downstream access to event.usage.output_tokens throws. The same applies to message_start — message.usage is required in the type but not validated. Add parsed.usage checks for consistency with the other cases. [fixable]

[dimakis]

🔵 regressions: Export gap: providers/index.ts exports individual event types (MessageStartEvent, ContentBlockStartEvent, ContentBlockDeltaEvent, ContentBlockStopEvent, MessageDeltaEvent) but the main barrel src/index.ts does not re-export them. Consumers using @mitzo/harness can only access the StreamEvent union — if they need to type a handler for a specific event, they must import from the deep path. Either re-export them or explicitly omit them from providers/index.ts to signal intent. [fixable]

[dimakis]

🟡 bugs: parseSSE validates message_delta by checking only parsed.delta, but the MessageDeltaEvent type also requires usage: { output_tokens: number }. If the API (or praxis-proxy) sends a message_delta without usage, the type assertion passes but downstream access to event.usage.output_tokens throws. The same applies to message_start — message.usage is required in the type but not validated. Add parsed.usage checks for consistency with the other cases. [fixable]