Uh oh!
There was an error while loading. Please reload this page.
fix(mobile): Catch up the thread feed after a long background - #6642
fix(mobile): Catch up the thread feed after a long background#6642mwolson wants to merge 335 commits into
Conversation
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI 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:
Comment |
Uh oh!
There was an error while loading. Please reload this page.
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — Despite the mobile-fix title, this is a 966-file orchestration V2 migration that changes production persistence, APIs, provider execution, authentication, MCP capabilities, and mobile synchronization behavior. It also changes defaults and static-analysis directives while unresolved comments identify stale-data, auth, UI, and functionality risks. Not approved because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
a50460b to
c610c31CompareUh oh!
There was an error while loading. Please reload this page.
c610c31 to
9159bcfCompareUh oh!
There was an error while loading. Please reload this page.
96891a3 to
993407dCompare5b5e9cb to
a005644Comparea005644 to
1eb46e5CompareUh oh!
There was an error while loading. Please reload this page.
541c25b to
26f73f5CompareUh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
26f73f5 to
6af29ddCompareUh oh!
There was an error while loading. Please reload this page.
Adapt grouped tool summaries and the floating working timer to V2 run, attempt, and queue state. Bring over the composer, keyboard, and disclosure transitions while retaining the V2 activity inspector and queue controls. Keep OV2 web composer and grouping behavior intact; share only the existing command label parser with mobile.
Restores main features dropped by the policy replay: pingdotgg#8569 theme wiring, pingdotgg#8850 composer banner follow-ups, pingdotgg#8855/pingdotgg#8904 composer fixes, pingdotgg#8831 settings search rework, pingdotgg#8803 workspace-mutation refresh (v2-adapted), pingdotgg#8840 circle-alert, pingdotgg#8584 codex artifact templates, pingdotgg#8688/pingdotgg#8807/pingdotgg#8936 video + image previews (web and mobile, v2-adapted), pingdotgg#8862 Expo glass, and the round's docs. Timeline thinking rows (pingdotgg#8984) stay on the v2 work-live system. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
a6e0049 to
50a57b6CompareThe v2 equivalents of main's pingdotgg#8984 and pingdotgg#8922: a "Working for ..." header anchors the active run, the trailing live tool row survives between actions in past tense instead of vanishing, and a shimmering Thinking row marks reasoning gaps. During workspace preparation the header shows "Setting up worktree..." (driven by the local dispatch flag or the v2 run's preparing status, so remote viewers see it too), the composer footer span is gone, and draft promotion waits until the run starts or startup fails instead of navigating mid-preparation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Clerk can sit on isLoaded=false after a background freeze, which pins Settings on Checking and blocks the account screen. Remount Clerk after a foreground timeout, keep T3 Account tappable, and offer Retry without clearing app data.
A cold HTTP seed still waits for synchronized, but disconnect and stream error cleared awaitingCompletion. Reconnect then published live while replay was still arriving. Keep the fence until synchronized or delete.
A 12-second foreground timer used to remount ClerkProvider, which rebuilds the whole app. Checking now stays until Clerk loads or the user taps Retry. Retry is still the only remount.
5755e9e to
6ff4bf9CompareUh oh!
There was an error while loading. Please reload this page.
| const next = b as typeof a; | ||
| return ( | ||
| a.runId === next.runId && | ||
| a.groupId === next.groupId && | ||
| a.hiddenCount === next.hiddenCount && | ||
| a.expanded === next.expanded && | ||
| a.onlyToolActivities === next.onlyToolActivities | ||
| ); |
There was a problem hiding this comment.
🟡 Mediumlib/threadActivity.ts:180
computeStableThreadFeedEntries reuses a stale work-toggle row when its group ID and count are unchanged, so live label, completion/failure status, and shimmer updates are not rendered. threadFeedEntryIsUnchanged must also compare summary, summaryKind, hasFailure, live, and shimmer before reusing the previous row.
| constnext=bastypeofa; | |
| return( | |
| a.runId===next.runId&& | |
| a.groupId===next.groupId&& | |
| a.hiddenCount===next.hiddenCount&& | |
| a.expanded===next.expanded&& | |
| a.onlyToolActivities===next.onlyToolActivities | |
| ); | |
| constnext=bastypeofa; | |
| return( | |
| a.runId===next.runId&& | |
| a.groupId===next.groupId&& | |
| a.hiddenCount===next.hiddenCount&& | |
| a.expanded===next.expanded&& | |
| a.onlyToolActivities===next.onlyToolActivities&& | |
| a.summary===next.summary&& | |
| a.summaryKind===next.summaryKind&& | |
| a.hasFailure===next.hasFailure&& | |
| a.live===next.live&& | |
| a.shimmer===next.shimmer | |
| ); |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/lib/threadActivity.ts around lines 180-187:
`computeStableThreadFeedEntries` reuses a stale `work-toggle` row when its group ID and count are unchanged, so live label, completion/failure status, and shimmer updates are not rendered. `threadFeedEntryIsUnchanged` must also compare `summary`, `summaryKind`, `hasFailure`, `live`, and `shimmer` before reusing the previous row.
| return "present" as const; | ||
| } | ||
| case "missing": { | ||
| yield* setDeleted(); |
There was a problem hiding this comment.
🟠 Highstate/threads.ts:480
A stale refreshAuthoritativeSnapshot response can mark an already-reconnected thread as deleted and remove its cache. The missing branch calls setDeleted() without verifying that the response belongs to the current prepared connection/session, so a delayed 404 from the old connection clears newer live data; add a request/session fence before treating this result as authoritative.
🤖 Copy this AI Prompt to have your agent fix this:
In file @packages/client-runtime/src/state/threads.ts around line 480:
A stale `refreshAuthoritativeSnapshot` response can mark an already-reconnected thread as deleted and remove its cache. The `missing` branch calls `setDeleted()` without verifying that the response belongs to the current prepared connection/session, so a delayed 404 from the old connection clears newer live data; add a request/session fence before treating this result as authoritative.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6ff4bf9. Configure here.
| a.expanded === next.expanded && | ||
| a.onlyToolActivities === next.onlyToolActivities | ||
| ); | ||
| } |
There was a problem hiding this comment.
Stable feed skips live work-toggle updates
Medium Severity
threadFeedEntryIsUnchanged treats a work-toggle as unchanged without comparing summary, summaryKind, hasFailure, live, or shimmer. Combined with reference-equal itemsAreEqual, LegendList keeps the old row, so the group label, failure mark, and shimmer can stay frozen through a live turn.
Reviewed by Cursor Bugbot for commit 6ff4bf9. Configure here.
| DateTime.toEpochMillis(activity.projectedItem.item.updatedAt) === | ||
| DateTime.toEpochMillis(nextActivity.projectedItem.item.updatedAt) | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Stable feed freezes activity live state
Medium Severity
threadFeedActivitiesAreUnchanged ignores lifecycleStatus and live. Presentation derives those from the active run, not only from item text or updatedAt, so an in-progress tool group can keep the previous activity objects after the run settles.
Reviewed by Cursor Bugbot for commit 6ff4bf9. Configure here.
| return; | ||
| } | ||
| return cleanUpAccount(previous, previousObservedAccount ?? null); | ||
| }); |
There was a problem hiding this comment.
Clerk remount can wipe the account
Medium Severity
Retry remounts Clerk while observedAccountRef still holds the signed-in user. If the new instance reports loaded and signed out, the bridge treats that as a real sign-out, deactivates the relay, and runs account cleanup.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 6ff4bf9. Configure here.
6081201 to
95bacddCompareThe pingdotgg#6642 row-sharing replay inserted computeStableThreadFeedEntries after hooks that read presentedFeed. Metro rewrites const to var, so those hooks observed presentedFeed as undefined on every render and never updated previousPresentedFeedRef or disclosure enter ids.
Local and anchored messages have no projectedItem. Comparing sourceItemId through it threw when the feed was rebuilt.
There was a problem hiding this comment.
Cross-cutting note (not repeated inline): the panel's row geometry now lives in apps/web/src/components/chat/threadDetailsPanelStyles.ts as class-string constants that six call sites splice into Button/SelectTrigger/ComboboxTrigger, overriding the primitive's height, radius, padding, text size and base/hover background — the hover tint needs !bg-... to win against the variant it is overriding. Since this is durable geometry shared by every panel row, a named Button size/variant (e.g. size="panel-row") would carry it inside the primitive contract and drop the !important overrides, leaving only contextual layout at the call sites. Not blocking this PR, but worth doing before more rows adopt the constants.
Posted via Macroscope — UI Consistency
| inset-inline-end: var(--thread-details-panel-inset); | ||
| } | ||
| .alert-glass { |
There was a problem hiding this comment.
alert-glass already exists as @utility alert-glass (line 284) with these exact declarations plus a @supports not ((-webkit-backdrop-filter: blur(1px)) or (backdrop-filter: blur(1px))) opaque fallback. This copy is in @layer components, and since both background declarations are !important the cascade reverses layer order, so the components copy wins — the utility's no-backdrop-filter fallback stops applying and the banner renders translucent where blur is unsupported.
Suggest dropping this block together with the four .alert-glass[data-variant=...] rules below it and leaving the @utility as the single owner (the alert-glass class name in ProviderStatusBanner already comes from it).
Posted via Macroscope — UI Consistency
| size="xs" | ||
| className="ps-[8.5px]" | ||
| className={isPanel ? THREAD_DETAILS_PANEL_SPLIT_PRIMARY_CLASS : undefined} |
There was a problem hiding this comment.
Toolbar mode lost the ps-[8.5px] optical padding this leading-icon button carried (same for the disabled variant above, line 1777). OpenInPicker keeps ps-[8.5px] for its matching toolbar button, so the two grouped controls in the header no longer align on their icon.
| className={isPanel ? THREAD_DETAILS_PANEL_SPLIT_PRIMARY_CLASS : undefined} | |
| className={isPanel ? THREAD_DETAILS_PANEL_SPLIT_PRIMARY_CLASS : "ps-[8.5px]"} |
Posted via Macroscope — UI Consistency
| {props.children} | ||
| </ul> | ||
| {props.hiddenCount > 0 ? ( | ||
| <button |
There was a problem hiding this comment.
This row rebuilds the panel row by hand (h-9, rounded-lg, px-2.5, text-[13px], gap-2.5, plus the light/dark hover tint) instead of composing Button, and unlike every other row in this file it has no focus-visible ring — so the one control that pages the list gives keyboard users no visible target, even though the comment above relies on rows being reachable by keyboard.
Suggest rendering it as <Button size="sm" variant="ghost" className={cn(THREAD_DETAILS_PANEL_ROW_CLASS, "text-muted-foreground/70 hover:text-foreground/80")}> (adding THREAD_DETAILS_PANEL_ROW_CLASS to the existing import from ./threadDetailsPanelStyles): same geometry and hover tint, with the primitive's focus ring and disabled semantics back.
Posted via Macroscope — UI Consistency
| if (item.kind === "open_pr") { | ||
| void openExistingPr(); | ||
| return; | ||
| } | ||
| if (item.dialogAction === "push") { |
There was a problem hiding this comment.
resolveQuickAction and buildMenuItems in packages/client-runtime/src/state/gitActions.ts still emit kind: "open_pr" ("View PR") whenever the branch has an open pull request and is clean, but every handler for it was removed here: this menu item now falls through to setIsCommitDialogOpen(true) and opens the Commit dialog, runQuickAction ends at if (quickAction.action) so the quick-action button does nothing, and GitQuickActionIcon falls through to InfoIcon. openExistingPr is left with no callers.
Either keep routing open_pr, or stop emitting it now that ThreadDetailsPrRow owns the pull request row:
if (item.disabled) return;
+ if (item.kind === "open_pr") {+ void openExistingPr();+ return;+ }
if (item.dialogAction === "push") {Posted via Macroscope — UI Consistency
| @@ -759,6 +762,182 @@ html[data-mobile-composer-route-transition="true"]::view-transition-old(t3-mobil | |||
| outline: none; | |||
| } | |||
| .surface-subheader { | |||
There was a problem hiding this comment.
.surface-subheader has no consumers: the four subheader call sites (DiffPanelShell, PreviewChromeRow, FileBrowserPanel, FilePreviewPanel) keep their own Tailwind classes and only set the data-surface-subheader attribute that the rule below matches. As written this rule never renders and the header geometry has two competing definitions. Suggest either applying the class at those call sites or dropping the rule.
Posted via Macroscope — UI Consistency
| --alert-glass-tint: var(--warning); | ||
| } | ||
| .glass-opacity-slider { |
There was a problem hiding this comment.
.glass-opacity-slider has no consumer, literal or dynamic: the glass-opacity input in settings/SettingsPanels.tsx uses className="settings-slider ..." and sets --settings-slider-progress/--settings-slider-fill-offset, and nothing anywhere reads --glass-slider-progress. That leaves ~110 lines of vendor pseudo-element CSS that never renders and duplicates .settings-slider below it (same track, thumb, hover/active, focus-visible and forced-colors treatment).
Suggest dropping this block and keeping .settings-slider as the single slider owner — or, if the glass slider was meant to diverge, pointing that input at this class so the rule has an owner.
Posted via Macroscope — UI Consistency
| className="z-[130] h-(--positioner-height) w-(--positioner-width) max-w-(--available-width) transition-transform data-instant:transition-none" | ||
| collisionAvoidance={collisionAvoidance} | ||
| className={cn( | ||
| "z-[130] h-(--positioner-height) w-(--positioner-width) max-w-(--available-width) transition-[top,left,right,bottom,transform] data-instant:transition-none", |
There was a problem hiding this comment.
This makes every popover in the app transition its positional properties, while the only new consumer immediately opts out (positionerClassName="... !transition-none" in PanelLayoutControls). Base UI writes the positioner's top/left inline styles on each anchor-tracking update — scroll, resize, collision flip — so unrelated popovers will now animate toward their anchor and visibly trail it while a surface scrolls.
Keeping the primitive on transition-transform and opting in per call site via the new positionerClassName avoids changing behavior for popovers that did not ask for it:
| "z-[130] h-(--positioner-height) w-(--positioner-width) max-w-(--available-width) transition-[top,left,right,bottom,transform] data-instant:transition-none", | |
| "z-[130] h-(--positioner-height) w-(--positioner-width) max-w-(--available-width) transition-transform data-instant:transition-none", |
Posted via Macroscope — UI Consistency
Macroscope skipped reviewing this pull request. Per-review cost limit exceeded (workspace setting). This review would cost an estimated $249.67, which exceeds your per-review limit of $10.00. The top 3 files driving up this estimate:
Tip To get this pull request reviewed, you can:
|
What Changed
On a large thread, every live token used to redraw the whole feed. Unchanged
rows now keep the same objects, so only the row that changed updates.
Coming back to a thread used to fetch the entire transcript. Warm opens
subscribe from the cached sequence, refresh a bounded snapshot in the
background, and replace a full socket fallback with that bounded window.
A short lock could flip a live thread to Syncing. A long lock could show old
messages as current, or sit on Syncing forever. A short lock or Home switch
stays live. After a long lock the feed shows Syncing over the cached transcript
until a socket snapshot or completion marker lands the latest content. HTTP can
refresh the visible window under that pill. Catch-up still ends on the socket
snapshot or marker. Reconnect restarts the live subscribe so the new session
actually gets a thread subscription.
T3 Account in Settings could sit on Checking after a long background, and
tapping it did nothing. The account row stays tappable. While Clerk is still
loading, the account screen shows Checking plus Retry. Retry remounts Clerk.
There is no 12-second timer and no automatic remount.
Checked on a signed iPhone, an Android tablet, the iOS Simulator, and Waydroid.
A 30-second lock stays live. A long lock shows Syncing, then catches up.
Why
The phone already had the recent history. Live updates, warm opens, and
reconnects were throwing that away or treating it as finished too soon. Catch-up
should look like Syncing until the latest rows are really there.
The Clerk hang is the same freeze after a long background: Settings looked busy
forever. Remounting Clerk from a timer tore down the whole app, so recovery is
manual Retry only.
UI Changes
Live turns no longer rebuild the whole feed. Warm opens stay on the recent
window. Short background stays live. Long background shows Syncing until latest
content lands. T3 Account stays Checking until Clerk loads, then Sign in or the
account email. Retry is on the account screen while Checking.
No before/after screenshots.
Known Limitations
On a Boox NoteAir5C, Clerk can load and still sit on Sign in. Retry remounts
Clerk and does not complete sign-in. Force-stopping the app recovers. That is
native Clerk or Android process state, not hung Checking.
Checklist
Note
High Risk
Changes thread live/synchronizing transitions, bounded snapshot persistence, and shell “live” semantics that affect outbox delivery—race-prone areas per review notes. Broad mobile V2 migration plus removal of the thread-transfer PR reporter also affect CI signal and regression visibility.
Overview
Thread sync and mobile runtime — Mobile wires into bounded thread snapshot loading, shared orchestration cache schemas, and
refreshCachedThreadOnSubscribe. Client-runtime thread state distinguishes catch-up vs completion waits, uses authoritative HTTP refresh on reconnect, and gates persistence while a bounded refresh is in flight. Shell status can promote to live when a usable cached snapshot exists. The mobile feed and related UI move to V2 projections (runtime vs session,RuntimeRequestId, checkpoint summaries derived from projection), with archive guards keyed off runtime status and approval/user-input cards respectingresponseCapability.Cloud auth and navigation — T3 Account stays navigable while Clerk is loading; the auth screen shows Checking with a manual Retry that remounts Clerk without timers. Cloud cleanup refs live on the parent so Clerk remounts do not drop in-flight sign-out work. In-memory navigation state is restored on app launch.
CI and housekeeping — The Thread Transfer Report workflow and its publisher scripts/tests are removed. CI installs
build-essentialso ACP process-tree fixtures compile instead of soft-skipping. Smaller updates: desktop user-data dir names in tests, composer context-strip default, mobile theme adaptive colors, README appearance doc link, marketing Cursor harness label.Reviewed by Cursor Bugbot for commit 088b2ab. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add orchestration V2 foundation and catch up mobile thread feed after background
ProviderAdapterV2service boundaryProjectionStoreV2,EffectOutboxandEffectWorkerfor durable effect processing,ProviderSessionManager,ProviderRuntimeRecoveryService, checkpoint capture/rollback, thread management, and MCP orchestrator/worktree toolkitsruntimeandlatestRunfields, pending approvals and user-input checks live response capability, and thread snapshots load through bounded HTTP routesEnvironmentApi.orchestrationIPC property withorchestrationV2and switches all WebSocket RPC methods to V2 constants; environments advertising a protocol version mismatch are now rejected at connection time viaorchestrationProtocolCompatibilityErrorMacroscope summarized 088b2ab.