sync: port upstream mobile perf, OTA updates, Thread List v2 default (#4607–#4984) - #194
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
vouch:trusted
|
@coderabbitai review |
✅ Action performedReview finished.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 694e87b1e6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Roll back by reference, not messageId: a retry enqueue with the same | ||
| // id may have optimistically replaced this attempt while the write was | ||
| // in flight, and its entry must survive this attempt's failure. | ||
| setMessages(currentMessages().filter((candidate) => candidate !== message)); |
There was a problem hiding this comment.
When a same-ID retry replaces an already persisted queued message and its storage.write fails, this rollback removes the replacement without restoring the previous in-memory entry. The old payload can remain on disk under that message ID, so it disappears for the current session but returns after restart and may dispatch stale work. Capture and restore the displaced entry when the replacement cannot be persisted.
AGENTS.md reference: AGENTS.md:L58-L62
Useful? React with 👍 / 👎.
| // append: the merge path slots existing attachments first and truncates | ||
| // at the send limit, which would silently drop this message's images if | ||
| // the user attached new ones while the write was in flight. | ||
| void mergeComposerDraftContent(threadKey, { text, attachments: [] }); |
There was a problem hiding this comment.
When the outbox write fails, mergeComposerDraftContent performs another filesystem write and can reject as well—for example, under the same disk-full condition—but this discarded promise has no rejection handler. That produces an unhandled rejection and can leave the restored prompt only in memory, so it is lost after a restart. Await or catch the restoration failure and report it through the existing error path.
AGENTS.md reference: AGENTS.md:L58-L62
Useful? React with 👍 / 👎.
| useEffect(() => { | ||
| if (environmentId !== null && environmentId !== previousEnvironmentIdRef.current) { | ||
| previousEnvironmentIdRef.current = environmentId; | ||
| setPathInput(getAddProjectInitialQuery(environmentBaseDirectory)); |
There was a problem hiding this comment.
When the environment is already selected before its server config arrives, pathInput is initialized from a null base directory and this effect ignores the later environmentBaseDirectory update because the environment ID did not change. Both local-folder and clone-destination screens then start at ~/ instead of the configured add-project base directory; update untouched initial input when the current environment's config becomes available.
Useful? React with 👍 / 👎.
…erer (19.2.3) (pingdotgg#4675) (cherry picked from commit 1153afb)
(cherry picked from commit a148e08)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 476d69c)
(cherry picked from commit b0c4992)
(cherry picked from commit d25b157)
(cherry picked from commit 9cf9fc9)
Co-authored-by: Claude <noreply@anthropic.com> (cherry picked from commit f1a68ac)
Co-authored-by: codex <codex@users.noreply.github.com> (cherry picked from commit 936593c)
Co-authored-by: codex <codex@users.noreply.github.com> (cherry picked from commit 8fe8f9a)
(cherry picked from commit 1b61ce1)
(cherry picked from commit 9e3e9bb)
(cherry picked from commit 758deca)
…dotgg#4943) (cherry picked from commit 00da6b5)
…ngdotgg#4882) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit f0f16e4)
(cherry picked from commit 2652fee)
…ingdotgg#4867) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 90f3913)
Co-authored-by: codex <codex@users.noreply.github.com> (cherry picked from commit 6154b46)
) (cherry picked from commit 323dc32)
16e9018 to
fe91fe1
Compare
694e87b to
857a5d8
Compare
Review triageEvery finding on this PR was checked against Deliberately not fixing them here: patching upstream bugs inside a 101-commit port would diverge the fork in exactly the files this stack exists to realign, and would make the next sync harder. They are better raised upstream or tracked as fork-local follow-ups on their own branch, where they can be reviewed on their merits. Notably, the files where this PR did carry fork-specific conflict resolutions drew no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 857a5d85c9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (options.changeRequestState === "merged" || options.changeRequestState === "closed") { | ||
| // Only an idle thread settles on the merge signal: the signal itself | ||
| // never clears, so without this guard fresh activity (a message sent in | ||
| // a settled thread) would re-settle the moment its turn completed. | ||
| const lastActivityAt = threadLastActivityAt(shell); | ||
| if ( | ||
| lastActivityAt === null || | ||
| Date.parse(lastActivityAt) < Date.parse(options.now) - CHANGE_REQUEST_SETTLE_IDLE_MS | ||
| ) { | ||
| return true; | ||
| } | ||
| return true; |
There was a problem hiding this comment.
When a user sends follow-up work to a merged or closed change-request thread, the server clears its settled override for that activity, but once the turn finishes the activity blockers disappear while the change-request state remains permanent; this unconditional return therefore moves the thread straight back into the settled tail. Restore an idle guard (or another durable active state) so recently active follow-up work remains visible instead of re-settling immediately.
AGENTS.md reference: AGENTS.md:L58-L62
Useful? React with 👍 / 👎.
| // at the send limit, which would silently drop this message's images if | ||
| // the user attached new ones while the write was in flight. | ||
| void mergeComposerDraftContent(threadKey, { text, attachments: [] }); | ||
| appendComposerDraftAttachments(threadKey, attachments); |
There was a problem hiding this comment.
If the outbox write fails after the user has attached images to a new draft, this uncapped append combines those new images with every attachment from the failed message and can exceed PROVIDER_SEND_TURN_MAX_ATTACHMENTS (8). The next send then queues a payload outside the provider send contract instead of asking the user which images to retain, so cap or otherwise reconcile the two attachment sets while reporting any overflow.
AGENTS.md reference: AGENTS.md:L58-L62
Useful? React with 👍 / 👎.
| let providers = drop.dropSession.items.map(\.itemProvider) | ||
| guard !providers.isEmpty, | ||
| providers.allSatisfy({ $0.canLoadObject(ofClass: UIImage.self) }) else { | ||
| return nil |
There was a problem hiding this comment.
When an iPadOS drop contains both an image item and a non-image item, allSatisfy rejects the entire drop and falls back to UIKit's suggested performer. UIKit can then insert the image as a regular NSTextAttachment; serializedText cannot preserve that attachment and onComposerPasteImages is never emitted, so the dragged image disappears from the controlled composer. Intercept the image providers even when the same drop also contains other item types, while explicitly preserving or inserting the remaining content.
Useful? React with 👍 / 👎.
6 participants
L5/7 of the 2026-07-30 upstream sync stack (20 commits). Stacked on #193.
What lands
@legendapp/list3.2.0 → 3.3.3Fork deviations preserved
AddProjectScreen: kept the fork'sincomingShareIdargument touseCreateProjectwhile adopting upstream'suseBrowsePathInputhook (which internalizesgetAddProjectInitialQuery, so behavior is unchanged).NewTaskRouteScreen: combined upstream'shasReadyEnvironmentgate with the fork's share-id params on both the toolbar action and the header button.Stack.tsx: added upstream'sThreadOutboxDrainWorkerwhile keeping the fork'sSHOWCASE_ENABLEDgate onShowcaseCaptureCoordinator.ThreadComposer: upstream's live-activity arming runs after a successful send, inside the fork's unhandled-rejection guard.pnpm-workspace.yaml: took the@legendapp/listbump but held@pierre/diffsat beta.5 — that bump belongs to Fix editable file focus and live syntax highlighting pingdotgg/t3code#3979, which lands in L6.HomeScreen: took only the toolbar-clearance half of fix(mobile): support pre-Liquid-Glass iOS bottom toolbar pingdotgg/t3code#4984's hunk; the thread-search half arrives with feat(search): find threads by conversation content pingdotgg/t3code#4959 in L6.Verification
vp run typecheckandvp checkpass.vp run lint:mobilecould not be run locally — swiftlint 0.65.0 fails to loadsourcekitdInProcon this host, identically onmain, so it is a local toolchain issue rather than a regression. CI runs it properly.