feat: MCP {env:VAR} key substitution + coder model/tool-result fixes + docs refactor (v2.7.9)
- MCP secrets: substituteEnvVars recursively resolves {env:NAME} in mcp.json string values from process.env before Zod (opencode-compatible); unset -> '' + boot warning, and invalid-config log names the unset vars (an empty {env:VAR} in a strict url/command field invalidates the whole config)
- data/mcp.json now untracked (.gitignore flips !data/mcp.json -> !data/mcp.example.json); tracked template data/mcp.example.json carries "{env:CONTEXT7_API_KEY}"; .env.example documents the key (9 mcp-config tests)
- Coder fix: message_complete frame model widened string -> string|null (server+web ws-frames parity); dispatcher publishes model: task.model at all 4 external completion points — a null model otherwise fail-closed in publishFrame and dropped the whole frame incl. status:'complete' (regression test)
- Coder fix: claude-sdk mapUserToolResults maps user-message tool_result blocks -> terminal tool_update events (completed/failed w/ output) so tool snapshots resolve instead of spinning forever
- Composer: AgentComposerBar drops §9b resumed/history/new chip + token readout, loses flex-wrap so the row stays one line; CoderPane gains a per-chat localStorage agent-config cache (restores last model on reopen) + threads model into the timeline/chip
- Docs: root CLAUDE.md slimmed (~190 lines), per-app refs split to apps/{coder,server,web}/CLAUDE.md; new docs/coder-backends.md, docs/project-discovery.md, docs/coding-standards/ (cross-app-contract-parity); ARCHITECTURE.md links the backends doc
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
93
apps/server/src/services/__tests__/mcp-config.test.ts
Normal file
93
apps/server/src/services/__tests__/mcp-config.test.ts
Normal file
@@ -0,0 +1,93 @@
|
||||
/**
|
||||
* Unit tests for `{env:VAR}` substitution in the MCP config loader.
|
||||
* Pure — no live MCP server. Verifies secrets resolve from process.env
|
||||
* (so real keys live in `.env`, not the gitignored config file).
|
||||
*/
|
||||
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||||
import { substituteEnvVars } from '../mcp-config.js';
|
||||
|
||||
// Minimal FastifyBaseLogger stub — only .warn is exercised here.
|
||||
function fakeLog() {
|
||||
const warnings: string[] = [];
|
||||
const log = {
|
||||
warn: (msg: unknown) => {
|
||||
warnings.push(typeof msg === 'string' ? msg : JSON.stringify(msg));
|
||||
},
|
||||
};
|
||||
return { log: log as never, warnings };
|
||||
}
|
||||
|
||||
describe('substituteEnvVars', () => {
|
||||
const SAVED = process.env.MCP_TEST_SECRET;
|
||||
beforeEach(() => {
|
||||
process.env.MCP_TEST_SECRET = 'resolved-value';
|
||||
});
|
||||
afterEach(() => {
|
||||
if (SAVED === undefined) delete process.env.MCP_TEST_SECRET;
|
||||
else process.env.MCP_TEST_SECRET = SAVED;
|
||||
delete process.env.MCP_TEST_MISSING;
|
||||
});
|
||||
|
||||
it('replaces a {env:VAR} reference in a string value', () => {
|
||||
const { log } = fakeLog();
|
||||
expect(substituteEnvVars('{env:MCP_TEST_SECRET}', log)).toBe('resolved-value');
|
||||
});
|
||||
|
||||
it('substitutes inside nested objects and arrays', () => {
|
||||
const { log } = fakeLog();
|
||||
const out = substituteEnvVars(
|
||||
{
|
||||
headers: { CONTEXT7_API_KEY: '{env:MCP_TEST_SECRET}' },
|
||||
args: ['--token', '{env:MCP_TEST_SECRET}'],
|
||||
},
|
||||
log,
|
||||
);
|
||||
expect(out).toEqual({
|
||||
headers: { CONTEXT7_API_KEY: 'resolved-value' },
|
||||
args: ['--token', 'resolved-value'],
|
||||
});
|
||||
});
|
||||
|
||||
it('leaves object keys untouched, only transforms values', () => {
|
||||
const { log } = fakeLog();
|
||||
const out = substituteEnvVars({ '{env:MCP_TEST_SECRET}': 'literal' }, log) as Record<string, string>;
|
||||
expect(Object.keys(out)).toEqual(['{env:MCP_TEST_SECRET}']);
|
||||
});
|
||||
|
||||
it('resolves an unset var to empty string and warns', () => {
|
||||
const { log, warnings } = fakeLog();
|
||||
expect(substituteEnvVars('{env:MCP_TEST_MISSING}', log)).toBe('');
|
||||
expect(warnings.some((w) => w.includes('MCP_TEST_MISSING'))).toBe(true);
|
||||
});
|
||||
|
||||
it('passes non-string scalars through unchanged', () => {
|
||||
const { log } = fakeLog();
|
||||
expect(substituteEnvVars(true, log)).toBe(true);
|
||||
expect(substituteEnvVars(42, log)).toBe(42);
|
||||
expect(substituteEnvVars(null, log)).toBe(null);
|
||||
});
|
||||
|
||||
it('leaves strings without a reference unchanged', () => {
|
||||
const { log } = fakeLog();
|
||||
expect(substituteEnvVars('https://mcp.context7.com/mcp', log)).toBe('https://mcp.context7.com/mcp');
|
||||
});
|
||||
|
||||
it('resolves multiple references in one string (global flag)', () => {
|
||||
const { log } = fakeLog();
|
||||
expect(substituteEnvVars('{env:MCP_TEST_SECRET}/{env:MCP_TEST_SECRET}', log)).toBe(
|
||||
'resolved-value/resolved-value',
|
||||
);
|
||||
});
|
||||
|
||||
it('passes an empty string through unchanged', () => {
|
||||
const { log } = fakeLog();
|
||||
expect(substituteEnvVars('', log)).toBe('');
|
||||
});
|
||||
|
||||
it('collects unset var names into the optional collector set', () => {
|
||||
const { log } = fakeLog();
|
||||
const unset = new Set<string>();
|
||||
substituteEnvVars({ url: '{env:MCP_TEST_MISSING}', headers: { k: '{env:MCP_TEST_SECRET}' } }, log, unset);
|
||||
expect([...unset]).toEqual(['MCP_TEST_MISSING']);
|
||||
});
|
||||
});
|
||||
@@ -111,6 +111,19 @@ describe('WsFrameSchema (v1.13.11-a)', () => {
|
||||
expect(result.success).toBe(true);
|
||||
});
|
||||
|
||||
it('accepts a message_complete frame with a null model (external coder, no model selected)', () => {
|
||||
// Regression guard: the dispatcher publishes `model: task.model` (string |
|
||||
// null). When null, this MUST validate or publishFrame fail-closes and drops
|
||||
// the whole frame, incl. the status:'complete' transition.
|
||||
const result = WsFrameSchema.safeParse({
|
||||
type: 'message_complete',
|
||||
message_id: VALID_UUID_A,
|
||||
chat_id: VALID_UUID_B,
|
||||
model: null,
|
||||
});
|
||||
expect(result.success).toBe(true);
|
||||
});
|
||||
|
||||
it('every KNOWN_FRAME_TYPES entry has a discriminated branch', () => {
|
||||
// Probe each known type by attempting a minimal valid construction.
|
||||
// Failure here means the union and the KNOWN_FRAME_TYPES list drifted.
|
||||
|
||||
@@ -4,6 +4,12 @@
|
||||
* Reads a JSON config file (default `/data/mcp.json`) that declares MCP
|
||||
* servers — their transport type, connection parameters, and enabled state.
|
||||
* Schema shape matches opencode's `mcpServers` key for copy-paste compat.
|
||||
*
|
||||
* Secrets stay out of the config file via `{env:VAR}` substitution
|
||||
* (opencode-compatible). Any string value can reference an environment
|
||||
* variable, e.g. a header `"CONTEXT7_API_KEY": "{env:CONTEXT7_API_KEY}"`
|
||||
* resolves from `process.env` at load. This keeps real keys in `.env`
|
||||
* (`env_file` in docker-compose) rather than the gitignored config.
|
||||
*/
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { z } from 'zod';
|
||||
@@ -38,6 +44,49 @@ export interface McpServerEntry {
|
||||
config: McpServerConfig;
|
||||
}
|
||||
|
||||
// ---- Env-var substitution ----
|
||||
|
||||
const ENV_VAR_PATTERN = /\{env:([A-Za-z_][A-Za-z0-9_]*)\}/g;
|
||||
|
||||
/**
|
||||
* Recursively replace `{env:VAR}` references in string values with the
|
||||
* matching environment variable (opencode-compatible). Runs before Zod
|
||||
* validation so a resolved value (e.g. a `{env:...}` URL) still validates.
|
||||
* An unset var resolves to '' and logs a warning so a missing secret is
|
||||
* visible in the boot log rather than silently sending a literal placeholder.
|
||||
* Pass an optional `unsetVars` set to collect the names that resolved to '';
|
||||
* the loader surfaces them on a validation failure (an empty value in a strict
|
||||
* url/command field invalidates the whole config — see loadMcpConfig).
|
||||
*/
|
||||
export function substituteEnvVars(
|
||||
value: unknown,
|
||||
log: FastifyBaseLogger,
|
||||
unsetVars?: Set<string>,
|
||||
): unknown {
|
||||
if (typeof value === 'string') {
|
||||
return value.replace(ENV_VAR_PATTERN, (_match, name: string) => {
|
||||
const resolved = process.env[name];
|
||||
if (resolved === undefined) {
|
||||
unsetVars?.add(name);
|
||||
log.warn(`mcp: env var ${name} referenced in config is unset; substituting empty string`);
|
||||
return '';
|
||||
}
|
||||
return resolved;
|
||||
});
|
||||
}
|
||||
if (Array.isArray(value)) {
|
||||
return value.map((v) => substituteEnvVars(v, log, unsetVars));
|
||||
}
|
||||
if (value && typeof value === 'object') {
|
||||
const out: Record<string, unknown> = {};
|
||||
for (const [k, v] of Object.entries(value as Record<string, unknown>)) {
|
||||
out[k] = substituteEnvVars(v, log, unsetVars);
|
||||
}
|
||||
return out;
|
||||
}
|
||||
return value;
|
||||
}
|
||||
|
||||
// ---- Loader ----
|
||||
|
||||
/**
|
||||
@@ -61,9 +110,19 @@ export function loadMcpConfig(configPath: string, log: FastifyBaseLogger): McpSe
|
||||
return [];
|
||||
}
|
||||
|
||||
const result = McpConfigSchema.safeParse(json);
|
||||
const unsetVars = new Set<string>();
|
||||
const result = McpConfigSchema.safeParse(substituteEnvVars(json, log, unsetVars));
|
||||
if (!result.success) {
|
||||
log.warn({ errors: result.error.flatten().fieldErrors }, `mcp: invalid config at ${configPath}`);
|
||||
// Connect the two otherwise-disconnected warnings: an unset {env:VAR} that
|
||||
// resolved to '' can invalidate a strict field (url/command) and drop the
|
||||
// whole config, so name the unset vars alongside the validation errors.
|
||||
const hint = unsetVars.size
|
||||
? ` — ${unsetVars.size} referenced env var(s) unset & substituted with '' (${[...unsetVars].join(', ')}); an unset {env:VAR} in a url/command field invalidates the whole config`
|
||||
: '';
|
||||
log.warn(
|
||||
{ errors: result.error.flatten().fieldErrors, unsetEnvVars: [...unsetVars] },
|
||||
`mcp: invalid config at ${configPath}${hint}`,
|
||||
);
|
||||
return [];
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user