feat(protocol): read.* surface — typed RPC for history + usage_summary (Phase 2c-2)#501
Merged
Merged
Conversation
…y (Phase 2c-2) First in the daemon-as-process arc (plan: thoughts/shared/plans/ 2026-05-22-daemon-as-process-arc.md). Adds the externally-callable read half of the protocol surface so Phase 2c-4 has concrete targets when it starts rewriting MCP handler bodies to talk through ProtocolClient. - protocol/contracts.py: HistoryRequest, UsageSummaryRequest, UsageSummaryResult. HistoryResponse stays in contracts.py (shared wire shape between MCP and daemon per the "flat envelopes" preflight constraint). - protocol/handlers/reads.py: server-side dispatchers that validate payloads via Pydantic, build a BicameralContext via a single-repo shim (multi-repo lands in 2c-3), and delegate to the existing handlers.history / handlers.usage_summary code. No MCP handler bodies are modified. - tests/test_protocol_read_conformance.py: 6 tests — dispatch shape for both methods, Pydantic input validation, idempotent registration, + two constraint tests encoding bicameral preflight findings (no LLM imports in read.*, ProtocolServer.__init__ stays connection-free). In-process tests only; the per-test daemon-subprocess fixture lands in 2c-4 alongside the first call-site migration (per Fowler's "build the test fixture when you migrate the first call site" advisory). Plan: thoughts/shared/plans/2026-05-22-2c-2-read-surface.md Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced May 23, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
First in the daemon-as-process arc — full plan in
thoughts/shared/plans/2026-05-22-daemon-as-process-arc.md. The arc commits to: daemon is a real process (not a wheel MCP imports); IPC is the only path; tests that verify the boundary spin up a real daemon subprocess.This PR adds the externally-callable read half of the protocol surface so Phase 2c-4 (the load-bearing first-call-site-migration PR) has concrete RPC targets when it starts rewiring MCP handler bodies through
ProtocolClient.What changed
protocol/contracts.py— new Pydantic models:HistoryRequest,UsageSummaryRequest,UsageSummaryResult.HistoryResponsestays incontracts.py(shared wire shape between MCP and daemon per the "flat envelopes" preflight constraint).protocol/handlers/reads.py— server-side dispatchers. Validate payloads, build aBicameralContextvia a single-repo shim (multi-repo lands in 2c-3), delegate to today'shandlers.history/handlers.usage_summarycode. No MCP handler bodies are modified by this PR.tests/test_protocol_read_conformance.py— 6 tests. Dispatch shape + Pydantic input validation + idempotent registration + two constraint tests encoding bicameral-preflight findings:test_read_handlers_have_no_llm_imports— pins the "deterministic read path" constraint (decision:wndwxgam2m8yjor0igya).test_protocol_server_init_does_not_open_db_connection— pins the "lazy ledger connect" constraint (decision:k44cko8xtkcswk55kytz).Test strategy (per Fowler advisory, 2026-05-22)
In-process loopback only in this PR. The per-test daemon-subprocess fixture lands in 2c-4 alongside the first real call-site migration — build the fixture when there's a call site to test through it, not before.
Compatibility
Pure-additive. MCP server unchanged. End-user MCP tool names (
bicameral.history,bicameral.usage_summary) keep working through the today's direct-import path. Nothing in this PR routes throughProtocolClientyet.Acceptance
pytest tests/test_protocol_read_conformance.py— 6 passingpytest tests/test_protocol_categorization.py+ other protocol tests — still green (no regressions)ruff check+ruff format --check— both cleanmypy protocol/— cleanPlan refs
thoughts/shared/plans/2026-05-22-daemon-as-process-arc.mdthoughts/shared/plans/2026-05-22-2c-2-read-surface.md🤖 Generated with Claude Code