Mobile Connection Opt#254
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Owner
Author
|
@codex review |
📝 WalkthroughWalkthroughThis PR substantially rewrites iOS simulator launch target discovery via robust Xcode PBX parsing, introduces comprehensive auto-update state management with Promise-based flow control, adds per-lane deletion progress tracking and transactional lane cleanup, and replaces direct connection-state checks with a new health-based model across iOS sync and UI layers. Four independent architectural improvements span desktop and iOS platforms. ChangesiOS Simulator Launch Target Discovery
Auto-Update Flow & Quit-Install Control
Lane Deletion Progress & Transactional Cleanup
iOS Connection Health Model
Estimated code review effort🎯 4 (Complex) | ⏱️ ~90 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Tighten iOS sync/connection settings, expand iOS simulator service with tests, refactor auto-update service with broader test coverage, rework the desktop Lanes page, and refresh related docs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
arul28
force-pushed
the
ade/mobile-connection-opt-eebafbf3
branch
from
e20d167 to
5079548
Compare
Owner
Author
|
@codex review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/ios/ADE/Services/SyncService.swift (1)
6088-6100:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDon't return the "Reconnecting now" timeout when this branch skips reconnect.
When recent inbound traffic suppresses the reconnect path here, the request still fails with
SyncRequestTimeout.message, so the user is told ADE is reconnecting even though the socket stays up. Use a non-reconnect timeout message in this branch.Suggested fix
if disconnectOnTimeout, syncShouldReconnectAfterRequestTimeout( now: ProcessInfo.processInfo.systemUptime, lastInboundMessageAt: lastInboundMessageAt ) { // handleTransportFailure tears the socket down before flipping the // reduced-load preference, so we must NOT call markConnectionLoadStrained // here — calling it pre-teardown re-subscribes chat events on the // doomed socket. The transport-failure path strains the load itself. handleTransportFailure(timeoutError) } else { + let resolvedError = + disconnectOnTimeout + ? SyncRequestTimeout.error(message: "The host took too long to respond. Try again.") + : timeoutError markConnectionLoadStrained() - resolve(requestId: requestId, result: .failure(timeoutError)) + resolve(requestId: requestId, result: .failure(resolvedError)) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ios/ADE/Services/SyncService.swift` around lines 6088 - 6100, The else branch currently resolves the request with the same reconnect-oriented timeout error even when syncShouldReconnectAfterRequestTimeout(...) returns false; change the error used in that branch so it signals a non-reconnect timeout. Specifically, when disconnectOnTimeout is true but syncShouldReconnectAfterRequestTimeout(...) is false, replace the reconnecting timeoutError passed to resolve(requestId: requestId, result: .failure(timeoutError)) with a distinct non-reconnect timeout error (e.g., a new SyncRequestTimeout case or a different error instance/message) while keeping the existing calls to markConnectionLoadStrained() and not calling handleTransportFailure().
🧹 Nitpick comments (3)
apps/ios/ADE/Views/Components/ADEDesignSystem.swift (1)
453-521: ⚡ Quick winExtract the connection-health presentation once.
This health-to-UI mapping is duplicated again at Lines 622-689. The two copies already have enough branching that the next state or accessibility tweak is likely to diverge between the standalone button and the root toolbar. A single private helper/view-model for tint, glow, and label text would make this safer to evolve.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ios/ADE/Views/Components/ADEDesignSystem.swift` around lines 453 - 521, Extract the duplicated health-to-UI logic into one small helper (e.g., a private struct or view-model like ConnectionHealthPresentation or makeConnectionPresentation()) that reads syncService and SyncConnectionHealth and exposes the three properties used throughout the UI: tint (Color), showsConnectedGlow (Bool) and accessibilityLabel (String); move the existing computed logic from the private vars tint, showsConnectedGlow, truncatedHostName and accessibilityLabel into that helper (keeping truncatedHostName as an internal helper) and replace both copies in the file with a single instantiation/usage of that helper (refer to the existing symbols health, syncService, truncatedHostName, and the switch on health.transport to preserve the exact branching and text).apps/desktop/src/renderer/components/lanes/LanesPage.test.ts (1)
136-165: ⚡ Quick winNew
resolveLaneDeleteStartSelectiontests are correctBoth traces verify cleanly against the implementation:
- Test 1 — deleting the selected+pinned lane ("lane-b"):
isAvailable("lane-b")isfalse, so selection falls through to the first non-deleting entry infilteredLaneIds→ "lane-c"; "lane-d" is preserved in bothactiveLaneIdsandpinnedLaneIds. Assertions are exact.- Test 2 — deleting a non-selected split lane ("lane-c"): current selection "lane-b" is available, so
nextSelectedLaneIdstays; "lane-c" is dropped fromactiveLaneIdswhile "lane-d" is retained as a preserved active.One untested path worth adding: all
filteredLaneIdsare deleting, forcing the fallback throughsortedLaneIds. The current two cases don't exercise that branch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/components/lanes/LanesPage.test.ts` around lines 136 - 165, Add a new test to cover the branch where every id in filteredLaneIds is being deleted so the function falls back to sortedLaneIds; in the LanesPage.test.ts suite add a case for resolveLaneDeleteStartSelection where deletingLaneIds contains all filteredLaneIds (e.g., ["lane-b","lane-c","lane-d"]), set selectedLaneId to a deleting entry, and assert that nextSelectedLaneId and active/pinned behavior come from the first non-deleting id in sortedLaneIds (using the same symbols resolveLaneDeleteStartSelection, filteredLaneIds, sortedLaneIds, deletingLaneIds, selectedLaneId, activeLaneIds, pinnedLaneIds).apps/desktop/src/renderer/components/lanes/LanesPage.tsx (1)
786-837: ⚡ Quick win
onDeleteEventre-subscribes on every lane selection — use refs for high-churn captured values
selectedLaneId(changes on every tab click) andlanesById(changes on everyrefreshLanes) are in the dependency array, so the IPC listener is torn down and re-added on each of those events.managedLaneIdsandmanageOpenadd further churn. In practice this means a live deletion's progress events could be processed by a stale listener instance, and in aggregate these rebuilds are wasteful.The fix is to capture the four high-churn values in refs so the handler always reads the live value without forcing a re-subscription:
♻️ Proposed refactor
+ const selectedLaneIdRef = useRef<string | null>(null); + useEffect(() => { selectedLaneIdRef.current = selectedLaneId; }); + const lanesByIdRef = useRef(lanesById); + useEffect(() => { lanesByIdRef.current = lanesById; }); + const managedLaneIdsRef = useRef(managedLaneIds); + useEffect(() => { managedLaneIdsRef.current = managedLaneIds; }); + const manageOpenRef = useRef(manageOpen); + useEffect(() => { manageOpenRef.current = manageOpen; }); useEffect(() => { const unsubscribe = window.ade.lanes.onDeleteEvent((event) => { const { laneId, overallStatus } = event.progress; setDeleteProgressByLaneId((prev) => { ... }); if (overallStatus === "failed" || overallStatus === "cancelled") { - const laneName = lanesById.get(laneId)?.name ?? laneId; + const laneName = lanesByIdRef.current.get(laneId)?.name ?? laneId; ... } ... - if (selectedLaneId === laneId) selectLane(null); + if (selectedLaneIdRef.current === laneId) selectLane(null); ... void refreshLanes() .then(() => { - if (manageOpen && (selectedLaneId === laneId || managedLaneIds.includes(laneId))) { + if (manageOpenRef.current && (selectedLaneIdRef.current === laneId || managedLaneIdsRef.current.includes(laneId))) { setManageOpen(false); } }) }); return unsubscribe; - }, [clearLaneInspectorTab, lanesById, managedLaneIds, manageOpen, refreshLanes, selectLane, selectedLaneId]); + }, [clearLaneInspectorTab, refreshLanes, selectLane]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/components/lanes/LanesPage.tsx` around lines 786 - 837, The useEffect subscribing to window.ade.lanes.onDeleteEvent re-subscribes on every change to high-churn values (selectedLaneId, lanesById, managedLaneIds, manageOpen); capture those live values in refs instead so the handler reads current state without forcing re-subscription. Create refs (e.g. selectedLaneIdRef, lanesByIdRef, managedLaneIdsRef, manageOpenRef), update them whenever the corresponding state changes, then read from those refs inside the onDeleteEvent handler; keep the useEffect dependency array minimal (include only stable functions/refs like clearLaneInspectorTab, refreshLanes, selectLane and completedLaneDeleteRefreshesRef) and remove the high-churn state vars from it. Ensure existing calls that used those state variables (e.g. the logic around selectLane(null), setActiveLaneIds, setPinnedLaneIds, setManagedLaneIds, setLaneActionError, setDeleteProgressByLaneId, clearLaneInspectorTab, refreshLanes and the completedLaneDeleteRefreshesRef checks) now reference the ref values where needed inside the handler so behavior stays correct.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/desktop/src/main/services/ios/iosSimulatorService.ts`:
- Around line 2735-2745: The code currently keys project targets using
targetId(["project", relativeProject, scheme]) and omits app-specific
identifiers, causing collisions when a scheme can produce multiple app bundles;
update the target object created in the targets.push call to include the
app-specific discriminator (carry appTarget.id and
appTarget.productName/productName) and modify the ResolvedLaunchTarget shape to
store that productName/id, then update findAppBundle() to prefer the provided
productName/appTarget.id when resolving the .app (instead of assuming
`${scheme}.app`), ensuring project targets are distinguished by both scheme and
appTarget identity.
In `@apps/desktop/src/main/services/lanes/laneService.ts`:
- Around line 3234-3241: The pack_dir_remove step currently swallows
fs.promises.rm errors silently; update the catch to call logger.warn including
the lanePackDir and the caught error (reference pack_dir_remove, runStep,
lanePackDir, resolveAdeLayout, projectRoot, fs.promises.rm) so operators can
diagnose failures, and also return or surface a progress "detail" from the async
function (e.g., return a { detail: string } or otherwise set the step's progress
detail) describing the cleanup failure so the UI shows the state.
In `@apps/ios/ADE/App/ContentView.swift`:
- Around line 145-153: The switch over syncService.connectionHealth.transport
currently collapses .unreachable and .disconnected to ADEColor.textMuted; change
it so .unreachable returns the danger tint (e.g. ADEColor.danger or the app's
error color) while only .disconnected returns ADEColor.textMuted, keeping the
existing behavior for .connected and .connecting; update the case arms in the
switch on health.transport in ContentView.swift (the switch handling
connectionHealth) accordingly so "Connection error" is visually distinct from
"No computer attached."
In `@apps/ios/ADE/Views/Settings/SettingsConnectionHeader.swift`:
- Around line 301-310: The dotColor computed property currently returns a
saturated purple for the .disconnected branch, which makes a disconnected state
appear active; update the .disconnected case inside the dotColor switch (in the
dotColor computed var) to use a neutral/inactive tint (for example
ADEColor.inactive or a desaturated Color like Color.gray or Color with lower
saturation/opacity) so the status dot visually matches the "Not connected" state
shown elsewhere.
---
Outside diff comments:
In `@apps/ios/ADE/Services/SyncService.swift`:
- Around line 6088-6100: The else branch currently resolves the request with the
same reconnect-oriented timeout error even when
syncShouldReconnectAfterRequestTimeout(...) returns false; change the error used
in that branch so it signals a non-reconnect timeout. Specifically, when
disconnectOnTimeout is true but syncShouldReconnectAfterRequestTimeout(...) is
false, replace the reconnecting timeoutError passed to resolve(requestId:
requestId, result: .failure(timeoutError)) with a distinct non-reconnect timeout
error (e.g., a new SyncRequestTimeout case or a different error
instance/message) while keeping the existing calls to
markConnectionLoadStrained() and not calling handleTransportFailure().
---
Nitpick comments:
In `@apps/desktop/src/renderer/components/lanes/LanesPage.test.ts`:
- Around line 136-165: Add a new test to cover the branch where every id in
filteredLaneIds is being deleted so the function falls back to sortedLaneIds; in
the LanesPage.test.ts suite add a case for resolveLaneDeleteStartSelection where
deletingLaneIds contains all filteredLaneIds (e.g.,
["lane-b","lane-c","lane-d"]), set selectedLaneId to a deleting entry, and
assert that nextSelectedLaneId and active/pinned behavior come from the first
non-deleting id in sortedLaneIds (using the same symbols
resolveLaneDeleteStartSelection, filteredLaneIds, sortedLaneIds,
deletingLaneIds, selectedLaneId, activeLaneIds, pinnedLaneIds).
In `@apps/desktop/src/renderer/components/lanes/LanesPage.tsx`:
- Around line 786-837: The useEffect subscribing to
window.ade.lanes.onDeleteEvent re-subscribes on every change to high-churn
values (selectedLaneId, lanesById, managedLaneIds, manageOpen); capture those
live values in refs instead so the handler reads current state without forcing
re-subscription. Create refs (e.g. selectedLaneIdRef, lanesByIdRef,
managedLaneIdsRef, manageOpenRef), update them whenever the corresponding state
changes, then read from those refs inside the onDeleteEvent handler; keep the
useEffect dependency array minimal (include only stable functions/refs like
clearLaneInspectorTab, refreshLanes, selectLane and
completedLaneDeleteRefreshesRef) and remove the high-churn state vars from it.
Ensure existing calls that used those state variables (e.g. the logic around
selectLane(null), setActiveLaneIds, setPinnedLaneIds, setManagedLaneIds,
setLaneActionError, setDeleteProgressByLaneId, clearLaneInspectorTab,
refreshLanes and the completedLaneDeleteRefreshesRef checks) now reference the
ref values where needed inside the handler so behavior stays correct.
In `@apps/ios/ADE/Views/Components/ADEDesignSystem.swift`:
- Around line 453-521: Extract the duplicated health-to-UI logic into one small
helper (e.g., a private struct or view-model like ConnectionHealthPresentation
or makeConnectionPresentation()) that reads syncService and SyncConnectionHealth
and exposes the three properties used throughout the UI: tint (Color),
showsConnectedGlow (Bool) and accessibilityLabel (String); move the existing
computed logic from the private vars tint, showsConnectedGlow, truncatedHostName
and accessibilityLabel into that helper (keeping truncatedHostName as an
internal helper) and replace both copies in the file with a single
instantiation/usage of that helper (refer to the existing symbols health,
syncService, truncatedHostName, and the switch on health.transport to preserve
the exact branching and text).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4d8f500b-4611-49e8-a616-6cdd72755920
⛔ Files ignored due to path filters (5)
docs/ARCHITECTURE.mdis excluded by!docs/**docs/features/ios-simulator/README.mdis excluded by!docs/**docs/features/lanes/README.mdis excluded by!docs/**docs/features/onboarding-and-settings/README.mdis excluded by!docs/**docs/features/sync-and-multi-device/ios-companion.mdis excluded by!docs/**
📒 Files selected for processing (23)
apps/ade-cli/README.mdapps/ade-cli/src/cli.tsapps/desktop/src/main/services/ios/iosSimulatorService.test.tsapps/desktop/src/main/services/ios/iosSimulatorService.tsapps/desktop/src/main/services/ipc/registerIpc.tsapps/desktop/src/main/services/lanes/laneService.tsapps/desktop/src/main/services/updates/autoUpdateService.test.tsapps/desktop/src/main/services/updates/autoUpdateService.tsapps/desktop/src/preload/global.d.tsapps/desktop/src/preload/preload.tsapps/desktop/src/renderer/browserMock.tsapps/desktop/src/renderer/components/app/AutoUpdateControl.tsxapps/desktop/src/renderer/components/chat/ChatIosSimulatorPanel.tsxapps/desktop/src/renderer/components/lanes/LanesPage.test.tsapps/desktop/src/renderer/components/lanes/LanesPage.tsxapps/desktop/src/shared/adeCliGuidance.tsapps/desktop/src/shared/types/core.tsapps/ios/ADE/App/ContentView.swiftapps/ios/ADE/Services/SyncService.swiftapps/ios/ADE/Views/Components/ADEDesignSystem.swiftapps/ios/ADE/Views/Settings/SettingsConnectionHeader.swiftapps/ios/ADE/Views/Settings/SettingsSupportTypes.swiftapps/ios/ADETests/ADETests.swift
![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: 5079548e37
ℹ️ 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".
Comment on lines
+567
to
+570
| async quitAndInstall(): Promise<boolean> { | ||
| if (snapshot.status !== "ready" || !snapshot.version) return false; | ||
| const refreshSucceeded = await refreshReadyUpdateBeforeInstall(); | ||
| if (!refreshSucceeded) return false; |
There was a problem hiding this comment.
quitAndInstall() now awaits refreshReadyUpdateBeforeInstall(), but there is no service-level in-flight guard before that await. If two IPC callers invoke install while status is still ready, both calls can pass the initial check, both await the same refresh, and both proceed to call updater.quitAndInstall(false, true). This race is new in the async flow and can trigger duplicate install attempts or inconsistent updater state when multiple renderer actions (or automation callers) fire close together.
Useful? React with 👍 / 👎.
- iosSim: key project targets by scheme + appTarget identity (incl productName) - lanes: log + surface pack_dir_remove cleanup failures - autoUpdate: serialize quitAndInstall via module-level guard (matches checkPromise) - iOS ContentView/SettingsConnectionHeader: distinguish .unreachable (danger) from .disconnected (muted); fix disconnected dot color - iOS SyncService: use non-reconnect timeout message when reconnect path is skipped - iOS ADEDesignSystem: extract ConnectionHealthPresentation, dedupe across button + toolbar - LanesPage: add all-deleting fallback test; capture high-churn vars via refs to stop onDeleteEvent re-subscription churn Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Owner
Author
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Describe the change.
What Changed
Key files and behaviors.
Validation
How you tested.
Risks
Anything to watch.
Summary by CodeRabbit
New Features
Improvements
Documentation
Greptile Summary
This PR improves mobile connection health visibility by introducing a structured
SyncConnectionHealthmodel on iOS, adds live delete-progress tracking for lane deletions with pre-navigation UX, upgrades the iOS Simulator target discovery to use xcscheme matching, and hardens the auto-update install flow with a concurrent-install guard and pre-install version refresh.SyncConnectionHealth/SyncTransportHealth/SyncLoadHealthtypes and a puresyncConnectionHealth()function. AConnectionHealthPresentationstruct consolidates previously duplicated switch/computed-var blocks acrossADEConnectionDotandADERootToolbarControls, and surfaces strained-load and syncing states in accessibility labels.LanesPagenow shows per-lane progress overlays (deleteProgressByLaneId) and navigates away from deleting lanes before the async delete call.laneServicewraps database cleanup in an explicitBEGIN IMMEDIATEtransaction and converts syncrmSynccalls to asyncfs.promises.rm.quitAndInstallPromiseto collapse concurrent install calls,compareUpdateVersionsto ignore superseded downloads, andrefreshReadyUpdateBeforeInstallto pull the latest build before quitting.Confidence Score: 5/5
Safe to merge — all changed paths are defensive, the concurrent-install race from the previous review is properly closed, and the iOS health model refactor is logically sound.
The three main change areas (iOS connection health, lane delete UX, auto-update hardening) are well-scoped and guarded. The concurrent-install race is closed via quitAndInstallPromise. The iOS SyncConnectionHealth refactor consolidates logic without changing observable behaviour. The single note raised (onError swallowing during readyRefreshInProgress) is a minor observability concern, not a correctness failure.
No files require special attention; the auto-update service is the most complex changed path and warrants a staging sanity-check of the readyRefreshInProgress / onError interaction.
Important Files Changed
Sequence Diagram
sequenceDiagram participant UI as AutoUpdateControl (Renderer) participant IPC as IPC / quitAndInstall participant Svc as autoUpdateService participant Upd as electron-updater UI->>IPC: updateQuitAndInstall() IPC->>Svc: quitAndInstall() Note over Svc: quitAndInstallPromise guard Svc->>Svc: refreshReadyUpdateBeforeInstall() Svc->>Upd: checkForUpdates() + await downloadPromise Upd-->>Svc: update-available / update-downloaded / error Note over Svc: ignoredDownloadVersion filters older downloads Svc-->>Svc: readyRefreshError check alt refresh OK Svc->>Svc: patchSnapshot(installing) Svc->>Upd: quitAndInstall(false, true) Upd-->>UI: app restarts else refresh failed Svc->>Svc: patchSnapshot(error) Svc-->>IPC: return false IPC-->>UI: setInstallRequested(false) endComments Outside Diff (3)
apps/desktop/src/main/services/updates/autoUpdateService.ts, line 521-558 (link)refreshReadyUpdateBeforeInstallloses the original ready update on cancelled downloadIf
checkForUpdatesemits a newerupdate-availablebut the subsequent download is cancelled (e.g. network drops),onUpdateCancelledsets status to"idle"andrefreshReadyUpdateBeforeInstallreturnsfalse, aborting the install. The user's original ready update is also lost becauseonUpdateCancelledcleans the cache. Worth considering whether cancellation during a pre-install refresh should revert to the previous ready state rather than aborting entirely.Prompt To Fix With AI
apps/desktop/src/renderer/components/lanes/LanesPage.tsx, line 2258-2296 (link)When
window.ade.lanes.delete(args)throws, the lane's entry is removed fromdeleteProgressByLaneIdso the tab reverts to a normal appearance. ButmoveAwayFromDeletingLanesalready navigated away and changed selection. The user ends up on a different lane while the failed lane's tab reappears with no obvious visual cue that it was the source of the error shown in the toolbar chip. Consider leaving the progress entry in the map (perhaps with a"failed"overallStatus) so the tab can render an error state, or scrolling the tab list to reveal the failed lane.Prompt To Fix With AI
apps/ios/ADE/Services/SyncService.swift, line 2800-2805 (link)load == .strainedreflects user preference, not actual host latencysyncConnectionHealthsetsload = .strainedwheneverprefersReducedSyncLoad == trueand the transport is connected — butprefersReducedSyncLoadappears to be a user-facing preference, not a server-side latency signal. TheADEConnectionDotandADERootToolbarControlsthen surface.strainedas "Host is responding slowly" in accessibility labels, which would be incorrect when the host is fast and the user simply enabled reduced-load mode. Consider using a separate signal derived from request timeouts or server-side metrics to drive the.strainedpath.Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (3): Last reviewed commit: "ship: iter 1 — address coderabbit + code..." | Re-trigger Greptile