Skip to content

test: live e2e suite against the real OpenRouter API + CI job - #21

Merged
LukasParke merged 2 commits into
mainfrom
ci/live-e2e
Jul 29, 2026
Merged

test: live e2e suite against the real OpenRouter API + CI job#21
LukasParke merged 2 commits into
mainfrom
ci/live-e2e

Conversation

@LukasParke

Copy link
Copy Markdown
Contributor

Adds real end-to-end CI testing: five live tests against the OpenRouter API mirroring upstream's packages/agent/tests/e2e coverage — text/stream agreement, a real tool round, approval pause/resume across two call_model invocations, lifecycle hooks on live traffic (asserting SessionEnd usage totals), and conversation-state serialize→deserialize round-trip resuming a live paused run.

The live suite immediately caught two real port bugs the mocked unit tests couldn't see, fixed in this PR:

  1. Every follow-up tool round failed SDK request validation. Internal items use upstream's camelCase callId but the generated SDK validates call_id, and response items echoed back from a live turn are SDK pydantic models where the validator wants plain dicts. _send now normalizes both at the transport boundary only — internal state/stream events keep the upstream shape. One unit test pinned the broken wire spelling and was updated.
  2. serialize_conversation_state raised TypeError on live states holding SDK pydantic items (OutputFunctionCallItem is not JSON serializable); now dumps them to plain dicts.

Test-design notes: approval tests pin the first turn with tool_choice="required" so model nondeterminism can't skip the pause being asserted; the plain tool-loop test deliberately doesn't (required persists across turns, matching upstream, and would trip the 20-turn safety limit). Model defaults to a small/cheap one, overridable via OPENROUTER_E2E_MODEL. Suite skips entirely without OPENROUTER_API_KEY.

CI: new e2e job — warns and exits 0 when the secret is missing (fork PRs), same pattern as upstream typescript-agent. Costs a few cents per run.

Verified: full e2e suite passed 3× consecutively live; unit suite, ruff, mypy green.

🤖 Generated with Claude Code

Replaces the single smoke test with five e2e tests mirroring upstream's
packages/agent/tests/e2e coverage: text/stream agreement, a real tool
round, approval pause/resume across two call_model invocations,
lifecycle hooks firing on live traffic (with SessionEnd usage totals),
and conversation-state serialize/deserialize round-trip resuming a live
paused run. Skipped without OPENROUTER_API_KEY; model pinned to a small
default, overridable via OPENROUTER_E2E_MODEL.

Running these live immediately caught two real port bugs the mocked
unit suite could not see, both fixed here:

- model_result._send now normalizes input items at the transport
  boundary: response items echoed back from a live turn are SDK pydantic
  models (the request validator wants plain dicts), and internal items
  use upstream's camelCase callId while the generated SDK validates
  snake_case call_id. Every follow-up tool round against the live API
  failed validation before this; mocked clients accepted anything.
  (test_tool_terminal_empty_final updated: it pinned the pre-fix wire
  spelling.)

- serialize_conversation_state now dumps SDK pydantic items to plain
  dicts (json.dumps default=dump); live states holding
  OutputFunctionCallItem raised TypeError before.

The approval tests pin the tool call with tool_choice="required" so
model nondeterminism can't skip the pause; the plain tool-loop test
deliberately does not (required persists across turns, matching
upstream, and would trip the 20-turn safety limit).

CI: new e2e job — warns and exits 0 when OPENROUTER_API_KEY is missing
(forks), same pattern as upstream typescript-agent.

Verified: full e2e suite passed 3x consecutively live; unit suite,
ruff, mypy all green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@perry-the-pr-reviewer perry-the-pr-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perry's Review

Verdict: 💬 Comments / questions
Risk: 🟢 Low

Details

This PR adds a live e2e test suite against the real OpenRouter API (mirroring upstream typescript-agent's packages/agent/tests/e2e coverage) and fixes two real bugs the new suite caught:

  1. _send transport normalization (model_result.py): Response items echoed back from a live turn are SDK pydantic models, and internal items use upstream's camelCase callId while the Python SDK validates call_id. The fix normalizes both at the transport boundary only — internal state and stream events keep the upstream shapes. This is the right place for the conversion: it's contained, doesn't leak into state serialization or event handling, and the shallow callIdcall_id conversion is sufficient since callId only appears at the top level of input items.

  2. serialize_conversation_state with default=dump (conversation_state.py): States built from live responses hold SDK pydantic items (e.g. OutputFunctionCallItem) that dataclasses.asdict passes through untouched, causing TypeError on json.dumps. Using dump as the default fallback is consistent with how the existing json_dumps helper already handles pydantic models, and the exclude_none=True from model_dump is safe for the round-trip since deserialize_conversation_state uses .get() with None defaults.

Test design is thoughtful: approval tests pin the first turn with tool_choice="required" to avoid model nondeterminism skipping the pause; the plain tool-loop test deliberately doesn't (since required persists and would trip the 20-turn safety limit). The MemoryState helper and _client factory are clean.

CI: The e2e job follows the upstream pattern (warn + exit 0 when OPENROUTER_API_KEY is missing), so fork PRs and local dev aren't blocked. All three CI checks are green (check, e2e, verify-port).

One suggestion (inline): the default=dump serialization fix is only exercised by the e2e test, which doesn't run without an API key (fork PRs, local dev, most CI environments). A unit test that puts a pydantic model into ConversationState.messages and asserts serialize_conversation_state produces valid JSON would guard against regressions in the unit test job.

  • Unit tests (103), ruff, and CI all green.
  • No security concerns — no hardcoded secrets, API key injected via GitHub secrets.
  • Blast radius is minimal: _send is only called from _send_and_track, and the normalization doesn't touch internal state.

Comment thread src/openrouter_agent/conversation_state.py
Review suggestion on #21: the pydantic-item serialization fix was only
exercised by the credit-gated e2e suite. This deterministic unit test
puts a pydantic model into state.messages and asserts valid JSON out,
so the regression is caught by the check job on every PR.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@LukasParke
LukasParke merged commit 52b4acf into main Jul 29, 2026
3 checks passed
@LukasParke
LukasParke deleted the ci/live-e2e branch July 29, 2026 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant