fix(hire): reject employee-level MCP grants no bundled engine can honor

[waterbro-8]

Tracking record

Refs #314.

Picker/UI acceptance (disable or annotate CapabilityPicker while no bundled host supports employee-level MCP) is owned by follow-up PR #320. This PR is the server-half only and must not auto-close the issue.

Summary

The hire flow and the profile editor both accept employee-level MCP grants, but no bundled engine honors them: every adapter spawns with --strict-mcp-config --mcp-config '{"mcpServers":{}}', so a granted binding survives creation and then fails every turn at spawn time (qoder.mcp_binding_unsupported), far away from its cause. #314 records the full diagnosis.

This PR is the server-half of option A: reject at the two server surfaces that accept a grant, with copy that names the limitation:

• POST /hire returns 422 hire_mcp_unsupported before staging, before the engine validator, and before any filesystem effect.
• PATCH /positions/:id/profile rejects a well-formed grant with 400 position_profile_invalid, while an explicit empty grant list (mcpServers: []) stays allowed so pre-existing packages keep a manual unbind workaround.

Both gates answer from the agent registry (agentHostSupportsEmployeeMcp / anyAgentHostSupportsEmployeeMcp) instead of a hardcoded list, so a future host that declares the "mcp" capability unlocks its own grants with no further edits here. The renderer maps the new failure code to explicit zh/en copy in the hire drawer.

Intentionally out of scope: disabling or annotating the CapabilityPicker's MCP section at bind time (the AC's UI half). The renderer has no registry data source today; shipping one is a renderer-surface change that deserves its own PR. Until then, a grant is refused at submit time with copy that says what to do (unbind the connectors and retry).

Acceptance criteria

Acceptance criterion	Status	Implementation / evidence
Hiring with a non-empty mcpServers grant is rejected at hire time with copy that names the limitation (A)	PASS (server half)	routes/hire.ts guard returns 422 hire_mcp_unsupported before staging; HireDrawer.tsx maps the code to zh/en copy naming the limitation; regression test in apps/server/test/hire.test.ts
The CapabilityPicker MCP section is rendered disabled (A) or annotated (B) while no host declares MCP support	NOT PART OF THIS PR	Renderer-side; needs a registry data source in the renderer. Tracked as the remaining half of option A — proposed follow-up
An employee hired without MCP grants behaves exactly as today	PASS	Default-path assertions in hire.test.ts / position-profile.test.ts now assert the no-grant package shape (empty mcpTools, no entrypoints.mcp, empty mcp.json servers) and pass unchanged in meaning
When a future host declares MCP support, the gate no longer triggers for that host without further edits	PASS	Gates read agentHostSupportsEmployeeMcp / anyAgentHostSupportsEmployeeMcp, which answer from HOST_DEFINITIONS[...].capabilities; regression test agent-registry.test.ts asserts the registry-derived answers

Validation ledger

Command or check	Expected	Actual	Evidence
V1: npm run build	Compiles clean	PASS	exit 0; Node 22.22.2 / TypeScript 5.9.3, Windows x64
V2: npm run typecheck:ui	No type errors	PASS	exit 0
V3: npm run typecheck:renderer	No type errors	PASS	exit 0
V4: node --test hire.test.js position-profile.test.js agent-registry.test.js (dist)	All pass	PASS	31 tests / 31 pass / 0 fail
V5: full server suite node --test "apps/server/dist/test/*.test.js"	Failure set == Windows environment baseline (4)	PASS	504 tests / 441 pass / 4 fail; the 4 failures are the pre-existing environment ones (qoder-engine codex run, review-group-persistence atomic-write 500, turn-driver trust probe, workbuddy-health-driver node CSPRNG crash) in files this PR does not touch; main 418bb6c4 CI is fully green (10/10 checks), so these are local-environment failures, not regressions
V6: npm run test:renderer	All pass; i18n key parity holds	PASS	53 test files / 523 tests passed, including i18n.test.tsx with the new hire.errMcpCapability key present in both locales
V7: tree diff against base	Exactly the 9 declared paths	PASS	GET /git/trees recursive compares of the published commit tree: 9 modified, 0 added, 0 removed — first against base 418bb6c4, then again after rebasing onto eb01db29 (two commits landed on main mid-flight; they touch http.ts / server.ts / CHANGELOG / docs / one unrelated test, zero overlap with the 9 paths here)

Environment note: validated on Windows x64. The suite has a small set of pre-existing Windows-environment baseline failures (missing real engine CLIs, POSIX-signal tests, macOS-helper tests); V5 compares against that baseline rather than demanding a fully green run, and main's own CI on the base commit is fully green.

Tests and coverage

