diff --git a/packages/core/agents/openclaw-runtime-config.test.ts b/packages/core/agents/openclaw-runtime-config.test.ts index e081d173357..ffb5c2bf290 100644 --- a/packages/core/agents/openclaw-runtime-config.test.ts +++ b/packages/core/agents/openclaw-runtime-config.test.ts @@ -1,8 +1,6 @@ import { describe, expect, it } from "vitest"; import { OPENCLAW_GATEWAY_TOKEN_MASK, - openclawRuntimeConfigEquals, - parseOpenclawRuntimeConfig, serializeOpenclawRuntimeConfig, } from "./openclaw-runtime-config"; @@ -63,63 +61,3 @@ describe("serializeOpenclawRuntimeConfig", () => { }); }); }); - -// `runtime_config` is one shared JSONB column and the OpenClaw tab persists its -// serialized output as the WHOLE object. Dropping unknown keys therefore let an -// unrelated routing save silently delete `mcp.inherit_runtime`, which controls -// whether the agent may reach the host's MCP servers (GitHub #6283). -describe("openclaw runtime_config passthrough", () => { - it("preserves keys owned by other tabs across a parse/serialize round-trip", () => { - const stored = { - mode: "gateway", - gateway: { host: "box.local", port: 4599 }, - mcp: { inherit_runtime: true }, - }; - - const parsed = parseOpenclawRuntimeConfig(stored); - expect(parsed.passthrough).toEqual({ mcp: { inherit_runtime: true } }); - - expect(serializeOpenclawRuntimeConfig(parsed)).toEqual(stored); - }); - - it("keeps the foreign keys when the OpenClaw settings themselves change", () => { - const parsed = parseOpenclawRuntimeConfig({ - mode: "gateway", - gateway: { host: "old.local" }, - mcp: { inherit_runtime: true }, - }); - - // Simulate the form switching back to local mode and saving. - const saved = serializeOpenclawRuntimeConfig({ - mode: "local", - passthrough: parsed.passthrough, - }); - - expect(saved).toEqual({ mode: "local", mcp: { inherit_runtime: true } }); - }); - - it("never lets a foreign key overwrite the fields this form owns", () => { - const saved = serializeOpenclawRuntimeConfig({ - mode: "local", - passthrough: { mode: "gateway", mcp: { inherit_runtime: false } }, - }); - - expect(saved.mode).toBe("local"); - expect(saved.mcp).toEqual({ inherit_runtime: false }); - }); - - it("omits passthrough entirely when there are no foreign keys", () => { - const parsed = parseOpenclawRuntimeConfig({ mode: "local" }); - expect(parsed.passthrough).toBeUndefined(); - expect(serializeOpenclawRuntimeConfig(parsed)).toEqual({ mode: "local" }); - }); - - it("does not report a config as dirty because of passthrough alone", () => { - expect( - openclawRuntimeConfigEquals( - { mode: "local", passthrough: { mcp: { inherit_runtime: true } } }, - { mode: "local" }, - ), - ).toBe(true); - }); -}); diff --git a/packages/core/agents/openclaw-runtime-config.ts b/packages/core/agents/openclaw-runtime-config.ts index 8fe641e56e6..c0b1190a2be 100644 --- a/packages/core/agents/openclaw-runtime-config.ts +++ b/packages/core/agents/openclaw-runtime-config.ts @@ -17,16 +17,6 @@ export interface OpenclawGatewayPin { export interface OpenclawRuntimeConfig { mode?: OpenclawRoutingMode; gateway?: OpenclawGatewayPin; - /** - * Keys of `runtime_config` this form does not own, carried through parse → - * serialize untouched so saving OpenClaw routing cannot delete settings - * stored alongside it — notably `mcp.inherit_runtime`, which decides whether - * the agent may reach the runtime host's MCP servers (GitHub #6283). - * - * Deliberately excluded from `openclawRuntimeConfigEquals`: the form never - * edits these, so they must not make it look dirty. - */ - passthrough?: Record; } // Sentinel the API substitutes for a non-empty `gateway.token` on every read. @@ -36,10 +26,9 @@ export interface OpenclawRuntimeConfig { export const OPENCLAW_GATEWAY_TOKEN_MASK = "***"; // Parse an arbitrary runtime_config payload into the typed schema. Unknown -// keys are preserved verbatim under `passthrough` (see below), malformed -// payloads collapse to an empty object. The form never throws on bad input — -// invalid configs simply render as defaults so the user can correct them -// without a JSON parse error blocking the UI. +// keys are dropped, malformed payloads collapse to an empty object. The form +// never throws on bad input — invalid configs simply render as defaults so +// the user can correct them without a JSON parse error blocking the UI. export function parseOpenclawRuntimeConfig( raw: unknown, ): OpenclawRuntimeConfig { @@ -58,18 +47,6 @@ export function parseOpenclawRuntimeConfig( if (typeof gw.tls === "boolean") pin.tls = gw.tls; if (Object.keys(pin).length > 0) out.gateway = pin; } - // Keep every key this form does not own. `runtime_config` is a single JSONB - // column shared with settings owned elsewhere — notably - // `mcp.inherit_runtime`, which controls whether the agent may reach the - // host's MCP servers (GitHub #6283). Dropping unknown keys here made saving - // an unrelated OpenClaw routing change silently reset that security setting, - // because the tab persists this parse result as the WHOLE object. - const passthrough: Record = {}; - for (const [key, value] of Object.entries(root)) { - if (key === "mode" || key === "gateway") continue; - passthrough[key] = value; - } - if (Object.keys(passthrough).length > 0) out.passthrough = passthrough; return out; } @@ -81,13 +58,6 @@ export function serializeOpenclawRuntimeConfig( cfg: OpenclawRuntimeConfig, ): Record { const out: Record = {}; - // Settings owned by other tabs go back first so this form can never clobber - // them; `mode`/`gateway` below are the only keys it owns. - if (cfg.passthrough) { - for (const [key, value] of Object.entries(cfg.passthrough)) { - out[key] = value; - } - } if (cfg.mode) out.mode = cfg.mode; if (cfg.gateway) { const gw: Record = {}; diff --git a/packages/core/api/client.test.ts b/packages/core/api/client.test.ts index 174e7b99a33..d79a1d165a7 100644 --- a/packages/core/api/client.test.ts +++ b/packages/core/api/client.test.ts @@ -1706,95 +1706,3 @@ describe("ApiClient unsubscribe endpoints", () => { ).rejects.toBeInstanceOf(ApiError); }); }); - -// Runtime capability discovery feeds the agent MCP tab's security copy, so a -// malformed body must degrade to an explicit failure that asserts NO MCP -// boundary — never a fabricated guarantee (GitHub #6283). -describe("ApiClient runtime capability discovery", () => { - function stubJSON(body: unknown) { - vi.stubGlobal( - "fetch", - vi.fn().mockResolvedValue( - new Response(JSON.stringify(body), { - status: 200, - headers: { "Content-Type": "application/json" }, - }), - ), - ); - } - - it("degrades a malformed initiate response to an explicit failure", async () => { - stubJSON({ status: 7, skills: "nope" }); - - const result = await new ApiClient( - "https://api.example.test", - ).initiateListLocalSkills("rt-1"); - - expect(result.status).toBe("failed"); - expect(result.authoritative_mcp).toBe(false); - expect(result.mcp_supported).toBe(false); - expect(result.error).toBe("invalid runtime capability response"); - expect(result.runtime_id).toBe("rt-1"); - }); - - it("degrades a malformed poll response to an explicit failure", async () => { - stubJSON("not-an-object"); - - const result = await new ApiClient( - "https://api.example.test", - ).getListLocalSkillsResult("rt-1", "req-1"); - - expect(result.status).toBe("failed"); - expect(result.authoritative_mcp).toBe(false); - expect(result.id).toBe("req-1"); - expect(result.runtime_id).toBe("rt-1"); - }); - - // An older daemon omits the flag entirely; the response is otherwise valid and - // must stay usable, with the flag reading false. - it("keeps an older daemon's response usable with authoritative_mcp false", async () => { - stubJSON({ - id: "req-1", - runtime_id: "rt-1", - status: "completed", - supported: true, - skills: [], - mcp_servers: [ - { name: "linear", transport: "http", enabled: true }, - ], - mcp_supported: true, - created_at: "", - updated_at: "", - }); - - const result = await new ApiClient( - "https://api.example.test", - ).getListLocalSkillsResult("rt-1", "req-1"); - - expect(result.status).toBe("completed"); - expect(result.mcp_supported).toBe(true); - expect(result.authoritative_mcp).toBe(false); - expect(result.mcp_servers?.[0]?.name).toBe("linear"); - }); - - it("passes a current daemon's authoritative_mcp through", async () => { - stubJSON({ - id: "req-1", - runtime_id: "rt-1", - status: "completed", - supported: true, - skills: [], - mcp_servers: [], - mcp_supported: true, - authoritative_mcp: true, - created_at: "", - updated_at: "", - }); - - const result = await new ApiClient( - "https://api.example.test", - ).getListLocalSkillsResult("rt-1", "req-1"); - - expect(result.authoritative_mcp).toBe(true); - }); -}); diff --git a/packages/core/api/client.ts b/packages/core/api/client.ts index 412b5e93bb8..43bb7eedea6 100644 --- a/packages/core/api/client.ts +++ b/packages/core/api/client.ts @@ -311,8 +311,6 @@ import { EMPTY_LIST_GITHUB_REPOSITORIES_RESPONSE, RuntimeModelListRequestSchema, MALFORMED_RUNTIME_MODEL_LIST_REQUEST, - RuntimeLocalSkillListRequestSchema, - MALFORMED_RUNTIME_LOCAL_SKILL_LIST_REQUEST, } from "./schemas"; /** Identifies the calling client to the server. @@ -1783,45 +1781,19 @@ export class ApiClient { ); } - // Both capability-discovery endpoints feed the same poll-then-render state - // machine as model discovery, and their `authoritative_mcp` flag decides - // whether the agent MCP tab may claim the runtime's own MCP servers are - // excluded from an agent. That is a security statement, so the body is - // validated rather than cast: an unparseable response degrades to an explicit - // "failed" record with authoritative_mcp false, never to a fabricated - // guarantee (GitHub #6283). async initiateListLocalSkills( runtimeId: string, ): Promise { - const raw = await this.fetch( - `/api/runtimes/${runtimeId}/local-skills`, - { method: "POST" }, - ); - return parseWithFallback( - raw, - RuntimeLocalSkillListRequestSchema, - { ...MALFORMED_RUNTIME_LOCAL_SKILL_LIST_REQUEST, runtime_id: runtimeId }, - { endpoint: "POST /api/runtimes/{id}/local-skills" }, - ); + return this.fetch(`/api/runtimes/${runtimeId}/local-skills`, { + method: "POST", + }); } async getListLocalSkillsResult( runtimeId: string, requestId: string, ): Promise { - const raw = await this.fetch( - `/api/runtimes/${runtimeId}/local-skills/${requestId}`, - ); - return parseWithFallback( - raw, - RuntimeLocalSkillListRequestSchema, - { - ...MALFORMED_RUNTIME_LOCAL_SKILL_LIST_REQUEST, - id: requestId, - runtime_id: runtimeId, - }, - { endpoint: "GET /api/runtimes/{id}/local-skills/{requestId}" }, - ); + return this.fetch(`/api/runtimes/${runtimeId}/local-skills/${requestId}`); } async initiateImportLocalSkill( diff --git a/packages/core/api/schemas.test.ts b/packages/core/api/schemas.test.ts index b7b37739c4c..7225e1d16c0 100644 --- a/packages/core/api/schemas.test.ts +++ b/packages/core/api/schemas.test.ts @@ -25,8 +25,6 @@ import { ListIssuesResponseSchema, ListPropertiesResponseSchema, MALFORMED_RUNTIME_MODEL_LIST_REQUEST, - RuntimeLocalSkillListRequestSchema, - MALFORMED_RUNTIME_LOCAL_SKILL_LIST_REQUEST, RuntimeModelListRequestSchema, SearchProjectsResponseSchema, RuntimeHourlyActivityListSchema, @@ -1109,109 +1107,3 @@ describe("RuntimeModelListRequestSchema", () => { ); }); }); - -// Runtime capability discovery. `authoritative_mcp` gates whether the agent MCP -// tab may tell an operator that the host's MCP servers are excluded from an -// agent, so every drift path must resolve to the fail-closed value rather than -// an unchecked assertion (GitHub #6283). -describe("RuntimeLocalSkillListRequestSchema", () => { - const completed = { - id: "req-1", - runtime_id: "rt-1", - status: "completed", - supported: true, - skills: [ - { - key: "review", - name: "Review", - source_path: "/home/u/.claude/skills/review", - provider: "claude", - file_count: 3, - }, - ], - mcp_servers: [ - { name: "linear", transport: "http", source: "User config", enabled: true }, - ], - mcp_supported: true, - authoritative_mcp: true, - created_at: "2026-08-03T00:00:00Z", - updated_at: "2026-08-03T00:00:01Z", - }; - - it("parses a live completed discovery, keeping the fields the UI branches on", () => { - const parsed = parseWithFallback( - completed, - RuntimeLocalSkillListRequestSchema, - MALFORMED_RUNTIME_LOCAL_SKILL_LIST_REQUEST, - { endpoint: "test" }, - ); - expect(parsed.status).toBe("completed"); - expect(parsed.mcp_supported).toBe(true); - expect(parsed.authoritative_mcp).toBe(true); - expect(parsed.mcp_servers?.[0]?.name).toBe("linear"); - expect(parsed.skills?.[0]?.key).toBe("review"); - }); - - // A daemon that predates the authoritative-MCP guarantee omits the flag. It - // must read as false — claiming enforcement off a missing field is exactly the - // false security guarantee this issue is about. - it("defaults authoritative_mcp to false when an older daemon omits it", () => { - const { authoritative_mcp: _omitted, ...withoutFlag } = completed; - const parsed = parseWithFallback( - withoutFlag, - RuntimeLocalSkillListRequestSchema, - MALFORMED_RUNTIME_LOCAL_SKILL_LIST_REQUEST, - { endpoint: "test" }, - ); - expect(parsed.status).toBe("completed"); - expect(parsed.authoritative_mcp).toBe(false); - }); - - it("defaults authoritative_mcp to false when it arrives as a non-boolean", () => { - for (const bad of ["true", 1, null, {}]) { - const parsed = parseWithFallback( - { ...completed, authoritative_mcp: bad }, - RuntimeLocalSkillListRequestSchema, - MALFORMED_RUNTIME_LOCAL_SKILL_LIST_REQUEST, - { endpoint: "test" }, - ); - expect(parsed.authoritative_mcp).toBe(false); - } - }); - - it("defaults mcp_supported to false when omitted", () => { - const { mcp_supported: _omitted, ...withoutFlag } = completed; - const parsed = parseWithFallback( - withoutFlag, - RuntimeLocalSkillListRequestSchema, - MALFORMED_RUNTIME_LOCAL_SKILL_LIST_REQUEST, - { endpoint: "test" }, - ); - expect(parsed.mcp_supported).toBe(false); - }); - - it("degrades a malformed body to an explicit failure with no MCP guarantee", () => { - for (const bad of ["not-an-object", 7, null, { status: 7 }]) { - const parsed = parseWithFallback( - bad, - RuntimeLocalSkillListRequestSchema, - MALFORMED_RUNTIME_LOCAL_SKILL_LIST_REQUEST, - { endpoint: "test" }, - ); - expect(parsed.status).toBe("failed"); - expect(parsed.authoritative_mcp).toBe(false); - expect(parsed.error).toBe("invalid runtime capability response"); - } - }); - - it("keeps unknown additive fields instead of failing the response", () => { - const parsed = parseWithFallback( - { ...completed, some_future_field: "x" }, - RuntimeLocalSkillListRequestSchema, - MALFORMED_RUNTIME_LOCAL_SKILL_LIST_REQUEST, - { endpoint: "test" }, - ); - expect(parsed.status).toBe("completed"); - expect(parsed.authoritative_mcp).toBe(true); - }); -}); diff --git a/packages/core/api/schemas.ts b/packages/core/api/schemas.ts index 940879038cd..61d344bbdf7 100644 --- a/packages/core/api/schemas.ts +++ b/packages/core/api/schemas.ts @@ -43,7 +43,6 @@ import type { ListWebhookDeliveriesResponse, NotificationPreferenceResponse, ResourceLabelsResponse, - RuntimeLocalSkillListRequest, RuntimeModelListRequest, SearchIssuesResponse, SearchProjectsResponse, @@ -2017,70 +2016,3 @@ export const MALFORMED_RUNTIME_MODEL_LIST_REQUEST: RuntimeModelListRequest = { created_at: "", updated_at: "", }; - -export const RuntimeLocalSkillSummarySchema = z - .object({ - key: z.string().default(""), - name: z.string().default(""), - description: z.string().optional(), - source_path: z.string().default(""), - provider: z.string().default(""), - root: z.string().optional(), - plugin: z.string().optional(), - can_disable: z.boolean().optional(), - file_count: z.number().default(0), - }) - .loose(); - -export const RuntimeLocalMcpServerSummarySchema = z - .object({ - name: z.string().default(""), - transport: z.string().optional(), - source: z.string().optional(), - enabled: z.boolean().default(false), - }) - .loose(); - -// Runtime capability discovery (skills + MCP inventory). Validated rather than -// cast because `authoritative_mcp` decides whether the agent MCP tab may tell -// an operator that the host's MCP servers are excluded from an agent — a -// security claim that must not rest on an unchecked type assertion. -// -// Both MCP booleans default to FALSE, which is the fail-closed direction: -// - `authoritative_mcp` absent → the daemon predates the guarantee, so the -// UI shows "needs upgrade" instead of claiming the boundary is enforced. -// - `mcp_supported` absent → the runtime cannot report an inventory. -export const RuntimeLocalSkillListRequestSchema = z - .object({ - id: z.string().default(""), - runtime_id: z.string().default(""), - status: z.string(), - skills: z.array(RuntimeLocalSkillSummarySchema).optional(), - supported: z.boolean().default(true), - mcp_servers: z.array(RuntimeLocalMcpServerSummarySchema).optional(), - mcp_supported: z.boolean().default(false), - authoritative_mcp: z.boolean().default(false), - error: z.string().optional(), - created_at: z.string().default(""), - updated_at: z.string().default(""), - }) - .loose(); - -// Fallback for an unparseable capability-discovery response. Mirrors the model -// discovery rationale: `failed` surfaces the problem immediately instead of -// spinning until the poll timeout, and — unlike `completed` — cannot be read as -// "this runtime genuinely has no MCP servers". Critically it leaves -// `authoritative_mcp` false, so a malformed response can never let the UI -// assert an MCP boundary it has not verified. -export const MALFORMED_RUNTIME_LOCAL_SKILL_LIST_REQUEST: RuntimeLocalSkillListRequest = - { - id: "", - runtime_id: "", - status: "failed", - supported: true, - mcp_supported: false, - authoritative_mcp: false, - error: "invalid runtime capability response", - created_at: "", - updated_at: "", - }; diff --git a/packages/core/dashboard/failure-class.test.ts b/packages/core/dashboard/failure-class.test.ts index aacba8c9352..e647ee4268e 100644 --- a/packages/core/dashboard/failure-class.test.ts +++ b/packages/core/dashboard/failure-class.test.ts @@ -18,10 +18,6 @@ describe("failureClassOf", () => { // MUL-5370: the run never reached the model provider, so this belongs // with the substrate failures an operator fixes by checking the daemon. expect(failureClassOf("skill_bundle_unavailable")).toBe("runtime"); - // GitHub #6283: the claim was refused because the runtime's daemon is too - // old to enforce the agent's MCP allowlist — an operator fixes it by - // upgrading that daemon, so it counts as substrate, not agent behaviour. - expect(failureClassOf("mcp_config_daemon_outdated")).toBe("runtime"); expect(failureClassOf("agent_error.process_failure")).toBe("agent"); }); diff --git a/packages/core/dashboard/failure-class.ts b/packages/core/dashboard/failure-class.ts index 02c11c17fd1..31ea676179f 100644 --- a/packages/core/dashboard/failure-class.ts +++ b/packages/core/dashboard/failure-class.ts @@ -1,6 +1,6 @@ // Display grouping for `agent_task_queue.failure_reason`. // -// The backend taxonomy (server/pkg/taskfailure) has 23 reasons, which is far +// The backend taxonomy (server/pkg/taskfailure) has 22 reasons, which is far // too many series for a stacked chart or a scannable breakdown list. These // seven classes are the granularity an operator actually acts on: an auth // spike means "go re-auth", a rate-limit spike means "back off or raise the @@ -23,7 +23,7 @@ export const FAILURE_CLASSES = [ export type FailureClass = (typeof FAILURE_CLASSES)[number]; -// Reason → class. Keys are the wire values written by the backend: the 23 +// Reason → class. Keys are the wire values written by the backend: the 22 // canonical `taskfailure.Reason` strings, the `"unclassified"` sentinel the // failure rollups substitute for a failed row with an empty column, and the // pre-MUL-1949 coarse values that still sit in historical rows. @@ -65,11 +65,6 @@ const REASON_CLASS: Record = { // operator response is "check the daemon's link to Multica", the same as a // daemon that went offline — the model provider is not involved. skill_bundle_unavailable: "runtime", - // The claiming daemon predates authoritative mcp_config enforcement, so the - // run was refused rather than given the host's MCP servers (GitHub #6283). - // Runtime class for the same reason as the line above: the operator response - // is "upgrade the daemon", and the model provider is not involved. - mcp_config_daemon_outdated: "runtime", // The agent process itself produced the failure. "agent_error.process_failure": "agent", diff --git a/packages/core/runtimes/local-skills.ts b/packages/core/runtimes/local-skills.ts index 54c9d371095..7be65e4f0fd 100644 --- a/packages/core/runtimes/local-skills.ts +++ b/packages/core/runtimes/local-skills.ts @@ -50,7 +50,6 @@ export async function resolveRuntimeLocalSkills( supported: current.supported, mcpServers: current.mcp_servers ?? [], mcpSupported: current.mcp_supported === true, - authoritativeMcp: current.authoritative_mcp === true, }; } diff --git a/packages/core/types/agent.ts b/packages/core/types/agent.ts index 4d2c3c26ebd..115acceb49f 100644 --- a/packages/core/types/agent.ts +++ b/packages/core/types/agent.ts @@ -949,7 +949,7 @@ export interface DashboardRunTimeDaily { // One (date, failure_reason) bucket of terminal-task counts for the workspace // dashboard's Errors metric. // -// `failure_reason` carries the backend's canonical failure taxonomy (the 23 +// `failure_reason` carries the backend's canonical failure taxonomy (the 21 // `taskfailure.Reason` values, plus `"unclassified"` for failed rows with an // empty column) — EXCEPT for the empty string, which is the *succeeded* // bucket. Shipping successes in the same series is deliberate: the error rate @@ -1132,13 +1132,6 @@ export interface RuntimeLocalSkillListRequest { supported: boolean; mcp_servers?: RuntimeLocalMcpServerSummary[]; mcp_supported?: boolean; - /** - * Whether the daemon enforces a managed `mcp_config` as an authoritative - * allowlist. Absent/false on daemons predating that fix, which still merge - * the host's own MCP servers underneath the managed set (GitHub #6283) — - * the UI must not claim those servers are excluded. - */ - authoritative_mcp?: boolean; error?: string; created_at: string; updated_at: string; @@ -1175,8 +1168,6 @@ export interface RuntimeLocalSkillsResult { supported: boolean; mcpServers: RuntimeLocalMcpServerSummary[]; mcpSupported: boolean; - /** See `RuntimeLocalSkillListRequest.authoritative_mcp`. */ - authoritativeMcp: boolean; } export interface RuntimeLocalSkillImportResult { diff --git a/packages/views/agents/components/tabs/activity-tab.tsx b/packages/views/agents/components/tabs/activity-tab.tsx index 516bf1a4e9a..e1cece70c31 100644 --- a/packages/views/agents/components/tabs/activity-tab.tsx +++ b/packages/views/agents/components/tabs/activity-tab.tsx @@ -585,7 +585,7 @@ function TaskRow({ // Failure reason. The back-end emits "" on non-failed tasks (omitempty // strips it on the wire) so the truthy guard is the right shape. - // failureReasonLabel takes the raw open string — the taxonomy has 23 + // failureReasonLabel takes the raw open string — the taxonomy has 21 // values and grows, so there is no enum to cast to. const failureLabel = task.status === "failed" ? failureReasonLabel(task.failure_reason) : null; diff --git a/packages/views/agents/components/tabs/mcp-config-model.test.ts b/packages/views/agents/components/tabs/mcp-config-model.test.ts index 049263b7ff7..0b308fce5cf 100644 --- a/packages/views/agents/components/tabs/mcp-config-model.test.ts +++ b/packages/views/agents/components/tabs/mcp-config-model.test.ts @@ -1,8 +1,5 @@ import { describe, expect, it } from "vitest"; import { - clearManagedMcpConfig, - hasManagedMcpConfig, - inheritsRuntimeMcp, listManagedMcpServers, removeManagedMcpServer, upsertManagedMcpServer, @@ -53,105 +50,15 @@ describe("mcp config compatibility model", () => { }); }); - // Deleting the last managed server must NOT clear the config to null: null - // means "inherit the runtime host's MCP servers" on the daemon side, so - // collapsing to it would turn a delete into a privilege widening — from one - // allowed server to every server on the host (GitHub #6283). - it("keeps an explicitly empty config when the last server is deleted", () => { + it("returns null only when deleting the last value from an otherwise empty document", () => { const value = { mcpServers: { fetch: { command: "uvx" } } }; const [fetch] = listManagedMcpServers(value); - expect(removeManagedMcpServer(value, fetch!)).toEqual({ mcpServers: {} }); + expect(removeManagedMcpServer(value, fetch!)).toBeNull(); const withMetadata = { version: 1, ...value }; const [withMetadataFetch] = listManagedMcpServers(withMetadata); expect(removeManagedMcpServer(withMetadata, withMetadataFetch!)).toEqual({ version: 1, - mcpServers: {}, }); }); - - it("removes only the named server when siblings remain", () => { - const value = { - mcpServers: { fetch: { command: "uvx" }, docs: { url: "https://x/mcp" } }, - }; - const fetch = listManagedMcpServers(value).find((s) => s.name === "fetch"); - expect(removeManagedMcpServer(value, fetch!)).toEqual({ - mcpServers: { docs: { url: "https://x/mcp" } }, - }); - }); - - // Restoring inheritance is a separate, deliberate action. - it("clearManagedMcpConfig is the only path back to inheritance", () => { - expect(clearManagedMcpConfig()).toBeNull(); - }); -}); - -// A managed mcp_config is an authoritative allowlist on the daemon side -// (GitHub #6283), so the tab has to know which mode an agent is in before it -// can describe the runtime servers honestly. -describe("hasManagedMcpConfig", () => { - it("treats an absent config as unmanaged", () => { - expect(hasManagedMcpConfig(null)).toBe(false); - expect(hasManagedMcpConfig(undefined)).toBe(false); - }); - - it("treats an explicitly empty object as managed", () => { - expect(hasManagedMcpConfig({})).toBe(true); - expect(hasManagedMcpConfig({ mcpServers: {} })).toBe(true); - }); - - it("treats a redacted config as managed even though it is unreadable", () => { - expect(hasManagedMcpConfig(null, true)).toBe(true); - }); -}); - -describe("inheritsRuntimeMcp", () => { - it("inherits when Multica manages nothing", () => { - expect(inheritsRuntimeMcp(null, false, undefined)).toBe(true); - expect(inheritsRuntimeMcp(null, false, {})).toBe(true); - }); - - it("does not inherit for an explicitly empty managed config", () => { - expect(inheritsRuntimeMcp({ mcpServers: {} }, false, undefined)).toBe( - false, - ); - }); - - it("does not inherit for a managed allowlist", () => { - expect( - inheritsRuntimeMcp( - { mcpServers: { a: { command: "a" } } }, - false, - undefined, - ), - ).toBe(false); - }); - - it("does not inherit for a redacted config", () => { - expect(inheritsRuntimeMcp(null, true, undefined)).toBe(false); - }); - - it("inherits again when the agent explicitly opts back in", () => { - expect( - inheritsRuntimeMcp({ mcpServers: {} }, false, { - mcp: { inherit_runtime: true }, - }), - ).toBe(true); - }); - - it("stays strict for a falsy or malformed opt-in", () => { - expect( - inheritsRuntimeMcp({ mcpServers: {} }, false, { - mcp: { inherit_runtime: false }, - }), - ).toBe(false); - expect( - inheritsRuntimeMcp({ mcpServers: {} }, false, { - mcp: { inherit_runtime: "true" }, - }), - ).toBe(false); - expect(inheritsRuntimeMcp({ mcpServers: {} }, false, { mcp: true })).toBe( - false, - ); - }); }); diff --git a/packages/views/agents/components/tabs/mcp-config-model.ts b/packages/views/agents/components/tabs/mcp-config-model.ts index 0ace9534b1d..8898c3ba61e 100644 --- a/packages/views/agents/components/tabs/mcp-config-model.ts +++ b/packages/views/agents/components/tabs/mcp-config-model.ts @@ -75,69 +75,19 @@ export function upsertManagedMcpServer( return document; } -/** - * Remove one managed server. - * - * Deleting the LAST managed server deliberately leaves an explicitly empty - * `{"mcpServers":{}}` rather than clearing the config to `null`. `null` means - * "inherit the runtime host's servers" on the daemon side, so collapsing to it - * here would turn a delete into a privilege *widening* — the agent would go - * from one allowed server to every server on the host (GitHub #6283). - * - * Use `clearManagedMcpConfig` for the separate, explicit "stop managing MCP and - * inherit the host's servers again" action. - */ export function removeManagedMcpServer( value: unknown, server: ManagedMcpServer, -): Record { - const document = isRecord(value) ? { ...value } : {}; +): Record | null { + if (!isRecord(value)) return null; + const document = { ...value }; const container = isRecord(document[server.container]) ? { ...(document[server.container] as Record) } : {}; delete container[server.name]; - document[server.container] = container; - return document; -} - -/** - * Clear the managed MCP config entirely, handing MCP scope back to the runtime - * host. Returns `null`, which the daemon reads as "inherit natively". - * - * This WIDENS the agent's tool surface, so it must stay a deliberate action - * with its own confirmation — never the incidental result of deleting a server. - */ -export function clearManagedMcpConfig(): null { - return null; -} - -/** - * True when Multica manages an MCP config for this agent — including an - * explicitly empty one, and including a config the viewer isn't allowed to - * read. A managed config is an authoritative allowlist on the daemon side - * (GitHub #6283), so this is what decides whether runtime servers reach the - * agent at all. - */ -export function hasManagedMcpConfig( - mcpConfig: unknown, - redacted?: boolean, -): boolean { - if (redacted === true) return true; - return isRecord(mcpConfig); -} + if (Object.keys(container).length > 0) document[server.container] = container; + else delete document[server.container]; -/** - * True when the local runtime's own MCP servers are still exposed to this - * agent. That happens when Multica manages no config for it, or when the agent - * explicitly opted back in via `runtime_config.mcp.inherit_runtime`. - */ -export function inheritsRuntimeMcp( - mcpConfig: unknown, - redacted: boolean | undefined, - runtimeConfig: Record | undefined, -): boolean { - if (!hasManagedMcpConfig(mcpConfig, redacted)) return true; - const mcp = runtimeConfig?.mcp; - return isRecord(mcp) && mcp.inherit_runtime === true; + return Object.keys(document).length > 0 ? document : null; } diff --git a/packages/views/agents/components/tabs/mcp-config-tab.test.tsx b/packages/views/agents/components/tabs/mcp-config-tab.test.tsx index e66239956f7..608af960487 100644 --- a/packages/views/agents/components/tabs/mcp-config-tab.test.tsx +++ b/packages/views/agents/components/tabs/mcp-config-tab.test.tsx @@ -224,9 +224,7 @@ describe("McpConfigTab", () => { }); }); - // Deleting the last managed server must leave a strict empty config, not - // clear it to null — null re-opens every host MCP server (GitHub #6283). - it("deletes the last managed server only after confirmation, keeping strict empty", async () => { + it("deletes the last managed server only after confirmation", async () => { const user = userEvent.setup(); const { onSave } = renderTab({ mcp_config: { mcpServers: { fetch: { command: "uvx" } } }, @@ -235,37 +233,12 @@ describe("McpConfigTab", () => { await user.click( screen.getByRole("button", { name: /delete mcp server fetch/i }), ); - expect( - screen.getByText(/the runtime's own mcp servers stay excluded/i), - ).toBeInTheDocument(); + expect(screen.getByText(/runtime servers are not affected/i)).toBeInTheDocument(); await user.click(screen.getByRole("button", { name: /delete server/i })); - expect(onSave).toHaveBeenCalledWith({ mcp_config: { mcpServers: {} } }); - }); - - it("restores inheritance only through the separate clear action", async () => { - const user = userEvent.setup(); - const { onSave } = renderTab({ - mcp_config: { mcpServers: { fetch: { command: "uvx" } } }, - }); - - await user.click(screen.getByRole("button", { name: /stop managing mcp/i })); - expect(screen.getByText(/that widens the tools it can reach/i)).toBeInTheDocument(); - await user.click( - screen.getByRole("button", { name: /stop managing and inherit/i }), - ); - expect(onSave).toHaveBeenCalledWith({ mcp_config: null }); }); - it("offers no clear action when Multica manages nothing", () => { - renderTab({ mcp_config: null }); - - expect( - screen.queryByRole("button", { name: /stop managing mcp/i }), - ).not.toBeInTheDocument(); - }); - it("blocks invalid single-server JSON", async () => { const user = userEvent.setup(); const { onSave } = renderTab(); @@ -297,87 +270,6 @@ describe("McpConfigTab", () => { expect(await screen.findByText("linear")).toBeInTheDocument(); }); - // A managed mcp_config is an authoritative allowlist on the daemon side - // (GitHub #6283). The tab previously told every operator that managed servers - // are "merged with runtime servers", which is how an explicitly empty config - // came to look like a tightened tool scope while exposing the whole host. - it("marks runtime servers as not exposed when Multica manages a config", async () => { - mockRuntimeCapabilities.mockResolvedValue({ - skills: [], - supported: true, - mcpServers: [ - { name: "linear", transport: "http", source: "User config", enabled: true }, - ], - mcpSupported: true, - authoritativeMcp: true, - }); - - renderTab({ mcp_config: { mcpServers: {} } }, undefined, onlineRuntime); - - expect(await screen.findByText("linear")).toBeInTheDocument(); - expect(screen.getByText("Not exposed")).toBeInTheDocument(); - // The hint interpolates the runtime label, so match the part of the copy - // that carries the security-relevant claim. - expect( - screen.getByText(/so these servers are not exposed to it/i), - ).toBeInTheDocument(); - expect( - screen.getByText(enAgents.tab_body.mcp_config.managed_hint_authoritative), - ).toBeInTheDocument(); - }); - - // The strict semantics are enforced by the daemon, so a UI that claims - // "Not exposed" against an old daemon repeats the exact false security - // guarantee this issue is about (GitHub #6283). - it("warns instead of claiming exclusion when the daemon predates the fix", async () => { - mockRuntimeCapabilities.mockResolvedValue({ - skills: [], - supported: true, - mcpServers: [ - { name: "linear", transport: "http", source: "User config", enabled: true }, - ], - mcpSupported: true, - authoritativeMcp: false, - }); - - renderTab({ mcp_config: { mcpServers: {} } }, undefined, onlineRuntime); - - expect(await screen.findByText("linear")).toBeInTheDocument(); - expect(screen.queryByText("Not exposed")).not.toBeInTheDocument(); - expect( - screen.getByText(enAgents.tab_body.mcp_config.daemon_upgrade_required), - ).toBeInTheDocument(); - expect( - screen.getByText(/upgrade it to exclude them/i), - ).toBeInTheDocument(); - }); - - it("still describes runtime servers as merged when the agent opts back in", async () => { - mockRuntimeCapabilities.mockResolvedValue({ - skills: [], - supported: true, - mcpServers: [ - { name: "linear", transport: "http", source: "User config", enabled: true }, - ], - mcpSupported: true, - }); - - renderTab( - { - mcp_config: { mcpServers: {} }, - runtime_config: { mcp: { inherit_runtime: true } }, - }, - undefined, - onlineRuntime, - ); - - expect(await screen.findByText("linear")).toBeInTheDocument(); - expect(screen.queryByText("Not exposed")).not.toBeInTheDocument(); - expect( - screen.getByText(enAgents.tab_body.mcp_config.managed_hint), - ).toBeInTheDocument(); - }); - it("shows a permission notice when capability discovery is forbidden", async () => { mockRuntimeCapabilities.mockRejectedValue( new ApiError("insufficient permissions", 403, "Forbidden"), diff --git a/packages/views/agents/components/tabs/mcp-config-tab.tsx b/packages/views/agents/components/tabs/mcp-config-tab.tsx index 89cf9952d06..0a1698181cd 100644 --- a/packages/views/agents/components/tabs/mcp-config-tab.tsx +++ b/packages/views/agents/components/tabs/mcp-config-tab.tsx @@ -32,8 +32,6 @@ import { Button } from "@multica/ui/components/ui/button"; import { toast } from "sonner"; import { useT } from "../../../i18n"; import { - clearManagedMcpConfig, - inheritsRuntimeMcp, listManagedMcpServers, removeManagedMcpServer, upsertManagedMcpServer, @@ -59,20 +57,6 @@ export function McpConfigTab({ : null; const runtimeQuery = useQuery(runtimeCapabilitiesOptions(runtimeId)); const redacted = agent.mcp_config_redacted === true; - // A managed mcp_config is an authoritative allowlist on the daemon side, so - // the runtime's own servers are only exposed when Multica manages nothing for - // this agent or it explicitly opted back in (GitHub #6283). The copy below - // has to say which of the two is in effect — an operator who believes the - // wrong one mis-scopes the agent's tool access. - const inheritsRuntime = useMemo( - () => - inheritsRuntimeMcp( - agent.mcp_config, - agent.mcp_config_redacted, - agent.runtime_config, - ), - [agent.mcp_config, agent.mcp_config_redacted, agent.runtime_config], - ); const managedServers = useMemo( () => listManagedMcpServers(agent.mcp_config), [agent.mcp_config], @@ -88,18 +72,6 @@ export function McpConfigTab({ const [deletingServer, setDeletingServer] = useState(null); const [deleting, setDeleting] = useState(false); - const [clearOpen, setClearOpen] = useState(false); - const [clearing, setClearing] = useState(false); - - // The strict semantics are enforced by the DAEMON, so a config saved against - // an older daemon is not yet in effect: that daemon still merges the host's - // MCP servers. Claiming "Not exposed" in that window would be the same false - // security guarantee this issue is about, so only trust a daemon that - // reports the capability (GitHub #6283). - const runtimeAuthoritative = runtimeQuery.data?.authoritativeMcp === true; - const daemonEnforcesManagedConfig = !inheritsRuntime && runtimeAuthoritative; - const daemonNeedsUpgradeForMcp = - !inheritsRuntime && runtimeQuery.data !== undefined && !runtimeAuthoritative; useEffect(() => onDirtyChange?.(false), [onDirtyChange]); @@ -160,23 +132,6 @@ export function McpConfigTab({ } }; - const handleClearManaged = async () => { - setClearing(true); - try { - await onSave({ mcp_config: clearManagedMcpConfig() }); - toast.success(t(($) => $.tab_body.mcp_config.cleared_toast)); - setClearOpen(false); - } catch (error) { - toast.error( - error instanceof Error && error.message - ? error.message - : t(($) => $.tab_body.mcp_config.save_failed_toast), - ); - } finally { - setClearing(false); - } - }; - return (

