You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by "me2seeks (via GitHub)" <gi...@apache.org> on 2026/09/09 04:30:19 UTC
[PR] feat(mcp): implement bounded form input_required continuation [maka]
me2seeks opened a new pull request, #5067:
URL: https://github.com/apache/maka/pull/5067
## 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 the dependent PR.
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. Known active continuation state is scrubbed from forms, results, errors and stderr.
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
- Native MCP build and complete MCP suite: **227/227 passed**, including real modern-only HTTP and stdio form round trips.
- Tests cover schemas, typed answers, multiple rounds, capability gating, stale bindings, cancellation, timeouts, privacy, output validation and parameter headers.
- Workspace dependency builds and full Desktop build passed; root typecheck, lint, format check, Desktop/UI Knip and diff checks passed.
- Independent subagent reviewed the implementation plan and code; lifecycle and stderr findings were fixed.
- No renderer changes in this layer; actual Desktop/TUI visual smoke was not performed.
## AI use
- [ ] No generative tool made a substantive contribution
- [x] 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
- [x] Tests cover the change and fail without it
- [x] Lint, format, typecheck and the affected suites pass locally
Does this PR entail a change in behavior?
- [x] Yes — described under Summary above
- [ ] No
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(mcp): implement bounded form input_required continuation [maka]
Posted by "jackwener (via GitHub)" <gi...@apache.org>.
jackwener commented on code in PR #5067:
URL: https://github.com/apache/maka/pull/5067#discussion_r3966207648
##########
packages/mcp/src/__tests__/form-manager.test.ts:
##########
@@ -0,0 +1,677 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import assert from 'node:assert/strict';
+import { afterEach, test } from 'node:test';
+import { setTimeout as delay } from 'node:timers/promises';
+import { CLIENT_CAPABILITIES_META_KEY, ProtocolError } from '@modelcontextprotocol/client';
+import { inputRequired } from '@modelcontextprotocol/server';
+import { MCP_CONFIG_VERSION } from '@maka/core/mcp';
+import type { InteractionFormResult } from '@maka/core/interaction';
+import { McpClientManager } from '../index.js';
+import { createMcpFormFixture, mcpFixtureFormRequest } from '../__fixtures__/form-server.js';
+
+const resources: Array<() => Promise<void>> = [];
+afterEach(async () => {
+ for (const close of resources.splice(0).reverse()) await close();
+});
+
+const accepted: InteractionFormResult = {
+ action: 'accept',
+ values: { name: 'Ada', email: 'ada@example.com', confirm: true },
+};
+
+async function setup(legacy = false) {
+ const fixture = await createMcpFormFixture({ legacy });
+ resources.push(() => fixture.close());
+ const manager = new McpClientManager({ timeouts: { callToolMs: 1_000 } });
+ resources.push(() => manager.close());
+ await manager.sync({
+ version: MCP_CONFIG_VERSION,
+ mcpServers: {
+ forms: {
+ transport: 'streamable-http',
+ url: fixture.url,
+ protocol: legacy ? 'legacy' : 'auto',
+ },
+ },
+ });
+ const binding = manager.toolSnapshot().tools[0]!.binding;
+ return { fixture, manager, binding };
+}
+
+function deferred<T>() {
+ let resolve!: (value: T) => void;
+ let reject!: (reason: unknown) => void;
+ const promise = new Promise<T>((yes, no) => {
+ resolve = yes;
+ reject = no;
+ });
+ return { promise, resolve, reject };
+}
+
+test('real modern form round trip preserves original arguments, IDs, state and per-call capability', async () => {
+ const { fixture, manager, binding } = await setup();
+ const args = { source: { value: 'original' } };
+ const result = await manager.callTool(binding, args, {
+ requestInteraction: async (form) => {
+ assert.deepEqual(form.requester, { name: 'ask_user', source: 'forms' });
+ assert.deepEqual(
+ form.fields.map((field) => field.name),
+ ['name', 'email', 'confirm'],
+ );
+ assert.equal(JSON.stringify(form).includes('opaque-state'), false);
+ args.source.value = 'changed';
+ return accepted;
+ },
+ });
+ assert.equal(result.content[0]?.type, 'text');
+ assert.equal(fixture.calls.length, 2);
+ assert.notEqual(fixture.calls[0]!.id, fixture.calls[1]!.id);
+ for (const call of fixture.calls) {
+ assert.deepEqual(call.params.arguments, { source: { value: 'original' } });
+ assert.deepEqual(call.params._meta?.[CLIENT_CAPABILITIES_META_KEY], {
+ elicitation: { form: {} },
+ });
+ }
+ assert.equal(fixture.calls[1]!.params.requestState, 'opaque-state');
+ assert.deepEqual(fixture.calls[1]!.params.inputResponses, {
+ form: { action: 'accept', content: accepted.values },
+ });
+});
+
+for (const action of ['decline', 'cancel'] as const) {
+ test(`explicit ${action} is a protocol response, not global Stop`, async () => {
+ const { fixture, manager, binding } = await setup();
+ await manager.callTool(binding, {}, { requestInteraction: async () => ({ action }) });
+ assert.deepEqual(fixture.calls[1]!.params.inputResponses, { form: { action } });
+ });
+}
+
+test('handler-less call does not advertise elicitation and does not retry', async () => {
+ const { fixture, manager, binding } = await setup();
+ await assert.rejects(manager.callTool(binding, {}), /server rejected/);
+ assert.equal(fixture.calls.length, 1);
+ assert.deepEqual(fixture.calls[0]!.params._meta?.[CLIENT_CAPABILITIES_META_KEY], {});
+});
+
+test('multiple rounds replace old responses and preserve exact opaque state', async () => {
+ const { fixture, manager, binding } = await setup();
+ const first = ' round-one \n opaque\u0000 ';
+ const second = 'round-two-state';
+ fixture.respond = () =>
+ fixture.calls.length === 1
+ ? inputRequired({
+ inputRequests: { z: mcpFixtureFormRequest(), a: mcpFixtureFormRequest() },
+ requestState: first,
+ })
+ : fixture.calls.length === 2
+ ? inputRequired({ inputRequests: { next: mcpFixtureFormRequest() }, requestState: second })
+ : { content: [] };
+ let forms = 0;
+ await manager.callTool(
+ binding,
+ {},
+ {
+ requestInteraction: async () => {
+ forms++;
+ return accepted;
+ },
+ },
+ );
+ assert.equal(forms, 3);
+ assert.equal(fixture.calls.length, 3);
+ assert.equal(fixture.calls[1]!.params.requestState, first);
+ assert.deepEqual(Object.keys(fixture.calls[1]!.params.inputResponses!), ['a', 'z']);
+ assert.equal(fixture.calls[2]!.params.requestState, second);
+ assert.deepEqual(Object.keys(fixture.calls[2]!.params.inputResponses!), ['next']);
+ assert.equal(new Set(fixture.calls.map(({ id }) => id)).size, 3);
+});
+
+test('state-only rounds are bounded and retain an empty state exactly', async () => {
+ const { fixture, manager, binding } = await setup();
+ fixture.respond = () => inputRequired({ requestState: '' });
+ await assert.rejects(
+ manager.callTool(
+ binding,
+ {},
+ {
+ requestInteraction: async () => assert.fail('unexpected form'),
+ },
+ ),
+ /round limit/,
+ );
+ assert.equal(fixture.calls.length, 9);
+ assert.equal(fixture.calls[1]!.params.requestState, '');
+ assert.equal(Object.hasOwn(fixture.calls[1]!.params, 'inputResponses'), false);
+});
+
+test('invalid sibling fails preflight without displaying the valid first form', async () => {
+ const { fixture, manager, binding } = await setup();
+ fixture.respond = () =>
+ inputRequired({
+ inputRequests: {
+ first: mcpFixtureFormRequest(),
+ second: {
+ ...mcpFixtureFormRequest(),
+ params: { ...mcpFixtureFormRequest().params, message: 'm'.repeat(2_049) },
+ },
+ },
+ requestState: 'opaque',
+ });
+ await assert.rejects(
+ manager.callTool(
+ binding,
+ {},
+ {
+ requestInteraction: async () => assert.fail('preflight was not atomic'),
+ },
+ ),
+ );
+ assert.equal(fixture.calls.length, 1);
+});
+
+test('invalid accepted answer never reaches server', async () => {
+ const { fixture, manager, binding } = await setup();
+ await assert.rejects(
+ manager.callTool(
+ binding,
+ {},
+ {
+ requestInteraction: async () => ({
+ action: 'accept',
+ values: { name: 'Ada', email: 'invalid', confirm: true },
+ }),
+ },
+ ),
+ );
+ assert.equal(fixture.calls.length, 1);
+});
+
+for (const invalidate of ['stop', 'disconnect', 'reconnect', 'close', 'changed refresh'] as const) {
Review Comment:
**[P3] Privacy exhaustion is a sixth way an open form gets invalidated, and it is the only one missing from the parameterised family directly above.**
This loop enumerates the causes that must abort a pending callback even when the callback ignores cancellation: `stop`, `disconnect`, `reconnect`, `close`, `changed refresh`. This head introduces a seventh abort source into the same composite signal - `exhaustConnectionDiagnosticPrivacy()` calls `privacy.failure.abort(...)`, and `privacy.failure.signal` is folded into every form-capable call on that connection - but the family was not extended to cover it.
It is reachable across concurrent calls rather than within one. Inside a single call, exhaustion always fires before that round's form is prepared, so nothing is open. Across two calls on one connection it is a different story, and **the behaviour today is correct** - I checked rather than assumed:
```
A 的表单已打开,现在发起 B
callB 拒绝: McpToolCallError: form privacy retention exhausted (state limit); reconnect required
callA 拒绝: McpToolCallError: form privacy retention exhausted (state limit); reconnect required
=> A 被中止
```
That probe is this file's own `setup()` shape: call A returns a small state and opens a form that never settles; call B returns a state above the 16 KiB limit, which exhausts retention; A is aborted rather than left hanging. So this is not a defect report - the composite signal already does the right thing, and an open Host form does get cancelled.
The reason to raise it is that #5068 makes these forms real user-visible Host forms, and this is the one invalidation cause whose correctness currently rests on signal composition alone with no test pinning it. Everything else in that list has a regression; a future refactor that builds the call signal differently would silently leave a user staring at a form for a call that has already failed.
**Minimal fix:** add `'exhaust'` to the array and a branch that drives the second call into the state-size limit. The rest of the test body - the `assert.rejects(call, /aborted|stale/)` race and the `fixture.calls.length` check - needs no change, though the expected message would be `form privacy retention exhausted` rather than aborted/stale.
> **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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(mcp): implement bounded form input_required continuation [maka]
Posted by "me2seeks (via GitHub)" <gi...@apache.org>.
me2seeks commented on code in PR #5067:
URL: https://github.com/apache/maka/pull/5067#discussion_r3966242405
##########
packages/mcp/src/__tests__/form-manager.test.ts:
##########
@@ -0,0 +1,677 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import assert from 'node:assert/strict';
+import { afterEach, test } from 'node:test';
+import { setTimeout as delay } from 'node:timers/promises';
+import { CLIENT_CAPABILITIES_META_KEY, ProtocolError } from '@modelcontextprotocol/client';
+import { inputRequired } from '@modelcontextprotocol/server';
+import { MCP_CONFIG_VERSION } from '@maka/core/mcp';
+import type { InteractionFormResult } from '@maka/core/interaction';
+import { McpClientManager } from '../index.js';
+import { createMcpFormFixture, mcpFixtureFormRequest } from '../__fixtures__/form-server.js';
+
+const resources: Array<() => Promise<void>> = [];
+afterEach(async () => {
+ for (const close of resources.splice(0).reverse()) await close();
+});
+
+const accepted: InteractionFormResult = {
+ action: 'accept',
+ values: { name: 'Ada', email: 'ada@example.com', confirm: true },
+};
+
+async function setup(legacy = false) {
+ const fixture = await createMcpFormFixture({ legacy });
+ resources.push(() => fixture.close());
+ const manager = new McpClientManager({ timeouts: { callToolMs: 1_000 } });
+ resources.push(() => manager.close());
+ await manager.sync({
+ version: MCP_CONFIG_VERSION,
+ mcpServers: {
+ forms: {
+ transport: 'streamable-http',
+ url: fixture.url,
+ protocol: legacy ? 'legacy' : 'auto',
+ },
+ },
+ });
+ const binding = manager.toolSnapshot().tools[0]!.binding;
+ return { fixture, manager, binding };
+}
+
+function deferred<T>() {
+ let resolve!: (value: T) => void;
+ let reject!: (reason: unknown) => void;
+ const promise = new Promise<T>((yes, no) => {
+ resolve = yes;
+ reject = no;
+ });
+ return { promise, resolve, reject };
+}
+
+test('real modern form round trip preserves original arguments, IDs, state and per-call capability', async () => {
+ const { fixture, manager, binding } = await setup();
+ const args = { source: { value: 'original' } };
+ const result = await manager.callTool(binding, args, {
+ requestInteraction: async (form) => {
+ assert.deepEqual(form.requester, { name: 'ask_user', source: 'forms' });
+ assert.deepEqual(
+ form.fields.map((field) => field.name),
+ ['name', 'email', 'confirm'],
+ );
+ assert.equal(JSON.stringify(form).includes('opaque-state'), false);
+ args.source.value = 'changed';
+ return accepted;
+ },
+ });
+ assert.equal(result.content[0]?.type, 'text');
+ assert.equal(fixture.calls.length, 2);
+ assert.notEqual(fixture.calls[0]!.id, fixture.calls[1]!.id);
+ for (const call of fixture.calls) {
+ assert.deepEqual(call.params.arguments, { source: { value: 'original' } });
+ assert.deepEqual(call.params._meta?.[CLIENT_CAPABILITIES_META_KEY], {
+ elicitation: { form: {} },
+ });
+ }
+ assert.equal(fixture.calls[1]!.params.requestState, 'opaque-state');
+ assert.deepEqual(fixture.calls[1]!.params.inputResponses, {
+ form: { action: 'accept', content: accepted.values },
+ });
+});
+
+for (const action of ['decline', 'cancel'] as const) {
+ test(`explicit ${action} is a protocol response, not global Stop`, async () => {
+ const { fixture, manager, binding } = await setup();
+ await manager.callTool(binding, {}, { requestInteraction: async () => ({ action }) });
+ assert.deepEqual(fixture.calls[1]!.params.inputResponses, { form: { action } });
+ });
+}
+
+test('handler-less call does not advertise elicitation and does not retry', async () => {
+ const { fixture, manager, binding } = await setup();
+ await assert.rejects(manager.callTool(binding, {}), /server rejected/);
+ assert.equal(fixture.calls.length, 1);
+ assert.deepEqual(fixture.calls[0]!.params._meta?.[CLIENT_CAPABILITIES_META_KEY], {});
+});
+
+test('multiple rounds replace old responses and preserve exact opaque state', async () => {
+ const { fixture, manager, binding } = await setup();
+ const first = ' round-one \n opaque\u0000 ';
+ const second = 'round-two-state';
+ fixture.respond = () =>
+ fixture.calls.length === 1
+ ? inputRequired({
+ inputRequests: { z: mcpFixtureFormRequest(), a: mcpFixtureFormRequest() },
+ requestState: first,
+ })
+ : fixture.calls.length === 2
+ ? inputRequired({ inputRequests: { next: mcpFixtureFormRequest() }, requestState: second })
+ : { content: [] };
+ let forms = 0;
+ await manager.callTool(
+ binding,
+ {},
+ {
+ requestInteraction: async () => {
+ forms++;
+ return accepted;
+ },
+ },
+ );
+ assert.equal(forms, 3);
+ assert.equal(fixture.calls.length, 3);
+ assert.equal(fixture.calls[1]!.params.requestState, first);
+ assert.deepEqual(Object.keys(fixture.calls[1]!.params.inputResponses!), ['a', 'z']);
+ assert.equal(fixture.calls[2]!.params.requestState, second);
+ assert.deepEqual(Object.keys(fixture.calls[2]!.params.inputResponses!), ['next']);
+ assert.equal(new Set(fixture.calls.map(({ id }) => id)).size, 3);
+});
+
+test('state-only rounds are bounded and retain an empty state exactly', async () => {
+ const { fixture, manager, binding } = await setup();
+ fixture.respond = () => inputRequired({ requestState: '' });
+ await assert.rejects(
+ manager.callTool(
+ binding,
+ {},
+ {
+ requestInteraction: async () => assert.fail('unexpected form'),
+ },
+ ),
+ /round limit/,
+ );
+ assert.equal(fixture.calls.length, 9);
+ assert.equal(fixture.calls[1]!.params.requestState, '');
+ assert.equal(Object.hasOwn(fixture.calls[1]!.params, 'inputResponses'), false);
+});
+
+test('invalid sibling fails preflight without displaying the valid first form', async () => {
+ const { fixture, manager, binding } = await setup();
+ fixture.respond = () =>
+ inputRequired({
+ inputRequests: {
+ first: mcpFixtureFormRequest(),
+ second: {
+ ...mcpFixtureFormRequest(),
+ params: { ...mcpFixtureFormRequest().params, message: 'm'.repeat(2_049) },
+ },
+ },
+ requestState: 'opaque',
+ });
+ await assert.rejects(
+ manager.callTool(
+ binding,
+ {},
+ {
+ requestInteraction: async () => assert.fail('preflight was not atomic'),
+ },
+ ),
+ );
+ assert.equal(fixture.calls.length, 1);
+});
+
+test('invalid accepted answer never reaches server', async () => {
+ const { fixture, manager, binding } = await setup();
+ await assert.rejects(
+ manager.callTool(
+ binding,
+ {},
+ {
+ requestInteraction: async () => ({
+ action: 'accept',
+ values: { name: 'Ada', email: 'invalid', confirm: true },
+ }),
+ },
+ ),
+ );
+ assert.equal(fixture.calls.length, 1);
+});
+
+for (const invalidate of ['stop', 'disconnect', 'reconnect', 'close', 'changed refresh'] as const) {
Review Comment:
Confirmed the coverage gap and added the `exhaust` case in 3c3b0b54f378f67f0612f8e2cb116baced4656a9. The first call opens an uncooperative form callback; a concurrent call exceeds the 16 KiB state limit. Both calls must reject, and a late answer must leave the wire count at two (the original call plus the exhausting call), with no retry.
Mutation verified: removing `privacy.failure.signal` from the interactive composite signal makes this exact test fail with `pending invocation did not abort`; restoring it passes. Native MCP build, Biome, diff check and the full MCP suite passed (250/250). A separate subagent reviewed the final test and found no issue. Production code is unchanged.
Automated response by OpenAI Codex; not an independent human review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(mcp): implement bounded form input_required continuation [maka]
Posted by "me2seeks (via GitHub)" <gi...@apache.org>.
me2seeks commented on PR #5067:
URL: https://github.com/apache/maka/pull/5067#issuecomment-5602880554
Addressed the P3 in [the latest review](https://github.com/apache/maka/pull/5067#pullrequestreview-5154957046) 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(mcp): implement bounded form input_required continuation [maka]
Posted by "me2seeks (via GitHub)" <gi...@apache.org>.
me2seeks commented on code in PR #5067:
URL: https://github.com/apache/maka/pull/5067#discussion_r3965616966
##########
packages/mcp/src/index.ts:
##########
@@ -800,94 +829,191 @@ export class McpClientManager {
cause: preparation.cause,
});
}
- let result;
+ const requestInteraction =
+ entry.status.negotiatedProtocol?.era === 'modern' ? options.requestInteraction : undefined;
+ const controller = requestInteraction ? new AbortController() : undefined;
+ const signal = controller
+ ? options.signal
+ ? AbortSignal.any([controller.signal, options.signal])
+ : controller.signal
+ : options.signal;
+ const states: string[] = [];
+ if (controller) this.formCalls.set(controller, { binding, serverId, states });
+ const assertCurrent = () => {
+ signal?.throwIfAborted();
+ if (
+ this.closed ||
+ entry.closing ||
+ this.connections.get(serverId) !== entry ||
+ entry.client !== client ||
+ entry.status.state !== 'connected' ||
+ entry.connectionGeneration !== identity.connectionGeneration ||
+ entry.toolSnapshot.get(toolName)?.binding !== binding
+ )
+ throw new McpToolCallError(serverId, toolName, 'tool binding is stale');
+ };
try {
- result = await client.callTool(
- { name: toolName, arguments: args },
- {
- signal: options.signal,
- timeout: options.timeoutMs ?? this.timeouts.callToolMs,
- toolDefinition: structuredClone(preparation.value.definitionForSdk),
- },
- );
+ const originalArguments = requestInteraction ? structuredClone(args) : args;
+ let continuation: { inputResponses?: Record<string, ElicitResult>; requestState?: string } =
+ {};
+ let rounds = 0;
+ let result;
+ while (true) {
+ if (requestInteraction) assertCurrent();
+ result = await client.callTool(
+ {
+ name: toolName,
+ arguments: originalArguments,
+ ...continuation,
+ ...(requestInteraction
+ ? {
+ _meta: { [CLIENT_CAPABILITIES_META_KEY]: { elicitation: { form: {} } } },
+ }
+ : {}),
+ },
+ {
+ signal,
+ timeout: options.timeoutMs ?? this.timeouts.callToolMs,
+ toolDefinition: structuredClone(preparation.value.definitionForSdk),
+ ...(requestInteraction ? { allowInputRequired: true } : {}),
+ },
+ );
+ if (requestInteraction) assertCurrent();
+ if (!requestInteraction || !isInputRequiredResult(result)) break;
+ if (++rounds > 8)
+ throw new McpToolCallError(serverId, toolName, 'form round limit exceeded');
+ const state = result.requestState;
+ if (
+ state !== undefined &&
+ (typeof state !== 'string' || Buffer.byteLength(state) > 16 * 1024)
+ ) {
+ throw new McpToolCallError(
+ serverId,
+ toolName,
+ 'form continuation state exceeds the limit',
+ );
+ }
+ if (state) {
+ states.push(state);
+ (state.length >= MIN_SUBSTITUTION_LENGTH
+ ? inventory.substitute
+ : inventory.withhold
+ ).push(state);
+ }
+ const requests = result.inputRequests ?? {};
+ const keys = Object.keys(requests).sort(compareText);
+ if (keys.length > 8 || serializedByteLength(requests, 64 * 1024) > 64 * 1024) {
+ throw new McpToolCallError(serverId, toolName, 'form input requests exceed the limit');
+ }
+ // Validate every sibling before publishing any user-facing Interaction.
+ const forms = keys.map((key) => ({
+ key,
+ prepared: prepareMcpForm(requests[key], { name: toolName, source: serverId }),
+ }));
+ for (const { prepared } of forms) {
+ if (
+ JSON.stringify(deepScrub(prepared.form, inventory)) !== JSON.stringify(prepared.form)
Review Comment:
Fixed in 3bec8388f361e6a396562246b2cd8a2b2001e4ff. Reproduced the real HTTP newline-state case: the old implementation displayed the form and retried. Raw embedded request keys and values are now checked against all retained connection states before `prepareMcpForm`, using direct containment rather than redacted-output equality. Regressions cover newline state, the literal redaction marker, message / property key / default, and reflection from a later invocation; rejected forms produce zero callbacks and no retry. Complete MCP suite: 249/249 passed.
Automated response by OpenAI Codex; not an independent human review.
##########
packages/mcp/src/index.ts:
##########
@@ -800,94 +829,191 @@ export class McpClientManager {
cause: preparation.cause,
});
}
- let result;
+ const requestInteraction =
+ entry.status.negotiatedProtocol?.era === 'modern' ? options.requestInteraction : undefined;
+ const controller = requestInteraction ? new AbortController() : undefined;
+ const signal = controller
+ ? options.signal
+ ? AbortSignal.any([controller.signal, options.signal])
+ : controller.signal
+ : options.signal;
+ const states: string[] = [];
+ if (controller) this.formCalls.set(controller, { binding, serverId, states });
+ const assertCurrent = () => {
+ signal?.throwIfAborted();
+ if (
+ this.closed ||
+ entry.closing ||
+ this.connections.get(serverId) !== entry ||
+ entry.client !== client ||
+ entry.status.state !== 'connected' ||
+ entry.connectionGeneration !== identity.connectionGeneration ||
+ entry.toolSnapshot.get(toolName)?.binding !== binding
+ )
+ throw new McpToolCallError(serverId, toolName, 'tool binding is stale');
+ };
try {
- result = await client.callTool(
- { name: toolName, arguments: args },
- {
- signal: options.signal,
- timeout: options.timeoutMs ?? this.timeouts.callToolMs,
- toolDefinition: structuredClone(preparation.value.definitionForSdk),
- },
- );
+ const originalArguments = requestInteraction ? structuredClone(args) : args;
+ let continuation: { inputResponses?: Record<string, ElicitResult>; requestState?: string } =
+ {};
+ let rounds = 0;
+ let result;
+ while (true) {
+ if (requestInteraction) assertCurrent();
+ result = await client.callTool(
+ {
+ name: toolName,
+ arguments: originalArguments,
+ ...continuation,
+ ...(requestInteraction
+ ? {
+ _meta: { [CLIENT_CAPABILITIES_META_KEY]: { elicitation: { form: {} } } },
+ }
+ : {}),
+ },
+ {
+ signal,
+ timeout: options.timeoutMs ?? this.timeouts.callToolMs,
+ toolDefinition: structuredClone(preparation.value.definitionForSdk),
+ ...(requestInteraction ? { allowInputRequired: true } : {}),
+ },
+ );
+ if (requestInteraction) assertCurrent();
+ if (!requestInteraction || !isInputRequiredResult(result)) break;
+ if (++rounds > 8)
+ throw new McpToolCallError(serverId, toolName, 'form round limit exceeded');
+ const state = result.requestState;
+ if (
+ state !== undefined &&
+ (typeof state !== 'string' || Buffer.byteLength(state) > 16 * 1024)
+ ) {
+ throw new McpToolCallError(
+ serverId,
+ toolName,
+ 'form continuation state exceeds the limit',
+ );
+ }
+ if (state) {
+ states.push(state);
+ (state.length >= MIN_SUBSTITUTION_LENGTH
+ ? inventory.substitute
+ : inventory.withhold
+ ).push(state);
+ }
+ const requests = result.inputRequests ?? {};
+ const keys = Object.keys(requests).sort(compareText);
+ if (keys.length > 8 || serializedByteLength(requests, 64 * 1024) > 64 * 1024) {
+ throw new McpToolCallError(serverId, toolName, 'form input requests exceed the limit');
+ }
+ // Validate every sibling before publishing any user-facing Interaction.
+ const forms = keys.map((key) => ({
+ key,
+ prepared: prepareMcpForm(requests[key], { name: toolName, source: serverId }),
+ }));
+ for (const { prepared } of forms) {
+ if (
+ JSON.stringify(deepScrub(prepared.form, inventory)) !== JSON.stringify(prepared.form)
+ ) {
+ throw new McpToolCallError(
+ serverId,
+ toolName,
+ 'form contains private continuation material',
+ );
+ }
+ }
+ const responses: Record<string, ElicitResult> = {};
+ for (const { key, prepared } of forms) {
+ assertCurrent();
+ // Providers may ignore cancellationSignal. Racing the callback still
+ // settles their invocation, which closes the canonical Host form.
+ const answer = await waitForMcpForm(
+ () => requestInteraction(prepared.form, { cancellationSignal: signal }),
+ signal!,
+ );
+ assertCurrent();
+ Object.defineProperty(responses, key, {
+ value: prepared.respond(answer),
+ enumerable: true,
+ configurable: true,
+ writable: true,
+ });
+ }
+ continuation = {
+ ...(keys.length ? { inputResponses: responses } : {}),
+ ...(state === undefined ? {} : { requestState: state }),
+ };
+ if (!keys.length) await delay(25, undefined, { signal });
+ }
+ // The SDK's legacy compatibility schema defaults a missing content array
+ // before returning, but retains deferred-result compatibility fields.
+ // Reject every decoded marker and any future content-less result.
+ if (
+ !Object.hasOwn(result, 'content') ||
+ Object.hasOwn(result, 'toolResult') ||
+ Object.hasOwn(result, 'task') ||
+ Object.hasOwn(result, 'inputRequests') ||
+ Object.hasOwn(result, 'requestState')
+ ) {
+ throw new McpToolCallError(
+ serverId,
+ toolName,
+ 'server returned an unsupported deferred tool result',
+ );
+ }
+ if (!Array.isArray(result.content)) {
+ throw new McpToolCallError(serverId, toolName, 'server returned invalid content');
+ }
+ if (result.isError) {
+ // The server writes this text; it can echo a secret it was sent.
+ throw new McpToolCallError(
+ serverId,
+ toolName,
+ scrubKnownSecrets(redactSecrets(summarizeErrorContent(result.content)), inventory),
+ );
+ }
+ assertSuccessfulToolResultBudget(serverId, toolName, result, { raw: true });
+ const validateOutput = preparation.value.validateOutput;
+ if (validateOutput) {
+ if (result.structuredContent === undefined) {
+ throw new McpToolCallError(serverId, toolName, 'server returned an invalid tool result');
+ }
+ let validation;
+ try {
+ validation = validateOutput(result.structuredContent);
+ } catch (cause) {
+ throw new McpToolCallError(serverId, toolName, 'server returned an invalid tool result', {
+ cause,
+ });
+ }
+ if (!validation.valid) {
+ throw new McpToolCallError(serverId, toolName, 'server returned an invalid tool result', {
+ cause: new Error(validation.errorMessage),
+ });
+ }
+ }
+ // Success payloads cross toward the renderer and the transcript too; a
+ // server can embed the credential it was just sent into a result.
+ const published = {
+ content: deepScrub(result.content.map(normalizeContent), inventory),
+ structuredContent: deepScrub(result.structuredContent, inventory),
+ };
+ assertSuccessfulToolResultBudget(serverId, toolName, published);
+ return published;
} catch (error) {
- // A 401 after connect means the server revoked the session, not that
- // one call hiccuped: leave `connected` and the UI never offers the
- // login it now needs.
+ const normalized = normalizeToolCallError(serverId, toolName, error, signal);
if (
isAuthRequiredError(error) &&
this.connections.get(serverId) === entry &&
entry.client === client
) {
- this.markError(entry, error);
+ this.markError(entry, scrubbedError(error, inventory));
}
- // The transport error can carry reflected request material (the body
- // of a failed POST); scrub it like every other outbound message. The
- // cause chain still holds the RAW transport error — the rejection
- // leaves the manager (IPC, logs, telemetry), and any cause-aware
- // serializer downstream would expose it — so the retained cause is an
- // allowlisted copy: typed identity (name, code, status, SDK brands)
- // with a scrubbed message and no deeper chain or payload fields.
- const normalized = normalizeToolCallError(serverId, toolName, error, options.signal);
normalized.message = scrubKnownSecrets(normalized.message, inventory);
normalized.cause = sanitizedCause(normalized.cause, inventory);
throw normalized;
+ } finally {
+ if (controller) this.formCalls.delete(controller);
Review Comment:
Fixed in 3bec8388f361e6a396562246b2cd8a2b2001e4ff. Real stdio regressions failed on the old manager and pass with connection-lifetime retention. The inventory is bounded to 64 states / 256 KiB without eviction; exhaustion aborts active calls and rejects new calls until reconnect. Stderr is withheld while interactive calls are pending, including partial / continued lines, and retained states scrub late diagnostics after completion. New states clear the already-formatted/truncated cache and notify subscribers; this cannot retract previously published text whose state was not yet known. Control/non-ASCII states suppress that connection's stderr until reconnect, covering UTF-8 chunk boundaries. Retired transport listeners cannot mutate a new generation. Added real stdio and concurrent-call regressions, plus scrubbed early diagnostic errors. MCP 249/249 and release checks 196/196 passed; workspace build, typecheck, lint and format passed.
Automated response by OpenAI Codex; not an independent human review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(mcp): implement bounded form input_required continuation [maka]
Posted by "hqhq1025 (via GitHub)" <gi...@apache.org>.
hqhq1025 commented on code in PR #5067:
URL: https://github.com/apache/maka/pull/5067#discussion_r3965420223
##########
packages/mcp/src/index.ts:
##########
@@ -800,94 +829,191 @@ export class McpClientManager {
cause: preparation.cause,
});
}
- let result;
+ const requestInteraction =
+ entry.status.negotiatedProtocol?.era === 'modern' ? options.requestInteraction : undefined;
+ const controller = requestInteraction ? new AbortController() : undefined;
+ const signal = controller
+ ? options.signal
+ ? AbortSignal.any([controller.signal, options.signal])
+ : controller.signal
+ : options.signal;
+ const states: string[] = [];
+ if (controller) this.formCalls.set(controller, { binding, serverId, states });
+ const assertCurrent = () => {
+ signal?.throwIfAborted();
+ if (
+ this.closed ||
+ entry.closing ||
+ this.connections.get(serverId) !== entry ||
+ entry.client !== client ||
+ entry.status.state !== 'connected' ||
+ entry.connectionGeneration !== identity.connectionGeneration ||
+ entry.toolSnapshot.get(toolName)?.binding !== binding
+ )
+ throw new McpToolCallError(serverId, toolName, 'tool binding is stale');
+ };
try {
- result = await client.callTool(
- { name: toolName, arguments: args },
- {
- signal: options.signal,
- timeout: options.timeoutMs ?? this.timeouts.callToolMs,
- toolDefinition: structuredClone(preparation.value.definitionForSdk),
- },
- );
+ const originalArguments = requestInteraction ? structuredClone(args) : args;
+ let continuation: { inputResponses?: Record<string, ElicitResult>; requestState?: string } =
+ {};
+ let rounds = 0;
+ let result;
+ while (true) {
+ if (requestInteraction) assertCurrent();
+ result = await client.callTool(
+ {
+ name: toolName,
+ arguments: originalArguments,
+ ...continuation,
+ ...(requestInteraction
+ ? {
+ _meta: { [CLIENT_CAPABILITIES_META_KEY]: { elicitation: { form: {} } } },
+ }
+ : {}),
+ },
+ {
+ signal,
+ timeout: options.timeoutMs ?? this.timeouts.callToolMs,
+ toolDefinition: structuredClone(preparation.value.definitionForSdk),
+ ...(requestInteraction ? { allowInputRequired: true } : {}),
+ },
+ );
+ if (requestInteraction) assertCurrent();
+ if (!requestInteraction || !isInputRequiredResult(result)) break;
+ if (++rounds > 8)
+ throw new McpToolCallError(serverId, toolName, 'form round limit exceeded');
+ const state = result.requestState;
+ if (
+ state !== undefined &&
+ (typeof state !== 'string' || Buffer.byteLength(state) > 16 * 1024)
+ ) {
+ throw new McpToolCallError(
+ serverId,
+ toolName,
+ 'form continuation state exceeds the limit',
+ );
+ }
+ if (state) {
+ states.push(state);
+ (state.length >= MIN_SUBSTITUTION_LENGTH
+ ? inventory.substitute
+ : inventory.withhold
+ ).push(state);
+ }
+ const requests = result.inputRequests ?? {};
+ const keys = Object.keys(requests).sort(compareText);
+ if (keys.length > 8 || serializedByteLength(requests, 64 * 1024) > 64 * 1024) {
+ throw new McpToolCallError(serverId, toolName, 'form input requests exceed the limit');
+ }
+ // Validate every sibling before publishing any user-facing Interaction.
+ const forms = keys.map((key) => ({
+ key,
+ prepared: prepareMcpForm(requests[key], { name: toolName, source: serverId }),
+ }));
+ for (const { prepared } of forms) {
+ if (
+ JSON.stringify(deepScrub(prepared.form, inventory)) !== JSON.stringify(prepared.form)
Review Comment:
[P1] Check continuation state before display projection
This comparison happens after `prepareMcpForm()` has projected server text through the display sanitizer. That transformation can make the state no longer match the registered secret: with `requestState = "opaque\nstate"` and the same bytes in `params.message`, a real modern server reaches `requestInteraction` with `message = "opaque\\u{A}state"`, and the manager sends the retry. The opaque state has therefore crossed into the Host/UI despite the check. Please reject state-bearing content against the raw embedded request before projection (without relying on redacted-output equality), and cover a transformed control-character value through the real manager path.
> **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.
##########
packages/mcp/src/index.ts:
##########
@@ -800,94 +829,191 @@ export class McpClientManager {
cause: preparation.cause,
});
}
- let result;
+ const requestInteraction =
+ entry.status.negotiatedProtocol?.era === 'modern' ? options.requestInteraction : undefined;
+ const controller = requestInteraction ? new AbortController() : undefined;
+ const signal = controller
+ ? options.signal
+ ? AbortSignal.any([controller.signal, options.signal])
+ : controller.signal
+ : options.signal;
+ const states: string[] = [];
+ if (controller) this.formCalls.set(controller, { binding, serverId, states });
+ const assertCurrent = () => {
+ signal?.throwIfAborted();
+ if (
+ this.closed ||
+ entry.closing ||
+ this.connections.get(serverId) !== entry ||
+ entry.client !== client ||
+ entry.status.state !== 'connected' ||
+ entry.connectionGeneration !== identity.connectionGeneration ||
+ entry.toolSnapshot.get(toolName)?.binding !== binding
+ )
+ throw new McpToolCallError(serverId, toolName, 'tool binding is stale');
+ };
try {
- result = await client.callTool(
- { name: toolName, arguments: args },
- {
- signal: options.signal,
- timeout: options.timeoutMs ?? this.timeouts.callToolMs,
- toolDefinition: structuredClone(preparation.value.definitionForSdk),
- },
- );
+ const originalArguments = requestInteraction ? structuredClone(args) : args;
+ let continuation: { inputResponses?: Record<string, ElicitResult>; requestState?: string } =
+ {};
+ let rounds = 0;
+ let result;
+ while (true) {
+ if (requestInteraction) assertCurrent();
+ result = await client.callTool(
+ {
+ name: toolName,
+ arguments: originalArguments,
+ ...continuation,
+ ...(requestInteraction
+ ? {
+ _meta: { [CLIENT_CAPABILITIES_META_KEY]: { elicitation: { form: {} } } },
+ }
+ : {}),
+ },
+ {
+ signal,
+ timeout: options.timeoutMs ?? this.timeouts.callToolMs,
+ toolDefinition: structuredClone(preparation.value.definitionForSdk),
+ ...(requestInteraction ? { allowInputRequired: true } : {}),
+ },
+ );
+ if (requestInteraction) assertCurrent();
+ if (!requestInteraction || !isInputRequiredResult(result)) break;
+ if (++rounds > 8)
+ throw new McpToolCallError(serverId, toolName, 'form round limit exceeded');
+ const state = result.requestState;
+ if (
+ state !== undefined &&
+ (typeof state !== 'string' || Buffer.byteLength(state) > 16 * 1024)
+ ) {
+ throw new McpToolCallError(
+ serverId,
+ toolName,
+ 'form continuation state exceeds the limit',
+ );
+ }
+ if (state) {
+ states.push(state);
+ (state.length >= MIN_SUBSTITUTION_LENGTH
+ ? inventory.substitute
+ : inventory.withhold
+ ).push(state);
+ }
+ const requests = result.inputRequests ?? {};
+ const keys = Object.keys(requests).sort(compareText);
+ if (keys.length > 8 || serializedByteLength(requests, 64 * 1024) > 64 * 1024) {
+ throw new McpToolCallError(serverId, toolName, 'form input requests exceed the limit');
+ }
+ // Validate every sibling before publishing any user-facing Interaction.
+ const forms = keys.map((key) => ({
+ key,
+ prepared: prepareMcpForm(requests[key], { name: toolName, source: serverId }),
+ }));
+ for (const { prepared } of forms) {
+ if (
+ JSON.stringify(deepScrub(prepared.form, inventory)) !== JSON.stringify(prepared.form)
+ ) {
+ throw new McpToolCallError(
+ serverId,
+ toolName,
+ 'form contains private continuation material',
+ );
+ }
+ }
+ const responses: Record<string, ElicitResult> = {};
+ for (const { key, prepared } of forms) {
+ assertCurrent();
+ // Providers may ignore cancellationSignal. Racing the callback still
+ // settles their invocation, which closes the canonical Host form.
+ const answer = await waitForMcpForm(
+ () => requestInteraction(prepared.form, { cancellationSignal: signal }),
+ signal!,
+ );
+ assertCurrent();
+ Object.defineProperty(responses, key, {
+ value: prepared.respond(answer),
+ enumerable: true,
+ configurable: true,
+ writable: true,
+ });
+ }
+ continuation = {
+ ...(keys.length ? { inputResponses: responses } : {}),
+ ...(state === undefined ? {} : { requestState: state }),
+ };
+ if (!keys.length) await delay(25, undefined, { signal });
+ }
+ // The SDK's legacy compatibility schema defaults a missing content array
+ // before returning, but retains deferred-result compatibility fields.
+ // Reject every decoded marker and any future content-less result.
+ if (
+ !Object.hasOwn(result, 'content') ||
+ Object.hasOwn(result, 'toolResult') ||
+ Object.hasOwn(result, 'task') ||
+ Object.hasOwn(result, 'inputRequests') ||
+ Object.hasOwn(result, 'requestState')
+ ) {
+ throw new McpToolCallError(
+ serverId,
+ toolName,
+ 'server returned an unsupported deferred tool result',
+ );
+ }
+ if (!Array.isArray(result.content)) {
+ throw new McpToolCallError(serverId, toolName, 'server returned invalid content');
+ }
+ if (result.isError) {
+ // The server writes this text; it can echo a secret it was sent.
+ throw new McpToolCallError(
+ serverId,
+ toolName,
+ scrubKnownSecrets(redactSecrets(summarizeErrorContent(result.content)), inventory),
+ );
+ }
+ assertSuccessfulToolResultBudget(serverId, toolName, result, { raw: true });
+ const validateOutput = preparation.value.validateOutput;
+ if (validateOutput) {
+ if (result.structuredContent === undefined) {
+ throw new McpToolCallError(serverId, toolName, 'server returned an invalid tool result');
+ }
+ let validation;
+ try {
+ validation = validateOutput(result.structuredContent);
+ } catch (cause) {
+ throw new McpToolCallError(serverId, toolName, 'server returned an invalid tool result', {
+ cause,
+ });
+ }
+ if (!validation.valid) {
+ throw new McpToolCallError(serverId, toolName, 'server returned an invalid tool result', {
+ cause: new Error(validation.errorMessage),
+ });
+ }
+ }
+ // Success payloads cross toward the renderer and the transcript too; a
+ // server can embed the credential it was just sent into a result.
+ const published = {
+ content: deepScrub(result.content.map(normalizeContent), inventory),
+ structuredContent: deepScrub(result.structuredContent, inventory),
+ };
+ assertSuccessfulToolResultBudget(serverId, toolName, published);
+ return published;
} catch (error) {
- // A 401 after connect means the server revoked the session, not that
- // one call hiccuped: leave `connected` and the UI never offers the
- // login it now needs.
+ const normalized = normalizeToolCallError(serverId, toolName, error, signal);
if (
isAuthRequiredError(error) &&
this.connections.get(serverId) === entry &&
entry.client === client
) {
- this.markError(entry, error);
+ this.markError(entry, scrubbedError(error, inventory));
}
- // The transport error can carry reflected request material (the body
- // of a failed POST); scrub it like every other outbound message. The
- // cause chain still holds the RAW transport error — the rejection
- // leaves the manager (IPC, logs, telemetry), and any cause-aware
- // serializer downstream would expose it — so the retained cause is an
- // allowlisted copy: typed identity (name, code, status, SDK brands)
- // with a scrubbed message and no deeper chain or payload fields.
- const normalized = normalizeToolCallError(serverId, toolName, error, options.signal);
normalized.message = scrubKnownSecrets(normalized.message, inventory);
normalized.cause = sanitizedCause(normalized.cause, inventory);
throw normalized;
+ } finally {
+ if (controller) this.formCalls.delete(controller);
Review Comment:
[P1] Keep requestState redaction valid outside the active call window
Deleting this entry drops the only retained inventory of continuation states. `attachStderrTail()` scrubs each line once against the inventory that exists at arrival time and never re-scrubs retained lines. In a real stdio probe, the server wrote the same state immediately before returning `input_required` and again just after the final response; both raw lines remained in `manager.status(...).stderrTail`, which Desktop renders directly on the MCP page. Thus ordinary timing around the response exposes a value the contract says must never enter logs or UI. Please retain a bounded per-server state inventory long enough to cover late diagnostics and retroactively scrub already-retained tail entries when a state is first learned; add regressions for both the pre-response and post-completion windows.
> **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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(mcp): implement bounded form input_required continuation [maka]
Posted by "me2seeks (via GitHub)" <gi...@apache.org>.
me2seeks merged PR #5067:
URL: https://github.com/apache/maka/pull/5067
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org