fix(server): MCP App forms now show when a write tool defers to them #23850 · pingdotgg/t3code

MmacroscopeappBotapproved these changes1d ago

Approvability

Verdict: Approved at 335e7e2

Macroscope's review found this PR approvable — This is a focused MCP App rendering fix: Codex-deferred write calls marked failed can now retain their app form, while unrelated failed tools remain ordinary tool rows. The server and web behavior are covered by targeted tests, with the mobile change being the same narrow presentation condition.

^{You can add or adjust custom eligibility rules. Learn more.}

CcoderabbitaiBot1d ago

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Source mode: Auto
Repository: pingdotgg/t3code
Branch: main
File: .coderabbit.config.ts
Commit: 2c8bdf6
Layers: Repository TS config
Review profile: CHILL
Plan: Advanced
Run ID: cf256265-277c-4b36-b8d9-57ab76d86b3f

📥 Commits

Reviewing files that changed from the base of the PR and between 2c8bdf6177159d8fcd12a897d7c9311f6eb93237 and 335e7e2fdfc0daa8dfee7f6814c06f99678c25be.

📒 Files selected for processing: 5

apps/mobile/src/lib/threadActivity.ts
apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts
apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts
apps/web/src/session-logic.test.ts
apps/web/src/session-logic.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

The Codex adapter now captures MCP App resources for eligible failed results. The web and mobile clients now check dynamic-tool items for MCP Apps regardless of status.

Changes: Failed MCP App results

Layer / File(s)	Summary
Capture resources on failed results `apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts`, `apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts`	The adapter captures MCP App resources for results without errors, including failed items. It marks attached results as errors for failed items. A replay test checks the resource URI, error flag, and pending-work state.
Extract MCP Apps in client timelines `apps/web/src/session-logic.ts`, `apps/mobile/src/lib/threadActivity.ts`, `apps/web/src/session-logic.test.ts`	The web and mobile clients attempt MCP App extraction for dynamic-tool items regardless of status. A web test checks the MCP App entry for a failed GitHub tool call.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: juliusmarminge, maria-rcks

Merge Risk: ⚪ Minimal · up to 335e7

The change enables deferred MCP App forms to appear after failed tool calls. No actionable merge-blocking risk is identified beyond normal validation.

Pre-merge checks: ^Passed 4

Check name	Status	Explanation
Title check	Passed	The title is concise, specific, and accurately identifies the fix for MCP App forms deferred by write tools.
Linked Issues check	Passed	Issue `#23825` asks that a deferred MCP App result either shows the form or shows an understandable deferred state with an escape hatch that keeps the tool result accessible. The PR takes the first opti…
Out of Scope Changes check	Passed	All changed files serve the objective of showing deferred MCP App forms for failed-status tool calls. The CodexAdapterV2.ts change widens the app-capture condition and adds the `isError` flag. The ses…
Description check	Passed	The description includes all required sections: Problem, Change, Scope and approval, and Verification. It explains the deferred MCP App failure, identifies the affected components, links issue `#23825`,…

Full details: Linked Issues check

Explanation

Issue #23825 asks that a deferred MCP App result either shows the form or shows an understandable deferred state with an escape hatch that keeps the tool result accessible. The PR takes the first option. Verified at head: in CodexAdapterV2.ts, the app reference is now built whenever item.result != null && item.error == null, not only when item.status === "completed". For status === "failed" the captured output sets result: { ...item.result, isError: true }, so the app receives the isError flag. In session-logic.ts, deriveTimelineEntriesFromVisibleTurnItems calls mcpAppFromToolItem for every dynamic_tool item, so the web timeline creates the app entry for a failed call. The mobile change in threadActivity.ts applies the same status-independent extraction. The added tests in CodexAdapterV2.test.ts and session-logic.test.ts cover a failed call with a deferred-form result. The author's before/after screenshots show the form rendering, the confirmation prompt, and the submitted write. The screenshots are not independently verifiable from the repository, so the PASS relies on the diff and the described tests. The mobile path has no device run; the issue marks mobile as unverified, so this does not block the verdict.

Full details: Out of Scope Changes check

Explanation

All changed files serve the objective of showing deferred MCP App forms for failed-status tool calls. The CodexAdapterV2.ts change widens the app-capture condition and adds the isError flag. The session-logic.ts and threadActivity.ts changes remove the completed-only status gate in the web and mobile timelines. The CodexAdapterV2.test.ts and session-logic.test.ts additions test this behavior. No unrelated refactoring, feature, or configuration change appears in the reviewed diffs. The Codex-only scope matches the linked issue and the PR description.

Full details: Description check

Explanation

The description includes all required sections: Problem, Change, Scope and approval, and Verification. It explains the deferred MCP App failure, identifies the affected components, links issue #23825, documents focused automated tests, manual web and mobile verification, screenshots, limitations, and agent details.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

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

fix(server): MCP App forms now show when a write tool defers to them#23850

Problem

Change

Scope and approval

Verification

Before / after (Linux, web UI, Codex)

Tests

With GitHub's real MCP server (web, Codex)

Mobile (Android, POCO F4, Codex, GitHub's real server)

Not covered

Checks

Approvability

1	claude-code ui extension: no
2	codex (via T3) ui extension: yes

1	tools/call create_note {"title":"Ship the fix"} -> isError, awaiting_user_submission
2	tools/call create_note {"title":"Ship the fix","_ui_submitted":true} -> Created note "Ship the fix".