@@ -190,9 +145,7 @@ export function McpConfigTab({ {t(($) => $.tab_body.mcp_config.managed_title)}

- {inheritsRuntime - ? t(($) => $.tab_body.mcp_config.managed_hint) - : t(($) => $.tab_body.mcp_config.managed_hint_authoritative)} + {t(($) => $.tab_body.mcp_config.managed_hint)}

{!redacted && ( @@ -203,14 +156,6 @@ export function McpConfigTab({ )} - {/* The saved config is only a real boundary once the daemon enforces - it. Warn instead of letting the operator assume it already does. */} - {daemonNeedsUpgradeForMcp && ( - $.tab_body.mcp_config.daemon_upgrade_required)} - /> - )} - {redacted ? (
$.tab_body.mcp_config.delete_aria)} /> ) : ( - $.tab_body.mcp_config.managed_empty) - : t(($) => $.tab_body.mcp_config.managed_empty_strict) - } - /> - )} - - {/* Restoring inheritance WIDENS the agent's tool surface, so it is its - own deliberate action rather than a side effect of deleting the last - server (GitHub #6283). Only offered when a managed config exists. */} - {!redacted && !inheritsRuntime && ( - + $.tab_body.mcp_config.managed_empty)} /> )} @@ -267,21 +192,9 @@ export function McpConfigTab({ {t(($) => $.tab_body.mcp_config.runtime_title)}

- {inheritsRuntime - ? t(($) => $.tab_body.mcp_config.runtime_hint, { - runtime: runtime ? runtimeDisplayLabel(runtime) : "Runtime", - }) - : daemonNeedsUpgradeForMcp - ? t(($) => $.tab_body.mcp_config.runtime_hint_needs_upgrade, { - runtime: runtime - ? runtimeDisplayLabel(runtime) - : "Runtime", - }) - : t(($) => $.tab_body.mcp_config.runtime_hint_excluded, { - runtime: runtime - ? runtimeDisplayLabel(runtime) - : "Runtime", - })} + {t(($) => $.tab_body.mcp_config.runtime_hint, { + runtime: runtime ? runtimeDisplayLabel(runtime) : "Runtime", + })}

{runtimeId && ( @@ -334,20 +247,10 @@ export function McpConfigTab({ transport: server.transport || "unknown", enabled: server.enabled, source: server.source, - // Only claim exclusion once the daemon actually enforces it. - // Against an older daemon these servers ARE still reachable, so - // fall back to the name-collision badge and let the section hint - // carry the "needs upgrade" warning. - overridden: daemonEnforcesManagedConfig - ? true - : managedNames.has(server.name), + overridden: managedNames.has(server.name), }))} disabledLabel={t(($) => $.tab_body.mcp_config.runtime_disabled_badge)} - overriddenLabel={ - daemonEnforcesManagedConfig - ? t(($) => $.tab_body.mcp_config.runtime_excluded_badge) - : t(($) => $.tab_body.mcp_config.runtime_overridden_badge) - } + overriddenLabel={t(($) => $.tab_body.mcp_config.runtime_overridden_badge)} /> )} @@ -397,42 +300,6 @@ export function McpConfigTab({ - - !open && !clearing && setClearOpen(false)} - > - - - - {t(($) => $.tab_body.mcp_config.clear_dialog_title)} - - - {t(($) => $.tab_body.mcp_config.clear_dialog_description, { - runtime: runtime ? runtimeDisplayLabel(runtime) : "Runtime", - })} - - - - - {t(($) => $.tab_body.mcp_config.dialog_cancel)} - - - {clearing && ( - - - - ); } diff --git a/packages/views/agents/components/tabs/runtime-config-tab.tsx b/packages/views/agents/components/tabs/runtime-config-tab.tsx index 2ee6653abc6..ea8bf11780a 100644 --- a/packages/views/agents/components/tabs/runtime-config-tab.tsx +++ b/packages/views/agents/components/tabs/runtime-config-tab.tsx @@ -53,17 +53,8 @@ function configToForm(cfg: OpenclawRuntimeConfig): FormState { }; } -function formToConfig( - state: FormState, - passthrough?: Record, -): OpenclawRuntimeConfig { +function formToConfig(state: FormState): OpenclawRuntimeConfig { const cfg: OpenclawRuntimeConfig = { mode: state.mode }; - // Carry keys owned by other tabs (e.g. mcp.inherit_runtime) so saving this - // form cannot delete them — the tab persists the serialized result as the - // WHOLE runtime_config object (GitHub #6283). - if (passthrough && Object.keys(passthrough).length > 0) { - cfg.passthrough = passthrough; - } if (state.mode === "gateway") { const gw: NonNullable = {}; if (state.host.trim() !== "") gw.host = state.host.trim(); @@ -116,10 +107,7 @@ export function RuntimeConfigTab({ previousFormRef.current = originalForm; }, [originalForm]); - const currentCfg = useMemo( - () => formToConfig(state, original.passthrough), - [state, original.passthrough], - ); + const currentCfg = useMemo(() => formToConfig(state), [state]); const dirty = !openclawRuntimeConfigEquals(original, currentCfg); useEffect(() => { diff --git a/packages/views/agents/components/tabs/task-failure.test.ts b/packages/views/agents/components/tabs/task-failure.test.ts index 1280789b4e7..ac8099ec523 100644 --- a/packages/views/agents/components/tabs/task-failure.test.ts +++ b/packages/views/agents/components/tabs/task-failure.test.ts @@ -42,17 +42,3 @@ describe("failureReasonLabel", () => { expect(failureReasonLabel(undefined)).toBeNull(); }); }); - -// The agent activity list and the issue execution log surface failure_reason -// directly, so an unmapped reason renders the raw machine value -// `mcp_config_daemon_outdated` and the operator cannot tell what to do -// (GitHub #6283). -describe("failureReasonLabel for the MCP daemon-upgrade refusal", () => { - it("names the operator action instead of leaking the wire value", () => { - const label = failureReasonLabel("mcp_config_daemon_outdated"); - expect(label).not.toBe("mcp_config_daemon_outdated"); - expect(label).toBe("Daemon too old for this agent's MCP limits — upgrade it"); - // The point of the label is that it says what to do. - expect(label).toMatch(/upgrade/i); - }); -}); diff --git a/packages/views/agents/components/tabs/task-failure.ts b/packages/views/agents/components/tabs/task-failure.ts index 95c4766dc9a..1e8ceb8f4b3 100644 --- a/packages/views/agents/components/tabs/task-failure.ts +++ b/packages/views/agents/components/tabs/task-failure.ts @@ -7,7 +7,7 @@ // failed tasks no longer have a top-level workload state; failure context // is purely a detail-page concern now. // -// Covers the canonical taxonomy in server/pkg/taskfailure — 9 platform-side +// Covers the canonical taxonomy in server/pkg/taskfailure — 7 platform-side // reasons plus 14 `agent_error.*` sub-reasons — and the pre-MUL-1949 coarse // values still present on historical rows. This used to be a // `Record` indexed with a cast, which silently @@ -24,10 +24,6 @@ const REASON_LABEL: Record = { agent_blocked: "Waiting on human input", api_invalid_request: "Rejected by the model API", skill_bundle_unavailable: "Couldn't download the agent's skills", - // Names the operator action, not just the condition: this task did not run - // because the runtime's daemon is too old to enforce the agent's MCP - // allowlist (GitHub #6283). - mcp_config_daemon_outdated: "Daemon too old for this agent's MCP limits — upgrade it", // Agent process side — provider. "agent_error.provider_auth_or_access": "Provider auth failed", diff --git a/packages/views/chat/components/chat-message-list.test.tsx b/packages/views/chat/components/chat-message-list.test.tsx index 01398e96fd4..ecc0cf030b6 100644 --- a/packages/views/chat/components/chat-message-list.test.tsx +++ b/packages/views/chat/components/chat-message-list.test.tsx @@ -344,19 +344,6 @@ describe("ChatMessageList failure copy (MUL-5370 regression)", () => { expect(screen.queryByText(FALLBACK)).not.toBeInTheDocument(); }); - // The daemon-upgrade refusal has to say what to upgrade. Without the map - // entry the chat showed only generic fallback copy and buried the actionable - // detail in the collapsed raw error (GitHub #6283). - it("renders dedicated copy when the daemon is too old to enforce MCP limits", async () => { - renderFailure("mcp_config_daemon_outdated"); - expect( - await screen.findByText( - enChat.message_list.failure.mcp_config_daemon_outdated, - ), - ).toBeInTheDocument(); - expect(screen.queryByText(FALLBACK)).not.toBeInTheDocument(); - }); - it("renders dedicated copy for a refined reason the map names", async () => { renderFailure("agent_error.provider_network"); expect( diff --git a/packages/views/chat/components/chat-message-list.tsx b/packages/views/chat/components/chat-message-list.tsx index e97eb764453..c1dae6be41c 100644 --- a/packages/views/chat/components/chat-message-list.tsx +++ b/packages/views/chat/components/chat-message-list.tsx @@ -893,9 +893,6 @@ function FailureBubble({ manual: t(($) => $.message_list.failure.manual), cancelled: t(($) => $.message_list.failure.manual), skill_bundle_unavailable: t(($) => $.message_list.failure.skill_bundle_unavailable), - mcp_config_daemon_outdated: t( - ($) => $.message_list.failure.mcp_config_daemon_outdated, - ), "agent_error.provider_network": t(($) => $.message_list.failure.provider_network), "agent_error.provider_auth_or_access": t(($) => $.message_list.failure.provider_auth_or_access), "agent_error.provider_quota_limit": t(($) => $.message_list.failure.provider_quota_limit), diff --git a/packages/views/locales/en/agents.json b/packages/views/locales/en/agents.json index 1c999e8dc5c..eb4a7f4e761 100644 --- a/packages/views/locales/en/agents.json +++ b/packages/views/locales/en/agents.json @@ -493,19 +493,16 @@ "save_failed_toast": "Failed to save custom arguments" }, "mcp_config": { - "intro": "Manage the MCP servers available to this agent. A config managed by Multica is authoritative — the local runtime's own servers are not exposed unless this agent opts back in.", + "intro": "Manage the MCP servers available to this agent. Servers managed by Multica are merged with servers inherited from the local runtime.", "managed_title": "Managed by Multica", "managed_hint": "Saved in Multica for this agent. These servers are merged with runtime servers; a Multica server wins when names match.", - "managed_hint_authoritative": "Saved in Multica for this agent. This set is authoritative: only these servers are exposed, and the runtime's own MCP servers are not.", "managed_empty": "No MCP servers are managed by Multica yet.", "add_action": "Add MCP", "agent_disabled_badge": "Off for agent", "runtime_disabled_badge": "Off in runtime", "runtime_overridden_badge": "Overridden by Multica", - "runtime_excluded_badge": "Not exposed", "runtime_title": "Inherited from runtime", "runtime_hint": "Discovered automatically from {{runtime}} and merged at task launch. URLs, headers, commands, and environment variables never leave the machine.", - "runtime_hint_excluded": "Discovered automatically from {{runtime}}. This agent has an MCP config managed by Multica, so these servers are not exposed to it.", "refresh_action": "Refresh", "runtime_missing": "Assign a local runtime to discover inherited MCP servers.", "runtime_offline": "The local runtime is offline. Reconnect it to refresh inherited MCP servers.", @@ -555,20 +552,12 @@ "dialog_add_action": "Add Server", "dialog_update_action": "Save Changes", "delete_dialog_title": "Delete MCP Server?", - "delete_dialog_description": "{{name}} will no longer be available to this agent. The other servers managed here are unchanged, and the runtime's own MCP servers stay excluded.", + "delete_dialog_description": "{{name}} will no longer be available to this agent. Runtime servers are not affected.", "delete_action": "Delete Server", "invalid_json": "Invalid JSON: {{error}}", "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.", - "managed_empty_strict": "No MCP servers. This agent has no MCP access at all — not even the runtime's own servers.", - "clear_action": "Stop managing MCP", - "clear_dialog_title": "Stop managing MCP for this agent?", - "clear_dialog_description": "This removes the managed config and lets the agent use every MCP server on {{runtime}} again. That WIDENS the tools it can reach. To keep it locked down with no MCP access, delete the individual servers instead.", - "clear_confirm_action": "Stop managing and inherit", - "cleared_toast": "Managed MCP config removed — runtime servers are inherited again", - "daemon_upgrade_required": "This runtime's daemon still merges its own MCP servers into managed configs. Upgrade the daemon for this config to take effect as an allowlist.", - "runtime_hint_needs_upgrade": "Discovered automatically from {{runtime}}. These servers are still reachable by this agent because the daemon predates authoritative MCP configs — upgrade it to exclude them." + "redacted_hint": "Only the agent owner or a workspace admin can read this config." }, "composio_mcp": { "subtitle": "Check a toolkit to let this agent mount it as an MCP server — but only when you (its creator) are the one who triggered the run, directly or down a sub-agent chain.", diff --git a/packages/views/locales/en/chat.json b/packages/views/locales/en/chat.json index 1e014edfecb..aa82eec11a9 100644 --- a/packages/views/locales/en/chat.json +++ b/packages/views/locales/en/chat.json @@ -45,7 +45,6 @@ "runtime_recovery": "The agent restarted before it could finish. Please try again.", "manual": "This reply was cancelled.", "skill_bundle_unavailable": "The agent's skills couldn't be downloaded, so it never got started. This is usually a connection problem — please try again.", - "mcp_config_daemon_outdated": "This agent's MCP servers are restricted, but its runtime is running an older daemon that can't enforce that limit yet. Upgrade the daemon on that machine to run this agent.", "provider_network": "The connection to the model provider dropped before the reply finished. Check your network and try again.", "provider_auth_or_access": "The agent's model account isn't signed in any more. Sign in again, then retry.", "provider_quota_limit": "The agent's model account has run out of credit. Top it up or switch accounts, then retry.", diff --git a/packages/views/locales/ja/agents.json b/packages/views/locales/ja/agents.json index 552b51bdb2c..a28f0b86b0b 100644 --- a/packages/views/locales/ja/agents.json +++ b/packages/views/locales/ja/agents.json @@ -375,19 +375,16 @@ "save_failed_toast": "カスタム引数を保存できませんでした" }, "mcp_config": { - "intro": "このエージェントが使用できる MCP Server を管理します。Multica が管理する設定は排他的です。エージェントが明示的に継承を有効にしない限り、ローカル Runtime 自身の Server は公開されません。", + "intro": "このエージェントが使用できる MCP Server を管理します。Multica 管理の Server とローカル Runtime から継承した Server は統合して使用されます。", "managed_title": "Multica で管理", "managed_hint": "このエージェント用に Multica に保存されます。Runtime MCP と統合され、同名の場合は Multica の設定が優先されます。", - "managed_hint_authoritative": "このエージェント用に Multica に保存されます。この集合は排他的で、これらの Server のみが公開され、Runtime 自身の MCP Server は公開されません。", "managed_empty": "Multica で管理されている MCP Server はありません。", "add_action": "MCP を追加", "agent_disabled_badge": "エージェントで無効", "runtime_disabled_badge": "ランタイムで無効", "runtime_overridden_badge": "Multica で上書き", - "runtime_excluded_badge": "非公開", "runtime_title": "ランタイムから継承", "runtime_hint": "{{runtime}} から自動検出し、タスク開始時にローカルで統合します。URL、ヘッダー、コマンド、環境変数はマシン外に送信されません。", - "runtime_hint_excluded": "{{runtime}} から自動検出します。このエージェントには Multica が管理する MCP 設定があるため、これらの Server は公開されません。", "refresh_action": "更新", "runtime_missing": "継承 MCP サーバーを検出するには、ローカルランタイムを割り当ててください。", "runtime_offline": "ローカルランタイムはオフラインです。再接続すると更新できます。", @@ -437,20 +434,12 @@ "dialog_add_action": "Server を追加", "dialog_update_action": "変更を保存", "delete_dialog_title": "MCP Server を削除しますか？", - "delete_dialog_description": "{{name}} はこのエージェントで使用できなくなります。ここで管理している他の Server は変更されず、Runtime 自身の MCP Server も引き続き公開されません。", + "delete_dialog_description": "{{name}} はこのエージェントで利用できなくなります。Runtime Server には影響しません。", "delete_action": "Server を削除", "invalid_json": "無効な JSON: {{error}}", "save_failed_toast": "MCP config を保存できませんでした", "redacted_title": "設定済み — 現在の表示では非表示", - "redacted_hint": "この config を読み取れるのは、エージェントのオーナーまたはワークスペースの admin のみです。", - "managed_empty_strict": "MCP Server がありません。このエージェントは MCP に一切アクセスできません（Runtime 自身の Server も含む）。", - "clear_action": "MCP の管理を停止", - "clear_dialog_title": "このエージェントの MCP 管理を停止しますか？", - "clear_dialog_description": "管理設定を削除し、{{runtime}} 上のすべての MCP Server を再びこのエージェントが使用できるようになります。これは利用できるツールの範囲を**拡大**します。MCP アクセスを完全に無くしたい場合は、個別の Server を削除してください。", - "clear_confirm_action": "管理を停止して継承する", - "cleared_toast": "管理 MCP 設定を削除しました — Runtime Server を再び継承します", - "daemon_upgrade_required": "この Runtime の daemon は、管理設定に自身の MCP Server を統合します。この設定を許可リストとして有効にするには daemon をアップグレードしてください。", - "runtime_hint_needs_upgrade": "{{runtime}} から自動検出します。この daemon は排他的な MCP 設定に未対応のため、これらの Server は現在も このエージェントから到達可能です。除外するには daemon をアップグレードしてください。" + "redacted_hint": "この config を読み取れるのは、エージェントのオーナーまたはワークスペースの admin のみです。" }, "composio_mcp": { "subtitle": "ツールキットにチェックを入れると、あなた（この agent の作成者）が直接または下位 agent のチェーン経由でこの agent をトリガーしたときに限り、MCP サーバーとしてマウントされます。", diff --git a/packages/views/locales/ja/chat.json b/packages/views/locales/ja/chat.json index 56a9fa8670f..1c02524b029 100644 --- a/packages/views/locales/ja/chat.json +++ b/packages/views/locales/ja/chat.json @@ -44,7 +44,6 @@ "runtime_recovery": "完了する前にエージェントが再起動しました。もう一度お試しください。", "manual": "この返信はキャンセルされました。", "skill_bundle_unavailable": "エージェントの skill をダウンロードできず、返信を開始できませんでした。多くの場合はネットワークの問題です。もう一度お試しください。", - "mcp_config_daemon_outdated": "このエージェントの MCP Server は制限されていますが、Runtime の daemon が古く、その制限をまだ適用できません。該当マシンの daemon をアップグレードしてください。", "provider_network": "モデルサービスとの接続が切断され、返信を完了できませんでした。ネットワークを確認してから再度お試しください。", "provider_auth_or_access": "エージェントのモデルアカウントのログインが無効になりました。再度ログインしてからお試しください。", "provider_quota_limit": "エージェントのモデルアカウントの残高が不足しています。チャージするかアカウントを切り替えてからお試しください。", diff --git a/packages/views/locales/ko/agents.json b/packages/views/locales/ko/agents.json index 2ebcc8f75e1..4af7f3744d5 100644 --- a/packages/views/locales/ko/agents.json +++ b/packages/views/locales/ko/agents.json @@ -383,19 +383,16 @@ "save_failed_toast": "사용자 지정 인자를 저장하지 못했습니다" }, "mcp_config": { - "intro": "이 에이전트가 사용할 수 있는 MCP Server를 관리합니다. Multica가 관리하는 설정은 배타적입니다. 에이전트가 상속을 명시적으로 허용하지 않으면 로컬 Runtime 자체 Server는 노출되지 않습니다.", + "intro": "이 에이전트가 사용할 수 있는 MCP Server를 관리합니다. Multica에서 관리하는 Server와 로컬 Runtime에서 상속한 Server가 함께 사용됩니다.", "managed_title": "Multica에서 관리", "managed_hint": "이 에이전트용으로 Multica에 저장됩니다. Runtime MCP와 병합되며 이름이 같으면 Multica 설정이 우선합니다.", - "managed_hint_authoritative": "이 에이전트용으로 Multica에 저장됩니다. 이 집합은 배타적이며 이 Server만 노출되고 Runtime 자체 MCP Server는 노출되지 않습니다.", "managed_empty": "Multica에서 관리하는 MCP Server가 없습니다.", "add_action": "MCP 추가", "agent_disabled_badge": "에이전트에서 꺼짐", "runtime_disabled_badge": "런타임에서 꺼짐", "runtime_overridden_badge": "Multica에서 재정의", - "runtime_excluded_badge": "노출 안 됨", "runtime_title": "런타임에서 상속", "runtime_hint": "{{runtime}}에서 자동으로 탐색하고 작업 시작 시 로컬에서 병합합니다. URL, 헤더, 명령 및 환경 변수는 머신 외부로 전송되지 않습니다.", - "runtime_hint_excluded": "{{runtime}}에서 자동으로 탐색합니다. 이 에이전트에는 Multica가 관리하는 MCP 설정이 있으므로 이 Server는 노출되지 않습니다.", "refresh_action": "새로고침", "runtime_missing": "상속된 MCP 서버를 탐색하려면 로컬 런타임을 할당하세요.", "runtime_offline": "로컬 런타임이 오프라인입니다. 다시 연결하면 새로고침할 수 있습니다.", @@ -445,20 +442,12 @@ "dialog_add_action": "Server 추가", "dialog_update_action": "변경 저장", "delete_dialog_title": "MCP Server를 삭제할까요?", - "delete_dialog_description": "{{name}}은(는) 이 에이전트에서 더 이상 사용할 수 없습니다. 여기에서 관리하는 다른 Server는 변경되지 않으며 Runtime 자체 MCP Server도 계속 노출되지 않습니다.", + "delete_dialog_description": "{{name}}은 더 이상 이 에이전트에서 사용할 수 없습니다. Runtime Server에는 영향을 주지 않습니다.", "delete_action": "Server 삭제", "invalid_json": "잘못된 JSON: {{error}}", "save_failed_toast": "MCP config를 저장하지 못했습니다", "redacted_title": "설정됨 - 현재 보기에서는 숨김", - "redacted_hint": "에이전트 소유자 또는 워크스페이스 관리자만 이 config를 읽을 수 있습니다.", - "managed_empty_strict": "MCP Server가 없습니다. 이 에이전트는 Runtime 자체 Server를 포함해 MCP에 전혀 접근할 수 없습니다.", - "clear_action": "MCP 관리 중단", - "clear_dialog_title": "이 에이전트의 MCP 관리를 중단할까요?", - "clear_dialog_description": "관리 설정을 제거하고 {{runtime}}의 모든 MCP Server를 이 에이전트가 다시 사용할 수 있게 됩니다. 접근 가능한 도구 범위가 **확대**됩니다. MCP 접근을 완전히 차단하려면 개별 Server를 삭제하세요.", - "clear_confirm_action": "관리 중단하고 상속", - "cleared_toast": "관리 MCP 설정을 제거했습니다 — Runtime Server를 다시 상속합니다", - "daemon_upgrade_required": "이 Runtime의 daemon은 여전히 자체 MCP Server를 관리 설정에 병합합니다. 이 설정이 허용 목록으로 적용되려면 daemon을 업그레이드하세요.", - "runtime_hint_needs_upgrade": "{{runtime}}에서 자동으로 탐색합니다. 이 daemon은 배타적 MCP 설정을 지원하지 않아 이 Server들은 현재도 이 에이전트에서 접근 가능합니다. 제외하려면 daemon을 업그레이드하세요." + "redacted_hint": "에이전트 소유자 또는 워크스페이스 관리자만 이 config를 읽을 수 있습니다." }, "composio_mcp": { "subtitle": "툴킷을 선택하면, 본인(이 에이전트의 생성자)이 직접 또는 하위 에이전트 체인을 통해 이 에이전트를 트리거할 때만 MCP 서버로 마운트됩니다.", diff --git a/packages/views/locales/ko/chat.json b/packages/views/locales/ko/chat.json index 0943696f0ce..d3984fe58f4 100644 --- a/packages/views/locales/ko/chat.json +++ b/packages/views/locales/ko/chat.json @@ -44,7 +44,6 @@ "runtime_recovery": "완료 전에 에이전트가 다시 시작되었습니다. 다시 시도해 주세요.", "manual": "이 답변이 취소되었습니다.", "skill_bundle_unavailable": "에이전트의 skill을 내려받지 못해 답변을 시작하지 못했습니다. 대개 네트워크 연결 문제입니다. 다시 시도해 주세요.", - "mcp_config_daemon_outdated": "이 에이전트의 MCP Server는 제한되어 있지만, Runtime의 daemon 버전이 오래되어 아직 해당 제한을 적용할 수 없습니다. 해당 머신의 daemon을 업그레이드해 주세요.", "provider_network": "모델 서비스와의 연결이 끊겨 답변을 완료하지 못했습니다. 네트워크를 확인한 뒤 다시 시도해 주세요.", "provider_auth_or_access": "에이전트의 모델 계정 로그인이 만료되었습니다. 다시 로그인한 뒤 시도해 주세요.", "provider_quota_limit": "에이전트의 모델 계정 잔액이 부족합니다. 충전하거나 계정을 변경한 뒤 시도해 주세요.", diff --git a/packages/views/locales/zh-Hans/agents.json b/packages/views/locales/zh-Hans/agents.json index a31f7e1859b..db626683047 100644 --- a/packages/views/locales/zh-Hans/agents.json +++ b/packages/views/locales/zh-Hans/agents.json @@ -482,19 +482,16 @@ "save_failed_toast": "保存自定义参数失败" }, "mcp_config": { - "intro": "管理该智能体可用的 MCP Server。由 Multica 管理的配置具有排他性——除非该智能体显式开启继承，本地 Runtime 自带的 Server 不会暴露给它。", + "intro": "管理该智能体可用的 MCP Server。由 Multica 管理的 Server 会与本地 Runtime 继承的 Server 合并使用。", "managed_title": "由 Multica 管理", "managed_hint": "保存在 Multica 中并仅配置给该智能体。执行任务时会与 Runtime MCP 合并；名称相同时以 Multica 配置为准。", - "managed_hint_authoritative": "保存在 Multica 中并仅配置给该智能体。该集合具有排他性：只有这些 Server 会暴露给智能体，Runtime 自带的 MCP Server 不会。", "managed_empty": "还没有由 Multica 管理的 MCP Server。", "add_action": "添加 MCP", "agent_disabled_badge": "已为智能体关闭", "runtime_disabled_badge": "运行时中已关闭", "runtime_overridden_badge": "已由 Multica 覆盖", - "runtime_excluded_badge": "未暴露", "runtime_title": "从运行时继承", "runtime_hint": "自动从 {{runtime}} 发现，并在任务启动时于本机完成合并。URL、Header、命令和环境变量等敏感值不会离开本机。", - "runtime_hint_excluded": "自动从 {{runtime}} 发现。该智能体已有由 Multica 管理的 MCP 配置，因此这些 Server 不会暴露给它。", "refresh_action": "刷新", "runtime_missing": "请先分配一个本地运行时，以发现继承的 MCP Server。", "runtime_offline": "本地运行时处于离线状态，重新连接后即可刷新继承的 MCP Server。", @@ -544,20 +541,12 @@ "dialog_add_action": "添加 Server", "dialog_update_action": "保存修改", "delete_dialog_title": "删除 MCP Server？", - "delete_dialog_description": "{{name}} 将不再对该智能体可用。这里管理的其他 Server 不受影响，Runtime 自带的 MCP Server 仍然不会暴露。", + "delete_dialog_description": "{{name}} 将不再提供给该智能体；Runtime 中的 Server 不受影响。", "delete_action": "删除 Server", "invalid_json": "JSON 无效：{{error}}", "save_failed_toast": "保存 MCP 配置失败", "redacted_title": "已配置 —— 当前账号无权查看", - "redacted_hint": "只有智能体所有者或工作区管理员可以读取该配置。", - "managed_empty_strict": "没有 MCP Server。该智能体完全没有 MCP 访问权限——连 Runtime 自带的 Server 也没有。", - "clear_action": "停止管理 MCP", - "clear_dialog_title": "停止为该智能体管理 MCP？", - "clear_dialog_description": "这会移除受管配置，让该智能体重新可以使用 {{runtime}} 上的全部 MCP Server，也就是**扩大**它能访问的工具范围。如果你是想让它完全没有 MCP 访问权限，请改为逐个删除 Server。", - "clear_confirm_action": "停止管理并恢复继承", - "cleared_toast": "已移除受管 MCP 配置——重新继承 Runtime Server", - "daemon_upgrade_required": "该 Runtime 的 daemon 仍会把自身的 MCP Server 合并进受管配置。请升级 daemon，该配置才会作为 allowlist 真正生效。", - "runtime_hint_needs_upgrade": "自动从 {{runtime}} 发现。由于该 daemon 版本尚不支持排他性 MCP 配置，这些 Server 目前**仍然**能被该智能体访问——升级 daemon 后才会被排除。" + "redacted_hint": "只有智能体所有者或工作区管理员可以读取该配置。" }, "composio_mcp": { "subtitle": "勾选 toolkit，只在你自己（这个 agent 的创建者）直接或通过下级 agent 链路触发这个 agent 时，才把它挂载为 MCP server。", diff --git a/packages/views/locales/zh-Hans/chat.json b/packages/views/locales/zh-Hans/chat.json index e7cb159028e..e5f6d592c2d 100644 --- a/packages/views/locales/zh-Hans/chat.json +++ b/packages/views/locales/zh-Hans/chat.json @@ -44,7 +44,6 @@ "runtime_recovery": "智能体在完成前重启了。请重试。", "manual": "这次回复已取消。", "skill_bundle_unavailable": "没能下载到智能体的 skill，这次回复没有启动。通常是网络连接问题，请重试。", - "mcp_config_daemon_outdated": "该智能体的 MCP Server 已被限定，但其 Runtime 上的 daemon 版本过旧，还无法执行这一限制。请升级该机器上的 daemon 后再运行。", "provider_network": "与模型服务的连接中断，回复没能完成。检查网络后重试。", "provider_auth_or_access": "智能体的模型账号登录已失效。重新登录后再试。", "provider_quota_limit": "智能体的模型账号额度已用尽。充值或更换账号后再试。", diff --git a/server/cmd/multica/cmd_agent.go b/server/cmd/multica/cmd_agent.go index 3d4b76864f1..c3e7095ca74 100644 --- a/server/cmd/multica/cmd_agent.go +++ b/server/cmd/multica/cmd_agent.go @@ -169,7 +169,7 @@ func init() { agentCreateCmd.Flags().String("custom-env", "", "Custom environment variables as JSON object, e.g. '{\"KEY\":\"value\"}'. Treated as secret material — never logged by the CLI, but values passed on the command line are visible to shell history and 'ps'; prefer --custom-env-stdin or --custom-env-file for real secrets. Pass '{}' to set an empty map.") agentCreateCmd.Flags().Bool("custom-env-stdin", false, "Read the --custom-env JSON object from stdin. Keeps secrets out of shell history and 'ps'. Mutually exclusive with --custom-env and --custom-env-file.") agentCreateCmd.Flags().String("custom-env-file", "", "Read the --custom-env JSON object from a file path (suggested mode: 0600). Mutually exclusive with --custom-env and --custom-env-stdin.") - agentCreateCmd.Flags().String("mcp-config", "", "MCP server configuration as a JSON object, e.g. '{\"mcpServers\":{\"shortcut\":{...}}}'. Authoritative: the agent sees exactly these servers and none of the runtime host's own, and '{\"mcpServers\":{}}' means no MCP at all. Omit the flag to inherit the host's servers instead, or set runtime_config.mcp.inherit_runtime=true to have both. Treated as secret material (MCP entries often carry API tokens) — never logged by the CLI, but values passed on the command line are visible to shell history and 'ps'; prefer --mcp-config-stdin or --mcp-config-file for real secrets.") + agentCreateCmd.Flags().String("mcp-config", "", "MCP server configuration as a JSON object, e.g. '{\"mcpServers\":{\"shortcut\":{...}}}'. Treated as secret material (MCP entries often carry API tokens) — never logged by the CLI, but values passed on the command line are visible to shell history and 'ps'; prefer --mcp-config-stdin or --mcp-config-file for real secrets.") agentCreateCmd.Flags().Bool("mcp-config-stdin", false, "Read the --mcp-config JSON object from stdin. Keeps secrets out of shell history and 'ps'. Mutually exclusive with --mcp-config and --mcp-config-file.") agentCreateCmd.Flags().String("mcp-config-file", "", "Read the --mcp-config JSON object from a file path (suggested mode: 0600). Mutually exclusive with --mcp-config and --mcp-config-stdin.") agentCreateCmd.Flags().String("visibility", "private", "Visibility: private or workspace (legacy; mapped to --permission-mode. private->private, workspace->public_to+workspace target)") @@ -198,7 +198,7 @@ func init() { // through the generic UpdateAgent endpoint (there is no dedicated // audited endpoint for it). The same three secret-safe input channels // as `agent create` are offered. Pass `--mcp-config null` to clear. - agentUpdateCmd.Flags().String("mcp-config", "", "New MCP server configuration as a JSON object, e.g. '{\"mcpServers\":{...}}'. Authoritative: the agent sees exactly these servers and none of the runtime host's own, and '{\"mcpServers\":{}}' means no MCP at all. Pass 'null' to clear and inherit the host's servers instead, or set runtime_config.mcp.inherit_runtime=true to have both. Treated as secret material — never logged by the CLI, but values passed on the command line are visible to shell history and 'ps'; prefer --mcp-config-stdin or --mcp-config-file for real secrets.") + agentUpdateCmd.Flags().String("mcp-config", "", "New MCP server configuration as a JSON object, e.g. '{\"mcpServers\":{...}}'. Pass 'null' to clear. Treated as secret material — never logged by the CLI, but values passed on the command line are visible to shell history and 'ps'; prefer --mcp-config-stdin or --mcp-config-file for real secrets.") agentUpdateCmd.Flags().Bool("mcp-config-stdin", false, "Read the --mcp-config JSON from stdin. Keeps secrets out of shell history and 'ps'. Mutually exclusive with --mcp-config and --mcp-config-file.") agentUpdateCmd.Flags().String("mcp-config-file", "", "Read the --mcp-config JSON from a file path (suggested mode: 0600). Mutually exclusive with --mcp-config and --mcp-config-stdin.") agentUpdateCmd.Flags().String("visibility", "", "New visibility: private or workspace (legacy; mapped to --permission-mode)") diff --git a/server/internal/daemon/client.go b/server/internal/daemon/client.go index 1fe28bcb7b9..4ac3fb1d64d 100644 --- a/server/internal/daemon/client.go +++ b/server/internal/daemon/client.go @@ -188,9 +188,6 @@ func daemonClientCapabilities() string { protocol.DaemonCapabilitySkillBundlesV1, protocol.DaemonCapabilityCoalescedCommentsV1, protocol.DaemonCapabilityRPCV1, - // Tells the server this daemon honours an authoritative mcp_config, so - // the claim path may hand it strictly-scoped MCP tasks (GitHub #6283). - protocol.DaemonCapabilityAuthoritativeMcpV1, }, ",") } diff --git a/server/internal/daemon/daemon.go b/server/internal/daemon/daemon.go index 90976947260..60cb240062d 100644 --- a/server/internal/daemon/daemon.go +++ b/server/internal/daemon/daemon.go @@ -3069,11 +3069,6 @@ func (d *Daemon) handleLocalSkillList(ctx context.Context, rt Runtime, requestID "supported": supported, "mcp_servers": mcpServers, "mcp_supported": mcpSupported, - // Additive: tells the server (and through it the agent MCP tab) that - // this daemon enforces a managed mcp_config as an authoritative - // allowlist. A daemon without this field still merges the host's MCP - // servers, so the UI must not claim they are excluded (GitHub #6283). - "authoritative_mcp": true, }) } @@ -4942,18 +4937,15 @@ func (d *Daemon) runTask(ctx context.Context, task Task, provider string, slot i var cursorMcpAuthSource string if task.Agent != nil { agentMcpConfig = task.Agent.McpConfig - // A managed mcp_config is an authoritative allowlist: it must NOT be - // silently widened with the host's own MCP servers (GitHub #6283). - // resolveEffectiveMcpConfig owns that decision, including the two - // explicit inherit paths (overlay-only tasks and the per-agent - // runtime_config.mcp.inherit_runtime opt-in). - effectiveMcpConfig = resolveEffectiveMcpConfig( - provider, - agentMcpConfig, - task.Agent.McpConfigOverlayOnly, - task.Agent.RuntimeConfig, - taskLog, - ) + effectiveMcpConfig = agentMcpConfig + if merged, mergeErr := mergeRuntimeAndAgentMcpConfig(provider, agentMcpConfig); mergeErr != nil { + taskLog.Warn("mcp_config: runtime merge failed; using agent configuration only", + "provider", provider, + "error", mergeErr, + ) + } else { + effectiveMcpConfig = merged + } if provider == "cursor" { cursorMcpAuthSource = strings.TrimSpace(task.Agent.CustomEnv[execenv.CursorMcpAuthSourceEnv]) } diff --git a/server/internal/daemon/mcp_capability_test.go b/server/internal/daemon/mcp_capability_test.go deleted file mode 100644 index ecc2c7277ee..00000000000 --- a/server/internal/daemon/mcp_capability_test.go +++ /dev/null @@ -1,22 +0,0 @@ -package daemon - -import ( - "strings" - "testing" - - "github.com/multica-ai/multica/server/pkg/protocol" -) - -// The server refuses to hand a strictly-scoped MCP task to a daemon that does -// not advertise this capability, so a daemon that enforces the semantics but -// forgets to advertise them would silently stop running those tasks -// (GitHub #6283). -func TestDaemonAdvertisesAuthoritativeMcpCapability(t *testing.T) { - caps := strings.Split(daemonClientCapabilities(), ",") - for _, c := range caps { - if c == protocol.DaemonCapabilityAuthoritativeMcpV1 { - return - } - } - t.Fatalf("daemon must advertise %q; got %v", protocol.DaemonCapabilityAuthoritativeMcpV1, caps) -} diff --git a/server/internal/daemon/mcp_inherit.go b/server/internal/daemon/mcp_inherit.go deleted file mode 100644 index 138888cb0d1..00000000000 --- a/server/internal/daemon/mcp_inherit.go +++ /dev/null @@ -1,105 +0,0 @@ -package daemon - -import ( - "bytes" - "encoding/json" - "log/slog" -) - -// hasManagedJSONPayload reports whether a raw JSON field carries an actual -// payload rather than being absent or the literal `null`. Mirrors the -// "absent" convention mergeRuntimeAndAgentMcpConfig and -// agent.hasManagedMcpConfig already use. -func hasManagedJSONPayload(raw json.RawMessage) bool { - trimmed := bytes.TrimSpace(raw) - return len(trimmed) > 0 && !bytes.Equal(trimmed, []byte("null")) -} - -// mcpRuntimeConfig is the agent-level knob that re-enables inheriting the -// host's own MCP servers on top of a managed `mcp_config`. -// -// It lives under `runtime_config.mcp` rather than inside `mcp_config` itself so -// the MCP document the daemon hands to a provider stays a pure MCP config — -// an extra sibling key there would leak into the generated Claude/Codex config -// file. `runtime_config` is already the established home for per-agent knobs -// the daemon decodes itself (see decodeOpenclawRuntimeConfig). -type mcpRuntimeConfig struct { - Mcp struct { - // InheritRuntime opts back in to the additive behavior #5277 - // shipped: the host's user-level MCP servers are merged underneath - // the agent's managed servers. Default false — a managed config is - // an authoritative allowlist (see resolveEffectiveMcpConfig). - InheritRuntime bool `json:"inherit_runtime"` - } `json:"mcp"` -} - -// decodeMcpInheritRuntime reports whether the agent explicitly opted into -// inheriting the runtime's own MCP servers. -// -// Fails CLOSED: absent, empty, or malformed `runtime_config` yields false, so a -// broken JSON blob can never silently widen the agent's tool surface. -func decodeMcpInheritRuntime(raw json.RawMessage, logger *slog.Logger) bool { - if !hasManagedJSONPayload(raw) { - return false - } - var cfg mcpRuntimeConfig - if err := json.Unmarshal(raw, &cfg); err != nil { - if logger != nil { - logger.Warn("mcp_config: runtime_config parse failed; not inheriting runtime MCP servers", - "error", err, - ) - } - return false - } - return cfg.Mcp.InheritRuntime -} - -// resolveEffectiveMcpConfig decides which MCP servers a task may actually see. -// -// This is the access-control decision for MCP tools, so the three states of -// `mcp_config` must stay distinguishable (GitHub #6283): -// -// - null / unset → inherit the provider's native MCP configuration. -// Returned as-is so the backend omits --mcp-config -// and Claude reads the user's own servers. -// - `{"mcpServers":{}}` → strict empty. No host servers, no tools. -// - non-empty object → strict allowlist. Exactly those servers. -// -// Before this, ANY non-null config was merged with the host's user-level MCP -// servers, so an explicitly empty config produced the *complete host set* — -// the opposite of what the operator configured. -// -// Two escape hatches keep the previous behavior reachable without weakening -// the default: -// -// - overlayOnly: the config the server sent is purely a per-task integration -// overlay (Composio) and the agent itself has no saved mcp_config. That -// agent was inheriting host MCP before the overlay existed, so enabling an -// integration must not silently strip its tools. -// - runtime_config.mcp.inherit_runtime: an explicit, per-agent opt-in to the -// additive behavior. -func resolveEffectiveMcpConfig( - provider string, - agentMcpConfig json.RawMessage, - overlayOnly bool, - runtimeConfig json.RawMessage, - logger *slog.Logger, -) json.RawMessage { - // A managed config is authoritative unless the agent (or the overlay-only - // carve-out) explicitly asks to inherit the host's servers. - if !overlayOnly && !decodeMcpInheritRuntime(runtimeConfig, logger) { - return agentMcpConfig - } - merged, err := mergeRuntimeAndAgentMcpConfig(provider, agentMcpConfig) - if err != nil { - if logger != nil { - logger.Warn("mcp_config: runtime merge failed; using agent configuration only", - "provider", provider, - "error", err, - ) - } - // Fail closed: the agent's own config only. - return agentMcpConfig - } - return merged -} diff --git a/server/internal/daemon/mcp_inherit_test.go b/server/internal/daemon/mcp_inherit_test.go deleted file mode 100644 index bb8613e7851..00000000000 --- a/server/internal/daemon/mcp_inherit_test.go +++ /dev/null @@ -1,228 +0,0 @@ -package daemon - -import ( - "encoding/json" - "os" - "path/filepath" - "sort" - "testing" -) - -// writeHostClaudeMcp seeds the host's user-level Claude MCP configuration — -// the servers an agent must NOT reach through an explicit mcp_config. -func writeHostClaudeMcp(t *testing.T, names ...string) { - t.Helper() - home := t.TempDir() - t.Setenv("HOME", home) - - servers := map[string]any{} - for _, name := range names { - servers[name] = map[string]any{"command": name + "-mcp"} - } - raw, err := json.Marshal(map[string]any{"mcpServers": servers}) - if err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(home, ".claude.json"), raw, 0o600); err != nil { - t.Fatal(err) - } -} - -// serverNames lists the mcpServers keys of an effective config. -func serverNames(t *testing.T, raw json.RawMessage) []string { - t.Helper() - if len(raw) == 0 { - return nil - } - var doc struct { - McpServers map[string]json.RawMessage `json:"mcpServers"` - } - if err := json.Unmarshal(raw, &doc); err != nil { - t.Fatalf("unmarshal effective config %q: %v", string(raw), err) - } - out := make([]string, 0, len(doc.McpServers)) - for name := range doc.McpServers { - out = append(out, name) - } - sort.Strings(out) - return out -} - -func equalNames(got, want []string) bool { - if len(got) != len(want) { - return false - } - for i := range got { - if got[i] != want[i] { - return false - } - } - return true -} - -// TestResolveEffectiveMcpConfigExplicitEmptyIsStrict is the direct regression -// test for GitHub #6283: an operator who saves `{"mcpServers":{}}` expects the -// agent to reach NO MCP server. Before the fix this resolved to the complete -// host set. -func TestResolveEffectiveMcpConfigExplicitEmptyIsStrict(t *testing.T) { - writeHostClaudeMcp(t, "host-prod-db", "host-secrets") - - got := resolveEffectiveMcpConfig( - "claude", - json.RawMessage(`{"mcpServers":{}}`), - false, - nil, - quietLogger(), - ) - - if names := serverNames(t, got); len(names) != 0 { - t.Fatalf("explicit empty mcp_config must expose no MCP servers, got %v (config %q)", names, string(got)) - } - // Must stay non-nil so the provider backend still passes - // --mcp-config/--strict-mcp-config instead of falling back to native - // inheritance. - if !hasManagedJSONPayload(got) { - t.Fatalf("explicit empty mcp_config must remain a managed config, got %q", string(got)) - } -} - -// TestResolveEffectiveMcpConfigNonEmptyIsAllowlist — a saved allowlist must not -// be widened with same-named or unrelated host servers. -func TestResolveEffectiveMcpConfigNonEmptyIsAllowlist(t *testing.T) { - writeHostClaudeMcp(t, "host-prod-db", "shared") - - got := resolveEffectiveMcpConfig( - "claude", - json.RawMessage(`{"mcpServers":{"shared":{"command":"agent-shared"},"agent-only":{"url":"https://agent.example/mcp"}}}`), - false, - nil, - quietLogger(), - ) - - want := []string{"agent-only", "shared"} - if names := serverNames(t, got); !equalNames(names, want) { - t.Fatalf("managed allowlist = %v, want %v (config %q)", names, want, string(got)) - } - // The agent's own entry must win for a colliding name, not the host's. - var doc struct { - McpServers map[string]map[string]any `json:"mcpServers"` - } - if err := json.Unmarshal(got, &doc); err != nil { - t.Fatal(err) - } - if cmd := doc.McpServers["shared"]["command"]; cmd != "agent-shared" { - t.Fatalf("shared.command = %#v, want agent-shared", cmd) - } -} - -// TestResolveEffectiveMcpConfigNullInheritsNatively — null must keep the -// provider's own inheritance path, which the backends detect by a nil config. -func TestResolveEffectiveMcpConfigNullInheritsNatively(t *testing.T) { - writeHostClaudeMcp(t, "host-prod-db") - - for _, raw := range []json.RawMessage{nil, json.RawMessage("null"), json.RawMessage(" null ")} { - got := resolveEffectiveMcpConfig("claude", raw, false, nil, quietLogger()) - if string(got) != string(raw) { - t.Fatalf("null mcp_config %q resolved to %q; want unchanged", string(raw), string(got)) - } - if hasManagedJSONPayload(got) { - t.Fatalf("null mcp_config must not become a managed config, got %q", string(got)) - } - } -} - -// TestResolveEffectiveMcpConfigOverlayOnlyKeepsRuntimeServers — enabling a -// per-task integration (Composio) on an agent that never configured MCP must -// not strip the host servers it was already inheriting. -func TestResolveEffectiveMcpConfigOverlayOnlyKeepsRuntimeServers(t *testing.T) { - writeHostClaudeMcp(t, "host-prod-db") - - got := resolveEffectiveMcpConfig( - "claude", - json.RawMessage(`{"mcpServers":{"composio":{"url":"https://composio.example/mcp"}}}`), - true, - nil, - quietLogger(), - ) - - want := []string{"composio", "host-prod-db"} - if names := serverNames(t, got); !equalNames(names, want) { - t.Fatalf("overlay-only config = %v, want %v (config %q)", names, want, string(got)) - } -} - -// TestResolveEffectiveMcpConfigInheritOptInMerges — the explicit per-agent -// opt-in restores the additive behavior #5277 shipped. -func TestResolveEffectiveMcpConfigInheritOptInMerges(t *testing.T) { - writeHostClaudeMcp(t, "host-prod-db") - - got := resolveEffectiveMcpConfig( - "claude", - json.RawMessage(`{"mcpServers":{"agent-only":{"command":"agent"}}}`), - false, - json.RawMessage(`{"mcp":{"inherit_runtime":true}}`), - quietLogger(), - ) - - want := []string{"agent-only", "host-prod-db"} - if names := serverNames(t, got); !equalNames(names, want) { - t.Fatalf("inherit_runtime opt-in = %v, want %v (config %q)", names, want, string(got)) - } -} - -// TestResolveEffectiveMcpConfigInheritOptInStillStrictWhenFalse — an explicit -// false, and an unrelated runtime_config, must both stay strict. -func TestResolveEffectiveMcpConfigInheritOptInStillStrictWhenFalse(t *testing.T) { - writeHostClaudeMcp(t, "host-prod-db") - - for _, rc := range []json.RawMessage{ - nil, - json.RawMessage("null"), - json.RawMessage(`{}`), - json.RawMessage(`{"mcp":{}}`), - json.RawMessage(`{"mcp":{"inherit_runtime":false}}`), - json.RawMessage(`{"mode":"gateway"}`), - } { - got := resolveEffectiveMcpConfig( - "claude", - json.RawMessage(`{"mcpServers":{}}`), - false, - rc, - quietLogger(), - ) - if names := serverNames(t, got); len(names) != 0 { - t.Fatalf("runtime_config %q must stay strict, got %v", string(rc), names) - } - } -} - -// TestDecodeMcpInheritRuntimeFailsClosedOnMalformed — a broken runtime_config -// must never widen the tool surface. -func TestDecodeMcpInheritRuntimeFailsClosedOnMalformed(t *testing.T) { - for _, rc := range []json.RawMessage{ - json.RawMessage(`{"mcp":{"inherit_runtime":true}`), // truncated - json.RawMessage(`{"mcp":"yes"}`), // wrong type - json.RawMessage(`not json`), - } { - if decodeMcpInheritRuntime(rc, quietLogger()) { - t.Fatalf("malformed runtime_config %q must not enable inheritance", string(rc)) - } - } -} - -// TestResolveEffectiveMcpConfigUnsupportedProviderStaysStrict — providers with -// no runtime MCP discovery must still honour the managed set verbatim. -func TestResolveEffectiveMcpConfigUnsupportedProviderStaysStrict(t *testing.T) { - writeHostClaudeMcp(t, "host-prod-db") - - got := resolveEffectiveMcpConfig( - "qwen", - json.RawMessage(`{"mcpServers":{}}`), - true, // even on the inherit path there is nothing to inherit - json.RawMessage(`{"mcp":{"inherit_runtime":true}}`), - quietLogger(), - ) - if names := serverNames(t, got); len(names) != 0 { - t.Fatalf("provider without runtime MCP discovery = %v, want none", names) - } -} diff --git a/server/internal/daemon/poisoned.go b/server/internal/daemon/poisoned.go index 1842cb96150..0dd0b36492f 100644 --- a/server/internal/daemon/poisoned.go +++ b/server/internal/daemon/poisoned.go @@ -30,7 +30,7 @@ import ( // to the canonical taskfailure values so the daemon and the in-flight // classifier (used by every other failure path) share a single source // of truth. agent_fallback_message and codex_semantic_inactivity are -// pre-existing operational reasons not in the canonical 23 — kept as +// pre-existing operational reasons not in the canonical 21 — kept as // string literals here until a follow-up PR migrates them or extends // the taxonomy. const ( diff --git a/server/internal/daemon/runtime_mcp.go b/server/internal/daemon/runtime_mcp.go index 1208114e4ff..9e48f2362c2 100644 --- a/server/internal/daemon/runtime_mcp.go +++ b/server/internal/daemon/runtime_mcp.go @@ -23,18 +23,15 @@ type runtimeLocalMcpServerSummary struct { } // mergeRuntimeAndAgentMcpConfig builds the task-local MCP configuration used -// when an agent inherits the runtime's own MCP servers on top of its managed -// ones. Runtime servers are the base layer and the agent's entries win on a -// same-name collision. The merge happens inside the local daemon so runtime -// URLs, headers, commands, and env values never need to leave the machine. +// when an agent has MCP servers managed by Multica. Runtime servers are the +// base layer and the agent's entries win on a same-name collision. The merge +// happens inside the local daemon so runtime URLs, headers, commands, and env +// values never need to leave the machine. // // A nil/null agent config keeps the provider's native inheritance path intact. -// -// This function does NOT decide whether inheriting is allowed — callers must go -// through resolveEffectiveMcpConfig, which defaults to treating a managed -// config as an authoritative allowlist. Calling this unconditionally is what -// made an explicitly empty `{"mcpServers":{}}` resolve to the complete host set -// (GitHub #6283). +// A present config (including an empty mcpServers map) opts into the merged, +// task-local config so adding one managed server no longer disables unrelated +// runtime servers. func mergeRuntimeAndAgentMcpConfig(provider string, agentConfig json.RawMessage) (json.RawMessage, error) { trimmed := bytes.TrimSpace(agentConfig) if len(trimmed) == 0 || bytes.Equal(trimmed, []byte("null")) { diff --git a/server/internal/daemon/types.go b/server/internal/daemon/types.go index e41b9f57f28..cea701b7fb9 100644 --- a/server/internal/daemon/types.go +++ b/server/internal/daemon/types.go @@ -185,17 +185,6 @@ type AgentData struct { // daemon decodes provider-specific fields (e.g. openclaw mode + // gateway endpoint, see issue #3260); other backends ignore it. RuntimeConfig json.RawMessage `json:"runtime_config,omitempty"` - // McpConfigOverlayOnly is set by the server when McpConfig carries ONLY - // a per-task integration overlay (the initiator's Composio servers) and - // the agent itself has no saved mcp_config. Such an agent was inheriting - // the runtime's native MCP servers before the overlay existed, so the - // daemon keeps inheriting them instead of treating the overlay as an - // authoritative allowlist (GitHub #6283). - // - // Absent on pre-#6283 servers, which pre-merge the overlay into - // mcp_config the same way. A false value there is indistinguishable from - // "the agent authored this config", which is the fail-closed direction. - McpConfigOverlayOnly bool `json:"mcp_config_overlay_only,omitempty"` } // DisabledRuntimeSkillData is the task-wire identity of one runtime-local diff --git a/server/internal/handler/agent.go b/server/internal/handler/agent.go index fe5dc222ac6..c191c27af1d 100644 --- a/server/internal/handler/agent.go +++ b/server/internal/handler/agent.go @@ -596,15 +596,6 @@ type TaskAgentData struct { // (issue #3260). Other providers ignore the payload entirely. Sent // raw so the daemon can evolve its schema without a server roundtrip. RuntimeConfig json.RawMessage `json:"runtime_config,omitempty"` - // McpConfigOverlayOnly tells the daemon that McpConfig carries ONLY the - // per-task integration overlay (currently Composio) because the agent - // itself has no saved mcp_config. The daemon needs this to keep MCP - // access control fail-closed without regressing those agents: a managed - // mcp_config is an authoritative allowlist (GitHub #6283), but an agent - // that never configured one was already inheriting the runtime's own MCP - // servers and must keep doing so rather than being narrowed to the - // overlay by the mere act of enabling an integration. - McpConfigOverlayOnly bool `json:"mcp_config_overlay_only,omitempty"` } // taskToResponse maps a queue row to its wire shape. workspaceID is threaded diff --git a/server/internal/handler/daemon.go b/server/internal/handler/daemon.go index 7d52ee95ef2..fbc4350acd8 100644 --- a/server/internal/handler/daemon.go +++ b/server/internal/handler/daemon.go @@ -31,7 +31,6 @@ import ( db "github.com/multica-ai/multica/server/pkg/db/generated" "github.com/multica-ai/multica/server/pkg/protocol" "github.com/multica-ai/multica/server/pkg/redact" - "github.com/multica-ai/multica/server/pkg/taskfailure" ) // --------------------------------------------------------------------------- @@ -1664,59 +1663,18 @@ func (h *Handler) buildClaimedTaskResponse(r *http.Request, task *db.AgentTaskQu if agent.McpConfig != nil { mcpConfig = json.RawMessage(agent.McpConfig) } - // Fail closed against a daemon that predates the authoritative - // mcp_config semantics: it would merge the runtime host's own MCP - // servers underneath this agent's managed set, handing the agent tools - // the operator explicitly scoped out (GitHub #6283). Running the task - // anyway is the vulnerability, and the UI cannot honestly report the - // boundary either. - // - // FailTask rather than CancelTask, because the default claim path is the - // machine-level BATCH endpoint, which skips build failures and still - // answers 200 {"tasks":[]} — a bare cancel would show the operator a - // task that vanished for no stated reason. The classified failure - // reason + message are stored on the task, so the upgrade requirement - // reaches the user on every claim path and on any daemon version. The - // reason is not auto-retryable: the same outdated daemon would claim the - // retry and fail it again. - if mcpConfigNeedsAuthoritativeDaemon(runtime.Provider, mcpConfig, agent.RuntimeConfig) && - !requestHasClientCapability(r, protocol.DaemonCapabilityAuthoritativeMcpV1) { - slog.Error("task claim: daemon predates authoritative mcp_config; failing task instead of widening the agent's MCP tools", - "task_id", uuidToString(task.ID), - "agent_id", uuidToString(agent.ID), - "runtime_id", runtimeID, - "provider", runtime.Provider, - "required_capability", protocol.DaemonCapabilityAuthoritativeMcpV1, - ) - if _, ferr := h.TaskService.FailTask(r.Context(), task.ID, - mcpConfigDaemonOutdatedMessage, "", "", - string(taskfailure.ReasonMcpConfigDaemonOutdated), false, ""); ferr != nil { - slog.Error("task claim: fail after authoritative mcp_config check failed", - "task_id", uuidToString(task.ID), "error", ferr) - } - return resp, deliveredCommentIDs, agentSkillCount, builtinSkillCount, &claimBuildFailure{ - outcome: "error_mcp_config_daemon_outdated", - status: http.StatusPreconditionFailed, - message: mcpConfigDaemonOutdatedMessage, - } - } // Layer the per-task overlay (set at enqueue from the initiator // user's active integrations — currently Composio) on top of the // agent's saved mcp_config. Overlay wins on server-name collisions // because it carries the live user-scoped session URL. Errors are // logged but never fail the claim: a broken overlay must not prevent // the agent from running with its base config. - // - // mcpConfigOverlayOnly tells the daemon the payload is purely the - // overlay so it keeps inheriting the runtime's MCP servers for an - // agent that never configured any (GitHub #6283). - var mcpConfigOverlayOnly bool if composioMCPEnabled && len(task.RuntimeMcpOverlay) > 0 { - resolved, overlayOnly, err := resolveClaimMcpConfig(mcpConfig, json.RawMessage(task.RuntimeMcpOverlay)) - if err != nil { + if merged, err := mergeMCPOverlay(mcpConfig, json.RawMessage(task.RuntimeMcpOverlay)); err != nil { slog.Warn("daemon claim: merge runtime_mcp_overlay failed; falling back to agent mcp_config", "task_id", uuidToString(task.ID), "error", err) + } else { + mcpConfig = merged } - mcpConfig, mcpConfigOverlayOnly = resolved, overlayOnly } // runtime_config is stored as JSONB and may legitimately be the // empty object `{}` for agents that haven't opted into any @@ -1733,7 +1691,6 @@ func (h *Handler) buildClaimedTaskResponse(r *http.Request, task *db.AgentTaskQu CustomEnv: customEnv, CustomArgs: customArgs, McpConfig: mcpConfig, - McpConfigOverlayOnly: mcpConfigOverlayOnly, Model: agent.Model.String, ThinkingLevel: agent.ThinkingLevel.String, ServiceTier: agent.ServiceTier.String, diff --git a/server/internal/handler/daemon_test.go b/server/internal/handler/daemon_test.go index 6f8c1f07719..4b26ff2d254 100644 --- a/server/internal/handler/daemon_test.go +++ b/server/internal/handler/daemon_test.go @@ -115,17 +115,6 @@ func newDaemonTokenRequest(method, path string, body any, workspaceID, daemonID } req := httptest.NewRequest(method, path, &buf) req.Header.Set("Content-Type", "application/json") - // Every current daemon advertises authoritative-mcp-v1, and the claim path - // refuses a managed mcp_config without it (GitHub #6283). Default it on so - // unrelated claim tests exercise a current daemon rather than accidentally - // simulating a pre-#6283 one. Tests that specifically cover the outdated - // case build their own request without this header — see - // TestClaimTaskByRuntime_OutdatedDaemonRefusesManagedMcpConfig. - // - // Deliberately only this capability: skill-bundles / coalesced-comments / - // rpc are feature negotiations whose absence tests real legacy behaviour, - // so those stay opt-in per test. - req.Header.Set("X-Client-Capabilities", protocol.DaemonCapabilityAuthoritativeMcpV1) // No X-User-ID — daemon tokens don't set it. ctx := middleware.WithDaemonContext(req.Context(), workspaceID, daemonID) return req.WithContext(ctx) diff --git a/server/internal/handler/dashboard.go b/server/internal/handler/dashboard.go index 2d60f5f558d..98c1ade3721 100644 --- a/server/internal/handler/dashboard.go +++ b/server/internal/handler/dashboard.go @@ -510,7 +510,7 @@ func (h *Handler) GetDashboardRunTimeDaily(w http.ResponseWriter, r *http.Reques // expired in the queue never started. // // FailureReason values are the canonical taxonomy from server/pkg/taskfailure -// (23 reasons), plus "unclassified" for failed rows with a NULL / empty +// (21 reasons), plus "unclassified" for failed rows with a NULL / empty // column. The client folds them into a handful of display classes; the raw // reason stays on the wire so that mapping can change without a backend // deploy. diff --git a/server/internal/handler/mcp_claim_config_test.go b/server/internal/handler/mcp_claim_config_test.go deleted file mode 100644 index 3ec44d1a6fd..00000000000 --- a/server/internal/handler/mcp_claim_config_test.go +++ /dev/null @@ -1,285 +0,0 @@ -package handler - -import ( - "encoding/json" - "testing" -) - -// The claim response must tell the daemon whether mcp_config is the agent's own -// managed allowlist or purely the per-task integration overlay. The daemon uses -// that to keep MCP access control fail-closed without stripping runtime servers -// from agents that never configured MCP (GitHub #6283). - -func TestResolveClaimMcpConfigNoOverlayIsNeverOverlayOnly(t *testing.T) { - t.Parallel() - - for _, agentCfg := range []json.RawMessage{ - nil, - json.RawMessage("null"), - json.RawMessage(`{"mcpServers":{}}`), - json.RawMessage(`{"mcpServers":{"a":{"command":"a"}}}`), - } { - got, overlayOnly, err := resolveClaimMcpConfig(agentCfg, nil) - if err != nil { - t.Fatalf("agent %q: %v", string(agentCfg), err) - } - if overlayOnly { - t.Fatalf("agent %q with no overlay must not be flagged overlay-only", string(agentCfg)) - } - if want := string(passthroughAgentMcpConfig(agentCfg)); string(got) != want { - t.Fatalf("agent %q: config = %q, want %q", string(agentCfg), string(got), want) - } - } -} - -func TestResolveClaimMcpConfigOverlayOnlyWhenAgentHasNoConfig(t *testing.T) { - t.Parallel() - - overlay := json.RawMessage(`{"mcpServers":{"composio":{"url":"https://composio.example/mcp"}}}`) - for _, agentCfg := range []json.RawMessage{nil, json.RawMessage("null")} { - got, overlayOnly, err := resolveClaimMcpConfig(agentCfg, overlay) - if err != nil { - t.Fatalf("agent %q: %v", string(agentCfg), err) - } - if !overlayOnly { - t.Fatalf("agent %q + overlay must be flagged overlay-only, got config %q", string(agentCfg), string(got)) - } - if !hasManagedJSON(got) { - t.Fatalf("agent %q + overlay must yield a managed config, got %q", string(agentCfg), string(got)) - } - } -} - -// An explicitly empty agent config is a deliberate "no MCP servers" decision, -// so it counts as agent-authored: the daemon must treat the result as strict -// and must not fold the host's servers back in. -func TestResolveClaimMcpConfigExplicitEmptyAgentConfigIsAuthored(t *testing.T) { - t.Parallel() - - overlay := json.RawMessage(`{"mcpServers":{"composio":{"url":"https://composio.example/mcp"}}}`) - got, overlayOnly, err := resolveClaimMcpConfig(json.RawMessage(`{"mcpServers":{}}`), overlay) - if err != nil { - t.Fatal(err) - } - if overlayOnly { - t.Fatalf("explicit empty agent mcp_config must not be treated as overlay-only (config %q)", string(got)) - } - var doc struct { - McpServers map[string]json.RawMessage `json:"mcpServers"` - } - if err := json.Unmarshal(got, &doc); err != nil { - t.Fatal(err) - } - if len(doc.McpServers) != 1 || doc.McpServers["composio"] == nil { - t.Fatalf("expected only the overlay server, got %q", string(got)) - } -} - -func TestResolveClaimMcpConfigNonEmptyAgentConfigIsAuthored(t *testing.T) { - t.Parallel() - - overlay := json.RawMessage(`{"mcpServers":{"composio":{"url":"https://composio.example/mcp"}}}`) - got, overlayOnly, err := resolveClaimMcpConfig(json.RawMessage(`{"mcpServers":{"a":{"command":"a"}}}`), overlay) - if err != nil { - t.Fatal(err) - } - if overlayOnly { - t.Fatalf("agent-authored mcp_config must not be flagged overlay-only (config %q)", string(got)) - } -} - -// A malformed overlay must fall back to the agent's saved config, and must not -// claim overlay-only provenance for it. -func TestResolveClaimMcpConfigBadOverlayFallsBackNotOverlayOnly(t *testing.T) { - t.Parallel() - - agentCfg := json.RawMessage(`{"mcpServers":{"a":{"command":"a"}}}`) - got, overlayOnly, err := resolveClaimMcpConfig(agentCfg, json.RawMessage(`{"mcpServers":`)) - if err == nil { - t.Fatal("expected a parse error for the malformed overlay") - } - if overlayOnly { - t.Fatal("malformed overlay must not be flagged overlay-only") - } - if string(got) != string(agentCfg) { - t.Fatalf("config = %q, want the agent config %q unchanged", string(got), string(agentCfg)) - } -} - -// Same failure with no agent config: nothing to fall back to, and the daemon -// must end up on its native-inheritance path rather than a strict empty set. -func TestResolveClaimMcpConfigBadOverlayWithNoAgentConfigYieldsNil(t *testing.T) { - t.Parallel() - - got, overlayOnly, err := resolveClaimMcpConfig(nil, json.RawMessage(`{"mcpServers":`)) - if err == nil { - t.Fatal("expected a parse error for the malformed overlay") - } - if overlayOnly { - t.Fatal("malformed overlay must not be flagged overlay-only") - } - if hasManagedJSON(got) { - t.Fatalf("config = %q, want absent so the daemon keeps native inheritance", string(got)) - } -} - -// The strict mcp_config semantics are enforced by the DAEMON. A daemon that -// predates them still merges the host's MCP servers into a managed config, so -// the claim path must refuse rather than run the agent with tools the operator -// scoped out (GitHub #6283). - -// The provider set is a FROZEN record of what pre-capability daemons actually -// merged — not a mirror of the daemon's current provider switch. Pinning the -// exact membership makes the difference explicit: growing this set because a new -// provider gained runtime MCP discovery would start failing tasks on old daemons -// that never merged for it, re-creating the qwen false-positive. Discovery for a -// new provider can only ship in a daemon that already advertises the capability, -// which is never gated. -func TestProvidersOldDaemonsMergedRuntimeMcpIsFrozen(t *testing.T) { - t.Parallel() - - want := map[string]bool{ - "claude": true, - "codebuddy": true, - "codex": true, - "cursor": true, - "opencode": true, - "openclaw": true, - } - if len(providersOldDaemonsMergedRuntimeMcp) != len(want) { - t.Fatalf("frozen provider set has %d entries, want %d: %v", - len(providersOldDaemonsMergedRuntimeMcp), len(want), providersOldDaemonsMergedRuntimeMcp) - } - for provider := range want { - if !providersOldDaemonsMergedRuntimeMcp[provider] { - t.Errorf("provider %q merged host MCP before the capability and must stay in the set", provider) - } - } - // A provider that only gains runtime MCP discovery AFTER the capability - // shipped must not be added: no pre-capability daemon ever merged for it. - for _, provider := range []string{"qwen", "hermes"} { - if providersOldDaemonsMergedRuntimeMcp[provider] { - t.Errorf("provider %q was never merged by a pre-capability daemon; gating it would fail safe tasks", provider) - } - } -} - -// The gate must only fire for providers whose pre-#6283 daemon actually merged -// host MCP. qwen was never in that switch and already had strict semantics, so -// gating it would cancel tasks that carry no risk at all. -func TestMcpConfigNeedsAuthoritativeDaemonOnlyForMergingProviders(t *testing.T) { - t.Parallel() - - managed := json.RawMessage(`{"mcpServers":{"a":{"command":"a"}}}`) - - for _, provider := range []string{"claude", "codebuddy", "codex", "cursor", "opencode", "openclaw", "CLAUDE", " claude "} { - if !mcpConfigNeedsAuthoritativeDaemon(provider, managed, nil) { - t.Fatalf("provider %q merged host MCP before the fix and must gate", provider) - } - } - // qwen is deliberately absent from the merge switch; unknown providers fail - // to the same non-gating side because we cannot point at old behaviour. - for _, provider := range []string{"qwen", "", "hermes", "something-new"} { - if mcpConfigNeedsAuthoritativeDaemon(provider, managed, nil) { - t.Fatalf("provider %q never merged host MCP and must not gate", provider) - } - } -} - -func TestMcpConfigNeedsAuthoritativeDaemonForManagedConfigs(t *testing.T) { - t.Parallel() - - for _, agentCfg := range []json.RawMessage{ - json.RawMessage(`{"mcpServers":{}}`), - json.RawMessage(`{"mcpServers":{"a":{"command":"a"}}}`), - json.RawMessage(`{}`), - } { - if !mcpConfigNeedsAuthoritativeDaemon("claude", agentCfg, nil) { - t.Fatalf("managed config %q must require an authoritative daemon", string(agentCfg)) - } - } -} - -func TestMcpConfigNeedsAuthoritativeDaemonSkipsUnmanaged(t *testing.T) { - t.Parallel() - - // Nothing to widen: the host's servers were always in scope for an agent - // with no config of its own. - for _, agentCfg := range []json.RawMessage{nil, json.RawMessage("null"), json.RawMessage(" null ")} { - if mcpConfigNeedsAuthoritativeDaemon("claude", agentCfg, nil) { - t.Fatalf("unmanaged config %q must not gate the claim", string(agentCfg)) - } - } -} - -// A non-object cannot carry `mcpServers`, so it expresses no boundary — and an -// old daemon does not widen it either, because mergeRuntimeAndAgentMcpConfig -// fails to unmarshal it and falls back to the agent config alone. Gating these -// blocked real tasks with no security benefit. -func TestMcpConfigNeedsAuthoritativeDaemonSkipsNonObjectConfigs(t *testing.T) { - t.Parallel() - - for _, agentCfg := range []json.RawMessage{ - json.RawMessage(`[]`), - json.RawMessage(`[{"mcpServers":{}}]`), - json.RawMessage(`"mcpServers"`), - json.RawMessage(`7`), - json.RawMessage(`true`), - } { - if mcpConfigNeedsAuthoritativeDaemon("claude", agentCfg, nil) { - t.Fatalf("non-object config %q must not gate the claim", string(agentCfg)) - } - } -} - -// The inherit opt-in doubles as the documented escape hatch: the operator has -// declared they accept the host's servers, so an old daemon already matches -// intent and the task must keep running. -func TestMcpConfigNeedsAuthoritativeDaemonSkipsExplicitInherit(t *testing.T) { - t.Parallel() - - if mcpConfigNeedsAuthoritativeDaemon( - "claude", - json.RawMessage(`{"mcpServers":{"a":{"command":"a"}}}`), - json.RawMessage(`{"mcp":{"inherit_runtime":true}}`), - ) { - t.Fatal("an explicit inherit opt-in must not gate the claim") - } -} - -func TestMcpConfigNeedsAuthoritativeDaemonFailsClosedOnBadRuntimeConfig(t *testing.T) { - t.Parallel() - - managed := json.RawMessage(`{"mcpServers":{}}`) - for _, rc := range []json.RawMessage{ - nil, - json.RawMessage(`{}`), - json.RawMessage(`{"mcp":{"inherit_runtime":false}}`), - json.RawMessage(`{"mcp":{"inherit_runtime":"true"}}`), // wrong type - json.RawMessage(`{"mcp":{"inherit_runtime":true}`), // truncated - json.RawMessage(`{"mode":"gateway"}`), - } { - if !mcpConfigNeedsAuthoritativeDaemon("claude", managed, rc) { - t.Fatalf("runtime_config %q must not count as an inherit opt-in", string(rc)) - } - } -} - -// Mirrors the daemon's decodeMcpInheritRuntime so the two sides cannot drift. -func TestRuntimeConfigInheritsRuntimeMcp(t *testing.T) { - t.Parallel() - - if !runtimeConfigInheritsRuntimeMcp(json.RawMessage(`{"mcp":{"inherit_runtime":true}}`)) { - t.Fatal("explicit true must be honoured") - } - for _, rc := range []json.RawMessage{ - nil, - json.RawMessage("null"), - json.RawMessage(`{"mcp":{}}`), - json.RawMessage(`not json`), - } { - if runtimeConfigInheritsRuntimeMcp(rc) { - t.Fatalf("runtime_config %q must not opt in", string(rc)) - } - } -} diff --git a/server/internal/handler/mcp_claim_gate_test.go b/server/internal/handler/mcp_claim_gate_test.go deleted file mode 100644 index e63bd439c00..00000000000 --- a/server/internal/handler/mcp_claim_gate_test.go +++ /dev/null @@ -1,352 +0,0 @@ -package handler - -import ( - "context" - "encoding/json" - "net/http" - "net/http/httptest" - "strings" - "testing" - - "github.com/multica-ai/multica/server/internal/daemonws" - "github.com/multica-ai/multica/server/pkg/protocol" - "github.com/multica-ai/multica/server/pkg/taskfailure" -) - -// The authoritative mcp_config semantics are enforced by the DAEMON, so handing -// a strictly-scoped task to a daemon that predates them would run the agent with -// the runtime host's MCP servers merged in — the GitHub #6283 vulnerability. The -// claim path fails the task closed instead, with a classified reason so the -// upgrade requirement reaches the operator on EVERY claim path (the default one -// is the batch endpoint, which does not surface per-task HTTP errors at all). - -const mcpGateDaemonID = "mcp-gate-daemon" - -// mcpGateAgent seeds an agent with the given provider / mcp_config / -// runtime_config plus one queued task. Returns the runtime and task ids. -func mcpGateAgent(t *testing.T, name, provider, mcpConfig, runtimeConfig string) (runtimeID, taskID string) { - t.Helper() - ctx := context.Background() - - agentID := createHandlerTestAgent(t, name, []byte(mcpConfig)) - if err := testPool.QueryRow(ctx, - `SELECT runtime_id FROM agent WHERE id = $1`, agentID).Scan(&runtimeID); err != nil { - t.Fatalf("get agent runtime: %v", err) - } - // The gate only fires for providers whose pre-#6283 daemon merged host MCP, - // so the provider is part of the fixture rather than an incidental default. - if _, err := testPool.Exec(ctx, - `UPDATE agent_runtime SET provider = $2 WHERE id = $1`, runtimeID, provider); err != nil { - t.Fatalf("set runtime provider: %v", err) - } - t.Cleanup(func() { - testPool.Exec(context.Background(), - `UPDATE agent_runtime SET provider = 'claude' WHERE id = $1`, runtimeID) - }) - if runtimeConfig != "" { - if _, err := testPool.Exec(ctx, - `UPDATE agent SET runtime_config = $2::jsonb WHERE id = $1`, agentID, runtimeConfig); err != nil { - t.Fatalf("set runtime_config: %v", err) - } - } - if err := testPool.QueryRow(ctx, ` - INSERT INTO agent_task_queue (agent_id, runtime_id, status, priority) - VALUES ($1, $2, 'queued', 9000) RETURNING id - `, agentID, runtimeID).Scan(&taskID); err != nil { - t.Fatalf("queue task: %v", err) - } - t.Cleanup(func() { - testPool.Exec(context.Background(), `DELETE FROM agent_task_queue WHERE id = $1`, taskID) - }) - return runtimeID, taskID -} - -// stripAuthoritativeMcp turns a request into a pre-#6283 daemon: the shared -// helper defaults the capability on, matching every current daemon. -func stripAuthoritativeMcp(req *http.Request) *http.Request { - req.Header.Del("X-Client-Capabilities") - return req -} - -func mcpGatePerRuntimeClaim(t *testing.T, runtimeID string, current bool) *httptest.ResponseRecorder { - t.Helper() - w := httptest.NewRecorder() - req := newDaemonTokenRequest(http.MethodPost, "/api/daemon/runtimes/"+runtimeID+"/claim", nil, testWorkspaceID, mcpGateDaemonID) - if !current { - req = stripAuthoritativeMcp(req) - } - req = withURLParam(req, "runtimeId", runtimeID) - testHandler.ClaimTaskByRuntime(w, req) - return w -} - -func mcpGateBatchClaim(t *testing.T, runtimeIDs []string, current bool) *httptest.ResponseRecorder { - t.Helper() - w := httptest.NewRecorder() - req := newDaemonTokenRequest(http.MethodPost, "/api/daemon/tasks/claim", - map[string]any{"daemon_id": mcpGateDaemonID, "runtime_ids": runtimeIDs, "max_tasks": 10}, - testWorkspaceID, mcpGateDaemonID) - if !current { - req = stripAuthoritativeMcp(req) - } - testHandler.ClaimTasksByRuntime(w, req) - return w -} - -// mcpGateWSRPCClaim drives the WS RPC claim, which reuses the batch handler via -// a synthesized request whose capabilities come from the handshake identity -// rather than a live header. -func mcpGateWSRPCClaim(t *testing.T, runtimeID, capabilities string) (int, string) { - t.Helper() - body, err := json.Marshal(map[string]any{ - "daemon_id": mcpGateDaemonID, - "runtime_ids": []string{runtimeID}, - "max_tasks": 10, - }) - if err != nil { - t.Fatal(err) - } - status, raw, rerr := testHandler.DaemonRPCHandler(context.Background(), daemonws.ClientIdentity{ - DaemonID: mcpGateDaemonID, - WorkspaceID: testWorkspaceID, - WorkspaceIDs: []string{testWorkspaceID}, - RuntimeIDs: []string{runtimeID}, - Capabilities: capabilities, - }, "tasks.claim", body) - if rerr != nil { - t.Fatalf("WS RPC claim: %v", rerr) - } - return status, string(raw) -} - -type mcpGateTaskRow struct { - status string - failureReason string - errorMessage string -} - -func mcpGateTask(t *testing.T, taskID string) mcpGateTaskRow { - t.Helper() - var row mcpGateTaskRow - var reason, errMsg *string - if err := testPool.QueryRow(context.Background(), - `SELECT status, failure_reason, error FROM agent_task_queue WHERE id = $1`, - taskID).Scan(&row.status, &reason, &errMsg); err != nil { - t.Fatalf("read task: %v", err) - } - if reason != nil { - row.failureReason = *reason - } - if errMsg != nil { - row.errorMessage = *errMsg - } - return row -} - -// assertRefused checks the outcome that must hold on every claim path: the task -// is failed (not silently cancelled) with the classified reason and the -// actionable message, so the operator can see WHY it did not run. -func assertRefused(t *testing.T, taskID string) { - t.Helper() - row := mcpGateTask(t, taskID) - if row.status != "failed" { - t.Fatalf("task status = %q, want failed", row.status) - } - if row.failureReason != string(taskfailure.ReasonMcpConfigDaemonOutdated) { - t.Fatalf("failure_reason = %q, want %q", row.failureReason, taskfailure.ReasonMcpConfigDaemonOutdated) - } - for _, want := range []string{"upgrade the local daemon", "inherit_runtime"} { - if !strings.Contains(row.errorMessage, want) { - t.Fatalf("error_message must mention %q; got %q", want, row.errorMessage) - } - } -} - -// Path 1 of 3 — legacy per-runtime claim. This one CAN carry the 412. -func TestMcpGate_PerRuntimeClaimRefusesOutdatedDaemon(t *testing.T) { - if testHandler == nil || testPool == nil { - t.Skip("database not available") - } - - runtimeID, taskID := mcpGateAgent(t, "McpGatePerRuntime", "claude", `{"mcpServers":{}}`, "") - - w := mcpGatePerRuntimeClaim(t, runtimeID, false) - if w.Code != http.StatusPreconditionFailed { - t.Fatalf("expected 412, got %d: %s", w.Code, w.Body.String()) - } - var body struct{ Error string } - if err := json.NewDecoder(w.Body).Decode(&body); err != nil { - t.Fatalf("decode error body: %v", err) - } - if !strings.Contains(body.Error, "upgrade the local daemon") { - t.Fatalf("412 body must be actionable; got %q", body.Error) - } - assertRefused(t, taskID) -} - -// Path 2 of 3 — the DEFAULT machine-level batch claim. It answers 200 with a -// task list and cannot express a per-task HTTP error, which is exactly why the -// refusal has to be recorded on the task itself. -func TestMcpGate_BatchClaimRecordsRefusalOnTheTask(t *testing.T) { - if testHandler == nil || testPool == nil { - t.Skip("database not available") - } - - runtimeID, taskID := mcpGateAgent(t, "McpGateBatch", "claude", `{"mcpServers":{}}`, "") - - w := mcpGateBatchClaim(t, []string{runtimeID}, false) - if w.Code != http.StatusOK { - t.Fatalf("batch claim should still answer 200, got %d: %s", w.Code, w.Body.String()) - } - var resp struct { - Tasks []map[string]any `json:"tasks"` - } - if err := json.NewDecoder(w.Body).Decode(&resp); err != nil { - t.Fatalf("decode batch response: %v", err) - } - if len(resp.Tasks) != 0 { - t.Fatalf("refused task must not be dispatched; got %d tasks", len(resp.Tasks)) - } - // The whole point: the reason is visible even though the batch response - // carried no error. - assertRefused(t, taskID) -} - -// A refusal must not cost the daemon the other tasks in the same batch — -// turning the whole batch into a 412 after finalizing siblings would drop them. -func TestMcpGate_BatchClaimStillDeliversHealthyTasks(t *testing.T) { - if testHandler == nil || testPool == nil { - t.Skip("database not available") - } - - refusedRuntime, refusedTask := mcpGateAgent(t, "McpGateBatchRefused", "claude", `{"mcpServers":{}}`, "") - healthyRuntime, healthyTask := mcpGateAgent(t, "McpGateBatchHealthy", "claude", "null", "") - - w := mcpGateBatchClaim(t, []string{refusedRuntime, healthyRuntime}, false) - if w.Code != http.StatusOK { - t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) - } - var resp struct { - Tasks []struct { - ID string `json:"id"` - } `json:"tasks"` - } - if err := json.NewDecoder(w.Body).Decode(&resp); err != nil { - t.Fatalf("decode batch response: %v", err) - } - if len(resp.Tasks) != 1 || resp.Tasks[0].ID != healthyTask { - t.Fatalf("healthy task must still be claimed; got %+v", resp.Tasks) - } - assertRefused(t, refusedTask) -} - -// Path 3 of 3 — the WS RPC claim, which reuses the batch handler through a -// synthesized request. Capabilities travel via the WS handshake identity, so a -// pre-#6283 daemon arrives with none and must be refused the same way. -func TestMcpGate_WSRPCClaimRefusesOutdatedDaemon(t *testing.T) { - if testHandler == nil || testPool == nil { - t.Skip("database not available") - } - - runtimeID, taskID := mcpGateAgent(t, "McpGateWSRPC", "claude", `{"mcpServers":{}}`, "") - - status, body := mcpGateWSRPCClaim(t, runtimeID, "") - if status != http.StatusOK { - t.Fatalf("WS RPC batch claim should answer 200, got %d: %s", status, body) - } - var resp struct { - Tasks []map[string]any `json:"tasks"` - } - if err := json.Unmarshal([]byte(body), &resp); err != nil { - t.Fatalf("decode WS RPC response: %v", err) - } - if len(resp.Tasks) != 0 { - t.Fatalf("refused task must not be dispatched over WS; got %d tasks", len(resp.Tasks)) - } - assertRefused(t, taskID) -} - -func TestMcpGate_WSRPCClaimAcceptsCurrentDaemon(t *testing.T) { - if testHandler == nil || testPool == nil { - t.Skip("database not available") - } - - runtimeID, taskID := mcpGateAgent(t, "McpGateWSRPCCurrent", "claude", `{"mcpServers":{}}`, "") - - status, body := mcpGateWSRPCClaim(t, runtimeID, protocol.DaemonCapabilityAuthoritativeMcpV1) - if status != http.StatusOK { - t.Fatalf("expected 200, got %d: %s", status, body) - } - var resp struct { - Tasks []struct { - ID string `json:"id"` - } `json:"tasks"` - } - if err := json.Unmarshal([]byte(body), &resp); err != nil { - t.Fatalf("decode WS RPC response: %v", err) - } - if len(resp.Tasks) != 1 || resp.Tasks[0].ID != taskID { - t.Fatalf("capability-advertising daemon must receive the task; got %+v", resp.Tasks) - } -} - -// A current daemon on the per-runtime path is unaffected. -func TestMcpGate_PerRuntimeClaimAcceptsCurrentDaemon(t *testing.T) { - if testHandler == nil || testPool == nil { - t.Skip("database not available") - } - - runtimeID, _ := mcpGateAgent(t, "McpGateCurrent", "claude", `{"mcpServers":{}}`, "") - - if w := mcpGatePerRuntimeClaim(t, runtimeID, true); w.Code != http.StatusOK { - t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) - } -} - -// Qwen's pre-#6283 daemon never merged host MCP (it is absent from the runtime -// MCP discovery switch), so gating it would cancel tasks that carry no risk. -func TestMcpGate_QwenOutdatedDaemonStillClaims(t *testing.T) { - if testHandler == nil || testPool == nil { - t.Skip("database not available") - } - - runtimeID, taskID := mcpGateAgent(t, "McpGateQwen", "qwen", `{"mcpServers":{"a":{"command":"a"}}}`, "") - - w := mcpGatePerRuntimeClaim(t, runtimeID, false) - if w.Code != http.StatusOK { - t.Fatalf("an outdated qwen daemon must still claim, got %d: %s", w.Code, w.Body.String()) - } - if got := mcpGateTask(t, taskID).status; got == "failed" { - t.Fatal("qwen task must not be failed by the MCP gate") - } -} - -// The inherit opt-in is the documented escape hatch: it declares the host's -// servers acceptable, so an old daemon already matches intent. -func TestMcpGate_InheritOptInAllowsOutdatedDaemon(t *testing.T) { - if testHandler == nil || testPool == nil { - t.Skip("database not available") - } - - runtimeID, _ := mcpGateAgent(t, "McpGateInherit", "claude", - `{"mcpServers":{"a":{"command":"a"}}}`, `{"mcp":{"inherit_runtime":true}}`) - - if w := mcpGatePerRuntimeClaim(t, runtimeID, false); w.Code != http.StatusOK { - t.Fatalf("expected 200 with inherit_runtime, got %d: %s", w.Code, w.Body.String()) - } -} - -// An agent that never configured MCP, or whose config is not an object, has -// nothing an old daemon could widen. -func TestMcpGate_UnmanagedAndNonObjectConfigsIgnoreGate(t *testing.T) { - if testHandler == nil || testPool == nil { - t.Skip("database not available") - } - - for _, cfg := range []string{"null", "[]"} { - runtimeID, _ := mcpGateAgent(t, "McpGateUnmanaged-"+cfg, "claude", cfg, "") - if w := mcpGatePerRuntimeClaim(t, runtimeID, false); w.Code != http.StatusOK { - t.Fatalf("mcp_config %s must not gate the claim, got %d: %s", cfg, w.Code, w.Body.String()) - } - } -} diff --git a/server/internal/handler/mcp_overlay.go b/server/internal/handler/mcp_overlay.go index b3dcc31579b..a6fe755fe21 100644 --- a/server/internal/handler/mcp_overlay.go +++ b/server/internal/handler/mcp_overlay.go @@ -5,123 +5,8 @@ import ( "encoding/json" "errors" "fmt" - "strings" ) -// mcpConfigDaemonOutdatedMessage is stored on the failed task AND returned on -// the per-runtime claim path, so the operator reads the same actionable text -// wherever they see the failure. Names both remedies: upgrading is the fix, -// inherit_runtime is the honest opt-out (it declares that the host's MCP servers -// are acceptable for this agent). -const mcpConfigDaemonOutdatedMessage = "this agent has a managed mcp_config, which requires a daemon that enforces it as an authoritative allowlist; upgrade the local daemon, or set runtime_config.mcp.inherit_runtime=true to accept the runtime host's MCP servers as well" - -// providersOldDaemonsMergedRuntimeMcp is a FROZEN historical set: the providers -// that daemons WITHOUT DaemonCapabilityAuthoritativeMcpV1 actually merged host -// MCP servers for. It describes shipped behaviour of old binaries, so it is -// finished — do NOT keep it in sync with the daemon's current provider switch. -// -// Why the distinction matters. Runtime MCP discovery for a NEW provider can only -// ship in a daemon that already advertises the capability, so such a daemon is -// never gated in the first place. Adding that provider here would instead start -// failing tasks on old daemons that never merged for it — re-creating exactly -// the false-positive this list was introduced to fix (qwen was gated for a risk -// that did not exist). A provider belongs here only if some released, -// pre-capability daemon merged host MCP for it. -// -// Every provider in this set was present in loadRuntimeMcpServerConfigs at the -// time the capability shipped; qwen was deliberately absent and already had -// strict semantics. -var providersOldDaemonsMergedRuntimeMcp = map[string]bool{ - "claude": true, - "codebuddy": true, - "codex": true, - "cursor": true, - "opencode": true, - "openclaw": true, -} - -// mcpConfigNeedsAuthoritativeDaemon reports whether handing this agent's -// mcp_config to a daemon that predates DaemonCapabilityAuthoritativeMcpV1 would -// widen the agent's tool surface beyond what the operator configured. -// -// True only when all of the following hold: -// -// - The agent itself saved a managed config OBJECT. Only an object can carry -// `mcpServers`, so a non-object (array or primitive) expresses no boundary. -// An old daemon does not widen one either: mergeRuntimeAndAgentMcpConfig -// fails to unmarshal it and falls back to the agent config alone. -// - The agent has NOT opted into inheriting the host's servers via -// runtime_config.mcp.inherit_runtime. That opt-in means the merge is exactly -// what the operator asked for, so an old daemon already matches intent — and -// it doubles as the documented escape hatch for an operator who cannot -// upgrade the daemon yet. -// - The runtime's provider is one an old daemon actually merged for. A stale -// qwen daemon, for example, never merged host MCP, so failing its claims -// would be a pure false positive. -// -// An unknown/empty provider is treated as NOT merging: the gate only fires where -// we can point at the concrete old behaviour it protects against. -func mcpConfigNeedsAuthoritativeDaemon(provider string, agentMcpConfig, runtimeConfig json.RawMessage) bool { - if !providersOldDaemonsMergedRuntimeMcp[strings.TrimSpace(strings.ToLower(provider))] { - return false - } - if !isManagedMcpConfigObject(agentMcpConfig) { - return false - } - return !runtimeConfigInheritsRuntimeMcp(runtimeConfig) -} - -// isManagedMcpConfigObject reports whether the raw value is a JSON object, the -// only shape that can express a managed MCP server set. -func isManagedMcpConfigObject(raw json.RawMessage) bool { - trimmed := bytes.TrimSpace(raw) - return len(trimmed) > 0 && trimmed[0] == '{' -} - -// runtimeConfigInheritsRuntimeMcp mirrors the daemon's -// decodeMcpInheritRuntime. Fails closed on malformed JSON: an unreadable -// runtime_config never counts as an opt-in to the wider tool surface. -func runtimeConfigInheritsRuntimeMcp(raw json.RawMessage) bool { - if !hasManagedJSON(raw) { - return false - } - var cfg struct { - Mcp struct { - InheritRuntime bool `json:"inherit_runtime"` - } `json:"mcp"` - } - if err := json.Unmarshal(raw, &cfg); err != nil { - return false - } - return cfg.Mcp.InheritRuntime -} - -// resolveClaimMcpConfig builds the `mcp_config` a claim response carries and -// reports whether that payload came ONLY from the per-task overlay. -// -// The daemon treats a managed mcp_config as an authoritative allowlist and will -// not merge the host's own MCP servers into it (GitHub #6283). That decision -// needs one fact only the server has: whether the agent itself saved an -// mcp_config, or whether the field is populated purely because the initiator -// has an integration enabled. Without it, enabling a Composio integration on an -// agent that never configured MCP would silently strip the runtime servers it -// was already inheriting. -// -// On a malformed overlay the agent's own config is returned unchanged and -// overlayOnly is false, matching the pre-existing "never surprise-disable the -// agent's saved servers" failure mode. The error is returned for logging. -func resolveClaimMcpConfig(agentMcpConfig, overlay json.RawMessage) (json.RawMessage, bool, error) { - agentAuthored := hasManagedJSON(agentMcpConfig) - if !hasManagedJSON(overlay) { - return passthroughAgentMcpConfig(agentMcpConfig), false, nil - } - merged, err := mergeMCPOverlay(agentMcpConfig, overlay) - if err != nil { - return passthroughAgentMcpConfig(agentMcpConfig), false, err - } - return merged, !agentAuthored && hasManagedJSON(merged), nil -} - // mergeMCPOverlay layers a per-task overlay on top of an agent's saved // mcp_config and returns the merged JSON for the daemon claim wire shape. // diff --git a/server/internal/handler/runtime_local_skills.go b/server/internal/handler/runtime_local_skills.go index c082945dc33..7d7eb0a8f9a 100644 --- a/server/internal/handler/runtime_local_skills.go +++ b/server/internal/handler/runtime_local_skills.go @@ -88,27 +88,10 @@ type LocalSkillListStore interface { // never start a claim they might have to abort. HasPending(ctx context.Context, runtimeID string) (bool, error) PopPending(ctx context.Context, runtimeID string) (*RuntimeLocalSkillListRequest, error) - Complete(ctx context.Context, id string, result RuntimeLocalSkillListResult) error + Complete(ctx context.Context, id string, skills []RuntimeLocalSkillSummary, supported bool, mcpServers []RuntimeLocalMcpServerSummary, mcpSupported bool) error Fail(ctx context.Context, id string, errMsg string) error } -// RuntimeLocalSkillListResult is the completed payload a daemon reports for a -// runtime capability listing. Grouped into a struct rather than positional -// arguments so adding a capability flag cannot silently swap two adjacent -// bools at a call site. -type RuntimeLocalSkillListResult struct { - Skills []RuntimeLocalSkillSummary - Supported bool - McpServers []RuntimeLocalMcpServerSummary - McpSupported bool - // AuthoritativeMcp reports whether the daemon enforces a managed - // mcp_config as an authoritative allowlist. False for any daemon - // predating that behaviour, which still merges the host's own MCP servers - // underneath the managed set (GitHub #6283) — the UI must say "needs - // upgrade" rather than claiming those servers are excluded. - AuthoritativeMcp bool -} - // LocalSkillImportRequestInput carries the fields needed to enqueue a // runtime-local-skill import. SupportsConflict gates the structured-conflict // contract: only clients that opt in receive the `conflict` terminal status; @@ -224,13 +207,10 @@ type RuntimeLocalSkillListRequest struct { Supported bool `json:"supported"` McpServers []RuntimeLocalMcpServerSummary `json:"mcp_servers,omitempty"` McpSupported bool `json:"mcp_supported"` - // AuthoritativeMcp is false for daemons predating GitHub #6283's fix, which - // still merge the host's MCP servers into a managed mcp_config. - AuthoritativeMcp bool `json:"authoritative_mcp"` - Error string `json:"error,omitempty"` - CreatedAt time.Time `json:"created_at"` - UpdatedAt time.Time `json:"updated_at"` - RunStartedAt *time.Time `json:"-"` + Error string `json:"error,omitempty"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` + RunStartedAt *time.Time `json:"-"` } type RuntimeLocalSkillImportRequest struct { @@ -339,17 +319,16 @@ func (s *InMemoryLocalSkillListStore) PopPending(_ context.Context, runtimeID st return oldest, nil } -func (s *InMemoryLocalSkillListStore) Complete(_ context.Context, id string, result RuntimeLocalSkillListResult) error { +func (s *InMemoryLocalSkillListStore) Complete(_ context.Context, id string, skills []RuntimeLocalSkillSummary, supported bool, mcpServers []RuntimeLocalMcpServerSummary, mcpSupported bool) error { s.mu.Lock() defer s.mu.Unlock() if req, ok := s.requests[id]; ok { req.Status = RuntimeLocalSkillCompleted - req.Skills = result.Skills - req.Supported = result.Supported - req.McpServers = result.McpServers - req.McpSupported = result.McpSupported - req.AuthoritativeMcp = result.AuthoritativeMcp + req.Skills = skills + req.Supported = supported + req.McpServers = mcpServers + req.McpSupported = mcpSupported req.UpdatedAt = time.Now() } return nil @@ -775,10 +754,7 @@ func (h *Handler) ReportLocalSkillListResult(w http.ResponseWriter, r *http.Requ Supported *bool `json:"supported"` McpServers []RuntimeLocalMcpServerSummary `json:"mcp_servers"` McpSupported *bool `json:"mcp_supported"` - // Absent from every daemon that predates GitHub #6283's fix, which is - // exactly the population that must NOT be reported as authoritative. - AuthoritativeMcp *bool `json:"authoritative_mcp"` - Error string `json:"error"` + Error string `json:"error"` } if err := json.NewDecoder(r.Body).Decode(&body); err != nil { writeError(w, http.StatusBadRequest, "invalid request body") @@ -794,17 +770,7 @@ func (h *Handler) ReportLocalSkillListResult(w http.ResponseWriter, r *http.Requ if body.McpSupported != nil { mcpSupported = *body.McpSupported } - authoritativeMcp := false - if body.AuthoritativeMcp != nil { - authoritativeMcp = *body.AuthoritativeMcp - } - if err := h.LocalSkillListStore.Complete(r.Context(), requestID, RuntimeLocalSkillListResult{ - Skills: body.Skills, - Supported: supported, - McpServers: body.McpServers, - McpSupported: mcpSupported, - AuthoritativeMcp: authoritativeMcp, - }); err != nil { + if err := h.LocalSkillListStore.Complete(r.Context(), requestID, body.Skills, supported, body.McpServers, mcpSupported); err != nil { // Surface the store failure as 5xx so the daemon can retry instead // of swallowing the report (leaves the request stuck in running // until the server-side timeout, which is exactly the "looks OK but diff --git a/server/internal/handler/runtime_local_skills_redis_store.go b/server/internal/handler/runtime_local_skills_redis_store.go index 2c345c23d06..f0dd7fec6ba 100644 --- a/server/internal/handler/runtime_local_skills_redis_store.go +++ b/server/internal/handler/runtime_local_skills_redis_store.go @@ -223,7 +223,7 @@ func (s *RedisLocalSkillListStore) PopPending(ctx context.Context, runtimeID str return nil, nil } -func (s *RedisLocalSkillListStore) Complete(ctx context.Context, id string, result RuntimeLocalSkillListResult) error { +func (s *RedisLocalSkillListStore) Complete(ctx context.Context, id string, skills []RuntimeLocalSkillSummary, supported bool, mcpServers []RuntimeLocalMcpServerSummary, mcpSupported bool) error { req, err := s.loadListRequest(ctx, id) if err != nil { return err @@ -232,11 +232,10 @@ func (s *RedisLocalSkillListStore) Complete(ctx context.Context, id string, resu return nil } req.Status = RuntimeLocalSkillCompleted - req.Skills = result.Skills - req.Supported = result.Supported - req.McpServers = result.McpServers - req.McpSupported = result.McpSupported - req.AuthoritativeMcp = result.AuthoritativeMcp + req.Skills = skills + req.Supported = supported + req.McpServers = mcpServers + req.McpSupported = mcpSupported req.UpdatedAt = time.Now() return s.persistListRequest(ctx, req) } diff --git a/server/internal/handler/runtime_local_skills_redis_store_test.go b/server/internal/handler/runtime_local_skills_redis_store_test.go index 62c00eaf8bb..0fa3d7b1b0d 100644 --- a/server/internal/handler/runtime_local_skills_redis_store_test.go +++ b/server/internal/handler/runtime_local_skills_redis_store_test.go @@ -78,13 +78,7 @@ func TestRedisLocalSkillListStore_CreateGetComplete(t *testing.T) { mcpServers := []RuntimeLocalMcpServerSummary{ {Name: "fetch", Transport: "stdio", Source: "User config", Enabled: true}, } - if err := store.Complete(ctx, req.ID, RuntimeLocalSkillListResult{ - Skills: skills, - Supported: true, - McpServers: mcpServers, - McpSupported: true, - AuthoritativeMcp: true, - }); err != nil { + if err := store.Complete(ctx, req.ID, skills, true, mcpServers, true); err != nil { t.Fatalf("complete: %v", err) } diff --git a/server/internal/handler/runtime_local_skills_test.go b/server/internal/handler/runtime_local_skills_test.go index a7d9a44f6b8..71fd795f4f1 100644 --- a/server/internal/handler/runtime_local_skills_test.go +++ b/server/internal/handler/runtime_local_skills_test.go @@ -140,7 +140,7 @@ func TestInMemoryLocalSkillListStore_PreservesSummaries(t *testing.T) { t.Fatalf("unmarshal report body: %v", err) } - if err := store.Complete(ctx, req.ID, RuntimeLocalSkillListResult{Skills: parsed.Skills, Supported: true}); err != nil { + if err := store.Complete(ctx, req.ID, parsed.Skills, true, nil, false); err != nil { t.Fatalf("complete: %v", err) } got, err := store.Get(ctx, req.ID) diff --git a/server/internal/metrics/failure_reason_label_test.go b/server/internal/metrics/failure_reason_label_test.go deleted file mode 100644 index 56a0c56d998..00000000000 --- a/server/internal/metrics/failure_reason_label_test.go +++ /dev/null @@ -1,44 +0,0 @@ -package metrics - -import ( - "testing" - - "github.com/multica-ai/multica/server/pkg/taskfailure" -) - -// NormalizeFailureReason falls back to free-text Classify() for any value that -// is not in taskfailure.AllReasons(), which silently relabels an unregistered -// platform-side reason as `agent_error.unknown` — the metric then blames the -// agent for a platform refusal, and the series is never pre-warmed. -// -// Pinning the WHOLE canonical set (rather than one reason) is deliberate: it -// makes forgetting to register the next new Reason a test failure instead of a -// monitoring mystery. This was a real miss for -// `mcp_config_daemon_outdated` (GitHub #6283). -func TestNormalizeFailureReasonKeepsEveryCanonicalReason(t *testing.T) { - for _, reason := range taskfailure.AllReasons() { - wire := reason.String() - if got := NormalizeFailureReason(wire); got != wire { - t.Errorf("NormalizeFailureReason(%q) = %q; every canonical reason must survive as itself (missing from allReasons?)", wire, got) - } - } -} - -// The claim-time MCP refusal is platform-side; a regression here would put it -// back in the agent-error bucket. -func TestNormalizeFailureReasonMcpConfigDaemonOutdated(t *testing.T) { - wire := taskfailure.ReasonMcpConfigDaemonOutdated.String() - if got := NormalizeFailureReason(wire); got != wire { - t.Fatalf("NormalizeFailureReason(%q) = %q, want it preserved", wire, got) - } - if taskfailure.ReasonMcpConfigDaemonOutdated.IsAgentError() { - t.Fatal("the MCP daemon-upgrade refusal must classify as platform-side, not agent error") - } -} - -// Unknown values still degrade rather than leaking unbounded label cardinality. -func TestNormalizeFailureReasonUnknownStillDegrades(t *testing.T) { - if got := NormalizeFailureReason("something-nobody-registered"); got == "something-nobody-registered" { - t.Fatal("an unregistered value must not become its own label") - } -} diff --git a/server/internal/service/builtin_skills/multica-creating-agents/SKILL.md b/server/internal/service/builtin_skills/multica-creating-agents/SKILL.md index 752fc5fad66..d34142395a7 100644 --- a/server/internal/service/builtin_skills/multica-creating-agents/SKILL.md +++ b/server/internal/service/builtin_skills/multica-creating-agents/SKILL.md @@ -224,37 +224,6 @@ and `ps`; the inline `--mcp-config ` does not. The CLI requires a JSON **object** or the literal `null`; a top-level array or primitive is rejected client-side, and empty stdin/file input errors rather than silently clearing. -**`mcp_config` is an authoritative allowlist, not an addition.** The three states -are distinct and are the agent's MCP access-control boundary (GitHub #6283): - -| value | what the agent can reach | -| --- | --- | -| omitted / `null` | the runtime host's own MCP servers (native inheritance) | -| `{"mcpServers":{}}` | nothing — no MCP servers at all | -| non-empty object | exactly those servers; the host's own are excluded | - -To get both the managed set AND the host's servers, set the persisted opt-in -`runtime_config.mcp.inherit_runtime` to `true`; it is `false` by default and a -malformed `runtime_config` never enables it. - -```bash -multica agent update --runtime-config '{"mcp":{"inherit_runtime":true}}' -``` - -`runtime_config` is replaced wholesale, so merge the agent's existing keys -(e.g. OpenClaw `mode`/`gateway`) into that JSON rather than dropping them. - -Enforcement lives in the **daemon**. The claim path refuses to hand a -strictly-scoped task to a daemon that does not advertise the -`authoritative-mcp-v1` capability: the task is failed with reason -`mcp_config_daemon_outdated` and an actionable message, rather than letting an -older daemon merge the host's servers in. Upgrade the daemon, or set -`inherit_runtime` to accept the wider surface deliberately. - -The gate is scoped to the providers whose older daemons actually merged host MCP -(`claude`, `codebuddy`, `codex`, `cursor`, `opencode`, `openclaw`). Qwen Code was -never merged and already had strict semantics, so its tasks are never gated. - Two ways `mcp_config` differs from `custom_env`: - **It IS settable through `agent update`.** Unlike `custom_env`, `mcp_config` diff --git a/server/internal/service/builtin_skills/multica-creating-agents/references/creating-agents-source-map.md b/server/internal/service/builtin_skills/multica-creating-agents/references/creating-agents-source-map.md index 13831e0adb1..5dab5533661 100644 --- a/server/internal/service/builtin_skills/multica-creating-agents/references/creating-agents-source-map.md +++ b/server/internal/service/builtin_skills/multica-creating-agents/references/creating-agents-source-map.md @@ -76,11 +76,6 @@ only. | `mcp_config` null-skip on create | 704–705 | raw JSON copied through unless the body value is the literal `null` | | `mcp_config` redacted on read | 54, 848–851 | `redactMcpConfig` sets `McpConfigRedacted=true`; a private agent read by a member also redacts (494, 509) | | Qwen Code managed-MCP injection | `pkg/agent/qwen.go` | Non-null `mcp_config` is written to a daemon-owned 0600 temporary JSON file and passed with `--mcp-config`; the file is removed after the process exits, while `null` preserves native inheritance. | -| `mcp_config` is an authoritative allowlist | `internal/daemon/mcp_inherit.go` `resolveEffectiveMcpConfig` | Three states: `null` → provider-native inheritance; `{"mcpServers":{}}` → strict empty; non-empty → exactly that set. The runtime host's own MCP servers are NOT merged in (GitHub #6283). | -| `runtime_config.mcp.inherit_runtime` opt-in | `internal/daemon/mcp_inherit.go` `decodeMcpInheritRuntime`; `internal/handler/mcp_overlay.go` `runtimeConfigInheritsRuntimeMcp` | Persisted bool, default false, that restores the additive merge. Fails closed on absent/malformed `runtime_config`. Both sides decode the same shape — keep them in lockstep. | -| Claim fails closed against an outdated daemon | `internal/handler/daemon.go` claim path; `internal/handler/mcp_overlay.go` `mcpConfigNeedsAuthoritativeDaemon`; `pkg/protocol/messages.go` `DaemonCapabilityAuthoritativeMcpV1`; `pkg/taskfailure/failure.go` `ReasonMcpConfigDaemonOutdated` | A managed, non-inheriting `mcp_config` claimed by a daemon that does not advertise `authoritative-mcp-v1` fails the task with reason `mcp_config_daemon_outdated` (not a bare cancel: the default batch claim path cannot carry a per-task HTTP error) and returns 412 on the per-runtime path. Scoped to providers in `providersOldDaemonsMergedRuntimeMcp`, so qwen is never gated. Not auto-retryable. | -| `mcp_config_overlay_only` claim field | `internal/handler/mcp_overlay.go` `resolveClaimMcpConfig`; `internal/handler/agent.go` `TaskAgentData`; `internal/daemon/types.go` `AgentData` | Distinguishes an agent-authored config from a per-task Composio overlay folded into the same field, so enabling an integration does not strip the host servers an unconfigured agent was inheriting. | -| Daemon reports `authoritative_mcp` capability | `internal/daemon/daemon.go` `handleLocalSkillList`; `internal/handler/runtime_local_skills.go` `RuntimeLocalSkillListResult` | Additive flag on the runtime-capabilities response; absent on older daemons so the agent MCP tab warns "needs upgrade" instead of claiming host servers are excluded. | | Random emoji avatar default | `agent_avatar.go` 11–32; `agent.go` 1127–1133 | Omitted, empty, or whitespace-only `avatar_url` becomes a cryptographically selected `emoji:` sentinel; explicit values are preserved. The template handler uses the same helper at `agent_template.go` 458. | | `CreateAgent` insert params | `agent.go` create path | Persists avatar_url, runtime_config, instructions, custom_env, custom_args, model, thinking_level, service_tier, mcp_config, visibility, max_concurrent_tasks | | `UpdateAgent` rejects `custom_env` | 910–913 | if `custom_env` present in body → 400 "use PUT /api/agents/{id}/env (or `multica agent env set`)" | diff --git a/server/pkg/db/generated/task_usage.sql.go b/server/pkg/db/generated/task_usage.sql.go index 939cc2a2a2e..7636ff6946f 100644 --- a/server/pkg/db/generated/task_usage.sql.go +++ b/server/pkg/db/generated/task_usage.sql.go @@ -272,7 +272,7 @@ type ListDashboardFailuresDailyRow struct { // failed row whose failure_reason column is NULL or empty (pre-MUL-1949 // rows, or a failure path that forgot to classify) collapses into the // 'unclassified' bucket so it stays countable instead of masquerading as a -// success. Cardinality is bounded by days x (23 reasons + 2), so the whole +// success. Cardinality is bounded by days x (21 reasons + 2), so the whole // window fits in one small payload. // // Unlike ListDashboardRunTimeDaily this does NOT require started_at — a task diff --git a/server/pkg/db/queries/task_usage.sql b/server/pkg/db/queries/task_usage.sql index 519d3ac462c..2d1531e0448 100644 --- a/server/pkg/db/queries/task_usage.sql +++ b/server/pkg/db/queries/task_usage.sql @@ -189,7 +189,7 @@ ORDER BY total_seconds DESC; -- failed row whose failure_reason column is NULL or empty (pre-MUL-1949 -- rows, or a failure path that forgot to classify) collapses into the -- 'unclassified' bucket so it stays countable instead of masquerading as a --- success. Cardinality is bounded by days x (23 reasons + 2), so the whole +-- success. Cardinality is bounded by days x (21 reasons + 2), so the whole -- window fits in one small payload. -- -- Unlike ListDashboardRunTimeDaily this does NOT require started_at — a task diff --git a/server/pkg/protocol/messages.go b/server/pkg/protocol/messages.go index 783e99dd68b..7e0f1888ca1 100644 --- a/server/pkg/protocol/messages.go +++ b/server/pkg/protocol/messages.go @@ -11,17 +11,6 @@ const ( // everyone else keeps using the HTTP claim endpoint. DaemonCapabilityRPCV1 = "rpc-v1" - // DaemonCapabilityAuthoritativeMcpV1 advertises that the daemon treats a - // managed agent mcp_config as an authoritative allowlist instead of - // merging the runtime host's own MCP servers underneath it (GitHub #6283). - // - // This is a SECURITY capability, not a feature negotiation: a daemon - // without it silently widens an explicitly-scoped mcp_config to the full - // host set. The claim path therefore refuses to hand a strictly-scoped - // task to a daemon that does not advertise it, rather than running the - // agent with more tools than the operator configured. - DaemonCapabilityAuthoritativeMcpV1 = "authoritative-mcp-v1" - // AppCapabilityChatDraftRestoreV1 is advertised (X-Client-Capabilities) by // app clients that understand the durable draft-restore recovery path: // chat:cancel_finalized as an invalidation hint plus the draft-restores diff --git a/server/pkg/taskfailure/classify.go b/server/pkg/taskfailure/classify.go index f2a02dfd87c..62c5e744f19 100644 --- a/server/pkg/taskfailure/classify.go +++ b/server/pkg/taskfailure/classify.go @@ -63,7 +63,7 @@ func Classify(rawError string) Reason { trimmed := strings.TrimSpace(rawError) if trimmed == "" { // SQL maps NULL/empty to a separate bucket ("empty_error"), - // but that bucket is not part of the canonical 23. In-flight + // but that bucket is not part of the canonical 22. In-flight // callers should never hand us empty input — if they do, the // safest landing is the catchall. return ReasonAgentUnknown diff --git a/server/pkg/taskfailure/failure.go b/server/pkg/taskfailure/failure.go index cef55ca84db..fa014785904 100644 --- a/server/pkg/taskfailure/failure.go +++ b/server/pkg/taskfailure/failure.go @@ -12,13 +12,13 @@ // This package lifts that classifier into the in-flight write path so the // stored failure_reason is already refined when the row is first // persisted, and so server / daemon / cloud share a single source of -// truth for the canonical 23 values. PR1 of the Grafana board plan +// truth for the canonical 22 values. PR1 of the Grafana board plan // ([MUL-2946](https://multica/issues/MUL-2946)). Subsequent PRs use // AllReasons() to pre-warm the Prometheus failure_reason label set. // -// The 23 canonical values fall into two groups: +// The 22 canonical values fall into two groups: // -// - 9 platform-side values (no `agent_error.` prefix) emitted by the +// - 8 platform-side values (no `agent_error.` prefix) emitted by the // server-side sweepers and daemon classifiers when the failure is // attributable to the platform/scheduler/runtime layer rather than // anything the agent process did: @@ -48,7 +48,7 @@ type Reason string // agentErrorPrefix marks the 14 sub-reasons that originate inside the // agent process (provider error, runner crash, context overflow, etc.) -// as opposed to the 9 platform-side reasons (queue expiry, runtime +// as opposed to the 8 platform-side reasons (queue expiry, runtime // offline, sweeper timeout, etc.). IsAgentError uses this prefix so // callers don't have to enumerate the agent-side reasons by hand. const agentErrorPrefix = "agent_error." @@ -111,21 +111,6 @@ const ( // taskRunFailureReason in daemon/daemon.go. ReasonSkillBundleUnavailable Reason = "skill_bundle_unavailable" - // ReasonMcpConfigDaemonOutdated: the agent has a managed mcp_config that - // must be enforced as an authoritative allowlist, but the daemon that - // claimed the task predates that enforcement and would have merged the - // runtime host's own MCP servers underneath it (GitHub #6283). The claim - // path refuses rather than run the agent with tools the operator scoped - // out, so the agent process was never launched. Platform-side: nothing the - // agent did caused it, and the operator fix is to upgrade the daemon (or - // set runtime_config.mcp.inherit_runtime to accept the host's servers). - // - // Deliberately NOT in retryableReasons: the same outdated daemon would - // claim the retry and fail it again, so an auto-retry would spin instead of - // surfacing the upgrade requirement. Written by the claim path in - // internal/handler/daemon.go. - ReasonMcpConfigDaemonOutdated Reason = "mcp_config_daemon_outdated" - // Agent process side: failure surfaced by the agent CLI / SDK as // an error string. Classify(rawError) is responsible for picking // the right sub-reason from the string. IsAgentError returns true @@ -201,15 +186,9 @@ const ( ReasonAgentUnknown Reason = "agent_error.unknown" ) -// allReasons is the canonical ordered list of the 23 reasons. Order is +// allReasons is the canonical ordered list of the 22 reasons. Order is // stable so callers (e.g. Prometheus collectors that pre-warm series via -// AllReasons) can build deterministic label sets across restarts. New reasons -// are APPENDED to their group so existing positions never shift. -// -// Membership is not cosmetic: metrics.NormalizeFailureReason only treats values -// in AllReasons() as known, and falls back to free-text Classify() otherwise — -// which silently relabels an unregistered platform-side reason as -// `agent_error.unknown`. Every new Reason must be added here. +// AllReasons) can build deterministic label sets across restarts. // // Ordering: // 1. Platform-side reasons in the same order they tend to fire in a @@ -226,9 +205,6 @@ var allReasons = []Reason{ ReasonAgentBlocked, ReasonAPIInvalidRequest, ReasonSkillBundleUnavailable, - // Fires at claim time — earlier than any of the above in lifecycle terms, - // but appended to keep the established label ordering stable. - ReasonMcpConfigDaemonOutdated, // Agent process side: provider errors. ReasonAgentProviderAuthOrAccess, @@ -268,7 +244,7 @@ func (r Reason) IsAgentError() bool { return strings.HasPrefix(string(r), agentErrorPrefix) } -// AllReasons returns the canonical 23 reasons in a stable order. The +// AllReasons returns the canonical 22 reasons in a stable order. The // caller MUST NOT mutate the returned slice; a copy is returned so // concurrent callers can append to their local copy without corrupting // the package-level fixture. diff --git a/server/pkg/taskfailure/failure_test.go b/server/pkg/taskfailure/failure_test.go index 937c4627ada..4f1a9a980d5 100644 --- a/server/pkg/taskfailure/failure_test.go +++ b/server/pkg/taskfailure/failure_test.go @@ -27,7 +27,6 @@ func TestReasonStringWireValues(t *testing.T) { {ReasonAgentBlocked, "agent_blocked"}, {ReasonAPIInvalidRequest, "api_invalid_request"}, {ReasonSkillBundleUnavailable, "skill_bundle_unavailable"}, - {ReasonMcpConfigDaemonOutdated, "mcp_config_daemon_outdated"}, // Agent-side. {ReasonAgentProviderAuthOrAccess, "agent_error.provider_auth_or_access"}, {ReasonAgentProviderQuotaLimit, "agent_error.provider_quota_limit"}, @@ -45,7 +44,7 @@ func TestReasonStringWireValues(t *testing.T) { {ReasonAgentUnknown, "agent_error.unknown"}, } - if got, want := len(cases), 23; got != want { + if got, want := len(cases), 22; got != want { t.Fatalf("constant count = %d, want %d (canonical taxonomy size)", got, want) } @@ -73,7 +72,6 @@ func TestIsAgentError(t *testing.T) { ReasonAgentBlocked, ReasonAPIInvalidRequest, ReasonSkillBundleUnavailable, - ReasonMcpConfigDaemonOutdated, } for _, r := range platformSide { if r.IsAgentError() { @@ -114,8 +112,8 @@ func TestAllReasonsContents(t *testing.T) { t.Parallel() got := AllReasons() - if len(got) != 23 { - t.Fatalf("AllReasons() returned %d entries, want 23", len(got)) + if len(got) != 22 { + t.Fatalf("AllReasons() returned %d entries, want 22", len(got)) } seen := make(map[Reason]bool, len(got)) @@ -132,8 +130,8 @@ func TestAllReasonsContents(t *testing.T) { } } - if platformCount != 9 { - t.Errorf("AllReasons(): platform-side count = %d, want 9", platformCount) + if platformCount != 8 { + t.Errorf("AllReasons(): platform-side count = %d, want 8", platformCount) } if agentCount != 14 { t.Errorf("AllReasons(): agent-side count = %d, want 14", agentCount) @@ -147,7 +145,6 @@ func TestAllReasonsContents(t *testing.T) { ReasonQueuedExpired, ReasonRuntimeOffline, ReasonRuntimeRecovery, ReasonTimeout, ReasonIterationLimit, ReasonAgentBlocked, ReasonAPIInvalidRequest, ReasonSkillBundleUnavailable, - ReasonMcpConfigDaemonOutdated, ReasonAgentProviderAuthOrAccess, ReasonAgentProviderQuotaLimit, ReasonAgentProviderCapacityOrRateLimit, ReasonAgentProviderServerError, ReasonAgentProviderNetwork, ReasonAgentProcessFailure,