Skip to content

feat(server): introduce ServeConfig dataclass to consolidate serve() kwargs#437

Draft
bokelley wants to merge 3 commits intomainfrom
claude/issue-414-serve-config
Draft

feat(server): introduce ServeConfig dataclass to consolidate serve() kwargs#437
bokelley wants to merge 3 commits intomainfrom
claude/issue-414-serve-config

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

@bokelley bokelley commented May 3, 2026

Closes #414

Summary

  • Adds @dataclass(frozen=True) ServeConfig to src/adcp/server/serve.py, grouping all 22 serve() keyword arguments into a single named, IDE-friendly object with the same defaults
  • serve() gains config: ServeConfig | None = None; when supplied, config fields win and individual kwargs are ignored (backwards-compatible — existing call sites work unchanged)
  • __post_init__ emits UserWarning on cross-transport field combinations (A2A-only fields set alongside MCP transport and vice versa); stacklevel=3 empirically verified to point at the ServeConfig(...) call site
  • Sequence moved from TYPE_CHECKING to runtime imports (fixes get_type_hints compatibility)
  • Exported from adcp.server and added to __all__
  • examples/typed_handler_demo.py updated to demonstrate the new pattern
  • src/adcp/decisioning/serve.py docstring updated to surface config= in **serve_kwargs for agent discoverability
  • 12 new tests in tests/test_serve_config.py; all 3200 existing tests pass

What tested

  • ruff check — clean
  • mypy src/adcp/server/ — no new errors (all pre-existing from optional deps not installed locally)
  • pytest tests/test_serve_config.py — 12/12 passed
  • pytest tests/ (excluding pre-existing network flake test_real_tls_handshake_still_validates_hostname) — 3200 passed, 0 new failures

Pre-PR review

  • code-reviewer: approved — blocker addressed: unconditional test assertions replacing vacuous if call_count: guards; dead helper removed; stacklevel=3 confirmed correct via empirical test (stacklevel=4 overshoots to sys)
  • dx-expert: approved — config= added to decisioning.serve() **serve_kwargs docstring for agent discoverability; note: DX expert's stacklevel=3→4 recommendation was empirically incorrect and reverted

Known nit (not fixed): SkillMiddleware and ContextFactory are defined after ServeConfig in the file. Under from __future__ import annotations (PEP 563) this is safe at runtime since the full module loads before any caller runs. Reorganising the ~400-line SkillMiddleware docstring block is out of scope for this PR.

Triage-managed PR. This bot does not currently iterate on
review comments or PR conversation threads (only on the source
issue). To unblock:

  • Push fixup commits directly: gh pr checkout <num>
    fix → push.
  • Or re-trigger: comment /triage execute on the source
    issue.

See adcp#3121
for context.

Session: https://claude.ai/code/session_016DD1aTKm7E12tgrceKdz1M

claude added 2 commits May 3, 2026 02:56
…kwargs

Issue #414 — the 22-arg serve() signature is noisy, hard to read, and
IDE autocomplete is unwieldy. Groups all kwargs into a frozen dataclass
with sensible defaults while keeping full backwards compatibility.

Closes #414

https://claude.ai/code/session_016DD1aTKm7E12tgrceKdz1M
- stacklevel=3 → stacklevel=4 in __post_init__ (warning was pointing
  at the synthesized __init__ frame rather than the caller site)
- Remove conditional `if mock_mcp.call_count:` guards in tests —
  assertions are now unconditional so vacuous passes are impossible
- Add config= mention to decisioning.serve() **serve_kwargs docstring
  so Copilot/Cursor see the param when generating decisioning calls

https://claude.ai/code/session_016DD1aTKm7E12tgrceKdz1M
Empirically verified: stacklevel=3 points warnings.warn at the
ServeConfig(...) call site; stacklevel=4 overshoots to sys. Previous
fix commit based on DX-expert analysis was incorrect.

Also removes dead _patch_serve_internals helper from tests.

https://claude.ai/code/session_016DD1aTKm7E12tgrceKdz1M
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(server): introduce ServeConfig dataclass to consolidate serve() kwargs

2 participants