feat(mcp): connect Desktop and TUI to Host form continuation

[me2seeks]

Summary

Desktop and TUI MCP tools now forward an admitted provider's form interaction callback into the MCP continuation adapter. A server's input_required can therefore create the existing Host-owned form, receive the user's typed answer and complete the original tool invocation.

Desktop injects requestUserForm into native tool execution; TUI forwards requestInteraction to the manager. Existing surfaces, persistence and IPC contracts remain the owners of rendering and answers.

Fixes #5066
Refs #4364

Stack

Depends on #5067. This PR targets feat/4364-mcp-form-adapter so its diff contains only provider composition and shared Host acceptance tests. Rebase onto main after the parent lands. Since PR CI only triggers for main-targeting PRs, the final stacked commit also has a manually dispatched CI run.

Verification

• Current head f7b36ec83 rebases onto parent e87fbd906, whose latest change only strengthens a cancellation test. The six-file incremental patch remains unchanged (range-diff equals); new-head CI passed on f7b36ec83. Earlier validation below remains tied to its stated heads.
• Synced the parent's test-only review follow-up: head d11e0d4f8, parent 3c3b0b54f. The six-file incremental patch is unchanged (range-diff equals). Full CI on the previous head ebe95c338 passed; the new-head run also passed on d11e0d4f8.
• After updating to parent 3bec8388f, 53/53 focused integration and adjacent lifecycle/provider tests passed on ebe95c338. The 14 new cases run real modern HTTP MCP servers through both production providers, Client Capability channel/broker, ToolRuntime, Host coordinator and SQLite InteractionStore.
• Covered typed accept, invalid answers, decline, cancel, a second form round, Stop, MCP disconnect and provider loss. Pending forms close without extra retries; a user wait longer than the network timeout succeeds with real broker timers paused.
• Clean isolated lockfile install with repository dependency patches, full workspace / Desktop build, root lint/format/typecheck and diff checks passed on the updated stack.
• Independent subagent re-reviewed the exact updated parent/head. The six-file incremental patch is unchanged (matching stable patch-id), and parent privacy exhaustion still withdraws pending forms through the existing Host lifecycle. Tests use an in-memory Host transport; no actual Desktop window or TUI terminal smoke was performed. Existing surface/action regression tests passed.

AI use

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: OpenAI Codex implemented provider composition, built the shared test fixture, reviewed lifecycle behavior and ran validation. The contributor authorized submission; AI review is not human approval.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[hqhq1025]

Reviewed exact head c9e72e09bff2b083246e88d6563e5035588bcba6 against its exact stacked parent b33116d8a267a1e47136fa879de2574baebf3697.

This six-file layer connects the already-admitted Host interaction callback to both production provider paths: Desktop injects it into the native tool execution context, while the CLI/TUI MCP provider forwards it into McpClientManager.callTool. I found no new P0-P3 issue in this layer. The callback remains bound to the existing Client Capability invocation and only becomes usable after admission; Stop, MCP disconnect, provider loss, duplicate/invalid answers, multiple form rounds, and the suspended execution deadline all converge through the existing Host-owned interaction lifecycle.

Validation on this head: clean install and dependency patches, build:test, full typecheck/lint/format, ASF headers, diff check, MCP 227/227, Runtime 3332 passed / 13 skipped, CLI 914 passed / 3 skipped, Desktop 2368/2368, and 50 focused provider/channel/broker/ToolRuntime form tests. Runtime Host was 1818 passed / 12 skipped / 1 failed; the sole failure is the unchanged managed-Bash sandbox integration because this runner rejects both unshare and bwrap. The manually dispatched CI run 34311649516 is green and bound to this exact head. The stack also forms a clean merge tree against current main a41d3ac4a76d8c5ac5154032acb9dbb123e847a6.

This is a technical GO for the incremental layer only. PR #5068 cannot merge independently: parent PR #5067 at b33116d8a267a1e47136fa879de2574baebf3697 remains blocked by two P1 privacy findings, and any parent-head change requires this layer to be re-reviewed. I did not run a real Desktop-window or terminal UI smoke, nor native Windows/macOS execution.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[me2seeks]

