Repository navigation
Conversation
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This changes the default mobile presentation of existing inline MCP apps from fixed-height rows to content-sized, host-reported rows and alters feed virtualization behavior. Because the change affects the production mobile path and product-default UI behavior, human review is warranted. You can add or adjust custom eligibility rules. Learn more. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughInline MCP app rows now use tracked, app-reported heights capped at 420 pixels. ThreadFeed no longer assigns MCP app rows a fixed height. ChangesInline MCP app sizing
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to Short inline apps can shrink while tall apps remain capped. No actionable merge-blocking issue was established. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to Embedded apps gain bounded control over their own displayed height, not additional permissions. Heights remain between 80 and 420 points. Delayed-message behavior during document replacement has not been verified, but the observed sizing path affects presentation rather than privileged operations. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
What Changed
Size inline mobile MCP Apps from their
ui/notifications/size-changedheight and let the feed measure the resulting row. Preserve the existing 420-point cap for taller apps and fullscreen sizing. Reset the initial height when a captured app or its document changes.Why
The mobile host supplied an exact 420-point height, ignored app size notifications, and reserved a fixed 428-point feed row. Short cards consequently left large gaps. Supplying
maxHeightlets the app report its natural content height; the sample document now occupies 150 points plus the existing 8-point row margin.Fixes #17032. Independent of the theme and sandbox-cookie fixes in #17026 and #17027; this PR only changes the mobile host and feed sizing.
UI Changes
Same fictional document, iPhone 17 simulator, portrait, dark mode, and 370-point embed width. Crops contain only the embed. The height difference is the behavior being fixed.
Reusable sanitized fixture bundle. The fixture preserves captured vendor HTML while replacing company and conversation data; it runs in a disposable T3 home.
Verification
vp test run apps/mobile/src/features/threads/mcpAppSizing.test.ts: 9 cases passed, covering invalid heights, rounding, minimum, and maximum.vp run --filter @t3tools/mobile typecheck: passed.ThreadFeed.tsxremain.Checklist
Model: GPT-6.1 Sol | Harness: Codex in T3 Code