diff --git a/apps/desktop/renderer/src/org/HireDrawer.tsx b/apps/desktop/renderer/src/org/HireDrawer.tsx index a40db3b2..dfd3e981 100644 --- a/apps/desktop/renderer/src/org/HireDrawer.tsx +++ b/apps/desktop/renderer/src/org/HireDrawer.tsx @@ -27,7 +27,7 @@ const AGENT_CONVERSATION_TIMEOUT_MS = 75_000; const PLATFORM_BUDGET_POOL = 10_000_000; const PHASE_COPY_KEYS: Record = { validate: "hire.phaseValidate", stage: "hire.phaseStage", apply: "hire.phaseApply" }; -const FAILURE_COPY_KEYS: Record = { hire_position_exists: "hire.errExists", hire_timeout: "hire.errTimeout", control_plane_unreachable: "hire.errOffline", engine_unavailable: "hire.errCli", engine_capability_missing: "hire.errCapability" }; +const FAILURE_COPY_KEYS: Record = { hire_position_exists: "hire.errExists", hire_timeout: "hire.errTimeout", control_plane_unreachable: "hire.errOffline", engine_unavailable: "hire.errCli", engine_capability_missing: "hire.errCapability", hire_mcp_unsupported: "hire.errMcpCapability" }; const MEMORY_OPTIONS: Array<{ kind: HireMemorySource["kind"]; labelKey: string; locator: string }> = [ { kind: "position_docs", labelKey: "hire.memoryPositionDocs", locator: "./knowledge/**" }, { kind: "workspace_docs", labelKey: "hire.memoryWorkspaceDocs", locator: "./**" }, diff --git a/apps/server/src/agent-registry.ts b/apps/server/src/agent-registry.ts index 421ed5fa..10a90562 100644 --- a/apps/server/src/agent-registry.ts +++ b/apps/server/src/agent-registry.ts @@ -320,3 +320,25 @@ export function getAgentHostCapabilities(host: Pick HOST_DEFINITIONS[id].capabilities.includes("mcp")); +} diff --git a/apps/server/src/org/profile-edit.ts b/apps/server/src/org/profile-edit.ts index 87489429..82e2ac68 100644 --- a/apps/server/src/org/profile-edit.ts +++ b/apps/server/src/org/profile-edit.ts @@ -45,6 +45,7 @@ import { ORGANIZATION_FILE } from "../workspace-state.js"; import { resolvePositionPackageDir } from "../context-sources.js"; import { derivePermissionArtifacts, permissionsFromPackage } from "./permission-artifacts.js"; import { validatePermissions } from "./permissions.js"; +import { anyAgentHostSupportsEmployeeMcp } from "../agent-registry.js"; /** Same bound `POST /hire` applies to a display name (UTF-8 bytes). */ const MAX_NAME_BYTES = 128; @@ -96,6 +97,12 @@ export function assertProfilePatch(raw: unknown): PositionProfilePatch { if (body.name !== undefined) patch.name = (body.name as string).trim(); if (body.mode !== undefined) patch.mode = body.mode as PositionProfilePatch["mode"]; if (body.permissions !== undefined) patch.permissions = validatePermissions(body.permissions, invalid); + // A grant no bundled Host can honor must not enter an existing package + // either; dropping every grant (empty array) stays allowed so the manual + // unbind workaround keeps working (#314). + if (patch.permissions !== undefined && (patch.permissions.mcpServers ?? []).length > 0 && !anyAgentHostSupportsEmployeeMcp()) { + throw invalid("permissions.mcpServers: no engine honors employee-level MCP bindings yet; leave MCP connectors unbound"); + } return patch; } diff --git a/apps/server/src/routes/hire.ts b/apps/server/src/routes/hire.ts index 78da422a..adf54a3a 100644 --- a/apps/server/src/routes/hire.ts +++ b/apps/server/src/routes/hire.ts @@ -32,6 +32,7 @@ import type { PositionBudget, TurnEngine, } from "@roleweave/shared"; +import { agentHostSupportsEmployeeMcp } from "../agent-registry.js"; import type { ControlPlaneContext } from "../context.js"; import { readJsonBody, sendJson } from "../http.js"; import { computeEnvelopeDigest } from "../turns/envelope.js"; @@ -250,6 +251,22 @@ async function hireUnlocked( body: { status: "failed", code: "hire_position_exists", message: `position already exists: ${request.positionId}`, retryable: false }, }; } + // Fail closed before any staging: while no bundled Host declares the "mcp" + // capability, a granted binding would survive hire and then kill every turn + // at spawn time, away from its cause. Gate here with a code the renderer + // maps to explicit copy; a Host that ships MCP support unlocks its own + // grants automatically (#314). + if ((request.permissions.mcpServers ?? []).length > 0 && !agentHostSupportsEmployeeMcp(request.agentEngine)) { + return { + status: 422, + body: { + status: "failed", + code: "hire_mcp_unsupported", + message: "permissions.mcpServers: no engine honors employee-level MCP bindings yet; leave MCP connectors unbound", + retryable: false, + }, + }; + } if (request.reportTo !== null && !ws.organization.roles.some((role) => role.id === request.reportTo)) { throw invalid(`reportTo position not found: ${request.reportTo}`); } diff --git a/apps/server/test/agent-registry.test.ts b/apps/server/test/agent-registry.test.ts index 327527ba..c7f6d7d9 100644 --- a/apps/server/test/agent-registry.test.ts +++ b/apps/server/test/agent-registry.test.ts @@ -2,6 +2,8 @@ import assert from "node:assert/strict"; import test from "node:test"; import { AgentHostRegistryError, + agentHostSupportsEmployeeMcp, + anyAgentHostSupportsEmployeeMcp, getAgentHostCapabilities, listRegisteredAgentHosts, selectAgentHost, @@ -290,3 +292,17 @@ test("fails closed when an explicitly supplied local probe is malformed", () => assert.equal(hosts[1]?.availability.ready, true); assert.equal(hosts[2]?.availability.ready, true); }); + +test("employee-MCP capability gate answers from the registry, not a list copy", () => { + // No bundled Host declares "mcp" today, so every per-host answer is false + // and the aggregate is false — the hire/profile gates reject on this. + for (const id of ["qoder", "claude-code", "claude-local", "codex", "codex-local", "workbuddy"]) { + assert.equal(agentHostSupportsEmployeeMcp(id), false); + } + assert.equal(anyAgentHostSupportsEmployeeMcp(), false); + + // Unknown or malformed ids fail closed rather than throwing. + assert.equal(agentHostSupportsEmployeeMcp("missing"), false); + assert.equal(agentHostSupportsEmployeeMcp(undefined), false); + assert.equal(agentHostSupportsEmployeeMcp({ id: "qoder" }), false); +}); diff --git a/apps/server/test/hire.test.ts b/apps/server/test/hire.test.ts index 62088a00..4904defb 100644 --- a/apps/server/test/hire.test.ts +++ b/apps/server/test/hire.test.ts @@ -29,10 +29,8 @@ const VALID_HIRE = { { scope: "position", resource: "./knowledge/**", actions: ["read"] }, { scope: "workspace", resource: "./reports/**", actions: ["read", "create"], approval: true }, { scope: "position", resource: "skill://issue-research", actions: ["execute"] }, - { scope: "workspace", resource: "mcp://workspace-drive", actions: ["execute"], approval: true }, ], skills: [{ id: "issue-research" }], - mcpServers: [{ id: "workspace-drive", tools: ["read"] }], }, prompt: "先阅读已批准资料，再输出带依据的文档结论。", memorySources: [ @@ -122,24 +120,24 @@ test("POST /hire: the bundled qoder-engine validates and applies a hire through assert.deepEqual(appliedRole.toolAllow, ["Read", "Grep", "Glob"], "package permissions flow into the org model"); const packageDir = path.join(dir, "positions", "repo-owner", "docs-writer"); const employee = await readJson<{ entrypoints: { mcp?: string }; policy: { mcpTools: Array<{ name: string; requestedMode: string }> }; assets: string[] }>(path.join(packageDir, "employee.json")); - assert.deepEqual(employee.policy.mcpTools, [{ name: "workspace-drive.read", requestedMode: "read" }], "MCP tool allowlist reaches the employee runtime policy"); - assert.equal(employee.entrypoints.mcp, "./mcp.json", "MCP grants require the package MCP entrypoint"); + assert.deepEqual(employee.policy.mcpTools, [], "a hire without MCP grants carries no MCP tools into the runtime policy"); + assert.equal(employee.entrypoints.mcp, undefined, "the package MCP entrypoint appears only alongside an MCP grant"); assert.ok(employee.assets.includes("./skills.json") && employee.assets.includes("./mcp.json"), "capability manifests are package assets"); - const packagePermissions = await readJson<{ model: string; defaultEffect: string; rules: unknown[] }>(path.join(packageDir, "permissions.json")); + const packagePermissions = await readJson<{ model: string; defaultEffect: string; rules: unknown[]; mcpServers: unknown[] }>(path.join(packageDir, "permissions.json")); assert.equal(packagePermissions.model, "chmod-inspired"); assert.equal(packagePermissions.defaultEffect, "deny"); - assert.equal(packagePermissions.rules.length, 4); + assert.equal(packagePermissions.rules.length, 3); assert.deepEqual((packagePermissions as { skills?: unknown[] }).skills, [{ id: "issue-research" }]); - assert.deepEqual((packagePermissions as { mcpServers?: unknown[] }).mcpServers, [{ id: "workspace-drive", tools: ["read"] }]); + assert.deepEqual(packagePermissions.mcpServers, [], "a hire without MCP grants persists an empty grant list"); assert.deepEqual(await readJson<{ skills: Array<{ id: string }> }>(path.join(packageDir, "skills.json")), { schemaVersion: "workbench-skills.v1", defaultEffect: "deny", skills: [{ id: "issue-research", name: "Issue 调研", description: "梳理 Issue / PR，输出带证据的研究结论。" }] }); - assert.deepEqual(await readJson<{ servers: Array<{ id: string; tools: string[] }> }>(path.join(packageDir, "mcp.json")), { schemaVersion: "workbench-mcp.v1", defaultEffect: "deny", servers: [{ id: "workspace-drive", name: "工作区网盘", description: "读取已接入的组织共享资料。", tools: ["read"] }] }); + assert.deepEqual(await readJson<{ servers: Array<{ id: string; tools: string[] }> }>(path.join(packageDir, "mcp.json")), { schemaVersion: "workbench-mcp.v1", defaultEffect: "deny", servers: [] }); assert.match(await fs.readFile(path.join(packageDir, "SKILL.md"), "utf8"), /先阅读已批准资料/); assert.match(await fs.readFile(path.join(packageDir, "SKILL.md"), "utf8"), /Issue 调研/); const positionResponse = await api(server.baseUrl, "/positions/docs-writer", { token: server.token }); assert.equal(positionResponse.status, 200); assert.deepEqual((positionResponse.body as { position: { capabilities: unknown } }).position.capabilities, { skills: [{ id: "issue-research", name: "Issue 调研" }], - mcpServers: [{ id: "workspace-drive", name: "工作区网盘", tools: ["read"] }], + mcpServers: [], }); const auditLines = (await fs.readFile(path.join(dir, ".digital-employee", "org-audit.jsonl"), "utf8")) .trim() @@ -372,6 +370,39 @@ test("POST /hire: static validation failure is fail-closed before any filesystem } }); +test("POST /hire: an employee-level MCP grant is refused 422 before the engine or the filesystem see it", async () => { + const driver = new FakeDriver({ status: "applied" }, emulateEngineHire); + const server = await startTestServer(driver); + const dir = await copyExampleWorkspace(); + try { + await seedAppliedState(dir); + await api(server.baseUrl, "/workspace/open", { method: "POST", token: server.token, body: { path: dir } }); + const before = await fs.readdir(path.join(dir, "positions", "repo-owner")); + const res = await api(server.baseUrl, "/hire", { + method: "POST", token: server.token, + body: { + ...VALID_HIRE, + permissions: { + ...VALID_HIRE.permissions, + rules: [...VALID_HIRE.permissions.rules, { scope: "workspace", resource: "mcp://workspace-drive", actions: ["execute"], approval: true }], + mcpServers: [{ id: "workspace-drive", tools: ["read"] }], + }, + }, + }); + assert.equal(res.status, 422); + const body = res.body as { status: string; code: string; retryable: boolean }; + assert.equal(body.status, "failed"); + assert.equal(body.code, "hire_mcp_unsupported", "#314: the grant names the capability gap instead of dying at the first turn"); + assert.equal(body.retryable, false); + assert.equal(driver.hireCalls.length, 0, "the request never reaches the engine validator"); + assert.deepEqual(driver.calls, [], "no engine adjudication follows a refused grant"); + assert.deepEqual(await fs.readdir(path.join(dir, "positions", "repo-owner")), before, "no staged skeleton survives"); + assert.equal((await readApplied(dir)).roles.some((role) => role.id === "docs-writer"), false, "the applied model is untouched"); + } finally { + await server.close(); + } +}); + test("POST /hire: engine adjudication failure rolls the staged skeleton back", async () => { const driver = new FakeDriver({ status: "failed", diff --git a/apps/server/test/position-profile.test.ts b/apps/server/test/position-profile.test.ts index 5da828b4..5f4d0bee 100644 --- a/apps/server/test/position-profile.test.ts +++ b/apps/server/test/position-profile.test.ts @@ -45,10 +45,9 @@ const PATCHED_PERMISSIONS = { { scope: "position", resource: "./knowledge/**", actions: ["read"] }, { scope: "workspace", resource: "./reports/**", actions: ["read", "create", "update"], approval: true }, { scope: "position", resource: "skill://docs-review", actions: ["execute"] }, - { scope: "workspace", resource: "mcp://issue-tracker", actions: ["execute"], approval: true }, ], skills: [{ id: "docs-review" }], - mcpServers: [{ id: "issue-tracker", tools: ["search"] }], + mcpServers: [], }; const QODER_ADAPTER = fileURLToPath(new URL("../../bin/qoder-engine.mjs", import.meta.url)); @@ -244,8 +243,8 @@ test("PATCH /positions/:id/profile: the bundled engine re-applies a renamed, re- assert.equal(employee.policy.mode, "approval_required"); assert.deepEqual(employee.policy.filesystem.read, ["./knowledge/**", "./reports/**"]); assert.deepEqual(employee.policy.filesystem.write, ["./reports/**"]); - assert.deepEqual(employee.policy.mcpTools, [{ name: "issue-tracker.search", requestedMode: "read" }]); - assert.equal(employee.entrypoints.mcp, "./mcp.json", "granting an MCP tool restores the package MCP entrypoint"); + assert.deepEqual(employee.policy.mcpTools, [], "the edit grants no MCP tools while no engine supports them (#314)"); + assert.equal(employee.entrypoints.mcp, undefined, "a package with no MCP grant must not advertise the entrypoint"); assert.ok(["./permissions.json", "./skills.json", "./mcp.json"].every((asset) => employee.assets.includes(asset))); assert.deepEqual(await readJson(path.join(packageDir, "permissions.json")), { schemaVersion: "workbench-permissions.v1", @@ -264,12 +263,12 @@ test("PATCH /positions/:id/profile: the bundled engine re-applies a renamed, re- assert.deepEqual(await readJson(path.join(packageDir, "mcp.json")), { schemaVersion: "workbench-mcp.v1", defaultEffect: "deny", - servers: [{ id: "issue-tracker", name: "Issue 跟踪", description: "搜索和读取已接入的 Issue / PR 数据。", tools: ["search"] }], + servers: [], }); const skill = await readText(path.join(packageDir, "SKILL.md")); assert.match(skill, /^# 文档工程师$/m, "the generated SKILL title follows the rename"); assert.match(skill, /### 文档审校（docs-review）/); - assert.match(skill, /- Issue 跟踪（issue-tracker）：search/); + assert.match(skill, /- 暂无 MCP 连接器/, "the generated MCP prose follows the emptied grant list"); assert.doesNotMatch(skill, /暂无附加 Skill/, "the stale capability prose is refreshed"); // 3. The position card the renderer reads reflects it without a reload race. @@ -336,27 +335,38 @@ test("PATCH /positions/:id/profile updates the workspace declaration, because th } }); -test("PATCH /positions/:id/profile: dropping every MCP grant removes the package MCP entrypoint", async (t) => { +test("PATCH /positions/:id/profile: an MCP grant is refused fail-closed while an explicit empty grant list stays allowed", async (t) => { const driver = new DigitalEmployeeCliDriver(QODER_ADAPTER_COMMAND); const server = await startTestServer(driver); const dir = await copyExampleWorkspace(); t.after(() => fs.rm(dir, { recursive: true, force: true })); try { await api(server.baseUrl, "/workspace/open", { method: "POST", token: server.token, body: { path: dir } }); - await api(server.baseUrl, "/hire", { - method: "POST", + await api(server.baseUrl, "/hire", { method: "POST", token: server.token, body: HIRE }); + const packageDir = path.join(dir, "positions", "repo-owner", "docs-writer"); + assert.equal((await readJson<{ entrypoints: { mcp?: string } }>(path.join(packageDir, "employee.json"))).entrypoints.mcp, undefined); + + // A well-formed grant still fails closed: no bundled Host declares the + // "mcp" capability, so accepting it would only move the failure to the + // employee's first turn (#314). + const employeeBefore = await readText(path.join(packageDir, "employee.json")); + const refused = await api(server.baseUrl, "/positions/docs-writer/profile", { + method: "PATCH", token: server.token, - body: { ...HIRE, permissions: { tools: ["Read"], rules: [], mcpServers: [{ id: "issue-tracker", tools: ["search"] }] } }, + body: { permissions: { tools: ["Read"], rules: [], skills: [], mcpServers: [{ id: "issue-tracker", tools: ["search"] }] } }, }); - const packageDir = path.join(dir, "positions", "repo-owner", "docs-writer"); - assert.equal((await readJson<{ entrypoints: { mcp?: string } }>(path.join(packageDir, "employee.json"))).entrypoints.mcp, "./mcp.json"); + assert.equal(refused.status, 400); + assert.equal((refused.body as { code: string }).code, "position_profile_invalid"); + assert.equal(await readText(path.join(packageDir, "employee.json")), employeeBefore, "a refused grant never touches the package"); - const response = await api(server.baseUrl, "/positions/docs-writer/profile", { + // The clearing path stays open so pre-existing packages keep a manual + // unbind workaround. + const cleared = await api(server.baseUrl, "/positions/docs-writer/profile", { method: "PATCH", token: server.token, body: { permissions: { tools: ["Read"], rules: [], skills: [], mcpServers: [] } }, }); - assert.equal(response.status, 200); + assert.equal(cleared.status, 200); const employee = await readJson<{ entrypoints: { mcp?: string }; policy: { mcpTools: unknown[] } }>(path.join(packageDir, "employee.json")); assert.equal(employee.entrypoints.mcp, undefined, "a package with no MCP grant must stop advertising the entrypoint"); assert.deepEqual(employee.policy.mcpTools, []); @@ -388,6 +398,8 @@ test("PATCH /positions/:id/profile: boundary matrix fails closed before any writ ["unregistered Skill grant", { permissions: { tools: ["Read"], rules: [], skills: [{ id: "ghost-skill" }] } }], ["unregistered MCP grant", { permissions: { tools: ["Read"], rules: [], mcpServers: [{ id: "ghost-server", tools: [] }] } }], ["MCP tool outside the catalog", { permissions: { tools: ["Read"], rules: [], mcpServers: [{ id: "issue-tracker", tools: ["delete"] }] } }], + // #314: a well-formed grant still fails closed while no Host declares "mcp". + ["MCP grant with no supporting engine", { permissions: { tools: ["Read"], rules: [], mcpServers: [{ id: "issue-tracker", tools: ["search"] }] } }], ["rule referencing an unregistered Skill", { permissions: { tools: ["Read"], rules: [{ scope: "position", resource: "skill://ghost", actions: ["execute"] }] } }], ["invalid rule scope", { permissions: { tools: ["Read"], rules: [{ scope: "galaxy", resource: "./x", actions: ["read"] }] } }], ["empty rule action list", { permissions: { tools: ["Read"], rules: [{ scope: "position", resource: "./x", actions: [] }] } }], diff --git a/packages/ui/src/locales/en.ts b/packages/ui/src/locales/en.ts index 6d921e32..8e5fe6ab 100644 --- a/packages/ui/src/locales/en.ts +++ b/packages/ui/src/locales/en.ts @@ -468,6 +468,7 @@ export const enCatalog: Record = { "hire.errOffline": "The local service is unavailable right now; nothing happened. Retryable.", "hire.errCli": "The local Agent runner is unavailable; check its configuration and retry.", "hire.errCapability": "This environment cannot create roles yet; upgrade and retry.", + "hire.errMcpCapability": "No engine supports employee-level MCP bindings yet; unbind the MCP connectors and retry.", "hire.errDenied": "Creation was rejected; nothing changed and your input was kept.", "hire.joined": "{name} joined the team", "hire.name": "Name*", diff --git a/packages/ui/src/locales/zh.ts b/packages/ui/src/locales/zh.ts index e6b7f017..5a487312 100644 --- a/packages/ui/src/locales/zh.ts +++ b/packages/ui/src/locales/zh.ts @@ -471,6 +471,7 @@ export const zhCatalog: Record = { "hire.errOffline": "本地服务当前不可用，未产生任何效果；可重试。", "hire.errCli": "本地 Agent 执行器不可用，检查配置后可重试。", "hire.errCapability": "当前运行环境暂不支持创建岗位，请升级后重试。", + "hire.errMcpCapability": "当前还没有引擎支持员工级 MCP 绑定，请取消勾选 MCP 连接器后重试。", "hire.errDenied": "创建被拒绝；没有产生任何更改，已填内容保留。", "hire.joined": "{name} 已加入团队", "hire.name": "姓名*",