Updated this stack after addressing both parent P1 findings. Current head is ebe95c338746ee73aca67c4117664f56cf03840e, with parent 3bec8388f361e6a396562246b2cd8a2b2001e4ff.

An independent subagent re-reviewed this exact pair. The six-file incremental patch is unchanged from the reviewed pair (range-diff equals; stable patch-id ea72799394f48fde657dcd66ad331b3a3baa91c8). No additional P0–P3 finding was identified. Parent privacy exhaustion still terminates the provider invocation, cancels the interaction producer and withdraws the canonical Host form.

Fresh isolated install with repository dependency patches, full workspace/Desktop build, root typecheck/lint/format and 53 focused integration/provider/lifecycle tests passed. Full updated-head CI has been dispatched: https://github.com/apache/maka/actions/runs/34322470803 . The previous green run is not evidence for this new head. Actual Desktop-window/TUI visual smoke was not performed.

Automated follow-up by OpenAI Codex; this is not independent human approval. No merge was performed.

[jackwener]

APPROVE, bound to ebe95c338746ee73aca67c4117664f56cf03840e, for this incremental layer only. Hosted test is terminal green on this exact head. No P0-P3 from my line.

This head exists because the parent moved, and the previous review made itself conditional on exactly that. @hqhq1025 reviewed c9e72e09b against parent b33116d8a and wrote that any parent-head change requires this layer to be re-reviewed. The parent has since moved to 3bec8388f, which closed two P1 privacy findings and added roughly 200 lines of new machinery. So the condition fired, and this is that re-review.

The layer's own content did not change. I compared each PR's diff against its own parent - b33116d8a...c9e72e09b versus 3bec8388f...ebe95c338 - and the two patch sets are byte-identical across all six files. This head is a pure rebase. Matching line counts alone would not have established that, so I hashed the patch bodies rather than trusting +144/-0 appearing twice. The earlier GO therefore transfers at the content level, and what actually needed re-checking is the interaction with the parent's new behaviour.

What the new parent adds that this layer must survive. Two failure modes that did not exist when this layer was first reviewed: a terminal form privacy retention exhausted error thrown from callTool, and a new manager-initiated abort source (privacy.failure) folded into the call signal, which can fire while a Host form is open. The earlier review's argument that "Stop, MCP disconnect, provider loss, duplicate/invalid answers, multiple form rounds and the suspended execution deadline all converge through the existing Host-owned interaction lifecycle" was made against a parent that had neither.

They converge correctly. The parent passes the composite signal into requestInteraction as cancellationSignal and races it in waitForMcpForm, so an exhaustion abort settles an open form rather than orphaning it. I confirmed this by driving two concurrent calls on one connection - one holding a form open, the other exhausting retention - and both rejected with form privacy retention exhausted; the open form was cancelled, not left hanging. Through this layer that surfaces as an ordinary failed tool call on both paths.

On the two forwarding sites themselves. The production surface is five lines: requestUserForm: options.requestInteraction into the Desktop native tool execution context, and requestInteraction: options.requestInteraction into manager.callTool from the CLI provider. The CLI path accepts admission before the call, so a parent-side terminal error arrives as a tool failure on an already-accepted capability, which is the ordinary shape. Nothing here needs to distinguish a caller abort from a manager abort.

One thing I did not report here. The exhaustion-while-a-form-is-open case has no regression pinning it. That belongs to the parent, where the parameterised invalidation family lives, so I filed it there as a P3 against #5067 rather than duplicating it on this layer.

Merge ordering is not a review question, but it is a fact worth restating: this PR targets feat/4364-mcp-form-adapter and cannot merge before #5067. This approval covers the incremental layer against parent 3bec8388f; if the parent moves again, this binding is stale by the same rule that produced this review.

Scope and limits. Byte-level patch comparison across the two stacked heads, static review of the five production lines and the two integration tests, and a local probe against a core + mcp build for the cross-call abort behaviour described above. I did not run the Desktop, CLI or Runtime Host suites on this head - hosted test covers those - and I did not exercise a real Desktop window or terminal.

简体中文

