feat(surface,sdk): f.mcp tools declaration + preflight-connect (#302) by kjgbot · Pull Request #313 · AgentWorkforce/flows

kjgbot · 2026-09-11T08:24:50Z

Closes #302.

Summary

Adds f.mcp.<server>.<tool>(args) for declared MCP tools. Header:

{ tools: { mcp: ['stripe', 'linear'] } }

Preflight opens each declared MCP server before the run; unreachable = mcp_unreachable.

Design decision

Per the closed-vocabulary covenant (SURFACE.md §3), the MCP verb lowers to the closed kernel agent primitive. Output receipt shape:

{ "type": "mcp", "server": "<name>", "tool": "<name>", "input": {...}, "output": {...}, "idempotencyKey": "<key>" }

No new kernel StepKind. Effect key uses standard kernel run+step lease key.

Refusals

mcp_unreachable — declared server won't handshake at preflight
mcp_undeclared_server — f.mcp.<name> referenced with <name> not in tools.mcp

Written by codex agent spec-C-f-mcp-v3 on finn-mini; head at f642da09.

Test plan

linux-x64-artifact green
packed-consumer green

🤖 Generated with Claude Code

Note

Medium Risk
Spawns MCP subprocesses and opens HTTP endpoints during preflight and execution, with journal/effect and lease/dispatch timing behavior that must stay correct; kernel is unchanged but authored-run failure modes and process cleanup are security- and reliability-sensitive.

Overview
Adds MCP tool calls to authored TypeScript flows via f.mcp.<server>.<tool>(args) when tools.mcp lists server names, with connections defined in the nearest flows.json mcp map (stdio subprocess or HTTP).

Preflight and CLI: flows check on .flow.ts now validates MCP declarations (composed with existing helper checks), connects to each declared server, caches tools/list inventory, and refuses with mcp_undeclared_server or mcp_unreachable. Project config parsing accepts the new mcp key and resolves stdio commands like other CLI paths.

Runtime: The SDK lowers each call to an existing agent effect (no kernel change): a short-lived worker opens an MCP session, records/confirms effects with the kernel attempt key, and journals an output receipt { type: "mcp", server, tool, input, output, idempotencyKey }. Failures complete as worker_error with diagnostics in trajectory_tail (e.g. mcp_unknown_tool, mcp_disconnected). Stdio uses a custom transport (empty env + allowlist, process-group cleanup); HTTP uses the official Streamable HTTP client.

Surface/types: Ctx gains mcp; dependency @modelcontextprotocol/sdk is added. Docs in SURFACE.md and local evidence in ops/reviews/20260911-spec-C-mcp.md describe the contract.

^{Reviewed by Cursor Bugbot for commit e3043b4. Bugbot is set up for automated code reviews on this repo. Configure here.}

coderabbitai · 2026-09-11T08:24:57Z

Important

🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: cc04fe80-4a28-4bb7-b01b-13b6507a0dee

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

cursor

Stale Bugbot comment from a previous run.

kjgbot · 2026-09-11T09:01:42Z

maintainability lens — FAIL

Maintainability review — PR #313

Concerns

packages/sdk/src/authored-mcp.ts:96-98 — the catch block distinguishes error types with a hardcoded string array ['mcp_unknown_tool', 'mcp_tool_error', 'mcp_result_unavailable']. Renaming any of these (or adding a fourth diagnostic) requires updating this untyped filter in lockstep. There's no shared union type, so a future maintainer adding a fifth diagnostic won't get any compiler help. Same concern with mcp-client.ts:56-63, where McpDiagnostic classification is done via nested ternaries on phase and errno.
packages/sdk/src/authored-mcp.ts:22-29 — the proxy silently makes any unknown property on f.mcp.foo a callable that later journals mcp_unknown_tool. The one-line WHY comment inside buildMcpProxy is helpful, but the invariant "reading undeclared tools is fine; calling them fails at runtime" is not documented on Ctx.mcp in packages/surface/src/context.ts:29. A stranger reading the type will not know this.
packages/sdk/src/authored-mcp.ts:44-108 — runMcpEffect interleaves five concerns (spec compile, worker attach, dispatch validation, effect performance, receipt readback) inside ~100 lines with settled!/failed! non-null assertions and three mutable closure flags (dispatchExpired, work, timer). The dispatch validator at line 66-71 collapses a mismatch of step_id, step_type, instruction, or stream pin into one generic error — future debugging of "why did my MCP call fail dispatch validation?" will be painful.
packages/sdk/src/authored-flow-executor.ts:127-128 and packages/sdk/src/cli/check-typescript.ts:26-27 duplicate the "filter tools, re-add tools.relayfile" header-support check. If a third supported subkey ships, both callsites must be updated together — nothing enforces this.
packages/sdk/src/cli/check-typescript.ts:31-38 uses a synthesized { id: 'header', type: 'deterministic', command: ':' } step just to reuse preflight, then strips its diagnostics with d.stepId !== 'header'. Any future preflight diagnostic that doesn't populate stepId will leak. The workaround is not commented.
packages/sdk/src/preflight.ts:130-141 — overloaded sync/async based on whether mcpServers is undefined is a hidden footgun. A caller who conditionally sets mcpServers gets PreflightResult | Promise<PreflightResult> and must always await.
Hardcoded timeouts: 30_000 at authored-mcp.ts:88, 10_000 at authored-mcp.ts:97 and preflight.ts:186, 1000 at mcp-stdio.ts:73. No named constants, no way to tune under test without vi.useFakeTimers().

Notes

mcp-stdio.ts:70-83 — the two-phase deadline (deadline awaited twice when maySettleOnChildExit() returns false) is subtle; the WHY comment on line 70-71 helps, but the sequence still requires careful reading.
mcp-client.ts:78-80 — pagination cycle detection throws handshake_rejected, conflating a server bug with an auth failure.
Tests in mcp.test.ts are dense (290 lines) with a shared fixture() helper that hides symlinking/JSON-writing setup — a stranger tracing a single test spends time in helpers before seeing intent.

Nothing here would prevent the change from working, but items 1-5 will make six-months-later edits risky.

REVIEW_FAILED

kjgbot · 2026-09-11T09:01:44Z

history lens — FAIL

Blocker — repeats the previously fixed unrenewed-worker pattern. In packages/sdk/src/authored-mcp.ts:80–106, the new agent worker performs the effect and submits completion without renewing or checking its lease. ops/DRIVE-LOG.md:6383–6412 records this exact lifecycle mistake; lines 7117–7144 identify commit a387317d (#247) as the landed repair, using withWorkerLease to renew ownership, abort execution on renewal failure, and reject completion after expiry.

The MCP transport’s separate deadlines (packages/sdk/src/mcp-client.ts:64–76,91–94) do not bound the entire leased operation: journal requests, initialization, tool execution, and cleanup accumulate. A delayed effect-record response followed by initialization and a tool call can cross the lease deadline without renewal. The provider can then perform an effect that the reclaimed attempt cannot confirm. This reintroduces a documented failure pattern through a new worker path; it is not a demand to complete future durability scaffolding. Apply the existing lease discipline across the MCP operation and propagate cancellation into session cleanup.

Concern — authored-body recovery remains deferred. ops/reviews/20260911-spec-C-mcp.md:29–33 explicitly discloses the existing lack of a durable authored root. That limitation is not independently blocking under this lens.

Notes. Lowering MCP calls into existing agent steps (packages/sdk/src/authored-mcp.ts:114–119) follows settled decision 13; the diff introduces no kernel primitive. The three commit messages describe their changes without claiming green CI or comprehensive test success. The evidence document explicitly labels the SDK suite “not green” (ops/reviews/20260911-spec-C-mcp.md:64–76).

This is a static history review; I did not rerun the reported tests. The rejection rests solely on the documented worker-lifecycle regression.

REVIEW_FAILED

kjgbot · 2026-09-11T09:01:45Z

structure lens — MISSING

kjgbot · 2026-09-11T09:01:46Z

🎯 review-swarm: FAILED (M:fail H:fail S:missing)

Lens transcripts posted as sibling comments above.

Session-Id: 01a08f7d-c504-7510-8651-0e2ebb694546 Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82

Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82

cursor

Stale Bugbot comment from a previous run.

The composed helper-body + trigger check should run against the same file extensions the pre-rebase helper-body check accepted (any .ts / .tsx / .mts / .mjs / .js). isAuthoredFlowPath only matches paths with the .flow. prefix, so a plain slack.mjs fixture was mis-routed to the YAML checkFlow path and refused with a spurious 'spec: expected an object' before the slack credential preflight could run. Fixes tests/authored-flow-slack.test.ts:106 refusing at the wrong layer. Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82

…hild run on deadline Two Bugbot findings on PR#313: HIGH — MCP worker skipped lease renewal. The perform-effect body ran the MCP session's callTool without wrapping in withWorkerLease, so any tool that took longer than the initial lease-deadline lost ownership and the write-back failed. Wrap the perform-effect body in withWorkerLease so the existing renewal path (see worker-lease.ts) fires while the tool is in flight. MED — Nested MCP run left indeterminate when the dispatch deadline fires. When run.start returned a run_id but no dispatch arrived in 30s, the child run was orphaned. Capture the run_id and, on dispatch-expiry, call journal.runCancel best-effort in the finally block. Cancel errors do not mask the deadline diagnostic the parent flow already sees. Adds two regression tests: - Cancel-on-deadline: run.start resolves, dispatch never fires, journal.runCancel called with the child run_id. - Lease-renewed-under-load: dispatch delivered with a 30s lease, callTool hangs 45s, stepHeartbeat fires at least twice (initial + one renewal). Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82

cursor

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit e3043b4. Configure here.}

cursor · 2026-09-11T10:46:35Z

+    const starting = journal.runStart(spec);
+    await Promise.race([starting, completed]);
+    const outcome = await starting;
+    childRunId = outcome.run_id;


Cancel misses late child run

Medium Severity

childRunId is assigned only after Promise.race against completed succeeds, so a dispatch-deadline rejection skips the new runCancel even when unbounded run.start later returns a run. run.start uses a null timeout, so a slow daemon can still create the child after the parent has already failed and closed its worker.

Additional Locations (1)

packages/sdk/src/authored-mcp.ts#L155-L157

^{Reviewed by Cursor Bugbot for commit e3043b4. Configure here.}

cursor Bot reviewed Sep 11, 2026

View reviewed changes

Comment thread packages/sdk/src/authored-mcp.ts

Comment thread packages/sdk/src/mcp-stdio.ts

miyaontherelay and others added 4 commits September 11, 2026 11:55

feat(surface,sdk): add MCP tools and preflight connections

bb2de85

Session-Id: 01a08f7d-c504-7510-8651-0e2ebb694546 Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82

docs: record approved MCP receipt representation

0a4d4ca

Session-Id: 01a08f7d-c504-7510-8651-0e2ebb694546 Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82

fix(sdk): bound MCP dispatch and terminate server process groups

4c85d45

Session-Id: 01a08f7d-c504-7510-8651-0e2ebb694546 Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82

chore(sdk): add @modelcontextprotocol/sdk dep after rebase

cdf85c1

Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82

kjgbot force-pushed the feat/spec-C-f-mcp branch from 619e07f to cdf85c1 Compare September 11, 2026 09:57

cursor Bot reviewed Sep 11, 2026

View reviewed changes

Comment thread packages/sdk/src/authored-mcp.ts

Comment thread packages/sdk/src/authored-mcp.ts

kjgbot added 2 commits September 11, 2026 12:20

cursor Bot reviewed Sep 11, 2026

View reviewed changes

kjgbot merged commit 333768a into main Sep 11, 2026
6 of 7 checks passed

kjgbot deleted the feat/spec-C-f-mcp branch September 11, 2026 11:05

Navigation Menu

feat(surface,sdk): f.mcp tools declaration + preflight-connect (#302) - #313

feat(surface,sdk): f.mcp tools declaration + preflight-connect (#302)#313
kjgbot merged 6 commits into
mainfrom
feat/spec-C-f-mcp

kjgbot commented Sep 11, 2026 •

edited by cursor Bot

Loading

Uh oh!

coderabbitai Bot commented Sep 11, 2026 •

edited

Loading

Uh oh!

cursor Bot left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

kjgbot commented Sep 11, 2026

Uh oh!

kjgbot commented Sep 11, 2026

Uh oh!

kjgbot commented Sep 11, 2026

Uh oh!

kjgbot commented Sep 11, 2026

Uh oh!

cursor Bot left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

cursor Bot left a comment

Uh oh!

cursor Bot Sep 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

kjgbot commented Sep 11, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Design decision

Refusals

Test plan

Uh oh!

coderabbitai Bot commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kjgbot commented Sep 11, 2026

maintainability lens — FAIL

Maintainability review — PR #313

Uh oh!

kjgbot commented Sep 11, 2026

history lens — FAIL

Uh oh!

kjgbot commented Sep 11, 2026

structure lens — MISSING

Uh oh!

kjgbot commented Sep 11, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot Sep 11, 2026

Choose a reason for hiding this comment

Cancel misses late child run

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kjgbot commented Sep 11, 2026 •

edited by cursor Bot

Loading

coderabbitai Bot commented Sep 11, 2026 •

edited

Loading

cursor Bot left a comment •

edited

Loading

cursor Bot left a comment •

edited

Loading