feat: adding umaaas mcp server#252
Conversation
142c7ba to
bca2460
Compare
Greptile SummaryThis PR introduces a new MCP (Model Context Protocol) server for the UMAaaS API, built with Express 5, the MCP TypeScript SDK, and Pino for logging. It supports both stdio and streamable HTTP transports, exposes a code-execution tool (local Deno or remote Stainless sandbox) and a documentation-search tool, and handles Basic-auth and per-header credential overrides for multi-tenant deployments.
Confidence Score: 3/5Not safe to merge as-is; uncaught exceptions in the HTTP transport and the code-execution tool will surface as unformatted 500s or MCP protocol errors rather than proper JSON-RPC error responses. Three distinct error-handling gaps exist: auth-scheme errors thrown by
|
| Filename | Overview |
|---|---|
| mcp-server/src/code-tool.ts | Core code-execution tool; remoteStainlessHandler can throw an unhandled exception via requireValue when credentials are absent instead of returning a proper error result. |
| mcp-server/src/auth.ts | Auth header parsing; throws on unsupported Authorization scheme but the caller in http.ts does not catch it, leading to an unformatted 500 response instead of a JSON-RPC error. |
| mcp-server/src/http.ts | Express 5 HTTP transport; lacks error middleware to catch exceptions from auth parsing, resulting in non-JSON-RPC 500 responses for unsupported auth schemes. |
| mcp-server/src/server.ts | MCP server setup and tool registration; client initialization errors are correctly caught and returned as error results, but tool handler exceptions are not. |
| mcp-server/src/options.ts | CLI and query-string option parsing; well-structured with zod validation, no significant issues. |
| mcp-server/src/instructions.ts | Per-API-key instruction cache with 15-minute TTL; concurrent cache misses for the same key can trigger parallel API fetches (thundering herd), but this is a minor efficiency concern. |
| mcp-server/src/code-tool-worker.ts | Deno worker for local code execution; non-null assertion on getRunFunctionSource is safe given surrounding null check; console monkey-patching and TypeScript eval are intentional design choices. |
| mcp-server/src/docs-search-tool.ts | Docs search tool with remote/local modes; reads DOCS_SEARCH_URL directly from process.env at module initialization rather than using the readEnv utility. |
| mcp-server/src/methods.ts | SDK method registry and allow/block filtering via regex; logic is correct and well-structured. |
Sequence Diagram
sequenceDiagram
participant C as MCP Client
participant H as Express HTTP Handler
participant A as auth.ts
participant S as server.ts (initMcpServer)
participant CT as code-tool.ts
participant W as Deno Worker / Stainless Sandbox
participant U as Umaaas API
C->>H: POST / (JSON-RPC + Auth headers)
H->>A: parseClientAuthHeaders(req)
A-->>H: "{username, password} or throws"
H->>S: newMcpServer() + initMcpServer()
S->>S: getClient() → new Umaaas(clientOptions)
C->>H: "tools/call {name:"execute", code:"..."}"
H->>S: CallToolRequestSchema handler
S->>CT: "codeTool.handler({reqContext, args})"
alt "codeExecutionMode === "local""
CT->>W: newDenoHTTPWorker(workerPath)
W->>U: SDK calls via Umaaas client
U-->>W: API response
W-->>CT: WorkerOutput
else "codeExecutionMode === "stainless-sandbox""
CT->>W: "fetch(codeModeEndpoint, {code, envs})"
W->>U: SDK calls via Umaaas client
U-->>W: API response
W-->>CT: WorkerOutput
end
CT-->>S: ToolCallResult
S-->>H: MCP response
H-->>C: JSON-RPC response
Prompt To Fix All With AI
Fix the following 5 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 5
mcp-server/src/code-tool.ts:154-164
**Unhandled exception from `requireValue` escapes the tool handler.** When neither the environment variables (`UMAAAS_CLIENT_ID`/`UMAAAS_CLIENT_SECRET`) nor the per-request client credentials are set, `requireValue` throws synchronously. That exception propagates out of `remoteStainlessHandler` through the `handler` closure, through `executeHandler`, and is never caught — the MCP SDK will see an unexpected rejection instead of a well-formed `isError: true` result. The established pattern in this codebase (see `server.ts` lines 119–131 for client init errors) is to wrap failures with `asErrorResult`.
```suggestion
let localClientEnvs: { UMAAAS_CLIENT_ID: string; UMAAAS_CLIENT_SECRET: string; UMAAAS_BASE_URL: string | undefined };
try {
localClientEnvs = {
UMAAAS_CLIENT_ID: requireValue(
readEnv('UMAAAS_CLIENT_ID') ?? client.username,
'set UMAAAS_CLIENT_ID environment variable or provide username client option',
),
UMAAAS_CLIENT_SECRET: requireValue(
readEnv('UMAAAS_CLIENT_SECRET') ?? client.password,
'set UMAAAS_CLIENT_SECRET environment variable or provide password client option',
),
UMAAAS_BASE_URL: readEnv('UMAAAS_BASE_URL') ?? client.baseURL ?? undefined,
};
} catch (e) {
return asErrorResult(e instanceof Error ? e.message : String(e));
}
```
### Issue 2 of 5
mcp-server/src/auth.ts:11-22
**Unsupported Authorization scheme throws an uncaught exception in HTTP mode.** `parseClientAuthHeaders` is called inside `newServer()` without a try/catch. When a client sends `Authorization: Bearer <token>` or any non-Basic scheme, this throws and propagates through Express's async handler. Express 5 will catch it and return a plain-text/HTML 500, not a JSON-RPC error body — MCP clients will be unable to parse the response. Wrapping the `case` body in braces also fixes the `no-case-declarations` linting issue (lexical declaration in switch case without a block).
```suggestion
switch (scheme) {
case 'Basic': {
const rawValue = Buffer.from(value, 'base64').toString();
return {
username: rawValue.slice(0, rawValue.search(':')),
password: rawValue.slice(rawValue.search(':') + 1),
};
}
default:
throw new Error(
'Unsupported authorization scheme. Expected the "Authorization" header to be a supported scheme (Basic).',
);
}
```
### Issue 3 of 5
mcp-server/src/http.ts:97-106
**No Express error middleware means auth errors return an unformatted 500.** The `post` handler awaits `newServer()`, which can throw (e.g., unsupported auth scheme from `parseClientAuthHeaders`). With Express 5 and no custom error handler, the default handler replies with a plain-text/HTML 500 body. MCP clients expecting JSON-RPC cannot parse that response. Adding a catch here converts the thrown error into a proper JSON-RPC error payload.
```suggestion
const post =
(options: { clientOptions: ClientOptions; mcpOptions: McpOptions }) =>
async (req: express.Request, res: express.Response) => {
let server: McpServer | null;
try {
server = await newServer({ ...options, req, res });
} catch (err) {
res.status(400).json({
jsonrpc: '2.0',
error: {
code: -32600,
message: err instanceof Error ? err.message : String(err),
},
id: req.body?.id ?? null,
});
return;
}
// If we return null, we already set the authorization error.
if (server === null) return;
const transport = new StreamableHTTPServerTransport();
await server.connect(transport as any);
await transport.handleRequest(req, res, req.body);
};
```
### Issue 4 of 5
mcp-server/src/code-tool.ts:235-238
**`command -v deno` is a bash built-in and will always fail on Windows.** When running in local execution mode on Windows, `execSync('command -v deno')` throws immediately (the `command` built-in is not available in cmd.exe or PowerShell), so the outer `catch` fires. The fallback to node_modules deno then runs, but if the user has deno installed in their Windows PATH it will be silently skipped. Using `deno --version` is cross-platform.
```suggestion
try {
execSync('deno --version', { stdio: 'ignore' });
denoPath = 'deno';
} catch {
```
### Issue 5 of 5
mcp-server/src/docs-search-tool.ts:44-45
**`DOCS_SEARCH_URL` is read from `process.env` at module load time, not via `readEnv()`.** Every other environment variable in this package uses the `readEnv` utility (which handles Deno environments and trims whitespace). Reading `process.env` directly at import time means the URL is fixed for the process lifetime and the Deno codepath is skipped.
```suggestion
const docsSearchURL =
readEnv('DOCS_SEARCH_URL') || 'https://api.stainless.com/api/projects/umaaas/docs/search';
```
Reviews (1): Last reviewed commit: "feat: adding umaaas mcp server" | Re-trigger Greptile
| const localClientEnvs = { | ||
| UMAAAS_CLIENT_ID: requireValue( | ||
| readEnv('UMAAAS_CLIENT_ID') ?? client.username, | ||
| 'set UMAAAS_CLIENT_ID environment variable or provide username client option', | ||
| ), | ||
| UMAAAS_CLIENT_SECRET: requireValue( | ||
| readEnv('UMAAAS_CLIENT_SECRET') ?? client.password, | ||
| 'set UMAAAS_CLIENT_SECRET environment variable or provide password client option', | ||
| ), | ||
| UMAAAS_BASE_URL: readEnv('UMAAAS_BASE_URL') ?? client.baseURL ?? undefined, | ||
| }; |
There was a problem hiding this comment.
Unhandled exception from
requireValue escapes the tool handler. When neither the environment variables (UMAAAS_CLIENT_ID/UMAAAS_CLIENT_SECRET) nor the per-request client credentials are set, requireValue throws synchronously. That exception propagates out of remoteStainlessHandler through the handler closure, through executeHandler, and is never caught — the MCP SDK will see an unexpected rejection instead of a well-formed isError: true result. The established pattern in this codebase (see server.ts lines 119–131 for client init errors) is to wrap failures with asErrorResult.
| const localClientEnvs = { | |
| UMAAAS_CLIENT_ID: requireValue( | |
| readEnv('UMAAAS_CLIENT_ID') ?? client.username, | |
| 'set UMAAAS_CLIENT_ID environment variable or provide username client option', | |
| ), | |
| UMAAAS_CLIENT_SECRET: requireValue( | |
| readEnv('UMAAAS_CLIENT_SECRET') ?? client.password, | |
| 'set UMAAAS_CLIENT_SECRET environment variable or provide password client option', | |
| ), | |
| UMAAAS_BASE_URL: readEnv('UMAAAS_BASE_URL') ?? client.baseURL ?? undefined, | |
| }; | |
| let localClientEnvs: { UMAAAS_CLIENT_ID: string; UMAAAS_CLIENT_SECRET: string; UMAAAS_BASE_URL: string | undefined }; | |
| try { | |
| localClientEnvs = { | |
| UMAAAS_CLIENT_ID: requireValue( | |
| readEnv('UMAAAS_CLIENT_ID') ?? client.username, | |
| 'set UMAAAS_CLIENT_ID environment variable or provide username client option', | |
| ), | |
| UMAAAS_CLIENT_SECRET: requireValue( | |
| readEnv('UMAAAS_CLIENT_SECRET') ?? client.password, | |
| 'set UMAAAS_CLIENT_SECRET environment variable or provide password client option', | |
| ), | |
| UMAAAS_BASE_URL: readEnv('UMAAAS_BASE_URL') ?? client.baseURL ?? undefined, | |
| }; | |
| } catch (e) { | |
| return asErrorResult(e instanceof Error ? e.message : String(e)); | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: mcp-server/src/code-tool.ts
Line: 154-164
Comment:
**Unhandled exception from `requireValue` escapes the tool handler.** When neither the environment variables (`UMAAAS_CLIENT_ID`/`UMAAAS_CLIENT_SECRET`) nor the per-request client credentials are set, `requireValue` throws synchronously. That exception propagates out of `remoteStainlessHandler` through the `handler` closure, through `executeHandler`, and is never caught — the MCP SDK will see an unexpected rejection instead of a well-formed `isError: true` result. The established pattern in this codebase (see `server.ts` lines 119–131 for client init errors) is to wrap failures with `asErrorResult`.
```suggestion
let localClientEnvs: { UMAAAS_CLIENT_ID: string; UMAAAS_CLIENT_SECRET: string; UMAAAS_BASE_URL: string | undefined };
try {
localClientEnvs = {
UMAAAS_CLIENT_ID: requireValue(
readEnv('UMAAAS_CLIENT_ID') ?? client.username,
'set UMAAAS_CLIENT_ID environment variable or provide username client option',
),
UMAAAS_CLIENT_SECRET: requireValue(
readEnv('UMAAAS_CLIENT_SECRET') ?? client.password,
'set UMAAAS_CLIENT_SECRET environment variable or provide password client option',
),
UMAAAS_BASE_URL: readEnv('UMAAAS_BASE_URL') ?? client.baseURL ?? undefined,
};
} catch (e) {
return asErrorResult(e instanceof Error ? e.message : String(e));
}
```
How can I resolve this? If you propose a fix, please make it concise.| switch (scheme) { | ||
| case 'Basic': | ||
| const rawValue = Buffer.from(value, 'base64').toString(); | ||
| return { | ||
| username: rawValue.slice(0, rawValue.search(':')), | ||
| password: rawValue.slice(rawValue.search(':') + 1), | ||
| }; | ||
| default: | ||
| throw new Error( | ||
| 'Unsupported authorization scheme. Expected the "Authorization" header to be a supported scheme (Basic).', | ||
| ); | ||
| } |
There was a problem hiding this comment.
Unsupported Authorization scheme throws an uncaught exception in HTTP mode.
parseClientAuthHeaders is called inside newServer() without a try/catch. When a client sends Authorization: Bearer <token> or any non-Basic scheme, this throws and propagates through Express's async handler. Express 5 will catch it and return a plain-text/HTML 500, not a JSON-RPC error body — MCP clients will be unable to parse the response. Wrapping the case body in braces also fixes the no-case-declarations linting issue (lexical declaration in switch case without a block).
| switch (scheme) { | |
| case 'Basic': | |
| const rawValue = Buffer.from(value, 'base64').toString(); | |
| return { | |
| username: rawValue.slice(0, rawValue.search(':')), | |
| password: rawValue.slice(rawValue.search(':') + 1), | |
| }; | |
| default: | |
| throw new Error( | |
| 'Unsupported authorization scheme. Expected the "Authorization" header to be a supported scheme (Basic).', | |
| ); | |
| } | |
| switch (scheme) { | |
| case 'Basic': { | |
| const rawValue = Buffer.from(value, 'base64').toString(); | |
| return { | |
| username: rawValue.slice(0, rawValue.search(':')), | |
| password: rawValue.slice(rawValue.search(':') + 1), | |
| }; | |
| } | |
| default: | |
| throw new Error( | |
| 'Unsupported authorization scheme. Expected the "Authorization" header to be a supported scheme (Basic).', | |
| ); | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: mcp-server/src/auth.ts
Line: 11-22
Comment:
**Unsupported Authorization scheme throws an uncaught exception in HTTP mode.** `parseClientAuthHeaders` is called inside `newServer()` without a try/catch. When a client sends `Authorization: Bearer <token>` or any non-Basic scheme, this throws and propagates through Express's async handler. Express 5 will catch it and return a plain-text/HTML 500, not a JSON-RPC error body — MCP clients will be unable to parse the response. Wrapping the `case` body in braces also fixes the `no-case-declarations` linting issue (lexical declaration in switch case without a block).
```suggestion
switch (scheme) {
case 'Basic': {
const rawValue = Buffer.from(value, 'base64').toString();
return {
username: rawValue.slice(0, rawValue.search(':')),
password: rawValue.slice(rawValue.search(':') + 1),
};
}
default:
throw new Error(
'Unsupported authorization scheme. Expected the "Authorization" header to be a supported scheme (Basic).',
);
}
```
How can I resolve this? If you propose a fix, please make it concise.| const post = | ||
| (options: { clientOptions: ClientOptions; mcpOptions: McpOptions }) => | ||
| async (req: express.Request, res: express.Response) => { | ||
| const server = await newServer({ ...options, req, res }); | ||
| // If we return null, we already set the authorization error. | ||
| if (server === null) return; | ||
| const transport = new StreamableHTTPServerTransport(); | ||
| await server.connect(transport as any); | ||
| await transport.handleRequest(req, res, req.body); | ||
| }; |
There was a problem hiding this comment.
No Express error middleware means auth errors return an unformatted 500. The
post handler awaits newServer(), which can throw (e.g., unsupported auth scheme from parseClientAuthHeaders). With Express 5 and no custom error handler, the default handler replies with a plain-text/HTML 500 body. MCP clients expecting JSON-RPC cannot parse that response. Adding a catch here converts the thrown error into a proper JSON-RPC error payload.
| const post = | |
| (options: { clientOptions: ClientOptions; mcpOptions: McpOptions }) => | |
| async (req: express.Request, res: express.Response) => { | |
| const server = await newServer({ ...options, req, res }); | |
| // If we return null, we already set the authorization error. | |
| if (server === null) return; | |
| const transport = new StreamableHTTPServerTransport(); | |
| await server.connect(transport as any); | |
| await transport.handleRequest(req, res, req.body); | |
| }; | |
| const post = | |
| (options: { clientOptions: ClientOptions; mcpOptions: McpOptions }) => | |
| async (req: express.Request, res: express.Response) => { | |
| let server: McpServer | null; | |
| try { | |
| server = await newServer({ ...options, req, res }); | |
| } catch (err) { | |
| res.status(400).json({ | |
| jsonrpc: '2.0', | |
| error: { | |
| code: -32600, | |
| message: err instanceof Error ? err.message : String(err), | |
| }, | |
| id: req.body?.id ?? null, | |
| }); | |
| return; | |
| } | |
| // If we return null, we already set the authorization error. | |
| if (server === null) return; | |
| const transport = new StreamableHTTPServerTransport(); | |
| await server.connect(transport as any); | |
| await transport.handleRequest(req, res, req.body); | |
| }; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: mcp-server/src/http.ts
Line: 97-106
Comment:
**No Express error middleware means auth errors return an unformatted 500.** The `post` handler awaits `newServer()`, which can throw (e.g., unsupported auth scheme from `parseClientAuthHeaders`). With Express 5 and no custom error handler, the default handler replies with a plain-text/HTML 500 body. MCP clients expecting JSON-RPC cannot parse that response. Adding a catch here converts the thrown error into a proper JSON-RPC error payload.
```suggestion
const post =
(options: { clientOptions: ClientOptions; mcpOptions: McpOptions }) =>
async (req: express.Request, res: express.Response) => {
let server: McpServer | null;
try {
server = await newServer({ ...options, req, res });
} catch (err) {
res.status(400).json({
jsonrpc: '2.0',
error: {
code: -32600,
message: err instanceof Error ? err.message : String(err),
},
id: req.body?.id ?? null,
});
return;
}
// If we return null, we already set the authorization error.
if (server === null) return;
const transport = new StreamableHTTPServerTransport();
await server.connect(transport as any);
await transport.handleRequest(req, res, req.body);
};
```
How can I resolve this? If you propose a fix, please make it concise.| try { | ||
| execSync('command -v deno', { stdio: 'ignore' }); | ||
| denoPath = 'deno'; | ||
| } catch { |
There was a problem hiding this comment.
command -v deno is a bash built-in and will always fail on Windows. When running in local execution mode on Windows, execSync('command -v deno') throws immediately (the command built-in is not available in cmd.exe or PowerShell), so the outer catch fires. The fallback to node_modules deno then runs, but if the user has deno installed in their Windows PATH it will be silently skipped. Using deno --version is cross-platform.
| try { | |
| execSync('command -v deno', { stdio: 'ignore' }); | |
| denoPath = 'deno'; | |
| } catch { | |
| try { | |
| execSync('deno --version', { stdio: 'ignore' }); | |
| denoPath = 'deno'; | |
| } catch { |
Prompt To Fix With AI
This is a comment left during a code review.
Path: mcp-server/src/code-tool.ts
Line: 235-238
Comment:
**`command -v deno` is a bash built-in and will always fail on Windows.** When running in local execution mode on Windows, `execSync('command -v deno')` throws immediately (the `command` built-in is not available in cmd.exe or PowerShell), so the outer `catch` fires. The fallback to node_modules deno then runs, but if the user has deno installed in their Windows PATH it will be silently skipped. Using `deno --version` is cross-platform.
```suggestion
try {
execSync('deno --version', { stdio: 'ignore' });
denoPath = 'deno';
} catch {
```
How can I resolve this? If you propose a fix, please make it concise.| const docsSearchURL = | ||
| process.env['DOCS_SEARCH_URL'] || 'https://api.stainless.com/api/projects/umaaas/docs/search'; |
There was a problem hiding this comment.
DOCS_SEARCH_URL is read from process.env at module load time, not via readEnv(). Every other environment variable in this package uses the readEnv utility (which handles Deno environments and trims whitespace). Reading process.env directly at import time means the URL is fixed for the process lifetime and the Deno codepath is skipped.
| const docsSearchURL = | |
| process.env['DOCS_SEARCH_URL'] || 'https://api.stainless.com/api/projects/umaaas/docs/search'; | |
| const docsSearchURL = | |
| readEnv('DOCS_SEARCH_URL') || 'https://api.stainless.com/api/projects/umaaas/docs/search'; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: mcp-server/src/docs-search-tool.ts
Line: 44-45
Comment:
**`DOCS_SEARCH_URL` is read from `process.env` at module load time, not via `readEnv()`.** Every other environment variable in this package uses the `readEnv` utility (which handles Deno environments and trims whitespace). Reading `process.env` directly at import time means the URL is fixed for the process lifetime and the Deno codepath is skipped.
```suggestion
const docsSearchURL =
readEnv('DOCS_SEARCH_URL') || 'https://api.stainless.com/api/projects/umaaas/docs/search';
```
How can I resolve this? If you propose a fix, please make it concise.bca2460 to
cbe58d3
Compare
c33b977 to
9c0c2df
Compare

No description provided.