feat(mcp-server): wire @gemini-markets/sdk client into mcp-server (PREDICT-8816) - #63
Conversation
…EDICT-8816) Adds the SDK dependency and a shared createSdkClient() factory, threaded through both process bootstraps (the MCP server and the alerts daemon) unused for now so every sibling migration ticket (PREDICT-8817-8820, 8823) can start consuming it without redoing this wiring. Pure plumbing — no tool behavior change. Also fixes a pre-existing broken import path in market-data.test.ts left over from the PREDICT-8815 file split, which was blocking npm run typecheck.
Nostradamus Risk Rating — LowThis PR introduces a |
svc-grace
left a comment
svc-grace
left a comment
There was a problem hiding this comment.
Agentic Review
The wiring is straightforward, but there are two things to fix before this can move on: invalid GEMINI_SDK_ENV values currently fall through to production, and the test leaves a live WebSocket with an unhandled subscription rejection. The sandbox test also could be stronger about proving the selected environment, but that’s a follow-up rather than a gate; the inline threads have the specifics.
Review process
Grace version: v0.0.210
Files reviewed (11): config.ts, index.ts, market-data.test.ts, package-lock.json, package.json, sdk.test.ts, sdk.ts, server.test.ts, server.ts, smoke-sdk.ts
Out-of-diff findings contained: 1 (rendered above as follow-up candidates; they do not gate this review)
Guidelines: none discovered
Verification: 4 of 8 findings verified
Findings: 0 critical, 2 important, 1 suggestion
Linked tickets: PREDICT-8816
LLM usage: 156 calls — gpt-5.6-luna: 113 calls, 3433621 tokens, us.anthropic.claude-opus-4-6-v1: 43 calls, 864971 tokens
Change Control Evidence CheckAuthorization — ✅ PassEvidence:
Testing — ✅ PassEvidence:
Approval — ✅ PassEvidence:
Segregation of Duties — ✅ PassValidated:
Last checked: 2026-09-18 18:49 UTC ↩ Re-run Clarissa if tickets, CI, or approvals have changed. |
- config.ts: fail startup on an invalid GEMINI_SDK_ENV instead of silently falling back to production (a typo like "sandbx" would otherwise send authenticated calls to live markets). - client/sdk.ts: add an optional test-only overrides param to createSdkClient so tests can inject a fake fetch instead of hitting the network. - client/sdk.test.ts: stop invoking client.websocket.private.orders() in the authenticated-client test — it opened a real WebSocket connection to production with fake credentials that never resolved or closed, leaking an unhandled rejection and live reconnect timers (this alone was adding ~70s to every full `npm test` run). The sandbox-selection test now injects a fake fetch and asserts the request actually targeted api.sandbox.gemini.com, instead of only checking that .predictions is truthy.
svc-grace
left a comment
svc-grace
left a comment
There was a problem hiding this comment.
Agentic Review
Good progress — the environment-selection issue and the WebSocket/rejection leak are fixed, and invalid SDK environments now fail safely instead of silently choosing production. One follow-up remains in the test coverage: the authenticated path still doesn't verify that HMAC auth is actually wired. That's a non-blocking suggestion; the inline comment has the details.
🔭 Follow-up candidates (outside this PR's diff)
Pre-existing observations in code this PR does not change — nothing here gates this review; consider a follow-up ticket.
- important — Sandbox SDK environment is lost when the alerts daemon is supervised (
packages/mcp-server/src/alerts/supervisor/types.ts): The newconfig.sdkEnvreadsGEMINI_SDK_ENV, but the alerts supervisor whitelist does not pass that variable into daemon units. When the daemon is installed withGEMINI_SDK_ENV=sandbox,pickWhitelistedEnv()omits it, so the daemon defaults to production while the MCP server uses sandbox; later SDK calls then hit live markets. AddGEMINI_SDK_ENVtoENV_WHITELIST.
Review process
Grace version: v0.0.210
Files reviewed (3): config.ts, sdk.test.ts, sdk.ts
Files skipped: 16
Out-of-diff findings contained: 1 (rendered above as follow-up candidates; they do not gate this review)
Guidelines: none discovered
Verification: 2 of 2 findings verified
Findings: 0 critical, 0 important, 1 suggestion
Linked tickets: PREDICT-8816
LLM usage: 63 calls — gpt-5.6-luna: 56 calls, 2259839 tokens, us.anthropic.claude-opus-4-6-v1: 7 calls, 99741 tokens
…low-up (PREDICT-8816) The previous fix for the leaked-WebSocket finding replaced the risky client.websocket.private.orders() call with a check that it's merely a function — true regardless of whether createSdkClient actually passed auth into createClient. Exercise a real authenticated REST call instead (via the same injected-fetch seam used for the sandbox test) and assert the request carried the expected X-GEMINI-APIKEY/X-GEMINI-SIGNATURE headers. Verified this catches the regression by temporarily dropping the auth arg from createSdkClient and confirming the test fails.
svc-grace
left a comment
svc-grace
left a comment
There was a problem hiding this comment.
Agentic Review
Good progress — all four prior concerns are resolved: the sandbox and authenticated tests now prove the intended wiring, the WebSocket test cleans up correctly, and invalid SDK environments no longer fall back to production. This pass found no new issues. The change stays focused on introducing and threading the configured SDK client through both bootstraps, with the main thing to preserve being the separation between the new SDK environment setting and the legacy client URLs.
Review process
Grace version: v0.0.210
Files reviewed (1): sdk.test.ts
Files skipped: 20
Suggestions damped: 1 (pass 3 with nothing blocking — new suggestion-level findings are withheld so the review converges, TOOLS-6878)
Guidelines: none discovered
Verification: 0 of 1 findings verified
Findings: 0 critical, 0 important, 0 suggestions
Linked tickets: PREDICT-8816
LLM usage: 35 calls — gpt-5.6-luna: 35 calls, 991144 tokens
kevinnguy
left a comment
kevinnguy
left a comment
There was a problem hiding this comment.
Before merging this in, test an authenticated call with the SDK. This integrates the SDK but we need to make sure it's done correctly by testing a call
Fixed HmacAuth's nonce mode for REST calls: it defaulted to "monotonic", which sends the nonce in milliseconds, but Gemini's REST API expects epoch seconds. This caused every authenticated call to fail with InvalidNonce (HTTP 400). createSdkClient now passes nonceMode: 'time-based' to match. Also updated the smoke script to target whichever environment is already configured (GEMINI_SDK_ENV, default production) instead of hardcoding sandbox, and to fail loudly rather than silently skip the authenticated call when credentials aren't set. Verified with a real authenticated call (predictions.getPositions) against production before and after the fix.
svc-grace
left a comment
svc-grace
left a comment
There was a problem hiding this comment.
Agentic Review
Good progress since the last pass: the environment-selection test, WebSocket leak, HMAC wiring coverage, and invalid-environment handling are all resolved. There’s one new correctness issue to address before this can move on: the smoke check can crash when positions include bigint IDs. The inline comment has the details.
Review process
Grace version: v0.0.210
Files reviewed (2): sdk.ts, smoke-sdk.ts
Files skipped: 18
Guidelines: none discovered
Verification: 1 of 3 findings verified
Findings: 0 critical, 1 important, 0 suggestions
Linked tickets: PREDICT-8816
LLM usage: 60 calls — gpt-5.6-luna: 41 calls, 1894301 tokens, us.anthropic.claude-opus-4-6-v1: 19 calls, 612391 tokens
getPositions() returns instrumentId as a bigint, which plain JSON.stringify cannot serialize — it throws instead of just losing precision. Any account with an open position would crash the script right after the authenticated call succeeded. Added a stringify helper that converts bigint values to strings before logging, used everywhere a response gets logged.
svc-grace
left a comment
svc-grace
left a comment
There was a problem hiding this comment.
Agentic Review
Good progress since the last pass — all five carried-over issues are resolved, including the SDK environment validation and the smoke/test coverage gaps. This pass found no new concerns. The change cleanly wires one configured SDK client through both bootstraps while preserving the legacy clients and behavior; the key thing to keep intact is that environment selection and optional HMAC authentication remain independent and correctly validated.
Review process
Grace version: v0.0.210
Files reviewed (1): smoke-sdk.ts
Files skipped: 20
Suggestions damped: 1 (pass 5 with nothing blocking — new suggestion-level findings are withheld so the review converges, TOOLS-6878)
Guidelines: none discovered
Verification: 0 of 1 findings verified
Findings: 0 critical, 0 important, 0 suggestions
Linked tickets: PREDICT-8816
LLM usage: 22 calls — gpt-5.6-luna: 21 calls, 618606 tokens, us.anthropic.claude-sonnet-4-6: 1 call, 17703 tokens
3 participants
Whats included
@gemini-markets/sdkas a dependency ofpackages/mcp-serverand a sharedcreateSdkClient()factory (src/client/sdk.ts) that constructs one configuredGeminiMarketsclient — HMAC auth when credentials are present, omitted otherwise (public-only mode, matching today's behavior).GeminiHttpClient/WebSocketManager— the MCP server (server.ts/index.ts) and the separategemini-mcp-alertsdaemon (alerts/daemon/index.ts) — so neither entry point is left behind for the sibling migration tickets.GEMINI_SDK_ENVenv var (sandbox|production, defaultproduction) selects the SDK client's environment, fully decoupled from the legacyGEMINI_API_BASE_URL/GEMINI_WS_URL, which keep governing the legacy client unchanged for every tool not yet migrated.sdkClient.datasources/predictions/market-data.test.tshad a broken import path left over from the PREDICT-8815 file-split PR (../client/http.js/./predictions.jsinstead of../../client/http.js/./market-data.js), which was failingnpm run typecheckonmain. Unrelated to this ticket but fixed here since it blocked verifying this change.Linear: https://linear.app/gemini/issue/PREDICT-8816/wire-gemini-marketssdk-client-into-mcp-server
Test plan
npm run build— cleannpm run typecheck— cleannpm test— 289/289 passing (all pre-existing tests + newclient/sdk.test.tsandserver.test.ts)tools/liststill returns the same 88 tools (19 prediction tools) with no behavior changecreateSdkClient()callnpm run smoke:sdkagainst production with real credentials — a public call (getCategories, just checks the connection works) and a read-only authenticated call (getPositions, checks our API key/secret are actually signed correctly and accepted by Gemini's server. Both succeeded:{"positions":[],"total":0}, i.e. no open positions) confirms the SDK is signing requests correctly and Gemini's server is accepting them.