Conversation
|
🧪 Testing To try out this version of the SDK: Expires at: Mon, 13 Apr 2026 16:09:29 GMT |
stainless-app
bot
force-pushed
the
release-please--branches--main--changes--next--components--hyperspell
branch
from
c7e6122 to
2505acc
Compare
![entelligence-ai-pr-reviews[bot]](https://avatars.githubusercontent.com/in/932812?s=60&v=4){kind=small}
| ...req, | ||
| headers: redactHeaders(req.raw.headers), | ||
| }; | ||
| }), | ||
| res: pino.stdSerializers.wrapResponseSerializer((res) => { | ||
| return { | ||
| ...res, | ||
| headers: redactHeaders(res.headers), | ||
| }; |
There was a problem hiding this comment.
Correctness: req.raw is undefined on the serialized request object, so req.raw.headers will throw a TypeError at runtime. Additionally, res.headers is not a property of the serialized response object. Use req.headers (which is already available) and access the raw response object to call getHeaders() for response redaction. 🛡️
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In `packages/mcp-server/src/http.ts` around the pino serializers, replace direct access to `req.raw.headers` and `res.headers` with safe fallbacks (e.g., `req.raw?.headers ?? req.headers ?? {}` and `res.getHeaders?.() ?? res.headers ?? {}`) so `redactHeaders` never receives `undefined`.
2505acc to
6435522
Compare
…e code tool calls
stainless-app
bot
force-pushed
the
release-please--branches--main--changes--next--components--hyperspell
branch
from
6435522 to
81ac8ff
Compare
stainless-app
bot
force-pushed
the
release-please--branches--main--changes--next--components--hyperspell
branch
from
81ac8ff to
0903fca
Compare
| query: body.query, | ||
| status: result.status, | ||
| statusText: result.statusText, | ||
| errorText, | ||
| }, | ||
| 'Got error response from docs search tool', | ||
| ); | ||
|
|
||
| if (result.status === 404 && !reqContext.stainlessApiKey) { | ||
| throw new Error( | ||
| 'Could not find docs for this project. You may need to provide a Stainless API key via the STAINLESS_API_KEY environment variable, the --stainless-api-key flag, or the x-stainless-api-key HTTP header.', | ||
| ); | ||
| } | ||
|
|
||
| throw new Error( | ||
| `${result.status}: ${result.statusText} when using doc search tool. Details: ${await result.text()}`, | ||
| `${result.status}: ${result.statusText} when using doc search tool. Details: ${errorText}`, | ||
| ); | ||
| } | ||
|
|
||
| return asTextContentResult(await result.json()); | ||
| const resultBody = await result.json(); | ||
| logger.info( | ||
| { | ||
| durationMs: Date.now() - startTime, | ||
| query: body.query, |
There was a problem hiding this comment.
Correctness: 🚫 args can be undefined per the handler signature, so body.query will throw during logging. Use optional chaining or a default to avoid crashing the handler just for telemetry.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
File: packages/mcp-server/src/docs-search-tool.ts. Lines 69-92. The new logging accesses body.query, but args can be undefined, causing a runtime error. Update both logger.warn and logger.info payloads to use optional chaining (body?.query) or a safe default so logging does not throw when args is missing.
| import { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp.js'; | ||
| import { ClientOptions } from 'hyperspell'; | ||
| import express from 'express'; | ||
| import morgan from 'morgan'; | ||
| import morganBody from 'morgan-body'; | ||
| import pino from 'pino'; | ||
| import pinoHttp from 'pino-http'; | ||
| import { getStainlessApiKey, parseClientAuthHeaders } from './auth'; | ||
| import { getLogger } from './logger'; | ||
| import { McpOptions } from './options'; | ||
| import { initMcpServer, newMcpServer } from './server'; | ||
|
|
There was a problem hiding this comment.
Correctness: The introduction of getLogger() in streamableHTTPApp and launchStreamableHTTPServer will cause a runtime crash. The getLogger() implementation in logger.ts is designed to throw an error if _logger is not initialized via configureLogger(). Since launchStreamableHTTPServer does not invoke configureLogger() before calling streamableHTTPApp, the server will fail to start. You must ensure the logger is configured at the start of the server lifecycle or provide a default logger instance.
stainless-app
bot
force-pushed
the
release-please--branches--main--changes--next--components--hyperspell
branch
from
0903fca to
23dbf39
Compare
stainless-app
bot
force-pushed
the
release-please--branches--main--changes--next--components--hyperspell
branch
from
23dbf39 to
423c7ed
Compare
| }); | ||
|
|
||
| const logger = getLogger(); | ||
|
|
There was a problem hiding this comment.
Correctness: Calling getLogger() introduces a reliability regression. Since getLogger() throws an error if the logger is not configured, this handler will now crash in environments where logging is not initialized, whereas it previously functioned correctly. Guard the logger call or provide a fallback to ensure the tool remains functional regardless of the logging state.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In packages/mcp-server/src/docs-search-tool.ts around the new logging additions (line ~63), guard getLogger() so the handler doesn’t throw if logging isn’t configured. Wrap getLogger() in try/catch and make logging calls optional (logger?.info/warn) or ensure configuration occurs before this handler runs.
09bff4e to
0fa259b
Compare
| // just do what it says, but otherwise calculate a default | ||
| if (!(timeoutMillis && 0 <= timeoutMillis && timeoutMillis < 60 * 1000)) { | ||
| // If the API asks us to wait a certain amount of time, just do what it | ||
| // says, but otherwise calculate a default |
There was a problem hiding this comment.
Correctness: retry-after values will now be used directly, causing immediate retry loops or multi-hour sleeps (DoS on the client). Restore bounds checking so only reasonable delays are accepted and fallback is used otherwise. ⏱️
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In `src/client.ts` around the retry delay logic (line ~640), restore validation of `timeoutMillis` so negative or excessively large values from Retry-After headers do not cause immediate loops or long sleeps. Reintroduce the bounds check (e.g., 0 <= timeoutMillis < 60s) and fall back to `calculateDefaultRetryTimeoutMillis` when out of range.
| const app = streamableHTTPApp({ mcpOptions, debug }); | ||
| const app = streamableHTTPApp({ mcpOptions }); | ||
| const server = app.listen(port); | ||
| const address = server.address(); |
There was a problem hiding this comment.
Correctness: Using getLogger() here can now throw if configureLogger() hasn’t been called, which is a behavior change from the previous console fallback. This makes launchStreamableHTTPServer crash for consumers that relied on the old behavior. Consider falling back to console when the logger isn’t configured. 🚨
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In `packages/mcp-server/src/http.ts` at the logger initialization inside `launchStreamableHTTPServer`, avoid throwing when `configureLogger()` hasn’t been called. Wrap `getLogger()` in a try/catch and fall back to `console` so existing callers don’t crash. Apply the suggested diff exactly.
| // says, but otherwise calculate a default | ||
| if (timeoutMillis === undefined) { | ||
| const maxRetries = options.maxRetries ?? this.maxRetries; | ||
| timeoutMillis = this.calculateDefaultRetryTimeoutMillis(retriesRemaining, maxRetries); |
There was a problem hiding this comment.
Correctness: retry-after values, so a negative or extremely large header now gets used directly, causing immediate retries or multi‑minute/hour stalls. Reintroduce a bounded check so only reasonable delays are honored and fallback to default otherwise.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In `src/client.ts` around the retryRequest logic (near line 643), reintroduce validation for `timeoutMillis` so negative or overly large retry-after values don't get used directly. Update the condition to fallback to the default when `timeoutMillis` is undefined, < 0, or >= 60*1000, while still allowing 0 if desired.
0fa259b to
1c23c70
Compare
| if (!(timeoutMillis && 0 <= timeoutMillis && timeoutMillis < 60 * 1000)) { | ||
| // If the API asks us to wait a certain amount of time, just do what it | ||
| // says, but otherwise calculate a default | ||
| if (timeoutMillis === undefined) { |
There was a problem hiding this comment.
Correctness: retry-after values (e.g., past date producing a negative delta) now get used directly, leading to immediate tight retries or very long sleeps. Previously these fell back to the default backoff. Reintroduce a sanity check so only reasonable, non‑negative values are honored.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In `src/client.ts` around lines 641, ensure retry delays only accept sane `retry-after` values. Restore the previous guard (non-negative and <60s) so negative/past dates or extremely large values fall back to `calculateDefaultRetryTimeoutMillis`.
1c23c70 to
2892393
Compare
| }); | ||
|
|
||
| const logger = getLogger(); | ||
|
|
There was a problem hiding this comment.
Correctness: Calling getLogger() now makes this handler throw when the logger isn’t configured (it throws in logger.ts), which is a new runtime failure path. Guard the call or use a safe fallback so doc search still works when logging isn’t set up.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In `packages/mcp-server/src/docs-search-tool.ts` around the `const logger = getLogger();` line, guard against `getLogger()` throwing when the logger isn’t configured. Replace that line with a safe fallback (e.g., try/catch returning a no-op logger with `info`/`warn` methods) so the handler does not fail when logging is not set up.
|
|
||
| const transport = new StdioServerTransport(); | ||
| await server.connect(transport); | ||
| console.error('MCP Server running on stdio'); | ||
| getLogger().info('MCP Server running on stdio'); | ||
| }; |
There was a problem hiding this comment.
Correctness:
In an MCP server using StdioServerTransport, stdout is strictly reserved for JSON-RPC protocol messages. Changing this log from console.error (which writes to stderr) to getLogger().info() (which typically writes to stdout) will likely corrupt the protocol stream, causing the client to fail to parse messages and terminate the connection.
Initialization logs must remain on stderr or use the MCP logging/message notification facility via the server instance. Since this log occurs after server.connect(), it is safer to keep it on stderr using console.error or explicitly configure the logger to use stderr.
| // says, but otherwise calculate a default | ||
| if (timeoutMillis === undefined) { | ||
| const maxRetries = options.maxRetries ?? this.maxRetries; |
There was a problem hiding this comment.
Correctness: This change removes safety bounds on timeoutMillis, allowing negative values, NaN (e.g., from a failed Date.parse), or excessively large delays from headers to be passed directly to sleep(). This can result in immediate retries that bypass backoff logic or lead to unbounded request hangs. Reintroduce validation to ensure timeoutMillis is a positive number within a reasonable range before use.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In src/client.ts around lines 640-642, reintroduce validation for timeoutMillis before using it. The header-derived value should be bounded (e.g., >=0 and <60s) and fall back to calculateDefaultRetryTimeoutMillis if invalid. Restore the previous guard or equivalent explicit checks.
2892393 to
fa3e4a7
Compare
| const transport = new StdioServerTransport(); | ||
| await server.connect(transport); | ||
| console.error('MCP Server running on stdio'); | ||
| getLogger().info('MCP Server running on stdio'); |
There was a problem hiding this comment.
Correctness: In a stdio MCP server, stdout is the communication channel with the client. If getLogger().info() writes to stdout (rather than stderr), it will corrupt the MCP message stream and break the protocol. The original console.error correctly used stderr.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In packages/mcp-server/src/stdio.ts at line 13, the change from `console.error(...)` to `getLogger().info(...)` may route the log message to stdout instead of stderr. In a stdio-based MCP server, stdout is the protocol transport channel — writing anything other than MCP messages to stdout corrupts the stream. Please verify that `getLogger().info()` writes exclusively to stderr (not stdout). If it does not guarantee stderr output, revert to `console.error(...)` or configure the logger to write to stderr.
| // If the API asks us to wait a certain amount of time, just do what it | ||
| // says, but otherwise calculate a default | ||
| if (timeoutMillis === undefined) { |
There was a problem hiding this comment.
Correctness: The old condition !(timeoutMillis && 0 <= timeoutMillis && timeoutMillis < 60 * 1000) guarded against negative or excessively large retry-after values (e.g., Date.parse returning a past timestamp gives a negative number, or a server sending a huge delay). The new check timeoutMillis === undefined removes that guard, so a negative timeoutMillis (e.g., Date.parse(retryAfterHeader) - Date.now() when the date is in the past) or an unreasonably large value will now be passed directly to sleep(), causing either an immediate wakeup with a negative delay or an extremely long hang.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In src/client.ts around line 639, the condition was changed from `!(timeoutMillis && 0 <= timeoutMillis && timeoutMillis < 60 * 1000)` to `timeoutMillis === undefined`. This removes validation that prevented negative values (e.g., when `Date.parse(retryAfterHeader) - Date.now()` is negative for past dates) or unreasonably large values from being passed to `sleep()`. Restore bounds checking so that negative or excessively large timeoutMillis values fall back to the default calculation, for example: `if (timeoutMillis === undefined || timeoutMillis < 0 || timeoutMillis >= 60 * 60 * 1000)`.
| }, | ||
| }); | ||
|
|
||
| const logger = getLogger(); |
There was a problem hiding this comment.
Correctness: getLogger() throws if the logger hasn't been configured — moving it after fetch but before the error handling means a mis-configured environment will throw a cryptic 'Logger has not been configured' error instead of the intended HTTP error message.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In packages/mcp-server/src/docs-search-tool.ts, the call to `getLogger()` is placed unconditionally after the fetch, before any error or success handling. Since `getLogger()` throws if the logger is not configured, move it inside each branch (error and success) so that a missing logger configuration doesn't shadow the actual HTTP error. Move the `const logger = getLogger()` line inside the `if (!result.ok)` block and also inside the success path, or alternatively initialize it at the top of the handler but ensure the thrown error message is preserved.
| '\n This is the hyperspell MCP server.\n\n Available tools:\n - search_docs: Search SDK documentation to find the right methods and parameters.\n - execute: Run TypeScript code against a pre-authenticated SDK client. Define an async run(client) function.\n\n Workflow:\n - If unsure about the API, call search_docs first.\n - Write complete solutions in a single execute call when possible. For large datasets, use API filters to narrow results or paginate within a single execute block.\n - If execute returns an error, read the error and fix your code rather than retrying the same approach.\n - Variables do not persist between execute calls. Return or log all data you need.\n - Individual HTTP requests to the API have a 30-second timeout. If a request times out, try a smaller query or add filters.\n - Code execution has a total timeout of approximately 5 minutes. If your code times out, simplify it or break it into smaller steps.\n '; | ||
| } | ||
|
|
||
| instructions ??= ((await response.json()) as { instructions: string }).instructions; |
There was a problem hiding this comment.
Correctness: If response.ok is true but the API response is missing the instructions property, the function will return undefined, violating the Promise<string> return type. The removal of the template wrapper exposed this edge case where a malformed API response is no longer coerced into a string.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In packages/mcp-server/src/instructions.ts at line 62, the nullish coalescing assignment `instructions ??= ((await response.json()) as { instructions: string }).instructions` can result in `instructions` being `undefined` if the API response JSON does not contain an `instructions` field. Add a fallback so the function always returns a valid string, e.g.: `instructions ??= ((await response.json()) as { instructions: string }).instructions ?? '<fallback instructions>';`
fa3e4a7 to
d519b6f
Compare
packages/mcp-server/src/code-tool.ts
Outdated
| ...(reqContext.stainlessApiKey && { Authorization: reqContext.stainlessApiKey }), | ||
| 'Content-Type': 'application/json', | ||
| client_envs: JSON.stringify({ | ||
| 'x-stainless-mcp-client-envs': JSON.stringify({ |
There was a problem hiding this comment.
Correctness: The header was renamed from client_envs to x-stainless-mcp-client-envs. If the remote Stainless server still expects the old client_envs header name, this change will silently break environment variable forwarding (API key will not be passed to the sandbox), causing all remote code executions to fail with auth errors.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In packages/mcp-server/src/code-tool.ts at line 158, the HTTP header was renamed from `client_envs` to `x-stainless-mcp-client-envs`. Verify that the remote Stainless API endpoint at https://api.stainless.com/api/ai/code-tool has been updated to read the new header name. If the server still reads `client_envs`, revert the rename or coordinate the server-side change first to avoid silent auth failures in remote code execution mode.
| "@types/morgan": "^1.9.10", | ||
| "@types/qs": "^6.14.0", | ||
| "@types/yargs": "^17.0.8", | ||
| "@typescript-eslint/eslint-plugin": "8.31.1", |
There was a problem hiding this comment.
Correctness: ESLint v9 uses a flat config system (eslint.config.js) by default, replacing the legacy .eslintrc.* format — if the project uses the old config format, linting will silently fail or throw errors after this upgrade.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In packages/mcp-server/package.json, ESLint is being upgraded from ^8.49.0 to ^9.39.1. ESLint v9 uses a flat config system (eslint.config.js) by default. Check if the project has an eslint.config.js or if it still uses .eslintrc.* format. If using the legacy format, either migrate to flat config or add ESLINT_USE_FLAT_CONFIG=false to scripts, or pin back to eslint v8 until migration is done.
| // If the API asks us to wait a certain amount of time (and it's a reasonable amount), | ||
| // just do what it says, but otherwise calculate a default | ||
| if (!(timeoutMillis && 0 <= timeoutMillis && timeoutMillis < 60 * 1000)) { | ||
| // If the API asks us to wait a certain amount of time, just do what it |
There was a problem hiding this comment.
Correctness: The old condition !(timeoutMillis && 0 <= timeoutMillis && timeoutMillis < 60 * 1000) rejected negative values (e.g. from Date.parse(retryAfterHeader) - Date.now() when the date is in the past) and unreasonably large values (≥60s). The new check timeoutMillis === undefined will blindly pass a negative or very large timeoutMillis to sleep(), potentially causing an instant sleep (negative) or a multi-minute hang (huge Retry-After date), rather than falling back to the calculated default.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In src/client.ts around line 639, the condition was changed from `!(timeoutMillis && 0 <= timeoutMillis && timeoutMillis < 60 * 1000)` to `timeoutMillis === undefined`. This removes the validation that rejected negative values (which can occur when `Date.parse(retryAfterHeader) - Date.now()` is negative, i.e. the Retry-After date is in the past) and values >= 60000ms. Restore the bounds check so that invalid/unreasonable values also fall back to the calculated default: change the condition back to `!(timeoutMillis != null && 0 <= timeoutMillis && timeoutMillis < 60 * 1000)`.
| customErrorMessage: function (req, res, err) { | ||
| return `Request ${req.method} to ${req.url} errored with status ${res.statusCode}`; | ||
| }, |
There was a problem hiding this comment.
Correctness: The err parameter in customErrorMessage is declared but never used, triggering the @typescript-eslint/no-unused-vars lint error. This will cause CI lint failures.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In packages/mcp-server/src/http.ts, the `customErrorMessage` function on approximately line 109 declares an `err` parameter that is never used, violating the `@typescript-eslint/no-unused-vars` rule. Rename it to `_err` to signal intentional non-use and fix the lint error.
| @@ -83,6 +84,8 @@ export function codeTool({ | |||
| }, | |||
| }; | |||
|
|
|||
There was a problem hiding this comment.
Correctness: getLogger() throws if configureLogger() hasn't been called yet — calling it at codeTool() invocation time (rather than inside handler) means any call to codeTool() before logger configuration will crash immediately instead of at handler execution time, but more critically, if the logger is not yet configured when this module initializes the tool factory, the error surfaces at an unexpected callsite rather than during actual request handling.
Affected Locations:
- packages/mcp-server/src/code-tool.ts:86-86
- packages/mcp-server/src/docs-search-tool.ts:63-64
- packages/mcp-server/src/instructions.ts:54-56
- packages/mcp-server/src/stdio.ts:13-13
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In packages/mcp-server/src/code-tool.ts at line 86, `const logger = getLogger()` is called at `codeTool()` factory invocation time. Since `getLogger()` throws if `configureLogger()` hasn't been called, this will throw at tool creation time rather than at handler execution time. Move `const logger = getLogger();` inside the `handler` async function body (around line 92) so the logger is resolved lazily when a request is actually processed.
d519b6f to
492d710
Compare
| }, | ||
| }; | ||
|
|
||
| const logger = getLogger(); |
There was a problem hiding this comment.
Correctness: getLogger() throws if configureLogger() hasn't been called yet — calling it eagerly at codeTool() construction time means any code that creates a codeTool before configuring the logger will crash, even if the logger is configured before the handler is ever invoked.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In packages/mcp-server/src/code-tool.ts, line 87, `const logger = getLogger()` is called eagerly inside `codeTool()` at construction time. `getLogger()` throws if the logger hasn't been configured yet. Move this line inside the `handler` async function (around line 91) so it is only evaluated when the handler is actually invoked, by which time `configureLogger()` is guaranteed to have been called.
| }, | ||
| }; | ||
|
|
||
| const logger = getLogger(); |
There was a problem hiding this comment.
Correctness: getLogger() throws if configureLogger() hasn't been called yet — calling it eagerly at codeTool() construction time means any code that creates a codeTool before configuring the logger will crash, even if the logger is configured before the handler is ever invoked.
Affected Locations:
- packages/mcp-server/src/code-tool.ts:87-87
- packages/mcp-server/src/docs-search-tool.ts:63-63
- packages/mcp-server/src/instructions.ts:53-56
- packages/mcp-server/src/stdio.ts:13-13
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In packages/mcp-server/src/code-tool.ts, line 87, `const logger = getLogger()` is called eagerly inside `codeTool()` at construction time. `getLogger()` throws if the logger hasn't been configured yet. Move this line inside the `handler` async function (around line 91) so it is only evaluated when the handler is actually invoked, by which time `configureLogger()` is guaranteed to have been called.
| "@types/yargs": "^17.0.8", | ||
| "@typescript-eslint/eslint-plugin": "8.31.1", | ||
| "@typescript-eslint/parser": "8.31.1", |
There was a problem hiding this comment.
Correctness: ESLint 9 introduced a breaking flat config format (eslint.config.js) that is incompatible with the legacy .eslintrc.* format used in ESLint 8. If the project still uses a legacy config file, linting will silently skip or error, and eslint-plugin-unused-imports v4 also requires ESLint 9's flat config API, so mixing old config with new plugins may break the lint pipeline entirely.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In packages/mcp-server/package.json, eslint was bumped from ^8.49.0 to ^9.39.1. ESLint 9 uses a new flat config system (eslint.config.js) and drops support for .eslintrc.* files by default. Check whether the project has a flat config file (eslint.config.js/mjs/cjs). If not, either migrate the existing .eslintrc config to flat config format, or pin eslint back to ^8.x. Also verify that eslint-plugin-unused-imports v4 and eslint-plugin-prettier v5.4.1 are compatible with whichever ESLint major version is chosen.
| // If the API asks us to wait a certain amount of time, just do what it | ||
| // says, but otherwise calculate a default | ||
| if (timeoutMillis === undefined) { |
There was a problem hiding this comment.
Correctness: The old condition !(timeoutMillis && 0 <= timeoutMillis && timeoutMillis < 60 * 1000) guarded against negative, zero, NaN, or excessively large retry-after values (e.g. a server sending Retry-After: 9999 seconds would previously be capped). The new check timeoutMillis === undefined removes all sanity bounds, so a malicious or misconfigured server can now force the client to sleep for an arbitrarily long time (e.g. hours or days) by setting a large Retry-After header.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In src/client.ts around line 639-641, the condition was changed from `!(timeoutMillis && 0 <= timeoutMillis && timeoutMillis < 60 * 1000)` to `timeoutMillis === undefined`. This removes the upper bound (60 seconds) and the non-negative guard on retry-after values derived from response headers. Restore the bounds check so that only valid, reasonable timeout values (0 <= timeoutMillis < 60000) from the server headers are honored; otherwise fall back to the calculated default. A fix like `if (!(timeoutMillis !== undefined && Number.isFinite(timeoutMillis) && timeoutMillis >= 0 && timeoutMillis < 60 * 1000))` would restore the original safety guarantees.
| const pathQuery = Object.fromEntries(url.searchParams); | ||
| if (!isEmptyObj(defaultQuery) || !isEmptyObj(pathQuery)) { | ||
| query = { ...pathQuery, ...defaultQuery, ...query }; |
There was a problem hiding this comment.
Correctness: Object.fromEntries(url.searchParams) only captures the last value for repeated keys (e.g., ?foo=1&foo=2 becomes {foo: '2'}), silently dropping multi-value query parameters that were part of the original path.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In src/client.ts at line 304, `Object.fromEntries(url.searchParams)` is used to extract query parameters already present in the URL path. This approach silently drops duplicate keys (e.g., `?foo=1&foo=2` becomes `{foo: '2'}`). Fix this by using `url.searchParams` directly or by collecting all values per key using `Array.from(url.searchParams.entries())` and merging them in a way that preserves multi-value params. Also ensure that after merging and setting `url.search`, the path's original params are not double-applied.
492d710 to
b565999
Compare
| "@types/yargs": "^17.0.8", | ||
| "@typescript-eslint/eslint-plugin": "8.31.1", | ||
| "@typescript-eslint/parser": "8.31.1", |
There was a problem hiding this comment.
Correctness: ESLint v9 introduces a flat config system (eslint.config.js) that is incompatible with the legacy .eslintrc.* format used by v8 — if the project still uses .eslintrc style config, linting will break entirely after this upgrade. Additionally, eslint-plugin-unused-imports v4 drops support for ESLint v8 and requires flat config, so both plugins may silently fail or error without config migration.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In packages/mcp-server/package.json, lines 65-67, ESLint is being upgraded from ^8.49.0 to ^9.39.1. ESLint v9 uses a flat config system (eslint.config.js) that is incompatible with .eslintrc.* style configs. Check if the project has an .eslintrc.js/.eslintrc.json/.eslintrc.yaml config file and migrate it to the new flat config format (eslint.config.js). Also verify that eslint-plugin-unused-imports v4 and eslint-plugin-prettier v5.4.1 are compatible with the flat config setup. If migration is not planned, revert to eslint ^8.x.
| // If the API asks us to wait a certain amount of time (and it's a reasonable amount), | ||
| // just do what it says, but otherwise calculate a default | ||
| if (!(timeoutMillis && 0 <= timeoutMillis && timeoutMillis < 60 * 1000)) { | ||
| // If the API asks us to wait a certain amount of time, just do what it |
There was a problem hiding this comment.
Correctness: The old condition !(timeoutMillis && 0 <= timeoutMillis && timeoutMillis < 60 * 1000) also rejected negative values, zero, NaN, and unreasonably large values (≥ 60 s) from the retry-after headers, falling back to the calculated default. The new timeoutMillis === undefined check means a negative, NaN, zero, or extremely large server-provided value will now be passed directly to sleep(), potentially causing an immediate retry loop (0/negative ms) or an arbitrarily long stall (multi-minute delay).
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In src/client.ts around line 639, the condition was changed from `!(timeoutMillis && 0 <= timeoutMillis && timeoutMillis < 60 * 1000)` to `timeoutMillis === undefined`. This removes guards against negative, NaN, zero, and excessively large values parsed from retry-after headers. Fix by also falling back to the default when timeoutMillis is not a finite positive number, e.g.: `if (timeoutMillis === undefined || !Number.isFinite(timeoutMillis) || timeoutMillis < 0)`.
| }, | ||
| }; | ||
|
|
||
| const logger = getLogger(); |
There was a problem hiding this comment.
Correctness: getLogger() throws if configureLogger() hasn't been called yet — calling it at codeTool() invocation time (rather than inside handler) means any call to codeTool() before the logger is configured will throw, making the tool completely unusable even before any code execution is attempted.
Affected Locations:
- packages/mcp-server/src/code-tool.ts:87-87
- packages/mcp-server/src/docs-search-tool.ts:63-63
- packages/mcp-server/src/instructions.ts:54-54
- packages/mcp-server/src/stdio.ts:13-13
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In `packages/mcp-server/src/code-tool.ts`, line 87, `const logger = getLogger()` is called at `codeTool()` invocation time. According to `packages/mcp-server/src/logger.ts`, `getLogger()` throws an error if `configureLogger()` hasn't been called yet. Move the `const logger = getLogger()` call to inside the `handler` async function body so that the logger is only retrieved when a request is actually being handled, by which time `configureLogger()` should have been called. This prevents `codeTool()` from throwing during setup.
| const pathQuery = Object.fromEntries(url.searchParams); | ||
| if (!isEmptyObj(defaultQuery) || !isEmptyObj(pathQuery)) { | ||
| query = { ...pathQuery, ...defaultQuery, ...query }; |
There was a problem hiding this comment.
Correctness: Using Object.fromEntries(url.searchParams) to extract pathQuery causes multi-value parameters in the path (e.g., ?a=1&a=2) to be flattened into a single value, as Object.fromEntries only keeps the last value for a given key. This results in data loss if the original path contained repeated query keys.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In src/client.ts around line 304, `Object.fromEntries(url.searchParams)` is used to extract query params from the path URL. `URLSearchParams` can have multiple values per key (e.g., `?foo=1&foo=2`), but `Object.fromEntries` only retains the last entry per key, silently dropping duplicates. Consider using a different approach to preserve multi-value params, or document that multi-value path params are not supported.
| import pino from 'pino'; | ||
| import pinoHttp from 'pino-http'; |
There was a problem hiding this comment.
Correctness: Both pino and pinoHttp are imported in the diff, but pino is only used in the complete file for pino.stdSerializers calls — this is fine. However, if morgan and morganBody were previously used for request body logging, replacing them with pinoHttp may silently drop request body logging since pino-http does not log request bodies by default.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In packages/mcp-server/src/http.ts, the diff replaces morgan and morgan-body with pino and pino-http. morgan-body previously logged request/response bodies, but pino-http does not log bodies by default. Verify whether request body logging is still required and, if so, add a custom serializer or middleware to capture it in the pinoHttp configuration.
…e worker environments
stainless-app
bot
force-pushed
the
release-please--branches--main--changes--next--components--hyperspell
branch
from
b565999 to
4a81b5c
Compare
| "@typescript-eslint/eslint-plugin": "8.31.1", | ||
| "@typescript-eslint/parser": "8.31.1", | ||
| "eslint": "^8.49.0", | ||
| "eslint-plugin-prettier": "^5.0.1", | ||
| "eslint-plugin-unused-imports": "^3.0.0", | ||
| "eslint": "^9.39.1", |
There was a problem hiding this comment.
Correctness: ESLint v9 uses a flat config system (eslint.config.js) instead of .eslintrc.* — if the project still uses the legacy config format, linting will break entirely after this upgrade. Additionally, eslint-plugin-unused-imports v4 requires ESLint v9's flat config API, so the plugin will fail if the config hasn't been migrated.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In packages/mcp-server/package.json, ESLint is being upgraded from ^8 to ^9. ESLint v9 introduces a breaking change: it uses flat config (eslint.config.js) by default instead of .eslintrc.*. Before merging, verify that the ESLint config file in this package has been migrated to flat config format, and that @typescript-eslint/eslint-plugin 8.31.1 and eslint-plugin-unused-imports ^4.1.4 are compatible with ESLint v9 flat config. If not migrated, add ESLINT_USE_FLAT_CONFIG=false env var as a temporary workaround or migrate the config.
| import { Tool } from '@modelcontextprotocol/sdk/types.js'; | ||
| import { readEnv, requireValue } from './util'; | ||
| import { WorkerInput, WorkerOutput } from './code-tool-types'; | ||
| import { getLogger } from './logger'; |
There was a problem hiding this comment.
Correctness: getLogger() throws if configureLogger() was never called, and it is invoked at codeTool() call time (not lazily). If any consumer constructs the codeTool before configuring the logger, the entire server will crash with "Logger has not been configured".
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In packages/mcp-server/src/code-tool.ts, the call to `getLogger()` (added via the new import of `./logger`) is placed at the top level of the `codeTool()` factory function body. `getLogger()` throws synchronously if `configureLogger()` has not been called first. Move the `getLogger()` call inside the `handler` async function (lazily), or ensure all callers of `codeTool()` are guaranteed to run after `configureLogger()` has been invoked. File: packages/mcp-server/src/code-tool.ts, around the line where `const logger = getLogger();` is placed inside `codeTool`.
| import { Tool } from '@modelcontextprotocol/sdk/types.js'; | ||
| import { readEnv, requireValue } from './util'; | ||
| import { WorkerInput, WorkerOutput } from './code-tool-types'; | ||
| import { getLogger } from './logger'; |
There was a problem hiding this comment.
Correctness: getLogger() throws if configureLogger() was never called, and it is invoked at codeTool() call time (not lazily). If any consumer constructs the codeTool before configuring the logger, the entire server will crash with "Logger has not been configured".
Affected Locations:
- packages/mcp-server/src/code-tool.ts:15-15
- packages/mcp-server/src/code-tool.ts:82-82
- packages/mcp-server/src/docs-search-tool.ts:63-63
- packages/mcp-server/src/instructions.ts:49-51
- packages/mcp-server/src/stdio.ts:13-13
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In packages/mcp-server/src/code-tool.ts, the call to `getLogger()` (added via the new import of `./logger`) is placed at the top level of the `codeTool()` factory function body. `getLogger()` throws synchronously if `configureLogger()` has not been called first. Move the `getLogger()` call inside the `handler` async function (lazily), or ensure all callers of `codeTool()` are guaranteed to run after `configureLogger()` has been invoked. File: packages/mcp-server/src/code-tool.ts, around the line where `const logger = getLogger();` is placed inside `codeTool`.
| // If the API asks us to wait a certain amount of time, just do what it | ||
| // says, but otherwise calculate a default | ||
| if (timeoutMillis === undefined) { |
There was a problem hiding this comment.
Correctness: The old condition !(timeoutMillis && 0 <= timeoutMillis && timeoutMillis < 60 * 1000) guarded against negative, zero, NaN, or excessively large values from a malicious/malformed retry-after header — e.g. Date.parse(retryAfterHeader) - Date.now() can return a negative number or a value of several hours. The new check timeoutMillis === undefined skips all of those guards, so the client will now blindly sleep() for any value the server returns, including negative milliseconds (sleep(negative) is harmless in most impls but semantically wrong) or arbitrarily large delays (e.g. hours), making the client trivially susceptible to a denial-of-service via a crafted Retry-After header.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In src/client.ts around line 639, the condition was changed from `!(timeoutMillis && 0 <= timeoutMillis && timeoutMillis < 60 * 1000)` to `timeoutMillis === undefined`. This removes important guards against negative values (from a bad Date.parse result), NaN, zero, and excessively large values that could come from a malformed or malicious Retry-After header. Restore the bounds check so that the calculated default is used whenever `timeoutMillis` is undefined, NaN, negative, or >= 60000 ms. For example: `if (!(timeoutMillis != null && Number.isFinite(timeoutMillis) && timeoutMillis >= 0 && timeoutMillis < 60 * 1000))`.
| query = { ...defaultQuery, ...query }; | ||
| const pathQuery = Object.fromEntries(url.searchParams); | ||
| if (!isEmptyObj(defaultQuery) || !isEmptyObj(pathQuery)) { | ||
| query = { ...pathQuery, ...defaultQuery, ...query }; |
There was a problem hiding this comment.
Correctness: The spread order { ...pathQuery, ...defaultQuery, ...query } means defaultQuery silently overrides any same-named key that was embedded in the path (e.g. /search?q=foo with defaultQuery = { q: 'bar' } will lose foo).
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In src/client.ts at line 306, the spread order is `{ ...pathQuery, ...defaultQuery, ...query }`. This means defaultQuery keys overwrite same-named keys that came from the path's own query string. The intended precedence should be: explicit per-request `query` wins, then path-embedded params, then client-level defaultQuery (lowest priority). Fix by changing the spread to `{ ...defaultQuery, ...pathQuery, ...query }`. Also verify that re-merging already-string-encoded pathQuery values into the object that is later passed to `stringifyQuery` does not cause double-encoding of percent-escaped values.
| "bin": { | ||
| "hyperspell": "bin/cli" | ||
| }, | ||
| "overrides": { | ||
| "minimatch": "^9.0.5" | ||
| }, | ||
| "pnpm": { | ||
| "overrides": { | ||
| "minimatch": "^9.0.5" | ||
| } | ||
| }, | ||
| "resolutions": { | ||
| "minimatch": "^9.0.5" | ||
| }, | ||
| "exports": { | ||
| ".": { | ||
| "import": "./dist/index.mjs", |
There was a problem hiding this comment.
Correctness: The packageManager field specifies yarn@1.22.22, which respects resolutions but ignores overrides and pnpm.overrides. Additionally, Yarn 1 resolutions officially require an exact version rather than a range (e.g., 9.0.5 instead of ^9.0.5) to ensure the dependency is pinned correctly.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In package.json lines 53-69, the diff adds `overrides`, `pnpm.overrides`, and `resolutions` all set to `minimatch: ^9.0.5`. The project uses Yarn 1 (yarn@1.22.22) which only respects `resolutions`, and Yarn 1 does not support range specifiers like `^9.0.5` in resolutions — it needs an exact version. Remove the `overrides` and `pnpm.overrides` blocks (they are irrelevant for Yarn 1), and change the `resolutions` value from `^9.0.5` to an exact version like `9.0.5`.
| }); | ||
|
|
||
| if (!res.ok) { | ||
| if (res.status === 404 && !reqContext.stainlessApiKey) { | ||
| throw new Error( | ||
| 'Could not access code tool for this project. You may need to provide a Stainless API key via the STAINLESS_API_KEY environment variable, the --stainless-api-key flag, or the x-stainless-api-key HTTP header.', |
There was a problem hiding this comment.
Correctness: The 404 check assumes that a missing API key is the only reason for a 404, but the endpoint could return 404 for other reasons (e.g., the project hyperspell is not found in Stainless, wrong CODE_MODE_ENDPOINT_URL override). This will surface a misleading "provide a Stainless API key" message when the real cause is something else entirely, masking the actual error response body.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In packages/mcp-server/src/code-tool.ts, lines 167-172, the condition `res.status === 404 && !reqContext.stainlessApiKey` produces a misleading error message when a 404 is returned for reasons unrelated to authentication (e.g., wrong endpoint URL, unknown project). Consider including the actual response body in the error message, or only show the API key hint when the response body specifically indicates an auth/access issue. Example fix: read `res.text()` first, check if it indicates an auth problem, then decide which message to surface.
4a81b5c to
ab78440
Compare
| cwd: path.dirname(workerPath), | ||
| // Merge any upstream client envs into the Deno subprocess environment, | ||
| // with the upstream env vars taking precedence. | ||
| env: { ...process.env, ...reqContext.upstreamClientEnvs }, |
There was a problem hiding this comment.
Correctness: Spreading process.env into the Deno subprocess environment exposes all host secrets (tokens, credentials, cloud keys, etc.) to untrusted user-supplied code running inside the worker. An attacker can exfiltrate any env var via the allowed network connection to baseURLHostname.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In packages/mcp-server/src/code-tool.ts at line 276, the change `env: { ...process.env, ...reqContext.upstreamClientEnvs }` spreads the entire host process environment (including secrets like AWS keys, tokens, etc.) into a Deno subprocess that runs user-supplied code. Fix by only forwarding an explicit allowlist of safe env vars (e.g. PATH, HOME, TMPDIR) plus the upstreamClientEnvs, instead of blindly spreading process.env.
|
|
||
| export const workerPath = require.resolve('./code-tool-worker.mjs'); | ||
| export function getWorkerPath(): string { | ||
| return require.resolve('./code-tool-worker.mjs'); | ||
| } |
There was a problem hiding this comment.
Correctness: The exported symbol changed from a constant workerPath to a function getWorkerPath() — any callers that reference workerPath directly will now get undefined or a compile error at runtime.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In packages/mcp-server/src/code-tool-paths.cts, the export changed from `export const workerPath = ...` to `export function getWorkerPath(): string { ... }`. Search the entire codebase for all usages of `workerPath` (the old exported constant name) and update them to call `getWorkerPath()` instead. Failure to do so will cause runtime errors or TypeScript compile errors wherever the old name is referenced.
| "@types/yargs": "^17.0.8", | ||
| "@typescript-eslint/eslint-plugin": "8.31.1", | ||
| "@typescript-eslint/parser": "8.31.1", |
There was a problem hiding this comment.
Correctness: ESLint v9 uses a flat config format (eslint.config.js) by default, which is a breaking change from v8's .eslintrc.* format — if the project still uses the old config format, linting will fail or produce unexpected results.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In packages/mcp-server/package.json, ESLint is being upgraded from ^8.49.0 to ^9.39.1. ESLint v9 uses flat config (eslint.config.js) by default and drops support for .eslintrc.* files. Check if the project has an eslint.config.js or if it's still using .eslintrc.* format. If using the old format, either migrate to flat config or add ESLINT_USE_FLAT_CONFIG=false environment variable as a temporary workaround, and also verify that eslint-plugin-unused-imports@^4.1.4 and eslint-plugin-prettier@^5.4.1 are compatible with ESLint v9 flat config.
| }, | ||
| }; | ||
|
|
||
| const logger = getLogger(); |
There was a problem hiding this comment.
Correctness: getLogger() throws if configureLogger() hasn't been called yet, and calling it at codeTool() invocation time (rather than inside the handler) means any call to codeTool() before the logger is configured will crash immediately instead of deferring the error to actual request handling.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In packages/mcp-server/src/code-tool.ts, line 82, `const logger = getLogger()` is called at the top of the `codeTool()` factory function. Since `getLogger()` throws if the logger hasn't been configured yet, this means calling `codeTool()` before `configureLogger()` is invoked will throw immediately. Move the `const logger = getLogger()` call inside the `handler` async function body (around line 86) so it is only evaluated at request time, by which point the logger should be configured.
| HYPERSPELL_BASE_URL: readEnv('HYPERSPELL_BASE_URL') ?? client.baseURL ?? undefined, | ||
| }; | ||
| // Merge any upstream client envs from the request header, with upstream values taking precedence. | ||
| const mergedClientEnvs = { ...localClientEnvs, ...reqContext.upstreamClientEnvs }; |
There was a problem hiding this comment.
Correctness: 🔒 reqContext.upstreamClientEnvs can override HYPERSPELL_API_KEY with a caller-supplied value, which then gets forwarded to the Stainless code-tool endpoint as the credential used to call the Hyperspell API. A malicious or misconfigured upstream client could substitute an arbitrary API key, causing all code execution to run under that key instead of the server-configured one — effectively an auth bypass / credential injection.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In packages/mcp-server/src/code-tool.ts at line 155, the spread `{ ...localClientEnvs, ...reqContext.upstreamClientEnvs }` allows a caller-supplied upstream env to override HYPERSPELL_API_KEY. Fix this by stripping HYPERSPELL_API_KEY (and potentially HYPERSPELL_BASE_URL) from upstreamClientEnvs before merging, so that server-configured credentials always take precedence over client-supplied ones.
| // If the API asks us to wait a certain amount of time, just do what it | ||
| // says, but otherwise calculate a default | ||
| if (timeoutMillis === undefined) { |
There was a problem hiding this comment.
Correctness: The old condition !(timeoutMillis && 0 <= timeoutMillis && timeoutMillis < 60 * 1000) also rejected negative values (e.g. from Date.parse(retryAfterHeader) - Date.now() when the date is in the past) and values ≥ 60 seconds that could cause an excessively long sleep. The new timeoutMillis === undefined check removes those safety guards, so a negative or arbitrarily large server-supplied value will now be passed directly to sleep(), potentially causing an immediate return (negative ms) or a very long hang.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In src/client.ts around line 639-641, the condition was changed from `!(timeoutMillis && 0 <= timeoutMillis && timeoutMillis < 60 * 1000)` to `timeoutMillis === undefined`. This removes the safety bounds check that prevented negative values (e.g. from a past Retry-After date) or unreasonably large values (≥ 60 s) from being passed directly to sleep(). Restore the bounds check: change the condition to `timeoutMillis === undefined || !(0 <= timeoutMillis && timeoutMillis < 60 * 1000)` so that out-of-range server-supplied values fall back to the calculated default.
| }, | ||
| }; | ||
|
|
||
| const logger = getLogger(); |
There was a problem hiding this comment.
Correctness: getLogger() throws if configureLogger() hasn't been called yet, and calling it at codeTool() invocation time (rather than inside the handler) means any call to codeTool() before the logger is configured will crash immediately instead of deferring the error to actual request handling.
Affected Locations:
- packages/mcp-server/src/code-tool.ts:82-82
- packages/mcp-server/src/docs-search-tool.ts:63-64
- packages/mcp-server/src/http.ts:163-163
- packages/mcp-server/src/instructions.ts:48-51
- packages/mcp-server/src/stdio.ts:13-13
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In packages/mcp-server/src/code-tool.ts, line 82, `const logger = getLogger()` is called at the top of the `codeTool()` factory function. Since `getLogger()` throws if the logger hasn't been configured yet, this means calling `codeTool()` before `configureLogger()` is invoked will throw immediately. Move the `const logger = getLogger()` call inside the `handler` async function body (around line 86) so it is only evaluated at request time, by which point the logger should be configured.
| const pathQuery = Object.fromEntries(url.searchParams); | ||
| if (!isEmptyObj(defaultQuery) || !isEmptyObj(pathQuery)) { | ||
| query = { ...pathQuery, ...defaultQuery, ...query }; |
There was a problem hiding this comment.
Correctness: When buildURL is called with a query object that already contains keys extracted from a previous URL's search params (e.g. during retries or pagination), pathQuery is extracted from url.searchParams and then merged back into query. If the caller's query argument already includes those same params (because they were originally part of the path), they will be duplicated/re-merged, potentially causing unexpected query string values. Additionally, Object.fromEntries(url.searchParams) silently drops duplicate keys (multi-value params), so repeated query params in the path will lose all but the last value.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In `src/client.ts` around line 304-306, the code does `const pathQuery = Object.fromEntries(url.searchParams)` which loses duplicate/multi-value query parameters (only the last value per key is kept). Additionally, merging pathQuery back into query could cause double-application of path params if callers already pass those params in the query object. Consider using `url.searchParams.getAll()` or iterating entries carefully to preserve multi-value params, and ensure path params are not double-applied when the caller's query already contains them.
| }); | ||
|
|
||
| if (!res.ok) { | ||
| if (res.status === 404 && !reqContext.stainlessApiKey) { | ||
| throw new Error( |
There was a problem hiding this comment.
Correctness: A 404 response without a Stainless API key doesn't necessarily mean the project needs authentication — the endpoint or project itself might genuinely not exist, or the URL could be misconfigured. This misleads users into providing an API key when the real issue may be something else entirely (e.g., wrong CODE_MODE_ENDPOINT_URL).
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In packages/mcp-server/src/code-tool.ts around line 171-175, the 404 check assumes a missing Stainless API key is the cause when no key is present. However, a 404 could also mean a wrong endpoint URL or a truly missing resource. Consider softening the message to indicate it 'may' be an auth issue or 'may' also be a misconfigured endpoint URL, so users are not misled into only looking at the API key as the root cause.
| try { | ||
| const parsed = JSON.parse(clientEnvsHeader); | ||
| if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) { | ||
| upstreamClientEnvs = parsed; | ||
| } | ||
| } catch { |
There was a problem hiding this comment.
Correctness: The object check typeof parsed === 'object' and !Array.isArray(parsed) confirms the top-level value is a plain object, but does not validate that all values within the object are strings — the type assertion upstreamClientEnvs = parsed will silently accept { key: 123 } or { key: null } as Record<string, string>, potentially causing downstream type confusion or runtime errors when string methods are called on non-string env values.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In packages/mcp-server/src/http.ts, lines 33-38, the parsed JSON object is assigned to `upstreamClientEnvs` typed as `Record<string, string>` without verifying that all values in the object are actually strings. Add a check using `Object.values(parsed).every((v) => typeof v === 'string')` before the assignment to ensure type safety at runtime.
Automated Release PR
0.33.0 (2026-03-14)
Full Changelog: v0.32.1...v0.33.0
Features
Bug Fixes
Chores
This pull request is managed by Stainless's GitHub App.
The semver version number is based on included commit messages. Alternatively, you can manually set the version number in the title of this pull request.
For a better experience, it is recommended to use either rebase-merge or squash-merge when merging this pull request.
🔗 Stainless website
📚 Read the docs
🙋 Reach out for help or questions