Skip to content

MRTR: drive input_required manually to keep pending-request UX (#1704)#1719

Merged
cliffhall merged 3 commits into
v2/mainfrom
v2/mrtr-manual-driver-1704
Jul 18, 2026
Merged

MRTR: drive input_required manually to keep pending-request UX (#1704)#1719
cliffhall merged 3 commits into
v2/mainfrom
v2/mrtr-manual-driver-1704

Conversation

@cliffhall

Copy link
Copy Markdown
Member

Closes #1704

Follow-up to #1627/#1705. Instead of letting the SDK auto-fulfil the modern (2026-07-28) MRTR retry loop, InspectorClient now drives it itself so the Inspector keeps its pending-request UX, owns the loop, and shows era-accurate semantics. Applies to all three clients (web/cli/tui) since the driver lives in core/mcp/inspectorClient.ts.

What changed

Core (core/mcp/inspectorClient.ts)

  • Construct the SDK Client with inputRequired: { autoFulfill: false } — unconditional and safe (a no-op on legacy, which never returns input_required; the negotiated era isn't known at construction time).
  • New requestWithInputRequired() driver: issues the request with allowInputRequired + withInputRequired(schema), and while the result is input_required, fulfils each embedded request through the same pending sampling/elicitation plumbing, then retries the original call with inputResponses + the echoed requestState on a fresh id, bounded by MRTR_MAX_ROUNDS.
  • Route tools/call, prompts/get, and resources/read through the driver.
  • Embedded roots/list is auto-answered from configured roots (no pending UX, consistent with the legacy roots handler); decline/cancel is echoed to the server (not an abort); a tool-call abort mid-round settles the pending request and surfaces ToolCallCancelledError.
  • Extract shared enqueuePendingSample / enqueuePendingElicitation helpers used by both the legacy inbound handlers and the driver; add a per-request origin ("server-request" | "input-required") to the pending wrappers.

Web UI

  • New MrtrOriginNote element renders an "input_required — your answer is sent back as a retry (MRTR)" note on a modern MRTR round; legacy panels are visually unchanged.
  • Thread origin from AppPendingClientRequestModal → the sampling/elicitation panels (and the inline panels).

Test servers

  • New MRTR presets: multi-round, roots, sampling, never-completes (maxRounds), and edge-case param-shaping (inputRequests-only / requestState-only rounds).

Tests

  • Extended the live modern-era integration suite: manual round-trip (asserting the pending UI pauses and the retry carries inputResponses + requestState on a new id), multi-round, roots auto-answer, embedded sampling, decline-echoed, MRTR_MAX_ROUNDS bound, abort mid-round, and param-shaping.
  • New MrtrOriginNote element tests/stories + panel/modal origin coverage.
  • Updated the URL-elicitation and coverage-backfill unit tests that mocked the SDK callTool to mock request (the tool-call path now uses client.request).
  • npm run validate (web/cli/tui/launcher), npm run coverage (≥90 per-file gate), and Storybook play tests all green.

Scope notes

  • The task-augmented (callToolStream) path is intentionally not MRTR-driven here (out of scope).
  • prompts/get / resources/read go through the same method-agnostic driver as tools/call; the MRTR-specific integration coverage exercises the driver via tools/call (the test server supports embedded requests on the tool path), while the existing prompt/resource integration tests validate the rewired call sites.
  • CLI/TUI run the driver (it's in core) but don't render pending sampling/elicitation, so an embedded MRTR request there awaits a responder — same as a legacy server→client request today; interactive MRTR in CLI/TUI is out of scope for this issue.

Verified end-to-end

Smoke-tested in the browser (Playwright) against a composable modern-era server (test-servers/configs/modern-mrtr-http.json): connecting in the Modern era and calling mrtr_confirm pauses at the pending modal with the era note, and answering completes the retry — shown as a grouped MRTR conversation in the Protocol view.

🤖 Generated with Claude Code

Follow-up to #1627/#1705. Instead of letting the SDK auto-fulfil the modern
(2026-07-28) MRTR retry loop, InspectorClient now drives it itself so the
Inspector keeps its pending-request UX, owns the loop, and shows era-accurate
semantics.

Core (core/mcp/inspectorClient.ts):
- Construct the SDK Client with `inputRequired: { autoFulfill: false }`
  (unconditional; a no-op on legacy, which never returns input_required).
- New `requestWithInputRequired()` driver: issues the request with
  `allowInputRequired` + `withInputRequired(schema)`, and while the result is
  `input_required`, fulfils each embedded request through the SAME pending
  sampling/elicitation plumbing, then retries the original call with
  `inputResponses` + the echoed `requestState` on a fresh id, bounded by
  MRTR_MAX_ROUNDS.
- Route tools/call, prompts/get, and resources/read through the driver.
- Embedded roots/list is auto-answered from configured roots (no pending UX);
  decline/cancel is echoed to the server (not treated as an abort); a tool-call
  abort mid-round settles the pending request and surfaces
  ToolCallCancelledError.
- Extract shared `enqueuePendingSample`/`enqueuePendingElicitation` helpers used
  by both the legacy inbound handlers and the driver; add a per-request `origin`
  ("server-request" | "input-required") to the pending wrappers.

Web UI:
- New `MrtrOriginNote` element shows an "input_required — your answer returns as
  a retry" note on a modern MRTR round; legacy panels are unchanged.
- Thread `origin` App -> PendingClientRequestModal -> the sampling/elicitation
  panels (and the inline panels).

Test servers: new MRTR presets (multi-round, roots, sampling, loop, edge-case
param shaping). Integration tests cover the manual round-trip (asserting the
pending UI pauses and the retry carries inputResponses + requestState on a new
id), multi-round, roots auto-answer, sampling, decline-echoed, maxRounds bound,
abort mid-round, and requestState-only/inputRequests-only param shaping.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5
@cliffhall cliffhall added the v2 Issues and PRs for v2 label Jul 18, 2026
@cliffhall

Copy link
Copy Markdown
Member Author

@claude review

@claude

claude Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Claude finished @cliffhall's task in 2m 46s —— View job


Review: MRTR manual driver (#1719)

I read the core driver (inspectorClient.ts), the pending-request helpers, the sampling/elicitation classes, the web UI (MrtrOriginNote, modal, panels), and the test-server MRTR presets. This is a clean, well-documented change — the manual input_required loop is correctly bounded, reuses the existing pending-request plumbing rather than forking it, and the origin threading through to the UI is tidy. Tests are unusually thorough (multi-round, roots auto-answer, embedded sampling, MRTR_MAX_ROUNDS bound, abort mid-round, and both param-shaping edge cases). Nothing blocking; a few minor observations below.

Detailed findings

Correctness — no bugs found

The driver logic is sound:

  • requestWithInputRequired (core/mcp/inspectorClient.ts:1809) correctly re-issues the original params plus each round's inputResponses + echoed requestState on a fresh id, matching the stateless-modern-leg contract the fixtures document.
  • The round > MRTR_MAX_ROUNDS guard (:1839) permits 10 fulfilled rounds and throws on the 11th — correct bound against a never-completing server (well-exercised by mrtr_loop).
  • The abort path (callToolWithRetries :2185) → ToolCallCancelledError correctly maps a deliberate cancel and lets other aborts fall through.
  • roots/list auto-answer with no pending UX (:1890) is consistent with the legacy roots handler.

1. enqueuePendingSample docstring slightly overstates the sampling path (nit — doc only)

core/mcp/inspectorClient.ts:1710 says "A declined/cancelled sampling still resolves normally so the bare result is echoed back to the server; only a genuine failure or an signal abort rejects." Sampling has no "decline/cancel" action (unlike elicitation) — the only non-answer path is the Reject button, which calls pendingSamples[0].reject(...) (App.tsx:3681) → rejectPromise → the driver throws → the tool call is recorded as failed. So for sampling, a user reject does reject (failing the call), which is the opposite of what the shared docstring implies. The behavior is reasonable; only the wording (shared with the elicitation case, which genuinely does resolve on decline) is imprecise.

2. Abort comment assumes an in-flight SDK request that may not exist between rounds (nit — comment only)

core/mcp/inspectorClient.ts:2180-2184 reasons "A deliberate cancelToolCall() … means the SDK already sent notifications/cancelled." That holds when the abort lands during a client.request leg, but if the user cancels while the driver is awaiting an embedded pending request between rounds, there is no active SDK request, so no notifications/cancelled is sent — the driver simply abandons the retry. That's fine for the stateless modern leg (no dangling server request to leak, as the fixtures note), and the UX outcome (ToolCallCancelledError, no failed-call record) is still correct — the comment just doesn't cover the between-rounds case.

3. Abort listeners accumulate on the shared signal across rounds (observation — harmless)

wirePendingAbort (:1775) registers a { once: true } abort listener per embedded request. On a successful multi-round call the earlier rounds' listeners are never explicitly removed (only fire on abort; their removePending* / rejectPromise are no-ops once the round resolved) — they're cleaned when the AbortController is dropped in callTool's finally and GC'd. No functional impact; noting only in case a future change makes the signal longer-lived.

Nits

  • prompts/get / resources/read route through the driver without an abort signal (:2908, :3080), so an embedded elicitation there can only be answered, not cancelled — consistent with those surfaces having no Cancel button and the modal being non-dismissable, so acceptable and in line with the PR's scope notes.
  • MrtrOriginNote uses color="blue" / c="dimmed" (Mantine tokens, not raw hex) — compliant with the styling rules.
· branch `v2/mrtr-manual-driver-1704`

)

Address review nits on PR #1719 (comment-only, no behavior change):
- enqueuePendingSample docstring: sampling has no decline/cancel action; the
  panel either sends a result (resolve) or Rejects (reject → fails the call).
  The shared "declined/cancelled resolves" wording was accurate only for the
  elicitation case.
- callToolWithRetries abort comment: note the between-rounds case — when the
  abort lands while an MRTR round awaits an embedded pending request there is no
  in-flight SDK request, so nothing is sent on the wire; the outcome
  (ToolCallCancelledError, no failed-call record) is identical.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5
@cliffhall

cliffhall commented Jul 18, 2026

Copy link
Copy Markdown
Member Author

Thanks for the thorough review! Addressed the two doc/comment nits in 2ce620b (comment-only, no behavior change):

1 — enqueuePendingSample docstring overstates the sampling path. Fixed. Reworded to note sampling has no decline/cancel action: the panel either sends a CreateMessageResult (resolve → echoed) or Rejects (reject → fails the tool call), and a signal abort likewise rejects. The old "declined/cancelled resolves" phrasing was accurate only for the elicitation case it was shared with.

2 — abort comment assumes an in-flight SDK request. Fixed. Expanded the callToolWithRetries catch comment to cover the between-rounds case: when the abort lands while an MRTR round is awaiting an embedded pending request, there's no in-flight client.request, so nothing goes on the wire — wirePendingAbort just rejects the pending request and the driver abandons the retry. The outcome (ToolCallCancelledError, no failed-call record) is identical, which is why the same branch handles both.

3 — abort listeners accumulate on the shared signal across rounds. Left as-is, per your "no functional impact" note. On a successful multi-round call the earlier rounds' { once: true } listeners never fire, and their removePending* / rejectPromise are no-ops once the round resolved; they're released when callTool's finally drops the AbortController. Explicitly removing them on settle would reintroduce the per-round listener bookkeeping I deliberately dropped when I simplified away the settled flag (Promise settle is already idempotent), for no runtime benefit. Happy to add it if you'd prefer the tidier teardown.

Nits (no change): the missing abort signal on prompts/get / resources/read is intentional — those surfaces have no Cancel button and the modal is non-dismissable (also called out in the PR scope notes). MrtrOriginNote styling confirmed on Mantine tokens.

@cliffhall

Copy link
Copy Markdown
Member Author

@claude review

@claude

claude Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Claude finished @cliffhall's task in 1m 40s —— View job


Re-review: MRTR manual driver (#1704)

Re-read the full changed set at the current head (2ce620b9), focusing on the core driver and the two doc/comment fixes from that commit. My earlier findings were all doc-only nits, and both are now resolved. Nothing blocking; the change is in good shape to merge.

Detailed findings

Fixes from 2ce620b — both confirmed correct

  • Progress notifications #1 sampling-reject wordingenqueuePendingSample (core/mcp/inspectorClient.ts:1710) now states sampling has no decline/cancel action and that a Reject/signal abort rejects (failing the tool call), rather than the old shared "declined/cancelled resolves" phrasing that was only true for the elicitation case. Accurate now.
  • Request logging #2 between-rounds abort — the callToolWithRetries catch comment (:2183) now explicitly covers the case where the abort lands while a round awaits an embedded pending request (no in-flight client.request, nothing on the wire, wirePendingAbort rejects and the driver abandons the retry). Correctly explains why the same branch handles both.
  • Completion requests #3 listener accumulation — left as-is per the "no functional impact" note; I agree. The { once: true } listeners on already-settled rounds are no-ops and are released when callTool's finally drops the AbortController. Adding explicit teardown would reintroduce the per-round bookkeeping deliberately dropped; not worth it.

Re-verified core logic — no new issues

  • Retry param shaping (:1849) rebuilds nextParams from the original params each round plus only that round's inputResponses + echoed requestState — so prior rounds' answers never accumulate on the wire, matching the stateless-modern-leg contract. The mrtr_edge fixture (inputRequests-only round → requestState-only round → complete) exercises both omission paths, and the round-detection there is self-consistent.
  • Round bound (:1839): round increments after each input_required, throws when round > MRTR_MAX_ROUNDS (10) — i.e. fulfils 10 rounds, throws on the 11th. Exercised by mrtr_loop.
  • Embedded dispatch (fulfilEmbeddedInputRequest, :1890): roots/list auto-answered from configured roots (no pending UX, consistent with the legacy handler), sampling/elicitation routed through the shared pending plumbing tagged "input-required"; the default throw is a justified v8 ignore defensive guard.
  • Construction (:449): inputRequired: { autoFulfill: false } unconditionally — correct, since the negotiated era isn't known at construction and legacy servers never return input_required.
  • origin threading is consistent end to end: types.tsSamplingCreateMessage/ElicitationCreateMessage (defaulted to "server-request", backward-compatible) → AppPendingClientRequestModal → each panel → MrtrOriginNote, which renders nothing on legacy so those panels are visually unchanged.
  • Scope boundaries are explicit and reasonable: the callToolStream task path deliberately does not opt into allowInputRequired (:2529 note); prompts/get / resources/read route through the same driver without an abort signal (no Cancel button on those surfaces).

Styling / conventions

  • MrtrOriginNote uses Mantine tokens (color="blue", c="dimmed"), .withProps() subcomponents, no raw hex or inline styles — compliant with the repo's Mantine-first rules.

Looks good — approving in spirit (I can't submit a formal GitHub approval). · branch v2/mrtr-manual-driver-1704

…1704)

A single modern-era streamable-HTTP server (port 3111) bundling all six MRTR
presets — mrtr_confirm, mrtr_two_step, mrtr_sample, mrtr_roots, mrtr_edge,
mrtr_loop — for manual testing of the manual-driving pending-request UX across
single-round, multi-round, sampling, roots, param-shaping, and maxRounds paths.
Document it in the README alongside modern-mrtr-http.json.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5
@cliffhall

cliffhall commented Jul 18, 2026

Copy link
Copy Markdown
Member Author

Manual smoke test (Playwright + composable modern server)

Verified the manual MRTR driving end to end in the browser against a real composable server running the modern (2026-07-28) leg.

Setup

  • Server: test-servers/configs/mrtr-showcase-http.json — a modern-era streamable-HTTP server (transport.modern: true, port 3111) bundling every MRTR preset: mrtr_confirm, mrtr_two_step, mrtr_sample, mrtr_roots, mrtr_edge, mrtr_loop.
  • Client: the inspector built from this branch, connected to the server with Protocol Era = Modern (server/discover negotiated, sessionless).

Steps exercised

  1. Connect (modern era)server/discover succeeds; all 7 tools list. Protocol view header shows the Modern era badge.
  2. mrtr_confirm (single round) — the tool returns input_required; the Inspector pauses at the pending Elicitation modal, which shows the new input_required badge and the note "The server returned input_required; your answer is sent back as a retry of the original request (MRTR)." Answering (confirm: true) drives the retry and the call completes (MRTR complete — …).
  3. mrtr_sample (embedded sampling) — same manual-driving path via the Sampling panel (not just elicitation): the panel shows the identical era note; sending a response completes the retry.
  4. mrtr_two_step (multi-round) — pauses at round-1 elicitation ("Step 1…"), and after answering, advances to a second input_required round ("Step 2…"), then completes — proving the driver loops across multiple rounds, echoing each round's inputResponses + requestState on a fresh id.
  5. History — each logical call renders as a grouped MRTR conversation (rendering from History: MRTR conversation grouping + modern message vocabulary #1705), with era-accurate framing throughout.

mrtr_roots (auto-answered silently from configured roots — no modal), mrtr_edge (inputRequests-only → requestState-only round shaping), and mrtr_loop (trips the MRTR_MAX_ROUNDS bound) are also available in the showcase server for manual poking.

Screenshots

1. Pending Elicitation modal with the input_required MRTR note (mrtr_confirm, round paused):
mrtr-02-pending-elicitation-note

2. Completed retry + grouped MRTR conversation ("MRTR complete", Protocol view):
mrtr-03-complete-grouped

3. Sampling panel showing the same era note (mrtr_sample):
sampling-panel-note

4. Two-step multi-round (round 1 → round 2 → complete):

mrtr-two-step

(Screenshots captured locally via Playwright; drop the PNGs in place of the placeholders above.)

@cliffhall
cliffhall merged commit 9b2c188 into v2/main Jul 18, 2026
3 checks passed
@cliffhall
cliffhall deleted the v2/mrtr-manual-driver-1704 branch July 18, 2026 03:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v2 Issues and PRs for v2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

History: drive MRTR manually to keep pending-request UX on modern connections

1 participant