mirror of
https://github.com/multica-ai/multica.git
synced 2026-08-02 01:45:52 +02:00
MUL-2764: feat(agents): add MCP config tab to agent detail page (#3419)
* MUL-2764: feat(agents): add MCP config tab to agent detail page
Backend already stores `mcp_config` and the daemon forwards it to the
runtime CLI via `--mcp-config`; this only adds the UI entry point.
The new tab presents a JSON editor that pretty-prints the existing
config, validates the buffer on every keystroke, and saves through the
existing `PUT /api/agents/{id}` path. Clearing the editor sends
`mcp_config: null`, which the handler reads as "wipe the column" and
the daemon falls back to the CLI's own default.
When the caller can't see secrets (agent actor, or a non-owner
non-admin member), the server already returns `mcp_config: null` with
`mcp_config_redacted: true`; the tab renders a read-only "configured
but hidden" state in that case so a non-privileged member cannot
silently overwrite an admin-owned config by saving an empty editor.
Co-authored-by: multica-agent <github@multica.ai>
* fix(agents): MCP tab — preserve in-flight edits + warn non-Claude runtimes
- Fix stale-editor sync: compare the local draft against the *previous*
original via a ref, so a background agent refetch updates an untouched
editor instead of being silently ignored. Without this, a draft equal to
the OLD original was treated as user-edited after the prop changed, and
the next Save would write the old config back over a concurrent admin
edit.
- Surface a notice inside the tab when the agent's runtime provider is not
Claude — today's daemon only forwards mcp_config via Claude's
--mcp-config, so saving on e.g. a Codex agent was silent but ineffective.
- Tests for both: rerender resyncs an untouched editor, rerender preserves
an in-flight edit, warning renders on non-Claude / hides on Claude.
MUL-2764
Co-authored-by: multica-agent <github@multica.ai>
* MUL-2764: feat(agents): codex MCP support + hide MCP tab on unsupported runtimes
- Backend: codex.go now translates agent.mcp_config (Claude-style
`{"mcpServers": {...}}`) into `-c mcp_servers.<name>=<inline-toml>`
flags for `codex app-server`, so MCP servers configured in the UI
reach Codex's per-task config layer. Bad mcp_config JSON downgrades
to a warn-and-skip so it can't break the agent launch.
- Frontend: AgentOverviewPane hides the MCP tab when the agent's
runtime provider doesn't read mcp_config — only `claude` and `codex`
are supported today, every other provider sees no MCP tab. The
previous in-tab warning is removed (no longer reachable).
- New shared helper `providerSupportsMcpConfig` lives in
`@multica/core/agents` so views and any future caller share one list
of MCP-aware providers.
- Tests: new go-side coverage for stdio + url + multi-server inputs,
TOML string escaping, malformed-input fallback, and arg ordering vs
custom_args; new views-side coverage for which providers surface the
MCP tab. En + zh-Hans copy and parity test refreshed.
Co-authored-by: multica-agent <github@multica.ai>
* MUL-2764: fix(agents): keep codex mcp_config secrets out of argv/logs
Move the agent's mcp_config from a `-c mcp_servers.<id>=<inline-toml>`
argv flag into a daemon-managed `[mcp_servers.*]` block inside the
per-task `$CODEX_HOME/config.toml`. mcp_servers.<id>.env is a documented
Codex config field and the UI already treats mcp_config as redacted for
non-admins; argv would have leaked those values into `ps aux` and the
`agent command` log line. The file is forced to 0600 to keep secrets in
the daemon owner's lane regardless of the seed file's mode.
Also drop user-supplied `-c/--config mcp_servers.*` entries from
custom_args. Codex `-c` is last-wins (verified against codex-cli 0.132.0),
so without filtering, a custom_args entry could silently shadow whatever
the MCP Tab saved.
Strip inherited `[mcp_servers.*]` tables from the per-task config.toml
when the agent has its own mcp_config, mirroring Claude's
`--strict-mcp-config`: avoids TOML "table already exists" errors on
name collisions and matches admin expectations that the MCP Tab is the
authoritative source for that task.
Co-authored-by: multica-agent <github@multica.ai>
* MUL-2764: fix(agents): codex mcp_config three-state semantics + custom_args compat
Address the third review pass:
1. Distinguish nil vs present-but-empty mcp_config. `{}` and
`{"mcpServers":{}}` now count as "admin saved an explicit (empty)
managed set" — strip inherited user `[mcp_servers.*]` and pin an
empty managed marker block. Only SQL NULL / JSON `null` map to
"absent" and fall back to the user's global `~/.codex/config.toml`.
This aligns Codex with the API's three-state contract (omit / null
/ object) and with Claude's `--strict-mcp-config` semantics.
2. Fail closed on `ensureCodexMcpConfig` errors and on managed
mcp_config without CODEX_HOME. Previous warn-and-launch would
silently inherit the user's global MCP servers and look identical
to a successful apply — exactly the surprise the MCP Tab is meant
to remove.
3. Only filter `-c mcp_servers.*` from `custom_args`/`extra_args`
when the agent has a managed mcp_config. Pre-MUL-2764 agents that
configured MCP via custom_args keep working; once an admin opts
in via the MCP Tab the daemon owns the `mcp_servers` namespace
and overrides are dropped (last-wins safety).
4. Update mcp_config locale intro to mention $CODEX_HOME/config.toml
instead of the now-removed `-c mcp_servers.*` argv path.
Tests:
- Split `TestEnsureCodexMcpConfigEmptyInputsAreNoop` into
`TestEnsureCodexMcpConfigAbsentLeavesUserTablesAlone` (nil/null)
and `TestEnsureCodexMcpConfigEmptyManagedSetStripsUserMcp` (`{}`,
`{"mcpServers":{}}`).
- Add `TestEnsureCodexMcpConfigEmptyManagedSetIdempotent` to pin
byte-identical reruns on the empty managed marker block.
- Add `TestHasManagedCodexMcpConfig` covering the eight relevant
inputs.
- Add `TestBuildCodexArgsPreservesCustomMcpOverridesWhenUnmanaged`
and `TestBuildCodexArgsDropsCustomMcpOverridesWhenManaged` to
pin the new gating.
- Add `TestCodexExecuteFailsClosedWhenMcpConfigInvalid` and
`TestCodexExecuteFailsClosedWhenManagedMcpButNoCodexHome` for the
Execute paths.
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: J <j@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
@@ -7,3 +7,4 @@ export * from "./use-workspace-presence-prefetch";
|
||||
export * from "./constants";
|
||||
export * from "./visibility-label";
|
||||
export * from "./use-workspace-agent-availability";
|
||||
export * from "./mcp-support";
|
||||
|
||||
11
packages/core/agents/mcp-support.ts
Normal file
11
packages/core/agents/mcp-support.ts
Normal file
@@ -0,0 +1,11 @@
|
||||
// The set of runtime providers whose backend reads `agent.mcp_config` and
|
||||
// forwards MCP servers to the underlying CLI. The MCP config tab is hidden
|
||||
// for every other provider so a user can't save a value the runtime will
|
||||
// silently ignore. Keep this list in sync with the backends in
|
||||
// `server/pkg/agent/` that read `ExecOptions.McpConfig`.
|
||||
const MCP_SUPPORTED_PROVIDERS = new Set(["claude", "codex"]);
|
||||
|
||||
export function providerSupportsMcpConfig(provider: string | undefined | null): boolean {
|
||||
if (!provider) return false;
|
||||
return MCP_SUPPORTED_PROVIDERS.has(provider);
|
||||
}
|
||||
@@ -153,6 +153,27 @@ export interface Agent {
|
||||
* alongside `has_custom_env`. Treat `undefined` as zero. MUL-2600.
|
||||
*/
|
||||
custom_env_key_count?: number;
|
||||
/**
|
||||
* MCP server configuration forwarded to the runtime CLI (Claude's
|
||||
* `--mcp-config`). The shape is opaque to the platform — whatever
|
||||
* JSON the CLI accepts, the daemon writes to disk verbatim. `null`
|
||||
* (or the field omitted on legacy backends) means no config; the
|
||||
* daemon falls back to the CLI's own default. MUL-2764.
|
||||
*
|
||||
* When the caller can't see secrets (an agent actor, or a non-owner
|
||||
* non-admin), the server replaces the value with `null` and sets
|
||||
* `mcp_config_redacted` to true so the UI can render a "configured
|
||||
* but hidden" state without exposing potentially sensitive fields.
|
||||
*/
|
||||
mcp_config?: unknown | null;
|
||||
/**
|
||||
* True when the server stripped `mcp_config` from this response
|
||||
* because the caller lacks permission to see secrets. The UI uses
|
||||
* this to distinguish "no config" (`mcp_config === null &&
|
||||
* !mcp_config_redacted`) from "config exists but you can't see it".
|
||||
* Older backends omit this field; treat `undefined` as false.
|
||||
*/
|
||||
mcp_config_redacted?: boolean;
|
||||
visibility: AgentVisibility;
|
||||
status: AgentStatus;
|
||||
max_concurrent_tasks: number;
|
||||
@@ -295,6 +316,15 @@ export interface UpdateAgentRequest {
|
||||
* MUL-2600.
|
||||
*/
|
||||
custom_args?: string[];
|
||||
/**
|
||||
* MCP server configuration. Tri-state semantics (MUL-2764):
|
||||
* - field omitted → no change
|
||||
* - `null` → clear the column; the daemon falls back to the CLI's
|
||||
* built-in default at launch
|
||||
* - object → replace the stored JSON verbatim; the platform does
|
||||
* not validate the shape (MCP CLI accepts whatever it accepts)
|
||||
*/
|
||||
mcp_config?: unknown | null;
|
||||
visibility?: AgentVisibility;
|
||||
status?: AgentStatus;
|
||||
max_concurrent_tasks?: number;
|
||||
|
||||
128
packages/views/agents/components/agent-overview-pane.test.tsx
Normal file
128
packages/views/agents/components/agent-overview-pane.test.tsx
Normal file
@@ -0,0 +1,128 @@
|
||||
// @vitest-environment jsdom
|
||||
|
||||
import { describe, it, expect, vi } from "vitest";
|
||||
import { render, screen } from "@testing-library/react";
|
||||
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||
import type { Agent, AgentRuntime } from "@multica/core/types";
|
||||
import { I18nProvider } from "@multica/core/i18n/react";
|
||||
import enCommon from "../../locales/en/common.json";
|
||||
import enAgents from "../../locales/en/agents.json";
|
||||
|
||||
const TEST_RESOURCES = { en: { common: enCommon, agents: enAgents } };
|
||||
|
||||
// AgentOverviewPane pulls in ActorIssuesPanel which in turn touches the api
|
||||
// layer. The test only cares about which top-of-pane tab buttons render,
|
||||
// not what each tab does, so we stub the heavy children.
|
||||
vi.mock("./tabs/activity-tab", () => ({
|
||||
ActivityTab: () => <div>activity-tab</div>,
|
||||
}));
|
||||
vi.mock("./tabs/instructions-tab", () => ({
|
||||
InstructionsTab: () => <div>instructions-tab</div>,
|
||||
}));
|
||||
vi.mock("./tabs/skills-tab", () => ({
|
||||
SkillsTab: () => <div>skills-tab</div>,
|
||||
}));
|
||||
vi.mock("./tabs/env-tab", () => ({
|
||||
EnvTab: () => <div>env-tab</div>,
|
||||
}));
|
||||
vi.mock("./tabs/custom-args-tab", () => ({
|
||||
CustomArgsTab: () => <div>custom-args-tab</div>,
|
||||
}));
|
||||
vi.mock("./tabs/mcp-config-tab", () => ({
|
||||
McpConfigTab: () => <div>mcp-config-tab</div>,
|
||||
}));
|
||||
vi.mock("../../common/actor-issues-panel", () => ({
|
||||
ActorIssuesPanel: () => <div>actor-issues-panel</div>,
|
||||
}));
|
||||
|
||||
import { AgentOverviewPane } from "./agent-overview-pane";
|
||||
|
||||
const baseAgent: Agent = {
|
||||
id: "agent-1",
|
||||
workspace_id: "ws-1",
|
||||
runtime_id: "runtime-1",
|
||||
name: "Agent",
|
||||
description: "",
|
||||
instructions: "",
|
||||
avatar_url: null,
|
||||
runtime_mode: "local",
|
||||
runtime_config: {},
|
||||
custom_args: [],
|
||||
visibility: "workspace",
|
||||
status: "idle",
|
||||
max_concurrent_tasks: 1,
|
||||
model: "",
|
||||
owner_id: "user-1",
|
||||
skills: [],
|
||||
created_at: "2026-05-28T00:00:00Z",
|
||||
updated_at: "2026-05-28T00:00:00Z",
|
||||
archived_at: null,
|
||||
archived_by: null,
|
||||
};
|
||||
|
||||
function makeRuntime(provider: string): AgentRuntime {
|
||||
return {
|
||||
id: "runtime-1",
|
||||
workspace_id: "ws-1",
|
||||
daemon_id: null,
|
||||
name: "Runtime",
|
||||
runtime_mode: "local",
|
||||
provider,
|
||||
launch_header: "",
|
||||
status: "online",
|
||||
device_info: "",
|
||||
metadata: {},
|
||||
owner_id: null,
|
||||
visibility: "private",
|
||||
last_seen_at: null,
|
||||
created_at: "2026-05-28T00:00:00Z",
|
||||
updated_at: "2026-05-28T00:00:00Z",
|
||||
};
|
||||
}
|
||||
|
||||
function renderPane(runtimes: AgentRuntime[]) {
|
||||
const queryClient = new QueryClient({
|
||||
defaultOptions: { queries: { retry: false } },
|
||||
});
|
||||
return render(
|
||||
<I18nProvider locale="en" resources={TEST_RESOURCES}>
|
||||
<QueryClientProvider client={queryClient}>
|
||||
<AgentOverviewPane
|
||||
agent={baseAgent}
|
||||
runtimes={runtimes}
|
||||
onUpdate={vi.fn().mockResolvedValue(undefined)}
|
||||
/>
|
||||
</QueryClientProvider>
|
||||
</I18nProvider>,
|
||||
);
|
||||
}
|
||||
|
||||
describe("AgentOverviewPane MCP tab visibility", () => {
|
||||
it("renders the MCP tab when the agent runs on the Claude runtime", () => {
|
||||
renderPane([makeRuntime("claude")]);
|
||||
expect(screen.getByRole("button", { name: /^MCP$/i })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("renders the MCP tab when the agent runs on the Codex runtime", () => {
|
||||
// Codex now reads mcp_config too — must not be hidden from the tab strip.
|
||||
renderPane([makeRuntime("codex")]);
|
||||
expect(screen.getByRole("button", { name: /^MCP$/i })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("hides the MCP tab for providers whose backend does not read mcp_config", () => {
|
||||
// Saving an MCP config on e.g. Gemini would be a silent no-op at run
|
||||
// time — that's the bug this hiding logic is meant to prevent.
|
||||
renderPane([makeRuntime("gemini")]);
|
||||
expect(
|
||||
screen.queryByRole("button", { name: /^MCP$/i }),
|
||||
).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("keeps the MCP tab visible when the runtime row hasn't loaded yet", () => {
|
||||
// Empty runtimes[] mimics the brief window between the page mounting and
|
||||
// the runtimes query resolving. Hiding the tab would flicker it off and
|
||||
// then back on, which reads as a bug.
|
||||
renderPane([]);
|
||||
expect(screen.getByRole("button", { name: /^MCP$/i })).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
@@ -1,15 +1,17 @@
|
||||
"use client";
|
||||
|
||||
import { useState } from "react";
|
||||
import { useMemo, useState } from "react";
|
||||
import {
|
||||
Activity,
|
||||
BookOpenText,
|
||||
FileText,
|
||||
KeyRound,
|
||||
ListTodo,
|
||||
Plug,
|
||||
Terminal,
|
||||
} from "lucide-react";
|
||||
import type { Agent, AgentRuntime } from "@multica/core/types";
|
||||
import { providerSupportsMcpConfig } from "@multica/core/agents";
|
||||
import {
|
||||
AlertDialog,
|
||||
AlertDialogAction,
|
||||
@@ -25,6 +27,7 @@ import { InstructionsTab } from "./tabs/instructions-tab";
|
||||
import { SkillsTab } from "./tabs/skills-tab";
|
||||
import { EnvTab } from "./tabs/env-tab";
|
||||
import { CustomArgsTab } from "./tabs/custom-args-tab";
|
||||
import { McpConfigTab } from "./tabs/mcp-config-tab";
|
||||
import { ActorIssuesPanel } from "../../common/actor-issues-panel";
|
||||
import { useT } from "../../i18n";
|
||||
|
||||
@@ -34,15 +37,17 @@ type DetailTab =
|
||||
| "instructions"
|
||||
| "skills"
|
||||
| "env"
|
||||
| "custom_args";
|
||||
| "custom_args"
|
||||
| "mcp_config";
|
||||
|
||||
const TAB_LABEL_KEY: Record<DetailTab, "activity" | "tasks" | "instructions" | "skills" | "environment" | "custom_args"> = {
|
||||
const TAB_LABEL_KEY: Record<DetailTab, "activity" | "tasks" | "instructions" | "skills" | "environment" | "custom_args" | "mcp_config"> = {
|
||||
activity: "activity",
|
||||
tasks: "tasks",
|
||||
instructions: "instructions",
|
||||
skills: "skills",
|
||||
env: "environment",
|
||||
custom_args: "custom_args",
|
||||
mcp_config: "mcp_config",
|
||||
};
|
||||
|
||||
const detailTabs: {
|
||||
@@ -55,6 +60,7 @@ const detailTabs: {
|
||||
{ id: "skills", icon: BookOpenText },
|
||||
{ id: "env", icon: KeyRound },
|
||||
{ id: "custom_args", icon: Terminal },
|
||||
{ id: "mcp_config", icon: Plug },
|
||||
];
|
||||
|
||||
interface AgentOverviewPaneProps {
|
||||
@@ -103,6 +109,24 @@ export function AgentOverviewPane({
|
||||
? runtimes.find((r) => r.id === agent.runtime_id) ?? null
|
||||
: null;
|
||||
|
||||
// The MCP tab is only shown when the agent's runtime backend actually
|
||||
// consumes mcp_config — see providerSupportsMcpConfig. We default to
|
||||
// showing it when the runtime row hasn't loaded yet so a slow fetch
|
||||
// can't transiently flicker the tab off and then on.
|
||||
const visibleTabs = useMemo(() => {
|
||||
const showMcp = runtime ? providerSupportsMcpConfig(runtime.provider) : true;
|
||||
return detailTabs.filter((tab) => tab.id !== "mcp_config" || showMcp);
|
||||
}, [runtime]);
|
||||
|
||||
// If the active tab disappears (e.g. user just switched the agent's
|
||||
// runtime to one that doesn't read mcp_config), fall back to Activity
|
||||
// for this render so the pane is never empty. The user's stored
|
||||
// activeTab is left alone — switching back to a supporting runtime
|
||||
// brings their selection back.
|
||||
const effectiveTab: DetailTab = visibleTabs.some((tab) => tab.id === activeTab)
|
||||
? activeTab
|
||||
: "activity";
|
||||
|
||||
const requestTabChange = (next: DetailTab) => {
|
||||
if (next === activeTab) return;
|
||||
if (activeDirty) {
|
||||
@@ -129,13 +153,13 @@ export function AgentOverviewPane({
|
||||
// the grid-driven full-height behavior on tablet and up.
|
||||
<div className="flex min-h-[60vh] flex-col overflow-hidden rounded-lg border bg-background md:h-full md:min-h-0">
|
||||
<div className="flex shrink-0 items-center gap-0 overflow-x-auto border-b px-2 md:px-4">
|
||||
{detailTabs.map((tab) => (
|
||||
{visibleTabs.map((tab) => (
|
||||
<button
|
||||
key={tab.id}
|
||||
type="button"
|
||||
onClick={() => requestTabChange(tab.id)}
|
||||
className={`flex shrink-0 items-center gap-1.5 whitespace-nowrap border-b-2 px-3 py-2.5 text-xs font-medium transition-colors ${
|
||||
activeTab === tab.id
|
||||
effectiveTab === tab.id
|
||||
? "border-foreground text-foreground"
|
||||
: "border-transparent text-muted-foreground hover:text-foreground"
|
||||
}`}
|
||||
@@ -147,13 +171,13 @@ export function AgentOverviewPane({
|
||||
</div>
|
||||
|
||||
<div className="flex-1 min-h-0 overflow-y-auto">
|
||||
{activeTab === "activity" && <ActivityTab agent={agent} />}
|
||||
{activeTab === "tasks" && (
|
||||
{effectiveTab === "activity" && <ActivityTab agent={agent} />}
|
||||
{effectiveTab === "tasks" && (
|
||||
<div className="flex h-full min-h-[520px] flex-col">
|
||||
<ActorIssuesPanel actorType="agent" actorId={agent.id} />
|
||||
</div>
|
||||
)}
|
||||
{activeTab === "instructions" && (
|
||||
{effectiveTab === "instructions" && (
|
||||
<TabContent>
|
||||
<InstructionsTab
|
||||
agent={agent}
|
||||
@@ -162,12 +186,12 @@ export function AgentOverviewPane({
|
||||
/>
|
||||
</TabContent>
|
||||
)}
|
||||
{activeTab === "skills" && (
|
||||
{effectiveTab === "skills" && (
|
||||
<TabContent>
|
||||
<SkillsTab agent={agent} />
|
||||
</TabContent>
|
||||
)}
|
||||
{activeTab === "env" && (
|
||||
{effectiveTab === "env" && (
|
||||
<TabContent>
|
||||
<EnvTab
|
||||
agent={agent}
|
||||
@@ -175,7 +199,7 @@ export function AgentOverviewPane({
|
||||
/>
|
||||
</TabContent>
|
||||
)}
|
||||
{activeTab === "custom_args" && (
|
||||
{effectiveTab === "custom_args" && (
|
||||
<TabContent>
|
||||
<CustomArgsTab
|
||||
agent={agent}
|
||||
@@ -185,6 +209,15 @@ export function AgentOverviewPane({
|
||||
/>
|
||||
</TabContent>
|
||||
)}
|
||||
{effectiveTab === "mcp_config" && (
|
||||
<TabContent>
|
||||
<McpConfigTab
|
||||
agent={agent}
|
||||
onSave={(updates) => onUpdate(agent.id, updates)}
|
||||
onDirtyChange={setActiveDirty}
|
||||
/>
|
||||
</TabContent>
|
||||
)}
|
||||
</div>
|
||||
|
||||
{pendingTab !== null && (
|
||||
|
||||
222
packages/views/agents/components/tabs/mcp-config-tab.test.tsx
Normal file
222
packages/views/agents/components/tabs/mcp-config-tab.test.tsx
Normal file
@@ -0,0 +1,222 @@
|
||||
// @vitest-environment jsdom
|
||||
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { fireEvent, render, screen } from "@testing-library/react";
|
||||
import userEvent from "@testing-library/user-event";
|
||||
import type { Agent } from "@multica/core/types";
|
||||
import { I18nProvider } from "@multica/core/i18n/react";
|
||||
import enCommon from "../../../locales/en/common.json";
|
||||
import enAgents from "../../../locales/en/agents.json";
|
||||
|
||||
const TEST_RESOURCES = { en: { common: enCommon, agents: enAgents } };
|
||||
|
||||
vi.mock("sonner", () => ({
|
||||
toast: {
|
||||
error: vi.fn(),
|
||||
success: vi.fn(),
|
||||
},
|
||||
}));
|
||||
|
||||
import { McpConfigTab } from "./mcp-config-tab";
|
||||
|
||||
const baseAgent: Agent = {
|
||||
id: "agent-1",
|
||||
workspace_id: "ws-1",
|
||||
runtime_id: "runtime-1",
|
||||
name: "Agent",
|
||||
description: "",
|
||||
instructions: "",
|
||||
avatar_url: null,
|
||||
runtime_mode: "local",
|
||||
runtime_config: {},
|
||||
custom_args: [],
|
||||
visibility: "workspace",
|
||||
status: "idle",
|
||||
max_concurrent_tasks: 1,
|
||||
model: "",
|
||||
owner_id: "user-1",
|
||||
skills: [],
|
||||
created_at: "2026-05-28T00:00:00Z",
|
||||
updated_at: "2026-05-28T00:00:00Z",
|
||||
archived_at: null,
|
||||
archived_by: null,
|
||||
};
|
||||
|
||||
function renderTab(
|
||||
overrides: Partial<Agent> = {},
|
||||
onSave = vi.fn().mockResolvedValue(undefined),
|
||||
) {
|
||||
const agent = { ...baseAgent, ...overrides };
|
||||
const result = render(
|
||||
<I18nProvider locale="en" resources={TEST_RESOURCES}>
|
||||
<McpConfigTab agent={agent} onSave={onSave} />
|
||||
</I18nProvider>,
|
||||
);
|
||||
return { ...result, onSave };
|
||||
}
|
||||
|
||||
describe("McpConfigTab", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("renders a read-only redacted state when the server omitted the value", () => {
|
||||
// mcp_config_redacted means the server knows there IS a config but
|
||||
// hid it from this caller. The tab must NOT expose the editor or
|
||||
// any input — even an empty textarea would let a non-privileged
|
||||
// member silently overwrite an admin-owned config on save.
|
||||
renderTab({ mcp_config: null, mcp_config_redacted: true });
|
||||
|
||||
expect(screen.getByText(/hidden from your view/i)).toBeInTheDocument();
|
||||
expect(screen.queryByRole("textbox")).not.toBeInTheDocument();
|
||||
expect(screen.queryByRole("button", { name: /save/i })).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("shows the editor empty when no config is set, and Save stays disabled", () => {
|
||||
renderTab({ mcp_config: null });
|
||||
|
||||
const editor = screen.getByLabelText(/MCP config JSON editor/i) as HTMLTextAreaElement;
|
||||
expect(editor.value).toBe("");
|
||||
|
||||
expect(screen.getByRole("button", { name: /save/i })).toBeDisabled();
|
||||
});
|
||||
|
||||
it("pretty-prints the existing config and saves a parsed object", async () => {
|
||||
const user = userEvent.setup();
|
||||
const stored = { mcpServers: { fetch: { command: "uvx" } } };
|
||||
const { onSave } = renderTab({ mcp_config: stored });
|
||||
|
||||
const editor = screen.getByLabelText(/MCP config JSON editor/i) as HTMLTextAreaElement;
|
||||
expect(editor.value).toBe(JSON.stringify(stored, null, 2));
|
||||
|
||||
// userEvent.type interprets `{` / `[` as keyboard modifiers, so a
|
||||
// raw JSON paste goes through fireEvent.change instead — the same
|
||||
// path the browser uses when the user pastes.
|
||||
const replacement = JSON.stringify({
|
||||
mcpServers: { fetch: { command: "npx" } },
|
||||
});
|
||||
fireEvent.change(editor, { target: { value: replacement } });
|
||||
|
||||
const save = screen.getByRole("button", { name: /save/i });
|
||||
expect(save).toBeEnabled();
|
||||
await user.click(save);
|
||||
|
||||
expect(onSave).toHaveBeenCalledTimes(1);
|
||||
// We pass the parsed object, not the raw string, so the backend
|
||||
// gets a real JSON shape and not an escaped string.
|
||||
expect(onSave).toHaveBeenCalledWith({
|
||||
mcp_config: { mcpServers: { fetch: { command: "npx" } } },
|
||||
});
|
||||
});
|
||||
|
||||
it("clearing the editor saves null to wipe the column", async () => {
|
||||
const user = userEvent.setup();
|
||||
const { onSave } = renderTab({ mcp_config: { mcpServers: {} } });
|
||||
|
||||
const editor = screen.getByLabelText(/MCP config JSON editor/i) as HTMLTextAreaElement;
|
||||
await user.clear(editor);
|
||||
|
||||
const save = screen.getByRole("button", { name: /save/i });
|
||||
await user.click(save);
|
||||
|
||||
// null is what the backend reads as "clear this column" — sending
|
||||
// an empty string or {} would either fail validation or store an
|
||||
// empty object, both of which surprise the user.
|
||||
expect(onSave).toHaveBeenCalledWith({ mcp_config: null });
|
||||
});
|
||||
|
||||
it("disables Save and surfaces an inline error on invalid JSON", () => {
|
||||
const { onSave } = renderTab({ mcp_config: null });
|
||||
|
||||
const editor = screen.getByLabelText(/MCP config JSON editor/i);
|
||||
fireEvent.change(editor, { target: { value: "{ not json" } });
|
||||
|
||||
expect(screen.getByText(/Invalid JSON/i)).toBeInTheDocument();
|
||||
expect(screen.getByRole("button", { name: /save/i })).toBeDisabled();
|
||||
expect(onSave).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects top-level arrays and primitives", () => {
|
||||
renderTab({ mcp_config: null });
|
||||
|
||||
const editor = screen.getByLabelText(/MCP config JSON editor/i);
|
||||
fireEvent.change(editor, { target: { value: "[1,2,3]" } });
|
||||
|
||||
expect(
|
||||
screen.getByText(/MCP config must be a JSON object/i),
|
||||
).toBeInTheDocument();
|
||||
expect(screen.getByRole("button", { name: /save/i })).toBeDisabled();
|
||||
});
|
||||
|
||||
it("syncs the editor to a refreshed agent prop when the user hasn't edited", () => {
|
||||
// Reproduces the stale-editor bug: a background refetch / WS event swaps
|
||||
// in a newer `agent.mcp_config`, and the editor must follow it (so the
|
||||
// next Save writes the new value, not the old one). Comparing the draft
|
||||
// against the *previous* original — not the new one — is what makes this
|
||||
// work. Without the ref, the effect would self-defeat: on re-render the
|
||||
// draft already equals the new original, the equality check is true,
|
||||
// but the conditional only re-assigns `original` to itself, so a draft
|
||||
// that started life equal to the OLD original is never touched.
|
||||
const initial = { mcpServers: { fetch: { command: "uvx" } } };
|
||||
const updated = { mcpServers: { fetch: { command: "npx" } } };
|
||||
const agent = { ...baseAgent, mcp_config: initial };
|
||||
|
||||
const { rerender } = render(
|
||||
<I18nProvider locale="en" resources={TEST_RESOURCES}>
|
||||
<McpConfigTab agent={agent} onSave={vi.fn()} />
|
||||
</I18nProvider>,
|
||||
);
|
||||
|
||||
const editor = screen.getByLabelText(
|
||||
/MCP config JSON editor/i,
|
||||
) as HTMLTextAreaElement;
|
||||
expect(editor.value).toBe(JSON.stringify(initial, null, 2));
|
||||
|
||||
rerender(
|
||||
<I18nProvider locale="en" resources={TEST_RESOURCES}>
|
||||
<McpConfigTab
|
||||
agent={{ ...agent, mcp_config: updated }}
|
||||
onSave={vi.fn()}
|
||||
/>
|
||||
</I18nProvider>,
|
||||
);
|
||||
|
||||
// Editor follows the new prop and the dirty hint is NOT shown — if it
|
||||
// were, the next Save would write the *old* JSON back over the new one.
|
||||
expect(editor.value).toBe(JSON.stringify(updated, null, 2));
|
||||
expect(screen.queryByText(/unsaved changes/i)).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("preserves an in-flight edit when the agent prop is refreshed underneath", () => {
|
||||
// The mirror of the test above: if the user IS editing, a background
|
||||
// refresh must not clobber their draft.
|
||||
const initial = { mcpServers: { fetch: { command: "uvx" } } };
|
||||
const updated = { mcpServers: { fetch: { command: "npx" } } };
|
||||
const agent = { ...baseAgent, mcp_config: initial };
|
||||
|
||||
const { rerender } = render(
|
||||
<I18nProvider locale="en" resources={TEST_RESOURCES}>
|
||||
<McpConfigTab agent={agent} onSave={vi.fn()} />
|
||||
</I18nProvider>,
|
||||
);
|
||||
|
||||
const editor = screen.getByLabelText(
|
||||
/MCP config JSON editor/i,
|
||||
) as HTMLTextAreaElement;
|
||||
const draft = JSON.stringify({ mcpServers: { fetch: { command: "wip" } } });
|
||||
fireEvent.change(editor, { target: { value: draft } });
|
||||
expect(editor.value).toBe(draft);
|
||||
|
||||
rerender(
|
||||
<I18nProvider locale="en" resources={TEST_RESOURCES}>
|
||||
<McpConfigTab
|
||||
agent={{ ...agent, mcp_config: updated }}
|
||||
onSave={vi.fn()}
|
||||
/>
|
||||
</I18nProvider>,
|
||||
);
|
||||
|
||||
expect(editor.value).toBe(draft);
|
||||
});
|
||||
|
||||
});
|
||||
187
packages/views/agents/components/tabs/mcp-config-tab.tsx
Normal file
187
packages/views/agents/components/tabs/mcp-config-tab.tsx
Normal file
@@ -0,0 +1,187 @@
|
||||
"use client";
|
||||
|
||||
import { useEffect, useMemo, useRef, useState } from "react";
|
||||
import { Eraser, Loader2, Lock, Save } from "lucide-react";
|
||||
import type { Agent } from "@multica/core/types";
|
||||
import { Button } from "@multica/ui/components/ui/button";
|
||||
import { Textarea } from "@multica/ui/components/ui/textarea";
|
||||
import { toast } from "sonner";
|
||||
import { useT } from "../../../i18n";
|
||||
|
||||
// `null` and the empty string are the two ways the user can mean "no
|
||||
// config" — the server stores either as a NULL column and the daemon
|
||||
// falls back to the runtime CLI default at launch. We normalise to
|
||||
// the empty string in the editor so the dirty check has one canonical
|
||||
// form to compare against.
|
||||
function configToText(value: unknown): string {
|
||||
if (value === null || value === undefined) return "";
|
||||
return JSON.stringify(value, null, 2);
|
||||
}
|
||||
|
||||
export function McpConfigTab({
|
||||
agent,
|
||||
onSave,
|
||||
onDirtyChange,
|
||||
}: {
|
||||
agent: Agent;
|
||||
onSave: (updates: { mcp_config: unknown | null }) => Promise<void>;
|
||||
onDirtyChange?: (dirty: boolean) => void;
|
||||
}) {
|
||||
const { t } = useT("agents");
|
||||
|
||||
const redacted = agent.mcp_config_redacted === true;
|
||||
const original = useMemo(() => configToText(agent.mcp_config), [agent.mcp_config]);
|
||||
const [text, setText] = useState(original);
|
||||
const [saving, setSaving] = useState(false);
|
||||
|
||||
// Sync local draft when the agent prop changes (e.g. after a successful
|
||||
// save invalidates the cache and a fresh agent arrives). We only sync
|
||||
// when the user has no in-flight edits — comparing the current draft
|
||||
// against the *previous* original (not the new one) is what tells us
|
||||
// "they haven't touched this since the last sync". Comparing against
|
||||
// the new original would skip the sync whenever the server-side value
|
||||
// changes underneath an untouched draft, leaving the editor showing a
|
||||
// stale value that a later Save would write back, clobbering another
|
||||
// admin's edit.
|
||||
const previousOriginalRef = useRef(original);
|
||||
useEffect(() => {
|
||||
setText((current) =>
|
||||
current === previousOriginalRef.current ? original : current,
|
||||
);
|
||||
previousOriginalRef.current = original;
|
||||
}, [original]);
|
||||
|
||||
const trimmed = text.trim();
|
||||
const parseResult = useMemo<
|
||||
| { ok: true; value: unknown | null }
|
||||
| { ok: false; error: string }
|
||||
>(() => {
|
||||
if (trimmed === "") return { ok: true, value: null };
|
||||
try {
|
||||
const value = JSON.parse(trimmed);
|
||||
// The MCP CLI accepts an object (`{"mcpServers": …}`); a top-level
|
||||
// array or primitive is almost certainly a user mistake, so reject
|
||||
// here rather than surprise them with a server-side error later.
|
||||
if (value === null || typeof value !== "object" || Array.isArray(value)) {
|
||||
return {
|
||||
ok: false,
|
||||
error: "mcp_config_not_object",
|
||||
};
|
||||
}
|
||||
return { ok: true, value };
|
||||
} catch (err) {
|
||||
return {
|
||||
ok: false,
|
||||
error: err instanceof Error ? err.message : "invalid JSON",
|
||||
};
|
||||
}
|
||||
}, [trimmed]);
|
||||
|
||||
const dirty = text !== original;
|
||||
|
||||
useEffect(() => {
|
||||
onDirtyChange?.(dirty);
|
||||
}, [dirty, onDirtyChange]);
|
||||
|
||||
if (redacted) {
|
||||
return (
|
||||
<div className="space-y-3">
|
||||
<p className="flex items-center gap-2 text-sm font-medium">
|
||||
<Lock className="h-3.5 w-3.5 text-muted-foreground" />
|
||||
{t(($) => $.tab_body.mcp_config.redacted_title)}
|
||||
</p>
|
||||
<p className="text-xs text-muted-foreground">
|
||||
{t(($) => $.tab_body.mcp_config.redacted_hint)}
|
||||
</p>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
const handleSave = async () => {
|
||||
if (!parseResult.ok) return;
|
||||
setSaving(true);
|
||||
try {
|
||||
await onSave({ mcp_config: parseResult.value });
|
||||
// Normalise the editor to the pretty-printed canonical form so the
|
||||
// dirty check stops firing after a successful save (the user's
|
||||
// raw input may differ from what configToText would emit).
|
||||
setText(configToText(parseResult.value));
|
||||
toast.success(t(($) => $.tab_body.mcp_config.saved_toast));
|
||||
} catch (err) {
|
||||
toast.error(
|
||||
err instanceof Error && err.message
|
||||
? err.message
|
||||
: t(($) => $.tab_body.mcp_config.save_failed_toast),
|
||||
);
|
||||
} finally {
|
||||
setSaving(false);
|
||||
}
|
||||
};
|
||||
|
||||
const handleClear = () => {
|
||||
setText("");
|
||||
};
|
||||
|
||||
const showInvalid = trimmed !== "" && !parseResult.ok;
|
||||
const invalidMessage = !parseResult.ok && parseResult.error === "mcp_config_not_object"
|
||||
? t(($) => $.tab_body.mcp_config.invalid_not_object)
|
||||
: !parseResult.ok
|
||||
? t(($) => $.tab_body.mcp_config.invalid_json, { error: parseResult.error })
|
||||
: "";
|
||||
|
||||
return (
|
||||
<div className="flex h-full flex-col space-y-3">
|
||||
<div className="flex items-start justify-between gap-3">
|
||||
<p className="text-xs text-muted-foreground">
|
||||
{t(($) => $.tab_body.mcp_config.intro)}
|
||||
</p>
|
||||
{trimmed !== "" && (
|
||||
<Button
|
||||
type="button"
|
||||
variant="outline"
|
||||
size="sm"
|
||||
onClick={handleClear}
|
||||
className="shrink-0"
|
||||
>
|
||||
<Eraser className="h-3 w-3" />
|
||||
{t(($) => $.tab_body.mcp_config.clear_action)}
|
||||
</Button>
|
||||
)}
|
||||
</div>
|
||||
|
||||
<Textarea
|
||||
value={text}
|
||||
onChange={(e) => setText(e.target.value)}
|
||||
placeholder={t(($) => $.tab_body.mcp_config.placeholder)}
|
||||
aria-invalid={showInvalid || undefined}
|
||||
aria-label={t(($) => $.tab_body.mcp_config.editor_aria)}
|
||||
spellCheck={false}
|
||||
className="min-h-[240px] flex-1 font-mono text-xs"
|
||||
/>
|
||||
|
||||
{showInvalid && (
|
||||
<p className="text-xs text-destructive">{invalidMessage}</p>
|
||||
)}
|
||||
|
||||
<div className="flex items-center justify-end gap-3">
|
||||
{dirty && (
|
||||
<span className="text-xs text-muted-foreground">
|
||||
{t(($) => $.tab_body.common.unsaved_changes)}
|
||||
</span>
|
||||
)}
|
||||
<Button
|
||||
onClick={handleSave}
|
||||
disabled={!dirty || !parseResult.ok || saving}
|
||||
size="sm"
|
||||
>
|
||||
{saving ? (
|
||||
<Loader2 className="h-3.5 w-3.5 animate-spin" />
|
||||
) : (
|
||||
<Save className="h-3.5 w-3.5" />
|
||||
)}
|
||||
{t(($) => $.tab_body.common.save)}
|
||||
</Button>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
@@ -195,6 +195,7 @@
|
||||
"skills": "Skills",
|
||||
"environment": "Environment",
|
||||
"custom_args": "Custom Args",
|
||||
"mcp_config": "MCP",
|
||||
"discard_dialog_title": "Discard unsaved changes?",
|
||||
"discard_dialog_description": "You have unsaved changes in this tab. Leaving now will discard them.",
|
||||
"discard_keep": "Keep editing",
|
||||
@@ -294,6 +295,18 @@
|
||||
"saved_toast": "Custom arguments saved",
|
||||
"save_failed_toast": "Failed to save custom arguments"
|
||||
},
|
||||
"mcp_config": {
|
||||
"intro": "MCP server configuration forwarded to the runtime CLI (Claude via --mcp-config, Codex via $CODEX_HOME/config.toml). Stored verbatim and may contain secrets — only the agent owner and workspace admins can read it. Leave empty to fall back to the CLI's own default.",
|
||||
"placeholder": "{\n \"mcpServers\": {\n \"fetch\": {\n \"command\": \"uvx\",\n \"args\": [\"mcp-server-fetch\"]\n }\n }\n}",
|
||||
"editor_aria": "MCP config JSON editor",
|
||||
"clear_action": "Clear",
|
||||
"invalid_json": "Invalid JSON: {{error}}",
|
||||
"invalid_not_object": "MCP config must be a JSON object (e.g. {\"mcpServers\": …}).",
|
||||
"saved_toast": "MCP config saved",
|
||||
"save_failed_toast": "Failed to save MCP config",
|
||||
"redacted_title": "Configured — hidden from your view",
|
||||
"redacted_hint": "Only the agent owner or a workspace admin can read this config."
|
||||
},
|
||||
"skills": {
|
||||
"intro": "Workspace skills assigned to this agent. Local runtime skills are always available automatically.",
|
||||
"add_action": "Add skill",
|
||||
|
||||
@@ -191,6 +191,7 @@
|
||||
"skills": "Skills",
|
||||
"environment": "环境变量",
|
||||
"custom_args": "自定义参数",
|
||||
"mcp_config": "MCP",
|
||||
"discard_dialog_title": "放弃未保存的修改?",
|
||||
"discard_dialog_description": "当前 tab 有未保存的修改,离开会丢弃这些修改。",
|
||||
"discard_keep": "继续编辑",
|
||||
@@ -288,6 +289,18 @@
|
||||
"saved_toast": "已保存自定义参数",
|
||||
"save_failed_toast": "保存自定义参数失败"
|
||||
},
|
||||
"mcp_config": {
|
||||
"intro": "转发给运行时 CLI 的 MCP 服务器配置(Claude 通过 --mcp-config,Codex 通过 $CODEX_HOME/config.toml)。原样保存,可能包含密钥 —— 只有智能体所有者和工作区管理员可以读取。留空则回退到 CLI 自身的默认设置。",
|
||||
"placeholder": "{\n \"mcpServers\": {\n \"fetch\": {\n \"command\": \"uvx\",\n \"args\": [\"mcp-server-fetch\"]\n }\n }\n}",
|
||||
"editor_aria": "MCP 配置 JSON 编辑器",
|
||||
"clear_action": "清空",
|
||||
"invalid_json": "JSON 无效:{{error}}",
|
||||
"invalid_not_object": "MCP 配置必须是 JSON 对象(例如 {\"mcpServers\": …})。",
|
||||
"saved_toast": "已保存 MCP 配置",
|
||||
"save_failed_toast": "保存 MCP 配置失败",
|
||||
"redacted_title": "已配置 —— 当前账号无权查看",
|
||||
"redacted_hint": "只有智能体所有者或工作区管理员可以读取该配置。"
|
||||
},
|
||||
"skills": {
|
||||
"intro": "分配给该智能体的工作区 skill。本地运行时 skill 会自动可用。",
|
||||
"add_action": "添加 skill",
|
||||
|
||||
@@ -2,6 +2,7 @@ package agent
|
||||
|
||||
import (
|
||||
"bufio"
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
@@ -9,13 +10,20 @@ import (
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"regexp"
|
||||
"sort"
|
||||
"strconv"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
)
|
||||
|
||||
// codexBlockedArgs are flags hardcoded by the daemon that must not be
|
||||
// overridden by user-configured custom_args.
|
||||
// overridden by user-configured custom_args. The mcp_servers config keys
|
||||
// live in the per-task `$CODEX_HOME/config.toml` (written by
|
||||
// ensureCodexMcpConfig); user-supplied `-c mcp_servers.…` overrides are
|
||||
// stripped separately by filterCodexCustomConfigOverrides because they
|
||||
// share the `-c` flag with legitimate non-MCP overrides like `-c model=…`.
|
||||
var codexBlockedArgs = map[string]blockedArgMode{
|
||||
"--listen": blockedWithValue, // stdio:// transport for daemon communication
|
||||
}
|
||||
@@ -68,11 +76,419 @@ type codexBackend struct {
|
||||
|
||||
func buildCodexArgs(opts ExecOptions, logger *slog.Logger) []string {
|
||||
args := []string{"app-server", "--listen", "stdio://"}
|
||||
args = append(args, filterCustomArgs(opts.ExtraArgs, codexBlockedArgs, logger)...)
|
||||
args = append(args, filterCustomArgs(opts.CustomArgs, codexBlockedArgs, logger)...)
|
||||
extra := filterCustomArgs(opts.ExtraArgs, codexBlockedArgs, logger)
|
||||
custom := filterCustomArgs(opts.CustomArgs, codexBlockedArgs, logger)
|
||||
// Only claim ownership of the `mcp_servers` namespace when the agent
|
||||
// actually has a managed mcp_config in the MCP Tab. Otherwise existing
|
||||
// users who configure MCP via `custom_args: ["-c", "mcp_servers.…"]`
|
||||
// would silently lose those entries after this PR ships. With managed
|
||||
// mcp_config present, daemon-written `$CODEX_HOME/config.toml` is the
|
||||
// authoritative source and stray `-c mcp_servers.*` overrides are
|
||||
// dropped to keep last-wins from re-shadowing it.
|
||||
if hasManagedCodexMcpConfig(opts.McpConfig) {
|
||||
extra = filterCodexCustomConfigOverrides(extra, logger)
|
||||
custom = filterCodexCustomConfigOverrides(custom, logger)
|
||||
}
|
||||
args = append(args, extra...)
|
||||
args = append(args, custom...)
|
||||
return args
|
||||
}
|
||||
|
||||
// hasManagedCodexMcpConfig reports whether the agent's mcp_config field is
|
||||
// "present" in the API three-state sense: a non-null JSON value. Both
|
||||
// `{}` and `{"mcpServers":{}}` count as present (the admin saved an empty
|
||||
// managed set — strict mode, no global fallback); only SQL NULL or the
|
||||
// literal JSON `null` count as absent (CLI default).
|
||||
func hasManagedCodexMcpConfig(raw json.RawMessage) bool {
|
||||
trimmed := bytes.TrimSpace(raw)
|
||||
if len(trimmed) == 0 {
|
||||
return false
|
||||
}
|
||||
if bytes.Equal(trimmed, []byte("null")) {
|
||||
return false
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
// codexManagedMcpConfigKeyRe matches the daemon-managed config namespace
|
||||
// (`mcp_servers.…`) when it appears as the value of a Codex `-c` /
|
||||
// `--config` flag. Used by filterCodexCustomConfigOverrides to drop user
|
||||
// overrides that would otherwise shadow what the MCP Tab writes into
|
||||
// `$CODEX_HOME/config.toml`.
|
||||
var codexManagedMcpConfigKeyRe = regexp.MustCompile(`^\s*mcp_servers(?:\s*\.|\s*=|\s*$)`)
|
||||
|
||||
// filterCodexCustomConfigOverrides drops `-c mcp_servers.…=` and
|
||||
// `--config mcp_servers.…=` entries from custom args. Codex's `-c` is
|
||||
// last-wins (verified against codex-cli 0.132.0), so without this filter a
|
||||
// user-written `-c mcp_servers.fetch=…` in custom_args would silently
|
||||
// override whatever the MCP Tab saved into the per-task config.toml. We
|
||||
// own the `mcp_servers` namespace via the managed block, so user attempts
|
||||
// to write into it are dropped with a warning rather than allowed to win.
|
||||
// Other `-c`/`--config` keys (e.g. `-c model="o3"`) pass through unchanged.
|
||||
func filterCodexCustomConfigOverrides(args []string, logger *slog.Logger) []string {
|
||||
if len(args) == 0 {
|
||||
return args
|
||||
}
|
||||
filtered := make([]string, 0, len(args))
|
||||
for i := 0; i < len(args); i++ {
|
||||
arg := args[i]
|
||||
flag := arg
|
||||
inlineValue := ""
|
||||
hasInlineValue := false
|
||||
if idx := strings.Index(arg, "="); idx > 0 {
|
||||
flag = arg[:idx]
|
||||
inlineValue = arg[idx+1:]
|
||||
hasInlineValue = true
|
||||
}
|
||||
if flag == "-c" || flag == "--config" {
|
||||
value := inlineValue
|
||||
if !hasInlineValue && i+1 < len(args) {
|
||||
value = args[i+1]
|
||||
}
|
||||
if codexManagedMcpConfigKeyRe.MatchString(value) {
|
||||
if logger != nil {
|
||||
// Log the key only, never the value — mcp_servers.<name>.env
|
||||
// is allowed to carry secrets and the whole point of moving
|
||||
// this to config.toml is to keep raw values out of logs/argv.
|
||||
key := value
|
||||
if eqIdx := strings.Index(value, "="); eqIdx >= 0 {
|
||||
key = value[:eqIdx]
|
||||
}
|
||||
logger.Warn("custom_args: blocked mcp_servers override; daemon manages this via CODEX_HOME/config.toml",
|
||||
"flag", flag, "key", strings.TrimSpace(key))
|
||||
}
|
||||
if !hasInlineValue && i+1 < len(args) {
|
||||
i++ // skip the value arg
|
||||
}
|
||||
continue
|
||||
}
|
||||
}
|
||||
filtered = append(filtered, arg)
|
||||
}
|
||||
return filtered
|
||||
}
|
||||
|
||||
// Markers delimiting the daemon-managed `[mcp_servers.*]` block in
|
||||
// `$CODEX_HOME/config.toml`. Match the existing sandbox / multi-agent /
|
||||
// memory marker pattern so ops can grep all managed blocks consistently.
|
||||
const (
|
||||
multicaCodexMcpBeginMarker = "# BEGIN multica-managed mcp_servers (do not edit; regenerated by daemon)"
|
||||
multicaCodexMcpEndMarker = "# END multica-managed mcp_servers"
|
||||
)
|
||||
|
||||
var codexMcpBlockRe = regexp.MustCompile(
|
||||
`(?ms)^` + regexp.QuoteMeta(multicaCodexMcpBeginMarker) +
|
||||
`.*?^` + regexp.QuoteMeta(multicaCodexMcpEndMarker) + `\n*`)
|
||||
|
||||
// userCodexMcpServersTableHeaderRe matches `[mcp_servers.<name>]` (and its
|
||||
// quoted-key form `[mcp_servers."<name>"]`) at the start of a line. Used
|
||||
// to strip user-provided mcp_servers tables from the per-task config when
|
||||
// the agent has its own mcp_config — mirrors Claude's `--strict-mcp-config`
|
||||
// model where the daemon's set is authoritative.
|
||||
var userCodexMcpServersTableHeaderRe = regexp.MustCompile(
|
||||
`^\s*\[\s*mcp_servers\s*\.\s*(?:"[^"]*"|[^\]\s]+)\s*\]\s*(?:#.*)?$`)
|
||||
|
||||
// ensureCodexMcpConfig writes (or clears) the daemon-managed
|
||||
// `[mcp_servers.*]` block in `$CODEX_HOME/config.toml`. The block is the
|
||||
// authoritative source of MCP servers for this run: with mcp_config set
|
||||
// in the agent UI the daemon also strips any inherited
|
||||
// `[mcp_servers.*]` tables from the per-task config so the user's global
|
||||
// `~/.codex/config.toml` doesn't shadow or collide with the managed set.
|
||||
//
|
||||
// The file mode is 0o600 because `mcp_servers.<id>.env` values may carry
|
||||
// secrets (API keys, bearer tokens); the per-task home is owned by the
|
||||
// daemon's user, so 0o600 keeps secrets out of any world-readable copy
|
||||
// while still letting the codex child read them.
|
||||
//
|
||||
// A malformed mcp_config is returned as an error and the caller decides
|
||||
// whether to surface or warn — same fail-soft contract the prior argv
|
||||
// path had.
|
||||
func ensureCodexMcpConfig(configPath string, mcpConfig json.RawMessage, logger *slog.Logger) error {
|
||||
data, err := os.ReadFile(configPath)
|
||||
if err != nil && !os.IsNotExist(err) {
|
||||
return fmt.Errorf("read config.toml: %w", err)
|
||||
}
|
||||
existing := string(data)
|
||||
|
||||
// Always strip a prior managed block so reruns and clear-config flows
|
||||
// converge on a clean state.
|
||||
stripped := codexMcpBlockRe.ReplaceAllString(existing, "")
|
||||
|
||||
managed := hasManagedCodexMcpConfig(mcpConfig)
|
||||
block, _, renderErr := renderCodexMcpServersBlock(mcpConfig)
|
||||
if renderErr != nil {
|
||||
return renderErr
|
||||
}
|
||||
|
||||
var updated string
|
||||
if managed {
|
||||
// Agent has a managed MCP set (possibly empty — `{}` /
|
||||
// `{"mcpServers":{}}` count as "saved an empty set" in the API's
|
||||
// three-state semantics, distinct from nil/null which means
|
||||
// "fall back to CLI default"). Strip any user-defined
|
||||
// `[mcp_servers.*]` tables inherited from `~/.codex/config.toml`
|
||||
// so the managed set is strict — mirrors Claude's
|
||||
// `--strict-mcp-config`. Two reasons we cannot mix:
|
||||
// 1. TOML rejects redefining the same table; a user table
|
||||
// named `[mcp_servers.fetch]` would crash codex if the
|
||||
// agent also defined `fetch`.
|
||||
// 2. An admin saving an explicit list in the MCP Tab would
|
||||
// otherwise see user-global servers silently joined in,
|
||||
// which contradicts the UI affordance.
|
||||
stripped = stripCodexUserMcpServerTables(stripped)
|
||||
stripped = strings.TrimRight(stripped, "\n")
|
||||
// When the managed set is empty we still write the marker
|
||||
// block (with no tables between). This pins "managed but
|
||||
// empty" on disk so the next run can find and strip the
|
||||
// markers, and so the file's intent is grep-able by ops.
|
||||
if block == "" {
|
||||
block = multicaCodexMcpBeginMarker + "\n" + multicaCodexMcpEndMarker + "\n"
|
||||
}
|
||||
if stripped == "" {
|
||||
updated = block
|
||||
} else {
|
||||
updated = stripped + "\n\n" + block
|
||||
}
|
||||
} else {
|
||||
// No managed config: just remove any prior managed block and
|
||||
// leave inherited user tables alone (CLI default fallback).
|
||||
updated = stripped
|
||||
}
|
||||
|
||||
if updated == existing {
|
||||
return nil
|
||||
}
|
||||
if err := os.WriteFile(configPath, []byte(updated), 0o600); err != nil {
|
||||
return fmt.Errorf("write config.toml: %w", err)
|
||||
}
|
||||
// os.WriteFile applies the mode only when creating a new file; if the
|
||||
// per-task config.toml was already on disk at 0o644 (the default mode
|
||||
// used by execenv.copyFile when seeding from ~/.codex/config.toml),
|
||||
// the secret-bearing values we just wrote would inherit that wider
|
||||
// mode. Chmod unconditionally to keep the secret in the daemon
|
||||
// owner's lane regardless of the prior mode.
|
||||
if err := os.Chmod(configPath, 0o600); err != nil {
|
||||
return fmt.Errorf("chmod config.toml to 0600: %w", err)
|
||||
}
|
||||
if logger != nil {
|
||||
logger.Debug("codex: wrote managed mcp_servers block to config.toml",
|
||||
"config_path", configPath, "managed", managed)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// renderCodexMcpServersBlock renders the agent's mcp_config JSON
|
||||
// (Claude-style `{"mcpServers": {...}}`) as a TOML block of
|
||||
// `[mcp_servers.<name>]` tables wrapped in BEGIN/END markers. Returns
|
||||
// (block, hasServers, err); hasServers=false means the input had no
|
||||
// servers to render (empty/null mcp_config) and the caller should only
|
||||
// strip the prior managed block.
|
||||
//
|
||||
// Claude-style camelCase keys (`args`, `env`, `command`, `url`) pass
|
||||
// through verbatim — Codex's config schema happens to use the same
|
||||
// names today. If they ever diverge, rename here rather than in the UI.
|
||||
func renderCodexMcpServersBlock(raw json.RawMessage) (string, bool, error) {
|
||||
if len(raw) == 0 {
|
||||
return "", false, nil
|
||||
}
|
||||
var parsed struct {
|
||||
McpServers map[string]json.RawMessage `json:"mcpServers"`
|
||||
}
|
||||
if err := json.Unmarshal(raw, &parsed); err != nil {
|
||||
return "", false, fmt.Errorf("parse mcp_config json: %w", err)
|
||||
}
|
||||
if len(parsed.McpServers) == 0 {
|
||||
return "", false, nil
|
||||
}
|
||||
|
||||
names := make([]string, 0, len(parsed.McpServers))
|
||||
for name := range parsed.McpServers {
|
||||
names = append(names, name)
|
||||
}
|
||||
sort.Strings(names)
|
||||
|
||||
var sb strings.Builder
|
||||
sb.WriteString(multicaCodexMcpBeginMarker)
|
||||
sb.WriteString("\n")
|
||||
for i, name := range names {
|
||||
if !isCodexBareTomlKey(name) {
|
||||
return "", false, fmt.Errorf("mcp server name %q must be ASCII alphanumeric / _ / - to fit Codex's bare-key requirement", name)
|
||||
}
|
||||
var serverVal map[string]any
|
||||
if err := json.Unmarshal(parsed.McpServers[name], &serverVal); err != nil {
|
||||
return "", false, fmt.Errorf("mcp_servers.%s: %w", name, err)
|
||||
}
|
||||
if serverVal == nil {
|
||||
return "", false, fmt.Errorf("mcp_servers.%s must be a JSON object", name)
|
||||
}
|
||||
if i > 0 {
|
||||
sb.WriteString("\n")
|
||||
}
|
||||
sb.WriteString("[mcp_servers.")
|
||||
sb.WriteString(name)
|
||||
sb.WriteString("]\n")
|
||||
keys := make([]string, 0, len(serverVal))
|
||||
for k := range serverVal {
|
||||
keys = append(keys, k)
|
||||
}
|
||||
sort.Strings(keys)
|
||||
for _, k := range keys {
|
||||
tomlValue, err := jsonValueToCodexTOMLInline(serverVal[k])
|
||||
if err != nil {
|
||||
return "", false, fmt.Errorf("mcp_servers.%s.%s: %w", name, k, err)
|
||||
}
|
||||
sb.WriteString(codexTOMLKey(k))
|
||||
sb.WriteString(" = ")
|
||||
sb.WriteString(tomlValue)
|
||||
sb.WriteString("\n")
|
||||
}
|
||||
}
|
||||
sb.WriteString(multicaCodexMcpEndMarker)
|
||||
sb.WriteString("\n")
|
||||
return sb.String(), true, nil
|
||||
}
|
||||
|
||||
// stripCodexUserMcpServerTables removes every `[mcp_servers.*]` table
|
||||
// (header + body lines until the next top-level table header or EOF) from
|
||||
// a TOML config string. Sub-tables like `[mcp_servers.fetch.env]` count
|
||||
// as part of the parent table and are dropped along with it.
|
||||
func stripCodexUserMcpServerTables(content string) string {
|
||||
lines := strings.Split(content, "\n")
|
||||
out := make([]string, 0, len(lines))
|
||||
skipping := false
|
||||
for _, line := range lines {
|
||||
if userCodexMcpServersTableHeaderRe.MatchString(line) {
|
||||
skipping = true
|
||||
continue
|
||||
}
|
||||
if skipping {
|
||||
trimmed := strings.TrimSpace(line)
|
||||
if strings.HasPrefix(trimmed, "[") {
|
||||
// Next table header. If it's still an `mcp_servers.*`
|
||||
// table (including a sub-table), keep skipping; otherwise
|
||||
// stop and emit this line.
|
||||
if userCodexMcpServersTableHeaderRe.MatchString(line) ||
|
||||
strings.HasPrefix(trimmed, "[mcp_servers.") ||
|
||||
strings.HasPrefix(trimmed, "[ mcp_servers.") {
|
||||
continue
|
||||
}
|
||||
skipping = false
|
||||
out = append(out, line)
|
||||
continue
|
||||
}
|
||||
continue
|
||||
}
|
||||
out = append(out, line)
|
||||
}
|
||||
return strings.Join(out, "\n")
|
||||
}
|
||||
|
||||
// jsonValueToCodexTOMLInline serialises a JSON value as a TOML inline
|
||||
// value. Only the subset Codex's `-c` accepts is supported: strings,
|
||||
// numbers, booleans, arrays, and inline tables. JSON nulls are rejected
|
||||
// because TOML has no null and silently dropping them would be confusing.
|
||||
func jsonValueToCodexTOMLInline(v any) (string, error) {
|
||||
switch x := v.(type) {
|
||||
case nil:
|
||||
return "", fmt.Errorf("null is not a valid TOML value")
|
||||
case bool:
|
||||
if x {
|
||||
return "true", nil
|
||||
}
|
||||
return "false", nil
|
||||
case float64:
|
||||
if x == float64(int64(x)) {
|
||||
return strconv.FormatInt(int64(x), 10), nil
|
||||
}
|
||||
return strconv.FormatFloat(x, 'f', -1, 64), nil
|
||||
case string:
|
||||
return codexTOMLBasicString(x), nil
|
||||
case []any:
|
||||
parts := make([]string, len(x))
|
||||
for i, e := range x {
|
||||
p, err := jsonValueToCodexTOMLInline(e)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
parts[i] = p
|
||||
}
|
||||
return "[" + strings.Join(parts, ", ") + "]", nil
|
||||
case map[string]any:
|
||||
keys := make([]string, 0, len(x))
|
||||
for k := range x {
|
||||
keys = append(keys, k)
|
||||
}
|
||||
sort.Strings(keys)
|
||||
parts := make([]string, len(keys))
|
||||
for i, k := range keys {
|
||||
p, err := jsonValueToCodexTOMLInline(x[k])
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
parts[i] = codexTOMLKey(k) + " = " + p
|
||||
}
|
||||
return "{ " + strings.Join(parts, ", ") + " }", nil
|
||||
default:
|
||||
return "", fmt.Errorf("unsupported value type %T", v)
|
||||
}
|
||||
}
|
||||
|
||||
func codexTOMLBasicString(s string) string {
|
||||
var sb strings.Builder
|
||||
sb.Grow(len(s) + 2)
|
||||
sb.WriteByte('"')
|
||||
for _, r := range s {
|
||||
switch r {
|
||||
case '\\':
|
||||
sb.WriteString(`\\`)
|
||||
case '"':
|
||||
sb.WriteString(`\"`)
|
||||
case '\b':
|
||||
sb.WriteString(`\b`)
|
||||
case '\t':
|
||||
sb.WriteString(`\t`)
|
||||
case '\n':
|
||||
sb.WriteString(`\n`)
|
||||
case '\f':
|
||||
sb.WriteString(`\f`)
|
||||
case '\r':
|
||||
sb.WriteString(`\r`)
|
||||
default:
|
||||
if r < 0x20 || r == 0x7f {
|
||||
sb.WriteString(fmt.Sprintf(`\u%04x`, r))
|
||||
} else {
|
||||
sb.WriteRune(r)
|
||||
}
|
||||
}
|
||||
}
|
||||
sb.WriteByte('"')
|
||||
return sb.String()
|
||||
}
|
||||
|
||||
func codexTOMLKey(s string) string {
|
||||
if isCodexBareTomlKey(s) {
|
||||
return s
|
||||
}
|
||||
return codexTOMLBasicString(s)
|
||||
}
|
||||
|
||||
func isCodexBareTomlKey(s string) bool {
|
||||
if s == "" {
|
||||
return false
|
||||
}
|
||||
for _, r := range s {
|
||||
switch {
|
||||
case r >= 'a' && r <= 'z':
|
||||
case r >= 'A' && r <= 'Z':
|
||||
case r >= '0' && r <= '9':
|
||||
case r == '_' || r == '-':
|
||||
default:
|
||||
return false
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
func (b *codexBackend) Execute(ctx context.Context, prompt string, opts ExecOptions) (*Session, error) {
|
||||
execPath := b.cfg.ExecutablePath
|
||||
if execPath == "" {
|
||||
@@ -92,6 +508,35 @@ func (b *codexBackend) Execute(ctx context.Context, prompt string, opts ExecOpti
|
||||
}
|
||||
runCtx, cancel := context.WithTimeout(ctx, timeout)
|
||||
|
||||
// Materialise the agent's MCP config into the per-task
|
||||
// `$CODEX_HOME/config.toml`. Argv would be the simpler path, but
|
||||
// `mcp_servers.<id>.env` is allowed to carry secrets (Codex docs:
|
||||
// https://developers.openai.com/codex/mcp#configure-with-configtoml)
|
||||
// and our UI already treats mcp_config as a redacted-for-non-admins
|
||||
// field. Process argv ends up in OS-level `ps` listings and is also
|
||||
// echoed into the daemon's `agent command` log line below, so any
|
||||
// inline env-bearing TOML would defeat the redaction. Writing through
|
||||
// config.toml at 0o600 keeps the secret values out of argv and logs.
|
||||
if codexHome := strings.TrimSpace(b.cfg.Env["CODEX_HOME"]); codexHome != "" {
|
||||
if err := ensureCodexMcpConfig(filepath.Join(codexHome, "config.toml"), opts.McpConfig, b.cfg.Logger); err != nil {
|
||||
// Fail closed when we can't materialise the managed config.
|
||||
// Warning-and-launching would silently fall back to the
|
||||
// user's global `~/.codex/config.toml` MCP servers and
|
||||
// look indistinguishable from "the saved config was
|
||||
// applied", which is exactly the surprise the MCP Tab is
|
||||
// supposed to remove.
|
||||
cancel()
|
||||
return nil, fmt.Errorf("apply codex mcp_config: %w", err)
|
||||
}
|
||||
} else if hasManagedCodexMcpConfig(opts.McpConfig) {
|
||||
// Managed mcp_config saved but no CODEX_HOME to anchor it.
|
||||
// Same reasoning as above: silently launching would inherit
|
||||
// whatever MCP setup the host user has, which is the wrong
|
||||
// shape of failure.
|
||||
cancel()
|
||||
return nil, fmt.Errorf("codex: mcp_config is set but CODEX_HOME env var is not configured; cannot apply managed MCP")
|
||||
}
|
||||
|
||||
codexArgs := buildCodexArgs(opts, b.cfg.Logger)
|
||||
cmd := exec.CommandContext(runCtx, execPath, codexArgs...)
|
||||
hideAgentWindow(cmd)
|
||||
|
||||
@@ -5,7 +5,9 @@ import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"reflect"
|
||||
"runtime"
|
||||
"strings"
|
||||
"sync"
|
||||
@@ -1440,3 +1442,533 @@ func TestBuildCodexArgsExtraArgsBeforeCustomArgsAndFiltersBoth(t *testing.T) {
|
||||
t.Fatalf("expected extra args before custom args, got %v", args)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildCodexArgsDoesNotLeakMcpToArgv(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// MCP config is materialised into $CODEX_HOME/config.toml, never into
|
||||
// argv — otherwise `mcp_servers.<id>.env` secrets would land in
|
||||
// `ps aux` output and in the daemon's `agent command` log line. This
|
||||
// test pins the contract: even with a non-empty mcp_config, no -c /
|
||||
// --config / mcp_servers.* entry shows up in buildCodexArgs output.
|
||||
raw := json.RawMessage(`{"mcpServers":{"fetch":{"command":"uvx","env":{"SECRET":"hunter2"}}}}`)
|
||||
args := buildCodexArgs(ExecOptions{
|
||||
McpConfig: raw,
|
||||
CustomArgs: []string{"-c", `model="o3"`},
|
||||
}, slog.Default())
|
||||
|
||||
joined := strings.Join(args, " ")
|
||||
if strings.Contains(joined, "mcp_servers") {
|
||||
t.Fatalf("argv must not mention mcp_servers (now lives in config.toml), got %v", args)
|
||||
}
|
||||
if strings.Contains(joined, "hunter2") {
|
||||
t.Fatalf("argv must not leak secret env values, got %v", args)
|
||||
}
|
||||
for i := 0; i+1 < len(args); i++ {
|
||||
if (args[i] == "-c" || args[i] == "--config") && strings.HasPrefix(args[i+1], "mcp_servers.") {
|
||||
t.Fatalf("expected no -c mcp_servers.* in argv, got %v", args)
|
||||
}
|
||||
}
|
||||
// Legitimate non-mcp `-c model=…` from custom_args must still survive.
|
||||
foundModel := false
|
||||
for i := 0; i+1 < len(args); i++ {
|
||||
if args[i] == "-c" && args[i+1] == `model="o3"` {
|
||||
foundModel = true
|
||||
}
|
||||
}
|
||||
if !foundModel {
|
||||
t.Fatalf("expected non-mcp -c override to be preserved, got %v", args)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCodexExecuteFailsClosedWhenMcpConfigInvalid(t *testing.T) {
|
||||
t.Parallel()
|
||||
if runtime.GOOS == "windows" {
|
||||
t.Skip("shell-script fixture is POSIX-only")
|
||||
}
|
||||
|
||||
// When the admin has a managed mcp_config but the JSON is malformed
|
||||
// (or any other reason ensureCodexMcpConfig fails), fail closed
|
||||
// instead of silently launching with the user's global MCP — that
|
||||
// would look indistinguishable from "the saved config was applied"
|
||||
// and is exactly the surprise the MCP Tab is supposed to remove.
|
||||
fakePath := writeFakeCodexAppServer(t, "exit 0\n")
|
||||
|
||||
codexHome := t.TempDir()
|
||||
backend, err := New("codex", Config{
|
||||
ExecutablePath: fakePath,
|
||||
Logger: slog.Default(),
|
||||
Env: map[string]string{"CODEX_HOME": codexHome},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("new codex backend: %v", err)
|
||||
}
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||
defer cancel()
|
||||
_, err = backend.Execute(ctx, "prompt", ExecOptions{
|
||||
Timeout: 2 * time.Second,
|
||||
McpConfig: json.RawMessage(`not json`),
|
||||
})
|
||||
if err == nil {
|
||||
t.Fatal("expected Execute to fail closed on malformed mcp_config, got nil error")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "mcp_config") {
|
||||
t.Fatalf("expected error to mention mcp_config, got %q", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCodexExecuteFailsClosedWhenManagedMcpButNoCodexHome(t *testing.T) {
|
||||
t.Parallel()
|
||||
if runtime.GOOS == "windows" {
|
||||
t.Skip("shell-script fixture is POSIX-only")
|
||||
}
|
||||
|
||||
// Managed mcp_config saved but no CODEX_HOME to anchor it — same
|
||||
// fail-closed reasoning: silently launching would inherit whatever
|
||||
// MCP setup the host user has, which is the wrong shape of failure.
|
||||
fakePath := writeFakeCodexAppServer(t, "exit 0\n")
|
||||
|
||||
backend, err := New("codex", Config{
|
||||
ExecutablePath: fakePath,
|
||||
Logger: slog.Default(),
|
||||
Env: map[string]string{}, // no CODEX_HOME
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("new codex backend: %v", err)
|
||||
}
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||
defer cancel()
|
||||
_, err = backend.Execute(ctx, "prompt", ExecOptions{
|
||||
Timeout: 2 * time.Second,
|
||||
McpConfig: json.RawMessage(`{"mcpServers":{"fetch":{"command":"uvx"}}}`),
|
||||
})
|
||||
if err == nil {
|
||||
t.Fatal("expected Execute to fail closed when managed mcp_config but no CODEX_HOME, got nil error")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "CODEX_HOME") {
|
||||
t.Fatalf("expected error to mention CODEX_HOME, got %q", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildCodexArgsPreservesCustomMcpOverridesWhenUnmanaged(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Existing Codex agents may rely on `custom_args: ["-c", "mcp_servers.…"]`
|
||||
// because before MUL-2764 there was no MCP Tab. When the agent has
|
||||
// no managed mcp_config saved, the daemon must leave those entries
|
||||
// alone — silently dropping them would break the only way those
|
||||
// users had to configure MCP. We only claim the `mcp_servers`
|
||||
// namespace once an admin opts in via the MCP Tab.
|
||||
args := buildCodexArgs(ExecOptions{
|
||||
CustomArgs: []string{"-c", `mcp_servers.fetch={ command = "uvx" }`, "-c", `model="o3"`},
|
||||
}, slog.Default())
|
||||
foundMcp := false
|
||||
for i := 0; i+1 < len(args); i++ {
|
||||
if args[i] == "-c" && strings.HasPrefix(args[i+1], "mcp_servers.") {
|
||||
foundMcp = true
|
||||
}
|
||||
}
|
||||
if !foundMcp {
|
||||
t.Fatalf("custom_args mcp_servers entry must survive when agent has no managed mcp_config, got %v", args)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildCodexArgsDropsCustomMcpOverridesWhenManaged(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Once an admin saves a managed mcp_config, the daemon owns
|
||||
// the `mcp_servers` namespace via $CODEX_HOME/config.toml. Codex's
|
||||
// `-c` is last-wins, so any `-c mcp_servers.…` left in custom_args
|
||||
// would silently shadow the saved managed entries.
|
||||
raw := json.RawMessage(`{"mcpServers":{"managed":{"command":"managed-cmd"}}}`)
|
||||
args := buildCodexArgs(ExecOptions{
|
||||
McpConfig: raw,
|
||||
CustomArgs: []string{"-c", `mcp_servers.fetch={ command = "evil" }`, "-c", `model="o3"`},
|
||||
}, slog.Default())
|
||||
for i := 0; i+1 < len(args); i++ {
|
||||
if args[i] == "-c" && strings.HasPrefix(args[i+1], "mcp_servers.") {
|
||||
t.Fatalf("custom_args mcp_servers must be filtered when managed mcp_config is present, got %v", args)
|
||||
}
|
||||
}
|
||||
// Unrelated -c key still passes through.
|
||||
foundModel := false
|
||||
for i := 0; i+1 < len(args); i++ {
|
||||
if args[i] == "-c" && args[i+1] == `model="o3"` {
|
||||
foundModel = true
|
||||
}
|
||||
}
|
||||
if !foundModel {
|
||||
t.Fatalf("unrelated -c override must still survive, got %v", args)
|
||||
}
|
||||
}
|
||||
|
||||
func TestFilterCodexCustomConfigOverridesDropsMcpServers(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Codex `-c` is last-wins, so a user-supplied `-c mcp_servers.…` in
|
||||
// custom_args would silently shadow whatever the MCP Tab wrote into
|
||||
// CODEX_HOME/config.toml. Verify that all spellings of the override
|
||||
// get dropped, while unrelated `-c` keys pass through.
|
||||
cases := []struct {
|
||||
name string
|
||||
in []string
|
||||
want []string
|
||||
}{
|
||||
{
|
||||
name: "separated -c mcp_servers.fetch=…",
|
||||
in: []string{"-c", `mcp_servers.fetch={ command = "evil" }`, "-c", `model="o3"`},
|
||||
want: []string{"-c", `model="o3"`},
|
||||
},
|
||||
{
|
||||
name: "inline -c=mcp_servers.fetch=…",
|
||||
in: []string{`-c=mcp_servers.fetch={ command = "evil" }`, "--listen=keep"},
|
||||
want: []string{"--listen=keep"},
|
||||
},
|
||||
{
|
||||
name: "long form --config mcp_servers.x.env.KEY=val",
|
||||
in: []string{"--config", `mcp_servers.x.env.KEY="leak"`, "--config", `sandbox="workspace-write"`},
|
||||
want: []string{"--config", `sandbox="workspace-write"`},
|
||||
},
|
||||
{
|
||||
name: "passes through unrelated -c overrides",
|
||||
in: []string{"-c", `model="o3"`, "-c", `sandbox.network_access=true`},
|
||||
want: []string{"-c", `model="o3"`, "-c", `sandbox.network_access=true`},
|
||||
},
|
||||
{
|
||||
name: "matches mcp_servers root assignment",
|
||||
in: []string{"-c", `mcp_servers={fetch={command="evil"}}`, "-c", `model="o3"`},
|
||||
want: []string{"-c", `model="o3"`},
|
||||
},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
tc := tc
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
got := filterCodexCustomConfigOverrides(tc.in, slog.Default())
|
||||
if !reflect.DeepEqual(got, tc.want) {
|
||||
t.Fatalf("filterCodexCustomConfigOverrides(%v) = %v, want %v", tc.in, got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestEnsureCodexMcpConfigEmptyClearsBlock(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// When agent.mcp_config is null/empty the managed block is removed
|
||||
// from config.toml, but unrelated content (sandbox block, user-level
|
||||
// `[mcp_servers.user]`) is left untouched.
|
||||
tmp := filepath.Join(t.TempDir(), "config.toml")
|
||||
initial := "sandbox_mode = \"workspace-write\"\n\n" +
|
||||
multicaCodexMcpBeginMarker + "\n" +
|
||||
"[mcp_servers.fetch]\ncommand = \"uvx\"\n" +
|
||||
multicaCodexMcpEndMarker + "\n\n" +
|
||||
"[mcp_servers.user_global]\ncommand = \"keep\"\n"
|
||||
if err := os.WriteFile(tmp, []byte(initial), 0o600); err != nil {
|
||||
t.Fatalf("seed config: %v", err)
|
||||
}
|
||||
|
||||
if err := ensureCodexMcpConfig(tmp, nil, slog.Default()); err != nil {
|
||||
t.Fatalf("ensure: %v", err)
|
||||
}
|
||||
data, err := os.ReadFile(tmp)
|
||||
if err != nil {
|
||||
t.Fatalf("read after: %v", err)
|
||||
}
|
||||
got := string(data)
|
||||
if strings.Contains(got, multicaCodexMcpBeginMarker) {
|
||||
t.Fatalf("managed block should be cleared, got:\n%s", got)
|
||||
}
|
||||
if !strings.Contains(got, "[mcp_servers.user_global]") {
|
||||
t.Fatalf("user-defined mcp_servers should be left alone when agent has no mcp_config, got:\n%s", got)
|
||||
}
|
||||
if !strings.Contains(got, `sandbox_mode = "workspace-write"`) {
|
||||
t.Fatalf("unrelated config preserved, got:\n%s", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestEnsureCodexMcpConfigWritesManagedBlock(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// A non-empty mcp_config writes one `[mcp_servers.<name>]` table per
|
||||
// server, in stable alphabetical order, into the managed block. The
|
||||
// file mode is 0o600 because env values may carry secrets.
|
||||
tmp := filepath.Join(t.TempDir(), "config.toml")
|
||||
if err := os.WriteFile(tmp, []byte("sandbox_mode = \"workspace-write\"\n"), 0o600); err != nil {
|
||||
t.Fatalf("seed: %v", err)
|
||||
}
|
||||
|
||||
raw := json.RawMessage(`{"mcpServers":{"zeta":{"command":"b"},"alpha":{"command":"a","env":{"K":"v"}}}}`)
|
||||
if err := ensureCodexMcpConfig(tmp, raw, slog.Default()); err != nil {
|
||||
t.Fatalf("ensure: %v", err)
|
||||
}
|
||||
data, err := os.ReadFile(tmp)
|
||||
if err != nil {
|
||||
t.Fatalf("read after: %v", err)
|
||||
}
|
||||
got := string(data)
|
||||
|
||||
if !strings.Contains(got, multicaCodexMcpBeginMarker) || !strings.Contains(got, multicaCodexMcpEndMarker) {
|
||||
t.Fatalf("expected managed block markers, got:\n%s", got)
|
||||
}
|
||||
alphaIdx := strings.Index(got, "[mcp_servers.alpha]")
|
||||
zetaIdx := strings.Index(got, "[mcp_servers.zeta]")
|
||||
if alphaIdx == -1 || zetaIdx == -1 {
|
||||
t.Fatalf("expected both server tables, got:\n%s", got)
|
||||
}
|
||||
if alphaIdx > zetaIdx {
|
||||
t.Fatalf("expected alpha before zeta (alphabetical), got:\n%s", got)
|
||||
}
|
||||
for _, want := range []string{
|
||||
`command = "a"`,
|
||||
`env = { K = "v" }`,
|
||||
`command = "b"`,
|
||||
`sandbox_mode = "workspace-write"`, // unrelated user content preserved
|
||||
} {
|
||||
if !strings.Contains(got, want) {
|
||||
t.Fatalf("expected %q in:\n%s", want, got)
|
||||
}
|
||||
}
|
||||
|
||||
fi, err := os.Stat(tmp)
|
||||
if err != nil {
|
||||
t.Fatalf("stat: %v", err)
|
||||
}
|
||||
if mode := fi.Mode().Perm(); mode != 0o600 {
|
||||
t.Fatalf("expected mode 0o600 for secret-bearing config, got %o", mode)
|
||||
}
|
||||
}
|
||||
|
||||
func TestEnsureCodexMcpConfigForces0600OnPreexistingFile(t *testing.T) {
|
||||
t.Parallel()
|
||||
if runtime.GOOS == "windows" {
|
||||
t.Skip("POSIX permissions only")
|
||||
}
|
||||
|
||||
// `execenv.copyFile` seeds the per-task config.toml at 0o644. Once we
|
||||
// add secret-bearing mcp_servers tables to it, the mode must drop to
|
||||
// 0o600 — `os.WriteFile` alone keeps the existing mode, so the chmod
|
||||
// is the part we need to pin.
|
||||
tmp := filepath.Join(t.TempDir(), "config.toml")
|
||||
if err := os.WriteFile(tmp, []byte("sandbox_mode = \"workspace-write\"\n"), 0o644); err != nil {
|
||||
t.Fatalf("seed: %v", err)
|
||||
}
|
||||
|
||||
raw := json.RawMessage(`{"mcpServers":{"fetch":{"command":"uvx","env":{"API_KEY":"secret"}}}}`)
|
||||
if err := ensureCodexMcpConfig(tmp, raw, slog.Default()); err != nil {
|
||||
t.Fatalf("ensure: %v", err)
|
||||
}
|
||||
fi, err := os.Stat(tmp)
|
||||
if err != nil {
|
||||
t.Fatalf("stat: %v", err)
|
||||
}
|
||||
if mode := fi.Mode().Perm(); mode != 0o600 {
|
||||
t.Fatalf("expected 0o600 after overwrite of pre-existing 0o644 file, got %o", mode)
|
||||
}
|
||||
}
|
||||
|
||||
func TestEnsureCodexMcpConfigStripsUserMcpServersWhenManaged(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// When agent.mcp_config is non-empty, ALL user-defined `[mcp_servers.*]`
|
||||
// tables (inherited from ~/.codex/config.toml) are stripped to avoid
|
||||
// (a) TOML "table already exists" errors when names collide and (b) the
|
||||
// user's global servers silently being mixed in with the strict
|
||||
// agent-managed list. Sub-tables like `[mcp_servers.x.env]` are also
|
||||
// dropped as part of their parent.
|
||||
tmp := filepath.Join(t.TempDir(), "config.toml")
|
||||
initial := "sandbox_mode = \"workspace-write\"\n\n" +
|
||||
"[mcp_servers.global_fetch]\ncommand = \"uvx-old\"\n\n" +
|
||||
"[mcp_servers.global_fetch.env]\nOLD_KEY = \"old\"\n\n" +
|
||||
"[other_section]\nkeep_me = true\n"
|
||||
if err := os.WriteFile(tmp, []byte(initial), 0o600); err != nil {
|
||||
t.Fatalf("seed: %v", err)
|
||||
}
|
||||
|
||||
raw := json.RawMessage(`{"mcpServers":{"new_server":{"command":"new"}}}`)
|
||||
if err := ensureCodexMcpConfig(tmp, raw, slog.Default()); err != nil {
|
||||
t.Fatalf("ensure: %v", err)
|
||||
}
|
||||
data, _ := os.ReadFile(tmp)
|
||||
got := string(data)
|
||||
|
||||
if strings.Contains(got, "global_fetch") {
|
||||
t.Fatalf("user mcp_servers tables must be stripped when agent has its own mcp_config, got:\n%s", got)
|
||||
}
|
||||
if strings.Contains(got, "OLD_KEY") {
|
||||
t.Fatalf("user mcp_servers sub-tables must be stripped too, got:\n%s", got)
|
||||
}
|
||||
if !strings.Contains(got, "[other_section]") || !strings.Contains(got, "keep_me = true") {
|
||||
t.Fatalf("unrelated tables must survive, got:\n%s", got)
|
||||
}
|
||||
if !strings.Contains(got, "[mcp_servers.new_server]") {
|
||||
t.Fatalf("managed server should be written, got:\n%s", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestEnsureCodexMcpConfigIdempotent(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Running ensure twice with the same input must produce byte-identical
|
||||
// output — needed because Prepare and Reuse may both call into this on
|
||||
// the same per-task config.toml across a task's lifetime.
|
||||
tmp := filepath.Join(t.TempDir(), "config.toml")
|
||||
raw := json.RawMessage(`{"mcpServers":{"fetch":{"command":"uvx","args":["a","b"]}}}`)
|
||||
|
||||
if err := ensureCodexMcpConfig(tmp, raw, slog.Default()); err != nil {
|
||||
t.Fatalf("first ensure: %v", err)
|
||||
}
|
||||
first, _ := os.ReadFile(tmp)
|
||||
|
||||
if err := ensureCodexMcpConfig(tmp, raw, slog.Default()); err != nil {
|
||||
t.Fatalf("second ensure: %v", err)
|
||||
}
|
||||
second, _ := os.ReadFile(tmp)
|
||||
|
||||
if string(first) != string(second) {
|
||||
t.Fatalf("non-idempotent write:\nfirst:\n%s\nsecond:\n%s", first, second)
|
||||
}
|
||||
}
|
||||
|
||||
func TestEnsureCodexMcpConfigRejectsBadShapes(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
raw string
|
||||
}{
|
||||
{"non-json", `not json`},
|
||||
{"server is array", `{"mcpServers":{"x":[1,2]}}`},
|
||||
{"server is string", `{"mcpServers":{"x":"oops"}}`},
|
||||
{"null value inside server", `{"mcpServers":{"x":{"command":null}}}`},
|
||||
{"bad server name", `{"mcpServers":{"has space":{"command":"a"}}}`},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
tc := tc
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
tmp := filepath.Join(t.TempDir(), "config.toml")
|
||||
if err := ensureCodexMcpConfig(tmp, json.RawMessage(tc.raw), slog.Default()); err == nil {
|
||||
t.Fatalf("expected error for %s, got nil", tc.name)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestEnsureCodexMcpConfigAbsentLeavesUserTablesAlone(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// nil / `null` map to the API's "absent" state: the agent has no
|
||||
// managed mcp_config, so the daemon must not touch the user's
|
||||
// inherited `[mcp_servers.*]` tables — the run falls back to the
|
||||
// user's global CLI config.
|
||||
for _, raw := range []json.RawMessage{nil, json.RawMessage(`null`)} {
|
||||
tmp := filepath.Join(t.TempDir(), "config.toml")
|
||||
initial := "sandbox_mode = \"workspace-write\"\n\n" +
|
||||
"[mcp_servers.user_global]\ncommand = \"keep\"\n"
|
||||
if err := os.WriteFile(tmp, []byte(initial), 0o600); err != nil {
|
||||
t.Fatalf("seed: %v", err)
|
||||
}
|
||||
if err := ensureCodexMcpConfig(tmp, raw, slog.Default()); err != nil {
|
||||
t.Fatalf("ensure (%q): %v", string(raw), err)
|
||||
}
|
||||
data, _ := os.ReadFile(tmp)
|
||||
got := string(data)
|
||||
if !strings.Contains(got, "[mcp_servers.user_global]") {
|
||||
t.Fatalf("absent mcp_config (%q) must leave user MCP tables alone, got:\n%s", string(raw), got)
|
||||
}
|
||||
if strings.Contains(got, multicaCodexMcpBeginMarker) {
|
||||
t.Fatalf("absent mcp_config (%q) must not write managed markers, got:\n%s", string(raw), got)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestEnsureCodexMcpConfigEmptyManagedSetStripsUserMcp(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// `{}` / `{"mcpServers":{}}` map to the API's "present, empty" state.
|
||||
// The admin saved an explicit (empty) MCP list, so the daemon must
|
||||
// strip inherited user `[mcp_servers.*]` tables and pin the managed
|
||||
// markers — equivalent to Claude's --strict-mcp-config with an empty
|
||||
// servers map. Falling back to the user's global MCP would defeat
|
||||
// the affordance.
|
||||
for _, raw := range []json.RawMessage{
|
||||
json.RawMessage(`{}`),
|
||||
json.RawMessage(`{"mcpServers":{}}`),
|
||||
} {
|
||||
tmp := filepath.Join(t.TempDir(), "config.toml")
|
||||
initial := "sandbox_mode = \"workspace-write\"\n\n" +
|
||||
"[mcp_servers.user_global]\ncommand = \"keep\"\n"
|
||||
if err := os.WriteFile(tmp, []byte(initial), 0o600); err != nil {
|
||||
t.Fatalf("seed: %v", err)
|
||||
}
|
||||
if err := ensureCodexMcpConfig(tmp, raw, slog.Default()); err != nil {
|
||||
t.Fatalf("ensure (%q): %v", string(raw), err)
|
||||
}
|
||||
data, _ := os.ReadFile(tmp)
|
||||
got := string(data)
|
||||
if strings.Contains(got, "user_global") {
|
||||
t.Fatalf("managed empty set (%q) must strip user MCP tables, got:\n%s", string(raw), got)
|
||||
}
|
||||
if !strings.Contains(got, multicaCodexMcpBeginMarker) || !strings.Contains(got, multicaCodexMcpEndMarker) {
|
||||
t.Fatalf("managed empty set (%q) must still write markers so future runs find them, got:\n%s", string(raw), got)
|
||||
}
|
||||
if !strings.Contains(got, `sandbox_mode = "workspace-write"`) {
|
||||
t.Fatalf("unrelated content must survive (%q), got:\n%s", string(raw), got)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestEnsureCodexMcpConfigEmptyManagedSetIdempotent(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Running ensure twice with the same `{}` input must produce
|
||||
// byte-identical output — guards against the empty-marker block
|
||||
// accreting blank lines or duplicate markers across reruns.
|
||||
tmp := filepath.Join(t.TempDir(), "config.toml")
|
||||
if err := os.WriteFile(tmp, []byte("sandbox_mode = \"workspace-write\"\n"), 0o600); err != nil {
|
||||
t.Fatalf("seed: %v", err)
|
||||
}
|
||||
raw := json.RawMessage(`{}`)
|
||||
if err := ensureCodexMcpConfig(tmp, raw, slog.Default()); err != nil {
|
||||
t.Fatalf("first ensure: %v", err)
|
||||
}
|
||||
first, _ := os.ReadFile(tmp)
|
||||
if err := ensureCodexMcpConfig(tmp, raw, slog.Default()); err != nil {
|
||||
t.Fatalf("second ensure: %v", err)
|
||||
}
|
||||
second, _ := os.ReadFile(tmp)
|
||||
if string(first) != string(second) {
|
||||
t.Fatalf("non-idempotent write:\nfirst:\n%s\nsecond:\n%s", first, second)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHasManagedCodexMcpConfig(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
raw json.RawMessage
|
||||
want bool
|
||||
}{
|
||||
{"nil", nil, false},
|
||||
{"empty bytes", json.RawMessage(""), false},
|
||||
{"whitespace only", json.RawMessage(" \n\t"), false},
|
||||
{"json null", json.RawMessage(`null`), false},
|
||||
{"json null with whitespace", json.RawMessage(" null \n"), false},
|
||||
{"empty object", json.RawMessage(`{}`), true},
|
||||
{"empty mcp servers map", json.RawMessage(`{"mcpServers":{}}`), true},
|
||||
{"populated", json.RawMessage(`{"mcpServers":{"x":{"command":"a"}}}`), true},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
tc := tc
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
if got := hasManagedCodexMcpConfig(tc.raw); got != tc.want {
|
||||
t.Fatalf("hasManagedCodexMcpConfig(%q) = %v, want %v", string(tc.raw), got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user