绑定 ebe95c338746ee73aca67c4117664f56cf03840e,仅针对这一增量层批准。托管 test 在此 exact head 终态为绿,我这条线无 P0–P3。

这个 head 之所以存在,是因为父层动了,而上一轮评审正是把自己的结论条件化在这件事上的。 @hqhq1025 在 c9e72e09b 上对着父 b33116d8a 审查,并写明「父 head 一旦变化,本层必须重审」。此后父层移到 3bec8388f,闭合了两条 P1 并新增约 200 行机制。条件已触发,这就是那次重审。

本层自身内容没有变化。 我分别取两个 PR 相对各自父的补丁(b33116d8a...c9e72e09b 与 3bec8388f...ebe95c338),六个文件逐字节相同 —— 这是一次纯 rebase。仅凭行数一致不足以下这个结论,所以我对补丁正文做了哈希,而不是看到两次 +144/-0 就采信。因此上一轮的 GO 在内容层面可以承接,真正需要重核的是它与父层新行为的交互。

新父带来、本层必须承受的两种失败模式:callTool 抛出的终态错误 form privacy retention exhausted,以及并入调用信号的管理器侧新中止源 privacy.failure —— 后者可在 Host 表单打开期间触发。上一轮那句「Stop、MCP 断连、provider 丢失、重复/非法答案、多轮表单与挂起执行截止都汇聚到既有 Host 交互生命周期」,是对着一个两者皆无的父层说的。

它们汇聚得是对的。父层把复合信号作为 cancellationSignal 传入 requestInteraction,并在 waitForMcpForm 中与之竞速,因此耗尽中止会结算已打开的表单而非遗弃它。我用同一连接上的两个并发调用做了验证 —— 一个开着表单、另一个触发耗尽,两者均以 form privacy retention exhausted 拒绝,已打开的表单被取消而不是悬挂。经由本层,它表现为一次普通的工具调用失败。

两处转发点本身:生产面只有五行。CLI 路径在调用前已接受准入,因此父层的终态错误表现为「已接受的能力上发生一次工具失败」,是常规形态;此处无需区分调用方中止与管理器中止。

有一条我没有报在这里:「表单打开期间发生耗尽」没有回归测试钉住。它属于父层(参数化作废测试族在那里),所以我作为 P3 提在了 #5067,而不是在本层重复报告。

合并顺序不是评审问题,但值得重申:本 PR 的目标分支是 feat/4364-mcp-form-adapter,不能先于 #5067 合入。本次批准覆盖的是相对父 3bec8388f 的增量层;父层若再变动,本绑定即按产生本次评审的同一条规则失效。

本次审查的边界:两个栈 head 的字节级补丁比对、五行生产代码与两个集成测试的静态审查,以及针对上述跨调用中止行为、基于 core + mcp 本地构建的一次探针。未在此 head 上运行 Desktop、CLI 或 Runtime Host 套件(由托管 test 覆盖),也未在真实 Desktop 窗口或终端中验证。

Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.

[jackwener]

Re-binding my earlier approval to d11e0d4f8212256c561559a91295c4011c4ef2f3. Hosted test is terminal green on this head. This adds no new findings; it exists because the binding moved without my involvement and I would rather the record be explicit than rely on that.

What happened. I approved this PR at ebe95c338, and the body of that review says so. After the rebase onto parent 3c3b0b54f, the API now reports that same review with commit_id = d11e0d4f8 — the new head, which I had not looked at when I wrote it. My COMMENTED review on the parent PR was not moved, and neither was @hqhq1025's earlier COMMENTED review here, so this appears to be approvals being carried forward across a force-push that leaves the diff unchanged, rather than anything general. I am stating that as an observation with its correlate, not as a claim about GitHub's internals.

It matters for anyone gating on exact heads: an approval's commit_id can name a SHA the reviewer never examined. In this instance it is harmless, because the substance really does transfer — but that is something I checked rather than something the field guarantees.

What I verified for this head, rather than assuming it.

• This layer is still byte-identical. I compared each PR's patch against its own parent — 3bec8388f...ebe95c338 versus 3c3b0b54f...d11e0d4f8 — and hashed the patch bodies. Both produce 99c119d672bd41fa, the same value I got before the rebase. Matching file counts would not have shown this; the hash does.
• The parent's delta is test-only. 3bec8388f → 3c3b0b54f is a single commit touching one file, packages/mcp/src/__tests__/form-manager.test.ts (+26/-4). No production code changed, so every conclusion in my previous review — including that a manager-initiated exhaustion abort settles an open Host form instead of orphaning it — carries without re-derivation.

Everything else from the earlier approval stands unchanged, including that this PR targets feat/4364-mcp-form-adapter and cannot merge before #5067. If the parent moves again in a way that touches production code, this binding is stale by the same rule.

简体中文

将先前的批准重新绑定到 d11e0d4f8212256c561559a91295c4011c4ef2f3,该 head 上托管 test 终态为绿。本次不含任何新发现;之所以发布,是因为绑定在我不知情的情况下发生了移动,与其依赖该行为,不如把记录写明确。

发生了什么:我在 ebe95c338 上批准,评审正文也写的是这个 SHA。在 rebase 到父 3c3b0b54f 之后,API 现在把同一条评审报告为 commit_id = d11e0d4f8 —— 一个我撰写时并未看过的新 head。我在父 PR 上的 COMMENTED 评审没有被移动,@hqhq1025 先前在本 PR 的 COMMENTED 评审也没有。因此这看起来是「diff 未变的 force-push 不会作废批准,批准被顺延」,而非普遍行为。我把它作为一项带相关证据的观察陈述,而不是对 GitHub 内部机制的断言。

这对任何按 exact head 设门禁的人都有影响:一条批准的 commit_id 可能指向评审者从未检查过的 SHA。 本例无害,因为其实质确实可以承接 —— 但这是我核过的结论,而不是该字段本身所能保证的。

针对本 head 我实际验证(而非假定)的内容:

• 这一层仍然逐字节相同。 我分别取两个 PR 相对各自父的补丁(3bec8388f...ebe95c338 与 3c3b0b54f...d11e0d4f8)并对正文哈希,两次都得到 99c119d672bd41fa,与 rebase 之前的值一致。仅凭文件数或行数一致无法说明这一点,哈希可以。
• 父层的增量是纯测试。 3bec8388f → 3c3b0b54f 只有一个提交、只触及 packages/mcp/src/__tests__/form-manager.test.ts(+26/-4),生产代码零改动。因此我上一条评审中的全部结论(包括「管理器侧的耗尽中止会结算已打开的 Host 表单而非遗弃它」)无需重新推导即可承接。

先前批准中的其余内容一律不变,包括本 PR 目标分支为 feat/4364-mcp-form-adapter、不能先于 #5067 合入。父层若再次发生触及生产代码的变动,本绑定按同一条规则失效。

Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.

[jackwener]

APPROVE on f7b36ec83ea4efe973423415a3721cc67f86981d for this incremental layer. I found no unresolved P0-P2 issue. The layer remains byte-identical after the parent test-only update, and the exact-head full CI run completed successfully.

This PR still depends on #5067 and must not merge before it.

---

Review notice: This review was prepared by an automated review agent operated by jackwener and is published at the direction of me2seeks, who has read these findings and is the human accountable for them.

[me2seeks]

Rebased the implementation onto current origin/main (8c02f3d32) after confirming the Windows report maps to this existing child issue rather than a new headless-session feature. Current head: 163a897f8df1e139f9b51a872889878e8fdddf1e.

The production delta remains the two required provider composition changes: Desktop passes options.requestInteraction as requestUserForm, and CLI passes it into McpClientManager.callTool. The six-file integration layer is otherwise unchanged. One test fixture was updated for the current Desktop provider interface (releaseDesktopInteractionSession).

Fresh isolated npm ci, dependency patches, full workspace/Desktop build, root typecheck/lint/format, ASF headers, diff check, and 54 focused real-server/provider/Host/lifecycle tests passed. GitHub CI run 34370888036 is now queued for this exact head. Actual Windows Desktop form rendering still requires a post-merge smoke test.

Automated update by OpenAI Codex; not an independent human review or approval. No merge was performed.