feat(mcp): implement bounded form input_required continuation

[me2seeks]

Summary

Modern MCP tools can request a Host-owned form and continue the original logical invocation with typed answers. The manager now handles bounded input_required rounds and advertises form capability only when a modern call supplies an interaction handler. This is the protocol adapter layer; Desktop/TUI provider composition follows in #5068.

The existing SDK call path retains parameter-header mirroring and final output validation. Every retry preserves original arguments, uses the current round's responses/state, and rechecks the original binding. Stop or binding retirement interrupts pending forms; unchanged tool refresh remains valid. Raw form keys and values are checked before display projection against the connection's known continuation states. A bounded inventory retains up to 64 states / 256 KiB without eviction. Pending interactive calls withhold stderr; learning a new state clears the already-formatted diagnostic cache. Control or non-ASCII states withhold stderr until reconnect. Exhaustion rejects new calls and aborts concurrent calls until reconnect. Results and errors use the retained inventory, including errors raised before SDK invocation.

Strings without a server maxLength receive a visible local limit of 256 characters; explicit server bounds and existing aggregate form/answer limits remain enforced. Legacy and handler-less calls retain their behavior.

Fixes #5065
Refs #4364

Verification

• Latest test-only follow-up e87fbd906 requires pending ordinary and interactive calls to reject within 500 ms while the server is still blocked. The ordinary-signal mutation passed the old test and fails the revised test; restored code passes all 250 MCP tests. Separate subagent review passed. Latest-head CI and dependency audit passed on e87fbd906. The latest automated follow-up review gives GO on this exact head and confirms the test gap is closed.
• Test-only follow-up 3c3b0b54f pins privacy exhaustion while an uncooperative form callback is already open. Removing the privacy failure signal makes the new regression fail; restored production code passes. Separate subagent review passed. Follow-up CI passed on 3c3b0b54f; its dependency audit passed.
• Reproduced both P1 review findings with real modern HTTP / stdio servers before fixing them. Added raw control-character and redaction-marker cases, cross-call reflection, pre-response / post-completion stderr, partial and continued lines, split UTF-8, bounded retention, concurrent exhaustion, reconnect, truncated cached diagnostics and early error paths.
• Production-fix main CI and dependency audit both passed on 3bec8388f.
• Native MCP build and complete MCP suite: 250/250 passed.
• Full workspace / Desktop build, root typecheck, lint, format, ASF headers and diff checks passed.
• npm run check:release: 196/196 passed.
• Rebased onto main a41d3ac4a, which contains the js-yaml advisory fix. Shipped dependency audit: 0 moderate-or-higher advisories.
• Independent subagents reviewed the privacy design and final code. Actual Desktop/TUI visual smoke was not performed.

AI use

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

Tool(s) and scope: OpenAI Codex prepared and reviewed the plan, implemented the adapter and tests, 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]

Technical review: NO-GO on this head. The bounded continuation loop and its binding/cancellation tests are otherwise coherent, but two reachable paths still expose opaque requestState through user-visible diagnostics or form copy.

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.

[jackwener]

Reviewed 3bec8388f361e6a396562246b2cd8a2b2001e4ff. Both P1s from the previous review are genuinely closed. One P3 inline. audit and test are terminal green on this head, and audit has gone from red to green since the earlier review.

I am not signing this yet, for one reason that is about coverage rather than about anything I found: this head adds roughly 550 lines of new privacy machinery, and a single review line over that much new security-relevant code is thin. A second line is running specifically on the new machinery (retention bounds and the exhaustion path as a possible denial-of-service lever, generation guards across reconnect, and the residual window outside interactive calls). I will convert this to a decision once it reports. Everything below is mine and stands on its own.

I verified the two fixes by mutation rather than by reading them. For each, I removed the fix from the compiled output and confirmed the new tests fail:

