Skip to content

fix(server): a submitted MCP App form no longer comes back empty - #24208

Open
shivamhwp wants to merge 13 commits into
mainfrom
issue-watch/23825-form-submit
Open

shivamhwp wants to merge 13 commits into
mainfrom
issue-watch/23825-form-submit

Conversation

@shivamhwp

@shivamhwp shivamhwp commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Includes the deferred-form rendering fix from https://git.cafe/pingdotgg/t3code/pulls/23850 and records successful MCP App writes so submitted forms retain their result after reopening.

Problem

An MCP App gets the result of the tool call that showed it. For a form that deferred a write (https://git.cafe/pingdotgg/t3code/pulls/23850), that result is GitHub's "form shown, the issue has NOT been created yet". After you submit, the app shows "Issue created successfully", but only in memory. Reload the page, scroll the app away, or open the thread on another device, and you get the empty form again, even though the issue exists. Submitting it again would create a duplicate, though the confirm prompt comes first.

Change

  • When an app calls its own tool, the call is a write (not marked read-only) and it succeeds, McpAppRequests dispatches a new internal command, mcp_app.call.record. The orchestrator stores the call's arguments and result on the tool call's output under t3McpAppCall, beside the original result, which stays as it was.
  • Web and mobile open the app with the recorded call when there is one (mcpAppToolCall in @t3tools/shared/mcpApp), so GitHub's form opens on its "created" view.
  • The update bumps the item's updatedAt, so clients refetch the item. No new RPC.
  • A later update of the same tool call from the provider keeps the recorded call, so it isn't dropped if Codex resends the item. EventSink merges both ways while writing, inside the same transaction: a provider's copy keeps the stored call, and a recorded call lands on the provider's latest copy, so neither writer can undo the other with a copy it read earlier.
  • A call is only kept if the item's output still fits what clients read whole (256 KiB). Past that, clients get truncated text and the app would lose both results. If a provider resend makes the merged output too big, the provider's own output is kept instead of the call.

Choices:

  • Internal command, not a direct write: the request service feeds the outcome back into orchestration like other outside work (secret_request.record is the same pattern). No client can send it.
  • Reads aren't recorded: an app can repeat a read on its own, and every write already needs the user to click Allow. This keeps the event log from growing on a refreshing app.
  • A failed record doesn't fail the call: the write already happened, and an error would make the app offer it again. The failure is logged with the thread, item and error tag only.

Scope and approval

A follow-up to https://git.cafe/pingdotgg/t3code/pulls/23850 for the gap found while testing it on a device. It only changes what an MCP App receives when it opens. No new settings or UI. Threads submitted before this change still open on the empty form, since nothing was recorded for them.

Verification

Linux, Codex, GitHub's real MCP server (github-mcp-server v2.0.3, --toolsets=issues) on a throwaway private repo.

Before: a thread whose form was submitted (the issue exists) opens on the empty form again (Android, POCO M3).

before

After, web: submitted, then reloaded. The app opens on "Issue created successfully".

after: web reload

After, Android: the same thread, opened on the phone after the submit on web, shows the created issue too.

after: phone

Tests

  • McpAppRequests.test.ts: a successful write through the app's own tool is recorded; reads, other tools and failed writes aren't; the call still returns when recording fails.
  • Orchestrator.control-reads.test.ts: mcp_app.call.record keeps the call beside the original result and moves updatedAt; it's refused for a tool call without an app.
  • mcpApp.test.ts: the app opens on the recorded call, or else on the original one.
  • ProviderEventIngestor.test.ts: a provider resending the tool call keeps the recorded call, image included, in the projection and in the stored event; an oversized merge keeps the provider's output; a call recorded from a stale copy lands on the provider's latest one. Each part fails without its change.
  • McpAppRequests.test.ts also covers a read-only own tool (not recorded), and Orchestrator.control-reads.test.ts an oversized call (refused, previous call kept).

Not run: iOS and the desktop app (they share the mobile and web code paths changed here).

Not covered

An app that is already open somewhere else doesn't switch to the submitted view until it is reopened. The host sends an app its tool result once per instance; sending a new one into a running app could reset whatever the user is doing in it, and the open app on the device that submitted already shows the outcome. The confirm prompt still guards a second submit from the stale copy.

Refs https://git.cafe/pingdotgg/t3code/issues/23825

Opus 5.5, Claude Code in T3 Code.

🤖 Generated with Claude Code

takeover verification, 2026-10-30

Merged github/main at cba30591f95859bc76154e5be97891ea4ae1b85a into the existing head with a merge commit, preserving both internal command handlers and transactional normalization. No feature expansion. Final head: 09cb4bb505a5df4671636edd6eb6d0a914070006.

Blacksmith vp run check:changed cba30591f95859bc76154e5be97891ea4ae1b85a passed formatting, lint, knip, generated GraphQL checks, 32 package typechecks, React Compiler checks, and 208 tests across nine files. Two independent gpt-6.1-sol reviewers approved this head. All 33 GitCafe check contexts pass; no conflicts or unresolved review threads remain.

Real Codex → GitHub MCP server v2.0.3 → web client: deferred update form appeared; cancel preserved the form and made no write; a rejected write stayed retryable; confirming successfully updated the existing disposable verification issue. Success survived reload, and read-only SQLite inspection confirmed both the original result and recorded call. The animation removes the reload loading pause.

real github form submission and success retained after reload

submitted github issue result after reload

Unverified this pass: native mobile, Electron, other providers, hosted GitHub MCP transport, and remote/relay/tunnel modes.

Takeover verification: gpt-6.1-sol through the Codex harness.

PR Batch Tester and others added 2 commits October 22, 2026 21:15
A write tool that defers to its MCP App form returns an `isError` result,
which Codex reports as a failed call. T3 only captured and showed apps for
completed calls, so the form never appeared and the write never happened.

Capture the app whenever the call has a result, pass `isError` on to the
app, and let web and mobile show the app for failed calls too.

Closes #23825

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An MCP App's tool call keeps the result it first returned. For a form
that deferred a write, that is "form shown, nothing created yet", so the
form came back empty after a reload or on another device, even though
the write had gone through.

When an app writes through its own tool and it succeeds, the server now
records that call on the tool call that shows the app, as an internal
orchestration command. Web and mobile open the app on the recorded call
when there is one. Reads aren't recorded, since apps can repeat them
without asking.

Refs #23825

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added the vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. label Oct 23, 2026
@github-actions github-actions Bot added the size:L 100-499 changed lines (additions + deletions). label Oct 23, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 09cb4bb

Macroscope's review found this PR approvable — Submitted MCP App forms now reopen with their successful submission result instead of their original empty state. Recording stays within the existing thread, preserves provider output, and does not repeat external writes if persistence fails.

You can add or adjust custom eligibility rules. Learn more.

@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 5.0 KiB 5.0 KiB +35 B (+0.7%) 6.8 KiB ✅
Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.2 KiB 1.3 KiB +35 B (+2.8%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 21.5 KiB 21.6 KiB +41 B (+0.2%) 24.4 KiB ✅
Codex Live turn messages 1 2 +1 (+100.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.3 KiB 1.3 KiB 0 B (0.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.9 KiB 21.9 KiB 0 B (0.0%) 24.4 KiB ✅
Claude Live turn messages 2 2 0 (0.0%) 8 ✅

Baseline: cba3059 · PR result: 09cb4bb · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

Review follow-ups for the recorded MCP App call:

- A provider resending the tool call no longer drops the recorded call;
  the ingestor carries it over, so the stored event and a replay keep it.
- A call that would push the item's output past what clients read whole
  is refused, keeping the previous one, since clients would otherwise
  get truncated text and lose both results.
- A failed record logs bounded fields instead of the error.
- Tests cover a read-only own tool, and use a per-test recorder.

Refs #23825

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PR Batch Tester and others added 2 commits October 23, 2026 19:18
The recorded call was carried over in the provider ingestor, from a read
made before the event was written. A record committed between that read
and the write was lost, and the ingestor's image stripping then ran over
the carried-over result.

The carry-over now happens in EventSink's per-item normalization, inside
the write's transaction and after the ingestor has stripped the
provider's own output.

Refs #23825

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t the detail limit

When a provider resends a tool call with a larger output, the recorded
call is only carried over if the merged output still fits what clients
read whole. Otherwise the provider's own output is kept, rather than
both results turning into truncated text.

Refs #23825

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Oct 23, 2026
…ol call

The record command builds its update from the item it read, and a
provider update can commit between that read and the command's write,
since provider writes don't take the thread's command lock. The record
then restored the provider's older copy.

EventSink now merges in both directions inside the write's transaction:
a provider's copy keeps the stored call, and a recorded call is rebuilt
on the stored item. The helper is an untraced Effect function like the
sink's other per-event steps.

Refs #23825

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 23, 2026 19:32

Dismissing prior approval to re-evaluate de53b23

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Oct 23, 2026
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 26, 2026 03:09

Dismissing prior approval to re-evaluate a84a131

@github-actions github-actions Bot added size:XXL 1,000+ changed lines (additions + deletions). 📱 Native Change Changes the native fingerprint; merging blocks production OTAs until a new store build ships. and removed size:L 100-499 changed lines (additions + deletions). labels Oct 26, 2026
@maria-rcks
maria-rcks changed the base branch from issue-watch/23825 to main October 26, 2026 03:11
@coderabbitai

coderabbitai Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Walkthrough

The change adds an internal command to record successful MCP App tool calls. The orchestrator stores call arguments and results on dynamic-tool items, and event normalization reconciles those records with provider output. Mobile and web clients use a shared helper to read recorded call data.

Changes: MCP App call flow

Layer / File(s) Summary
Call data contract
packages/shared/src/mcpApp.ts, packages/shared/src/mcpApp.test.ts, packages/contracts/src/orchestrationV2.ts
A shared key and helper define recorded-call extraction and fallback behavior. The internal mcp_app.call.record command carries the turn item, arguments, and result.
Record successful calls
apps/server/src/mcpApps/McpAppRequests.ts, apps/server/src/mcpApps/McpAppRequests.test.ts, apps/server/src/orchestration-v2/Orchestrator.ts, apps/server/src/orchestration-v2/Orchestrator.control-reads.test.ts, apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts, apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts
The request handler dispatches records for successful, non-read-only calls to the app’s own tool. The orchestrator validates the item and result size before updating it. Tests cover recording eligibility, invalid targets, oversized results, and dispatch failures. Codex adapter comments describe its handling of isError results.
Reconcile provider updates
apps/server/src/orchestration-v2/EventSink.ts, apps/server/src/orchestration-v2/WireProjection.ts, apps/server/src/orchestration-v2/ProviderEventIngestor.test.ts
Event normalization merges recorded call output with provider output. The detail-size check excludes image data. Tests cover provider updates, replay, stale copies, and oversized combined output.
Read recorded calls in clients
apps/mobile/src/features/threads/McpAppWebView.tsx, apps/web/src/components/chat/McpAppFrame.tsx
Mobile and web call extraction use the shared helper. The web frame synchronizes its stored app reference with the prop when the references differ.

Priority: Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant McpAppRequests
  participant Orchestrator
  participant EventSink
  McpAppRequests->>Orchestrator: Dispatch mcp_app.call.record
  Orchestrator->>EventSink: Commit updated turn item
  EventSink->>EventSink: Normalize recorded call output
Loading

Possibly related issues

  • Issue 23825 — It concerns deferred MCP App writes and accessible results. This change records successful write arguments and results, but the supplied summary does not show a change to form display.

Suggested labels: bugfix

Suggested reviewers: maria-rcks, juliusmarminge

Merge Risk: Minimal · up to 09cb4

No merge-blocking issue was established in the reviewed call-recording flow; merge after normal checks.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly and concisely describes the primary fix: preserving submitted MCP App form results so they do not reopen empty.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description check Passed The description covers the problem, expected behavior, implementation, scope, approval context, verification results, limitations, tests, and UI evidence. It identifies the follow-up issue and explain…
Full details: Description check

Explanation

The description covers the problem, expected behavior, implementation, scope, approval context, verification results, limitations, tests, and UI evidence. It identifies the follow-up issue and explains why the change is focused. It does not include an explicit maintainer approval comment, but the remaining required information is complete.

Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@github-actions github-actions Bot added size:L 100-499 changed lines (additions + deletions). and removed size:XXL 1,000+ changed lines (additions + deletions). 📱 Native Change Changes the native fingerprint; merging blocks production OTAs until a new store build ships. labels Oct 26, 2026

maria-rcks commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator

Note

Written by gpt-6.1-sol on behalf of Maria

kept @shivamhwp's commits and the original persistence design. merged current main and retargeted this pr to main, so the inherited deferred-form fix is included without waiting on https://git.cafe/pingdotgg/t3code/pulls/23850.

fixed two nearby regressions: the transactional merge now identifies recording by command type, so later item/status updates survive; web preserves an equal app reference so recording does not tear down the live app host. no new test files, dependencies, rpc, or settings.

verification on 1262ccf90fcbb0c82e8be88f375504b2e4dd176c:

  • regression first: main dispatched no recording; the original transactional merge discarded a later completed status. both reproduced in the existing tests before fixing.
  • blacksmith vp run check:changed passed formatting, lint, knip, generated-code checks, 30 affected package typechecks, and 268 tests across 11 files.
  • real codex 0.164.0 → local stdio mcp server → web client: submit, confirmation, successful result, reload, and retained successful result passed. main reopened the empty form after its successful write.
  • two independent gpt-6.1-sol reviews approve this head.
  • unverified: github's hosted mcp service, native mobile, electron, and remote/relay/tunnel. existing open apps on another device still refresh only when reopened; oversized results keep the original provider output.

before, after a successful write and reload on main:

main reopens the empty submitted form

after, submitted result retained after reload:

the submitted result survives reload

submit, confirmation, success, and reopened success

macroscope's human-approval explanation mentions infrastructure, deployment, and configuration changes from the former stacked comparison. the current diff against main contains only the 17 form-rendering/persistence files. its correctness check passes; effect-conventions and ui-consistency are skipped on the final head. human approval remains a landing gate.

final ci: github CI completed successfully on this head, with no failing or pending checks. macroscope's human-approval gate remains; no new correctness findings were posted. coderabbit could not review because its organization review budget is exhausted.

maria-rcks commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

verified unchanged head 99c900314732bf019e5fe77c461a832c6704f990 on this linux boat vm. no source changes or pushes.

  • installed and ran github/main (48a292364ab37574c67fb377b5227e5d281d9203) in ~/main, then stopped it before running the pr. drove both web clients with the repo's playwright and /usr/bin/google-chrome.
  • created a real project, thread, and codex gpt-6.1-sol turn through the app. used github-mcp-server v2.0.3 over stdio against a disposable private repository, with no mocks or seeded scenario data.
  • main: github.issue_write returned the real deferred-form result (isError: true, awaiting submission), but no form appeared. github had zero issues.
  • pr: the same request displayed the prefilled form. canceling the confirmation left the form usable and github still had zero issues. confirming created exactly one issue with the requested title and body.
  • the success view survived reload, opening in another browser context, and restart with this task's app-created state copied into an explicit isolated home. live ~/.t3/userdata was untouched; the initial root-checkout run used its separate ~/.t3/dev directory.
  • a subsequent real codex github.issue_read turn returned the created issue's title and url. another reload still showed success. github still had exactly one issue. read-only database inspection confirmed both the original deferred result and t3McpAppCall were retained.
  • blacksmith: six focused files passed 194 tests. vp run check:changed becf853defb6127dbe4447860383777d69346b6f then passed format, lint, both knip passes, graphql generation checks, 31 affected package typechecks, and 265 tests across 11 files. the explicit base was needed because the worker snapshot had no origin/main history.

short runtime transcript

main, same request, no form:

main hides the real github deferred issue form

pr, submitted result retained after reload:

pr retains the created issue after reload

shortened recording of the real form, confirmation, success, and reload. embedded as a gif for github compatibility; the mp4 export was also checked through full playback and seeking:

real github issue submission and retained success after reload

Not verified

  • native ios/android and electron shells; this vm pass exercised the web client.
  • other providers, github's hosted mcp transport, and remote/relay/tunnel connections. this pass used real codex and github's released stdio server calling the real github api.
  • the separate post-submit reset on main could not be reached through github's deferred form because main hides that form entirely. the before capture proves that earlier missing behavior; the pr's full submission and persistence path was exercised.
  • oversized results and concurrent provider-resend races were covered by existing automated tests, not induced in the live app. an already-open app in another browser stayed on its prior view until reopened, matching the documented limitation.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants