Skip to content

[FEATURE] S2 — Consolidate MCP admission onto the shared core and enforce it at approval #177

Description

@Joncallim

Issue Type

Feature — child slice S2 of EPIC #172. Depends on #176 (S1). Canonical architecture: docs/adr/0009-mcp-admission-contract.md.

Problem Statement

The four admission paths (see #176) currently disagree, which is the exact #172 defect: an approved task can hit a handoff block that was already computable at approval time.

  • Approval never runs the handoff broker. web/app/api/tasks/[id]/approve/route.ts gates only on requiresFilesystemGrantApproval; it never calls evaluateWorkPackageMcpBroker. A package the broker will block at handoff for a non-filesystem reason (required GitHub MCP unhealthy, an unsafe GitHub write, an MCP-aware subtask capability not covered by an approved grant) passes approval and only blocks at handoff (web/worker/work-package-handoff.ts:1084-1136). Root defect.
  • Run-scoped prompt-only context is detected two incompatible ways: designHasRunScopedMcpInstructionsForAgentRequirement (mcp-execution-design.ts:785-801, preview) vs metadataHasRunScopedMcpInstructionsForMcp (:488-496, broker).
  • Requirement field sets differ: grant gate reads permissions+capabilities+requiredCapabilities+mcpCapabilities; broker reads only capabilities+permissions; UI reads a third set. A requirement expressed via requiredCapabilities blocks on the server but shows "no grant needed".
  • filesystem.project.write has three verdicts (warn / dropped / unsafe).

Desired Outcome

validateMcpExecutionDesign, deriveMcpGrantDecisions, evaluateWorkPackageMcpBroker, and requiresFilesystemGrantApproval all become thin adapters over the S1 admission core, and approval enforces exactly what handoff enforces. Preview and handoff are structurally incapable of disagreeing.

User Story

As a Forge operator, I want a plan that approves to actually hand off, and any block to be visible and identical at approval and handoff time, so I never approve a task that then silently stalls.

Requirements

Migrate per the ADR 0009 consolidation map:

  • validateMcpExecutionDesign (mcp-execution-design.ts:326-454): build admitMcpRequirement per (requirement, agent); run the subtask-coverage loop against coverageKeysForGrant; return decisionsToValidation(decisions). Delete local unsafeCapability/decisionStatus branches.
  • deriveMcpGrantDecisions (:803-867): call admitMcpRequirement per (requirement, agent); map via decisionsToGrantPreview. Delete decisionStatus (:753-783). Keep decisionId/sourceRequirementIndex/promptOverlayPresent that the materializer (workforce-materializer.ts:157-170) and UI depend on.
  • evaluateWorkPackageMcpBroker (:599-739): build entries via brokerEntries; call admitMcpRequirement using mergeCapabilityFields(entry) for requestedCapabilities and metadataHasRunScopedMcpInstructionsForMcp for hasPromptOnlyContext; run the subtask-coverage loop with shared coverage keys; return decisionsToBrokerCheck(...). Delete the local copies normalizeCapability(:537), coverageCapabilityKey(:541), filesystemProjectAlias(:545), approvedCoverageCapabilityKeys(:551), filesystemUnqualifiedAlias(:561), prohibitedCoverageCapabilityKeys(:567), unsafeCapability(:582), capabilityArray(:498), healthyStatus(:297), canProceedWithoutMcp(:207), isPlanningOnlyFilesystemWrite(:318), SAFE_BETA_CAPABILITY_PATTERNS(:7) — import from S1 modules.
  • filesystem-grants.ts: import normalizeCapability, classifyCapability, mergeCapabilityFields, isMcpHealthy/mcpHealthReason, canProceedWithoutMcp, REQUIREMENT_CAPABILITY_FIELDS from S1; delete private copies (:34-45,77-84,101,137-156). Rebuild canonicalFilesystemProjectCapability on classifyCapability so filesystem.project.write is classified planning_only consistently (decide explicitly whether summarizeFilesystemCapabilities surfaces it as requested-but-non-blocking rather than silently dropping it). Keep the FilesystemProjectCapability nominal type and ProjectFilesystemGrant persistence.
  • work-package-executor.ts:1252-1260 (mcpCapabilityList): import mergeCapabilityFields.
  • execution-design-metadata.ts:99-232: keep reading identical grantDecisions/validation JSON; extend to carry mode/recoveryAction/normalizedCapabilities/evidenceRefs (back-derive mode for old artifacts).

Enforce admission at approval: in web/app/api/tasks/[id]/approve/route.ts, after the requiresFilesystemGrantApproval check (~:189-209), run the admission decision over every package and refuse approval (mirroring the existing missingFilesystemGrant early return) when any decision is status:'blocked', returning the normalized reason + recoveryAction.

Replace retryability heuristics: derive retryable from recoveryAction === 'install_or_fix_mcp' at failWorkPackageForMcpBroker (work-package-handoff.ts:737) instead of isRetryableMcpBrokerBlock regex-matching (mcp-execution-design.ts:90-93); persist recoveryAction into the block metadata.

Acceptance Criteria

  • All four functions delegate to the S1 admission core; every duplicated helper listed above is deleted and imported from S1.
  • web/app/api/tasks/[id]/approve/route.ts refuses approval when any package admission decision is blocked, with the normalized reason + recoveryAction (filesystem and non-filesystem alike).
  • Invariant test (web/__tests__/mcp-admission-invariant.test.ts): for a fixed package, deriveMcpGrantDecisions, evaluateWorkPackageMcpBroker, and requiresFilesystemGrantApproval produce the same mode/status for: required no-capability grants, prompt-only context, filesystem read/list/search, filesystem.project.write, an unsafe/deferred GitHub write, an unknown MCP, and a requirement expressed via each of the four capability fields.
  • Retryability is derived from recoveryAction, not string matching. All prior safety behavior preserved (unsafe-capability blocking, prohibited-capability global removal, no live MCP handles).
  • npm run lint, npx tsc --noEmit, npm test, npm run build pass.

Dependencies

Implementation Scope

Large — architecture change across worker, lib, and approval route (behavior-preserving migration + approval enforcement).

Metadata

Metadata

Assignees

No one assigned

    Labels

    P0Priority 0: urgent bug or vulnerabilityenhancementNew feature or requestready-for-agent

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions