refactor: build Petstore TypeMCP HTTP agent example - #28
Conversation
sjungwon03-ai
left a comment
There was a problem hiding this comment.
Code Review — APPROVE
PR: refactor: build Petstore TypeMCP HTTP agent example
Head: bcd366f0b283b59be58a5983dd7dc1e49a9c602e
CI: verify ✅ (lint, build, 9 tests, fixtures, audit — all green)
Summary
Clean, well-scoped refactoring that replaces the mixed example catalog with a single deterministic Petstore MCP flow. The architecture is sound: loopback-only HTTP runtime → official MCP SDK client → TypeChain @Agent() façade. Proper lifecycle management, protocol validation, and security boundaries throughout.
Highlights
- Security: Runtime binds exclusively to
127.0.0.1on an ephemeral port; no public listener, credentials, or write operations. - Correctness: MCP client validates protocol results via Zod (
CallToolResultSchema), handles connection failures, and performs idempotent session cleanup. - Testing: 5 test files / 9 tests cover the full E2E MCP lifecycle deterministically (fixture data, no network). The
verify-workflow.test.tsregression guard prevents stale CI script references. - CI fix: The workflow now references the correct fixture scripts, resolving the prior
Missing scriptfailure from the earlier head. - Code quality: Clear separation of concerns (runtime/client/handler/agent/fixture), readonly types, factory-based session safety, and proper
try/finallyresource cleanup.
No blocking findings.
sjungwon03-ai
left a comment
There was a problem hiding this comment.
Independent Review — exact head 73c3428
Verdict: REQUEST_CHANGES — one blocking CI failure; architecture and boundaries verified correct.
Blocker
.github/workflows/verify.yml(unchanged by this PR) still runsnpm run example:typechain,example:policy,example:typemcp, andexample:bridge— all four npm scripts were deleted inpackage.json. TheverifyGitHub Actions check fails at exact head 73c3428 (Missing script: "example:typechain", exit 1).npm testdoes pass becausetest/verify-workflow.test.tsreads the workflow file but the file on disk at this head is still the old version. Update the workflow to runexample:petstore:mcp:fixtureandexample:petstore:mcp-agent:fixture.
Verified correct (non-blocking)
- Petstore REST: fixed host
https://petstore.swagger.io/v2, GET-only via#getJson, exactly three tools. - TypeMCP handler:
createMcpHandler+createMcpServerwith explicitresolve: () => serverper session — session-safe. - Node runtime: binds literal
127.0.0.1:0, routes only/mcp(404 otherwise), idempotent close. - MCP SDK client: official
Client+StreamableHTTPClientTransport,connect()/listTools()/callTool()only — no directserver/metadatacalls. - TypeChain facade:
@Agent+@Tooldelegates exclusively to the MCP client; usesbuildAgent, not the in-process TypeMCP adapter. - FakeToolCallingModel fixture: real agent loop through the protocol boundary;
finallycloses client then runtime. - Cleanup: all generic/in-process examples, tests, and scripts removed; README boundaries truthful.
- Threads: zero review threads, zero unresolved.
sjungwon03-ai
left a comment
There was a problem hiding this comment.
Formal Independent Review — exact HEAD bcd366f
Reviewed via GitHub diff, source at HEAD, Actions run, rulesets, and review threads. No local execution performed.
Verification summary
1. Petstore REST fixed host, GET-only, three read-only tools ✅
petstore-client.ts hardcodes https://petstore.swagger.io/v2, sends only GET with accept: application/json, validates responses with Zod, and has no write/create/update/delete, credential, or API-key path. PetstoreServer declares exactly search_available_pets, get_pet, get_petstore_inventory — all delegating to GET-only client methods.
2. Session-safe createMcpHandler / createMcpServer resolver ✅
petstore-mcp-handler.ts calls createMcpHandler(async () => { const server = serverFactory(); return createMcpServer(PetstoreServer, { resolve: () => server }); }). Each Streamable HTTP session receives a freshly compiled server via the factory closure. No global singleton.
3. Literal 127.0.0.1:0, /mcp-only routing, close lifecycle ✅
petstore-mcp-runtime.ts binds nodeServer.listen({ host: "127.0.0.1", port: 0 }), returns http://127.0.0.1:<port>/mcp, routes only /mcp (all other paths → 404), and provides an idempotent close() via a closed guard. Fixtures use nested try/finally to close client before runtime.
4. Official StreamableHTTPClientTransport / Client — real protocol flow ✅
petstore-mcp-client.ts imports Client and StreamableHTTPClientTransport from @modelcontextprotocol/sdk/client/. client.connect(transport) performs MCP initialization; listTools() and callTool() use the SDK protocol with CallToolResultSchema validation. close() calls terminateSession() (tolerating decline) then transport.close(). No direct TypeMCP metadata or Petstore class access.
5. TypeChain @agent / @tool facade — MCP client delegation only ✅
typechain-petstore-mcp-agent.ts imports Tool from @theorvane/type-chain, Agent/buildAgent from @theorvane/type-chain/agent, and PetstoreMcpClient type from the local wrapper. Each @Tool() method calls this.client.callTool(…) exclusively. No imports of PetstoreClient, PetstoreServer, TypeMCP decorators, createMcpServer, or adapters. buildPetstoreMcpAgent uses buildAgent(new PetstoreMcpAgentTools(client), { model }).
6. FakeToolCallingModel — real agent→facade→MCP protocol flow ✅
typechain-petstore-mcp-agent-fixture.ts starts a loopback runtime with fixture fetch, connects the official MCP SDK client, builds the agent with FakeToolCallingModel (selects search_available_pets({ limit: 2 })), invokes the LangChain agent loop, and asserts the ToolMessage carries Milo/Nori fixture data through the actual HTTP MCP session.
7. Generic/in-process examples removed; verify.yml runs both fixtures ✅
Ten old example files and seven old test files deleted. Repository retains only Petstore MCP-centered sources. verify.yml replaces four removed scripts with example:petstore:mcp:fixture and example:petstore:mcp-agent:fixture. A new verify-workflow.test.ts regression-guards the workflow against reintroducing removed script patterns.
8. README boundaries truthful ✅
No credentials, provider SDK, process.env, public hosting, write endpoints, or live LLM claims. Explicitly states the loopback runtime is not a production recipe; the live REST command is a GET-only smoke run outside CI; the model is application-owned.
9. Exact-head CI and governance ✅
verify completed success at bcd366f0b283 (run 30426351713, event pull_request). Zero review threads. Dev ruleset requires 1 approval + verify status check + thread resolution; all satisfied by this review.
Non-blocking observations
petstore-mcp-handler.tspasses{ resolve: () => server }directly tocreateMcpServer— CI type-checking confirms this matches the published@theorvane/type-mcp@^0.2.2API, though the design doc shows theresolver: { resolve }wrapper form.- The
transport as Parameters<typeof client.connect>[0]cast inpetstore-mcp-client.tsis a type-alignment shim with no runtime effect. - Double
CallToolResultSchemavalidation (SDK return + explicitsafeParse) is redundant but defensively safe.
Approved for squash merge into dev.
* ci: require verified dev-to-main promotions * fix: pin examples MCP SDK to audited graph (#3) Co-authored-by: sjungwon03 <> * fix(deps): adopt TypeMCP 0.2.2 remediation (#7) * chore: reconcile main release history into dev (#11) * release: publish verified TypeChain and TypeMCP examples (#1) Co-authored-by: sjungwon03 <> * release: promote audited MCP SDK graph to production (#5) * ci: require verified dev-to-main promotions * fix: pin examples MCP SDK to audited graph (#3) Co-authored-by: sjungwon03 <> --------- Co-authored-by: sjungwon03 <> * fix: retain published TypeMCP dependency contract * fix: remove obsolete reconciliation override * feat: add read-only Swagger Petstore agent examples (#16) * docs(planning): add Swagger Petstore agent plan * feat: add read-only Swagger Petstore client * feat: wrap read-only Petstore API in TypeMCP * feat: add TypeChain Petstore agent workflow * feat: add live Swagger Petstore demonstrations * feat: add real TypeChain Petstore agent factory (#22) * docs(planning): define real Petstore agent delivery * feat: add real TypeChain Petstore agent factory * docs: explain application-owned Petstore agent runtime * refactor: build Petstore TypeMCP HTTP agent example (#28) * docs: design real Petstore MCP agent example * docs(planning): detail Petstore MCP agent rewrite * feat: add loopback Petstore MCP runtime and client * feat: add TypeChain Petstore MCP agent * refactor: center examples on Petstore MCP agent * ci: verify Petstore MCP fixtures --------- Co-authored-by: sjungwon03 <>
Summary
Closes #27.
Rebuild
Theorvane/examplesaround one read-only Swagger Petstore scenario that proves a real MCP HTTP boundary before TypeChain agent use.Included flow
/mcphost, official MCP SDK client wrapper, deterministic MCP fixture command, and thePetstoreMcpAgentTools@Agent()class.tools/list, andtools/call; aFakeToolCallingModelselectssearch_available_petsand the@Tool()façade calls the connected MCP SDK client.GET-only Petstore operations and the exact three read-only tools:search_available_pets,get_pet,get_petstore_inventory.Intentional boundaries
process.envmodel configuration, live LLM command, public listener, auth, durable MCP session storage, or deployment policy.@Agent()tools use the MCP client only; they do not import/directly call the REST client, TypeMCP server, compiler, or adapter.Verification
Feature head:
73c3428d226ca5cce08eb2515928d5b856b1e224Clean clone verification:
Results: 5 test files / 9 tests passed;
npm audit --omit=dev --audit-level=lowreported 0 vulnerabilities. Both fixture commands used actual loopback Streamable HTTP MCP sessions and returned fixture pets Milo/Nori.Manual public REST smoke also passed with
npm run example:petstore:live; it is not used by CI because public demo data is mutable.