feat(mcp): expand ${VAR} env vars in mcpServers header values (#2148)#2233
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3fd8ca2f4b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!record) return undefined; | ||
| const out: Record<string, string> = {}; | ||
| for (const [k, v] of Object.entries(record)) { | ||
| out[k] = v.replace(/\$\{(?:env:)?([^}]+)\}/g, (match, name: string) => { |
There was a problem hiding this comment.
Support Claude default env syntax in headers
Claude Code's .mcp.json expansion syntax includes ${VAR:-default} for headers (see https://code.claude.com/docs/en/mcp#environment-variable-expansion-in-mcpjson), but this regex captures API_KEY:-fallback as the variable name and looks up that literal key in process.env. Any migrated header such as Authorization: "Bearer ${API_KEY:-dev-token}" is therefore left unexpanded even when API_KEY is set, so the remote MCP server receives the placeholder and authentication still fails.
Useful? React with 👍 / 👎.
…ne#2148) Claude Code .mcp.json files commonly use ${TOKEN_NAME} or ${env:TOKEN_NAME} in header values (e.g. Authorization: Bearer ${API_KEY}). Reasonix was passing these through as literal strings, making migration from Claude Code require manual header editing. Add expandEnvInRecord() which resolves ${VAR} and ${env:VAR} patterns in header records at normalization time. Unset variables are left as-is.
3fd8ca2 to
25e2be3
Compare
|
Fixed in Two new test cases added. @codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
esengine
left a comment
There was a problem hiding this comment.
Clean migration-compat feature. expandEnvInRecord handles ${VAR} / ${env:VAR} / ${VAR:-default} in mcpServers header values, expanding only from process.env (the user's own env → their own config, so no injection surface), and leaves an unset-without-default reference as-is so it surfaces a meaningful error at bridge-connect time rather than silently becoming empty. That's the right call. Test covers it, CI green. Closes the Claude-Code .mcp.json migration gap (#2148). Merging.
问题
关闭 #2148。从 Claude Code 迁移到 Reasonix 时，
.mcp.json里 headers 常见写法：{ "mcpServers": { "my-server": { "url": "https://api.example.com/mcp", "transport": "sse", "headers": { "Authorization": "Bearer ${MY_TOKEN}", "X-Api-Key": "${env:ANOTHER_KEY}" } } } }Reasonix 把这些
${VAR}字面量原样传给服务端，导致认证失败。修改
在
normalizeMcpConfig处理 SSE/streamable-HTTP headers 时，用expandEnvInRecord()展开${VAR_NAME}和${env:VAR_NAME}两种格式。变量未设置则原样保留（保证连接时能看到具体错误）。验证
新增测试：展开、env: 前缀、static 值不受影响、未设置变量原样保留。