• Tests added or changed:
  • apps/server/test/hire.test.ts: the fixture hire no longer carries an MCP grant; the bundled-engine gate test now asserts the no-grant package shape (empty mcpTools, no MCP entrypoint, empty grant list, empty mcp.json, position-card capabilities.mcpServers: []); new fail-closed test — an MCP grant is refused 422 hire_mcp_unsupported with zero engine validator calls, zero apply calls, and no staged skeleton.
  • apps/server/test/position-profile.test.ts: the re-grant fixture drops the MCP grant; the bundled-engine end-to-end test asserts the no-grant package shape; the old "dropping every MCP grant" test is rewritten as "an MCP grant is refused fail-closed while an explicit empty grant list stays allowed" (refused patch leaves the package byte-identical; the clearing patch still returns 200); boundary matrix gains a "MCP grant with no supporting engine" case (well-formed grant, 400 position_profile_invalid).
  • apps/server/test/agent-registry.test.ts: new test — the employee-MCP capability gate answers from the registry, not a list copy (all six hosts report false, anyAgentHostSupportsEmployeeMcp() is false, unknown ids fail closed).
• Coverage before / after, when measured: not measured (repo does not run coverage in CI for these suites).
• Intentionally uncovered behavior and reason: the renderer-side disabled/annotated picker (follow-up); packaged-build smoke (requires electron-builder packaging, covered by release CI).

Change classification

• User-visible behavior
• Internal refactor or maintenance
• Documentation only
• Build, CI, dependency, or repository configuration
• Breaking change
• Security-sensitive change

Risk and rollback

• Risk level and affected components: low. The gates only trigger for requests carrying a non-empty mcpServers grant; the default hire path (no MCP) and every existing package are untouched. The clear-grant profile patch keeps working.
• Compatibility, migration, privacy, performance, or operational impact: a hire request that previously "succeeded" but produced an employee that failed every turn now fails fast at submit time with an explicit code and copy. Workspaces that already contain packages with MCP bindings (created before this change) keep loading; operators can still clear the grants via the profile editor.
• Rollback procedure: revert the single commit; no data or schema changes.

Breaking or security notes

Behavior change, not an API break: one previously-accepted (but guaranteed-to-fail-at-runtime) request shape is now refused at submit time with 422 hire_mcp_unsupported / 400 position_profile_invalid. No security impact. No migration needed.

Author checklist

• A maintainer confirmed that the linked issue or tracking record was ready before implementation began, or the automation was pre-authorized under GOVERNANCE.md.
• This branch was created from an up-to-date default branch and contains no unrelated changes.
• I ran the repository's applicable tests, lint, type checks, builds, coverage, and security checks.
• I added a regression test for a bug fix, or explained why one is impractical.
• I updated relevant documentation and changelog files.
• I reviewed the diff for secrets, personal data, generated artifacts, and dependency risk.
• The PR is ready for CI and review by someone other than the sole author.
• All reported results are reproducible.

Note on the first item: the linked issue #314 was filed by the same author immediately before this implementation (diagnosis first, then the fix), so no maintainer pre-confirmation exists; the issue stands at status: reported awaiting triage, and this PR is the implementation it describes. The changelog item is left for the maintainer's release cut, consistent with recent merged PRs.

Reviewer notes

• The core judgment call is in agent-registry.ts: the gates read the registry rather than a hardcoded engine list, so they are self-disarming once a host declares "mcp". Please scrutinize agentHostSupportsEmployeeMcp's fail-closed behavior for unknown host ids.
• position-profile.test.ts carries the largest test adaptation: the old grant-flow assertions (grant → package entrypoint restored) now describe a request shape the API refuses, so they are rewritten as refusal + clearing-path assertions. If you'd rather keep the grant-flow coverage behind a registry override, that is a reasonable ask — the grant pipeline itself (derivePermissionArtifacts) is intentionally left intact.
• Suggested independent reproduction: npm run build && node --test apps/server/dist/test/hire.test.js apps/server/dist/test/position-profile.test.js apps/server/dist/test/agent-registry.test.js.

[PeterGuy326]

Request changes on current head 1640e3e.

The server-side fail-closed gate and registry-derived capability check are technically sound, and the hosted CI is green. The remaining blocker is scope/acceptance: this PR says Closes #314, but the issue acceptance criteria also require the CapabilityPicker MCP section to be disabled or persistently annotated while no bundled host supports employee-level MCP. The current diff only adds submit-time failure copy in HireDrawer; it explicitly leaves the picker unchanged, so users can still select and bind a grant before receiving the error.

Please either (a) add the chosen Option-A picker gate/warning using the registry-backed capability state, with focused renderer coverage, or (b) split/retarget this as the server-half implementation, remove Closes #314, and link a follow-up that owns the picker acceptance criterion. After that, update the branch onto current main before requesting re-review; it is currently BEHIND main.

[waterbro-8]

PeterGuy326 的 CHANGES_REQUESTED 按 option (b) 处理：

• 已去掉 Closes #314，改为 Refs #314。本 PR 只覆盖 server-half（hire 422 / profile 400 + HireDrawer 提交失败文案）。
• CapabilityPicker 禁用/标注 AC 跟 fix(hire): warn when MCP grants cannot run on bundled engines #320（https://github.com/bytefolk/roleweave/pull/320）。
• 分支已 update onto current main（原先 BEHIND）。等这轮 CI 绿了再请 re-review。

[xiaocui-big]

平台组 Review（崔泽生，assigned by @冯浩然）

总体评价