1. Removing the pre-projection raw check (containsMcpFormState(requests, …) before prepareMcpForm) fails 7 tests, including state "opaque\nstate" echoed into a form message fails before projection — the exact scenario from the earlier finding — plus the [redacted] literal in message, property key and default, which is the redacted-output-equality bypass that finding warned about, plus a later call cannot project earlier connection state ….
2. Narrowing the check from the connection inventory to the current call (privacy ? [...privacy.states] : states → states) fails the two cross-invocation tests. So checking against every retained connection state is load-bearing and separately pinned, not incidental.
3. Removing the pending-call stderr suppression (stderrSuppressed || pendingCalls > 0 → stderrSuppressed) fails stdio before cannot expose continuation state before response or after settlement — which names both windows the earlier finding described.

Baseline before mutating: 63 tests green across the three form files, with individual test names visible rather than a file-level count.

Three suspicions of mine that did not hold, recorded so nobody re-investigates them:

• The round limit looked deleted — if (++rounds > 8) disappears from its old position in the fix commit's diff. It was relocated, not removed; it now sits after state retention and the stderr cache clear.
• inventory.withhold.push('') on the exhaustion path looked like it might be inert, since containsMcpFormState deliberately skips empty states with state.length > 0. It is not: scrubKnownMcpSecrets withholds on message.includes(secret), and includes('') is always true, so the empty token really does withhold every diagnostic. The fail-closed works.
• pendingCalls looked like it could leak on an exception path and suppress a connection's stderr forever. It cannot: the decrement is in a finally guarded by the same if (controller) as the increment, and discardPendingStderr() runs before the decrement, so buffered lines are discarded rather than released when suppression lifts. That ordering is right and worth keeping.

Scope and limits. I focused on whether the two P1s are closed and did not re-walk the ground the earlier review covered. Local verification used a core + mcp build in a worktree; core reports pre-existing errors for a missing generated model-metadata module, which does not affect these tests. I did not run the Desktop, CLI or Runtime Host suites, and did not exercise a real terminal or Desktop window.

简体中文

上一轮的两条 P1 已确实闭合,行内留一条 P3。audit 与 test 在此 head 终态为绿,且 audit 已由红转绿。

我暂不签署,原因只关乎覆盖面,与我发现的任何问题无关:此 head 新增约 550 行隐私机制,这么多新的安全相关代码只有一条评审线偏薄。另一条线正在专门审查这套新机制(保留上界与耗尽路径是否可被当作拒绝服务杠杆、重连竞态下的代际守卫、以及非交互调用期间的残留窗口)。它回报后我再转为结论。以下内容是我这条线的,独立成立。

两条修复我是用变异检验核的,不是读过去的。 逐条把修复从编译产物中移除,确认新测试会红:

1. 移除投影前的原始检查 → 7 项失败,含 state "opaque\nstate" echoed into a form message fails before projection(上一轮报告的原样场景),以及 [redacted] 字面量出现在 message / 属性键 / 默认值三处(正是那条 finding 警告的"脱敏后等值比较"绕过),再加跨调用反射两项。
2. 把检查范围从连接清单收窄为本次调用 → 两项跨调用测试失败。说明"对照连接级全部保留状态"是承重且被单独钉住的。
3. 移除挂起调用期间的 stderr 扣留 → stdio before cannot expose continuation state before response or after settlement 失败 —— 用例名恰好点出上一轮描述的两个窗口。

变异前基线:三个 form 测试文件 63 项全绿,且输出中可见每个具体用例名而非文件级计数。

三条不成立的怀疑,记下以免重复调查:

• 轮次上限看似被删(if (++rounds > 8) 在修复提交的 diff 中从原位置消失)。实际是移位,现位于状态保留与 stderr 清缓存之后。
• 耗尽路径上的 inventory.withhold.push('') 看似可能是空操作,因为 containsMcpFormState 特意用 state.length > 0 跳过空串。并非如此:scrubKnownMcpSecrets 以 message.includes(secret) 判定扣留,而 includes('') 恒为 true,空令牌确实会扣留一切诊断,fail-closed 有效。
• pendingCalls 看似可能在异常路径上泄漏、导致该连接 stderr 永久被扣留。不会:递减位于 finally 中,并与递增共用 if (controller) 守卫;且 discardPendingStderr() 在递减之前执行,因此缓冲行是被丢弃而非在解除扣留时放行 —— 这个顺序是对的,值得保留。

