Fix Mobile App Crashes#648
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
@copilot review but do not make fixes |
📝 WalkthroughWalkthroughDatabase-change notifications now dispatch on the main queue. SyncService adds ordered endpoint-attempt generation, published running chat session counts, and tailnet probe result classification. Tests cover the new connection ordering and notification behavior. ChangesiOS sync and notification behavior
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
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)
9568-9573: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBreak the
NWConnectionhandler retain cycle on completion.
connectionretainsstateUpdateHandler, the handler retainscomplete, andcompletecapturesconnection. Clear the handler when completing so periodic probes do not retain finished connections.Proposed fix
let complete: (ProbeResult) -> Void = { result in guard !completed else { return } completed = true + connection.stateUpdateHandler = nil connection.cancel() continuation.resume(returning: result) }As per path instructions,
apps/ios/**/*.swift: “iOS Swift app — check for memory management, Swift conventions, and proper SwiftUI patterns.”🤖 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 9568 - 9573, The completion path in the probe helper still leaves an NWConnection retain cycle because the stateUpdateHandler keeps the complete closure alive while complete captures connection. Update the completion logic around complete so it clears the connection’s stateUpdateHandler before or while calling connection.cancel(), then resumes the continuation as it does now. Use the complete closure and the NWConnection setup in SyncService to locate the fix.Source: Path instructions
🤖 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/ios/ADE/Services/SyncService.swift`:
- Around line 9426-9428: The Work tab badge count is being computed from the
unfiltered sessions list instead of the active running roster, which can include
archived or ended chats. Update the runningChatSessionCount logic in SyncService
so it reuses the same filtered set used for active chat handling above,
including archivedAt exclusion, terminal runtime/status checks, lowercase
matching, and recency rules. Keep the count derived from that filtered
collection and use the existing session filtering path around
runningChatSessionCount to avoid badge inflation.
---
Outside diff comments:
In `@apps/ios/ADE/Services/SyncService.swift`:
- Around line 9568-9573: The completion path in the probe helper still leaves an
NWConnection retain cycle because the stateUpdateHandler keeps the complete
closure alive while complete captures connection. Update the completion logic
around complete so it clears the connection’s stateUpdateHandler before or while
calling connection.cancel(), then resumes the continuation as it does now. Use
the complete closure and the NWConnection setup in SyncService to locate the
fix.
🪄 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: 7071e9bc-ad81-449d-83ec-af65c03837bb
📒 Files selected for processing (3)
apps/ios/ADE/Services/Database.swiftapps/ios/ADE/Services/SyncService.swiftapps/ios/ADETests/ADETests.swift
|
@codex review |
![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: a469258be1
ℹ️ 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".
|
|
||
| activeSessions = allAgents | ||
| awaitingInputSessionsCount = awaitingInputCount | ||
| runningChatSessionCount = runningAgents.count |
There was a problem hiding this comment.
When a chat is still runtimeState == "running" but its cached activity timestamp is more than two minutes old, runningAgents intentionally excludes it for the lock-screen/live roster; assigning the Work tab badge count from that same list makes ContentView's Work badge disappear on the next refresh even though the active chat turn is still running. Preserve the cached property, but count active running chat sessions separately from the recency-gated roster.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
Summary
Describe the change.
What Changed
Key files and behaviors.
Validation
How you tested.
Risks
Anything to watch.
Summary by CodeRabbit
Bug Fixes
Tests
Greptile Summary
This PR improves mobile stability around database notifications, connection retries, and Work tab session counts. The main changes are:
Confidence Score: 5/5
The changes are narrowly scoped to mobile database notifications, connection retry ordering, tailnet probing, and session count refresh behavior.
No blocking correctness or security concerns were identified, and the touched areas include targeted regression coverage for the main behavioral changes.
What T-Rex did
Comments Outside Diff (1)
apps/ios/ADE/Services/SyncService.swift, line 6816-6826 (link)The reconnect path races addresses using only the first port, then reuses that reordered address list for every fallback port. If address A is unreachable on the primary port but reachable on a fallback port, while address B wins the primary-port probe but fails the actual reconnect, the code tries B's fallback before A's known-good fallback. This can delay reconnects or select the wrong route compared with trying each address's fallback ports before moving to the next address.
Prompt To Fix With AI
Reviews (3): Last reviewed commit: "ship: preserve running chat badge count" | Re-trigger Greptile