Uh oh!
There was an error while loading. Please reload this page.
feat(mobile): pick, share, and receive files in threads - #231
Merged
Conversation
Adopted from upstream `pingdotgg/t3code#8237` (`86c9a9288`). Mobile gains file attachments: the composer's + control adds Files when the server supports uploads, the system share sheet can send a file into Pylon, and received files can be saved or opened elsewhere. Brings a new `features/sharing` surface, upload/download helpers, and outbox support. Conflict resolution notes, because the shape is not obvious: Upstream landed this at 13:35 on the same day it landed #8793 at 12:22, so #8237's tree already contains #8793's composer restructure. Every ThreadComposer conflict here was that restructure re-arriving as context, not the attachment feature. Pylon declined it in #223 to keep ComposerToolbarScroller and the thirteen toolbar controls that need it, so it is declined again: `useLayoutEffect` (only used by the ComposerSurface radius animation) and the collapsed-row wrapper are not taken. Upstream's conflict side also carried Pylon's attachment and send blocks wholesale. Pylon's copies of those sit below the conflict as context and include `ContextWindowIndicator`, which upstream has no equivalent of, so taking that side would have duplicated all three. The file-glyph rendering is ported into Pylon's own block instead, reading `--color-icon-subtle` rather than upstream's hardcoded `#a3a3a3`. `onQueueFollowUp` has no upload step, so it now filters to images, matching what `projectThreadStartTurn` does when no upload has run. Queuing a follow-up that carries a file is a known gap, recorded rather than papered over with a cast. 41 of the 43 changed source files apply upstream's diff line-for-line; the two that differ are the two resolved by hand above.
The previous commit filtered files out of onQueueFollowUp and called it a queue limitation. That was wrong, and worse than it looked. Pylon's follow-up command already takes the same attachment union as thread.turn.start (orchestration.ts), its own comment says the server normalizer branches on `"dataUrl" in attachment` for both, and web passes identical attachments to followUpInputQueue and startTurn. Files were always supported; the filter was a fix for a type error dressed up as a design decision. The consequence was silent data loss on an ordinary path, not an edge case: the composer's send button routes to handleQueueFollowUp whenever a session is running, so attaching a file mid-turn and pressing send filtered it out of the payload, and because clearComposerDraftContent ran without deferAttachmentCleanup the sweep then deleted the local copy. Composer cleared, message queued without the file, no warning, bytes gone. Now mirrors onSendMessage and use-project-actions: enforce the attachment cap, validate against the server's advertised limits, upload through prepareTurnAttachments, defer cleanup until the queue call succeeds, release minted uploads and restore with allowOverflow on failure. Also switches the four file glyphs to `tintColorClassName` rather than upstream's hardcoded #a3a3a3. An earlier attempt subscribed to the theme directly and tripped no-mobile-uniwind-theme-escape-hatches; AppSymbol exists precisely so callers do not do that.
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: Scenario and decoded snapshot size10 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.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Third of the mobile batch. Adopted from upstream
pingdotgg/t3code#8237(86c9a9288). Follows #223 and #224.Mobile gains file attachments: the composer's + offers Files when the server supports uploads, the system share sheet can send a file into Pylon, and received files can be saved or opened elsewhere. Brings a new
features/sharingsurface, upload/download helpers, and outbox support.Port fidelity
41 of the 43 changed source files apply upstream's diff line-for-line. The two that differ are the two resolved by hand, and I diffed the diffs to confirm the deltas are only what was intended.
7 files conflicted. The shape is not obvious, so it's worth recording:
Upstream landed this at 13:35 the same day it landed #8793 at 12:22, so #8237's tree already contains #8793's composer restructure. Every
ThreadComposerconflict here was that restructure re-arriving as context, not the attachment feature. Pylon declined it in #223 to keepComposerToolbarScrollerand the thirteen toolbar controls that need it, so it is declined again —useLayoutEffect(used only by the ComposerSurface radius animation) and the collapsed-row wrapper are not taken.Upstream's conflict side also carried copies of Pylon's attachment strip and stop/send blocks. Pylon's own copies sit below the conflict as context and include
ContextWindowIndicator, which upstream has no equivalent of — taking that side would have duplicated all three. The file-glyph rendering is ported into Pylon's block instead.Review confirmed no Pylon features were lost: occurrence counts match
origin/pylonexactly forComposerToolbarScroller(10),ContextWindowIndicator(3),QuickQuestionTrigger(6),sessionGoal(14),renderCodexDirectivesForCopy(2), and all 12ComposerToolbarButtoninstances. No duplication. Pylon identity intact across all three app variants.A real bug this PR introduced and then fixed
Worth calling out because the first commit got it wrong.
onQueueFollowUporiginally filtered files out, described as a queue limitation. It isn't one: Pylon's follow-up command already takes the same attachment union asthread.turn.start, its own contract comment says the server normalizer branches on"dataUrl" in attachmentfor both, and web passes identical attachments tofollowUpInputQueueandstartTurn. The filter was a fix for a type error dressed as a design decision.The consequence was silent data loss on an ordinary path — the composer's send button routes to
handleQueueFollowUpwhenever a session is running. Attaching a file mid-turn and pressing send filtered it from the payload, and becauseclearComposerDraftContentran withoutdeferAttachmentCleanup, the sweep then deleted the local copy. Composer cleared, message queued without the file, no warning, bytes gone.Now mirrors
onSendMessage: enforce the cap, validate against the server's advertised limits, upload viaprepareTurnAttachments, defer cleanup until the queue call succeeds, release minted uploads and restore withallowOverflowon failure. This also makes the docs sentence "queued messages keep their files until they deliver" true, which it wasn't.Pylon-first details
The four file glyphs use
tintColorClassName="accent-icon-subtle"rather than upstream's hardcoded#a3a3a3. A first attempt subscribed to the theme directly and trippedno-mobile-uniwind-theme-escape-hatches—AppSymbolexists precisely so callers don't do that.docs/user/composer.mdwas hand-merged to keep Pylon's voice and branding; upstream's text said "T3 Code".Verification
@t3tools/mobiletypecheck clean, lint clean, 1021 tests passing across 141 files (including 15 new test files this brings).Not yet run on a simulator. The share-sheet path in particular wants a device pass.
Filed, not fixed
Three upstream design issues found in review, filed rather than folded in to keep this to one concern: #228 (share inbox has no cap, expiry, or way out), #229 (mixed image+file share skips the image size gate), #230 (one corrupt share file permanently disables attachment cleanup).
Reviewed and integrated with Claude Opus 5 in Claude Code.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.