Repository navigation
feat(helius-mcp): Add an Optional Per-Request Context - #182
Conversation
Credential and network state is module-global: one process, one caller. That holds for stdio and breaks for anything serving concurrent callers, where a key set by one request answers for every other. This adds the type that will carry per-caller state and threads it into the resolvers as an *optional* parameter. Nothing is converted and nothing is deleted, so behaviour is unchanged: with no context supplied the existing module state answers exactly as before, in exactly the same order. The point of the optional parameter is bisectability. Converting 57 call sites in one change is unreviewable and unbisectable; with the fallback in place each subsequent migration is individually safe, because a partly migrated tree still passes the full suite. The globals come out once every call site is converted. `RequestContext` carries the caller's key, their project when the transport can resolve it, the network, and an opaque session key. That last one buckets `expandResult` handles, so it is stable per caller rather than per request — a handle is issued by one request and redeemed by the next. It is a hash, so the raw key is not also a map key. `contextFromExtra` reads the SDK's `extra.authInfo`, which already reaches every one of the 95 handlers and is currently read by none of them. It returns null when a request carries no identity; that is the stdio case and not an error. Placed in `utils/` rather than `router/` as originally sketched: `utils/` never imports from `router/`, and `utils/helius.ts` is the main consumer. A caller-supplied context gets its own Helius client rather than the module cache. Caching per key would be a cross-tenant hazard for no gain — construction measures ~0.012ms, far below the network call it wraps. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review found the context half-applied in getEnhancedWebSocketUrl: with a context and a shared credential, the network came from the request while the key was overwritten with the placeholder — `atlas-devnet` proving one field won and the other lost in the same expression. A supplied context means the key belongs to the caller being answered, so returning it is what the tool is for. The placeholder exists for the opposite case, where the key is the deployment's. Context is now checked first. This mattered to settle here rather than later: the precedence would otherwise be copied across every converted call site. Also from review: - `AuthInfoLike` was dead. After the `isRecord` guard the cast narrowed nothing; deleting it type-checks clean. Removed. - `projectId` now reads from `authInfo.extra`, not `clientId`. `clientId` identifies an OAuth client application and a Helius project is not one, so under any registration model that field would hold something else. Both of our values now sit under `extra`, which the SDK documents as the home for additional token data. Two findings deliberately left for the migration that removes the globals, each now documented where it lives: - `redact.ts`'s secret registry is unbounded and process-global. Capping it in isolation would silently stop scrubbing an evicted key, which is worse than the growth. - `RequestContext.sessionKey` duplicates a responsibility `RouterContext` holds today. Nothing can bridge them until call sites pass a context. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| // the network call it wraps. | ||
| if (ctx) { | ||
| return wrapClientWithResilience( | ||
| createHelius({ apiKey: getApiKey(ctx), userAgent: MCP_USER_AGENT }), |
There was a problem hiding this comment.
createHelius accepts network: 'mainnet' | 'devnet' and derives the RPC host from it, but this only passes the key. A devnet context gets devnet WebSocket and LaserStream URLs from the two helpers below, while every RPC read and Sender submission through this client goes to mainnet
We should pass network: getNetwork(ctx) === 'devnet' ? 'devnet' : 'mainnet' here. This is also the root of the address/cluster mismatch in #187
| // shared-credential placeholder below. Getting this order wrong half-applies | ||
| // the context: the network comes from the request while the key does not. | ||
| if (ctx) { | ||
| return `${host}/?api-key=${getApiKey(ctx)}`; |
There was a problem hiding this comment.
This branch never reaches the caller intact. Every mcpText / mcpError runs redactSecrets, whose API_KEY_QUERY_RE rewrites any api-key= value in shared-credential mode, and getApiKey(ctx) above registers the caller's key as a secret. The caller gets ?api-key=***REDACTED***, which is worse than the YOUR_HELIUS_API_KEY placeholder on line 127. The test asserts the resolver's return, and not the response path
We should either exempt the caller's own key from redaction whena context is present, or keep the placeholder
| export function getApiKey(): string { | ||
| const apiKey = sessionApiKey || process.env.HELIUS_API_KEY || getSharedApiKey(); | ||
| export function getApiKey(ctx?: RequestContext): string { | ||
| const apiKey = ctx?.apiKey || sessionApiKey || process.env.HELIUS_API_KEY || getSharedApiKey(); |
There was a problem hiding this comment.
ctx?.apiKey || ... means a supplied context with an empty key silently falls through to the deployment's credential. getNetwork(ctx) treats a supplied context as authoritative. We should do the same here: if ctx is present, resolve only from it and throw NO_API_KEY when it's empty. We should also do the same on line 70
| return null; | ||
| } | ||
|
|
||
| const apiKey = typeof authInfo.token === 'string' ? authInfo.token : ''; |
There was a problem hiding this comment.
Using the bearer token verbatim as the Helius key couples the transport credential to the billing credential. The moment the hosted entrypoint fronts this with real OAuth, every access token gets appended as ?api-key= to Helius hosts. We should read the key from authInfo.extra where projectId and network already live, with token as an explicit opt-in fallback
| const rawProjectId = authExtra?.projectId; | ||
| const projectId = typeof rawProjectId === 'string' && rawProjectId ? rawProjectId : undefined; | ||
|
|
||
| const network = authExtra?.network === 'devnet' ? 'devnet' : 'mainnet-beta'; |
There was a problem hiding this comment.
Anything other than the exact string 'devnet' coerces to mainnet with no error, and the test locks in 'nonsense' to mainnet. We should reject unknown values as 'Devnet' or 'testnet' routing a caller's writes to mainnet is not a safe default
| export function getLaserstreamUrl(region?: 'ewr' | 'pitt' | 'slc' | 'lax' | 'lon' | 'ams' | 'fra' | 'tyo' | 'sgp'): string { | ||
| export function getLaserstreamUrl( | ||
| region?: 'ewr' | 'pitt' | 'slc' | 'lax' | 'lon' | 'ams' | 'fra' | 'tyo' | 'sgp', | ||
| ctx?: RequestContext, |
There was a problem hiding this comment.
ctx after existing optional params forces #185 to write restRequest(path, {}, ctx) and getLaserstreamUrl(undefined, ctx)everywhere.ctx`-first or an options object makes the migration more mechanical and greppable
Six findings from review, three of them real bugs. **The client ignored the network.** `createHelius` accepts `network` and derives its RPC host from it; we passed only the key. A devnet context got devnet WebSocket and LaserStream URLs from the helpers while every RPC read and Sender submission went to mainnet. Verified at the wire: a devnet context now reaches `devnet.helius-rpc.com`, where it previously reached `mainnet.helius-rpc.com`. This is also the root of the address/cluster mismatch flagged downstream. **An empty context key fell through to the deployment's.** `ctx?.apiKey || ...` meant a context that failed to carry a key silently billed the operator. `getNetwork` already treated a supplied context as authoritative; `getApiKey` and `hasApiKey` now do the same and throw `NO_API_KEY` instead. **The transport token was used verbatim as the Helius key.** That couples the credential that authenticates the request to the one that gets billed, and the moment a hosted entrypoint fronts this with OAuth, every access token would be appended as `?api-key=` to a Helius host. The key now comes from `authInfo.extra.apiKey`, alongside `projectId` and `network`, with `token` as an explicit fallback for a transport that authenticates with the Helius key itself. Also: - An unknown network no longer coerces to mainnet. `'Devnet'` or `'testnet'` routing a caller's writes to mainnet is not a safe default, so `contextFromExtra` throws `INVALID_NETWORK`. - `network` is optional on the context. Absent means the request did not state one, which is not the same as mainnet: a self-hosted server with `HELIUS_NETWORK=devnet` and a middleware that sets a token but no network must stay on devnet, so the resolver falls through to env and session. - `getLaserstreamUrl` takes the context first, ahead of the optional region. On the Enhanced WebSocket URL, review is right that the context branch reaches the caller as `***REDACTED***`, but the premise that this is worse than the placeholder does not hold — the placeholder was equally clobbered, on `main`, before any of this. `API_KEY_QUERY_RE` rewrites any `api-key=` value in shared-credential mode, including `YOUR_HELIUS_API_KEY`, so that tool has been returning an opaque string rather than an instruction. Both are fixed: the scrubber now leaves the placeholder alone, and the placeholder wins over a context in shared-credential mode. Those two settings are contradictory — one says every caller shares the deployment key, the other says this caller brought their own — and an unusable URL that says what to substitute beats one that does not. They become orthogonal when hosted mode is split out, and a bring-your-own-key deployment will run with redaction off. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Credential and network state is module-global: one process, one caller. That holds for stdio and breaks for anything serving concurrent callers, where a key set by one request answers for every other.
This PR adds the type that will carry per-caller state and threads it into the resolvers as an optional parameter. Note that no call site is converted and no global is deleted — with no context supplied the existing module state answers exactly as before, in exactly the same order.
Why optional
Converting all 57 inline call sites in one change is unreviewable and unbisectable. With the fallback in place, each subsequent migration is individually safe: a partly migrated tree still passes the full suite, because unconverted call sites keep reading the globals. The globals come out in a later PR, once every call site is converted.
Expand → migrate → contract. This is the expand.
What's here
src/utils/request-context.ts—RequestContextpluscontextFromExtractx?on the resolvers —getApiKey,hasApiKey,getHeliusClient,getNetwork,getEnhancedWebSocketUrl,getLaserstreamUrl,restRequestDesign notes
contextFromExtrareadsextra.authInfo. The SDK's per-requestextraalready reaches all 95 handlers and is currently read by none of them, so the plumbing from transport to tool exists — it just carries nothing.AuthInfois{ token, clientId, scopes[], extra? }, andextrais an open record, so the network rides there. Returnsnullwhen a request carries no identity; that is the stdio case and not an error.sessionKeyis stable per caller, not per request. It bucketsexpandResulthandles, and a handle is issued by one request and redeemed by the next — per-request would break expansion immediately. It is a SHA-256 prefix, so the raw key is not also a map key.Placed in
utils/, notrouter/.utils/never imports fromrouter/, andutils/helius.tsis the main consumer; putting it inrouter/would invert the dependency.A supplied context gets its own Helius client rather than the module cache. Caching per key would be a cross-tenant hazard for no gain — construction measures ~0.012ms, far below the network call it wraps.
Context wins where supplied, including over
HELIUS_NETWORK. Without one, the existing env-then-session order is untouched.Verification
pnpm buildclean,pnpm validatepassesContext
Second PR of the hosted-MCP work, after #181. Nothing consumes
contextFromExtrayet; the call-site migrations follow.🤖 Generated with Claude Code