本次审查的边界:我聚焦于两条 P1 是否闭合,未重走上一轮已覆盖的面。本地验证在 worktree 中构建了 core 与 mcp;core 报告的是缺失生成式 model-metadata 模块的既有错误,不影响这些测试。未运行 Desktop、CLI 或 Runtime Host 套件,未在真实终端或 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]

Technical result: GO on 3c3b0b54f378f67f0612f8e2cb116baced4656a9, with no P0-P2 findings. The two earlier privacy defects are closed. I found one P3 test gap.

[P3] The ordinary-call exhaustion test does not prove prompt cancellation. The production call signal correctly includes the connection privacy-failure signal for both interactive and ordinary calls. However, removing that signal only from the ordinary-call branch still leaves retention exhaustion fences an already pending ordinary call green. The test releases its blocked server response before awaiting the pending rejection, so the call eventually returns from the server, observes the already-exhausted connection, and fails for the expected text even though it was never cancelled promptly. A regression could therefore leave a real ordinary request waiting until its normal timeout. The minimal fix is to assert that the pending ordinary call rejects within a short bound before releasing the fixture, as the open-form invalidation test now does.

I independently checked the security-relevant boundaries rather than relying on the issue description. Retained states are connection-scoped with no eviction; after 64 unique states or 256 KiB the manager fails closed, aborts concurrent calls, clears and permanently suppresses that generation's stderr, and rejects later calls before sending them. Reconnect constructs a new privacy identity, while old stderr callbacks check object identity and cannot append into the replacement generation. During an interactive call all stderr is withheld, and after settlement ordinary calls scrub against every retained state. The 64-state cutoff is a deliberate availability tradeoff: it can require a reconnect after sustained form use, but does not create a broader denial-of-service capability for a server that already controls whether its own calls settle.

The final commit is test-only and closes the prior open-form exhaustion P3. A clean Core/MCP rebuild and all three affected form suites passed, 64/64 tests. The exact-head hosted test and dependency audit checks are terminal green. A synthetic merge with current main 09c73430cc7ff0d106d617cdb4a93015d49deb5b is clean at tree 44a38e89e75b3e22c0a5395c8f43fa37c1d061e6.

---

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.

[me2seeks]

Addressed the P3 in the latest review with test-only commit e87fbd9067cad9ebbc81f7d9430c35cd16c001ed.

Confirmed the blind spot experimentally: removing the privacy-failure signal only from the ordinary-call branch leaves the previous test green. The test now requires rejection within 500 ms before releasing the blocked server response, for both ordinary and interactive calls. Repeating the same mutation now fails with pending request was not promptly cancelled; restoring production code passes. Cleanup still releases the fixture in finally.

Native MCP build, Biome, diff check and the complete MCP suite passed (250/250). A separate subagent reviewed the final change and found no issue. No production code changed. New-head CI is tracked separately from the previous green head.

Automated response by OpenAI Codex; not an independent human review or approval.

[jackwener]

Follow-up review: GO on e87fbd9067cad9ebbc81f7d9430c35cd16c001ed, with no P0-P2 findings. The previous test gap is closed.

The new regression now requires the already-pending request to reject within 500 ms while the mock server is still blocked. Releasing the server response happens only in finally, so a later post-response fence can no longer make the test pass by accident. I verified this by removing the privacy-failure signal from the ordinary-call branch: the revised test failed with pending request was not promptly cancelled; restoring the production signal made the affected form suites pass, 64/64.

This follow-up changes only the test. The reviewed production privacy and cancellation paths are unchanged. Exact-head hosted test and audit checks are terminal green. A synthetic merge with current main 09c73430cc7ff0d106d617cdb4a93015d49deb5b is clean at tree d89b6385f4e43305d5f1781f3aefdeea2756de90, and the merged patch is byte-identical to the reviewed PR patch.

---

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 e87fbd9067cad9ebbc81f7d9430c35cd16c001ed. I found no unresolved P0-P2 issue. The exact-head test and audit checks are terminal green, and the follow-up cancellation regression closes the remaining test gap with mutation-backed evidence.

---

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.