定位精准、实现扎实的 server-half 修复。hire 和 profile-edit 两个入口同时加 fail-closed MCP grant 拒绝逻辑，把"创建成功但每个 turn 静默失败"的缺陷前移到提交时刻。9 文件 +131/-24 diff 干净，无夹带。CI 11/11 全绿。

结论：LGTM。server 侧逻辑可以 approve。

1. MCP 拒绝逻辑——正确

• hire 路径：guard 在 hire_position_exists 之后、staging / engine validator 之前。422 + hire_mcp_unsupported。测试断言零 engine 调用、零 filesystem 副作用 ✅
• profile-edit 路径：assertProfilePatch 内、文件写入前拒绝。400 + position_profile_invalid。空数组 mcpServers: [] 允许通过，保留 unbind workaround ✅
• agent-registry 抽象：agentHostSupportsEmployeeMcp / anyAgentHostSupportsEmployeeMcp 从 HOST_DEFINITIONS[...].capabilities 读取，未知 id fail-closed ✅

2. 一个设计不对称（非 blocker，建议记 note）

hire gate 用 agentHostSupportsEmployeeMcp(request.agentEngine) 做 per-host 检查，profile-edit gate 用 anyAgentHostSupportsEmployeeMcp() 做 全局 检查。当某个 Host 率先声明 "mcp" capability 后，通过该 Host hire 的员工可在 profile editor 改 MCP grant，但其他 Host hire 的员工会被拒绝——因为 profile editor 不区分员工绑定的 engine。

当前（无 Host 声明 MCP）不是 bug，只是保守行为。建议：在 anyAgentHostSupportsEmployeeMcp() 的 JSDoc 加一行 note 标记这个已知限制，方便后续 PR 定位。

3. 错误文案——清晰可操作

Key	文案	评价
en	"No engine supports employee-level MCP bindings yet; unbind the MCP connectors and retry."	清晰、可操作
zh	"当前还没有引擎支持员工级 MCP 绑定，请取消勾选 MCP 连接器后重试。"	自然流畅，"取消勾选"精确对应 CapabilityPicker 交互

4. 测试覆盖——充分

• 拒绝路径（hire 422 + profile-edit 400）✅
• 默认路径无回归 ✅
• 清除路径（unbind workaround）保留 ✅
• 引擎未触达 + 文件系统未触达 ✅
• Registry 抽象层 fail-closed ✅

5. 安全分析——无 bypass 路径

• 直接 API 调用：guard 在 HTTP 必经路径上
• 伪造 agentEngine：assertHireRequest 在 MCP guard 之前已验证
• 已有带 MCP grant 的 package：不受影响，guard 只在 create/modify 时触发

6. 合并阻塞

PR 当前 blocked，需要 PeterGuy326 re-review approve。从 review 内容看，两个要求（拆分 scope / rebase）都已完成，approve 应该是形式上的。

建议（非 blocker）

1. anyAgentHostSupportsEmployeeMcp() JSDoc 补充 per-host vs 全局检查的已知限制 note
2. hire 拒绝测试中增加对 error message 内容的断言（目前只断言了 code 和 retryable）

[waterbro-8]

Follow-up to PeterGuy326 CHANGES_REQUESTED (option b):

The previous head still had Closes #314 in the commit message, so GitHub kept closingIssuesReferences: #314 even after the PR body said Refs #314 / Does not close #314.

New head 031ea7d rewrites that commit: Refs #314 only. CapabilityPicker disable / persistent warning stays on follow-up #320.

Branch is on current main (ecc0294). CI is re-running. Please re-review when green; I cannot self-approve.

[waterbro-8]

Correction on the previous note: GitHub still treated Does not close #314 as a close keyword. Head is now bce8f35 (commit + PR body only Refs #314). closingIssuesReferences is empty. CI re-running.

[waterbro-8]

Rebased onto current main abb8bcc (#330). Head f870756. Still Refs #314 only (no close keyword). Cannot self-approve.

[PeterGuy326]

Re-review approve on head f870756.

This takes the option (b) path from my earlier review on 1640e3e: the PR is now the server-half only — hire returns 422 hire_mcp_unsupported fail-closed before staging or engine adjudication, and the profile editor refuses new grants while keeping the explicit empty-grant unbind path. Capability answers come from the registry (agentHostSupportsEmployeeMcp / anyAgentHostSupportsEmployeeMcp), not a copied list. The Closes keyword is gone from body and commit (closingIssuesReferences is empty, #314 stays open), and the picker acceptance criterion is owned by follow-up #320. Unknown and malformed host ids fail closed rather than throwing, and the boundary matrix plus the new hire/profile tests cover the refusal paths. Branch is current with main and hosted CI is green.

[waterbro-8]

Node 24 / ubuntu-latest failed on renderer/test/service-connections.test.tsx (unrelated to this hire MCP gate; this PR does not touch that file). macos-14 on the same head was green. Treating it as a flake: reran the failed verify job only, no new commit, so the existing APPROVE on fbaf30b is preserved.

Cannot self-merge. Still Refs #314 only; #314 stays open.